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
