[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 31/52] xen/mpu: make early_fdt_map support in MPU systems
Hi,
On 03/07/2023 06:12, Penny Zheng wrote:
Hi,
On 2023/6/30 23:02, Julien Grall wrote:
Hi,
On 30/06/2023 15:42, Ayan Kumar Halder wrote:
Hi Julien,
On 30/06/2023 12:22, Julien Grall wrote:
On 30/06/2023 11:49, Ayan Kumar Halder wrote:
On 30/06/2023 05:07, Penny Zheng wrote:
Hi,
Hi Penny,
On 2023/6/30 01:22, 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.
In MPU system, MPU memory region is always mapped PAGE_ALIGN, so
in order to
not access unexpected memory area, dtb section in xen.lds.S
should be made
page-aligned too.
We add . = ALIGN(PAGE_SIZE); in the head of dtb section to make
it happen.
In this commit, we map early FDT with a transient MPU memory
region, as
it will be relocated into heap and unmapped at the end of boot.
Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
---
v3:
- map the first 2MB. Check the size and then re-map with an
extra 2MB if needed
---
xen/arch/arm/include/asm/arm64/mpu.h | 3 ++-
xen/arch/arm/include/asm/page.h | 5 +++++
xen/arch/arm/mm.c | 26
++++++++++++++++++++------
xen/arch/arm/mpu/mm.c | 1 +
xen/arch/arm/xen.lds.S | 5 ++++-
5 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/xen/arch/arm/include/asm/arm64/mpu.h
b/xen/arch/arm/include/asm/arm64/mpu.h
index a6b07bab02..715ea69884 100644
--- a/xen/arch/arm/include/asm/arm64/mpu.h
+++ b/xen/arch/arm/include/asm/arm64/mpu.h
@@ -72,7 +72,8 @@ typedef union {
unsigned long ns:1; /* Not-Secure */
unsigned long res:1; /* Reserved 0 by hardware */
unsigned long limit:42; /* Limit Address */
- unsigned long pad:16;
+ unsigned long pad:15;
+ unsigned long tran:1; /* Transient region */
} reg;
uint64_t bits;
} prlar_t;
diff --git a/xen/arch/arm/include/asm/page.h
b/xen/arch/arm/include/asm/page.h
index 85ecd5e4de..a434e2205a 100644
--- a/xen/arch/arm/include/asm/page.h
+++ b/xen/arch/arm/include/asm/page.h
@@ -97,19 +97,24 @@
* [3:4] Execute Never
* [5:6] Access Permission
* [7] Region Present
+ * [8] Transient Region, e.g. MPU memory region is temproraily
+ * mapped for a short time
*/
#define _PAGE_AI_BIT 0
#define _PAGE_XN_BIT 3
#define _PAGE_AP_BIT 5
#define _PAGE_PRESENT_BIT 7
+#define _PAGE_TRANSIENT_BIT 8
I don't think this is related to MPU. At least when I look at the
bit representation of PRBAR_EL1/2,
This set of _PAGE_xxx flags aren't compliant with PRBAR_EL1/2
register map.
It is a flag passed to function map_pages_to_xen() to indicate memory
attributes and permission.
But aren't you writing these flags to PRBAR_EL1/EL2 when you call
xen_mpumap_update_entry().
In the below snippet of xen_mpumap_update_entry(), IIUC, you are
writing these flags.
xen_mpumap[idx].prbar.reg.ap = PAGE_AP_MASK(flags);
xen_mpumap[idx].prbar.reg.xn = PAGE_XN_MASK(flags);
write_protection_region((const pr_t*)(&xen_mpumap[idx]),
idx);
Please clarify here.
In this case, I don't prefer mixing hardware specific bits with
software only representation for these reasons :-
1. It makes it confusing and hard to differentiate the hardware
specific attrbutes from software only.
Penny's approach matches what we are doing in the MMU code. We want
to have a way for the caller to pass just set of flags and let the
callee to decide what to do with them.
This may be flags converted for HW fields or just used by the logic.
If you disagree with this approach, then can you propose a different
way that we can discuss?
Thanks ayan for pointing out that RES0 is not suitable for storing
software-only flags, agreed.
Then, maybe we should refine the existing "struct pr_t" to store these
sw bits, like:
```
typedef union {
struct {
uint8_t tran:1; /* Transient region */
uint8_t p2m_type:4; /* Used to store p2m types.*/
Why do you need the p2m_type?
};
uint8_t bits;
} pr_flags;
/* MPU Protection Region */
typedef struct {
prbar_t prbar;
prlar_t prlar;
pr_flags flags;
} pr_t;
```
The drawback is that, considering the padding, "struct pr_t" expands
from 16 bytes to 24 bytes.
For clarifications, pr_t is going to be used to create an array in Xen,
right? If so, what's the expected size of the array?
Cheers,
--
Julien Grall
|
