Re: [Xen-devel] [PATCH RFC v4] x86/mm: Clean up p2m_finish_type_change return value

[Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>]

On 27.03.2019 18:07, Jan Beulich wrote:
>>>> On 27.03.19 at 16:21, <aisaila@xxxxxxxxxxxxxxx> wrote:
>> @@ -621,7 +623,7 @@ bool_t ept_handle_misconfig(uint64_t gpa)
>>   
>>       p2m_unlock(p2m);
>>   
>> -    return spurious ? (rc >= 0) : (rc > 0);
>> +    return spurious && !rc;
>>   }
> 
> I think you've gone too far now: This is - afaict - the one place
> where the distinction matters. Looking back at Paul's reply and
> my subsequent one on v3, I'm afraid I've managed to misguide
> you by not looking closely enough at what Paul did sketch out.
> I'm sorry for this.
> 
> I think you either want to leave EPT code untouched, and zap
> the positive return value in finish_type_change() instead. Or
> EPT's resolve_misconfig() would need to gain a wrapper for use
> as the ->recalc hook, to squash the positive value for the outside
> world. Iirc I've avoided introducing such a wrapper originally
> just to limit the number of functions layered on top of one
> another, while using resolve_misconfig() directly appeared to
> be fine.
> It's alright, it's an honest mistake and in this case I will go back and 
have finish_type_change() cut the positive value if that is ok with 
everyone.

Regards,
Alex

>> @@ -668,7 +670,7 @@ ept_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t 
>> mfn,
>>   
>>       /* Carry out any eventually pending earlier changes first. */
>>       ret = resolve_misconfig(p2m, gfn);
>> -    if ( ret < 0 )
>> +    if ( ret )
>>           return ret;
> 
> This would then need undoing as well.
> 
>> @@ -1201,7 +1201,7 @@ int p2m_finish_type_change(struct domain *d,
>>   
>>       rc = finish_type_change(hostp2m, first_gfn, max_nr);
>>   
>> -    if ( rc < 0 )
>> +    if ( rc )
>>           goto out;
> 
> While I don't really object to this change, I also don't think it's
> strictly necessary.
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel