xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Konrad Rzeszutek Wilk <konrad@darnok.org>
Cc: Boris Ostrovsky <boris.ostrovsky@amd.com>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Jan Beulich <jbeulich@suse.com>,
	Stefan Bader <stefan.bader@canonical.com>
Subject: Re: Workings/effectiveness of the xen-acpi-processor driver
Date: Thu, 3 May 2012 13:08:58 -0400	[thread overview]
Message-ID: <20120503170858.GC9992@phenom.dumpdata.com> (raw)
In-Reply-To: <20120503154620.GB3464@andromeda.dapyr.net>

> > 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.

commit 4bb450ea9dca1b8d845f1b53ab6476615a32badf
Author: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
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@canonical.com>
    Suggested-by:  Boris Ostrovsky <boris.ostrovsky@amd.com>
    [v2: Bit-shift APIC ID by 24 bits]
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

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;

  parent reply	other threads:[~2012-05-03 17:08 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-25 13:00 Workings/effectiveness of the xen-acpi-processor driver Stefan Bader
2012-04-26 15:50 ` Konrad Rzeszutek Wilk
2012-04-26 16:25   ` Stefan Bader
2012-04-26 17:04     ` Konrad Rzeszutek Wilk
2012-05-06 15:23       ` Pasi Kärkkäinen
2012-05-07 17:33         ` Konrad Rzeszutek Wilk
2012-05-07 17:44           ` Pasi Kärkkäinen
2012-05-01 20:02     ` Konrad Rzeszutek Wilk
2012-05-01 22:35       ` Boris Ostrovsky
2012-05-01 22:54         ` Konrad Rzeszutek Wilk
2012-05-02  0:47           ` Konrad Rzeszutek Wilk
2012-05-02  1:11             ` Boris Ostrovsky
2012-05-02  9:19               ` Jan Beulich
2012-05-02 14:56           ` Stefan Bader
2012-05-02  8:36         ` Stefan Bader
2012-05-02 15:01         ` Stefan Bader
2012-05-02 16:08           ` Konrad Rzeszutek Wilk
2012-05-02 17:06             ` Boris Ostrovsky
2012-05-02 17:14               ` Konrad Rzeszutek Wilk
2012-05-02 21:31                 ` Boris Ostrovsky
2012-05-02 21:41                   ` Konrad Rzeszutek Wilk
2012-05-02 22:09                     ` Boris Ostrovsky
2012-05-03  6:55                       ` Stefan Bader
2012-05-03 10:00                         ` Stefan Bader
2012-05-03 12:58                       ` Stefan Bader
2012-05-03 14:47                         ` Stefan Bader
2012-05-03 15:46                           ` Konrad Rzeszutek Wilk
2012-05-03 17:02                             ` Boris Ostrovsky
2012-05-03 17:08                             ` Konrad Rzeszutek Wilk [this message]
2012-05-04  8:00                               ` Stefan Bader
2012-05-03 16:14                       ` Konrad Rzeszutek Wilk
2012-05-02 21:29             ` Stefan Bader
2012-05-02  8:22       ` Stefan Bader

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120503170858.GC9992@phenom.dumpdata.com \
    --to=konrad.wilk@oracle.com \
    --cc=boris.ostrovsky@amd.com \
    --cc=jbeulich@suse.com \
    --cc=konrad@darnok.org \
    --cc=stefan.bader@canonical.com \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).