qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 2/2] cpu/apic: drop icc bus/bridge/


From: Andreas Färber
Subject: Re: [Qemu-devel] [PATCH v3 2/2] cpu/apic: drop icc bus/bridge/
Date: Tue, 14 Apr 2015 15:41:13 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0

Am 14.04.2015 um 15:27 schrieb Igor Mammedov:
> On Fri, 10 Apr 2015 16:18:06 +0800
> Chen Fan <address@hidden> wrote:
> 
>> ICC bus was invented only to provide hotplug capability to
>> CPU and APIC because at the time being hotplug was available only for
>> BUS attached devices.
>>
>> Now this patch is to drop ICC bus impl, and switch to bus-less
>> CPU+APIC hotplug, handling them in the same manner as pc-dimm.
>> and due to APIC is bus-less, so we should reset it as CPU reset.
>>
>> Signed-off-by: Chen Fan <address@hidden>
>> ---
>>  default-configs/i386-softmmu.mak   |   1 -
>>  default-configs/x86_64-softmmu.mak |   1 -
>>  hw/cpu/Makefile.objs               |   2 -
>>  hw/cpu/icc_bus.c                   | 118 
>> -------------------------------------
>>  hw/i386/pc.c                       |  19 ++----
>>  hw/i386/pc_piix.c                  |   9 +--
>>  hw/i386/pc_q35.c                   |   9 +--
>>  hw/intc/apic.c                     |   6 +-
>>  hw/intc/apic_common.c              |   5 +-
>>  include/hw/cpu/icc_bus.h           |  82 --------------------------
>>  include/hw/i386/apic_internal.h    |   5 +-
>>  include/hw/i386/pc.h               |   2 +-
>>  target-i386/cpu.c                  |  13 ++--
>>  13 files changed, 24 insertions(+), 248 deletions(-)
>>  delete mode 100644 hw/cpu/icc_bus.c
>>  delete mode 100644 include/hw/cpu/icc_bus.h
>>
>> diff --git a/default-configs/i386-softmmu.mak 
>> b/default-configs/i386-softmmu.mak
>> index 6a74e00..3ac6324 100644
>> --- a/default-configs/i386-softmmu.mak
>> +++ b/default-configs/i386-softmmu.mak
>> @@ -39,7 +39,6 @@ CONFIG_LPC_ICH9=y
>>  CONFIG_PCI_Q35=y
>>  CONFIG_APIC=y
>>  CONFIG_IOAPIC=y
>> -CONFIG_ICC_BUS=y
>>  CONFIG_PVPANIC=y
>>  CONFIG_MEM_HOTPLUG=y
>>  CONFIG_XIO3130=y
>> diff --git a/default-configs/x86_64-softmmu.mak 
>> b/default-configs/x86_64-softmmu.mak
>> index 46b87dd..5c89ea8 100644
>> --- a/default-configs/x86_64-softmmu.mak
>> +++ b/default-configs/x86_64-softmmu.mak
>> @@ -39,7 +39,6 @@ CONFIG_LPC_ICH9=y
>>  CONFIG_PCI_Q35=y
>>  CONFIG_APIC=y
>>  CONFIG_IOAPIC=y
>> -CONFIG_ICC_BUS=y
>>  CONFIG_PVPANIC=y
>>  CONFIG_MEM_HOTPLUG=y
>>  CONFIG_XIO3130=y
>> diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
>> index 6381238..193e489 100644
>> --- a/hw/cpu/Makefile.objs
>> +++ b/hw/cpu/Makefile.objs
>> @@ -2,5 +2,3 @@ obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
>>  obj-$(CONFIG_REALVIEW) += realview_mpcore.o
>>  obj-$(CONFIG_A9MPCORE) += a9mpcore.o
>>  obj-$(CONFIG_A15MPCORE) += a15mpcore.o
>> -obj-$(CONFIG_ICC_BUS) += icc_bus.o
>> -
>> diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
>> deleted file mode 100644
>> index 6646ea2..0000000
>> --- a/hw/cpu/icc_bus.c
>> +++ /dev/null
>> @@ -1,118 +0,0 @@
>> -/* icc_bus.c
>> - * emulate x86 ICC (Interrupt Controller Communications) bus
>> - *
>> - * Copyright (c) 2013 Red Hat, Inc
>> - *
>> - * Authors:
>> - *     Igor Mammedov <address@hidden>
>> - *
>> - * This library is free software; you can redistribute it and/or
>> - * modify it under the terms of the GNU Lesser General Public
>> - * License as published by the Free Software Foundation; either
>> - * version 2 of the License, or (at your option) any later version.
>> - *
>> - * This library is distributed in the hope that it will be useful,
>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> - * Lesser General Public License for more details.
>> - *
>> - * You should have received a copy of the GNU Lesser General Public
>> - * License along with this library; if not, see 
>> <http://www.gnu.org/licenses/>
>> - */
>> -#include "hw/cpu/icc_bus.h"
>> -#include "hw/sysbus.h"
>> -
>> -/* icc-bridge implementation */
>> -
>> -static const TypeInfo icc_bus_info = {
>> -    .name = TYPE_ICC_BUS,
>> -    .parent = TYPE_BUS,
>> -    .instance_size = sizeof(ICCBus),
>> -};
>> -
>> -
>> -/* icc-device implementation */
>> -
>> -static void icc_device_realize(DeviceState *dev, Error **errp)
>> -{
>> -    ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev);
>> -
>> -    /* convert to QOM */
>> -    if (idc->realize) {
>> -        idc->realize(dev, errp);
>> -    }
>> -
>> -}
>> -
>> -static void icc_device_class_init(ObjectClass *oc, void *data)
>> -{
>> -    DeviceClass *dc = DEVICE_CLASS(oc);
>> -
>> -    dc->realize = icc_device_realize;
>> -    dc->bus_type = TYPE_ICC_BUS;
>> -}
>> -
>> -static const TypeInfo icc_device_info = {
>> -    .name = TYPE_ICC_DEVICE,
>> -    .parent = TYPE_DEVICE,
>> -    .abstract = true,
>> -    .instance_size = sizeof(ICCDevice),
>> -    .class_size = sizeof(ICCDeviceClass),
>> -    .class_init = icc_device_class_init,
>> -};
>> -
>> -
>> -/*  icc-bridge implementation */
>> -
>> -typedef struct ICCBridgeState {
>> -    /*< private >*/
>> -    SysBusDevice parent_obj;
>> -    /*< public >*/
>> -
>> -    ICCBus icc_bus;
>> -    MemoryRegion apic_container;
>> -} ICCBridgeState;
>> -
>> -#define ICC_BRIDGE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
>> -
>> -static void icc_bridge_init(Object *obj)
>> -{
>> -    ICCBridgeState *s = ICC_BRIDGE(obj);
>> -    SysBusDevice *sb = SYS_BUS_DEVICE(obj);
>> -
>> -    qbus_create_inplace(&s->icc_bus, sizeof(s->icc_bus), TYPE_ICC_BUS,
>> -                        DEVICE(s), "icc");
>> -
>> -    /* Do not change order of registering regions,
>> -     * APIC must be first registered region, board maps it by 0 index
>> -     */
>> -    memory_region_init(&s->apic_container, obj, "icc-apic-container",
>> -                       APIC_SPACE_SIZE);
>> -    sysbus_init_mmio(sb, &s->apic_container);
>> -    s->icc_bus.apic_address_space = &s->apic_container;
>> -}
>> -
>> -static void icc_bridge_class_init(ObjectClass *oc, void *data)
>> -{
>> -    DeviceClass *dc = DEVICE_CLASS(oc);
>> -
>> -    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>> -}
>> -
>> -static const TypeInfo icc_bridge_info = {
>> -    .name  = TYPE_ICC_BRIDGE,
>> -    .parent = TYPE_SYS_BUS_DEVICE,
>> -    .instance_init  = icc_bridge_init,
>> -    .instance_size  = sizeof(ICCBridgeState),
>> -    .class_init = icc_bridge_class_init,
>> -};
>> -
>> -
>> -static void icc_bus_register_types(void)
>> -{
>> -    type_register_static(&icc_bus_info);
>> -    type_register_static(&icc_device_info);
>> -    type_register_static(&icc_bridge_info);
>> -}
>> -
>> -type_init(icc_bus_register_types)

I think it may help to split this patch into two - one doing the APIC
changes and once unused dropping the large files.

>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index a5e2a27..00b3ddd 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -58,7 +58,6 @@
>>  #include "qemu/config-file.h"
>>  #include "hw/acpi/acpi.h"
>>  #include "hw/acpi/cpu_hotplug.h"
>> -#include "hw/cpu/icc_bus.h"
>>  #include "hw/boards.h"
>>  #include "hw/pci/pci_host.h"
>>  #include "acpi-build.h"
>> @@ -990,22 +989,17 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int 
>> level)
>>  }
>>  
>>  static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
>> -                          DeviceState *icc_bridge, Error **errp)
>> +                          Error **errp)
>>  {
>>      X86CPU *cpu = NULL;
>>      Error *local_err = NULL;
>>  
>> -    if (icc_bridge == NULL) {
>> -        error_setg(&local_err, "Invalid icc-bridge value");
>> -        goto out;
>> -    }
>> -
>>      cpu = cpu_x86_create(cpu_model, &local_err);
>>      if (local_err != NULL) {
>>          goto out;
>>      }
>>  
>> -    qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, "icc"));
>> +    qdev_set_parent_bus(DEVICE(cpu), sysbus_get_default());
>>      object_unref(OBJECT(cpu));
>>  
>>      object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err);
>> @@ -1024,7 +1018,6 @@ static const char *current_cpu_model;
>>  
>>  void pc_hot_add_cpu(const int64_t id, Error **errp)
>>  {
>> -    DeviceState *icc_bridge;
>>      int64_t apic_id = x86_cpu_apic_id_from_index(id);
>>  
>>      if (id < 0) {
>> @@ -1051,12 +1044,10 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
>>          return;
>>      }
>>  
>> -    icc_bridge = DEVICE(object_resolve_path_type("icc-bridge",
>> -                                                 TYPE_ICC_BRIDGE, NULL));
>> -    pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp);
>> +    pc_new_cpu(current_cpu_model, apic_id, errp);
>>  }
>>  
>> -void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>> +void pc_cpus_init(const char *cpu_model)
>>  {
>>      int i;
>>      X86CPU *cpu = NULL;
>> @@ -1082,7 +1073,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState 
>> *icc_bridge)
>>  
>>      for (i = 0; i < smp_cpus; i++) {
>>          cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
>> -                         icc_bridge, &error);
>> +                         &error);
>>          if (error) {
>>              error_report_err(error);
>>              exit(1);
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 1fe7bfb..6004580 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -39,7 +39,6 @@
>>  #include "hw/kvm/clock.h"
>>  #include "sysemu/sysemu.h"
>>  #include "hw/sysbus.h"
>> -#include "hw/cpu/icc_bus.h"
>>  #include "sysemu/arch_init.h"
>>  #include "sysemu/block-backend.h"
>>  #include "hw/i2c/smbus.h"
>> @@ -98,7 +97,6 @@ static void pc_init1(MachineState *machine,
>>      MemoryRegion *ram_memory;
>>      MemoryRegion *pci_memory;
>>      MemoryRegion *rom_memory;
>> -    DeviceState *icc_bridge;
>>      FWCfgState *fw_cfg = NULL;
>>      PcGuestInfo *guest_info;
>>      ram_addr_t lowmem;
>> @@ -143,11 +141,7 @@ static void pc_init1(MachineState *machine,
>>          exit(1);
>>      }
>>  
>> -    icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
>> -    object_property_add_child(qdev_get_machine(), "icc-bridge",
>> -                              OBJECT(icc_bridge), NULL);
>> -
>> -    pc_cpus_init(machine->cpu_model, icc_bridge);
>> +    pc_cpus_init(machine->cpu_model);
>>  
>>      if (kvm_enabled() && kvmclock_enabled) {
>>          kvmclock_create();
>> @@ -230,7 +224,6 @@ static void pc_init1(MachineState *machine,
>>      if (pci_enabled) {
>>          ioapic_init_gsi(gsi_state, "i440fx");
>>      }
>> -    qdev_init_nofail(icc_bridge);
>>  
>>      pc_register_ferr_irq(gsi[13]);
>>  
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index dcc17c0..3b89e6a 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -43,7 +43,6 @@
>>  #include "hw/ide/pci.h"
>>  #include "hw/ide/ahci.h"
>>  #include "hw/usb.h"
>> -#include "hw/cpu/icc_bus.h"
>>  #include "qemu/error-report.h"
>>  
>>  /* ICH9 AHCI has 6 ports */
>> @@ -85,7 +84,6 @@ static void pc_q35_init(MachineState *machine)
>>      int i;
>>      ICH9LPCState *ich9_lpc;
>>      PCIDevice *ahci;
>> -    DeviceState *icc_bridge;
>>      PcGuestInfo *guest_info;
>>      ram_addr_t lowmem;
>>      DriveInfo *hd[MAX_SATA_PORTS];
>> @@ -132,11 +130,7 @@ static void pc_q35_init(MachineState *machine)
>>          exit(1);
>>      }
>>  
>> -    icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
>> -    object_property_add_child(qdev_get_machine(), "icc-bridge",
>> -                              OBJECT(icc_bridge), NULL);
>> -
>> -    pc_cpus_init(machine->cpu_model, icc_bridge);
>> +    pc_cpus_init(machine->cpu_model);
>>      pc_acpi_init("q35-acpi-dsdt.aml");
>>  
>>      kvmclock_create();
>> @@ -240,7 +234,6 @@ static void pc_q35_init(MachineState *machine)
>>      if (pci_enabled) {
>>          ioapic_init_gsi(gsi_state, "q35");
>>      }
>> -    qdev_init_nofail(icc_bridge);
>>  
>>      pc_register_ferr_irq(gsi[13]);
>>  
>> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
>> index 0f97b47..00ae0ec 100644
>> --- a/hw/intc/apic.c
>> +++ b/hw/intc/apic.c
>> @@ -376,7 +376,7 @@ static void apic_update_irq(APICCommonState *s)
>>          cpu_interrupt(cpu, CPU_INTERRUPT_POLL);
>>      } else if (apic_irq_pending(s) > 0) {
>>          cpu_interrupt(cpu, CPU_INTERRUPT_HARD);
>> -    } else if (!apic_accept_pic_intr(&s->busdev.qdev) || 
>> !pic_get_output(isa_pic)) {
>> +    } else if (!apic_accept_pic_intr(DEVICE(s)) || 
>> !pic_get_output(isa_pic)) {
>>          cpu_reset_interrupt(cpu, CPU_INTERRUPT_HARD);
>>      }
>>  }
>> @@ -549,10 +549,10 @@ static void apic_deliver(DeviceState *dev, uint8_t 
>> dest, uint8_t dest_mode,
>>  
>>  static bool apic_check_pic(APICCommonState *s)
>>  {
>> -    if (!apic_accept_pic_intr(&s->busdev.qdev) || !pic_get_output(isa_pic)) 
>> {
>> +    if (!apic_accept_pic_intr(DEVICE(s)) || !pic_get_output(isa_pic)) {
>>          return false;
>>      }
>> -    apic_deliver_pic_intr(&s->busdev.qdev, 1);
>> +    apic_deliver_pic_intr(DEVICE(s), 1);
>>      return true;
>>  }
>>  

Can you put these cast fixes in a preparatory patch please?

>> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
>> index 004821a..bdc0dc7 100644
>> --- a/hw/intc/apic_common.c
>> +++ b/hw/intc/apic_common.c
>> @@ -436,13 +436,12 @@ static Property apic_properties_common[] = {
>>  
>>  static void apic_common_class_init(ObjectClass *klass, void *data)
>>  {
>> -    ICCDeviceClass *idc = ICC_DEVICE_CLASS(klass);
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>>  
>>      dc->vmsd = &vmstate_apic_common;
>>      dc->reset = apic_reset_common;
>>      dc->props = apic_properties_common;
>> -    idc->realize = apic_common_realize;
>> +    dc->realize = apic_common_realize;
>>      /*
>>       * Reason: APIC and CPU need to be wired up by
>>       * x86_cpu_apic_create()
>> @@ -452,7 +451,7 @@ static void apic_common_class_init(ObjectClass *klass, 
>> void *data)
>>  
>>  static const TypeInfo apic_common_type = {
>>      .name = TYPE_APIC_COMMON,
>> -    .parent = TYPE_ICC_DEVICE,
>> +    .parent = TYPE_DEVICE,
>>      .instance_size = sizeof(APICCommonState),
>>      .class_size = sizeof(APICCommonClass),
>>      .class_init = apic_common_class_init,
>> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
>> deleted file mode 100644
>> index 98a979f..0000000
>> --- a/include/hw/cpu/icc_bus.h
>> +++ /dev/null
>> @@ -1,82 +0,0 @@
>> -/* icc_bus.h
>> - * emulate x86 ICC (Interrupt Controller Communications) bus
>> - *
>> - * Copyright (c) 2013 Red Hat, Inc
>> - *
>> - * Authors:
>> - *     Igor Mammedov <address@hidden>
>> - *
>> - * This library is free software; you can redistribute it and/or
>> - * modify it under the terms of the GNU Lesser General Public
>> - * License as published by the Free Software Foundation; either
>> - * version 2 of the License, or (at your option) any later version.
>> - *
>> - * This library is distributed in the hope that it will be useful,
>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> - * Lesser General Public License for more details.
>> - *
>> - * You should have received a copy of the GNU Lesser General Public
>> - * License along with this library; if not, see 
>> <http://www.gnu.org/licenses/>
>> - */
>> -#ifndef ICC_BUS_H
>> -#define ICC_BUS_H
>> -
>> -#include "exec/memory.h"
>> -#include "hw/qdev-core.h"
>> -
>> -#define TYPE_ICC_BUS "icc-bus"
>> -
>> -#ifndef CONFIG_USER_ONLY
>> -
>> -/**
>> - * ICCBus:
>> - *
>> - * ICC bus
>> - */
>> -typedef struct ICCBus {
>> -    /*< private >*/
>> -    BusState parent_obj;
>> -    /*< public >*/
>> -
>> -    MemoryRegion *apic_address_space;
>> -} ICCBus;
>> -
>> -#define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS)
>> -
>> -/**
>> - * ICCDevice:
>> - *
>> - * ICC device
>> - */
>> -typedef struct ICCDevice {
>> -    /*< private >*/
>> -    DeviceState qdev;
>> -    /*< public >*/
>> -} ICCDevice;
>> -
>> -/**
>> - * ICCDeviceClass:
>> - * @init: Initialization callback for derived classes.
>> - *
>> - * ICC device class
>> - */
>> -typedef struct ICCDeviceClass {
>> -    /*< private >*/
>> -    DeviceClass parent_class;
>> -    /*< public >*/
>> -
>> -    DeviceRealize realize;
>> -} ICCDeviceClass;
>> -
>> -#define TYPE_ICC_DEVICE "icc-device"
>> -#define ICC_DEVICE(obj) OBJECT_CHECK(ICCDevice, (obj), TYPE_ICC_DEVICE)
>> -#define ICC_DEVICE_CLASS(klass) \
>> -     OBJECT_CLASS_CHECK(ICCDeviceClass, (klass), TYPE_ICC_DEVICE)
>> -#define ICC_DEVICE_GET_CLASS(obj) \
>> -     OBJECT_GET_CLASS(ICCDeviceClass, (obj), TYPE_ICC_DEVICE)
>> -
>> -#define TYPE_ICC_BRIDGE "icc-bridge"
>> -
>> -#endif /* CONFIG_USER_ONLY */
>> -#endif
>> diff --git a/include/hw/i386/apic_internal.h 
>> b/include/hw/i386/apic_internal.h
>> index dc7a89d..6eeceff 100644
>> --- a/include/hw/i386/apic_internal.h
>> +++ b/include/hw/i386/apic_internal.h
>> @@ -21,7 +21,6 @@
>>  #define QEMU_APIC_INTERNAL_H
>>  
>>  #include "exec/memory.h"
>> -#include "hw/cpu/icc_bus.h"
>>  #include "qemu/timer.h"
>>  
>>  /* APIC Local Vector Table */
>> @@ -78,7 +77,7 @@ typedef struct APICCommonState APICCommonState;
>>  
>>  typedef struct APICCommonClass
>>  {
>> -    ICCDeviceClass parent_class;
>> +    DeviceClass parent_class;
>>  
>>      DeviceRealize realize;
>>      void (*set_base)(APICCommonState *s, uint64_t val);
>> @@ -93,7 +92,7 @@ typedef struct APICCommonClass
>>  } APICCommonClass;
>>  
>>  struct APICCommonState {
>> -    ICCDevice busdev;
>> +    DeviceState dev;

Please name parent_obj. While at it, can you please add private/public
markers around the two?

>>  
>>      MemoryRegion io_memory;
>>      X86CPU *cpu;
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 1b35168..7c9d044 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -163,7 +163,7 @@ extern int fd_bootchk;
>>  void pc_register_ferr_irq(qemu_irq irq);
>>  void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
>>  
>> -void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
>> +void pc_cpus_init(const char *cpu_model);
>>  void pc_hot_add_cpu(const int64_t id, Error **errp);
>>  void pc_acpi_init(const char *default_dsdt);
>>  
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index 03b33cf..d11b219 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -42,7 +42,6 @@
>>  
>>  #include "sysemu/sysemu.h"
>>  #include "hw/qdev-properties.h"
>> -#include "hw/cpu/icc_bus.h"
>>  #ifndef CONFIG_USER_ONLY
>>  #include "hw/xen/xen.h"
>>  #include "hw/i386/apic_internal.h"
>> @@ -2634,6 +2633,14 @@ static void x86_cpu_reset(CPUState *s)
>>  
>>      xcc->parent_reset(s);
>>  
>> +    /*
>> +     * due to apic doesn't attach to sysbus, so when
>> +     * cpu reset, we should reset apic in the meanwhile.
> s/.../since APIC is a bus-less device, propagate reset to it manually
> 
>> +     */
>> +    if (cpu->apic_state) {
>> +        device_reset(DEVICE(cpu->apic_state));
>> +    }
>> +
>>      memset(env, 0, offsetof(CPUX86State, cpuid_level));
>>  
>>      tlb_flush(s, 1);
>> @@ -2757,7 +2764,6 @@ static void mce_init(X86CPU *cpu)
>>  #ifndef CONFIG_USER_ONLY
>>  static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
>>  {
>> -    DeviceState *dev = DEVICE(cpu);
>>      APICCommonState *apic;
>>      const char *apic_type = "apic";
>>  
>> @@ -2767,7 +2773,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error 
>> **errp)
>>          apic_type = "xen-apic";
>>      }
>>  
>> -    cpu->apic_state = qdev_try_create(qdev_get_parent_bus(dev), apic_type);
>> +    cpu->apic_state = DEVICE(object_new(apic_type));
>>      if (cpu->apic_state == NULL) {
> object_new() can't fail, hence drop this NULL check

Hm, depending on why this was a "try", there's multiple options for
keeping the current error handling:
* check whether the type exits before object_new(), cf. CPU
* use g_try_malloc0()+object_initialize() if it's a size issue

That said, I like where this is going. It should help with cleaning up
the recently discussed reset back-and-forth with BSP.

Regards,
Andreas

>>          error_setg(errp, "APIC device '%s' could not be created", 
>> apic_type);
>>          return;
>> @@ -3009,7 +3015,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
>> void *data)
>>  
>>      xcc->parent_realize = dc->realize;
>>      dc->realize = x86_cpu_realizefn;
>> -    dc->bus_type = TYPE_ICC_BUS;
>>      dc->props = x86_cpu_properties;
>>  
>>      xcc->parent_reset = cc->reset;

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]