On Wed, Sep 13, 2017 at 03:37:42PM +0100, Alex Bennée wrote: > > Dave Martin <Dave.Martin@xxxxxxx> writes: [...] > > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c > > index 945e79c..35a90b8 100644 > > --- a/arch/arm64/kvm/hyp/switch.c > > +++ b/arch/arm64/kvm/hyp/switch.c > > @@ -81,11 +81,17 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu) > > * it will cause an exception. > > */ > > val = vcpu->arch.hcr_el2; > > + > > if (!(val & HCR_RW) && system_supports_fpsimd()) { > > write_sysreg(1 << 30, fpexc32_el2); > > isb(); > > } > > + > > + if (val & HCR_RW) /* for AArch64 only: */ > > + val |= HCR_TID3; /* TID3: trap feature register accesses */ > > + > > I wondered as this is the hyp switch can we make use of testing val & > HCR_RW for both this and above. But it seems minimal in the generated > code so probably not. I figured that the code was cleaner ths way, since they're independent bits of code that both happen to be applicable only to AArch64 guests. [...] > > + > > + /* > > + * ID regs: all ID_SANITISED() entries here must have corresponding > > + * entries in arm64_ftr_regs[]. > > + */ > > arm64_ftr_regs isn't updated in this commit. Does this break bisection? This commit only adds ID_SANITISED() entries for regs that are already present in arm64_ftr_regs[]. (If you spot any that are missing, give me a shout...) SVE only adds one new ID register, ID_AA64ZFR0_EL1 -- but SVE defines no fields in there yet, so I just leave it ID_UNALLOCATED() which will cause it to read as zero for the guest. > > + > > + /* AArch64 mappings of the AArch32 ID registers */ > > + /* CRm=1 */ > > + ID_SANITISED(ID_PFR0_EL1), > > + ID_SANITISED(ID_PFR1_EL1), [...] > > + /* CRm=7 */ > > + ID_SANITISED(ID_AA64MMFR0_EL1), > > + ID_SANITISED(ID_AA64MMFR1_EL1), > > + ID_SANITISED(ID_AA64MMFR2_EL1), > > + ID_UNALLOCATED(7,3), > > + ID_UNALLOCATED(7,4), > > + ID_UNALLOCATED(7,5), > > + ID_UNALLOCATED(7,6), > > + ID_UNALLOCATED(7,7), > > + > > I think it might be worthwhile adding a test to kvm-unit-tests to walk > all the ID registers to check this. Sounds sensible, I'll take a look at that. [...] Cheers ---Dave