diff mbox series

[v2,3/7] xen/arm: Introduce frame_table and virt_to_page

Message ID 20250316192445.2376484-4-luca.fancellu@arm.com (mailing list archive)
State New
Headers show
Series MPU mm subsystem skeleton | expand

Commit Message

Luca Fancellu March 16, 2025, 7:24 p.m. UTC
Introduce frame_table in order to provide the implementation of
virt_to_page for MPU system, move the MMU variant in mmu/mm.h.

Introduce FRAMETABLE_NR that is required for 'pdx_group_valid' in
pdx.c, but leave the initialisation of the frame table to a later
stage.
Define FRAMETABLE_SIZE for MPU to support up to 1TB of ram, as the
only current implementation of armv8-r aarch64, which is cortex R82,
can address up to that memory.

Take the occasion to sort alphabetically the headers following
the Xen code style and add the emacs footer in mpu/mm.c.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
v2 changes:
 - sorted headers in mm.c
 - modified commit message
 - moved virt_to_page to MMU and MPU
 - removed frametable_pdx_end, used mfn_valid
---
 xen/arch/arm/include/asm/mm.h         | 14 --------------
 xen/arch/arm/include/asm/mmu/mm.h     | 14 ++++++++++++++
 xen/arch/arm/include/asm/mpu/layout.h |  3 +++
 xen/arch/arm/include/asm/mpu/mm.h     | 15 +++++++++++++++
 xen/arch/arm/mpu/mm.c                 | 14 +++++++++++++-
 5 files changed, 45 insertions(+), 15 deletions(-)

Comments

Orzel, Michal March 17, 2025, 9:29 a.m. UTC | #1
On 16/03/2025 20:24, Luca Fancellu wrote:
> 
> 
> Introduce frame_table in order to provide the implementation of
> virt_to_page for MPU system, move the MMU variant in mmu/mm.h.
> 
> Introduce FRAMETABLE_NR that is required for 'pdx_group_valid' in
> pdx.c, but leave the initialisation of the frame table to a later
> stage.
> Define FRAMETABLE_SIZE for MPU to support up to 1TB of ram, as the
> only current implementation of armv8-r aarch64, which is cortex R82,
> can address up to that memory.
When mentioning support statements like this one, it'd be beneficial to provide
a reference to a doc of some sort.

Also, shouldn't this be occasion to clarify SUPPORT statement as for max RAM
supported for ARMv8R-AArch64? ARMv8R support is experimental, so I'm not 100%
sure if we need to provide support statement for it at this stage though. Better
check with others.

> 
> Take the occasion to sort alphabetically the headers following
> the Xen code style and add the emacs footer in mpu/mm.c.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> ---
> v2 changes:
>  - sorted headers in mm.c
>  - modified commit message
>  - moved virt_to_page to MMU and MPU
>  - removed frametable_pdx_end, used mfn_valid
> ---
>  xen/arch/arm/include/asm/mm.h         | 14 --------------
>  xen/arch/arm/include/asm/mmu/mm.h     | 14 ++++++++++++++
>  xen/arch/arm/include/asm/mpu/layout.h |  3 +++
>  xen/arch/arm/include/asm/mpu/mm.h     | 15 +++++++++++++++
>  xen/arch/arm/mpu/mm.c                 | 14 +++++++++++++-
>  5 files changed, 45 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index 444fd03823ec..fbffaccef49b 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -294,20 +294,6 @@ static inline uint64_t gvirt_to_maddr(vaddr_t va, paddr_t *pa,
>  #error "Unknown memory management layout"
>  #endif
> 
> -/* Convert between Xen-heap virtual addresses and page-info structures. */
> -static inline struct page_info *virt_to_page(const void *v)
> -{
> -    unsigned long va = (unsigned long)v;
> -    unsigned long pdx;
> -
> -    ASSERT(va >= XENHEAP_VIRT_START);
> -    ASSERT(va < directmap_virt_end);
> -
> -    pdx = (va - XENHEAP_VIRT_START) >> PAGE_SHIFT;
> -    pdx += mfn_to_pdx(directmap_mfn_start);
> -    return frame_table + pdx - frametable_base_pdx;
> -}
> -
>  static inline void *page_to_virt(const struct page_info *pg)
>  {
>      return mfn_to_virt(mfn_x(page_to_mfn(pg)));
> diff --git a/xen/arch/arm/include/asm/mmu/mm.h b/xen/arch/arm/include/asm/mmu/mm.h
> index 6737c3ede783..caba987edc85 100644
> --- a/xen/arch/arm/include/asm/mmu/mm.h
> +++ b/xen/arch/arm/include/asm/mmu/mm.h
> @@ -70,6 +70,20 @@ static inline void *maddr_to_virt(paddr_t ma)
>  }
>  #endif
> 
> +/* Convert between Xen-heap virtual addresses and page-info structures. */
> +static inline struct page_info *virt_to_page(const void *v)
> +{
> +    unsigned long va = (unsigned long)v;
> +    unsigned long pdx;
> +
> +    ASSERT(va >= XENHEAP_VIRT_START);
> +    ASSERT(va < directmap_virt_end);
> +
> +    pdx = (va - XENHEAP_VIRT_START) >> PAGE_SHIFT;
> +    pdx += mfn_to_pdx(directmap_mfn_start);
> +    return frame_table + pdx - frametable_base_pdx;
> +}
> +
>  /*
>   * Print a walk of a page table or p2m
>   *
> diff --git a/xen/arch/arm/include/asm/mpu/layout.h b/xen/arch/arm/include/asm/mpu/layout.h
> index 248e55f8882d..c331d1feaa84 100644
> --- a/xen/arch/arm/include/asm/mpu/layout.h
> +++ b/xen/arch/arm/include/asm/mpu/layout.h
> @@ -3,6 +3,9 @@
>  #ifndef __ARM_MPU_LAYOUT_H__
>  #define __ARM_MPU_LAYOUT_H__
> 
> +#define FRAMETABLE_SIZE   GB(16)
> +#define FRAMETABLE_NR     (FRAMETABLE_SIZE / sizeof(*frame_table))
> +
>  #define XEN_START_ADDRESS CONFIG_XEN_START_ADDRESS
> 
>  /*
> diff --git a/xen/arch/arm/include/asm/mpu/mm.h b/xen/arch/arm/include/asm/mpu/mm.h
> index 6cfd0f5cd2c2..3a0a60dbfa18 100644
> --- a/xen/arch/arm/include/asm/mpu/mm.h
> +++ b/xen/arch/arm/include/asm/mpu/mm.h
> @@ -3,9 +3,13 @@
>  #ifndef __ARM_MPU_MM_H__
>  #define __ARM_MPU_MM_H__
> 
> +#include <xen/bug.h>
>  #include <xen/macros.h>
>  #include <xen/page-size.h>
>  #include <xen/types.h>
> +#include <asm/mm.h>
> +
> +extern struct page_info *frame_table;
> 
>  #define virt_to_maddr(va) ((paddr_t)((vaddr_t)(va) & PADDR_MASK))
> 
> @@ -15,6 +19,17 @@ static inline void *maddr_to_virt(paddr_t ma)
>      return _p(ma);
>  }
> 
> +/* Convert between virtual address to page-info structure. */
> +static inline struct page_info *virt_to_page(const void *v)
> +{
> +    paddr_t paddr = virt_to_maddr(v);
> +    unsigned long pdx = paddr_to_pdx(paddr);
> +
> +    ASSERT(mfn_valid(maddr_to_mfn(paddr)));
> +
> +    return frame_table + pdx - frametable_base_pdx;
> +}
This could be simplified (and number of conversions reduced) by doing sth like:
mfn_t mfn = virt_to_mfn(v);

ASSERT(mfn_valid(mfn));

return mfn_to_page(mfn);

Other than that:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal
Luca Fancellu March 17, 2025, 9:33 a.m. UTC | #2
Hi Michal,

> On 17 Mar 2025, at 09:29, Orzel, Michal <michal.orzel@amd.com> wrote:
> 
> 
> 
> On 16/03/2025 20:24, Luca Fancellu wrote:
>> 
>> 
>> Introduce frame_table in order to provide the implementation of
>> virt_to_page for MPU system, move the MMU variant in mmu/mm.h.
>> 
>> Introduce FRAMETABLE_NR that is required for 'pdx_group_valid' in
>> pdx.c, but leave the initialisation of the frame table to a later
>> stage.
>> Define FRAMETABLE_SIZE for MPU to support up to 1TB of ram, as the
>> only current implementation of armv8-r aarch64, which is cortex R82,
>> can address up to that memory.
> When mentioning support statements like this one, it'd be beneficial to provide
> a reference to a doc of some sort.

So the only reference I have is this: https://developer.arm.com/Processors/Cortex-R82

but I would not be confident to use the link in the commit message as it could go stale
very quickly. So I’m not sure about what I can do more.

> 
> Also, shouldn't this be occasion to clarify SUPPORT statement as for max RAM
> supported for ARMv8R-AArch64? ARMv8R support is experimental, so I'm not 100%
> sure if we need to provide support statement for it at this stage though. Better
> check with others.
> 

Ok, I’ll stay tuned for the opinion of the others


>> 
>> diff --git a/xen/arch/arm/include/asm/mpu/mm.h b/xen/arch/arm/include/asm/mpu/mm.h
>> index 6cfd0f5cd2c2..3a0a60dbfa18 100644
>> --- a/xen/arch/arm/include/asm/mpu/mm.h
>> +++ b/xen/arch/arm/include/asm/mpu/mm.h
>> @@ -3,9 +3,13 @@
>> #ifndef __ARM_MPU_MM_H__
>> #define __ARM_MPU_MM_H__
>> 
>> +#include <xen/bug.h>
>> #include <xen/macros.h>
>> #include <xen/page-size.h>
>> #include <xen/types.h>
>> +#include <asm/mm.h>
>> +
>> +extern struct page_info *frame_table;
>> 
>> #define virt_to_maddr(va) ((paddr_t)((vaddr_t)(va) & PADDR_MASK))
>> 
>> @@ -15,6 +19,17 @@ static inline void *maddr_to_virt(paddr_t ma)
>>     return _p(ma);
>> }
>> 
>> +/* Convert between virtual address to page-info structure. */
>> +static inline struct page_info *virt_to_page(const void *v)
>> +{
>> +    paddr_t paddr = virt_to_maddr(v);
>> +    unsigned long pdx = paddr_to_pdx(paddr);
>> +
>> +    ASSERT(mfn_valid(maddr_to_mfn(paddr)));
>> +
>> +    return frame_table + pdx - frametable_base_pdx;
>> +}
> This could be simplified (and number of conversions reduced) by doing sth like:
> mfn_t mfn = virt_to_mfn(v);
> 
> ASSERT(mfn_valid(mfn));
> 
> return mfn_to_page(mfn);

Right, I’ll test these and I’ll use them.


> 
> Other than that:
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Thanks!

> 
> ~Michal
>
Orzel, Michal March 17, 2025, 9:51 a.m. UTC | #3
On 17/03/2025 10:33, Luca Fancellu wrote:
> 
> 
> Hi Michal,
> 
>> On 17 Mar 2025, at 09:29, Orzel, Michal <michal.orzel@amd.com> wrote:
>>
>>
>>
>> On 16/03/2025 20:24, Luca Fancellu wrote:
>>>
>>>
>>> Introduce frame_table in order to provide the implementation of
>>> virt_to_page for MPU system, move the MMU variant in mmu/mm.h.
>>>
>>> Introduce FRAMETABLE_NR that is required for 'pdx_group_valid' in
>>> pdx.c, but leave the initialisation of the frame table to a later
>>> stage.
>>> Define FRAMETABLE_SIZE for MPU to support up to 1TB of ram, as the
>>> only current implementation of armv8-r aarch64, which is cortex R82,
>>> can address up to that memory.
>> When mentioning support statements like this one, it'd be beneficial to provide
>> a reference to a doc of some sort.
> 
> So the only reference I have is this: https://developer.arm.com/Processors/Cortex-R82
> 
> but I would not be confident to use the link in the commit message as it could go stale
> very quickly. So I’m not sure about what I can do more.
Well, not really. Max physical memory is advertised via ID_AA64MMFR0_EL1. I
found some old R82 technical manual (you can surely find the latest one and
provide reference to it - not the web page) and indeed it mentions PARange as
0b0010 which is 40bit which is 1TB. With the R82 being the only CPU model
implementing ARMv8R-AArch64, that's solid information.

~Michal
Luca Fancellu March 17, 2025, 9:53 a.m. UTC | #4
> On 17 Mar 2025, at 09:51, Orzel, Michal <michal.orzel@amd.com> wrote:
> 
> 
> 
> On 17/03/2025 10:33, Luca Fancellu wrote:
>> 
>> 
>> Hi Michal,
>> 
>>> On 17 Mar 2025, at 09:29, Orzel, Michal <michal.orzel@amd.com> wrote:
>>> 
>>> 
>>> 
>>> On 16/03/2025 20:24, Luca Fancellu wrote:
>>>> 
>>>> 
>>>> Introduce frame_table in order to provide the implementation of
>>>> virt_to_page for MPU system, move the MMU variant in mmu/mm.h.
>>>> 
>>>> Introduce FRAMETABLE_NR that is required for 'pdx_group_valid' in
>>>> pdx.c, but leave the initialisation of the frame table to a later
>>>> stage.
>>>> Define FRAMETABLE_SIZE for MPU to support up to 1TB of ram, as the
>>>> only current implementation of armv8-r aarch64, which is cortex R82,
>>>> can address up to that memory.
>>> When mentioning support statements like this one, it'd be beneficial to provide
>>> a reference to a doc of some sort.
>> 
>> So the only reference I have is this: https://developer.arm.com/Processors/Cortex-R82
>> 
>> but I would not be confident to use the link in the commit message as it could go stale
>> very quickly. So I’m not sure about what I can do more.
> Well, not really. Max physical memory is advertised via ID_AA64MMFR0_EL1. I
> found some old R82 technical manual (you can surely find the latest one and
> provide reference to it - not the web page) and indeed it mentions PARange as
> 0b0010 which is 40bit which is 1TB. With the R82 being the only CPU model
> implementing ARMv8R-AArch64, that's solid information.

Right, I forgot about it, thanks for pointing that out, I’ll add a reference about it

Cheers,
Luca
diff mbox series

Patch

diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index 444fd03823ec..fbffaccef49b 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -294,20 +294,6 @@  static inline uint64_t gvirt_to_maddr(vaddr_t va, paddr_t *pa,
 #error "Unknown memory management layout"
 #endif
 
-/* Convert between Xen-heap virtual addresses and page-info structures. */
-static inline struct page_info *virt_to_page(const void *v)
-{
-    unsigned long va = (unsigned long)v;
-    unsigned long pdx;
-
-    ASSERT(va >= XENHEAP_VIRT_START);
-    ASSERT(va < directmap_virt_end);
-
-    pdx = (va - XENHEAP_VIRT_START) >> PAGE_SHIFT;
-    pdx += mfn_to_pdx(directmap_mfn_start);
-    return frame_table + pdx - frametable_base_pdx;
-}
-
 static inline void *page_to_virt(const struct page_info *pg)
 {
     return mfn_to_virt(mfn_x(page_to_mfn(pg)));
diff --git a/xen/arch/arm/include/asm/mmu/mm.h b/xen/arch/arm/include/asm/mmu/mm.h
index 6737c3ede783..caba987edc85 100644
--- a/xen/arch/arm/include/asm/mmu/mm.h
+++ b/xen/arch/arm/include/asm/mmu/mm.h
@@ -70,6 +70,20 @@  static inline void *maddr_to_virt(paddr_t ma)
 }
 #endif
 
+/* Convert between Xen-heap virtual addresses and page-info structures. */
+static inline struct page_info *virt_to_page(const void *v)
+{
+    unsigned long va = (unsigned long)v;
+    unsigned long pdx;
+
+    ASSERT(va >= XENHEAP_VIRT_START);
+    ASSERT(va < directmap_virt_end);
+
+    pdx = (va - XENHEAP_VIRT_START) >> PAGE_SHIFT;
+    pdx += mfn_to_pdx(directmap_mfn_start);
+    return frame_table + pdx - frametable_base_pdx;
+}
+
 /*
  * Print a walk of a page table or p2m
  *
diff --git a/xen/arch/arm/include/asm/mpu/layout.h b/xen/arch/arm/include/asm/mpu/layout.h
index 248e55f8882d..c331d1feaa84 100644
--- a/xen/arch/arm/include/asm/mpu/layout.h
+++ b/xen/arch/arm/include/asm/mpu/layout.h
@@ -3,6 +3,9 @@ 
 #ifndef __ARM_MPU_LAYOUT_H__
 #define __ARM_MPU_LAYOUT_H__
 
+#define FRAMETABLE_SIZE   GB(16)
+#define FRAMETABLE_NR     (FRAMETABLE_SIZE / sizeof(*frame_table))
+
 #define XEN_START_ADDRESS CONFIG_XEN_START_ADDRESS
 
 /*
diff --git a/xen/arch/arm/include/asm/mpu/mm.h b/xen/arch/arm/include/asm/mpu/mm.h
index 6cfd0f5cd2c2..3a0a60dbfa18 100644
--- a/xen/arch/arm/include/asm/mpu/mm.h
+++ b/xen/arch/arm/include/asm/mpu/mm.h
@@ -3,9 +3,13 @@ 
 #ifndef __ARM_MPU_MM_H__
 #define __ARM_MPU_MM_H__
 
+#include <xen/bug.h>
 #include <xen/macros.h>
 #include <xen/page-size.h>
 #include <xen/types.h>
+#include <asm/mm.h>
+
+extern struct page_info *frame_table;
 
 #define virt_to_maddr(va) ((paddr_t)((vaddr_t)(va) & PADDR_MASK))
 
@@ -15,6 +19,17 @@  static inline void *maddr_to_virt(paddr_t ma)
     return _p(ma);
 }
 
+/* Convert between virtual address to page-info structure. */
+static inline struct page_info *virt_to_page(const void *v)
+{
+    paddr_t paddr = virt_to_maddr(v);
+    unsigned long pdx = paddr_to_pdx(paddr);
+
+    ASSERT(mfn_valid(maddr_to_mfn(paddr)));
+
+    return frame_table + pdx - frametable_base_pdx;
+}
+
 #endif /* __ARM_MPU_MM_H__ */
 
 /*
diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
index 0b8748e57598..3632011c1013 100644
--- a/xen/arch/arm/mpu/mm.c
+++ b/xen/arch/arm/mpu/mm.c
@@ -1,9 +1,12 @@ 
 /* SPDX-License-Identifier: GPL-2.0-only */
 
-#include <xen/lib.h>
 #include <xen/init.h>
+#include <xen/lib.h>
+#include <xen/mm.h>
 #include <xen/sizes.h>
 
+struct page_info *frame_table;
+
 static void __init __maybe_unused build_assertions(void)
 {
     /*
@@ -13,3 +16,12 @@  static void __init __maybe_unused build_assertions(void)
      */
     BUILD_BUG_ON(PAGE_SIZE != SZ_4K);
 }
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */