Re: [PATCH v2 14/17] xen/riscv: implement p2m_next_level()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.07.2025 13:19, Oleksii Kurochko wrote:
> On 7/17/25 12:37 PM, Jan Beulich wrote:
>> On 17.07.2025 11:42, Oleksii Kurochko wrote:
>>> On 7/16/25 6:12 PM, Jan Beulich wrote:
>>>> On 16.07.2025 17:53, Oleksii Kurochko wrote:
>>>>> In this case, the P2M PTE valid bit will be set to 0, but the P2M PTE 
>>>>> type bits
>>>>> will be set to something other than|p2m_invalid| (even for a table 
>>>>> entries),
>>>>> so when an MMU fault occurs, we can properly resolve it.
>>>>>
>>>>> So, if the P2M PTE type (what|p2m_is_valid()| checks) is set 
>>>>> to|p2m_invalid|, it
>>>>> means that the valid bit (what|pte_is_valid()| checks) should be set to 
>>>>> 0, so
>>>>> the P2M PTE is genuinely invalid.
>>>>>
>>>>> It could also be the case that the P2M PTE type isn't|p2m_invalid (and 
>>>>> P2M PTE valid will be intentionally set to 0 to have
>>>>> ability to track which pages were accessed for the reason I wrote 
>>>>> above)|, and when MMU fault occurs we could
>>>>> properly handle it and set to 1 P2M PTE valid bit to 1...
>>>>>
>>>>>> An intermediate page table entry is something Xen controls entirely. 
>>>>>> Hence
>>>>>> it has no (guest induced) type.
>>>>> ... And actually it is a reason why it is needed to set a type even for an
>>>>> intermediate page table entry.
>>>>>
>>>>> I hope now it is a lit bit clearer what and why was done.
>>>> Sadly not. I still don't see what use the P2M type in of an intermediate 
>>>> page
>>>> table is going to be. It surely can't reliably describe all of the entries 
>>>> that
>>>> page table holds. Intermediate page tables and leaf pages are just too 
>>>> different
>>>> to share a concept like this, I think. That said, I'll be happy to be 
>>>> shown code
>>>> demonstrating the contrary.
>>> Then it is needed to introduce new p2m_type_t - p2m_table and use it.
>>> Would it be better?
>>>
>>> I still need some type to have ability to distinguish if p2m is valid or 
>>> not from
>>> p2m management and hardware point of view.
>>> If there is no need for such distinguish why all archs introduce 
>>> p2m_invalid?
>>> Isn't enough just to use P2M PTE valid bit?
>> At least on x86 we don't tag intermediate page tables with P2M types. For
>> ordinary leaf entries the situation is different, as there may be varying
>> reasons why a PTE has its valid (on x86: present) bit cleared. Hence the
>> type is relevant there, just to know what to do when a page is accessed
>> through such a not-present PTE.
> 
> I think that I got your idea now.
> 
> Does it make sense to have such optimization when we have 2Mb memory range and
> it was mapped using 4k pages instead of 1 super-page, could it be useful to
> invalidate just just page table entry of L1 which corresponds to the start of
> this 2mb memory range, instead of invalidating each entry on L0?
> If it could useful then intermediate page tables should be tagged too. Arm has
> such use cases:
>    
> https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/arm/mmu/p2m.c#L1286

I don't currently see how that's related to the topic at hand.

Furthermore range-constrained TLB flushing is never at just an address, i.e.
"L1 which corresponds to the start of this 2mb memory range" isn't meaningful
here. It's always a range (typically expressed by address and size), and it
always needs to be the full range that is invalidated. This can be a solitary
low-level flush operation when you know a large page mapping would _not_ be
split. When splitting is done in software or when hardware may split behind
your back, you always need to invalidate the entire range. Or else, in your
example, 4k TLB entries may remain for any but the first page of the 2M
super-page. (Whether such a range can still be done in a single invalidation
operation is a separate question. But I don't see how maintaining the type
at the L1 level would help there.)

Jan