Re: [PATCH v2 10/11] iommu/amd: Add IO page fault notifier handler

[Jason Gunthorpe <jgg@ziepe.ca>]

On Mon, Sep 11, 2023 at 12:10:45PM +0000, Vasant Hegde wrote:

> @@ -285,7 +286,7 @@ static struct iommu_dev_data *find_dev_data(struct amd_iommu *iommu, u16 devid)
>  {
>  	struct iommu_dev_data *dev_data;
>  
> -	dev_data = search_dev_data(iommu, devid);
> +	dev_data = amd_iommu_search_dev_data(iommu, devid);

> +static bool ppr_is_valid(struct amd_iommu *iommu, u64 *raw)
> +{
> +	struct device *dev = iommu->iommu.dev;
> +	u16 devid = PPR_DEVID(raw[0]);
> +
> +	if (!(PPR_FLAGS(raw[0]) & PPR_FLAG_GN)) {
> +		dev_warn(dev, "PPR logged [Request ignored due to GN=0 (device=%04x:%02x:%02x.%x "
> +			 "pasid=0x%05llx address=0x%llx flags=0x%04llx tag=0x%03llx]\n",
> +			 iommu->pci_seg->id, PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
> +			 PPR_PASID(raw[0]), raw[1], PPR_FLAGS(raw[0]), PPR_TAG(raw[0]));
> +		return false;
> +	}
> +
> +	if (PPR_FLAGS(raw[0]) & PPR_FLAG_RVSD) {
> +		dev_warn(dev, "PPR logged [Invalid request format (device=%04x:%02x:%02x.%x "
> +			 "pasid=0x%05llx address=0x%llx flags=0x%04llx tag=0x%03llx]\n",
> +			 iommu->pci_seg->id, PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
> +			 PPR_PASID(raw[0]), raw[1], PPR_FLAGS(raw[0]), PPR_TAG(raw[0]));
> +		return false;
> +	}

Please be careful that no guest can trigger these warnings..

> +
> +	return true;
> +}
> +
> +static void iommu_call_iopf_notifier(struct amd_iommu *iommu, u64 *raw)
> +{
> +	struct iopf_fault event;
> +	struct pci_dev *pdev;
> +	int ret = -EINVAL;
> +	u16 devid = PPR_DEVID(raw[0]);
> +
> +	if (PPR_REQ_TYPE(raw[0]) != PPR_REQ_FAULT) {
> +		pr_err_ratelimited("Unknown PPR request received\n");
> +		return;
> +	}
> +
> +	if (!ppr_is_valid(iommu, raw))
> +		goto out;
> +
> +	pdev = pci_get_domain_bus_and_slot(iommu->pci_seg->id, PCI_BUS_NUM(devid),
> +					   devid & 0xff);
> +	if (!pdev)
> +		goto out;

Lu, here is another case where the core PRI code could make use of a
core helper for a getting from the RID to the iommu world.

> +
> +	memset(&event, 0, sizeof(struct iopf_fault));
> +
> +	event.fault.type = IOMMU_FAULT_PAGE_REQ;
> +	event.fault.prm.perm = ppr_flag_to_fault_perm(PPR_FLAGS(raw[0]));
> +	event.fault.prm.addr = (u64)(raw[1] & PAGE_MASK);
> +	event.fault.prm.pasid = PPR_PASID(raw[0]);
> +	event.fault.prm.grpid = PPR_TAG(raw[0]) & 0x1FF;
> +
> +	/*
> +	 * PASID zero is used for requests from the I/O device without
> +	 * a PASID
> +	 */
> +	if (event.fault.prm.pasid == 0 ||
> +	    event.fault.prm.pasid >= pdev->dev.iommu->max_pasids) {
> +		pr_info_ratelimited("Invalid PASID : 0x%x, device : 0x%x\n",
> +				    event.fault.prm.pasid, pdev->dev.id);
> +		goto out;
> +	}
> +
> +
> +	event.fault.prm.flags |= IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID;
> +	event.fault.prm.flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
> +	if (PPR_TAG(raw[0]) & 0x200)
> +		event.fault.prm.flags |= IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE;
> +
> +	/* Submit event */
> +	ret = iommu_report_device_fault(&pdev->dev, &event);
> +
> +out:
> +	if (ret) {
> +		/* Nobody cared, abort */
> +		struct iommu_page_response resp = {
> +			.pasid = PPR_PASID(raw[0]),
> +			.grpid = PPR_TAG(raw[0]) & 0x1FF,
> +			.code = IOMMU_PAGE_RESP_FAILURE,
> +		};
> +		amd_iommu_page_response(&pdev->dev, &event, &resp);

Just to call amd_iommu_complete_ppr(), this already has the pci_dev,
we don't need amd_iommu_page_response() to get it.

Jason