Re: [PATCH v4 2/4] xen/arm: Handle cases when hardware_domain is NULL

[Julien Grall <julien@xen.org>]

Hi Luca,

On 14/04/2021 12:29, Luca Fancellu wrote:
>> On 14 Apr 2021, at 12:16, Julien Grall <julien@xen.org> wrote:
>>
>> Hi Luca,
>>
>> On 14/04/2021 10:14, Luca Fancellu wrote:
>>> Among the common and arm codebase there are few cases where
>>> the hardware_domain variable is checked to see if the current
>>> domain is equal to the hardware_domain, change this cases to
>>> use is_hardware_domain() function instead. >
>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>>> ---
>>> v4 changes:
>>> - removed unneeded check for domain NULL from is_hardware_domain
>>>    introduced in v3
>>
>> After this change, this patch is only avoid to open-code is_hardware_domain(). Although, it adds an extra speculation barrier.
>>
>> I am not against the change, however I think the commit message needs to updated to match what the patch is doing.
>>
>> Can you propose a new commit message?
> 
> Hi Julien,
> 
> Yes I agree, what about:
> 
> xen/arm: Reinforce use of is_hardware_domain
> 
> Among the common and arm codebase there are few cases where

I would drop 'common' because you are only modifying the arm codebase.

> the hardware_domain variable is checked to see if the current
> domain is equal to the hardware_domain, change this cases to
> use is_hardware_domain() function instead.


> In the eventuality that hardware_domain is NULL, is_hardware_domain
> will return false because an analysis of the common and arm codebase
> shows that is_hardware_domain is called always with a non NULL
> domain pointer.

This paragraph seems to come out of the blue. I would drop it.

How about:

"
There are a few places on Arm where we use pretty much an open-coded 
version of is_hardware_domain(). The main difference, is the helper will 
also block speculation (not yet implemented on Arm).

The existing users are not in hot path, so blocking speculation would 
not hurt when it is implemented. So remove the open-coded version within 
the arm codebase.
"

If you are happy with the commit message, I will commit it the series 
tomorrow (to give an opportunity to Stefano to review).

Cheers,

-- 
Julien Grall