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

[Vitaly Cheptsov]

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