On Fri, Nov 27, 2020 at 10:26:47AM +0000, Marc Zyngier wrote: > On 2020-11-24 15:50, Will Deacon wrote: > > If a vCPU is caught running 32-bit code on a system with mismatched > > support at EL0, then we should kill it. > > > > Acked-by: Marc Zyngier <maz@xxxxxxxxxx> > > Signed-off-by: Will Deacon <will@xxxxxxxxxx> > > --- > > arch/arm64/kvm/arm.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > > index 5750ec34960e..d322ac0f4a8e 100644 > > --- a/arch/arm64/kvm/arm.c > > +++ b/arch/arm64/kvm/arm.c > > @@ -633,6 +633,15 @@ static void check_vcpu_requests(struct kvm_vcpu > > *vcpu) > > } > > } > > > > +static bool vcpu_mode_is_bad_32bit(struct kvm_vcpu *vcpu) > > +{ > > + if (likely(!vcpu_mode_is_32bit(vcpu))) > > + return false; > > + > > + return !system_supports_32bit_el0() || > > + static_branch_unlikely(&arm64_mismatched_32bit_el0); > > +} > > + > > /** > > * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute > > guest code > > * @vcpu: The VCPU pointer > > @@ -816,7 +825,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) > > * with the asymmetric AArch32 case), return to userspace with > > * a fatal error. > > */ > > - if (!system_supports_32bit_el0() && vcpu_mode_is_32bit(vcpu)) { > > + if (vcpu_mode_is_bad_32bit(vcpu)) { > > /* > > * As we have caught the guest red-handed, decide that > > * it isn't fit for purpose anymore by making the vcpu > > Given the new definition of system_supports_32bit_el0() in the previous > patch, > why do we need this patch at all? I think the check is still needed, as this is an unusual case where we want to reject the mismatched system. For example, imagine 'arm64_mismatched_32bit_el0' is true and we're on a mismatched system: in this case system_supports_32bit_el0() will return 'true' because we allow 32-bit applications to run, we support the 32-bit personality etc. However, we still want to terminate 32-bit vCPUs if we spot them in this situation, so we have to check for: !system_supports_32bit_el0() || static_branch_unlikely(&arm64_mismatched_32bit_el0) so that we only allow 32-bit vCPUs when all of the physical CPUs support it at EL0. I could make this clearer either by adding a comment, or avoiding system_supports_32bit_el0() entirely here and just checking the sanitised SYS_ID_AA64PFR0_EL1 register directly instead. What do you prefer? Will