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

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

[Jan Beulich]

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@suse.com>
---
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".

--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -753,7 +753,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
 
@@ -814,6 +815,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.
+
 ### dom0-cpuid
     = List of comma separated booleans
 
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -293,6 +293,12 @@ static int __init cf_check parse_dom0_pa
             opt_dom0_cpuid_faulting = val;
         else if ( (val = parse_boolean("msr-relaxed", s, ss)) >= 0 )
             opt_dom0_msr_relaxed = val;
+#ifdef CONFIG_HVM
+        else if ( (val = parse_boolean("assisted-xapic", s, ss)) >= 0 )
+            opt_dom0_assisted_xapic = val;
+        else if ( (val = parse_boolean("assisted-x2apic", s, ss)) >= 0 )
+            opt_dom0_assisted_x2apic = val;
+#endif
         else
             rc = -EINVAL;
 
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -55,6 +55,9 @@
  */
 #define HVM_VM86_TSS_SIZE 265
 
+bool __initdata opt_dom0_assisted_xapic = true;
+bool __initdata opt_dom0_assisted_x2apic = true;
+
 static unsigned int __initdata acpi_intr_overrides;
 static struct acpi_madt_interrupt_override __initdata *intsrcovr;
 
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -68,6 +68,14 @@ extern bool opt_dom0_verbose;
 extern bool opt_dom0_cpuid_faulting;
 extern bool opt_dom0_msr_relaxed;
 
+#ifdef CONFIG_HVM
+extern bool opt_dom0_assisted_xapic;
+extern bool opt_dom0_assisted_x2apic;
+#else
+#define opt_dom0_assisted_xapic false
+#define opt_dom0_assisted_x2apic false
+#endif
+
 #define max_init_domid (0)
 
 #endif
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -784,6 +784,11 @@ static struct domain *__init create_dom0
 
         dom0_cfg.arch.emulation_flags |=
             XEN_X86_EMU_LAPIC | XEN_X86_EMU_IOAPIC | XEN_X86_EMU_VPCI;
+
+        if ( opt_dom0_assisted_xapic )
+            dom0_cfg.arch.misc_flags |= XEN_X86_ASSISTED_XAPIC;
+        if ( opt_dom0_assisted_x2apic )
+            dom0_cfg.arch.misc_flags |= XEN_X86_ASSISTED_X2APIC;
     }
 
     if ( iommu_enabled )

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

[Roger Pau Monné]

On Tue, Aug 23, 2022 at 01:56:22PM +0200, Jan Beulich wrote:
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -55,6 +55,9 @@
>   */
>  #define HVM_VM86_TSS_SIZE 265
>  
> +bool __initdata opt_dom0_assisted_xapic = true;
> +bool __initdata opt_dom0_assisted_x2apic = true;

Defaulting those to true unconditionally is troublesome, as the check
in arch_sanitise_domain_config() will fail if either
assisted_x{2,}apic_available is not true, and dom0 domain creation
will fail, so...

> +
>  static unsigned int __initdata acpi_intr_overrides;
>  static struct acpi_madt_interrupt_override __initdata *intsrcovr;
>  
> --- a/xen/arch/x86/include/asm/setup.h
> +++ b/xen/arch/x86/include/asm/setup.h
> @@ -68,6 +68,14 @@ extern bool opt_dom0_verbose;
>  extern bool opt_dom0_cpuid_faulting;
>  extern bool opt_dom0_msr_relaxed;
>  
> +#ifdef CONFIG_HVM
> +extern bool opt_dom0_assisted_xapic;
> +extern bool opt_dom0_assisted_x2apic;
> +#else
> +#define opt_dom0_assisted_xapic false
> +#define opt_dom0_assisted_x2apic false
> +#endif
> +
>  #define max_init_domid (0)
>  
>  #endif
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -784,6 +784,11 @@ static struct domain *__init create_dom0
>  
>          dom0_cfg.arch.emulation_flags |=
>              XEN_X86_EMU_LAPIC | XEN_X86_EMU_IOAPIC | XEN_X86_EMU_VPCI;
> +
> +        if ( opt_dom0_assisted_xapic )
> +            dom0_cfg.arch.misc_flags |= XEN_X86_ASSISTED_XAPIC;
> +        if ( opt_dom0_assisted_x2apic )
> +            dom0_cfg.arch.misc_flags |= XEN_X86_ASSISTED_X2APIC;

...the values of assisted_x{2,}apic_available need to be taken into
account here in order to avoid requesting an invalid configuration.

I could swear I have checked PVH dom0 interaction when reviewing the
original patch, but I clearly missed it.

Thanks, Roger.