Re: [PATCH v4 07/29] KVM: arm64: Save/restore POE registers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 16, 2024 at 03:55:11PM +0100, Marc Zyngier wrote:
> On Fri, 03 May 2024 14:01:25 +0100,
> Joey Gouly <joey.gouly@xxxxxxx> wrote:
> > 
> > Define the new system registers that POE introduces and context switch them.
> > 
> > Signed-off-by: Joey Gouly <joey.gouly@xxxxxxx>
> > Cc: Marc Zyngier <maz@xxxxxxxxxx>
> > Cc: Oliver Upton <oliver.upton@xxxxxxxxx>
> > Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> > Cc: Will Deacon <will@xxxxxxxxxx>
> > ---
> >  arch/arm64/include/asm/kvm_host.h          |  4 +++
> >  arch/arm64/include/asm/vncr_mapping.h      |  1 +
> >  arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 29 ++++++++++++++++++++++
> >  arch/arm64/kvm/sys_regs.c                  |  8 ++++--
> >  4 files changed, 40 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 9e8a496fb284..28042da0befd 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -419,6 +419,8 @@ enum vcpu_sysreg {
> >  	GCR_EL1,	/* Tag Control Register */
> >  	TFSRE0_EL1,	/* Tag Fault Status Register (EL0) */
> >  
> > +	POR_EL0,	/* Permission Overlay Register 0 (EL0) */
> > +
> >  	/* 32bit specific registers. */
> >  	DACR32_EL2,	/* Domain Access Control Register */
> >  	IFSR32_EL2,	/* Instruction Fault Status Register */
> > @@ -489,6 +491,8 @@ enum vcpu_sysreg {
> >  	VNCR(PIR_EL1),	 /* Permission Indirection Register 1 (EL1) */
> >  	VNCR(PIRE0_EL1), /*  Permission Indirection Register 0 (EL1) */
> >  
> > +	VNCR(POR_EL1),	/* Permission Overlay Register 1 (EL1) */
> > +
> >  	VNCR(HFGRTR_EL2),
> >  	VNCR(HFGWTR_EL2),
> >  	VNCR(HFGITR_EL2),
> > diff --git a/arch/arm64/include/asm/vncr_mapping.h b/arch/arm64/include/asm/vncr_mapping.h
> > index df2c47c55972..06f8ec0906a6 100644
> > --- a/arch/arm64/include/asm/vncr_mapping.h
> > +++ b/arch/arm64/include/asm/vncr_mapping.h
> > @@ -52,6 +52,7 @@
> >  #define VNCR_PIRE0_EL1		0x290
> >  #define VNCR_PIRE0_EL2		0x298
> >  #define VNCR_PIR_EL1		0x2A0
> > +#define VNCR_POR_EL1		0x2A8
> >  #define VNCR_ICH_LR0_EL2        0x400
> >  #define VNCR_ICH_LR1_EL2        0x408
> >  #define VNCR_ICH_LR2_EL2        0x410
> > diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> > index 4be6a7fa0070..1c9536557bae 100644
> > --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> > +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> > @@ -16,9 +16,15 @@
> >  #include <asm/kvm_hyp.h>
> >  #include <asm/kvm_mmu.h>
> >  
> > +static inline bool ctxt_has_s1poe(struct kvm_cpu_context *ctxt);
> > +
> >  static inline void __sysreg_save_common_state(struct kvm_cpu_context *ctxt)
> >  {
> >  	ctxt_sys_reg(ctxt, MDSCR_EL1)	= read_sysreg(mdscr_el1);
> > +
> > +	// POR_EL0 can affect uaccess, so must be saved/restored early.
> > +	if (ctxt_has_s1poe(ctxt))
> > +		ctxt_sys_reg(ctxt, POR_EL0)	= read_sysreg_s(SYS_POR_EL0);
> >  }
> >  
> >  static inline void __sysreg_save_user_state(struct kvm_cpu_context *ctxt)
> > @@ -55,6 +61,17 @@ static inline bool ctxt_has_s1pie(struct kvm_cpu_context *ctxt)
> >  	return kvm_has_feat(kern_hyp_va(vcpu->kvm), ID_AA64MMFR3_EL1, S1PIE, IMP);
> >  }
> >  
> > +static inline bool ctxt_has_s1poe(struct kvm_cpu_context *ctxt)
> > +{
> > +	struct kvm_vcpu *vcpu;
> > +
> > +	if (!system_supports_poe())
> > +		return false;
> > +
> > +	vcpu = ctxt_to_vcpu(ctxt);
> > +	return kvm_has_feat(kern_hyp_va(vcpu->kvm), ID_AA64MMFR3_EL1, S1POE, IMP);
> > +}
> > +
> >  static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
> >  {
> >  	ctxt_sys_reg(ctxt, SCTLR_EL1)	= read_sysreg_el1(SYS_SCTLR);
> > @@ -77,6 +94,10 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
> >  		ctxt_sys_reg(ctxt, PIR_EL1)	= read_sysreg_el1(SYS_PIR);
> >  		ctxt_sys_reg(ctxt, PIRE0_EL1)	= read_sysreg_el1(SYS_PIRE0);
> >  	}
> > +
> > +	if (ctxt_has_s1poe(ctxt))
> > +		ctxt_sys_reg(ctxt, POR_EL1)	= read_sysreg_el1(SYS_POR);
> > +
> >  	ctxt_sys_reg(ctxt, PAR_EL1)	= read_sysreg_par();
> >  	ctxt_sys_reg(ctxt, TPIDR_EL1)	= read_sysreg(tpidr_el1);
> >  
> > @@ -107,6 +128,10 @@ static inline void __sysreg_save_el2_return_state(struct kvm_cpu_context *ctxt)
> >  static inline void __sysreg_restore_common_state(struct kvm_cpu_context *ctxt)
> >  {
> >  	write_sysreg(ctxt_sys_reg(ctxt, MDSCR_EL1),  mdscr_el1);
> > +
> > +	// POR_EL0 can affect uaccess, so must be saved/restored early.
> > +	if (ctxt_has_s1poe(ctxt))
> > +		write_sysreg_s(ctxt_sys_reg(ctxt, POR_EL0),	SYS_POR_EL0);
> >  }
> >  
> >  static inline void __sysreg_restore_user_state(struct kvm_cpu_context *ctxt)
> > @@ -153,6 +178,10 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt)
> >  		write_sysreg_el1(ctxt_sys_reg(ctxt, PIR_EL1),	SYS_PIR);
> >  		write_sysreg_el1(ctxt_sys_reg(ctxt, PIRE0_EL1),	SYS_PIRE0);
> >  	}
> > +
> > +	if (ctxt_has_s1poe(ctxt))
> > +		write_sysreg_el1(ctxt_sys_reg(ctxt, POR_EL1),	SYS_POR);
> > +
> >  	write_sysreg(ctxt_sys_reg(ctxt, PAR_EL1),	par_el1);
> >  	write_sysreg(ctxt_sys_reg(ctxt, TPIDR_EL1),	tpidr_el1);
> >  
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index c9f4f387155f..be04fae35afb 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -2423,6 +2423,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> >  	{ SYS_DESC(SYS_MAIR_EL1), access_vm_reg, reset_unknown, MAIR_EL1 },
> >  	{ SYS_DESC(SYS_PIRE0_EL1), NULL, reset_unknown, PIRE0_EL1 },
> >  	{ SYS_DESC(SYS_PIR_EL1), NULL, reset_unknown, PIR_EL1 },
> > +	{ SYS_DESC(SYS_POR_EL1), NULL, reset_unknown, POR_EL1 },
> >  	{ SYS_DESC(SYS_AMAIR_EL1), access_vm_reg, reset_amair_el1, AMAIR_EL1 },
> >  
> >  	{ SYS_DESC(SYS_LORSA_EL1), trap_loregion },
> > @@ -2506,6 +2507,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> >  	  .access = access_pmovs, .reg = PMOVSSET_EL0,
> >  	  .get_user = get_pmreg, .set_user = set_pmreg },
> >  
> > +	{ SYS_DESC(SYS_POR_EL0), NULL, reset_unknown, POR_EL0 },
> >  	{ SYS_DESC(SYS_TPIDR_EL0), NULL, reset_unknown, TPIDR_EL0 },
> >  	{ SYS_DESC(SYS_TPIDRRO_EL0), NULL, reset_unknown, TPIDRRO_EL0 },
> >  	{ SYS_DESC(SYS_TPIDR2_EL0), undef_access },
> > @@ -4057,8 +4059,6 @@ void kvm_init_sysreg(struct kvm_vcpu *vcpu)
> >  	kvm->arch.fgu[HFGxTR_GROUP] = (HFGxTR_EL2_nAMAIR2_EL1		|
> >  				       HFGxTR_EL2_nMAIR2_EL1		|
> >  				       HFGxTR_EL2_nS2POR_EL1		|
> > -				       HFGxTR_EL2_nPOR_EL1		|
> > -				       HFGxTR_EL2_nPOR_EL0		|
> >  				       HFGxTR_EL2_nACCDATA_EL1		|
> >  				       HFGxTR_EL2_nSMPRI_EL1_MASK	|
> >  				       HFGxTR_EL2_nTPIDR2_EL0_MASK);
> > @@ -4093,6 +4093,10 @@ void kvm_init_sysreg(struct kvm_vcpu *vcpu)
> >  		kvm->arch.fgu[HFGxTR_GROUP] |= (HFGxTR_EL2_nPIRE0_EL1 |
> >  						HFGxTR_EL2_nPIR_EL1);
> >  
> > +	if (!kvm_has_feat(kvm, ID_AA64MMFR3_EL1, S1POE, IMP))
> > +		kvm->arch.fgu[HFGxTR_GROUP] |= (HFGxTR_EL2_nPOR_EL1 |
> > +						HFGxTR_EL2_nPOR_EL0);
> > +
> 
> As Broonie pointed out in a separate thread, this cannot work, short
> of making ID_AA64MMFR3_EL1 writable.
> 
> This can be done in a separate patch, but it needs doing as it
> otherwise breaks migration.
> 
> Thanks,
> 
> 	M.
> 

Looks like it's wrong for PIE currently too, but your patch here fixes that:
	https://lore.kernel.org/kvmarm/20240813144738.2048302-11-maz@xxxxxxxxxx/

If I basically apply that patch, but only for POE, the conflict can be resolved
later, or a rebase will fix it up, depending on what goes through first.

Thanks,
Joey




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux