Re: [PATCH v2 2/2] domain: expose newly introduced hypercalls as XENFEAT

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.10.2023 16:01, Roger Pau Monné wrote:
> On Mon, Oct 16, 2023 at 03:58:22PM +0200, Jan Beulich wrote:
>> On 16.10.2023 15:00, Roger Pau Monné wrote:
>>> On Mon, Oct 16, 2023 at 02:35:44PM +0200, Jan Beulich wrote:
>>>> On 06.10.2023 15:00, Roger Pau Monne wrote:
>>>>> --- a/xen/common/domain.c
>>>>> +++ b/xen/common/domain.c
>>>>> @@ -1998,6 +1998,10 @@ long common_vcpu_op(int cmd, struct vcpu *v, 
>>>>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>>>>      {
>>>>>          struct vcpu_register_runstate_memory_area area;
>>>>>  
>>>>> +        rc = -ENOSYS;
>>>>> +        if ( 0 /* TODO: Dom's XENFEAT_runstate_phys_area setting */ )
>>>>> +            break;
>>>>> +
>>>>>          rc = -EFAULT;
>>>>>          if ( copy_from_guest(&area.addr.p, arg, 1) )
>>>>>              break;
>>>>
>>>> ENOSYS is not correct here. EPERM, EACCES, or EOPNOTSUPP would all be more
>>>> correct.
>>>
>>> I don't think so, common_vcpu_op() default case does return -ENOSYS,
>>> and the point of this path is to mimic the behavior of an hypervisor
>>> that doesn't have the hypercalls implemented, hence -ENOSYS is the
>>> correct behavior.
>>
>> Besides that other ENOSYS being wrong too, I question such "mimic-ing".
>> Imo error codes should be the best representation of the real reason,
>> not be arbitrarily "made up".
> 
> The point is for the guest to not take any action that it won't take
> on an hypervisor that doesn't have the hypercalls implemented, and the
> only way to be sure about that is to return the same exact error code.

I don't follow this kind of reasoning. Why would a guest, whose code to
use the new sub-functions has to be newly written, key its decision to
the specific error code it gets, when at the same time you expose
feature bits that it can use to decide whether to invoke the hypercall
in the first place.

Jan