From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59594) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cTQOu-0000fR-3V for qemu-devel@nongnu.org; Tue, 17 Jan 2017 04:50:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cTQOq-0003hU-6r for qemu-devel@nongnu.org; Tue, 17 Jan 2017 04:50:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52026) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cTQOp-0003fJ-Un for qemu-devel@nongnu.org; Tue, 17 Jan 2017 04:49:56 -0500 Date: Tue, 17 Jan 2017 10:49:50 +0100 From: Andrew Jones Message-ID: <20170117094950.5dqdpbk6u7p4yhms@kamzik.brq.redhat.com> References: <1484328738-21149-1-git-send-email-ard.biesheuvel@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v2] hw/arm/virt-acpi - reserve ECAM space as PNP0C02 device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ard Biesheuvel Cc: Peter Maydell , Leif Lindholm , QEMU Developers , Graeme Gregory , Al Stone , mst@redhat.com, Igor Mammedov On Mon, Jan 16, 2017 at 07:31:33PM +0000, Ard Biesheuvel wrote: > On 16 January 2017 at 18:20, Peter Maydell wrote: > > On 16 January 2017 at 17:30, Ard Biesheuvel wrote: > >> On 16 January 2017 at 17:25, Peter Maydell wrote: > >>> On 13 January 2017 at 17:32, Ard Biesheuvel wrote: > >>>> Linux for arm64 v4.10 and later will complain if the ECAM config space is > >>>> not reserved in the ACPI namespace: > >>>> > >>>> acpi PNP0A08:00: [Firmware Bug]: ECAM area [mem 0x3f000000-0x3fffffff] not reserved in ACPI namespace > >>>> > >>>> The rationale is that OSes that don't consume the MCFG table should still > >>>> be able to infer that the PCI config space MMIO region is occupied. > >>>> > >>>> So update the ACPI table generation routine to add this reservation. > >>>> > >>>> Signed-off-by: Ard Biesheuvel > >>>> --- > >>>> hw/arm/virt-acpi-build.c | 7 +++++++ > >>>> 1 file changed, 7 insertions(+) > >>>> > >>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > >>>> index 085a61117378..50d52f685f68 100644 > >>>> --- a/hw/arm/virt-acpi-build.c > >>>> +++ b/hw/arm/virt-acpi-build.c > >>>> @@ -310,6 +310,13 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, > >>>> Aml *dev_rp0 = aml_device("%s", "RP0"); > >>>> aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0))); > >>>> aml_append(dev, dev_rp0); > >>>> + > >>>> + Aml *dev_res0 = aml_device("%s", "RES0"); > >>>> + aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02"))); > >>>> + crs = aml_resource_template(); > >>>> + aml_append(crs, aml_memory32_fixed(base_ecam, size_ecam, AML_READ_WRITE)); > >>>> + aml_append(dev_res0, aml_name_decl("_CRS", crs)); > >>>> + aml_append(dev, dev_res0); > >>>> aml_append(scope, dev); > >>>> } > >>> > >>> This needs to be controlled via the machine class back-compat > >>> machinery in hw/arm/virt.c so that it only happens for virt-2.9 > >>> and later. > >>> > >> > >> Why exactly? > > > > Because the "virt-2.8" machine has to present to the guest > > exactly what "virt" did as of the QEMU 2.8 release, including > > any bugs or missing things we happened to have in our ACPI > > tables. This allows cross-version compatibility (including > > VM migration). Drew will have a more detailed explanation > > if you need it. > > > > I suspected as much. > > But in this case, I am not sure if it is worth the trouble: the > generated data is only consumed at boot time by the firmware, and I > suppose migration involves freezing a VM, including whatever resident > firmware image was used to boot the OS, and so this is unlikely to > affect migration. > > But I will let Drew explain ... > In some cases the problem we're solving with the compat guards is a bit hypothetical, but, IMHO, nonetheless a good practice. While we may be sure that AAVMF and Linux will be fine with this table changing under their feet, we can't be sure there aren't other mach-virt users that have more sensitive firmwares/OSes. An ACPI- sensitive OS may notice the change on its next reboot after a migration, and then simply refuse to continue. Now, that said, I just spoke with Igor in order to learn the x86 practice. He says that the policy has been more lax than what I suggest above. Hypothetical, low-risk issues are left unguarded, and only when a bug is found during testing is it then managed. The idea is to try and reduce the amount of compat variables and conditions needed in the ACPI generation code, but, of course, at some level of risk to users expecting their versioned machine type to always appear the same. So far we've been strict with mach-virt, guarding all hypothetical issues. Perhaps this patch is a good example to get a discussion started on whether or not we should be so strict though. Thanks, drew