On 11/20/25 4:47 PM, Jan Beulich wrote:
On 20.11.2025 16:38, Oleksii Kurochko wrote:
On 11/18/25 7:58 AM, Jan Beulich wrote:
On 17.11.2025 20:51, Oleksii Kurochko wrote:
On 11/12/25 12:49 PM, Jan Beulich wrote:
On 20.10.2025 17:58, Oleksii Kurochko wrote:
+ if ( *md_pg )
+ metadata = __map_domain_page(*md_pg);
+
+ if ( t < p2m_first_external )
+ {
pte->pte |= MASK_INSR(t, P2M_TYPE_PTE_BITS_MASK);
- return rc;
+ if ( metadata )
+ metadata[ctx->index].pte = p2m_invalid;
Shouldn't this be accompanied with a BUILD_BUG_ON(p2m_invalid), as otherwise
p2m_alloc_page()'s clearing of the page won't have the intended effect?
I think that, at least, at the moment we are always explicitly set p2m type and
do not rely on that by default 0==p2m_invalid.
You don't, and ...
Just to be safe, I will add after "if ( metadata )" suggested
BUILD_BUG_ON(p2m_invalid):
if ( metadata )
metadata[ctx->index].type = p2m_invalid;
/*
* metadata.type is expected to be p2m_invalid (0) after the page is
* allocated and zero-initialized in p2m_alloc_page().
*/
BUILD_BUG_ON(p2m_invalid);
...
... this leaves me with the impression that you didn't read my reply correctly.
p2m_alloc_page() clear the page, thus_implicitly_ setting all entries to
p2m_invalid. That's where the BUILD_BUG_ON() would want to go (the call site,
ftaod).
I think I still don’t fully understand what the issue would be if|p2m_invalid|
were
ever equal to 1 instead of 0 in the context of a metadata page.
Yes, if|p2m_invalid| were 1, there would be a problem if someone tried to read
this
metadata pagebefore it was assigned any type. They would find a value of 0,
which
corresponds to a valid type rather than to|p2m_invalid|, as one might expect.
However, I’m not sure I currently see a scenario in which the metadata page
would
be read before being initialized.
Are you sure walks can only happen for GFNs that were set up? What you need to
do walks on is under guest control, after all.
If a GFN lies within the range[p2m->lowest_mapped_gfn, p2m->max_mapped_gfn],
then
|p2m_set_entry()| must already have been called for this GFN. This means that
either
- a metadata page has been created and its entry filled with the appropriate
type, or
- no metadata page was needed and the type was stored directly in|pte->pte|
For a GFN outside the range(p2m->lowest_mapped_gfn, p2m->max_mapped_gfn),
|p2m_get_entry()| will not even attempt a walk because of the boundary checks:
static mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
p2m_type_t *t,
unsigned int *page_order)
...
if ( check_outside_boundary(p2m, gfn, p2m->lowest_mapped_gfn, true,
&level) )
goto out;
if ( check_outside_boundary(p2m, gfn, p2m->max_mapped_gfn, false, &level) )
goto out;
If I am misunderstanding something and there are other cases where a walk can
occur for
GFNs that were never set up, then such GFNs would have neither an allocated
metadata
page nor a type stored in|pte->pte|, which looks like we are in trouble.
~ Oleksii
