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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/10/2023 2:19 pm, Roger Pau Monné wrote:
> On Fri, Oct 06, 2023 at 02:05:18PM +0100, Andrew Cooper wrote:
>> On 06/10/2023 2:00 pm, Roger Pau Monne wrote:
>>> diff --git a/xen/include/public/features.h b/xen/include/public/features.h
>>> index d2a9175aae67..22713a51b520 100644
>>> --- a/xen/include/public/features.h
>>> +++ b/xen/include/public/features.h
>>> @@ -111,6 +111,15 @@
>>>  #define XENFEAT_not_direct_mapped         16
>>>  #define XENFEAT_direct_mapped             17
>>>  
>>> +/*
>>> + * Signal whether the domain is permitted to use the following hypercalls:
>>> + *
>>> + * VCPUOP_register_runstate_phys_area
>>> + * VCPUOP_register_vcpu_time_phys_area
>>> + */
>>> +#define XENFEAT_runstate_phys_area   18
>>> +#define XENFEAT_vcpu_time_phys_area          19
>>> +
>>>  #define XENFEAT_NR_SUBMAPS 1
>>>  
>>>  #endif /* __XEN_PUBLIC_FEATURES_H__ */
>>> diff --git a/xen/include/public/vcpu.h b/xen/include/public/vcpu.h
>>> index 8fb0bd1b6c03..03b031a3e557 100644
>>> --- a/xen/include/public/vcpu.h
>>> +++ b/xen/include/public/vcpu.h
>>> @@ -236,6 +236,9 @@ 
>>> DEFINE_XEN_GUEST_HANDLE(vcpu_register_time_memory_area_t);
>>>   * Note that the area registered via VCPUOP_register_runstate_memory_area 
>>> will
>>>   * be updated in the same manner as the one registered via virtual address 
>>> PLUS
>>>   * VMASST_TYPE_runstate_update_flag engaged by the domain.
>>> + *
>>> + * XENFEAT_{runstate,vcpu_time}_phys_area feature bits signal if the 
>>> domain is
>>> + * permitted the usage of the hypercalls.
>>>   */
>>>  #define VCPUOP_register_runstate_phys_area      14
>>>  #define VCPUOP_register_vcpu_time_phys_area     15
>> For both of these, I'd suggest s/permitted/able/.  For older versions of
>> Xen which don't advertise the XENFEAT, it's a matter of capability, not
>> permission.
>>
>> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> and
>> I'm happy to adjust on commit to save sending out a v3.
> TBH I've used permitted because that's the wording you used in your
> reply to v1, I'm perfectly fine with switching to able.
>
> https://lore.kernel.org/xen-devel/ac4842a9-7f62-4c64-9a3a-2ec2b1e97699@xxxxxxxxxx/

Yeah, sorry.  I didn't think fully before making the suggestion.

I was mainly looking to avoid saying "what the hypervisor is capable of" :)

~Andrew