[PATCH qemu-xen-traditional 0/2] Xen HVM unplug changes

[PATCH qemu-xen-traditional 0/2] Xen HVM unplug changes

[Olaf Hering]

Backport from qemu-2.8:
Update unplug in Xen HVM guests to cover more cases.
The patches are used since years in the SUSE xen-tools package.

Olaf



Olaf Hering (2):
  xen_platform: unplug also SCSI disks
  xen_platform: SUSE xenlinux unplug for emulated PCI

 hw/pci.c          | 41 +++++++++++++++++++++++++++++++++++++++++
 hw/xen_platform.c | 37 ++++++++++++++++++++++++++++++++++++-
 qemu-xen.h        |  1 +
 3 files changed, 78 insertions(+), 1 deletion(-)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH qemu-xen-traditional 1/2] xen_platform: unplug also SCSI disks

[Olaf Hering]

From: Olaf Hering <ohering@suse.de>

Using 'vdev=sd[a-o]' will create an emulated LSI controller, which can
be used by the emulated BIOS to boot from disk. If the HVM domU has also
PV driver the disk may appear twice in the guest. To avoid this an
unplug of the emulated hardware is needed, similar to what is done for
IDE and NIC drivers already.

Since the SCSI controller provides only disks the entire controller can
be unplugged at once.

Impact of the change for classic and pvops based guest kernels:

 vdev=sda:disk0
before: pvops:   disk0=pv xvda + emulated sda
        classic: disk0=pv sda  + emulated sdq
after:  pvops:   disk0=pv xvda
        classic: disk0=pv sda

 vdev=hda:disk0, vdev=sda:disk1
before: pvops:   disk0=pv xvda
                 disk1=emulated sda
        classic: disk0=pv hda
                 disk1=pv sda  + emulated sdq
after:  pvops:   disk0=pv xvda
                 disk1=not accessible by blkfront, index hda==index sda
        classic: disk0=pv hda
                 disk1=pv sda

 vdev=hda:disk0, vdev=sda:disk1, vdev=sdb:disk2
before: pvops:   disk0=pv xvda
                 disk1=emulated sda
                 disk2=pv xvdb + emulated sdb
        classic: disk0=pv hda
                 disk1=pv sda  + emulated sdq
                 disk2=pv sdb  + emulated sdr
after:  pvops:   disk0=pv xvda
                 disk1=not accessible by blkfront, index hda==index sda
                 disk2=pv xvdb
        classic: disk0=pv hda
                 disk1=pv sda
                 disk2=pv sda

Upstream commit 78f66897ddf58d1ffe5e0b95f7c1a1dad103a8da

Signed-off-by: Olaf Hering <ohering@suse.de>
---
 hw/pci.c          | 41 +++++++++++++++++++++++++++++++++++++++++
 hw/xen_platform.c |  4 +++-
 qemu-xen.h        |  1 +
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/hw/pci.c b/hw/pci.c
index c423285..7d8e3b6 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -871,6 +871,47 @@ void pci_unplug_netifs(void)
     }
 }
 
+void pci_unplug_scsi(void)
+{
+    PCIBus *bus;
+    PCIDevice *dev;
+    PCIIORegion *region;
+    int x;
+    int i;
+
+    /* We only support one PCI bus */
+    for (bus = first_bus; bus; bus = NULL) {
+       for (x = 0; x < 256; x++) {
+           dev = bus->devices[x];
+           if (dev &&
+               dev->config[0xa] == 0 &&
+               dev->config[0xb] == 1
+#ifdef CONFIG_PASSTHROUGH
+               && test_pci_devfn(x) != 1
+#endif
+               ) {
+               /* Found a scsi disk.  Remove it from the bus.  Note that
+                  we don't free it here, since there could still be
+                  references to it floating around.  There are only
+                  ever one or two structures leaked, and it's not
+                  worth finding them all. */
+               bus->devices[x] = NULL;
+               for (i = 0; i < PCI_NUM_REGIONS; i++) {
+                   region = &dev->io_regions[i];
+                   if (region->addr == (uint32_t)-1 ||
+                       region->size == 0)
+                       continue;
+                   if (region->type == PCI_ADDRESS_SPACE_IO) {
+                       isa_unassign_ioport(region->addr, region->size);
+                   } else if (region->type == PCI_ADDRESS_SPACE_MEM) {
+                       unregister_iomem(region->addr);
+                   }
+               }
+           }
+       }
+    }
+}
+
 typedef struct {
     PCIDevice dev;
     PCIBus *bus;
diff --git a/hw/xen_platform.c b/hw/xen_platform.c
index d282f8e..0a94e0d 100644
--- a/hw/xen_platform.c
+++ b/hw/xen_platform.c
@@ -154,8 +154,10 @@ static void platform_fixed_ioport_write2(void *opaque, uint32_t addr, uint32_t v
         /* Unplug devices.  Value is a bitmask of which devices to
            unplug, with bit 0 the IDE devices, bit 1 the network
            devices, and bit 2 the non-primary-master IDE devices. */
-        if (val & UNPLUG_ALL_IDE_DISKS)
+        if (val & UNPLUG_ALL_IDE_DISKS) {
             ide_unplug_harddisks();
+            pci_unplug_scsi();
+        }
         if (val & UNPLUG_ALL_NICS) {
             pci_unplug_netifs();
             net_tap_shutdown_all();
diff --git a/qemu-xen.h b/qemu-xen.h
index 0598668..d315623 100644
--- a/qemu-xen.h
+++ b/qemu-xen.h
@@ -47,6 +47,7 @@ void unset_vram_mapping(void *opaque);
 #endif
 
 void pci_unplug_netifs(void);
+void pci_unplug_scsi(void);
 void destroy_hvm_domain(void);
 void unregister_iomem(target_phys_addr_t start);
 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH qemu-xen-traditional 1/2] xen_platform: unplug also SCSI disks [and 1 more messages]

[Ian Jackson]

Olaf Hering writes ("[PATCH qemu-xen-traditional 1/2] xen_platform: unplug also SCSI disks"):
> From: Olaf Hering <ohering@suse.de>
> 
> Using 'vdev=sd[a-o]' will create an emulated LSI controller, which can
> be used by the emulated BIOS to boot from disk. If the HVM domU has also
> PV driver the disk may appear twice in the guest. To avoid this an
> unplug of the emulated hardware is needed, similar to what is done for
> IDE and NIC drivers already.

qemu-xen-traditional is in the deep freeze.  I'm only fixes for quite
serious problems (such as security problems).

> Impact of the change for classic and pvops based guest kernels:

While I think this change has a risk of breaking things, and certainly
wouldn't warrant backporting to earlier Xen releases, I think there is
an arguable case for this patch.  At the very least it only affects
users with scsi disks.

That the patch has been in use in SUSE for a long time is also a
helpful datapoint.

I am inclined to accept this patch but would welcome opinions from
others.  Olaf, if no-one replies, please ping me about this again, and
I will apply it.

Olaf Hering writes ("[PATCH qemu-xen-traditional 2/2] xen_platform: SUSE xenlinux unplug for emulated PCI"):
> From: Olaf Hering <ohering@suse.de>
> 
> Implement SUSE specific unplug protocol for emulated PCI devices
> in PVonHVM guests. Its a simple 'outl(1, (ioaddr + 4));'.
> This protocol was implemented and used since Xen 3.0.4.
> It is used in all SUSE/SLES/openSUSE releases up to SLES11SP3 and
> openSUSE 12.3.
> In addition old (pre-2011) VMDP versions are handled as well.

On the other hand, I am very reluctant to apply this.

I don't see a good reason for SUSE to have a custom unplug protocol.
Why can't your guests use the standard one ?  Why haven't they been
updated to use the standard one some time in the last ?5-10 years ?

We had a similar question about certain Citrix XenServer behaviours.
I forget the details.  But if I remember that was also a case where a
downstream had invented a private variant of an upstream protocol,
failed to coordinate with upstream, and simply shipped a revised
protocol.  Again there we resisted incorporation of ad-hoc protocols.

Sorry.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH qemu-xen-traditional 1/2] xen_platform: unplug also SCSI disks [and 1 more messages]

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 1173 bytes --]

On Mon, Jan 09, Ian Jackson wrote:

> Olaf Hering writes ("[PATCH qemu-xen-traditional 2/2] xen_platform: SUSE xenlinux unplug for emulated PCI"):
> > From: Olaf Hering <ohering@suse.de>
> > 
> > Implement SUSE specific unplug protocol for emulated PCI devices
> > in PVonHVM guests. Its a simple 'outl(1, (ioaddr + 4));'.
> > This protocol was implemented and used since Xen 3.0.4.
> > It is used in all SUSE/SLES/openSUSE releases up to SLES11SP3 and
> > openSUSE 12.3.
> > In addition old (pre-2011) VMDP versions are handled as well.
> 
> On the other hand, I am very reluctant to apply this.
> 
> I don't see a good reason for SUSE to have a custom unplug protocol.
> Why can't your guests use the standard one ?  Why haven't they been
> updated to use the standard one some time in the last ?5-10 years ?

The protocol was introduced before upstream had one, xen-3.0 vs.
xen-3.x. I have not digged into 10 year old emails why it was done that
way, if it was ever proposed for upstream inclusion. Meanwhile the
upstream unplug is used since a two years. This patch would allow to run
old SUSE domUs on new non-SUSE dom0s with qemu-trad.

Olaf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH qemu-xen-traditional 1/2] xen_platform: unplug also SCSI disks [and 1 more messages]

[George Dunlap]

On Mon, Jan 9, 2017 at 4:39 PM, Olaf Hering <olaf@aepfle.de> wrote:
> On Mon, Jan 09, Ian Jackson wrote:
>
>> Olaf Hering writes ("[PATCH qemu-xen-traditional 2/2] xen_platform: SUSE xenlinux unplug for emulated PCI"):
>> > From: Olaf Hering <ohering@suse.de>
>> >
>> > Implement SUSE specific unplug protocol for emulated PCI devices
>> > in PVonHVM guests. Its a simple 'outl(1, (ioaddr + 4));'.
>> > This protocol was implemented and used since Xen 3.0.4.
>> > It is used in all SUSE/SLES/openSUSE releases up to SLES11SP3 and
>> > openSUSE 12.3.
>> > In addition old (pre-2011) VMDP versions are handled as well.
>>
>> On the other hand, I am very reluctant to apply this.
>>
>> I don't see a good reason for SUSE to have a custom unplug protocol.
>> Why can't your guests use the standard one ?  Why haven't they been
>> updated to use the standard one some time in the last ?5-10 years ?
>
> The protocol was introduced before upstream had one, xen-3.0 vs.
> xen-3.x. I have not digged into 10 year old emails why it was done that
> way, if it was ever proposed for upstream inclusion. Meanwhile the
> upstream unplug is used since a two years. This patch would allow to run
> old SUSE domUs on new non-SUSE dom0s with qemu-trad.

We make efforts to allow guest OSes that use Hyper-V and VMWare
interfaces run properly, this seems small in comparison.

Given that SuSE kernels are now using the new unplug protocol, I think
there's no longer any reason to stand on principle.  We take a
practical approach for emulating other hypervisor interfaces (Hyper-V
and VMWare); making accomodation for "one of our own" seems pretty
reasonable.  (I haven't reviewed the patch itself.)

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH qemu-xen-traditional 1/2] xen_platform: unplug also SCSI disks [and 1 more messages]

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 903 bytes --]

On Tue, Jan 10, George Dunlap wrote:

> On Mon, Jan 9, 2017 at 4:39 PM, Olaf Hering <olaf@aepfle.de> wrote:
> > The protocol was introduced before upstream had one, xen-3.0 vs.
> > xen-3.x. I have not digged into 10 year old emails why it was done that
> > way, if it was ever proposed for upstream inclusion. Meanwhile the
> > upstream unplug is used since a two years. This patch would allow to run
> > old SUSE domUs on new non-SUSE dom0s with qemu-trad.
> Given that SuSE kernels are now using the new unplug protocol, I think
> there's no longer any reason to stand on principle.  We take a
> practical approach for emulating other hypervisor interfaces (Hyper-V
> and VMWare); making accomodation for "one of our own" seems pretty
> reasonable.  (I haven't reviewed the patch itself.)


So what should be done with the two patches?
Are they acceptable for staging, or will they be rejected?

Olaf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH qemu-xen-traditional 1/2] xen_platform: unplug also SCSI disks [and 1 more messages]

[Ian Jackson]

Olaf Hering writes ("Re: [Xen-devel] [PATCH qemu-xen-traditional 1/2] xen_platform: unplug also SCSI disks [and 1 more messages]"):
> So what should be done with the two patches?
> Are they acceptable for staging, or will they be rejected?

I am happy with the first patch, as I say.  Would you like me to apply
it while we discuss the 2nd ?

As for the 2nd patch: the last new feature was added to qemu-xen in
late 2011 ("qemu-xen: add vkbd support for PV on HVM guests"
9b33a3e5603e) or maybe early 2012 ("xen: introduce an event channel
for buffered io event notifications" a5af89a7d40d).

You are asking for a new feature to be accepted in a codebase which
has been on receiving only security fixes and bitrot fixes for many
years.

AFAICT it seems unlikely that this new feature will add new security
bugs, but might it break some old guests which mistakenly write to
this ioport ?

I think if you can reassure me about this point, I may be persuaded to
accept the patch.

Very sorry to be so awkward.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH qemu-xen-traditional 1/2] xen_platform: unplug also SCSI disks [and 1 more messages]

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 448 bytes --]

On Tue, Jan 31, Ian Jackson wrote:

> Olaf Hering writes ("Re: [Xen-devel] [PATCH qemu-xen-traditional 1/2] xen_platform: unplug also SCSI disks [and 1 more messages]"):
> > So what should be done with the two patches?
> > Are they acceptable for staging, or will they be rejected?
> 
> I am happy with the first patch, as I say.  Would you like me to apply
> it while we discuss the 2nd ?

Yes. Will think about the 2nd. Thanks.

Olaf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH qemu-xen-traditional 2/2] xen_platform: SUSE xenlinux unplug for emulated PCI

[Olaf Hering]

From: Olaf Hering <ohering@suse.de>

Implement SUSE specific unplug protocol for emulated PCI devices
in PVonHVM guests. Its a simple 'outl(1, (ioaddr + 4));'.
This protocol was implemented and used since Xen 3.0.4.
It is used in all SUSE/SLES/openSUSE releases up to SLES11SP3 and
openSUSE 12.3.
In addition old (pre-2011) VMDP versions are handled as well.

Upstream commit 35132016dc1c27de2b1354b161df6cc22f3ac5bf

Signed-off-by: Olaf Hering <ohering@suse.de>
---
 hw/xen_platform.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/hw/xen_platform.c b/hw/xen_platform.c
index 0a94e0d..3c2fc0e 100644
--- a/hw/xen_platform.c
+++ b/hw/xen_platform.c
@@ -337,9 +337,42 @@ static void xen_platform_ioport_writeb(void *opaque, uint32_t addr, uint32_t val
     }
 }
 
+static void suse_platform_ioport_writel(void *opaque, uint32_t addr, uint32_t val)
+{
+    addr &= 0x0f;
+    switch (addr) {
+    case 4:
+        /*
+         * SUSE unplug for Xenlinux
+         * xen-kmp used this since xen-3.0.4, instead the official protocol
+         * from xen-3.3+ It did an unconditional "outl(1, (ioaddr + 4));"
+         * Pre VMDP 1.7 used 4 and 8 depending on how VMDP was configured.
+         * If VMDP was to control both disk and LAN it would use 4.
+         * If it controlled just disk or just LAN, it would use 8 below.
+         * */
+        ide_unplug_harddisks();
+        pci_unplug_scsi();
+        pci_unplug_netifs();
+        net_tap_shutdown_all();
+        break;
+    case 8:
+        if (val ==1 ) {
+            ide_unplug_harddisks();
+            pci_unplug_scsi();
+        } else if (val == 2) {
+            pci_unplug_netifs();
+            net_tap_shutdown_all();
+        }
+        break;
+    default:
+        break;
+    }
+}
+
 static void platform_ioport_map(PCIDevice *pci_dev, int region_num, uint32_t addr, uint32_t size, int type)
 {
     PCIXenPlatformState *d = (PCIXenPlatformState *)pci_dev;
+    register_ioport_write(addr, 16, 4, suse_platform_ioport_writel, d);
     register_ioport_write(addr, size, 1, xen_platform_ioport_writeb, d);
     register_ioport_read(addr, size, 1, xen_platform_ioport_readb, d);
 }

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH qemu-xen-traditional 0/2] Xen HVM unplug changes

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 254 bytes --]

On Thu, Nov 24, Olaf Hering wrote:

> Backport from qemu-2.8:
> Update unplug in Xen HVM guests to cover more cases.
> The patches are used since years in the SUSE xen-tools package.

Are these patches in the review queue, or did they get dropped?

Olaf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH qemu-xen-traditional 0/2] Xen HVM unplug changes

[Ian Jackson]

Olaf Hering writes ("Re: [Xen-devel] [PATCH qemu-xen-traditional 0/2] Xen HVM unplug changes"):
> On Thu, Nov 24, Olaf Hering wrote:
> > Backport from qemu-2.8:
> > Update unplug in Xen HVM guests to cover more cases.
> > The patches are used since years in the SUSE xen-tools package.
> 
> Are these patches in the review queue, or did they get dropped?

Sorry, I was away in November and don't always manage to catch up on
all my email.  In future do feel free to ping after a couple of weeks
rather than a couple of months...

I'll reply to the emails now.

Regards,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel