Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-10 Thread Steven Sistare

On 6/10/2025 12:31 PM, Michael S. Tsirkin wrote:

On Wed, Jun 04, 2025 at 03:48:40PM +0200, Cédric Le Goater wrote:

I don't see any advantage to making this a class attribute.  I looked for 
examples
of using such attributes for vfio to configure pci, and found very little.  It
sounds like overkill since vfio already sets and gets PCIDevice members directly
in many places.

I defined skip_reset_on_cpr based on this existing example:

vfio_instance_init()
      pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS


pci_dev->cap_present can be modified at realize time. skip_reset_on_cpr
is a constant, for which a class attribute are more appropriate.
This is minor.

Michael,

   Are you ok with the 'skip_reset_on_cpr' bool ?


Generally yes, but maybe cap_present bit is even cleaner?
vfio already pokes at it, and we have history of encoding
quirks there, see QEMU_PCIE_LNKSTA_DLLLA_BITNR for example.


Sure, I can send a new version based on a cap_present bit 
QEMU_PCI_SKIP_RESET_ON_CPR.

- Steve



Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-10 Thread Cédric Le Goater

On 6/10/25 19:14, Steven Sistare wrote:

On 6/10/2025 1:11 PM, Cédric Le Goater wrote:

On 6/10/25 19:05, Steven Sistare wrote:

On 6/10/2025 12:31 PM, Michael S. Tsirkin wrote:

On Wed, Jun 04, 2025 at 03:48:40PM +0200, Cédric Le Goater wrote:

I don't see any advantage to making this a class attribute.  I looked for 
examples
of using such attributes for vfio to configure pci, and found very little.  It
sounds like overkill since vfio already sets and gets PCIDevice members directly
in many places.

I defined skip_reset_on_cpr based on this existing example:

vfio_instance_init()
      pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS


pci_dev->cap_present can be modified at realize time. skip_reset_on_cpr
is a constant, for which a class attribute are more appropriate.
This is minor.

Michael,

   Are you ok with the 'skip_reset_on_cpr' bool ?


Generally yes, but maybe cap_present bit is even cleaner?
vfio already pokes at it, and we have history of encoding
quirks there, see QEMU_PCIE_LNKSTA_DLLLA_BITNR for example.


Sure, I can send a new version based on a cap_present bit 
QEMU_PCI_SKIP_RESET_ON_CPR.


Please send an update of patch "pci: skip reset during cpr" in the v5 series.
Hopefully it will apply cleanly.


Please clarify: do you want me to send a delta that applies on top of the
old patch, or send a new version of the old patch?


a new version of the patch.

Thanks,

C.





Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-10 Thread Steven Sistare

On 6/10/2025 1:11 PM, Cédric Le Goater wrote:

On 6/10/25 19:05, Steven Sistare wrote:

On 6/10/2025 12:31 PM, Michael S. Tsirkin wrote:

On Wed, Jun 04, 2025 at 03:48:40PM +0200, Cédric Le Goater wrote:

I don't see any advantage to making this a class attribute.  I looked for 
examples
of using such attributes for vfio to configure pci, and found very little.  It
sounds like overkill since vfio already sets and gets PCIDevice members directly
in many places.

I defined skip_reset_on_cpr based on this existing example:

vfio_instance_init()
      pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS


pci_dev->cap_present can be modified at realize time. skip_reset_on_cpr
is a constant, for which a class attribute are more appropriate.
This is minor.

Michael,

   Are you ok with the 'skip_reset_on_cpr' bool ?


Generally yes, but maybe cap_present bit is even cleaner?
vfio already pokes at it, and we have history of encoding
quirks there, see QEMU_PCIE_LNKSTA_DLLLA_BITNR for example.


Sure, I can send a new version based on a cap_present bit 
QEMU_PCI_SKIP_RESET_ON_CPR.


Please send an update of patch "pci: skip reset during cpr" in the v5 series.
Hopefully it will apply cleanly.


Please clarify: do you want me to send a delta that applies on top of the
old patch, or send a new version of the old patch?

- Steve




Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-10 Thread Cédric Le Goater

On 6/10/25 19:05, Steven Sistare wrote:

On 6/10/2025 12:31 PM, Michael S. Tsirkin wrote:

On Wed, Jun 04, 2025 at 03:48:40PM +0200, Cédric Le Goater wrote:

I don't see any advantage to making this a class attribute.  I looked for 
examples
of using such attributes for vfio to configure pci, and found very little.  It
sounds like overkill since vfio already sets and gets PCIDevice members directly
in many places.

I defined skip_reset_on_cpr based on this existing example:

vfio_instance_init()
      pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS


pci_dev->cap_present can be modified at realize time. skip_reset_on_cpr
is a constant, for which a class attribute are more appropriate.
This is minor.

Michael,

   Are you ok with the 'skip_reset_on_cpr' bool ?


Generally yes, but maybe cap_present bit is even cleaner?
vfio already pokes at it, and we have history of encoding
quirks there, see QEMU_PCIE_LNKSTA_DLLLA_BITNR for example.


Sure, I can send a new version based on a cap_present bit 
QEMU_PCI_SKIP_RESET_ON_CPR.


Please send an update of patch "pci: skip reset during cpr" in the v5 series.
Hopefully it will apply cleanly.


Thanks,

C.









Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-10 Thread Cédric Le Goater

On 6/10/25 18:31, Michael S. Tsirkin wrote:

On Wed, Jun 04, 2025 at 03:48:40PM +0200, Cédric Le Goater wrote:

I don't see any advantage to making this a class attribute.  I looked for 
examples
of using such attributes for vfio to configure pci, and found very little.  It
sounds like overkill since vfio already sets and gets PCIDevice members directly
in many places.

I defined skip_reset_on_cpr based on this existing example:

vfio_instance_init()
      pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS


pci_dev->cap_present can be modified at realize time. skip_reset_on_cpr
is a constant, for which a class attribute are more appropriate.
This is minor.

Michael,

   Are you ok with the 'skip_reset_on_cpr' bool ?


Generally yes, but maybe cap_present bit is even cleaner?
vfio already pokes at it, and we have history of encoding
quirks there, see QEMU_PCIE_LNKSTA_DLLLA_BITNR for example.


I agree. An extra bit in the cap_present field would be cleaner.

Thanks,

C.




Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-10 Thread Michael S. Tsirkin
On Wed, Jun 04, 2025 at 03:48:40PM +0200, Cédric Le Goater wrote:
> > I don't see any advantage to making this a class attribute.  I looked for 
> > examples
> > of using such attributes for vfio to configure pci, and found very little.  
> > It
> > sounds like overkill since vfio already sets and gets PCIDevice members 
> > directly
> > in many places.
> > 
> > I defined skip_reset_on_cpr based on this existing example:
> > 
> > vfio_instance_init()
> >      pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS
> 
> pci_dev->cap_present can be modified at realize time. skip_reset_on_cpr
> is a constant, for which a class attribute are more appropriate.
> This is minor.
> 
> Michael,
> 
>   Are you ok with the 'skip_reset_on_cpr' bool ?

Generally yes, but maybe cap_present bit is even cleaner?
vfio already pokes at it, and we have history of encoding
quirks there, see QEMU_PCIE_LNKSTA_DLLLA_BITNR for example.


> > > I wonder if the resettable interface, and more specifically the
> > > RESET_TYPE_SNAPSHOT_LOAD type, might be useful. Have you explored
> > > this alternative ?
> > 
> > RESET_TYPE_SNAPSHOT_LOAD (or a new type such as RESET_TYPE_CPR) would skip
> > reset for all devices, but we only skip for vfio_pci.  All other devices
> > (including virtio) save and restore state using standard migration vmstate,
> > and must call reset.
> OK.
> 
> C.




Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-04 Thread Cédric Le Goater

I don't see any advantage to making this a class attribute.  I looked for 
examples
of using such attributes for vfio to configure pci, and found very little.  It
sounds like overkill since vfio already sets and gets PCIDevice members directly
in many places.

I defined skip_reset_on_cpr based on this existing example:

vfio_instance_init()
     pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS


pci_dev->cap_present can be modified at realize time. skip_reset_on_cpr
is a constant, for which a class attribute are more appropriate.
This is minor.

Michael,

  Are you ok with the 'skip_reset_on_cpr' bool ?


I wonder if the resettable interface, and more specifically the
RESET_TYPE_SNAPSHOT_LOAD type, might be useful. Have you explored
this alternative ?


RESET_TYPE_SNAPSHOT_LOAD (or a new type such as RESET_TYPE_CPR) would skip
reset for all devices, but we only skip for vfio_pci.  All other devices
(including virtio) save and restore state using standard migration vmstate,
and must call reset.

OK.

C.




Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-04 Thread Steven Sistare

On 6/4/2025 7:59 AM, Cédric Le Goater wrote:

On 6/4/25 09:09, Cédric Le Goater wrote:

On 6/2/25 14:36, Steven Sistare wrote:

On 6/1/2025 3:07 PM, Michael S. Tsirkin wrote:

On Sun, Jun 01, 2025 at 06:38:43PM +0200, Cédric Le Goater wrote:

On 5/29/25 21:24, Steve Sistare wrote:

Do not reset a vfio-pci device during CPR.

Signed-off-by: Steve Sistare 
---
   include/hw/pci/pci_device.h | 3 +++
   hw/pci/pci.c    | 5 +
   hw/vfio/pci.c   | 7 +++
   3 files changed, 15 insertions(+)

diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index e41d95b..b481c5d 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -181,6 +181,9 @@ struct PCIDevice {
   uint32_t max_bounce_buffer_size;
   char *sriov_pf;
+
+    /* CPR */
+    bool skip_reset_on_cpr;
   };
   static inline int pci_intx(PCIDevice *pci_dev)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index f5ab510..21eb11c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -32,6 +32,7 @@
   #include "hw/pci/pci_host.h"
   #include "hw/qdev-properties.h"
   #include "hw/qdev-properties-system.h"
+#include "migration/cpr.h"
   #include "migration/qemu-file-types.h"
   #include "migration/vmstate.h"
   #include "net/net.h"
@@ -531,6 +532,10 @@ static void pci_reset_regions(PCIDevice *dev)
   static void pci_do_device_reset(PCIDevice *dev)
   {
+    if (dev->skip_reset_on_cpr && cpr_is_incoming()) {
+    return;
+    }


Since ->skip_reset_on_cpr is only true for vfio-pci devices, it could be
replaced by : object_dynamic_cast(OBJECT(dev), "vfio-pci")

Thanks,

C.


True but I don't really like driver dependent hacks.
what exactly about vfio makes it survive without this reset?


The kernel descriptors remain open and all the active kernel PCI state
remains in place.  The device was never quiesced or de-configured in old QEMU.

The cast is fine with me; it depends on what Michael wants.

I don't see any good ways to avoid doing the reset when a cpr resume
is in progress. I agree the cast is pretty ugly. We could keep the
'skip_reset_on_cpr' attribute and make it a class attribute instead.


I don't see any advantage to making this a class attribute.  I looked for 
examples
of using such attributes for vfio to configure pci, and found very little.  It
sounds like overkill since vfio already sets and gets PCIDevice members directly
in many places.

I defined skip_reset_on_cpr based on this existing example:

vfio_instance_init()
pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS


Also,

I wonder if the resettable interface, and more specifically the
RESET_TYPE_SNAPSHOT_LOAD type, might be useful. Have you explored
this alternative ?


RESET_TYPE_SNAPSHOT_LOAD (or a new type such as RESET_TYPE_CPR) would skip
reset for all devices, but we only skip for vfio_pci.  All other devices
(including virtio) save and restore state using standard migration vmstate,
and must call reset.

- Steve




Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-04 Thread Cédric Le Goater

On 6/4/25 09:09, Cédric Le Goater wrote:

On 6/2/25 14:36, Steven Sistare wrote:

On 6/1/2025 3:07 PM, Michael S. Tsirkin wrote:

On Sun, Jun 01, 2025 at 06:38:43PM +0200, Cédric Le Goater wrote:

On 5/29/25 21:24, Steve Sistare wrote:

Do not reset a vfio-pci device during CPR.

Signed-off-by: Steve Sistare 
---
   include/hw/pci/pci_device.h | 3 +++
   hw/pci/pci.c    | 5 +
   hw/vfio/pci.c   | 7 +++
   3 files changed, 15 insertions(+)

diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index e41d95b..b481c5d 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -181,6 +181,9 @@ struct PCIDevice {
   uint32_t max_bounce_buffer_size;
   char *sriov_pf;
+
+    /* CPR */
+    bool skip_reset_on_cpr;
   };
   static inline int pci_intx(PCIDevice *pci_dev)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index f5ab510..21eb11c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -32,6 +32,7 @@
   #include "hw/pci/pci_host.h"
   #include "hw/qdev-properties.h"
   #include "hw/qdev-properties-system.h"
+#include "migration/cpr.h"
   #include "migration/qemu-file-types.h"
   #include "migration/vmstate.h"
   #include "net/net.h"
@@ -531,6 +532,10 @@ static void pci_reset_regions(PCIDevice *dev)
   static void pci_do_device_reset(PCIDevice *dev)
   {
+    if (dev->skip_reset_on_cpr && cpr_is_incoming()) {
+    return;
+    }


Since ->skip_reset_on_cpr is only true for vfio-pci devices, it could be
replaced by : object_dynamic_cast(OBJECT(dev), "vfio-pci")

Thanks,

C.


True but I don't really like driver dependent hacks.
what exactly about vfio makes it survive without this reset?


The kernel descriptors remain open and all the active kernel PCI state
remains in place.  The device was never quiesced or de-configured in old QEMU.

The cast is fine with me; it depends on what Michael wants.

I don't see any good ways to avoid doing the reset when a cpr resume
is in progress. I agree the cast is pretty ugly. We could keep the
'skip_reset_on_cpr' attribute and make it a class attribute instead.

Also,

I wonder if the resettable interface, and more specifically the
RESET_TYPE_SNAPSHOT_LOAD type, might be useful. Have you explored
this alternative ?


Thanks,

C.





Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-04 Thread Cédric Le Goater

On 6/2/25 14:36, Steven Sistare wrote:

On 6/1/2025 3:07 PM, Michael S. Tsirkin wrote:

On Sun, Jun 01, 2025 at 06:38:43PM +0200, Cédric Le Goater wrote:

On 5/29/25 21:24, Steve Sistare wrote:

Do not reset a vfio-pci device during CPR.

Signed-off-by: Steve Sistare 
---
   include/hw/pci/pci_device.h | 3 +++
   hw/pci/pci.c    | 5 +
   hw/vfio/pci.c   | 7 +++
   3 files changed, 15 insertions(+)

diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index e41d95b..b481c5d 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -181,6 +181,9 @@ struct PCIDevice {
   uint32_t max_bounce_buffer_size;
   char *sriov_pf;
+
+    /* CPR */
+    bool skip_reset_on_cpr;
   };
   static inline int pci_intx(PCIDevice *pci_dev)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index f5ab510..21eb11c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -32,6 +32,7 @@
   #include "hw/pci/pci_host.h"
   #include "hw/qdev-properties.h"
   #include "hw/qdev-properties-system.h"
+#include "migration/cpr.h"
   #include "migration/qemu-file-types.h"
   #include "migration/vmstate.h"
   #include "net/net.h"
@@ -531,6 +532,10 @@ static void pci_reset_regions(PCIDevice *dev)
   static void pci_do_device_reset(PCIDevice *dev)
   {
+    if (dev->skip_reset_on_cpr && cpr_is_incoming()) {
+    return;
+    }


Since ->skip_reset_on_cpr is only true for vfio-pci devices, it could be
replaced by : object_dynamic_cast(OBJECT(dev), "vfio-pci")

Thanks,

C.


True but I don't really like driver dependent hacks.
what exactly about vfio makes it survive without this reset?


The kernel descriptors remain open and all the active kernel PCI state
remains in place.  The device was never quiesced or de-configured in old QEMU.

The cast is fine with me; it depends on what Michael wants.

I don't see any good ways to avoid doing the reset when a cpr resume
is in progress. I agree the cast is pretty ugly. We could keep the
'skip_reset_on_cpr' attribute and make it a class attribute instead.


Which raises another question : is this specific to vfio-pci. What
about virtio devices ?

Thanks,

C.






Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-02 Thread Steven Sistare

On 6/1/2025 3:07 PM, Michael S. Tsirkin wrote:

On Sun, Jun 01, 2025 at 06:38:43PM +0200, Cédric Le Goater wrote:

On 5/29/25 21:24, Steve Sistare wrote:

Do not reset a vfio-pci device during CPR.

Signed-off-by: Steve Sistare 
---
   include/hw/pci/pci_device.h | 3 +++
   hw/pci/pci.c| 5 +
   hw/vfio/pci.c   | 7 +++
   3 files changed, 15 insertions(+)

diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index e41d95b..b481c5d 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -181,6 +181,9 @@ struct PCIDevice {
   uint32_t max_bounce_buffer_size;
   char *sriov_pf;
+
+/* CPR */
+bool skip_reset_on_cpr;
   };
   static inline int pci_intx(PCIDevice *pci_dev)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index f5ab510..21eb11c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -32,6 +32,7 @@
   #include "hw/pci/pci_host.h"
   #include "hw/qdev-properties.h"
   #include "hw/qdev-properties-system.h"
+#include "migration/cpr.h"
   #include "migration/qemu-file-types.h"
   #include "migration/vmstate.h"
   #include "net/net.h"
@@ -531,6 +532,10 @@ static void pci_reset_regions(PCIDevice *dev)
   static void pci_do_device_reset(PCIDevice *dev)
   {
+if (dev->skip_reset_on_cpr && cpr_is_incoming()) {
+return;
+}


Since ->skip_reset_on_cpr is only true for vfio-pci devices, it could be
replaced by : object_dynamic_cast(OBJECT(dev), "vfio-pci")

Thanks,

C.


True but I don't really like driver dependent hacks.
what exactly about vfio makes it survive without this reset?


The kernel descriptors remain open and all the active kernel PCI state
remains in place.  The device was never quiesced or de-configured in old QEMU.

The cast is fine with me; it depends on what Michael wants.

- Steve


   pci_device_deassert_intx(dev);
   assert(dev->irq_state == 0);
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 7d3b9ff..56e7fdd 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3402,6 +3402,13 @@ static void vfio_instance_init(Object *obj)
   /* QEMU_PCI_CAP_EXPRESS initialization does not depend on QEMU command
* line, therefore, no need to wait to realize like other devices */
   pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
+
+/*
+ * A device that is resuming for cpr is already configured, so do not
+ * reset it during qemu_system_reset prior to cpr load, else interrupts
+ * may be lost.
+ */
+pci_dev->skip_reset_on_cpr = true;
   }> static void vfio_pci_base_dev_class_init(ObjectClass *klass,
const void *data)







Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-01 Thread Michael S. Tsirkin
On Sun, Jun 01, 2025 at 06:38:43PM +0200, Cédric Le Goater wrote:
> On 5/29/25 21:24, Steve Sistare wrote:
> > Do not reset a vfio-pci device during CPR.
> > 
> > Signed-off-by: Steve Sistare 
> > ---
> >   include/hw/pci/pci_device.h | 3 +++
> >   hw/pci/pci.c| 5 +
> >   hw/vfio/pci.c   | 7 +++
> >   3 files changed, 15 insertions(+)
> > 
> > diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> > index e41d95b..b481c5d 100644
> > --- a/include/hw/pci/pci_device.h
> > +++ b/include/hw/pci/pci_device.h
> > @@ -181,6 +181,9 @@ struct PCIDevice {
> >   uint32_t max_bounce_buffer_size;
> >   char *sriov_pf;
> > +
> > +/* CPR */
> > +bool skip_reset_on_cpr;
> >   };
> >   static inline int pci_intx(PCIDevice *pci_dev)
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index f5ab510..21eb11c 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -32,6 +32,7 @@
> >   #include "hw/pci/pci_host.h"
> >   #include "hw/qdev-properties.h"
> >   #include "hw/qdev-properties-system.h"
> > +#include "migration/cpr.h"
> >   #include "migration/qemu-file-types.h"
> >   #include "migration/vmstate.h"
> >   #include "net/net.h"
> > @@ -531,6 +532,10 @@ static void pci_reset_regions(PCIDevice *dev)
> >   static void pci_do_device_reset(PCIDevice *dev)
> >   {
> > +if (dev->skip_reset_on_cpr && cpr_is_incoming()) {
> > +return;
> > +}
> 
> Since ->skip_reset_on_cpr is only true for vfio-pci devices, it could be
> replaced by : object_dynamic_cast(OBJECT(dev), "vfio-pci")
> 
> Thanks,
> 
> C.

True but I don't really like driver dependent hacks.
what exactly about vfio makes it survive without this reset?

> 
> > +
> >   pci_device_deassert_intx(dev);
> >   assert(dev->irq_state == 0);
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > index 7d3b9ff..56e7fdd 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -3402,6 +3402,13 @@ static void vfio_instance_init(Object *obj)
> >   /* QEMU_PCI_CAP_EXPRESS initialization does not depend on QEMU command
> >* line, therefore, no need to wait to realize like other devices */
> >   pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
> > +
> > +/*
> > + * A device that is resuming for cpr is already configured, so do not
> > + * reset it during qemu_system_reset prior to cpr load, else interrupts
> > + * may be lost.
> > + */
> > +pci_dev->skip_reset_on_cpr = true;
> >   }> static void vfio_pci_base_dev_class_init(ObjectClass *klass,
> > const void *data)




Re: [PATCH V4 16/43] pci: skip reset during cpr

2025-06-01 Thread Cédric Le Goater

On 5/29/25 21:24, Steve Sistare wrote:

Do not reset a vfio-pci device during CPR.

Signed-off-by: Steve Sistare 
---
  include/hw/pci/pci_device.h | 3 +++
  hw/pci/pci.c| 5 +
  hw/vfio/pci.c   | 7 +++
  3 files changed, 15 insertions(+)

diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index e41d95b..b481c5d 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -181,6 +181,9 @@ struct PCIDevice {
  uint32_t max_bounce_buffer_size;
  
  char *sriov_pf;

+
+/* CPR */
+bool skip_reset_on_cpr;
  };
  
  static inline int pci_intx(PCIDevice *pci_dev)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index f5ab510..21eb11c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -32,6 +32,7 @@
  #include "hw/pci/pci_host.h"
  #include "hw/qdev-properties.h"
  #include "hw/qdev-properties-system.h"
+#include "migration/cpr.h"
  #include "migration/qemu-file-types.h"
  #include "migration/vmstate.h"
  #include "net/net.h"
@@ -531,6 +532,10 @@ static void pci_reset_regions(PCIDevice *dev)
  
  static void pci_do_device_reset(PCIDevice *dev)

  {
+if (dev->skip_reset_on_cpr && cpr_is_incoming()) {
+return;
+}


Since ->skip_reset_on_cpr is only true for vfio-pci devices, it could be
replaced by : object_dynamic_cast(OBJECT(dev), "vfio-pci")

Thanks,

C.



+
  pci_device_deassert_intx(dev);
  assert(dev->irq_state == 0);
  
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c

index 7d3b9ff..56e7fdd 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3402,6 +3402,13 @@ static void vfio_instance_init(Object *obj)
  /* QEMU_PCI_CAP_EXPRESS initialization does not depend on QEMU command
   * line, therefore, no need to wait to realize like other devices */
  pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
+
+/*
+ * A device that is resuming for cpr is already configured, so do not
+ * reset it during qemu_system_reset prior to cpr load, else interrupts
+ * may be lost.
+ */
+pci_dev->skip_reset_on_cpr = true;
  }>   
  static void vfio_pci_base_dev_class_init(ObjectClass *klass, const void *data)