diff mbox series

xen/arm: set CPSR Z bit when creating aarch32 guests

Message ID 20220325010052.1597420-1-sstabellini@kernel.org (mailing list archive)
State New, archived
Headers show
Series xen/arm: set CPSR Z bit when creating aarch32 guests | expand

Commit Message

Stefano Stabellini March 25, 2022, 1 a.m. UTC
From: Stefano Stabellini <stefano.stabellini@xilinx.com>

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@xilinx.com>
---
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(-)

Comments

Wei Chen March 25, 2022, 2:51 a.m. UTC | #1
Hi Stefano,

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
> Stefano Stabellini
> Sent: 2022年3月25日 9:01
> To: xen-devel@lists.xenproject.org
> Cc: julien@xen.org; sstabellini@kernel.org; Bertrand Marquis
> <Bertrand.Marquis@arm.com>; Volodymyr_Babchuk@epam.com; Stefano Stabellini
> <stefano.stabellini@xilinx.com>
> Subject: [PATCH] xen/arm: set CPSR Z bit when creating aarch32 guests
> 
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> 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@xilinx.com>
> ---
> 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 : )

Reviewed-by: Wei Chen <Wei.Chen@arm.com>

>  #define SCTLR_GUEST_INIT    xen_mk_ullong(0x00c50078)
> --
> 2.25.1
>
Bertrand Marquis March 25, 2022, 9:33 a.m. UTC | #2
Hi Stefano,

> On 25 Mar 2022, at 02:00, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> 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@xilinx.com>

Thanks for the comment and commit message fixes.

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> 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)
> 
> #define SCTLR_GUEST_INIT    xen_mk_ullong(0x00c50078)
> -- 
> 2.25.1
>
Julien Grall March 25, 2022, 9:40 a.m. UTC | #3
Hi Stefano,

On 25/03/2022 01:00, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> 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

typo: s/UNPREDICATABLE/UNPREDICTABLE/

I will fix it on commit.

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,
Julien Grall March 25, 2022, 9:51 a.m. UTC | #4
Hi Wei,

On 25/03/2022 02:51, Wei Chen wrote:
>> -----Original Message-----
>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
>> Stefano Stabellini
>> Sent: 2022年3月25日 9:01
>> To: xen-devel@lists.xenproject.org
>> Cc: julien@xen.org; sstabellini@kernel.org; Bertrand Marquis
>> <Bertrand.Marquis@arm.com>; Volodymyr_Babchuk@epam.com; Stefano Stabellini
>> <stefano.stabellini@xilinx.com>
>> Subject: [PATCH] xen/arm: set CPSR Z bit when creating aarch32 guests
>>
>> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>
>> 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@xilinx.com>
>> ---
>> 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,
diff mbox series

Patch

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)
 
 #define SCTLR_GUEST_INIT    xen_mk_ullong(0x00c50078)