Re: [Xen-devel] [PATCH 1/3] x86/PoD: correctly handle non-order-0 decrease-reservation requests

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.12.17 at 16:58, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 04/12/17 11:06, Jan Beulich wrote:
>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -284,13 +284,15 @@ int guest_remove_page(struct domain *d,
>>  
>>  #ifdef CONFIG_X86
>>      mfn = get_gfn_query(d, gmfn, &p2mt);
>> +    if ( unlikely(p2mt == p2m_invalid) || unlikely(p2mt == p2m_mmio_dm) )
>> +        return -ENOENT;
> 
> Newline.
> 
>>      if ( unlikely(p2m_is_paging(p2mt)) )
>>      {
>>          rc = guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
> 
> Somewhere in this callchain, you truncate unsigned long to int.  It is
> ok (I think) at the moment because ORDER_1G fits within int, but is
> liable to break subtly in the future.

I don't understand this: I can't seem to see gfn or mfn to be truncated
(and that would be a pre-existing problem then afaict). And passing
order values as unsigned long is plain pointless - even with 64-bit
frame numbers this can't exceed 64, i.e. is limited to a 6-bit value in
practice. Please clarify what truncation you're suspecting.

>> @@ -375,9 +373,10 @@ int guest_remove_page(struct domain *d,
>>          put_page(page);
>>  
>>      put_page(page);
>> + out_put_gfn: __maybe_unused
> 
> What is this annotation for?

For the !CONFIG_X86 case.

Jan


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