On Thu, Feb 27, 2014 at 02:05:05AM +0100, BALATON Zoltan wrote:
> Signed-off-by: BALATON Zoltan <bala...@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 <gso...@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 <m...@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
