[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <44356002-eeca-41ee-8a42-5ef528f69465@amd.com>
Date: Tue, 29 Oct 2024 14:20:39 +0100
From: Christian König <christian.koenig@....com>
To: Michał Winiarski <michal.winiarski@...el.com>,
linux-pci@...r.kernel.org, intel-xe@...ts.freedesktop.org,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
Bjorn Helgaas <bhelgaas@...gle.com>, Krzysztof Wilczyński
<kw@...ux.com>, Ilpo Järvinen
<ilpo.jarvinen@...ux.intel.com>
Cc: 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 v4 3/7] PCI: Add a helper to convert between standard and
IOV resources
Am 25.10.24 um 23:50 schrieb Michał Winiarski:
> There are multiple places where conversions between IOV resources and
> standard resources are done.
>
> Extract the logic to pci_resource_to_iov() and pci_resource_from_iov()
> helpers.
>
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@...el.com>
I don't work enough with that code to fully judge if that is useful or not.
But feel free to add Acked-by: Christian König
<christian.koenig@....com> since style etc.. looks good to me.
Regards,
Christian.
> ---
> drivers/pci/iov.c | 20 ++++++++++----------
> drivers/pci/pci.h | 18 ++++++++++++++++++
> drivers/pci/setup-bus.c | 2 +-
> 3 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 6bdc9950b9787..eedc1df56c49e 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -151,7 +151,7 @@ resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
> if (!dev->is_physfn)
> return 0;
>
> - return dev->sriov->barsz[resno - PCI_IOV_RESOURCES];
> + return dev->sriov->barsz[pci_resource_from_iov(resno)];
> }
>
> static void pci_read_vf_config_common(struct pci_dev *virtfn)
> @@ -322,12 +322,12 @@ int pci_iov_add_virtfn(struct pci_dev *dev, int id)
> virtfn->multifunction = 0;
>
> for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> - res = &dev->resource[i + PCI_IOV_RESOURCES];
> + res = &dev->resource[pci_resource_to_iov(i)];
> if (!res->parent)
> continue;
> virtfn->resource[i].name = pci_name(virtfn);
> virtfn->resource[i].flags = res->flags;
> - size = pci_iov_resource_size(dev, i + PCI_IOV_RESOURCES);
> + size = pci_iov_resource_size(dev, pci_resource_to_iov(i));
> virtfn->resource[i].start = res->start + size * id;
> virtfn->resource[i].end = virtfn->resource[i].start + size - 1;
> rc = request_resource(res, &virtfn->resource[i]);
> @@ -624,8 +624,8 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
>
> nres = 0;
> for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> - bars |= (1 << (i + PCI_IOV_RESOURCES));
> - res = &dev->resource[i + PCI_IOV_RESOURCES];
> + bars |= (1 << pci_resource_to_iov(i));
> + res = &dev->resource[pci_resource_to_iov(i)];
> if (res->parent)
> nres++;
> }
> @@ -786,8 +786,8 @@ static int sriov_init(struct pci_dev *dev, int pos)
>
> nres = 0;
> for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> - res = &dev->resource[i + PCI_IOV_RESOURCES];
> - res_name = pci_resource_name(dev, i + PCI_IOV_RESOURCES);
> + res = &dev->resource[pci_resource_to_iov(i)];
> + res_name = pci_resource_name(dev, pci_resource_to_iov(i));
>
> /*
> * If it is already FIXED, don't change it, something
> @@ -844,7 +844,7 @@ static int sriov_init(struct pci_dev *dev, int pos)
> dev->is_physfn = 0;
> failed:
> for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> - res = &dev->resource[i + PCI_IOV_RESOURCES];
> + res = &dev->resource[pci_resource_to_iov(i)];
> res->flags = 0;
> }
>
> @@ -906,7 +906,7 @@ static void sriov_restore_state(struct pci_dev *dev)
> pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, ctrl);
>
> for (i = 0; i < PCI_SRIOV_NUM_BARS; i++)
> - pci_update_resource(dev, i + PCI_IOV_RESOURCES);
> + pci_update_resource(dev, pci_resource_to_iov(i));
>
> pci_write_config_dword(dev, iov->pos + PCI_SRIOV_SYS_PGSIZE, iov->pgsz);
> pci_iov_set_numvfs(dev, iov->num_VFs);
> @@ -972,7 +972,7 @@ void pci_iov_update_resource(struct pci_dev *dev, int resno)
> {
> struct pci_sriov *iov = dev->is_physfn ? dev->sriov : NULL;
> struct resource *res = dev->resource + resno;
> - int vf_bar = resno - PCI_IOV_RESOURCES;
> + int vf_bar = pci_resource_from_iov(resno);
> struct pci_bus_region region;
> u16 cmd;
> u32 new;
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 48d345607e57e..1f8d88f0243b7 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -584,6 +584,15 @@ static inline bool pci_resource_is_iov(int resno)
> {
> return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
> }
> +static inline int pci_resource_to_iov(int resno)
> +{
> + return resno + PCI_IOV_RESOURCES;
> +}
> +
> +static inline int pci_resource_from_iov(int resno)
> +{
> + return resno - PCI_IOV_RESOURCES;
> +}
> extern const struct attribute_group sriov_pf_dev_attr_group;
> extern const struct attribute_group sriov_vf_dev_attr_group;
> #else
> @@ -608,6 +617,15 @@ static inline bool pci_resource_is_iov(int resno)
> {
> return false;
> }
> +static inline int pci_resource_to_iov(int resno)
> +{
> + return -ENODEV;
> +}
> +
> +static inline int pci_resource_from_iov(int resno)
> +{
> + return -ENODEV;
> +}
> #endif /* CONFIG_PCI_IOV */
>
> #ifdef CONFIG_PCIE_PTM
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index ba293df10c050..c5ad7c4ad6eb1 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1778,7 +1778,7 @@ static int iov_resources_unassigned(struct pci_dev *dev, void *data)
> bool *unassigned = data;
>
> for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> - struct resource *r = &dev->resource[i + PCI_IOV_RESOURCES];
> + struct resource *r = &dev->resource[pci_resource_to_iov(i)];
> struct pci_bus_region region;
>
> /* Not assigned or rejected by kernel? */
Powered by blists - more mailing lists