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

> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -536,6 +536,7 @@ int p2m_set_entry(struct p2m_domain *p2m, unsigned long 
> gfn, mfn_t mfn,
>      struct domain *d = p2m->domain;
>      unsigned long todo = 1ul << page_order;
>      unsigned int order;
> +    unsigned long mfn_mask;

Please move the declaration ...

> @@ -543,12 +544,15 @@ int p2m_set_entry(struct p2m_domain *p2m, unsigned long 
> gfn, mfn_t mfn,
>      while ( todo )
>      {
>          if ( hap_enabled(d) )
> -            order = (!((gfn | mfn_x(mfn) | todo) &
> +        {
> +            mfn_mask = mfn_valid(mfn) ? mfn_x(mfn) : 0;

... here, perhaps at once making this the initializer. However, ...

> +            order = (!((gfn | mfn_mask | todo) &
>                         ((1ul << PAGE_ORDER_1G) - 1)) &&
>                       hap_has_1gb) ? PAGE_ORDER_1G :
> -                    (!((gfn | mfn_x(mfn) | todo) &
> +                    (!((gfn | mfn_mask | todo) &

... seeing the recurring expression, it may be worth considering to
instead introduce a local variable holding "gfn | mfn_mask | todo".

Jan


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