Re: [PATCH v2 11/12] ArmVirtPkg/PlatformHasAcpiDtDxe: don't expose DT if QEMU provides ACPI

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

On 17 March 2017 at 21:25, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/17/17 22:10, Ard Biesheuvel wrote:
>> On 17 March 2017 at 20:47, Laszlo Ersek <lersek@redhat.com> wrote:
>>> This will let QEMU's "-no-acpi" option exclusively expose DT vs. ACPI to
>>> the guest. Showing both is never needed (it is actually detrimental to the
>>> adoption of standards, such as SBSA / SBBR).
>>>
>>> * Without "-no-acpi", the firmware logs (from PlatformHasAcpiDtDxe)
>>>
>>>> Found FwCfg @ 0x9020008/0x9020000
>>>> Found FwCfg DMA @ 0x9020010
>>>> InstallProtocolInterface: [EdkiiPlatformHasAcpiProtocol] 0
>>>
>>> plus the usual messages. Later the guest kernel logs
>>>
>>>> [    0.000000] efi:  SMBIOS 3.0=0x13bdb0000  ACPI 2.0=0x138440000
>>
>> Why is there no MEMATTR table in this case? Or was it omitted for brevity?
>
> I don't have the slightest clue. What is the MEMATTR table?
>
> ... Hm, grepping the kernel, it's apparently the memory attributes
> table. You added it in the following kernel commit:
>
> a604af075a32 ("efi: Add support for the EFI_MEMORY_ATTRIBUTES_TABLE
> config table", 2016-04-25)
>
> and it is part of release v4.7.
>
> Ah, I understand now. I used two kernels (two guests) for testing this,
> namely "4.7.7-200.fc24.aarch64" and "4.5.0-15.el7.aarch64".
>
> And, from the logs I collected, the "-no-acpi" log belongs to the Fedora
> kernel, which has your MEMATTR patch; while the log without "-no-acpi"
> (above) belongs to the RHEL-7.3 kernel, which does *not* have your
> MEMATTR patch.
>
> I've now removed "-no-acpi" from the Fedora 24 guest's config, and
> repeated the test. It prints:
>
> [    0.000000] efi:  SMBIOS 3.0=0x13bdb0000  ACPI 2.0=0x138440000
> MEMATTR=0x13a689018
>
> If you wish (and I don't have to submit a v3 for any other reason), I
> can refresh the above line in the commit message.
>

I was only concerned that these changes would affect whether the table
was published or not. But apparently not. Thanks for double checking.