[PATCH v1] piix: fix regression during unplug in Xen HVM domUs

Olaf Hering posted 1 patch 3 years, 1 month ago
Failed in applying to current master (apply log)
There is a newer version of this series
hw/ide/piix.c | 5 +++++
1 file changed, 5 insertions(+)
[PATCH v1] piix: fix regression during unplug in Xen HVM domUs
Posted by Olaf Hering 3 years, 1 month ago
Commit ee358e919e385fdc79d59d0d47b4a81e349cd5c9 causes a regression in
Xen HVM domUs which run xenlinux based kernels.

If the domU has an USB device assigned, for example with
"usbdevice=['tablet']" in domU.cfg, the late unplug of devices will
kill the emulated USB host. As a result the khubd thread hangs, and as
a result the entire boot process.

For some reason this does not affect pvops based kernels. This is
most likely caused by the fact that unplugging happens very early
during boot.

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

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index b9860e35a5..2a380a90e9 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -34,6 +34,7 @@
 #include "hw/ide/pci.h"
 #include "trace.h"
 
+static bool pci_piix3_xen_ide_unplug_done;
 static uint64_t bmdma_read(void *opaque, hwaddr addr, unsigned size)
 {
     BMDMAState *bm = opaque;
@@ -109,6 +110,8 @@ static void piix_ide_reset(DeviceState *dev)
     uint8_t *pci_conf = pd->config;
     int i;
 
+    if (pci_piix3_xen_ide_unplug_done == true)
+        return;
     for (i = 0; i < 2; i++) {
         ide_bus_reset(&d->bus[i]);
     }
@@ -169,6 +172,7 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
     IDEBus *idebus;
     BlockBackend *blk;
 
+    pci_piix3_xen_ide_unplug_done = true;
     pci_ide = PCI_IDE(dev);
 
     for (i = aux ? 1 : 0; i < 4; i++) {
@@ -259,6 +263,7 @@ static const TypeInfo piix4_ide_info = {
 
 static void piix_ide_register_types(void)
 {
+    pci_piix3_xen_ide_unplug_done = false;
     type_register_static(&piix3_ide_info);
     type_register_static(&piix3_ide_xen_info);
     type_register_static(&piix4_ide_info);

Re: [PATCH v1] piix: fix regression during unplug in Xen HVM domUs
Posted by no-reply@patchew.org 3 years, 1 month ago
Patchew URL: https://patchew.org/QEMU/20210316224412.11609-1-olaf@aepfle.de/



Hi,

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

Type: series
Message-id: 20210316224412.11609-1-olaf@aepfle.de
Subject: [PATCH v1] piix: fix regression during unplug in Xen HVM domUs

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20210311143958.562625-1-richard.henderson@linaro.org -> patchew/20210311143958.562625-1-richard.henderson@linaro.org
 * [new tag]         patchew/20210316224412.11609-1-olaf@aepfle.de -> patchew/20210316224412.11609-1-olaf@aepfle.de
Switched to a new branch 'test'
9b7ec0f piix: fix regression during unplug in Xen HVM domUs

=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#41: FILE: hw/ide/piix.c:113:
+    if (pci_piix3_xen_ide_unplug_done == true)
[...]

total: 1 errors, 0 warnings, 29 lines checked

Commit 9b7ec0fe5d7c (piix: fix regression during unplug in Xen HVM domUs) 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


The full log is available at
http://patchew.org/logs/20210316224412.11609-1-olaf@aepfle.de/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH v1] piix: fix regression during unplug in Xen HVM domUs
Posted by Philippe Mathieu-Daudé 3 years, 1 month ago
On 3/16/21 11:44 PM, Olaf Hering wrote:
> Commit ee358e919e385fdc79d59d0d47b4a81e349cd5c9 causes a regression in
> Xen HVM domUs which run xenlinux based kernels.
> 
> If the domU has an USB device assigned, for example with
> "usbdevice=['tablet']" in domU.cfg, the late unplug of devices will
> kill the emulated USB host. As a result the khubd thread hangs, and as
> a result the entire boot process.
> 
> For some reason this does not affect pvops based kernels. This is
> most likely caused by the fact that unplugging happens very early
> during boot.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  hw/ide/piix.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index b9860e35a5..2a380a90e9 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -34,6 +34,7 @@
>  #include "hw/ide/pci.h"
>  #include "trace.h"
>  
> +static bool pci_piix3_xen_ide_unplug_done;

This field belongs to the device state (PCIIDEState or BMDMAState).

>  static uint64_t bmdma_read(void *opaque, hwaddr addr, unsigned size)
>  {
>      BMDMAState *bm = opaque;
> @@ -109,6 +110,8 @@ static void piix_ide_reset(DeviceState *dev)
>      uint8_t *pci_conf = pd->config;
>      int i;
>  
> +    if (pci_piix3_xen_ide_unplug_done == true)
> +        return;
>      for (i = 0; i < 2; i++) {
>          ide_bus_reset(&d->bus[i]);
>      }
> @@ -169,6 +172,7 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
>      IDEBus *idebus;
>      BlockBackend *blk;
>  
> +    pci_piix3_xen_ide_unplug_done = true;
>      pci_ide = PCI_IDE(dev);
>  
>      for (i = aux ? 1 : 0; i < 4; i++) {
> @@ -259,6 +263,7 @@ static const TypeInfo piix4_ide_info = {
>  
>  static void piix_ide_register_types(void)
>  {
> +    pci_piix3_xen_ide_unplug_done = false;
>      type_register_static(&piix3_ide_info);
>      type_register_static(&piix3_ide_xen_info);
>      type_register_static(&piix4_ide_info);
>