MISRA Rule 11.3 prohibits casting between pointers to different object
types (except for char types). The violation  with '__init_begin' in
the original code was that it was being cast directly to a 'uint32_t *'
pointer.

To resolve this, replace the direct cast of '__init_begin' to 'uint32_t *'
with a cast to 'uint8_t *', and use 'memcpy()' to write the instruction
value. Using 'memcpy()' avoids undefined behavior related to alignment
and type-punning, and is guaranteed to be safe by the C standard.

No functional changes.

Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
Second possible solution (I hope) is using SAF comment.
The '__init_begin' is defined in linker script and has proper alignment,
so it's safe to cast it to 'uint32_t *' pointer type.

Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2125897167
---
 xen/arch/arm/mmu/setup.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
index eb8ed19ca1..00c4c8832d 100644
--- a/xen/arch/arm/mmu/setup.c
+++ b/xen/arch/arm/mmu/setup.c
@@ -481,7 +481,7 @@ void free_init_memory(void)
     unsigned long len = __init_end - __init_begin;
     uint32_t insn;
     unsigned int i, nr = len / sizeof(insn);
-    uint32_t *p;
+    uint8_t *p;
     int rc;
 
     rc = modify_xen_mappings((unsigned long)__init_begin,
@@ -501,9 +501,11 @@ void free_init_memory(void)
 #else
     insn = AARCH64_BREAK_FAULT;
 #endif
-    p = (uint32_t *)__init_begin;
     for ( i = 0; i < nr; i++ )
-        *(p + i) = insn;
+    {
+        p = (uint8_t *)__init_begin + i * sizeof(insn);
+        memcpy(p, &insn, sizeof(insn));
+    }
 
     rc = destroy_xen_mappings((unsigned long)__init_begin,
                               (unsigned long)__init_end);
-- 
2.43.0

On 28/10/2025 5:45 pm, Dmytro Prokopchuk1 wrote:
> diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
> index eb8ed19ca1..00c4c8832d 100644
> --- a/xen/arch/arm/mmu/setup.c
> +++ b/xen/arch/arm/mmu/setup.c
> @@ -481,7 +481,7 @@ void free_init_memory(void)
>      unsigned long len = __init_end - __init_begin;
>      uint32_t insn;
>      unsigned int i, nr = len / sizeof(insn);
> -    uint32_t *p;
> +    uint8_t *p;
>      int rc;
>  
>      rc = modify_xen_mappings((unsigned long)__init_begin,
> @@ -501,9 +501,11 @@ void free_init_memory(void)
>  #else
>      insn = AARCH64_BREAK_FAULT;
>  #endif
> -    p = (uint32_t *)__init_begin;
>      for ( i = 0; i < nr; i++ )
> -        *(p + i) = insn;
> +    {
> +        p = (uint8_t *)__init_begin + i * sizeof(insn);
> +        memcpy(p, &insn, sizeof(insn));
> +    }
>  
>      rc = destroy_xen_mappings((unsigned long)__init_begin,
>                                (unsigned long)__init_end);

I'm in agreement with Eclair, this is horrible code.

Putting an undefined instruction here is pretty useless.  By the time
destroy_xen_mappings() completes, you'll suffer a pagefault from trying
to execute there, rather than actually getting to execute code.

On x86 we simply zero the memory and then hand it back to the heap
allocator.  ARM doesn't seem to do this yet.

Irrespective, it either wants zeroing, or maybe SCRUB_PATTERN as we use
for other invalid memory.

Both of these can be done with a simple memset(), which simplifies
everything.

~Andrew