* Re: [PATCH 01/18] iommu: remove the unused domain_window_disable method [not found] ` <20210316153825.135976-2-hch@lst.de> @ 2021-03-30 12:04 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 12:04 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:07PM +0100, Christoph Hellwig wrote: > domain_window_disable is wired up by fsl_pamu, but never actually called. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/fsl_pamu_domain.c | 48 --------------------------------- > include/linux/iommu.h | 2 -- > 2 files changed, 50 deletions(-) Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-3-hch@lst.de>]
* Re: [PATCH 02/18] iommu/fsl_pamu: remove fsl_pamu_get_domain_attr [not found] ` <20210316153825.135976-3-hch@lst.de> @ 2021-03-30 12:10 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 12:10 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:08PM +0100, Christoph Hellwig wrote: > None of the values returned by this function are ever queried. Also > remove the DOMAIN_ATTR_FSL_PAMUV1 enum value that is not otherwise used. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/fsl_pamu_domain.c | 30 ------------------------------ > include/linux/iommu.h | 4 ---- > 2 files changed, 34 deletions(-) Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-4-hch@lst.de>]
* Re: [PATCH 03/18] iommu/fsl_pamu: remove support for setting DOMAIN_ATTR_GEOMETRY [not found] ` <20210316153825.135976-4-hch@lst.de> @ 2021-03-30 12:15 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 12:15 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:09PM +0100, Christoph Hellwig wrote: > The default geometry is the same as the one set by qman_port given > that FSL_PAMU depends on having 64-bit physical and thus DMA addresses. > > Remove the support to update the geometry and remove the now pointless > geom_size field. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/fsl_pamu_domain.c | 55 +++-------------------------- > drivers/iommu/fsl_pamu_domain.h | 6 ---- > drivers/soc/fsl/qbman/qman_portal.c | 12 ------- > 3 files changed, 5 insertions(+), 68 deletions(-) Took me a minute to track down the other magic '36' which ends up in aperture_end, but I found it eventually so: Acked-by: Will Deacon <will@kernel.org> (It does make me wonder what all this glue was intended to be used for) Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-5-hch@lst.de>]
* Re: [PATCH 04/18] iommu/fsl_pamu: merge iommu_alloc_dma_domain into fsl_pamu_domain_alloc [not found] ` <20210316153825.135976-5-hch@lst.de> @ 2021-03-30 12:17 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 12:17 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:10PM +0100, Christoph Hellwig wrote: > Keep the functionality to allocate the domain together. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/fsl_pamu_domain.c | 34 ++++++++++----------------------- > 1 file changed, 10 insertions(+), 24 deletions(-) Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-6-hch@lst.de>]
* Re: [PATCH 05/18] iommu/fsl_pamu: remove support for multiple windows [not found] ` <20210316153825.135976-6-hch@lst.de> @ 2021-03-30 12:22 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 12:22 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:11PM +0100, Christoph Hellwig wrote: > The only domains allocated forces use of a single window. Remove all > the code related to multiple window support, as well as the need for > qman_portal to force a single window. > > Remove the now unused DOMAIN_ATTR_WINDOWS iommu_attr. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/fsl_pamu.c | 264 +------------------------- > drivers/iommu/fsl_pamu.h | 10 +- > drivers/iommu/fsl_pamu_domain.c | 275 +++++----------------------- > drivers/iommu/fsl_pamu_domain.h | 12 +- > drivers/soc/fsl/qbman/qman_portal.c | 7 - > include/linux/iommu.h | 1 - > 6 files changed, 59 insertions(+), 510 deletions(-) [...] > + set_bf(ppaace->impl_attr, PAACE_IA_ATM, PAACE_ATM_WINDOW_XLATE); > + ppaace->twbah = rpn >> 20; > + set_bf(ppaace->win_bitfields, PAACE_WIN_TWBAL, rpn); > + set_bf(ppaace->addr_bitfields, PAACE_AF_AP, prot); > + set_bf(ppaace->impl_attr, PAACE_IA_WCE, 0); > + set_bf(ppaace->addr_bitfields, PPAACE_AF_MW, 0); > mb(); (I wonder what on Earth that mb() is doing...) > diff --git a/drivers/iommu/fsl_pamu_domain.h b/drivers/iommu/fsl_pamu_domain.h > index 53d359d66fe577..b9236fb5a8f82e 100644 > --- a/drivers/iommu/fsl_pamu_domain.h > +++ b/drivers/iommu/fsl_pamu_domain.h > @@ -17,23 +17,13 @@ struct dma_window { > }; > > struct fsl_dma_domain { > - /* > - * Number of windows assocaited with this domain. > - * During domain initialization, it is set to the > - * the maximum number of subwindows allowed for a LIODN. > - * Minimum value for this is 1 indicating a single PAMU > - * window, without any sub windows. Value can be set/ > - * queried by set_attr/get_attr API for DOMAIN_ATTR_WINDOWS. > - * Value can only be set once the geometry has been configured. > - */ > - u32 win_cnt; > /* > * win_arr contains information of the configured > * windows for a domain. This is allocated only > * when the number of windows for the domain are > * set. > */ The last part of this comment is now stale ^^ > - struct dma_window *win_arr; > + struct dma_window win_arr[1]; > /* list of devices associated with the domain */ > struct list_head devices; > /* dma_domain states: Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-7-hch@lst.de>]
* Re: [PATCH 06/18] iommu/fsl_pamu: remove ->domain_window_enable [not found] ` <20210316153825.135976-7-hch@lst.de> @ 2021-03-30 12:40 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 12:40 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:12PM +0100, Christoph Hellwig wrote: > The only thing that fsl_pamu_window_enable does for the current caller > is to fill in the prot value in the only dma_window structure, and to > propagate a few values from the iommu_domain_geometry struture into the > dma_window. Remove the dma_window entirely, hardcode the prot value and > otherwise use the iommu_domain_geometry structure instead. > > Remove the now unused ->domain_window_enable iommu method. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/fsl_pamu_domain.c | 182 +++------------------------- > drivers/iommu/fsl_pamu_domain.h | 17 --- > drivers/iommu/iommu.c | 11 -- > drivers/soc/fsl/qbman/qman_portal.c | 7 -- > include/linux/iommu.h | 17 --- > 5 files changed, 14 insertions(+), 220 deletions(-) > > diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c > index e6bdd38fc18409..fd2bc88b690465 100644 > --- a/drivers/iommu/fsl_pamu_domain.c > +++ b/drivers/iommu/fsl_pamu_domain.c > @@ -54,34 +54,18 @@ static int __init iommu_init_mempool(void) > return 0; > } > > -static phys_addr_t get_phys_addr(struct fsl_dma_domain *dma_domain, dma_addr_t iova) > -{ > - struct dma_window *win_ptr = &dma_domain->win_arr[0]; > - struct iommu_domain_geometry *geom; > - > - geom = &dma_domain->iommu_domain.geometry; > - > - if (win_ptr->valid) > - return win_ptr->paddr + (iova & (win_ptr->size - 1)); > - > - return 0; > -} > - > /* Map the DMA window corresponding to the LIODN */ > static int map_liodn(int liodn, struct fsl_dma_domain *dma_domain) > { > int ret; > - struct dma_window *wnd = &dma_domain->win_arr[0]; > - phys_addr_t wnd_addr = dma_domain->iommu_domain.geometry.aperture_start; > + struct iommu_domain_geometry *geom = &dma_domain->iommu_domain.geometry; > unsigned long flags; > > spin_lock_irqsave(&iommu_lock, flags); > - ret = pamu_config_ppaace(liodn, wnd_addr, > - wnd->size, > - ~(u32)0, > - wnd->paddr >> PAMU_PAGE_SHIFT, > - dma_domain->snoop_id, dma_domain->stash_id, > - wnd->prot); > + ret = pamu_config_ppaace(liodn, geom->aperture_start, > + geom->aperture_end - 1, ~(u32)0, You're passing 'geom->aperture_end - 1' as the size here, but the old code seemed to _add_ 1: > -static int fsl_pamu_window_enable(struct iommu_domain *domain, u32 wnd_nr, > - phys_addr_t paddr, u64 size, int prot) > -{ > - struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain); > - struct dma_window *wnd; > - int pamu_prot = 0; > - int ret; > - unsigned long flags; > - u64 win_size; > - > - if (prot & IOMMU_READ) > - pamu_prot |= PAACE_AP_PERMS_QUERY; > - if (prot & IOMMU_WRITE) > - pamu_prot |= PAACE_AP_PERMS_UPDATE; > - > - spin_lock_irqsave(&dma_domain->domain_lock, flags); > - if (wnd_nr > 0) { > - pr_debug("Invalid window index\n"); > - spin_unlock_irqrestore(&dma_domain->domain_lock, flags); > - return -EINVAL; > - } > - > - win_size = (domain->geometry.aperture_end + 1) >> ilog2(1); here ^^ when calculating the exclusive upper bound. In other words, I think '1ULL << 36' used to get passed to pamu_config_ppaace(). Is that an intentional change? Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-8-hch@lst.de>]
* Re: [PATCH 07/18] iommu/fsl_pamu: replace DOMAIN_ATTR_FSL_PAMU_STASH with a direct call [not found] ` <20210316153825.135976-8-hch@lst.de> @ 2021-03-30 12:44 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 12:44 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:13PM +0100, Christoph Hellwig wrote: > Add a fsl_pamu_configure_l1_stash API that qman_portal can call directly > instead of indirecting through the iommu attr API. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > arch/powerpc/include/asm/fsl_pamu_stash.h | 12 +++--------- > drivers/iommu/fsl_pamu_domain.c | 16 +++------------- > drivers/iommu/fsl_pamu_domain.h | 2 -- > drivers/soc/fsl/qbman/qman_portal.c | 18 +++--------------- > include/linux/iommu.h | 1 - > 5 files changed, 9 insertions(+), 40 deletions(-) Heh, this thing is so over-engineered. Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-9-hch@lst.de>]
* Re: [PATCH 08/18] iommu/fsl_pamu: merge pamu_set_liodn and map_liodn [not found] ` <20210316153825.135976-9-hch@lst.de> @ 2021-03-30 12:46 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 12:46 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:14PM +0100, Christoph Hellwig wrote: > Merge the two fuctions that configure the ppaace into a single coherent > function. I somehow doubt we need the two pamu_config_ppaace calls, > but keep the existing behavior just to be on the safe side. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/fsl_pamu_domain.c | 65 +++++++++------------------------ > 1 file changed, 17 insertions(+), 48 deletions(-) > > diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c > index 40eff4b7bc5d42..4a4944332674f7 100644 > --- a/drivers/iommu/fsl_pamu_domain.c > +++ b/drivers/iommu/fsl_pamu_domain.c > @@ -54,25 +54,6 @@ static int __init iommu_init_mempool(void) > return 0; > } > > -/* Map the DMA window corresponding to the LIODN */ > -static int map_liodn(int liodn, struct fsl_dma_domain *dma_domain) > -{ > - int ret; > - struct iommu_domain_geometry *geom = &dma_domain->iommu_domain.geometry; > - unsigned long flags; > - > - spin_lock_irqsave(&iommu_lock, flags); > - ret = pamu_config_ppaace(liodn, geom->aperture_start, > - geom->aperture_end - 1, ~(u32)0, > - 0, dma_domain->snoop_id, dma_domain->stash_id, > - PAACE_AP_PERMS_QUERY | PAACE_AP_PERMS_UPDATE); > - spin_unlock_irqrestore(&iommu_lock, flags); > - if (ret) > - pr_debug("PAACE configuration failed for liodn %d\n", liodn); > - > - return ret; > -} > - > static int update_liodn_stash(int liodn, struct fsl_dma_domain *dma_domain, > u32 val) > { > @@ -94,11 +75,11 @@ static int update_liodn_stash(int liodn, struct fsl_dma_domain *dma_domain, > } > > /* Set the geometry parameters for a LIODN */ > -static int pamu_set_liodn(int liodn, struct device *dev, > - struct fsl_dma_domain *dma_domain, > - struct iommu_domain_geometry *geom_attr) > +static int pamu_set_liodn(struct fsl_dma_domain *dma_domain, struct device *dev, > + int liodn) > { > - phys_addr_t window_addr, window_size; > + struct iommu_domain *domain = &dma_domain->iommu_domain; > + struct iommu_domain_geometry *geom = &domain->geometry; > u32 omi_index = ~(u32)0; > unsigned long flags; > int ret; > @@ -110,22 +91,25 @@ static int pamu_set_liodn(int liodn, struct device *dev, > */ > get_ome_index(&omi_index, dev); > > - window_addr = geom_attr->aperture_start; > - window_size = geom_attr->aperture_end + 1; > - > spin_lock_irqsave(&iommu_lock, flags); > ret = pamu_disable_liodn(liodn); > - if (!ret) > - ret = pamu_config_ppaace(liodn, window_addr, window_size, omi_index, > - 0, dma_domain->snoop_id, > - dma_domain->stash_id, 0); > + if (ret) > + goto out_unlock; > + ret = pamu_config_ppaace(liodn, geom->aperture_start, > + geom->aperture_end - 1, omi_index, 0, > + dma_domain->snoop_id, dma_domain->stash_id, 0); > + if (ret) > + goto out_unlock; > + ret = pamu_config_ppaace(liodn, geom->aperture_start, > + geom->aperture_end - 1, ~(u32)0, > + 0, dma_domain->snoop_id, dma_domain->stash_id, > + PAACE_AP_PERMS_QUERY | PAACE_AP_PERMS_UPDATE); There's more '+1' / '-1' confusion here with aperture_end which I'm not managing to follow. What am I missing? Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-10-hch@lst.de>]
* Re: [PATCH 09/18] iommu/fsl_pamu: merge handle_attach_device into fsl_pamu_attach_device [not found] ` <20210316153825.135976-10-hch@lst.de> @ 2021-03-30 12:48 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 12:48 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:15PM +0100, Christoph Hellwig wrote: > No good reason to split this functionality over two functions. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/fsl_pamu_domain.c | 59 +++++++++++---------------------- > 1 file changed, 20 insertions(+), 39 deletions(-) Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-11-hch@lst.de>]
* Re: [PATCH 10/18] iommu/fsl_pamu: enable the liodn when attaching a device [not found] ` <20210316153825.135976-11-hch@lst.de> @ 2021-03-30 12:53 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 12:53 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:16PM +0100, Christoph Hellwig wrote: > Instead of a separate call to enable all devices from the list, just > enablde the liodn one the device is attached to the iommu domain. (typos: "enablde" and "one" probably needs to be "once"?) > This also remove the DOMAIN_ATTR_FSL_PAMU_ENABLE iommu_attr. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/fsl_pamu_domain.c | 47 ++--------------------------- > drivers/iommu/fsl_pamu_domain.h | 10 ------ > drivers/soc/fsl/qbman/qman_portal.c | 11 ------- > include/linux/iommu.h | 1 - > 4 files changed, 3 insertions(+), 66 deletions(-) Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-12-hch@lst.de>]
* Re: [PATCH 11/18] iommu/fsl_pamu: remove the snoop_id field [not found] ` <20210316153825.135976-12-hch@lst.de> @ 2021-03-30 12:58 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 12:58 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:17PM +0100, Christoph Hellwig wrote: > The snoop_id is always set to ~(u32)0. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/fsl_pamu_domain.c | 5 ++--- > drivers/iommu/fsl_pamu_domain.h | 1 - > 2 files changed, 2 insertions(+), 4 deletions(-) pamu_config_ppaace() takes quite a few useless parameters at this stage, but anyway: Acked-by: Will Deacon <will@kernel.org> Do you know if this driver is actually useful? Once the complexity has been stripped back, the stubs and default values really stand out. Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-13-hch@lst.de>]
* Re: [PATCH 12/18] iommu: remove DOMAIN_ATTR_PAGING [not found] ` <20210316153825.135976-13-hch@lst.de> @ 2021-03-30 12:58 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 12:58 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:18PM +0100, Christoph Hellwig wrote: > DOMAIN_ATTR_PAGING is never used. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/iommu.c | 5 ----- > include/linux/iommu.h | 1 - > 2 files changed, 6 deletions(-) Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-14-hch@lst.de>]
* Re: [PATCH 13/18] iommu: remove DOMAIN_ATTR_GEOMETRY [not found] ` <20210316153825.135976-14-hch@lst.de> @ 2021-03-30 13:00 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 13:00 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:19PM +0100, Christoph Hellwig wrote: > The geometry information can be trivially queried from the iommu_domain > struture. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/iommu.c | 20 +++----------------- > drivers/vfio/vfio_iommu_type1.c | 26 ++++++++++++-------------- > drivers/vhost/vdpa.c | 10 +++------- > include/linux/iommu.h | 1 - > 4 files changed, 18 insertions(+), 39 deletions(-) Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-15-hch@lst.de>]
* Re: [PATCH 14/18] iommu: remove DOMAIN_ATTR_NESTING [not found] ` <20210316153825.135976-15-hch@lst.de> @ 2021-03-30 13:04 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 13:04 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:20PM +0100, Christoph Hellwig wrote: > Use an explicit enable_nesting method instead. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 43 ++++++++------------- > drivers/iommu/arm/arm-smmu/arm-smmu.c | 30 +++++++------- > drivers/iommu/intel/iommu.c | 31 +++++---------- > drivers/iommu/iommu.c | 10 +++++ > drivers/vfio/vfio_iommu_type1.c | 5 +-- > include/linux/iommu.h | 4 +- > 6 files changed, 55 insertions(+), 68 deletions(-) Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-16-hch@lst.de>]
* Re: [PATCH 15/18] iommu: remove iommu_set_cmd_line_dma_api and iommu_cmd_line_dma_api [not found] ` <20210316153825.135976-16-hch@lst.de> @ 2021-03-30 13:05 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 13:05 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:21PM +0100, Christoph Hellwig wrote: > Don't obsfucate the trivial bit flag check. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/iommu/iommu.c | 23 +++++------------------ > 1 file changed, 5 insertions(+), 18 deletions(-) Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-18-hch@lst.de>]
* Re: [PATCH 17/18] iommu: remove DOMAIN_ATTR_IO_PGTABLE_CFG [not found] ` <20210316153825.135976-18-hch@lst.de> @ 2021-03-30 13:14 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 13:14 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:23PM +0100, Christoph Hellwig wrote: > Use an explicit set_pgtable_quirks method instead that just passes > the actual quirk bitmask instead. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Acked-by: Li Yang <leoyang.li@nxp.com> > --- > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 5 +- > drivers/iommu/arm/arm-smmu/arm-smmu.c | 64 +++++-------------------- > drivers/iommu/arm/arm-smmu/arm-smmu.h | 2 +- > drivers/iommu/iommu.c | 11 +++++ > include/linux/io-pgtable.h | 4 -- > include/linux/iommu.h | 12 ++++- > 6 files changed, 35 insertions(+), 63 deletions(-) I'm fine with this for now, although there has been talk about passing things other than boolean flags as page-table quirks. We can cross that bridge when we get there, so: Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-19-hch@lst.de>]
* Re: [PATCH 18/18] iommu: remove iommu_domain_{get,set}_attr [not found] ` <20210316153825.135976-19-hch@lst.de> @ 2021-03-30 13:16 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-03-30 13:16 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:24PM +0100, Christoph Hellwig wrote: > Remove the now unused iommu attr infrastructure. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/iommu/iommu.c | 26 -------------------------- > include/linux/iommu.h | 36 ------------------------------------ > 2 files changed, 62 deletions(-) Acked-by: Will Deacon <will@kernel.org> Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210316153825.135976-17-hch@lst.de>]
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE [not found] ` <20210316153825.135976-17-hch@lst.de> @ 2021-03-30 13:11 ` Will Deacon 2021-03-30 13:19 ` Robin Murphy 2021-03-31 16:07 ` Robin Murphy 1 sibling, 1 reply; 27+ messages in thread From: Will Deacon @ 2021-03-30 13:11 UTC (permalink / raw) To: Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, Robin Murphy, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: > From: Robin Murphy <robin.murphy@arm.com> > > Instead make the global iommu_dma_strict paramete in iommu.c canonical by > exporting helpers to get and set it and use those directly in the drivers. > > This make sure that the iommu.strict parameter also works for the AMD and > Intel IOMMU drivers on x86. As those default to lazy flushing a new > IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to > represent the default if not overriden by an explicit parameter. > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>. > [ported on top of the other iommu_attr changes and added a few small > missing bits] > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/iommu/amd/iommu.c | 23 +------- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - > drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- > drivers/iommu/dma-iommu.c | 9 +-- > drivers/iommu/intel/iommu.c | 64 ++++----------------- > drivers/iommu/iommu.c | 27 ++++++--- > include/linux/iommu.h | 4 +- > 8 files changed, 40 insertions(+), 165 deletions(-) I really like this cleanup, but I can't help wonder if it's going in the wrong direction. With SoCs often having multiple IOMMU instances and a distinction between "trusted" and "untrusted" devices, then having the flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound unreasonable to me, but this change makes it a global property. For example, see the recent patch from Lu Baolu: https://lore.kernel.org/r/20210225061454.2864009-1-baolu.lu@linux.intel.com Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE 2021-03-30 13:11 ` [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE Will Deacon @ 2021-03-30 13:19 ` Robin Murphy 2021-03-30 13:58 ` Will Deacon 0 siblings, 1 reply; 27+ messages in thread From: Robin Murphy @ 2021-03-30 13:19 UTC (permalink / raw) To: Will Deacon, Christoph Hellwig Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, linux-arm-kernel, Lu Baolu On 2021-03-30 14:11, Will Deacon wrote: > On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: >> From: Robin Murphy <robin.murphy@arm.com> >> >> Instead make the global iommu_dma_strict paramete in iommu.c canonical by >> exporting helpers to get and set it and use those directly in the drivers. >> >> This make sure that the iommu.strict parameter also works for the AMD and >> Intel IOMMU drivers on x86. As those default to lazy flushing a new >> IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to >> represent the default if not overriden by an explicit parameter. >> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com>. >> [ported on top of the other iommu_attr changes and added a few small >> missing bits] >> Signed-off-by: Christoph Hellwig <hch@lst.de> >> --- >> drivers/iommu/amd/iommu.c | 23 +------- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - >> drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- >> drivers/iommu/dma-iommu.c | 9 +-- >> drivers/iommu/intel/iommu.c | 64 ++++----------------- >> drivers/iommu/iommu.c | 27 ++++++--- >> include/linux/iommu.h | 4 +- >> 8 files changed, 40 insertions(+), 165 deletions(-) > > I really like this cleanup, but I can't help wonder if it's going in the > wrong direction. With SoCs often having multiple IOMMU instances and a > distinction between "trusted" and "untrusted" devices, then having the > flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound > unreasonable to me, but this change makes it a global property. The intent here was just to streamline the existing behaviour of stuffing a global property into a domain attribute then pulling it out again in the illusion that it was in any way per-domain. We're still checking dev_is_untrusted() before making an actual decision, and it's not like we can't add more factors at that point if we want to. > For example, see the recent patch from Lu Baolu: > > https://lore.kernel.org/r/20210225061454.2864009-1-baolu.lu@linux.intel.com Erm, this patch is based on that one, it's right there in the context :/ Thanks, Robin. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE 2021-03-30 13:19 ` Robin Murphy @ 2021-03-30 13:58 ` Will Deacon 2021-03-30 16:28 ` Robin Murphy 0 siblings, 1 reply; 27+ messages in thread From: Will Deacon @ 2021-03-30 13:58 UTC (permalink / raw) To: Robin Murphy Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, Christoph Hellwig, linux-arm-kernel, Lu Baolu On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote: > On 2021-03-30 14:11, Will Deacon wrote: > > On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: > > > From: Robin Murphy <robin.murphy@arm.com> > > > > > > Instead make the global iommu_dma_strict paramete in iommu.c canonical by > > > exporting helpers to get and set it and use those directly in the drivers. > > > > > > This make sure that the iommu.strict parameter also works for the AMD and > > > Intel IOMMU drivers on x86. As those default to lazy flushing a new > > > IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to > > > represent the default if not overriden by an explicit parameter. > > > > > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>. > > > [ported on top of the other iommu_attr changes and added a few small > > > missing bits] > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > > --- > > > drivers/iommu/amd/iommu.c | 23 +------- > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - > > > drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- > > > drivers/iommu/dma-iommu.c | 9 +-- > > > drivers/iommu/intel/iommu.c | 64 ++++----------------- > > > drivers/iommu/iommu.c | 27 ++++++--- > > > include/linux/iommu.h | 4 +- > > > 8 files changed, 40 insertions(+), 165 deletions(-) > > > > I really like this cleanup, but I can't help wonder if it's going in the > > wrong direction. With SoCs often having multiple IOMMU instances and a > > distinction between "trusted" and "untrusted" devices, then having the > > flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound > > unreasonable to me, but this change makes it a global property. > > The intent here was just to streamline the existing behaviour of stuffing a > global property into a domain attribute then pulling it out again in the > illusion that it was in any way per-domain. We're still checking > dev_is_untrusted() before making an actual decision, and it's not like we > can't add more factors at that point if we want to. Like I say, the cleanup is great. I'm just wondering whether there's a better way to express the complicated logic to decide whether or not to use the flush queue than what we end up with: if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) which is mixing up globals, device properties and domain properties. The result is that the driver code ends up just using the global to determine whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code, which is a departure from the current way of doing things. > > For example, see the recent patch from Lu Baolu: > > > > https://lore.kernel.org/r/20210225061454.2864009-1-baolu.lu@linux.intel.com > > Erm, this patch is based on that one, it's right there in the context :/ Ah, sorry, I didn't spot that! I was just trying to illustrate that this is per-device. Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE 2021-03-30 13:58 ` Will Deacon @ 2021-03-30 16:28 ` Robin Murphy 2021-03-31 11:49 ` Will Deacon 0 siblings, 1 reply; 27+ messages in thread From: Robin Murphy @ 2021-03-30 16:28 UTC (permalink / raw) To: Will Deacon Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, Christoph Hellwig, linux-arm-kernel, Lu Baolu On 2021-03-30 14:58, Will Deacon wrote: > On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote: >> On 2021-03-30 14:11, Will Deacon wrote: >>> On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: >>>> From: Robin Murphy <robin.murphy@arm.com> >>>> >>>> Instead make the global iommu_dma_strict paramete in iommu.c canonical by >>>> exporting helpers to get and set it and use those directly in the drivers. >>>> >>>> This make sure that the iommu.strict parameter also works for the AMD and >>>> Intel IOMMU drivers on x86. As those default to lazy flushing a new >>>> IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to >>>> represent the default if not overriden by an explicit parameter. >>>> >>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>. >>>> [ported on top of the other iommu_attr changes and added a few small >>>> missing bits] >>>> Signed-off-by: Christoph Hellwig <hch@lst.de> >>>> --- >>>> drivers/iommu/amd/iommu.c | 23 +------- >>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- >>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - >>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- >>>> drivers/iommu/dma-iommu.c | 9 +-- >>>> drivers/iommu/intel/iommu.c | 64 ++++----------------- >>>> drivers/iommu/iommu.c | 27 ++++++--- >>>> include/linux/iommu.h | 4 +- >>>> 8 files changed, 40 insertions(+), 165 deletions(-) >>> >>> I really like this cleanup, but I can't help wonder if it's going in the >>> wrong direction. With SoCs often having multiple IOMMU instances and a >>> distinction between "trusted" and "untrusted" devices, then having the >>> flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound >>> unreasonable to me, but this change makes it a global property. >> >> The intent here was just to streamline the existing behaviour of stuffing a >> global property into a domain attribute then pulling it out again in the >> illusion that it was in any way per-domain. We're still checking >> dev_is_untrusted() before making an actual decision, and it's not like we >> can't add more factors at that point if we want to. > > Like I say, the cleanup is great. I'm just wondering whether there's a > better way to express the complicated logic to decide whether or not to use > the flush queue than what we end up with: > > if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && > domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) > > which is mixing up globals, device properties and domain properties. The > result is that the driver code ends up just using the global to determine > whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code, > which is a departure from the current way of doing things. But previously, SMMU only ever saw the global policy piped through the domain attribute by iommu_group_alloc_default_domain(), so there's no functional change there. Obviously some of the above checks could be factored out into some kind of iommu_use_flush_queue() helper that IOMMU drivers can also call if they need to keep in sync. Or maybe we just allow iommu-dma to set IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're treating that as a generic thing now. >>> For example, see the recent patch from Lu Baolu: >>> >>> https://lore.kernel.org/r/20210225061454.2864009-1-baolu.lu@linux.intel.com >> >> Erm, this patch is based on that one, it's right there in the context :/ > > Ah, sorry, I didn't spot that! I was just trying to illustrate that this > is per-device. Sure, I understand - and I'm just trying to bang home that despite appearances it's never actually been treated as such for SMMU, so anything that's wrong after this change was already wrong before. Robin. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE 2021-03-30 16:28 ` Robin Murphy @ 2021-03-31 11:49 ` Will Deacon 2021-03-31 13:09 ` Robin Murphy 0 siblings, 1 reply; 27+ messages in thread From: Will Deacon @ 2021-03-31 11:49 UTC (permalink / raw) To: Robin Murphy Cc: freedreno, kvm, Michael Ellerman, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, linux-arm-msm, virtualization, David Woodhouse, Christoph Hellwig, linux-arm-kernel, Lu Baolu On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote: > On 2021-03-30 14:58, Will Deacon wrote: > > On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote: > > > On 2021-03-30 14:11, Will Deacon wrote: > > > > On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: > > > > > From: Robin Murphy <robin.murphy@arm.com> > > > > > > > > > > Instead make the global iommu_dma_strict paramete in iommu.c canonical by > > > > > exporting helpers to get and set it and use those directly in the drivers. > > > > > > > > > > This make sure that the iommu.strict parameter also works for the AMD and > > > > > Intel IOMMU drivers on x86. As those default to lazy flushing a new > > > > > IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to > > > > > represent the default if not overriden by an explicit parameter. > > > > > > > > > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>. > > > > > [ported on top of the other iommu_attr changes and added a few small > > > > > missing bits] > > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > > > > --- > > > > > drivers/iommu/amd/iommu.c | 23 +------- > > > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- > > > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - > > > > > drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- > > > > > drivers/iommu/dma-iommu.c | 9 +-- > > > > > drivers/iommu/intel/iommu.c | 64 ++++----------------- > > > > > drivers/iommu/iommu.c | 27 ++++++--- > > > > > include/linux/iommu.h | 4 +- > > > > > 8 files changed, 40 insertions(+), 165 deletions(-) > > > > > > > > I really like this cleanup, but I can't help wonder if it's going in the > > > > wrong direction. With SoCs often having multiple IOMMU instances and a > > > > distinction between "trusted" and "untrusted" devices, then having the > > > > flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound > > > > unreasonable to me, but this change makes it a global property. > > > > > > The intent here was just to streamline the existing behaviour of stuffing a > > > global property into a domain attribute then pulling it out again in the > > > illusion that it was in any way per-domain. We're still checking > > > dev_is_untrusted() before making an actual decision, and it's not like we > > > can't add more factors at that point if we want to. > > > > Like I say, the cleanup is great. I'm just wondering whether there's a > > better way to express the complicated logic to decide whether or not to use > > the flush queue than what we end up with: > > > > if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && > > domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) > > > > which is mixing up globals, device properties and domain properties. The > > result is that the driver code ends up just using the global to determine > > whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code, > > which is a departure from the current way of doing things. > > But previously, SMMU only ever saw the global policy piped through the > domain attribute by iommu_group_alloc_default_domain(), so there's no > functional change there. For DMA domains sure, but I don't think that's the case for unmanaged domains such as those used by VFIO. > Obviously some of the above checks could be factored out into some kind of > iommu_use_flush_queue() helper that IOMMU drivers can also call if they need > to keep in sync. Or maybe we just allow iommu-dma to set > IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're > treating that as a generic thing now. I think a helper that takes a domain would be a good starting point. Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE 2021-03-31 11:49 ` Will Deacon @ 2021-03-31 13:09 ` Robin Murphy 2021-03-31 15:32 ` Will Deacon 0 siblings, 1 reply; 27+ messages in thread From: Robin Murphy @ 2021-03-31 13:09 UTC (permalink / raw) To: Will Deacon Cc: kvm, Michael Ellerman, linux-arm-msm, linuxppc-dev, dri-devel, Li Yang, iommu, Christoph Hellwig, netdev, virtualization, freedreno, David Woodhouse, linux-arm-kernel On 2021-03-31 12:49, Will Deacon wrote: > On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote: >> On 2021-03-30 14:58, Will Deacon wrote: >>> On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote: >>>> On 2021-03-30 14:11, Will Deacon wrote: >>>>> On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: >>>>>> From: Robin Murphy <robin.murphy@arm.com> >>>>>> >>>>>> Instead make the global iommu_dma_strict paramete in iommu.c canonical by >>>>>> exporting helpers to get and set it and use those directly in the drivers. >>>>>> >>>>>> This make sure that the iommu.strict parameter also works for the AMD and >>>>>> Intel IOMMU drivers on x86. As those default to lazy flushing a new >>>>>> IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to >>>>>> represent the default if not overriden by an explicit parameter. >>>>>> >>>>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>. >>>>>> [ported on top of the other iommu_attr changes and added a few small >>>>>> missing bits] >>>>>> Signed-off-by: Christoph Hellwig <hch@lst.de> >>>>>> --- >>>>>> drivers/iommu/amd/iommu.c | 23 +------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- >>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - >>>>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- >>>>>> drivers/iommu/dma-iommu.c | 9 +-- >>>>>> drivers/iommu/intel/iommu.c | 64 ++++----------------- >>>>>> drivers/iommu/iommu.c | 27 ++++++--- >>>>>> include/linux/iommu.h | 4 +- >>>>>> 8 files changed, 40 insertions(+), 165 deletions(-) >>>>> >>>>> I really like this cleanup, but I can't help wonder if it's going in the >>>>> wrong direction. With SoCs often having multiple IOMMU instances and a >>>>> distinction between "trusted" and "untrusted" devices, then having the >>>>> flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound >>>>> unreasonable to me, but this change makes it a global property. >>>> >>>> The intent here was just to streamline the existing behaviour of stuffing a >>>> global property into a domain attribute then pulling it out again in the >>>> illusion that it was in any way per-domain. We're still checking >>>> dev_is_untrusted() before making an actual decision, and it's not like we >>>> can't add more factors at that point if we want to. >>> >>> Like I say, the cleanup is great. I'm just wondering whether there's a >>> better way to express the complicated logic to decide whether or not to use >>> the flush queue than what we end up with: >>> >>> if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && >>> domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) >>> >>> which is mixing up globals, device properties and domain properties. The >>> result is that the driver code ends up just using the global to determine >>> whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code, >>> which is a departure from the current way of doing things. >> >> But previously, SMMU only ever saw the global policy piped through the >> domain attribute by iommu_group_alloc_default_domain(), so there's no >> functional change there. > > For DMA domains sure, but I don't think that's the case for unmanaged > domains such as those used by VFIO. Eh? This is only relevant to DMA domains anyway. Flush queues are part of the IOVA allocator that VFIO doesn't even use. It's always been the case that unmanaged domains only use strict invalidation. >> Obviously some of the above checks could be factored out into some kind of >> iommu_use_flush_queue() helper that IOMMU drivers can also call if they need >> to keep in sync. Or maybe we just allow iommu-dma to set >> IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're >> treating that as a generic thing now. > > I think a helper that takes a domain would be a good starting point. You mean device, right? The one condition we currently have is at the device level, and there's really nothing inherent to the domain itself that matters (since the type is implicitly IOMMU_DOMAIN_DMA to even care about this). Another idea that's just come to mind is now that IOMMU_DOMAIN_DMA has a standard meaning, maybe we could split out a separate IOMMU_DOMAIN_DMA_STRICT type such that it can all propagate from iommu_get_def_domain_type()? That feels like it might be quite promising, but I'd still do it as an improvement on top of this patch, since it's beyond just cleaning up the abuse of domain attributes to pass a command-line option around. Robin. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE 2021-03-31 13:09 ` Robin Murphy @ 2021-03-31 15:32 ` Will Deacon 2021-03-31 16:05 ` Robin Murphy 0 siblings, 1 reply; 27+ messages in thread From: Will Deacon @ 2021-03-31 15:32 UTC (permalink / raw) To: Robin Murphy Cc: kvm, Michael Ellerman, linux-arm-msm, linuxppc-dev, dri-devel, Li Yang, iommu, Christoph Hellwig, netdev, virtualization, freedreno, David Woodhouse, linux-arm-kernel On Wed, Mar 31, 2021 at 02:09:37PM +0100, Robin Murphy wrote: > On 2021-03-31 12:49, Will Deacon wrote: > > On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote: > > > On 2021-03-30 14:58, Will Deacon wrote: > > > > On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote: > > > > > On 2021-03-30 14:11, Will Deacon wrote: > > > > > > On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: > > > > > > > From: Robin Murphy <robin.murphy@arm.com> > > > > > > > > > > > > > > Instead make the global iommu_dma_strict paramete in iommu.c canonical by > > > > > > > exporting helpers to get and set it and use those directly in the drivers. > > > > > > > > > > > > > > This make sure that the iommu.strict parameter also works for the AMD and > > > > > > > Intel IOMMU drivers on x86. As those default to lazy flushing a new > > > > > > > IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to > > > > > > > represent the default if not overriden by an explicit parameter. > > > > > > > > > > > > > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>. > > > > > > > [ported on top of the other iommu_attr changes and added a few small > > > > > > > missing bits] > > > > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > > > > > > --- > > > > > > > drivers/iommu/amd/iommu.c | 23 +------- > > > > > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- > > > > > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - > > > > > > > drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- > > > > > > > drivers/iommu/dma-iommu.c | 9 +-- > > > > > > > drivers/iommu/intel/iommu.c | 64 ++++----------------- > > > > > > > drivers/iommu/iommu.c | 27 ++++++--- > > > > > > > include/linux/iommu.h | 4 +- > > > > > > > 8 files changed, 40 insertions(+), 165 deletions(-) > > > > > > > > > > > > I really like this cleanup, but I can't help wonder if it's going in the > > > > > > wrong direction. With SoCs often having multiple IOMMU instances and a > > > > > > distinction between "trusted" and "untrusted" devices, then having the > > > > > > flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound > > > > > > unreasonable to me, but this change makes it a global property. > > > > > > > > > > The intent here was just to streamline the existing behaviour of stuffing a > > > > > global property into a domain attribute then pulling it out again in the > > > > > illusion that it was in any way per-domain. We're still checking > > > > > dev_is_untrusted() before making an actual decision, and it's not like we > > > > > can't add more factors at that point if we want to. > > > > > > > > Like I say, the cleanup is great. I'm just wondering whether there's a > > > > better way to express the complicated logic to decide whether or not to use > > > > the flush queue than what we end up with: > > > > > > > > if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && > > > > domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) > > > > > > > > which is mixing up globals, device properties and domain properties. The > > > > result is that the driver code ends up just using the global to determine > > > > whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code, > > > > which is a departure from the current way of doing things. > > > > > > But previously, SMMU only ever saw the global policy piped through the > > > domain attribute by iommu_group_alloc_default_domain(), so there's no > > > functional change there. > > > > For DMA domains sure, but I don't think that's the case for unmanaged > > domains such as those used by VFIO. > > Eh? This is only relevant to DMA domains anyway. Flush queues are part of > the IOVA allocator that VFIO doesn't even use. It's always been the case > that unmanaged domains only use strict invalidation. Maybe I'm going mad. With this patch, the SMMU driver unconditionally sets IO_PGTABLE_QUIRK_NON_STRICT for page-tables if iommu_get_dma_strict() is true, no? In which case, that will get set for page-tables corresponding to unmanaged domains as well as DMA domains when it is enabled. That didn't happen before because you couldn't set the attribute for unmanaged domains. What am I missing? > > > Obviously some of the above checks could be factored out into some kind of > > > iommu_use_flush_queue() helper that IOMMU drivers can also call if they need > > > to keep in sync. Or maybe we just allow iommu-dma to set > > > IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're > > > treating that as a generic thing now. > > > > I think a helper that takes a domain would be a good starting point. > > You mean device, right? The one condition we currently have is at the device > level, and there's really nothing inherent to the domain itself that matters > (since the type is implicitly IOMMU_DOMAIN_DMA to even care about this). Device would probably work too; you'd pass the first device to attach to the domain when querying this from the SMMU driver, I suppose. Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE 2021-03-31 15:32 ` Will Deacon @ 2021-03-31 16:05 ` Robin Murphy [not found] ` <20210401095945.GA6726@lst.de> 0 siblings, 1 reply; 27+ messages in thread From: Robin Murphy @ 2021-03-31 16:05 UTC (permalink / raw) To: Will Deacon Cc: kvm, Michael Ellerman, linux-arm-msm, linuxppc-dev, dri-devel, Li Yang, iommu, Christoph Hellwig, netdev, virtualization, freedreno, David Woodhouse, linux-arm-kernel On 2021-03-31 16:32, Will Deacon wrote: > On Wed, Mar 31, 2021 at 02:09:37PM +0100, Robin Murphy wrote: >> On 2021-03-31 12:49, Will Deacon wrote: >>> On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote: >>>> On 2021-03-30 14:58, Will Deacon wrote: >>>>> On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote: >>>>>> On 2021-03-30 14:11, Will Deacon wrote: >>>>>>> On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote: >>>>>>>> From: Robin Murphy <robin.murphy@arm.com> >>>>>>>> >>>>>>>> Instead make the global iommu_dma_strict paramete in iommu.c canonical by >>>>>>>> exporting helpers to get and set it and use those directly in the drivers. >>>>>>>> >>>>>>>> This make sure that the iommu.strict parameter also works for the AMD and >>>>>>>> Intel IOMMU drivers on x86. As those default to lazy flushing a new >>>>>>>> IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to >>>>>>>> represent the default if not overriden by an explicit parameter. >>>>>>>> >>>>>>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>. >>>>>>>> [ported on top of the other iommu_attr changes and added a few small >>>>>>>> missing bits] >>>>>>>> Signed-off-by: Christoph Hellwig <hch@lst.de> >>>>>>>> --- >>>>>>>> drivers/iommu/amd/iommu.c | 23 +------- >>>>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +--------------- >>>>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 - >>>>>>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +-------- >>>>>>>> drivers/iommu/dma-iommu.c | 9 +-- >>>>>>>> drivers/iommu/intel/iommu.c | 64 ++++----------------- >>>>>>>> drivers/iommu/iommu.c | 27 ++++++--- >>>>>>>> include/linux/iommu.h | 4 +- >>>>>>>> 8 files changed, 40 insertions(+), 165 deletions(-) >>>>>>> >>>>>>> I really like this cleanup, but I can't help wonder if it's going in the >>>>>>> wrong direction. With SoCs often having multiple IOMMU instances and a >>>>>>> distinction between "trusted" and "untrusted" devices, then having the >>>>>>> flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound >>>>>>> unreasonable to me, but this change makes it a global property. >>>>>> >>>>>> The intent here was just to streamline the existing behaviour of stuffing a >>>>>> global property into a domain attribute then pulling it out again in the >>>>>> illusion that it was in any way per-domain. We're still checking >>>>>> dev_is_untrusted() before making an actual decision, and it's not like we >>>>>> can't add more factors at that point if we want to. >>>>> >>>>> Like I say, the cleanup is great. I'm just wondering whether there's a >>>>> better way to express the complicated logic to decide whether or not to use >>>>> the flush queue than what we end up with: >>>>> >>>>> if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) && >>>>> domain->ops->flush_iotlb_all && !iommu_get_dma_strict()) >>>>> >>>>> which is mixing up globals, device properties and domain properties. The >>>>> result is that the driver code ends up just using the global to determine >>>>> whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code, >>>>> which is a departure from the current way of doing things. >>>> >>>> But previously, SMMU only ever saw the global policy piped through the >>>> domain attribute by iommu_group_alloc_default_domain(), so there's no >>>> functional change there. >>> >>> For DMA domains sure, but I don't think that's the case for unmanaged >>> domains such as those used by VFIO. >> >> Eh? This is only relevant to DMA domains anyway. Flush queues are part of >> the IOVA allocator that VFIO doesn't even use. It's always been the case >> that unmanaged domains only use strict invalidation. > > Maybe I'm going mad. With this patch, the SMMU driver unconditionally sets > IO_PGTABLE_QUIRK_NON_STRICT for page-tables if iommu_get_dma_strict() is > true, no? In which case, that will get set for page-tables corresponding > to unmanaged domains as well as DMA domains when it is enabled. That didn't > happen before because you couldn't set the attribute for unmanaged domains. > > What am I missing? Oh cock... sorry, all this time I've been saying what I *expect* it to do, while overlooking the fact that the IO_PGTABLE_QUIRK_NON_STRICT hunks were the bits I forgot to write and Christoph had to fix up. Indeed, those should be checking the domain type too to preserve the existing behaviour. Apologies for the confusion. Robin. >>>> Obviously some of the above checks could be factored out into some kind of >>>> iommu_use_flush_queue() helper that IOMMU drivers can also call if they need >>>> to keep in sync. Or maybe we just allow iommu-dma to set >>>> IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're >>>> treating that as a generic thing now. >>> >>> I think a helper that takes a domain would be a good starting point. >> >> You mean device, right? The one condition we currently have is at the device >> level, and there's really nothing inherent to the domain itself that matters >> (since the type is implicitly IOMMU_DOMAIN_DMA to even care about this). > > Device would probably work too; you'd pass the first device to attach to the > domain when querying this from the SMMU driver, I suppose. > > Will > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20210401095945.GA6726@lst.de>]
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE [not found] ` <20210401095945.GA6726@lst.de> @ 2021-04-01 13:26 ` Will Deacon 0 siblings, 0 replies; 27+ messages in thread From: Will Deacon @ 2021-04-01 13:26 UTC (permalink / raw) To: Christoph Hellwig Cc: kvm, Robin Murphy, linux-arm-msm, linuxppc-dev, dri-devel, Li Yang, iommu, netdev, Michael Ellerman, virtualization, freedreno, David Woodhouse, linux-arm-kernel On Thu, Apr 01, 2021 at 11:59:45AM +0200, Christoph Hellwig wrote: > For now I'll just pass the iommu_domain to iommu_get_dma_strict, > so that we can check for it. We can do additional cleanups on top > of that later. Sounds good to me, cheers! Will _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE [not found] ` <20210316153825.135976-17-hch@lst.de> 2021-03-30 13:11 ` [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE Will Deacon @ 2021-03-31 16:07 ` Robin Murphy 1 sibling, 0 replies; 27+ messages in thread From: Robin Murphy @ 2021-03-31 16:07 UTC (permalink / raw) To: Christoph Hellwig, Joerg Roedel, Will Deacon, Li Yang Cc: kvm, Michael Ellerman, linux-arm-msm, linuxppc-dev, dri-devel, virtualization, iommu, netdev, freedreno, David Woodhouse, linux-arm-kernel On 2021-03-16 15:38, Christoph Hellwig wrote: [...] > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > index f1e38526d5bd40..996dfdf9d375dd 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -2017,7 +2017,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain, > .iommu_dev = smmu->dev, > }; > > - if (smmu_domain->non_strict) > + if (!iommu_get_dma_strict()) As Will raised, this also needs to be checking "domain->type == IOMMU_DOMAIN_DMA" to maintain equivalent behaviour to the attribute code below. > pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT; > > pgtbl_ops = alloc_io_pgtable_ops(fmt, &pgtbl_cfg, smmu_domain); > @@ -2449,52 +2449,6 @@ static struct iommu_group *arm_smmu_device_group(struct device *dev) > return group; > } > > -static int arm_smmu_domain_get_attr(struct iommu_domain *domain, > - enum iommu_attr attr, void *data) > -{ > - struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); > - > - switch (domain->type) { > - case IOMMU_DOMAIN_DMA: > - switch (attr) { > - case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE: > - *(int *)data = smmu_domain->non_strict; > - return 0; > - default: > - return -ENODEV; > - } > - break; > - default: > - return -EINVAL; > - } > -} [...] > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > index f985817c967a25..edb1de479dd1a7 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > @@ -668,7 +668,6 @@ struct arm_smmu_domain { > struct mutex init_mutex; /* Protects smmu pointer */ > > struct io_pgtable_ops *pgtbl_ops; > - bool non_strict; > atomic_t nr_ats_masters; > > enum arm_smmu_domain_stage stage; > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c > index 0aa6d667274970..3dde22b1f8ffb0 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c > @@ -761,6 +761,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain, > .iommu_dev = smmu->dev, > }; > > + if (!iommu_get_dma_strict()) Ditto here. Sorry for not spotting that sooner :( Robin. > + pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT; > + > if (smmu->impl && smmu->impl->init_context) { > ret = smmu->impl->init_context(smmu_domain, &pgtbl_cfg, dev); > if (ret) _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2021-04-01 13:27 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20210316153825.135976-1-hch@lst.de> [not found] ` <20210316153825.135976-2-hch@lst.de> 2021-03-30 12:04 ` [PATCH 01/18] iommu: remove the unused domain_window_disable method Will Deacon [not found] ` <20210316153825.135976-3-hch@lst.de> 2021-03-30 12:10 ` [PATCH 02/18] iommu/fsl_pamu: remove fsl_pamu_get_domain_attr Will Deacon [not found] ` <20210316153825.135976-4-hch@lst.de> 2021-03-30 12:15 ` [PATCH 03/18] iommu/fsl_pamu: remove support for setting DOMAIN_ATTR_GEOMETRY Will Deacon [not found] ` <20210316153825.135976-5-hch@lst.de> 2021-03-30 12:17 ` [PATCH 04/18] iommu/fsl_pamu: merge iommu_alloc_dma_domain into fsl_pamu_domain_alloc Will Deacon [not found] ` <20210316153825.135976-6-hch@lst.de> 2021-03-30 12:22 ` [PATCH 05/18] iommu/fsl_pamu: remove support for multiple windows Will Deacon [not found] ` <20210316153825.135976-7-hch@lst.de> 2021-03-30 12:40 ` [PATCH 06/18] iommu/fsl_pamu: remove ->domain_window_enable Will Deacon [not found] ` <20210316153825.135976-8-hch@lst.de> 2021-03-30 12:44 ` [PATCH 07/18] iommu/fsl_pamu: replace DOMAIN_ATTR_FSL_PAMU_STASH with a direct call Will Deacon [not found] ` <20210316153825.135976-9-hch@lst.de> 2021-03-30 12:46 ` [PATCH 08/18] iommu/fsl_pamu: merge pamu_set_liodn and map_liodn Will Deacon [not found] ` <20210316153825.135976-10-hch@lst.de> 2021-03-30 12:48 ` [PATCH 09/18] iommu/fsl_pamu: merge handle_attach_device into fsl_pamu_attach_device Will Deacon [not found] ` <20210316153825.135976-11-hch@lst.de> 2021-03-30 12:53 ` [PATCH 10/18] iommu/fsl_pamu: enable the liodn when attaching a device Will Deacon [not found] ` <20210316153825.135976-12-hch@lst.de> 2021-03-30 12:58 ` [PATCH 11/18] iommu/fsl_pamu: remove the snoop_id field Will Deacon [not found] ` <20210316153825.135976-13-hch@lst.de> 2021-03-30 12:58 ` [PATCH 12/18] iommu: remove DOMAIN_ATTR_PAGING Will Deacon [not found] ` <20210316153825.135976-14-hch@lst.de> 2021-03-30 13:00 ` [PATCH 13/18] iommu: remove DOMAIN_ATTR_GEOMETRY Will Deacon [not found] ` <20210316153825.135976-15-hch@lst.de> 2021-03-30 13:04 ` [PATCH 14/18] iommu: remove DOMAIN_ATTR_NESTING Will Deacon [not found] ` <20210316153825.135976-16-hch@lst.de> 2021-03-30 13:05 ` [PATCH 15/18] iommu: remove iommu_set_cmd_line_dma_api and iommu_cmd_line_dma_api Will Deacon [not found] ` <20210316153825.135976-18-hch@lst.de> 2021-03-30 13:14 ` [PATCH 17/18] iommu: remove DOMAIN_ATTR_IO_PGTABLE_CFG Will Deacon [not found] ` <20210316153825.135976-19-hch@lst.de> 2021-03-30 13:16 ` [PATCH 18/18] iommu: remove iommu_domain_{get,set}_attr Will Deacon [not found] ` <20210316153825.135976-17-hch@lst.de> 2021-03-30 13:11 ` [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE Will Deacon 2021-03-30 13:19 ` Robin Murphy 2021-03-30 13:58 ` Will Deacon 2021-03-30 16:28 ` Robin Murphy 2021-03-31 11:49 ` Will Deacon 2021-03-31 13:09 ` Robin Murphy 2021-03-31 15:32 ` Will Deacon 2021-03-31 16:05 ` Robin Murphy [not found] ` <20210401095945.GA6726@lst.de> 2021-04-01 13:26 ` Will Deacon 2021-03-31 16:07 ` Robin Murphy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).