On Thu, Oct 17, 2013 at 02:27:43PM +0800, liu ping fan wrote:
> On Thu, Oct 17, 2013 at 1:44 PM, Michael S. Tsirkin <m...@redhat.com> wrote:
> > On Thu, Oct 17, 2013 at 11:16:05AM +0800, Liu Ping Fan wrote:
> >> For pc-piix-*, hpet's intcap is always hard coded as IRQ2.
> >> For q35, if it is pc-q35-1.7 and earlier, we use IRQ2 for compat
> >> reason, otherwise IRQ2, IRQ8, and IRQ16~23 are allowed.
> >>
> >> Signed-off-by: Liu Ping Fan <pingf...@linux.vnet.ibm.com>
> >> ---
> >> hw/i386/pc.c | 20 +++++++++++++++-----
> >> hw/i386/pc_piix.c | 7 ++++++-
> >> hw/i386/pc_q35.c | 6 +++++-
> >> include/hw/i386/pc.h | 11 ++++++++++-
> >> 4 files changed, 36 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index 72cc850..3e98ff0 100644
> >> --- a/hw/i386/pc.c
> >> +++ b/hw/i386/pc.c
> >> @@ -1219,7 +1219,8 @@ static const MemoryRegionOps ioportF0_io_ops = {
> >> void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
> >> ISADevice **rtc_state,
> >> ISADevice **floppy,
> >> - bool no_vmport)
> >> + bool no_vmport,
> >> + bool hpet_irqs)
> >> {
> >> int i;
> >> DriveInfo *fd[MAX_FD];
> >> @@ -1249,10 +1250,19 @@ void pc_basic_device_init(ISABus *isa_bus,
> >> qemu_irq *gsi,
> >> /* In order to set property, here not using
> >> sysbus_try_create_simple */
> >> hpet = qdev_try_create(NULL, "hpet");
> >> if (hpet) {
> >> - /* tmp fix. For compat, hard code to IRQ2 until we have
> >> correct
> >> - * compat property and differentiate pc-iix with pc-q35
> >> - */
> >> - qdev_prop_set_uint32(hpet, HPET_INTCAP, 0x4);
> >> + /* For pc-piix-*, hpet's intcap is always IRQ2. */
> >> + if (!hpet_irqs) {
> >
> > So for piix the property is ignored even if set.
> >
> Yes.
> >> + qdev_prop_set_uint32(hpet, HPET_INTCAP, 0x4);
> >> + } else {
> >> + /* For pc-q35-1.7 and earlier, use IRQ2 for compat.
> >
> > Now you lost me. It's still 4 for 1.7?
> > You only target 1.8 then?
> >
> Yes, since the compat for guest. And start this fix with 1.8
> >> + * Otherwise, use IRQ16~23, IRQ8 and IRQ2.
> >> + */
> >> + uint8_t compat = object_property_get_int(OBJECT(hpet),
> >> + HPET_INTCAP, NULL);
> >> + if (!compat) {
> >> + qdev_prop_set_uint32(hpet, HPET_INTCAP, 0xff0104);
> >> + }
> >> + }
> >> qdev_init_nofail(hpet);
> >> sysbus_mmio_map(SYS_BUS_DEVICE(hpet), 0, HPET_BASE);
> >>
> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >> index c6042c7..a45ce11 100644
> >> --- a/hw/i386/pc_piix.c
> >> +++ b/hw/i386/pc_piix.c
> >> @@ -180,7 +180,8 @@ static void pc_init1(QEMUMachineInitArgs *args,
> >> pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
> >>
> >> /* init basic PC hardware */
> >> - pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy,
> >> xen_enabled());
> >> + pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, xen_enabled(),
> >> + false);
> >>
> >> pc_nic_init(isa_bus, pci_bus);
> >>
> >> @@ -346,6 +347,10 @@ static QEMUMachine pc_i440fx_machine_v1_7 = {
> >> .alias = "pc",
> >> .init = pc_init_pci,
> >> .is_default = 1,
> >> + .compat_props = (GlobalProperty[]) {
> >> + PC_COMPAT_1_7,
> >
> > So you add this property for PIIX but it's then ignored?
> > That's kind of ugly.
>
> Yes a little ugly, but PIIX and ich9 diverge for hpet's hardware.
Maybe only add it for Q35 then?
Create PC_Q35_COMPAT_1_7 and use everywhere ...
> > Not sure how to fix this. Paolo?
> >
> >> + { /* end of list */ }
> >> + },
> >> };
> >>
> >> #define PC_I440FX_1_6_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
> >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> >> index ca84e1c..124ecc1 100644
> >> --- a/hw/i386/pc_q35.c
> >> +++ b/hw/i386/pc_q35.c
> >> @@ -181,7 +181,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
> >> pc_register_ferr_irq(gsi[13]);
> >>
> >> /* init basic PC hardware */
> >> - pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, false);
> >> + pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, false, true);
> >>
> >> /* connect pm stuff to lpc */
> >> ich9_lpc_pm_init(lpc);
> >> @@ -270,6 +270,10 @@ static QEMUMachine pc_q35_machine_v1_7 = {
> >> .name = "pc-q35-1.7",
> >> .alias = "q35",
> >> .init = pc_q35_init,
> >> + .compat_props = (GlobalProperty[]) {
> >> + PC_COMPAT_1_7,
> >> + { /* end of list */ }
> >> + },
> >> };
> >>
> >> #define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS
> >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >> index 1361a27..bfeccf2 100644
> >> --- a/include/hw/i386/pc.h
> >> +++ b/include/hw/i386/pc.h
> >> @@ -136,7 +136,8 @@ DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus
> >> *pci_bus);
> >> void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
> >> ISADevice **rtc_state,
> >> ISADevice **floppy,
> >> - bool no_vmport);
> >> + bool no_vmport,
> >> + bool hpet_irqs);
> >> void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd);
> >> void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
> >> const char *boot_device,
> >> @@ -227,7 +228,15 @@ void pvpanic_init(ISABus *bus);
> >>
> >> int e820_add_entry(uint64_t, uint64_t, uint32_t);
> >>
> >> +#define PC_COMPAT_1_7 \
> >> + {\
> >> + .driver = "hpet",\
> >> + .property = HPET_INTCAP,\
> >> + .value = stringify(4),\
> >> + }
> >> +
> >> #define PC_COMPAT_1_6 \
> >> + PC_COMPAT_1_7, \
> >> {\
> >> .driver = "e1000",\
> >> .property = "mitigation",\
> >> --
> >> 1.8.1.4
