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

[Andreas Färber]

Am 30.08.2013 10:17, schrieb Paolo Bonzini:
> 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).

Stefan already has than in his net tree and just needs to (rebase and)
flush his queue!

Andreas

>  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 */
>>
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg