[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3] vfio/pci: migration: Skip config space check for Vendor S
From: |
Alex Williamson |
Subject: |
Re: [PATCH v3] vfio/pci: migration: Skip config space check for Vendor Specific Information in VSC during restore/load |
Date: |
Wed, 27 Mar 2024 14:52:35 -0600 |
On Wed, 27 Mar 2024 16:11:37 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Mar 27, 2024 at 11:39:15AM -0600, Alex Williamson wrote:
> > On Fri, 22 Mar 2024 12:12:10 +0530
> > Vinayak Kale <vkale@nvidia.com> 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.
> > >
> > > Signed-off-by: Vinayak Kale <vkale@nvidia.com>
> > > ---
> > > Version History
> > > 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/vfio/pci.c | 24 ++++++++++++++++++++++++
> > > 1 file changed, 24 insertions(+)
> >
> >
> > Acked-by: Alex Williamson <alex.williamson@redhat.com>
>
>
> A very reasonable way to do it.
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>
> Merge through the VFIO tree I presume?
Yep, Cédric said he´d grab it for 9.1. Thanks,
Alex
> > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > > index d7fe06715c..1026cdba18 100644
> > > --- a/hw/vfio/pci.c
> > > +++ b/hw/vfio/pci.c
> > > @@ -2132,6 +2132,27 @@ 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 (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)
> > > {
> > > PCIDevice *pdev = &vdev->pdev;
> > > @@ -2199,6 +2220,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;
>