Re: [PATCH v2 15/16] powerpc/powernv/sriov: Make single PE mode a per-BAR setting

[Nathan Chancellor <natechancellor@gmail.com>]

On Sun, Aug 02, 2020 at 11:12:23PM +1000, Michael Ellerman wrote:
> Nathan Chancellor <natechancellor@gmail.com> writes:
> > On Wed, Jul 22, 2020 at 04:57:14PM +1000, Oliver O'Halloran wrote:
> >> Using single PE BARs to map an SR-IOV BAR is really a choice about what
> >> strategy to use when mapping a BAR. It doesn't make much sense for this to
> >> be a global setting since a device might have one large BAR which needs to
> >> be mapped with single PE windows and another smaller BAR that can be mapped
> >> with a regular segmented window. Make the segmented vs single decision a
> >> per-BAR setting and clean up the logic that decides which mode to use.
> >> 
> >> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> >> ---
> >> v2: Dropped unused total_vfs variables in pnv_pci_ioda_fixup_iov_resources()
> >>     Dropped bar_no from pnv_pci_iov_resource_alignment()
> >>     Minor re-wording of comments.
> >> ---
> >>  arch/powerpc/platforms/powernv/pci-sriov.c | 131 ++++++++++-----------
> >>  arch/powerpc/platforms/powernv/pci.h       |  11 +-
> >>  2 files changed, 73 insertions(+), 69 deletions(-)
> >> 
> >> diff --git a/arch/powerpc/platforms/powernv/pci-sriov.c b/arch/powerpc/platforms/powernv/pci-sriov.c
> >> index ce8ad6851d73..76215d01405b 100644
> >> --- a/arch/powerpc/platforms/powernv/pci-sriov.c
> >> +++ b/arch/powerpc/platforms/powernv/pci-sriov.c
> >> @@ -260,42 +256,40 @@ void pnv_pci_ioda_fixup_iov(struct pci_dev *pdev)
> >>  resource_size_t pnv_pci_iov_resource_alignment(struct pci_dev *pdev,
> >>  						      int resno)
> >>  {
> >> -	struct pnv_phb *phb = pci_bus_to_pnvhb(pdev->bus);
> >>  	struct pnv_iov_data *iov = pnv_iov_get(pdev);
> >>  	resource_size_t align;
> >>  
> >> +	/*
> >> +	 * iov can be null if we have an SR-IOV device with IOV BAR that can't
> >> +	 * be placed in the m64 space (i.e. The BAR is 32bit or non-prefetch).
> >> +	 * In that case we don't allow VFs to be enabled since one of their
> >> +	 * BARs would not be placed in the correct PE.
> >> +	 */
> >> +	if (!iov)
> >> +		return align;
> >> +	if (!iov->vfs_expanded)
> >> +		return align;
> >> +
> >> +	align = pci_iov_resource_size(pdev, resno);
> 
> That's, oof.
> 
> > I am not sure if it has been reported yet but clang points out that
> > align is initialized after its use:
> >
> > arch/powerpc/platforms/powernv/pci-sriov.c:267:10: warning: variable 'align' is uninitialized when used here [-Wuninitialized]
> >                 return align;
> >                        ^~~~~
> > arch/powerpc/platforms/powernv/pci-sriov.c:258:23: note: initialize the variable 'align' to silence this warning
> >         resource_size_t align;
> >                              ^
> >                               = 0
> > 1 warning generated.
> 
> But I can't get gcc to warn about it?
> 
> It produces some code, so it's not like the whole function has been
> elided or something. I'm confused.
> 
> cheers

-Wmaybe-uninitialized was disabled in commit 78a5255ffb6a ("Stop the
ad-hoc games with -Wno-maybe-initialized") upstream so GCC won't warn on
stuff like this anymore.

I would assume the function should still be generated since those checks
are relevant, just the return value is bogus.

Cheers,
Nathan