[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ab3pdsun5a2g3aqquuopr7cghs5trprx4lvbd26mujkp5acabj@e4vaxikjkg2b>
Date: Wed, 2 Apr 2025 12:31:07 +0200
From: Michał Winiarski <michal.winiarski@...el.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC: <linux-pci@...r.kernel.org>, <intel-xe@...ts.freedesktop.org>,
<dri-devel@...ts.freedesktop.org>, LKML <linux-kernel@...r.kernel.org>,
"Bjorn Helgaas" <bhelgaas@...gle.com>, Christian König
<christian.koenig@....com>, Krzysztof Wilczyński
<kw@...ux.com>, Rodrigo Vivi <rodrigo.vivi@...el.com>, Michal Wajdeczko
<michal.wajdeczko@...el.com>, Lucas De Marchi <lucas.demarchi@...el.com>,
Thomas Hellström <thomas.hellstrom@...ux.intel.com>,
"Maarten Lankhorst" <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
<mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David Airlie
<airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Matt Roper
<matthew.d.roper@...el.com>
Subject: Re: [PATCH v6 4/6] PCI/IOV: Check that VF BAR fits within the
reservation
On Wed, Mar 26, 2025 at 05:11:04PM +0200, Ilpo Järvinen wrote:
> On Thu, 20 Mar 2025, Michał Winiarski wrote:
>
> > When the resource representing VF MMIO BAR reservation is created, its
> > size is always large enough to accommodate the BAR of all SR-IOV Virtual
> > Functions that can potentially be created (total VFs). If for whatever
> > reason it's not possible to accommodate all VFs - the resource is not
> > assigned and no VFs can be created.
> >
> > The following patch will allow VF BAR size to be modified by drivers at
>
> "The following patch" sounds to be like you're referring to patch that
> follows this description, ie., the patch below. "An upcoming change" is
> alternative that doesn't suffer from the same problem.
Ok.
>
> > a later point in time, which means that the check for resource
> > assignment is no longer sufficient.
> >
> > Add an additional check that verifies that VF BAR for all enabled VFs
> > fits within the underlying reservation resource.
>
> So this does not solve the case where the initial size was too large to
> fix and such VF BARs remain unassigned, right?
Right - and in my opinion VF enabling is not the right point in time to
try and salvage the PF resource resevation.
-Michał
>
> > Signed-off-by: Michał Winiarski <michal.winiarski@...el.com>
> > ---
> > drivers/pci/iov.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > index cbf335725d4fb..861273ad9a580 100644
> > --- a/drivers/pci/iov.c
> > +++ b/drivers/pci/iov.c
> > @@ -646,8 +646,13 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
> >
> > nres = 0;
> > for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> > + resource_size_t vf_bar_sz =
> > + pci_iov_resource_size(dev,
> > + pci_resource_num_from_vf_bar(i));
>
> Please add int idx = pci_resource_num_from_vf_bar(i);
>
> > bars |= (1 << pci_resource_num_from_vf_bar(i));
> > res = &dev->resource[pci_resource_num_from_vf_bar(i)];
> > + if (vf_bar_sz * nr_virtfn > resource_size(res))
> > + continue;
>
> Not directly related to this patch, I suspect this could actually try to
> assign an unassigned resource by doing something like this (perhaps in own
> patch, it doesn't even need to be part of this series but can be sent
> later if you find the suggestion useful):
>
> /* Retry assignment if the initial size didn't fit */
> if (!res->parent && pci_assign_resource(res, idx))
> continue;
>
> Although I suspect reset_resource() might have been called for the
> resource and IIRC it breaks the resource somehow but it could have been
> that IOV resources can be resummoned from that state though thanks to
> their size not being stored into the resource itself but comes from iov
> structures.
>
> > if (res->parent)
> > nres++;
> > }
> >
>
> --
> i.
Powered by blists - more mailing lists