On 6/24/20 1:20 PM, Laszlo Ersek via groups.io wrote:
(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
<include/hw/arm/boot.h>, 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".)


I wasn't aware that we even expose the flash in the DSDT. In any case, no driver exists in Linux today that claims the LNRO0015 _HID, and so I agree we should simply remove it entirely.

However, I am no longer able to contribute to QEMU, so I was hoping you or Phil could take the action?

Thanks,
Ard.




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61650): https://edk2.groups.io/g/devel/message/61650
Mute This Topic: https://groups.io/mt/75065345/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to