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

[Vitaly Cheptsov]

They can simply set the 5.2 VM version in such a case. I do not want to let 
this legacy hack to be enabled in any modern QEMU VM version, as it violates 
ACPI specification and makes the life more difficult for various other software 
like bootloaders and operating systems.

> 23 марта 2021 г., в 17:48, 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)
>>> 
>> 
> 
>

signature.asc
Description: Message signed with OpenPGP