On Thu, Apr 30, 2020 at 01:45:51PM +0100, Marc Zyngier wrote:
> On 2020-04-30 13:31, Will Deacon wrote:
> > On Thu, Apr 30, 2020 at 11:59:05AM +0100, Marc Zyngier wrote:
> > > On 2020-04-30 11:25, Will Deacon wrote:
> > > > On Thu, Apr 30, 2020 at 11:15:13AM +0100, Marc Zyngier wrote:
> > > > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > > > > index 23ebe51410f0..2a159af82429 100644
> > > > > --- a/arch/arm64/kvm/guest.c
> > > > > +++ b/arch/arm64/kvm/guest.c
> > > > > @@ -200,6 +200,10 @@ static int set_core_reg(struct kvm_vcpu *vcpu,
> > > > > const struct kvm_one_reg *reg)
> > > > > }
> > > > >
> > > > > memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id));
> > > > > +
> > > > > + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK)
> > > > > + *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu));
> > > >
> > > > It seems slightly odd to me that we don't enforce this for *all* the
> > > > registers when running as a 32-bit guest. Couldn't userspace be equally
> > > > confused by a 64-bit lr or sp?
> > >
> > > Fair point. How about this on top, which wipes the upper 32 bits for
> > > each and every register in the current mode:
> > >
> > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > > index 2a159af82429..f958c3c7bf65 100644
> > > --- a/arch/arm64/kvm/guest.c
> > > +++ b/arch/arm64/kvm/guest.c
> > > @@ -201,9 +201,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu,
> > > const
> > > struct kvm_one_reg *reg)
> > >
> > > memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id));
> > >
> > > - if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK)
> > > - *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu));
> > > + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) {
> > > + int i;
> > >
> > > + for (i = 0; i < 16; i++)
> > > + *vcpu_reg32(vcpu, i) = (u32)*vcpu_reg32(vcpu, i);
> >
> > I think you're missing all the funny banked registers that live all the
> > way
> > up to x30 iirc.
>
> No, they are all indirected via vcpu_reg32(), which has the magic tables.
> And the whole point is that we only want to affect the current mode (no
> point
> in repainting the FIQ registers if the PSR says USR).
>
> Or am I missing something obvious?
Nope, just my inability to parse vcpu_reg32 the first time around! So, for
the updated patch:
Acked-by: Will Deacon <will@xxxxxxxxxx?
Thanks,
Will
