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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.02.19 at 13:16, <roger.pau@xxxxxxxxxx> wrote:
> On Thu, Feb 14, 2019 at 04:38:54AM -0700, Jan Beulich wrote:
>> >>> 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.
> 
> Yes, I could use p2m_is_any_ram both in the assert and in the return
> condition:
> 
> ASSERT(mfn_valid(mfn) || !p2m_is_any_ram(*t) || p2m_is_paging(*t));
> return (p2m_is_valid(*t) || p2m_is_any_ram(*t)) ? mfn : INVALID_MFN;

Ah, indeed, it can be used in the return statement as well (and even
helps its legibility).

Jan



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