On 09/06/2025 09:42, Julien Grall wrote:
Hi Ayan,
Hi Julien,

On 09/06/2025 09:27, Ayan Kumar Halder wrote:
On 09/06/2025 08:41, Orzel, Michal wrote:
diff --git a/xen/arch/arm/include/asm/mpu/regions.inc b/xen/arch/arm/ include/asm/mpu/regions.inc
index 6b8c233e6c..631b0b2b86 100644
--- a/xen/arch/arm/include/asm/mpu/regions.inc
+++ b/xen/arch/arm/include/asm/mpu/regions.inc
@@ -24,7 +24,7 @@
  #define XEN_MPUMAP_ENTRY_SHIFT  0x3     /* 8 byte structure */
  .macro store_pair reg1, reg2, dst
-    .word 0xe7f000f0                    /* unimplemented */
+    stm \dst, {\reg1, \reg2}  /* reg2 should be a higher register than reg1 */
Didn't we agree not to use STM (I suggested it but then Julien pointed out that
it's use in macro might not be the best)?

Ah my last response was not sent.

I realized that I cannot use strd due to the following error

Error: first transfer register must be even -- `strd r3,r4,[r1]'

Ah I missed the "even" part when reading the specification. However, we control the set of registers, so we can't we follow the restriction? This would be better...

I am ok to follow this. I just want to make sure that this looks ok to you

prepare_xen_region() invokes store_pair(). They are in common header.

So we need to make the change wherever prepare_xen_region() is invoked from arm32/mpu/head.S. This would look like

--- a/xen/arch/arm/arm32/mpu/head.S
+++ b/xen/arch/arm/arm32/mpu/head.S
@@ -58,33 +58,33 @@ FUNC(enable_boot_cpu_mm)
     /* Xen text section. */
     mov_w   r1, _stext
     mov_w   r2, _etext
-    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_TEXT_PRBAR
+    prepare_xen_region r0, r1, r2, r4, r3, r5, attr_prbar=REGION_TEXT_PRBAR

     /* Xen read-only data section. */
     mov_w   r1, _srodata
     mov_w   r2, _erodata
-    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_RO_PRBAR
+    prepare_xen_region r0, r1, r2, r4, r3, r5, attr_prbar=REGION_RO_PRBAR

     /* Xen read-only after init and data section. (RW data) */
     mov_w   r1, __ro_after_init_start
     mov_w   r2, __init_begin
-    prepare_xen_region r0, r1, r2, r3, r4, r5
+    prepare_xen_region r0, r1, r2, r4, r3, r5

     /* Xen code section. */
     mov_w   r1, __init_begin
     mov_w   r2, __init_data_begin
-    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_TEXT_PRBAR
+    prepare_xen_region r0, r1, r2, r4, r3, r5, attr_prbar=REGION_TEXT_PRBAR

     /* Xen data and BSS section. */
     mov_w   r1, __init_data_begin
     mov_w   r2, __bss_end
-    prepare_xen_region r0, r1, r2, r3, r4, r5
+    prepare_xen_region r0, r1, r2, r4, r3, r5

 #ifdef CONFIG_EARLY_PRINTK
     /* Xen early UART section. */
     mov_w   r1, CONFIG_EARLY_UART_BASE_ADDRESS
     mov_w   r2, (CONFIG_EARLY_UART_BASE_ADDRESS + CONFIG_EARLY_UART_SIZE)
-    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR +    prepare_xen_region r0, r1, r2, r4, r3, r5, attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR
 #endif

 zero_mpu:
@@ -93,7 +93,7 @@ zero_mpu:
     beq   out_zero_mpu
     mov   r1, #0
     mov   r2, #1
-    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prlar=REGION_DISABLED_PRLAR +    prepare_xen_region r0, r1, r2, r4, r3, r5, attr_prlar=REGION_DISABLED_PRLAR

So this would look a different different from its arm64 counterpart.

Are you ok with this change ?

- Ayan



So,  I am using stm with the following comment

... than a comment and hoping the developper/reviewer will notice it (the code is also misplaced). I am ready to bet this will likely cause some problem in the future.

Cheers,


Reply via email to