Re: [PATCH v2 07/10] xen/arm: Release maintenance interrupt when CPU is hot-unplugged

From: Julien Grall <julien.grall@arm.com>
To: Mirela Simonovic <mirela.simonovic@aggios.com>
Cc: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>,
	Andre Przywara <andre.przywara@arm.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Tim Deegan <tim@xen.org>, Xen Devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH v2 07/10] xen/arm: Release maintenance interrupt when CPU is hot-unplugged
Date: Wed, 9 May 2018 12:15:12 +0100	[thread overview]
Message-ID: <14f31734-7975-1ad9-80b4-149df61b1faf@arm.com> (raw)
In-Reply-To: <CAKPH-NhGzza4b4wcG8Tf3FgcjOyprAa89+zhJHQ0Xh=n46aRWg@mail.gmail.com>

On 09/05/18 12:14, Mirela Simonovic wrote:
> Hi Julien,
> 
> On Wed, May 9, 2018 at 1:01 PM, Julien Grall <julien.grall@arm.com> wrote:
>>
>>
>> On 09/05/18 11:10, Mirela Simonovic wrote:
>>>
>>> On Fri, Apr 27, 2018 at 5:12 PM, Julien Grall <julien.grall@arm.com>
>>> wrote:
>>>>
>>>> On 27/04/18 15:38, Mirela Simonovic wrote:
>>>>>
>>>>> On Fri, Apr 27, 2018 at 4:15 PM, Tim Deegan <tim@xen.org> wrote:
>>>>>>
>>>>>> At 10:28 +0100 on 27 Apr (1524824906), Julien Grall wrote:
>>>>>>>
>>>>>>> On 26/04/18 15:23, Tim Deegan wrote:
>>>>>>>>
>>>>>>>> At 11:08 +0100 on 26 Apr (1524740921), Julien Grall wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 20/04/18 13:25, Mirela Simonovic wrote:
>>>>
>>>> While I guess this code makes no harm, it does not do what is expected
>>>> (i.e
>>>> draining the interrupt). I can't see any reason to keep wrong code, we
>>>> should really aim to have code that match the architecture. And better to
>>>> fix it when we discover the problem rather than waiting until we
>>>> rediscovered it later.
>>>>
>>>> So at least a patch to update the code/comment should be done.
>>>>
>>>
>>> I don't feel comfortable removing these 3 lines because I have no way
>>> to test and guarantee that the change will not introduce any issues.
>>
>>
>> The work you are doing (suspend/resume, hotplug) is not easy to test and
>> very subtle to get it right. Testing can only uncover obvious bug on your
>> platform. IMHO, we can only rely on the specifications (ARM ARM, PSCI...)
>> and extensive review of the series (and the code around).
>>
>>> However, if despite all you really want me to remove these lines
>>> within this series I don't have a problem doing that in a separate
>>> patch. Please just confirm the plan.
>>
>>
>> I am quite confident that this code should not be there or at least not in
>> its current form.
>>
> 
> Do you want me to include the removal of that code in this series or not?

It is not necessary to be in that series.

Cheers,

> 
> Thanks,
> Mirela
> 
>> Cheers,
>>
>> --
>> Julien Grall

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel