On 2024-02-29 17:34, Jan Beulich wrote:
On 29.02.2024 16:27, Nicola Vetrini wrote:
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -462,8 +462,8 @@ static bool has_ssbd_mitigation(const struct
arm_cpu_capabilities *entry)
#define MIDR_RANGE(model, min, max) \
.matches = is_affected_midr_range, \
.midr_model = model, \
- .midr_range_min = min, \
- .midr_range_max = max
+ .midr_range_min = (min), \
+ .midr_range_max = (max)
Why min and max, but not model?
All the constants in the full expansions are parenthesized via
MIDR_CPU_MODEL, so it doesn't trigger any violation right now, but for
consistency I'd better put parentheses there as well.
--- a/xen/arch/arm/include/asm/smccc.h
+++ b/xen/arch/arm/include/asm/smccc.h
@@ -122,7 +122,7 @@ struct arm_smccc_res {
#define __constraint_read_7 __constraint_read_6, "r" (r7)
#define __declare_arg_0(a0, res) \
- struct arm_smccc_res *___res = res; \
+ struct arm_smccc_res *___res = (res); \
register unsigned long r0 ASM_REG(0) = (uint32_t)a0; \
Why res but not a0?
Seems like it's never used in a non-compliant way, but you do have a
point. Here and also below, to keep it consistent. I didn't look at all
the violations yet, so I may have missed some. I did want to show a few
patches also to gather opinions on what may/may not be accepted.
--- a/xen/arch/arm/include/asm/vgic-emul.h
+++ b/xen/arch/arm/include/asm/vgic-emul.h
@@ -6,11 +6,11 @@
* a range of registers
*/
-#define VREG32(reg) reg ... reg + 3
-#define VREG64(reg) reg ... reg + 7
+#define VREG32(reg) (reg) ... (reg) + 3
+#define VREG64(reg) (reg) ... (reg) + 7
#define VREG32(reg) (reg) ... ((reg) + 3)
#define VREG64(reg) (reg) ... ((reg) + 7)
?
The outer parentheses are not required, but I can add them if the
maintainers share your view.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
