[Qemu-devel] [PULL 00/21] Misc patches for 2019-05-15

[Qemu-devel] [PULL 00/21] Misc patches for 2019-05-15

[Paolo Bonzini]

The following changes since commit e329ad2ab72c43b56df88b34954c2c7d839bb373:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20190513' into staging (2019-05-14 10:08:47 +0100)

are available in the git repository at:


  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 88f1090e9020057022ac04531ca87d25f67f57ca:

  hw/net/ne2000: Extract the PCI device from the chipset common code (2019-05-15 11:56:54 +0200)

----------------------------------------------------------------
Mostly bugfixes and cleanups, the most important being
"megasas: fix mapped frame size" from Peter Lieven.
In addition, -realtime is marked as deprecated.

----------------------------------------------------------------
Chen Zhang via Qemu-devel (1):
      hvf: Add missing break statement

Igor Mammedov (1):
      roms: assert if max rom size is less than the used size

Laurent Vivier (5):
      trace: only include trace-event-subdirs when they are needed
      build: replace GENERATED_FILES by generated-files-y
      configure: qemu-ga is only needed with softmmu targets
      build: chardev is only needed for softmmu targets
      build: don't build hardware objects with linux-user

Marc-André Lureau (1):
      vl: fix -sandbox parsing crash when seccomp support is disabled

Paolo Bonzini (2):
      mips-fulong2e: obey -vga none
      sun4m: obey -vga none

Peter Lieven (1):
      megasas: fix mapped frame size

Philippe Mathieu-Daudé (5):
      vl: Add missing descriptions to the VGA adapters list
      hw/acpi/piix4: Move TYPE_PIIX4_PM to a public header
      hw/i386/acpi: Add object_resolve_type_unambiguous to improve modularity
      hw/i386/acpi: Assert a pointer is not null BEFORE using it
      hw/net/ne2000: Extract the PCI device from the chipset common code

Thomas Huth (3):
      hw/input: Add a CONFIG_PS2 switch for the ps2.c file
      Declare -realtime as deprecated
      hw/char: Move multi-serial devices into separate file

Vitaly Kuznetsov (1):
      ioapic: allow buggy guests mishandling level-triggered interrupts to make progress

Wei Yang (1):
      memory: correct the comment to DIRTY_MEMORY_MIGRATION

 Makefile                          |  43 ++++----
 Makefile.objs                     |  22 ++--
 Makefile.target                   |   6 +-
 configure                         |   4 +-
 hw/acpi/piix4.c                   |  13 ---
 hw/char/Kconfig                   |   6 ++
 hw/char/Makefile.objs             |   1 +
 hw/char/serial-pci-multi.c        | 208 ++++++++++++++++++++++++++++++++++++++
 hw/char/serial-pci.c              | 170 -------------------------------
 hw/core/loader.c                  |   1 +
 hw/i386/acpi-build.c              |  22 +++-
 hw/input/Kconfig                  |   5 +
 hw/input/Makefile.objs            |   2 +-
 hw/intc/ioapic.c                  |  57 ++++++++++-
 hw/intc/trace-events              |   1 +
 hw/isa/lpc_ich9.c                 |  11 --
 hw/mips/mips_fulong2e.c           |  10 +-
 hw/net/Kconfig                    |   7 +-
 hw/net/Makefile.objs              |   3 +-
 hw/net/ne2000-pci.c               | 132 ++++++++++++++++++++++++
 hw/net/ne2000.c                   | 105 -------------------
 hw/scsi/megasas.c                 |   2 +-
 hw/sparc/sun4m.c                  |   6 +-
 include/hw/acpi/piix4.h           |   2 +-
 include/hw/i386/ich9.h            |   2 -
 include/hw/i386/ioapic_internal.h |   3 +
 memory.c                          |   4 +-
 qemu-deprecated.texi              |   5 +
 target/i386/hvf/hvf.c             |   1 +
 target/s390x/Makefile.objs        |   2 +-
 tests/Makefile.include            | 116 ++++++++++-----------
 vl.c                              |  22 ++--
 32 files changed, 568 insertions(+), 426 deletions(-)
 create mode 100644 hw/char/serial-pci-multi.c
 create mode 100644 hw/net/ne2000-pci.c
-- 
1.8.3.1

[Qemu-devel] [PULL 01/21] hw/input: Add a CONFIG_PS2 switch for the ps2.c file

[Paolo Bonzini]

From: Thomas Huth <thuth@redhat.com>

ps2.c only needs to be compiled if we are building pckbd.c or pl050.c.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20190411182240.5957-1-thuth@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/input/Kconfig       | 5 +++++
 hw/input/Makefile.objs | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/input/Kconfig b/hw/input/Kconfig
index 889363d..287f088 100644
--- a/hw/input/Kconfig
+++ b/hw/input/Kconfig
@@ -8,10 +8,15 @@ config LM832X
 config PCKBD
     bool
     default y
+    select PS2
     depends on ISA_BUS
 
 config PL050
     bool
+    select PS2
+
+config PS2
+    bool
 
 config STELLARIS_INPUT
     bool
diff --git a/hw/input/Makefile.objs b/hw/input/Makefile.objs
index d1de307..a1bc502 100644
--- a/hw/input/Makefile.objs
+++ b/hw/input/Makefile.objs
@@ -3,7 +3,7 @@ common-obj-y += hid.o
 common-obj-$(CONFIG_LM832X) += lm832x.o
 common-obj-$(CONFIG_PCKBD) += pckbd.o
 common-obj-$(CONFIG_PL050) += pl050.o
-common-obj-y += ps2.o
+common-obj-$(CONFIG_PS2) += ps2.o
 common-obj-$(CONFIG_STELLARIS_INPUT) += stellaris_input.o
 common-obj-$(CONFIG_TSC2005) += tsc2005.o
 
-- 
1.8.3.1

[Qemu-devel] [PULL 02/21] roms: assert if max rom size is less than the used size

[Paolo Bonzini]

From: Igor Mammedov <imammedo@redhat.com>

It would ensure that we would notice attempt to write beyond
the allocated buffer. In case of MemoryRegion backed ROM it's
the host buffer and the guest RAM otherwise.

assert can be triggered with:
  dd if=/dev/zero of=/tmp/blob bs=63k count=1
  qemu-system-x86_64 `for  i in {1..33}; do echo -n " -acpitable /tmp/blob"; done`

Fixes: (a1666142db acpi-build: make ROMs RAM blocks resizeable)

Reported-by: Wei Yang <richardw.yang@linux.intel.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <1554982098-336210-1-git-send-email-imammedo@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/loader.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index fe5cb24..a097bbe 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1025,6 +1025,7 @@ MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
     rom->addr     = addr;
     rom->romsize  = max_len ? max_len : len;
     rom->datasize = len;
+    g_assert(rom->romsize >= rom->datasize);
     rom->data     = g_malloc0(rom->datasize);
     memcpy(rom->data, blob, len);
     rom_insert(rom);
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 02/21] roms: assert if max rom size is less than the used size

[Thomas Huth]

On 15/05/2019 22.50, Paolo Bonzini wrote:
> From: Igor Mammedov <imammedo@redhat.com>
> 
> It would ensure that we would notice attempt to write beyond
> the allocated buffer. In case of MemoryRegion backed ROM it's
> the host buffer and the guest RAM otherwise.
> 
> assert can be triggered with:
>   dd if=/dev/zero of=/tmp/blob bs=63k count=1
>   qemu-system-x86_64 `for  i in {1..33}; do echo -n " -acpitable /tmp/blob"; done`

Sorry, but a user should normally never be able to trigger an assert,
even if passing lots of garbage to qemu via the command line. asserts
are for guarding against programming errors.

Could you please change this into a proper error message instead?

 Thanks,
  Thomas

[Qemu-devel] [PULL 03/21] Declare -realtime as deprecated

[Paolo Bonzini]

From: Thomas Huth <thuth@redhat.com>

The old -realtime mlock=on|off parameter does exactly the same as the
new -overcommit mem-lock=on|off parameter. Additionally, "-realtime"
does not activate any additional "realtime" capabilities as the name
might indicate. We should avoid to confuse the users this way, so
let's deprecate the old -realtime option.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Message-Id: <20190411175345.19414-1-thuth@redhat.com>
---
 qemu-deprecated.texi | 5 +++++
 vl.c                 | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 842e71b..ab62dd7 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -72,6 +72,11 @@ backend settings instead of environment variables.  To ease migration to
 the new format, the ``-audiodev-help'' option can be used to convert
 the current values of the environment variables to ``-audiodev'' options.
 
+@subsection -realtime (since 4.1)
+
+The @code{-realtime mlock=on|off} argument has been replaced by the
+@code{-overcommit mem-lock=on|off} argument.
+
 @section QEMU Machine Protocol (QMP) commands
 
 @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
diff --git a/vl.c b/vl.c
index b670951..2e44f7d 100644
--- a/vl.c
+++ b/vl.c
@@ -3927,6 +3927,8 @@ int main(int argc, char **argv, char **envp)
                 }
                 break;
             case QEMU_OPTION_realtime:
+                warn_report("'-realtime mlock=...' is deprecated, please use "
+                             "'-overcommit mem-lock=...' instead");
                 opts = qemu_opts_parse_noisily(qemu_find_opts("realtime"),
                                                optarg, false);
                 if (!opts) {
-- 
1.8.3.1

[Qemu-devel] [PULL 04/21] vl: Add missing descriptions to the VGA adapters list

[Paolo Bonzini]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Some VGA adapters do not contain an helpful description,
this can be confusing:

  $ qemu-system-arm -M virt -vga help
  none
  std                  standard VGA
  cirrus               Cirrus VGA (default)
  vmware               VMWare SVGA
  xenfb

Add a description to the missing adapters:

  $ qemu-system-arm -M virt -vga help
  none                 no graphic card
  std                  standard VGA
  cirrus               Cirrus VGA (default)
  vmware               VMWare SVGA
  xenfb                Xen paravirtualized framebuffer

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Based-on: <20190412152713.16018-1-marcandre.lureau@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
Message-Id: <20190412163706.3878-1-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/vl.c b/vl.c
index 2e44f7d..f49b119 100644
--- a/vl.c
+++ b/vl.c
@@ -2018,6 +2018,7 @@ typedef struct VGAInterfaceInfo {
 static const VGAInterfaceInfo vga_interfaces[VGA_TYPE_MAX] = {
     [VGA_NONE] = {
         .opt_name = "none",
+        .name = "no graphic card",
     },
     [VGA_STD] = {
         .opt_name = "std",
@@ -2056,6 +2057,7 @@ static const VGAInterfaceInfo vga_interfaces[VGA_TYPE_MAX] = {
     },
     [VGA_XENFB] = {
         .opt_name = "xenfb",
+        .name = "Xen paravirtualized framebuffer",
     },
 };
 
-- 
1.8.3.1

[Qemu-devel] [PULL 05/21] megasas: fix mapped frame size

[Paolo Bonzini]

From: Peter Lieven <pl@kamp.de>

the current value of 1024 bytes (16 * MFI_FRAME_SIZE) we map is not enough to hold
the maximum number of scatter gather elements we advertise. We actually need a
maximum of 2048 bytes. This is 128 max sg elements * 16 bytes (sizeof (union mfi_sgl)).

Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Lieven <pl@kamp.de>
Message-Id: <20190404121015.28634-1-pl@kamp.de>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/megasas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index a56317e..5ad762d 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -477,7 +477,7 @@ static MegasasCmd *megasas_enqueue_frame(MegasasState *s,
 {
     PCIDevice *pcid = PCI_DEVICE(s);
     MegasasCmd *cmd = NULL;
-    int frame_size = MFI_FRAME_SIZE * 16;
+    int frame_size = MEGASAS_MAX_SGE * sizeof(union mfi_sgl);
     hwaddr frame_size_p = frame_size;
     unsigned long index;
 
-- 
1.8.3.1

[Qemu-devel] [PULL 06/21] hvf: Add missing break statement

[Paolo Bonzini]

From: Chen Zhang via Qemu-devel <qemu-devel@nongnu.org>

In target/i386/hvf/hvf.c, a break statement was probably missing in
`hvf_vcpu_exec()`, in handling EXIT_REASON_HLT.

These lines seemed to be equivalent to `kvm_handle_halt()`.

Signed-off-by: Chen Zhang <tgfbeta@me.com>
Message-Id: <087F1D9C-109D-41D1-BE2C-CE5D840C981B@me.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/hvf/hvf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 42f9447..2751c81 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -708,6 +708,7 @@ int hvf_vcpu_exec(CPUState *cpu)
                 !(idtvec_info & VMCS_IDT_VEC_VALID)) {
                 cpu->halted = 1;
                 ret = EXCP_HLT;
+                break;
             }
             ret = EXCP_INTERRUPT;
             break;
-- 
1.8.3.1

[Qemu-devel] [PULL 07/21] vl: fix -sandbox parsing crash when seccomp support is disabled

[Paolo Bonzini]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

$ ./x86_64-softmmu/qemu-system-x86_64 -sandbox off
qemu-system-x86_64: -sandbox off: There is no option group 'sandbox'
Segmentation fault

Commit 5780760f5e ("seccomp: check TSYNC host capability") wrapped one
use of the sandbox option group to produce a sensible error, it didn't
do the same for another call to qemu_opts_parse_noisily():

(gdb) bt
    at util/qemu-option.c:829
 #0  0x00000000105b36d8 in opts_parse (list=0x0, params=0x3ffffffffab5 "off", permit_abbrev=true, defaults=false, errp=0x3ffffffff080)
     at util/qemu-option.c:829
 #1  0x00000000105b3b74 in qemu_opts_parse_noisily (list=<optimized out>, params=<optimized out>, permit_abbrev=<optimized out>) at util/qemu-option.c:890
 #2  0x0000000010024964 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:3589

Fixes: 5780760f5ea6163939a5dabe7427318b4f07d1a2
Cc: david@gibson.dropbear.id.au
Cc: otubo@redhat.com
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20190429134757.13570-1-marcandre.lureau@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/vl.c b/vl.c
index f49b119..c8ca9ff 100644
--- a/vl.c
+++ b/vl.c
@@ -3896,17 +3896,19 @@ int main(int argc, char **argv, char **envp)
                 qtest_log = optarg;
                 break;
             case QEMU_OPTION_sandbox:
-#ifdef CONFIG_SECCOMP
-                opts = qemu_opts_parse_noisily(qemu_find_opts("sandbox"),
-                                               optarg, true);
+                olist = qemu_find_opts("sandbox");
+                if (!olist) {
+#ifndef CONFIG_SECCOMP
+                    error_report("-sandbox support is not enabled "
+                                 "in this QEMU binary");
+#endif
+                    exit(1);
+                }
+
+                opts = qemu_opts_parse_noisily(olist, optarg, true);
                 if (!opts) {
                     exit(1);
                 }
-#else
-                error_report("-sandbox support is not enabled "
-                             "in this QEMU binary");
-                exit(1);
-#endif
                 break;
             case QEMU_OPTION_add_fd:
 #ifndef _WIN32
-- 
1.8.3.1

[Qemu-devel] [PULL 08/21] memory: correct the comment to DIRTY_MEMORY_MIGRATION

[Paolo Bonzini]

From: Wei Yang <richardw.yang@linux.intel.com>

The dirty bit is DIRTY_MEMORY_MIGRATION. Correct the comment.

Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
Message-Id: <20190426020927.25470-1-richardw.yang@linux.intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/memory.c b/memory.c
index bb2b71e..3071c4b 100644
--- a/memory.c
+++ b/memory.c
@@ -2584,7 +2584,7 @@ void memory_global_dirty_log_start(void)
 
     MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward);
 
-    /* Refresh DIRTY_LOG_MIGRATION bit.  */
+    /* Refresh DIRTY_MEMORY_MIGRATION bit.  */
     memory_region_transaction_begin();
     memory_region_update_pending = true;
     memory_region_transaction_commit();
@@ -2594,7 +2594,7 @@ static void memory_global_dirty_log_do_stop(void)
 {
     global_dirty_log = false;
 
-    /* Refresh DIRTY_LOG_MIGRATION bit.  */
+    /* Refresh DIRTY_MEMORY_MIGRATION bit.  */
     memory_region_transaction_begin();
     memory_region_update_pending = true;
     memory_region_transaction_commit();
-- 
1.8.3.1

[Qemu-devel] [PULL 09/21] hw/acpi/piix4: Move TYPE_PIIX4_PM to a public header

[Paolo Bonzini]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Move the TYPE_PIIX4_PM definition to the corresponding header,
so other files can use it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190427144025.22880-2-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/acpi/piix4.c         | 2 --
 include/hw/acpi/piix4.h | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 9c079d6..160e730 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -92,8 +92,6 @@ typedef struct PIIX4PMState {
     MemHotplugState acpi_memory_hotplug;
 } PIIX4PMState;
 
-#define TYPE_PIIX4_PM "PIIX4_PM"
-
 #define PIIX4_PM(obj) \
     OBJECT_CHECK(PIIX4PMState, (obj), TYPE_PIIX4_PM)
 
diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h
index 26c2370..57d7e1c 100644
--- a/include/hw/acpi/piix4.h
+++ b/include/hw/acpi/piix4.h
@@ -1,6 +1,8 @@
 #ifndef HW_ACPI_PIIX4_H
 #define HW_ACPI_PIIX4_H
 
+#define TYPE_PIIX4_PM "PIIX4_PM"
+
 Object *piix4_pm_find(void);
 
 #endif
-- 
1.8.3.1

[Qemu-devel] [PULL 10/21] hw/i386/acpi: Add object_resolve_type_unambiguous to improve modularity

[Paolo Bonzini]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

When building with CONFIG_Q35=n, we get:

    LINK    x86_64-softmmu/qemu-system-x86_64
  /usr/bin/ld: hw/i386/acpi-build.o: in function `acpi_get_misc_info':
  /source/qemu/hw/i386/acpi-build.c:243: undefined reference to `ich9_lpc_find'
  collect2: error: ld returned 1 exit status
  make[1]: *** [Makefile:204: qemu-system-x86_64] Error 1

This is due to a dependency in acpi-build.c on the ICH9_LPC
(via ich9_lpc_find) and PIIX4_PM (via piix4_pm_find) devices.

To allow better modularity (compile acpi-build.c with only
Q35/ICH9 or ISAPC/PIIX4), refactor the similar helper as
object_resolve_type_unambiguous(). This way we relax the
linker dependencies and can build the x86 targets with a
selection of machines (instead of all of them).

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190427144025.22880-3-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/acpi/piix4.c         | 11 -----------
 hw/i386/acpi-build.c    | 20 ++++++++++++++++----
 hw/isa/lpc_ich9.c       | 11 -----------
 include/hw/acpi/piix4.h |  2 --
 include/hw/i386/ich9.h  |  2 --
 5 files changed, 16 insertions(+), 30 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 160e730..c903e65 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -552,17 +552,6 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp)
     piix4_pm_add_propeties(s);
 }
 
-Object *piix4_pm_find(void)
-{
-    bool ambig;
-    Object *o = object_resolve_path_type("", TYPE_PIIX4_PM, &ambig);
-
-    if (ambig || !o) {
-        return NULL;
-    }
-    return o;
-}
-
 I2CBus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
                       qemu_irq sci_irq, qemu_irq smi_irq,
                       int smm_enabled, DeviceState **piix4_pm)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 416da31..123ff2b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -35,6 +35,7 @@
 #include "hw/acpi/acpi-defs.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/cpu.h"
+#include "hw/acpi/piix4.h"
 #include "hw/nvram/fw_cfg.h"
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/loader.h"
@@ -164,10 +165,21 @@ static void init_common_fadt_data(Object *o, AcpiFadtData *data)
     *data = fadt;
 }
 
+static Object *object_resolve_type_unambiguous(const char *typename)
+{
+    bool ambig;
+    Object *o = object_resolve_path_type("", typename, &ambig);
+
+    if (ambig || !o) {
+        return NULL;
+    }
+    return o;
+}
+
 static void acpi_get_pm_info(AcpiPmInfo *pm)
 {
-    Object *piix = piix4_pm_find();
-    Object *lpc = ich9_lpc_find();
+    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM);
+    Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE);
     Object *obj = piix ? piix : lpc;
     QObject *o;
     pm->cpu_hp_io_base = 0;
@@ -228,8 +240,8 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
 
 static void acpi_get_misc_info(AcpiMiscInfo *info)
 {
-    Object *piix = piix4_pm_find();
-    Object *lpc = ich9_lpc_find();
+    Object *piix = object_resolve_type_unambiguous(TYPE_PIIX4_PM);
+    Object *lpc = object_resolve_type_unambiguous(TYPE_ICH9_LPC_DEVICE);
     assert(!!piix != !!lpc);
 
     if (piix) {
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index ac44aa5..031ee9c 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -624,17 +624,6 @@ static const MemoryRegionOps ich9_rst_cnt_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN
 };
 
-Object *ich9_lpc_find(void)
-{
-    bool ambig;
-    Object *o = object_resolve_path_type("", TYPE_ICH9_LPC_DEVICE, &ambig);
-
-    if (ambig) {
-        return NULL;
-    }
-    return o;
-}
-
 static void ich9_lpc_get_sci_int(Object *obj, Visitor *v, const char *name,
                                  void *opaque, Error **errp)
 {
diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h
index 57d7e1c..028bb53 100644
--- a/include/hw/acpi/piix4.h
+++ b/include/hw/acpi/piix4.h
@@ -3,6 +3,4 @@
 
 #define TYPE_PIIX4_PM "PIIX4_PM"
 
-Object *piix4_pm_find(void);
-
 #endif
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 673d13d..046bcf3 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -81,8 +81,6 @@ typedef struct ICH9LPCState {
     qemu_irq gsi[GSI_NUM_PINS];
 } ICH9LPCState;
 
-Object *ich9_lpc_find(void);
-
 #define Q35_MASK(bit, ms_bit, ls_bit) \
 ((uint##bit##_t)(((1ULL << ((ms_bit) + 1)) - 1) & ~((1ULL << ls_bit) - 1)))
 
-- 
1.8.3.1

[Qemu-devel] [PULL 11/21] hw/i386/acpi: Assert a pointer is not null BEFORE using it

[Paolo Bonzini]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Commit 72c194f7e75c added a non-null check on the 'obj' pointer.
Later, commit 500b11ea5095 added code which uses the 'obj'
pointer _before_ the assertion check. Move the assertion
_before_ the pointer use.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190427144025.22880-4-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/acpi-build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 123ff2b..b4ec14e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -186,6 +186,7 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
     pm->pcihp_io_base = 0;
     pm->pcihp_io_len = 0;
 
+    assert(obj);
     init_common_fadt_data(obj, &pm->fadt);
     if (piix) {
         /* w2k requires FADT(rev1) or it won't boot, keep PC compatible */
@@ -204,7 +205,6 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
         pm->fadt.flags |= 1 << ACPI_FADT_F_RESET_REG_SUP;
         pm->cpu_hp_io_base = ICH9_CPU_HOTPLUG_IO_BASE;
     }
-    assert(obj);
 
     /* The above need not be conditional on machine type because the reset port
      * happens to be the same on PIIX (pc) and ICH9 (q35). */
-- 
1.8.3.1

[Qemu-devel] [PULL 12/21] mips-fulong2e: obey -vga none

[Paolo Bonzini]

Do not create an ATI VGA if "-vga none" was passed on the command line.

Cc: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mips/mips_fulong2e.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 9d7480e..05a5a82 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -349,10 +349,12 @@ static void mips_fulong2e_init(MachineState *machine)
                                &smbus, &isa_bus);
 
     /* GPU */
-    dev = DEVICE(pci_create(pci_bus, -1, "ati-vga"));
-    qdev_prop_set_uint32(dev, "vgamem_mb", 16);
-    qdev_prop_set_uint16(dev, "x-device-id", 0x5159);
-    qdev_init_nofail(dev);
+    if (vga_interface_type != VGA_NONE) {
+        dev = DEVICE(pci_create(pci_bus, -1, "ati-vga"));
+        qdev_prop_set_uint32(dev, "vgamem_mb", 16);
+        qdev_prop_set_uint16(dev, "x-device-id", 0x5159);
+        qdev_init_nofail(dev);
+    }
 
     /* Populate SPD eeprom data */
     spd_data = spd_data_generate(DDR, ram_size, &err);
-- 
1.8.3.1

[Qemu-devel] [PULL 13/21] sun4m: obey -vga none

[Paolo Bonzini]

Do not create a TCX if "-vga none" was passed on the command line.
Remove some dead code along the way to avoid big reindentation.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/sparc/sun4m.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index ca1e382..07d126a 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -850,7 +850,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     uint32_t initrd_size;
     DriveInfo *fd[MAX_FD];
     FWCfgState *fw_cfg;
-    unsigned int num_vsimms;
     DeviceState *dev;
     SysBusDevice *s;
 
@@ -909,8 +908,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
         error_report("Unsupported depth: %d", graphic_depth);
         exit (1);
     }
-    num_vsimms = 0;
-    if (num_vsimms == 0) {
+    if (vga_interface_type != VGA_NONE) {
         if (vga_interface_type == VGA_CG3) {
             if (graphic_depth != 8) {
                 error_report("Unsupported depth: %d", graphic_depth);
@@ -945,7 +943,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
         }
     }
 
-    for (i = num_vsimms; i < MAX_VSIMMS; i++) {
+    for (i = 0; i < MAX_VSIMMS; i++) {
         /* vsimm registers probed by OBP */
         if (hwdef->vsimm[i].reg_base) {
             empty_slot_init(hwdef->vsimm[i].reg_base, 0x2000);
-- 
1.8.3.1

[Qemu-devel] [PULL 14/21] trace: only include trace-event-subdirs when they are needed

[Paolo Bonzini]

From: Laurent Vivier <lvivier@redhat.com>

Some directories are built only for softmmu targets,
and the related trace-event-subdirs must do the same

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20190401141222.30034-2-lvivier@redhat.com>
---
 Makefile.objs | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index cf065de..dda5bbc 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -127,11 +127,12 @@ rdmacm-mux-obj-y = contrib/rdmacm-mux/
 trace-events-subdirs =
 trace-events-subdirs += accel/kvm
 trace-events-subdirs += accel/tcg
-trace-events-subdirs += audio
 trace-events-subdirs += authz
 trace-events-subdirs += block
 trace-events-subdirs += chardev
 trace-events-subdirs += crypto
+ifeq ($(CONFIG_SOFTMMU),y)
+trace-events-subdirs += audio
 trace-events-subdirs += hw/9pfs
 trace-events-subdirs += hw/acpi
 trace-events-subdirs += hw/alpha
@@ -140,7 +141,6 @@ trace-events-subdirs += hw/audio
 trace-events-subdirs += hw/block
 trace-events-subdirs += hw/block/dataplane
 trace-events-subdirs += hw/char
-trace-events-subdirs += hw/display
 trace-events-subdirs += hw/dma
 trace-events-subdirs += hw/hppa
 trace-events-subdirs += hw/i2c
@@ -173,11 +173,14 @@ trace-events-subdirs += hw/virtio
 trace-events-subdirs += hw/watchdog
 trace-events-subdirs += hw/xen
 trace-events-subdirs += hw/gpio
+trace-events-subdirs += migration
+trace-events-subdirs += net
+trace-events-subdirs += ui
+endif
+trace-events-subdirs += hw/display
 trace-events-subdirs += io
 trace-events-subdirs += linux-user
-trace-events-subdirs += migration
 trace-events-subdirs += nbd
-trace-events-subdirs += net
 trace-events-subdirs += qapi
 trace-events-subdirs += qom
 trace-events-subdirs += scsi
@@ -189,7 +192,6 @@ trace-events-subdirs += target/ppc
 trace-events-subdirs += target/riscv
 trace-events-subdirs += target/s390x
 trace-events-subdirs += target/sparc
-trace-events-subdirs += ui
 trace-events-subdirs += util
 
 trace-events-files = $(SRC_PATH)/trace-events $(trace-events-subdirs:%=$(SRC_PATH)/%/trace-events)
-- 
1.8.3.1

[Qemu-devel] [PULL 15/21] build: replace GENERATED_FILES by generated-files-y

[Paolo Bonzini]

From: Laurent Vivier <lvivier@redhat.com>

When possible use generated-files-$(FLAG) to disable
some targets (like KEYCODEMAP_FILES).

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Message-Id: <20190401141222.30034-3-lvivier@redhat.com>
---
 Makefile                   | 36 +++++++++++++++++-------------------
 Makefile.target            |  6 +++---
 target/s390x/Makefile.objs |  2 +-
 tests/Makefile.include     | 26 +++++++++++++-------------
 4 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/Makefile b/Makefile
index 66d5c65..d9a3040 100644
--- a/Makefile
+++ b/Makefile
@@ -101,7 +101,7 @@ QEMU_PKGVERSION := $(if $(PKGVERSION),$(PKGVERSION),$(shell \
 # Either "version (pkgversion)", or just "version" if pkgversion not set
 FULL_VERSION := $(if $(QEMU_PKGVERSION),$(VERSION) ($(QEMU_PKGVERSION)),$(VERSION))
 
-GENERATED_FILES = qemu-version.h config-host.h qemu-options.def
+generated-files-y = qemu-version.h config-host.h qemu-options.def
 
 GENERATED_QAPI_FILES = qapi/qapi-builtin-types.h qapi/qapi-builtin-types.c
 GENERATED_QAPI_FILES += qapi/qapi-types.h qapi/qapi-types.c
@@ -121,20 +121,18 @@ GENERATED_QAPI_FILES += $(QAPI_MODULES:%=qapi/qapi-events-%.c)
 GENERATED_QAPI_FILES += qapi/qapi-introspect.c qapi/qapi-introspect.h
 GENERATED_QAPI_FILES += qapi/qapi-doc.texi
 
-GENERATED_FILES += $(GENERATED_QAPI_FILES)
+generated-files-y += $(GENERATED_QAPI_FILES)
 
-GENERATED_FILES += trace/generated-tcg-tracers.h
+generated-files-y += trace/generated-tcg-tracers.h
 
-GENERATED_FILES += trace/generated-helpers-wrappers.h
-GENERATED_FILES += trace/generated-helpers.h
-GENERATED_FILES += trace/generated-helpers.c
+generated-files-y += trace/generated-helpers-wrappers.h
+generated-files-y += trace/generated-helpers.h
+generated-files-y += trace/generated-helpers.c
 
-ifdef CONFIG_TRACE_UST
-GENERATED_FILES += trace-ust-all.h
-GENERATED_FILES += trace-ust-all.c
-endif
+generated-files-$(CONFIG_TRACE_UST) += trace-ust-all.h
+generated-files-$(CONFIG_TRACE_UST) += trace-ust-all.c
 
-GENERATED_FILES += module_block.h
+generated-files-y += module_block.h
 
 TRACE_HEADERS = trace-root.h $(trace-events-subdirs:%=%/trace.h)
 TRACE_SOURCES = trace-root.c $(trace-events-subdirs:%=%/trace.c)
@@ -147,10 +145,10 @@ ifdef CONFIG_TRACE_UST
 TRACE_HEADERS += trace-ust-root.h $(trace-events-subdirs:%=%/trace-ust.h)
 endif
 
-GENERATED_FILES += $(TRACE_HEADERS)
-GENERATED_FILES += $(TRACE_SOURCES)
-GENERATED_FILES += $(BUILD_DIR)/trace-events-all
-GENERATED_FILES += .git-submodule-status
+generated-files-y += $(TRACE_HEADERS)
+generated-files-y += $(TRACE_SOURCES)
+generated-files-y += $(BUILD_DIR)/trace-events-all
+generated-files-y += .git-submodule-status
 
 trace-group-name = $(shell dirname $1 | sed -e 's/[^a-zA-Z0-9]/_/g')
 
@@ -281,7 +279,7 @@ KEYCODEMAP_FILES = \
 		 ui/input-keymap-osx-to-qcode.c \
 		 $(NULL)
 
-GENERATED_FILES += $(KEYCODEMAP_FILES)
+generated-files-$(CONFIG_SOFTMMU) += $(KEYCODEMAP_FILES)
 
 ui/input-keymap-%.c: $(KEYCODEMAP_GEN) $(KEYCODEMAP_CSV) $(SRC_PATH)/ui/Makefile.objs
 	$(call quiet-command,\
@@ -643,10 +641,10 @@ clean:
 	rm -f fsdev/*.pod scsi/*.pod
 	rm -f qemu-img-cmds.h
 	rm -f ui/shader/*-vert.h ui/shader/*-frag.h
-	@# May not be present in GENERATED_FILES
+	@# May not be present in generated-files-y
 	rm -f trace/generated-tracers-dtrace.dtrace*
 	rm -f trace/generated-tracers-dtrace.h*
-	rm -f $(foreach f,$(GENERATED_FILES),$(f) $(f)-timestamp)
+	rm -f $(foreach f,$(generated-files-y),$(f) $(f)-timestamp)
 	rm -f qapi-gen-timestamp
 	rm -rf qga/qapi-generated
 	for d in $(ALL_SUBDIRS); do \
@@ -1062,7 +1060,7 @@ endif # CONFIG_WIN
 # rebuilt before other object files
 ifneq ($(wildcard config-host.mak),)
 ifneq ($(filter-out $(UNCHECKED_GOALS),$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fail))
-Makefile: $(GENERATED_FILES)
+Makefile: $(generated-files-y)
 endif
 endif
 
diff --git a/Makefile.target b/Makefile.target
index ae02495..fdbe7c8 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -166,7 +166,7 @@ else
 obj-y += hw/$(TARGET_BASE_ARCH)/
 endif
 
-GENERATED_FILES += hmp-commands.h hmp-commands-info.h
+generated-files-y += hmp-commands.h hmp-commands-info.h
 
 endif # CONFIG_SOFTMMU
 
@@ -236,5 +236,5 @@ ifdef CONFIG_TRACE_SYSTEMTAP
 	$(INSTALL_DATA) $(QEMU_PROG)-log.stp "$(DESTDIR)$(qemu_datadir)/../systemtap/tapset/$(QEMU_PROG)-log.stp"
 endif
 
-GENERATED_FILES += config-target.h
-Makefile: $(GENERATED_FILES)
+generated-files-y += config-target.h
+Makefile: $(generated-files-y)
diff --git a/target/s390x/Makefile.objs b/target/s390x/Makefile.objs
index 68eeee3..312bf4f 100644
--- a/target/s390x/Makefile.objs
+++ b/target/s390x/Makefile.objs
@@ -12,7 +12,7 @@ obj-$(call lnot,$(CONFIG_TCG)) += tcg-stub.o
 feat-src = $(SRC_PATH)/target/$(TARGET_BASE_ARCH)/
 feat-dst = $(BUILD_DIR)/$(TARGET_DIR)
 ifneq ($(MAKECMDGOALS),clean)
-GENERATED_FILES += $(feat-dst)gen-features.h
+generated-files-y += $(feat-dst)gen-features.h
 endif
 
 $(feat-dst)gen-features.h: $(feat-dst)gen-features.h-timestamp
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 60de085..ad95a14 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -468,19 +468,19 @@ qapi-schema += unknown-expr-key.json
 
 check-qapi-schema-y := $(addprefix tests/qapi-schema/, $(qapi-schema))
 
-GENERATED_FILES += tests/test-qapi-types.h \
-	tests/include/test-qapi-types-sub-module.h \
-	tests/test-qapi-types-sub-sub-module.h \
-	tests/test-qapi-visit.h \
-	tests/include/test-qapi-visit-sub-module.h \
-	tests/test-qapi-visit-sub-sub-module.h \
-	tests/test-qapi-commands.h \
-	tests/include/test-qapi-commands-sub-module.h \
-	tests/test-qapi-commands-sub-sub-module.h \
-	tests/test-qapi-events.h \
-	tests/include/test-qapi-events-sub-module.h \
-	tests/test-qapi-events-sub-sub-module.h \
-	tests/test-qapi-introspect.h
+generated-files-y += tests/test-qapi-types.h
+generated-files-y += tests/include/test-qapi-types-sub-module.h
+generated-files-y += tests/test-qapi-types-sub-sub-module.h
+generated-files-y += tests/test-qapi-visit.h
+generated-files-y += tests/include/test-qapi-visit-sub-module.h
+generated-files-y += tests/test-qapi-visit-sub-sub-module.h
+generated-files-y += tests/test-qapi-commands.h
+generated-files-y += tests/include/test-qapi-commands-sub-module.h
+generated-files-y += tests/test-qapi-commands-sub-sub-module.h
+generated-files-y += tests/test-qapi-events.h
+generated-files-y += tests/include/test-qapi-events-sub-module.h
+generated-files-y += tests/test-qapi-events-sub-sub-module.h
+generated-files-y += tests/test-qapi-introspect.h
 
 QEMU_CFLAGS += -I$(SRC_PATH)/tests
 
-- 
1.8.3.1

[Qemu-devel] [PULL 16/21] configure: qemu-ga is only needed with softmmu targets

[Paolo Bonzini]

From: Laurent Vivier <lvivier@redhat.com>

Remove it from the list of tools if --disable-system
and --disable-tools are used as we don't need it for
linux-user targets.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
[lv: I also disable it with disable-tools, not only with disable-system]
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Message-Id: <20190401141222.30034-4-lvivier@redhat.com>
---
 configure | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 8999698..d2fc346 100755
--- a/configure
+++ b/configure
@@ -6079,7 +6079,9 @@ fi
 # Probe for guest agent support/options
 
 if [ "$guest_agent" != "no" ]; then
-  if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then
+  if [ "$softmmu" = no -a "$want_tools" = no ] ; then
+      guest_agent=no
+  elif [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then
       tools="qemu-ga $tools"
       guest_agent=yes
   elif [ "$guest_agent" != yes ]; then
-- 
1.8.3.1

[Qemu-devel] [PULL 17/21] build: chardev is only needed for softmmu targets

[Paolo Bonzini]

From: Laurent Vivier <lvivier@redhat.com>

Move the dependency from SUBDIR_RULES to SOFTMMU_SUBDIR_RULES

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Message-Id: <20190401141222.30034-5-lvivier@redhat.com>
---
 Makefile      | 3 ++-
 Makefile.objs | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index d9a3040..1851f8c 100644
--- a/Makefile
+++ b/Makefile
@@ -444,6 +444,7 @@ SOFTMMU_SUBDIR_RULES=$(filter %-softmmu,$(SUBDIR_RULES))
 
 $(SOFTMMU_SUBDIR_RULES): $(authz-obj-y)
 $(SOFTMMU_SUBDIR_RULES): $(block-obj-y)
+$(SOFTMMU_SUBDIR_RULES): $(chardev-obj-y)
 $(SOFTMMU_SUBDIR_RULES): $(crypto-obj-y)
 $(SOFTMMU_SUBDIR_RULES): $(io-obj-y)
 $(SOFTMMU_SUBDIR_RULES): config-all-devices.mak
@@ -480,7 +481,7 @@ subdir-capstone: .git-submodule-status
 subdir-slirp: .git-submodule-status
 	$(call quiet-command,$(MAKE) -C $(SRC_PATH)/slirp BUILD_DIR="$(BUILD_DIR)/slirp" CC="$(CC)" AR="$(AR)" LD="$(LD)" RANLIB="$(RANLIB)" CFLAGS="$(QEMU_CFLAGS) $(CFLAGS)" LDFLAGS="$(LDFLAGS)")
 
-$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) \
+$(SUBDIR_RULES): libqemuutil.a $(common-obj-y) \
 	$(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))
 
 ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
diff --git a/Makefile.objs b/Makefile.objs
index dda5bbc..43c9e45 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -129,9 +129,9 @@ trace-events-subdirs += accel/kvm
 trace-events-subdirs += accel/tcg
 trace-events-subdirs += authz
 trace-events-subdirs += block
-trace-events-subdirs += chardev
 trace-events-subdirs += crypto
 ifeq ($(CONFIG_SOFTMMU),y)
+trace-events-subdirs += chardev
 trace-events-subdirs += audio
 trace-events-subdirs += hw/9pfs
 trace-events-subdirs += hw/acpi
-- 
1.8.3.1

[Qemu-devel] [PULL 18/21] build: don't build hardware objects with linux-user

[Paolo Bonzini]

From: Laurent Vivier <lvivier@redhat.com>

Some objects are only needed for system emulation and tools.
We can ignore them for the user mode case

Update tests to run accordingly: conditionally build some tests
on CONFIG_BLOCK.

Some tests use components that are only built when softmmu or
block tools are enabled, not for linux-user. So, if these components
are not available, disable the tests.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20190401141222.30034-6-lvivier@redhat.com>
---
 Makefile               |  4 +++
 Makefile.objs          | 14 +++++---
 tests/Makefile.include | 90 +++++++++++++++++++++++++-------------------------
 3 files changed, 58 insertions(+), 50 deletions(-)

diff --git a/Makefile b/Makefile
index 1851f8c..155f066 100644
--- a/Makefile
+++ b/Makefile
@@ -87,6 +87,10 @@ endif
 
 include $(SRC_PATH)/rules.mak
 
+# notempy and lor are defined in rules.mak
+CONFIG_TOOLS := $(call notempty,$(TOOLS))
+CONFIG_BLOCK := $(call lor,$(CONFIG_SOFTMMU),$(CONFIG_TOOLS))
+
 # Create QEMU_PKGVERSION and FULL_VERSION strings
 # If PKGVERSION is set, use that; otherwise get version and -dirty status from git
 QEMU_PKGVERSION := $(if $(PKGVERSION),$(PKGVERSION),$(shell \
diff --git a/Makefile.objs b/Makefile.objs
index 43c9e45..2b0793e 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -127,9 +127,17 @@ rdmacm-mux-obj-y = contrib/rdmacm-mux/
 trace-events-subdirs =
 trace-events-subdirs += accel/kvm
 trace-events-subdirs += accel/tcg
+trace-events-subdirs += crypto
+ifeq ($(CONFIG_USER_ONLY),y)
+trace-events-subdirs += linux-user
+endif
+ifeq ($(CONFIG_BLOCK),y)
 trace-events-subdirs += authz
 trace-events-subdirs += block
-trace-events-subdirs += crypto
+trace-events-subdirs += io
+trace-events-subdirs += nbd
+trace-events-subdirs += scsi
+endif
 ifeq ($(CONFIG_SOFTMMU),y)
 trace-events-subdirs += chardev
 trace-events-subdirs += audio
@@ -178,12 +186,8 @@ trace-events-subdirs += net
 trace-events-subdirs += ui
 endif
 trace-events-subdirs += hw/display
-trace-events-subdirs += io
-trace-events-subdirs += linux-user
-trace-events-subdirs += nbd
 trace-events-subdirs += qapi
 trace-events-subdirs += qom
-trace-events-subdirs += scsi
 trace-events-subdirs += target/arm
 trace-events-subdirs += target/hppa
 trace-events-subdirs += target/i386
diff --git a/tests/Makefile.include b/tests/Makefile.include
index ad95a14..1865f6b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -45,7 +45,7 @@ SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \
 
 check-unit-y += tests/check-qdict$(EXESUF)
 check-unit-y += tests/check-block-qdict$(EXESUF)
-check-unit-y += tests/test-char$(EXESUF)
+check-unit-$(CONFIG_SOFTMMU) += tests/test-char$(EXESUF)
 check-unit-y += tests/check-qnum$(EXESUF)
 check-unit-y += tests/check-qstring$(EXESUF)
 check-unit-y += tests/check-qlist$(EXESUF)
@@ -61,21 +61,21 @@ check-unit-y += tests/test-string-input-visitor$(EXESUF)
 check-unit-y += tests/test-string-output-visitor$(EXESUF)
 check-unit-y += tests/test-qmp-event$(EXESUF)
 check-unit-y += tests/test-opts-visitor$(EXESUF)
-check-unit-y += tests/test-coroutine$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-coroutine$(EXESUF)
 check-unit-y += tests/test-visitor-serialization$(EXESUF)
 check-unit-y += tests/test-iov$(EXESUF)
-check-unit-y += tests/test-aio$(EXESUF)
-check-unit-y += tests/test-aio-multithread$(EXESUF)
-check-unit-y += tests/test-throttle$(EXESUF)
-check-unit-y += tests/test-thread-pool$(EXESUF)
-check-unit-y += tests/test-hbitmap$(EXESUF)
-check-unit-y += tests/test-bdrv-drain$(EXESUF)
-check-unit-y += tests/test-bdrv-graph-mod$(EXESUF)
-check-unit-y += tests/test-blockjob$(EXESUF)
-check-unit-y += tests/test-blockjob-txn$(EXESUF)
-check-unit-y += tests/test-block-backend$(EXESUF)
-check-unit-y += tests/test-block-iothread$(EXESUF)
-check-unit-y += tests/test-image-locking$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-aio$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-aio-multithread$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-throttle$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-thread-pool$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-hbitmap$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-bdrv-drain$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-bdrv-graph-mod$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-blockjob$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-blockjob-txn$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-block-backend$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-block-iothread$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-image-locking$(EXESUF)
 check-unit-y += tests/test-x86-cpuid$(EXESUF)
 # all code tested by test-x86-cpuid is inside topology.h
 ifeq ($(CONFIG_SOFTMMU),y)
@@ -101,40 +101,40 @@ check-unit-y += tests/check-qom-interface$(EXESUF)
 check-unit-y += tests/check-qom-proplist$(EXESUF)
 check-unit-y += tests/test-qemu-opts$(EXESUF)
 check-unit-y += tests/test-keyval$(EXESUF)
-check-unit-y += tests/test-write-threshold$(EXESUF)
-check-unit-y += tests/test-crypto-hash$(EXESUF)
-check-speed-y += tests/benchmark-crypto-hash$(EXESUF)
-check-unit-y += tests/test-crypto-hmac$(EXESUF)
-check-speed-y += tests/benchmark-crypto-hmac$(EXESUF)
-check-unit-y += tests/test-crypto-cipher$(EXESUF)
-check-speed-y += tests/benchmark-crypto-cipher$(EXESUF)
-check-unit-y += tests/test-crypto-secret$(EXESUF)
-check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF)
-check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlssession$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-write-threshold$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-crypto-hash$(EXESUF)
+check-speed-$(CONFIG_BLOCK) += tests/benchmark-crypto-hash$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-crypto-hmac$(EXESUF)
+check-speed-$(CONFIG_BLOCK) += tests/benchmark-crypto-hmac$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-crypto-cipher$(EXESUF)
+check-speed-$(CONFIG_BLOCK) += tests/benchmark-crypto-cipher$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-crypto-secret$(EXESUF)
+check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_GNUTLS)) += tests/test-crypto-tlscredsx509$(EXESUF)
+check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_GNUTLS)) += tests/test-crypto-tlssession$(EXESUF)
 ifneq (,$(findstring qemu-ga,$(TOOLS)))
 check-unit-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_SERIAL)) += tests/test-qga$(EXESUF)
 endif
 check-unit-y += tests/test-timed-average$(EXESUF)
 check-unit-$(CONFIG_INOTIFY1) += tests/test-util-filemonitor$(EXESUF)
 check-unit-y += tests/test-util-sockets$(EXESUF)
-check-unit-y += tests/test-authz-simple$(EXESUF)
-check-unit-y += tests/test-authz-list$(EXESUF)
-check-unit-y += tests/test-authz-listfile$(EXESUF)
-check-unit-$(CONFIG_AUTH_PAM) += tests/test-authz-pam$(EXESUF)
-check-unit-y += tests/test-io-task$(EXESUF)
-check-unit-y += tests/test-io-channel-socket$(EXESUF)
-check-unit-y += tests/test-io-channel-file$(EXESUF)
-check-unit-$(CONFIG_GNUTLS) += tests/test-io-channel-tls$(EXESUF)
-check-unit-y += tests/test-io-channel-command$(EXESUF)
-check-unit-y += tests/test-io-channel-buffer$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-authz-simple$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-authz-list$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-authz-listfile$(EXESUF)
+check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_AUTH_PAM)) += tests/test-authz-pam$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-io-task$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-io-channel-socket$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-io-channel-file$(EXESUF)
+check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_GNUTLS)) += tests/test-io-channel-tls$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-io-channel-command$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-io-channel-buffer$(EXESUF)
 check-unit-y += tests/test-base64$(EXESUF)
-check-unit-$(if $(CONFIG_NETTLE),y,$(CONFIG_GCRYPT)) += tests/test-crypto-pbkdf$(EXESUF)
-check-unit-y += tests/test-crypto-ivgen$(EXESUF)
-check-unit-y += tests/test-crypto-afsplit$(EXESUF)
-check-unit-y += tests/test-crypto-xts$(EXESUF)
-check-unit-y += tests/test-crypto-block$(EXESUF)
+check-unit-$(call land,$(CONFIG_BLOCK),$(if $(CONFIG_NETTLE),y,$(CONFIG_GCRYPT))) += tests/test-crypto-pbkdf$(EXESUF)
+check-unit-$(CONFIG_BLOCK) += tests/test-crypto-ivgen$(EXESUF)
+check-unit-$(CONFIG_BLOCK)  += tests/test-crypto-afsplit$(EXESUF)
+check-unit-$(CONFIG_BLOCK)  += tests/test-crypto-xts$(EXESUF)
+check-unit-$(CONFIG_BLOCK)  += tests/test-crypto-block$(EXESUF)
 check-unit-y += tests/test-logging$(EXESUF)
-check-unit-$(CONFIG_REPLICATION) += tests/test-replication$(EXESUF)
+check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_REPLICATION)) += tests/test-replication$(EXESUF)
 check-unit-y += tests/test-bufferiszero$(EXESUF)
 check-unit-y += tests/test-uuid$(EXESUF)
 check-unit-y += tests/ptimer-test$(EXESUF)
@@ -496,11 +496,11 @@ test-qapi-obj-y = tests/test-qapi-types.o \
 	tests/test-qapi-visit-sub-sub-module.o \
 	tests/test-qapi-introspect.o \
 	$(test-qom-obj-y)
-benchmark-crypto-obj-y = $(authz-obj-y) $(crypto-obj-y) $(test-qom-obj-y)
-test-crypto-obj-y = $(authz-obj-y) $(crypto-obj-y) $(test-qom-obj-y)
-test-io-obj-y = $(io-obj-y) $(test-crypto-obj-y)
-test-authz-obj-y = $(test-qom-obj-y) $(authz-obj-y)
-test-block-obj-y = $(block-obj-y) $(test-io-obj-y) tests/iothread.o
+benchmark-crypto-obj-$(CONFIG_BLOCK) = $(authz-obj-y) $(crypto-obj-y) $(test-qom-obj-y)
+test-crypto-obj-$(CONFIG_BLOCK) = $(authz-obj-y) $(crypto-obj-y) $(test-qom-obj-y)
+test-io-obj-$(CONFIG_BLOCK) = $(io-obj-y) $(test-crypto-obj-y)
+test-authz-obj-$(CONFIG_BLOCK) = $(test-qom-obj-y) $(authz-obj-y)
+test-block-obj-$(CONFIG_BLOCK) = $(block-obj-y) $(test-io-obj-y) tests/iothread.o
 
 tests/check-qnum$(EXESUF): tests/check-qnum.o $(test-util-obj-y)
 tests/check-qstring$(EXESUF): tests/check-qstring.o $(test-util-obj-y)
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 18/21] build: don't build hardware objects with linux-user

[Daniel P. Berrangé]

On Wed, May 15, 2019 at 10:50:30PM +0200, Paolo Bonzini wrote:
> From: Laurent Vivier <lvivier@redhat.com>
> 
> Some objects are only needed for system emulation and tools.
> We can ignore them for the user mode case
> 
> Update tests to run accordingly: conditionally build some tests
> on CONFIG_BLOCK.
> 
> Some tests use components that are only built when softmmu or
> block tools are enabled, not for linux-user. So, if these components
> are not available, disable the tests.
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> Message-Id: <20190401141222.30034-6-lvivier@redhat.com>
> ---
>  Makefile               |  4 +++
>  Makefile.objs          | 14 +++++---
>  tests/Makefile.include | 90 +++++++++++++++++++++++++-------------------------
>  3 files changed, 58 insertions(+), 50 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1851f8c..155f066 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -87,6 +87,10 @@ endif
>  
>  include $(SRC_PATH)/rules.mak
>  
> +# notempy and lor are defined in rules.mak
> +CONFIG_TOOLS := $(call notempty,$(TOOLS))
> +CONFIG_BLOCK := $(call lor,$(CONFIG_SOFTMMU),$(CONFIG_TOOLS))

IMHO calling this CONFIG_BLOCK is a really poor choice, as
the decision is completely unrelated to block modules. It
made really confused when trying to understand why all the
crypto or auth code had been made conditional on the block
drivers.  The block code is just one part of QEMU that
is used in tools & softmmu.

It would be better as CONFIG_SOFTMMU_TOOLS, or
CONFIG_NOT_USER, or something else.

> +trace-events-subdirs += crypto
> +ifeq ($(CONFIG_USER_ONLY),y)
> +trace-events-subdirs += linux-user
> +endif
> +ifeq ($(CONFIG_BLOCK),y)
>  trace-events-subdirs += authz
>  trace-events-subdirs += block
> -trace-events-subdirs += crypto
> +trace-events-subdirs += io
> +trace-events-subdirs += nbd
> +trace-events-subdirs += scsi
> +endif
>  ifeq ($(CONFIG_SOFTMMU),y)
>  trace-events-subdirs += chardev
>  trace-events-subdirs += audio
> @@ -178,12 +186,8 @@ trace-events-subdirs += net
>  trace-events-subdirs += ui
>  endif
>  trace-events-subdirs += hw/display
> -trace-events-subdirs += io
> -trace-events-subdirs += linux-user
> -trace-events-subdirs += nbd
>  trace-events-subdirs += qapi
>  trace-events-subdirs += qom
> -trace-events-subdirs += scsi
>  trace-events-subdirs += target/arm
>  trace-events-subdirs += target/hppa
>  trace-events-subdirs += target/i386
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index ad95a14..1865f6b 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -45,7 +45,7 @@ SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \
>  
>  check-unit-y += tests/check-qdict$(EXESUF)
>  check-unit-y += tests/check-block-qdict$(EXESUF)
> -check-unit-y += tests/test-char$(EXESUF)
> +check-unit-$(CONFIG_SOFTMMU) += tests/test-char$(EXESUF)
>  check-unit-y += tests/check-qnum$(EXESUF)
>  check-unit-y += tests/check-qstring$(EXESUF)
>  check-unit-y += tests/check-qlist$(EXESUF)
> @@ -61,21 +61,21 @@ check-unit-y += tests/test-string-input-visitor$(EXESUF)
>  check-unit-y += tests/test-string-output-visitor$(EXESUF)
>  check-unit-y += tests/test-qmp-event$(EXESUF)
>  check-unit-y += tests/test-opts-visitor$(EXESUF)
> -check-unit-y += tests/test-coroutine$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-coroutine$(EXESUF)
>  check-unit-y += tests/test-visitor-serialization$(EXESUF)
>  check-unit-y += tests/test-iov$(EXESUF)
> -check-unit-y += tests/test-aio$(EXESUF)
> -check-unit-y += tests/test-aio-multithread$(EXESUF)
> -check-unit-y += tests/test-throttle$(EXESUF)
> -check-unit-y += tests/test-thread-pool$(EXESUF)
> -check-unit-y += tests/test-hbitmap$(EXESUF)
> -check-unit-y += tests/test-bdrv-drain$(EXESUF)
> -check-unit-y += tests/test-bdrv-graph-mod$(EXESUF)
> -check-unit-y += tests/test-blockjob$(EXESUF)
> -check-unit-y += tests/test-blockjob-txn$(EXESUF)
> -check-unit-y += tests/test-block-backend$(EXESUF)
> -check-unit-y += tests/test-block-iothread$(EXESUF)
> -check-unit-y += tests/test-image-locking$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-aio$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-aio-multithread$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-throttle$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-thread-pool$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-hbitmap$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-bdrv-drain$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-bdrv-graph-mod$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-blockjob$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-blockjob-txn$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-block-backend$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-block-iothread$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-image-locking$(EXESUF)
>  check-unit-y += tests/test-x86-cpuid$(EXESUF)
>  # all code tested by test-x86-cpuid is inside topology.h
>  ifeq ($(CONFIG_SOFTMMU),y)
> @@ -101,40 +101,40 @@ check-unit-y += tests/check-qom-interface$(EXESUF)
>  check-unit-y += tests/check-qom-proplist$(EXESUF)
>  check-unit-y += tests/test-qemu-opts$(EXESUF)
>  check-unit-y += tests/test-keyval$(EXESUF)
> -check-unit-y += tests/test-write-threshold$(EXESUF)
> -check-unit-y += tests/test-crypto-hash$(EXESUF)
> -check-speed-y += tests/benchmark-crypto-hash$(EXESUF)
> -check-unit-y += tests/test-crypto-hmac$(EXESUF)
> -check-speed-y += tests/benchmark-crypto-hmac$(EXESUF)
> -check-unit-y += tests/test-crypto-cipher$(EXESUF)
> -check-speed-y += tests/benchmark-crypto-cipher$(EXESUF)
> -check-unit-y += tests/test-crypto-secret$(EXESUF)
> -check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF)
> -check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlssession$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-write-threshold$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-crypto-hash$(EXESUF)
> +check-speed-$(CONFIG_BLOCK) += tests/benchmark-crypto-hash$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-crypto-hmac$(EXESUF)
> +check-speed-$(CONFIG_BLOCK) += tests/benchmark-crypto-hmac$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-crypto-cipher$(EXESUF)
> +check-speed-$(CONFIG_BLOCK) += tests/benchmark-crypto-cipher$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-crypto-secret$(EXESUF)
> +check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_GNUTLS)) += tests/test-crypto-tlscredsx509$(EXESUF)
> +check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_GNUTLS)) += tests/test-crypto-tlssession$(EXESUF)
>  ifneq (,$(findstring qemu-ga,$(TOOLS)))
>  check-unit-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_SERIAL)) += tests/test-qga$(EXESUF)
>  endif
>  check-unit-y += tests/test-timed-average$(EXESUF)
>  check-unit-$(CONFIG_INOTIFY1) += tests/test-util-filemonitor$(EXESUF)
>  check-unit-y += tests/test-util-sockets$(EXESUF)
> -check-unit-y += tests/test-authz-simple$(EXESUF)
> -check-unit-y += tests/test-authz-list$(EXESUF)
> -check-unit-y += tests/test-authz-listfile$(EXESUF)
> -check-unit-$(CONFIG_AUTH_PAM) += tests/test-authz-pam$(EXESUF)
> -check-unit-y += tests/test-io-task$(EXESUF)
> -check-unit-y += tests/test-io-channel-socket$(EXESUF)
> -check-unit-y += tests/test-io-channel-file$(EXESUF)
> -check-unit-$(CONFIG_GNUTLS) += tests/test-io-channel-tls$(EXESUF)
> -check-unit-y += tests/test-io-channel-command$(EXESUF)
> -check-unit-y += tests/test-io-channel-buffer$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-authz-simple$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-authz-list$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-authz-listfile$(EXESUF)
> +check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_AUTH_PAM)) += tests/test-authz-pam$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-io-task$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-io-channel-socket$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-io-channel-file$(EXESUF)
> +check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_GNUTLS)) += tests/test-io-channel-tls$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-io-channel-command$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-io-channel-buffer$(EXESUF)
>  check-unit-y += tests/test-base64$(EXESUF)
> -check-unit-$(if $(CONFIG_NETTLE),y,$(CONFIG_GCRYPT)) += tests/test-crypto-pbkdf$(EXESUF)
> -check-unit-y += tests/test-crypto-ivgen$(EXESUF)
> -check-unit-y += tests/test-crypto-afsplit$(EXESUF)
> -check-unit-y += tests/test-crypto-xts$(EXESUF)
> -check-unit-y += tests/test-crypto-block$(EXESUF)
> +check-unit-$(call land,$(CONFIG_BLOCK),$(if $(CONFIG_NETTLE),y,$(CONFIG_GCRYPT))) += tests/test-crypto-pbkdf$(EXESUF)
> +check-unit-$(CONFIG_BLOCK) += tests/test-crypto-ivgen$(EXESUF)
> +check-unit-$(CONFIG_BLOCK)  += tests/test-crypto-afsplit$(EXESUF)
> +check-unit-$(CONFIG_BLOCK)  += tests/test-crypto-xts$(EXESUF)
> +check-unit-$(CONFIG_BLOCK)  += tests/test-crypto-block$(EXESUF)
>  check-unit-y += tests/test-logging$(EXESUF)
> -check-unit-$(CONFIG_REPLICATION) += tests/test-replication$(EXESUF)
> +check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_REPLICATION)) += tests/test-replication$(EXESUF)
>  check-unit-y += tests/test-bufferiszero$(EXESUF)
>  check-unit-y += tests/test-uuid$(EXESUF)
>  check-unit-y += tests/ptimer-test$(EXESUF)
> @@ -496,11 +496,11 @@ test-qapi-obj-y = tests/test-qapi-types.o \
>  	tests/test-qapi-visit-sub-sub-module.o \
>  	tests/test-qapi-introspect.o \
>  	$(test-qom-obj-y)
> -benchmark-crypto-obj-y = $(authz-obj-y) $(crypto-obj-y) $(test-qom-obj-y)
> -test-crypto-obj-y = $(authz-obj-y) $(crypto-obj-y) $(test-qom-obj-y)
> -test-io-obj-y = $(io-obj-y) $(test-crypto-obj-y)
> -test-authz-obj-y = $(test-qom-obj-y) $(authz-obj-y)
> -test-block-obj-y = $(block-obj-y) $(test-io-obj-y) tests/iothread.o
> +benchmark-crypto-obj-$(CONFIG_BLOCK) = $(authz-obj-y) $(crypto-obj-y) $(test-qom-obj-y)
> +test-crypto-obj-$(CONFIG_BLOCK) = $(authz-obj-y) $(crypto-obj-y) $(test-qom-obj-y)
> +test-io-obj-$(CONFIG_BLOCK) = $(io-obj-y) $(test-crypto-obj-y)
> +test-authz-obj-$(CONFIG_BLOCK) = $(test-qom-obj-y) $(authz-obj-y)
> +test-block-obj-$(CONFIG_BLOCK) = $(block-obj-y) $(test-io-obj-y) tests/iothread.o
>  
>  tests/check-qnum$(EXESUF): tests/check-qnum.o $(test-util-obj-y)
>  tests/check-qstring$(EXESUF): tests/check-qstring.o $(test-util-obj-y)
> -- 
> 1.8.3.1
> 
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PULL 18/21] build: don't build hardware objects with linux-user

[Laurent Vivier]

On 21/05/2019 13:52, Daniel P. Berrangé wrote:
> On Wed, May 15, 2019 at 10:50:30PM +0200, Paolo Bonzini wrote:
>> From: Laurent Vivier <lvivier@redhat.com>
>>
>> Some objects are only needed for system emulation and tools.
>> We can ignore them for the user mode case
>>
>> Update tests to run accordingly: conditionally build some tests
>> on CONFIG_BLOCK.
>>
>> Some tests use components that are only built when softmmu or
>> block tools are enabled, not for linux-user. So, if these components
>> are not available, disable the tests.
>>
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> Message-Id: <20190401141222.30034-6-lvivier@redhat.com>
>> ---
>>   Makefile               |  4 +++
>>   Makefile.objs          | 14 +++++---
>>   tests/Makefile.include | 90 +++++++++++++++++++++++++-------------------------
>>   3 files changed, 58 insertions(+), 50 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 1851f8c..155f066 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -87,6 +87,10 @@ endif
>>   
>>   include $(SRC_PATH)/rules.mak
>>   
>> +# notempy and lor are defined in rules.mak
>> +CONFIG_TOOLS := $(call notempty,$(TOOLS))
>> +CONFIG_BLOCK := $(call lor,$(CONFIG_SOFTMMU),$(CONFIG_TOOLS))
> 
> IMHO calling this CONFIG_BLOCK is a really poor choice, as
> the decision is completely unrelated to block modules. It
> made really confused when trying to understand why all the
> crypto or auth code had been made conditional on the block
> drivers.  The block code is just one part of QEMU that
> is used in tools & softmmu.
> 
> It would be better as CONFIG_SOFTMMU_TOOLS, or
> CONFIG_NOT_USER, or something else.

Do you think it's worth a patch to change the name?

Thanks,
Laurent

Re: [Qemu-devel] [PULL 18/21] build: don't build hardware objects with linux-user

[Daniel P. Berrangé]

On Tue, May 21, 2019 at 02:52:42PM +0200, Laurent Vivier wrote:
> On 21/05/2019 13:52, Daniel P. Berrangé wrote:
> > On Wed, May 15, 2019 at 10:50:30PM +0200, Paolo Bonzini wrote:
> > > From: Laurent Vivier <lvivier@redhat.com>
> > > 
> > > Some objects are only needed for system emulation and tools.
> > > We can ignore them for the user mode case
> > > 
> > > Update tests to run accordingly: conditionally build some tests
> > > on CONFIG_BLOCK.
> > > 
> > > Some tests use components that are only built when softmmu or
> > > block tools are enabled, not for linux-user. So, if these components
> > > are not available, disable the tests.
> > > 
> > > Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> > > Message-Id: <20190401141222.30034-6-lvivier@redhat.com>
> > > ---
> > >   Makefile               |  4 +++
> > >   Makefile.objs          | 14 +++++---
> > >   tests/Makefile.include | 90 +++++++++++++++++++++++++-------------------------
> > >   3 files changed, 58 insertions(+), 50 deletions(-)
> > > 
> > > diff --git a/Makefile b/Makefile
> > > index 1851f8c..155f066 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -87,6 +87,10 @@ endif
> > >   include $(SRC_PATH)/rules.mak
> > > +# notempy and lor are defined in rules.mak
> > > +CONFIG_TOOLS := $(call notempty,$(TOOLS))
> > > +CONFIG_BLOCK := $(call lor,$(CONFIG_SOFTMMU),$(CONFIG_TOOLS))
> > 
> > IMHO calling this CONFIG_BLOCK is a really poor choice, as
> > the decision is completely unrelated to block modules. It
> > made really confused when trying to understand why all the
> > crypto or auth code had been made conditional on the block
> > drivers.  The block code is just one part of QEMU that
> > is used in tools & softmmu.
> > 
> > It would be better as CONFIG_SOFTMMU_TOOLS, or
> > CONFIG_NOT_USER, or something else.
> 
> Do you think it's worth a patch to change the name?

Personally I'd like to see a patch to give it a better name.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PULL 18/21] build: don't build hardware objects with linux-user

[Laurent Vivier]

On 21/05/2019 14:54, Daniel P. Berrangé wrote:
> On Tue, May 21, 2019 at 02:52:42PM +0200, Laurent Vivier wrote:
>> On 21/05/2019 13:52, Daniel P. Berrangé wrote:
>>> On Wed, May 15, 2019 at 10:50:30PM +0200, Paolo Bonzini wrote:
>>>> From: Laurent Vivier <lvivier@redhat.com>
>>>>
>>>> Some objects are only needed for system emulation and tools.
>>>> We can ignore them for the user mode case
>>>>
>>>> Update tests to run accordingly: conditionally build some tests
>>>> on CONFIG_BLOCK.
>>>>
>>>> Some tests use components that are only built when softmmu or
>>>> block tools are enabled, not for linux-user. So, if these components
>>>> are not available, disable the tests.
>>>>
>>>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>>>> Message-Id: <20190401141222.30034-6-lvivier@redhat.com>
>>>> ---
>>>>    Makefile               |  4 +++
>>>>    Makefile.objs          | 14 +++++---
>>>>    tests/Makefile.include | 90 +++++++++++++++++++++++++-------------------------
>>>>    3 files changed, 58 insertions(+), 50 deletions(-)
>>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index 1851f8c..155f066 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -87,6 +87,10 @@ endif
>>>>    include $(SRC_PATH)/rules.mak
>>>> +# notempy and lor are defined in rules.mak
>>>> +CONFIG_TOOLS := $(call notempty,$(TOOLS))
>>>> +CONFIG_BLOCK := $(call lor,$(CONFIG_SOFTMMU),$(CONFIG_TOOLS))
>>>
>>> IMHO calling this CONFIG_BLOCK is a really poor choice, as
>>> the decision is completely unrelated to block modules. It
>>> made really confused when trying to understand why all the
>>> crypto or auth code had been made conditional on the block
>>> drivers.  The block code is just one part of QEMU that
>>> is used in tools & softmmu.
>>>
>>> It would be better as CONFIG_SOFTMMU_TOOLS, or
>>> CONFIG_NOT_USER, or something else.
>>
>> Do you think it's worth a patch to change the name?
> 
> Personally I'd like to see a patch to give it a better name.

I'm wondering which name to use.

CONFIG_NOT_USER is not correct because the flag can be 'y' with 
CONFIG_USER_ONLY set.
CONFIG_SOFTMMU_TOOLS is too fuzzy.

CONFIG_HW ?
CONFIG_IO ?
CONFIG_DEVICE ?

Thanks,
Laurent

Re: [Qemu-devel] [PULL 18/21] build: don't build hardware objects with linux-user

[Philippe Mathieu-Daudé]

On 5/21/19 10:16 PM, Laurent Vivier wrote:
> On 21/05/2019 14:54, Daniel P. Berrangé wrote:
>> On Tue, May 21, 2019 at 02:52:42PM +0200, Laurent Vivier wrote:
>>> On 21/05/2019 13:52, Daniel P. Berrangé wrote:
>>>> On Wed, May 15, 2019 at 10:50:30PM +0200, Paolo Bonzini wrote:
>>>>> From: Laurent Vivier <lvivier@redhat.com>
>>>>>
>>>>> Some objects are only needed for system emulation and tools.
>>>>> We can ignore them for the user mode case
>>>>>
>>>>> Update tests to run accordingly: conditionally build some tests
>>>>> on CONFIG_BLOCK.
>>>>>
>>>>> Some tests use components that are only built when softmmu or
>>>>> block tools are enabled, not for linux-user. So, if these components
>>>>> are not available, disable the tests.
>>>>>
>>>>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>>>>> Message-Id: <20190401141222.30034-6-lvivier@redhat.com>
>>>>> ---
>>>>>    Makefile               |  4 +++
>>>>>    Makefile.objs          | 14 +++++---
>>>>>    tests/Makefile.include | 90
>>>>> +++++++++++++++++++++++++-------------------------
>>>>>    3 files changed, 58 insertions(+), 50 deletions(-)
>>>>>
>>>>> diff --git a/Makefile b/Makefile
>>>>> index 1851f8c..155f066 100644
>>>>> --- a/Makefile
>>>>> +++ b/Makefile
>>>>> @@ -87,6 +87,10 @@ endif
>>>>>    include $(SRC_PATH)/rules.mak
>>>>> +# notempy and lor are defined in rules.mak
>>>>> +CONFIG_TOOLS := $(call notempty,$(TOOLS))
>>>>> +CONFIG_BLOCK := $(call lor,$(CONFIG_SOFTMMU),$(CONFIG_TOOLS))
>>>>
>>>> IMHO calling this CONFIG_BLOCK is a really poor choice, as
>>>> the decision is completely unrelated to block modules. It
>>>> made really confused when trying to understand why all the
>>>> crypto or auth code had been made conditional on the block
>>>> drivers.  The block code is just one part of QEMU that
>>>> is used in tools & softmmu.
>>>>
>>>> It would be better as CONFIG_SOFTMMU_TOOLS, or
>>>> CONFIG_NOT_USER, or something else.
>>>
>>> Do you think it's worth a patch to change the name?
>>
>> Personally I'd like to see a patch to give it a better name.
> 
> I'm wondering which name to use.
> 
> CONFIG_NOT_USER is not correct because the flag can be 'y' with
> CONFIG_USER_ONLY set.
> CONFIG_SOFTMMU_TOOLS is too fuzzy.
> 
> CONFIG_HW ?
> CONFIG_IO ?
> CONFIG_DEVICE ?

CONFIG_MACHINE_HW?
CONFIG_MACHINE_IO_HELPERS?
CONFIG_AIO?

BTW looking at util/Makefile.objs there is various think you can skip
for linux-user IMO.

Regards,

Phil.

[Qemu-devel] [PULL 19/21] ioapic: allow buggy guests mishandling level-triggered interrupts to make progress

[Paolo Bonzini]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

It was found that Hyper-V 2016 on KVM in some configurations (q35 machine +
piix4-usb-uhci) hangs on boot. Root-cause was that one of Hyper-V
level-triggered interrupt handler performs EOI before fixing the cause of
the interrupt. This results in IOAPIC keep re-raising the level-triggered
interrupt after EOI because irq-line remains asserted.

Gory details: https://www.spinics.net/lists/kvm/msg184484.html
(the whole thread).

Turns out we were dealing with similar issues before; in-kernel IOAPIC
implementation has commit 184564efae4d ("kvm: ioapic: conditionally delay
irq delivery duringeoi broadcast") which describes a very similar issue.

Steal the idea from the above mentioned commit for IOAPIC implementation in
QEMU. SUCCESSIVE_IRQ_MAX_COUNT, delay and the comment are borrowed as well.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Message-Id: <20190402080215.10747-1-vkuznets@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/intc/ioapic.c                  | 57 +++++++++++++++++++++++++++++++++++----
 hw/intc/trace-events              |  1 +
 include/hw/i386/ioapic_internal.h |  3 +++
 3 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 9d75f84..7074489 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -139,6 +139,15 @@ static void ioapic_service(IOAPICCommonState *s)
     }
 }
 
+#define SUCCESSIVE_IRQ_MAX_COUNT 10000
+
+static void delayed_ioapic_service_cb(void *opaque)
+{
+    IOAPICCommonState *s = opaque;
+
+    ioapic_service(s);
+}
+
 static void ioapic_set_irq(void *opaque, int vector, int level)
 {
     IOAPICCommonState *s = opaque;
@@ -222,13 +231,39 @@ void ioapic_eoi_broadcast(int vector)
         }
         for (n = 0; n < IOAPIC_NUM_PINS; n++) {
             entry = s->ioredtbl[n];
-            if ((entry & IOAPIC_LVT_REMOTE_IRR)
-                && (entry & IOAPIC_VECTOR_MASK) == vector) {
-                trace_ioapic_clear_remote_irr(n, vector);
-                s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
-                if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
+
+            if ((entry & IOAPIC_VECTOR_MASK) != vector ||
+                ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) != IOAPIC_TRIGGER_LEVEL) {
+                continue;
+            }
+
+            if (!(entry & IOAPIC_LVT_REMOTE_IRR)) {
+                continue;
+            }
+
+            trace_ioapic_clear_remote_irr(n, vector);
+            s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
+
+            if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
+                ++s->irq_eoi[vector];
+                if (s->irq_eoi[vector] >= SUCCESSIVE_IRQ_MAX_COUNT) {
+                    /*
+                     * Real hardware does not deliver the interrupt immediately
+                     * during eoi broadcast, and this lets a buggy guest make
+                     * slow progress even if it does not correctly handle a
+                     * level-triggered interrupt. Emulate this behavior if we
+                     * detect an interrupt storm.
+                     */
+                    s->irq_eoi[vector] = 0;
+                    timer_mod_anticipate(s->delayed_ioapic_service_timer,
+                                         qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
+                                         NANOSECONDS_PER_SECOND / 100);
+                    trace_ioapic_eoi_delayed_reassert(vector);
+                } else {
                     ioapic_service(s);
                 }
+            } else {
+                s->irq_eoi[vector] = 0;
             }
         }
     }
@@ -401,6 +436,9 @@ static void ioapic_realize(DeviceState *dev, Error **errp)
     memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
                           "ioapic", 0x1000);
 
+    s->delayed_ioapic_service_timer =
+        timer_new_ns(QEMU_CLOCK_VIRTUAL, delayed_ioapic_service_cb, s);
+
     qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS);
 
     ioapics[ioapic_no] = s;
@@ -408,6 +446,14 @@ static void ioapic_realize(DeviceState *dev, Error **errp)
     qemu_add_machine_init_done_notifier(&s->machine_done);
 }
 
+static void ioapic_unrealize(DeviceState *dev, Error **errp)
+{
+    IOAPICCommonState *s = IOAPIC_COMMON(dev);
+
+    timer_del(s->delayed_ioapic_service_timer);
+    timer_free(s->delayed_ioapic_service_timer);
+}
+
 static Property ioapic_properties[] = {
     DEFINE_PROP_UINT8("version", IOAPICCommonState, version, IOAPIC_VER_DEF),
     DEFINE_PROP_END_OF_LIST(),
@@ -419,6 +465,7 @@ static void ioapic_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     k->realize = ioapic_realize;
+    k->unrealize = ioapic_unrealize;
     /*
      * If APIC is in kernel, we need to update the kernel cache after
      * migration, otherwise first 24 gsi routes will be invalid.
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index a28bdce..90c9d07 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -25,6 +25,7 @@ apic_mem_writel(uint64_t addr, uint32_t val) "0x%"PRIx64" = 0x%08x"
 ioapic_set_remote_irr(int n) "set remote irr for pin %d"
 ioapic_clear_remote_irr(int n, int vector) "clear remote irr for pin %d vector %d"
 ioapic_eoi_broadcast(int vector) "EOI broadcast for vector %d"
+ioapic_eoi_delayed_reassert(int vector) "delayed reassert on EOI broadcast for vector %d"
 ioapic_mem_read(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapic mem read addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8" retval 0x%"PRIx32
 ioapic_mem_write(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapic mem write addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8" val 0x%"PRIx32
 ioapic_set_irq(int vector, int level) "vector: %d level: %d"
diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h
index 9848f39..07002f9 100644
--- a/include/hw/i386/ioapic_internal.h
+++ b/include/hw/i386/ioapic_internal.h
@@ -96,6 +96,7 @@ typedef struct IOAPICCommonClass {
     SysBusDeviceClass parent_class;
 
     DeviceRealize realize;
+    DeviceUnrealize unrealize;
     void (*pre_save)(IOAPICCommonState *s);
     void (*post_load)(IOAPICCommonState *s);
 } IOAPICCommonClass;
@@ -111,6 +112,8 @@ struct IOAPICCommonState {
     uint8_t version;
     uint64_t irq_count[IOAPIC_NUM_PINS];
     int irq_level[IOAPIC_NUM_PINS];
+    int irq_eoi[IOAPIC_NUM_PINS];
+    QEMUTimer *delayed_ioapic_service_timer;
 };
 
 void ioapic_reset_common(DeviceState *dev);
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 19/21] ioapic: allow buggy guests mishandling level-triggered interrupts to make progress

[Marc-André Lureau]

Hi

On Thu, May 16, 2019 at 1:04 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> From: Vitaly Kuznetsov <vkuznets@redhat.com>
>
> It was found that Hyper-V 2016 on KVM in some configurations (q35 machine +
> piix4-usb-uhci) hangs on boot. Root-cause was that one of Hyper-V
> level-triggered interrupt handler performs EOI before fixing the cause of
> the interrupt. This results in IOAPIC keep re-raising the level-triggered
> interrupt after EOI because irq-line remains asserted.
>
> Gory details: https://www.spinics.net/lists/kvm/msg184484.html
> (the whole thread).
>
> Turns out we were dealing with similar issues before; in-kernel IOAPIC
> implementation has commit 184564efae4d ("kvm: ioapic: conditionally delay
> irq delivery duringeoi broadcast") which describes a very similar issue.
>
> Steal the idea from the above mentioned commit for IOAPIC implementation in
> QEMU. SUCCESSIVE_IRQ_MAX_COUNT, delay and the comment are borrowed as well.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Message-Id: <20190402080215.10747-1-vkuznets@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

After this commit, I get the following ASAN error when booting a VM:

/home/elmarco/src/qq/hw/intc/ioapic.c:266:27: runtime error: index 41
out of bounds for type 'int [24]'
=================================================================
==9761==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x61b00000312c at pc 0x55e40b8f9e74 bp 0x7f269c1d32a0 sp
0x7f269c1d3290
WRITE of size 4 at 0x61b00000312c thread T4 (CPU 0/KVM)
    #0 0x55e40b8f9e73 in ioapic_eoi_broadcast
/home/elmarco/src/qq/hw/intc/ioapic.c:266
    #1 0x55e40bd3fa4e in kvm_arch_handle_exit
/home/elmarco/src/qq/target/i386/kvm.c:3644
    #2 0x55e40b7d1fd7 in kvm_cpu_exec
/home/elmarco/src/qq/accel/kvm/kvm-all.c:2083
    #3 0x55e40b6e435f in qemu_kvm_cpu_thread_fn /home/elmarco/src/qq/cpus.c:1282
    #4 0x55e40ce9ba42 in qemu_thread_start
/home/elmarco/src/qq/util/qemu-thread-posix.c:502
    #5 0x7f26ac3435a1 in start_thread (/lib64/libpthread.so.0+0x85a1)
    #6 0x7f26ac270302 in __clone (/lib64/libc.so.6+0xfb302)

Address 0x61b00000312c is a wild pointer.
SUMMARY: AddressSanitizer: heap-buffer-overflow
/home/elmarco/src/qq/hw/intc/ioapic.c:266 in ioapic_eoi_broadcast
Shadow bytes around the buggy address:
  0x0c367fff85d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c367fff85e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c367fff85f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c367fff8600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c367fff8610: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa
=>0x0c367fff8620: fa fa fa fa fa[fa]fa fa fa fa fa fa fa fa fa fa
  0x0c367fff8630: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fff8640: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fff8650: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fff8660: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c367fff8670: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
Thread T4 (CPU 0/KVM) created by T0 here:
    #0 0x7f26af884965 in pthread_create (/lib64/libasan.so.5+0x3a965)
    #1 0x55e40ce9be8b in qemu_thread_create
/home/elmarco/src/qq/util/qemu-thread-posix.c:539
    #2 0x55e40b6ea347 in qemu_kvm_start_vcpu /home/elmarco/src/qq/cpus.c:2018
    #3 0x55e40b6eb03b in qemu_init_vcpu /home/elmarco/src/qq/cpus.c:2084
    #4 0x55e40bb52352 in x86_cpu_realizefn
/home/elmarco/src/qq/target/i386/cpu.c:5382
    #5 0x55e40c017aed in device_set_realized
/home/elmarco/src/qq/hw/core/qdev.c:834
    #6 0x55e40c95e703 in property_set_bool
/home/elmarco/src/qq/qom/object.c:2074
    #7 0x55e40c959332 in object_property_set
/home/elmarco/src/qq/qom/object.c:1266
    #8 0x55e40c961edb in object_property_set_qobject
/home/elmarco/src/qq/qom/qom-qobject.c:27
    #9 0x55e40c959700 in object_property_set_bool
/home/elmarco/src/qq/qom/object.c:1332
    #10 0x55e40ba831a3 in pc_new_cpu /home/elmarco/src/qq/hw/i386/pc.c:1531
    #11 0x55e40ba838a0 in pc_cpus_init /home/elmarco/src/qq/hw/i386/pc.c:1579
    #12 0x55e40ba9f394 in pc_q35_init /home/elmarco/src/qq/hw/i386/pc_q35.c:183
    #13 0x55e40baa19ab in pc_init_v4_0 /home/elmarco/src/qq/hw/i386/pc_q35.c:385
    #14 0x55e40c035aa7 in machine_run_board_init
/home/elmarco/src/qq/hw/core/machine.c:1033
    #15 0x55e40bda59d8 in main /home/elmarco/src/qq/vl.c:4494
    #16 0x7f26ac198f32 in __libc_start_main (/lib64/libc.so.6+0x23f32)

I start investigating.

> ---
>  hw/intc/ioapic.c                  | 57 +++++++++++++++++++++++++++++++++++----
>  hw/intc/trace-events              |  1 +
>  include/hw/i386/ioapic_internal.h |  3 +++
>  3 files changed, 56 insertions(+), 5 deletions(-)
>
> diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
> index 9d75f84..7074489 100644
> --- a/hw/intc/ioapic.c
> +++ b/hw/intc/ioapic.c
> @@ -139,6 +139,15 @@ static void ioapic_service(IOAPICCommonState *s)
>      }
>  }
>
> +#define SUCCESSIVE_IRQ_MAX_COUNT 10000
> +
> +static void delayed_ioapic_service_cb(void *opaque)
> +{
> +    IOAPICCommonState *s = opaque;
> +
> +    ioapic_service(s);
> +}
> +
>  static void ioapic_set_irq(void *opaque, int vector, int level)
>  {
>      IOAPICCommonState *s = opaque;
> @@ -222,13 +231,39 @@ void ioapic_eoi_broadcast(int vector)
>          }
>          for (n = 0; n < IOAPIC_NUM_PINS; n++) {
>              entry = s->ioredtbl[n];
> -            if ((entry & IOAPIC_LVT_REMOTE_IRR)
> -                && (entry & IOAPIC_VECTOR_MASK) == vector) {
> -                trace_ioapic_clear_remote_irr(n, vector);
> -                s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
> -                if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
> +
> +            if ((entry & IOAPIC_VECTOR_MASK) != vector ||
> +                ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) != IOAPIC_TRIGGER_LEVEL) {
> +                continue;
> +            }
> +
> +            if (!(entry & IOAPIC_LVT_REMOTE_IRR)) {
> +                continue;
> +            }
> +
> +            trace_ioapic_clear_remote_irr(n, vector);
> +            s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
> +
> +            if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
> +                ++s->irq_eoi[vector];
> +                if (s->irq_eoi[vector] >= SUCCESSIVE_IRQ_MAX_COUNT) {
> +                    /*
> +                     * Real hardware does not deliver the interrupt immediately
> +                     * during eoi broadcast, and this lets a buggy guest make
> +                     * slow progress even if it does not correctly handle a
> +                     * level-triggered interrupt. Emulate this behavior if we
> +                     * detect an interrupt storm.
> +                     */
> +                    s->irq_eoi[vector] = 0;
> +                    timer_mod_anticipate(s->delayed_ioapic_service_timer,
> +                                         qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> +                                         NANOSECONDS_PER_SECOND / 100);
> +                    trace_ioapic_eoi_delayed_reassert(vector);
> +                } else {
>                      ioapic_service(s);
>                  }
> +            } else {
> +                s->irq_eoi[vector] = 0;
>              }
>          }
>      }
> @@ -401,6 +436,9 @@ static void ioapic_realize(DeviceState *dev, Error **errp)
>      memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
>                            "ioapic", 0x1000);
>
> +    s->delayed_ioapic_service_timer =
> +        timer_new_ns(QEMU_CLOCK_VIRTUAL, delayed_ioapic_service_cb, s);
> +
>      qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS);
>
>      ioapics[ioapic_no] = s;
> @@ -408,6 +446,14 @@ static void ioapic_realize(DeviceState *dev, Error **errp)
>      qemu_add_machine_init_done_notifier(&s->machine_done);
>  }
>
> +static void ioapic_unrealize(DeviceState *dev, Error **errp)
> +{
> +    IOAPICCommonState *s = IOAPIC_COMMON(dev);
> +
> +    timer_del(s->delayed_ioapic_service_timer);
> +    timer_free(s->delayed_ioapic_service_timer);
> +}
> +
>  static Property ioapic_properties[] = {
>      DEFINE_PROP_UINT8("version", IOAPICCommonState, version, IOAPIC_VER_DEF),
>      DEFINE_PROP_END_OF_LIST(),
> @@ -419,6 +465,7 @@ static void ioapic_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>
>      k->realize = ioapic_realize;
> +    k->unrealize = ioapic_unrealize;
>      /*
>       * If APIC is in kernel, we need to update the kernel cache after
>       * migration, otherwise first 24 gsi routes will be invalid.
> diff --git a/hw/intc/trace-events b/hw/intc/trace-events
> index a28bdce..90c9d07 100644
> --- a/hw/intc/trace-events
> +++ b/hw/intc/trace-events
> @@ -25,6 +25,7 @@ apic_mem_writel(uint64_t addr, uint32_t val) "0x%"PRIx64" = 0x%08x"
>  ioapic_set_remote_irr(int n) "set remote irr for pin %d"
>  ioapic_clear_remote_irr(int n, int vector) "clear remote irr for pin %d vector %d"
>  ioapic_eoi_broadcast(int vector) "EOI broadcast for vector %d"
> +ioapic_eoi_delayed_reassert(int vector) "delayed reassert on EOI broadcast for vector %d"
>  ioapic_mem_read(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapic mem read addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8" retval 0x%"PRIx32
>  ioapic_mem_write(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapic mem write addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8" val 0x%"PRIx32
>  ioapic_set_irq(int vector, int level) "vector: %d level: %d"
> diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h
> index 9848f39..07002f9 100644
> --- a/include/hw/i386/ioapic_internal.h
> +++ b/include/hw/i386/ioapic_internal.h
> @@ -96,6 +96,7 @@ typedef struct IOAPICCommonClass {
>      SysBusDeviceClass parent_class;
>
>      DeviceRealize realize;
> +    DeviceUnrealize unrealize;
>      void (*pre_save)(IOAPICCommonState *s);
>      void (*post_load)(IOAPICCommonState *s);
>  } IOAPICCommonClass;
> @@ -111,6 +112,8 @@ struct IOAPICCommonState {
>      uint8_t version;
>      uint64_t irq_count[IOAPIC_NUM_PINS];
>      int irq_level[IOAPIC_NUM_PINS];
> +    int irq_eoi[IOAPIC_NUM_PINS];
> +    QEMUTimer *delayed_ioapic_service_timer;
>  };
>
>  void ioapic_reset_common(DeviceState *dev);
> --
> 1.8.3.1
>
>
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PULL 19/21] ioapic: allow buggy guests mishandling level-triggered interrupts to make progress

[Li Qiang]

I have posted a fix for this several weeks ago:

-->https://www.mail-archive.com/qemu-devel@nongnu.org/msg626186.html


Thanks,
Li Qiang

Marc-André Lureau <marcandre.lureau@gmail.com> 于2019年7月4日周四 下午8:57写道：

> Hi
>
> On Thu, May 16, 2019 at 1:04 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > From: Vitaly Kuznetsov <vkuznets@redhat.com>
> >
> > It was found that Hyper-V 2016 on KVM in some configurations (q35
> machine +
> > piix4-usb-uhci) hangs on boot. Root-cause was that one of Hyper-V
> > level-triggered interrupt handler performs EOI before fixing the cause of
> > the interrupt. This results in IOAPIC keep re-raising the level-triggered
> > interrupt after EOI because irq-line remains asserted.
> >
> > Gory details: https://www.spinics.net/lists/kvm/msg184484.html
> > (the whole thread).
> >
> > Turns out we were dealing with similar issues before; in-kernel IOAPIC
> > implementation has commit 184564efae4d ("kvm: ioapic: conditionally delay
> > irq delivery duringeoi broadcast") which describes a very similar issue.
> >
> > Steal the idea from the above mentioned commit for IOAPIC implementation
> in
> > QEMU. SUCCESSIVE_IRQ_MAX_COUNT, delay and the comment are borrowed as
> well.
> >
> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> > Message-Id: <20190402080215.10747-1-vkuznets@redhat.com>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> After this commit, I get the following ASAN error when booting a VM:
>
> /home/elmarco/src/qq/hw/intc/ioapic.c:266:27: runtime error: index 41
> out of bounds for type 'int [24]'
> =================================================================
> ==9761==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x61b00000312c at pc 0x55e40b8f9e74 bp 0x7f269c1d32a0 sp
> 0x7f269c1d3290
> WRITE of size 4 at 0x61b00000312c thread T4 (CPU 0/KVM)
>     #0 0x55e40b8f9e73 in ioapic_eoi_broadcast
> /home/elmarco/src/qq/hw/intc/ioapic.c:266
>     #1 0x55e40bd3fa4e in kvm_arch_handle_exit
> /home/elmarco/src/qq/target/i386/kvm.c:3644
>     #2 0x55e40b7d1fd7 in kvm_cpu_exec
> /home/elmarco/src/qq/accel/kvm/kvm-all.c:2083
>     #3 0x55e40b6e435f in qemu_kvm_cpu_thread_fn
> /home/elmarco/src/qq/cpus.c:1282
>     #4 0x55e40ce9ba42 in qemu_thread_start
> /home/elmarco/src/qq/util/qemu-thread-posix.c:502
>     #5 0x7f26ac3435a1 in start_thread (/lib64/libpthread.so.0+0x85a1)
>     #6 0x7f26ac270302 in __clone (/lib64/libc.so.6+0xfb302)
>
> Address 0x61b00000312c is a wild pointer.
> SUMMARY: AddressSanitizer: heap-buffer-overflow
> /home/elmarco/src/qq/hw/intc/ioapic.c:266 in ioapic_eoi_broadcast
> Shadow bytes around the buggy address:
>   0x0c367fff85d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x0c367fff85e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x0c367fff85f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x0c367fff8600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x0c367fff8610: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa
> =>0x0c367fff8620: fa fa fa fa fa[fa]fa fa fa fa fa fa fa fa fa fa
>   0x0c367fff8630: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>   0x0c367fff8640: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>   0x0c367fff8650: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>   0x0c367fff8660: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>   0x0c367fff8670: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> Shadow byte legend (one shadow byte represents 8 application bytes):
>   Addressable:           00
>   Partially addressable: 01 02 03 04 05 06 07
>   Heap left redzone:       fa
>   Freed heap region:       fd
>   Stack left redzone:      f1
>   Stack mid redzone:       f2
>   Stack right redzone:     f3
>   Stack after return:      f5
>   Stack use after scope:   f8
>   Global redzone:          f9
>   Global init order:       f6
>   Poisoned by user:        f7
>   Container overflow:      fc
>   Array cookie:            ac
>   Intra object redzone:    bb
>   ASan internal:           fe
>   Left alloca redzone:     ca
>   Right alloca redzone:    cb
>   Shadow gap:              cc
> Thread T4 (CPU 0/KVM) created by T0 here:
>     #0 0x7f26af884965 in pthread_create (/lib64/libasan.so.5+0x3a965)
>     #1 0x55e40ce9be8b in qemu_thread_create
> /home/elmarco/src/qq/util/qemu-thread-posix.c:539
>     #2 0x55e40b6ea347 in qemu_kvm_start_vcpu
> /home/elmarco/src/qq/cpus.c:2018
>     #3 0x55e40b6eb03b in qemu_init_vcpu /home/elmarco/src/qq/cpus.c:2084
>     #4 0x55e40bb52352 in x86_cpu_realizefn
> /home/elmarco/src/qq/target/i386/cpu.c:5382
>     #5 0x55e40c017aed in device_set_realized
> /home/elmarco/src/qq/hw/core/qdev.c:834
>     #6 0x55e40c95e703 in property_set_bool
> /home/elmarco/src/qq/qom/object.c:2074
>     #7 0x55e40c959332 in object_property_set
> /home/elmarco/src/qq/qom/object.c:1266
>     #8 0x55e40c961edb in object_property_set_qobject
> /home/elmarco/src/qq/qom/qom-qobject.c:27
>     #9 0x55e40c959700 in object_property_set_bool
> /home/elmarco/src/qq/qom/object.c:1332
>     #10 0x55e40ba831a3 in pc_new_cpu /home/elmarco/src/qq/hw/i386/pc.c:1531
>     #11 0x55e40ba838a0 in pc_cpus_init
> /home/elmarco/src/qq/hw/i386/pc.c:1579
>     #12 0x55e40ba9f394 in pc_q35_init
> /home/elmarco/src/qq/hw/i386/pc_q35.c:183
>     #13 0x55e40baa19ab in pc_init_v4_0
> /home/elmarco/src/qq/hw/i386/pc_q35.c:385
>     #14 0x55e40c035aa7 in machine_run_board_init
> /home/elmarco/src/qq/hw/core/machine.c:1033
>     #15 0x55e40bda59d8 in main /home/elmarco/src/qq/vl.c:4494
>     #16 0x7f26ac198f32 in __libc_start_main (/lib64/libc.so.6+0x23f32)
>
> I start investigating.
>
> > ---
> >  hw/intc/ioapic.c                  | 57
> +++++++++++++++++++++++++++++++++++----
> >  hw/intc/trace-events              |  1 +
> >  include/hw/i386/ioapic_internal.h |  3 +++
> >  3 files changed, 56 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
> > index 9d75f84..7074489 100644
> > --- a/hw/intc/ioapic.c
> > +++ b/hw/intc/ioapic.c
> > @@ -139,6 +139,15 @@ static void ioapic_service(IOAPICCommonState *s)
> >      }
> >  }
> >
> > +#define SUCCESSIVE_IRQ_MAX_COUNT 10000
> > +
> > +static void delayed_ioapic_service_cb(void *opaque)
> > +{
> > +    IOAPICCommonState *s = opaque;
> > +
> > +    ioapic_service(s);
> > +}
> > +
> >  static void ioapic_set_irq(void *opaque, int vector, int level)
> >  {
> >      IOAPICCommonState *s = opaque;
> > @@ -222,13 +231,39 @@ void ioapic_eoi_broadcast(int vector)
> >          }
> >          for (n = 0; n < IOAPIC_NUM_PINS; n++) {
> >              entry = s->ioredtbl[n];
> > -            if ((entry & IOAPIC_LVT_REMOTE_IRR)
> > -                && (entry & IOAPIC_VECTOR_MASK) == vector) {
> > -                trace_ioapic_clear_remote_irr(n, vector);
> > -                s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
> > -                if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 <<
> n))) {
> > +
> > +            if ((entry & IOAPIC_VECTOR_MASK) != vector ||
> > +                ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) !=
> IOAPIC_TRIGGER_LEVEL) {
> > +                continue;
> > +            }
> > +
> > +            if (!(entry & IOAPIC_LVT_REMOTE_IRR)) {
> > +                continue;
> > +            }
> > +
> > +            trace_ioapic_clear_remote_irr(n, vector);
> > +            s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
> > +
> > +            if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
> > +                ++s->irq_eoi[vector];
> > +                if (s->irq_eoi[vector] >= SUCCESSIVE_IRQ_MAX_COUNT) {
> > +                    /*
> > +                     * Real hardware does not deliver the interrupt
> immediately
> > +                     * during eoi broadcast, and this lets a buggy
> guest make
> > +                     * slow progress even if it does not correctly
> handle a
> > +                     * level-triggered interrupt. Emulate this behavior
> if we
> > +                     * detect an interrupt storm.
> > +                     */
> > +                    s->irq_eoi[vector] = 0;
> > +
> timer_mod_anticipate(s->delayed_ioapic_service_timer,
> > +
>  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> > +                                         NANOSECONDS_PER_SECOND / 100);
> > +                    trace_ioapic_eoi_delayed_reassert(vector);
> > +                } else {
> >                      ioapic_service(s);
> >                  }
> > +            } else {
> > +                s->irq_eoi[vector] = 0;
> >              }
> >          }
> >      }
> > @@ -401,6 +436,9 @@ static void ioapic_realize(DeviceState *dev, Error
> **errp)
> >      memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
> >                            "ioapic", 0x1000);
> >
> > +    s->delayed_ioapic_service_timer =
> > +        timer_new_ns(QEMU_CLOCK_VIRTUAL, delayed_ioapic_service_cb, s);
> > +
> >      qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS);
> >
> >      ioapics[ioapic_no] = s;
> > @@ -408,6 +446,14 @@ static void ioapic_realize(DeviceState *dev, Error
> **errp)
> >      qemu_add_machine_init_done_notifier(&s->machine_done);
> >  }
> >
> > +static void ioapic_unrealize(DeviceState *dev, Error **errp)
> > +{
> > +    IOAPICCommonState *s = IOAPIC_COMMON(dev);
> > +
> > +    timer_del(s->delayed_ioapic_service_timer);
> > +    timer_free(s->delayed_ioapic_service_timer);
> > +}
> > +
> >  static Property ioapic_properties[] = {
> >      DEFINE_PROP_UINT8("version", IOAPICCommonState, version,
> IOAPIC_VER_DEF),
> >      DEFINE_PROP_END_OF_LIST(),
> > @@ -419,6 +465,7 @@ static void ioapic_class_init(ObjectClass *klass,
> void *data)
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> >
> >      k->realize = ioapic_realize;
> > +    k->unrealize = ioapic_unrealize;
> >      /*
> >       * If APIC is in kernel, we need to update the kernel cache after
> >       * migration, otherwise first 24 gsi routes will be invalid.
> > diff --git a/hw/intc/trace-events b/hw/intc/trace-events
> > index a28bdce..90c9d07 100644
> > --- a/hw/intc/trace-events
> > +++ b/hw/intc/trace-events
> > @@ -25,6 +25,7 @@ apic_mem_writel(uint64_t addr, uint32_t val)
> "0x%"PRIx64" = 0x%08x"
> >  ioapic_set_remote_irr(int n) "set remote irr for pin %d"
> >  ioapic_clear_remote_irr(int n, int vector) "clear remote irr for pin %d
> vector %d"
> >  ioapic_eoi_broadcast(int vector) "EOI broadcast for vector %d"
> > +ioapic_eoi_delayed_reassert(int vector) "delayed reassert on EOI
> broadcast for vector %d"
> >  ioapic_mem_read(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t
> val) "ioapic mem read addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8"
> retval 0x%"PRIx32
> >  ioapic_mem_write(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t
> val) "ioapic mem write addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8"
> val 0x%"PRIx32
> >  ioapic_set_irq(int vector, int level) "vector: %d level: %d"
> > diff --git a/include/hw/i386/ioapic_internal.h
> b/include/hw/i386/ioapic_internal.h
> > index 9848f39..07002f9 100644
> > --- a/include/hw/i386/ioapic_internal.h
> > +++ b/include/hw/i386/ioapic_internal.h
> > @@ -96,6 +96,7 @@ typedef struct IOAPICCommonClass {
> >      SysBusDeviceClass parent_class;
> >
> >      DeviceRealize realize;
> > +    DeviceUnrealize unrealize;
> >      void (*pre_save)(IOAPICCommonState *s);
> >      void (*post_load)(IOAPICCommonState *s);
> >  } IOAPICCommonClass;
> > @@ -111,6 +112,8 @@ struct IOAPICCommonState {
> >      uint8_t version;
> >      uint64_t irq_count[IOAPIC_NUM_PINS];
> >      int irq_level[IOAPIC_NUM_PINS];
> > +    int irq_eoi[IOAPIC_NUM_PINS];
> > +    QEMUTimer *delayed_ioapic_service_timer;
> >  };
> >
> >  void ioapic_reset_common(DeviceState *dev);
> > --
> > 1.8.3.1
> >
> >
> >
>
>
> --
> Marc-André Lureau
>
>

Re: [Qemu-devel] [PULL 19/21] ioapic: allow buggy guests mishandling level-triggered interrupts to make progress

[Marc-André Lureau]

Hi

On Thu, Jul 4, 2019 at 5:01 PM Li Qiang <liq3ea@gmail.com> wrote:
>
> I have posted a fix for this several weeks ago:
>
> -->https://www.mail-archive.com/qemu-devel@nongnu.org/msg626186.html

Your patch looks reasonable, but I am not really able to review it.

I hope Paolo and Vitaly will take care of it.

Thanks

>
>
> Thanks,
> Li Qiang
>
> Marc-André Lureau <marcandre.lureau@gmail.com> 于2019年7月4日周四 下午8:57写道：
>>
>> Hi
>>
>> On Thu, May 16, 2019 at 1:04 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>> >
>> > From: Vitaly Kuznetsov <vkuznets@redhat.com>
>> >
>> > It was found that Hyper-V 2016 on KVM in some configurations (q35 machine +
>> > piix4-usb-uhci) hangs on boot. Root-cause was that one of Hyper-V
>> > level-triggered interrupt handler performs EOI before fixing the cause of
>> > the interrupt. This results in IOAPIC keep re-raising the level-triggered
>> > interrupt after EOI because irq-line remains asserted.
>> >
>> > Gory details: https://www.spinics.net/lists/kvm/msg184484.html
>> > (the whole thread).
>> >
>> > Turns out we were dealing with similar issues before; in-kernel IOAPIC
>> > implementation has commit 184564efae4d ("kvm: ioapic: conditionally delay
>> > irq delivery duringeoi broadcast") which describes a very similar issue.
>> >
>> > Steal the idea from the above mentioned commit for IOAPIC implementation in
>> > QEMU. SUCCESSIVE_IRQ_MAX_COUNT, delay and the comment are borrowed as well.
>> >
>> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> > Message-Id: <20190402080215.10747-1-vkuznets@redhat.com>
>> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>
>> After this commit, I get the following ASAN error when booting a VM:
>>
>> /home/elmarco/src/qq/hw/intc/ioapic.c:266:27: runtime error: index 41
>> out of bounds for type 'int [24]'
>> =================================================================
>> ==9761==ERROR: AddressSanitizer: heap-buffer-overflow on address
>> 0x61b00000312c at pc 0x55e40b8f9e74 bp 0x7f269c1d32a0 sp
>> 0x7f269c1d3290
>> WRITE of size 4 at 0x61b00000312c thread T4 (CPU 0/KVM)
>>     #0 0x55e40b8f9e73 in ioapic_eoi_broadcast
>> /home/elmarco/src/qq/hw/intc/ioapic.c:266
>>     #1 0x55e40bd3fa4e in kvm_arch_handle_exit
>> /home/elmarco/src/qq/target/i386/kvm.c:3644
>>     #2 0x55e40b7d1fd7 in kvm_cpu_exec
>> /home/elmarco/src/qq/accel/kvm/kvm-all.c:2083
>>     #3 0x55e40b6e435f in qemu_kvm_cpu_thread_fn /home/elmarco/src/qq/cpus.c:1282
>>     #4 0x55e40ce9ba42 in qemu_thread_start
>> /home/elmarco/src/qq/util/qemu-thread-posix.c:502
>>     #5 0x7f26ac3435a1 in start_thread (/lib64/libpthread.so.0+0x85a1)
>>     #6 0x7f26ac270302 in __clone (/lib64/libc.so.6+0xfb302)
>>
>> Address 0x61b00000312c is a wild pointer.
>> SUMMARY: AddressSanitizer: heap-buffer-overflow
>> /home/elmarco/src/qq/hw/intc/ioapic.c:266 in ioapic_eoi_broadcast
>> Shadow bytes around the buggy address:
>>   0x0c367fff85d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>   0x0c367fff85e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>   0x0c367fff85f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>   0x0c367fff8600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>   0x0c367fff8610: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa
>> =>0x0c367fff8620: fa fa fa fa fa[fa]fa fa fa fa fa fa fa fa fa fa
>>   0x0c367fff8630: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>   0x0c367fff8640: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>   0x0c367fff8650: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>   0x0c367fff8660: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>   0x0c367fff8670: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>> Shadow byte legend (one shadow byte represents 8 application bytes):
>>   Addressable:           00
>>   Partially addressable: 01 02 03 04 05 06 07
>>   Heap left redzone:       fa
>>   Freed heap region:       fd
>>   Stack left redzone:      f1
>>   Stack mid redzone:       f2
>>   Stack right redzone:     f3
>>   Stack after return:      f5
>>   Stack use after scope:   f8
>>   Global redzone:          f9
>>   Global init order:       f6
>>   Poisoned by user:        f7
>>   Container overflow:      fc
>>   Array cookie:            ac
>>   Intra object redzone:    bb
>>   ASan internal:           fe
>>   Left alloca redzone:     ca
>>   Right alloca redzone:    cb
>>   Shadow gap:              cc
>> Thread T4 (CPU 0/KVM) created by T0 here:
>>     #0 0x7f26af884965 in pthread_create (/lib64/libasan.so.5+0x3a965)
>>     #1 0x55e40ce9be8b in qemu_thread_create
>> /home/elmarco/src/qq/util/qemu-thread-posix.c:539
>>     #2 0x55e40b6ea347 in qemu_kvm_start_vcpu /home/elmarco/src/qq/cpus.c:2018
>>     #3 0x55e40b6eb03b in qemu_init_vcpu /home/elmarco/src/qq/cpus.c:2084
>>     #4 0x55e40bb52352 in x86_cpu_realizefn
>> /home/elmarco/src/qq/target/i386/cpu.c:5382
>>     #5 0x55e40c017aed in device_set_realized
>> /home/elmarco/src/qq/hw/core/qdev.c:834
>>     #6 0x55e40c95e703 in property_set_bool
>> /home/elmarco/src/qq/qom/object.c:2074
>>     #7 0x55e40c959332 in object_property_set
>> /home/elmarco/src/qq/qom/object.c:1266
>>     #8 0x55e40c961edb in object_property_set_qobject
>> /home/elmarco/src/qq/qom/qom-qobject.c:27
>>     #9 0x55e40c959700 in object_property_set_bool
>> /home/elmarco/src/qq/qom/object.c:1332
>>     #10 0x55e40ba831a3 in pc_new_cpu /home/elmarco/src/qq/hw/i386/pc.c:1531
>>     #11 0x55e40ba838a0 in pc_cpus_init /home/elmarco/src/qq/hw/i386/pc.c:1579
>>     #12 0x55e40ba9f394 in pc_q35_init /home/elmarco/src/qq/hw/i386/pc_q35.c:183
>>     #13 0x55e40baa19ab in pc_init_v4_0 /home/elmarco/src/qq/hw/i386/pc_q35.c:385
>>     #14 0x55e40c035aa7 in machine_run_board_init
>> /home/elmarco/src/qq/hw/core/machine.c:1033
>>     #15 0x55e40bda59d8 in main /home/elmarco/src/qq/vl.c:4494
>>     #16 0x7f26ac198f32 in __libc_start_main (/lib64/libc.so.6+0x23f32)
>>
>> I start investigating.
>>
>> > ---
>> >  hw/intc/ioapic.c                  | 57 +++++++++++++++++++++++++++++++++++----
>> >  hw/intc/trace-events              |  1 +
>> >  include/hw/i386/ioapic_internal.h |  3 +++
>> >  3 files changed, 56 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
>> > index 9d75f84..7074489 100644
>> > --- a/hw/intc/ioapic.c
>> > +++ b/hw/intc/ioapic.c
>> > @@ -139,6 +139,15 @@ static void ioapic_service(IOAPICCommonState *s)
>> >      }
>> >  }
>> >
>> > +#define SUCCESSIVE_IRQ_MAX_COUNT 10000
>> > +
>> > +static void delayed_ioapic_service_cb(void *opaque)
>> > +{
>> > +    IOAPICCommonState *s = opaque;
>> > +
>> > +    ioapic_service(s);
>> > +}
>> > +
>> >  static void ioapic_set_irq(void *opaque, int vector, int level)
>> >  {
>> >      IOAPICCommonState *s = opaque;
>> > @@ -222,13 +231,39 @@ void ioapic_eoi_broadcast(int vector)
>> >          }
>> >          for (n = 0; n < IOAPIC_NUM_PINS; n++) {
>> >              entry = s->ioredtbl[n];
>> > -            if ((entry & IOAPIC_LVT_REMOTE_IRR)
>> > -                && (entry & IOAPIC_VECTOR_MASK) == vector) {
>> > -                trace_ioapic_clear_remote_irr(n, vector);
>> > -                s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
>> > -                if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
>> > +
>> > +            if ((entry & IOAPIC_VECTOR_MASK) != vector ||
>> > +                ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) != IOAPIC_TRIGGER_LEVEL) {
>> > +                continue;
>> > +            }
>> > +
>> > +            if (!(entry & IOAPIC_LVT_REMOTE_IRR)) {
>> > +                continue;
>> > +            }
>> > +
>> > +            trace_ioapic_clear_remote_irr(n, vector);
>> > +            s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
>> > +
>> > +            if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
>> > +                ++s->irq_eoi[vector];
>> > +                if (s->irq_eoi[vector] >= SUCCESSIVE_IRQ_MAX_COUNT) {
>> > +                    /*
>> > +                     * Real hardware does not deliver the interrupt immediately
>> > +                     * during eoi broadcast, and this lets a buggy guest make
>> > +                     * slow progress even if it does not correctly handle a
>> > +                     * level-triggered interrupt. Emulate this behavior if we
>> > +                     * detect an interrupt storm.
>> > +                     */
>> > +                    s->irq_eoi[vector] = 0;
>> > +                    timer_mod_anticipate(s->delayed_ioapic_service_timer,
>> > +                                         qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
>> > +                                         NANOSECONDS_PER_SECOND / 100);
>> > +                    trace_ioapic_eoi_delayed_reassert(vector);
>> > +                } else {
>> >                      ioapic_service(s);
>> >                  }
>> > +            } else {
>> > +                s->irq_eoi[vector] = 0;
>> >              }
>> >          }
>> >      }
>> > @@ -401,6 +436,9 @@ static void ioapic_realize(DeviceState *dev, Error **errp)
>> >      memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
>> >                            "ioapic", 0x1000);
>> >
>> > +    s->delayed_ioapic_service_timer =
>> > +        timer_new_ns(QEMU_CLOCK_VIRTUAL, delayed_ioapic_service_cb, s);
>> > +
>> >      qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS);
>> >
>> >      ioapics[ioapic_no] = s;
>> > @@ -408,6 +446,14 @@ static void ioapic_realize(DeviceState *dev, Error **errp)
>> >      qemu_add_machine_init_done_notifier(&s->machine_done);
>> >  }
>> >
>> > +static void ioapic_unrealize(DeviceState *dev, Error **errp)
>> > +{
>> > +    IOAPICCommonState *s = IOAPIC_COMMON(dev);
>> > +
>> > +    timer_del(s->delayed_ioapic_service_timer);
>> > +    timer_free(s->delayed_ioapic_service_timer);
>> > +}
>> > +
>> >  static Property ioapic_properties[] = {
>> >      DEFINE_PROP_UINT8("version", IOAPICCommonState, version, IOAPIC_VER_DEF),
>> >      DEFINE_PROP_END_OF_LIST(),
>> > @@ -419,6 +465,7 @@ static void ioapic_class_init(ObjectClass *klass, void *data)
>> >      DeviceClass *dc = DEVICE_CLASS(klass);
>> >
>> >      k->realize = ioapic_realize;
>> > +    k->unrealize = ioapic_unrealize;
>> >      /*
>> >       * If APIC is in kernel, we need to update the kernel cache after
>> >       * migration, otherwise first 24 gsi routes will be invalid.
>> > diff --git a/hw/intc/trace-events b/hw/intc/trace-events
>> > index a28bdce..90c9d07 100644
>> > --- a/hw/intc/trace-events
>> > +++ b/hw/intc/trace-events
>> > @@ -25,6 +25,7 @@ apic_mem_writel(uint64_t addr, uint32_t val) "0x%"PRIx64" = 0x%08x"
>> >  ioapic_set_remote_irr(int n) "set remote irr for pin %d"
>> >  ioapic_clear_remote_irr(int n, int vector) "clear remote irr for pin %d vector %d"
>> >  ioapic_eoi_broadcast(int vector) "EOI broadcast for vector %d"
>> > +ioapic_eoi_delayed_reassert(int vector) "delayed reassert on EOI broadcast for vector %d"
>> >  ioapic_mem_read(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapic mem read addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8" retval 0x%"PRIx32
>> >  ioapic_mem_write(uint8_t addr, uint8_t regsel, uint8_t size, uint32_t val) "ioapic mem write addr 0x%"PRIx8" regsel: 0x%"PRIx8" size 0x%"PRIx8" val 0x%"PRIx32
>> >  ioapic_set_irq(int vector, int level) "vector: %d level: %d"
>> > diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h
>> > index 9848f39..07002f9 100644
>> > --- a/include/hw/i386/ioapic_internal.h
>> > +++ b/include/hw/i386/ioapic_internal.h
>> > @@ -96,6 +96,7 @@ typedef struct IOAPICCommonClass {
>> >      SysBusDeviceClass parent_class;
>> >
>> >      DeviceRealize realize;
>> > +    DeviceUnrealize unrealize;
>> >      void (*pre_save)(IOAPICCommonState *s);
>> >      void (*post_load)(IOAPICCommonState *s);
>> >  } IOAPICCommonClass;
>> > @@ -111,6 +112,8 @@ struct IOAPICCommonState {
>> >      uint8_t version;
>> >      uint64_t irq_count[IOAPIC_NUM_PINS];
>> >      int irq_level[IOAPIC_NUM_PINS];
>> > +    int irq_eoi[IOAPIC_NUM_PINS];
>> > +    QEMUTimer *delayed_ioapic_service_timer;
>> >  };
>> >
>> >  void ioapic_reset_common(DeviceState *dev);
>> > --
>> > 1.8.3.1
>> >
>> >
>> >
>>
>>
>> --
>> Marc-André Lureau
>>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PULL 19/21] ioapic: allow buggy guests mishandling level-triggered interrupts to make progress

[Paolo Bonzini]

On 04/07/19 15:05, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Jul 4, 2019 at 5:01 PM Li Qiang <liq3ea@gmail.com> wrote:
>>
>> I have posted a fix for this several weeks ago:
>>
>> -->https://www.mail-archive.com/qemu-devel@nongnu.org/msg626186.html
> 
> Your patch looks reasonable, but I am not really able to review it.
> 
> I hope Paolo and Vitaly will take care of it.

I have queued the patch, thanks Marc-André and Li Qiang.

Paolo

[Qemu-devel] [PULL 20/21] hw/char: Move multi-serial devices into separate file

[Paolo Bonzini]

From: Thomas Huth <thuth@redhat.com>

In our downstream distribution of QEMU, we'd like to ship the binary
without the multi-serial PCI devices. To make this disablement easier,
let's move the devices into a separate file and add a proper Kconfig-
switch for these devices.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <1554036028-31410-1-git-send-email-thuth@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/char/Kconfig            |   6 ++
 hw/char/Makefile.objs      |   1 +
 hw/char/serial-pci-multi.c | 208 +++++++++++++++++++++++++++++++++++++++++++++
 hw/char/serial-pci.c       | 170 ------------------------------------
 4 files changed, 215 insertions(+), 170 deletions(-)
 create mode 100644 hw/char/serial-pci-multi.c

diff --git a/hw/char/Kconfig b/hw/char/Kconfig
index 6360c9f..40e7a8b 100644
--- a/hw/char/Kconfig
+++ b/hw/char/Kconfig
@@ -24,6 +24,12 @@ config SERIAL_PCI
     depends on PCI
     select SERIAL
 
+config SERIAL_PCI_MULTI
+    bool
+    default y if PCI_DEVICES
+    depends on PCI
+    select SERIAL
+
 config VIRTIO_SERIAL
     bool
     default y
diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index cf086e7..02d8a66 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -7,6 +7,7 @@ common-obj-$(CONFIG_PL011) += pl011.o
 common-obj-$(CONFIG_SERIAL) += serial.o
 common-obj-$(CONFIG_SERIAL_ISA) += serial-isa.o
 common-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o
+common-obj-$(CONFIG_SERIAL_PCI_MULTI) += serial-pci-multi.o
 common-obj-$(CONFIG_VIRTIO_SERIAL) += virtio-console.o
 common-obj-$(CONFIG_XILINX) += xilinx_uartlite.o
 common-obj-$(CONFIG_XEN) += xen_console.o
diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
new file mode 100644
index 0000000..63dcbaa
--- /dev/null
+++ b/hw/char/serial-pci-multi.c
@@ -0,0 +1,208 @@
+/*
+ * QEMU 16550A multi UART emulation
+ *
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2008 Citrix Systems, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/* see docs/specs/pci-serial.txt */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/char/serial.h"
+#include "hw/pci/pci.h"
+
+#define PCI_SERIAL_MAX_PORTS 4
+
+typedef struct PCIMultiSerialState {
+    PCIDevice    dev;
+    MemoryRegion iobar;
+    uint32_t     ports;
+    char         *name[PCI_SERIAL_MAX_PORTS];
+    SerialState  state[PCI_SERIAL_MAX_PORTS];
+    uint32_t     level[PCI_SERIAL_MAX_PORTS];
+    qemu_irq     *irqs;
+    uint8_t      prog_if;
+} PCIMultiSerialState;
+
+static void multi_serial_pci_exit(PCIDevice *dev)
+{
+    PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
+    SerialState *s;
+    int i;
+
+    for (i = 0; i < pci->ports; i++) {
+        s = pci->state + i;
+        serial_exit_core(s);
+        memory_region_del_subregion(&pci->iobar, &s->io);
+        g_free(pci->name[i]);
+    }
+    qemu_free_irqs(pci->irqs, pci->ports);
+}
+
+static void multi_serial_irq_mux(void *opaque, int n, int level)
+{
+    PCIMultiSerialState *pci = opaque;
+    int i, pending = 0;
+
+    pci->level[n] = level;
+    for (i = 0; i < pci->ports; i++) {
+        if (pci->level[i]) {
+            pending = 1;
+        }
+    }
+    pci_set_irq(&pci->dev, pending);
+}
+
+static void multi_serial_pci_realize(PCIDevice *dev, Error **errp)
+{
+    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
+    PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
+    SerialState *s;
+    Error *err = NULL;
+    int i, nr_ports = 0;
+
+    switch (pc->device_id) {
+    case 0x0003:
+        nr_ports = 2;
+        break;
+    case 0x0004:
+        nr_ports = 4;
+        break;
+    }
+    assert(nr_ports > 0);
+    assert(nr_ports <= PCI_SERIAL_MAX_PORTS);
+
+    pci->dev.config[PCI_CLASS_PROG] = pci->prog_if;
+    pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
+    memory_region_init(&pci->iobar, OBJECT(pci), "multiserial", 8 * nr_ports);
+    pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar);
+    pci->irqs = qemu_allocate_irqs(multi_serial_irq_mux, pci,
+                                   nr_ports);
+
+    for (i = 0; i < nr_ports; i++) {
+        s = pci->state + i;
+        s->baudbase = 115200;
+        serial_realize_core(s, &err);
+        if (err != NULL) {
+            error_propagate(errp, err);
+            multi_serial_pci_exit(dev);
+            return;
+        }
+        s->irq = pci->irqs[i];
+        pci->name[i] = g_strdup_printf("uart #%d", i + 1);
+        memory_region_init_io(&s->io, OBJECT(pci), &serial_io_ops, s,
+                              pci->name[i], 8);
+        memory_region_add_subregion(&pci->iobar, 8 * i, &s->io);
+        pci->ports++;
+    }
+}
+
+static const VMStateDescription vmstate_pci_multi_serial = {
+    .name = "pci-serial-multi",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(dev, PCIMultiSerialState),
+        VMSTATE_STRUCT_ARRAY(state, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS,
+                             0, vmstate_serial, SerialState),
+        VMSTATE_UINT32_ARRAY(level, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property multi_2x_serial_pci_properties[] = {
+    DEFINE_PROP_CHR("chardev1",  PCIMultiSerialState, state[0].chr),
+    DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
+    DEFINE_PROP_UINT8("prog_if",  PCIMultiSerialState, prog_if, 0x02),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static Property multi_4x_serial_pci_properties[] = {
+    DEFINE_PROP_CHR("chardev1",  PCIMultiSerialState, state[0].chr),
+    DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
+    DEFINE_PROP_CHR("chardev3",  PCIMultiSerialState, state[2].chr),
+    DEFINE_PROP_CHR("chardev4",  PCIMultiSerialState, state[3].chr),
+    DEFINE_PROP_UINT8("prog_if",  PCIMultiSerialState, prog_if, 0x02),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void multi_2x_serial_pci_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+    pc->realize = multi_serial_pci_realize;
+    pc->exit = multi_serial_pci_exit;
+    pc->vendor_id = PCI_VENDOR_ID_REDHAT;
+    pc->device_id = PCI_DEVICE_ID_REDHAT_SERIAL2;
+    pc->revision = 1;
+    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
+    dc->vmsd = &vmstate_pci_multi_serial;
+    dc->props = multi_2x_serial_pci_properties;
+    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+}
+
+static void multi_4x_serial_pci_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+    pc->realize = multi_serial_pci_realize;
+    pc->exit = multi_serial_pci_exit;
+    pc->vendor_id = PCI_VENDOR_ID_REDHAT;
+    pc->device_id = PCI_DEVICE_ID_REDHAT_SERIAL4;
+    pc->revision = 1;
+    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
+    dc->vmsd = &vmstate_pci_multi_serial;
+    dc->props = multi_4x_serial_pci_properties;
+    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+}
+
+static const TypeInfo multi_2x_serial_pci_info = {
+    .name          = "pci-serial-2x",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCIMultiSerialState),
+    .class_init    = multi_2x_serial_pci_class_initfn,
+    .interfaces = (InterfaceInfo[]) {
+        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { },
+    },
+};
+
+static const TypeInfo multi_4x_serial_pci_info = {
+    .name          = "pci-serial-4x",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCIMultiSerialState),
+    .class_init    = multi_4x_serial_pci_class_initfn,
+    .interfaces = (InterfaceInfo[]) {
+        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { },
+    },
+};
+
+static void multi_serial_pci_register_types(void)
+{
+    type_register_static(&multi_2x_serial_pci_info);
+    type_register_static(&multi_4x_serial_pci_info);
+}
+
+type_init(multi_serial_pci_register_types)
diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
index cb0d04c..2d5ffae 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -30,26 +30,12 @@
 #include "hw/char/serial.h"
 #include "hw/pci/pci.h"
 
-#define PCI_SERIAL_MAX_PORTS 4
-
 typedef struct PCISerialState {
     PCIDevice dev;
     SerialState state;
     uint8_t prog_if;
 } PCISerialState;
 
-typedef struct PCIMultiSerialState {
-    PCIDevice    dev;
-    MemoryRegion iobar;
-    uint32_t     ports;
-    char         *name[PCI_SERIAL_MAX_PORTS];
-    SerialState  state[PCI_SERIAL_MAX_PORTS];
-    uint32_t     level[PCI_SERIAL_MAX_PORTS];
-    qemu_irq     *irqs;
-    uint8_t      prog_if;
-} PCIMultiSerialState;
-
-static void multi_serial_pci_exit(PCIDevice *dev);
 
 static void serial_pci_realize(PCIDevice *dev, Error **errp)
 {
@@ -72,64 +58,6 @@ static void serial_pci_realize(PCIDevice *dev, Error **errp)
     pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
 }
 
-static void multi_serial_irq_mux(void *opaque, int n, int level)
-{
-    PCIMultiSerialState *pci = opaque;
-    int i, pending = 0;
-
-    pci->level[n] = level;
-    for (i = 0; i < pci->ports; i++) {
-        if (pci->level[i]) {
-            pending = 1;
-        }
-    }
-    pci_set_irq(&pci->dev, pending);
-}
-
-static void multi_serial_pci_realize(PCIDevice *dev, Error **errp)
-{
-    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
-    PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
-    SerialState *s;
-    Error *err = NULL;
-    int i, nr_ports = 0;
-
-    switch (pc->device_id) {
-    case 0x0003:
-        nr_ports = 2;
-        break;
-    case 0x0004:
-        nr_ports = 4;
-        break;
-    }
-    assert(nr_ports > 0);
-    assert(nr_ports <= PCI_SERIAL_MAX_PORTS);
-
-    pci->dev.config[PCI_CLASS_PROG] = pci->prog_if;
-    pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
-    memory_region_init(&pci->iobar, OBJECT(pci), "multiserial", 8 * nr_ports);
-    pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar);
-    pci->irqs = qemu_allocate_irqs(multi_serial_irq_mux, pci,
-                                   nr_ports);
-
-    for (i = 0; i < nr_ports; i++) {
-        s = pci->state + i;
-        s->baudbase = 115200;
-        serial_realize_core(s, &err);
-        if (err != NULL) {
-            error_propagate(errp, err);
-            multi_serial_pci_exit(dev);
-            return;
-        }
-        s->irq = pci->irqs[i];
-        pci->name[i] = g_strdup_printf("uart #%d", i+1);
-        memory_region_init_io(&s->io, OBJECT(pci), &serial_io_ops, s,
-                              pci->name[i], 8);
-        memory_region_add_subregion(&pci->iobar, 8 * i, &s->io);
-        pci->ports++;
-    }
-}
-
 static void serial_pci_exit(PCIDevice *dev)
 {
     PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
@@ -139,21 +67,6 @@ static void serial_pci_exit(PCIDevice *dev)
     qemu_free_irq(s->irq);
 }
 
-static void multi_serial_pci_exit(PCIDevice *dev)
-{
-    PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
-    SerialState *s;
-    int i;
-
-    for (i = 0; i < pci->ports; i++) {
-        s = pci->state + i;
-        serial_exit_core(s);
-        memory_region_del_subregion(&pci->iobar, &s->io);
-        g_free(pci->name[i]);
-    }
-    qemu_free_irqs(pci->irqs, pci->ports);
-}
-
 static const VMStateDescription vmstate_pci_serial = {
     .name = "pci-serial",
     .version_id = 1,
@@ -165,41 +78,12 @@ static const VMStateDescription vmstate_pci_serial = {
     }
 };
 
-static const VMStateDescription vmstate_pci_multi_serial = {
-    .name = "pci-serial-multi",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .fields = (VMStateField[]) {
-        VMSTATE_PCI_DEVICE(dev, PCIMultiSerialState),
-        VMSTATE_STRUCT_ARRAY(state, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS,
-                             0, vmstate_serial, SerialState),
-        VMSTATE_UINT32_ARRAY(level, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 static Property serial_pci_properties[] = {
     DEFINE_PROP_CHR("chardev",  PCISerialState, state.chr),
     DEFINE_PROP_UINT8("prog_if",  PCISerialState, prog_if, 0x02),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static Property multi_2x_serial_pci_properties[] = {
-    DEFINE_PROP_CHR("chardev1",  PCIMultiSerialState, state[0].chr),
-    DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
-    DEFINE_PROP_UINT8("prog_if",  PCIMultiSerialState, prog_if, 0x02),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static Property multi_4x_serial_pci_properties[] = {
-    DEFINE_PROP_CHR("chardev1",  PCIMultiSerialState, state[0].chr),
-    DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
-    DEFINE_PROP_CHR("chardev3",  PCIMultiSerialState, state[2].chr),
-    DEFINE_PROP_CHR("chardev4",  PCIMultiSerialState, state[3].chr),
-    DEFINE_PROP_UINT8("prog_if",  PCIMultiSerialState, prog_if, 0x02),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void serial_pci_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -215,36 +99,6 @@ static void serial_pci_class_initfn(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
 
-static void multi_2x_serial_pci_class_initfn(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
-    pc->realize = multi_serial_pci_realize;
-    pc->exit = multi_serial_pci_exit;
-    pc->vendor_id = PCI_VENDOR_ID_REDHAT;
-    pc->device_id = PCI_DEVICE_ID_REDHAT_SERIAL2;
-    pc->revision = 1;
-    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
-    dc->vmsd = &vmstate_pci_multi_serial;
-    dc->props = multi_2x_serial_pci_properties;
-    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
-}
-
-static void multi_4x_serial_pci_class_initfn(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
-    pc->realize = multi_serial_pci_realize;
-    pc->exit = multi_serial_pci_exit;
-    pc->vendor_id = PCI_VENDOR_ID_REDHAT;
-    pc->device_id = PCI_DEVICE_ID_REDHAT_SERIAL4;
-    pc->revision = 1;
-    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
-    dc->vmsd = &vmstate_pci_multi_serial;
-    dc->props = multi_4x_serial_pci_properties;
-    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
-}
-
 static const TypeInfo serial_pci_info = {
     .name          = "pci-serial",
     .parent        = TYPE_PCI_DEVICE,
@@ -256,33 +110,9 @@ static const TypeInfo serial_pci_info = {
     },
 };
 
-static const TypeInfo multi_2x_serial_pci_info = {
-    .name          = "pci-serial-2x",
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PCIMultiSerialState),
-    .class_init    = multi_2x_serial_pci_class_initfn,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-        { },
-    },
-};
-
-static const TypeInfo multi_4x_serial_pci_info = {
-    .name          = "pci-serial-4x",
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PCIMultiSerialState),
-    .class_init    = multi_4x_serial_pci_class_initfn,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-        { },
-    },
-};
-
 static void serial_pci_register_types(void)
 {
     type_register_static(&serial_pci_info);
-    type_register_static(&multi_2x_serial_pci_info);
-    type_register_static(&multi_4x_serial_pci_info);
 }
 
 type_init(serial_pci_register_types)
-- 
1.8.3.1

[Qemu-devel] [PULL 21/21] hw/net/ne2000: Extract the PCI device from the chipset common code

[Paolo Bonzini]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The ne2000.c file contains functions common the the ISA and PCI
devices. To allow to build with one or another, extract the PCI
specific part into a new file.

This fix an issue where the NE2000_ISA Kconfig had to pull the
full PCI core objects.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190504123538.14952-1-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/net/Kconfig       |   7 ++-
 hw/net/Makefile.objs |   3 +-
 hw/net/ne2000-pci.c  | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/net/ne2000.c      | 105 ----------------------------------------
 4 files changed, 139 insertions(+), 108 deletions(-)
 create mode 100644 hw/net/ne2000-pci.c

diff --git a/hw/net/Kconfig b/hw/net/Kconfig
index 7d7bbc5..4ef86dc 100644
--- a/hw/net/Kconfig
+++ b/hw/net/Kconfig
@@ -1,10 +1,14 @@
 config DP8393X
     bool
 
+config NE2000_COMMON
+    bool
+
 config NE2000_PCI
     bool
     default y if PCI_DEVICES
     depends on PCI
+    select NE2000_COMMON
 
 config EEPRO100_PCI
     bool
@@ -51,8 +55,7 @@ config NE2000_ISA
     bool
     default y
     depends on ISA_BUS
-    depends on PCI # for NE2000State
-    select NE2000_PCI
+    select NE2000_COMMON
 
 config OPENCORES_ETH
     bool
diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
index ea63715..9904273 100644
--- a/hw/net/Makefile.objs
+++ b/hw/net/Makefile.objs
@@ -1,8 +1,9 @@
 common-obj-$(CONFIG_DP8393X) += dp8393x.o
 common-obj-$(CONFIG_XEN) += xen_nic.o
+common-obj-$(CONFIG_NE2000_COMMON) += ne2000.o
 
 # PCI network cards
-common-obj-$(CONFIG_NE2000_PCI) += ne2000.o
+common-obj-$(CONFIG_NE2000_PCI) += ne2000-pci.o
 common-obj-$(CONFIG_EEPRO100_PCI) += eepro100.o
 common-obj-$(CONFIG_PCNET_PCI) += pcnet-pci.o
 common-obj-$(CONFIG_PCNET_COMMON) += pcnet.o
diff --git a/hw/net/ne2000-pci.c b/hw/net/ne2000-pci.c
new file mode 100644
index 0000000..cb05744
--- /dev/null
+++ b/hw/net/ne2000-pci.c
@@ -0,0 +1,132 @@
+/*
+ * QEMU NE2000 emulation (PCI bus)
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "ne2000.h"
+#include "sysemu/sysemu.h"
+
+typedef struct PCINE2000State {
+    PCIDevice dev;
+    NE2000State ne2000;
+} PCINE2000State;
+
+static const VMStateDescription vmstate_pci_ne2000 = {
+    .name = "ne2000",
+    .version_id = 3,
+    .minimum_version_id = 3,
+    .fields = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(dev, PCINE2000State),
+        VMSTATE_STRUCT(ne2000, PCINE2000State, 0, vmstate_ne2000, NE2000State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static NetClientInfo net_ne2000_info = {
+    .type = NET_CLIENT_DRIVER_NIC,
+    .size = sizeof(NICState),
+    .receive = ne2000_receive,
+};
+
+static void pci_ne2000_realize(PCIDevice *pci_dev, Error **errp)
+{
+    PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
+    NE2000State *s;
+    uint8_t *pci_conf;
+
+    pci_conf = d->dev.config;
+    pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin A */
+
+    s = &d->ne2000;
+    ne2000_setup_io(s, DEVICE(pci_dev), 0x100);
+    pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
+    s->irq = pci_allocate_irq(&d->dev);
+
+    qemu_macaddr_default_if_unset(&s->c.macaddr);
+    ne2000_reset(s);
+
+    s->nic = qemu_new_nic(&net_ne2000_info, &s->c,
+                          object_get_typename(OBJECT(pci_dev)),
+                          pci_dev->qdev.id, s);
+    qemu_format_nic_info_str(qemu_get_queue(s->nic), s->c.macaddr.a);
+}
+
+static void pci_ne2000_exit(PCIDevice *pci_dev)
+{
+    PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
+    NE2000State *s = &d->ne2000;
+
+    qemu_del_nic(s->nic);
+    qemu_free_irq(s->irq);
+}
+
+static void ne2000_instance_init(Object *obj)
+{
+    PCIDevice *pci_dev = PCI_DEVICE(obj);
+    PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
+    NE2000State *s = &d->ne2000;
+
+    device_add_bootindex_property(obj, &s->c.bootindex,
+                                  "bootindex", "/ethernet-phy@0",
+                                  &pci_dev->qdev, NULL);
+}
+
+static Property ne2000_properties[] = {
+    DEFINE_NIC_PROPERTIES(PCINE2000State, ne2000.c),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ne2000_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->realize = pci_ne2000_realize;
+    k->exit = pci_ne2000_exit;
+    k->romfile = "efi-ne2k_pci.rom",
+    k->vendor_id = PCI_VENDOR_ID_REALTEK;
+    k->device_id = PCI_DEVICE_ID_REALTEK_8029;
+    k->class_id = PCI_CLASS_NETWORK_ETHERNET;
+    dc->vmsd = &vmstate_pci_ne2000;
+    dc->props = ne2000_properties;
+    set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
+}
+
+static const TypeInfo ne2000_info = {
+    .name          = "ne2k_pci",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCINE2000State),
+    .class_init    = ne2000_class_init,
+    .instance_init = ne2000_instance_init,
+    .interfaces = (InterfaceInfo[]) {
+        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { },
+    },
+};
+
+static void ne2000_register_types(void)
+{
+    type_register_static(&ne2000_info);
+}
+
+type_init(ne2000_register_types)
diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index 037afc8..ca792d9 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -22,7 +22,6 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
-#include "hw/pci/pci.h"
 #include "net/eth.h"
 #include "ne2000.h"
 #include "sysemu/sysemu.h"
@@ -118,11 +117,6 @@
 #define ENTSR_CDH 0x40	/* The collision detect "heartbeat" signal was lost. */
 #define ENTSR_OWC 0x80  /* There was an out-of-window collision. */
 
-typedef struct PCINE2000State {
-    PCIDevice dev;
-    NE2000State ne2000;
-} PCINE2000State;
-
 void ne2000_reset(NE2000State *s)
 {
     int i;
@@ -644,17 +638,6 @@ const VMStateDescription vmstate_ne2000 = {
     }
 };
 
-static const VMStateDescription vmstate_pci_ne2000 = {
-    .name = "ne2000",
-    .version_id = 3,
-    .minimum_version_id = 3,
-    .fields = (VMStateField[]) {
-        VMSTATE_PCI_DEVICE(dev, PCINE2000State),
-        VMSTATE_STRUCT(ne2000, PCINE2000State, 0, vmstate_ne2000, NE2000State),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 static uint64_t ne2000_read(void *opaque, hwaddr addr,
                             unsigned size)
 {
@@ -711,91 +694,3 @@ void ne2000_setup_io(NE2000State *s, DeviceState *dev, unsigned size)
 {
     memory_region_init_io(&s->io, OBJECT(dev), &ne2000_ops, s, "ne2000", size);
 }
-
-static NetClientInfo net_ne2000_info = {
-    .type = NET_CLIENT_DRIVER_NIC,
-    .size = sizeof(NICState),
-    .receive = ne2000_receive,
-};
-
-static void pci_ne2000_realize(PCIDevice *pci_dev, Error **errp)
-{
-    PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
-    NE2000State *s;
-    uint8_t *pci_conf;
-
-    pci_conf = d->dev.config;
-    pci_conf[PCI_INTERRUPT_PIN] = 1; /* interrupt pin A */
-
-    s = &d->ne2000;
-    ne2000_setup_io(s, DEVICE(pci_dev), 0x100);
-    pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
-    s->irq = pci_allocate_irq(&d->dev);
-
-    qemu_macaddr_default_if_unset(&s->c.macaddr);
-    ne2000_reset(s);
-
-    s->nic = qemu_new_nic(&net_ne2000_info, &s->c,
-                          object_get_typename(OBJECT(pci_dev)), pci_dev->qdev.id, s);
-    qemu_format_nic_info_str(qemu_get_queue(s->nic), s->c.macaddr.a);
-}
-
-static void pci_ne2000_exit(PCIDevice *pci_dev)
-{
-    PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
-    NE2000State *s = &d->ne2000;
-
-    qemu_del_nic(s->nic);
-    qemu_free_irq(s->irq);
-}
-
-static void ne2000_instance_init(Object *obj)
-{
-    PCIDevice *pci_dev = PCI_DEVICE(obj);
-    PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
-    NE2000State *s = &d->ne2000;
-
-    device_add_bootindex_property(obj, &s->c.bootindex,
-                                  "bootindex", "/ethernet-phy@0",
-                                  &pci_dev->qdev, NULL);
-}
-
-static Property ne2000_properties[] = {
-    DEFINE_NIC_PROPERTIES(PCINE2000State, ne2000.c),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void ne2000_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    k->realize = pci_ne2000_realize;
-    k->exit = pci_ne2000_exit;
-    k->romfile = "efi-ne2k_pci.rom",
-    k->vendor_id = PCI_VENDOR_ID_REALTEK;
-    k->device_id = PCI_DEVICE_ID_REALTEK_8029;
-    k->class_id = PCI_CLASS_NETWORK_ETHERNET;
-    dc->vmsd = &vmstate_pci_ne2000;
-    dc->props = ne2000_properties;
-    set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
-}
-
-static const TypeInfo ne2000_info = {
-    .name          = "ne2k_pci",
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PCINE2000State),
-    .class_init    = ne2000_class_init,
-    .instance_init = ne2000_instance_init,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-        { },
-    },
-};
-
-static void ne2000_register_types(void)
-{
-    type_register_static(&ne2000_info);
-}
-
-type_init(ne2000_register_types)
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 00/21] Misc patches for 2019-05-15

[Peter Maydell]

On Wed, 15 May 2019 at 21:52, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit e329ad2ab72c43b56df88b34954c2c7d839bb373:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20190513' into staging (2019-05-14 10:08:47 +0100)
>
> are available in the git repository at:
>
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 88f1090e9020057022ac04531ca87d25f67f57ca:
>
>   hw/net/ne2000: Extract the PCI device from the chipset common code (2019-05-15 11:56:54 +0200)
>
> ----------------------------------------------------------------
> Mostly bugfixes and cleanups, the most important being
> "megasas: fix mapped frame size" from Peter Lieven.
> In addition, -realtime is marked as deprecated.
>
> ----------------------------------------------------------------
> Chen Zhang via Qemu-devel (1):
>       hvf: Add missing break statement

Hi -- looks like this commit needs its author
email tidying up so it isn't attributed to the mailing list.

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/21] Misc patches for 2019-05-15

[Paolo Bonzini]

On 16/05/19 14:14, Peter Maydell wrote:
>> Chen Zhang via Qemu-devel (1):
>>       hvf: Add missing break statement
> Hi -- looks like this commit needs its author
> email tidying up so it isn't attributed to the mailing list.

Oops, it's the first time I see this.  I'll fix and resend.

Paolo