Re: [PATCH] i386/acpi: restore device paths for pre-5.1 vms

["Michael S. Tsirkin" <mst@redhat.com>]

The issue is with people who installed a VM using 5.1 qemu,
migrated to 5.2, booted there and set a config on a device
e.g. IP on a NIC.
They now have a 5.1 machine type but changing uid back
like we do will break these VMs.

Unlikley to be common but let's at least create a way for these people
to used these VMs.


On Mon, Mar 22, 2021 at 06:49:09PM +0300, Vitaly Cheptsov wrote:
> Hi Michael,
> 
> That makes little sense in my opinion, these people can simply upgrade the VM version, which apparently looks the right way to do it in my opinion.
> 
> Best regards,
> Vitaly
> 
> > 22 марта 2021 г., в 18:43, Michael S. Tsirkin <mst@redhat.com> написал(а):
> > 
> > On Mon, Mar 01, 2021 at 10:59:18PM +0300, Vitaly Cheptsov wrote:
> >> After fixing the _UID value for the primary PCI root bridge in
> >> af1b80ae it was discovered that this change updates Windows
> >> configuration in an incompatible way causing network configuration
> >> failure unless DHCP is used. More details provided on the list:
> >> 
> >> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08484.html
> >> 
> >> This change reverts the _UID update from 1 to 0 for q35 and i440fx
> >> VMs before version 5.2 to maintain the original behaviour when
> >> upgrading.
> >> 
> >> Cc: qemu-stable@nongnu.org
> >> Cc: qemu-devel@nongnu.org
> >> Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> >> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> >> Signed-off-by: Vitaly Cheptsov <cheptsov@ispras.ru>
> >> ---
> >> hw/i386/acpi-build.c | 4 ++--
> >> hw/i386/pc_piix.c    | 2 ++
> >> hw/i386/pc_q35.c     | 2 ++
> >> include/hw/i386/pc.h | 1 +
> >> 4 files changed, 7 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >> index 31a5f6f4a5..442b4629a9 100644
> >> --- a/hw/i386/acpi-build.c
> >> +++ b/hw/i386/acpi-build.c
> >> @@ -1277,7 +1277,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >>         dev = aml_device("PCI0");
> >>         aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
> >>         aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> >> -        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> >> +        aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid)));
> >>         aml_append(sb_scope, dev);
> >>         aml_append(dsdt, sb_scope);
> >> 
> >> @@ -1296,7 +1296,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >>         aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
> >>         aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
> >>         aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> >> -        aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> >> +        aml_append(dev, aml_name_decl("_UID", aml_int(pcmc->pci_root_uid)));
> >>         aml_append(dev, build_q35_osc_method());
> >>         aml_append(sb_scope, dev);
> >>         if (mcfg_valid) {
> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >> index 2904b40163..46cc951073 100644
> >> --- a/hw/i386/pc_piix.c
> >> +++ b/hw/i386/pc_piix.c
> >> @@ -405,6 +405,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
> >> {
> >>     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> >>     pcmc->default_nic_model = "e1000";
> >> +    pcmc->pci_root_uid = 0;
> >> 
> >>     m->family = "pc_piix";
> >>     m->desc = "Standard PC (i440FX + PIIX, 1996)";
> >> @@ -448,6 +449,7 @@ static void pc_i440fx_5_1_machine_options(MachineClass *m)
> >>     compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len);
> >>     compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len);
> >>     pcmc->kvmclock_create_always = false;
> >> +    pcmc->pci_root_uid = 1;
> >> }
> >> 
> >> DEFINE_I440FX_MACHINE(v5_1, "pc-i440fx-5.1", NULL,
> >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> >> index 0a212443aa..53450190f5 100644
> >> --- a/hw/i386/pc_q35.c
> >> +++ b/hw/i386/pc_q35.c
> >> @@ -329,6 +329,7 @@ static void pc_q35_machine_options(MachineClass *m)
> >> {
> >>     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> >>     pcmc->default_nic_model = "e1000e";
> >> +    pcmc->pci_root_uid = 0;
> >> 
> >>     m->family = "pc_q35";
> >>     m->desc = "Standard PC (Q35 + ICH9, 2009)";
> >> @@ -375,6 +376,7 @@ static void pc_q35_5_1_machine_options(MachineClass *m)
> >>     compat_props_add(m->compat_props, hw_compat_5_1, hw_compat_5_1_len);
> >>     compat_props_add(m->compat_props, pc_compat_5_1, pc_compat_5_1_len);
> >>     pcmc->kvmclock_create_always = false;
> >> +    pcmc->pci_root_uid = 1;
> >> }
> >> 
> >> DEFINE_Q35_MACHINE(v5_1, "pc-q35-5.1", NULL,
> >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >> index c9d194a5e7..d4c3d73c11 100644
> >> --- a/include/hw/i386/pc.h
> >> +++ b/include/hw/i386/pc.h
> >> @@ -105,6 +105,7 @@ struct PCMachineClass {
> >>     int legacy_acpi_table_size;
> >>     unsigned acpi_data_size;
> >>     bool do_not_add_smb_acpi;
> >> +    int pci_root_uid;
> >> 
> >>     /* SMBIOS compat: */
> >>     bool smbios_defaults;
> > 
> > So this upstream, but I think we should also have a property
> > that allows people to have VMs installed with the old
> > machine type booted with a new machine type.
> > 
> > E.g.  "compat-pci-root-uid".
> > 
> > Vitaly could you take a look?
> > 
> > 
> >> --
> >> 2.24.3 (Apple Git-128)
> > 
>