[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86/x2apic: introduce a mixed physical/cluster mode
On 2023-11-07 11:11, Elliott Mitchell wrote: > On Mon, Oct 30, 2023 at 04:27:22PM +01 00, Roger Pau Monné wrote: > > On Mon, Oct 30, 2023 at 07:50:27AM -0700, Elliott Mitchell wrote: > > > On Tue, Oct 24, 2023 at 03:51:50PM +0200, Roger Pau Monne wrote: > > > > diff --git a/xen/arch/x86/genapic/x2apic.c > > > > b/xen/arch/x86/genapic/x2apic.c > > > > index 707deef98c27..15632cc7332e 100644 > > > > --- a/xen/arch/x86/genapic/x2apic.c > > > > +++ b/xen/arch/x86/genapic/x2apic.c > > > > @@ -220,38 +239,56 @@ static struct notifier_block x2apic_cpu_nfb = { > > > > static int8_t __initdata x2apic_phys = -1; > > > > boolean_param("x2apic_phys", x2apic_phys); > > > > > > > > +enum { > > > > + unset, physical, cluster, mixed > > > > +} static __initdata x2apic_mode = unset; > > > > + > > > > +static int __init parse_x2apic_mode(const char *s) > > > > +{ > > > > + if ( !cmdline_strcmp(s, "physical") ) > > > > + x2apic_mode = physical; > > > > + else if ( !cmdline_strcmp(s, "cluster") ) > > > > + x2apic_mode = cluster; > > > > + else if ( !cmdline_strcmp(s, "mixed") ) > > > > + x2apic_mode = mixed; > > > > + else > > > > + return EINVAL; > > > > + > > > > + return 0; > > > > +} > > > > +custom_param("x2apic-mode", parse_x2apic_mode); > > > > + > > > > const struct genapic *__init apic_x2apic_probe(void) > > > > { > > > > - if ( x2apic_phys < 0 ) > > > > + /* x2apic-mode option has preference over x2apic_phys. */ > > > > + if ( x2apic_phys >= 0 && x2apic_mode == unset ) > > > > + x2apic_mode = x2apic_phys ? physical : cluster; > > > > + > > > > + if ( x2apic_mode == unset ) > > > > { > > > > - /* > > > > - * Force physical mode if there's no (full) interrupt > > > > remapping support: > > > > - * The ID in clustered mode requires a 32 bit destination > > > > field due to > > > > - * the usage of the high 16 bits to hold the cluster ID. > > > > - */ > > > > - x2apic_phys = iommu_intremap != iommu_intremap_full || > > > > - (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL) > > > > || > > > > - IS_ENABLED(CONFIG_X2APIC_PHYSICAL); > > > > - } > > > > - else if ( !x2apic_phys ) > > > > - switch ( iommu_intremap ) > > > > + if ( acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL ) > > > > { > > > > > > Could this explain the issues with recent AMD processors/motherboards? > > > > > > Mainly the firmware had been setting this flag, but Xen was previously > > > ignoring it? > > > > No, not unless you pass {no-}x2apic_phys={false,0} on the Xen command > > line to force logical (clustered) destination mode. > > > > > As such Xen had been attempting to use cluster mode on an > > > x2APIC where that mode was broken for physical interrupts? > > > > No, not realy, x2apic_phys was already forced to true if > > acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL is set on the FADT (I > > just delete that line in this same chunk and move it here). > > Okay, that was from a quick look at the patch. Given the symptoms and > workaround with recent AMD motherboards, this looked suspicious. > > In that case it might be a bug in what AMD is providing to motherboard > manufacturers. Mainly this bit MUST be set, but AMD's implementation > leaves it unset. > > Could also be if the setup is done correctly the bit can be cleared, but > multiple motherboard manufacturers are breaking this. Perhaps the steps > are fragile and AMD needed to provide better guidance. > > > Neowutran, are you still setup to and interested in doing > experimentation/testing with Xen on your AMD computer? Would you be up > for trying the patch here: > > https://lore.kernel.org/xen-devel/20231106142739.19650-1-roger.pau@xxxxxxxxxx/raw > > I have a suspicion this *might* fix the x2APIC issue everyone has been > seeing. > > How plausible would it be to release this as a bugfix/workaround on 4.17? > I'm expecting a "no", but figured I should ask given how widespread the > issue is. > > > -- > (\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/) > \BS ( | ehem+sigmsg@m5p. com PGP 87145445 | ) / > \_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/ > 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445 > I just applied the patch on my setup ( https://lore.kernel.org/xen-devel/20231106142739.19650-1-roger.pau@xxxxxxxxxx/raw ) It seems to fix the x2APIC issue I was having. I only did some quick tests: I tried all the differents values in my bios for the X2APIC settings. Now the system successfully boot in all the cases, without needing manual override of the x2apic_phys/x2apic_mode parameter in boot commandline .
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |