Re: [RFC XEN PATCH] x86/cpuid: Expose max_vcpus field in HVM hypervisor leaf

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.07.2024 13:11, Alejandro Vallejo wrote:
> I'll pitch in, seeing as I created the GitLab ticket.
> 
> On Tue Jul 9, 2024 at 7:40 AM BST, Jan Beulich wrote:
>> On 08.07.2024 17:42, Matthew Barnes wrote:
>>> Currently, OVMF is hard-coded to set up a maximum of 64 vCPUs on
>>> startup.
>>>
>>> There are efforts to support a maximum of 128 vCPUs, which would involve
>>> bumping the OVMF constant from 64 to 128.
>>>
>>> However, it would be more future-proof for OVMF to access the maximum
>>> number of vCPUs for a domain and set itself up appropriately at
>>> run-time.
>>>
>>> For OVMF to access the maximum vCPU count, Xen will have to expose this
>>> property via cpuid.
>>
>> Why "have to"? The information is available from xenstore, isn't it?
> 
> That would create an avoidable dependency between OVMF and xenstore, 
> precluding
> xenstoreless UEFI-enabled domUs.

Right, but that's a desirable thing, so still not "have to".

>>> This patch exposes the max_vcpus field via cpuid on the HVM hypervisor
>>> leaf in edx.
>>
>> If exposing via CPUID, why only for HVM?
>>
>>> --- a/xen/include/public/arch-x86/cpuid.h
>>> +++ b/xen/include/public/arch-x86/cpuid.h
>>> @@ -87,6 +87,7 @@
>>>   * Sub-leaf 0: EAX: Features
>>>   * Sub-leaf 0: EBX: vcpu id (iff EAX has XEN_HVM_CPUID_VCPU_ID_PRESENT 
>>> flag)
>>>   * Sub-leaf 0: ECX: domain id (iff EAX has XEN_HVM_CPUID_DOMID_PRESENT 
>>> flag)
>>> + * Sub-leaf 0: EDX: max vcpus (iff EAX has XEN_HVM_CPUID_MAX_VCPUS_PRESENT 
>>> flag)
>>>   */
>>
>> Unlike EBX and ECX, the proposed value for EDX cannot be zero. I'm therefore
>> not entirely convinced that we need a qualifying flag. Things would be
>> different if the field was "highest possible vCPU ID", which certainly would
>> be the better approach if the field wasn't occupying the entire register.
>> Even with it being 32 bits, I'd still suggest switching its meaning this way.
> 
> Using max_vcpu_id instead of max_vcpus is also fine, but the flag is important
> as otherwise it's impossible to retroactively change the meaning of EDX (i.e: 
> to
> stop advertising this datum, or repurpose EDX altogether)

Hmm, re-purposing. Very interesting thought. I don't think we should ever do
that.

> We could also reserve only the lower 16bits of EDX rather than the whole 
> thing;
> but we have plenty of subleafs for growth, so I'm not sure it's worth it.

And I was only mentioning it, without meaning to suggest to shrink.

Jan