Re: [PATCH v2 09/11] xen/riscv: introduce p2m_gpa_bits

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.03.2026 18:02, Oleksii Kurochko wrote:
> On 3/30/26 5:34 PM, Jan Beulich wrote:
>> On 23.03.2026 17:29, Oleksii Kurochko wrote:
>>> p2m_gpa_bits is used by common/device-tree/domain-build.c thereby when
>>> CONFIG_DOMAIN_BUILD_HELPERS=y it is necessary to have p2m_gpa_bits properly
>>> defined as it is going to be used to find unused regions.
>>>
>>> Introduce default_gstage_mode to have ability to limit p2m_gpa_bits before
>>> p2m_init() is being called as it will be too late.
>>
>> This is a somewhat strange way of describing things. Of course you want to
>> establish globals before doing any per-domain setup.
> 
> Then I will drop that sentence now and avoid similar in the future.
> 
>>> Limit p2m_gpa_bits in guest_mm_init() as it could be that default G-stage
>>> MMU mode uses less VA wide bits than IOMMU,
>>
>> How does a VA come into play here?
> 
> It is what spec uses, for example:
>   Figure 108. Sv39x4 virtual address (guest physical address).

Note the difference between what you quote and what your sentence said:
You used VA entirely unqualified. Yes, please ...

> I can just use GPA.

... use GPA whenever you mean one. Using VA for two distinct purposes
is simply confusing. Even the qualifying by the mode is only of limited
help imo, as the casual reader may not be fluent in those modes and
their acronyms.

>> And what is "less VA wide bits"?
> 
> They could be configured to different modes: IOMMU lets say Sv39 and MMU 
> - Sv48, so IOMMU could work with 39-bit GPA, but MMU - with 48-bit GPAs.

I guessed as much, but this wants wording differently. E.g. "... uses
fewer GPA bits than ...".

>>> @@ -191,8 +209,13 @@ static void __init gstage_mode_detect(void)
>>>   
>>>   void __init guest_mm_init(void)
>>>   {
>>> +    unsigned int gpa_bits;
>>> +    unsigned int paging_levels = default_gstage_mode.paging_levels;
>>
>> Deriving a global from a default, when ...
>>
>>>       gstage_mode_detect();
>>>   
>>> +    ASSERT(default_gstage_mode.paging_levels <= 
>>> max_gstage_mode.paging_levels);
>>
>> ... the default isn't the maximum possible, isn't going to fly.
> 
> I didn't get you here.
> 
> If we want Xen uses Sv39 for G-stage, we want to limit guest's 56-bit 
> GPA to 39-bit GPA, but not the maximum supported by h/w mode for G-stage 
> mode.

I can only repeat what I thought I had got across already on an earlier
series of yours: What mode a guest is going to use is going to be a guest
property. The default mode therefore isn't the only mode that may be used
at runtime.

Jan