On Tue, Nov 29, 2022 at 03:47:53PM +0100, Jan Beulich wrote:
> Neither page_get_owner() nor mfn_to_page() are entirely trivial
> operations - don't do the same thing twice in close succession. Instead
> help CSE (when MEM_SHARING=y) by introducing a local variable holding
> the page owner.
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
Acked-by: Roger Pau Monné <roger....@citrix.com>
> ---
> According to my observations gcc12 manages to CSE mfn_to_page() but not
> (all of) page_get_owner(). The overall savings there are, partly due to
> knock-on effects, 64 bytes of code.
>
> While looking there, "mfn_eq(omfn, mfn_add(mfn, i))" near the end of the
> same loop caught my eye: Is that really correct? Shouldn't we fail the
> operation if the MFN which "ogfn" was derived from doesn't match the MFN
> "ogfn" maps to?
Getting into that state possibly means something has gone wrong if we
have rules out grants and foreign maps?
So it should be:
if ( !mfn_eq(omfn, mfn_add(mfn, i)) )
{
/* Something has gone wrong, ASSERT_UNREACHABLE()? */
goto out;
}
rc = p2m_remove_entry(p2m, ogfn, omfn, 0)
if ( rc )
goto out;
but maybe I'm missing the point of the check there, I have to admit I
sometimes find the p2m code difficult to follow.
Thanks, Roger.
