Re: [Xen-devel] [PATCH for-4.9] x86/mm: Fix incorrect unmapping of 2MB and 1GB pages

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.05.17 at 12:51, <george.dunlap@xxxxxxxxxx> wrote:
> On 10/05/17 11:26, Jan Beulich wrote:
>>>>> On 10.05.17 at 11:43, <igor.druzhinin@xxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/mm/p2m-ept.c
>>> +++ b/xen/arch/x86/mm/p2m-ept.c
>>> @@ -681,6 +681,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long 
>>> gfn, mfn_t mfn,
>>>      ept_entry_t *table, *ept_entry = NULL;
>>>      unsigned long gfn_remainder = gfn;
>>>      unsigned int i, target = order / EPT_TABLE_ORDER;
>>> +    unsigned long mfn_mask = mfn_valid(mfn) ? mfn_x(mfn) : 0;
>> 
>> Aiui MMIO pages will come here too, so an mfn_valid() check here
>> (and below) is too lax.
> 
> The resulting order will never be higher than the order passed in by the
> caller.  Assuming that the caller is setting an entire 2MiB (or 1GiB)
> region as MMIO, is it not valid to set a 2MiB or 1GiB entry as such?
> The code seems to be written in such a way that such entries are expected.

In the case here you're right (albeit that's pretty implicit). In the
p2m_set_entry() case though the order is being determined from
the (gfn,mfn,todo) tuple, so I don't think the code could have
been left as is (seeing that Igor has already sent v2).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel