sh_unshadow_for_p2m_change() vs p2m_set_entry()

[Jan Beulich <jbeulich@xxxxxxxx>]

Tim,

I'm afraid you're still my best guess to hopefully get an insight
on issues like this one.

While doing IOMMU superpage work I was, just in the background,
considering in how far the superpage re-coalescing to be used there
couldn't be re-used for P2M / EPT / NPT. Which got me to think about
shadow mode's using of p2m-pt.c: That's purely software use of the
tables in that case, isn't it? In which case hardware support for
superpages shouldn't matter at all.

The only place where I could spot that P2M superpages would actually
make a difference to shadow code was sh_unshadow_for_p2m_change().
That one appears to have been dealing with 2M pages (more below)
already at the time of 0ca1669871f8a ("P2M: check whether hap mode
is enabled before using 2mb pages"), so I wonder what "potential
errors when hap is disabled" this commit's description might be
talking about. (Really, if it's software use of the tables only, in
principle even 512Gb superpages could be made use of there. But of
course sh_unshadow_for_p2m_change() wouldn't really like this, just
like it doesn't like 1Gb pages. So that's purely a theoretical
consideration.)

As to sh_unshadow_for_p2m_change()'s readiness for at least 2Mb
pages: The 4k page handling there differs from the 2M one primarily
in the p2m type checks: "p2m_is_valid(...) || p2m_is_grant(...)"
vs "p2m_is_valid(...)" plus later "!p2m_is_ram(...)", the first
three acting on the type taken from the old PTE, while the latter
acts on the type in the new (split) PTEs. Shouldn't the exact same
checks be used in both cases (less the p2m_is_grant() check which
can't be true for superpages)? IOW isn't !p2m_is_ram() at least
superfluous (and perhaps further redundant with the subsequent
!mfn_eq(l1e_get_mfn(npte[i]), omfn))? Instead I'm missing an
entry-is-present check, without which l1e_get_mfn(npte[i]) looks
risky at best. Or is p2m_is_ram() considered a sufficient
replacement, making assumptions on the behavior of a lot of other
code?

The 2M logic also first checks _PAGE_PRESENT (and _PAGE_PSE), while
the 4k logic appears to infer that the old page was present from
p2m_is_{valid,grant}().

And isn't this 2M page handling code (because of the commit pointed
at above) dead right now anyway? If not, where would P2M superpages
come from?

Thanks much,
Jan