Re: [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 21 March 2017 at 18:00, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/21/17 10:02, Laszlo Ersek wrote:
>> On 03/21/17 09:43, Ard Biesheuvel wrote:
>>> On 21 March 2017 at 07:10, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>>> On 21 March 2017 at 02:27, Zeng, Star <star.zeng@intel.com> wrote:
>>>>> Just an idea.
>>>>>
>>>>
>>>> I like it!
>>>>
>>>>> There is a way to do not introduce a new PCD, but reuse PcdAcpiExposedTableVersions.
>>>>> When PcdAcpiExposedTableVersions is configured to *0* (means no ANY ACPI table should be exposed), AcpiTableDxe returns EFI_UNSUPPORTED in the driver entrypoint.
>>>>>
>>>>
>>>> So could we then upgrade the definition so it is possible to use a
>>>> dynamic PCD for this?
>>>>
>>>
>>> Oh, and how is the ordering ensured in this case? We need to set the
>>> dynamic PCD at runtime, but obviously before AcpiTableDxe gets a
>>> chance to load. If the only way to achieve this is another NULL class
>>> library, then this is not much of an improvement.
>>
>> I've been silently waiting for this to get noticed :)
>>
>> In the ArmVirtQemu case, the PCD value would be set from fw_cfg. But in
>> ArmVirtQemu, fw_cfg is not available in PEI, only in DXE, so yes, a NULL
>> class library would be needed.
>>
>> In physical firmware (also in the planned "showcase QEMU" case), the
>> toggle would be exposed by another DXE driver (a platform driver) using
>> an HII form. Generally such a driver translates an nvvar to a PcdSet in
>> its entry point, plus registers HII stuff that allows the nvvar to be
>> toggled interactively in the setup browser, for the next boot. Either
>> way, the PcdSet would again only happen in DXE.
>>
>>> Or could we detect
>>> the table-loader node from PEI as well?
>>
>> Not at the moment; fw_cfg is currently not available in PEI, in ArmVirtQemu.
>>
>> ArmVirtQemu's PEI phase executes in-place from pflash. That means no
>> writeable global variables (unless you make a PEIM, or the PEI library
>> instance, dependent on the "memory discovered" PPI). Even using just the
>> MMIO (no DMA) interface to fw_cfg, you'd have to parse the DTB on every
>> invocation. (No writeable global variables imply you can't pre-parse the
>> stuff in the library constructor.)
>>
>> A similar example is
>> "ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.c",
>> where each serial port write has to re-parse the base address from the DT.
>>
>> (OVMF is different because OVMF's PEI runs from memory.)
>>
>> So, whatever you saved on the PcdSet ordering in the DXE phase, you
>> would triply lose in fw_cfg complexity in PEI. A new PEI library
>> instance would be necessary for fw_cfg, which would either have to parse
>> the DT on each invocation, or else depend on permanent PEI RAM
>> availability (which would be the same (or worse) kind of ordering
>> restriction that you're trying to avoid in DXE in the first place).
>
> So... does that make this patch set (relatively) less "unwieldy" to you?
>

I suppose, yes :-)

I guess we will never have a perfect solution for this, and using
library classes and protocol dependencies is much preferred over
having to reason about when a dynamic PCD assumes its correct value.