Re: [Qemu-devel] [PATCH v3 2/3] qdev: interface for SysBusDevice to change property on requirement

[liu ping fan]

On Fri, Aug 30, 2013 at 4:17 PM, Paolo Bonzini <address@hidden> wrote:
> 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

Oh, just aware of this tricky method to set qdev's property. Thanks,
will try this way!

Regards,
Pingfan
> 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 <address@hidden>
>> ---
>>  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 */
>>
>