Re: [PATCH 3/3] xen/public: arch-arm: All PSR_* defines should be unsigned
Hi Jan, On 18/08/2023 09:14, Jan Beulich wrote: On 18.08.2023 09:39, Julien Grall wrote: On 18/08/2023 07:33, Jan Beulich wrote: As an aside I wonder why they're here: They look like definitions of processor registers, which aren't under our (Xen's) control. I agree they are not under Xen's control. However, they are used by the toolstack and IIRC back then they were not available in any other headers. Note that they are only available by the tools and the hypervisor (see #ifdef above). Yes, I did notice that (or else I would have used stronger wording). I ask in part because the presence of such constants may then be taken as justification to add similar things in new ports. Yet such definitions shouldn't be put here. From my understanding we are using the public headers to provide macros/defines that are used by both the toolstack and the hypervisor. If they are not meant to be exposed to the guest, then they will be protected with "#if defined(__XEN__) || defined(__XEN_TOOLS__)". I think we are in a similar situation here. So it is not clear where they should be put if we need to share them between the hypervisor and the toolstack. On x86 we simply arrange for certain hypervisor headers to be re-usable from the toolstack. See in particular arch/x86/include/asm/x86-*.h. And of course everything under include/xen/lib/x86/, but those are our own definitions, not ones meant to solely express relevant hw spec aspects. Even if they are defined by the HW, we still need them to easily create some hypercalls field. If we are planning to have a stable toolstack ABI (as Juergen suggested), then I would find a bit odd that onewould need to include lib/ (or provide their own header) in order to update the fields. Anyway, I am not planning to do any re-ordering anytime soon for the public. But I would be happy to take part of the discussion if there are a general agreement to split further and someone wants to write patches. Cheers, -- Julien Grall
Re: [PATCH 3/3] xen/public: arch-arm: All PSR_* defines should be unsigned
Hi Juergen, On 18/08/2023 09:25, Juergen Gross wrote: On 18.08.23 10:05, Julien Grall wrote: Hi, On 18/08/2023 09:00, Juergen Gross wrote: On 18.08.23 09:39, Julien Grall wrote: Hi Jan, On 18/08/2023 07:33, Jan Beulich wrote: On 17.08.2023 23:43, Julien Grall wrote: --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -339,36 +339,36 @@ typedef uint64_t xen_callback_t; /* PSR bits (CPSR, SPSR) */ -#define PSR_THUMB (1<<5) /* Thumb Mode enable */ -#define PSR_FIQ_MASK (1<<6) /* Fast Interrupt mask */ -#define PSR_IRQ_MASK (1<<7) /* Interrupt mask */ -#define PSR_ABT_MASK (1<<8) /* Asynchronous Abort mask */ -#define PSR_BIG_ENDIAN (1<<9) /* arm32: Big Endian Mode */ -#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 */ +#define PSR_THUMB (1U <<5) /* Thumb Mode enable */ +#define PSR_FIQ_MASK (1U <<6) /* Fast Interrupt mask */ +#define PSR_IRQ_MASK (1U <<7) /* Interrupt mask */ +#define PSR_ABT_MASK (1U <<8) /* Asynchronous Abort mask */ Nit: Did you mean to insert blanks also on the rhs of the <<, like you ... +#define PSR_BIG_ENDIAN (1U << 9) /* arm32: Big Endian Mode */ +#define PSR_DBG_MASK (1U << 9) /* arm64: Debug Exception mask */ +#define PSR_IT_MASK (0x0600fc00U) /* Thumb If-Then Mask */ +#define PSR_JAZELLE (1U << 24) /* Jazelle Mode */ +#define PSR_Z (1U << 30) /* Zero condition flag */ ... did everywhere here? Yes I did. I will update the patch. As an aside I wonder why they're here: They look like definitions of processor registers, which aren't under our (Xen's) control. I agree they are not under Xen's control. However, they are used by the toolstack and IIRC back then they were not available in any other headers. Note that they are only available by the tools and the hypervisor (see #ifdef above). I ask in part because the presence of such constants may then be taken as justification to add similar things in new ports. Yet such definitions shouldn't be put here. From my understanding we are using the public headers to provide macros/defines that are used by both the toolstack and the hypervisor. If they are not meant to be exposed to the guest, then they will be protected with "#if defined(__XEN__) || defined(__XEN_TOOLS__)". I think we are in a similar situation here. So it is not clear where they should be put if we need to share them between the hypervisor and the toolstack. What about include/xen/lib? There are headers below that linked at build time via tools/include/Makefile to tools/include/xen/lib. To me, the question is why would we want to move PSR_* in xen/lib (or whatever name we decide) but all the other defines that are only used by the toolstack would still be in public/. So are you suggesting to move all the tools only information in xen/lib? I didn't want to suggest that, especially with our general desire to switch the tools' interfaces to stable ones. Ok. If there are a desire to switch the tools interface to stables one. Then... I just wanted to point out that there are other locations available already where such information could be shared between hypervisor and tools. Especially information related to hardware (so not an interface we are defining) might be a good candidate for such an alternative location. ... I disagree with moving PSR_* outside of the public interface because they are those defines are used in hypercalls. So they would be part of the ABI. Cheers, -- Julien Grall
Re: [PATCH 3/3] xen/public: arch-arm: All PSR_* defines should be unsigned
On 18.08.23 10:05, Julien Grall wrote: Hi, On 18/08/2023 09:00, Juergen Gross wrote: On 18.08.23 09:39, Julien Grall wrote: Hi Jan, On 18/08/2023 07:33, Jan Beulich wrote: On 17.08.2023 23:43, Julien Grall wrote: --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -339,36 +339,36 @@ typedef uint64_t xen_callback_t; /* PSR bits (CPSR, SPSR) */ -#define PSR_THUMB (1<<5) /* Thumb Mode enable */ -#define PSR_FIQ_MASK (1<<6) /* Fast Interrupt mask */ -#define PSR_IRQ_MASK (1<<7) /* Interrupt mask */ -#define PSR_ABT_MASK (1<<8) /* Asynchronous Abort mask */ -#define PSR_BIG_ENDIAN (1<<9) /* arm32: Big Endian Mode */ -#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 */ +#define PSR_THUMB (1U <<5) /* Thumb Mode enable */ +#define PSR_FIQ_MASK (1U <<6) /* Fast Interrupt mask */ +#define PSR_IRQ_MASK (1U <<7) /* Interrupt mask */ +#define PSR_ABT_MASK (1U <<8) /* Asynchronous Abort mask */ Nit: Did you mean to insert blanks also on the rhs of the <<, like you ... +#define PSR_BIG_ENDIAN (1U << 9) /* arm32: Big Endian Mode */ +#define PSR_DBG_MASK (1U << 9) /* arm64: Debug Exception mask */ +#define PSR_IT_MASK (0x0600fc00U) /* Thumb If-Then Mask */ +#define PSR_JAZELLE (1U << 24) /* Jazelle Mode */ +#define PSR_Z (1U << 30) /* Zero condition flag */ ... did everywhere here? Yes I did. I will update the patch. As an aside I wonder why they're here: They look like definitions of processor registers, which aren't under our (Xen's) control. I agree they are not under Xen's control. However, they are used by the toolstack and IIRC back then they were not available in any other headers. Note that they are only available by the tools and the hypervisor (see #ifdef above). I ask in part because the presence of such constants may then be taken as justification to add similar things in new ports. Yet such definitions shouldn't be put here. From my understanding we are using the public headers to provide macros/defines that are used by both the toolstack and the hypervisor. If they are not meant to be exposed to the guest, then they will be protected with "#if defined(__XEN__) || defined(__XEN_TOOLS__)". I think we are in a similar situation here. So it is not clear where they should be put if we need to share them between the hypervisor and the toolstack. What about include/xen/lib? There are headers below that linked at build time via tools/include/Makefile to tools/include/xen/lib. To me, the question is why would we want to move PSR_* in xen/lib (or whatever name we decide) but all the other defines that are only used by the toolstack would still be in public/. So are you suggesting to move all the tools only information in xen/lib? I didn't want to suggest that, especially with our general desire to switch the tools' interfaces to stable ones. I just wanted to point out that there are other locations available already where such information could be shared between hypervisor and tools. Especially information related to hardware (so not an interface we are defining) might be a good candidate for such an alternative location. But in the end it is not me to decide what to use. I was just giving a hint. :-) Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH 3/3] xen/public: arch-arm: All PSR_* defines should be unsigned
On 18.08.2023 09:39, Julien Grall wrote: > On 18/08/2023 07:33, Jan Beulich wrote: >> As an aside I wonder why they're here: They look like definitions of >> processor registers, which aren't under our (Xen's) control. > > I agree they are not under Xen's control. However, they are used by the > toolstack and IIRC back then they were not available in any other headers. > > Note that they are only available by the tools and the hypervisor (see > #ifdef above). Yes, I did notice that (or else I would have used stronger wording). >> I ask in >> part because the presence of such constants may then be taken as >> justification to add similar things in new ports. Yet such definitions >> shouldn't be put here. > > From my understanding we are using the public headers to provide > macros/defines that are used by both the toolstack and the hypervisor. > If they are not meant to be exposed to the guest, then they will be > protected with "#if defined(__XEN__) || defined(__XEN_TOOLS__)". > > I think we are in a similar situation here. So it is not clear where > they should be put if we need to share them between the hypervisor and > the toolstack. On x86 we simply arrange for certain hypervisor headers to be re-usable from the toolstack. See in particular arch/x86/include/asm/x86-*.h. And of course everything under include/xen/lib/x86/, but those are our own definitions, not ones meant to solely express relevant hw spec aspects. Jan
Re: [PATCH 3/3] xen/public: arch-arm: All PSR_* defines should be unsigned
Hi, On 18/08/2023 09:00, Juergen Gross wrote: On 18.08.23 09:39, Julien Grall wrote: Hi Jan, On 18/08/2023 07:33, Jan Beulich wrote: On 17.08.2023 23:43, Julien Grall wrote: --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -339,36 +339,36 @@ typedef uint64_t xen_callback_t; /* PSR bits (CPSR, SPSR) */ -#define PSR_THUMB (1<<5) /* Thumb Mode enable */ -#define PSR_FIQ_MASK (1<<6) /* Fast Interrupt mask */ -#define PSR_IRQ_MASK (1<<7) /* Interrupt mask */ -#define PSR_ABT_MASK (1<<8) /* Asynchronous Abort mask */ -#define PSR_BIG_ENDIAN (1<<9) /* arm32: Big Endian Mode */ -#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 */ +#define PSR_THUMB (1U <<5) /* Thumb Mode enable */ +#define PSR_FIQ_MASK (1U <<6) /* Fast Interrupt mask */ +#define PSR_IRQ_MASK (1U <<7) /* Interrupt mask */ +#define PSR_ABT_MASK (1U <<8) /* Asynchronous Abort mask */ Nit: Did you mean to insert blanks also on the rhs of the <<, like you ... +#define PSR_BIG_ENDIAN (1U << 9) /* arm32: Big Endian Mode */ +#define PSR_DBG_MASK (1U << 9) /* arm64: Debug Exception mask */ +#define PSR_IT_MASK (0x0600fc00U) /* Thumb If-Then Mask */ +#define PSR_JAZELLE (1U << 24) /* Jazelle Mode */ +#define PSR_Z (1U << 30) /* Zero condition flag */ ... did everywhere here? Yes I did. I will update the patch. As an aside I wonder why they're here: They look like definitions of processor registers, which aren't under our (Xen's) control. I agree they are not under Xen's control. However, they are used by the toolstack and IIRC back then they were not available in any other headers. Note that they are only available by the tools and the hypervisor (see #ifdef above). I ask in part because the presence of such constants may then be taken as justification to add similar things in new ports. Yet such definitions shouldn't be put here. From my understanding we are using the public headers to provide macros/defines that are used by both the toolstack and the hypervisor. If they are not meant to be exposed to the guest, then they will be protected with "#if defined(__XEN__) || defined(__XEN_TOOLS__)". I think we are in a similar situation here. So it is not clear where they should be put if we need to share them between the hypervisor and the toolstack. What about include/xen/lib? There are headers below that linked at build time via tools/include/Makefile to tools/include/xen/lib. To me, the question is why would we want to move PSR_* in xen/lib (or whatever name we decide) but all the other defines that are only used by the toolstack would still be in public/. So are you suggesting to move all the tools only information in xen/lib? Cheers, -- Julien Grall
Re: [PATCH 3/3] xen/public: arch-arm: All PSR_* defines should be unsigned
On 18.08.23 09:39, Julien Grall wrote: Hi Jan, On 18/08/2023 07:33, Jan Beulich wrote: On 17.08.2023 23:43, Julien Grall wrote: --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -339,36 +339,36 @@ typedef uint64_t xen_callback_t; /* PSR bits (CPSR, SPSR) */ -#define PSR_THUMB (1<<5) /* Thumb Mode enable */ -#define PSR_FIQ_MASK (1<<6) /* Fast Interrupt mask */ -#define PSR_IRQ_MASK (1<<7) /* Interrupt mask */ -#define PSR_ABT_MASK (1<<8) /* Asynchronous Abort mask */ -#define PSR_BIG_ENDIAN (1<<9) /* arm32: Big Endian Mode */ -#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 */ +#define PSR_THUMB (1U <<5) /* Thumb Mode enable */ +#define PSR_FIQ_MASK (1U <<6) /* Fast Interrupt mask */ +#define PSR_IRQ_MASK (1U <<7) /* Interrupt mask */ +#define PSR_ABT_MASK (1U <<8) /* Asynchronous Abort mask */ Nit: Did you mean to insert blanks also on the rhs of the <<, like you ... +#define PSR_BIG_ENDIAN (1U << 9) /* arm32: Big Endian Mode */ +#define PSR_DBG_MASK (1U << 9) /* arm64: Debug Exception mask */ +#define PSR_IT_MASK (0x0600fc00U) /* Thumb If-Then Mask */ +#define PSR_JAZELLE (1U << 24) /* Jazelle Mode */ +#define PSR_Z (1U << 30) /* Zero condition flag */ ... did everywhere here? Yes I did. I will update the patch. As an aside I wonder why they're here: They look like definitions of processor registers, which aren't under our (Xen's) control. I agree they are not under Xen's control. However, they are used by the toolstack and IIRC back then they were not available in any other headers. Note that they are only available by the tools and the hypervisor (see #ifdef above). I ask in part because the presence of such constants may then be taken as justification to add similar things in new ports. Yet such definitions shouldn't be put here. From my understanding we are using the public headers to provide macros/defines that are used by both the toolstack and the hypervisor. If they are not meant to be exposed to the guest, then they will be protected with "#if defined(__XEN__) || defined(__XEN_TOOLS__)". I think we are in a similar situation here. So it is not clear where they should be put if we need to share them between the hypervisor and the toolstack. What about include/xen/lib? There are headers below that linked at build time via tools/include/Makefile to tools/include/xen/lib. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH 3/3] xen/public: arch-arm: All PSR_* defines should be unsigned
Hi Jan, On 18/08/2023 07:33, Jan Beulich wrote: On 17.08.2023 23:43, Julien Grall wrote: --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -339,36 +339,36 @@ typedef uint64_t xen_callback_t; /* PSR bits (CPSR, SPSR) */ -#define PSR_THUMB (1<<5)/* Thumb Mode enable */ -#define PSR_FIQ_MASK(1<<6)/* Fast Interrupt mask */ -#define PSR_IRQ_MASK(1<<7)/* Interrupt mask */ -#define PSR_ABT_MASK(1<<8)/* Asynchronous Abort mask */ -#define PSR_BIG_ENDIAN (1<<9)/* arm32: Big Endian Mode */ -#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 */ +#define PSR_THUMB (1U <<5) /* Thumb Mode enable */ +#define PSR_FIQ_MASK(1U <<6) /* Fast Interrupt mask */ +#define PSR_IRQ_MASK(1U <<7) /* Interrupt mask */ +#define PSR_ABT_MASK(1U <<8) /* Asynchronous Abort mask */ Nit: Did you mean to insert blanks also on the rhs of the <<, like you ... +#define PSR_BIG_ENDIAN (1U << 9) /* arm32: Big Endian Mode */ +#define PSR_DBG_MASK(1U << 9) /* arm64: Debug Exception mask */ +#define PSR_IT_MASK (0x0600fc00U) /* Thumb If-Then Mask */ +#define PSR_JAZELLE (1U << 24)/* Jazelle Mode */ +#define PSR_Z (1U << 30)/* Zero condition flag */ ... did everywhere here? Yes I did. I will update the patch. As an aside I wonder why they're here: They look like definitions of processor registers, which aren't under our (Xen's) control. I agree they are not under Xen's control. However, they are used by the toolstack and IIRC back then they were not available in any other headers. Note that they are only available by the tools and the hypervisor (see #ifdef above). I ask in part because the presence of such constants may then be taken as justification to add similar things in new ports. Yet such definitions shouldn't be put here. From my understanding we are using the public headers to provide macros/defines that are used by both the toolstack and the hypervisor. If they are not meant to be exposed to the guest, then they will be protected with "#if defined(__XEN__) || defined(__XEN_TOOLS__)". I think we are in a similar situation here. So it is not clear where they should be put if we need to share them between the hypervisor and the toolstack. Cheers, -- Julien Grall
Re: [PATCH 3/3] xen/public: arch-arm: All PSR_* defines should be unsigned
On 17.08.2023 23:43, Julien Grall wrote: > --- a/xen/include/public/arch-arm.h > +++ b/xen/include/public/arch-arm.h > @@ -339,36 +339,36 @@ typedef uint64_t xen_callback_t; > > /* PSR bits (CPSR, SPSR) */ > > -#define PSR_THUMB (1<<5)/* Thumb Mode enable */ > -#define PSR_FIQ_MASK(1<<6)/* Fast Interrupt mask */ > -#define PSR_IRQ_MASK(1<<7)/* Interrupt mask */ > -#define PSR_ABT_MASK(1<<8)/* Asynchronous Abort mask */ > -#define PSR_BIG_ENDIAN (1<<9)/* arm32: Big Endian Mode */ > -#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 */ > +#define PSR_THUMB (1U <<5) /* Thumb Mode enable */ > +#define PSR_FIQ_MASK(1U <<6) /* Fast Interrupt mask */ > +#define PSR_IRQ_MASK(1U <<7) /* Interrupt mask */ > +#define PSR_ABT_MASK(1U <<8) /* Asynchronous Abort mask */ Nit: Did you mean to insert blanks also on the rhs of the <<, like you ... > +#define PSR_BIG_ENDIAN (1U << 9) /* arm32: Big Endian Mode */ > +#define PSR_DBG_MASK(1U << 9) /* arm64: Debug Exception mask */ > +#define PSR_IT_MASK (0x0600fc00U) /* Thumb If-Then Mask */ > +#define PSR_JAZELLE (1U << 24)/* Jazelle Mode */ > +#define PSR_Z (1U << 30)/* Zero condition flag */ ... did everywhere here? As an aside I wonder why they're here: They look like definitions of processor registers, which aren't under our (Xen's) control. I ask in part because the presence of such constants may then be taken as justification to add similar things in new ports. Yet such definitions shouldn't be put here. Jan
Re: [PATCH 3/3] xen/public: arch-arm: All PSR_* defines should be unsigned
Hi Julien, > On Aug 18, 2023, at 05:43, Julien Grall wrote: > > From: Julien Grall > > The defines PSR_* are field in registers and always unsigned. So > add 'U' to clarify. > > This should help with MISRA Rule 7.2. > > Signed-off-by: Julien Grall Reviewed-by: Henry Wang Tested-by: Henry Wang Kind regards, Henry
Re: [PATCH 3/3] xen/public: arch-arm: All PSR_* defines should be unsigned
On Thu, 17 Aug 2023, Julien Grall wrote: > From: Julien Grall > > The defines PSR_* are field in registers and always unsigned. So > add 'U' to clarify. > > This should help with MISRA Rule 7.2. > > Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini > --- > xen/include/public/arch-arm.h | 52 +-- > 1 file changed, 26 insertions(+), 26 deletions(-) > > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > index c6449893e493..492819ad22c9 100644 > --- a/xen/include/public/arch-arm.h > +++ b/xen/include/public/arch-arm.h > @@ -339,36 +339,36 @@ typedef uint64_t xen_callback_t; > > /* PSR bits (CPSR, SPSR) */ > > -#define PSR_THUMB (1<<5)/* Thumb Mode enable */ > -#define PSR_FIQ_MASK(1<<6)/* Fast Interrupt mask */ > -#define PSR_IRQ_MASK(1<<7)/* Interrupt mask */ > -#define PSR_ABT_MASK(1<<8)/* Asynchronous Abort mask */ > -#define PSR_BIG_ENDIAN (1<<9)/* arm32: Big Endian Mode */ > -#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 */ > +#define PSR_THUMB (1U <<5) /* Thumb Mode enable */ > +#define PSR_FIQ_MASK(1U <<6) /* Fast Interrupt mask */ > +#define PSR_IRQ_MASK(1U <<7) /* Interrupt mask */ > +#define PSR_ABT_MASK(1U <<8) /* Asynchronous Abort mask */ > +#define PSR_BIG_ENDIAN (1U << 9) /* arm32: Big Endian Mode */ > +#define PSR_DBG_MASK(1U << 9) /* arm64: Debug Exception mask */ > +#define PSR_IT_MASK (0x0600fc00U) /* Thumb If-Then Mask */ > +#define PSR_JAZELLE (1U << 24)/* Jazelle Mode */ > +#define PSR_Z (1U << 30)/* Zero condition flag */ > > /* 32 bit modes */ > -#define PSR_MODE_USR 0x10 > -#define PSR_MODE_FIQ 0x11 > -#define PSR_MODE_IRQ 0x12 > -#define PSR_MODE_SVC 0x13 > -#define PSR_MODE_MON 0x16 > -#define PSR_MODE_ABT 0x17 > -#define PSR_MODE_HYP 0x1a > -#define PSR_MODE_UND 0x1b > -#define PSR_MODE_SYS 0x1f > +#define PSR_MODE_USR 0x10U > +#define PSR_MODE_FIQ 0x11U > +#define PSR_MODE_IRQ 0x12U > +#define PSR_MODE_SVC 0x13U > +#define PSR_MODE_MON 0x16U > +#define PSR_MODE_ABT 0x17U > +#define PSR_MODE_HYP 0x1aU > +#define PSR_MODE_UND 0x1bU > +#define PSR_MODE_SYS 0x1fU > > /* 64 bit modes */ > -#define PSR_MODE_BIT 0x10 /* Set iff AArch32 */ > -#define PSR_MODE_EL3h 0x0d > -#define PSR_MODE_EL3t 0x0c > -#define PSR_MODE_EL2h 0x09 > -#define PSR_MODE_EL2t 0x08 > -#define PSR_MODE_EL1h 0x05 > -#define PSR_MODE_EL1t 0x04 > -#define PSR_MODE_EL0t 0x00 > +#define PSR_MODE_BIT 0x10U /* Set iff AArch32 */ > +#define PSR_MODE_EL3h 0x0dU > +#define PSR_MODE_EL3t 0x0cU > +#define PSR_MODE_EL2h 0x09U > +#define PSR_MODE_EL2t 0x08U > +#define PSR_MODE_EL1h 0x05U > +#define PSR_MODE_EL1t 0x04U > +#define PSR_MODE_EL0t 0x00U > > /* > * We set PSR_Z to be able to boot Linux kernel versions with an invalid > -- > 2.40.1 >
[PATCH 3/3] xen/public: arch-arm: All PSR_* defines should be unsigned
From: Julien Grall The defines PSR_* are field in registers and always unsigned. So add 'U' to clarify. This should help with MISRA Rule 7.2. Signed-off-by: Julien Grall --- xen/include/public/arch-arm.h | 52 +-- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h index c6449893e493..492819ad22c9 100644 --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -339,36 +339,36 @@ typedef uint64_t xen_callback_t; /* PSR bits (CPSR, SPSR) */ -#define PSR_THUMB (1<<5)/* Thumb Mode enable */ -#define PSR_FIQ_MASK(1<<6)/* Fast Interrupt mask */ -#define PSR_IRQ_MASK(1<<7)/* Interrupt mask */ -#define PSR_ABT_MASK(1<<8)/* Asynchronous Abort mask */ -#define PSR_BIG_ENDIAN (1<<9)/* arm32: Big Endian Mode */ -#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 */ +#define PSR_THUMB (1U <<5) /* Thumb Mode enable */ +#define PSR_FIQ_MASK(1U <<6) /* Fast Interrupt mask */ +#define PSR_IRQ_MASK(1U <<7) /* Interrupt mask */ +#define PSR_ABT_MASK(1U <<8) /* Asynchronous Abort mask */ +#define PSR_BIG_ENDIAN (1U << 9) /* arm32: Big Endian Mode */ +#define PSR_DBG_MASK(1U << 9) /* arm64: Debug Exception mask */ +#define PSR_IT_MASK (0x0600fc00U) /* Thumb If-Then Mask */ +#define PSR_JAZELLE (1U << 24)/* Jazelle Mode */ +#define PSR_Z (1U << 30)/* Zero condition flag */ /* 32 bit modes */ -#define PSR_MODE_USR 0x10 -#define PSR_MODE_FIQ 0x11 -#define PSR_MODE_IRQ 0x12 -#define PSR_MODE_SVC 0x13 -#define PSR_MODE_MON 0x16 -#define PSR_MODE_ABT 0x17 -#define PSR_MODE_HYP 0x1a -#define PSR_MODE_UND 0x1b -#define PSR_MODE_SYS 0x1f +#define PSR_MODE_USR 0x10U +#define PSR_MODE_FIQ 0x11U +#define PSR_MODE_IRQ 0x12U +#define PSR_MODE_SVC 0x13U +#define PSR_MODE_MON 0x16U +#define PSR_MODE_ABT 0x17U +#define PSR_MODE_HYP 0x1aU +#define PSR_MODE_UND 0x1bU +#define PSR_MODE_SYS 0x1fU /* 64 bit modes */ -#define PSR_MODE_BIT 0x10 /* Set iff AArch32 */ -#define PSR_MODE_EL3h 0x0d -#define PSR_MODE_EL3t 0x0c -#define PSR_MODE_EL2h 0x09 -#define PSR_MODE_EL2t 0x08 -#define PSR_MODE_EL1h 0x05 -#define PSR_MODE_EL1t 0x04 -#define PSR_MODE_EL0t 0x00 +#define PSR_MODE_BIT 0x10U /* Set iff AArch32 */ +#define PSR_MODE_EL3h 0x0dU +#define PSR_MODE_EL3t 0x0cU +#define PSR_MODE_EL2h 0x09U +#define PSR_MODE_EL2t 0x08U +#define PSR_MODE_EL1h 0x05U +#define PSR_MODE_EL1t 0x04U +#define PSR_MODE_EL0t 0x00U /* * We set PSR_Z to be able to boot Linux kernel versions with an invalid -- 2.40.1
