[PATCH 2/3] drm/xe: Add proper detection of the SR-IOV PF mode
Rodrigo Vivi
rodrigo.vivi at intel.com
Tue Apr 9 18:42:37 UTC 2024
On Tue, Apr 09, 2024 at 08:39:36PM +0200, Michal Wajdeczko wrote:
>
>
> On 09.04.2024 19:47, Rodrigo Vivi wrote:
> > On Thu, Apr 04, 2024 at 05:44:30PM +0200, Michal Wajdeczko wrote:
> >> SR-IOV PF mode detection is based on PCI capability as reported by
> >> the PCI dev_is_pf() function and additionally on 'max_vfs' module
> >> parameter which could be also used to disable PF capability even
> >> if SR-IOV PF capability is reported by the hardware.
> >>
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> >> ---
> >> drivers/gpu/drm/xe/Makefile | 3 +-
> >> drivers/gpu/drm/xe/xe_device_types.h | 4 ++
> >> drivers/gpu/drm/xe/xe_sriov.c | 3 +
> >> drivers/gpu/drm/xe/xe_sriov_pf.c | 89 ++++++++++++++++++++++++++++
> >> drivers/gpu/drm/xe/xe_sriov_pf.h | 24 ++++++++
> >> drivers/gpu/drm/xe/xe_sriov_types.h | 15 +++++
> >> 6 files changed, 137 insertions(+), 1 deletion(-)
> >> create mode 100644 drivers/gpu/drm/xe/xe_sriov_pf.c
> >> create mode 100644 drivers/gpu/drm/xe/xe_sriov_pf.h
> >>
> >> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> >> index e5b1715f721e..8d79df05b84f 100644
> >> --- a/drivers/gpu/drm/xe/Makefile
> >> +++ b/drivers/gpu/drm/xe/Makefile
> >> @@ -159,7 +159,8 @@ xe-$(CONFIG_PCI_IOV) += \
> >> xe_gt_sriov_pf_control.o \
> >> xe_lmtt.o \
> >> xe_lmtt_2l.o \
> >> - xe_lmtt_ml.o
> >> + xe_lmtt_ml.o \
> >> + xe_sriov_pf.o
> >>
> >> # include helpers for tests even when XE is built-in
> >> ifdef CONFIG_DRM_XE_KUNIT_TEST
> >> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> >> index c710cec835a7..8244b177a6a3 100644
> >> --- a/drivers/gpu/drm/xe/xe_device_types.h
> >> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> >> @@ -321,6 +321,10 @@ struct xe_device {
> >> struct {
> >> /** @sriov.__mode: SR-IOV mode (Don't access directly!) */
> >> enum xe_sriov_mode __mode;
> >> +
> >> + /** @sriov.pf: PF specific data */
> >> + struct xe_device_pf pf;
> >> +
> >> /** @sriov.wq: workqueue used by the virtualization workers */
> >> struct workqueue_struct *wq;
> >> } sriov;
> >> diff --git a/drivers/gpu/drm/xe/xe_sriov.c b/drivers/gpu/drm/xe/xe_sriov.c
> >> index 3e103edf7174..94fa98d8206e 100644
> >> --- a/drivers/gpu/drm/xe/xe_sriov.c
> >> +++ b/drivers/gpu/drm/xe/xe_sriov.c
> >> @@ -11,6 +11,7 @@
> >> #include "xe_device.h"
> >> #include "xe_mmio.h"
> >> #include "xe_sriov.h"
> >> +#include "xe_sriov_pf.h"
> >>
> >> /**
> >> * xe_sriov_mode_to_string - Convert enum value to string.
> >> @@ -58,6 +59,8 @@ void xe_sriov_probe_early(struct xe_device *xe)
> >> if (has_sriov) {
> >> if (test_is_vf(xe))
> >> mode = XE_SRIOV_MODE_VF;
> >> + else if (xe_sriov_pf_readiness(xe))
> >> + mode = XE_SRIOV_MODE_PF;
> >> }
> >>
> >> xe_assert(xe, !xe->sriov.__mode);
> >> diff --git a/drivers/gpu/drm/xe/xe_sriov_pf.c b/drivers/gpu/drm/xe/xe_sriov_pf.c
> >> new file mode 100644
> >> index 000000000000..030c2b69ecc4
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/xe/xe_sriov_pf.c
> >> @@ -0,0 +1,89 @@
> >> +// SPDX-License-Identifier: MIT
> >> +/*
> >> + * Copyright © 2023-2024 Intel Corporation
> >> + */
> >> +
> >> +#include "xe_assert.h"
> >> +#include "xe_device.h"
> >> +#include "xe_module.h"
> >> +#include "xe_sriov.h"
> >> +#include "xe_sriov_pf.h"
> >> +#include "xe_sriov_printk.h"
> >> +
> >> +static unsigned int wanted_max_vfs(struct xe_device *xe)
> >> +{
> >> + return xe_modparam.max_vfs;
> >> +}
> >> +
> >> +static int pf_reduce_totalvfs(struct xe_device *xe, int limit)
> >> +{
> >> + struct device *dev = xe->drm.dev;
> >> + struct pci_dev *pdev = to_pci_dev(dev);
> >> + int err;
> >> +
> >> + err = pci_sriov_set_totalvfs(pdev, limit);
> >> + if (err)
> >> + xe_sriov_notice(xe, "Failed to set number of VFs to %d (%pe)\n",
> >> + limit, ERR_PTR(err));
> >> + return err;
> >> +}
> >> +
> >> +static bool pf_continue_as_native(struct xe_device *xe, const char *why)
> >> +{
> >> + xe_sriov_dbg(xe, "%s, continuing as native\n", why);
> >> + pf_reduce_totalvfs(xe, 0);
> >> + return false;
> >> +}
> >> +
> >> +/**
> >> + * xe_sriov_pf_readiness - Check if PF functionality can be enabled.
> >> + * @xe: the &xe_device to check
> >> + *
> >> + * This function is called as part of the SR-IOV probe to validate if all
> >> + * PF prerequisites are satisfied and we can continue with enabling PF mode.
> >> + *
> >> + * Return: true if the PF mode can be turned on.
> >> + */
> >> +bool xe_sriov_pf_readiness(struct xe_device *xe)
> >> +{
> >> + struct device *dev = xe->drm.dev;
> >> + struct pci_dev *pdev = to_pci_dev(dev);
> >> + int totalvfs = pci_sriov_get_totalvfs(pdev);
> >> + int newlimit = min_t(u16, wanted_max_vfs(xe), totalvfs);
> >> +
> >> + xe_assert(xe, totalvfs <= U16_MAX);
> >
> > perhaps we should do this check before the min_t?
>
> this assert is just to express/confirm SLA with the PCI function
>
> >
> > should we also check what we are getting from the params?
>
> we don't care, param could be any number up to U32_MAX, as we will just
> use it to limit what PCI function (and thus our HW) reports
>
> >
> >> +
> >> + if (!dev_is_pf(dev))
> >> + return false;
> >> +
> >> + if (!xe_device_uc_enabled(xe))
> >> + return pf_continue_as_native(xe, "Guc submission disabled");
> >> +
> >> + if (!newlimit)
> >> + return pf_continue_as_native(xe, "all VFs disabled");
> >> +
> >> + pf_reduce_totalvfs(xe, newlimit);
> >
> > do we really need to always call this or only when newlimit != totalvfs?
>
> it's harmless to call it even if there is no change as this just updates
> internal data in PCI core, does not touch any HW, so one "if" spared
>
> >
> >> +
> >> + xe->sriov.pf.device_total_vfs = totalvfs;
> >> + xe->sriov.pf.driver_max_vfs = newlimit;
> >> +
> >> + return true;
> >> +}
> >> +
> >> +/**
> >> + * xe_sriov_pf_print_vfs_summary - Print SR-IOV PF information.
> >> + * @xe: the &xe_device to print info from
> >> + * @p: the &drm_printer
> >> + *
> >> + * Print SR-IOV PF related information into provided DRM printer.
> >> + */
> >> +void xe_sriov_pf_print_vfs_summary(struct xe_device *xe, struct drm_printer *p)
> >> +{
> >> + struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> >> +
> >> + xe_assert(xe, IS_SRIOV_PF(xe));
> >> +
> >> + drm_printf(p, "total: %u\n", xe->sriov.pf.device_total_vfs);
> >> + drm_printf(p, "supported: %u\n", xe->sriov.pf.driver_max_vfs);
> >> + drm_printf(p, "enabled: %u\n", pci_num_vf(pdev));
> >> +}
> >> diff --git a/drivers/gpu/drm/xe/xe_sriov_pf.h b/drivers/gpu/drm/xe/xe_sriov_pf.h
> >> new file mode 100644
> >> index 000000000000..ebef2e01838a
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/xe/xe_sriov_pf.h
> >> @@ -0,0 +1,24 @@
> >> +/* SPDX-License-Identifier: MIT */
> >> +/*
> >> + * Copyright © 2023-2024 Intel Corporation
> >> + */
> >> +
> >> +#ifndef _XE_SRIOV_PF_H_
> >> +#define _XE_SRIOV_PF_H_
> >> +
> >> +#include <linux/types.h>
> >> +
> >> +struct drm_printer;
> >> +struct xe_device;
> >> +
> >> +#ifdef CONFIG_PCI_IOV
> >> +bool xe_sriov_pf_readiness(struct xe_device *xe);
> >> +void xe_sriov_pf_print_vfs_summary(struct xe_device *xe, struct drm_printer *p);
> >> +#else
> >> +static inline bool xe_sriov_pf_readiness(struct xe_device *xe)
> >> +{
> >> + return false;
> >> +}
> >
> > missing the vfs_summary function in this block?!
>
> only xe_sriov_pf_readiness() function will be used regardless of the
> CONFIG_PCI_IOV, the other xe_sriov_pf_print_vfs_summary() function and
> few more functions (coming soon) will be used only for CONFIG_PCI_IOV=y
> and thus will not require empty stubs
Thanks for all the explanations.
Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
> >
> >> +#endif
> >> +
> >> +#endif
> >> diff --git a/drivers/gpu/drm/xe/xe_sriov_types.h b/drivers/gpu/drm/xe/xe_sriov_types.h
> >> index 1a138108d139..fa583e8fa0c2 100644
> >> --- a/drivers/gpu/drm/xe/xe_sriov_types.h
> >> +++ b/drivers/gpu/drm/xe/xe_sriov_types.h
> >> @@ -7,6 +7,7 @@
> >> #define _XE_SRIOV_TYPES_H_
> >>
> >> #include <linux/build_bug.h>
> >> +#include <linux/types.h>
> >>
> >> /**
> >> * VFID - Virtual Function Identifier
> >> @@ -37,4 +38,18 @@ enum xe_sriov_mode {
> >> };
> >> static_assert(XE_SRIOV_MODE_NONE);
> >>
> >> +/**
> >> + * struct xe_device_pf - Xe PF related data
> >> + *
> >> + * The data in this structure is valid only if driver is running in the
> >> + * @XE_SRIOV_MODE_PF mode.
> >> + */
> >> +struct xe_device_pf {
> >> + /** @device_total_vfs: Maximum number of VFs supported by the device. */
> >> + u16 device_total_vfs;
> >> +
> >> + /** @driver_max_vfs: Maximum number of VFs supported by the driver. */
> >> + u16 driver_max_vfs;
> >> +};
> >> +
> >> #endif
> >> --
> >> 2.43.0
> >>
More information about the Intel-xe
mailing list