[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 2/3] arm/mpu: Provide and populate MPU C data structures


  • To: "Orzel, Michal" <michal.orzel@xxxxxxx>, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Ayan Kumar Halder <ayankuma@xxxxxxx>
  • Date: Thu, 5 Jun 2025 15:27:39 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=DHooqomRs93lb8qYsexZdpkQ3AHpuxhKmbs61F9u2CM=; b=vOJmodazRg545qyeI0ED3WlwmecgFoZe5d/df5CBEu9vTAAOGgebdAmejFtv3ck7sU/zI+/guF/nBDsff1LapjNf58i7i1XS2IyudPAtz+9au8P9n+x9mPy6ajrIWrkXusbOiuZnn7mL21rdr6nYOtu6rZUo6ZqR/zbN3lHpK+B7XRx6fj4O6EEBkb/B6ei4Jlahur2wBEwVgwgZdKMZpxklK+ZhBTDfaPTNSdld2sDKXW9KdV8ImV6sDYTgKfHibfTG7rZZjywZ89hBWDVT1YLVcB4BYs5vnZ7c++iNSBWUCrAQoSi/ml0ioAYlnFOqaP7AU78wFGSgelSDDTLzjQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=cm7+fef0sNWc/TWNS5XfXRNQ/eLZ5I/qCYAWvX9aNGELRF3TZTI615IF6aFhMzG7p66laEnHNUwNyYJivI/RVBt4pJbRTaPYzE5eTSnd7UXwq37aRGzdy6Qp6JyBPknoJiicn09B1UUlNdT/f7/PT8PPzyFx33omO8VMA8f1pkVIGJyBmTFPS8ADfR3rDonjunwebbEfsiFhNsGp0hLpWGDy9spkHuhDimGfcEl5ZSkt5jq0JHEp23k+6ZHjt82rGTgxP2okZ+YLfjQ8pdNfywgphjccrayYoXE4xjabSG5IUHYQ0brSy/eUxj5HY+vyNFlPBwJrxF1F/4Pacf1IVg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com;
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Thu, 05 Jun 2025 14:27:57 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Michal/Julien,

On 05/06/2025 08:44, Orzel, Michal wrote:

On 04/06/2025 19:43, Ayan Kumar Halder wrote:
Do the arm32 equivalent initialization for commit id ca5df936c4.
This is not a good commit msg.
Also, we somewhat require passing 12 char long IDs.

Modify Arm32 assembly boot code to reset any unused MPU region, initialise 'max_mpu_regions' with the number of supported MPU regions and set/clear the bitmap 'xen_mpumap_mask' used to track the enabled regions.

Use the macro definition for "dcache_line_size" from linux.

Does ^^^ read fine ?


Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
---
  xen/arch/arm/arm32/asm-offsets.c         |  6 +++
  xen/arch/arm/arm32/mpu/head.S            | 57 ++++++++++++++++++++++++
  xen/arch/arm/include/asm/mpu/regions.inc |  8 +++-
  3 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/arm32/asm-offsets.c b/xen/arch/arm/arm32/asm-offsets.c
index 8bbb0f938e..c203ce269d 100644
--- a/xen/arch/arm/arm32/asm-offsets.c
+++ b/xen/arch/arm/arm32/asm-offsets.c
@@ -75,6 +75,12 @@ void __dummy__(void)
OFFSET(INITINFO_stack, struct init_info, stack);
     BLANK();
+
+#ifdef CONFIG_MPU
+   DEFINE(XEN_MPUMAP_MASK_sizeof, sizeof(xen_mpumap_mask));
+   DEFINE(XEN_MPUMAP_sizeof, sizeof(xen_mpumap));
+   BLANK();
+#endif
  }
/*
diff --git a/xen/arch/arm/arm32/mpu/head.S b/xen/arch/arm/arm32/mpu/head.S
index b2c5245e51..1f9eec6e68 100644
--- a/xen/arch/arm/arm32/mpu/head.S
+++ b/xen/arch/arm/arm32/mpu/head.S
@@ -10,6 +10,38 @@
  #include <asm/mpu/regions.inc>
  #include <asm/page.h>
+/*
+ * dcache_line_size - get the minimum D-cache line size from the CTR register.
+ */
I do think we should have a cache.S file to store cache related ops just like
for Arm64.
ok, I will introduce a new file.
Also, no need for multiline comment.
ack.

+    .macro  dcache_line_size, reg, tmp1, tmp2
I would prefer to use the macro from Linux that uses one temporary register
/*
 * dcache_line_size - get the minimum D-cache line size from the CTR register
 * on ARMv7.
 */
    .macro  dcache_line_size, reg, tmp
    mrc p15, 0, \tmp, c0, c0, 1     /* read ctr */
    lsr \tmp, \tmp, #16
    and \tmp, \tmp, #0xf             /* cache line size encoding */
    mov \reg, #4                          /* bytes per word */
    mov \reg, \reg, lsl \tmp         /* actual cache line size */
    .endm


+    mrc CP32(\reg, CTR)           // read CTR
NIT: wrong comment style + wrong alignment
yes, I should use /* ... */

+    ubfx   \tmp1, \reg, #16, #4   // Extract DminLine (bits 19:16) into tmp1
+    mov    \tmp2, #1
+    lsl    \tmp2, \tmp2, \tmp1    // tmp2 = 2^DminLine
+    lsl    \tmp2, \tmp2, #2       // tmp2 = 4 * 2^DminLine = cache line size 
in bytes
+    .endm
+
+/*
+ * __invalidate_dcache_area(addr, size)
+ *
+ * Ensure that the data held in the cache for the buffer is invalidated.
+ *
+ * - addr - start address of the buffer
+ * - size - size of the buffer
+ */
+FUNC(__invalidate_dcache_area)
+    dcache_line_size r2, r3, r4
+    add   r1, r0, r1
+    sub   r4, r2, #1
+    bic   r0, r0, r4
+1:  mcr   CP32(r0, DCIMVAC)     /* invalidate D line / unified line */
+    add   r0, r0, r2
+    cmp   r0, r1
+    blo   1b
+    dsb   sy
+    ret
+END(__invalidate_dcache_area)
+
  /*
   * Set up the memory attribute type tables and enable EL2 MPU and data cache.
   * If the Background region is enabled, then the MPU uses the default memory
@@ -49,6 +81,10 @@ FUNC(enable_boot_cpu_mm)
      mrc   CP32(r5, MPUIR_EL2)
      and   r5, r5, #NUM_MPU_REGIONS_MASK
+ ldr r0, =max_mpu_regions
Why ldr and not mov_w?
mov_w   r0, max_mpu_regions

+    str   r5, [r0]
+    mcr   CP32(r0, DCIMVAC) /* Invalidate cache for max_mpu_regions addr */
+
      /* x0: region sel */
      mov   r0, #0
      /* Xen text section. */
@@ -83,6 +119,27 @@ FUNC(enable_boot_cpu_mm)
      prepare_xen_region r0, r1, r2, r3, r4, r5, 
attr_prbar=REGION_DEVICE_PRBAR, attr_prlar=REGION_DEVICE_PRLAR
  #endif
+zero_mpu:
+    /* Reset remaining MPU regions */
+    cmp   r0, r5
+    beq   out_zero_mpu
+    mov   r1, #0
+    mov   r2, #1
+    prepare_xen_region r0, r1, r2, r3, r4, r5, attr_prlar=REGION_DISABLED_PRLAR
+    b     zero_mpu
+
+out_zero_mpu:
+    /* Invalidate data cache for MPU data structures */
+    mov r5, lr
+    ldr r0, =xen_mpumap_mask
Why not mov_w?
mov_w r0, xen_mpumap_mask

+    mov r1, #XEN_MPUMAP_MASK_sizeof
+    bl __invalidate_dcache_area
+
+    ldr r0, =xen_mpumap
+    mov r1, #XEN_MPUMAP_sizeof
+    bl __invalidate_dcache_area
+    mov lr, r5
+
      b    enable_mpu
  END(enable_boot_cpu_mm)
diff --git a/xen/arch/arm/include/asm/mpu/regions.inc b/xen/arch/arm/include/asm/mpu/regions.inc
index 6b8c233e6c..943bcda346 100644
--- a/xen/arch/arm/include/asm/mpu/regions.inc
+++ b/xen/arch/arm/include/asm/mpu/regions.inc
@@ -24,7 +24,13 @@
  #define XEN_MPUMAP_ENTRY_SHIFT  0x3     /* 8 byte structure */
.macro store_pair reg1, reg2, dst
-    .word 0xe7f000f0                    /* unimplemented */
+    str \reg1, [\dst]
+    add \dst, \dst, #4
+    str \reg2, [\dst]
AFAIR there is STM instruction to do the same
strd \reg1, \reg2, [\dst]

+.endm
+
+.macro invalidate_dcache_one reg
+    mcr CP32(\reg, DCIMVAC)
Why? You don't seem to use this macro

oh, this can be removed.

- Ayan


  .endm
#endif
~Michal




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.