On Mon, Jun 26, 2023 at 11:10:22AM +0800, Baolu Lu wrote: > > > > I think that, whether the guest has an IOPF capability or not, > > > > the host should always forward any stage-1 fault/error back to > > > > the guest. Yet, the implementation of this series builds with > > > > the IOPF framework that doesn't report IOMMU_FAULT_DMA_UNRECOV. > > > > > > > > And I have my doubt at the using the IOPF framework with that > > > > IOMMU_PAGE_RESP_ASYNC flag: using the IOPF framework is for > > > > its bottom half workqueue, because a page response could take > > > > a long cycle. But adding that flag feels like we don't really > > > > need the bottom half workqueue, i.e. losing the point of using > > > > the IOPF framework, IMHO. > > > > > > > > Combining the two facts above, I wonder if we really need to > > > > go through the IOPF framework; can't we just register a user > > > > fault handler in the iommufd directly upon a valid event_fd? > > > Agreed. We should avoid workqueue in sva iopf framework. Perhaps we > > > could go ahead with below code? It will be registered to device with > > > iommu_register_device_fault_handler() in IOMMU_DEV_FEAT_IOPF enabling > > > path. Un-registering in the disable path of cause. > > Well, for a virtualization use case, I still think it's should > > be registered in iommufd. > > Emm.. you suggest iommufd calls iommu_register_device_fault_handler() to > register its own page fault handler, right? > > I have a different opinion, iommu_register_device_fault_handler() is > called to register a fault handler for a device. It should be called > or initiated by a device driver. The iommufd only needs to install a > per-domain io page fault handler. > > I am considering a use case on Intel platform. Perhaps it's similar > on other platforms. An SIOV-capable device can support host SVA and > assigning mediated devices to user space at the same time. Both host > SVA and mediated devices require IOPF. So there will be multiple places > where a page fault handler needs to be registered. Okay, the narrative makes sense to me. I was more thinking of the nesting case. The iommu_register_device_fault_handler() is registered per device, as its name implies, while the handler probably should be slightly different by the attaching domain. It seems that your io_pgfault_handler() in the previous email can potentially handle this, i.e. a IOMMU_DOMAIN_NESTED could set domain->iopf_handler to forward DMA faults to user space. We just need to make sure this pathway would be unconditional at the handler registration and fault->type. > > Having a device without an IOPF/PRI > > capability, a guest OS should receive some faults too, if that > > device causes a translation failure. > > Yes. DMA faults are also a consideration. But I would like to have it > supported in a separated series. As I explained in the previous reply, > we also need to consider the software nested translation case. I see. Thanks Nic