Re: [PATCH v2] x86/PVH: restore VMX APIC assist for Dom0

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.09.2022 16:32, Roger Pau Monné wrote:
> On Mon, Sep 26, 2022 at 11:58:34AM +0200, Jan Beulich wrote:
>> I don't expect it was intended to default PVH Dom0 to "no assist" mode.
>> Introduce command line (sub-)options allowing to suppress enabling of
>> the assists, paralleling the guest config settings for DomU, but restore
>> the defaulting to "enabled".
>>
>> Fixes: 2ce11ce249a3 ("x86/HVM: allow per-domain usage of hardware 
>> virtualized APIC")
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.

>> ---
>> v2: Guard the setting of XEN_X86_ASSISTED_X{,2}APIC by assists actually
>>     being available.
>> ---
>> Besides the issue caused here (the manifestation of which appears to
>> correlate with the other fallout Andrew is trying to deal with) I'm
>> observing further warnings, but I guess these have been there for some
>> time (perhaps forever): When parsing AML and encountering the objects
>> describing the CPUs, Linux would find entries with the original APIC
>> IDs. If those don't match the ones we assign in pvh_setup_acpi_madt(),
>> the kernel will wrongly consider the entries to describe further CPUs,
>> which it therefore would deem hot-pluggable. This again results in
>> warnings, this time "NR_CPUS/possible_cpus limit of ... reached".
> 
> Hm, I'm handling this differently on FreeBSD AFAICT, by using a Xen
> specific driver for the Processor objects when running as dom0, which
> replaces the usage of the native driver.  The only function of that
> driver being the uploading of the performance states in the Processor
> object to Xen.
> 
> I think we ought to do something similar in Linux and just use the
> Processor objects in order to upload the performance related data to
> Xen, but ignore anything else.
> 
> What happens on PV when the number of vCPU available for dom0 is
> smaller than the number of physical CPUs?  Does it also consider the
> unmatched Processor AML objects to be hotpluggable CPUs?

I have to admit that I don't recall for sure, and I'd rather not write
something I'm not sure of.

>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -767,7 +767,8 @@ Specify the bit width of the DMA heap.
>>  
>>  ### dom0
>>      = List of [ pv | pvh, shadow=<bool>, verbose=<bool>,
>> -                cpuid-faulting=<bool>, msr-relaxed=<bool> ]
>> +                cpuid-faulting=<bool>, msr-relaxed=<bool>,
>> +                assisted-xapic=<bool>, assisted-x2apic=<bool> ]
>>  
>>      Applicability: x86
>>  
>> @@ -828,6 +829,10 @@ Controls for how dom0 is constructed on
>>  
>>      If using this option is necessary to fix an issue, please report a bug.
>>  
>> +*   The `assisted-xapic` and `assisted-x2apic` options, defaulting to true,
>> +    allow disabling of the respective hardware assists.  These are 
>> applicable
>> +    to PVH Dom0 only, and their effect is limited to VT-x.
> 
> Explicitly mentioning VT-x here is likely to become stale if AMD is
> also updated to support the options.  I might suggest to leave it out,
> albeit I won insist if you have a strong opinion about it.

At this point the statement expresses reality. Imo the half sentence
wants dropping when AMD gains respective functionality.

Jan