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 18:09:26 -0400 Message-ID: <4FA1B096.5010009@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> <4FA1A79B.5040701@amd.com> <20120502214147.GA7670@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: <20120502214147.GA7670@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" , Stefan Bader , Jan Beulich List-Id: xen-devel@lists.xenproject.org On 05/02/2012 05:41 PM, Konrad Rzeszutek Wilk wrote: > On Wed, May 02, 2012 at 02:31:07PM -0700, Boris Ostrovsky wrote: >> 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? > > The issue was only with the bootup CPU - so was using the Xen's > bootup CPU number - which is zero (as is Linux's). I agree that for this particular problem this may be sufficient. My concern is that in the future someone may decide to use apic_read(APIC_ID) or read_apic_id() for some other purpose and they won't get expected result (i.e. on all CPUs they will get the same answer). > >> >>> >>>> 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? > > if (!xen_initial_domain() || smp_processor_id()) I don't see this line --- neither in the mainline nor in your kernel. Which kernel and which routine is this in? BTW, this patch doesn't quite work, xen-acpi-processor driver fails to load with the same error as before. I'll look at this tomorrow more carefully. -boris > > >> >> -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) >>>>> >>>> >>> >> >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> http://lists.xen.org/xen-devel >