[PATCH v2 11/11] KVM: arm64: Get rid of the AArch32 register mapping code

[Marc Zyngier]

Hi Nina,

On Thu, 23 May 2024 15:25:21 +0100,
Nina Schoetterl-Glausch <nsg at linux.ibm.com> wrote:
> 
> On Mon, 2020-11-02 at 16:40 +0000, Marc Zyngier wrote:

Wow, you're digging out the old dregs... But it is worth it!

>
> [...]
> 
> > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > index dfb5218137ca..3f23f7478d2a 100644
> > --- a/arch/arm64/kvm/guest.c
> > +++ b/arch/arm64/kvm/guest.c
> > @@ -252,10 +252,32 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> >  	memcpy(addr, valp, KVM_REG_SIZE(reg->id));
> 
> I was looking at KVM_(G|S)ET_ONE_REG implementations and something looks off to me here:
> 
> ...
> 
> 	if (off == KVM_REG_ARM_CORE_REG(regs.pstate)) {
> 		u64 mode = (*(u64 *)valp) & PSR_AA32_MODE_MASK;
> 		switch (mode) {
> 
> Masking and switch over mode here...
> 
> 		case PSR_AA32_MODE_USR:
> 			if (!kvm_supports_32bit_el0())
> 				return -EINVAL;
> 			break;
> 		case PSR_AA32_MODE_FIQ:
> 		case PSR_AA32_MODE_IRQ:
> ...
> >  
> >  	if (*vcpu_cpsr(vcpu) & PSR_MODE32_BIT) {
> > -		int i;
> > +		int i, nr_reg;
> > +
> > +		switch (*vcpu_cpsr(vcpu)) {
> 
> ...but switching over mode without masking here.
> I don't know if this is as intended, but I thought I'd mention it.

Amazing. Thanks for spotting this. This is indeed broken. I guess this
was not spotted because userspace is not totally broken itself.

Do you want to submit a fix adding the masking back? or should I do it
myself?

Thanks again,

	M.

-- 
Without deviation from the norm, progress is not possible.