[PATCH 2/3] drm/xe: Add proper detection of the SR-IOV PF mode
Michal Wajdeczko
michal.wajdeczko at intel.com
Tue Apr 9 18:39:36 UTC 2024
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
>
>> +#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