diff mbox series

[v1,2/4] xen/arm: mpu: Define Xen start address for MPU systems

Message ID 20240823163127.3443404-3-ayan.kumar.halder@amd.com (mailing list archive)
State New
Headers show
Series Enable early bootup of AArch64 MPU systems. | expand

Commit Message

Ayan Kumar Halder Aug. 23, 2024, 4:31 p.m. UTC
From: Wei Chen <wei.chen@arm.com>

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 in this patch, we reuse the existing arm/platforms to store
Armv8-R platforms' parameters. And `XEN_START_ADDRESS` is one
kind of MPU based platform's parameters. So we define default
`XEN_START_ADDRESS` in the MPU specific header file.

We also introduce one Kconfig option for users to override the
default Xen start address of selected platform, if they think
the default address doesn't suit their scenarios. For this
Kconfig option, we use an unaligned address "0xffffffff" as the
default value to indicate that users haven't used a customized
Xen start address.

Signed-off-by: Wei Chen <wei.chen@arm.com>
Signed-off-by: Jiamei.Xie <jiamei.xie@arm.com>
Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---
 xen/arch/arm/Kconfig                  | 11 +++++++++++
 xen/arch/arm/include/asm/mpu/layout.h | 25 +++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
 create mode 100644 xen/arch/arm/include/asm/mpu/layout.h

Comments

Julien Grall Aug. 28, 2024, 2:49 p.m. UTC | #1
Hi,

On 23/08/2024 17:31, Ayan Kumar Halder wrote:
> From: Wei Chen <wei.chen@arm.com>
> 
> 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 in this patch, we reuse the existing arm/platforms to store
> Armv8-R platforms' parameters. And `XEN_START_ADDRESS` is one
> kind of MPU based platform's parameters. So we define default
> `XEN_START_ADDRESS` in the MPU specific header file.

This seems a left-over from as previous work?

> 
> We also introduce one Kconfig option for users to override the
> default Xen start address of selected platform, if they think
> the default address doesn't suit their scenarios. For this
> Kconfig option, we use an unaligned address "0xffffffff" as the
> default value to indicate that users haven't used a customized
> Xen start address.
> 
> Signed-off-by: Wei Chen <wei.chen@arm.com>
> Signed-off-by: Jiamei.Xie <jiamei.xie@arm.com>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> ---
>   xen/arch/arm/Kconfig                  | 11 +++++++++++
>   xen/arch/arm/include/asm/mpu/layout.h | 25 +++++++++++++++++++++++++
>   2 files changed, 36 insertions(+)
>   create mode 100644 xen/arch/arm/include/asm/mpu/layout.h
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 686948cefd..d722ca690c 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -23,6 +23,17 @@ config ARCH_DEFCONFIG
>   	default "arch/arm/configs/arm32_defconfig" if ARM_32
>   	default "arch/arm/configs/arm64_defconfig" if ARM_64
>   
> +config XEN_START_ADDRESS
> +	hex "Xen start address: keep default to use platform defined address"
> +	default 0xFFFFFFFF
> +	depends on MPU
> +	help
> +	  This option is allowed to set customized address at which Xen will be
> +	  linked on MPU systems.

I don't quite understand what you mean by "allowed".

 >  This address must be aligned to a page size.> +	  Use 0xFFFFFFFF as 
the default value to indicate that user hasn't
> +	  customized this address, and Xen use use the default value that has
> +	  been defined in platform files.

At least in this patch, there is no default set. Instead you will throw 
an error.

> +
>   menu "Architecture Features"
>   
>   choice
> diff --git a/xen/arch/arm/include/asm/mpu/layout.h b/xen/arch/arm/include/asm/mpu/layout.h
> new file mode 100644
> index 0000000000..82a74b8b5b
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/mpu/layout.h
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0-only

Coding style: /* SPDX-... */

> +/*
> + * config_mpu.h: A Linux-style configuration list for Arm MPU systems,
 > + *               only can be included by config.h
> + */

I don't understand how this description match the content below. Also, 
shouldn't you include it from xen/config.h?

> +
> +#ifndef __ARM_CONFIG_MPU_H__
> +#define __ARM_CONFIG_MPU_H__

This doesn't match the name of the file.

> +
> +
> +#define XEN_START_ADDRESS CONFIG_XEN_START_ADDRESS
> +
> +/*
> + * All MPU platforms need to provide a XEN_START_ADDRESS for linker. This
> + * address indicates where Xen image will be loaded and run from. This
> + * address must be aligned to a PAGE_SIZE.
> + */
> +#if (XEN_START_ADDRESS % PAGE_SIZE) != 0
> +#error "XEN_START_ADDRESS must be aligned to PAGE_SIZE"
> +#endif
> +
> +#define XEN_VIRT_START         _AT(paddr_t, XEN_START_ADDRESS)
> +
> +#endif /* __ARM_CONFIG_MPU_H__ */
> +

Cheers,
Ayan Kumar Halder Sept. 2, 2024, 2:48 p.m. UTC | #2
On 28/08/2024 15:49, Julien Grall wrote:
> Hi,
Hi Julien,
>
> On 23/08/2024 17:31, Ayan Kumar Halder wrote:
>> From: Wei Chen <wei.chen@arm.com>
>>
>> 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 in this patch, we reuse the existing arm/platforms to store
>> Armv8-R platforms' parameters. And `XEN_START_ADDRESS` is one
>> kind of MPU based platform's parameters. So we define default
>> `XEN_START_ADDRESS` in the MPU specific header file.
>
> This seems a left-over from as previous work?
Yes, this needs to be removed.
>
>>
>> We also introduce one Kconfig option for users to override the
>> default Xen start address of selected platform, if they think
>> the default address doesn't suit their scenarios. For this
>> Kconfig option, we use an unaligned address "0xffffffff" as the
>> default value to indicate that users haven't used a customized
>> Xen start address.
>>
>> Signed-off-by: Wei Chen <wei.chen@arm.com>
>> Signed-off-by: Jiamei.Xie <jiamei.xie@arm.com>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>> ---
>>   xen/arch/arm/Kconfig                  | 11 +++++++++++
>>   xen/arch/arm/include/asm/mpu/layout.h | 25 +++++++++++++++++++++++++
>>   2 files changed, 36 insertions(+)
>>   create mode 100644 xen/arch/arm/include/asm/mpu/layout.h
>>
>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>> index 686948cefd..d722ca690c 100644
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -23,6 +23,17 @@ config ARCH_DEFCONFIG
>>       default "arch/arm/configs/arm32_defconfig" if ARM_32
>>       default "arch/arm/configs/arm64_defconfig" if ARM_64
>>   +config XEN_START_ADDRESS
>> +    hex "Xen start address: keep default to use platform defined 
>> address"
>> +    default 0xFFFFFFFF
>> +    depends on MPU
>> +    help
>> +      This option is allowed to set customized address at which Xen 
>> will be
>> +      linked on MPU systems.
>
> I don't quite understand what you mean by "allowed".

I will rephrase this as ...

"Used to set customized address at which which Xen will be linked

on MPU systems. This address must be aligned to a page size.
0xFFFFFFFF is used as the default value to indicate that user hasn't
customized this address."

>
> >  This address must be aligned to a page size.> +      Use 0xFFFFFFFF 
> as the default value to indicate that user hasn't
>> +      customized this address, and Xen use use the default value 
>> that has
>> +      been defined in platform files.
>
> At least in this patch, there is no default set. Instead you will 
> throw an error.
Ack. See above.
>
>> +
>>   menu "Architecture Features"
>>     choice
>> diff --git a/xen/arch/arm/include/asm/mpu/layout.h 
>> b/xen/arch/arm/include/asm/mpu/layout.h
>> new file mode 100644
>> index 0000000000..82a74b8b5b
>> --- /dev/null
>> +++ b/xen/arch/arm/include/asm/mpu/layout.h
>> @@ -0,0 +1,25 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>
> Coding style: /* SPDX-... */
Ack
>
>> +/*
>> + * config_mpu.h: A Linux-style configuration list for Arm MPU systems,
> > + *               only can be included by config.h
>> + */
>
> I don't understand how this description match the content below. Also, 
> shouldn't you include it from xen/config.h?

My bad. This description is incorrect. I will remove the description as 
it is not needed at this point in time.

>
>> +
>> +#ifndef __ARM_CONFIG_MPU_H__
>> +#define __ARM_CONFIG_MPU_H__
>
> This doesn't match the name of the file.

__ARM_MPU_LAYOUT_H__

>
>> +
>> +
>> +#define XEN_START_ADDRESS CONFIG_XEN_START_ADDRESS
>> +
>> +/*
>> + * All MPU platforms need to provide a XEN_START_ADDRESS for linker. 
>> This
>> + * address indicates where Xen image will be loaded and run from. This
>> + * address must be aligned to a PAGE_SIZE.
>> + */
>> +#if (XEN_START_ADDRESS % PAGE_SIZE) != 0
>> +#error "XEN_START_ADDRESS must be aligned to PAGE_SIZE"
>> +#endif
>> +
>> +#define XEN_VIRT_START         _AT(paddr_t, XEN_START_ADDRESS)
>> +
>> +#endif /* __ARM_CONFIG_MPU_H__ */
>> +
>
> Cheers,
- Ayan
Julien Grall Sept. 8, 2024, 9:13 p.m. UTC | #3
Hi,

On 02/09/2024 15:48, Ayan Kumar Halder wrote:
> 
> On 28/08/2024 15:49, Julien Grall wrote:
>> Hi,
> Hi Julien,
>>
>> On 23/08/2024 17:31, Ayan Kumar Halder wrote:
>>> From: Wei Chen <wei.chen@arm.com>
>>>
>>> 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 in this patch, we reuse the existing arm/platforms to store
>>> Armv8-R platforms' parameters. And `XEN_START_ADDRESS` is one
>>> kind of MPU based platform's parameters. So we define default
>>> `XEN_START_ADDRESS` in the MPU specific header file.
>>
>> This seems a left-over from as previous work?
> Yes, this needs to be removed.
>>
>>>
>>> We also introduce one Kconfig option for users to override the
>>> default Xen start address of selected platform, if they think
>>> the default address doesn't suit their scenarios. For this
>>> Kconfig option, we use an unaligned address "0xffffffff" as the
>>> default value to indicate that users haven't used a customized
>>> Xen start address.
>>>
>>> Signed-off-by: Wei Chen <wei.chen@arm.com>
>>> Signed-off-by: Jiamei.Xie <jiamei.xie@arm.com>
>>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
>>> ---
>>>   xen/arch/arm/Kconfig                  | 11 +++++++++++
>>>   xen/arch/arm/include/asm/mpu/layout.h | 25 +++++++++++++++++++++++++
>>>   2 files changed, 36 insertions(+)
>>>   create mode 100644 xen/arch/arm/include/asm/mpu/layout.h
>>>
>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>> index 686948cefd..d722ca690c 100644
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -23,6 +23,17 @@ config ARCH_DEFCONFIG
>>>       default "arch/arm/configs/arm32_defconfig" if ARM_32
>>>       default "arch/arm/configs/arm64_defconfig" if ARM_64
>>>   +config XEN_START_ADDRESS
>>> +    hex "Xen start address: keep default to use platform defined 
>>> address"
>>> +    default 0xFFFFFFFF
>>> +    depends on MPU
>>> +    help
>>> +      This option is allowed to set customized address at which Xen 
>>> will be
>>> +      linked on MPU systems.
>>
>> I don't quite understand what you mean by "allowed".
> 
> I will rephrase this as ...
> 
> "Used to set customized address at which which Xen will be linked
> 
> on MPU systems. This address must be aligned to a page size.
> 0xFFFFFFFF is used as the default value to indicate that user hasn't
> customized this address."

Reading this comment, I would like to ask some clarification. In the 
context of the MPU how do you define a page size? The definition is 
pretty clear when using the MMU because the granularity if defined by 
the HW. But for the MPU, it is a bit blur. Is it still 4KB? If so, is it 
actually realistic (we don't have that many MPU regions)?

Cheers,
Ayan Kumar Halder Sept. 9, 2024, 10:29 a.m. UTC | #4
On 08/09/2024 22:13, Julien Grall wrote:
> Hi,
Hi Julien,
>
> On 02/09/2024 15:48, Ayan Kumar Halder wrote:

>> I will rephrase this as ...
>>
>> "Used to set customized address at which which Xen will be linked
>>
>> on MPU systems. This address must be aligned to a page size.
>> 0xFFFFFFFF is used as the default value to indicate that user hasn't
>> customized this address."
>
> Reading this comment, I would like to ask some clarification. In the 
> context of the MPU how do you define a page size? The definition is 
> pretty clear when using the MMU because the granularity if defined by 
> the HW. But for the MPU, it is a bit blur. Is it still 4KB? If so, is 
> it actually realistic (we don't have that many MPU regions)?

 From ARM DDI 0600A.d ID120821, C1.1.1 Protection regions

"Protection regions have a minimum size of 64 bytes."

Thus, I would infer that the minimum page size (in context of MPU) is 64 
bytes.

Also, if you see the register fields of PRBAR and PRLAR, the lower 6 
bits are 0 extended to provide the address.

So, may be I should say

".... address must be aligned to the minimum region size (ie 64 bytes). 
0xFFFFFFFF is used ...."


Let me know if this sounds ok.

- Ayan
Julien Grall Sept. 9, 2024, 2:45 p.m. UTC | #5
Hi Ayan,

On 09/09/2024 11:29, Ayan Kumar Halder wrote:
> 
> On 08/09/2024 22:13, Julien Grall wrote:
>> Hi,
> Hi Julien,
>>
>> On 02/09/2024 15:48, Ayan Kumar Halder wrote:
> 
>>> I will rephrase this as ...
>>>
>>> "Used to set customized address at which which Xen will be linked
>>>
>>> on MPU systems. This address must be aligned to a page size.
>>> 0xFFFFFFFF is used as the default value to indicate that user hasn't
>>> customized this address."
>>
>> Reading this comment, I would like to ask some clarification. In the 
>> context of the MPU how do you define a page size? The definition is 
>> pretty clear when using the MMU because the granularity if defined by 
>> the HW. But for the MPU, it is a bit blur. Is it still 4KB? If so, is 
>> it actually realistic (we don't have that many MPU regions)?
> 
>  From ARM DDI 0600A.d ID120821, C1.1.1 Protection regions
> 
> "Protection regions have a minimum size of 64 bytes."
> 
> Thus, I would infer that the minimum page size (in context of MPU) is 64 
> bytes.
 > > Also, if you see the register fields of PRBAR and PRLAR, the lower 6
> bits are 0 extended to provide the address.
> 
> So, may be I should say
> 
> ".... address must be aligned to the minimum region size (ie 64 bytes). 
> 0xFFFFFFFF is used ...."
> 
> 
> Let me know if this sounds ok.

So what you provided is a minimum size/alignment of a region from the HW 
point of view. How about Xen? Will it be able to cope if Xen is booted 
at a 64-byte alignment?

Asking because it is unclear how the allocator will work with the MPU. 
Are we going to continue to allocate 4KB chunk at the time? Will it be 
smaller/larger?

Cheers,
Ayan Kumar Halder Sept. 10, 2024, 1:42 p.m. UTC | #6
On 09/09/2024 15:45, Julien Grall wrote:
> Hi Ayan,
Hi Julien,
>
> On 09/09/2024 11:29, Ayan Kumar Halder wrote:
>>
>> On 08/09/2024 22:13, Julien Grall wrote:
>>> Hi,
>> Hi Julien,
>>>
>>> On 02/09/2024 15:48, Ayan Kumar Halder wrote:
>>
>>>> I will rephrase this as ...
>>>>
>>>> "Used to set customized address at which which Xen will be linked
>>>>
>>>> on MPU systems. This address must be aligned to a page size.
>>>> 0xFFFFFFFF is used as the default value to indicate that user hasn't
>>>> customized this address."
>>>
>>> Reading this comment, I would like to ask some clarification. In the 
>>> context of the MPU how do you define a page size? The definition is 
>>> pretty clear when using the MMU because the granularity if defined 
>>> by the HW. But for the MPU, it is a bit blur. Is it still 4KB? If 
>>> so, is it actually realistic (we don't have that many MPU regions)?
>>
>>  From ARM DDI 0600A.d ID120821, C1.1.1 Protection regions
>>
>> "Protection regions have a minimum size of 64 bytes."
>>
>> Thus, I would infer that the minimum page size (in context of MPU) is 
>> 64 bytes.
> > > Also, if you see the register fields of PRBAR and PRLAR, the lower 6
yes.
>> bits are 0 extended to provide the address.
>>
>> So, may be I should say
>>
>> ".... address must be aligned to the minimum region size (ie 64 
>> bytes). 0xFFFFFFFF is used ...."
>>
>>
>> Let me know if this sounds ok.
>
> So what you provided is a minimum size/alignment of a region from the 
> HW point of view. How about Xen? Will it be able to cope if Xen is 
> booted at a 64-byte alignment?
>
> Asking because it is unclear how the allocator will work with the MPU. 
> Are we going to continue to allocate 4KB chunk at the time? Will it be 
> smaller/larger?

MPU will work with static memory allocation only. It can allocate a 
fixed number of regions using PRBAR and PRLAR. We can make the addresses 
to be page aligned (ie 4KB) so that it is in parity with MMU. And we 
will keep the page size as 4 KB.  It makes sense to use the same page 
size to do the allocation irrespective of MPU or MMU. Also, it helps to 
keep a large part of the code as common.

However for MPU, we will calculate the limit address based on the number 
of pages requested and do the allocation. For eg in map_pages_to_xen(), 
we will invoke a function to calculate the limit address based on the 
base address and nr_mfns.

Let me know if this is clear.

- Ayan
Julien Grall Sept. 10, 2024, 9:34 p.m. UTC | #7
Hi Ayan,

On 10/09/2024 14:42, Ayan Kumar Halder wrote:
> 
> On 09/09/2024 15:45, Julien Grall wrote:
>> Hi Ayan,
> Hi Julien,
>>
>> On 09/09/2024 11:29, Ayan Kumar Halder wrote:
>>>
>>> On 08/09/2024 22:13, Julien Grall wrote:
>>>> Hi,
>>> Hi Julien,
>>>>
>>>> On 02/09/2024 15:48, Ayan Kumar Halder wrote:
>>>
>>>>> I will rephrase this as ...
>>>>>
>>>>> "Used to set customized address at which which Xen will be linked
>>>>>
>>>>> on MPU systems. This address must be aligned to a page size.
>>>>> 0xFFFFFFFF is used as the default value to indicate that user hasn't
>>>>> customized this address."
>>>>
>>>> Reading this comment, I would like to ask some clarification. In the 
>>>> context of the MPU how do you define a page size? The definition is 
>>>> pretty clear when using the MMU because the granularity if defined 
>>>> by the HW. But for the MPU, it is a bit blur. Is it still 4KB? If 
>>>> so, is it actually realistic (we don't have that many MPU regions)?
>>>
>>>  From ARM DDI 0600A.d ID120821, C1.1.1 Protection regions
>>>
>>> "Protection regions have a minimum size of 64 bytes."
>>>
>>> Thus, I would infer that the minimum page size (in context of MPU) is 
>>> 64 bytes.
>> > > Also, if you see the register fields of PRBAR and PRLAR, the lower 6
> yes.
>>> bits are 0 extended to provide the address.
>>>
>>> So, may be I should say
>>>
>>> ".... address must be aligned to the minimum region size (ie 64 
>>> bytes). 0xFFFFFFFF is used ...."
>>>
>>>
>>> Let me know if this sounds ok.
>>
>> So what you provided is a minimum size/alignment of a region from the 
>> HW point of view. How about Xen? Will it be able to cope if Xen is 
>> booted at a 64-byte alignment?
>>
>> Asking because it is unclear how the allocator will work with the MPU. 
>> Are we going to continue to allocate 4KB chunk at the time? Will it be 
>> smaller/larger?
> 
> MPU will work with static memory allocation only. It can allocate a 
> fixed number of regions using PRBAR and PRLAR. We can make the addresses 
> to be page aligned (ie 4KB) so that it is in parity with MMU. And we 
> will keep the page size as 4 KB.  It makes sense to use the same page 
> size to do the allocation irrespective of MPU or MMU. Also, it helps to 
> keep a large part of the code as common.

TL;DR: I think we can use different chunk size (I find "page" confusing 
in the context of the MPU) and still sharing a large part of Xen. For 
now, I would be ok to use the same size. In the future, it would be 
beneficial to change it.

Now the longer answer. Looking at the specification, the architecture 
allows up to 128 regions. So the chance we will want to share a 4KB RAM 
region at the time is close to zero.

Yet we would still have one struct page_info per 4KB chunk to carry the 
metadata. This is not only going to be a waste of memory but also 
requires extra work. So to me it would make more sense to track larger 
chunk (maybe 2MB).

Now regarding the change necessary in Xen. I think we can divide it in 
two parts:
   * Allocation within region
   * Frame in the public interface

For the first part, Xen on Arm is now mainly page agnostic. So you could 
bump the PAGE_SIZE (barring the public interface see below) without any 
trouble. You may have to deal with extra memory wasted when using 
alloc_*heap_page*(). But that could be easily solved by using xmalloc() 
and/or introduce new wrappers.

For the frame definition in the public interface. So far, a frame covers 
4KB (same as Xen page granularity) and this is bake into OSes (Xen 
doesn't have a way to return the page granularity used). So for the MMU, 
you sadly can't easily increase the page granularity used by Xen...

However for the MPU I think this is a different story. The bulk of the 
interface using frame number (such as increase/decrease memory) are not 
applicable for the MPU or would be unusable/severely limited (e.g grant 
table) given the number of MPU regions available.

Anyway, I am probably thinking quite far ahead :). For now, I am fine if 
you want to use 4KB chunk. So in the Kconfig you could write "Xen start 
address should be 4KB aligned" (let's not mention page).

Cheers,
Ayan Kumar Halder Sept. 11, 2024, 9:53 a.m. UTC | #8
On 10/09/2024 22:34, Julien Grall wrote:
> Hi Ayan,
Hi Julien,
>
> On 10/09/2024 14:42, Ayan Kumar Halder wrote:
>>
>> On 09/09/2024 15:45, Julien Grall wrote:
>>> Hi Ayan,
>> Hi Julien,
>>>
>>> On 09/09/2024 11:29, Ayan Kumar Halder wrote:
>>>>
>>>> On 08/09/2024 22:13, Julien Grall wrote:
>>>>> Hi,
>>>> Hi Julien,
>>>>>
>>>>> On 02/09/2024 15:48, Ayan Kumar Halder wrote:
>>>>
>>>>>> I will rephrase this as ...
>>>>>>
>>>>>> "Used to set customized address at which which Xen will be linked
>>>>>>
>>>>>> on MPU systems. This address must be aligned to a page size.
>>>>>> 0xFFFFFFFF is used as the default value to indicate that user hasn't
>>>>>> customized this address."
>>>>>
>>>>> Reading this comment, I would like to ask some clarification. In 
>>>>> the context of the MPU how do you define a page size? The 
>>>>> definition is pretty clear when using the MMU because the 
>>>>> granularity if defined by the HW. But for the MPU, it is a bit 
>>>>> blur. Is it still 4KB? If so, is it actually realistic (we don't 
>>>>> have that many MPU regions)?
>>>>
>>>>  From ARM DDI 0600A.d ID120821, C1.1.1 Protection regions
>>>>
>>>> "Protection regions have a minimum size of 64 bytes."
>>>>
>>>> Thus, I would infer that the minimum page size (in context of MPU) 
>>>> is 64 bytes.
>>> > > Also, if you see the register fields of PRBAR and PRLAR, the 
>>> lower 6
>> yes.
>>>> bits are 0 extended to provide the address.
>>>>
>>>> So, may be I should say
>>>>
>>>> ".... address must be aligned to the minimum region size (ie 64 
>>>> bytes). 0xFFFFFFFF is used ...."
>>>>
>>>>
>>>> Let me know if this sounds ok.
>>>
>>> So what you provided is a minimum size/alignment of a region from 
>>> the HW point of view. How about Xen? Will it be able to cope if Xen 
>>> is booted at a 64-byte alignment?
>>>
>>> Asking because it is unclear how the allocator will work with the 
>>> MPU. Are we going to continue to allocate 4KB chunk at the time? 
>>> Will it be smaller/larger?
>>
>> MPU will work with static memory allocation only. It can allocate a 
>> fixed number of regions using PRBAR and PRLAR. We can make the 
>> addresses to be page aligned (ie 4KB) so that it is in parity with 
>> MMU. And we will keep the page size as 4 KB.  It makes sense to use 
>> the same page size to do the allocation irrespective of MPU or MMU. 
>> Also, it helps to keep a large part of the code as common.
>
> TL;DR: I think we can use different chunk size (I find "page" 
> confusing in the context of the MPU) and still sharing a large part of 
> Xen. For now, I would be ok to use the same size. In the future, it 
> would be beneficial to change it.
>
> Now the longer answer. Looking at the specification, the architecture 
> allows up to 128 regions. So the chance we will want to share a 4KB 
> RAM region at the time is close to zero.
>
> Yet we would still have one struct page_info per 4KB chunk to carry 
> the metadata. This is not only going to be a waste of memory but also 
> requires extra work. So to me it would make more sense to track larger 
> chunk (maybe 2MB).
>
> Now regarding the change necessary in Xen. I think we can divide it in 
> two parts:
>   * Allocation within region
>   * Frame in the public interface
>
> For the first part, Xen on Arm is now mainly page agnostic. So you 
> could bump the PAGE_SIZE (barring the public interface see below) 
> without any trouble. You may have to deal with extra memory wasted 
> when using alloc_*heap_page*(). But that could be easily solved by 
> using xmalloc() and/or introduce new wrappers.
>
> For the frame definition in the public interface. So far, a frame 
> covers 4KB (same as Xen page granularity) and this is bake into OSes 
> (Xen doesn't have a way to return the page granularity used). So for 
> the MMU, you sadly can't easily increase the page granularity used by 
> Xen...
>
> However for the MPU I think this is a different story. The bulk of the 
> interface using frame number (such as increase/decrease memory) are 
> not applicable for the MPU or would be unusable/severely limited (e.g 
> grant table) given the number of MPU regions available.
Thanks for the detailed explaination. :) Yes, the initial MPU support 
will be limited to booting of domains in dom0less mode. IOW, support for 
grant tables will not be a part of the initial set of patches.
>
> Anyway, I am probably thinking quite far ahead :). For now, I am fine 
> if you want to use 4KB chunk. So in the Kconfig you could write "Xen 
> start address should be 4KB aligned" (let's not mention page).

Yes, I will use these words. When we get Xen up and running on R52 and 
R82 (with the basic features), then we can revisit this.

- Ayan
diff mbox series

Patch

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 686948cefd..d722ca690c 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -23,6 +23,17 @@  config ARCH_DEFCONFIG
 	default "arch/arm/configs/arm32_defconfig" if ARM_32
 	default "arch/arm/configs/arm64_defconfig" if ARM_64
 
+config XEN_START_ADDRESS
+	hex "Xen start address: keep default to use platform defined address"
+	default 0xFFFFFFFF
+	depends on MPU
+	help
+	  This option is allowed to set customized address at which Xen will be
+	  linked on MPU systems. This address must be aligned to a page size.
+	  Use 0xFFFFFFFF as the default value to indicate that user hasn't
+	  customized this address, and Xen use use the default value that has
+	  been defined in platform files.
+
 menu "Architecture Features"
 
 choice
diff --git a/xen/arch/arm/include/asm/mpu/layout.h b/xen/arch/arm/include/asm/mpu/layout.h
new file mode 100644
index 0000000000..82a74b8b5b
--- /dev/null
+++ b/xen/arch/arm/include/asm/mpu/layout.h
@@ -0,0 +1,25 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * config_mpu.h: A Linux-style configuration list for Arm MPU systems,
+ *               only can be included by config.h
+ */
+
+#ifndef __ARM_CONFIG_MPU_H__
+#define __ARM_CONFIG_MPU_H__
+
+
+#define XEN_START_ADDRESS CONFIG_XEN_START_ADDRESS
+
+/*
+ * All MPU platforms need to provide a XEN_START_ADDRESS for linker. This
+ * address indicates where Xen image will be loaded and run from. This
+ * address must be aligned to a PAGE_SIZE.
+ */
+#if (XEN_START_ADDRESS % PAGE_SIZE) != 0
+#error "XEN_START_ADDRESS must be aligned to PAGE_SIZE"
+#endif
+
+#define XEN_VIRT_START         _AT(paddr_t, XEN_START_ADDRESS)
+
+#endif /* __ARM_CONFIG_MPU_H__ */
+