[Qemu-devel] [PATCH 0/2] Xen HVM unplug changes

[Qemu-devel] [PATCH 0/2] Xen HVM unplug changes

[Olaf Hering]

Update unplug in Xen HVM guests to cover more cases.
Please review.

Olaf

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

 hw/i386/xen/xen_platform.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

[Qemu-devel] [PATCH 1/2] xen_platform: unplug also SCSI disks

[Olaf Hering]

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

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 hw/i386/xen/xen_platform.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index aa78393..d94b53c 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -115,6 +115,11 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void *o)
             && strcmp(d->name, "xen-pci-passthrough") != 0) {
         pci_piix3_xen_ide_unplug(DEVICE(d));
     }
+    else if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
+            PCI_CLASS_STORAGE_SCSI
+            && strcmp(d->name, "xen-pci-passthrough") != 0) {
+        object_unparent(OBJECT(d));
+    }
 }
 
 static void pci_unplug_disks(PCIBus *bus)

[Qemu-devel] [PATCH 2/2] xen_platform: SUSE xenlinux unplug for emulated PCI

[Olaf Hering]

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.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 hw/i386/xen/xen_platform.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index d94b53c..8802482 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -314,13 +314,42 @@ static void xen_platform_ioport_writeb(void *opaque, hwaddr addr,
                                        uint64_t val, unsigned int size)
 {
     PCIXenPlatformState *s = opaque;
+    PCIDevice *pci_dev = PCI_DEVICE(s);
 
     switch (addr) {
     case 0: /* Platform flags */
         platform_fixed_ioport_writeb(opaque, 0, (uint32_t)val);
         break;
+    case 4:
+        if (val == 1) {
+            /*
+             * 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 made use of 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.
+             */
+            blk_drain_all();
+            blk_flush_all();
+            pci_unplug_disks(pci_dev->bus);
+            pci_unplug_nics(pci_dev->bus);
+        }
+        break;
     case 8:
-        log_writeb(s, (uint32_t)val);
+        switch (val) {
+        case 1:
+            blk_drain_all();
+            blk_flush_all();
+            pci_unplug_disks(pci_dev->bus);
+            break;
+        case 2:
+            pci_unplug_nics(pci_dev->bus);
+            break;
+        default:
+            log_writeb(s, (uint32_t)val);
+            break;
+        }
         break;
     default:
         break;

Re: [Qemu-devel] [PATCH 0/2] Xen HVM unplug changes

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH 0/2] Xen HVM unplug changes
Type: series
Message-id: 20160901121131.16007-1-olaf@aepfle.de

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20160901121131.16007-1-olaf@aepfle.de -> patchew/20160901121131.16007-1-olaf@aepfle.de
Switched to a new branch 'test'
bc12567 xen_platform: SUSE xenlinux unplug for emulated PCI
cfa2940 xen_platform: unplug also SCSI disks

=== OUTPUT BEGIN ===
Checking PATCH 1/2: xen_platform: unplug also SCSI disks...
ERROR: else should follow close brace '}'
#58: FILE: hw/i386/xen/xen_platform.c:118:
     }
+    else if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==

total: 1 errors, 0 warnings, 11 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/2: xen_platform: SUSE xenlinux unplug for emulated PCI...
ERROR: line over 90 characters
#33: FILE: hw/i386/xen/xen_platform.c:327:
+             * xen-kmp used this since xen-3.0.4, instead the official protocol from xen-3.3+

WARNING: line over 80 characters
#35: FILE: hw/i386/xen/xen_platform.c:329:
+             * Pre VMDP 1.7 made use of 4 and 8 depending on how VMDP was configured.

total: 1 errors, 1 warnings, 43 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH 0/2] Xen HVM unplug changes

[Paolo Bonzini]

On 01/09/2016 14:11, Olaf Hering wrote:
> Update unplug in Xen HVM guests to cover more cases.
> Please review.
> 
> Olaf
> 
> Olaf Hering (2):
>   xen_platform: unplug also SCSI disks
>   xen_platform: SUSE xenlinux unplug for emulated PCI
> 
>  hw/i386/xen/xen_platform.c | 36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 

Looks good with the checkpatch complaints fixed.

Paolo

Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xen_platform: SUSE xenlinux unplug for emulated PCI

[Konrad Rzeszutek Wilk]

On Thu, Sep 01, 2016 at 02:11:31PM +0200, Olaf Hering wrote:
> 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.

Should this be documented in the protocol?

> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  hw/i386/xen/xen_platform.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
> index d94b53c..8802482 100644
> --- a/hw/i386/xen/xen_platform.c
> +++ b/hw/i386/xen/xen_platform.c
> @@ -314,13 +314,42 @@ static void xen_platform_ioport_writeb(void *opaque, hwaddr addr,
>                                         uint64_t val, unsigned int size)
>  {
>      PCIXenPlatformState *s = opaque;
> +    PCIDevice *pci_dev = PCI_DEVICE(s);
>  
>      switch (addr) {
>      case 0: /* Platform flags */
>          platform_fixed_ioport_writeb(opaque, 0, (uint32_t)val);
>          break;
> +    case 4:
> +        if (val == 1) {
> +            /*
> +             * 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 made use of 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.
> +             */
> +            blk_drain_all();
> +            blk_flush_all();
> +            pci_unplug_disks(pci_dev->bus);
> +            pci_unplug_nics(pci_dev->bus);
> +        }
> +        break;
>      case 8:
> -        log_writeb(s, (uint32_t)val);
> +        switch (val) {
> +        case 1:
> +            blk_drain_all();
> +            blk_flush_all();
> +            pci_unplug_disks(pci_dev->bus);
> +            break;
> +        case 2:
> +            pci_unplug_nics(pci_dev->bus);
> +            break;
> +        default:
> +            log_writeb(s, (uint32_t)val);
> +            break;
> +        }
>          break;
>      default:
>          break;
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xen_platform: SUSE xenlinux unplug for emulated PCI

[Stefano Stabellini]

On Thu, 1 Sep 2016, Konrad Rzeszutek Wilk wrote:
> On Thu, Sep 01, 2016 at 02:11:31PM +0200, Olaf Hering wrote:
> > 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.
> 
> Should this be documented in the protocol?

Yes, please. The file is docs/misc/hvm-emulated-unplug.markdown in the
Xen repository. Please also document the behavior with SCSI disks, which
is currently missing.


> > 
> > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > ---
> >  hw/i386/xen/xen_platform.c | 31 ++++++++++++++++++++++++++++++-
> >  1 file changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
> > index d94b53c..8802482 100644
> > --- a/hw/i386/xen/xen_platform.c
> > +++ b/hw/i386/xen/xen_platform.c
> > @@ -314,13 +314,42 @@ static void xen_platform_ioport_writeb(void *opaque, hwaddr addr,
> >                                         uint64_t val, unsigned int size)
> >  {
> >      PCIXenPlatformState *s = opaque;
> > +    PCIDevice *pci_dev = PCI_DEVICE(s);
> >  
> >      switch (addr) {
> >      case 0: /* Platform flags */
> >          platform_fixed_ioport_writeb(opaque, 0, (uint32_t)val);
> >          break;
> > +    case 4:
> > +        if (val == 1) {
> > +            /*
> > +             * 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 made use of 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.
> > +             */
> > +            blk_drain_all();
> > +            blk_flush_all();
> > +            pci_unplug_disks(pci_dev->bus);
> > +            pci_unplug_nics(pci_dev->bus);
> > +        }
> > +        break;
> >      case 8:
> > -        log_writeb(s, (uint32_t)val);
> > +        switch (val) {
> > +        case 1:
> > +            blk_drain_all();
> > +            blk_flush_all();
> > +            pci_unplug_disks(pci_dev->bus);
> > +            break;
> > +        case 2:
> > +            pci_unplug_nics(pci_dev->bus);
> > +            break;
> > +        default:
> > +            log_writeb(s, (uint32_t)val);
> > +            break;
> > +        }
> >          break;
> >      default:
> >          break;
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > https://lists.xen.org/xen-devel
>