From: "Cédric Le Goater" <clg@redhat.com>
To: Vinayak Kale <vkale@nvidia.com>, qemu-devel@nongnu.org
Cc: alex.williamson@redhat.com, mst@redhat.com,
marcel.apfelbaum@gmail.com, avihaih@nvidia.com,
acurrid@nvidia.com, cjia@nvidia.com, zhiw@nvidia.com,
targupta@nvidia.com, kvm@vger.kernel.org
Subject: Re: [PATCH v4] vfio/pci: migration: Skip config space check for Vendor Specific Information in VSC during restore/load
Date: Mon, 6 May 2024 14:41:43 +0200 [thread overview]
Message-ID: <76a741bc-a7b8-4c8a-bc7f-7648bc421cb9@redhat.com> (raw)
In-Reply-To: <20240503145142.2806030-1-vkale@nvidia.com>
On 5/3/24 16:51, Vinayak Kale wrote:
> In case of migration, during restore operation, qemu checks config space of the
> pci device with the config space in the migration stream captured during save
> operation. In case of config space data mismatch, restore operation is failed.
>
> config space check is done in function get_pci_config_device(). By default VSC
> (vendor-specific-capability) in config space is checked.
>
> Due to qemu's config space check for VSC, live migration is broken across NVIDIA
> vGPU devices in situation where source and destination host driver is different.
> In this situation, Vendor Specific Information in VSC varies on the destination
> to ensure vGPU feature capabilities exposed to the guest driver are compatible
> with destination host.
>
> If a vfio-pci device is migration capable and vfio-pci vendor driver is OK with
> volatile Vendor Specific Info in VSC then qemu should exempt config space check
> for Vendor Specific Info. It is vendor driver's responsibility to ensure that
> VSC is consistent across migration. Here consistency could mean that VSC format
> should be same on source and destination, however actual Vendor Specific Info
> may not be byte-to-byte identical.
>
> This patch skips the check for Vendor Specific Information in VSC for VFIO-PCI
> device by clearing pdev->cmask[] offsets. Config space check is still enforced
> for 3 byte VSC header. If cmask[] is not set for an offset, then qemu skips
> config space check for that offset.
>
> VSC check is skipped for machine types >= 9.1. The check would be enforced on
> older machine types (<= 9.0).
>
> Signed-off-by: Vinayak Kale <vkale@nvidia.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Cédric Le Goater <clg@redhat.com>
LGTM,
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
> ---
> Version History
> v3->v4:
> - VSC check is skipped for machine types >= 9.1. The check would be enforced
> on older machine types (<= 9.0).
> v2->v3:
> - Config space check skipped only for Vendor Specific Info in VSC, check is
> still enforced for 3 byte VSC header.
> - Updated commit description with live migration failure scenario.
> v1->v2:
> - Limited scope of change to vfio-pci devices instead of all pci devices.
>
> hw/core/machine.c | 1 +
> hw/vfio/pci.c | 26 ++++++++++++++++++++++++++
> hw/vfio/pci.h | 1 +
> 3 files changed, 28 insertions(+)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 4ff60911e7..fc3eb5115f 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -35,6 +35,7 @@
>
> GlobalProperty hw_compat_9_0[] = {
> {"arm-cpu", "backcompat-cntfrq", "true" },
> + {"vfio-pci", "skip-vsc-check", "false" },
> };
> const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 64780d1b79..2ece9407cc 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2134,6 +2134,28 @@ static void vfio_check_af_flr(VFIOPCIDevice *vdev, uint8_t pos)
> }
> }
>
> +static int vfio_add_vendor_specific_cap(VFIOPCIDevice *vdev, int pos,
> + uint8_t size, Error **errp)
> +{
> + PCIDevice *pdev = &vdev->pdev;
> +
> + pos = pci_add_capability(pdev, PCI_CAP_ID_VNDR, pos, size, errp);
> + if (pos < 0) {
> + return pos;
> + }
> +
> + /*
> + * Exempt config space check for Vendor Specific Information during
> + * restore/load.
> + * Config space check is still enforced for 3 byte VSC header.
> + */
> + if (vdev->skip_vsc_check && size > 3) {
> + memset(pdev->cmask + pos + 3, 0, size - 3);
> + }
> +
> + return pos;
> +}
> +
> static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
> {
> ERRP_GUARD();
> @@ -2202,6 +2224,9 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
> vfio_check_af_flr(vdev, pos);
> ret = pci_add_capability(pdev, cap_id, pos, size, errp);
> break;
> + case PCI_CAP_ID_VNDR:
> + ret = vfio_add_vendor_specific_cap(vdev, pos, size, errp);
> + break;
> default:
> ret = pci_add_capability(pdev, cap_id, pos, size, errp);
> break;
> @@ -3390,6 +3415,7 @@ static Property vfio_pci_dev_properties[] = {
> DEFINE_PROP_LINK("iommufd", VFIOPCIDevice, vbasedev.iommufd,
> TYPE_IOMMUFD_BACKEND, IOMMUFDBackend *),
> #endif
> + DEFINE_PROP_BOOL("skip-vsc-check", VFIOPCIDevice, skip_vsc_check, true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index 6e64a2654e..92cd62d115 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -177,6 +177,7 @@ struct VFIOPCIDevice {
> OnOffAuto ramfb_migrate;
> bool defer_kvm_irq_routing;
> bool clear_parent_atomics_on_exit;
> + bool skip_vsc_check;
> VFIODisplay *dpy;
> Notifier irqchip_change_notifier;
> };
next prev parent reply other threads:[~2024-05-06 12:41 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-03 14:51 [PATCH v4] vfio/pci: migration: Skip config space check for Vendor Specific Information in VSC during restore/load Vinayak Kale
2024-05-06 12:41 ` Cédric Le Goater [this message]
2024-05-16 16:29 ` Cédric Le Goater
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=76a741bc-a7b8-4c8a-bc7f-7648bc421cb9@redhat.com \
--to=clg@redhat.com \
--cc=acurrid@nvidia.com \
--cc=alex.williamson@redhat.com \
--cc=avihaih@nvidia.com \
--cc=cjia@nvidia.com \
--cc=kvm@vger.kernel.org \
--cc=marcel.apfelbaum@gmail.com \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=targupta@nvidia.com \
--cc=vkale@nvidia.com \
--cc=zhiw@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).