| 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: <6yf4u3i5ufcuyidqhopnwf2ieq3hsb7w32qamr4jbdjpo3zitq@zpz6iy3odt5g>
Date: Tue, 4 Nov 2025 13:12:27 +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 v3 18/28] drm/xe/pf: Handle GGTT migration data as part
of PF control
On Fri, Oct 31, 2025 at 07:26:29PM +0100, Michal Wajdeczko wrote:
>
>
> On 10/30/2025 9:31 PM, Michał Winiarski wrote:
> > Connect the helpers to allow save and restore of GGTT migration data in
> > stop_copy / resume device state.
> >
> > Signed-off-by: Michał Winiarski <michal.winiarski@...el.com>
> > ---
> > drivers/gpu/drm/xe/xe_gt_sriov_pf_control.c | 13 ++
> > drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c | 113 ++++++++++++++++++
> > drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.h | 3 +
> > 3 files changed, 129 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_control.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_control.c
> > index cb45e97f4c4d9..e7ea9b88fd246 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_control.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_control.c
> > @@ -860,6 +860,16 @@ static int pf_handle_vf_save_data(struct xe_gt *gt, unsigned int vfid)
> > return -EAGAIN;
> > }
> >
> > + if (xe_gt_sriov_pf_migration_save_test(gt, vfid, XE_SRIOV_MIGRATION_DATA_TYPE_GGTT)) {
> > + ret = xe_gt_sriov_pf_migration_ggtt_save(gt, vfid);
> > + if (ret)
> > + return ret;
> > +
> > + xe_gt_sriov_pf_migration_save_clear(gt, vfid, XE_SRIOV_MIGRATION_DATA_TYPE_GGTT);
> > +
> > + return -EAGAIN;
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -1066,6 +1076,9 @@ static int pf_handle_vf_restore_data(struct xe_gt *gt, unsigned int vfid)
> > xe_gt_assert(gt, data);
> >
> > switch (data->type) {
> > + case XE_SRIOV_MIGRATION_DATA_TYPE_GGTT:
> > + ret = xe_gt_sriov_pf_migration_ggtt_restore(gt, vfid, data);
> > + break;
> > case XE_SRIOV_MIGRATION_DATA_TYPE_GUC:
> > ret = xe_gt_sriov_pf_migration_guc_restore(gt, vfid, data);
> > break;
> > diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c
> > index fbb451767712a..6f2ee5820bdd4 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.c
> > @@ -7,6 +7,9 @@
> >
> > #include "abi/guc_actions_sriov_abi.h"
> > #include "xe_bo.h"
> > +#include "xe_ggtt.h"
> > +#include "xe_gt.h"
> > +#include "xe_gt_sriov_pf_config.h"
> > #include "xe_gt_sriov_pf_control.h"
> > #include "xe_gt_sriov_pf_helpers.h"
> > #include "xe_gt_sriov_pf_migration.h"
> > @@ -39,6 +42,106 @@ static void pf_dump_mig_data(struct xe_gt *gt, unsigned int vfid,
> > }
> > }
> >
> > +static ssize_t pf_migration_ggtt_size(struct xe_gt *gt, unsigned int vfid)
> > +{
> > + if (!xe_gt_is_main_type(gt))
> > + return 0;
> > +
> > + return xe_gt_sriov_pf_config_ggtt_save(gt, vfid, NULL, 0);
> > +}
> > +
> > +static int pf_save_vf_ggtt_mig_data(struct xe_gt *gt, unsigned int vfid)
> > +{
> > + struct xe_sriov_migration_data *data;
> > + size_t size;
> > + int ret;
> > +
> > + size = pf_migration_ggtt_size(gt, vfid);
> > + xe_gt_assert(gt, size);
> > +
> > + data = xe_sriov_migration_data_alloc(gt_to_xe(gt));
> > + if (!data)
> > + return -ENOMEM;
> > +
> > + ret = xe_sriov_migration_data_init(data, gt->tile->id, gt->info.id,
> > + XE_SRIOV_MIGRATION_DATA_TYPE_GGTT, 0, size);
> > + if (ret)
> > + goto fail;
> > +
> > + ret = xe_gt_sriov_pf_config_ggtt_save(gt, vfid, data->vaddr, size);
> > + if (ret)
> > + goto fail;
> > +
> > + xe_gt_sriov_dbg_verbose(gt, "VF%u GGTT data save (%zu bytes)\n", vfid, size);
> > + pf_dump_mig_data(gt, vfid, data);
> > +
> > + ret = xe_gt_sriov_pf_migration_save_produce(gt, vfid, data);
> > + if (ret)
> > + goto fail;
> > +
> > + return 0;
> > +
> > +fail:
> > + xe_sriov_migration_data_free(data);
> > + xe_gt_sriov_err(gt, "Failed to save VF%u GGTT data (%pe)\n", vfid, ERR_PTR(ret));
> > + return ret;
> > +}
> > +
> > +static int pf_restore_vf_ggtt_mig_data(struct xe_gt *gt, unsigned int vfid,
> > + struct xe_sriov_migration_data *data)
> > +{
> > + int ret;
> > +
> > + xe_gt_sriov_dbg_verbose(gt, "VF%u GGTT data restore (%llu bytes)\n", vfid, data->size);
> > + pf_dump_mig_data(gt, vfid, data);
> > +
> > + ret = xe_gt_sriov_pf_config_ggtt_restore(gt, vfid, data->vaddr, data->size);
> > + if (ret) {
> > + xe_gt_sriov_err(gt, "Failed to restore VF%u GGTT data (%pe)\n",
> > + vfid, ERR_PTR(ret));
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xe_gt_sriov_pf_migration_ggtt_save() - Save VF GGTT migration data.
> > + * @gt: the &xe_gt
> > + * @vfid: the VF identifier (can't be 0)
> > + *
> > + * This function is for PF only.
> > + *
> > + * Return: 0 on success or a negative error code on failure.
> > + */
> > +int xe_gt_sriov_pf_migration_ggtt_save(struct xe_gt *gt, unsigned int vfid)
> > +{
> > + xe_gt_assert(gt, IS_SRIOV_PF(gt_to_xe(gt)));
> > + xe_gt_assert(gt, vfid != PFID);
> > + xe_gt_assert(gt, vfid <= xe_sriov_pf_get_totalvfs(gt_to_xe(gt)));
> > +
> > + return pf_save_vf_ggtt_mig_data(gt, vfid);
> > +}
> > +
> > +/**
> > + * xe_gt_sriov_pf_migration_ggtt_restore() - Restore VF GGTT migration data.
> > + * @gt: the &xe_gt
> > + * @vfid: the VF identifier (can't be 0)
> > + *
> > + * This function is for PF only.
> > + *
> > + * Return: 0 on success or a negative error code on failure.
> > + */
> > +int xe_gt_sriov_pf_migration_ggtt_restore(struct xe_gt *gt, unsigned int vfid,
> > + struct xe_sriov_migration_data *data)
> > +{
> > + xe_gt_assert(gt, IS_SRIOV_PF(gt_to_xe(gt)));
> > + xe_gt_assert(gt, vfid != PFID);
> > + xe_gt_assert(gt, vfid <= xe_sriov_pf_get_totalvfs(gt_to_xe(gt)));
> > +
> > + return pf_restore_vf_ggtt_mig_data(gt, vfid, data);
> > +}
> > +
> > /* Return: number of dwords saved/restored/required or a negative error code on failure */
> > static int guc_action_vf_save_restore(struct xe_guc *guc, u32 vfid, u32 opcode,
> > u64 addr, u32 ndwords)
> > @@ -279,6 +382,13 @@ ssize_t xe_gt_sriov_pf_migration_size(struct xe_gt *gt, unsigned int vfid)
> > size += sizeof(struct xe_sriov_pf_migration_hdr);
> > total += size;
> >
> > + size = pf_migration_ggtt_size(gt, vfid);
> > + if (size < 0)
> > + return size;
> > + if (size > 0)
> > + size += sizeof(struct xe_sriov_pf_migration_hdr);
> > + total += size;
> > +
> > return total;
> > }
> >
> > @@ -340,6 +450,9 @@ void xe_gt_sriov_pf_migration_save_init(struct xe_gt *gt, unsigned int vfid)
> >
> > xe_gt_assert(gt, pf_migration_guc_size(gt, vfid) > 0);
> > set_bit(XE_SRIOV_MIGRATION_DATA_TYPE_GUC, &migration->save.data_remaining);
> > +
> > + if (pf_migration_ggtt_size(gt, vfid) > 0)
> > + set_bit(XE_SRIOV_MIGRATION_DATA_TYPE_GGTT, &migration->save.data_remaining);
>
> btw, does it make sense to save GuC data if there is no GGTT data on the main GT?
Not in a sense that such provisioning is usable, but we do not want to
have any dependencies between data types, and we do need to handle this
corner case (not fail the migration if GGTT size is set to 0).
>
> > }
> >
> > /**
> > diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.h b/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.h
> > index 66a45d6234245..bc201d8f3147a 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.h
> > +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_migration.h
> > @@ -19,6 +19,9 @@ int xe_gt_sriov_pf_migration_init(struct xe_gt *gt);
> > int xe_gt_sriov_pf_migration_guc_save(struct xe_gt *gt, unsigned int vfid);
> > int xe_gt_sriov_pf_migration_guc_restore(struct xe_gt *gt, unsigned int vfid,
> > struct xe_sriov_migration_data *data);
> > +int xe_gt_sriov_pf_migration_ggtt_save(struct xe_gt *gt, unsigned int vfid);
> > +int xe_gt_sriov_pf_migration_ggtt_restore(struct xe_gt *gt, unsigned int vfid,
> > + struct xe_sriov_migration_data *data);
> >
> > ssize_t xe_gt_sriov_pf_migration_size(struct xe_gt *gt, unsigned int vfid);
> >
>
> only one nit question about corner case, so
>
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@...el.com>
>
Thanks,
-Michał
Powered by blists - more mailing lists