[PATCH 00/14] address half the remaining Misra rule 11.8 violations

[PATCH 00/14] address half the remaining Misra rule 11.8 violations

[Jan Beulich]

Casting away const-ness (or volatile-ness) is generally bad practices,
so this is perhaps one of the more important rules to get into clean
state. Despite being bad practice in general, there are cases where
doing so is pretty much unavoidable; in some cases even the library
spec mandates doing so (without Misra having any provisions for that).
How to deal with most of those cases is still up for discussion, only
very few of them are actually dealt with here.

As per [1] this addresses about half the remaining violations in all
four analyze jobs we have in gitlab-CI (patch 06 wasn't included
there, yet).

01: lib: obey to Misra rule 11.8 where possible
02: x86/bitops: don't cast away volatile-ness
03: x86: have cmpxchg16b() obey to Misra rule 11.8
04: x86/boot: don't cast away const-ness
05: x86/altcall: hide casting away of const
06: Arm/alternative: hide casting away of const
07: Arm64/GICv3: have gicv3_its_find_quirk() not cast away const-ness
08: Arm/guestcopy: deviate copy_guest() uses just like their x86 counterparts
09: ACPI: address a Misra rule 11.8 violation in Arm code
10: ELF/notes: use pointer-to-const by default in ELFNOTE_...()
11: crypto/vmac: don't cast away const-ness in aes_key_setup()
12: gnttab: don't cast away constness
13: passthrough/PCI: rework (s,b,d,f) init of struct pci_dev
14: xhci-dbc: don't cast away const-ness in xhci_find_dbc()

Jan

[1] https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/2810122986

[PATCH 01/14] lib: obey to Misra rule 11.8 where possible

[Jan Beulich]

Casting away const-ness (or volatile-ness) is never a good idea, but
some library functions (e.g. strchr()) require doing so. Where not
required, remove / replace respective casts.

While there convert touched functions to Xen style.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/lib/memcpy.c
+++ b/xen/lib/memcpy.c
@@ -15,12 +15,13 @@
  */
 void *(memcpy)(void *dest, const void *src, size_t n)
 {
-	char *tmp = (char *) dest, *s = (char *) src;
+    char *tmp = dest;
+    const char *s = src;
 
-	while (n--)
-		*tmp++ = *s++;
+    while ( n-- )
+        *tmp++ = *s++;
 
-	return dest;
+    return dest;
 }
 
 /*
--- a/xen/lib/memmove.c
+++ b/xen/lib/memmove.c
@@ -14,21 +14,25 @@
  */
 void *(memmove)(void *dest, const void *src, size_t n)
 {
-	char *tmp, *s;
+    char *tmp;
+    const char *s;
 
-	if (dest <= src) {
-		tmp = (char *) dest;
-		s = (char *) src;
-		while (n--)
-			*tmp++ = *s++;
-	} else {
-		tmp = (char *) dest + n;
-		s = (char *) src + n;
-		while (n--)
-			*--tmp = *--s;
-	}
+    if ( dest <= src )
+    {
+        tmp = dest;
+        s = src;
+        while ( n-- )
+            *tmp++ = *s++;
+    }
+    else
+    {
+        tmp = dest + n;
+        s = src + n;
+        while ( n-- )
+            *--tmp = *--s;
+    }
 
-	return dest;
+    return dest;
 }
 
 /*
--- a/xen/lib/strcmp.c
+++ b/xen/lib/strcmp.c
@@ -11,16 +11,15 @@
  */
 int (strcmp)(const char *cs, const char *ct)
 {
-	unsigned char *csu = (unsigned char *)cs;
-	unsigned char *ctu = (unsigned char *)ct;
-	int res;
+    const unsigned char *csu = (const void *)cs;
+    const unsigned char *ctu = (const void *)ct;
+    int res;
 
-	while (1) {
-		if ((res = *csu - *ctu++) != 0 || !*csu++)
-			break;
-	}
+    for ( ; ; )
+        if ( (res = *csu - *ctu++) != 0 || !*csu++ )
+            break;
 
-	return res;
+    return res;
 }
 
 /*
--- a/xen/lib/strncmp.c
+++ b/xen/lib/strncmp.c
@@ -12,17 +12,18 @@
  */
 int (strncmp)(const char *cs, const char *ct, size_t count)
 {
-	unsigned char *csu = (unsigned char *)cs;
-	unsigned char *ctu = (unsigned char *)ct;
-	int res = 0;
+    const unsigned char *csu = (const void *)cs;
+    const unsigned char *ctu = (const void *)ct;
+    int res = 0;
 
-	while (count) {
-		if ((res = *csu - *ctu++) != 0 || !*csu++)
-			break;
-		count--;
-	}
+    while ( count )
+    {
+        if ( (res = *csu - *ctu++) != 0 || !*csu++ )
+            break;
+        count--;
+    }
 
-	return res;
+    return res;
 }
 
 /*

[PATCH 02/14] x86/bitops: don't cast away volatile-ness

[Jan Beulich]

Doing so, besides being a bad idea in general, violates Misra rule 11.8.
Use the helper macro we have available anyway.

No functional change intended.

Fixes: f1879b2c2584 ("xen: introduce generic non-atomic test_*bit()")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/include/asm/bitops.h
+++ b/xen/arch/x86/include/asm/bitops.h
@@ -188,7 +188,7 @@ static inline int arch__test_and_set_bit
     asm volatile ( "btsl %[nr], %[addr]\n\t"
                    ASM_FLAG_OUT(, "sbbl %[old], %[old]\n\t")
                    : [old] ASM_FLAG_OUT("=@ccc", "=r") (oldbit),
-                     [addr] "+m" (*(int *)addr) : [nr] "Ir" (nr) : "memory" );
+                     [addr] "+m" (ADDR) : [nr] "Ir" (nr) : "memory" );
 
     return oldbit;
 }
@@ -234,7 +234,7 @@ static inline int arch__test_and_clear_b
     asm volatile ( "btrl %[nr], %[addr]\n\t"
                    ASM_FLAG_OUT(, "sbbl %[old], %[old]\n\t")
                    : [old] ASM_FLAG_OUT("=@ccc", "=r") (oldbit),
-                     [addr] "+m" (*(int *)addr) : [nr] "Ir" (nr) : "memory" );
+                     [addr] "+m" (ADDR) : [nr] "Ir" (nr) : "memory" );
 
     return oldbit;
 }
@@ -248,7 +248,7 @@ static inline int arch__test_and_change_
     asm volatile ( "btcl %[nr], %[addr]\n\t"
                    ASM_FLAG_OUT(, "sbbl %[old], %[old]\n\t")
                    : [old] ASM_FLAG_OUT("=@ccc", "=r") (oldbit),
-                     [addr] "+m" (*(int *)addr) : [nr] "Ir" (nr) : "memory" );
+                     [addr] "+m" (ADDR) : [nr] "Ir" (nr) : "memory" );
 
     return oldbit;
 }

[PATCH 03/14] x86: have cmpxchg16b() obey to Misra rule 11.8

[Jan Beulich]

Casting away const-ness (or volatile-ness) is never a good idea, and
there's no need to in cmpxchg16b().

No functional change intended.

Fixes: e0116acbbd0b ("x86: add cmpxchg16b support")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/include/asm/x86_64/system.h
+++ b/xen/arch/x86/include/asm/x86_64/system.h
@@ -56,7 +56,7 @@ static always_inline __uint128_t cmpxchg
     ASSERT(!((unsigned long)_p & 0xf));                    \
     BUILD_BUG_ON(sizeof(*(o)) != sizeof(__uint128_t));     \
     BUILD_BUG_ON(sizeof(*(n)) != sizeof(__uint128_t));     \
-    __cmpxchg16b(_p, (void *)(o), (void *)(n));            \
+    __cmpxchg16b(_p, (const void *)(o), (const void *)(n));\
 })
 
 #endif /* __X86_64_SYSTEM_H__ */

[PATCH 04/14] x86/boot: don't cast away const-ness

[Jan Beulich]

While the general C library strchr() has to do so, the early boot cmdline
parsing has no such requirement.

No functional change.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/boot/cmdline.c
+++ b/xen/arch/x86/boot/cmdline.c
@@ -77,12 +77,12 @@ static int strncmp(const char *cs, const
     return 0;
 }
 
-static char *strchr(const char *s, int c)
+static const char *strchr(const char *s, int c)
 {
     for ( ; *s != (char)c; ++s )
         if ( *s == '\0' )
             return NULL;
-    return (char *)s;
+    return s;
 }
 
 static size_t strspn(const char *s, const char *accept)

[PATCH 05/14] x86/altcall: hide casting away of const

[Jan Beulich]

ALT_CALL_PTR() really means to get rid of const (when necessary, i.e. when
used from apply_alt_calls()): We want to patch the original call site,
after all, and memory-wise that's unrelated to the struct alt_call * that
we start from.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/include/asm/alternative-call.h
+++ b/xen/arch/x86/include/asm/alternative-call.h
@@ -9,7 +9,7 @@
 struct alt_call {
     int32_t offset;
 };
-#define ALT_CALL_PTR(a) ((void *)&(a)->offset + (a)->offset)
+#define ALT_CALL_PTR(a) ((void *)((long)&(a)->offset + (a)->offset))
 #define ALT_CALL_LEN(a) (6)
 
 /*

[PATCH 06/14] Arm/alternative: hide casting away of const

[Jan Beulich]

We want to patch the original call site, which memory-wise is unrelated
to the struct alt_instr * that we start from.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/arm/alternative.c
+++ b/xen/arch/arm/alternative.c
@@ -134,7 +134,7 @@ static int __apply_alternatives(const st
             BUG_ON(alt->repl_len != alt->orig_len);
 
         origptr = ALT_ORIG_PTR(alt);
-        updptr = (void *)origptr + update_offset;
+        updptr = (void *)(long)origptr + update_offset;
 
         nr_inst = alt->orig_len / ARCH_PATCH_INSN_SIZE;
 

[PATCH 07/14] Arm64/GICv3: have gicv3_its_find_quirk() not cast away const-ness

[Jan Beulich]

Doing so violates Misra rule 11.8, while being entirely unnecessary here:
All callers already store the return value in pointer-to-const variables.

No functional change.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -84,14 +84,14 @@ static const struct its_quirk its_quirks
     }
 };
 
-static struct its_quirk* gicv3_its_find_quirk(uint32_t iidr)
+static const struct its_quirk *gicv3_its_find_quirk(uint32_t iidr)
 {
     const struct its_quirk *quirks = its_quirks;
 
     for ( ; quirks->desc; quirks++ )
     {
         if ( quirks->iidr == (quirks->mask & iidr) )
-            return (struct its_quirk *)quirks;
+            return quirks;
     }
 
     return NULL;

[PATCH 08/14] Arm/guestcopy: deviate copy_guest() uses just like their x86 counterparts

[Jan Beulich]

Like x86/HVM's __hvm_copy(), Arm's copy_guest() is used for both to-guest
and from-guest copying. Naturally in the latter case the hypervisor buffer
needs writing to, hence the function parameter cannot be pointer-to-const.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Of course for both the pre-existing x86 deviation and the new Arm one it
might be more robust if the deviation was also limited to the respective
source file. Can this be expressed together with the needed regex?

--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -433,6 +433,12 @@ Fixing this violation would require to i
 -config=MC3A2.R11.8,reports+={safe,"any_area(any_loc(any_exp(macro(^container_of$))))"}
 -doc_end
 
+-doc_begin="Function copy_guest() in xen/arch/arm/guestcopy.c is a double-use
+function, where the parameter needs to not be const because it can be set for
+write or not"
+-config=MC3A2.R11.8,reports+={safe,"any_area(any_loc(text(^.*copy_guest.*COPY_to_guest doesn't modify.*$)))"}
+-doc_end
+
 -doc_begin="Function __hvm_copy in xen/arch/x86/hvm/hvm.c is a double-use
 function, where the parameter needs to not be const because it can be set for
 write or not"
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -109,14 +109,16 @@ static unsigned long copy_guest(void *bu
 
 unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int len)
 {
-    return copy_guest((void *)from, (vaddr_t)to, len,
-                      GVA_INFO(current), COPY_to_guest | COPY_linear);
+    return copy_guest((void *)from, /* COPY_to_guest doesn't modify */
+                      (vaddr_t)to, len, GVA_INFO(current),
+                      COPY_to_guest | COPY_linear);
 }
 
 unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
                                              unsigned int len)
 {
-    return copy_guest((void *)from, (vaddr_t)to, len, GVA_INFO(current),
+    return copy_guest((void *)from, /* COPY_to_guest doesn't modify */
+                      (vaddr_t)to, len, GVA_INFO(current),
                       COPY_to_guest | COPY_flush_dcache | COPY_linear);
 }
 

[PATCH 09/14] ACPI: address a Misra rule 11.8 violation in Arm code

[Jan Beulich]

Outside of drivers/acpi/tables/ (which is excluded from Eclair reporting
for dubious reasons), arch/arm/acpi/domain_build.c is the only user of
ACPI_COMPARE_NAME(). Make the macro const correct, at the expense of
introducing a "const" variant of ACPI_CAST_PTR(), and at the expense of
altering an imported file.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/include/acpi/acmacros.h
+++ b/xen/include/acpi/acmacros.h
@@ -103,6 +103,7 @@
  * Pointer manipulation
  */
 #define ACPI_CAST_PTR(t, p)             ((t *) (acpi_uintptr_t) (p))
+#define ACPI_CAST_CPTR(t, p)            ((const t *) (const acpi_uintptr_t) (p))
 #define ACPI_CAST_INDIRECT_PTR(t, p)    ((t **) (acpi_uintptr_t) (p))
 #define ACPI_ADD_PTR(t,a,b)             ACPI_CAST_PTR (t, (ACPI_CAST_PTR (u8,(a)) + (acpi_native_uint)(b)))
 #define ACPI_PTR_DIFF(a,b)              (acpi_native_uint) (ACPI_CAST_PTR (u8,(a)) - ACPI_CAST_PTR (u8,(b)))
@@ -116,9 +117,12 @@
 #define ACPI_PTR_TO_PHYSADDR(i)         ACPI_TO_INTEGER(i)
 
 #ifndef ACPI_MISALIGNMENT_NOT_SUPPORTED
-#define ACPI_COMPARE_NAME(a,b)          (*ACPI_CAST_PTR (u32,(a)) == *ACPI_CAST_PTR (u32,(b)))
+#define ACPI_COMPARE_NAME(a, b)         (*ACPI_CAST_CPTR (u32, a) == \
+                                         *ACPI_CAST_CPTR (u32, b))
 #else
-#define ACPI_COMPARE_NAME(a,b)          (!ACPI_STRNCMP (ACPI_CAST_PTR (char,(a)), ACPI_CAST_PTR (char,(b)), ACPI_NAME_SIZE))
+#define ACPI_COMPARE_NAME(a, b)         (!ACPI_STRNCMP (ACPI_CAST_CPTR (char, a), \
+                                                        ACPI_CAST_CPTR (char, b), \
+                                                        ACPI_NAME_SIZE))
 #endif
 
 /*

[PATCH 10/14] ELF/notes: use pointer-to-const by default in ELFNOTE_...()

[Jan Beulich]

Not doing so results in a number of Misra rule 11.8 (casting away of
const-ness) violations. We need to allow kexec to use pointer to non-
const though, so provide a means to override the default.

For ELFNOTE_NEXT() we can do better and simply re-apply the type of the
incoming pointer.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -6,6 +6,9 @@
  * - Magnus Damm <magnus@valinux.co.jp>
  */
 
+/* We're producing ELF notes here. */
+#define ELFNOTE_CONST
+
 #include <xen/acpi.h>
 #include <xen/console.h>
 #include <xen/cpu.h>
--- a/xen/include/xen/elf.h
+++ b/xen/include/xen/elf.h
@@ -29,9 +29,14 @@
 
 #include <xen/elfstructs.h>
 
+#ifndef ELFNOTE_CONST
+#define ELFNOTE_CONST const
+#endif
+
 #define ELFNOTE_ALIGN(_n_) (((_n_)+3)&~3)
-#define ELFNOTE_NAME(_n_) ((char*)(_n_) + sizeof(*(_n_)))
+#define ELFNOTE_NAME(_n_) ((ELFNOTE_CONST char *)(_n_) + sizeof(*(_n_)))
 #define ELFNOTE_DESC(_n_) (ELFNOTE_NAME(_n_) + ELFNOTE_ALIGN((_n_)->namesz))
-#define ELFNOTE_NEXT(_n_) ((Elf_Note *)(ELFNOTE_DESC(_n_) + ELFNOTE_ALIGN((_n_)->descsz)))
+#define ELFNOTE_NEXT(_n_) ((typeof(_n_))(ELFNOTE_DESC(_n_) + \
+                                         ELFNOTE_ALIGN((_n_)->descsz)))
 
 #endif /* __XEN_ELF_H__ */

[PATCH 11/14] crypto/vmac: don't cast away const-ness in aes_key_setup()

[Jan Beulich]

vmac_set_key() passes in a pointer-to-const, which has const-ness removed
despite rijndaelKeySetupEnc() properly taking pointer-to-const.

No functional change.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I was wondering whether I shouldn't adjust the adjacent aes_encryption()
right away as well.

--- a/xen/include/crypto/vmac.h
+++ b/xen/include/crypto/vmac.h
@@ -69,7 +69,7 @@ typedef u32 aes_int_key[4*(VMAC_KEY_LEN/
 	    				    (u8 *)(in), (u8 *)(out))
 #define aes_key_setup(user_key,int_key)                 \
 	    	rijndaelKeySetupEnc((u32 *)(int_key),       \
-	    	                    (u8 *)(user_key), \
+	    	                    (const u8 *)(user_key), \
 	    	                    VMAC_KEY_LEN)
 #endif
 

[PATCH 12/14] gnttab: don't cast away constness

[Jan Beulich]

While _set_status_v2() indeed doesn't alter the grant_entry_header_t it
is handed a pointer to, _set_status_v1() does. Drop the const from the
parameter of the latter (and then necessarily also from _set_status()'s),
while adding const to the local variable of the former.

No functional change.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Really I think it would be best if we did away with the raw_shah local
variables (which looks reasonably simple for at least _set_status_v2()).
I'm unconvinced that we really need to use ACCESS_ONCE() here. Torn reads
aren't a problem; what we require is that we look at a stable local copy,
and that can be achieved by putting barrier() after the reads.

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -740,7 +740,7 @@ static unsigned int nr_grant_entries(str
     return 0;
 }
 
-static int _set_status_v1(const grant_entry_header_t *shah,
+static int _set_status_v1(grant_entry_header_t *shah,
                           struct domain *rd,
                           struct active_grant_entry *act,
                           int readonly,
@@ -832,7 +832,7 @@ static int _set_status_v2(const grant_en
                           domid_t  ldomid)
 {
     int      rc    = GNTST_okay;
-    uint32_t *raw_shah = (uint32_t *)shah;
+    const uint32_t *raw_shah = (const uint32_t *)shah;
     union grant_combo scombo;
     uint16_t mask  = GTF_type_mask;
 
@@ -909,7 +909,7 @@ done:
 }
 
 
-static int _set_status(const grant_entry_header_t *shah,
+static int _set_status(grant_entry_header_t *shah,
                        grant_status_t *status,
                        struct domain *rd,
                        unsigned int rgt_version,

[PATCH 13/14] passthrough/PCI: rework (s,b,d,f) init of struct pci_dev

[Jan Beulich]

Right now we're casting away const-ness, to initialize the individual
elements despite the field(s) being declared const. Eclair validly
recognizes this as a Misra rule 11.8 violation. Hide this by switching to
the use of memcpy(), deriving the destination address from the (mutable)
struct pci_dev * which we hold in hands.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Subsequently we may want to further leverage the sbdf local variable we
now have in the function. Yet of course the primary question is: Is this
an okay game to play in the first place?

--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -318,6 +318,7 @@ static void apply_quirks(struct pci_dev
 static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
 {
     struct pci_dev *pdev;
+    pci_sbdf_t sbdf = { .seg = pseg->nr, .bus = bus, .devfn = devfn };
     unsigned int pos;
     int rc;
 
@@ -329,9 +330,15 @@ static struct pci_dev *alloc_pdev(struct
     if ( !pdev )
         return NULL;
 
-    *(u16*) &pdev->seg = pseg->nr;
-    *((u8*) &pdev->bus) = bus;
-    *((u8*) &pdev->devfn) = devfn;
+    /*
+     * pdev->sbdf is deliberately const, i.e. it can't be written by structure
+     * or field assignment.  Writing by memcpy() works, as long as it's not
+     * &pdev->sbdf which is passed.  Since memcpy() isn't type-safe, have an
+     * explicit type check first.
+     */
+    (void)(&pdev->sbdf != &sbdf);
+    memcpy((void *)pdev + offsetof(struct pci_dev, sbdf), &sbdf, sizeof(sbdf));
+
     pdev->domain = NULL;
 
     INIT_LIST_HEAD(&pdev->vf_list);
@@ -390,7 +397,6 @@ static struct pci_dev *alloc_pdev(struct
                          phantom_devs[i].slot == PCI_SLOT(devfn) &&
                          phantom_devs[i].stride > PCI_FUNC(devfn) )
                     {
-                        pci_sbdf_t sbdf = pdev->sbdf;
                         unsigned int stride = phantom_devs[i].stride;
 
                         while ( (sbdf.fn += stride) > PCI_FUNC(devfn) )

[PATCH 14/14] xhci-dbc: don't cast away const-ness in xhci_find_dbc()

[Jan Beulich]

Doing so (at the return statement), besides being a bad idea anyway, is a
violation of Misra rule 11.8.

Make "xcap" have an initializer while adjusting the code anyway.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/char/xhci-dbc.c
+++ b/xen/drivers/char/xhci-dbc.c
@@ -384,16 +384,15 @@ static bool __init dbc_init_xhc(struct d
  */
 static struct dbc_reg __iomem *xhci_find_dbc(struct dbc *dbc)
 {
-    const uint32_t __iomem *xcap;
     uint32_t xcap_val;
     uint32_t next;
     uint32_t id = 0;
-    const void __iomem *mmio = dbc->xhc_mmio;
+    void __iomem *mmio = dbc->xhc_mmio;
     const uint32_t __iomem *hccp1 = mmio + 0x10;
+    uint32_t __iomem *xcap = mmio;
     const uint32_t DBC_ID = 0xA;
     int ttl = 48;
 
-    xcap = mmio;
     /*
      * This is initially an offset to the first capability. All the offsets
      * (both in HCCP1 and then next capability pointer) are dword-based.

Re: [PATCH 14/14] xhci-dbc: don't cast away const-ness in xhci_find_dbc()

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 1265 bytes --]

On Wed, Sep 02, 2026 at 08:37:26AM +0200, Jan Beulich wrote:
> Doing so (at the return statement), besides being a bad idea anyway, is a
> violation of Misra rule 11.8.
> 
> Make "xcap" have an initializer while adjusting the code anyway.
> 
> No functional change intended.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

> --- a/xen/drivers/char/xhci-dbc.c
> +++ b/xen/drivers/char/xhci-dbc.c
> @@ -384,16 +384,15 @@ static bool __init dbc_init_xhc(struct d
>   */
>  static struct dbc_reg __iomem *xhci_find_dbc(struct dbc *dbc)
>  {
> -    const uint32_t __iomem *xcap;
>      uint32_t xcap_val;
>      uint32_t next;
>      uint32_t id = 0;
> -    const void __iomem *mmio = dbc->xhc_mmio;
> +    void __iomem *mmio = dbc->xhc_mmio;
>      const uint32_t __iomem *hccp1 = mmio + 0x10;
> +    uint32_t __iomem *xcap = mmio;
>      const uint32_t DBC_ID = 0xA;
>      int ttl = 48;
>  
> -    xcap = mmio;
>      /*
>       * This is initially an offset to the first capability. All the offsets
>       * (both in HCCP1 and then next capability pointer) are dword-based.
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 07/14] Arm64/GICv3: have gicv3_its_find_quirk() not cast away const-ness

[Orzel, Michal]

On 02-Sep-26 08:33, Jan Beulich wrote:
> Doing so violates Misra rule 11.8, while being entirely unnecessary here:
> All callers already store the return value in pointer-to-const variables.
> 
> No functional change.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH 02/14] x86/bitops: don't cast away volatile-ness

[Andrew Cooper]

On 02/09/2026 7:30 am, Jan Beulich wrote:
> Doing so, besides being a bad idea in general, violates Misra rule 11.8.
> Use the helper macro we have available anyway.
>
> No functional change intended.
>
> Fixes: f1879b2c2584 ("xen: introduce generic non-atomic test_*bit()")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

I really want to kill ADDR, not expand it's use.

I'll do a series and pull this patch into it.

~Andrew

Re: [PATCH 06/14] Arm/alternative: hide casting away of const

[Orzel, Michal]

On 02-Sep-26 08:32, Jan Beulich wrote:
> We want to patch the original call site, which memory-wise is unrelated
> to the struct alt_instr * that we start from.
> 
> No functional change intended.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/arm/alternative.c
> +++ b/xen/arch/arm/alternative.c
> @@ -134,7 +134,7 @@ static int __apply_alternatives(const st
>              BUG_ON(alt->repl_len != alt->orig_len);
>  
>          origptr = ALT_ORIG_PTR(alt);
> -        updptr = (void *)origptr + update_offset;
> +        updptr = (void *)(long)origptr + update_offset;
NIT: Why long and not unsigned long like other places in this file?

Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH 06/14] Arm/alternative: hide casting away of const

[Jan Beulich]

On 02.09.2026 13:47, Orzel, Michal wrote:
> On 02-Sep-26 08:32, Jan Beulich wrote:
>> We want to patch the original call site, which memory-wise is unrelated
>> to the struct alt_instr * that we start from.
>>
>> No functional change intended.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/arm/alternative.c
>> +++ b/xen/arch/arm/alternative.c
>> @@ -134,7 +134,7 @@ static int __apply_alternatives(const st
>>              BUG_ON(alt->repl_len != alt->orig_len);
>>  
>>          origptr = ALT_ORIG_PTR(alt);
>> -        updptr = (void *)origptr + update_offset;
>> +        updptr = (void *)(long)origptr + update_offset;
> NIT: Why long and not unsigned long like other places in this file?

Hmm, that was inherited from "x86/altcall: hide casting away of const",
yet apparently wrongly so: update_offset is of unsigned type here. I've
switched locally.

> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Thanks.

Jan

Re: [PATCH 13/14] passthrough/PCI: rework (s,b,d,f) init of struct pci_dev

[Roger Pau Monné]

On Wed, Sep 02, 2026 at 08:36:27AM +0200, Jan Beulich wrote:
> Right now we're casting away const-ness, to initialize the individual
> elements despite the field(s) being declared const. Eclair validly
> recognizes this as a Misra rule 11.8 violation. Hide this by switching to
> the use of memcpy(), deriving the destination address from the (mutable)
> struct pci_dev * which we hold in hands.
> 
> No functional change intended.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Subsequently we may want to further leverage the sbdf local variable we
> now have in the function. Yet of course the primary question is: Is this
> an okay game to play in the first place?

I've wondered the same, while this might be obfuscated enough for
Eclair to complain, aren't we still violating the spirit of the rule?

Regards, Roger.

Re: [PATCH 13/14] passthrough/PCI: rework (s,b,d,f) init of struct pci_dev

[Jan Beulich]

On 02.09.2026 14:44, Roger Pau Monné wrote:
> On Wed, Sep 02, 2026 at 08:36:27AM +0200, Jan Beulich wrote:
>> Right now we're casting away const-ness, to initialize the individual
>> elements despite the field(s) being declared const. Eclair validly
>> recognizes this as a Misra rule 11.8 violation. Hide this by switching to
>> the use of memcpy(), deriving the destination address from the (mutable)
>> struct pci_dev * which we hold in hands.
>>
>> No functional change intended.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Subsequently we may want to further leverage the sbdf local variable we
>> now have in the function. Yet of course the primary question is: Is this
>> an okay game to play in the first place?
> 
> I've wondered the same, while this might be obfuscated enough for
> Eclair to complain, aren't we still violating the spirit of the rule?

We do, but what do you do without dropping that "const" (which I'd really
like to keep), and without C++ concepts of initialization?

Jan

Re: [PATCH 13/14] passthrough/PCI: rework (s,b,d,f) init of struct pci_dev

[Roger Pau Monné]

On Wed, Sep 02, 2026 at 02:53:58PM +0200, Jan Beulich wrote:
> On 02.09.2026 14:44, Roger Pau Monné wrote:
> > On Wed, Sep 02, 2026 at 08:36:27AM +0200, Jan Beulich wrote:
> >> Right now we're casting away const-ness, to initialize the individual
> >> elements despite the field(s) being declared const. Eclair validly
> >> recognizes this as a Misra rule 11.8 violation. Hide this by switching to
> >> the use of memcpy(), deriving the destination address from the (mutable)
> >> struct pci_dev * which we hold in hands.
> >>
> >> No functional change intended.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> ---
> >> Subsequently we may want to further leverage the sbdf local variable we
> >> now have in the function. Yet of course the primary question is: Is this
> >> an okay game to play in the first place?
> > 
> > I've wondered the same, while this might be obfuscated enough for
> > Eclair to complain, aren't we still violating the spirit of the rule?
> 
> We do, but what do you do without dropping that "const" (which I'd really
> like to keep), and without C++ concepts of initialization?

Is it possible to "tag" this with a comment?  Noting we are aware of
the MISRA violation, but the result of the field being const outweigh
the violation.

At the end the memcpy() is also a violation, and hence is likely to
also be flagged by Eclair in the future - it might be best to simply
come clean and accept we have an intentional violation here.

Thanks, Roger.

Re: [PATCH 13/14] passthrough/PCI: rework (s,b,d,f) init of struct pci_dev

[Jan Beulich]

On 02.09.2026 17:07, Roger Pau Monné wrote:
> On Wed, Sep 02, 2026 at 02:53:58PM +0200, Jan Beulich wrote:
>> On 02.09.2026 14:44, Roger Pau Monné wrote:
>>> On Wed, Sep 02, 2026 at 08:36:27AM +0200, Jan Beulich wrote:
>>>> Right now we're casting away const-ness, to initialize the individual
>>>> elements despite the field(s) being declared const. Eclair validly
>>>> recognizes this as a Misra rule 11.8 violation. Hide this by switching to
>>>> the use of memcpy(), deriving the destination address from the (mutable)
>>>> struct pci_dev * which we hold in hands.
>>>>
>>>> No functional change intended.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> ---
>>>> Subsequently we may want to further leverage the sbdf local variable we
>>>> now have in the function. Yet of course the primary question is: Is this
>>>> an okay game to play in the first place?
>>>
>>> I've wondered the same, while this might be obfuscated enough for
>>> Eclair to complain, aren't we still violating the spirit of the rule?
>>
>> We do, but what do you do without dropping that "const" (which I'd really
>> like to keep), and without C++ concepts of initialization?
> 
> Is it possible to "tag" this with a comment?  Noting we are aware of
> the MISRA violation, but the result of the field being const outweigh
> the violation.

Surely we could make up a SAF comment for this, just that SAF comments
aren't liked by everyone either (personally I consider them ugly, but
kind of tolerable).

Jan

Re: [PATCH 08/14] Arm/guestcopy: deviate copy_guest() uses just like their x86 counterparts

[Orzel, Michal]

On 02-Sep-26 08:33, Jan Beulich wrote:
> Like x86/HVM's __hvm_copy(), Arm's copy_guest() is used for both to-guest
> and from-guest copying. Naturally in the latter case the hypervisor buffer
> needs writing to, hence the function parameter cannot be pointer-to-const.
> 
> No functional change intended.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH 05/14] x86/altcall: hide casting away of const

[Roger Pau Monné]

On Wed, Sep 02, 2026 at 08:32:09AM +0200, Jan Beulich wrote:
> ALT_CALL_PTR() really means to get rid of const (when necessary, i.e. when
> used from apply_alt_calls()): We want to patch the original call site,
> after all, and memory-wise that's unrelated to the struct alt_call * that
> we start from.
> 
> No functional change intended.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger@xenproject.org>

Thanks, Roger.

Re: [PATCH 04/14] x86/boot: don't cast away const-ness

[Roger Pau Monné]

On Wed, Sep 02, 2026 at 08:31:45AM +0200, Jan Beulich wrote:
> While the general C library strchr() has to do so, the early boot cmdline
> parsing has no such requirement.
> 
> No functional change.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger@xenproject.org>

I won't usually agree to changing the interface of C library
functions, but this is limited enough to early boot cmdline parsing
that's IMO OK.  It might be good to rename the function so it doesn't
alias with the C library namespace now, since it's a different
interface.

Thanks, Roger.

Re: [PATCH 08/14] Arm/guestcopy: deviate copy_guest() uses just like their x86 counterparts

[Nicola Vetrini]

On 2026-09-02 08:33, Jan Beulich wrote:
> Like x86/HVM's __hvm_copy(), Arm's copy_guest() is used for both 
> to-guest
> and from-guest copying. Naturally in the latter case the hypervisor 
> buffer
> needs writing to, hence the function parameter cannot be 
> pointer-to-const.
> 
> No functional change intended.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

See comment below, but I can do as a follow-up if you'd like.

> ---
> Of course for both the pre-existing x86 deviation and the new Arm one 
> it
> might be more robust if the deviation was also limited to the 
> respective
> source file. Can this be expressed together with the needed regex?

Yes, e.g.

-config=MC3A2.R11.8,reports+={safe,"any_area(any_loc(file(^xen/arch/arm/...$)&&text(^.*copy_guest.*COPY_to_guest 
doesn't modify.*$)))"}

not compile-tested, so YMMV.

> 
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -433,6 +433,12 @@ Fixing this violation would require to i
>  
> -config=MC3A2.R11.8,reports+={safe,"any_area(any_loc(any_exp(macro(^container_of$))))"}
>  -doc_end
> 
> +-doc_begin="Function copy_guest() in xen/arch/arm/guestcopy.c is a 
> double-use
> +function, where the parameter needs to not be const because it can be 
> set for
> +write or not"
> +-config=MC3A2.R11.8,reports+={safe,"any_area(any_loc(text(^.*copy_guest.*COPY_to_guest 
> doesn't modify.*$)))"}
> +-doc_end
> +
>  -doc_begin="Function __hvm_copy in xen/arch/x86/hvm/hvm.c is a 
> double-use
>  function, where the parameter needs to not be const because it can be 
> set for
>  write or not"
> --- a/xen/arch/arm/guestcopy.c
> +++ b/xen/arch/arm/guestcopy.c
> @@ -109,14 +109,16 @@ static unsigned long copy_guest(void *bu
> 
>  unsigned long raw_copy_to_guest(void *to, const void *from, unsigned 
> int len)
>  {
> -    return copy_guest((void *)from, (vaddr_t)to, len,
> -                      GVA_INFO(current), COPY_to_guest | COPY_linear);
> +    return copy_guest((void *)from, /* COPY_to_guest doesn't modify */
> +                      (vaddr_t)to, len, GVA_INFO(current),
> +                      COPY_to_guest | COPY_linear);
>  }
> 
>  unsigned long raw_copy_to_guest_flush_dcache(void *to, const void 
> *from,
>                                               unsigned int len)
>  {
> -    return copy_guest((void *)from, (vaddr_t)to, len, 
> GVA_INFO(current),
> +    return copy_guest((void *)from, /* COPY_to_guest doesn't modify */
> +                      (vaddr_t)to, len, GVA_INFO(current),
>                        COPY_to_guest | COPY_flush_dcache | 
> COPY_linear);
>  }

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [PATCH 08/14] Arm/guestcopy: deviate copy_guest() uses just like their x86 counterparts

[Jan Beulich]

On 21.09.2026 13:46, Nicola Vetrini wrote:
> On 2026-09-02 08:33, Jan Beulich wrote:
>> Like x86/HVM's __hvm_copy(), Arm's copy_guest() is used for both 
>> to-guest
>> and from-guest copying. Naturally in the latter case the hypervisor 
>> buffer
>> needs writing to, hence the function parameter cannot be 
>> pointer-to-const.
>>
>> No functional change intended.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Thanks.

> See comment below, but I can do as a follow-up if you'd like.
> 
>> ---
>> Of course for both the pre-existing x86 deviation and the new Arm one 
>> it
>> might be more robust if the deviation was also limited to the 
>> respective
>> source file. Can this be expressed together with the needed regex?
> 
> Yes, e.g.
> 
> -config=MC3A2.R11.8,reports+={safe,"any_area(any_loc(file(^xen/arch/arm/...$)&&text(^.*copy_guest.*COPY_to_guest 
> doesn't modify.*$)))"}
> 
> not compile-tested, so YMMV.

Yes, please make this a follow-up, then also covering the x86 counterpart.

Jan

Re: [PATCH 01/14] lib: obey to Misra rule 11.8 where possible

[Volodymyr Babchuk]

Hi Jan,

Jan Beulich <jbeulich@suse.com> writes:

> Casting away const-ness (or volatile-ness) is never a good idea, but
> some library functions (e.g. strchr()) require doing so. Where not
> required, remove / replace respective casts.
>
> While there convert touched functions to Xen style.
>
> No functional change intended.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

>
> --- a/xen/lib/memcpy.c
> +++ b/xen/lib/memcpy.c
> @@ -15,12 +15,13 @@
>   */
>  void *(memcpy)(void *dest, const void *src, size_t n)
>  {
> -	char *tmp = (char *) dest, *s = (char *) src;
> +    char *tmp = dest;
> +    const char *s = src;
>  
> -	while (n--)
> -		*tmp++ = *s++;
> +    while ( n-- )
> +        *tmp++ = *s++;
>  
> -	return dest;
> +    return dest;
>  }
>  
>  /*
> --- a/xen/lib/memmove.c
> +++ b/xen/lib/memmove.c
> @@ -14,21 +14,25 @@
>   */
>  void *(memmove)(void *dest, const void *src, size_t n)
>  {
> -	char *tmp, *s;
> +    char *tmp;
> +    const char *s;
>  
> -	if (dest <= src) {
> -		tmp = (char *) dest;
> -		s = (char *) src;
> -		while (n--)
> -			*tmp++ = *s++;
> -	} else {
> -		tmp = (char *) dest + n;
> -		s = (char *) src + n;
> -		while (n--)
> -			*--tmp = *--s;
> -	}
> +    if ( dest <= src )
> +    {
> +        tmp = dest;
> +        s = src;
> +        while ( n-- )
> +            *tmp++ = *s++;
> +    }
> +    else
> +    {
> +        tmp = dest + n;
> +        s = src + n;
> +        while ( n-- )
> +            *--tmp = *--s;
> +    }
>  
> -	return dest;
> +    return dest;
>  }
>  
>  /*
> --- a/xen/lib/strcmp.c
> +++ b/xen/lib/strcmp.c
> @@ -11,16 +11,15 @@
>   */
>  int (strcmp)(const char *cs, const char *ct)
>  {
> -	unsigned char *csu = (unsigned char *)cs;
> -	unsigned char *ctu = (unsigned char *)ct;
> -	int res;
> +    const unsigned char *csu = (const void *)cs;
> +    const unsigned char *ctu = (const void *)ct;
> +    int res;
>  
> -	while (1) {
> -		if ((res = *csu - *ctu++) != 0 || !*csu++)
> -			break;
> -	}
> +    for ( ; ; )
> +        if ( (res = *csu - *ctu++) != 0 || !*csu++ )
> +            break;
>  
> -	return res;
> +    return res;
>  }
>  
>  /*
> --- a/xen/lib/strncmp.c
> +++ b/xen/lib/strncmp.c
> @@ -12,17 +12,18 @@
>   */
>  int (strncmp)(const char *cs, const char *ct, size_t count)
>  {
> -	unsigned char *csu = (unsigned char *)cs;
> -	unsigned char *ctu = (unsigned char *)ct;
> -	int res = 0;
> +    const unsigned char *csu = (const void *)cs;
> +    const unsigned char *ctu = (const void *)ct;
> +    int res = 0;
>  
> -	while (count) {
> -		if ((res = *csu - *ctu++) != 0 || !*csu++)
> -			break;
> -		count--;
> -	}
> +    while ( count )
> +    {
> +        if ( (res = *csu - *ctu++) != 0 || !*csu++ )
> +            break;
> +        count--;
> +    }
>  
> -	return res;
> +    return res;
>  }
>  
>  /*

-- 
WBR, Volodymyr

Re: [PATCH 09/14] ACPI: address a Misra rule 11.8 violation in Arm code

[Volodymyr Babchuk]

Hi Jan,

Jan Beulich <jbeulich@suse.com> writes:

> Outside of drivers/acpi/tables/ (which is excluded from Eclair reporting
> for dubious reasons), arch/arm/acpi/domain_build.c is the only user of
> ACPI_COMPARE_NAME(). Make the macro const correct, at the expense of
> introducing a "const" variant of ACPI_CAST_PTR(), and at the expense of

I think you can avoid adding ACPI_CAST_PTR() by providing a const
type. Like this:

+#define ACPI_COMPARE_NAME(a, b)         (*ACPI_CAST_PTR (const u32, a) == \
+                                         *ACPI_CAST_PTR (const u32, b))


> altering an imported file.
>
> No functional change intended.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/include/acpi/acmacros.h
> +++ b/xen/include/acpi/acmacros.h
> @@ -103,6 +103,7 @@
>   * Pointer manipulation
>   */
>  #define ACPI_CAST_PTR(t, p)             ((t *) (acpi_uintptr_t) (p))
> +#define ACPI_CAST_CPTR(t, p)            ((const t *) (const acpi_uintptr_t) (p))
>  #define ACPI_CAST_INDIRECT_PTR(t, p)    ((t **) (acpi_uintptr_t) (p))
>  #define ACPI_ADD_PTR(t,a,b)             ACPI_CAST_PTR (t, (ACPI_CAST_PTR (u8,(a)) + (acpi_native_uint)(b)))
>  #define ACPI_PTR_DIFF(a,b)              (acpi_native_uint) (ACPI_CAST_PTR (u8,(a)) - ACPI_CAST_PTR (u8,(b)))
> @@ -116,9 +117,12 @@
>  #define ACPI_PTR_TO_PHYSADDR(i)         ACPI_TO_INTEGER(i)
>  
>  #ifndef ACPI_MISALIGNMENT_NOT_SUPPORTED
> -#define ACPI_COMPARE_NAME(a,b)          (*ACPI_CAST_PTR (u32,(a)) == *ACPI_CAST_PTR (u32,(b)))
> +#define ACPI_COMPARE_NAME(a, b)         (*ACPI_CAST_CPTR (u32, a) == \
> +                                         *ACPI_CAST_CPTR (u32, b))
>  #else
> -#define ACPI_COMPARE_NAME(a,b)          (!ACPI_STRNCMP (ACPI_CAST_PTR (char,(a)), ACPI_CAST_PTR (char,(b)), ACPI_NAME_SIZE))
> +#define ACPI_COMPARE_NAME(a, b)         (!ACPI_STRNCMP (ACPI_CAST_CPTR (char, a), \
> +                                                        ACPI_CAST_CPTR (char, b), \
> +                                                        ACPI_NAME_SIZE))
>  #endif
>  
>  /*

-- 
WBR, Volodymyr

Re: [PATCH 10/14] ELF/notes: use pointer-to-const by default in ELFNOTE_...()

[Volodymyr Babchuk]

Hi Jan,

Jan Beulich <jbeulich@suse.com> writes:

> Not doing so results in a number of Misra rule 11.8 (casting away of
> const-ness) violations. We need to allow kexec to use pointer to non-
> const though, so provide a means to override the default.
>
> For ELFNOTE_NEXT() we can do better and simply re-apply the type of the
> incoming pointer.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/common/kexec.c
> +++ b/xen/common/kexec.c
> @@ -6,6 +6,9 @@
>   * - Magnus Damm <magnus@valinux.co.jp>
>   */
>  
> +/* We're producing ELF notes here. */
> +#define ELFNOTE_CONST

Frankly, it feels backwards. When reading this line of code I am
assuming that you are adding constness to ELF notes because you are
defining ELFNOTE_CONST. And I had to check the elf.h to understand that
you are doing exactly opposite. I am pretty sure that other people will
confused by this as well.

> +
>  #include <xen/acpi.h>
>  #include <xen/console.h>
>  #include <xen/cpu.h>
> --- a/xen/include/xen/elf.h
> +++ b/xen/include/xen/elf.h
> @@ -29,9 +29,14 @@
>  
>  #include <xen/elfstructs.h>
>  
> +#ifndef ELFNOTE_CONST
> +#define ELFNOTE_CONST const
> +#endif
> +
>  #define ELFNOTE_ALIGN(_n_) (((_n_)+3)&~3)
> -#define ELFNOTE_NAME(_n_) ((char*)(_n_) + sizeof(*(_n_)))
> +#define ELFNOTE_NAME(_n_) ((ELFNOTE_CONST char *)(_n_) + sizeof(*(_n_)))
>  #define ELFNOTE_DESC(_n_) (ELFNOTE_NAME(_n_) + ELFNOTE_ALIGN((_n_)->namesz))
> -#define ELFNOTE_NEXT(_n_) ((Elf_Note *)(ELFNOTE_DESC(_n_) + ELFNOTE_ALIGN((_n_)->descsz)))
> +#define ELFNOTE_NEXT(_n_) ((typeof(_n_))(ELFNOTE_DESC(_n_) + \
> +                                         ELFNOTE_ALIGN((_n_)->descsz)))
>  
>  #endif /* __XEN_ELF_H__ */

-- 
WBR, Volodymyr

Re: [PATCH 11/14] crypto/vmac: don't cast away const-ness in aes_key_setup()

[Volodymyr Babchuk]

Jan Beulich <jbeulich@suse.com> writes:

> vmac_set_key() passes in a pointer-to-const, which has const-ness removed
> despite rijndaelKeySetupEnc() properly taking pointer-to-const.
>
> No functional change.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

> ---
> I was wondering whether I shouldn't adjust the adjacent aes_encryption()
> right away as well.
>
> --- a/xen/include/crypto/vmac.h
> +++ b/xen/include/crypto/vmac.h
> @@ -69,7 +69,7 @@ typedef u32 aes_int_key[4*(VMAC_KEY_LEN/
>  	    				    (u8 *)(in), (u8 *)(out))
>  #define aes_key_setup(user_key,int_key)                 \
>  	    	rijndaelKeySetupEnc((u32 *)(int_key),       \
> -	    	                    (u8 *)(user_key), \
> +	    	                    (const u8 *)(user_key), \
>  	    	                    VMAC_KEY_LEN)
>  #endif
>  

-- 
WBR, Volodymyr

Re: [PATCH 10/14] ELF/notes: use pointer-to-const by default in ELFNOTE_...()

[Jan Beulich]

On 22.09.2026 03:45, Volodymyr Babchuk wrote:
> Jan Beulich <jbeulich@suse.com> writes:
>> Not doing so results in a number of Misra rule 11.8 (casting away of
>> const-ness) violations. We need to allow kexec to use pointer to non-
>> const though, so provide a means to override the default.
>>
>> For ELFNOTE_NEXT() we can do better and simply re-apply the type of the
>> incoming pointer.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/common/kexec.c
>> +++ b/xen/common/kexec.c
>> @@ -6,6 +6,9 @@
>>   * - Magnus Damm <magnus@valinux.co.jp>
>>   */
>>  
>> +/* We're producing ELF notes here. */
>> +#define ELFNOTE_CONST
> 
> Frankly, it feels backwards. When reading this line of code I am
> assuming that you are adding constness to ELF notes because you are
> defining ELFNOTE_CONST. And I had to check the elf.h to understand that
> you are doing exactly opposite. I am pretty sure that other people will
> confused by this as well.

Well, that's certainly possible. Yet then you or them are asked to make
an alternative suggestion. An option I could think of would be to merely
rename what is ELFNOTE_CONST right now, to no longer have the word
"CONST" in it. ELFNOTE_MODIFIER maybe, albeit that reads a little clumsy
to me.

Jan

Re: [PATCH 09/14] ACPI: address a Misra rule 11.8 violation in Arm code

[Jan Beulich]

On 22.09.2026 03:38, Volodymyr Babchuk wrote:
> Jan Beulich <jbeulich@suse.com> writes:
>> Outside of drivers/acpi/tables/ (which is excluded from Eclair reporting
>> for dubious reasons), arch/arm/acpi/domain_build.c is the only user of
>> ACPI_COMPARE_NAME(). Make the macro const correct, at the expense of
>> introducing a "const" variant of ACPI_CAST_PTR(), and at the expense of
> 
> I think you can avoid adding ACPI_CAST_PTR() by providing a const
> type. Like this:
> 
> +#define ACPI_COMPARE_NAME(a, b)         (*ACPI_CAST_PTR (const u32, a) == \
> +                                         *ACPI_CAST_PTR (const u32, b))

I don't think so. You did notice ...

>> --- a/xen/include/acpi/acmacros.h
>> +++ b/xen/include/acpi/acmacros.h
>> @@ -103,6 +103,7 @@
>>   * Pointer manipulation
>>   */
>>  #define ACPI_CAST_PTR(t, p)             ((t *) (acpi_uintptr_t) (p))
>> +#define ACPI_CAST_CPTR(t, p)            ((const t *) (const acpi_uintptr_t) (p))

... this, I assume. On the surface it looks odd, because one would expect
acpi_uintptr_t to be a scalar type, like uintptr_t is. But it isn't:

#ifndef acpi_uintptr_t
#define acpi_uintptr_t                  void *
#endif

The only alternative I see (somewhat more risky overall) would be to
introduce

#define acpi_uintptr_t                  uintptr_t

in acpi/platform/aclinux.h (or, imo less desirable, acpi/platform/acenv.h).

Then what you suggest would no longer cast away const-ness at any of the
individual casting steps.

Jan

Re: [PATCH 09/14] ACPI: address a Misra rule 11.8 violation in Arm code

[Volodymyr Babchuk]

Hi Jan,

Jan Beulich <jbeulich@suse.com> writes:

> On 22.09.2026 03:38, Volodymyr Babchuk wrote:
>> Jan Beulich <jbeulich@suse.com> writes:
>>> Outside of drivers/acpi/tables/ (which is excluded from Eclair reporting
>>> for dubious reasons), arch/arm/acpi/domain_build.c is the only user of
>>> ACPI_COMPARE_NAME(). Make the macro const correct, at the expense of
>>> introducing a "const" variant of ACPI_CAST_PTR(), and at the expense of
>> 
>> I think you can avoid adding ACPI_CAST_PTR() by providing a const
>> type. Like this:
>> 
>> +#define ACPI_COMPARE_NAME(a, b)         (*ACPI_CAST_PTR (const u32, a) == \
>> +                                         *ACPI_CAST_PTR (const u32, b))
>
> I don't think so. You did notice ...
>
>>> --- a/xen/include/acpi/acmacros.h
>>> +++ b/xen/include/acpi/acmacros.h
>>> @@ -103,6 +103,7 @@
>>>   * Pointer manipulation
>>>   */
>>>  #define ACPI_CAST_PTR(t, p)             ((t *) (acpi_uintptr_t) (p))
>>> +#define ACPI_CAST_CPTR(t, p)            ((const t *) (const acpi_uintptr_t) (p))
>
> ... this, I assume. On the surface it looks odd, because one would expect
> acpi_uintptr_t to be a scalar type, like uintptr_t is. But it isn't:
>
> #ifndef acpi_uintptr_t
> #define acpi_uintptr_t                  void *
> #endif

Well, that was unexpected. Talk about principle of least surprise...

>
> The only alternative I see (somewhat more risky overall) would be to
> introduce
>
> #define acpi_uintptr_t                  uintptr_t

What are the risks? I'd really prefer to have fewer gotchas in the code.


-- 
WBR, Volodymyr

Re: [PATCH 10/14] ELF/notes: use pointer-to-const by default in ELFNOTE_...()

[Volodymyr Babchuk]

Hi Jan,

Jan Beulich <jbeulich@suse.com> writes:

> On 22.09.2026 03:45, Volodymyr Babchuk wrote:
>> Jan Beulich <jbeulich@suse.com> writes:
>>> Not doing so results in a number of Misra rule 11.8 (casting away of
>>> const-ness) violations. We need to allow kexec to use pointer to non-
>>> const though, so provide a means to override the default.
>>>
>>> For ELFNOTE_NEXT() we can do better and simply re-apply the type of the
>>> incoming pointer.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> --- a/xen/common/kexec.c
>>> +++ b/xen/common/kexec.c
>>> @@ -6,6 +6,9 @@
>>>   * - Magnus Damm <magnus@valinux.co.jp>
>>>   */
>>>  
>>> +/* We're producing ELF notes here. */
>>> +#define ELFNOTE_CONST
>> 
>> Frankly, it feels backwards. When reading this line of code I am
>> assuming that you are adding constness to ELF notes because you are
>> defining ELFNOTE_CONST. And I had to check the elf.h to understand that
>> you are doing exactly opposite. I am pretty sure that other people will
>> confused by this as well.
>
> Well, that's certainly possible. Yet then you or them are asked to make
> an alternative suggestion. An option I could think of would be to merely
> rename what is ELFNOTE_CONST right now, to no longer have the word
> "CONST" in it. ELFNOTE_MODIFIER maybe, albeit that reads a little clumsy
> to me.

Taking into account that we need 'const' to be enabled by default, maybe
something like that?

#ifndef ELFNOTE_NO_CONST
#define __ELFNOTE_CONST const
#else
#define __ELFNOTE_CONST
#endif

?

And then use

#define ELFNOTE_NO_CONST

in this hunk

-- 
WBR, Volodymyr

Re: [PATCH 09/14] ACPI: address a Misra rule 11.8 violation in Arm code

[Jan Beulich]

On 22.09.2026 12:57, Volodymyr Babchuk wrote:
> Jan Beulich <jbeulich@suse.com> writes:
>> On 22.09.2026 03:38, Volodymyr Babchuk wrote:
>>> Jan Beulich <jbeulich@suse.com> writes:
>>>> Outside of drivers/acpi/tables/ (which is excluded from Eclair reporting
>>>> for dubious reasons), arch/arm/acpi/domain_build.c is the only user of
>>>> ACPI_COMPARE_NAME(). Make the macro const correct, at the expense of
>>>> introducing a "const" variant of ACPI_CAST_PTR(), and at the expense of
>>>
>>> I think you can avoid adding ACPI_CAST_PTR() by providing a const
>>> type. Like this:
>>>
>>> +#define ACPI_COMPARE_NAME(a, b)         (*ACPI_CAST_PTR (const u32, a) == \
>>> +                                         *ACPI_CAST_PTR (const u32, b))
>>
>> I don't think so. You did notice ...
>>
>>>> --- a/xen/include/acpi/acmacros.h
>>>> +++ b/xen/include/acpi/acmacros.h
>>>> @@ -103,6 +103,7 @@
>>>>   * Pointer manipulation
>>>>   */
>>>>  #define ACPI_CAST_PTR(t, p)             ((t *) (acpi_uintptr_t) (p))
>>>> +#define ACPI_CAST_CPTR(t, p)            ((const t *) (const acpi_uintptr_t) (p))
>>
>> ... this, I assume. On the surface it looks odd, because one would expect
>> acpi_uintptr_t to be a scalar type, like uintptr_t is. But it isn't:
>>
>> #ifndef acpi_uintptr_t
>> #define acpi_uintptr_t                  void *
>> #endif
> 
> Well, that was unexpected. Talk about principle of least surprise...
> 
>> The only alternative I see (somewhat more risky overall) would be to
>> introduce
>>
>> #define acpi_uintptr_t                  uintptr_t
> 
> What are the risks? I'd really prefer to have fewer gotchas in the code.

It hides casting away of const-ness. Right here we would like that, yet in
the general case I think we don't. Otoh Linux switched to the above in the
5.18 dev cycle.

Jan

Re: [PATCH 10/14] ELF/notes: use pointer-to-const by default in ELFNOTE_...()

[Jan Beulich]

On 22.09.2026 13:01, Volodymyr Babchuk wrote:
> Hi Jan,
> 
> Jan Beulich <jbeulich@suse.com> writes:
> 
>> On 22.09.2026 03:45, Volodymyr Babchuk wrote:
>>> Jan Beulich <jbeulich@suse.com> writes:
>>>> Not doing so results in a number of Misra rule 11.8 (casting away of
>>>> const-ness) violations. We need to allow kexec to use pointer to non-
>>>> const though, so provide a means to override the default.
>>>>
>>>> For ELFNOTE_NEXT() we can do better and simply re-apply the type of the
>>>> incoming pointer.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>
>>>> --- a/xen/common/kexec.c
>>>> +++ b/xen/common/kexec.c
>>>> @@ -6,6 +6,9 @@
>>>>   * - Magnus Damm <magnus@valinux.co.jp>
>>>>   */
>>>>  
>>>> +/* We're producing ELF notes here. */
>>>> +#define ELFNOTE_CONST
>>>
>>> Frankly, it feels backwards. When reading this line of code I am
>>> assuming that you are adding constness to ELF notes because you are
>>> defining ELFNOTE_CONST. And I had to check the elf.h to understand that
>>> you are doing exactly opposite. I am pretty sure that other people will
>>> confused by this as well.
>>
>> Well, that's certainly possible. Yet then you or them are asked to make
>> an alternative suggestion. An option I could think of would be to merely
>> rename what is ELFNOTE_CONST right now, to no longer have the word
>> "CONST" in it. ELFNOTE_MODIFIER maybe, albeit that reads a little clumsy
>> to me.
> 
> Taking into account that we need 'const' to be enabled by default, maybe
> something like that?
> 
> #ifndef ELFNOTE_NO_CONST
> #define __ELFNOTE_CONST const
> #else
> #define __ELFNOTE_CONST
> #endif
> 
> ?
> 
> And then use
> 
> #define ELFNOTE_NO_CONST
> 
> in this hunk

Can do, just without the leading underscores that you did introduce.

Jan

Re: [PATCH 09/14] ACPI: address a Misra rule 11.8 violation in Arm code

[Volodymyr Babchuk]

Hi Jan,

Sorry for the late reply

Jan Beulich <jbeulich@suse.com> writes:

> On 22.09.2026 12:57, Volodymyr Babchuk wrote:
>> Jan Beulich <jbeulich@suse.com> writes:
>>> On 22.09.2026 03:38, Volodymyr Babchuk wrote:
>>>> Jan Beulich <jbeulich@suse.com> writes:
>>>>> Outside of drivers/acpi/tables/ (which is excluded from Eclair reporting
>>>>> for dubious reasons), arch/arm/acpi/domain_build.c is the only user of
>>>>> ACPI_COMPARE_NAME(). Make the macro const correct, at the expense of
>>>>> introducing a "const" variant of ACPI_CAST_PTR(), and at the expense of
>>>>
>>>> I think you can avoid adding ACPI_CAST_PTR() by providing a const
>>>> type. Like this:
>>>>
>>>> +#define ACPI_COMPARE_NAME(a, b)         (*ACPI_CAST_PTR (const u32, a) == \
>>>> +                                         *ACPI_CAST_PTR (const u32, b))
>>>
>>> I don't think so. You did notice ...
>>>
>>>>> --- a/xen/include/acpi/acmacros.h
>>>>> +++ b/xen/include/acpi/acmacros.h
>>>>> @@ -103,6 +103,7 @@
>>>>>   * Pointer manipulation
>>>>>   */
>>>>>  #define ACPI_CAST_PTR(t, p)             ((t *) (acpi_uintptr_t) (p))
>>>>> +#define ACPI_CAST_CPTR(t, p)            ((const t *) (const acpi_uintptr_t) (p))
>>>
>>> ... this, I assume. On the surface it looks odd, because one would expect
>>> acpi_uintptr_t to be a scalar type, like uintptr_t is. But it isn't:
>>>
>>> #ifndef acpi_uintptr_t
>>> #define acpi_uintptr_t                  void *
>>> #endif
>> 
>> Well, that was unexpected. Talk about principle of least surprise...
>> 
>>> The only alternative I see (somewhat more risky overall) would be to
>>> introduce
>>>
>>> #define acpi_uintptr_t                  uintptr_t
>> 
>> What are the risks? I'd really prefer to have fewer gotchas in the code.
>
> It hides casting away of const-ness. Right here we would like that, yet in
> the general case I think we don't. Otoh Linux switched to the above in the
> 5.18 dev cycle.

... but for different reason. clang complained about pointer
subtraction with a null pointer. This does not fired in Xen because we
are not using ACPI_PTR_DIFF() yet. But, if we are going to use it, we'll
face the same problem.

So, if we are already touching this parts, maybe it is better port that
Linux patch ([1]) and use const types as I suggested?


[1] https://lore.kernel.org/linux-acpi/20210927121338.938994-1-arnd@kernel.org, 

-- 
WBR, Volodymyr

Re: [PATCH 03/14] x86: have cmpxchg16b() obey to Misra rule 11.8

[Andrew Cooper]

On 02/09/2026 7:31 am, Jan Beulich wrote:
> Casting away const-ness (or volatile-ness) is never a good idea, and
> there's no need to in cmpxchg16b().
>
> No functional change intended.
>
> Fixes: e0116acbbd0b ("x86: add cmpxchg16b support")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH 12/14] gnttab: don't cast away constness

[Roger Pau Monné]

On Wed, Sep 02, 2026 at 08:35:47AM +0200, Jan Beulich wrote:
> While _set_status_v2() indeed doesn't alter the grant_entry_header_t it
> is handed a pointer to, _set_status_v1() does. Drop the const from the
> parameter of the latter (and then necessarily also from _set_status()'s),
> while adding const to the local variable of the former.
> 
> No functional change.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger@xenproject.org>

Could we also use a union in the functions themselves to avoid the cast?

union {
    const grant_entry_header_t *shah;
    uint32_t *raw;
} grant = { .shah = shah };

> ---
> Really I think it would be best if we did away with the raw_shah local
> variables (which looks reasonably simple for at least _set_status_v2()).
> I'm unconvinced that we really need to use ACCESS_ONCE() here. Torn reads
> aren't a problem; what we require is that we look at a stable local copy,
> and that can be achieved by putting barrier() after the reads.

Possibly - I would be fine with that, which kinds of render my
previous suggestion to use a union not very useful, if we can get rid
of those variables in the first place.

Thanks, Roger.

Re: [PATCH 12/14] gnttab: don't cast away constness

[Jan Beulich]

On 05.10.2026 18:59, Roger Pau Monné wrote:
> On Wed, Sep 02, 2026 at 08:35:47AM +0200, Jan Beulich wrote:
>> While _set_status_v2() indeed doesn't alter the grant_entry_header_t it
>> is handed a pointer to, _set_status_v1() does. Drop the const from the
>> parameter of the latter (and then necessarily also from _set_status()'s),
>> while adding const to the local variable of the former.
>>
>> No functional change.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Roger Pau Monné <roger@xenproject.org>

Thanks.

> Could we also use a union in the functions themselves to avoid the cast?
> 
> union {
>     const grant_entry_header_t *shah;
>     uint32_t *raw;
> } grant = { .shah = shah };

Possibly. However, irrespective of us using such all over the place, it
still is - strictly speaking - UB.

>> ---
>> Really I think it would be best if we did away with the raw_shah local
>> variables (which looks reasonably simple for at least _set_status_v2()).
>> I'm unconvinced that we really need to use ACCESS_ONCE() here. Torn reads
>> aren't a problem; what we require is that we look at a stable local copy,
>> and that can be achieved by putting barrier() after the reads.
> 
> Possibly - I would be fine with that, which kinds of render my
> previous suggestion to use a union not very useful, if we can get rid
> of those variables in the first place.

I'll add a todo item to my list to give this a try.

Jan

Re: [PATCH 12/14] gnttab: don't cast away constness

[Jan Beulich]

On 06.10.2026 07:59, Jan Beulich wrote:
> On 05.10.2026 18:59, Roger Pau Monné wrote:
>> On Wed, Sep 02, 2026 at 08:35:47AM +0200, Jan Beulich wrote:
>>> While _set_status_v2() indeed doesn't alter the grant_entry_header_t it
>>> is handed a pointer to, _set_status_v1() does. Drop the const from the
>>> parameter of the latter (and then necessarily also from _set_status()'s),
>>> while adding const to the local variable of the former.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> Acked-by: Roger Pau Monné <roger@xenproject.org>
> 
> Thanks.
> 
>> Could we also use a union in the functions themselves to avoid the cast?
>>
>> union {
>>     const grant_entry_header_t *shah;
>>     uint32_t *raw;
>> } grant = { .shah = shah };
> 
> Possibly. However, irrespective of us using such all over the place, it
> still is - strictly speaking - UB.

Then again we already have union grant_combo, which may want leveraging here.

Jan

Re: [PATCH 09/14] ACPI: address a Misra rule 11.8 violation in Arm code

[Jan Beulich]

On 22.09.2026 23:18, Volodymyr Babchuk wrote:
> Jan Beulich <jbeulich@suse.com> writes:
>> On 22.09.2026 12:57, Volodymyr Babchuk wrote:
>>> Jan Beulich <jbeulich@suse.com> writes:
>>>> On 22.09.2026 03:38, Volodymyr Babchuk wrote:
>>>>> Jan Beulich <jbeulich@suse.com> writes:
>>>>>> Outside of drivers/acpi/tables/ (which is excluded from Eclair reporting
>>>>>> for dubious reasons), arch/arm/acpi/domain_build.c is the only user of
>>>>>> ACPI_COMPARE_NAME(). Make the macro const correct, at the expense of
>>>>>> introducing a "const" variant of ACPI_CAST_PTR(), and at the expense of
>>>>>
>>>>> I think you can avoid adding ACPI_CAST_PTR() by providing a const
>>>>> type. Like this:
>>>>>
>>>>> +#define ACPI_COMPARE_NAME(a, b)         (*ACPI_CAST_PTR (const u32, a) == \
>>>>> +                                         *ACPI_CAST_PTR (const u32, b))
>>>>
>>>> I don't think so. You did notice ...
>>>>
>>>>>> --- a/xen/include/acpi/acmacros.h
>>>>>> +++ b/xen/include/acpi/acmacros.h
>>>>>> @@ -103,6 +103,7 @@
>>>>>>   * Pointer manipulation
>>>>>>   */
>>>>>>  #define ACPI_CAST_PTR(t, p)             ((t *) (acpi_uintptr_t) (p))
>>>>>> +#define ACPI_CAST_CPTR(t, p)            ((const t *) (const acpi_uintptr_t) (p))
>>>>
>>>> ... this, I assume. On the surface it looks odd, because one would expect
>>>> acpi_uintptr_t to be a scalar type, like uintptr_t is. But it isn't:
>>>>
>>>> #ifndef acpi_uintptr_t
>>>> #define acpi_uintptr_t                  void *
>>>> #endif
>>>
>>> Well, that was unexpected. Talk about principle of least surprise...
>>>
>>>> The only alternative I see (somewhat more risky overall) would be to
>>>> introduce
>>>>
>>>> #define acpi_uintptr_t                  uintptr_t
>>>
>>> What are the risks? I'd really prefer to have fewer gotchas in the code.
>>
>> It hides casting away of const-ness. Right here we would like that, yet in
>> the general case I think we don't. Otoh Linux switched to the above in the
>> 5.18 dev cycle.
> 
> ... but for different reason. clang complained about pointer
> subtraction with a null pointer. This does not fired in Xen because we
> are not using ACPI_PTR_DIFF() yet. But, if we are going to use it, we'll
> face the same problem.
> 
> So, if we are already touching this parts, maybe it is better port that
> Linux patch ([1]) and use const types as I suggested?
> 
> 
> [1] https://lore.kernel.org/linux-acpi/20210927121338.938994-1-arnd@kernel.org, 

This has nothing to do with acpi_uintptr_t, and also looks to never have
been committed. Were you instead meaning to refer to Linux commit
ae57857b9b63? (Oddly enough that commit links the URL you quote above,
just that that's not the patch which was committed.) If so, from a Misra
perspective this has a significant downside: ACPI_CAST_PTR() could then
be used to silently cast away constness. This, imo, is something we want
to avoid when the goal is to make our code more safe.

Jan