diff mbox series

[QEMU,PATCHv2,1/8] xen: when unplugging emulated devices skip virtio devices

Message ID 20231025212422.30371-2-vikram.garhwal@amd.com (mailing list archive)
State New, archived
Headers show
Series Xen: support grant mappings. | expand

Commit Message

Vikram Garhwal Oct. 25, 2023, 9:24 p.m. UTC 
From: Juergen Gross <jgross@suse.com>

Virtio devices should never be unplugged at boot time, as they are
similar to pci passthrough devices.

Signed-off-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com>
---
 docs/system/i386/xen.rst   |  3 ---
 hw/i386/xen/xen_platform.c | 10 ++++++++--
 2 files changed, 8 insertions(+), 5 deletions(-)

Comments

David Woodhouse Oct. 25, 2023, 11:22 p.m. UTC | #1 
On Wed, 2023-10-25 at 14:24 -0700, Vikram Garhwal wrote:
> From: Juergen Gross <jgross@suse.com>
> 
> Virtio devices should never be unplugged at boot time, as they are
> similar to pci passthrough devices.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com>

Hm, do your virtio NICs still actually *work* after that? Or are they
all disconnected from their netdev peers? 

I suspect you're going to want a variant of
https://lore.kernel.org/qemu-devel/20231025145042.627381-19-dwmw2@infradead.org/T/#u
which also leave the peers of your virtio devices intact?
Stefano Stabellini Oct. 26, 2023, 1:23 a.m. UTC | #2 
On Thu, 26 Oct 2023, David Woodhouse wrote:
> On Wed, 2023-10-25 at 14:24 -0700, Vikram Garhwal wrote:
> > From: Juergen Gross <jgross@suse.com>
> > 
> > Virtio devices should never be unplugged at boot time, as they are
> > similar to pci passthrough devices.
> > 
> > Signed-off-by: Juergen Gross <jgross@suse.com>
> > Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com>
> 
> Hm, do your virtio NICs still actually *work* after that? Or are they
> all disconnected from their netdev peers? 
> 
> I suspect you're going to want a variant of
> https://lore.kernel.org/qemu-devel/20231025145042.627381-19-dwmw2@infradead.org/T/#u
> which also leave the peers of your virtio devices intact?

Hi David, device unplug is an x86-only thing (see the definition of
xen_emul_unplug in Linux under arch/x86/xen/platform-pci-unplug.c) I
suspect Vikram who is working on ARM hasn't tested it.

Vikram, a simple option is to drop this patch if you don't need it.
David Woodhouse Oct. 26, 2023, 3:45 p.m. UTC | #3 
On Wed, 2023-10-25 at 18:23 -0700, Stefano Stabellini wrote:
> On Thu, 26 Oct 2023, David Woodhouse wrote:
> > On Wed, 2023-10-25 at 14:24 -0700, Vikram Garhwal wrote:
> > > From: Juergen Gross <jgross@suse.com>
> > > 
> > > Virtio devices should never be unplugged at boot time, as they are
> > > similar to pci passthrough devices.
> > > 
> > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > > Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com>
> > 
> > Hm, do your virtio NICs still actually *work* after that? Or are they
> > all disconnected from their netdev peers? 
> > 
> > I suspect you're going to want a variant of
> > https://lore.kernel.org/qemu-devel/20231025145042.627381-19-dwmw2@infradead.org/T/#u
> > which also leave the peers of your virtio devices intact?
> 
> Hi David, device unplug is an x86-only thing (see the definition of
> xen_emul_unplug in Linux under arch/x86/xen/platform-pci-unplug.c) I
> suspect Vikram who is working on ARM hasn't tested it.

Ah, I had assumed there was something else coming along later which
would make it actually get used. 

> Vikram, a simple option is to drop this patch if you don't need it.

That works. Although I may revive it in that case.
Vikram Garhwal Oct. 26, 2023, 5:13 p.m. UTC | #4 
On Thu, Oct 26, 2023 at 04:45:21PM +0100, David Woodhouse wrote:
> On Wed, 2023-10-25 at 18:23 -0700, Stefano Stabellini wrote:
> > On Thu, 26 Oct 2023, David Woodhouse wrote:
> > > On Wed, 2023-10-25 at 14:24 -0700, Vikram Garhwal wrote:
> > > > From: Juergen Gross <jgross@suse.com>
> > > > 
> > > > Virtio devices should never be unplugged at boot time, as they are
> > > > similar to pci passthrough devices.
> > > > 
> > > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > > > Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com>
> > > 
> > > Hm, do your virtio NICs still actually *work* after that? Or are they
> > > all disconnected from their netdev peers? 
> > > 
> > > I suspect you're going to want a variant of
> > > https://lore.kernel.org/qemu-devel/20231025145042.627381-19-dwmw2@infradead.org/T/#u
> > > which also leave the peers of your virtio devices intact?
> > 
> > Hi David, device unplug is an x86-only thing (see the definition of
> > xen_emul_unplug in Linux under arch/x86/xen/platform-pci-unplug.c) I
> > suspect Vikram who is working on ARM hasn't tested it.
> 
> Ah, I had assumed there was something else coming along later which
> would make it actually get used. 
> 
> > Vikram, a simple option is to drop this patch if you don't need it.
> 
> That works. Although I may revive it in that case. 
> 
Hopefully, Juergen is also okay with dropping the patch. Then, i will remove it
from v3.

Thanks David & Stefano!
diff mbox series

Patch

diff --git a/docs/system/i386/xen.rst b/docs/system/i386/xen.rst
index f06765e88c..b86d57af6e 100644
--- a/docs/system/i386/xen.rst
+++ b/docs/system/i386/xen.rst
@@ -52,9 +52,6 @@  It is necessary to use the pc machine type, as the q35 machine uses AHCI instead
 of legacy IDE, and AHCI disks are not unplugged through the Xen PV unplug
 mechanism.
 
-VirtIO devices can also be used; Linux guests may need to be dissuaded from
-umplugging them by adding 'xen_emul_unplug=never' on their command line.
-
 Properties
 ----------
 
diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index 17457ff3de..0187b73eeb 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -28,6 +28,7 @@ 
 #include "hw/ide/pci.h"
 #include "hw/pci/pci.h"
 #include "migration/vmstate.h"
+#include "hw/virtio/virtio-bus.h"
 #include "net/net.h"
 #include "trace.h"
 #include "sysemu/xen.h"
@@ -129,10 +130,11 @@  static bool pci_device_is_passthrough(PCIDevice *d)
 
 static void unplug_nic(PCIBus *b, PCIDevice *d, void *o)
 {
-    /* We have to ignore passthrough devices */
+    /* We have to ignore passthrough devices  and virtio devices. */
     if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
             PCI_CLASS_NETWORK_ETHERNET
-            && !pci_device_is_passthrough(d)) {
+            && !pci_device_is_passthrough(d)
+            && !qdev_get_child_bus(&d->qdev, TYPE_VIRTIO_BUS)) {
         object_unparent(OBJECT(d));
     }
 }
@@ -208,6 +210,10 @@  static void unplug_disks(PCIBus *b, PCIDevice *d, void *opaque)
     /* We have to ignore passthrough devices */
     if (pci_device_is_passthrough(d))
         return;
+    /* Ignore virtio devices */
+    if (qdev_get_child_bus(&d->qdev, TYPE_VIRTIO_BUS)) {
+        return;
+    }
 
     switch (pci_get_word(d->config + PCI_CLASS_DEVICE)) {
     case PCI_CLASS_STORAGE_IDE: