Re: [PATCH v3 16/20] xen/riscv: Implement superpage splitting for p2m mappings

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.08.2025 17:19, Oleksii Kurochko wrote:
> On 8/11/25 1:59 PM, Jan Beulich wrote:
>> On 31.07.2025 17:58, Oleksii Kurochko wrote:
>>> --- a/xen/arch/riscv/p2m.c
>>> +++ b/xen/arch/riscv/p2m.c
>>> @@ -539,6 +539,91 @@ static void p2m_free_subtree(struct p2m_domain *p2m,
>>>       p2m_free_page(p2m, pg);
>>>   }
>>>   
>>> +static bool p2m_split_superpage(struct p2m_domain *p2m, pte_t *entry,
>>> +                                unsigned int level, unsigned int target,
>>> +                                const unsigned int *offsets)
>>> +{
>>> +    struct page_info *page;
>>> +    unsigned long i;
>>> +    pte_t pte, *table;
>>> +    bool rv = true;
>>> +
>>> +    /* Convenience aliases */
>>> +    mfn_t mfn = pte_get_mfn(*entry);
>>> +    unsigned int next_level = level - 1;
>>> +    unsigned int level_order = XEN_PT_LEVEL_ORDER(next_level);
>>> +
>>> +    /*
>>> +     * This should only be called with target != level and the entry is
>>> +     * a superpage.
>>> +     */
>>> +    ASSERT(level > target);
>>> +    ASSERT(pte_is_superpage(*entry, level));
>>> +
>>> +    page = p2m_alloc_page(p2m->domain);
>>> +    if ( !page )
>>> +    {
>>> +        /*
>>> +         * The caller is in charge to free the sub-tree.
>>> +         * As we didn't manage to allocate anything, just tell the
>>> +         * caller there is nothing to free by invalidating the PTE.
>>> +         */
>>> +        memset(entry, 0, sizeof(*entry));
>>> +        return false;
>>> +    }
>>> +
>>> +    table = __map_domain_page(page);
>>> +
>>> +    /*
>>> +     * We are either splitting a second level 1G page into 512 first level
>>> +     * 2M pages, or a first level 2M page into 512 zero level 4K pages.
>>> +     */
>> Such a comment is at risk of (silently) going stale when support for 512G
>> mappings is added. I wonder if it's really that informative to have here.
> 
> Good point, I think we could really drop it.
> Regarding support for 512G mappings. Is it really make sense to support
> such big mappings?

I think so, yes (in the longer run). And yes, ...

> It seems like some operations as splitting or sub-entry
> freeing could be pretty long under some circumstances.

... such will need sorting.

Jan