Re: [PATCH] dma-mapping: move hint unlikely for dma_mapping_error from drivers to core
Heiner Kallweit writes: > Zillions of drivers use the unlikely() hint when checking the result of > dma_mapping_error(). This is an inline function anyway, so we can move > the hint into the function and remove it from drivers. >>From time to time discussions pop up how effective unlikely() is, > and that it should be used only if something is really very unlikely. > I think that's the case here. > > Patch was created with some help from coccinelle. > > @@ > expression dev, dma_addr; > @@ > > - unlikely(dma_mapping_error(dev, dma_addr)) > + dma_mapping_error(dev, dma_addr) > > Signed-off-by: Heiner Kallweit > --- > If ok, then tbd through which tree this is supposed to go. > Patch is based on linux-next-20201210. > --- [...] > drivers/net/wireless/ath/ath10k/htt_rx.c | 2 +- > drivers/net/wireless/ath/ath10k/pci.c | 2 +- > drivers/net/wireless/ath/ath10k/snoc.c| 2 +- > drivers/net/wireless/ath/ath11k/ce.c | 2 +- > drivers/net/wireless/ath/ath11k/dp_rx.c | 2 +- > drivers/net/wireless/ath/ath5k/base.c | 2 +- > drivers/net/wireless/ath/ath9k/beacon.c | 2 +- > drivers/net/wireless/ath/ath9k/recv.c | 21 +++- > drivers/net/wireless/ath/ath9k/xmit.c | 2 +- > drivers/net/wireless/ath/wil6210/txrx.c | 10 > drivers/net/wireless/ath/wil6210/txrx_edma.c | 4 +-- > drivers/net/wireless/broadcom/b43/dma.c | 2 +- > drivers/net/wireless/broadcom/b43legacy/dma.c | 2 +- > drivers/net/wireless/intel/iwlwifi/pcie/tx.c | 10 > drivers/net/wireless/intel/iwlwifi/queue/tx.c | 10 > drivers/net/wireless/mediatek/mt76/dma.c | 8 +++--- > .../net/wireless/ralink/rt2x00/rt2x00queue.c | 4 +-- For wireless drivers: Acked-by: Kalle Valo -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 05/27] dt-bindings: memory: mediatek: Rename header guard for SMI header file
On Wed, 09 Dec 2020 16:00:40 +0800, Yong Wu wrote: > Only rename the header guard for all the SoC larb port header file. > No funtional change. > > Suggested-by: Krzysztof Kozlowski > Signed-off-by: Yong Wu > --- > include/dt-bindings/memory/mt2701-larb-port.h | 4 ++-- > include/dt-bindings/memory/mt2712-larb-port.h | 4 ++-- > include/dt-bindings/memory/mt6779-larb-port.h | 4 ++-- > include/dt-bindings/memory/mt8167-larb-port.h | 4 ++-- > include/dt-bindings/memory/mt8173-larb-port.h | 4 ++-- > include/dt-bindings/memory/mt8183-larb-port.h | 4 ++-- > 6 files changed, 12 insertions(+), 12 deletions(-) > Acked-by: Rob Herring ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/1] vfio/type1: Add vfio_group_iommu_domain()
On Wed, 9 Dec 2020 09:44:44 +0800
Lu Baolu wrote:
> Add the API for getting the domain from a vfio group. This could be used
> by the physical device drivers which rely on the vfio/mdev framework for
> mediated device user level access. The typical use case like below:
>
> unsigned int pasid;
> struct vfio_group *vfio_group;
> struct iommu_domain *iommu_domain;
> struct device *dev = mdev_dev(mdev);
> struct device *iommu_device = mdev_get_iommu_device(dev);
>
> if (!iommu_device ||
> !iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
> return -EINVAL;
>
> vfio_group = vfio_group_get_external_user_from_dev(dev);
> if (IS_ERR_OR_NULL(vfio_group))
> return -EFAULT;
>
> iommu_domain = vfio_group_iommu_domain(vfio_group);
> if (IS_ERR_OR_NULL(iommu_domain)) {
> vfio_group_put_external_user(vfio_group);
> return -EFAULT;
> }
>
> pasid = iommu_aux_get_pasid(iommu_domain, iommu_device);
> if (pasid < 0) {
> vfio_group_put_external_user(vfio_group);
> return -EFAULT;
> }
>
> /* Program device context with pasid value. */
> ...
>
> Signed-off-by: Lu Baolu
> ---
> drivers/vfio/vfio.c | 18 ++
> drivers/vfio/vfio_iommu_type1.c | 24
> include/linux/vfio.h| 4
> 3 files changed, 46 insertions(+)
Applied to vfio next branch for v5.11. Thanks,
Alex
> Change log:
> - v3:
> https://lore.kernel.org/linux-iommu/20201201012328.2465735-1-baolu...@linux.intel.com/
> - Changed according to comments @
> https://lore.kernel.org/linux-iommu/20201202144834.1dd09...@w520.home/
>- Rename group_domain to group_iommu_domain;
>- Remove an unnecessary else branch.
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 2151bc7f87ab..4ad8a35667a7 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -2331,6 +2331,24 @@ int vfio_unregister_notifier(struct device *dev, enum
> vfio_notify_type type,
> }
> EXPORT_SYMBOL(vfio_unregister_notifier);
>
> +struct iommu_domain *vfio_group_iommu_domain(struct vfio_group *group)
> +{
> + struct vfio_container *container;
> + struct vfio_iommu_driver *driver;
> +
> + if (!group)
> + return ERR_PTR(-EINVAL);
> +
> + container = group->container;
> + driver = container->iommu_driver;
> + if (likely(driver && driver->ops->group_iommu_domain))
> + return driver->ops->group_iommu_domain(container->iommu_data,
> +group->iommu_group);
> +
> + return ERR_PTR(-ENOTTY);
> +}
> +EXPORT_SYMBOL_GPL(vfio_group_iommu_domain);
> +
> /**
> * Module/class support
> */
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 67e827638995..0b4dedaa9128 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -2980,6 +2980,29 @@ static int vfio_iommu_type1_dma_rw(void *iommu_data,
> dma_addr_t user_iova,
> return ret;
> }
>
> +static struct iommu_domain *
> +vfio_iommu_type1_group_iommu_domain(void *iommu_data,
> + struct iommu_group *iommu_group)
> +{
> + struct iommu_domain *domain = ERR_PTR(-ENODEV);
> + struct vfio_iommu *iommu = iommu_data;
> + struct vfio_domain *d;
> +
> + if (!iommu || !iommu_group)
> + return ERR_PTR(-EINVAL);
> +
> + mutex_lock(>lock);
> + list_for_each_entry(d, >domain_list, next) {
> + if (find_iommu_group(d, iommu_group)) {
> + domain = d->domain;
> + break;
> + }
> + }
> + mutex_unlock(>lock);
> +
> + return domain;
> +}
> +
> static const struct vfio_iommu_driver_ops vfio_iommu_driver_ops_type1 = {
> .name = "vfio-iommu-type1",
> .owner = THIS_MODULE,
> @@ -2993,6 +3016,7 @@ static const struct vfio_iommu_driver_ops
> vfio_iommu_driver_ops_type1 = {
> .register_notifier = vfio_iommu_type1_register_notifier,
> .unregister_notifier= vfio_iommu_type1_unregister_notifier,
> .dma_rw = vfio_iommu_type1_dma_rw,
> + .group_iommu_domain = vfio_iommu_type1_group_iommu_domain,
> };
>
> static int __init vfio_iommu_type1_init(void)
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 38d3c6a8dc7e..f45940b38a02 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -90,6 +90,8 @@ struct vfio_iommu_driver_ops {
> struct notifier_block *nb);
> int (*dma_rw)(void *iommu_data, dma_addr_t user_iova,
> void *data, size_t count, bool write);
> + struct iommu_domain *(*group_iommu_domain)(void *iommu_data,
> +
Re: [PATCH] dma-mapping: move hint unlikely for dma_mapping_error from drivers to core
On Thu, Dec 10, 2020 at 03:47:50PM +0100, Heiner Kallweit wrote: > Zillions of drivers use the unlikely() hint when checking the result of > dma_mapping_error(). This is an inline function anyway, so we can move > the hint into the function and remove it from drivers. > From time to time discussions pop up how effective unlikely() is, > and that it should be used only if something is really very unlikely. > I think that's the case here. > > Patch was created with some help from coccinelle. > > @@ > expression dev, dma_addr; > @@ > > - unlikely(dma_mapping_error(dev, dma_addr)) > + dma_mapping_error(dev, dma_addr) > > Signed-off-by: Heiner Kallweit Acked-by: Wolfram Sang # for I2C signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/7] vfio: iommu_type1: Clear added dirty bit when unwind pin
On Thu, 10 Dec 2020 15:34:19 +0800
Keqian Zhu wrote:
> Currently we do not clear added dirty bit of bitmap when unwind
> pin, so if pin failed at halfway, we set unnecessary dirty bit
> in bitmap. Clearing added dirty bit when unwind pin, userspace
> will see less dirty page, which can save much time to handle them.
>
> Note that we should distinguish the bits added by pin and the bits
> already set before pin, so introduce bitmap_added to record this.
>
> Signed-off-by: Keqian Zhu
> ---
> drivers/vfio/vfio_iommu_type1.c | 33 ++---
> 1 file changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 67e827638995..f129d24a6ec3 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -637,7 +637,11 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
> struct vfio_iommu *iommu = iommu_data;
> struct vfio_group *group;
> int i, j, ret;
> + unsigned long pgshift = __ffs(iommu->pgsize_bitmap);
> unsigned long remote_vaddr;
> + unsigned long bitmap_offset;
> + unsigned long *bitmap_added;
> + dma_addr_t iova;
> struct vfio_dma *dma;
> bool do_accounting;
>
> @@ -650,6 +654,12 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
>
> mutex_lock(>lock);
>
> + bitmap_added = bitmap_zalloc(npage, GFP_KERNEL);
> + if (!bitmap_added) {
> + ret = -ENOMEM;
> + goto pin_done;
> + }
This is allocated regardless of whether dirty tracking is enabled, so
this adds overhead to the common case in order to reduce user overhead
(not correctness) in the rare condition that dirty tracking is enabled,
and the even rarer condition that there's a fault during that case.
This is not a good trade-off. Thanks,
Alex
> +
> /* Fail if notifier list is empty */
> if (!iommu->notifier.head) {
> ret = -EINVAL;
> @@ -664,7 +674,6 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
> do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu);
>
> for (i = 0; i < npage; i++) {
> - dma_addr_t iova;
> struct vfio_pfn *vpfn;
>
> iova = user_pfn[i] << PAGE_SHIFT;
> @@ -699,14 +708,10 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
> }
>
> if (iommu->dirty_page_tracking) {
> - unsigned long pgshift = __ffs(iommu->pgsize_bitmap);
> -
> - /*
> - * Bitmap populated with the smallest supported page
> - * size
> - */
> - bitmap_set(dma->bitmap,
> -(iova - dma->iova) >> pgshift, 1);
> + /* Populated with the smallest supported page size */
> + bitmap_offset = (iova - dma->iova) >> pgshift;
> + if (!test_and_set_bit(bitmap_offset, dma->bitmap))
> + set_bit(i, bitmap_added);
> }
> }
> ret = i;
> @@ -722,14 +727,20 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
> pin_unwind:
> phys_pfn[i] = 0;
> for (j = 0; j < i; j++) {
> - dma_addr_t iova;
> -
> iova = user_pfn[j] << PAGE_SHIFT;
> dma = vfio_find_dma(iommu, iova, PAGE_SIZE);
> vfio_unpin_page_external(dma, iova, do_accounting);
> phys_pfn[j] = 0;
> +
> + if (test_bit(j, bitmap_added)) {
> + bitmap_offset = (iova - dma->iova) >> pgshift;
> + clear_bit(bitmap_offset, dma->bitmap);
> + }
> }
> pin_done:
> + if (bitmap_added)
> + bitmap_free(bitmap_added);
> +
> mutex_unlock(>lock);
> return ret;
> }
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Add sanity check for interrupt remapping table length macros
Suravee Suthikulpanit @ 2020-12-10 09:24 MST:
> Currently, macros related to the interrupt remapping table length are
> defined separately. This has resulted in an oversight in which one of
> the macros were missed when changing the length. To prevent this,
> redefine the macros to add built-in sanity check.
>
> Also, rename macros to use the name of the DTE[IntTabLen] field as
> specified in the AMD IOMMU specification. There is no functional change.
>
> Suggested-by: Linus Torvalds
> Reviewed-by: Tom Lendacky
> Signed-off-by: Suravee Suthikulpanit
> Cc: Will Deacon
> Cc: Jerry Snitselaar
> Cc: Joerg Roedel
> ---
> drivers/iommu/amd/amd_iommu_types.h | 19 ++-
> drivers/iommu/amd/init.c| 6 +++---
> drivers/iommu/amd/iommu.c | 2 +-
> 3 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h
> b/drivers/iommu/amd/amd_iommu_types.h
> index 494b42a31b7a..899ce62df3f0 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -255,11 +255,19 @@
> /* Bit value definition for dte irq remapping fields*/
> #define DTE_IRQ_PHYS_ADDR_MASK (((1ULL << 45)-1) << 6)
> #define DTE_IRQ_REMAP_INTCTL_MASK(0x3ULL << 60)
> -#define DTE_IRQ_TABLE_LEN_MASK (0xfULL << 1)
> #define DTE_IRQ_REMAP_INTCTL(2ULL << 60)
> -#define DTE_IRQ_TABLE_LEN (9ULL << 1)
> #define DTE_IRQ_REMAP_ENABLE1ULL
>
> +/*
> + * AMD IOMMU hardware only support 512 IRTEs despite
> + * the architectural limitation of 2048 entries.
> + */
> +#define DTE_INTTAB_ALIGNMENT128
> +#define DTE_INTTABLEN_VALUE 9ULL
> +#define DTE_INTTABLEN (DTE_INTTABLEN_VALUE << 1)
> +#define DTE_INTTABLEN_MASK (0xfULL << 1)
> +#define MAX_IRQS_PER_TABLE (1 << DTE_INTTABLEN_VALUE)
> +
> #define PAGE_MODE_NONE0x00
> #define PAGE_MODE_1_LEVEL 0x01
> #define PAGE_MODE_2_LEVEL 0x02
> @@ -409,13 +417,6 @@ extern bool amd_iommu_np_cache;
> /* Only true if all IOMMUs support device IOTLBs */
> extern bool amd_iommu_iotlb_sup;
>
> -/*
> - * AMD IOMMU hardware only support 512 IRTEs despite
> - * the architectural limitation of 2048 entries.
> - */
> -#define MAX_IRQS_PER_TABLE 512
> -#define IRQ_TABLE_ALIGNMENT 128
> -
> struct irq_remap_table {
> raw_spinlock_t lock;
> unsigned min_index;
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 23a790f8f550..6bec8913d064 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -989,10 +989,10 @@ static bool copy_device_table(void)
>
> irq_v = old_devtb[devid].data[2] & DTE_IRQ_REMAP_ENABLE;
> int_ctl = old_devtb[devid].data[2] & DTE_IRQ_REMAP_INTCTL_MASK;
> - int_tab_len = old_devtb[devid].data[2] & DTE_IRQ_TABLE_LEN_MASK;
> + int_tab_len = old_devtb[devid].data[2] & DTE_INTTABLEN_MASK;
> if (irq_v && (int_ctl || int_tab_len)) {
> if ((int_ctl != DTE_IRQ_REMAP_INTCTL) ||
> - (int_tab_len != DTE_IRQ_TABLE_LEN)) {
> + (int_tab_len != DTE_INTTABLEN)) {
> pr_err("Wrong old irq remapping flag: %#x\n",
> devid);
> return false;
> }
> @@ -2674,7 +2674,7 @@ static int __init early_amd_iommu_init(void)
> remap_cache_sz = MAX_IRQS_PER_TABLE * (sizeof(u64) * 2);
> amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
> remap_cache_sz,
> - IRQ_TABLE_ALIGNMENT,
> + DTE_INTTAB_ALIGNMENT,
> 0, NULL);
> if (!amd_iommu_irq_cache)
> goto out;
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index b9cf59443843..f7abf16d1e3a 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3191,7 +3191,7 @@ static void set_dte_irq_entry(u16 devid, struct
> irq_remap_table *table)
> dte &= ~DTE_IRQ_PHYS_ADDR_MASK;
> dte |= iommu_virt_to_phys(table->table);
> dte |= DTE_IRQ_REMAP_INTCTL;
> - dte |= DTE_IRQ_TABLE_LEN;
> + dte |= DTE_INTTABLEN;
> dte |= DTE_IRQ_REMAP_ENABLE;
>
> amd_iommu_dev_table[devid].data[2] = dte;
Reviewed-by: Jerry Snitselaar
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 04/15] lib/scatterlist: Add flag for indicating P2PDMA segments in an SGL
On 2020-12-09 9:04 p.m., Dan Williams wrote: > On Wed, Dec 9, 2020 at 6:07 PM Logan Gunthorpe wrote: >> >> >> >> On 2020-12-09 6:22 p.m., Dan Williams wrote: >>> On Mon, Nov 9, 2020 at 8:47 AM Logan Gunthorpe wrote: On 2020-11-09 2:12 a.m., Christoph Hellwig wrote: > On Fri, Nov 06, 2020 at 10:00:25AM -0700, Logan Gunthorpe wrote: >> We make use of the top bit of the dma_length to indicate a P2PDMA >> segment. > > I don't think "we" can. There is nothing limiting the size of a SGL > segment. Yes, I expected this would be the unacceptable part. Any alternative ideas? >>> >>> Why is the SG_P2PDMA_FLAG needed as compared to checking the SGL >>> segment-pages for is_pci_p2pdma_page()? >> >> Because the DMA and page segments in the SGL aren't necessarily aligned... >> >> The IOMMU implementations can coalesce multiple pages into fewer DMA >> address ranges, so the page pointed to by sg->page_link may not be the >> one that corresponds to the address in sg->dma_address for a given segment. >> >> If that makes sense -- it's not the easiest thing to explain. > > It does... > > Did someone already grab, or did you already consider the 3rd > available bit in page_link? AFAICS only SG_CHAIN and SG_END are > reserved. However, if you have a CONFIG_64BIT dependency for > user-directed p2pdma that would seem to allow SG_P2PDMA_FLAG to be > (0x4) in page_link. Hmm, I half considered that, but I had came to the conclusion that given the mis-alignment I shouldn't be using the page side of the SGL. However, reconsidering now, that might actually be a reasonable option. However, the CONFIG_64BIT dependency would have to be on all P2PDMA, because we'd need to replace pci_p2pdma_map_sg() in all cases. I'm not sure if this would be a restriction people care about. Thanks, Logan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken
On Thu, Dec 10, 2020 at 03:36:36PM +, Deucher, Alexander wrote: > [AMD Public Use] > > > -Original Message- > > From: Merger, Edgar [AUTOSOL/MAS/AUGS] > > Sent: Thursday, December 10, 2020 5:48 AM > > To: Deucher, Alexander ; Huang, Ray > > ; Kuehling, Felix > > Cc: Will Deacon ; linux-ker...@vger.kernel.org; > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn > > Helgaas ; Joerg Roedel ; Zhu, > > Changfeng > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as > > broken > > > > Alright. Done that. > > This should be it finally I believe. > > Which will be the initial kernel-version that incorporates that? > > Looks good to me. Bjorn, can you pick this up for PCI? Didn't apply cleanly, but I applied it by hand to pci/misc for v5.11. If all goes well it should appear in v5.11-rc1. https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/misc=23bb0d9a9fe70a8ff23f53af822f2c6e6f261818 > > -Original Message- > > From: Deucher, Alexander > > Sent: Mittwoch, 9. Dezember 2020 15:24 > > To: Merger, Edgar [AUTOSOL/MAS/AUGS] ; > > Huang, Ray ; Kuehling, Felix > > > > Cc: Will Deacon ; linux-ker...@vger.kernel.org; > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn > > Helgaas ; Joerg Roedel ; Zhu, > > Changfeng > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as > > broken > > > > [AMD Public Use] > > > > > -Original Message- > > > From: Merger, Edgar [AUTOSOL/MAS/AUGS] > > > > > Sent: Wednesday, December 9, 2020 2:59 AM > > > To: Deucher, Alexander ; Huang, Ray > > > ; Kuehling, Felix > > > Cc: Will Deacon ; linux-ker...@vger.kernel.org; > > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn > > > Helgaas ; Joerg Roedel ; Zhu, > > > Changfeng > > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as > > > broken > > > > > > Alex, > > > > > > I had to revise the patch. Please see attachment. It is actually two > > > more SSIDs affected to that. > > > > Other than some minor whitespace issues, the patch looks fine to me. > > Please align the subsystem_device lines and put the closing > > parenthesis on the same line as the last check. > > > > Thanks! > > > > Alex > > > > > > > > Best regards, > > > Edgar > > > > > > -Original Message- > > > From: Merger, Edgar [AUTOSOL/MAS/AUGS] > > > Sent: Dienstag, 8. Dezember 2020 09:23 > > > To: 'Deucher, Alexander' ; 'Huang, Ray' > > > ; 'Kuehling, Felix' > > > Cc: 'Will Deacon' ; 'linux-ker...@vger.kernel.org' > > > ; 'linux-...@vger.kernel.org' > > p...@vger.kernel.org>; 'iommu@lists.linux-foundation.org' > > > ; 'Bjorn Helgaas' > > > ; 'Joerg Roedel' ; 'Zhu, > > > Changfeng' > > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as > > > broken > > > > > > Applied the patch as in attachment. Verified that ATS for GPU-Device > > > had been disabled. See attachment "dmesg_ATS.log". > > > > > > Was running that build over night successfully. > > > > > > -Original Message- > > > From: Merger, Edgar [AUTOSOL/MAS/AUGS] > > > Sent: Montag, 7. Dezember 2020 05:53 > > > To: Deucher, Alexander ; Huang, Ray > > > ; Kuehling, Felix > > > Cc: Will Deacon ; linux-ker...@vger.kernel.org; > > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn > > > Helgaas ; Joerg Roedel ; Zhu, > > > Changfeng > > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as > > > broken > > > > > > Hi Alex, > > > > > > I believe in the patch file, this > > > + (pdev->subsystem_device == 0x0c19 || > > > + pdev->subsystem_device == 0x0c10)) > > > > > > Has to be changed to: > > > + (pdev->subsystem_device == 0xce19 || > > > + pdev->subsystem_device == 0xcc10)) > > > > > > Because our SSIDs are "ea50:ce19" and "ea50:cc10" respectively and > > > another one would "ea50:cc08". > > > > > > I will apply that patch and feedback the results soon plus the patch > > > file that I actually had applied. > > > > > > > > > -Original Message- > > > From: Deucher, Alexander > > > Sent: Montag, 30. November 2020 19:36 > > > To: Merger, Edgar [AUTOSOL/MAS/AUGS] > > ; > > > Huang, Ray ; Kuehling, Felix > > > > > > Cc: Will Deacon ; linux-ker...@vger.kernel.org; > > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn > > > Helgaas ; Joerg Roedel ; Zhu, > > > Changfeng > > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as > > > broken > > > > > > [AMD Public Use] > > > > > > > -Original Message- > > > > From: Merger, Edgar [AUTOSOL/MAS/AUGS] > > > > > > > Sent: Thursday, November 26, 2020 4:24 AM > > > > To: Deucher, Alexander ; Huang, Ray > > > > ; Kuehling, Felix > > > > Cc: Will Deacon ; linux-ker...@vger.kernel.org; > > > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; > > > > Bjorn Helgaas ; Joerg Roedel > > > > ; Zhu,
[PATCH] iommu/amd: Add sanity check for interrupt remapping table length macros
Currently, macros related to the interrupt remapping table length are
defined separately. This has resulted in an oversight in which one of
the macros were missed when changing the length. To prevent this,
redefine the macros to add built-in sanity check.
Also, rename macros to use the name of the DTE[IntTabLen] field as
specified in the AMD IOMMU specification. There is no functional change.
Suggested-by: Linus Torvalds
Reviewed-by: Tom Lendacky
Signed-off-by: Suravee Suthikulpanit
Cc: Will Deacon
Cc: Jerry Snitselaar
Cc: Joerg Roedel
---
drivers/iommu/amd/amd_iommu_types.h | 19 ++-
drivers/iommu/amd/init.c| 6 +++---
drivers/iommu/amd/iommu.c | 2 +-
3 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/amd/amd_iommu_types.h
b/drivers/iommu/amd/amd_iommu_types.h
index 494b42a31b7a..899ce62df3f0 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -255,11 +255,19 @@
/* Bit value definition for dte irq remapping fields*/
#define DTE_IRQ_PHYS_ADDR_MASK (((1ULL << 45)-1) << 6)
#define DTE_IRQ_REMAP_INTCTL_MASK (0x3ULL << 60)
-#define DTE_IRQ_TABLE_LEN_MASK (0xfULL << 1)
#define DTE_IRQ_REMAP_INTCTL(2ULL << 60)
-#define DTE_IRQ_TABLE_LEN (9ULL << 1)
#define DTE_IRQ_REMAP_ENABLE1ULL
+/*
+ * AMD IOMMU hardware only support 512 IRTEs despite
+ * the architectural limitation of 2048 entries.
+ */
+#define DTE_INTTAB_ALIGNMENT128
+#define DTE_INTTABLEN_VALUE 9ULL
+#define DTE_INTTABLEN (DTE_INTTABLEN_VALUE << 1)
+#define DTE_INTTABLEN_MASK (0xfULL << 1)
+#define MAX_IRQS_PER_TABLE (1 << DTE_INTTABLEN_VALUE)
+
#define PAGE_MODE_NONE0x00
#define PAGE_MODE_1_LEVEL 0x01
#define PAGE_MODE_2_LEVEL 0x02
@@ -409,13 +417,6 @@ extern bool amd_iommu_np_cache;
/* Only true if all IOMMUs support device IOTLBs */
extern bool amd_iommu_iotlb_sup;
-/*
- * AMD IOMMU hardware only support 512 IRTEs despite
- * the architectural limitation of 2048 entries.
- */
-#define MAX_IRQS_PER_TABLE 512
-#define IRQ_TABLE_ALIGNMENT128
-
struct irq_remap_table {
raw_spinlock_t lock;
unsigned min_index;
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 23a790f8f550..6bec8913d064 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -989,10 +989,10 @@ static bool copy_device_table(void)
irq_v = old_devtb[devid].data[2] & DTE_IRQ_REMAP_ENABLE;
int_ctl = old_devtb[devid].data[2] & DTE_IRQ_REMAP_INTCTL_MASK;
- int_tab_len = old_devtb[devid].data[2] & DTE_IRQ_TABLE_LEN_MASK;
+ int_tab_len = old_devtb[devid].data[2] & DTE_INTTABLEN_MASK;
if (irq_v && (int_ctl || int_tab_len)) {
if ((int_ctl != DTE_IRQ_REMAP_INTCTL) ||
- (int_tab_len != DTE_IRQ_TABLE_LEN)) {
+ (int_tab_len != DTE_INTTABLEN)) {
pr_err("Wrong old irq remapping flag: %#x\n",
devid);
return false;
}
@@ -2674,7 +2674,7 @@ static int __init early_amd_iommu_init(void)
remap_cache_sz = MAX_IRQS_PER_TABLE * (sizeof(u64) * 2);
amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
remap_cache_sz,
- IRQ_TABLE_ALIGNMENT,
+ DTE_INTTAB_ALIGNMENT,
0, NULL);
if (!amd_iommu_irq_cache)
goto out;
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index b9cf59443843..f7abf16d1e3a 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3191,7 +3191,7 @@ static void set_dte_irq_entry(u16 devid, struct
irq_remap_table *table)
dte &= ~DTE_IRQ_PHYS_ADDR_MASK;
dte |= iommu_virt_to_phys(table->table);
dte |= DTE_IRQ_REMAP_INTCTL;
- dte |= DTE_IRQ_TABLE_LEN;
+ dte |= DTE_INTTABLEN;
dte |= DTE_IRQ_REMAP_ENABLE;
amd_iommu_dev_table[devid].data[2] = dte;
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken
[AMD Public Use] > -Original Message- > From: Merger, Edgar [AUTOSOL/MAS/AUGS] > Sent: Thursday, December 10, 2020 5:48 AM > To: Deucher, Alexander ; Huang, Ray > ; Kuehling, Felix > Cc: Will Deacon ; linux-ker...@vger.kernel.org; > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn > Helgaas ; Joerg Roedel ; Zhu, > Changfeng > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as > broken > > Alright. Done that. > This should be it finally I believe. > Which will be the initial kernel-version that incorporates that? Looks good to me. Bjorn, can you pick this up for PCI? Alex > > -Original Message- > From: Deucher, Alexander > Sent: Mittwoch, 9. Dezember 2020 15:24 > To: Merger, Edgar [AUTOSOL/MAS/AUGS] ; > Huang, Ray ; Kuehling, Felix > > Cc: Will Deacon ; linux-ker...@vger.kernel.org; > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn > Helgaas ; Joerg Roedel ; Zhu, > Changfeng > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as > broken > > [AMD Public Use] > > > -Original Message- > > From: Merger, Edgar [AUTOSOL/MAS/AUGS] > > > Sent: Wednesday, December 9, 2020 2:59 AM > > To: Deucher, Alexander ; Huang, Ray > > ; Kuehling, Felix > > Cc: Will Deacon ; linux-ker...@vger.kernel.org; > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn > > Helgaas ; Joerg Roedel ; Zhu, > > Changfeng > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as > > broken > > > > Alex, > > > > I had to revise the patch. Please see attachment. It is actually two > > more SSIDs affected to that. > > Other than some minor whitespace issues, the patch looks fine to me. > Please align the subsystem_device lines and put the closing > parenthesis on the same line as the last check. > > Thanks! > > Alex > > > > > Best regards, > > Edgar > > > > -Original Message- > > From: Merger, Edgar [AUTOSOL/MAS/AUGS] > > Sent: Dienstag, 8. Dezember 2020 09:23 > > To: 'Deucher, Alexander' ; 'Huang, Ray' > > ; 'Kuehling, Felix' > > Cc: 'Will Deacon' ; 'linux-ker...@vger.kernel.org' > > ; 'linux-...@vger.kernel.org' > p...@vger.kernel.org>; 'iommu@lists.linux-foundation.org' > > ; 'Bjorn Helgaas' > > ; 'Joerg Roedel' ; 'Zhu, > > Changfeng' > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as > > broken > > > > Applied the patch as in attachment. Verified that ATS for GPU-Device > > had been disabled. See attachment "dmesg_ATS.log". > > > > Was running that build over night successfully. > > > > -Original Message- > > From: Merger, Edgar [AUTOSOL/MAS/AUGS] > > Sent: Montag, 7. Dezember 2020 05:53 > > To: Deucher, Alexander ; Huang, Ray > > ; Kuehling, Felix > > Cc: Will Deacon ; linux-ker...@vger.kernel.org; > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn > > Helgaas ; Joerg Roedel ; Zhu, > > Changfeng > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as > > broken > > > > Hi Alex, > > > > I believe in the patch file, this > > + (pdev->subsystem_device == 0x0c19 || > > +pdev->subsystem_device == 0x0c10)) > > > > Has to be changed to: > > + (pdev->subsystem_device == 0xce19 || > > +pdev->subsystem_device == 0xcc10)) > > > > Because our SSIDs are "ea50:ce19" and "ea50:cc10" respectively and > > another one would "ea50:cc08". > > > > I will apply that patch and feedback the results soon plus the patch > > file that I actually had applied. > > > > > > -Original Message- > > From: Deucher, Alexander > > Sent: Montag, 30. November 2020 19:36 > > To: Merger, Edgar [AUTOSOL/MAS/AUGS] > ; > > Huang, Ray ; Kuehling, Felix > > > > Cc: Will Deacon ; linux-ker...@vger.kernel.org; > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn > > Helgaas ; Joerg Roedel ; Zhu, > > Changfeng > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as > > broken > > > > [AMD Public Use] > > > > > -Original Message- > > > From: Merger, Edgar [AUTOSOL/MAS/AUGS] > > > > > Sent: Thursday, November 26, 2020 4:24 AM > > > To: Deucher, Alexander ; Huang, Ray > > > ; Kuehling, Felix > > > Cc: Will Deacon ; linux-ker...@vger.kernel.org; > > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; > > > Bjorn Helgaas ; Joerg Roedel > > > ; Zhu, Changfeng > > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS > > > as broken > > > > > > Alex, > > > > > > This is pretty much the same patch as what I have received from > > > Joerg previously, except that it is tied to the particular Emerson > > > platform and its derivatives (listed with Subsystem IDs). > > > > Right. As per my original point, I don't want to disable ATS on all > > Picasso chips because doing so would break GPU compute on them, so > > I'd like to apply this quirk as narrowly as possible. > > > > > > > > Below patch
[PATCH] dma-mapping: move hint unlikely for dma_mapping_error from drivers to core
Zillions of drivers use the unlikely() hint when checking the result of dma_mapping_error(). This is an inline function anyway, so we can move the hint into the function and remove it from drivers. >From time to time discussions pop up how effective unlikely() is, and that it should be used only if something is really very unlikely. I think that's the case here. Patch was created with some help from coccinelle. @@ expression dev, dma_addr; @@ - unlikely(dma_mapping_error(dev, dma_addr)) + dma_mapping_error(dev, dma_addr) Signed-off-by: Heiner Kallweit --- If ok, then tbd through which tree this is supposed to go. Patch is based on linux-next-20201210. --- drivers/crypto/cavium/cpt/cptvf_reqmanager.c | 3 +-- drivers/crypto/hisilicon/hpre/hpre_crypto.c | 2 +- .../marvell/octeontx/otx_cptvf_reqmgr.c | 5 ++-- drivers/crypto/mediatek/mtk-aes.c | 2 +- drivers/crypto/mediatek/mtk-sha.c | 6 ++--- drivers/crypto/qat/qat_common/qat_algs.c | 8 +++--- drivers/crypto/qat/qat_common/qat_asym_algs.c | 25 +-- drivers/i2c/busses/i2c-amd-mp2-plat.c | 2 +- drivers/infiniband/hw/hfi1/sdma.c | 2 +- drivers/net/ethernet/aeroflex/greth.c | 4 +-- drivers/net/ethernet/amazon/ena/ena_netdev.c | 8 +++--- .../net/ethernet/apm/xgene/xgene_enet_main.c | 2 +- .../net/ethernet/aquantia/atlantic/aq_nic.c | 5 ++-- .../net/ethernet/aquantia/atlantic/aq_ring.c | 2 +- drivers/net/ethernet/arc/emac_main.c | 2 +- .../net/ethernet/atheros/atl1c/atl1c_main.c | 6 ++--- drivers/net/ethernet/broadcom/bgmac.c | 4 +-- .../net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 10 .../ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 2 +- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 +-- drivers/net/ethernet/chelsio/cxgb4/sge.c | 4 +-- drivers/net/ethernet/chelsio/cxgb4vf/sge.c| 4 +-- drivers/net/ethernet/faraday/ftgmac100.c | 2 +- drivers/net/ethernet/faraday/ftmac100.c | 4 +-- .../net/ethernet/freescale/dpaa/dpaa_eth.c| 13 +- .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 12 - drivers/net/ethernet/freescale/enetc/enetc.c | 4 +-- drivers/net/ethernet/freescale/gianfar.c | 6 ++--- drivers/net/ethernet/google/gve/gve_tx.c | 4 +-- drivers/net/ethernet/hisilicon/hisi_femac.c | 2 +- drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 4 +-- .../net/ethernet/hisilicon/hns3/hns3_enet.c | 4 +-- drivers/net/ethernet/lantiq_xrx200.c | 5 ++-- drivers/net/ethernet/marvell/mv643xx_eth.c| 3 +-- drivers/net/ethernet/marvell/mvneta.c | 9 +++ drivers/net/ethernet/marvell/mvneta_bm.c | 2 +- .../net/ethernet/marvell/mvpp2/mvpp2_main.c | 8 +++--- .../marvell/octeontx2/nic/otx2_common.c | 2 +- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 10 drivers/net/ethernet/mellanox/mlx4/en_rx.c| 2 +- .../mellanox/mlx5/core/diag/rsc_dump.c| 2 +- .../net/ethernet/mellanox/mlx5/core/en/xdp.c | 2 +- .../mellanox/mlx5/core/en_accel/ktls_rx.c | 2 +- .../mellanox/mlx5/core/en_accel/ktls_tx.c | 2 +- .../net/ethernet/mellanox/mlx5/core/en_rx.c | 2 +- .../net/ethernet/mellanox/mlx5/core/en_tx.c | 6 ++--- .../net/ethernet/neterion/vxge/vxge-config.c | 6 ++--- .../net/ethernet/neterion/vxge/vxge-main.c| 6 ++--- drivers/net/ethernet/nvidia/forcedeth.c | 21 ++-- .../net/ethernet/pensando/ionic/ionic_txrx.c | 2 +- drivers/net/ethernet/qlogic/qed/qed_ll2.c | 4 +-- .../net/ethernet/qlogic/qede/qede_ethtool.c | 2 +- drivers/net/ethernet/qlogic/qede/qede_fp.c| 8 +++--- drivers/net/ethernet/realtek/r8169_main.c | 2 +- drivers/net/ethernet/rocker/rocker_main.c | 2 +- drivers/net/ethernet/sfc/falcon/rx.c | 3 +-- drivers/net/ethernet/sfc/falcon/tx.c | 4 +-- drivers/net/ethernet/sfc/rx_common.c | 3 +-- drivers/net/ethernet/sfc/tx_common.c | 4 +-- drivers/net/ethernet/sfc/tx_tso.c | 2 +- drivers/net/ethernet/sis/sis900.c | 24 -- drivers/net/ethernet/socionext/sni_ave.c | 2 +- drivers/net/ethernet/sun/sunhme.c | 8 +++--- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 6 ++--- drivers/net/ethernet/ti/netcp_core.c | 4 +-- drivers/net/ethernet/via/via-rhine.c | 2 +- .../net/ethernet/xilinx/xilinx_axienet_main.c | 6 ++--- drivers/net/wireless/ath/ath10k/htt_rx.c | 2 +- drivers/net/wireless/ath/ath10k/pci.c | 2 +- drivers/net/wireless/ath/ath10k/snoc.c| 2 +- drivers/net/wireless/ath/ath11k/ce.c | 2 +- drivers/net/wireless/ath/ath11k/dp_rx.c | 2 +- drivers/net/wireless/ath/ath5k/base.c | 2 +- drivers/net/wireless/ath/ath9k/beacon.c | 2 +- drivers/net/wireless/ath/ath9k/recv.c | 21 +++- drivers/net/wireless/ath/ath9k/xmi
Re: [PATCH] drivers/iommu: fix a null-ptr-deref bug in fsl_pamu_domain.c
On Wed, Dec 09, 2020 at 04:34:30AM -0800, tangzhenhao wrote:
> At line 362 in drivers/iommu/fsl_pamu_domain.c, the ret-val of
> kmem_cache_zalloc should be checked to avoid null-ptr-deref bug.
>
> Signed-off-by: tangzhenhao
> ---
> drivers/iommu/fsl_pamu_domain.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
> index b2110767caf4..9ebd5135f4a8 100644
> --- a/drivers/iommu/fsl_pamu_domain.c
> +++ b/drivers/iommu/fsl_pamu_domain.c
> @@ -360,6 +360,10 @@ static void attach_device(struct fsl_dma_domain
> *dma_domain, int liodn, struct d
> }
>
> info = kmem_cache_zalloc(iommu_devinfo_cache, GFP_ATOMIC);
> + if (!info) {
> + pr_debug("device_domain_info allocation failed\n");
> + return;
> + }
All the comments I made on your other patch [1] apply to this one too.
Will
[1] https://lore.kernel.org/r/20201210091326.GA9633@willie-the-truck
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] drivers/iommu: fix a null-ptr-deref bug in msm_iommu.c
On Sun, Dec 06, 2020 at 01:43:51AM -0800, tangzhenhao wrote:
> At line 600 in drivers/iommu/msm_iommu.c, the ret-val of kzalloc should be
> checked to avoid null-ptr-deref bug.
There's no need to mention the line number of the file name in the commit
message -- that information is already available in the diff. Instead,
please try to describe the problem that you're solving.
Have a look at Documentation/process/submitting-patches.rst.
> Signed-off-by: tangzhenhao
> ---
> drivers/iommu/msm_iommu.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index 3615cd6241c4..e3c576e5babb 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -598,6 +598,10 @@ static void insert_iommu_master(struct device *dev,
>
> if (list_empty(&(*iommu)->ctx_list)) {
> master = kzalloc(sizeof(*master), GFP_ATOMIC);
> + if (!master) {
> + dev_err(dev, "Failed to allocate IOMMU context bank
> instance\n");
No need to print an error here -- kzalloc should be plenty noisy enough
if an atomic allocation fails.
> + return;
Hmm, and then what? We haven't propagated the error, so how much further do
we get?
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
