1. Patchew
  2. Xen
  3. View series

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

Stefano Stabellini posted 1 patch 2 years, 1 month ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20220325010052.1597420-1-sstabellini@kernel.org
There is a newer version of this series
xen/include/public/arch-arm.h | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
[PATCH] xen/arm: set CPSR Z bit when creating aarch32 guests
Posted by Stefano Stabellini 2 years, 1 month ago 
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)
 
 #define SCTLR_GUEST_INIT    xen_mk_ullong(0x00c50078)
-- 
2.25.1
Re: [PATCH] xen/arm: set CPSR Z bit when creating aarch32 guests
Posted by Julien Grall 2 years, 1 month ago 
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
Re: [PATCH] xen/arm: set CPSR Z bit when creating aarch32 guests
Posted by Bertrand Marquis 2 years, 1 month ago 
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
>
RE: [PATCH] xen/arm: set CPSR Z bit when creating aarch32 guests
Posted by Wei Chen 2 years, 1 month ago 
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
>
Re: [PATCH] xen/arm: set CPSR Z bit when creating aarch32 guests
Posted by Julien Grall 2 years, 1 month ago 
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,

-- 
Julien Grall

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.