Hi Julien,
On 24/09/2024 17:44, Julien Grall wrote:
On 23/09/2024 12:22, Ayan Kumar Halder wrote:
On 19/09/2024 13:42, Julien Grall wrote:
Hi Ayan,
Hi Julien,
On 18/09/2024 19:51, Ayan Kumar Halder wrote:
From: Wei Chen <[email protected]>
On Armv8-A, Xen has a fixed virtual start address (link address
too) for all
Armv8-A platforms. In an MMU based system, Xen can map its loaded
address to
this virtual start address. So, on Armv8-A platforms, the Xen start
address does
not need to be configurable. But on Armv8-R platforms, there is no
MMU to map
loaded address to a fixed virtual address and different platforms
will have very
different address space layout. So Xen cannot use a fixed physical
address on
MPU based system and need to have it configurable.
So, we introduce a Kconfig option for users to set the start
address. The start
address needs to be aligned to 4KB. We have a check for this
alignment.
It would suggest to add some explanation why you want the start
address to be 4KB aligned.
Let me know if this makes sense.
"MPU allows us to define regions which are 64 bits aligned. This
restriction comes from the bitfields of PRBAR, PRLAR (the lower 6
bits are 0 extended to provide the base and limit address of a
region). This means that the start address of Xen needs to be at
least 64 bits aligned (as it will correspond to the start address of
memory protection region).
As for now Xen on MPU tries to use the same memory alignment
restrictions as it has been for MMU. Unlike MMU where the starting
virtual address is 2MB, Xen on MPU needs the start address to be 4 KB
(ie page size) aligned."
Thinking about this a bit more (and based on the discussion in "Re:
[PATCH v1 2/4] xen/arm: mpu: Define Xen start address for MPU
systems"), I think we can keep the 4 KB restriction for now and relax
it later. Let me know what you think.
I am fine with that.
In case if the user forgets to set the start address, then
0xffffffff is used
as default. This is to trigger the error (on alignment check) and
thereby prompt
user to set the start address.
Also updated config.h so that it includes mpu/layout.h when
CONFIG_MPU is
defined.
Signed-off-by: Wei Chen <[email protected]>
Signed-off-by: Jiamei.Xie <[email protected]>
Signed-off-by: Ayan Kumar Halder <[email protected]>
---
Changes from :-
v1 - 1. Fixed some of the coding style issues.
2. Reworded the help message.
3. Updated the commit message.
xen/arch/arm/Kconfig | 10 ++++++++++
xen/arch/arm/include/asm/config.h | 4 +++-
xen/arch/arm/include/asm/mpu/layout.h | 27
+++++++++++++++++++++++++++
Looking at this patch again, I don't see any modification in
xen.lds.S. Is it intended?
If we agree with the justification provided before, then this should
be the modification.
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -232,6 +232,12 @@ SECTIONS
*/
ASSERT(_start == XEN_VIRT_START, "_start != XEN_VIRT_START")
+/*
+ * On MPU based platforms, the starting address is to be provided by
user.
+ * One need to check that it is 4KB aligned.
+ */
+ASSERT(IS_ALIGNED(_start, 4096), "starting address is
misaligned")
+
Ah I wasn't asking to add an ASSERT (although it would be good so long
it is protected by CONFIG_MPU). Instead I was expecting XEN_VIRT_START
to be replaced. But looking at the code again, I see you define
XEN_VIRT_START for the MPU.
I think this is a little bit odd to use XEN_VIRT_* defined for the MPU
but it would be ok. Is the expectation that all the MPU code should be
using XEN_START_ADDRESS? If so, maybe it would be worth add a comment
on top of XEN_VIRT_ADDRESS.
I have defined XEN_VIRT_START as XEN_START_ADDRESS , so that the code
continues to use XEN_VIRT_START (as it does for the MMU) and we avoid
introducing #if-defs.
Does this comment make sense ?
/*
* For MPU, XEN's virtual start address is same as the physical address.
* The reason being MPU treats VA == PA. IOW, it cannot map the physical
* address to a different fixed virtual address. So, the virtual start
* address is determined by the physical address at which Xen is loaded.
*/
#define XEN_VIRT_START _AT(paddr_t, XEN_START_ADDRESS)
- Ayan
Cheers,