Re: [PATCH v2] xen/riscv: Increase XEN_VIRT_SIZE

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.04.2025 11:06, Oleksii Kurochko wrote:
> On 4/8/25 4:04 PM, Jan Beulich wrote:
>> On 08.04.2025 15:46, Oleksii Kurochko wrote:
>>> On 4/8/25 2:02 PM, Jan Beulich wrote:
>>>> On 08.04.2025 13:51, Oleksii Kurochko wrote:
>>>>> On 4/7/25 12:09 PM, Jan Beulich wrote:
>>>>>> On 04.04.2025 18:04, Oleksii Kurochko wrote:
>>>>>>> --- a/xen/arch/riscv/mm.c
>>>>>>> +++ b/xen/arch/riscv/mm.c
>>>>>>> @@ -31,20 +31,27 @@ unsigned long __ro_after_init phys_offset; /* = 
>>>>>>> load_start - XEN_VIRT_START */
>>>>>>>     #define LOAD_TO_LINK(addr) ((unsigned long)(addr) - phys_offset)
>>>>>>>     
>>>>>>>     /*
>>>>>>> - * It is expected that Xen won't be more then 2 MB.
>>>>>>> + * It is expected that Xen won't be more then XEN_VIRT_SIZE MB.
>>>>>>>      * The check in xen.lds.S guarantees that.
>>>>>>> - * At least 3 page tables (in case of Sv39 ) are needed to cover 2 MB.
>>>>>>> - * One for each page level table with PAGE_SIZE = 4 Kb.
>>>>>>>      *
>>>>>>> - * One L0 page table can cover 2 MB(512 entries of one page table * 
>>>>>>> PAGE_SIZE).
>>>>>>> + * Root page table is shared with the initial mapping and is declared
>>>>>>> + * separetely. (look at stage1_pgtbl_root)
>>>>>>>      *
>>>>>>> - * It might be needed one more page table in case when Xen load address
>>>>>>> - * isn't 2 MB aligned.
>>>>>>> + * An amount of page tables between root page table and L0 page table
>>>>>>> + * (in the case of Sv39 it covers L1 table):
>>>>>>> + *   (CONFIG_PAGING_LEVELS - 2) are needed for an identity mapping and
>>>>>>> + *   the same amount are needed for Xen.
>>>>>>>      *
>>>>>>> - * CONFIG_PAGING_LEVELS page tables are needed for the identity 
>>>>>>> mapping,
>>>>>>> - * except that the root page table is shared with the initial mapping
>>>>>>> + * An amount of L0 page tables:
>>>>>>> + *   (512 entries of one L0 page table covers 2MB == 
>>>>>>> 1<<XEN_PT_LEVEL_SHIFT(1))
>>>>>>> + *   XEN_VIRT_SIZE >> XEN_PT_LEVEL_SHIFT(1) are needed for Xen and
>>>>>>> + *   one L0 is needed for indenity mapping.
>>>>>>> + *
>>>>>>> + *   It might be needed one more page table in case when Xen load
>>>>>>> + *   address isn't 2 MB aligned.
>>>>>> Shouldn't we guarantee that?
>>>>> I think it's sufficient to guarantee 4KB alignment.
>>>>>
>>>>> The only real benefit I see in enforcing larger alignment is that it 
>>>>> likely enables
>>>>> the use of superpages for mapping, which would reduce TLB pressure.
>>>>> But perhaps I'm missing something?
>>>> No, it's indeed mainly that.
>>> But then the linker address and the load address should both be aligned to 
>>> a 2MB or 1GB boundary.
>>> This likely isn't an issue at all, but could it be a problem if we require 
>>> 1GB alignment for the
>>> load address? In that case, might it be difficult for the platform to find 
>>> a suitable place in
>>> memory to load Xen for some reason? (I don't think so but maybe I'm missing 
>>> something)
>> Why would load address need to be 1Gb aligned? That (as well as 
>> 2Mb-)alignment
>> matters only once you set up paging?
> 
> Mostly yes, it matters only once during paging set up.
> 
> I was thinking that if, one day, 2MB (or larger) alignment is used and the 
> load address isn't
> properly aligned, some space in a page might be lost.
> (The word "should" above wasn't entirely accurate.)

Actually I think I was wrong with my question. Load address of course matters to
a sufficient degree, especially if at 2Mb boundaries to want to be able to 
change
what permissions to use (without sacrificing the 2Mb mappings).

Jan