On Sun, Aug 02, 2020 at 11:12:23PM +1000, Michael Ellerman wrote:
> Nathan Chancellor <natechancel...@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 <ooh...@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
