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

[Michael Ellerman]

Alexey Kardashevskiy <aik at ozlabs.ru> writes:
> On 19/12/2018 11:17, Michael Ellerman wrote:
>> Alexey Kardashevskiy <aik at ozlabs.ru> writes:
>>> 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 {
>> ...
>>> +
>>> +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