| Message ID | 20140514200030.E7E842F456@mono.eik.bme.hu |
|---|---|
| State | New |
| Headers | show |
On Thu, Feb 27, 2014 at 02:05:05AM +0100, BALATON Zoltan wrote: > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- This looks sane, a minor comment below (hopefully last). Thanks! > v2: resubmission after pc-2.1 is added with the multiport case > v3: added compatibility check to avoid changing earlier than pc-2.1 > > hw/char/serial-pci.c | 11 +++++++++++ > include/hw/i386/pc.h | 15 +++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c > index 991c99f..ae57098 100644 > --- a/hw/char/serial-pci.c > +++ b/hw/char/serial-pci.c > @@ -34,6 +34,7 @@ > typedef struct PCISerialState { > PCIDevice dev; > SerialState state; > + uint8_t compat; > } PCISerialState; > > typedef struct PCIMultiSerialState { > @@ -44,6 +45,7 @@ typedef struct PCIMultiSerialState { > SerialState state[PCI_SERIAL_MAX_PORTS]; > uint32_t level[PCI_SERIAL_MAX_PORTS]; > qemu_irq *irqs; > + uint8_t compat; > } PCIMultiSerialState; > > static int serial_pci_init(PCIDevice *dev) This name isn't very informative. I think this should be a prog_if property defaulting to 0x2 and over-written for old machine types, Devices don't care why their prog_if is changed, it's the machine type that cares about compatibility. See commit aa93200b88fb1071eaf21bf766711762ed4630e2 Author: Gabriel L. Somlo <gsomlo@gmail.com> Date: Mon May 5 10:52:51 2014 -0400 apic: use emulated lapic version 0x14 on pc machines >= 2.1 as an example. Alternatively, create a bit property legacy_prog_if and change field name to compat_flags, have property set flags here. See commit 2af234e61d59f39ae16ba882271e7c4fef2c41c1 Author: Michael S. Tsirkin <mst@redhat.com> Date: Thu Feb 14 19:11:27 2013 +0200 e1000: unbreak the guest network migration to 1.3 for an example. I think the 1st option is better but second one would be more or less ok. > @@ -60,6 +62,9 @@ static int serial_pci_init(PCIDevice *dev) > return -1; > } > > + if (!pci->compat) { > + pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ > + } > pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; > s->irq = pci_allocate_irq(&pci->dev); > > @@ -101,6 +106,9 @@ static int multi_serial_pci_init(PCIDevice *dev) > assert(pci->ports > 0); > assert(pci->ports <= PCI_SERIAL_MAX_PORTS); > > + if (!pci->compat) { > + pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ > + } > pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; > memory_region_init(&pci->iobar, OBJECT(pci), "multiserial", 8 * pci->ports); > pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar); > @@ -177,12 +185,14 @@ static const VMStateDescription vmstate_pci_multi_serial = { > > static Property serial_pci_properties[] = { > DEFINE_PROP_CHR("chardev", PCISerialState, state.chr), > + DEFINE_PROP_UINT8("compat", PCISerialState, compat, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > static Property multi_2x_serial_pci_properties[] = { > DEFINE_PROP_CHR("chardev1", PCIMultiSerialState, state[0].chr), > DEFINE_PROP_CHR("chardev2", PCIMultiSerialState, state[1].chr), > + DEFINE_PROP_UINT8("compat", PCIMultiSerialState, compat, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -191,6 +201,7 @@ static Property multi_4x_serial_pci_properties[] = { > DEFINE_PROP_CHR("chardev2", PCIMultiSerialState, state[1].chr), > DEFINE_PROP_CHR("chardev3", PCIMultiSerialState, state[2].chr), > DEFINE_PROP_CHR("chardev4", PCIMultiSerialState, state[3].chr), > + DEFINE_PROP_UINT8("compat", PCIMultiSerialState, compat, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 32a7687..8fb8046 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -271,6 +271,21 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > .driver = "apic",\ > .property = "version",\ > .value = stringify(0x11),\ > + },\ > + {\ > + .driver = "pci-serial",\ > + .property = "compat",\ > + .value = stringify(1),\ > + },\ > + {\ > + .driver = "pci-serial-2x",\ > + .property = "compat",\ > + .value = stringify(1),\ > + },\ > + {\ > + .driver = "pci-serial-4x",\ > + .property = "compat",\ > + .value = stringify(1),\ > } > > #define PC_COMPAT_1_7 \ > -- > 1.8.1.5
On Do, 2014-02-27 at 02:05 +0100, BALATON Zoltan wrote:
^^^^^^^^^^
Looks like your clock is _way_ off.
> + if (!pci->compat) {
> + pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */
> + }
> static Property serial_pci_properties[] = {
> DEFINE_PROP_CHR("chardev", PCISerialState, state.chr),
> + DEFINE_PROP_UINT8("compat", PCISerialState, compat, 0),
> DEFINE_PROP_END_OF_LIST(),
> };
> + {\
> + .driver = "pci-serial",\
> + .property = "compat",\
> + .value = stringify(1),\
> + },\
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
mst, do you take that through the pci tree?
cheers,
Gerd
On Thu, May 15, 2014 at 08:42:17AM +0200, Gerd Hoffmann wrote: > On Do, 2014-02-27 at 02:05 +0100, BALATON Zoltan wrote: > ^^^^^^^^^^ > Looks like your clock is _way_ off. > > > + if (!pci->compat) { > > + pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ > > + } > > > static Property serial_pci_properties[] = { > > DEFINE_PROP_CHR("chardev", PCISerialState, state.chr), > > + DEFINE_PROP_UINT8("compat", PCISerialState, compat, 0), > > DEFINE_PROP_END_OF_LIST(), > > }; > > > + {\ > > + .driver = "pci-serial",\ > > + .property = "compat",\ > > + .value = stringify(1),\ > > + },\ > > Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> > > mst, do you take that through the pci tree? > > cheers, > Gerd > Yes but I'd like the property renamed. Agree?
> > > static Property serial_pci_properties[] = { > > > DEFINE_PROP_CHR("chardev", PCISerialState, state.chr), > > > + DEFINE_PROP_UINT8("compat", PCISerialState, compat, 0), > > > DEFINE_PROP_END_OF_LIST(), > > > }; > > > > mst, do you take that through the pci tree? > > > > cheers, > > Gerd > > > > Yes but I'd like the property renamed. Agree? Yes, something more descriptive is reasonable. cheers, Gerd
diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c index 991c99f..ae57098 100644 --- a/hw/char/serial-pci.c +++ b/hw/char/serial-pci.c @@ -34,6 +34,7 @@ typedef struct PCISerialState { PCIDevice dev; SerialState state; + uint8_t compat; } PCISerialState; typedef struct PCIMultiSerialState { @@ -44,6 +45,7 @@ typedef struct PCIMultiSerialState { SerialState state[PCI_SERIAL_MAX_PORTS]; uint32_t level[PCI_SERIAL_MAX_PORTS]; qemu_irq *irqs; + uint8_t compat; } PCIMultiSerialState; static int serial_pci_init(PCIDevice *dev) @@ -60,6 +62,9 @@ static int serial_pci_init(PCIDevice *dev) return -1; } + if (!pci->compat) { + pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ + } pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; s->irq = pci_allocate_irq(&pci->dev); @@ -101,6 +106,9 @@ static int multi_serial_pci_init(PCIDevice *dev) assert(pci->ports > 0); assert(pci->ports <= PCI_SERIAL_MAX_PORTS); + if (!pci->compat) { + pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ + } pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; memory_region_init(&pci->iobar, OBJECT(pci), "multiserial", 8 * pci->ports); pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar); @@ -177,12 +185,14 @@ static const VMStateDescription vmstate_pci_multi_serial = { static Property serial_pci_properties[] = { DEFINE_PROP_CHR("chardev", PCISerialState, state.chr), + DEFINE_PROP_UINT8("compat", PCISerialState, compat, 0), DEFINE_PROP_END_OF_LIST(), }; static Property multi_2x_serial_pci_properties[] = { DEFINE_PROP_CHR("chardev1", PCIMultiSerialState, state[0].chr), DEFINE_PROP_CHR("chardev2", PCIMultiSerialState, state[1].chr), + DEFINE_PROP_UINT8("compat", PCIMultiSerialState, compat, 0), DEFINE_PROP_END_OF_LIST(), }; @@ -191,6 +201,7 @@ static Property multi_4x_serial_pci_properties[] = { DEFINE_PROP_CHR("chardev2", PCIMultiSerialState, state[1].chr), DEFINE_PROP_CHR("chardev3", PCIMultiSerialState, state[2].chr), DEFINE_PROP_CHR("chardev4", PCIMultiSerialState, state[3].chr), + DEFINE_PROP_UINT8("compat", PCIMultiSerialState, compat, 0), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 32a7687..8fb8046 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -271,6 +271,21 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); .driver = "apic",\ .property = "version",\ .value = stringify(0x11),\ + },\ + {\ + .driver = "pci-serial",\ + .property = "compat",\ + .value = stringify(1),\ + },\ + {\ + .driver = "pci-serial-2x",\ + .property = "compat",\ + .value = stringify(1),\ + },\ + {\ + .driver = "pci-serial-4x",\ + .property = "compat",\ + .value = stringify(1),\ } #define PC_COMPAT_1_7 \
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- v2: resubmission after pc-2.1 is added with the multiport case v3: added compatibility check to avoid changing earlier than pc-2.1 hw/char/serial-pci.c | 11 +++++++++++ include/hw/i386/pc.h | 15 +++++++++++++++ 2 files changed, 26 insertions(+)