From mboxrd@z Thu Jan  1 00:00:00 1970
From: Anup Patel <anup.patel@linaro.org>
Subject: Re: [PATCH] xen/arm: Fix smp_send_call_function_mask()
	for current CPU
Date: Fri, 15 Aug 2014 15:42:20 +0530
Message-ID: <CALrVBktrOOk-Cw-bMc-3fs9RF6c87BHE+3kDTCazikfq14THyQ@mail.gmail.com>
References: <1408016846-12149-1-git-send-email-anup.patel@linaro.org>
	<alpine.DEB.2.02.1408151023200.2293@kaball.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <alpine.DEB.2.02.1408151023200.2293@kaball.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>, Julien Grall <julien.grall@linaro.org>, "stefano.stabellini" <stefano.stabellini@citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, xen-devel <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

Hi Stefano,

On 15 August 2014 14:58, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> Hi Anup,
> thanks for the patch.
>
>
> On Thu, 14 Aug 2014, Anup Patel wrote:
>> The smp_send_call_function_mask() does not work on Foundation v8
>> model with one CPU. The reason being gicv2_send_SGI() is called
>> with irqmode==SGI_TARGET_LIST and *cpu_mask=0x1 on CPU0 which
>> does not work on Foundation v8 model.
>
> I read the GIC spec again, and this has to be a bug in the Foundation v8
> model. Please make sure to raise this issue with ARM so that they can
> fix it.

I had the same feeling when I cross-checked GIC spec after
reading Julien's comment.

>
>
>> Further, it is really strange that smp_send_call_function_mask()
>> depends on GIC SGIs for calling function on current CPU.
>>
>> This patch fixes smp_send_call_function_mask() for current CPU
>> by directly calling smp_call_function_interrupt() on current CPU.
>> This is very similar to what Xen x86 does.
>
> It could be a reasonable improvement regardless of the Foundation v8
> bug. I would recommend to:
>
> - Avoid mentioning the bug in the Foundation model in the commit
> message, because a bug in an emulator is not a good reason to make
> changes in Xen. If that was the only reason, then we would need to
> introduce the change as a per-platform quirk, like
> PLATFORM_QUIRK_GIC_64K_STRIDE.

Sure, I will update the wordings in patch description as per
your suggestion.

>
> - Clear the smp_processor_id() bit from mask, otherwise we end up
> calling the function twice on the processor we are running on at the
> moment.

Sure, I will update this.

Thanks,
Anup

>
>
>
>> Signed-off-by: Anup Patel <anup.patel@linaro.org>
>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org>
>> ---
>>  xen/arch/arm/smp.c |    7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/xen/arch/arm/smp.c b/xen/arch/arm/smp.c
>> index 30203b8..4fde3f9 100644
>> --- a/xen/arch/arm/smp.c
>> +++ b/xen/arch/arm/smp.c
>> @@ -20,6 +20,13 @@ void smp_send_event_check_mask(const cpumask_t *mask)
>>  void smp_send_call_function_mask(const cpumask_t *mask)
>>  {
>>      send_SGI_mask(mask, GIC_SGI_CALL_FUNCTION);
>> +
>> +    if ( cpumask_test_cpu(smp_processor_id(), mask) )
>> +    {
>> +        local_irq_disable();
>> +        smp_call_function_interrupt();
>> +        local_irq_enable();
>> +    }
>>  }
>>
>>  /*
>> --
>> 1.7.9.5
>>