Re: [Qemu-devel] Expand ECAM region in machvirt 2_13?

[Laszlo Ersek <lersek@redhat.com>]

On 05/02/18 16:38, Auger Eric wrote:
> Hi Laszlo, Ard,
> 
> On 05/02/2018 04:23 PM, Ard Biesheuvel wrote:
>> On 2 May 2018 at 15:54, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 05/02/18 14:34, Ard Biesheuvel wrote:
>>>> On 2 May 2018 at 13:31, Laszlo Ersek <lersek@redhat.com> wrote:
>>>>> On 05/01/18 17:59, Auger Eric wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I would like to resume the discussion on extending the number of PCI
>>>>>> buses to 256 (as in Q35) as a follow-up of past discussions:
>>>>>> https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg03631.html.
>>>>>>
>>>>>> With the current 16MB ECAM region we are limited to 16 PCIe busses.
>>>>>>
>>>>>> Could we envision to have a 256MB ECAM region and move it to another
>>>>>> location beyond 256GB, in virt2_13 machine type?
>>>>>>
>>>>>> Current ECAM range within [0x3f000000, 0x40000000] would be kept
>>>>>> unchanged for legacy and when vms->highmem is set to false.
>>>>>> Migration from <2_13 to >=2_13 would be allowed whereas migration
>>>>>> from >=2.13 to <2.13 wouldn't.
>>>>>
>>>>> If I understand correctly, the idea is to *move* the current one
>>>>> range, if the virt machine type is >= 2.13 and highmem is set to true
>>>>> (which is the default IIUC, from 2.12 onward).
>>>>>
>>>>> For 64-bit (AARCH64) ArmVirtQemu, that should work fine. The firmware
>>>>> takes the ECAM base and size from the "pci-host-ecam-generic" DT
>>>>> node, property "reg", uint64_t elements #0 and #1. (Sorry if this
>>>>> isn't exact DT lingo, I'm paraphrasing the firmware source code.) If
>>>>> the QEMU patch just changes the values, that should work
>>>>> transparently.
>>>>>
>>>>> For 32-bit (ARM) ArmVirtQemu, this change (the new ECAM default)
>>>>> could be a problem. PCI stuff in the firmware wouldn't work unless
>>>>> people specified highmem=off on the QEMU command line.
>>>>>
>>>>> Now, I notice highmen defaults to "on" starting with 2.12 even for
>>>>> "qemu-system-arm -M virt", not just "qemu-system-aarch64 -M virt", so
>>>>> why doesn't that already cause a problem with PCI in the 32-bit guest
>>>>> fw?
>>>>>
>>>>> Because, currently "highmen" only controls the presence of the 64-bit
>>>>> PCI MMIO aperture for BAR allocation; it has no effect on config
>>>>> space. And if the 64-bit PCI MMIO aperture is exposed to the 32-bit
>>>>> guest firmware, the latter simply ignores the former, and works with
>>>>> the 32-bit aperture solely (which is always there).
>>>>>
>>>>> So, for "qemu-system-arm -M virt" compatibility, I think we might
>>>>> need a separate machine type property, which should default to "on"
>>>>> only on qemu-system-aarch64 (if such distinctions are allowed).
>>>>>
>>>>> Of course, I can't tell if the 32-bit ArmVirtQemu firmware is
>>>>> possible to run on "qemu-system-aarch64 -M virt". (I think it is; I
>>>>> recall something something about ARMv8 having ARMv7 compat, but I
>>>>> don't remember ever trying.) If that's the case, then even the above
>>>>> suggestion won't work, because it would break 32-bit guest fw that
>>>>> the user has run (for whatever reason) on "qemu-system-aarch64 -M
>>>>> virt". In this case, I believe we can't just change the contents of
>>>>> the current "pci-host-ecam-generic" node, but we should implement
>>>>> some structural DTB addition that old firmware will simply not
>>>>> notice, while new (64-bit) firmware will specifically look for (and
>>>>> prefer over the old DT stuff).
>>>>>
>>>>> Ard, what's your take? (Sorry if you've already followed up, my email
>>>>> processing lags.)
>>>>>
>>>>
>>>> Do we have any examples of ACPI platforms where the config space is
>>>> mapped above 4 GB? I'd like to make sure that all existing code copes
>>>> with that before even considering it.
>>>
>>> Well, we could consider this virtual machine feature a way to root out
>>> any 64-bit bugs that lurk in code that consumes ECAM :) That would help
>>> physical platforms. It means that we shouldn't enable the feature by
>>> default, in 2.13 at least.
>>>
>>> Anyway, I've just checked my oldie A3 Mustang for this (it uses UEFI and
>>> ACPI), and surprisingly, it does put the ECAM range above 4GB:
>>>
>>> [    0.000000] ACPI: MCFG 0x00000043FA690000 00003C (v01 APM    XGENE    00000002 INTL 20140724)
>>> [    0.088654] ACPI: MCFG table detected, 1 entries
>>> [    0.126613] acpi PNP0A08:00: MCFG quirk: ECAM at [mem 0xe0d0000000-0xe0dfffffff] for [bus 00-ff] with xgene_v1_pcie_ecam_ops
>>> [    0.127552] acpi PNP0A08:00: [Firmware Bug]: ECAM area [mem 0xe0d0000000-0xe0dfffffff] not reserved in ACPI namespace
>>> [    0.127601] acpi PNP0A08:00: ECAM at [mem 0xe0d0000000-0xe0dfffffff] for [bus 00-ff]
>>>
>>> The base address is 899 GB + 256 MB.
>>>
>>> My kernel is 4.11.0-44.6.1.el7a.aarch64.
>>>
>>
>> Interesting. So Linux deals with that fine. How about the missing
>> PNP0C02 device:
>>
>> Device (RES0)
>> {
>>    Name (_CID, "PNP0C02")
>>    Name (_CRS, ResourceTemplate () {
>>      Memory32Fixed (ReadWrite, 0x... , 0x1000000)
>>    })
>> }
>>
>> Anyone care to venture a guess how one expresses this, given that
>> Memory64Fixed does not appear to exist?
>>
>> (Perhaps our QEMU code only needs a minor tweak here, but I honestly don't know)
> 
> Thank you for your inputs,
> 
> Maybe we can use aml_dword_memory(), as it is done for highmem MMIO? I
> will give this a try.

Right; after looking at the ACPI-6.2 spec for a bit, I think that
Memory32Fixed is just a specialized resource descriptor (see "6.4.3.4
32-Bit Fixed Memory Range Descriptor"). A 64-bit variant was not added
to the spec likely because another descriptor type can express the same
thing as a special case; see "6.4.3.5 Address Space Resource
Descriptors". So I think what's needed here is "6.4.3.5.1 QWord Address
Space Descriptor".

And QEMU already has aml_qword_memory() -- it's used in
"hw/arm/virt-acpi-build.c" too, in the acpi_dsdt_add_pci() function. It
is used to describe the 64-bit PCI MMIO aperture (VIRT_PCIE_MMIO_HIGH),
IIUC.

Thanks
Laszlo