Re: [PATCH] iommu/amd: Fix loop timeout issue in iommu_ga_log_enable()
Hi Joerg,
I love your patch! Yet something to improve:
[auto build test ERROR on joro-iommu/next]
[also build test ERROR on v5.17-rc2 next-20220131]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Joerg-Roedel/iommu-amd-Fix-loop-timeout-issue-in-iommu_ga_log_enable/20220201-002214
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: x86_64-allyesconfig
(https://download.01.org/0day-ci/archive/20220201/202202011448.2ufiizhd-...@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
#
https://github.com/0day-ci/linux/commit/5b5785526da0e2a6e793d2107d09afc9f310f17f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Joerg-Roedel/iommu-amd-Fix-loop-timeout-issue-in-iommu_ga_log_enable/20220201-002214
git checkout 5b5785526da0e2a6e793d2107d09afc9f310f17f
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
drivers/iommu/amd/init.c: In function 'iommu_ga_log_enable':
>> drivers/iommu/amd/init.c:837:3: error: implicit declaration of function
>> 'udelay' [-Werror=implicit-function-declaration]
837 | udelay(1);
| ^~
cc1: some warnings being treated as errors
vim +/udelay +837 drivers/iommu/amd/init.c
804
805 static int iommu_ga_log_enable(struct amd_iommu *iommu)
806 {
807 #ifdef CONFIG_IRQ_REMAP
808 u32 status, i;
809 u64 entry;
810
811 if (!iommu->ga_log)
812 return -EINVAL;
813
814 /* Check if already running */
815 status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
816 if (WARN_ON(status & (MMIO_STATUS_GALOG_RUN_MASK)))
817 return 0;
818
819 entry = iommu_virt_to_phys(iommu->ga_log) | GA_LOG_SIZE_512;
820 memcpy_toio(iommu->mmio_base + MMIO_GA_LOG_BASE_OFFSET,
821 , sizeof(entry));
822 entry = (iommu_virt_to_phys(iommu->ga_log_tail) &
823 (BIT_ULL(52)-1)) & ~7ULL;
824 memcpy_toio(iommu->mmio_base + MMIO_GA_LOG_TAIL_OFFSET,
825 , sizeof(entry));
826 writel(0x00, iommu->mmio_base + MMIO_GA_HEAD_OFFSET);
827 writel(0x00, iommu->mmio_base + MMIO_GA_TAIL_OFFSET);
828
829
830 iommu_feature_enable(iommu, CONTROL_GAINT_EN);
831 iommu_feature_enable(iommu, CONTROL_GALOG_EN);
832
833 for (i = 0; i < LOOP_TIMEOUT; ++i) {
834 status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
835 if (status & (MMIO_STATUS_GALOG_RUN_MASK))
836 break;
> 837 udelay(1);
838 }
839
840 if (WARN_ON(i >= LOOP_TIMEOUT))
841 return -EINVAL;
842 #endif /* CONFIG_IRQ_REMAP */
843 return 0;
844 }
845
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch V3 28/35] PCI/MSI: Simplify pci_irq_get_affinity()
On Mon, Jan 31, 2022 at 10:16:41PM +0100, Thomas Gleixner wrote:
> Guenter,
>
> On Mon, Jan 31 2022 at 07:21, Guenter Roeck wrote:
> > Sure. Please see http://server.roeck-us.net/qemu/x86/.
> > The logs are generated with with v5.16.4.
>
> thanks for providing the data. It definitely helped me to leave the
> state of not seeing the wood for the trees. Fix below.
>
> Thanks,
>
> tglx
> ---
> Subject: PCI/MSI: Remove bogus warning in pci_irq_get_affinity()
> From: Thomas Gleixner
> Date: Mon, 31 Jan 2022 22:02:46 +0100
>
> The recent overhaul of pci_irq_get_affinity() introduced a regression when
> pci_irq_get_affinity() is called for an MSI-X interrupt which was not
> allocated with affinity descriptor information.
>
> The original code just returned a NULL pointer in that case, but the rework
> added a WARN_ON() under the assumption that the corresponding WARN_ON() in
> the MSI case can be applied to MSI-X as well.
>
> In fact the MSI warning in the original code does not make sense either
> because it's legitimate to invoke pci_irq_get_affinity() for a MSI
> interrupt which was not allocated with affinity descriptor information.
>
> Remove it and just return NULL as the original code did.
>
> Fixes: f48235900182 ("PCI/MSI: Simplify pci_irq_get_affinity()")
> Reported-by: Guenter Roeck
> Signed-off-by: Thomas Gleixner
Tested-by: Guenter Roeck
Guenter
> ---
> drivers/pci/msi/msi.c |3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> --- a/drivers/pci/msi/msi.c
> +++ b/drivers/pci/msi/msi.c
> @@ -,7 +,8 @@ const struct cpumask *pci_irq_get_affini
> if (!desc)
> return cpu_possible_mask;
>
> - if (WARN_ON_ONCE(!desc->affinity))
> + /* MSI[X] interrupts can be allocated without affinity descriptor */
> + if (!desc->affinity)
> return NULL;
>
> /*
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch V3 28/35] PCI/MSI: Simplify pci_irq_get_affinity()
Guenter,
On Mon, Jan 31 2022 at 07:21, Guenter Roeck wrote:
> Sure. Please see http://server.roeck-us.net/qemu/x86/.
> The logs are generated with with v5.16.4.
thanks for providing the data. It definitely helped me to leave the
state of not seeing the wood for the trees. Fix below.
Thanks,
tglx
---
Subject: PCI/MSI: Remove bogus warning in pci_irq_get_affinity()
From: Thomas Gleixner
Date: Mon, 31 Jan 2022 22:02:46 +0100
The recent overhaul of pci_irq_get_affinity() introduced a regression when
pci_irq_get_affinity() is called for an MSI-X interrupt which was not
allocated with affinity descriptor information.
The original code just returned a NULL pointer in that case, but the rework
added a WARN_ON() under the assumption that the corresponding WARN_ON() in
the MSI case can be applied to MSI-X as well.
In fact the MSI warning in the original code does not make sense either
because it's legitimate to invoke pci_irq_get_affinity() for a MSI
interrupt which was not allocated with affinity descriptor information.
Remove it and just return NULL as the original code did.
Fixes: f48235900182 ("PCI/MSI: Simplify pci_irq_get_affinity()")
Reported-by: Guenter Roeck
Signed-off-by: Thomas Gleixner
---
drivers/pci/msi/msi.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -,7 +,8 @@ const struct cpumask *pci_irq_get_affini
if (!desc)
return cpu_possible_mask;
- if (WARN_ON_ONCE(!desc->affinity))
+ /* MSI[X] interrupts can be allocated without affinity descriptor */
+ if (!desc->affinity)
return NULL;
/*
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 00/24] Userspace P2PDMA with O_DIRECT NVMe devices
Hi Logan, On 1/27/2022 5:25 PM, Logan Gunthorpe wrote: Hi, This patchset continues my work to add userspace P2PDMA access using O_DIRECT NVMe devices. This posting fixes a lot of issues that were addresed in the last posting, which is here[1]. The patchset is rebased onto v5.17-rc1 as well as a rebased version of Ralph Campbell's patches to drop the ZONE_DEVICE page ref count offset. My understanding is this patch has some problems that are yet to be resolved but this will be the direction taken going forward and is included here to show that it is compatible with this patchset. The patchset enables userspace P2PDMA by allowing userspace to mmap() allocated chunks of the CMB. The resulting VMA can be passed only to O_DIRECT IO on NVMe backed files or block devices. A flag is added to GUP() in Patch 16, then Patches 17 through 21 wire this flag up based on whether the block queue indicates P2PDMA support. Patches 22 through 24 enable the CMB to be mapped into userspace by mmaping the nvme char device. This is relatively straightforward, however the one significant problem is that, presently, pci_p2pdma_map_sg() requires a homogeneous SGL with all P2PDMA pages or all regular pages. Enhancing GUP to support enforcing this rule would require a huge hack that I don't expect would be all that pallatable. So patches 3 to 16 add support for P2PDMA pages to dma_map_sg[table]() to the dma-direct and dma-iommu implementations. Thus systems without an IOMMU plus Intel and AMD IOMMUs are supported. (Other IOMMU implementations would then be unsupported, notably ARM and PowerPC but support would be added when they convert to dma-iommu). Am I understanding that an IO may use a mix of p2pdma and system pages? Would that cause inconsistent latencies? dma_map_sgtable() is preferred when dealing with P2PDMA memory as it will return -EREMOTEIO when the DMA device cannot map specific P2PDMA pages based on the existing rules in calc_map_type_and_dist(). The other issue is dma_unmap_sg() needs a flag to determine whether a given dma_addr_t was mapped regularly or as a PCI bus address. To allow this, a third flag is added to the page_link field in struct scatterlist. This effectively means support for P2PDMA will now depend on CONFIG_64BIT. Feedback welcome. This series is based on v5.17-rc1. A git branch is available here: https://github.com/sbates130272/linux-p2pmem/ p2pdma_user_cmb_v5 Thanks, Logan [1] https://lore.kernel.org/all/2027215410.3695-1-log...@deltatee.com/T/#u -- Changes since v4: - Rebase onto v5.17-rc1. - Included Ralph Cambell's patches which removes the ZONE_DEVICE page reference count offset. This is just to demonstrate that this series is compatible with that direction. - Added a comment in pci_p2pdma_map_sg_attrs(), per Chaitanya and included his Reviewed-by tags. - Patch 1 in the last series which cleaned up scatterlist.h has been upstreamed. - Dropped NEED_SG_DMA_BUS_ADDR_FLAG seeing depends on doesn't work with selected symbols, per Christoph. - Switched iov_iter_get_pages_[alloc_]flags to be exported with EXPORT_SYMBOL_GPL, per Christoph. - Renamed zone_device_pages_are_mergeable() to zone_device_pages_have_same_pgmap(), per Christoph. - Renamed .mmap_file_open operation in nvme_ctrl_ops to cdev_file_open(), per Christoph. Changes since v3: - Add some comment and commit message cleanups I had missed for v3, also moved the prototypes for some of the p2pdma helpers to dma-map-ops.h (which I missed in v3 and was suggested in v2). - Add separate cleanup patch for scatterlist.h and change the macros to functions. (Suggested by Chaitanya and Jason, respectively) - Rename sg_dma_mark_pci_p2pdma() and sg_is_dma_pci_p2pdma() to sg_dma_mark_bus_address() and sg_is_dma_bus_address() which is a more generic name (As requested by Jason) - Fixes to some comments and commit messages as suggested by Bjorn and Jason. - Ensure swiotlb is not used with P2PDMA pages. (Per Jason) - The sgtable coversion in RDMA was split out and sent upstream separately, the new patch is only the removal. (Per Jason) - Moved the FOLL_PCI_P2PDMA check outside of get_dev_pagemap() as Jason suggested this will be removed in the near term. - Add two patches to ensure that zone device pages with different pgmaps are never merged in the block layer or sg_alloc_append_table_from_pages() (Per Jason) - Ensure synchronize_rcu() or call_rcu() is used before returning pages to the genalloc. (Jason pointed out that pages are not gauranteed to be unused in all architectures until at least after an RCU grace period, and that synchronize_rcu() was likely too slow to use in the vma close operation. - Collected Acks and Reviews by Bjorn, Jason and Max. Logan Gunthorpe (22): lib/scatterlist: add flag for indicating P2PDMA segments in an SGL PCI/P2PDMA: Attempt to
Re: [PATCH v5 00/24] Userspace P2PDMA with O_DIRECT NVMe devices
On 2022-01-31 11:56 a.m., Jonathan Derrick wrote: >> This is relatively straightforward, however the one significant >> problem is that, presently, pci_p2pdma_map_sg() requires a homogeneous >> SGL with all P2PDMA pages or all regular pages. Enhancing GUP to >> support enforcing this rule would require a huge hack that I don't >> expect would be all that pallatable. So patches 3 to 16 add >> support for P2PDMA pages to dma_map_sg[table]() to the dma-direct >> and dma-iommu implementations. Thus systems without an IOMMU plus >> Intel and AMD IOMMUs are supported. (Other IOMMU implementations would >> then be unsupported, notably ARM and PowerPC but support would be added >> when they convert to dma-iommu). > Am I understanding that an IO may use a mix of p2pdma and system pages? > Would that cause inconsistent latencies? Yes, that certainly would be a possibility. People developing applications that do such mixing would have to weight that issue if latency is something they care about. But it's counter productive and causes other difficulties for the kernel to enforce only homogenous IO. Logan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Fix loop timeout issue in iommu_ga_log_enable()
On 31/01/2022 16:17, Joerg Roedel wrote:
From: Joerg Roedel
The polling loop for the register change in iommu_ga_log_enable() needs
to have a udelay() in it. Otherwise the CPU might be faster than the
IOMMU hardware and wrongly trigger the WARN_ON() further down the code
stream.
Fixes: 8bda0cfbdc1a ("iommu/amd: Detect and initialize guest vAPIC log")
Signed-off-by: Joerg Roedel
---
drivers/iommu/amd/init.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index dc338acf3338..d2e09d53851f 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -834,6 +834,7 @@ static int iommu_ga_log_enable(struct amd_iommu *iommu)
status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
if (status & (MMIO_STATUS_GALOG_RUN_MASK))
break;
+ udelay(1);
Maybe readl_relaxed_poll_timeout_atomic() could be used instead
Thanks,
John
}
if (WARN_ON(i >= LOOP_TIMEOUT))
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/amd: Fix loop timeout issue in iommu_ga_log_enable()
From: Joerg Roedel
The polling loop for the register change in iommu_ga_log_enable() needs
to have a udelay() in it. Otherwise the CPU might be faster than the
IOMMU hardware and wrongly trigger the WARN_ON() further down the code
stream.
Fixes: 8bda0cfbdc1a ("iommu/amd: Detect and initialize guest vAPIC log")
Signed-off-by: Joerg Roedel
---
drivers/iommu/amd/init.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index dc338acf3338..d2e09d53851f 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -834,6 +834,7 @@ static int iommu_ga_log_enable(struct amd_iommu *iommu)
status = readl(iommu->mmio_base + MMIO_STATUS_OFFSET);
if (status & (MMIO_STATUS_GALOG_RUN_MASK))
break;
+ udelay(1);
}
if (WARN_ON(i >= LOOP_TIMEOUT))
--
2.34.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] iommu/vt-d: Fix potential memory leak in intel_setup_irq_remapping()
On Fri, Jan 28, 2022 at 11:10:02AM +0800, Lu Baolu wrote:
> From: Guoqing Jiang
>
> After commit e3beca48a45b ("irqdomain/treewide: Keep firmware node
> unconditionally allocated"). For tear down scenario, fn is only freed
> after fail to allocate ir_domain, though it also should be freed in case
> dmar_enable_qi returns error.
>
> Besides free fn, irq_domain and ir_msi_domain need to be removed as well
> if intel_setup_irq_remapping fails to enable queued invalidation.
>
> Improve the rewinding path by add out_free_ir_domain and out_free_fwnode
> lables per Baolu's suggestion.
>
> Fixes: e3beca48a45b ("irqdomain/treewide: Keep firmware node unconditionally
> allocated")
> Suggested-by: Lu Baolu
> Signed-off-by: Guoqing Jiang
> Link: https://lore.kernel.org/r/20220119063640.16864-1-guoqing.ji...@linux.dev
> Signed-off-by: Lu Baolu
> ---
> drivers/iommu/intel/irq_remapping.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
Applied this one, thanks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] iommu/vt-d: Fix PCI bus rescan device hot add
On Mon, Jan 31, 2022 at 01:53:06PM +, Robin Murphy wrote:
> Indeed I very nearly asked whether we couldn't just call these from
> intel_iommu_{probe,release}_device() directly, but it looks like they also
> interact with the interrupt remapping stuff which can be built independently
> of the IOMMU API :(
Okay, but having two notifiers is still ugly. Can we only register a
notifier when IRQ-remapping is used without IOMMU-API? In this case a
single notifier be sufficient.
Regards,
Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Fix some W=1 warnings
On Fri, Jan 28, 2022 at 06:44:33PM +0800, John Garry wrote: > The code is mostly free of W=1 warning, so fix the following: > > drivers/iommu/iommu.c:996: warning: expecting prototype for > iommu_group_for_each_dev(). Prototype was for __iommu_group_for_each_dev() > instead > drivers/iommu/iommu.c:3048: warning: Function parameter or member 'drvdata' > not described in 'iommu_sva_bind_device' > drivers/iommu/ioasid.c:354: warning: Function parameter or member 'ioasid' > not described in 'ioasid_get' > drivers/iommu/omap-iommu.c:1098: warning: expecting prototype for > omap_iommu_suspend_prepare(). Prototype was for omap_iommu_prepare() instead > > Signed-off-by: John Garry Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: nvme: IO_PAGE_FAULT logged with Intel SSDPEKKF512G8
On Tue, Jan 18, 2022 at 06:01:06PM +0100, Paul Menzel wrote: > > > $ dmesg --level=err > > > [4.194306] nvme :01:00.0: AMD-Vi: Event logged > > > [IO_PAGE_FAULT domain=0x000c address=0xc080 flags=0x0050] > > > [4.206970] nvme :01:00.0: AMD-Vi: Event logged > > > [IO_PAGE_FAULT domain=0x000c address=0xc000 flags=0x0050] This was caused by a DMA read to a write-only page. Looks like a bug in the driver or the devices firmware. Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [REPOST PATCH v4] iommu: Fix potential use-after-free during probe
On Mon, Jan 31, 2022 at 12:42:35PM +0530, Vijayanand Jitta wrote: > drivers/iommu/iommu.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) Applied, thanks Vijayanand. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix missing put_device() call in tegra_smmu_find
Thierry,
does this look correct to you?
Thanks,
Joerg
On Fri, Jan 07, 2022 at 08:09:11AM +, Miaoqian Lin wrote:
> The reference taken by 'of_find_device_by_node()' must be released when
> not needed anymore.
> Add the corresponding 'put_device()' in the error handling path.
>
> Fixes: 765a9d1d02b2 ("iommu/tegra-smmu: Fix mc errors on tegra124-nyan")
> Signed-off-by: Miaoqian Lin
> ---
> drivers/iommu/tegra-smmu.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index e900e3c46903..2561ce8a2ce8 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -808,8 +808,10 @@ static struct tegra_smmu *tegra_smmu_find(struct
> device_node *np)
> return NULL;
>
> mc = platform_get_drvdata(pdev);
> - if (!mc)
> + if (!mc) {
> + put_device(>dev);
> return NULL;
> + }
>
> return mc->smmu;
> }
> --
> 2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch V3 28/35] PCI/MSI: Simplify pci_irq_get_affinity()
On 1/31/22 03:27, Thomas Gleixner wrote: On Sun, Jan 30 2022 at 09:12, Guenter Roeck wrote: On Fri, Dec 10, 2021 at 11:19:26PM +0100, Thomas Gleixner wrote: This patch results in the following runtime warning when booting x86 (32 bit) nosmp images from NVME in qemu. [ 14.825482] nvme nvme0: 1/0/0 default/read/poll queues ILLOPC: ca7c6d10: 0f 0b [ 14.826188] [ cut here ] [ 14.826307] WARNING: CPU: 0 PID: 7 at drivers/pci/msi/msi.c:1114 pci_irq_get_affinity+0x80/0x90 This complains about msi_desc->affinity being NULL. git bisect bad f48235900182d64537c6e8f8dc0932b57a1a0638 # first bad commit: [f48235900182d64537c6e8f8dc0932b57a1a0638] PCI/MSI: Simplify pci_irq_get_affinity() Hrm. Can you please provide dmesg and /proc/interrupts from a kernel before that commit? Sure. Please see http://server.roeck-us.net/qemu/x86/. The logs are generated with with v5.16.4. Guenter ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] iommu/vt-d: Fix PCI bus rescan device hot add
On 2022-01-30 07:43, Joerg Roedel wrote:
Hi Jacob, Baolu,
On Fri, Jan 28, 2022 at 11:10:01AM +0800, Lu Baolu wrote:
During PCI bus rescan, adding new devices involve two notifiers.
1. dmar_pci_bus_notifier()
2. iommu_bus_notifier()
The current code sets #1 as low priority (INT_MIN) which resulted in #2
being invoked first. The result is that struct device pointer cannot be
found in DRHD search for the new device's DMAR/IOMMU. Subsequently, the
device is put under the "catch-all" IOMMU instead of the correct one.
There are actually iommu_ops pointers invoked from iommu_bus_notifier()
into IOMMU driver code. Can those be used to enforce the ordering in a
more reliable way?
Indeed I very nearly asked whether we couldn't just call these from
intel_iommu_{probe,release}_device() directly, but it looks like they
also interact with the interrupt remapping stuff which can be built
independently of the IOMMU API :(
Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 02/13] iommu/mediatek-v1: Free the existed fwspec if the master dev already has
On 28/01/2022 13:45, Mauro Carvalho Chehab wrote:
Em Fri, 28 Jan 2022 13:40:55 +0100
Mauro Carvalho Chehab escreveu:
Hi Matthias/Yong,
Are you ok if this patch gets merged via the media tree together with the
remaining series, or do you prefer to apply it via SoC tree instead?
Same questions for other patches touching files outside drivers/media
on this pull request:
https://patchwork.kernel.org/project/linux-mediatek/patch/7af52d61-47c7-581d-62ed-76a7f8315...@xs4all.nl/
Looks good to me.
Please let me know once you accepted the pull request and I'll queue the DTS
related changes from this series.
Regards,
Matthias
Like those:
0004-0013-iommu-mediatek-v1-Free-the-existed-fwspec-if-the-mas.patch
0005-0013-iommu-mediatek-Return-ENODEV-if-the-device-is-NULL.patch
0006-0013-iommu-mediatek-Add-probe_defer-for-smi-larb.patch
0007-0013-iommu-mediatek-Add-device_link-between-the-consumer-.patch
Regards,
Mauro
Regards,
Mauro
Em Mon, 17 Jan 2022 15:04:59 +0800
Yong Wu escreveu:
When the iommu master device enters of_iommu_xlate, the ops may be
NULL(iommu dev is defered), then it will initialize the fwspec here:
[] (dev_iommu_fwspec_set) from []
(iommu_fwspec_init+0xbc/0xd4)
[] (iommu_fwspec_init) from []
(of_iommu_xlate+0x7c/0x12c)
[] (of_iommu_xlate) from []
(of_iommu_configure+0x144/0x1e8)
BUT the mtk_iommu_v1.c only supports arm32, the probing flow still is a bit
weird. We always expect create the fwspec internally. otherwise it will
enter here and return fail.
static int mtk_iommu_create_mapping(struct device *dev,
struct of_phandle_args *args)
{
...
if (!fwspec) {
} else if (dev_iommu_fwspec_get(dev)->ops != _iommu_ops) {
>>Enter here. return fail.
return -EINVAL;
}
...
}
Thus, Free the existed fwspec if the master device already has fwspec.
This issue is reported at:
https://lore.kernel.org/linux-mediatek/trinity-7d9ebdc9-4849-4d93-bfb5-429dcb4ee449-1626253158870@3c-app-gmx-bs01/
Reported-by: Frank Wunderlich
Tested-by: Frank Wunderlich # BPI-R2/MT7623
Signed-off-by: Yong Wu
Acked-by: Joerg Roedel
Acked-by: AngeloGioacchino Del Regno
---
drivers/iommu/mtk_iommu_v1.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index be22fcf988ce..1467ba1e4417 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -425,6 +425,15 @@ static struct iommu_device *mtk_iommu_probe_device(struct
device *dev)
struct mtk_iommu_data *data;
int err, idx = 0;
+ /*
+* In the deferred case, free the existed fwspec.
+* Always initialize the fwspec internally.
+*/
+ if (fwspec) {
+ iommu_fwspec_free(dev);
+ fwspec = dev_iommu_fwspec_get(dev);
+ }
+
while (!of_parse_phandle_with_args(dev->of_node, "iommus",
"#iommu-cells",
idx, _spec)) {
Thanks,
Mauro
Thanks,
Mauro
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [patch V3 28/35] PCI/MSI: Simplify pci_irq_get_affinity()
On Sun, Jan 30 2022 at 09:12, Guenter Roeck wrote: > On Fri, Dec 10, 2021 at 11:19:26PM +0100, Thomas Gleixner wrote: > This patch results in the following runtime warning when booting x86 > (32 bit) nosmp images from NVME in qemu. > > [ 14.825482] nvme nvme0: 1/0/0 default/read/poll queues > ILLOPC: ca7c6d10: 0f 0b > [ 14.826188] [ cut here ] > [ 14.826307] WARNING: CPU: 0 PID: 7 at drivers/pci/msi/msi.c:1114 > pci_irq_get_affinity+0x80/0x90 This complains about msi_desc->affinity being NULL. > git bisect bad f48235900182d64537c6e8f8dc0932b57a1a0638 > # first bad commit: [f48235900182d64537c6e8f8dc0932b57a1a0638] PCI/MSI: > Simplify pci_irq_get_affinity() Hrm. Can you please provide dmesg and /proc/interrupts from a kernel before that commit? Thanks, tglx ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] component: Add common helpers for compare/release functions
On Fri, 28 Jan 2022, Daniel Vetter wrote:
> On Fri, Jan 28, 2022 at 04:11:01PM +0800, Yong Wu wrote:
>> The component requires the compare/release functions, there are so many
>> copy in current kernel. Just define three common helpers for them.
>> No functional change.
>>
>> Signed-off-by: Yong Wu
>> ---
>> Base on v5.17-rc1
>> ---
>> .../gpu/drm/arm/display/komeda/komeda_drv.c| 5 -
>> drivers/gpu/drm/arm/hdlcd_drv.c| 7 +--
>> drivers/gpu/drm/armada/armada_drv.c| 5 -
>> drivers/gpu/drm/drm_of.c | 8 +---
>> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 7 ---
>> drivers/gpu/drm/exynos/exynos_drm_drv.c| 5 -
>> .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c| 5 -
>> drivers/gpu/drm/imx/imx-drm-core.c | 4 ++--
>> drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 5 -
>> drivers/gpu/drm/mcde/mcde_drv.c| 7 +--
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 -
>> drivers/gpu/drm/meson/meson_drv.c | 8
>> drivers/gpu/drm/msm/msm_drv.c | 9 -
>> drivers/gpu/drm/omapdrm/dss/dss.c | 8 +---
>> drivers/gpu/drm/rockchip/rockchip_drm_drv.c| 5 -
>> drivers/gpu/drm/sti/sti_drv.c | 5 -
>> drivers/gpu/drm/sun4i/sun4i_drv.c | 9 -
>> drivers/gpu/drm/vc4/vc4_drv.c | 5 -
>> drivers/iommu/mtk_iommu.h | 10 --
>> drivers/power/supply/ab8500_charger.c | 8 +---
>> drivers/video/fbdev/omap2/omapfb/dss/dss.c | 8 +---
>> include/linux/component.h | 18 ++
>> sound/soc/codecs/wcd938x.c | 16 ++--
>
> Seems like a neat idea. Please add kerneldoc for the new functions you're
> adding (bonus point for an example in there) and make sure it all renders
> correctly in
>
> $ make htmldoc
>
> Also please split up the patch series per-driver and add the maintainers
> to each patches' Cc: list. With that I think this should be ready for
> merging.
Aren't the function names perhaps a bit short and generic for the global
namespace though? If you encounter compare_of, release_of, or
compare_dev in code, component.h is not where you'd expect to find them.
BR,
Jani.
>> diff --git a/include/linux/component.h b/include/linux/component.h
>> index 16de18f473d7..5a7468ea827c 100644
>> --- a/include/linux/component.h
>> +++ b/include/linux/component.h
>> @@ -2,6 +2,8 @@
>> #ifndef COMPONENT_H
>> #define COMPONENT_H
>>
>> +#include
>> +#include
>> #include
>>
>>
>> @@ -82,6 +84,22 @@ struct component_master_ops {
>> void (*unbind)(struct device *master);
>> };
>>
>> +/* A set common helpers for compare/release functions */
>> +static inline int compare_of(struct device *dev, void *data)
>> +{
>> +return dev->of_node == data;
>> +}
>> +
>> +static inline void release_of(struct device *dev, void *data)
>> +{
>> +of_node_put(data);
>> +}
>> +
>> +static inline int compare_dev(struct device *dev, void *data)
>> +{
>> +return dev == data;
>> +}
>> +
>> void component_master_del(struct device *,
>> const struct component_master_ops *);
>>
--
Jani Nikula, Intel Open Source Graphics Center
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 00/13] Clean up "mediatek,larb"
On 1/17/22 12:49, Matthias Brugger wrote: > > > On 17/01/2022 11:27, AngeloGioacchino Del Regno wrote: >> Il 17/01/22 08:04, Yong Wu ha scritto: >>> MediaTek IOMMU block diagram always like below: >>> >>> M4U >>> | >>> smi-common >>> | >>> - >>> | | ... >>> | | >>> larb1 larb2 >>> | | >>> vdec venc >>> >>> All the consumer connect with smi-larb, then connect with smi-common. >>> >>> When the consumer works, it should enable the smi-larb's power which also >>> need enable the smi-common's power firstly. >>> >>> Thus, Firstly, use the device link connect the consumer and the >>> smi-larbs. then add device link between the smi-larb and smi-common. >>> >>> After adding the device_link, then "mediatek,larb" property can be removed. >>> the iommu consumer don't need call the mtk_smi_larb_get/put to enable >>> the power and clock of smi-larb and smi-common. >>> >>> Base on the media branch [1] and a jpeg dtbinding patchset[2] that already >>> got >>> the necessary R-b. >>> >>> [1] git://linuxtv.org/hverkuil/media_tree.git tags/br-v5.18d >>> [2] >>> https://lore.kernel.org/linux-mediatek/20211206130425.184420-1-hsi...@chromium.org/ >>> >>> Change notes: >>> v10: a) Rebase on the media tree. Respin the "media: mtk-vcodec:" patches. >>> b) Add Joerg's Ack for iommu patches. >>> >>> v9: >>> https://lore.kernel.org/linux-mediatek/2022105509.12010-1-yong...@mediatek.com/ >>> 1) Add return -ENODEV when the dev is null. >>> 2) Add more strict about the case that a iommu consume device use the >>> ports in >>> different larbs. Don't allow this case. >>> 3) Remove two codec interface: mtk_vcodec_release_enc/dec_pm since it >>> only has one >>> line now. >>> >>> v8: >>> https://lore.kernel.org/linux-mediatek/20210929013719.25120-1-yong...@mediatek.com/ >>> 1) Rebase on v5.15-rc1. >>> 2) Don't rebase the below mdp patchset that may still need more >>> discuss. >>> >>> https://lore.kernel.org/linux-mediatek/20210709022324.1607884-1-ei...@chromium.org/ >>> 3) Add Frank's Tested-by. Remove Dafna's Tested-by as he requested. >>> >>> v7: >>> https://lore.kernel.org/linux-mediatek/20210730025238.22456-1-yong...@mediatek.com/ >>> 1) Fix a arm32 boot fail issue. reported from Frank. >>> 2) Add a return fail in the mtk drm. suggested by Dafna. >>> >>> v6: >>> https://lore.kernel.org/linux-mediatek/20210714025626.5528-1-yong...@mediatek.com/ >>> 1) rebase on v5.14-rc1. >>> 2) Fix the issue commented in v5 from Dafna and Hsin-Yi. >>> 3) Remove the patches about using pm_runtime_resume_and_get since they >>> have >>> already been merged by other patches. >>> >>> v5: >>> https://lore.kernel.org/linux-mediatek/20210410091128.31823-1-yong...@mediatek.com/ >>> 1) Base v5.12-rc2. >>> 2) Remove changing the mtk-iommu to module_platform_driver patch, It >>> have already been a >>> independent patch. >>> >>> v4: >>> https://lore.kernel.org/linux-mediatek/1590826218-23653-1-git-send-email-yong...@mediatek.com/ >>> base on v5.7-rc1. >>> 1) Move drm PM patch before smi patchs. >>> 2) Change builtin_platform_driver to module_platform_driver since we may >>> need >>> build as module. >>> 3) Rebase many patchset as above. >>> >>> v3: >>> https://lore.kernel.org/linux-iommu/1567503456-24725-1-git-send-email-yong...@mediatek.com/ >>> 1) rebase on v5.3-rc1 and the latest mt8183 patchset. >>> 2) Use device_is_bound to check whether the driver is ready from >>> Matthias. >>> 3) Add DL_FLAG_STATELESS flag when calling device_link_add and explain >>> the >>> reason in the commit message[3/14]. >>> 4) Add a display patch[12/14] into this series. otherwise it may affect >>> display HW fastlogo even though it don't happen in mt8183. >>> v2: >>> https://lore.kernel.org/linux-iommu/1560171313-28299-1-git-send-email-yong...@mediatek.com/ >>> 1) rebase on v5.2-rc1. >>> 2) Move adding device_link between the consumer and smi-larb into >>> iommu_add_device from Robin. >>> 3) add DL_FLAG_AUTOREMOVE_CONSUMER even though the smi is built-in from >>> Evan. >>> 4) Remove the shutdown callback in iommu. >>> >>> v1: >>> https://lore.kernel.org/linux-iommu/1546318276-18993-1-git-send-email-yong...@mediatek.com/ >>> >>> Yong Wu (12): >>> dt-binding: mediatek: Get rid of mediatek,larb for multimedia HW >>> iommu/mediatek-v1: Free the existed fwspec if the master dev already >>> has >>> iommu/mediatek: Return ENODEV if the device is NULL >>> iommu/mediatek: Add probe_defer for smi-larb >>> iommu/mediatek: Add device_link between the consumer and the larb >>> devices >>> media: mtk-jpeg: Get rid of mtk_smi_larb_get/put >>> media: mtk-mdp: Get rid of mtk_smi_larb_get/put >>> drm/mediatek: Get rid of mtk_smi_larb_get/put >>> media: mtk-vcodec: Get rid of
Re: [PATCH 2/2] iommu/mediatek: Add mt8186 iommu support
Il 28/01/22 10:39, Yong Wu ha scritto:
On Thu, 2022-01-27 at 12:28 +0100, AngeloGioacchino Del Regno wrote:
Il 25/01/22 10:32, Yong Wu ha scritto:
Add mt8186 iommu supports.
Signed-off-by: Anan Sun
Signed-off-by: Yong Wu
---
drivers/iommu/mtk_iommu.c | 17 +
1 file changed, 17 insertions(+)
[snip]
static const struct mtk_iommu_plat_data mt8192_data = {
.m4u_plat = M4U_MT8192,
.flags = HAS_BCLK | HAS_SUB_COMM_2BITS |
OUT_ORDER_WR_EN |
@@ -1470,6 +1486,7 @@ static const struct of_device_id
mtk_iommu_of_ids[] = {
{ .compatible = "mediatek,mt8167-m4u", .data = _data},
{ .compatible = "mediatek,mt8173-m4u", .data = _data},
{ .compatible = "mediatek,mt8183-m4u", .data = _data},
+ { .compatible = "mediatek,mt8186-iommu-mm", .data =
_data_mm},
Hello!
Is there any particular reason why this compatible is not
"mediatek,mt8186-m4u"?
There is no special reason. In the previous SoC, We only support MM
IOMMU, it was called by "m4u". In the lastest SoC, We have the other
types IOMMU, like for INFRA masters and APU, thus they are called "mm
iommu", "infra iommu" and "apu iommu". Of course, "m4u" means "mm
iommu".
I suggest, at this point, to change it to "mediatek,mt8186-m4u" for naming
consistency with the other bindings and to avoid any kind of confusion.
Thank you!
Thanks,
Angelo
{ .compatible = "mediatek,mt8192-m4u", .data = _data},
{ .compatible = "mediatek,mt8195-iommu-infra", .data =
_data_infra},
{ .compatible = "mediatek,mt8195-iommu-vdo", .data =
_data_vdo},
___
Linux-mediatek mailing list
linux-media...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
