| Message ID | 20171129174635.3462-1-julien.grall@linaro.org |
|---|---|
| State | Accepted |
| Commit | ac2d8d402370f6f93f82871f3b34ddb9a9ccae05 |
| Headers | show |
| Series | [Xen-devel] xen/arm: Surround HSR_SYSREG macro value with () | expand |
Hi, Ping? Cheers, On 29/11/17 17:46, Julien Grall wrote: > The value of the macro HCR_SYSREG is not surrounded by (). This means > the behavior may change depend on how it is used. > > Thanksfully recent GCC will issue a warning for that. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > > --- > > I am not aware of any "bad" usage today in Xen. This was found whilst > playing with sysreg emulation and GCC complaining about the missing (). > --- > xen/include/asm-arm/arm64/sysregs.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h > index 084d2a1e5d..1811234249 100644 > --- a/xen/include/asm-arm/arm64/sysregs.h > +++ b/xen/include/asm-arm/arm64/sysregs.h > @@ -32,11 +32,11 @@ > > /* These are used to decode traps with HSR.EC==HSR_EC_SYSREG */ > #define HSR_SYSREG(op0,op1,crn,crm,op2) \ > - ((__HSR_SYSREG_##op0) << HSR_SYSREG_OP0_SHIFT) | \ > - ((__HSR_SYSREG_##op1) << HSR_SYSREG_OP1_SHIFT) | \ > - ((__HSR_SYSREG_##crn) << HSR_SYSREG_CRN_SHIFT) | \ > - ((__HSR_SYSREG_##crm) << HSR_SYSREG_CRM_SHIFT) | \ > - ((__HSR_SYSREG_##op2) << HSR_SYSREG_OP2_SHIFT) > + (((__HSR_SYSREG_##op0) << HSR_SYSREG_OP0_SHIFT) | \ > + ((__HSR_SYSREG_##op1) << HSR_SYSREG_OP1_SHIFT) | \ > + ((__HSR_SYSREG_##crn) << HSR_SYSREG_CRN_SHIFT) | \ > + ((__HSR_SYSREG_##crm) << HSR_SYSREG_CRM_SHIFT) | \ > + ((__HSR_SYSREG_##op2) << HSR_SYSREG_OP2_SHIFT)) > > #define HSR_SYSREG_DCISW HSR_SYSREG(1,0,c7,c6,2) > #define HSR_SYSREG_DCCSW HSR_SYSREG(1,0,c7,c10,2) >
On Fri, 8 Dec 2017, Julien Grall wrote: > Hi, > > Ping? > > Cheers, > > On 29/11/17 17:46, Julien Grall wrote: > > The value of the macro HCR_SYSREG is not surrounded by (). This means > > the behavior may change depend on how it is used. > > > > Thanksfully recent GCC will issue a warning for that. > > > > Signed-off-by: Julien Grall <julien.grall@linaro.org> reviewed and committed > > --- > > > > I am not aware of any "bad" usage today in Xen. This was found whilst > > playing with sysreg emulation and GCC complaining about the missing (). > > --- > > xen/include/asm-arm/arm64/sysregs.h | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/xen/include/asm-arm/arm64/sysregs.h > > b/xen/include/asm-arm/arm64/sysregs.h > > index 084d2a1e5d..1811234249 100644 > > --- a/xen/include/asm-arm/arm64/sysregs.h > > +++ b/xen/include/asm-arm/arm64/sysregs.h > > @@ -32,11 +32,11 @@ > > /* These are used to decode traps with HSR.EC==HSR_EC_SYSREG */ > > #define HSR_SYSREG(op0,op1,crn,crm,op2) \ > > - ((__HSR_SYSREG_##op0) << HSR_SYSREG_OP0_SHIFT) | \ > > - ((__HSR_SYSREG_##op1) << HSR_SYSREG_OP1_SHIFT) | \ > > - ((__HSR_SYSREG_##crn) << HSR_SYSREG_CRN_SHIFT) | \ > > - ((__HSR_SYSREG_##crm) << HSR_SYSREG_CRM_SHIFT) | \ > > - ((__HSR_SYSREG_##op2) << HSR_SYSREG_OP2_SHIFT) > > + (((__HSR_SYSREG_##op0) << HSR_SYSREG_OP0_SHIFT) | \ > > + ((__HSR_SYSREG_##op1) << HSR_SYSREG_OP1_SHIFT) | \ > > + ((__HSR_SYSREG_##crn) << HSR_SYSREG_CRN_SHIFT) | \ > > + ((__HSR_SYSREG_##crm) << HSR_SYSREG_CRM_SHIFT) | \ > > + ((__HSR_SYSREG_##op2) << HSR_SYSREG_OP2_SHIFT)) > > #define HSR_SYSREG_DCISW HSR_SYSREG(1,0,c7,c6,2) > > #define HSR_SYSREG_DCCSW HSR_SYSREG(1,0,c7,c10,2) > > > > -- > Julien Grall >
diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h index 084d2a1e5d..1811234249 100644 --- a/xen/include/asm-arm/arm64/sysregs.h +++ b/xen/include/asm-arm/arm64/sysregs.h @@ -32,11 +32,11 @@ /* These are used to decode traps with HSR.EC==HSR_EC_SYSREG */ #define HSR_SYSREG(op0,op1,crn,crm,op2) \ - ((__HSR_SYSREG_##op0) << HSR_SYSREG_OP0_SHIFT) | \ - ((__HSR_SYSREG_##op1) << HSR_SYSREG_OP1_SHIFT) | \ - ((__HSR_SYSREG_##crn) << HSR_SYSREG_CRN_SHIFT) | \ - ((__HSR_SYSREG_##crm) << HSR_SYSREG_CRM_SHIFT) | \ - ((__HSR_SYSREG_##op2) << HSR_SYSREG_OP2_SHIFT) + (((__HSR_SYSREG_##op0) << HSR_SYSREG_OP0_SHIFT) | \ + ((__HSR_SYSREG_##op1) << HSR_SYSREG_OP1_SHIFT) | \ + ((__HSR_SYSREG_##crn) << HSR_SYSREG_CRN_SHIFT) | \ + ((__HSR_SYSREG_##crm) << HSR_SYSREG_CRM_SHIFT) | \ + ((__HSR_SYSREG_##op2) << HSR_SYSREG_OP2_SHIFT)) #define HSR_SYSREG_DCISW HSR_SYSREG(1,0,c7,c6,2) #define HSR_SYSREG_DCCSW HSR_SYSREG(1,0,c7,c10,2)
The value of the macro HCR_SYSREG is not surrounded by (). This means the behavior may change depend on how it is used. Thanksfully recent GCC will issue a warning for that. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- I am not aware of any "bad" usage today in Xen. This was found whilst playing with sysreg emulation and GCC complaining about the missing (). --- xen/include/asm-arm/arm64/sysregs.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)