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

Re: [PATCH] xen/arm: don't read aarch32 regs when aarch32 isn't available



On Tue, 12 Jan 2021 at 19:09, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
>
> On Tue, 12 Jan 2021, Julien Grall wrote:
> > > +    aarch32 = c->pfr64.el1 == 2;
> >
> > This is checking that AArch32 is available in EL1. However, it may not be 
> > the
> > case yet it would be available in EL0.
> >
> > As a consequence, 32-bit userspace wouldn't work properly after this patch.
> >
> > The Arm Arm mandates that if AArch32 is available at EL(n), then it must be
> > available at EL(n - 1).
> >
> > So we should check that AArch32 is available at EL0 because this would
> > cover the case where AArch32 is enabled at EL1.
>
> OK
>
>
> > Furthermore, I would also like to avoid hardcoding value in the code as it 
> > is
> > less readable. We already define cpu_has_el0_32 which use the boot CPU
> > feature. Maybe we want to expand the macro or split it?
>
> I agree
>
> Technically, cpu_has_el0_32 works as is, because it is called after
> boot_cpu_data has been updated. So we could just use it. What do you
> think?

I thought about that when I wrote the first e-mail. However, this
would not work properly for heterogeneous platforms.
There is still a risk to read garbage (or UNDEF) if the boot CPU
supports AArch32 but the others.

Cheers,



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.