Re: [PATCH] x86/vcpu: relax VCPUOP_initialise restriction for non-PV vCPUs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.03.2024 16:20, Andrew Cooper wrote:
> On 20/03/2024 3:09 pm, Jan Beulich wrote:
>> On 20.03.2024 14:57, Roger Pau Monne wrote:
>>> There's no reason to force HVM guests to have a valid vcpu_info area when
>>> initializing a vCPU, as the vCPU can also be brought online using the local
>>> APIC, and on that path there's no requirement for vcpu_info to be setup 
>>> ahead
>>> of the bring up.  Note an HVM vCPU can operate normally without making use 
>>> of
>>> vcpu_info.
>> While I'd agree if you started with "There's no real need to force ...", I
>> still think there is a reason: If one wants to use paravirt interfaces (i.e.
>> hypercalls), they would better do so consistently. After all there's also
>> no need to use VCPUOP_initialise, yet you're not disabling its use.
> 
> I firmly disagree.
> 
> There are good reasons to use VCPUOP_initialise over INIT-SIPI-SIPI
> (like avoiding 16bit mode - in the case we want it here, to fix APIC_ID
> enumeration in a way that doesn't involve putting more complexity into
> HVMLoader), and forcing us to set up a useless structure before we can
> boot vCPU number 32 is just wrong.

Just to mention it: I can certainly accept this as one possible valid
viewpoint, and my looking at it differently is not an objection to the
patch. It's just that the other aspect mentioned needs sorting (perhaps
by just extending the patch description).

> It was dumb to design a hypercall like this in the first place for PV
> guests, and it definitely isn't OK to keep guests broken because of it.

And again just to mention it: Originally, with a vCPU limit of 32, all
vCPU-s would reliably have had vcpu_info (by way of that being embedded
in shared_info). When raising the limit, the goal was to not chance
overlooking any vcpu_info access in Xen. Hence why, instead of putting
a NULL pointer there (or perhaps some non-canonical sentinel), the
dummy approach was chosen. It then seemed quite desirable to prevent
vCPU-s coming online without them first being detached from that dummy
structure. And I think this firmly needs to continue to hold for PV.

Jan