Messages in this thread | | | Subject | Re: [PATCH 6/7] iommu: Use right way to retrieve iommu_ops | From | Lu Baolu <> | Date | Tue, 25 Jan 2022 11:31:50 +0800 |
| |
On 1/25/22 10:08 AM, Tian, Kevin wrote: >> From: Robin Murphy <robin.murphy@arm.com> >> Sent: Tuesday, January 25, 2022 9:52 AM >> >> On 2022-01-25 01:11, Tian, Kevin wrote: >>>> From: Jason Gunthorpe via iommu >>>> Sent: Tuesday, January 25, 2022 1:37 AM >>>>> @@ -1295,7 +1298,7 @@ int iommu_page_response(struct device *dev, >>>>> msg->pasid = 0; >>>>> } >>>>> >>>>> - ret = domain->ops->page_response(dev, evt, msg); >>>>> + ret = ops->page_response(dev, evt, msg); >>>>> list_del(&evt->list); >>>>> kfree(evt); >>>>> break; >>>> >>>> Feels weird that page_response is not connected to a domain, the fault >>>> originated from a domain after all. I would say this op should be >>>> moved to the domain and the caller should provide the a pointer to the >>>> domain that originated the fault. >>>> >>> >>> In concept yes. >> >> Not even that, really. It's true that the "fault" itself is logically >> associated with the domain, but we never see that - the ATS request and >> response which encapsulate it all happen automatically on the PCI side. >> It's the endpoint that then decides to handle ATS translation failure >> via PRI, so all we actually get is a page request message from a >> RID/PASID, which most definitely represents the "device" (and in fact >> having to work backwards from there to figure out which domain/context >> it is currently attached to can be a bit of a pain). Similarly the >> response is a message directly back to the device itself - an operation >> on a domain may (or may not) have happened off the back of receiving the >> initial request, but even if the content of the response is to reflect >> that, the operation of responding is clearly focused on the device. >> >> I fully agree that it's a weird-looking model, but that's how PCI SIG >> made it - and no IOMMU architecture seems to have tried to wrap it up in >> anything nicer either - so I don't see that we'd gain much from trying >> to pretend otherwise :) >> > > I think the point here is that although page requests are received > per device from the wire the low level iommu driver should convert > those requests into domain-wide requests (with RID/PASID recorded > as private data in the request) which then can be handled by domain > ops in iommu core. Once a domain-wide request is completed by > the iommu core, the low level iommu driver then retrieves RID/PASID > information from private data of the completed request and triggers > page response per RID/PASID in bus specific way.
I also have a pending series to associate the sva with an iommu domain and make the existing I/O page fault framework generic (vs. sva specific). Perhaps we can discuss the page fault handle/response there with the real code.
> > Does it sound reasonable? > > Thanks > Kevin >
Best regards, baolu
| |