Re: [PATCH] xen/arm: set CPSR Z bit when creating aarch32 guests

[Julien Grall <julien@xxxxxxx>]

Hi Wei,

On 25/03/2022 02:51, Wei Chen wrote:
> > -----Original Message-----
> > From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of
> > Stefano Stabellini
> > Sent: 2022年3月25日 9:01
> > To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: julien@xxxxxxx; sstabellini@xxxxxxxxxx; Bertrand Marquis
> > <Bertrand.Marquis@xxxxxxx>; Volodymyr_Babchuk@xxxxxxxx; Stefano Stabellini
> > <stefano.stabellini@xxxxxxxxxx>
> > Subject: [PATCH] xen/arm: set CPSR Z bit when creating aarch32 guests
> >
> > From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> >
> > The first 32 bytes of zImage are NOPs. When CONFIG_EFI is enabled in the
> > kernel, certain versions of Linux will use an UNPREDICATABLE NOP
> > encoding, sometimes resulting in an unbootable kernel. Whether the
> > resulting kernel is bootable or not depends on the processor. See commit
> > a92882a4d270 in the Linux kernel for all the details.
> >
> > All kernel releases starting from Linux 4.9 without commit a92882a4d270
> > are affected.
> >
> > Fortunately there is a simple workaround: setting the "Z" bit in CPSR
> > make it so those invalid NOP instructions are never executed. That is
> > because the instruction is conditional (not equal). So, on QEMU at
> > least, the instruction will end up to be ignored and not generate an
> > exception. Setting the "Z" bit makes those kernel versions bootable
> > again and it is harmless in the other cases.
> >
> > Note that both U-Boot and QEMU -kernel set the "Z" bit in CPSR when
> > booting a zImage kernel on aarch32.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> > ---
> > Changes in v3:
> > - improve commit message
> > - improve in-code comment
> >
> > Changes in v2:
> > - improve commit message
> > - add in-code comment
> > - move PSR_Z to the beginning
> > ---
> >   xen/include/public/arch-arm.h | 11 ++++++++++-
> >   1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> > index 94b31511dd..c0c1149e27 100644
> > --- a/xen/include/public/arch-arm.h
> > +++ b/xen/include/public/arch-arm.h
> > @@ -361,6 +361,7 @@ typedef uint64_t xen_callback_t;
> >   #define PSR_DBG_MASK    (1<<9)        /* arm64: Debug Exception mask */
> >   #define PSR_IT_MASK     (0x0600fc00)  /* Thumb If-Then Mask */
> >   #define PSR_JAZELLE     (1<<24)       /* Jazelle Mode */
> > +#define PSR_Z           (1<<30)       /* Zero condition flag */
> >
> >   /* 32 bit modes */
> >   #define PSR_MODE_USR 0x10
> > @@ -383,7 +384,15 @@ typedef uint64_t xen_callback_t;
> >   #define PSR_MODE_EL1t 0x04
> >   #define PSR_MODE_EL0t 0x00
> >
> > -#define PSR_GUEST32_INIT
> > (PSR_ABT_MASK|PSR_FIQ_MASK|PSR_IRQ_MASK|PSR_MODE_SVC)
> > +/*
> > + * We set PSR_Z to be able to boot Linux kernel versions with an invalid
> > + * encoding of the first 8 NOP instructions. See commit a92882a4d270 in
> > + * Linux.
> > + *
> > + * Note that PSR_Z is also set by U-Boot and QEMU -kernel when loading
> > + * zImage kernels on aarch32.
> > + */
> > +#define PSR_GUEST32_INIT
> > (PSR_Z|PSR_ABT_MASK|PSR_FIQ_MASK|PSR_IRQ_MASK|PSR_MODE_SVC)
> >   #define PSR_GUEST64_INIT
> > (PSR_ABT_MASK|PSR_FIQ_MASK|PSR_IRQ_MASK|PSR_MODE_EL1h)
> Maybe this is a good opportunity to fix the alignment of the two macros : )
I have dropped one space for PSR_GUEST32_INIT and committed.

Cheers,

--
Julien Grall