| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <idh5nc3urbbwrtklgbmqujkrhvmlxw4ourhkov6z23b4cw5u7l@hmf43vjxnfq7>
Date: Tue, 21 Oct 2025 02:31:48 +0200
From: Michał Winiarski <michal.winiarski@...el.com>
To: Michal Wajdeczko <michal.wajdeczko@...el.com>
CC: Alex Williamson <alex.williamson@...hat.com>, Lucas De Marchi
<lucas.demarchi@...el.com>, Thomas Hellström
<thomas.hellstrom@...ux.intel.com>, Rodrigo Vivi <rodrigo.vivi@...el.com>,
Jason Gunthorpe <jgg@...pe.ca>, Yishai Hadas <yishaih@...dia.com>, Kevin Tian
<kevin.tian@...el.com>, Shameer Kolothum
<shameerali.kolothum.thodi@...wei.com>, <intel-xe@...ts.freedesktop.org>,
<linux-kernel@...r.kernel.org>, <kvm@...r.kernel.org>,
<dri-devel@...ts.freedesktop.org>, Matthew Brost <matthew.brost@...el.com>,
Jani Nikula <jani.nikula@...ux.intel.com>, Joonas Lahtinen
<joonas.lahtinen@...ux.intel.com>, Tvrtko Ursulin <tursulin@...ulin.net>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, "Lukasz
Laguna" <lukasz.laguna@...el.com>
Subject: Re: [PATCH 08/26] drm/xe/pf: Add minimalistic migration descriptor
On Mon, Oct 13, 2025 at 12:56:34PM +0200, Michal Wajdeczko wrote:
>
>
> On 10/11/2025 9:38 PM, Michał Winiarski wrote:
> > The descriptor reuses the KLV format used by GuC and contains metadata
> > that can be used to quickly fail migration when source is incompatible
> > with destination.
> >
> > Signed-off-by: Michał Winiarski <michal.winiarski@...el.com>
> > ---
> > drivers/gpu/drm/xe/xe_sriov_pf_migration.c | 6 +-
> > .../gpu/drm/xe/xe_sriov_pf_migration_data.c | 82 ++++++++++++++++++-
> > .../gpu/drm/xe/xe_sriov_pf_migration_data.h | 2 +
> > 3 files changed, 87 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_sriov_pf_migration.c b/drivers/gpu/drm/xe/xe_sriov_pf_migration.c
> > index 9cc178126cbdc..a0cfac456ba0b 100644
> > --- a/drivers/gpu/drm/xe/xe_sriov_pf_migration.c
> > +++ b/drivers/gpu/drm/xe/xe_sriov_pf_migration.c
> > @@ -186,10 +186,14 @@ xe_sriov_pf_migration_consume(struct xe_device *xe, unsigned int vfid)
> > static int pf_handle_descriptor(struct xe_device *xe, unsigned int vfid,
> > struct xe_sriov_pf_migration_data *data)
> > {
> > + int ret;
> > +
> > if (data->tile != 0 || data->gt != 0)
> > return -EINVAL;
> >
> > - xe_sriov_pf_migration_data_free(data);
> > + ret = xe_sriov_pf_migration_data_process_desc(xe, vfid, data);
> > + if (ret)
> > + return ret;
> >
> > return 0;
> > }
> > diff --git a/drivers/gpu/drm/xe/xe_sriov_pf_migration_data.c b/drivers/gpu/drm/xe/xe_sriov_pf_migration_data.c
> > index 9a2777dcf9a6b..307b16b027a5e 100644
> > --- a/drivers/gpu/drm/xe/xe_sriov_pf_migration_data.c
> > +++ b/drivers/gpu/drm/xe/xe_sriov_pf_migration_data.c
> > @@ -5,6 +5,7 @@
> >
> > #include "xe_bo.h"
> > #include "xe_device.h"
> > +#include "xe_guc_klv_helpers.h"
> > #include "xe_sriov_pf_helpers.h"
> > #include "xe_sriov_pf_migration.h"
> > #include "xe_sriov_pf_migration_data.h"
> > @@ -404,11 +405,17 @@ ssize_t xe_sriov_pf_migration_data_write(struct xe_device *xe, unsigned int vfid
> > return produced;
> > }
> >
> > -#define MIGRATION_DESC_SIZE 4
> > +#define MIGRATION_KLV_DEVICE_DEVID_KEY 0xf001u
> > +#define MIGRATION_KLV_DEVICE_DEVID_LEN 1u
> > +#define MIGRATION_KLV_DEVICE_REVID_KEY 0xf002u
> > +#define MIGRATION_KLV_DEVICE_REVID_LEN 1u
>
> as we aim to have unique KLVs across GuC ABI, maybe we should ask GuC team to reserve some KLVs range (0xf000-0xffff) for our (driver) use ?
I'll start the discussion.
>
> > +
> > +#define MIGRATION_DESC_DWORDS 4
>
> maybe:
> (GUC_KLV_LEN_MIN + MIGRATION_KLV_DEVICE_DEVID_LEN +
> GUC_KLV_LEN_MIN + MIGRATION_KLV_DEVICE_REVID_LEN)
Ok.
>
> > static size_t pf_desc_init(struct xe_device *xe, unsigned int vfid)
> > {
> > struct xe_sriov_pf_migration_data **desc = pf_pick_descriptor(xe, vfid);
> > struct xe_sriov_pf_migration_data *data;
> > + u32 *klvs;
> > int ret;
> >
> > data = xe_sriov_pf_migration_data_alloc(xe);
> > @@ -416,17 +423,88 @@ static size_t pf_desc_init(struct xe_device *xe, unsigned int vfid)
> > return -ENOMEM;
> >
> > ret = xe_sriov_pf_migration_data_init(data, 0, 0, XE_SRIOV_MIG_DATA_DESCRIPTOR,
> > - 0, MIGRATION_DESC_SIZE);
> > + 0, MIGRATION_DESC_DWORDS * sizeof(u32));
> > if (ret) {
> > xe_sriov_pf_migration_data_free(data);
> > return ret;
> > }
> >
> > + klvs = data->vaddr;
> > + *klvs++ = PREP_GUC_KLV_CONST(MIGRATION_KLV_DEVICE_DEVID_KEY,
> > + MIGRATION_KLV_DEVICE_DEVID_LEN);
> > + *klvs++ = xe->info.devid;
> > + *klvs++ = PREP_GUC_KLV_CONST(MIGRATION_KLV_DEVICE_REVID_KEY,
> > + MIGRATION_KLV_DEVICE_REVID_LEN);
> > + *klvs++ = xe->info.revid;
> > +
> > *desc = data;
> >
> > return 0;
> > }
> >
> > +/**
> > + * xe_sriov_pf_migration_data_process_desc() - Process migration data descriptor.
> > + * @gt: the &struct xe_device
>
> @xe
Ok.
>
> > + * @vfid: the VF identifier
> > + * @data: the &struct xe_sriov_pf_migration_data containing the descriptor
> > + *
> > + * The descriptor uses the same KLV format as GuC, and contains metadata used for
> > + * checking migration data compatibility.
> > + *
> > + * Return: 0 on success, -errno on failure
> > + */
> > +int xe_sriov_pf_migration_data_process_desc(struct xe_device *xe, unsigned int vfid,
> > + struct xe_sriov_pf_migration_data *data)
> > +{
> > + u32 num_dwords = data->size / sizeof(u32);
> > + u32 *klvs = data->vaddr;
> > +
> > + xe_assert(xe, data->type == XE_SRIOV_MIG_DATA_DESCRIPTOR);
> > + if (data->size % sizeof(u32) != 0)
> > + return -EINVAL;
> > + if (data->size != num_dwords * sizeof(u32))
> > + return -EINVAL;
>
> isn't that redundant ?
It is - will remove.
>
> > +
> > + while (num_dwords >= GUC_KLV_LEN_MIN) {
> > + u32 key = FIELD_GET(GUC_KLV_0_KEY, klvs[0]);
> > + u32 len = FIELD_GET(GUC_KLV_0_LEN, klvs[0]);
> > +
> > + klvs += GUC_KLV_LEN_MIN;
> > + num_dwords -= GUC_KLV_LEN_MIN;
> > +
> > + switch (key) {
> > + case MIGRATION_KLV_DEVICE_DEVID_KEY:
> > + if (*klvs != xe->info.devid) {
> > + xe_sriov_info(xe,
>
> maybe it should be more that info() ?
Promoted to warn().
Thanks,
-Michał
>
> > + "Aborting migration, devid mismatch %#04x!=%#04x\n",
> > + *klvs, xe->info.devid);
> > + return -ENODEV;
> > + }
> > + break;
> > + case MIGRATION_KLV_DEVICE_REVID_KEY:
> > + if (*klvs != xe->info.revid) {
> > + xe_sriov_info(xe,
> > + "Aborting migration, revid mismatch %#04x!=%#04x\n",
> > + *klvs, xe->info.revid);
> > + return -ENODEV;
> > + }
> > + break;
> > + default:
> > + xe_sriov_dbg(xe,
> > + "Unknown migration descriptor key %#06x - skipping\n", key);
> > + break;
> > + }
> > +
> > + if (len > num_dwords)
> > + return -EINVAL;
> > +
> > + klvs += len;
> > + num_dwords -= len;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static void pf_pending_init(struct xe_device *xe, unsigned int vfid)
> > {
> > struct xe_sriov_pf_migration_data **data = pf_pick_pending(xe, vfid);
> > diff --git a/drivers/gpu/drm/xe/xe_sriov_pf_migration_data.h b/drivers/gpu/drm/xe/xe_sriov_pf_migration_data.h
> > index 5b96c7f224002..7cfd61005c00f 100644
> > --- a/drivers/gpu/drm/xe/xe_sriov_pf_migration_data.h
> > +++ b/drivers/gpu/drm/xe/xe_sriov_pf_migration_data.h
> > @@ -32,6 +32,8 @@ ssize_t xe_sriov_pf_migration_data_read(struct xe_device *xe, unsigned int vfid,
> > char __user *buf, size_t len);
> > ssize_t xe_sriov_pf_migration_data_write(struct xe_device *xe, unsigned int vfid,
> > const char __user *buf, size_t len);
> > +int xe_sriov_pf_migration_data_process_desc(struct xe_device *xe, unsigned int vfid,
> > + struct xe_sriov_pf_migration_data *data);
> > int xe_sriov_pf_migration_data_save_init(struct xe_device *xe, unsigned int vfid);
> >
> > #endif
>
Powered by blists - more mailing lists