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

[Julien Grall <julien.grall.oss@xxxxxxxxx>]

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,