linux-kernel - RE: [PATCH v5 12/12] iommu: Improve iopf_queue_flush

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, 25 Sep 2023 07:00:43 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Lu Baolu <baolu.lu@...ux.intel.com>,
        Joerg Roedel <joro@...tes.org>,
        "Will Deacon" <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        "Jason Gunthorpe" <jgg@...pe.ca>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>,
        Nicolin Chen <nicolinc@...dia.com>
CC:     "Liu, Yi L" <yi.l.liu@...el.com>,
        Jacob Pan <jacob.jun.pan@...ux.intel.com>,
        "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v5 12/12] iommu: Improve iopf_queue_flush_dev()

> From: Lu Baolu <baolu.lu@...ux.intel.com>
> Sent: Thursday, September 14, 2023 4:57 PM
> @@ -300,6 +299,7 @@ EXPORT_SYMBOL_GPL(iommu_page_response);
>  /**
>   * iopf_queue_flush_dev - Ensure that all queued faults have been
> processed
>   * @dev: the endpoint whose faults need to be flushed.
> + * @pasid: the PASID of the endpoint.
>   *
>   * The IOMMU driver calls this before releasing a PASID, to ensure that all
>   * pending faults for this PASID have been handled, and won't hit the
> address

the comment should be updated too.

> @@ -309,17 +309,53 @@ EXPORT_SYMBOL_GPL(iommu_page_response);
>   *
>   * Return: 0 on success and <0 on error.
>   */
> -int iopf_queue_flush_dev(struct device *dev)
> +int iopf_queue_flush_dev(struct device *dev, ioasid_t pasid)

iopf_queue_flush_dev_pasid()?

>  {
>  	struct iommu_fault_param *iopf_param =
> iopf_get_dev_fault_param(dev);
> +	const struct iommu_ops *ops = dev_iommu_ops(dev);
> +	struct iommu_page_response resp;
> +	struct iopf_fault *iopf, *next;
> +	int ret = 0;
> 
>  	if (!iopf_param)
>  		return -ENODEV;
> 
>  	flush_workqueue(iopf_param->queue->wq);
> +
> +	mutex_lock(&iopf_param->lock);
> +	list_for_each_entry_safe(iopf, next, &iopf_param->partial, list) {
> +		if (!(iopf->fault.prm.flags &
> IOMMU_FAULT_PAGE_REQUEST_PASID_VALID) ||
> +		    iopf->fault.prm.pasid != pasid)
> +			break;
> +
> +		list_del(&iopf->list);
> +		kfree(iopf);
> +	}
> +
> +	list_for_each_entry_safe(iopf, next, &iopf_param->faults, list) {
> +		if (!(iopf->fault.prm.flags &
> IOMMU_FAULT_PAGE_REQUEST_PASID_VALID) ||
> +		    iopf->fault.prm.pasid != pasid)
> +			continue;
> +
> +		memset(&resp, 0, sizeof(struct iommu_page_response));
> +		resp.pasid = iopf->fault.prm.pasid;
> +		resp.grpid = iopf->fault.prm.grpid;
> +		resp.code = IOMMU_PAGE_RESP_INVALID;
> +
> +		if (iopf->fault.prm.flags &
> IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID)
> +			resp.flags = IOMMU_PAGE_RESP_PASID_VALID;
> +
> +		ret = ops->page_response(dev, iopf, &resp);
> +		if (ret)
> +			break;
> +
> +		list_del(&iopf->list);
> +		kfree(iopf);
> +	}
> +	mutex_unlock(&iopf_param->lock);
>  	iopf_put_dev_fault_param(iopf_param);
> 
> -	return 0;
> +	return ret;
>  }

Is it more accurate to call this function as iopf_queue_drop_dev_pasid()?
The added logic essentially implies that the caller doesn't care about
responses and all the in-fly states are either flushed (request) or
abandoned (response).

A normal flush() helper usually means just the flush action. If there is
a need to wait for responses after flush then we could add a
flush_dev_pasid_wait_timeout() later when there is a demand...