Re: Question about apic ipi interface

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

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

On 23.04.2013 14:23, Stefan Bader wrote:
> On 23.04.2013 14:15, Ben Guthro wrote:
>> On Tue, Apr 23, 2013 at 11:07 AM, Stefan Bader
>> <stefan.bader@canonical.com> wrote:
>>> I was looking at some older patch and there is one thing I do not understand.
>>>
>>> commit f447d56d36af18c5104ff29dcb1327c0c0ac3634
>>>     xen: implement apic ipi interface
>>>
>>> Specifically there the implementation of xen_send_IPI_mask_allbutself().
>>>
>>> void xen_send_IPI_mask_allbutself(const struct cpumask *mask,
>>>                                 int vector)
>>> {
>>>         unsigned cpu;
>>>         unsigned int this_cpu = smp_processor_id();
>>>
>>>         if (!(num_online_cpus() > 1))
>>>                 return;
>>>
>>>         for_each_cpu_and(cpu, mask, cpu_online_mask) {
>>>                 if (this_cpu == cpu)
>>>                         continue;
>>>
>>>                 xen_smp_send_call_function_single_ipi(cpu);
>>>         }
>>> }
>>>
>>> Why is this using xen_smp_send_call_function_single_ipi()? This dumps the
>>> supplied vector and always uses XEN_CALL_FUNCTION_SINGLE_VECTOR. In contrast the
>>> xen_send_IPI_all() and xen_send_IPI_self() keep the (mapped) vector.
>>>
>>> Mildly wondering about whether call function would need special casing (just
>>> because xen_smp_send_call_function_ipi() is special). But I don't have the big
>>> picture there.
>>>
>>
>> Adding Lin Ming here, since this was an evolution of an incomplete
>> implementation of mine that was
>> ultimately used in a larger context, outside of my original use case
>> for it (kgdb of dom0) that ultimately
>> gave me credit for this part of the patch, as part of a larger series.
>>
>> I must admit that I don't recall the reasoning, if there was one.
>> It may be an oversight.
>>
>> This was the original (incomplete) patch, in context:
>> http://markmail.org/message/d6ca5zfdmiqipurt
>>
>>
>> Are you seeing issues with the code, or just doing code inspection?
> 
> No issues, I was just looking at the patch because we were asked to backport it
> to fix another issue (access to the apic IPI functions without checking whether
> there is a pointer). Since things did work in most cases before, maybe there is
> no real usage. :) I was just curious.
> 
> Stefan

Oh, and while looking at it... why does arch/x86/xen/smp.h includes a definition
for physflat_send_IPI_allbutself? (introduced by the same change. If its not
hidden by some hideous macro magic there is only one place that needs it and
that is in the same file (apic_flat_64.c).

> 
>>
>> Ben
>>
> 
> 
> 
> 
> _______________________________________________
> 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: 899 bytes --]

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

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