Re: [PATCH v4 08/16] xen/mm: Drop the parameter mfn from populate_pt_range

[Julien Grall <julien.grall@arm.com>]

Hi Wei,

On 03/09/2018 05:29 PM, Wei Liu wrote:
> On Mon, Mar 05, 2018 at 07:38:36AM -0700, Jan Beulich wrote:
>>>>> On 05.03.18 at 15:11, <julien.grall@arm.com> wrote:
>>> On 05/03/18 14:00, Jan Beulich wrote:
>>>>>>> On 05.03.18 at 14:43, <julien.grall@arm.com> wrote:
>>>>> Anyway, I don't have much knowledge on the x86 to make the modification
>>>>> that you suggested. So I am going to revert to _mfn(0) for x86.
>>>>
>>>> I'd prefer if you didn't, but well, it'll be one of us to clean it up
>>>> then.
>>> I can keep as INVALID_MFN. But then either you or Andrew (or anyone x86
>>> folks) would have to provide the patch to skip incrementing invalid MFN
>>> (if I understood correctly your request).
>>
>> Sigh - this should go together imo. While wrongly incrementing from
>> zero was bad, wrongly wrapping from INVALID_MFN makes things
>> worse.
>>
> 
> Try this patch?

I am happy to carry this patch at the beginning of my series if you want.

> 
> ---8<---
>  From 8f0024c690c736d17adde0fa765cbbf6fa2846dc Mon Sep 17 00:00:00 2001
> From: Wei Liu <wei.liu2@citrix.com>
> Date: Fri, 9 Mar 2018 17:20:14 +0000
> Subject: [PATCH] x86/mm: skip incrementing mfn if it is not a valid mfn
> 
> The function is called to fill in page table entries in
> populate_pt_range. Skip incrementing mfn if it is invalid.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>   xen/arch/x86/mm.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 9b559448a7..5f5577c7c2 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4731,7 +4731,8 @@ int map_pages_to_xen(
>               }
>   
>               virt    += 1UL << L3_PAGETABLE_SHIFT;
> -            mfn     += 1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT);
> +            if ( !mfn_eq(_mfn(mfn), INVALID_MFN) )
> +                mfn += 1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT);
>               nr_mfns -= 1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT);
>               continue;
>           }
> @@ -4756,7 +4757,8 @@ int map_pages_to_xen(
>                   if ( i > nr_mfns )
>                       i = nr_mfns;
>                   virt    += i << PAGE_SHIFT;
> -                mfn     += i;
> +                if ( !mfn_eq(_mfn(mfn), INVALID_MFN) )
> +                    mfn += i;
>                   nr_mfns -= i;
>                   continue;
>               }
> @@ -4824,7 +4826,8 @@ int map_pages_to_xen(
>               }
>   
>               virt    += 1UL << L2_PAGETABLE_SHIFT;
> -            mfn     += 1UL << PAGETABLE_ORDER;
> +            if ( !mfn_eq(_mfn(mfn), INVALID_MFN) )
> +                mfn += 1UL << PAGETABLE_ORDER;
>               nr_mfns -= 1UL << PAGETABLE_ORDER;
>           }
>           else
> @@ -4853,7 +4856,8 @@ int map_pages_to_xen(
>                       if ( i > nr_mfns )
>                           i = nr_mfns;
>                       virt    += i << L1_PAGETABLE_SHIFT;
> -                    mfn     += i;
> +                    if ( !mfn_eq(_mfn(mfn), INVALID_MFN) )
> +                        mfn += i;
>                       nr_mfns -= i;
>                       goto check_l3;
>                   }
> @@ -4898,7 +4902,8 @@ int map_pages_to_xen(
>               }
>   
>               virt    += 1UL << L1_PAGETABLE_SHIFT;
> -            mfn     += 1UL;
> +            if ( !mfn_eq(_mfn(mfn), INVALID_MFN) )
> +                mfn += 1UL;
>               nr_mfns -= 1UL;
>   
>               if ( (flags == PAGE_HYPERVISOR) &&
> 

Cheers,

-- 
Julien Grall

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