From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web12.11337.1592997641910837585 for ; Wed, 24 Jun 2020 04:20:42 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=i/ZUNY9x; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1592997641; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=FvxC+cxAavR0YjfnetsC3JgemDICX9zrapajN1bQVFo=; b=i/ZUNY9xwFUpEat5vcfPqJIHV9ub7uPxRWERH7F79XTMrjxSg8tdLyhnM4rmKJNOHKE+G0 9oYgNh6U7U2xnC4A1eZpyjCJcK7LXeJeJnpjEM51vfq9KZTIJhAZS4zCAhDdiTvaGX6Ux/ oC4boR8b2CNcJo3pP5s1Vix1wbsXvWE= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-80-DUcKjLCION6zvfzUHOgS_A-1; Wed, 24 Jun 2020 07:20:39 -0400 X-MC-Unique: DUcKjLCION6zvfzUHOgS_A-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 18A76804003; Wed, 24 Jun 2020 11:20:38 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-56.ams2.redhat.com [10.36.112.56]) by smtp.corp.redhat.com (Postfix) with ESMTP id 85FFA5C290; Wed, 24 Jun 2020 11:20:33 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] ArmVirtPkg/NorFlashQemuLib: disable NOR flash DT nodes upon discovery To: ard.biesheuvel@arm.com Cc: devel@edk2.groups.io, =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , Drew Jones , Eric Auger References: <20200623175700.1564281-1-ard.biesheuvel@arm.com> <85a63fc4-f3d1-1e17-bf1d-dace59bb02a8@arm.com> From: "Laszlo Ersek" Message-ID: <2a43b904-5172-0fb3-6d40-e1fd3b652fe7@redhat.com> Date: Wed, 24 Jun 2020 13:20:32 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <85a63fc4-f3d1-1e17-bf1d-dace59bb02a8@arm.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit (CC Drew, Eric) On 06/24/20 09:19, Ard Biesheuvel wrote: > The arm64 defconfig was recently updated with MTD support, and so the > flash banks are now claimed by the CFI flash driver. I saw the same on > 32-bit ARM today, and I am not sure if it is a change there or whether > it was always like that (for multi_v7_defconfig) but there is no good > reason to keep supporting this. Can the same (problematic) kernel driver binding occur via the ACPI DSDT? In this fw patch we hide the flash chip(s) from the guest kernel via Device Tree only. There isn't much I guess we can (or should) do about ACPI in the firmware, but it would still be a conflict between the fw driver and the kernel driver -- we might have to address that in QEMU (hide the pflash in the ACPI generator when UEFI is used as guest firmware). IIRC, in QEMU, we use "arm_boot_info.firmware_loaded", from , to represent UEFI: > /* Boot firmware has been loaded, typically at address 0, with -bios or > * -pflash. It also implies that fw_cfg_find() will succeed. > */ > bool firmware_loaded; And we already seem to have this exact kind of distinction in QEMU; see for example "hw/arm/virt.c": > if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) { > vms->acpi_dev = create_acpi_ged(vms); > } else { > create_gpio(vms); > } coming from commit cff51ac978c4 ("hw/arm/virt: Enable device memory cold/hot plug with ACPI boot", 2019-10-05). And (same file): > if (!vms->bootinfo.firmware_loaded || !virt_is_acpi_enabled(vms)) { > return HOTPLUG_HANDLER(machine); > } coming from commit 70e89132c9e6 ("hw/arm/virt: Add the virtio-iommu device tree mappings", 2020-02-27). ... Ah, I think I've found it [hw/arm/virt-acpi-build.c]: > /* DSDT */ > static void > build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) > { > Aml *scope, *dsdt; > MachineState *ms = MACHINE(vms); > const MemMapEntry *memmap = vms->memmap; > const int *irqmap = vms->irqmap; > > dsdt = init_aml_allocator(); > /* Reserve space for header */ > acpi_data_push(dsdt->buf, sizeof(AcpiTableHeader)); > > /* When booting the VM with UEFI, UEFI takes ownership of the RTC hardware. > * While UEFI can use libfdt to disable the RTC device node in the DTB that > * it passes to the OS, it cannot modify AML. Therefore, we won't generate > * the RTC ACPI device at all when using UEFI. > */ > scope = aml_scope("\\_SB"); > acpi_dsdt_add_cpus(scope, vms->smp_cpus); > acpi_dsdt_add_uart(scope, &memmap[VIRT_UART], > (irqmap[VIRT_UART] + ARM_SPI_BASE)); > acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]); The ACPI generator already takes the RTC into account; see QEMU commit 67736a25f865 ("ARM: virt: Don't generate RTC ACPI device when using UEFI", 2016-01-15). Should we do the same for the acpi_dsdt_add_flash() call, now? (This also suggests that my consideration of "firmware_loaded" above is irrelevant, if we end up modifying the build_dsdt() function -- on AARCH64, ACPI is only defined in UEFI terms (namely, as a UEFI system config table), so in this function, the presence of UEFI can be assumed "yes".) Thanks! Laszlo