From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34850) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VFJtX-0007lN-RI for qemu-devel@nongnu.org; Fri, 30 Aug 2013 04:17:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VFJtR-0005WF-Ph for qemu-devel@nongnu.org; Fri, 30 Aug 2013 04:17:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39367) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VFJtR-0005WB-IZ for qemu-devel@nongnu.org; Fri, 30 Aug 2013 04:17:21 -0400 Message-ID: <52205509.8080208@redhat.com> Date: Fri, 30 Aug 2013 10:17:13 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1377849232-27822-1-git-send-email-pingfank@linux.vnet.ibm.com> <1377849232-27822-3-git-send-email-pingfank@linux.vnet.ibm.com> In-Reply-To: <1377849232-27822-3-git-send-email-pingfank@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/3] qdev: interface for SysBusDevice to change property on requirement List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Liu Ping Fan Cc: Jan Kiszka , qemu-devel@nongnu.org, Anthony Liguori , =?ISO-8859-15?Q?Andreas_F=E4rber?= Il 30/08/2013 09:53, Liu Ping Fan ha scritto: > qdev's property can not be set after realized, but there is a > requirement of adjusting device's behavior on different mother > boards. So introducing a callback in sysbus_try_create_simple() > to adjust device's property on board's demand. > > (This patch is needed by the later one which changes hpet's intcap > property) I don't think it is useful to add a new mechanism since there is an existing mechanism to set properties for compatibility (which I pointed you to earlier). It is also incorrect because this will have an effect on all PC boards including pc-q35-1.7 and newer. You need to create a 1.7 machine like commit 45053fd (pc: Create pc-*-1.6 machine-types, 2013-05-27). On top of this: * the 1.6 machines need to have a compatibility property in hw/i386/pc.h. * the pc-i440fx-1.7 machine needs to have a compatibility property for the same thing in hw/i386/pc_piix.c Paolo > Signed-off-by: Liu Ping Fan > --- > hw/core/sysbus.c | 5 ++++- > hw/i386/pc.c | 2 +- > include/hw/sysbus.h | 8 +++++--- > 3 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c > index 9004d8c..e894bbb 100644 > --- a/hw/core/sysbus.c > +++ b/hw/core/sysbus.c > @@ -172,7 +172,7 @@ DeviceState *sysbus_create_varargs(const char *name, > return dev; > } > > -DeviceState *sysbus_try_create_varargs(const char *name, > +DeviceState *sysbus_try_create_varargs(const char *name, CompatSet set, > hwaddr addr, ...) > { > DeviceState *dev; > @@ -185,6 +185,9 @@ DeviceState *sysbus_try_create_varargs(const char *name, > if (!dev) { > return NULL; > } > + if (set) { > + set(dev); > + } > s = SYS_BUS_DEVICE(dev); > qdev_init_nofail(dev); > if (addr != (hwaddr)-1) { > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index e8bc8ce..09c10ac 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1247,7 +1247,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, > * when the HPET wants to take over. Thus we have to disable the latter. > */ > if (!no_hpet && (!kvm_irqchip_in_kernel() || kvm_has_pit_state2())) { > - hpet = sysbus_try_create_simple("hpet", HPET_BASE, NULL); > + hpet = sysbus_try_create_simple("hpet", NULL, HPET_BASE, NULL); > > if (hpet) { > for (i = 0; i < GSI_NUM_PINS; i++) { > diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h > index bb50a87..47337f2 100644 > --- a/include/hw/sysbus.h > +++ b/include/hw/sysbus.h > @@ -58,6 +58,8 @@ struct SysBusDevice { > pio_addr_t pio[QDEV_MAX_PIO]; > }; > > +typedef void (*CompatSet)(DeviceState *dev); > + > void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory); > MemoryRegion *sysbus_mmio_get_region(SysBusDevice *dev, int n); > void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p); > @@ -77,7 +79,7 @@ MemoryRegion *sysbus_address_space(SysBusDevice *dev); > /* Legacy helper function for creating devices. */ > DeviceState *sysbus_create_varargs(const char *name, > hwaddr addr, ...); > -DeviceState *sysbus_try_create_varargs(const char *name, > +DeviceState *sysbus_try_create_varargs(const char *name, CompatSet set, > hwaddr addr, ...); > static inline DeviceState *sysbus_create_simple(const char *name, > hwaddr addr, > @@ -86,11 +88,11 @@ static inline DeviceState *sysbus_create_simple(const char *name, > return sysbus_create_varargs(name, addr, irq, NULL); > } > > -static inline DeviceState *sysbus_try_create_simple(const char *name, > +static inline DeviceState *sysbus_try_create_simple(const char *name, CompatSet set, > hwaddr addr, > qemu_irq irq) > { > - return sysbus_try_create_varargs(name, addr, irq, NULL); > + return sysbus_try_create_varargs(name, set, addr, irq, NULL); > } > > #endif /* !HW_SYSBUS_H */ >