Re: [Xen-devel] [PATCH for-4.12] x86/p2m: fix p2m_finish_type_change()

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

> On Jan 9, 2019, at 9:16 PM, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
>>>> On 09.01.19 at 11:41, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> finish_type_change() returns a negative int on error, but the
>> current code checks if ( !rc ).
> 
> For the purpose of determining the backporting scope (none here)
> it would be nice if in such a case you could point out the commit
> introducing the breakage.
> 
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -1176,7 +1176,7 @@ int p2m_finish_type_change(struct domain *d,
>> 
>>     rc = finish_type_change(hostp2m, first_gfn, max_nr);
>> 
>> -    if ( !rc )
>> +    if ( rc < 0 )
>>         goto out;
>> 
>> #ifdef CONFIG_HVM
>> @@ -1193,13 +1193,13 @@ int p2m_finish_type_change(struct domain *d,
>>                 rc = finish_type_change(altp2m, first_gfn, max_nr);
>>                 p2m_unlock(altp2m);
>> 
>> -                if ( !rc )
>> +                if ( rc < 0 )
>>                     goto out;
>>             }
>>     }
>> #endif
> 
> May I ask that you also fix the other return value issue here: Either
> we mean to indicate to the caller the 0/1 distinction (in which case
> it should be correct, i.e. cumulative across all finish_type_change()
> calls made here), which was the behavior before 0fb4b58c8b
> ("x86/altp2m: fix display frozen when switching to a new view early"),
> or we mean to signal success to the caller only by handing back zero.

The “root” caller of p2m_finish_type_change() is 
XEN_DMOP_map_mem_type_to_ioreq_server; It doesn’t look like it does anything 
useful with positive values, and it’s not clear from an interface perspective 
that returning a positive value from that hypercall makes any sense.

Perhaps it would be best to just discard the positive values for now.

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