Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-27 Thread liu ping fan 
Hi,

I want to rework on it according to your comments. Before that, just want to 
make clear that I understand your meanings exactly :)

According to the previous discussion, I will model the system according the 
rule -- if there is APIC in the system (including UP and MP), ICC bus will be 
created, otherwise no.

But there is a special case in UP scene,that is, if we make 8259a connect 
directly to APIC without using IOAPIC, as showed by Figure 3-3 in intel's 
MultiProcessor Specification, I think the rule can also be suitable.

So in board level initialization--pc1_init(), I will check _cpuid_features_  
CPUID_APIC to judge whether to create ICC or not.

Any objection?

Thanks and regards,
ping fan


On Tue, Oct 25, 2011 at 08:24:21PM +, Blue Swirl wrote:
 On Tue, Oct 25, 2011 at 08:55, liu ping fan qemul...@gmail.com wrote:
  On Sun, Oct 23, 2011 at 12:40:08PM +, Blue Swirl wrote:
  On Wed, Oct 19, 2011 at 01:55,  pingf...@linux.vnet.ibm.com wrote:
   From: Liu Ping Fan pingf...@linux.vnet.ibm.com
  
   Introduce a new structure CPUS as the controller of ICC (INTERRUPT
   CONTROLLER COMMUNICATIONS), and new bus ICC to hold APIC,instead
   of sysbus. So we can support APIC hot-plug feature.
 
  Is this ICC bus or APIC hot plugging documented somewhere?
 
   Signed-off-by: liu ping fan pingf...@linux.vnet.ibm.com
   ---
    Makefile.target |    1 +
    hw/apic.c       |   25 +++
    hw/apic.h       |    1 +
    hw/icc_bus.c    |   91 
   +++
    hw/icc_bus.h    |   56 ++
    hw/pc.c         |   11 --
    6 files changed, 174 insertions(+), 11 deletions(-)
    create mode 100644 hw/icc_bus.c
    create mode 100644 hw/icc_bus.h
  
   diff --git a/Makefile.target b/Makefile.target
   index 9011f28..5607c6d 100644
   --- a/Makefile.target
   +++ b/Makefile.target
   @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
    obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
    obj-i386-y += testdev.o
    obj-i386-y += acpi.o acpi_piix4.o
   +obj-i386-y += icc_bus.o
  
    obj-i386-y += pcspk.o i8254.o
    obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
   diff --git a/hw/apic.c b/hw/apic.c
   index 69d6ac5..00d2297 100644
   --- a/hw/apic.c
   +++ b/hw/apic.c
   @@ -21,9 +21,10 @@
    #include ioapic.h
    #include qemu-timer.h
    #include host-utils.h
   -#include sysbus.h
   +#include icc_bus.h
    #include trace.h
    #include kvm.h
   +#include exec-memory.h
  
    /* APIC Local Vector Table */
    #define APIC_LVT_TIMER   0
   @@ -80,7 +81,7 @@
    typedef struct APICState APICState;
  
    struct APICState {
   -    SysBusDevice busdev;
   +    ICCBusDevice busdev;
       MemoryRegion io_memory;
       void *cpu_env;
       uint32_t apicbase;
   @@ -1104,9 +1105,20 @@ static const MemoryRegionOps apic_io_ops = {
       .endianness = DEVICE_NATIVE_ENDIAN,
    };
  
   -static int apic_init1(SysBusDevice *dev)
   +/**/
   +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
    {
   -    APICState *s = FROM_SYSBUS(APICState, dev);
   +    APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
   +
   +    memory_region_add_subregion(get_system_memory(),
   +                                base,
   +                                s-io_memory);
   +    return 0;
   +}
   +
   +static int apic_init1(ICCBusDevice *dev)
   +{
   +    APICState *s = DO_UPCAST(APICState, busdev, dev);
       static int last_apic_idx;
  
       if (last_apic_idx = MAX_APICS) {
   @@ -1114,7 +1126,6 @@ static int apic_init1(SysBusDevice *dev)
       }
       memory_region_init_io(s-io_memory, apic_io_ops, s, apic,
                             MSI_ADDR_SIZE);
   -    sysbus_init_mmio_region(dev, s-io_memory);
  
       s-timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
       s-idx = last_apic_idx++;
   @@ -1122,7 +1133,7 @@ static int apic_init1(SysBusDevice *dev)
       return 0;
    }
  
   -static SysBusDeviceInfo apic_info = {
   +static ICCBusDeviceInfo apic_info = {
       .init = apic_init1,
       .qdev.name = apic,
       .qdev.size = sizeof(APICState),
   @@ -1138,7 +1149,7 @@ static SysBusDeviceInfo apic_info = {
  
    static void apic_register_devices(void)
    {
   -    sysbus_register_withprop(apic_info);
   +    iccbus_register_devinfo(apic_info);
    }
  
    device_init(apic_register_devices)
   diff --git a/hw/apic.h b/hw/apic.h
   index c857d52..e2c0af5 100644
   --- a/hw/apic.h
   +++ b/hw/apic.h
   @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
    uint8_t cpu_get_apic_tpr(DeviceState *s);
    void apic_init_reset(DeviceState *s);
    void apic_sipi(DeviceState *s);
   +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);
  
    /* pc.c */
    int cpu_is_bsp(CPUState *env);
   diff --git a/hw/icc_bus.c b/hw/icc_bus.c
   new file mode 100644
   index 000..61a408e
   --- /dev/null
   +++ b/hw/icc_bus.c
   @@ -0,0 +1,91 @@
   +/* icc_bus.c
   + *

Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-27 Thread Blue Swirl 
On Thu, Oct 27, 2011 at 08:41, liu ping fan qemul...@gmail.com wrote:
 Hi,

 I want to rework on it according to your comments. Before that, just want to 
 make clear that I understand your meanings exactly :)

 According to the previous discussion, I will model the system according the 
 rule -- if there is APIC in the system (including UP and MP), ICC bus will be 
 created, otherwise no.

 But there is a special case in UP scene,that is, if we make 8259a connect 
 directly to APIC without using IOAPIC, as showed by Figure 3-3 in intel's 
 MultiProcessor Specification, I think the rule can also be suitable.

I think this is handled for some part by pic_irq_request() in pc.c.

 So in board level initialization--pc1_init(), I will check _cpuid_features_  
 CPUID_APIC to judge whether to create ICC or not.

 Any objection?

OK for me.

 Thanks and regards,
 ping fan


 On Tue, Oct 25, 2011 at 08:24:21PM +, Blue Swirl wrote:
 On Tue, Oct 25, 2011 at 08:55, liu ping fan qemul...@gmail.com wrote:
  On Sun, Oct 23, 2011 at 12:40:08PM +, Blue Swirl wrote:
  On Wed, Oct 19, 2011 at 01:55,  pingf...@linux.vnet.ibm.com wrote:
   From: Liu Ping Fan pingf...@linux.vnet.ibm.com
  
   Introduce a new structure CPUS as the controller of ICC (INTERRUPT
   CONTROLLER COMMUNICATIONS), and new bus ICC to hold APIC,instead
   of sysbus. So we can support APIC hot-plug feature.
 
  Is this ICC bus or APIC hot plugging documented somewhere?
 
   Signed-off-by: liu ping fan pingf...@linux.vnet.ibm.com
   ---
    Makefile.target |    1 +
    hw/apic.c       |   25 +++
    hw/apic.h       |    1 +
    hw/icc_bus.c    |   91 
   +++
    hw/icc_bus.h    |   56 ++
    hw/pc.c         |   11 --
    6 files changed, 174 insertions(+), 11 deletions(-)
    create mode 100644 hw/icc_bus.c
    create mode 100644 hw/icc_bus.h
  
   diff --git a/Makefile.target b/Makefile.target
   index 9011f28..5607c6d 100644
   --- a/Makefile.target
   +++ b/Makefile.target
   @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
    obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
    obj-i386-y += testdev.o
    obj-i386-y += acpi.o acpi_piix4.o
   +obj-i386-y += icc_bus.o
  
    obj-i386-y += pcspk.o i8254.o
    obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
   diff --git a/hw/apic.c b/hw/apic.c
   index 69d6ac5..00d2297 100644
   --- a/hw/apic.c
   +++ b/hw/apic.c
   @@ -21,9 +21,10 @@
    #include ioapic.h
    #include qemu-timer.h
    #include host-utils.h
   -#include sysbus.h
   +#include icc_bus.h
    #include trace.h
    #include kvm.h
   +#include exec-memory.h
  
    /* APIC Local Vector Table */
    #define APIC_LVT_TIMER   0
   @@ -80,7 +81,7 @@
    typedef struct APICState APICState;
  
    struct APICState {
   -    SysBusDevice busdev;
   +    ICCBusDevice busdev;
       MemoryRegion io_memory;
       void *cpu_env;
       uint32_t apicbase;
   @@ -1104,9 +1105,20 @@ static const MemoryRegionOps apic_io_ops = {
       .endianness = DEVICE_NATIVE_ENDIAN,
    };
  
   -static int apic_init1(SysBusDevice *dev)
   +/**/
   +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
    {
   -    APICState *s = FROM_SYSBUS(APICState, dev);
   +    APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
   +
   +    memory_region_add_subregion(get_system_memory(),
   +                                base,
   +                                s-io_memory);
   +    return 0;
   +}
   +
   +static int apic_init1(ICCBusDevice *dev)
   +{
   +    APICState *s = DO_UPCAST(APICState, busdev, dev);
       static int last_apic_idx;
  
       if (last_apic_idx = MAX_APICS) {
   @@ -1114,7 +1126,6 @@ static int apic_init1(SysBusDevice *dev)
       }
       memory_region_init_io(s-io_memory, apic_io_ops, s, apic,
                             MSI_ADDR_SIZE);
   -    sysbus_init_mmio_region(dev, s-io_memory);
  
       s-timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
       s-idx = last_apic_idx++;
   @@ -1122,7 +1133,7 @@ static int apic_init1(SysBusDevice *dev)
       return 0;
    }
  
   -static SysBusDeviceInfo apic_info = {
   +static ICCBusDeviceInfo apic_info = {
       .init = apic_init1,
       .qdev.name = apic,
       .qdev.size = sizeof(APICState),
   @@ -1138,7 +1149,7 @@ static SysBusDeviceInfo apic_info = {
  
    static void apic_register_devices(void)
    {
   -    sysbus_register_withprop(apic_info);
   +    iccbus_register_devinfo(apic_info);
    }
  
    device_init(apic_register_devices)
   diff --git a/hw/apic.h b/hw/apic.h
   index c857d52..e2c0af5 100644
   --- a/hw/apic.h
   +++ b/hw/apic.h
   @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
    uint8_t cpu_get_apic_tpr(DeviceState *s);
    void apic_init_reset(DeviceState *s);
    void apic_sipi(DeviceState *s);
   +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);
  
    /* pc.c */
    int cpu_is_bsp(CPUState *env);
   diff

Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-25 Thread liu ping fan 
On Sun, Oct 23, 2011 at 12:40:08PM +, Blue Swirl wrote:
 On Wed, Oct 19, 2011 at 01:55,  pingf...@linux.vnet.ibm.com wrote:
  From: Liu Ping Fan pingf...@linux.vnet.ibm.com
 
  Introduce a new structure CPUS as the controller of ICC (INTERRUPT
  CONTROLLER COMMUNICATIONS), and new bus ICC to hold APIC,instead
  of sysbus. So we can support APIC hot-plug feature.
 
 Is this ICC bus or APIC hot plugging documented somewhere?
 
  Signed-off-by: liu ping fan pingf...@linux.vnet.ibm.com
  ---
   Makefile.target |    1 +
   hw/apic.c       |   25 +++
   hw/apic.h       |    1 +
   hw/icc_bus.c    |   91 
  +++
   hw/icc_bus.h    |   56 ++
   hw/pc.c         |   11 --
   6 files changed, 174 insertions(+), 11 deletions(-)
   create mode 100644 hw/icc_bus.c
   create mode 100644 hw/icc_bus.h
 
  diff --git a/Makefile.target b/Makefile.target
  index 9011f28..5607c6d 100644
  --- a/Makefile.target
  +++ b/Makefile.target
  @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
   obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
   obj-i386-y += testdev.o
   obj-i386-y += acpi.o acpi_piix4.o
  +obj-i386-y += icc_bus.o
 
   obj-i386-y += pcspk.o i8254.o
   obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
  diff --git a/hw/apic.c b/hw/apic.c
  index 69d6ac5..00d2297 100644
  --- a/hw/apic.c
  +++ b/hw/apic.c
  @@ -21,9 +21,10 @@
   #include ioapic.h
   #include qemu-timer.h
   #include host-utils.h
  -#include sysbus.h
  +#include icc_bus.h
   #include trace.h
   #include kvm.h
  +#include exec-memory.h
 
   /* APIC Local Vector Table */
   #define APIC_LVT_TIMER   0
  @@ -80,7 +81,7 @@
   typedef struct APICState APICState;
 
   struct APICState {
  -    SysBusDevice busdev;
  +    ICCBusDevice busdev;
      MemoryRegion io_memory;
      void *cpu_env;
      uint32_t apicbase;
  @@ -1104,9 +1105,20 @@ static const MemoryRegionOps apic_io_ops = {
      .endianness = DEVICE_NATIVE_ENDIAN,
   };
 
  -static int apic_init1(SysBusDevice *dev)
  +/**/
  +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
   {
  -    APICState *s = FROM_SYSBUS(APICState, dev);
  +    APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
  +
  +    memory_region_add_subregion(get_system_memory(),
  +                                base,
  +                                s-io_memory);
  +    return 0;
  +}
  +
  +static int apic_init1(ICCBusDevice *dev)
  +{
  +    APICState *s = DO_UPCAST(APICState, busdev, dev);
      static int last_apic_idx;
 
      if (last_apic_idx = MAX_APICS) {
  @@ -1114,7 +1126,6 @@ static int apic_init1(SysBusDevice *dev)
      }
      memory_region_init_io(s-io_memory, apic_io_ops, s, apic,
                            MSI_ADDR_SIZE);
  -    sysbus_init_mmio_region(dev, s-io_memory);
 
      s-timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
      s-idx = last_apic_idx++;
  @@ -1122,7 +1133,7 @@ static int apic_init1(SysBusDevice *dev)
      return 0;
   }
 
  -static SysBusDeviceInfo apic_info = {
  +static ICCBusDeviceInfo apic_info = {
      .init = apic_init1,
      .qdev.name = apic,
      .qdev.size = sizeof(APICState),
  @@ -1138,7 +1149,7 @@ static SysBusDeviceInfo apic_info = {
 
   static void apic_register_devices(void)
   {
  -    sysbus_register_withprop(apic_info);
  +    iccbus_register_devinfo(apic_info);
   }
 
   device_init(apic_register_devices)
  diff --git a/hw/apic.h b/hw/apic.h
  index c857d52..e2c0af5 100644
  --- a/hw/apic.h
  +++ b/hw/apic.h
  @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
   uint8_t cpu_get_apic_tpr(DeviceState *s);
   void apic_init_reset(DeviceState *s);
   void apic_sipi(DeviceState *s);
  +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);
 
   /* pc.c */
   int cpu_is_bsp(CPUState *env);
  diff --git a/hw/icc_bus.c b/hw/icc_bus.c
  new file mode 100644
  index 000..61a408e
  --- /dev/null
  +++ b/hw/icc_bus.c
  @@ -0,0 +1,91 @@
  +/* icc_bus.c
  + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
  + *
  + * 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 icc_bus.h
  +
  +static CPUS *dummy_cpus;
  +
  +
  +static ICCBusInfo icc_bus_info = {
  +    .qinfo.name = icc,
  +    .qinfo.size = sizeof(ICCBus),
  +    .qinfo.props = (Property[]) {

Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-25 Thread Blue Swirl 
On Tue, Oct 25, 2011 at 08:55, liu ping fan qemul...@gmail.com wrote:
 On Sun, Oct 23, 2011 at 12:40:08PM +, Blue Swirl wrote:
 On Wed, Oct 19, 2011 at 01:55,  pingf...@linux.vnet.ibm.com wrote:
  From: Liu Ping Fan pingf...@linux.vnet.ibm.com
 
  Introduce a new structure CPUS as the controller of ICC (INTERRUPT
  CONTROLLER COMMUNICATIONS), and new bus ICC to hold APIC,instead
  of sysbus. So we can support APIC hot-plug feature.

 Is this ICC bus or APIC hot plugging documented somewhere?

  Signed-off-by: liu ping fan pingf...@linux.vnet.ibm.com
  ---
   Makefile.target |    1 +
   hw/apic.c       |   25 +++
   hw/apic.h       |    1 +
   hw/icc_bus.c    |   91 
  +++
   hw/icc_bus.h    |   56 ++
   hw/pc.c         |   11 --
   6 files changed, 174 insertions(+), 11 deletions(-)
   create mode 100644 hw/icc_bus.c
   create mode 100644 hw/icc_bus.h
 
  diff --git a/Makefile.target b/Makefile.target
  index 9011f28..5607c6d 100644
  --- a/Makefile.target
  +++ b/Makefile.target
  @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
   obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
   obj-i386-y += testdev.o
   obj-i386-y += acpi.o acpi_piix4.o
  +obj-i386-y += icc_bus.o
 
   obj-i386-y += pcspk.o i8254.o
   obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
  diff --git a/hw/apic.c b/hw/apic.c
  index 69d6ac5..00d2297 100644
  --- a/hw/apic.c
  +++ b/hw/apic.c
  @@ -21,9 +21,10 @@
   #include ioapic.h
   #include qemu-timer.h
   #include host-utils.h
  -#include sysbus.h
  +#include icc_bus.h
   #include trace.h
   #include kvm.h
  +#include exec-memory.h
 
   /* APIC Local Vector Table */
   #define APIC_LVT_TIMER   0
  @@ -80,7 +81,7 @@
   typedef struct APICState APICState;
 
   struct APICState {
  -    SysBusDevice busdev;
  +    ICCBusDevice busdev;
      MemoryRegion io_memory;
      void *cpu_env;
      uint32_t apicbase;
  @@ -1104,9 +1105,20 @@ static const MemoryRegionOps apic_io_ops = {
      .endianness = DEVICE_NATIVE_ENDIAN,
   };
 
  -static int apic_init1(SysBusDevice *dev)
  +/**/
  +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
   {
  -    APICState *s = FROM_SYSBUS(APICState, dev);
  +    APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
  +
  +    memory_region_add_subregion(get_system_memory(),
  +                                base,
  +                                s-io_memory);
  +    return 0;
  +}
  +
  +static int apic_init1(ICCBusDevice *dev)
  +{
  +    APICState *s = DO_UPCAST(APICState, busdev, dev);
      static int last_apic_idx;
 
      if (last_apic_idx = MAX_APICS) {
  @@ -1114,7 +1126,6 @@ static int apic_init1(SysBusDevice *dev)
      }
      memory_region_init_io(s-io_memory, apic_io_ops, s, apic,
                            MSI_ADDR_SIZE);
  -    sysbus_init_mmio_region(dev, s-io_memory);
 
      s-timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
      s-idx = last_apic_idx++;
  @@ -1122,7 +1133,7 @@ static int apic_init1(SysBusDevice *dev)
      return 0;
   }
 
  -static SysBusDeviceInfo apic_info = {
  +static ICCBusDeviceInfo apic_info = {
      .init = apic_init1,
      .qdev.name = apic,
      .qdev.size = sizeof(APICState),
  @@ -1138,7 +1149,7 @@ static SysBusDeviceInfo apic_info = {
 
   static void apic_register_devices(void)
   {
  -    sysbus_register_withprop(apic_info);
  +    iccbus_register_devinfo(apic_info);
   }
 
   device_init(apic_register_devices)
  diff --git a/hw/apic.h b/hw/apic.h
  index c857d52..e2c0af5 100644
  --- a/hw/apic.h
  +++ b/hw/apic.h
  @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
   uint8_t cpu_get_apic_tpr(DeviceState *s);
   void apic_init_reset(DeviceState *s);
   void apic_sipi(DeviceState *s);
  +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);
 
   /* pc.c */
   int cpu_is_bsp(CPUState *env);
  diff --git a/hw/icc_bus.c b/hw/icc_bus.c
  new file mode 100644
  index 000..61a408e
  --- /dev/null
  +++ b/hw/icc_bus.c
  @@ -0,0 +1,91 @@
  +/* icc_bus.c
  + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
  + *
  + * 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 icc_bus.h
  +
  +static CPUS *dummy_cpus;
  +
  +
  +static ICCBusInfo icc_bus_info = {
  +    .qinfo.name = icc,
  +    

Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-23 Thread Blue Swirl 
On Wed, Oct 19, 2011 at 01:55,  pingf...@linux.vnet.ibm.com wrote:
 From: Liu Ping Fan pingf...@linux.vnet.ibm.com

 Introduce a new structure CPUS as the controller of ICC (INTERRUPT
 CONTROLLER COMMUNICATIONS), and new bus ICC to hold APIC,instead
 of sysbus. So we can support APIC hot-plug feature.

Is this ICC bus or APIC hot plugging documented somewhere?

 Signed-off-by: liu ping fan pingf...@linux.vnet.ibm.com
 ---
  Makefile.target |    1 +
  hw/apic.c       |   25 +++
  hw/apic.h       |    1 +
  hw/icc_bus.c    |   91 
 +++
  hw/icc_bus.h    |   56 ++
  hw/pc.c         |   11 --
  6 files changed, 174 insertions(+), 11 deletions(-)
  create mode 100644 hw/icc_bus.c
  create mode 100644 hw/icc_bus.h

 diff --git a/Makefile.target b/Makefile.target
 index 9011f28..5607c6d 100644
 --- a/Makefile.target
 +++ b/Makefile.target
 @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
  obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
  obj-i386-y += testdev.o
  obj-i386-y += acpi.o acpi_piix4.o
 +obj-i386-y += icc_bus.o

  obj-i386-y += pcspk.o i8254.o
  obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
 diff --git a/hw/apic.c b/hw/apic.c
 index 69d6ac5..00d2297 100644
 --- a/hw/apic.c
 +++ b/hw/apic.c
 @@ -21,9 +21,10 @@
  #include ioapic.h
  #include qemu-timer.h
  #include host-utils.h
 -#include sysbus.h
 +#include icc_bus.h
  #include trace.h
  #include kvm.h
 +#include exec-memory.h

  /* APIC Local Vector Table */
  #define APIC_LVT_TIMER   0
 @@ -80,7 +81,7 @@
  typedef struct APICState APICState;

  struct APICState {
 -    SysBusDevice busdev;
 +    ICCBusDevice busdev;
     MemoryRegion io_memory;
     void *cpu_env;
     uint32_t apicbase;
 @@ -1104,9 +1105,20 @@ static const MemoryRegionOps apic_io_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
  };

 -static int apic_init1(SysBusDevice *dev)
 +/**/
 +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
  {
 -    APICState *s = FROM_SYSBUS(APICState, dev);
 +    APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
 +
 +    memory_region_add_subregion(get_system_memory(),
 +                                base,
 +                                s-io_memory);
 +    return 0;
 +}
 +
 +static int apic_init1(ICCBusDevice *dev)
 +{
 +    APICState *s = DO_UPCAST(APICState, busdev, dev);
     static int last_apic_idx;

     if (last_apic_idx = MAX_APICS) {
 @@ -1114,7 +1126,6 @@ static int apic_init1(SysBusDevice *dev)
     }
     memory_region_init_io(s-io_memory, apic_io_ops, s, apic,
                           MSI_ADDR_SIZE);
 -    sysbus_init_mmio_region(dev, s-io_memory);

     s-timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
     s-idx = last_apic_idx++;
 @@ -1122,7 +1133,7 @@ static int apic_init1(SysBusDevice *dev)
     return 0;
  }

 -static SysBusDeviceInfo apic_info = {
 +static ICCBusDeviceInfo apic_info = {
     .init = apic_init1,
     .qdev.name = apic,
     .qdev.size = sizeof(APICState),
 @@ -1138,7 +1149,7 @@ static SysBusDeviceInfo apic_info = {

  static void apic_register_devices(void)
  {
 -    sysbus_register_withprop(apic_info);
 +    iccbus_register_devinfo(apic_info);
  }

  device_init(apic_register_devices)
 diff --git a/hw/apic.h b/hw/apic.h
 index c857d52..e2c0af5 100644
 --- a/hw/apic.h
 +++ b/hw/apic.h
 @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
  uint8_t cpu_get_apic_tpr(DeviceState *s);
  void apic_init_reset(DeviceState *s);
  void apic_sipi(DeviceState *s);
 +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);

  /* pc.c */
  int cpu_is_bsp(CPUState *env);
 diff --git a/hw/icc_bus.c b/hw/icc_bus.c
 new file mode 100644
 index 000..61a408e
 --- /dev/null
 +++ b/hw/icc_bus.c
 @@ -0,0 +1,91 @@
 +/* icc_bus.c
 + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
 + *
 + * 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 icc_bus.h
 +
 +static CPUS *dummy_cpus;
 +
 +
 +static ICCBusInfo icc_bus_info = {
 +    .qinfo.name = icc,
 +    .qinfo.size = sizeof(ICCBus),
 +    .qinfo.props = (Property[]) {
 +        DEFINE_PROP_END_OF_LIST(),
 +    }
 +};
 +
 +
 +
 +static int iccbus_device_init(DeviceState *dev, DeviceInfo *base)
 +{
 +    ICCBusDeviceInfo *info = container_of(base, ICCBusDeviceInfo, qdev);
 +    

Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-23 Thread Jan Kiszka 
On 2011-10-23 14:40, Blue Swirl wrote:
 I'm not sure that a full bus is needed for now, even if it could match
 real HW better, since the memory API already provides the separation
 needed. Perhaps this would be needed later to make IRQs per-CPU also,
 or to put IOAPIC also to the bus?

The ICC interconnects LAPICs and IOAPICs. So it should next take over
the management of the local_apics array from apic.c and the ioapics
array from ioapic.c. It could implement generic message delivery
services. Every bus participant would then have a reception handler that
first checks the type and additional fields of a generic ICC message
and, on match, forwards it to the corresponding device model functions.
That would allow for something nicer than global apic_deliver_irq or
ioapic_eoi_broadcast.

That's clearly beyond the scope of this series but a good reason to
model the ICC as accurately as qdev allows right from the start.

Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-23 Thread Blue Swirl 
On Sun, Oct 23, 2011 at 15:45, Jan Kiszka jan.kis...@web.de wrote:
 On 2011-10-23 14:40, Blue Swirl wrote:
 I'm not sure that a full bus is needed for now, even if it could match
 real HW better, since the memory API already provides the separation
 needed. Perhaps this would be needed later to make IRQs per-CPU also,
 or to put IOAPIC also to the bus?

 The ICC interconnects LAPICs and IOAPICs.

But not between CPU core and its LAPIC?

 So it should next take over
 the management of the local_apics array from apic.c and the ioapics
 array from ioapic.c. It could implement generic message delivery
 services. Every bus participant would then have a reception handler that
 first checks the type and additional fields of a generic ICC message
 and, on match, forwards it to the corresponding device model functions.
 That would allow for something nicer than global apic_deliver_irq or
 ioapic_eoi_broadcast.

 That's clearly beyond the scope of this series but a good reason to
 model the ICC as accurately as qdev allows right from the start.

OK then, ICC could be a major cleanup.

Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-23 Thread Jan Kiszka 
On 2011-10-23 17:54, Blue Swirl wrote:
 On Sun, Oct 23, 2011 at 15:45, Jan Kiszka jan.kis...@web.de wrote:
 On 2011-10-23 14:40, Blue Swirl wrote:
 I'm not sure that a full bus is needed for now, even if it could match
 real HW better, since the memory API already provides the separation
 needed. Perhaps this would be needed later to make IRQs per-CPU also,
 or to put IOAPIC also to the bus?

 The ICC interconnects LAPICs and IOAPICs.
 
 But not between CPU core and its LAPIC?

Nope, that link is core-internal and has nothing to do with the ICC. See
e.g. Figure 2-2 of Intel's MultiProcessor Specification.

Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-20 Thread liu ping fan 
On Wed, Oct 19, 2011 at 03:42:27PM +0200, Jan Kiszka wrote:
 On 2011-10-19 15:33, Jan Kiszka wrote:
  On 2011-10-19 14:54, Anthony Liguori wrote:
  On 10/19/2011 05:53 AM, Jan Kiszka wrote:
  On 2011-10-19 03:55, pingf...@linux.vnet.ibm.com wrote:
  From: Liu Ping Fanpingf...@linux.vnet.ibm.com
 
  Introduce a new structure CPUS as the controller of ICC (INTERRUPT
  CONTROLLER COMMUNICATIONS), and new bus ICC to hold APIC,instead
  of sysbus. So we can support APIC hot-plug feature.
 
  Signed-off-by: liu ping fanpingf...@linux.vnet.ibm.com
  ---
Makefile.target |1 +
hw/apic.c   |   25 +++
hw/apic.h   |1 +
hw/icc_bus.c|   91 
  +++
hw/icc_bus.h|   56 ++
hw/pc.c |   11 --
6 files changed, 174 insertions(+), 11 deletions(-)
create mode 100644 hw/icc_bus.c
create mode 100644 hw/icc_bus.h
 
  diff --git a/Makefile.target b/Makefile.target
  index 9011f28..5607c6d 100644
  --- a/Makefile.target
  +++ b/Makefile.target
  @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
obj-i386-y += testdev.o
obj-i386-y += acpi.o acpi_piix4.o
  +obj-i386-y += icc_bus.o
 
obj-i386-y += pcspk.o i8254.o
obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
  diff --git a/hw/apic.c b/hw/apic.c
  index 69d6ac5..00d2297 100644
  --- a/hw/apic.c
  +++ b/hw/apic.c
  @@ -21,9 +21,10 @@
#include ioapic.h
#include qemu-timer.h
#include host-utils.h
  -#include sysbus.h
  +#include icc_bus.h
#include trace.h
#include kvm.h
  +#include exec-memory.h
 
  Mmh, don't your rather want memory.h?
 
 
/* APIC Local Vector Table */
#define APIC_LVT_TIMER   0
  @@ -80,7 +81,7 @@
typedef struct APICState APICState;
 
struct APICState {
  -SysBusDevice busdev;
  +ICCBusDevice busdev;
MemoryRegion io_memory;
void *cpu_env;
uint32_t apicbase;
  @@ -1104,9 +1105,20 @@ static const MemoryRegionOps apic_io_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
 
  -static int apic_init1(SysBusDevice *dev)
  +/**/
 
  Empty comment.
 
  +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
{
  -APICState *s = FROM_SYSBUS(APICState, dev);
  +APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
  +
  +memory_region_add_subregion(get_system_memory(),
  +base,
  +s-io_memory);
  +return 0;
  +}
  +
  +static int apic_init1(ICCBusDevice *dev)
  +{
  +APICState *s = DO_UPCAST(APICState, busdev, dev);
static int last_apic_idx;
 
if (last_apic_idx= MAX_APICS) {
  @@ -1114,7 +1126,6 @@ static int apic_init1(SysBusDevice *dev)
}
memory_region_init_io(s-io_memory,apic_io_ops, s, apic,
  MSI_ADDR_SIZE);
  -sysbus_init_mmio_region(dev,s-io_memory);
 
s-timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
s-idx = last_apic_idx++;
  @@ -1122,7 +1133,7 @@ static int apic_init1(SysBusDevice *dev)
return 0;
}
 
  -static SysBusDeviceInfo apic_info = {
  +static ICCBusDeviceInfo apic_info = {
.init = apic_init1,
.qdev.name = apic,
.qdev.size = sizeof(APICState),
  @@ -1138,7 +1149,7 @@ static SysBusDeviceInfo apic_info = {
 
static void apic_register_devices(void)
{
  -sysbus_register_withprop(apic_info);
  +iccbus_register_devinfo(apic_info);
}
 
device_init(apic_register_devices)
  diff --git a/hw/apic.h b/hw/apic.h
  index c857d52..e2c0af5 100644
  --- a/hw/apic.h
  +++ b/hw/apic.h
  @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
uint8_t cpu_get_apic_tpr(DeviceState *s);
void apic_init_reset(DeviceState *s);
void apic_sipi(DeviceState *s);
  +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);
 
/* pc.c */
int cpu_is_bsp(CPUState *env);
  diff --git a/hw/icc_bus.c b/hw/icc_bus.c
  new file mode 100644
  index 000..61a408e
  --- /dev/null
  +++ b/hw/icc_bus.c
  @@ -0,0 +1,91 @@
  +/* icc_bus.c
  + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
 
  Copyright?
 
  + *
  + * 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, 
  seehttp://www.gnu.org/licenses/
  + */
  +#include icc_bus.h
  +
  +static

Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-19 Thread Jan Kiszka 
On 2011-10-19 03:55, pingf...@linux.vnet.ibm.com wrote:
 From: Liu Ping Fan pingf...@linux.vnet.ibm.com
 
 Introduce a new structure CPUS as the controller of ICC (INTERRUPT
 CONTROLLER COMMUNICATIONS), and new bus ICC to hold APIC,instead
 of sysbus. So we can support APIC hot-plug feature.
 
 Signed-off-by: liu ping fan pingf...@linux.vnet.ibm.com
 ---
  Makefile.target |1 +
  hw/apic.c   |   25 +++
  hw/apic.h   |1 +
  hw/icc_bus.c|   91 
 +++
  hw/icc_bus.h|   56 ++
  hw/pc.c |   11 --
  6 files changed, 174 insertions(+), 11 deletions(-)
  create mode 100644 hw/icc_bus.c
  create mode 100644 hw/icc_bus.h
 
 diff --git a/Makefile.target b/Makefile.target
 index 9011f28..5607c6d 100644
 --- a/Makefile.target
 +++ b/Makefile.target
 @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
  obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
  obj-i386-y += testdev.o
  obj-i386-y += acpi.o acpi_piix4.o
 +obj-i386-y += icc_bus.o
  
  obj-i386-y += pcspk.o i8254.o
  obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
 diff --git a/hw/apic.c b/hw/apic.c
 index 69d6ac5..00d2297 100644
 --- a/hw/apic.c
 +++ b/hw/apic.c
 @@ -21,9 +21,10 @@
  #include ioapic.h
  #include qemu-timer.h
  #include host-utils.h
 -#include sysbus.h
 +#include icc_bus.h
  #include trace.h
  #include kvm.h
 +#include exec-memory.h

Mmh, don't your rather want memory.h?

  
  /* APIC Local Vector Table */
  #define APIC_LVT_TIMER   0
 @@ -80,7 +81,7 @@
  typedef struct APICState APICState;
  
  struct APICState {
 -SysBusDevice busdev;
 +ICCBusDevice busdev;
  MemoryRegion io_memory;
  void *cpu_env;
  uint32_t apicbase;
 @@ -1104,9 +1105,20 @@ static const MemoryRegionOps apic_io_ops = {
  .endianness = DEVICE_NATIVE_ENDIAN,
  };
  
 -static int apic_init1(SysBusDevice *dev)
 +/**/

Empty comment.

 +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
  {
 -APICState *s = FROM_SYSBUS(APICState, dev);
 +APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
 +
 +memory_region_add_subregion(get_system_memory(),
 +base,
 +s-io_memory);
 +return 0;
 +}
 +
 +static int apic_init1(ICCBusDevice *dev)
 +{
 +APICState *s = DO_UPCAST(APICState, busdev, dev);
  static int last_apic_idx;
  
  if (last_apic_idx = MAX_APICS) {
 @@ -1114,7 +1126,6 @@ static int apic_init1(SysBusDevice *dev)
  }
  memory_region_init_io(s-io_memory, apic_io_ops, s, apic,
MSI_ADDR_SIZE);
 -sysbus_init_mmio_region(dev, s-io_memory);
  
  s-timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
  s-idx = last_apic_idx++;
 @@ -1122,7 +1133,7 @@ static int apic_init1(SysBusDevice *dev)
  return 0;
  }
  
 -static SysBusDeviceInfo apic_info = {
 +static ICCBusDeviceInfo apic_info = {
  .init = apic_init1,
  .qdev.name = apic,
  .qdev.size = sizeof(APICState),
 @@ -1138,7 +1149,7 @@ static SysBusDeviceInfo apic_info = {
  
  static void apic_register_devices(void)
  {
 -sysbus_register_withprop(apic_info);
 +iccbus_register_devinfo(apic_info);
  }
  
  device_init(apic_register_devices)
 diff --git a/hw/apic.h b/hw/apic.h
 index c857d52..e2c0af5 100644
 --- a/hw/apic.h
 +++ b/hw/apic.h
 @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
  uint8_t cpu_get_apic_tpr(DeviceState *s);
  void apic_init_reset(DeviceState *s);
  void apic_sipi(DeviceState *s);
 +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);
  
  /* pc.c */
  int cpu_is_bsp(CPUState *env);
 diff --git a/hw/icc_bus.c b/hw/icc_bus.c
 new file mode 100644
 index 000..61a408e
 --- /dev/null
 +++ b/hw/icc_bus.c
 @@ -0,0 +1,91 @@
 +/* icc_bus.c
 + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus

Copyright?

 + *
 + * 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 icc_bus.h
 +
 +static CPUS *dummy_cpus;

Why dummy? Also, no statics please. The bus owner is the chipset
(440fx), so embedded it there. That also avoid that strange cpus
device below.

 +
 +
 +static ICCBusInfo icc_bus_info = {
 +.qinfo.name = icc,
 +.qinfo.size = sizeof(ICCBus),
 +.qinfo.props = (Property[]) {
 +

Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-19 Thread Anthony Liguori 

On 10/19/2011 05:53 AM, Jan Kiszka wrote:

On 2011-10-19 03:55, pingf...@linux.vnet.ibm.com wrote:

From: Liu Ping Fanpingf...@linux.vnet.ibm.com

Introduce a new structure CPUS as the controller of ICC (INTERRUPT
CONTROLLER COMMUNICATIONS), and new bus ICC to hold APIC,instead
of sysbus. So we can support APIC hot-plug feature.

Signed-off-by: liu ping fanpingf...@linux.vnet.ibm.com
---
  Makefile.target |1 +
  hw/apic.c   |   25 +++
  hw/apic.h   |1 +
  hw/icc_bus.c|   91 +++
  hw/icc_bus.h|   56 ++
  hw/pc.c |   11 --
  6 files changed, 174 insertions(+), 11 deletions(-)
  create mode 100644 hw/icc_bus.c
  create mode 100644 hw/icc_bus.h

diff --git a/Makefile.target b/Makefile.target
index 9011f28..5607c6d 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
  obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
  obj-i386-y += testdev.o
  obj-i386-y += acpi.o acpi_piix4.o
+obj-i386-y += icc_bus.o

  obj-i386-y += pcspk.o i8254.o
  obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
diff --git a/hw/apic.c b/hw/apic.c
index 69d6ac5..00d2297 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -21,9 +21,10 @@
  #include ioapic.h
  #include qemu-timer.h
  #include host-utils.h
-#include sysbus.h
+#include icc_bus.h
  #include trace.h
  #include kvm.h
+#include exec-memory.h


Mmh, don't your rather want memory.h?



  /* APIC Local Vector Table */
  #define APIC_LVT_TIMER   0
@@ -80,7 +81,7 @@
  typedef struct APICState APICState;

  struct APICState {
-SysBusDevice busdev;
+ICCBusDevice busdev;
  MemoryRegion io_memory;
  void *cpu_env;
  uint32_t apicbase;
@@ -1104,9 +1105,20 @@ static const MemoryRegionOps apic_io_ops = {
  .endianness = DEVICE_NATIVE_ENDIAN,
  };

-static int apic_init1(SysBusDevice *dev)
+/**/


Empty comment.


+int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
  {
-APICState *s = FROM_SYSBUS(APICState, dev);
+APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
+
+memory_region_add_subregion(get_system_memory(),
+base,
+s-io_memory);
+return 0;
+}
+
+static int apic_init1(ICCBusDevice *dev)
+{
+APICState *s = DO_UPCAST(APICState, busdev, dev);
  static int last_apic_idx;

  if (last_apic_idx= MAX_APICS) {
@@ -1114,7 +1126,6 @@ static int apic_init1(SysBusDevice *dev)
  }
  memory_region_init_io(s-io_memory,apic_io_ops, s, apic,
MSI_ADDR_SIZE);
-sysbus_init_mmio_region(dev,s-io_memory);

  s-timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
  s-idx = last_apic_idx++;
@@ -1122,7 +1133,7 @@ static int apic_init1(SysBusDevice *dev)
  return 0;
  }

-static SysBusDeviceInfo apic_info = {
+static ICCBusDeviceInfo apic_info = {
  .init = apic_init1,
  .qdev.name = apic,
  .qdev.size = sizeof(APICState),
@@ -1138,7 +1149,7 @@ static SysBusDeviceInfo apic_info = {

  static void apic_register_devices(void)
  {
-sysbus_register_withprop(apic_info);
+iccbus_register_devinfo(apic_info);
  }

  device_init(apic_register_devices)
diff --git a/hw/apic.h b/hw/apic.h
index c857d52..e2c0af5 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
  uint8_t cpu_get_apic_tpr(DeviceState *s);
  void apic_init_reset(DeviceState *s);
  void apic_sipi(DeviceState *s);
+int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);

  /* pc.c */
  int cpu_is_bsp(CPUState *env);
diff --git a/hw/icc_bus.c b/hw/icc_bus.c
new file mode 100644
index 000..61a408e
--- /dev/null
+++ b/hw/icc_bus.c
@@ -0,0 +1,91 @@
+/* icc_bus.c
+ * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus


Copyright?


+ *
+ * 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, seehttp://www.gnu.org/licenses/
+ */
+#include icc_bus.h
+
+static CPUS *dummy_cpus;


Why dummy? Also, no statics please. The bus owner is the chipset
(440fx), so embedded it there. That also avoid that strange cpus
device below.


That's an odd model IMHO.  The i440fx doesn't implement the ICC bus.  The ICC 
bus is entirely independent of the northbridge.


Maybe CPUSockets would be a better device name?

Regards,

Anthony Liguori




+
+
+static ICCBusInfo icc_bus_info = {
+

Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-19 Thread Jan Kiszka 
On 2011-10-19 14:54, Anthony Liguori wrote:
 On 10/19/2011 05:53 AM, Jan Kiszka wrote:
 On 2011-10-19 03:55, pingf...@linux.vnet.ibm.com wrote:
 From: Liu Ping Fanpingf...@linux.vnet.ibm.com

 Introduce a new structure CPUS as the controller of ICC (INTERRUPT
 CONTROLLER COMMUNICATIONS), and new bus ICC to hold APIC,instead
 of sysbus. So we can support APIC hot-plug feature.

 Signed-off-by: liu ping fanpingf...@linux.vnet.ibm.com
 ---
   Makefile.target |1 +
   hw/apic.c   |   25 +++
   hw/apic.h   |1 +
   hw/icc_bus.c|   91 
 +++
   hw/icc_bus.h|   56 ++
   hw/pc.c |   11 --
   6 files changed, 174 insertions(+), 11 deletions(-)
   create mode 100644 hw/icc_bus.c
   create mode 100644 hw/icc_bus.h

 diff --git a/Makefile.target b/Makefile.target
 index 9011f28..5607c6d 100644
 --- a/Makefile.target
 +++ b/Makefile.target
 @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
   obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
   obj-i386-y += testdev.o
   obj-i386-y += acpi.o acpi_piix4.o
 +obj-i386-y += icc_bus.o

   obj-i386-y += pcspk.o i8254.o
   obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
 diff --git a/hw/apic.c b/hw/apic.c
 index 69d6ac5..00d2297 100644
 --- a/hw/apic.c
 +++ b/hw/apic.c
 @@ -21,9 +21,10 @@
   #include ioapic.h
   #include qemu-timer.h
   #include host-utils.h
 -#include sysbus.h
 +#include icc_bus.h
   #include trace.h
   #include kvm.h
 +#include exec-memory.h

 Mmh, don't your rather want memory.h?


   /* APIC Local Vector Table */
   #define APIC_LVT_TIMER   0
 @@ -80,7 +81,7 @@
   typedef struct APICState APICState;

   struct APICState {
 -SysBusDevice busdev;
 +ICCBusDevice busdev;
   MemoryRegion io_memory;
   void *cpu_env;
   uint32_t apicbase;
 @@ -1104,9 +1105,20 @@ static const MemoryRegionOps apic_io_ops = {
   .endianness = DEVICE_NATIVE_ENDIAN,
   };

 -static int apic_init1(SysBusDevice *dev)
 +/**/

 Empty comment.

 +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
   {
 -APICState *s = FROM_SYSBUS(APICState, dev);
 +APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
 +
 +memory_region_add_subregion(get_system_memory(),
 +base,
 +s-io_memory);
 +return 0;
 +}
 +
 +static int apic_init1(ICCBusDevice *dev)
 +{
 +APICState *s = DO_UPCAST(APICState, busdev, dev);
   static int last_apic_idx;

   if (last_apic_idx= MAX_APICS) {
 @@ -1114,7 +1126,6 @@ static int apic_init1(SysBusDevice *dev)
   }
   memory_region_init_io(s-io_memory,apic_io_ops, s, apic,
 MSI_ADDR_SIZE);
 -sysbus_init_mmio_region(dev,s-io_memory);

   s-timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
   s-idx = last_apic_idx++;
 @@ -1122,7 +1133,7 @@ static int apic_init1(SysBusDevice *dev)
   return 0;
   }

 -static SysBusDeviceInfo apic_info = {
 +static ICCBusDeviceInfo apic_info = {
   .init = apic_init1,
   .qdev.name = apic,
   .qdev.size = sizeof(APICState),
 @@ -1138,7 +1149,7 @@ static SysBusDeviceInfo apic_info = {

   static void apic_register_devices(void)
   {
 -sysbus_register_withprop(apic_info);
 +iccbus_register_devinfo(apic_info);
   }

   device_init(apic_register_devices)
 diff --git a/hw/apic.h b/hw/apic.h
 index c857d52..e2c0af5 100644
 --- a/hw/apic.h
 +++ b/hw/apic.h
 @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
   uint8_t cpu_get_apic_tpr(DeviceState *s);
   void apic_init_reset(DeviceState *s);
   void apic_sipi(DeviceState *s);
 +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);

   /* pc.c */
   int cpu_is_bsp(CPUState *env);
 diff --git a/hw/icc_bus.c b/hw/icc_bus.c
 new file mode 100644
 index 000..61a408e
 --- /dev/null
 +++ b/hw/icc_bus.c
 @@ -0,0 +1,91 @@
 +/* icc_bus.c
 + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus

 Copyright?

 + *
 + * 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, 
 seehttp://www.gnu.org/licenses/
 + */
 +#include icc_bus.h
 +
 +static CPUS *dummy_cpus;

 Why dummy? Also, no statics please. The bus owner is the chipset
 (440fx), so embedded it there. That also avoid that strange cpus
 device below.
 
 That's an odd model IMHO.  The i440fx doesn't implement the ICC bus.  The ICC
 bus is

Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus ICC to connect APIC

2011-10-19 Thread Jan Kiszka 
On 2011-10-19 15:33, Jan Kiszka wrote:
 On 2011-10-19 14:54, Anthony Liguori wrote:
 On 10/19/2011 05:53 AM, Jan Kiszka wrote:
 On 2011-10-19 03:55, pingf...@linux.vnet.ibm.com wrote:
 From: Liu Ping Fanpingf...@linux.vnet.ibm.com

 Introduce a new structure CPUS as the controller of ICC (INTERRUPT
 CONTROLLER COMMUNICATIONS), and new bus ICC to hold APIC,instead
 of sysbus. So we can support APIC hot-plug feature.

 Signed-off-by: liu ping fanpingf...@linux.vnet.ibm.com
 ---
   Makefile.target |1 +
   hw/apic.c   |   25 +++
   hw/apic.h   |1 +
   hw/icc_bus.c|   91 
 +++
   hw/icc_bus.h|   56 ++
   hw/pc.c |   11 --
   6 files changed, 174 insertions(+), 11 deletions(-)
   create mode 100644 hw/icc_bus.c
   create mode 100644 hw/icc_bus.h

 diff --git a/Makefile.target b/Makefile.target
 index 9011f28..5607c6d 100644
 --- a/Makefile.target
 +++ b/Makefile.target
 @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
   obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
   obj-i386-y += testdev.o
   obj-i386-y += acpi.o acpi_piix4.o
 +obj-i386-y += icc_bus.o

   obj-i386-y += pcspk.o i8254.o
   obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
 diff --git a/hw/apic.c b/hw/apic.c
 index 69d6ac5..00d2297 100644
 --- a/hw/apic.c
 +++ b/hw/apic.c
 @@ -21,9 +21,10 @@
   #include ioapic.h
   #include qemu-timer.h
   #include host-utils.h
 -#include sysbus.h
 +#include icc_bus.h
   #include trace.h
   #include kvm.h
 +#include exec-memory.h

 Mmh, don't your rather want memory.h?


   /* APIC Local Vector Table */
   #define APIC_LVT_TIMER   0
 @@ -80,7 +81,7 @@
   typedef struct APICState APICState;

   struct APICState {
 -SysBusDevice busdev;
 +ICCBusDevice busdev;
   MemoryRegion io_memory;
   void *cpu_env;
   uint32_t apicbase;
 @@ -1104,9 +1105,20 @@ static const MemoryRegionOps apic_io_ops = {
   .endianness = DEVICE_NATIVE_ENDIAN,
   };

 -static int apic_init1(SysBusDevice *dev)
 +/**/

 Empty comment.

 +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
   {
 -APICState *s = FROM_SYSBUS(APICState, dev);
 +APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
 +
 +memory_region_add_subregion(get_system_memory(),
 +base,
 +s-io_memory);
 +return 0;
 +}
 +
 +static int apic_init1(ICCBusDevice *dev)
 +{
 +APICState *s = DO_UPCAST(APICState, busdev, dev);
   static int last_apic_idx;

   if (last_apic_idx= MAX_APICS) {
 @@ -1114,7 +1126,6 @@ static int apic_init1(SysBusDevice *dev)
   }
   memory_region_init_io(s-io_memory,apic_io_ops, s, apic,
 MSI_ADDR_SIZE);
 -sysbus_init_mmio_region(dev,s-io_memory);

   s-timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
   s-idx = last_apic_idx++;
 @@ -1122,7 +1133,7 @@ static int apic_init1(SysBusDevice *dev)
   return 0;
   }

 -static SysBusDeviceInfo apic_info = {
 +static ICCBusDeviceInfo apic_info = {
   .init = apic_init1,
   .qdev.name = apic,
   .qdev.size = sizeof(APICState),
 @@ -1138,7 +1149,7 @@ static SysBusDeviceInfo apic_info = {

   static void apic_register_devices(void)
   {
 -sysbus_register_withprop(apic_info);
 +iccbus_register_devinfo(apic_info);
   }

   device_init(apic_register_devices)
 diff --git a/hw/apic.h b/hw/apic.h
 index c857d52..e2c0af5 100644
 --- a/hw/apic.h
 +++ b/hw/apic.h
 @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
   uint8_t cpu_get_apic_tpr(DeviceState *s);
   void apic_init_reset(DeviceState *s);
   void apic_sipi(DeviceState *s);
 +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);

   /* pc.c */
   int cpu_is_bsp(CPUState *env);
 diff --git a/hw/icc_bus.c b/hw/icc_bus.c
 new file mode 100644
 index 000..61a408e
 --- /dev/null
 +++ b/hw/icc_bus.c
 @@ -0,0 +1,91 @@
 +/* icc_bus.c
 + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus

 Copyright?

 + *
 + * 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, 
 seehttp://www.gnu.org/licenses/
 + */
 +#include icc_bus.h
 +
 +static CPUS *dummy_cpus;

 Why dummy? Also, no statics please. The bus owner is the chipset
 (440fx), so embedded it there. That also avoid that strange cpus
 device below.

 That's an odd model IMHO.  The i440fx doesn't