[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] Workings/effectiveness of the xen-acpi-processor driver
On 03.05.2012 19:08, Konrad Rzeszutek Wilk wrote: >>> Hmmm, so xen_apic_read is still correct... >>> >>> [ 0.000000] ACPI: Local APIC address 0xfee00000 >>> [ 0.000000] xxx xen_apic_read(20) >>> [ 0.000000] xxx xen_apic_read -> 10 >>> [ 0.000000] boot_cpu_physical_apicid = 0 >>> [ 0.000000] xxx xen_apic_read(30) >>> [ 0.000000] +- apic version = 10 >>> >>> there seems to be a slightly strange tweak (at least for me) in >>> read_apic_id... >>> >>> static inline unsigned int read_apic_id(void) >>> { >>> unsigned int reg; >>> >>> reg = apic_read(APIC_ID); // calls apic->read(reg) >>> >>> return apic->get_apic_id(reg); >> >> Duh!! Let me spin out a new patch that will do this. > > Meaning bit-shift it. We ended up doing 10 >> 24 (get_apic_id does that) > which results in zero. So lets be a bit more cautious and over-write > the get_apic_id and set_apic_id and as well do the proper bit-shifting. > I can confirm that with this patch applied I can load the xen-acpi-processor driver and see P-states changing in xenpm. No BIOS bug messages. Also tested on the i7 and it does still work. I am attaching the dmesg output of both runs. On the i7 the xen apic functions are called in two places and for cpu#0 only. On AMD, I see additional call later and for all cpus while enabling them. apic->read bails out, but apic->get_apic_id returns 0 (though I could see no immediate problem from that). Maybe that info is helpful for the next stage. -Stefan > commit 4bb450ea9dca1b8d845f1b53ab6476615a32badf > Author: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> > Date: Wed May 2 15:04:51 2012 -0400 > > xen/apic: Return the APIC ID (and version) for CPU 0. > > On x86_64 on AMD machines where the first APIC_ID is not zero, we get: > > ACPI: LAPIC (acpi_id[0x01] lapic_id[0x10] enabled) > BIOS bug: APIC version is 0 for CPU 1/0x10, fixing up to 0x10 > BIOS bug: APIC version mismatch, boot CPU: 0, CPU 1: version 10 > > which means that when the ACPI processor driver loads and > tries to parse the _Pxx states it fails to do as, as it > ends up calling acpi_get_cpuid which does this: > > for_each_possible_cpu(i) { > if (cpu_physical_id(i) == apic_id) > return i; > } > > And the bootup CPU, has not been found so it fails and returns -1 > for the first CPU - which then subsequently in the loop that > "acpi_processor_get_info" does results in returning an error, which > means that "acpi_processor_add" failing and per_cpu(processor) > is never set (and is NULL). > > That means that when xen-acpi-processor tries to load (much much > later on) and parse the P-states it gets -ENODEV from > acpi_processor_register_performance() (which tries to read > the per_cpu(processor)) and fails to parse the data. > > Reported-by: Stefan Bader <stefan.bader@xxxxxxxxxxxxx> > Suggested-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxx> > [v2: Bit-shift APIC ID by 24 bits] > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> Tested-by: Stefan Bader <stefan.bader@xxxxxxxxxxxxx> > > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c > index a8f8844..63d6c22 100644 > --- a/arch/x86/xen/enlighten.c > +++ b/arch/x86/xen/enlighten.c > @@ -53,6 +53,9 @@ > #include <asm/processor.h> > #include <asm/proto.h> > #include <asm/msr-index.h> > +#ifdef CONFIG_NUMA > +#include <asm/numa.h> > +#endif > #include <asm/traps.h> > #include <asm/setup.h> > #include <asm/desc.h> > @@ -809,9 +812,40 @@ static void xen_io_delay(void) > } > > #ifdef CONFIG_X86_LOCAL_APIC > +static unsigned long xen_set_apic_id(unsigned int x) > +{ > + WARN_ON(1); > + return x; > +} > +static unsigned int xen_get_apic_id(unsigned long x) > +{ > + return (((x)>>24) & 0xFFu); > +} > static u32 xen_apic_read(u32 reg) > { > - return 0; > + struct xen_platform_op op = { > + .cmd = XENPF_get_cpuinfo, > + .interface_version = XENPF_INTERFACE_VERSION, > + .u.pcpu_info.xen_cpuid = 0, > + }; > + int ret = 0; > + > + /* Shouldn't need this as APIC is turned off for PV, and we only > + * get called on the bootup processor. But just in case. */ > + if (!xen_initial_domain() || smp_processor_id()) > + return 0; > + > + if (reg == APIC_LVR) > + return 0x10; > + > + if (reg != APIC_ID) > + return 0; > + > + ret = HYPERVISOR_dom0_op(&op); > + if (ret) > + return 0; > + > + return op.u.pcpu_info.apic_id << 24; > } > > static void xen_apic_write(u32 reg, u32 val) > @@ -849,6 +883,8 @@ static void set_xen_basic_apic_ops(void) > apic->icr_write = xen_apic_icr_write; > apic->wait_icr_idle = xen_apic_wait_icr_idle; > apic->safe_wait_icr_idle = xen_safe_apic_wait_icr_idle; > + apic->set_apic_id = xen_set_apic_id; > + apic->get_apic_id = xen_get_apic_id; > } > > #endif > @@ -1294,6 +1330,9 @@ asmlinkage void __init xen_start_kernel(void) > */ > acpi_numa = -1; > #endif > +#if defined(CONFIG_NUMA) && defined(CONFIG_X86_32) > + numa_off = 1; > +#endif > > pgd = (pgd_t *)xen_start_info->pt_base; > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxx > http://lists.xen.org/xen-devel Attachment:
dmesg.i7.txt Attachment:
dmesg.opt.txt Attachment:
signature.asc _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |