linux-kernel - Re: [PATCH v5 5/9] iommufd: Add iommufd fault object

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 20 May 2024 09:15:35 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: "Tian, Kevin" <kevin.tian@...el.com>, Jason Gunthorpe <jgg@...pe.ca>,
 Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
 Robin Murphy <robin.murphy@....com>,
 Jean-Philippe Brucker <jean-philippe@...aro.org>,
 Nicolin Chen <nicolinc@...dia.com>, "Liu, Yi L" <yi.l.liu@...el.com>,
 Jacob Pan <jacob.jun.pan@...ux.intel.com>,
 Joel Granados <j.granados@...sung.com>
Cc: baolu.lu@...ux.intel.com, "iommu@...ts.linux.dev"
 <iommu@...ts.linux.dev>,
 "virtualization@...ts.linux-foundation.org"
 <virtualization@...ts.linux-foundation.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 5/9] iommufd: Add iommufd fault object

On 5/15/24 4:37 PM, Tian, Kevin wrote:
>> @@ -395,6 +396,8 @@ struct iommufd_device {
>>   	/* always the physical device */
>>   	struct device *dev;
>>   	bool enforce_cache_coherency;
>> +	/* outstanding faults awaiting response indexed by fault group id */
>> +	struct xarray faults;
> this...
> 
>> +struct iommufd_fault {
>> +	struct iommufd_object obj;
>> +	struct iommufd_ctx *ictx;
>> +	struct file *filep;
>> +
>> +	/* The lists of outstanding faults protected by below mutex. */
>> +	struct mutex mutex;
>> +	struct list_head deliver;
>> +	struct list_head response;
> ...and here worth a discussion.
> 
> First the response list is not used. If continuing the choice of queueing
> faults per device it should be removed.

You are right. I have removed the response list in the new version.

> 
> But I wonder whether it makes more sense to keep this response
> queue per fault object. sounds simpler to me.
> 
> Also it's unclear why we need the response message to carry the
> same info as the request while only id/code/cookie are used.
> 
> +struct iommu_hwpt_page_response {
> +	__u32 size;
> +	__u32 flags;
> +	__u32 dev_id;
> +	__u32 pasid;
> +	__u32 grpid;
> +	__u32 code;
> +	__u32 cookie;
> +	__u32 reserved;
> +};
> 
> If we keep the response queue in the fault object, the response message
> only needs to carry size/flags/code/cookie and cookie can identify the
> pending message uniquely in the response queue.

It seems fine from the code's point of view. Let's wait and see whether
there are any concerns from the uAPI's perspective.

Best regards,
baolu