Re: sh_unshadow_for_p2m_change() vs p2m_set_entry()

[Tim Deegan <tim@xxxxxxx>]

At 07:59 +0200 on 01 Oct (1633075173), Jan Beulich wrote:
> On 27.09.2021 22:25, Tim Deegan wrote:
> > At 13:31 +0200 on 24 Sep (1632490304), Jan Beulich wrote:
> >> 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}().
> > 
> > I think the p2m_type check is the right one (rather than
> > _PAGE_PRESENT), since that's the one that the p2m lookups will obey
> > when causing things to get shadowed.  But the extra _PAGE_PSE check
> > should stay.
> 
> Actually, having transformed things into patch form, I'm now puzzled
> by you explicitly saying this. Wasn't this check wrong in the first
> place? I don't see anything preventing an L1 page table getting
> zapped (or replaced by a 2M mapping) all in one go.

ISTR that this couldn't happen, but I don't remember exactly exactly
why.  It may just be that the p2m update code didn't have that path,
but it's a bit fragile to rely on that.

> The full range
> of GFNs would need checking in this case as well, just like in the
> opposite case (2M mapping getting replaced by an L1 pt).

Yes.  Any case where we're inserting or removing an L1 table would
need to check all 512 L1Es.

Cheers,

Tim.