[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 7/7] npt/shadow: allow getting foreign page table entries

>>> On 11.02.19 at 18:46, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/mm/p2m-pt.c
> +++ b/xen/arch/x86/mm/p2m-pt.c
> @@ -865,7 +865,8 @@ pod_retry_l1:
>      unmap_domain_page(l1e);
>      ASSERT(mfn_valid(mfn) || !p2m_is_ram(*t) || p2m_is_paging(*t));
> -    return (p2m_is_valid(*t) || p2m_is_grant(*t)) ? mfn : INVALID_MFN;
> +    return (p2m_is_valid(*t) || p2m_is_grant(*t) || p2m_is_foreign(*t))
> +           ? mfn : INVALID_MFN;
>  }

Wouldn't you better alter the ASSERT() as well, using p2m_is_any_ram()
instead of p2m_is_ram() now? Grants should have been included there
before, but omitting foreign ones there before was benign.

I also have an unrelated question here, perhaps more for George:
p2m_is_valid() includes p2m_mmio_dm. There are no valid MFNs
associated with that type iirc. The MFN taken out of the P2M entry,
however, can't possibly be INVALID_MFN (due to bit width
constraints). It would seem better to me though to return
INVALID_MFN for that type. (This would then also apply to the 2M
and 1G cases, which have separate return paths.)


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.