Re: [Xen-devel] [PATCH v5 01/16] x86/mm: skip incrementing mfn if it is not a valid mfn

2018-03-15 Thread George Dunlap 
On Thu, Mar 15, 2018 at 12:16 PM, Andrew Cooper
 wrote:
> On 15/03/18 07:52, Jan Beulich wrote:
> On 14.03.18 at 19:31,  wrote:
>>> On 14/03/18 18:19, julien.gr...@arm.com wrote:
 From: Wei Liu 

 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 
>>> Remind me what the purpose of this patch is?
>> This is in preparation to switch callers to pass INVALID_MFN
>> instead of zero for non-present mappings. The incrementing
>> from zero was wrong here already (but couldn't be as easily
>> avoided, to not cause problems with possible legitimate uses of
>> MFN 0), but incrementing (and wrapping) from INVALID_MFN is
>> (imo) even worse, which is why I had asked the conversion to
>> INVALID_MFN to not be done without this change.
>
> Yes.  My reply was a (clearly too) thinly veiled hint that a sentence to
> this effect should be in the commit message.

You mean "a (clearly not thinly enough) veiled hint". :-)

I agree with Andy re both the code and the commit message.

 -George

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

Re: [Xen-devel] [PATCH v5 01/16] x86/mm: skip incrementing mfn if it is not a valid mfn

2018-03-15 Thread Julien Grall 



On 15/03/18 07:49, Jan Beulich wrote:

On 14.03.18 at 19:19,  wrote:

From: Wei Liu 

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 


Reviewed-by: Jan Beulich 

(You should have Cc-ed Andrew and me.)


I forgot to update the CCs for this new patch. Sorry for that.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v5 01/16] x86/mm: skip incrementing mfn if it is not a valid mfn

2018-03-15 Thread Andrew Cooper 
On 15/03/18 07:52, Jan Beulich wrote:
 On 14.03.18 at 19:31,  wrote:
>> On 14/03/18 18:19, julien.gr...@arm.com wrote:
>>> From: Wei Liu 
>>>
>>> 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 
>> Remind me what the purpose of this patch is?
> This is in preparation to switch callers to pass INVALID_MFN
> instead of zero for non-present mappings. The incrementing
> from zero was wrong here already (but couldn't be as easily
> avoided, to not cause problems with possible legitimate uses of
> MFN 0), but incrementing (and wrapping) from INVALID_MFN is
> (imo) even worse, which is why I had asked the conversion to
> INVALID_MFN to not be done without this change.

Yes.  My reply was a (clearly too) thinly veiled hint that a sentence to
this effect should be in the commit message.

The code itself is fine.

~Andrew

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

Re: [Xen-devel] [PATCH v5 01/16] x86/mm: skip incrementing mfn if it is not a valid mfn

2018-03-15 Thread Jan Beulich 
>>> On 14.03.18 at 19:31,  wrote:
> On 14/03/18 18:19, julien.gr...@arm.com wrote:
>> From: Wei Liu 
>>
>> 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 
> 
> Remind me what the purpose of this patch is?

This is in preparation to switch callers to pass INVALID_MFN
instead of zero for non-present mappings. The incrementing
from zero was wrong here already (but couldn't be as easily
avoided, to not cause problems with possible legitimate uses of
MFN 0), but incrementing (and wrapping) from INVALID_MFN is
(imo) even worse, which is why I had asked the conversion to
INVALID_MFN to not be done without this change.

Jan


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

Re: [Xen-devel] [PATCH v5 01/16] x86/mm: skip incrementing mfn if it is not a valid mfn

2018-03-15 Thread Jan Beulich 
>>> On 14.03.18 at 19:19,  wrote:
> From: Wei Liu 
> 
> 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 

Reviewed-by: Jan Beulich 

(You should have Cc-ed Andrew and me.)

Jan


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

Re: [Xen-devel] [PATCH v5 01/16] x86/mm: skip incrementing mfn if it is not a valid mfn

2018-03-14 Thread Andrew Cooper 
On 14/03/18 18:19, julien.gr...@arm.com wrote:
> From: Wei Liu 
>
> 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 

Remind me what the purpose of this patch is?

~Andrew

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