From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58929) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RNB3W-0002qL-Ro for qemu-devel@nongnu.org; Sun, 06 Nov 2011 17:19:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RNB3U-0001UF-MM for qemu-devel@nongnu.org; Sun, 06 Nov 2011 17:19:10 -0500 Received: from fmmailgate02.web.de ([217.72.192.227]:50498) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RNB3T-0001U1-Cv for qemu-devel@nongnu.org; Sun, 06 Nov 2011 17:19:08 -0500 Received: from moweb001.kundenserver.de (moweb001.kundenserver.de [172.19.20.114]) by fmmailgate02.web.de (Postfix) with ESMTP id 034401BA6D277 for ; Sun, 6 Nov 2011 19:06:34 +0100 (CET) Message-ID: <4EB6CCA5.1080507@web.de> Date: Sun, 06 Nov 2011 19:06:29 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <1320312611-9439-1-git-send-email-pingfank@linux.vnet.ibm.com> In-Reply-To: <1320312611-9439-1-git-send-email-pingfank@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigA61896A61D5F34A5D519EE57" Subject: Re: [Qemu-devel] [PATCH V3] Introduce a new bus "ICC" to connect APIC List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: pingfank@linux.vnet.ibm.com Cc: blauwirbel@gmail.com, aliguori@us.ibm.com, ryanh@us.ibm.com, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigA61896A61D5F34A5D519EE57 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable On 2011-11-03 10:30, pingfank@linux.vnet.ibm.com wrote: > From: Liu Ping Fan >=20 > 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. >=20 > Signed-off-by: liu ping fan > --- > Makefile.target | 1 + > hw/apic.c | 24 +++++++++---- > hw/apic.h | 1 + > hw/icc_bus.c | 92 +++++++++++++++++++++++++++++++++++++++++++= ++++++++ > hw/icc_bus.h | 61 +++++++++++++++++++++++++++++++++ > hw/pc.c | 9 +++-- > hw/pc_piix.c | 14 +++++++- > target-i386/cpu.h | 1 + > target-i386/cpuid.c | 16 +++++++++ > 9 files changed, 207 insertions(+), 12 deletions(-) > create mode 100644 hw/icc_bus.c > create mode 100644 hw/icc_bus.h >=20 > 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) +=3D kvmclock.o > obj-i386-$(CONFIG_SPICE) +=3D qxl.o qxl-logger.o qxl-render.o > obj-i386-y +=3D testdev.o > obj-i386-y +=3D acpi.o acpi_piix4.o > +obj-i386-y +=3D icc_bus.o > =20 > obj-i386-y +=3D pcspk.o i8254.o > obj-i386-$(CONFIG_KVM_PIT) +=3D i8254-kvm.o > diff --git a/hw/apic.c b/hw/apic.c > index 69d6ac5..34fa1dd 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" > =20 > /* APIC Local Vector Table */ > #define APIC_LVT_TIMER 0 > @@ -80,7 +81,7 @@ > typedef struct APICState APICState; > =20 > struct APICState { > - SysBusDevice busdev; > + ICCBusDevice busdev; > MemoryRegion io_memory; > void *cpu_env; > uint32_t apicbase; > @@ -1104,9 +1105,19 @@ static const MemoryRegionOps apic_io_ops =3D { > .endianness =3D DEVICE_NATIVE_ENDIAN, > }; > =20 > -static int apic_init1(SysBusDevice *dev) > +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base) > { > - APICState *s =3D FROM_SYSBUS(APICState, dev); > + APICState *s =3D 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 =3D DO_UPCAST(APICState, busdev, dev); > static int last_apic_idx; > =20 > if (last_apic_idx >=3D MAX_APICS) { > @@ -1114,7 +1125,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); > =20 > s->timer =3D qemu_new_timer_ns(vm_clock, apic_timer, s); > s->idx =3D last_apic_idx++; > @@ -1122,7 +1132,7 @@ static int apic_init1(SysBusDevice *dev) > return 0; > } > =20 > -static SysBusDeviceInfo apic_info =3D { > +static ICCBusDeviceInfo apic_info =3D { > .init =3D apic_init1, > .qdev.name =3D "apic", > .qdev.size =3D sizeof(APICState), > @@ -1138,7 +1148,7 @@ static SysBusDeviceInfo apic_info =3D { > =20 > static void apic_register_devices(void) > { > - sysbus_register_withprop(&apic_info); > + iccbus_register_devinfo(&apic_info); > } > =20 > 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); > =20 > /* 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 0000000..ac88f2e > --- /dev/null > +++ b/hw/icc_bus.c > @@ -0,0 +1,92 @@ > +/* icc_bus.c > + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus > + * > + * Copyright IBM, Corp. 2011 > + * > + * 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 > + */ > +#include "icc_bus.h" > + > +static CPUSockets *cpu_sockets; > + > +static ICCBusInfo icc_bus_info =3D { > + .qinfo.name =3D "icc", > + .qinfo.size =3D sizeof(ICCBus), > + .qinfo.props =3D (Property[]) { > + DEFINE_PROP_END_OF_LIST(), > + } > +}; > + > +static int iccbus_device_init(DeviceState *dev, DeviceInfo *base) > +{ > + ICCBusDeviceInfo *info =3D container_of(base, ICCBusDeviceInfo, qd= ev); > + ICCBusDevice *idev =3D DO_UPCAST(ICCBusDevice, qdev, dev); > + > + return info->init(idev); > +} > + > +void iccbus_register_devinfo(ICCBusDeviceInfo *info) > +{ > + info->qdev.init =3D iccbus_device_init; > + info->qdev.bus_info =3D &icc_bus_info.qinfo; > + assert(info->qdev.size >=3D sizeof(ICCBusDevice)); > + qdev_register(&info->qdev); > +} > + > +BusState *get_icc_bus(void) > +{ > + return &cpu_sockets->icc_bus->qbus; > +} > + > +/*Must be called before vcpu's creation*/ > +static int cpusockets_init(SysBusDevice *dev) > +{ > + CPUSockets *cpus =3D DO_UPCAST(CPUSockets, sysdev, dev); > + BusState *b =3D qbus_create(&icc_bus_info.qinfo, > + &cpu_sockets->sysdev.qdev, > + "icc"); > + if (b =3D=3D NULL) { > + return -1; > + } > + b->allow_hotplug =3D 1; /* Yes, we can */ > + cpus->icc_bus =3D DO_UPCAST(ICCBus, qbus, b); > + cpu_sockets =3D cpus; > + return 0; > + > +} > + > +static const VMStateDescription vmstate_cpusockets =3D { > + .name =3D "cpusockets", > + .version_id =3D 1, > + .minimum_version_id =3D 0, > + .fields =3D (VMStateField[]) { > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static SysBusDeviceInfo cpusockets_info =3D { > + .init =3D cpusockets_init, > + .qdev.name =3D "cpusockets", > + .qdev.size =3D sizeof(CPUSockets), > + .qdev.vmsd =3D &vmstate_cpusockets, > + .qdev.reset =3D NULL, > + .qdev.no_user =3D 1, > +}; > + > +static void cpusockets_register_devices(void) > +{ > + sysbus_register_withprop(&cpusockets_info); > +} > + > +device_init(cpusockets_register_devices) > diff --git a/hw/icc_bus.h b/hw/icc_bus.h > new file mode 100644 > index 0000000..9f10e1e > --- /dev/null > +++ b/hw/icc_bus.h > @@ -0,0 +1,61 @@ > +/* ICCBus.h > + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus > + * > + * Copyright IBM, Corp. 2011 > + * > + * 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 > + */ > +#ifndef QEMU_ICC_H > +#define QEMU_ICC_H > + > +#include "qdev.h" > +#include "sysbus.h" > + > +typedef struct CPUSockets CPUSockets; > +typedef struct ICCBus ICCBus; > +typedef struct ICCBusInfo ICCBusInfo; > +typedef struct ICCBusDevice ICCBusDevice; > +typedef struct ICCBusDeviceInfo ICCBusDeviceInfo; > + > +struct CPUSockets { > + SysBusDevice sysdev; > + ICCBus *icc_bus; > +}; > + > +struct CPUSInfo { > + DeviceInfo info; > +}; > + > +struct ICCBus { > + BusState qbus; > +}; > + > +struct ICCBusInfo { > + BusInfo qinfo; > +}; > +struct ICCBusDevice { > + DeviceState qdev; > +}; > + > +typedef int (*iccbus_initfn)(ICCBusDevice *dev); > + > +struct ICCBusDeviceInfo { > + DeviceInfo qdev; > + iccbus_initfn init; > +}; > + > +void iccbus_register_devinfo(ICCBusDeviceInfo *info); > +BusState *get_icc_bus(void); > + > +#endif > diff --git a/hw/pc.c b/hw/pc.c > index 6b3662e..ffdca64 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -24,6 +24,7 @@ > #include "hw.h" > #include "pc.h" > #include "apic.h" > +#include "icc_bus.h" > #include "fdc.h" > #include "ide.h" > #include "pci.h" > @@ -875,21 +876,21 @@ DeviceState *cpu_get_current_apic(void) > static DeviceState *apic_init(void *env, uint8_t apic_id) > { > DeviceState *dev; > - SysBusDevice *d; > + BusState *b; > static int apic_mapped; > =20 > - dev =3D qdev_create(NULL, "apic"); > + b =3D get_icc_bus(); > + dev =3D qdev_create(b, "apic"); > qdev_prop_set_uint8(dev, "id", apic_id); > qdev_prop_set_ptr(dev, "cpu_env", env); > qdev_init_nofail(dev); > - d =3D sysbus_from_qdev(dev); > =20 > /* XXX: mapping more APICs at the same memory location */ > if (apic_mapped =3D=3D 0) { > /* NOTE: the APIC is directly connected to the CPU - it is not= > on the global memory bus. */ > /* XXX: what if the base changes? */ > - sysbus_mmio_map(d, 0, MSI_ADDR_BASE); > + apic_mmio_map(dev, MSI_ADDR_BASE); > apic_mapped =3D 1; > } > =20 > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > index 7055591..09aece9 100644 > --- a/hw/pc_piix.c > +++ b/hw/pc_piix.c > @@ -41,7 +41,6 @@ > #include "blockdev.h" > #include "smbus.h" > #include "xen.h" > -#include "memory.h" That change looks stray. > #include "exec-memory.h" > #ifdef CONFIG_XEN > # include > @@ -102,8 +101,21 @@ static void pc_init1(MemoryRegion *system_memory, > MemoryRegion *ram_memory; > MemoryRegion *pci_memory; > =20 > + if (cpu_model =3D=3D NULL) { > +#ifdef TARGET_X86_64 > + cpu_model =3D "qemu64"; > +#else > + cpu_model =3D "qemu32"; > +#endif > + } > + > global_cpu_model =3D cpu_model; > =20 > + if (cpu_has_apic_feature(cpu_model) || smp_cpus > 1) { > + DeviceState *d =3D qdev_create(NULL, "cpusockets"); > + qdev_init_nofail(d); Indention is still off. While at it, why not move all this into pc_cpus_init? That way other PC chipsets could once make use of it as well. > + } > + > pc_cpus_init(cpu_model); > =20 > if (kvmclock_enabled) { > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index 2a071f2..abdeb40 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -884,6 +884,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index= , uint32_t count, > uint32_t *eax, uint32_t *ebx, > uint32_t *ecx, uint32_t *edx); > int cpu_x86_register (CPUX86State *env, const char *cpu_model); > +int cpu_has_apic_feature(const char *cpu_model); > void cpu_clear_apic_feature(CPUX86State *env); > void host_cpuid(uint32_t function, uint32_t count, > uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t = *edx); > diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c > index f179999..beaef5b 100644 > --- a/target-i386/cpuid.c > +++ b/target-i386/cpuid.c > @@ -850,6 +850,22 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fp= rintf, const char *optarg) > } > } > =20 > +int cpu_has_apic_feature(const char *cpu_model) > +{ > + x86_def_t def1, *def =3D &def1; > + > + memset(def, 0, sizeof(*def)); > + > + if (cpu_x86_find_by_name(def, cpu_model) < 0) { > + return 0; > + } > + if (def->features & CPUID_APIC) { > + return 1; > + } else { > + return 0; > + } > +} > + > int cpu_x86_register (CPUX86State *env, const char *cpu_model) > { > x86_def_t def1, *def =3D &def1; Ack from my side if you address that one thing. Jan --------------enigA61896A61D5F34A5D519EE57 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk62zKUACgkQitSsb3rl5xTJVgCgi3OYq8nxsvGXCUxwfnuEC0W/ yyoAn0+93t/7zYTtLuOVGxXBrIOA6TeY =PcWj -----END PGP SIGNATURE----- --------------enigA61896A61D5F34A5D519EE57--