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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.)

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel