| 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: <frft6t5wwtqppqi2nc6hfo6dx3vgfzpwc3jumatharhv2zegbk@jnaafyx7fzrk>
Date: Thu, 6 Nov 2025 12:24:48 +0100
From: Michał Winiarski <michal.winiarski@...el.com>
To: Michal Wajdeczko <michal.wajdeczko@...el.com>
CC: Alex Williamson <alex@...zbot.org>, 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 <skolothumtho@...dia.com>,
<intel-xe@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
<kvm@...r.kernel.org>, Matthew Brost <matthew.brost@...el.com>,
<dri-devel@...ts.freedesktop.org>, 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>, Christoph Hellwig
<hch@...radead.org>
Subject: Re: [PATCH v4 05/28] drm/xe/pf: Add data structures and handlers for
migration rings
On Wed, Nov 05, 2025 at 09:17:05PM +0100, Michal Wajdeczko wrote:
>
>
> On 11/5/2025 4:10 PM, Michał Winiarski wrote:
> > Migration data is queued in a per-GT ptr_ring to decouple the worker
> > responsible for handling the data transfer from the .read() and .write()
> > syscalls.
> > Add the data structures and handlers that will be used in future
> > commits.
> >
> > Signed-off-by: Michał Winiarski <michal.winiarski@...el.com>
> > ---
> > drivers/gpu/drm/xe/xe_gt_sriov_pf_control.c | 310 +++++++++++++++++-
> > drivers/gpu/drm/xe/xe_gt_sriov_pf_control.h | 6 +
> > .../gpu/drm/xe/xe_gt_sriov_pf_control_types.h | 12 +
> > drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c | 199 +++++++++++
> > drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.h | 14 +
> > .../drm/xe/xe_gt_sriov_pf_migration_types.h | 11 +
> > drivers/gpu/drm/xe/xe_gt_sriov_pf_types.h | 3 +
> > drivers/gpu/drm/xe/xe_sriov_pf_migration.c | 143 ++++++++
> > drivers/gpu/drm/xe/xe_sriov_pf_migration.h | 7 +
> > .../gpu/drm/xe/xe_sriov_pf_migration_types.h | 47 +++
> > drivers/gpu/drm/xe/xe_sriov_pf_types.h | 2 +
> > 11 files changed, 741 insertions(+), 13 deletions(-)
> >
(...)
> > diff --git a/drivers/gpu/drm/xe/xe_sriov_pf_migration.c b/drivers/gpu/drm/xe/xe_sriov_pf_migration.c
> > index 8c523c392f98b..ed44eda9418cc 100644
> > --- a/drivers/gpu/drm/xe/xe_sriov_pf_migration.c
> > +++ b/drivers/gpu/drm/xe/xe_sriov_pf_migration.c
> > @@ -3,8 +3,36 @@
> > * Copyright © 2025 Intel Corporation
> > */
> >
> > +#include <drm/drm_managed.h>
> > +
> > +#include "xe_device.h"
> > +#include "xe_gt_sriov_pf_control.h"
> > +#include "xe_gt_sriov_pf_migration.h"
> > +#include "xe_pm.h"
> > #include "xe_sriov.h"
> > +#include "xe_sriov_pf_helpers.h"
> > #include "xe_sriov_pf_migration.h"
> > +#include "xe_sriov_printk.h"
> > +
> > +static struct xe_sriov_migration_state *pf_pick_migration(struct xe_device *xe, unsigned int vfid)
> > +{
> > + xe_assert(xe, IS_SRIOV_PF(xe));
> > + xe_assert(xe, vfid <= xe_sriov_pf_get_totalvfs(xe));
> > +
> > + return &xe->sriov.pf.vfs[vfid].migration;
> > +}
> > +
> > +/**
> > + * xe_sriov_pf_migration_waitqueue - Get waitqueue for migration.
>
> nit:
>
> * xe_sriov_pf_migration_waitqueue() - ...
Ok.
>
> > + * @xe: the &xe_device
> > + * @vfid: the VF identifier
> > + *
> > + * Return: pointer to the migration waitqueue.
> > + */
> > +wait_queue_head_t *xe_sriov_pf_migration_waitqueue(struct xe_device *xe, unsigned int vfid)
> > +{
> > + return &pf_pick_migration(xe, vfid)->wq;
> > +}
> >
(...)
> > diff --git a/drivers/gpu/drm/xe/xe_sriov_pf_migration_types.h b/drivers/gpu/drm/xe/xe_sriov_pf_migration_types.h
> > index 43ca60b8982c7..3177ca24215cb 100644
> > --- a/drivers/gpu/drm/xe/xe_sriov_pf_migration_types.h
> > +++ b/drivers/gpu/drm/xe/xe_sriov_pf_migration_types.h
> > @@ -7,6 +7,7 @@
> > #define _XE_SRIOV_PF_MIGRATION_TYPES_H_
> >
> > #include <linux/types.h>
> > +#include <linux/wait.h>
> >
> > /**
> > * struct xe_sriov_pf_migration - Xe device level VF migration data
> > @@ -16,4 +17,50 @@ struct xe_sriov_pf_migration {
> > bool supported;
> > };
> >
> > +/**
> > + * struct xe_sriov_migration_state - Per VF device-level migration related data
> > + */
> > +struct xe_sriov_migration_state {
> > + /** @wq: waitqueue used to avoid busy-waiting for snapshot production/consumption */
> > + wait_queue_head_t wq;
> > +};
> > +
> > +/**
> > + * struct xe_sriov_packet - Xe SR-IOV VF migration data packet
> > + */
> > +struct xe_sriov_packet {
>
> hmm, shouldn't this be defined in xe_sriov_packet_types.h ?
>
> in the very next patch we will have:
> xe_sriov_packet.c
> xe_sriov_packet.h
Sure, let's introduce xe_sriov_packet_types.h
>
> > + /** @xe: Xe device */
>
> nit:
>
> /** @xe: the PF Xe device this data packet belongs to */
Ok.
>
> > + struct xe_device *xe;
> > + /** @vaddr: CPU pointer to payload data */
> > + void *vaddr;
> > + /** @remaining: payload data remaining */
> > + size_t remaining;
> > + /** @hdr_remaining: header data remaining */
> > + size_t hdr_remaining;
> > + union {
> > + /** @bo: Buffer object with migration data */
> > + struct xe_bo *bo;
> > + /** @buff: Buffer with migration data */
> > + void *buff;
> > + };
> > + __struct_group(xe_sriov_pf_migration_hdr, hdr, __packed,
> > + /** @hdr.version: migration data protocol version */
> > + u8 version;
> > + /** @hdr.type: migration data type */
> > + u8 type;
> > + /** @hdr.tile: migration data tile id */
> > + u8 tile;
>
> as in this struct we already have "xe" which represents pointer to the xe_device, as used/named elsewhere in the driver,
> maybe this "tile" (and below "gt") should have "_id" suffix to avoid confusion with "tile" (and "gt") members used elsewhere in the driver where they are pointer to tile/gt?
Ok.
>
> > + /** @hdr.gt: migration data gt id */
> > + u8 gt;
> > + /** @hdr.flags: migration data flags */
> > + u32 flags;
> > + /** @hdr.offset: offset into the resource;
> > + * used when multiple packets of given type are used for migration
> > + */
> > + u64 offset;
> > + /** @hdr.size: migration data size */
> > + u64 size;
>
> btw, it looks that this __struct_group() confuses kernel-doc:
>
> Warning: drivers/gpu/drm/xe/xe_sriov_pf_migration_types.h:72 struct member 'version' not described in 'xe_sriov_packet'
> Warning: drivers/gpu/drm/xe/xe_sriov_pf_migration_types.h:72 struct member 'type' not described in 'xe_sriov_packet'
> Warning: drivers/gpu/drm/xe/xe_sriov_pf_migration_types.h:72 struct member 'tile' not described in 'xe_sriov_packet'
> Warning: drivers/gpu/drm/xe/xe_sriov_pf_migration_types.h:72 struct member 'gt' not described in 'xe_sriov_packet'
> Warning: drivers/gpu/drm/xe/xe_sriov_pf_migration_types.h:72 struct member 'flags' not described in 'xe_sriov_packet'
> Warning: drivers/gpu/drm/xe/xe_sriov_pf_migration_types.h:72 struct member 'offset' not described in 'xe_sriov_packet'
> Warning: drivers/gpu/drm/xe/xe_sriov_pf_migration_types.h:72 struct member 'size' not described in 'xe_sriov_packet'
Looks like struct_group() usage is just a leftover from development
process and we don't really need it here.
I'll just replace it with regular named struct xe_sriov_packet_hdr.
>
>
> > + );
> > +};
> > +
> > #endif
> > diff --git a/drivers/gpu/drm/xe/xe_sriov_pf_types.h b/drivers/gpu/drm/xe/xe_sriov_pf_types.h
> > index b15d8ca2894c2..d1af2c0aef866 100644
> > --- a/drivers/gpu/drm/xe/xe_sriov_pf_types.h
> > +++ b/drivers/gpu/drm/xe/xe_sriov_pf_types.h
> > @@ -24,6 +24,8 @@ struct xe_sriov_metadata {
> >
> > /** @version: negotiated VF/PF ABI version */
> > struct xe_sriov_pf_service_version version;
> > + /** @migration: migration state */
> > + struct xe_sriov_migration_state migration;
> > };
> >
> > /**
>
> otherwise LGTM
>
Thanks,
-Michał
Powered by blists - more mailing lists