On 07.10.2021 10:21, Michal Orzel wrote:
> Hi Jan,
>
> On 07.10.2021 10:03, Jan Beulich wrote:
>> On 06.10.2021 12:58, Michal Orzel wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -692,6 +692,12 @@ int arch_sanitise_domain_config(struct
>>> xen_domctl_createdomain *config)
>>> return -EINVAL;
>>> }
>>>
>>> + if ( config->flags & XEN_DOMCTL_CDF_vpmu )
>>> + {
>>> + dprintk(XENLOG_INFO, "vpmu support not ready yet\n");
>>> + return -EINVAL;
>>> + }
>>
>> I consider this message potentially misleading (as x86 does have vPMU
>> support, it merely doesn't get enabled this way). But isn't this redundant
>> with ...
>>
>>> @@ -534,6 +535,12 @@ static int sanitise_domain_config(struct
>>> xen_domctl_createdomain *config)
>>> return -EINVAL;
>>> }
>>>
>>> + if ( vpmu && !vpmu_is_available )
>>> + {
>>> + dprintk(XENLOG_INFO, "vpmu requested but not available\n");
>>> + return -EINVAL;
>>> + }
>>
>> ... this? (This message is again potentially misleading.)
>>
> Ok. vpmu_is_available is false for x86 so the check in x86's
> arch_sanitise_domain_config is redundant.
> I will fix it. When it comes to the message itself "vpmu requested but not
> available".
> Does the following sound better for you?
> "vpmu requested but the platform does not support it"
> If not, can you please suggest a better message?
While it gets a little long then, appending "at domain creation time" would
disambiguate the text. Or maybe "vPMU cannot be enabled this way"? It's a
debug-only message after all, so its wording can quite well be developer-
focused imo.
Jan
