[PATCH 0/5] x86: allow building without PC machine types

[PATCH 0/5] x86: allow building without PC machine types

[Paolo Bonzini]

With this series it is possible to build QEMU with this configuration:

   CONFIG_MICROVM=y
   CONFIG_SERIAL_ISA=y
   CONFIG_VIRTIO=y
   CONFIG_VIRTIO_BALLOON=y
   CONFIG_VIRTIO_BLK=y
   CONFIG_VIRTIO_NET=y
   CONFIG_VIRTIO_RNG=y
   CONFIG_VIRTIO_SCSI=y
   CONFIG_VIRTIO_SERIAL=y
   CONFIG_WDT_IB700=y
   CONFIG_I8254=y

Thanks,

Paolo

Paolo Bonzini (5):
  i386: conditionally compile more files
  fw_cfg: allow building without other devices
  x86: move more functions to X86MachineState and x86.c
  pci-stub: add more MSI functions
  kvm: check before accessing PCMachineState fields

 hw/i386/Makefile.objs     |   6 +--
 hw/i386/fw_cfg.c          |   7 ++++
 hw/i386/kvm/Makefile.objs |   6 ++-
 hw/i386/pc.c              | 101 ----------------------------------------------
 hw/i386/x86.c             | 101 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/pci/pci-stub.c         |  27 +++++++++++++
 include/hw/i386/pc.h      |  12 ------
 include/hw/i386/x86.h     |  13 ++++++
 roms/SLOF                 |   2 +-
 target/i386/kvm.c         |  11 ++++-
 10 files changed, 166 insertions(+), 120 deletions(-)

-- 
1.8.3.1

[PATCH 1/5] i386: conditionally compile more files

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/Makefile.objs     | 6 +++---
 hw/i386/kvm/Makefile.objs | 6 +++++-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 0d195b5..01ae202 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -1,11 +1,11 @@
 obj-$(CONFIG_KVM) += kvm/
 obj-y += e820_memory_layout.o multiboot.o
 obj-y += x86.o
-obj-y += pc.o
+obj-$(CONFIG_PC) += pc.o pc_sysfw.o
 obj-$(CONFIG_I440FX) += pc_piix.o
 obj-$(CONFIG_Q35) += pc_q35.o
 obj-$(CONFIG_MICROVM) += microvm.o
-obj-y += fw_cfg.o pc_sysfw.o
+obj-y += fw_cfg.o
 obj-y += x86-iommu.o
 obj-$(CONFIG_VTD) += intel_iommu.o
 obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o
@@ -14,4 +14,4 @@ obj-$(CONFIG_VMPORT) += vmport.o
 obj-$(CONFIG_VMMOUSE) += vmmouse.o
 
 obj-y += kvmvapic.o
-obj-y += acpi-build.o
+obj-$(CONFIG_PC) += acpi-build.o
diff --git a/hw/i386/kvm/Makefile.objs b/hw/i386/kvm/Makefile.objs
index 4224ed9..0c8d5f2 100644
--- a/hw/i386/kvm/Makefile.objs
+++ b/hw/i386/kvm/Makefile.objs
@@ -1 +1,5 @@
-obj-y += clock.o apic.o i8259.o ioapic.o i8254.o
+obj-y += clock.o
+obj-$(CONFIG_APIC) += apic.o
+obj-$(CONFIG_IOAPIC) += ioapic.o
+obj-$(CONFIG_I8254) += i8254.o
+obj-$(CONFIG_I8259) += i8259.o
-- 
1.8.3.1

Re: [PATCH 1/5] i386: conditionally compile more files

[Sergio Lopez]

[-- Attachment #1: Type: text/plain, Size: 281 bytes --]


Paolo Bonzini <pbonzini@redhat.com> writes:

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/i386/Makefile.objs     | 6 +++---
>  hw/i386/kvm/Makefile.objs | 6 +++++-
>  2 files changed, 8 insertions(+), 4 deletions(-)

Reviewed-by: Sergio Lopez <slp@redhat.com>

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

[PATCH 2/5] fw_cfg: allow building without other devices

[Paolo Bonzini]

The microvm machine type uses fw_cfg but lacks SMBIOS and ACPI.  Do not include
the files if the symbol is not present in QEMU and remove dependencies on
machine-specific files.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/fw_cfg.c | 7 +++++++
 hw/i386/pc.c     | 2 --
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index 39b6bc6..326e33e 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -22,6 +22,9 @@
 #include "hw/nvram/fw_cfg.h"
 #include "e820_memory_layout.h"
 #include "kvm_i386.h"
+#include "config-devices.h"
+
+struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
 
 const char *fw_cfg_arch_key_name(uint16_t key)
 {
@@ -46,6 +49,7 @@ const char *fw_cfg_arch_key_name(uint16_t key)
 
 void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
 {
+#ifdef CONFIG_SMBIOS
     uint8_t *smbios_tables, *smbios_anchor;
     size_t smbios_tables_len, smbios_anchor_len;
     struct smbios_phys_mem_area *mem_array;
@@ -83,6 +87,7 @@ void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
         fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-anchor",
                         smbios_anchor, smbios_anchor_len);
     }
+#endif
 }
 
 FWCfgState *fw_cfg_arch_create(MachineState *ms,
@@ -114,8 +119,10 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
      */
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, apic_id_limit);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+#ifdef CONFIG_ACPI
     fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
                      acpi_tables, acpi_tables_len);
+#endif
     fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
 
     fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b9f3b38..3d2c5d8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -101,8 +101,6 @@
 #define DPRINTF(fmt, ...)
 #endif
 
-struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
-
 GlobalProperty pc_compat_4_1[] = {};
 const size_t pc_compat_4_1_len = G_N_ELEMENTS(pc_compat_4_1);
 
-- 
1.8.3.1

Re: [PATCH 2/5] fw_cfg: allow building without other devices

[Sergio Lopez]

[-- Attachment #1: Type: text/plain, Size: 446 bytes --]


Paolo Bonzini <pbonzini@redhat.com> writes:

> The microvm machine type uses fw_cfg but lacks SMBIOS and ACPI.  Do not include
> the files if the symbol is not present in QEMU and remove dependencies on
> machine-specific files.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/i386/fw_cfg.c | 7 +++++++
>  hw/i386/pc.c     | 2 --
>  2 files changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Sergio Lopez <slp@redhat.com>

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

[PATCH 3/5] x86: move more functions to X86MachineState and x86.c

[Paolo Bonzini]

These are needed by microvm too, so move them outside of PC-specific files.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/pc.c          |  99 -------------------------------------------------
 hw/i386/x86.c         | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/pc.h  |  12 ------
 include/hw/i386/x86.h |  13 +++++++
 4 files changed, 114 insertions(+), 111 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3d2c5d8..308248d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -342,17 +342,6 @@ GlobalProperty pc_compat_1_4[] = {
 };
 const size_t pc_compat_1_4_len = G_N_ELEMENTS(pc_compat_1_4);
 
-void gsi_handler(void *opaque, int n, int level)
-{
-    GSIState *s = opaque;
-
-    DPRINTF("pc: %s GSI %d\n", level ? "raising" : "lowering", n);
-    if (n < ISA_NUM_IRQS) {
-        qemu_set_irq(s->i8259_irq[n], level);
-    }
-    qemu_set_irq(s->ioapic_irq[n], level);
-}
-
 GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled)
 {
     GSIState *s;
@@ -392,55 +381,6 @@ static uint64_t ioportF0_read(void *opaque, hwaddr addr, unsigned size)
     return 0xffffffffffffffffULL;
 }
 
-/* TSC handling */
-uint64_t cpu_get_tsc(CPUX86State *env)
-{
-    return cpu_get_ticks();
-}
-
-/* IRQ handling */
-int cpu_get_pic_interrupt(CPUX86State *env)
-{
-    X86CPU *cpu = env_archcpu(env);
-    int intno;
-
-    if (!kvm_irqchip_in_kernel()) {
-        intno = apic_get_interrupt(cpu->apic_state);
-        if (intno >= 0) {
-            return intno;
-        }
-        /* read the irq from the PIC */
-        if (!apic_accept_pic_intr(cpu->apic_state)) {
-            return -1;
-        }
-    }
-
-    intno = pic_read_irq(isa_pic);
-    return intno;
-}
-
-static void pic_irq_request(void *opaque, int irq, int level)
-{
-    CPUState *cs = first_cpu;
-    X86CPU *cpu = X86_CPU(cs);
-
-    DPRINTF("pic_irqs: %s irq %d\n", level? "raise" : "lower", irq);
-    if (cpu->apic_state && !kvm_irqchip_in_kernel()) {
-        CPU_FOREACH(cs) {
-            cpu = X86_CPU(cs);
-            if (apic_accept_pic_intr(cpu->apic_state)) {
-                apic_deliver_pic_intr(cpu->apic_state, level);
-            }
-        }
-    } else {
-        if (level) {
-            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
-        } else {
-            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
-        }
-    }
-}
-
 /* PC cmos mappings */
 
 #define REG_EQUIPMENT_BYTE          0x14
@@ -884,16 +824,6 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
     nb_ne2k++;
 }
 
-DeviceState *cpu_get_current_apic(void)
-{
-    if (current_cpu) {
-        X86CPU *cpu = X86_CPU(current_cpu);
-        return cpu->apic_state;
-    } else {
-        return NULL;
-    }
-}
-
 void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
 {
     X86CPU *cpu = opaque;
@@ -1289,11 +1219,6 @@ uint64_t pc_pci_hole64_start(void)
     return ROUND_UP(hole64_start, 1 * GiB);
 }
 
-qemu_irq pc_allocate_cpu_irq(void)
-{
-    return qemu_allocate_irq(pic_irq_request, NULL, 0);
-}
-
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus)
 {
     DeviceState *dev = NULL;
@@ -1480,30 +1405,6 @@ void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs)
     g_free(i8259);
 }
 
-void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
-{
-    DeviceState *dev;
-    SysBusDevice *d;
-    unsigned int i;
-
-    if (kvm_ioapic_in_kernel()) {
-        dev = qdev_create(NULL, TYPE_KVM_IOAPIC);
-    } else {
-        dev = qdev_create(NULL, TYPE_IOAPIC);
-    }
-    if (parent_name) {
-        object_property_add_child(object_resolve_path(parent_name, NULL),
-                                  "ioapic", OBJECT(dev), NULL);
-    }
-    qdev_init_nofail(dev);
-    d = SYS_BUS_DEVICE(dev);
-    sysbus_mmio_map(d, 0, IO_APIC_DEFAULT_ADDRESS);
-
-    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
-        gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
-    }
-}
-
 static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
                                Error **errp)
 {
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 394edc2..028bbf4 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -41,11 +41,15 @@
 #include "hw/i386/fw_cfg.h"
 
 #include "hw/acpi/cpu_hotplug.h"
+#include "hw/i386/pc.h"
+#include "hw/irq.h"
 #include "hw/nmi.h"
 #include "hw/loader.h"
 #include "multiboot.h"
 #include "elf.h"
 #include "standard-headers/asm-x86/bootparam.h"
+#include "config-devices.h"
+#include "kvm_i386.h"
 
 #define BIOS_FILENAME "bios.bin"
 
@@ -220,6 +224,103 @@ static long get_file_size(FILE *f)
     return size;
 }
 
+/* TSC handling */
+uint64_t cpu_get_tsc(CPUX86State *env)
+{
+    return cpu_get_ticks();
+}
+
+/* IRQ handling */
+static void pic_irq_request(void *opaque, int irq, int level)
+{
+    CPUState *cs = first_cpu;
+    X86CPU *cpu = X86_CPU(cs);
+
+    if (cpu->apic_state && !kvm_irqchip_in_kernel()) {
+        CPU_FOREACH(cs) {
+            cpu = X86_CPU(cs);
+            if (apic_accept_pic_intr(cpu->apic_state)) {
+                apic_deliver_pic_intr(cpu->apic_state, level);
+            }
+        }
+    } else {
+        if (level) {
+            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+        } else {
+            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
+        }
+    }
+}
+
+qemu_irq pc_allocate_cpu_irq(void)
+{
+    return qemu_allocate_irq(pic_irq_request, NULL, 0);
+}
+
+int cpu_get_pic_interrupt(CPUX86State *env)
+{
+    X86CPU *cpu = env_archcpu(env);
+    int intno;
+
+    if (!kvm_irqchip_in_kernel()) {
+        intno = apic_get_interrupt(cpu->apic_state);
+        if (intno >= 0) {
+            return intno;
+        }
+        /* read the irq from the PIC */
+        if (!apic_accept_pic_intr(cpu->apic_state)) {
+            return -1;
+        }
+    }
+
+    intno = pic_read_irq(isa_pic);
+    return intno;
+}
+
+DeviceState *cpu_get_current_apic(void)
+{
+    if (current_cpu) {
+        X86CPU *cpu = X86_CPU(current_cpu);
+        return cpu->apic_state;
+    } else {
+        return NULL;
+    }
+}
+
+void gsi_handler(void *opaque, int n, int level)
+{
+    GSIState *s = opaque;
+
+    if (n < ISA_NUM_IRQS) {
+        qemu_set_irq(s->i8259_irq[n], level);
+    }
+    qemu_set_irq(s->ioapic_irq[n], level);
+}
+
+void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
+{
+    DeviceState *dev;
+    SysBusDevice *d;
+    unsigned int i;
+
+    if (kvm_ioapic_in_kernel()) {
+        dev = qdev_create(NULL, TYPE_KVM_IOAPIC);
+    } else {
+        dev = qdev_create(NULL, TYPE_IOAPIC);
+    }
+    if (parent_name) {
+        object_property_add_child(object_resolve_path(parent_name, NULL),
+                                  "ioapic", OBJECT(dev), NULL);
+    }
+    qdev_init_nofail(dev);
+    d = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(d, 0, IO_APIC_DEFAULT_ADDRESS);
+
+    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+        gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
+    }
+}
+
 struct setup_data {
     uint64_t next;
     uint32_t type;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1f86eba..0ab4a2a 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -3,11 +3,9 @@
 
 #include "exec/memory.h"
 #include "hw/boards.h"
-#include "hw/isa/isa.h"
 #include "hw/block/fdc.h"
 #include "hw/block/flash.h"
 #include "net/net.h"
-#include "hw/i386/ioapic.h"
 #include "hw/i386/x86.h"
 
 #include "qemu/range.h"
@@ -144,15 +142,6 @@ int pic_get_output(DeviceState *d);
 
 /* ioapic.c */
 
-/* Global System Interrupts */
-
-#define GSI_NUM_PINS IOAPIC_NUM_PINS
-
-typedef struct GSIState {
-    qemu_irq i8259_irq[ISA_NUM_IRQS];
-    qemu_irq ioapic_irq[IOAPIC_NUM_PINS];
-} GSIState;
-
 void gsi_handler(void *opaque, int n, int level);
 
 GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled);
@@ -217,7 +206,6 @@ void pc_pci_device_init(PCIBus *pci_bus);
 typedef void (*cpu_set_smm_t)(int smm, void *arg);
 
 void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs);
-void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
 
 ISADevice *pc_find_fdc0(void);
 int cmos_get_fd_drive_type(FloppyDriveType fd0);
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index 4b84917..a6c23ef 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -23,6 +23,8 @@
 
 #include "hw/boards.h"
 #include "hw/nmi.h"
+#include "hw/isa/isa.h"
+#include "hw/i386/ioapic.h"
 
 typedef struct {
     /*< private >*/
@@ -95,4 +97,15 @@ void x86_load_linux(X86MachineState *x86ms,
                     bool pvh_enabled,
                     bool linuxboot_dma_enabled);
 
+/* Global System Interrupts */
+
+#define GSI_NUM_PINS IOAPIC_NUM_PINS
+
+typedef struct GSIState {
+    qemu_irq i8259_irq[ISA_NUM_IRQS];
+    qemu_irq ioapic_irq[IOAPIC_NUM_PINS];
+} GSIState;
+
+void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
+
 #endif
-- 
1.8.3.1

Re: [PATCH 3/5] x86: move more functions to X86MachineState and x86.c

[Sergio Lopez]

[-- Attachment #1: Type: text/plain, Size: 10265 bytes --]


Paolo Bonzini <pbonzini@redhat.com> writes:

> These are needed by microvm too, so move them outside of PC-specific files.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/i386/pc.c          |  99 -------------------------------------------------
>  hw/i386/x86.c         | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/i386/pc.h  |  12 ------
>  include/hw/i386/x86.h |  13 +++++++
>  4 files changed, 114 insertions(+), 111 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 3d2c5d8..308248d 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -342,17 +342,6 @@ GlobalProperty pc_compat_1_4[] = {
>  };
>  const size_t pc_compat_1_4_len = G_N_ELEMENTS(pc_compat_1_4);
>  
> -void gsi_handler(void *opaque, int n, int level)
> -{
> -    GSIState *s = opaque;
> -
> -    DPRINTF("pc: %s GSI %d\n", level ? "raising" : "lowering", n);
> -    if (n < ISA_NUM_IRQS) {
> -        qemu_set_irq(s->i8259_irq[n], level);
> -    }
> -    qemu_set_irq(s->ioapic_irq[n], level);
> -}
> -
>  GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled)
>  {
>      GSIState *s;
> @@ -392,55 +381,6 @@ static uint64_t ioportF0_read(void *opaque, hwaddr addr, unsigned size)
>      return 0xffffffffffffffffULL;
>  }
>  
> -/* TSC handling */
> -uint64_t cpu_get_tsc(CPUX86State *env)
> -{
> -    return cpu_get_ticks();
> -}
> -
> -/* IRQ handling */
> -int cpu_get_pic_interrupt(CPUX86State *env)
> -{
> -    X86CPU *cpu = env_archcpu(env);
> -    int intno;
> -
> -    if (!kvm_irqchip_in_kernel()) {
> -        intno = apic_get_interrupt(cpu->apic_state);
> -        if (intno >= 0) {
> -            return intno;
> -        }
> -        /* read the irq from the PIC */
> -        if (!apic_accept_pic_intr(cpu->apic_state)) {
> -            return -1;
> -        }
> -    }
> -
> -    intno = pic_read_irq(isa_pic);
> -    return intno;
> -}
> -
> -static void pic_irq_request(void *opaque, int irq, int level)
> -{
> -    CPUState *cs = first_cpu;
> -    X86CPU *cpu = X86_CPU(cs);
> -
> -    DPRINTF("pic_irqs: %s irq %d\n", level? "raise" : "lower", irq);
> -    if (cpu->apic_state && !kvm_irqchip_in_kernel()) {
> -        CPU_FOREACH(cs) {
> -            cpu = X86_CPU(cs);
> -            if (apic_accept_pic_intr(cpu->apic_state)) {
> -                apic_deliver_pic_intr(cpu->apic_state, level);
> -            }
> -        }
> -    } else {
> -        if (level) {
> -            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> -        } else {
> -            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> -        }
> -    }
> -}
> -
>  /* PC cmos mappings */
>  
>  #define REG_EQUIPMENT_BYTE          0x14
> @@ -884,16 +824,6 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
>      nb_ne2k++;
>  }
>  
> -DeviceState *cpu_get_current_apic(void)
> -{
> -    if (current_cpu) {
> -        X86CPU *cpu = X86_CPU(current_cpu);
> -        return cpu->apic_state;
> -    } else {
> -        return NULL;
> -    }
> -}
> -
>  void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>  {
>      X86CPU *cpu = opaque;
> @@ -1289,11 +1219,6 @@ uint64_t pc_pci_hole64_start(void)
>      return ROUND_UP(hole64_start, 1 * GiB);
>  }
>  
> -qemu_irq pc_allocate_cpu_irq(void)
> -{
> -    return qemu_allocate_irq(pic_irq_request, NULL, 0);
> -}
> -
>  DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus)
>  {
>      DeviceState *dev = NULL;
> @@ -1480,30 +1405,6 @@ void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs)
>      g_free(i8259);
>  }
>  
> -void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
> -{
> -    DeviceState *dev;
> -    SysBusDevice *d;
> -    unsigned int i;
> -
> -    if (kvm_ioapic_in_kernel()) {
> -        dev = qdev_create(NULL, TYPE_KVM_IOAPIC);
> -    } else {
> -        dev = qdev_create(NULL, TYPE_IOAPIC);
> -    }
> -    if (parent_name) {
> -        object_property_add_child(object_resolve_path(parent_name, NULL),
> -                                  "ioapic", OBJECT(dev), NULL);
> -    }
> -    qdev_init_nofail(dev);
> -    d = SYS_BUS_DEVICE(dev);
> -    sysbus_mmio_map(d, 0, IO_APIC_DEFAULT_ADDRESS);
> -
> -    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> -        gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
> -    }
> -}
> -
>  static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>                                 Error **errp)
>  {
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 394edc2..028bbf4 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -41,11 +41,15 @@
>  #include "hw/i386/fw_cfg.h"
>  
>  #include "hw/acpi/cpu_hotplug.h"
> +#include "hw/i386/pc.h"

Including pc.h from x86.h looks a bit weird to me. I think it'd be nicer
moving the i8259 declarations to a separate header, and the all the
GSI-related stuff to x86.h. I have a couple patches doing that, in case
they can save you some time.

> +#include "hw/irq.h"
>  #include "hw/nmi.h"
>  #include "hw/loader.h"
>  #include "multiboot.h"
>  #include "elf.h"
>  #include "standard-headers/asm-x86/bootparam.h"
> +#include "config-devices.h"
> +#include "kvm_i386.h"
>  
>  #define BIOS_FILENAME "bios.bin"
>  
> @@ -220,6 +224,103 @@ static long get_file_size(FILE *f)
>      return size;
>  }
>  
> +/* TSC handling */
> +uint64_t cpu_get_tsc(CPUX86State *env)
> +{
> +    return cpu_get_ticks();
> +}
> +
> +/* IRQ handling */
> +static void pic_irq_request(void *opaque, int irq, int level)
> +{
> +    CPUState *cs = first_cpu;
> +    X86CPU *cpu = X86_CPU(cs);
> +
> +    if (cpu->apic_state && !kvm_irqchip_in_kernel()) {
> +        CPU_FOREACH(cs) {
> +            cpu = X86_CPU(cs);
> +            if (apic_accept_pic_intr(cpu->apic_state)) {
> +                apic_deliver_pic_intr(cpu->apic_state, level);
> +            }
> +        }
> +    } else {
> +        if (level) {
> +            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
> +        } else {
> +            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
> +        }
> +    }
> +}
> +
> +qemu_irq pc_allocate_cpu_irq(void)
> +{
> +    return qemu_allocate_irq(pic_irq_request, NULL, 0);
> +}

Should we rename this one to "x86_allocate_cpu_irq(void)" ?

> +
> +int cpu_get_pic_interrupt(CPUX86State *env)
> +{
> +    X86CPU *cpu = env_archcpu(env);
> +    int intno;
> +
> +    if (!kvm_irqchip_in_kernel()) {
> +        intno = apic_get_interrupt(cpu->apic_state);
> +        if (intno >= 0) {
> +            return intno;
> +        }
> +        /* read the irq from the PIC */
> +        if (!apic_accept_pic_intr(cpu->apic_state)) {
> +            return -1;
> +        }
> +    }
> +
> +    intno = pic_read_irq(isa_pic);
> +    return intno;
> +}
> +
> +DeviceState *cpu_get_current_apic(void)
> +{
> +    if (current_cpu) {
> +        X86CPU *cpu = X86_CPU(current_cpu);
> +        return cpu->apic_state;
> +    } else {
> +        return NULL;
> +    }
> +}
> +
> +void gsi_handler(void *opaque, int n, int level)
> +{
> +    GSIState *s = opaque;
> +
> +    if (n < ISA_NUM_IRQS) {
> +        qemu_set_irq(s->i8259_irq[n], level);
> +    }
> +    qemu_set_irq(s->ioapic_irq[n], level);
> +}
> +
> +void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
> +{
> +    DeviceState *dev;
> +    SysBusDevice *d;
> +    unsigned int i;
> +
> +    if (kvm_ioapic_in_kernel()) {
> +        dev = qdev_create(NULL, TYPE_KVM_IOAPIC);
> +    } else {
> +        dev = qdev_create(NULL, TYPE_IOAPIC);
> +    }
> +    if (parent_name) {
> +        object_property_add_child(object_resolve_path(parent_name, NULL),
> +                                  "ioapic", OBJECT(dev), NULL);
> +    }
> +    qdev_init_nofail(dev);
> +    d = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(d, 0, IO_APIC_DEFAULT_ADDRESS);
> +
> +    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> +        gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
> +    }
> +}
> +
>  struct setup_data {
>      uint64_t next;
>      uint32_t type;
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 1f86eba..0ab4a2a 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -3,11 +3,9 @@
>  
>  #include "exec/memory.h"
>  #include "hw/boards.h"
> -#include "hw/isa/isa.h"
>  #include "hw/block/fdc.h"
>  #include "hw/block/flash.h"
>  #include "net/net.h"
> -#include "hw/i386/ioapic.h"
>  #include "hw/i386/x86.h"
>  
>  #include "qemu/range.h"
> @@ -144,15 +142,6 @@ int pic_get_output(DeviceState *d);
>  
>  /* ioapic.c */
>  
> -/* Global System Interrupts */
> -
> -#define GSI_NUM_PINS IOAPIC_NUM_PINS
> -
> -typedef struct GSIState {
> -    qemu_irq i8259_irq[ISA_NUM_IRQS];
> -    qemu_irq ioapic_irq[IOAPIC_NUM_PINS];
> -} GSIState;
> -
>  void gsi_handler(void *opaque, int n, int level);
>  
>  GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled);
> @@ -217,7 +206,6 @@ void pc_pci_device_init(PCIBus *pci_bus);
>  typedef void (*cpu_set_smm_t)(int smm, void *arg);
>  
>  void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs);
> -void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
>  
>  ISADevice *pc_find_fdc0(void);
>  int cmos_get_fd_drive_type(FloppyDriveType fd0);
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index 4b84917..a6c23ef 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -23,6 +23,8 @@
>  
>  #include "hw/boards.h"
>  #include "hw/nmi.h"
> +#include "hw/isa/isa.h"
> +#include "hw/i386/ioapic.h"
>  
>  typedef struct {
>      /*< private >*/
> @@ -95,4 +97,15 @@ void x86_load_linux(X86MachineState *x86ms,
>                      bool pvh_enabled,
>                      bool linuxboot_dma_enabled);
>  
> +/* Global System Interrupts */
> +
> +#define GSI_NUM_PINS IOAPIC_NUM_PINS
> +
> +typedef struct GSIState {
> +    qemu_irq i8259_irq[ISA_NUM_IRQS];
> +    qemu_irq ioapic_irq[IOAPIC_NUM_PINS];
> +} GSIState;
> +
> +void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
> +
>  #endif


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

Re: [PATCH 3/5] x86: move more functions to X86MachineState and x86.c

[Paolo Bonzini]

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

On 12/12/19 16:59, Sergio Lopez wrote:
> 
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> These are needed by microvm too, so move them outside of PC-specific files.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  hw/i386/pc.c          |  99 -------------------------------------------------
>>  hw/i386/x86.c         | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/i386/pc.h  |  12 ------
>>  include/hw/i386/x86.h |  13 +++++++
>>  4 files changed, 114 insertions(+), 111 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 3d2c5d8..308248d 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -342,17 +342,6 @@ GlobalProperty pc_compat_1_4[] = {
>>  };
>>  const size_t pc_compat_1_4_len = G_N_ELEMENTS(pc_compat_1_4);
>>  
>> -void gsi_handler(void *opaque, int n, int level)
>> -{
>> -    GSIState *s = opaque;
>> -
>> -    DPRINTF("pc: %s GSI %d\n", level ? "raising" : "lowering", n);
>> -    if (n < ISA_NUM_IRQS) {
>> -        qemu_set_irq(s->i8259_irq[n], level);
>> -    }
>> -    qemu_set_irq(s->ioapic_irq[n], level);
>> -}
>> -
>>  GSIState *pc_gsi_create(qemu_irq **irqs, bool pci_enabled)
>>  {
>>      GSIState *s;
>> @@ -392,55 +381,6 @@ static uint64_t ioportF0_read(void *opaque, hwaddr addr, unsigned size)
>>      return 0xffffffffffffffffULL;
>>  }
>>  
>> -/* TSC handling */
>> -uint64_t cpu_get_tsc(CPUX86State *env)
>> -{
>> -    return cpu_get_ticks();
>> -}
>> -
>> -/* IRQ handling */
>> -int cpu_get_pic_interrupt(CPUX86State *env)
>> -{
>> -    X86CPU *cpu = env_archcpu(env);
>> -    int intno;
>> -
>> -    if (!kvm_irqchip_in_kernel()) {
>> -        intno = apic_get_interrupt(cpu->apic_state);
>> -        if (intno >= 0) {
>> -            return intno;
>> -        }
>> -        /* read the irq from the PIC */
>> -        if (!apic_accept_pic_intr(cpu->apic_state)) {
>> -            return -1;
>> -        }
>> -    }
>> -
>> -    intno = pic_read_irq(isa_pic);
>> -    return intno;
>> -}
>> -
>> -static void pic_irq_request(void *opaque, int irq, int level)
>> -{
>> -    CPUState *cs = first_cpu;
>> -    X86CPU *cpu = X86_CPU(cs);
>> -
>> -    DPRINTF("pic_irqs: %s irq %d\n", level? "raise" : "lower", irq);
>> -    if (cpu->apic_state && !kvm_irqchip_in_kernel()) {
>> -        CPU_FOREACH(cs) {
>> -            cpu = X86_CPU(cs);
>> -            if (apic_accept_pic_intr(cpu->apic_state)) {
>> -                apic_deliver_pic_intr(cpu->apic_state, level);
>> -            }
>> -        }
>> -    } else {
>> -        if (level) {
>> -            cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>> -        } else {
>> -            cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
>> -        }
>> -    }
>> -}
>> -
>>  /* PC cmos mappings */
>>  
>>  #define REG_EQUIPMENT_BYTE          0x14
>> @@ -884,16 +824,6 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
>>      nb_ne2k++;
>>  }
>>  
>> -DeviceState *cpu_get_current_apic(void)
>> -{
>> -    if (current_cpu) {
>> -        X86CPU *cpu = X86_CPU(current_cpu);
>> -        return cpu->apic_state;
>> -    } else {
>> -        return NULL;
>> -    }
>> -}
>> -
>>  void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>>  {
>>      X86CPU *cpu = opaque;
>> @@ -1289,11 +1219,6 @@ uint64_t pc_pci_hole64_start(void)
>>      return ROUND_UP(hole64_start, 1 * GiB);
>>  }
>>  
>> -qemu_irq pc_allocate_cpu_irq(void)
>> -{
>> -    return qemu_allocate_irq(pic_irq_request, NULL, 0);
>> -}
>> -
>>  DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus)
>>  {
>>      DeviceState *dev = NULL;
>> @@ -1480,30 +1405,6 @@ void pc_i8259_create(ISABus *isa_bus, qemu_irq *i8259_irqs)
>>      g_free(i8259);
>>  }
>>  
>> -void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
>> -{
>> -    DeviceState *dev;
>> -    SysBusDevice *d;
>> -    unsigned int i;
>> -
>> -    if (kvm_ioapic_in_kernel()) {
>> -        dev = qdev_create(NULL, TYPE_KVM_IOAPIC);
>> -    } else {
>> -        dev = qdev_create(NULL, TYPE_IOAPIC);
>> -    }
>> -    if (parent_name) {
>> -        object_property_add_child(object_resolve_path(parent_name, NULL),
>> -                                  "ioapic", OBJECT(dev), NULL);
>> -    }
>> -    qdev_init_nofail(dev);
>> -    d = SYS_BUS_DEVICE(dev);
>> -    sysbus_mmio_map(d, 0, IO_APIC_DEFAULT_ADDRESS);
>> -
>> -    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
>> -        gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
>> -    }
>> -}
>> -
>>  static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>>                                 Error **errp)
>>  {
>> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
>> index 394edc2..028bbf4 100644
>> --- a/hw/i386/x86.c
>> +++ b/hw/i386/x86.c
>> @@ -41,11 +41,15 @@
>>  #include "hw/i386/fw_cfg.h"
>>  
>>  #include "hw/acpi/cpu_hotplug.h"
>> +#include "hw/i386/pc.h"
> 
> Including pc.h from x86.h looks a bit weird to me. I think it'd be nicer
> moving the i8259 declarations to a separate header, and the all the
> GSI-related stuff to x86.h. I have a couple patches doing that, in case
> they can save you some time.

Hmm I wonder if the include is actually needed.  Thanks for pointing it out.

Paolo


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 4/5] pci-stub: add more MSI functions

[Paolo Bonzini]

On x86, KVM needs some function from the PCI subsystem in order to set
up interrupt routes.  Provide some stubs to support x86 machines that
lack PCI.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pci/pci-stub.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
index c04a5df..cc2a2e1 100644
--- a/hw/pci/pci-stub.c
+++ b/hw/pci/pci-stub.c
@@ -26,6 +26,7 @@
 #include "qapi/qmp/qerror.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/msi.h"
+#include "hw/pci/msix.h"
 
 bool msi_nonbroken;
 bool pci_available;
@@ -64,3 +65,29 @@ void msi_notify(PCIDevice *dev, unsigned int vector)
 {
     g_assert_not_reached();
 }
+
+/* Required by target/i386/kvm.c */
+bool msi_is_masked(const PCIDevice *dev, unsigned vector)
+{
+    g_assert_not_reached();
+}
+
+MSIMessage msi_get_message(PCIDevice *dev, unsigned int vector)
+{
+    g_assert_not_reached();
+}
+
+int msix_enabled(PCIDevice *dev)
+{
+    return false;
+}
+
+bool msix_is_masked(PCIDevice *dev, unsigned vector)
+{
+    g_assert_not_reached();
+}
+
+MSIMessage msix_get_message(PCIDevice *dev, unsigned int vector)
+{
+    g_assert_not_reached();
+}
-- 
1.8.3.1

Re: [PATCH 4/5] pci-stub: add more MSI functions

[Sergio Lopez]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]


Paolo Bonzini <pbonzini@redhat.com> writes:

> On x86, KVM needs some function from the PCI subsystem in order to set
> up interrupt routes.  Provide some stubs to support x86 machines that
> lack PCI.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/pci/pci-stub.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)

Reviewed-by: Sergio Lopez <slp@redhat.com>

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

[PATCH 5/5] kvm: check before accessing PCMachineState fields

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 roms/SLOF         |  2 +-
 target/i386/kvm.c | 11 +++++++++--
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/roms/SLOF b/roms/SLOF
index 8ebf2f5..bcc3c4e 160000
--- a/roms/SLOF
+++ b/roms/SLOF
@@ -1 +1 @@
-Subproject commit 8ebf2f55e1ba1492b942ba4b682160e644fc0f98
+Subproject commit bcc3c4e5c21a015f4680894c4ec978a90d4a2d69
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index ef63f3a..2ad5a94 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -49,6 +49,7 @@
 #include "migration/blocker.h"
 #include "exec/memattrs.h"
 #include "trace.h"
+#include "config-devices.h"
 
 //#define DEBUG_KVM
 
@@ -2172,9 +2173,15 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
         }
     }
 
-    if (kvm_check_extension(s, KVM_CAP_X86_SMM) &&
+    if (
+#ifdef CONFIG_PC
+        kvm_check_extension(s, KVM_CAP_X86_SMM) &&
         object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE) &&
-        pc_machine_is_smm_enabled(PC_MACHINE(ms))) {
+        pc_machine_is_smm_enabled(PC_MACHINE(ms))
+#else
+	0
+#endif
+       ) {
         smram_machine_done.notify = register_smram_listener;
         qemu_add_machine_init_done_notifier(&smram_machine_done);
     }
-- 
1.8.3.1

Re: [PATCH 5/5] kvm: check before accessing PCMachineState fields

[Sergio Lopez]

[-- Attachment #1: Type: text/plain, Size: 1588 bytes --]


Paolo Bonzini <pbonzini@redhat.com> writes:

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  roms/SLOF         |  2 +-
>  target/i386/kvm.c | 11 +++++++++--
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/roms/SLOF b/roms/SLOF
> index 8ebf2f5..bcc3c4e 160000
> --- a/roms/SLOF
> +++ b/roms/SLOF
> @@ -1 +1 @@
> -Subproject commit 8ebf2f55e1ba1492b942ba4b682160e644fc0f98
> +Subproject commit bcc3c4e5c21a015f4680894c4ec978a90d4a2d69

I guess this was included by mistake.

> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index ef63f3a..2ad5a94 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -49,6 +49,7 @@
>  #include "migration/blocker.h"
>  #include "exec/memattrs.h"
>  #include "trace.h"
> +#include "config-devices.h"
>  
>  //#define DEBUG_KVM
>  
> @@ -2172,9 +2173,15 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>          }
>      }
>  
> -    if (kvm_check_extension(s, KVM_CAP_X86_SMM) &&
> +    if (
> +#ifdef CONFIG_PC
> +        kvm_check_extension(s, KVM_CAP_X86_SMM) &&
>          object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE) &&
> -        pc_machine_is_smm_enabled(PC_MACHINE(ms))) {
> +        pc_machine_is_smm_enabled(PC_MACHINE(ms))
> +#else
> +	0
> +#endif
> +       ) {
>          smram_machine_done.notify = register_smram_listener;
>          qemu_add_machine_init_done_notifier(&smram_machine_done);
>      }

I'm not familiar with SMM, can we consider it a PC specific thing? If it
isn't, perhaps we should move smm to X86MachineState.

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

Re: [PATCH 5/5] kvm: check before accessing PCMachineState fields

[Paolo Bonzini]

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

On 12/12/19 17:11, Sergio Lopez wrote:
>>      }
>>  
>> -    if (kvm_check_extension(s, KVM_CAP_X86_SMM) &&
>> +    if (
>> +#ifdef CONFIG_PC
>> +        kvm_check_extension(s, KVM_CAP_X86_SMM) &&
>>          object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE) &&
>> -        pc_machine_is_smm_enabled(PC_MACHINE(ms))) {
>> +        pc_machine_is_smm_enabled(PC_MACHINE(ms))
>> +#else
>> +	0
>> +#endif
>> +       ) {
>>          smram_machine_done.notify = register_smram_listener;
>>          qemu_add_machine_init_done_notifier(&smram_machine_done);
>>      }
> I'm not familiar with SMM, can we consider it a PC specific thing? If it
> isn't, perhaps we should move smm to X86MachineState.

Ok, I will do that.

Paolo


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]