Received-SPF: pass (google.com: domain of linux-kernel+bounces-196330-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <07b715e3-72d0-4943-bfe8-80a9739eeb75@amd.com>
Date: Fri, 31 May 2024 11:39:52 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/1] iommu: Make iommu_sva_domain_alloc() static
To: Baolu Lu <baolu.lu@linux.intel.com>, Joerg Roedel <joro@8bytes.org>,
 Will Deacon <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>,
 Jason Gunthorpe <jgg@ziepe.ca>, Kevin Tian <kevin.tian@intel.com>,
 Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: iommu@lists.linux.dev, linux-kernel@vger.kernel.org
References: <20240528045458.81458-1-baolu.lu@linux.intel.com>
 <fdc3bffa-7149-48a4-949a-dd469c6c2726@amd.com>
 <cb8f4a3f-75f4-436a-96dc-301b6d65a55c@linux.intel.com>
Content-Language: en-US
From: Vasant Hegde <vasant.hegde@amd.com>
In-Reply-To: <cb8f4a3f-75f4-436a-96dc-301b6d65a55c@linux.intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?ODAvNG1QSEdkc3VmdDNjay9JajhUZFdzYWZaYTF0UXNYbnpSR3RwS2xRemlQ?=
 =?utf-8?B?NjJrbVg3dmFycUI4SThZTG5UL2FsWjBabC82a250Zi9GSlRRM2lZdTQyaDBN?=
 =?utf-8?B?WXdMdmR1ZEJCOWFabW8wWGRQeEpzbm1uSytVejJFaTZlOHVkOXRoSE5PR1dV?=
 =?utf-8?B?dm1sN29DWjJ0WkxyQ1VUL2lZYlg2QTRZWmdCSXk5RlFlUE81UUtYLzRnZ0xE?=
 =?utf-8?B?MktJYjEwYmYzYTV5ZEdQNm5WVFlrOHd5VlJtZFA2YXpBZitKaUxadVhzQVZt?=
 =?utf-8?B?aE9QdzdWaG5yWnhVWGg3RGVHZGQxWUVQenlmT2hXZjZDOWtLaUEwVFN1VlUx?=
 =?utf-8?B?akNORWh1dEVxand0K2RYZ2YwQ0RlbmdHM05tS3YyRVRGeEZqUkM4a2VHSTR1?=
 =?utf-8?B?SEt0MWg2Z1Jmc2hLbWgwS3cvYWl6N2tWYjdaU2pHSk5NUGIyLzcyVlhmTVJW?=
 =?utf-8?B?OEpGakd6WjdNRWtTbnFvV3hMYW9Xa1kzaThyb1JiVWdFMTQvWWZFQXptaFZV?=
 =?utf-8?B?UlZONVNrZEZlTkNUd1h5WSt0Zng2SFFNZmo3SHJlem1RaDkxUkNwR3R6bVhZ?=
 =?utf-8?B?NmNaaktYQ1pVOUxsODhLTXBlQ3NuRjJpVGtUWk1pcEhOZW1tbldMdnRCT0R0?=
 =?utf-8?B?dnpzb25wNXpVT21DY0dKb0x3MU53NENUVGp6TkV6RWx6Vm9pT2h0UlVCOG1U?=
 =?utf-8?B?VUJleE1oRGJmU21iaWlqT29TMEtuQm45R1RHbEIzakVxNjBNNG9LaHZDbVZD?=
 =?utf-8?B?blpQYnhoOUFUTXlYRlBObFJRci94VWdLQmUraU9vcDM0ZzBmMGg1SlZyRDdY?=
 =?utf-8?B?dGVHTEpITTVSWHdDUTNLT0V1VjlPMFoyNWZkbmw2M2VxNEtPeU9CMUhLekVT?=
 =?utf-8?B?QVMweEJtWGQ2QmhKQzlON2FYejFIRmcrb2FnNWZEWE93RTZxTmZPMFJLY05I?=
 =?utf-8?B?Ykl1UE9xbmdYWmVETmt4RU9Dc0JoMHVIbVdwZFVFajdFczkzZjNUWG10RVRq?=
 =?utf-8?B?d2RhMnV5bzNFTVFDZVVKejNNcStnZkVVMlNTNTlYZndLZ3pZOHpMSVNMZFBE?=
 =?utf-8?B?K2tTUldJbFBIQnVlOUtHa3BqZTMvdWVIbW9udHJUbG9tVWxVU3AwOVRHZTFt?=
 =?utf-8?B?ajMvODNUb0lvNjNvVDNXM1dndExOeEs0czlzWVpScGthcGw5Qm9TMmNpRXh4?=
 =?utf-8?B?b3FsSGhWamoxWjhsUFJQekZzeUtpaGlJU1lUMUZMbTJudkZKaVJCdk5CTXdU?=
 =?utf-8?B?WWx2TFVOQm1ITm5xQjJRSDRQU3NreERRZmQzUFg3Um5tYXZoVk01dlh5T2N2?=
 =?utf-8?B?SnA4SGtVZkRXOWZ1dmdxUkNFZWNLenBSeDBRU2wyZytYTkFKc3RFUTlvY1pK?=
 =?utf-8?B?WTJsNHhFaDErbHR1VUthbE9nbHBjekZWMUoyaHRBRy9RVmlPS3I2YUMwYlZw?=
 =?utf-8?B?aHJxWTZ5ZHE5eXNRczhSZ1k2eGhMYnp0Y01iN0lkWlJTTm9YMmNTQ3JFam80?=
 =?utf-8?B?TjY5T0ZldU5Bd3VVei9oc1JLR0tWRll6bzkweEZGdzB4V3BFNWYvZDg2N3J4?=
 =?utf-8?B?T0NtQ3dQdEQwd3g3NFNlWkowUWxyMzYrNGFTWFY1aW9Oc3J6VkRJVmduZzA3?=
 =?utf-8?B?K1ovelJ3SnliTjQxNmhiekI1R0VBVnp0aWdNaWFVVnZSUUx4eEhlN2JQZkRP?=
 =?utf-8?B?YS83dEJUdHRXTUFGNnVpY3lhMTV1S2Rkd25oQ29LSVEyZUVNdTZWTldrSks2?=
 =?utf-8?B?TWhIem96azE1bGJwaHRpdnpEenNyNms5SjJESy9xT3loV0RvL1IvZGgyM1Bq?=
 =?utf-8?B?dzRjM3JmZWk2TGI5VWF2cXJwM0Y1Q2h6RTl5UmoreWhDc1VsaXI4R0g5V3c2?=
 =?utf-8?B?enlDNGN4VDEwbEQ0OGMwVXdNNitsZVVpYzYzUjJQcCtwaVB2clo3Y3Fia3Bp?=
 =?utf-8?B?V05mU0NHV1FVc1pzakwyanNOTWtOWkl1eEF6K1dmeE80cmx5S1daaFBMZnRi?=
 =?utf-8?B?U1cyQVhSNlhJbU5IODVWNWRRVTJIMWNrVmVOZTcyNmJsNUYrNXlxdFVFZE9n?=
 =?utf-8?B?OGp3c21YTkx6bXYwV1QwN1NvcE4rTCtvbzJZSStwVE5xTkxrU043bTZLTlho?=
 =?utf-8?Q?iLTRwKzi2S+3/TYrMlUQbizh+?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 27910867-1cdc-4979-e895-08dc81384de8
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB6048.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 May 2024 06:10:00.8270
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: m+sMu7T3xRyjyui1ZFlZE8Mfq2mZg8gSj76EeK4R/aBPw01uhiTu6LmXYGqZQU8F4xPVQiI7A4TtnBORDT7DVw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB7815


On 5/31/2024 6:29 AM, Baolu Lu wrote:
> On 5/30/24 4:52 PM, Vasant Hegde wrote:
>> On 5/28/2024 10:24 AM, Lu Baolu wrote:
>>> iommu_sva_domain_alloc() is only called in iommu-sva.c, hence make it
>>> static.
>>>
>>> On the other hand, iommu_sva_domain_alloc() should not return NULL anymore
>>> after commit <80af5a452024> ("iommu: Add ops->domain_alloc_sva()"), the
>>> removal of inline code avoids potential confusion.
>>>
>>> Fixes: 80af5a452024 ("iommu: Add ops->domain_alloc_sva()")
>>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
>>> ---
>>>   include/linux/iommu.h     | 8 --------
>>>   drivers/iommu/iommu-sva.c | 6 ++++--
>>>   2 files changed, 4 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>> index 7bc8dff7cf6d..5cdd3d41b87b 100644
>>> --- a/include/linux/iommu.h
>>> +++ b/include/linux/iommu.h
>>> @@ -1527,8 +1527,6 @@ struct iommu_sva *iommu_sva_bind_device(struct device
>>> *dev,
>>>                       struct mm_struct *mm);
>>>   void iommu_sva_unbind_device(struct iommu_sva *handle);
>>>   u32 iommu_sva_get_pasid(struct iommu_sva *handle);
>>> -struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
>>> -                        struct mm_struct *mm);
>>>   #else
>>>   static inline struct iommu_sva *
>>>   iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
>>> @@ -1553,12 +1551,6 @@ static inline u32 mm_get_enqcmd_pasid(struct mm_struct
>>> *mm)
>>>   }
>>>     static inline void mm_pasid_drop(struct mm_struct *mm) {}
>>> -
>>> -static inline struct iommu_domain *
>>> -iommu_sva_domain_alloc(struct device *dev, struct mm_struct *mm)
>>> -{
>>> -    return NULL;
>>> -}
>>>   #endif /* CONFIG_IOMMU_SVA */
>>>     #ifdef CONFIG_IOMMU_IOPF
>>> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
>>> index 18a35e798b72..25e581299226 100644
>>> --- a/drivers/iommu/iommu-sva.c
>>> +++ b/drivers/iommu/iommu-sva.c
>>> @@ -10,6 +10,8 @@
>>>   #include "iommu-priv.h"
>>>     static DEFINE_MUTEX(iommu_sva_lock);
>>> +static struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
>>> +                           struct mm_struct *mm);
>> If we move move iommu_sva_domain_alloc() before iommu_sva_bind_device() then we
>> can avoid this forward declaration.
> 
> Yeah, but that means moving 144 lines of code. That's why I went with
> this approach

Ah right. I didn't realized we have 'iopf_handler' in iommu_sva_domain_alloc().

-Vasant