Re: [PATCH v6 19/19] xen/riscv: introduce metadata table to store P2M type

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 12/10/25 8:06 AM, Jan Beulich wrote:
> On 09.12.2025 18:09, Oleksii Kurochko wrote:
> > On 12/9/25 2:47 PM, Jan Beulich wrote:
> > > On 24.11.2025 13:33, Oleksii Kurochko wrote:
> > > > +            *md_pg = p2m_alloc_page(p2m);
> > > > +            if ( !*md_pg )
> > > > +            {
> > > > +                printk("%pd: can't allocate metadata page\n", p2m->domain);
> > > > +                domain_crash(p2m->domain);
> > > > +
> > > > +                return;
> > > > +            }
> > > > +        }
> > > > +    }
> > > > +
> > > > +    if ( *md_pg )
> > > > +        metadata = __map_domain_page(*md_pg);
> > > > +
> > > > +    if ( t >= p2m_first_external )
> > > > +    {
> > > > +        metadata[ctx->index].type = t;
> > > > +
> > > > +        t = p2m_ext_storage;
> > > > +    }
> > > > +    else if ( metadata )
> > > > +        metadata[ctx->index].type = p2m_invalid;
> > > > +
> > > > +    pte->pte |= MASK_INSR(t, P2M_TYPE_PTE_BITS_MASK);
> > > > +
> > > > +    unmap_domain_page(metadata);
> > > >    }
> > > Just to mention (towards future work): Once a metadata page goes back to be
> > > entirely zero-filled, it could as well be hooked off and returned to the pool.
> > > Not doing so may mean detaining an unused page indefinitely.
> > Won’t that already happen when p2m_free_table() is called?
> Well, that's when both page table and metadata table are freed. But what if a
> leaf page table is moving back to holding all p2m_ram_rw mappings? Then the
> metadata page is unused, but will remain allocated.
Good point...

This could be a rather expensive operation, since in the code:
  +    else if ( metadata )
  +        metadata[ctx->index].type = p2m_invalid;
we would have to check all other metadata entries to determine whether they are
(p2m_invalid) or not, and return the page to the pool.

It would be nice to have something like metadata.used_entries_num, but the 
entire
page is used for type entries.
As an option, we could reserve 8 bits to store a counter of the number of used
entries in the metadata page, and then use metadata[0].used_entries_num to check
whether it is zero. If it is zero, we could simply return the metadata page to 
the
pool in the “else if (metadata)” case mentioned above.

How bad is this idea? Any better suggestions?

~ Oleksii