On 6/24/20 1:43 PM, Ard Biesheuvel wrote:
> 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?
I try to stay as far as possible from ACPI, but here it seems
fair I assign myself to this (except if Drew/Eric prefer to
do it, of course!).
>
> Thanks,
> Ard.
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61659): https://edk2.groups.io/g/devel/message/61659
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]
-=-=-=-=-=-=-=-=-=-=-=-
