Re: [PATCH kernel v5 14/20] powerpc/powernv/npu: Add compound IOMMU groups

Alexey Kardashevskiy [off-list ref] writes:

On 19/12/2018 11:17, Michael Ellerman wrote:

quoted

Alexey Kardashevskiy [off-list ref] writes:

quoted

diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
index dc629ee..3468eaa 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c

@@ -372,8 +358,263 @@ struct npu {

...

quoted

+
+static void pnv_comp_attach_table_group(struct npu_comp *npucomp,
+		struct pnv_ioda_pe *pe)
+{
+	if (WARN_ON(npucomp->pe_num == NV_NPU_MAX_PE_NUM))
+		return;
+
+	npucomp->pe[npucomp->pe_num] = pe;
+	++npucomp->pe_num;
+}
+
+struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
+{
+	struct iommu_table_group *table_group;
+	struct npu_comp *npucomp;
+	struct pci_dev *gpdev = NULL;
+	struct pci_controller *hose;
+	struct pci_dev *npdev;
+
+	list_for_each_entry(gpdev, &pe->pbus->devices, bus_list) {
+		npdev = pnv_pci_get_npu_dev(gpdev, 0);
+		if (npdev)
+			break;
+	}
+
+	if (!npdev)
+		/* It is not an NPU attached device, skip */
+		return NULL;

This breaks some configs with:

  arch/powerpc/platforms/powernv/npu-dma.c:550:5: error: 'npdev' may be used uninitialized in this function [-Werror=uninitialized]

gcc 5, 7 and 8 do not warn about this, I have to disable
list_for_each_entry() above to recreate this.

I even compiled gcc 5.5 which some of your buildmachines use and yet no
error on this:

make O=/home/aik/pbuild/kernel-le/ KCFLAGS=-Werror=all ARCH=powerpc
CROSS_COMPILE=/opt/cross/gcc-powerpc64le-linux-5.5.0-nolibc/bin/powerpc64le-linux-
arch/powerpc/platforms/powernv/npu-dma.o

Odd. That error is from kisskb like the others.

http://kisskb.ellerman.id.au/kisskb/buildresult/13622793/

Seems it's GCC 4.6.3 that is producing that one. Not sure why newer
compilers aren't warning about it.


It's pretty obviously correct though, unless you can prove that the list
is never empty?

	struct pci_dev *npdev;

	list_for_each_entry(gpdev, &pe->pbus->devices, bus_list) {
		npdev = pnv_pci_get_npu_dev(gpdev, 0);
		if (npdev)
			break;
	}

	if (!npdev)
		/* It is not an NPU attached device, skip */
		return NULL;


cheers