[Qemu-devel] [PATCH v2 5/5] pc: pci-info add compat support

[Michael S. Tsirkin]

We can't change fw cfg entries or add new ones
without breaking cross version migration.
Add a flag to skip adding new entry when
running with 1.5 compat machine type.

Signed-off-by: Michael S. Tsirkin 
---
 hw/i386/pc.c |  7 ++-
 hw/i386/pc_piix.c| 12 ++--
 include/hw/i386/pc.h |  1 +
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 64101cb..8b548d0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -988,7 +988,12 @@ typedef struct PcRomPciInfo {
 
 static void pc_fw_cfg_guest_info(PcGuestInfo *guest_info)
 {
-PcRomPciInfo *info = g_malloc(sizeof *info);
+PcRomPciInfo *info;
+if (guest_info->compat_v1_5) {
+return;
+}
+
+info = g_malloc(sizeof *info);
 info->w32_min = cpu_to_le64(guest_info->pci_info.w32.begin);
 info->w32_max = cpu_to_le64(guest_info->pci_info.w32.end);
 info->w64_min = cpu_to_le64(guest_info->pci_info.w64.begin);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2717d83..5b281d1 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -57,7 +57,7 @@ static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
 static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 
 static bool has_pvpanic = true;
-static bool has_pci_info = true;
+static bool guest_info_compat_v1_5 = false;
 
 /* PC hardware initialisation */
 static void pc_init1(MemoryRegion *system_memory,
@@ -122,6 +122,7 @@ static void pc_init1(MemoryRegion *system_memory,
 }
 
 guest_info = pc_guest_info_init(below_4g_mem_size, above_4g_mem_size);
+guest_info->compat_v1_5 = guest_info_compat_v1_5;
 
 /* Set PCI window size the way seabios has always done it. */
 /* TODO: consider just starting at below_4g_mem_size */
@@ -259,6 +260,12 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
  initrd_filename, cpu_model, 1, 1);
 }
 
+static void pc_init_pci_1_5(QEMUMachineInitArgs *args)
+{
+guest_info_compat_v1_5 = true;
+pc_init_pci(args);
+}
+
 static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
 {
 has_pvpanic = false;
@@ -355,7 +362,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
 static QEMUMachine pc_i440fx_machine_v1_5 = {
 .name = "pc-i440fx-1.5",
 .desc = "Standard PC (i440FX + PIIX, 1996)",
-.init = pc_init_pci,
+.init = pc_init_pci_1_5,
 .hot_add_cpu = pc_hot_add_cpu,
 .max_cpus = 255,
 .is_default = 1,
@@ -759,6 +766,7 @@ static QEMUMachine xenfv_machine = {
 
 static void pc_machine_init(void)
 {
+qemu_register_machine(&pc_i440fx_machine_v1_6);
 qemu_register_machine(&pc_i440fx_machine_v1_5);
 qemu_register_machine(&pc_i440fx_machine_v1_4);
 qemu_register_machine(&pc_machine_v1_3);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1bf5219..6419da8 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -20,6 +20,7 @@ typedef struct PcPciInfo {
 
 struct PcGuestInfo {
 PcPciInfo pci_info;
+bool compat_v1_5;
 FWCfgState *fw_cfg;
 };
 
-- 
MST

Re: [Qemu-devel] [PATCH v2 5/5] pc: pci-info add compat support

[Laszlo Ersek]

On 05/30/13 13:07, Michael S. Tsirkin wrote:

>  /* PC hardware initialisation */
>  static void pc_init1(MemoryRegion *system_memory,
> @@ -122,6 +122,7 @@ static void pc_init1(MemoryRegion *system_memory,
>  }
>  
>  guest_info = pc_guest_info_init(below_4g_mem_size, above_4g_mem_size);
> +guest_info->compat_v1_5 = guest_info_compat_v1_5;

I believe I can see the advantage of delaying this "compat_v1_5" until
init-done-notifier time: init code gradually building up / rewriting
guest_info doesn't have to tiptoe around conditions.

Style: would it be worth passing "guest_info_compat_v1_5" as a parameter
to pc_guest_info_init()? Currently you have an _init() function that
partially initializes the struct, and right after _init() returns you
fill in what's still missing form basic initialization.

No more comments for the series.

Thanks,
Laszlo

Re: [Qemu-devel] [PATCH v2 5/5] pc: pci-info add compat support

[Michael S. Tsirkin]

On Thu, May 30, 2013 at 06:32:19PM +0200, Laszlo Ersek wrote:
> On 05/30/13 13:07, Michael S. Tsirkin wrote:
> 
> >  /* PC hardware initialisation */
> >  static void pc_init1(MemoryRegion *system_memory,
> > @@ -122,6 +122,7 @@ static void pc_init1(MemoryRegion *system_memory,
> >  }
> >  
> >  guest_info = pc_guest_info_init(below_4g_mem_size, above_4g_mem_size);
> > +guest_info->compat_v1_5 = guest_info_compat_v1_5;
> 
> I believe I can see the advantage of delaying this "compat_v1_5" until
> init-done-notifier time: init code gradually building up / rewriting
> guest_info doesn't have to tiptoe around conditions.
> 
> Style: would it be worth passing "guest_info_compat_v1_5" as a parameter
> to pc_guest_info_init()? Currently you have an _init() function that
> partially initializes the struct, and right after _init() returns you
> fill in what's still missing form basic initialization.

This seems to be the style used otherwise in this file ...

> No more comments for the series.
> 
> Thanks,
> Laszlo