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

Re: [PATCH v3 34/52] xen/mpu: destroy an existing entry in Xen MPU memory mapping table


  • To: Ayan Kumar Halder <ayankuma@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Penny Zheng <penny.zheng@xxxxxxx>
  • Date: Mon, 3 Jul 2023 15:08:41 +0800
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 40.67.248.234) smtp.rcpttodomain=amd.com smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=none (message not signed); arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; 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=Uj/cDl03UCXZVJBoV0kYatgjVvUdCe940CYqzl1ONTc=; b=BJskarfGdW2RKr8VyGTQh1q/BpvRq44vhD3LL+kjOTwR1U2DAfceLeJJUD60fsqQxjvN/NZkA3eqUIovfZXjNBIuNJ1t41LIMyFS3yLBjFoEIjzspL5nh7O4B1GL4E4bUyZ5iAelobTbYs25JCip6EsxRo6EN17L9YKf2PkyJ5rCPdkPgutAXUE3triiUGoUsl9uMls1dBEt9B3T+tBAaJibHnYctQ8gV89RnnTPrB9CuKrTz1x99rKC5Yswm0hA7TUBfoKH4G4ix4XCoxW+prwtx/FtwWU03F4CrKlsCu5KUuzYVMN4HNTEBhvNJY8bo9Uzy+dGkGvMonHB8dhf2w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SfUzYA30LaFQqnxlyohNaWI11It6WRwUxWU88ksfqhX2wNdXLVrMzFqa9abCzC52WeeGPhzWdJ/H/wMqeDcdKuffRjKeGsWucyrdKFFCnmCNy6556tigc8/P647u+7mtGOSSexzipj9pfGtSeE23E3jppkKCGi1NEWI7LiP0L65cgyXsHtHqErIan3XST006bjEjCnJ++sTn+vkGYKgRW2CV1U8X6Jn6pdKyFX6jf8LN3528SVeTkAb5/6Q3tZZ3XPADcf5MHyk425ke7GfvlcOA7WLxYsz12qBFcQZlGT9cbgaVa/10a7vX+6nm/EAzHxW1sGDPCkoeW8xB8536lw==
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, "Volodymyr Babchuk" <Volodymyr_Babchuk@xxxxxxxx>, Wei Chen <wei.chen@xxxxxxx>
  • Delivery-date: Mon, 03 Jul 2023 07:09:22 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true

Hi Ayan

On 2023/7/1 00:17, Ayan Kumar Halder wrote:

On 26/06/2023 04:34, Penny Zheng wrote:
CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.


This commit expands xen_mpumap_update/xen_mpumap_update_entry to include
destroying an existing entry.

We define a new helper "control_xen_mpumap_region_from_index" to enable/disable the MPU region based on index. If region is within [0, 31], we could quickly disable the MPU region through PRENR_EL2 which provides direct access to the
PRLAR_EL2.EN bits of EL2 MPU regions.

Rignt now, we only support destroying a *WHOLE* MPU memory region,
part-region removing is not supported, as in worst case, it will
leave two fragments behind.

Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
---
v3:
- make pr_get_base()/pr_get_limit() static inline
- need an isb to ensure register write visible before zeroing the entry
---
  xen/arch/arm/include/asm/arm64/mpu.h     |  2 +
  xen/arch/arm/include/asm/arm64/sysregs.h |  3 +
  xen/arch/arm/mm.c                        |  5 ++
  xen/arch/arm/mpu/mm.c                    | 74 ++++++++++++++++++++++++
  4 files changed, 84 insertions(+)

diff --git a/xen/arch/arm/include/asm/arm64/mpu.h b/xen/arch/arm/include/asm/arm64/mpu.h
index 715ea69884..aee7947223 100644
--- a/xen/arch/arm/include/asm/arm64/mpu.h
+++ b/xen/arch/arm/include/asm/arm64/mpu.h
@@ -25,6 +25,8 @@
  #define REGION_UART_SEL            0x07
  #define MPUIR_REGION_MASK          ((_AC(1, UL) << 8) - 1)

+#define MPU_PRENR_BITS             32

This is common to R52 and R82.

Thus, you can put it in the common file (may be xen/arch/arm/include/asm/mpu/mm.h)


Will do.

+
  /* Access permission attributes. */
  /* Read/Write at EL2, No Access at EL1/EL0. */
  #define AP_RW_EL2 0x0
diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h
index c8a679afdd..96c025053b 100644
--- a/xen/arch/arm/include/asm/arm64/sysregs.h
+++ b/xen/arch/arm/include/asm/arm64/sysregs.h
@@ -509,6 +509,9 @@
  /* MPU Type registers encode */
  #define MPUIR_EL2   S3_4_C0_C0_4

+/* MPU Protection Region Enable Register encode */
+#define PRENR_EL2   S3_4_C6_C1_1
+
  #endif

  /* Access to system registers */
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 8625066256..247d17cfa1 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -164,7 +164,12 @@ int destroy_xen_mappings(unsigned long s, unsigned long e)
      ASSERT(IS_ALIGNED(s, PAGE_SIZE));
      ASSERT(IS_ALIGNED(e, PAGE_SIZE));
      ASSERT(s <= e);
+#ifndef CONFIG_HAS_MPU
      return xen_pt_update(s, INVALID_MFN, (e - s) >> PAGE_SHIFT, 0);
+#else
+    return xen_mpumap_update(virt_to_maddr((void *)s),
+                             virt_to_maddr((void *)e), 0);
+#endif
  }

Refer my comment in previous patch.

You can have two implementations of this function 1) xen/arch/arm/mmu/mm.c 2) xen/arch/arm/mpu/mm.h


Refer my comment in previous patch.

I prefer #ifdef in destroy_xen_mappings()


  int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 0a65b58dc4..a40055ae5e 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -425,6 +425,59 @@ static int mpumap_contain_region(pr_t *table, uint8_t nr_regions,
      return MPUMAP_REGION_FAILED;
  }

+/* Disable or enable EL2 MPU memory region at index #index */
+static void control_mpu_region_from_index(uint8_t index, bool enable)
+{
+    pr_t region;
+
+    read_protection_region(&region, index);
+    if ( !region_is_valid(&region) ^ enable )
+    {
+        printk(XENLOG_WARNING
+               "mpu: MPU memory region[%u] is already %s\n", index,
+               enable ? "enabled" : "disabled");
+        return;
+    }
+
+    /*
+     * ARM64v8R provides PRENR_EL2 to have direct access to the
+     * PRLAR_EL2.EN bits of EL2 MPU regions from 0 to 31.
+     */
+    if ( index < MPU_PRENR_BITS )
+    {
+        uint64_t orig, after;
+
+        orig = READ_SYSREG(PRENR_EL2);
+        if ( enable )
+            /* Set respective bit */
+            after = orig | (1UL << index);
+        else
+            /* Clear respective bit */
+            after = orig & (~(1UL << index));
+        WRITE_SYSREG(after, PRENR_EL2);
+    }
+    else
+    {
+        region.prlar.reg.en = enable ? 1 : 0;
+        write_protection_region((const pr_t*)&region, index);
+    }
+    /* Ensure the write before zeroing the entry */
dsb(); /* to ensure write completes */
+    isb();
+
+    /* Update according bitfield in xen_mpumap_mask */
+    spin_lock(&xen_mpumap_alloc_lock);
+
+    if ( enable )
+        set_bit(index, xen_mpumap_mask);
+    else
+    {
+        clear_bit(index, xen_mpumap_mask);
+        memset(&xen_mpumap[index], 0, sizeof(pr_t));
+    }
+
+    spin_unlock(&xen_mpumap_alloc_lock);
+}
+
  /*
   * Update an entry in Xen MPU memory region mapping table(xen_mpumap) at
   * the index @idx.
@@ -461,6 +514,27 @@ static int xen_mpumap_update_entry(paddr_t base, paddr_t limit,

          write_protection_region((const pr_t*)(&xen_mpumap[idx]), idx);
      }
+    else
+    {
+        /*
+         * Currently, we only support destroying a *WHOLE* MPU memory region, +         * part-region removing is not supported, as in worst case, it will
+         * leave two fragments behind.
+         * part-region removing will be introduced only when actual usage
+         * comes.
+         */
+        if ( rc == MPUMAP_REGION_INCLUSIVE )
+        {
+            region_printk("mpu: part-region removing is not supported\n");
+            return -EINVAL;
+        }
+
+        /* We are removing the region */
+        if ( rc != MPUMAP_REGION_FOUND )
+            return -EINVAL;
+
+        control_mpu_region_from_index(idx, false);
+    }

      return 0;
  }
--
2.25.1


- Ayan



 


Rackspace

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