Message ID | 20231025212422.30371-2-vikram.garhwal@amd.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [QEMU,PATCHv2,1/8] xen: when unplugging emulated devices skip virtio devices | expand |
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?
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.
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.
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 --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: