Re: Workings/effectiveness of the xen-acpi-processor driver

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, May 02, 2012 at 05:01:00PM +0200, Stefan Bader wrote:
> On 02.05.2012 00:35, Boris Ostrovsky wrote:
> > On 05/01/2012 04:02 PM, Konrad Rzeszutek Wilk wrote:
> >> On Thu, Apr 26, 2012 at 06:25:28PM +0200, Stefan Bader wrote:
> >>> On 26.04.2012 17:50, Konrad Rzeszutek Wilk wrote:
> >>>> On Wed, Apr 25, 2012 at 03:00:58PM +0200, Stefan Bader wrote:
> >>>>> Since there have been requests about that driver to get backported into 3.2, I
> >>>>> was interested to find out what or how much would be gained by that.
> >>>>>
> >>>>> The first system I tried was an AMD based one (8 core Opteron 6128@2GHz).
> >>>>> Which
> >>>>> was not very successful as the drivers bail out of the init function
> >>>>> because the
> >>>>> first call to acpi_processor_register_performance() returns -ENODEV. There is
> >>>>> some frequency scaling when running without Xen, so I need to do some more
> >>>>> debugging there.
> > 
> > I believe this is caused by the somewhat under-enlightened xen_apic_read():
> > 
> > static u32 xen_apic_read(u32 reg)
> > {
> >         return 0;
> > }
> > 
> > This results in some data, most importantly boot_cpu_physical_apicid, not being
> > set correctly and, in turn, causes x86_cpu_to_apicid to be broken.
> > 
> > On larger AMD systems boot processor is typically APICID=0x20 (I don't have
> > Intel system handy to see how it looks there).
> > 
> > As a quick and dirty test you can try:
> > 
> > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > index edc2448..1f78998 100644
> > --- a/arch/x86/kernel/apic/apic.c
> > +++ b/arch/x86/kernel/apic/apic.c
> > @@ -1781,6 +1781,7 @@ void __init register_lapic_address(unsigned long address)
> >         }
> >         if (boot_cpu_physical_apicid == -1U) {
> >                 boot_cpu_physical_apicid  = read_apic_id();
> > +               boot_cpu_physical_apicid = 32;
> >                 apic_version[boot_cpu_physical_apicid] =
> >                          GET_APIC_VERSION(apic_read(APIC_LVR));
> >         }
> > 
> > 
> > (Set it to whatever APICID on core0 is, I suspect it won't be zero).
> > 
> 
> Indeed, when I hack the above id to be 0x10 (as my dmesg tells) most of the BIOS
> bug messages are gone and the xen-acpi-processor driver successfully loads and
> seems to be switching frequencies ok (just a quick tight loop which made one cpu
> go from P4 to P0).

OK.  Can you try the attached patch pls? It should do the same thing
as what the debug patch does - get the _real_ APIC ID from the hypervisor.
(And as bonus it also removes the annoying:

BIOS bug: APIC version is 0 for CPU 0/0x0, fixing up to 0x10


diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index a8f8844..d816448 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -811,7 +811,29 @@ static void xen_io_delay(void)
 #ifdef CONFIG_X86_LOCAL_APIC
 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;
 }
 
 static void xen_apic_write(u32 reg, u32 val)