On 30.12.2025 16:47, Oleksii Kurochko wrote:
>
> On 12/29/25 4:13 PM, Jan Beulich wrote:
>> On 22.12.2025 17:37, Oleksii Kurochko wrote:
>>> @@ -370,24 +396,96 @@ static struct page_info *p2m_alloc_page(struct
>>> p2m_domain *p2m)
>>> return pg;
>>> }
>>>
>>> -static int p2m_set_type(pte_t *pte, p2m_type_t t)
>>> +/*
>>> + * `pte` – PTE entry for which the type `t` will be stored.
>>> + *
>>> + * If `t` >= p2m_first_external, a valid `ctx` must be provided.
>>> + */
>>> +static void p2m_set_type(pte_t *pte, p2m_type_t t,
>>> + const struct p2m_pte_ctx *ctx)
>>> {
>>> - int rc = 0;
>>> + struct page_info **md_pg;
>>> + struct md_t *metadata = NULL;
>>>
>>> - if ( t > p2m_first_external )
>>> - panic("unimplemeted\n");
>>> - else
>>> - pte->pte |= MASK_INSR(t, P2M_TYPE_PTE_BITS_MASK);
>>> + /*
>>> + * It is sufficient to compare ctx->index with PAGETABLE_ENTRIES
>>> because,
>>> + * even for the p2m root page table (which is a 16 KB page allocated as
>>> + * four 4 KB pages), calc_offset() guarantees that the page-table index
>>> + * will always fall within the range [0, 511].
>>> + */
>>> + ASSERT(ctx && ctx->index < PAGETABLE_ENTRIES);
>>>
>>> - return rc;
>>> + /*
>>> + * At the moment, p2m_get_root_pointer() returns one of four possible
>>> p2m
>>> + * root pages, so there is no need to search for the correct ->pt_page
>>> + * here.
>>> + * Non-root page tables are 4 KB pages, so simply using ->pt_page is
>>> + * sufficient.
>>> + */
>>> + md_pg = &ctx->pt_page->v.md.pg;
>>> +
>>> + if ( !*md_pg && (t >= p2m_first_external) )
>>> + {
>>> + /*
>>> + * Since p2m_alloc_page() initializes an allocated page with
>>> + * zeros, p2m_invalid is expected to have the value 0 as well.
>>> + */
>>> + BUILD_BUG_ON(p2m_invalid);
>>> +
>>> + ASSERT(ctx->p2m);
>> I think I previously asked for this to be moved out of the if(). Else you
>> may not notice a caller side issue until a point where a metadata page
>> actually needs allocating. (This could simply be folded into the earlier
>> ASSERT().)
>
> I think that I understand your intention and okay to fold ASSERT(ctx->p2m)
> into the earlier ASSERT().
> Just want to note that generally if the metadata page has been already
> allocated and then p2m_set_type() will be called with ctx->p2m == NULL then
> nothing serious will happen as basically ctx->p2m is needed in this function
> only for allocation of metadata page.
Correct, but how would any particular caller know? Yes, there may be special
cases where a caller does know, but imo the code is going to be more robust
if the check is always being made (forcing all callers to set ->p2m).
Jan
