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

[Stefan Bader <stefan.bader@canonical.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3721 bytes --]

On 03.05.2012 00:09, Boris Ostrovsky wrote:
> 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?

This is in the inline patch Konrad asked me to check.
> 
> 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.

It does fail but at least for me it seems slightly different. I do the modprobe
after turning on all acpi debugging levels and layers. And without any change
there are queries visible. With that patch the driver just fails but there are
no queries. I plan to have another go with messages sprinkled to all exit paths
today, too (was just a bit too late and two pints later yesterday).


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



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 900 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel