Re: [XEN][PATCH v8 09/19] xen/iommu: Move spin_lock from iommu_dt_device_is_assigned to caller

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.08.2023 21:52, Vikram Garhwal wrote:
> On Thu, Aug 17, 2023 at 09:05:44AM +0200, Jan Beulich wrote:
>> On 17.08.2023 02:39, Vikram Garhwal wrote:
>>> --- /dev/null
>>> +++ b/xen/include/xen/iommu-private.h
>>
>> I don't think private headers should live in include/xen/. Judging from only
>> the patches I was Cc-ed on, ...
> Thank you for suggestion. Do you where can i place it then?

Irrespective of Julien's reply (potentially rendering this moot), see ...

> Please see another comment down regarding who might be using this function.
>>
>>> @@ -0,0 +1,28 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * xen/iommu-private.h
>>> + */
>>> +#ifndef __XEN_IOMMU_PRIVATE_H__
>>> +#define __XEN_IOMMU_PRIVATE_H__
>>> +
>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>> +#include <xen/device_tree.h>
>>> +
>>> +/*
>>> + * Checks if dt_device_node is assigned to a domain or not. This function
>>> + * expects to be called with dtdevs_lock acquired by caller.
>>> + */
>>> +bool_t iommu_dt_device_is_assigned_locked(const struct dt_device_node 
>>> *dev);
>>> +#endif
>>
>> ... I don't even see the need for the declaration, as the function is used
>> only from the file also defining it. But of course if there is a use
>> elsewhere (in Arm-only code, as is suggested by the description here), then
>> the header (under a suitable name) wants to live under drivers/passthrough/

... my earlier reply.

Jan

>> (and of course be included only from anywhere in that sub-tree).
>>
> This is also use in smmu.c:arm_smmu_dt_remove_device_legacy(). This is added 
> in
> 12/19 patch(xen/smmu: Add remove_device callback for smmu_iommu ops).
> 
> I will make sure to cc you for all the patches in v9 series. I plan to send
> it today.
> 
>> Jan