| Message ID | 20210401154718.307519-1-jean-philippe@linaro.org |
|---|---|
| Headers | show |
| Series | iommu: I/O page faults for SMMUv3 | expand |
On Thu, Apr 01, 2021 at 05:47:16PM +0200, Jean-Philippe Brucker wrote: > When handling faults from the event or PRI queue, we need to find the > struct device associated with a SID. Add a rb_tree to keep track of > SIDs. > > Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Reviewed-by: Eric Auger <eric.auger@redhat.com> > Reviewed-by: Keqian Zhu <zhukeqian1@huawei.com> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> Acked-by: Will Deacon <will@kernel.org> Will
On Thu, Apr 01, 2021 at 05:47:19PM +0200, Jean-Philippe Brucker wrote: > The SMMU provides a Stall model for handling page faults in platform > devices. It is similar to PCIe PRI, but doesn't require devices to have > their own translation cache. Instead, faulting transactions are parked > and the OS is given a chance to fix the page tables and retry the > transaction. > > Enable stall for devices that support it (opt-in by firmware). When an > event corresponds to a translation error, call the IOMMU fault handler. > If the fault is recoverable, it will call us back to terminate or > continue the stall. Which hardware is this useful for? Stalling adds a fair amount of complexity to the driver, so I don't think we should support it unless we're likely to see platforms that both implement it and do something useful with it. Will
On Thu, Apr 01, 2021 at 05:47:09PM +0200, Jean-Philippe Brucker wrote: > Add stall support to the SMMUv3 driver, along with a common I/O Page > Fault handler. > > Since [v13] I added review and ack tags (Thanks!), and a lockdep_assert. > It would be good to have all of it in v5.13, since patch 10 introduces > the first user for the IOPF interface from patch 6. But if that's not > possible, please pick patches 1-6 so the Vt-d driver can start using > them. Patches 1-7 look good to me, but I'm not convinced about the utility of stalling faults so I'd prefer the later patches to come along with a real user. Will
On 2021/4/1 23:47, Jean-Philippe Brucker wrote: > The pasid-num-bits property shouldn't need a dedicated fwspec field, > it's a job for device properties. Add properties for IORT, and access > the number of PASID bits using device_property_read_u32(). > > Suggested-by: Robin Murphy<robin.murphy@arm.com> > Acked-by: Jonathan Cameron<Jonathan.Cameron@huawei.com> > Acked-by: Will Deacon<will@kernel.org> > Reviewed-by: Eric Auger<eric.auger@redhat.com> > Signed-off-by: Jean-Philippe Brucker<jean-philippe@linaro.org> > --- > include/linux/iommu.h | 2 -- > drivers/acpi/arm64/iort.c | 13 +++++++------ Acked-by: Hanjun Guo <guohanjun@huawei.com>
On 2021/4/1 23:47, Jean-Philippe Brucker wrote: > Copy the "Stall supported" bit, that tells whether a named component > supports stall, into the dma-can-stall device property. > > Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > --- > drivers/acpi/arm64/iort.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > index 3912a1f6058e..0828f70cb782 100644 > --- a/drivers/acpi/arm64/iort.c > +++ b/drivers/acpi/arm64/iort.c > @@ -968,13 +968,15 @@ static int iort_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data) > static void iort_named_component_init(struct device *dev, > struct acpi_iort_node *node) > { > - struct property_entry props[2] = {}; > + struct property_entry props[3] = {}; > struct acpi_iort_named_component *nc; > > nc = (struct acpi_iort_named_component *)node->node_data; > props[0] = PROPERTY_ENTRY_U32("pasid-num-bits", > FIELD_GET(ACPI_IORT_NC_PASID_BITS, > nc->node_flags)); > + if (nc->node_flags & ACPI_IORT_NC_STALL_SUPPORTED) > + props[1] = PROPERTY_ENTRY_BOOL("dma-can-stall"); > > if (device_add_properties(dev, props)) > dev_warn(dev, "Could not add device properties\n"); Acked-by: Hanjun Guo <guohanjun@huawei.com>
Hi, Will On 2021/4/2 上午1:11, Will Deacon wrote: > On Thu, Apr 01, 2021 at 05:47:19PM +0200, Jean-Philippe Brucker wrote: >> The SMMU provides a Stall model for handling page faults in platform >> devices. It is similar to PCIe PRI, but doesn't require devices to have >> their own translation cache. Instead, faulting transactions are parked >> and the OS is given a chance to fix the page tables and retry the >> transaction. >> >> Enable stall for devices that support it (opt-in by firmware). When an >> event corresponds to a translation error, call the IOMMU fault handler. >> If the fault is recoverable, it will call us back to terminate or >> continue the stall. > Which hardware is this useful for? Stalling adds a fair amount of complexity > to the driver, so I don't think we should support it unless we're likely to > see platforms that both implement it and do something useful with it. I have tested the stall mode on Hisilicon Kunpeng920 board, as well as using drivers/misc/uacce/uacce.c. Thanks
On 2021/4/2 1:11, Will Deacon wrote: > On Thu, Apr 01, 2021 at 05:47:19PM +0200, Jean-Philippe Brucker wrote: >> The SMMU provides a Stall model for handling page faults in platform >> devices. It is similar to PCIe PRI, but doesn't require devices to have >> their own translation cache. Instead, faulting transactions are parked >> and the OS is given a chance to fix the page tables and retry the >> transaction. >> >> Enable stall for devices that support it (opt-in by firmware). When an >> event corresponds to a translation error, call the IOMMU fault handler. >> If the fault is recoverable, it will call us back to terminate or >> continue the stall. > > Which hardware is this useful for? Stalling adds a fair amount of complexity > to the driver, so I don't think we should support it unless we're likely to > see platforms that both implement it and do something useful with it. Hi Will， HiSilicon Kunpeng920's ZIP/SEC/HPRE engines(drivers/crypto/hisilicon/) are using stall mode. UACCE driver(drivers/misc/uacce/) is used to export these engines to user space. A user space library: https://github.com/Linaro/uadk offers APIs to help users to use these engines. In fact, we only need a quirk(https://lkml.org/lkml/2021/3/8/1506) based on this IOPF series to make whole solution mainline ready. So please also take this patch, we need it! :) Best, Zhou > > Will > > . >
On Thu, Apr 01, 2021 at 06:15:02PM +0100, Will Deacon wrote: > On Thu, Apr 01, 2021 at 05:47:09PM +0200, Jean-Philippe Brucker wrote: > > Add stall support to the SMMUv3 driver, along with a common I/O Page > > Fault handler. > > > > Since [v13] I added review and ack tags (Thanks!), and a lockdep_assert. > > It would be good to have all of it in v5.13, since patch 10 introduces > > the first user for the IOPF interface from patch 6. But if that's not > > possible, please pick patches 1-6 so the Vt-d driver can start using > > them. > > Patches 1-7 look good to me, but I'm not convinced about the utility of > stalling faults so I'd prefer the later patches to come along with a > real user. As others said, it is possible to assign queues from the compression and crypto accelerators on the Kunpeng920 to userspace, using the uacce char device (upstream since last year, but waiting for implementations of the SVA API in IOMMU drivers). I've been using that platform for testing my code for the past year, with the UADK tool as well as an openssl plugin. Securely assignig a queue to userspace requires full SVA support in SMMUv3, which consists of PASID, page table sharing, and I/O page faults. The first two were already merged, and the third one requires either Stall or PRI. I'm not submitting PRI support at the moment because there is no hardware, but the Hisilicon platform implements stall and will be able to use it right away. Thanks, Jean
On Thu, Apr 01, 2021 at 05:47:18PM +0200, Jean-Philippe Brucker wrote: > Copy the "Stall supported" bit, that tells whether a named component > supports stall, into the dma-can-stall device property. > > Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > --- > drivers/acpi/arm64/iort.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > index 3912a1f6058e..0828f70cb782 100644 > --- a/drivers/acpi/arm64/iort.c > +++ b/drivers/acpi/arm64/iort.c > @@ -968,13 +968,15 @@ static int iort_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data) > static void iort_named_component_init(struct device *dev, > struct acpi_iort_node *node) > { > - struct property_entry props[2] = {}; > + struct property_entry props[3] = {}; > struct acpi_iort_named_component *nc; > > nc = (struct acpi_iort_named_component *)node->node_data; > props[0] = PROPERTY_ENTRY_U32("pasid-num-bits", > FIELD_GET(ACPI_IORT_NC_PASID_BITS, > nc->node_flags)); > + if (nc->node_flags & ACPI_IORT_NC_STALL_SUPPORTED) > + props[1] = PROPERTY_ENTRY_BOOL("dma-can-stall"); > > if (device_add_properties(dev, props)) > dev_warn(dev, "Could not add device properties\n"); > -- > 2.31.1 >
On Thu, Apr 01, 2021 at 05:47:09PM +0200, Jean-Philippe Brucker wrote: > Jean-Philippe Brucker (10): > iommu: Fix comment for struct iommu_fwspec > iommu/arm-smmu-v3: Use device properties for pasid-num-bits > iommu: Separate IOMMU_DEV_FEAT_IOPF from IOMMU_DEV_FEAT_SVA > iommu/vt-d: Support IOMMU_DEV_FEAT_IOPF > uacce: Enable IOMMU_DEV_FEAT_IOPF > iommu: Add a page fault handler > iommu/arm-smmu-v3: Maintain a SID->device structure Applied the first 7 patches, thanks.