From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: Workings/effectiveness of the xen-acpi-processor driver Date: Wed, 2 May 2012 17:31:07 -0400 Message-ID: <4FA1A79B.5040701@amd.com> References: <4F97F58A.8090409@canonical.com> <20120426155033.GE26830@phenom.dumpdata.com> <4F9976F8.8040502@canonical.com> <20120501200207.GA15313@phenom.dumpdata.com> <4FA06541.7050607@amd.com> <4FA14C2C.5030104@canonical.com> <20120502160812.GA6611@phenom.dumpdata.com> <4FA1699A.9070405@amd.com> <20120502171415.GA17477@phenom.dumpdata.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20120502171415.GA17477@phenom.dumpdata.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk Cc: "xen-devel@lists.xensource.com" , Jan Beulich , Stefan Bader List-Id: xen-devel@lists.xenproject.org On 05/02/2012 01:14 PM, Konrad Rzeszutek Wilk wrote: > On Wed, May 02, 2012 at 01:06:34PM -0400, Boris Ostrovsky wrote: >> On 05/02/2012 12:08 PM, Konrad Rzeszutek Wilk wrote: >>> 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, >> >> >> Is this always zero? This will probably solve the current problem > > Its a CPU number (not tied in to APIC or ACPI IDs). Why not use CPU number instead of zero here? > >> but I am wondering whether in the future we might hit another bug >> because this routine will return the same APICID for all VCPUs. > > Later on it does a check for 'smp_processor_id()' - and if > that is anything but zero it will bail out. Can you point me to the check you are referring to? -boris > > So this shoudl solve the problem for the bootup processor. >> >> -boris >> >> >>> + }; >>> + 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) >>> >> >