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

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi Michal,

> On 17 Mar 2025, at 09:29, Orzel, Michal <michal.orzel@xxxxxxx> 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@xxxxxxx>

Thanks!

> 
> ~Michal
>