On Thu, May 01, 2014 at 05:43:23PM -0400, Don Slutz wrote:
> On 05/01/14 14:52, Alexander Graf wrote:
> >With qdev we basically had an array of constructor parameters in the qdev
> >definition. You could set these from the outside between create and init,
> >basically:
> >
> > dev = dev_create()
> > set_prop(dev, "foo", bar);
> > dev_init(dev)
> >
> >which semantically translated to
> >
> > dev = new dev(foo = bar);
> >
> >The way to do this with QOM is similar, but I keep forgetting the details.
> >I'm sure you'll easily find out :).
> >
> >
>
> It looks like
>
> http://permalink.gmane.org/gmane.comp.emulators.qemu/268337
So, after a bit more digging, it appears qdev would be the more
appropriate fit:
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 7ecce2d..9cb418f 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -380,6 +380,7 @@ static const VMStateDescription vmstate_apic_common = {
static Property apic_properties_common[] = {
DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
+ DEFINE_PROP_UINT32("version", APICCommonState, version, 0x14),
DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
true),
DEFINE_PROP_END_OF_LIST(),
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 70542a6..0ac1462 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -98,6 +98,7 @@ struct APICCommonState {
X86CPU *cpu;
uint32_t apicbase;
uint8_t id;
+ uint32_t version;
uint8_t arb_id;
uint8_t tpr;
uint32_t spurious_vec;
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 2f40cba..ef19e55 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -675,7 +675,7 @@ static uint32_t apic_mem_readl(void *opaque, hwaddr addr)
val = s->id << 24;
break;
case 0x03: /* version */
- val = 0x11 | ((APIC_LVT_NB - 1) << 16); /* version 0x11 */
+ val = s->version | ((APIC_LVT_NB - 1) << 16);
break;
case 0x08:
apic_sync_vapic(s, SYNC_FROM_VAPIC);
However, at this point we hit a (IMHO, huge) snag (using pc_q35.c as
an example, pc_piix.c would have the exact same issue).
Modifying the "version" property on an apic using
object_property_set_uint32() would require its "APICCommonState *",
which doesn't exist before pc_q35_init() calls pc_cpus_init(), and
results in an error (modifying property on already realized apic) after
pc_cpus_init() completes.
I could pass the "bool soft_apic_compat" value from pc_q35_init()
all the way down into the bowels of target-i386/cpu.c, but many of
those calls are also made from cpu hotplug, and things get complicated
really fast. More complicated than just setting a global apic version...
I'm not sure the timing of the various constructors works out in a way
that would allow us to avoid a global variable :(
Did I miss anything ? Is there a way to override the default for all
apics, which I set in DEFINE_PROP_UINT32, *before* anything gets
initialized/realized/constructed/whatever ? :)
Thanks,
--Gabriel
