[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] 64-bit MMIO aperture expansion
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] 64-bit MMIO aperture expansion |
Date: |
Fri, 21 Sep 2018 19:34:36 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 09/21/18 17:01, Marcel Apfelbaum wrote:
> On 09/20/2018 05:49 PM, Laszlo Ersek wrote:
> I had to read this mail a few times...
Sorry :)
>> Now consider the following scenario:
>>
>> - the firmware programs some BARs with 64-bit addresses such that the
>> aperture that we deduce starts at 32GB,
>>
>> - the guest has 4GB of RAM, and no DIMM hotplug range.
>>
>> Consequences:
>>
>> - Because the "32-bit RAM split" for Q35 is at 2GB, the
>> pc_pci_hole64_start() function will return 6GB.
>>
>> - The q35_host_get_pci_hole64_start() function will return 32GB. (It
>> will not fall back to pc_pci_hole64_start() -- correctly --
>> because the firmware has programmed some BARs with 64-bit
>> addresses.)
>>
>> - The q35_host_get_pci_hole64_end() function *intends* to return
>> 64GB, because -- let's say -- the guest assigned BARs covering the
>> 32GB..34GB range, which is 2GB in size, and we *intend* to round
>> that size up to 32GB, so that 30GB be left for hotplug purposes.
>> (This is the original intent of commit 9fa99d2519cb.)
>> - However, because we initialize "hole64_start" from
>> pc_pci_hole64_start(), and not from
>> q35_host_get_pci_hole64_start(), we add "mch.pci_hole64_size"
>> (32GB by default) to 6GB (the end of RAM), and not to 32GB (the
>> aperture base deduced from the firmware's programming). As a
>> result, we'll extend the aperture end address only to 38GB, and
>> not to 64GB.
>
> Right, there is no reason to use pc_pci_hole64_start, it looks
> like a plain bug. We diverged from pc and the fact that
> q35_host_get_pci_hole64_star uses it is only an implementation
> detail.
Small correction (not affecting the main point):
If you compare q35_host_get_pci_hole64_end() and
i440fx_pcihost_get_pci_hole64_end(), you see that they do the exact same
thing, and pc_pci_hole64_start() is a common helper function that they
both call.
This is also matched by the files that define these functions:
- i440fx_pcihost_get_pci_hole64_end(): hw/pci-host/piix.c
- q35_host_get_pci_hole64_end(): hw/pci-host/q35.c
- pc_pci_hole64_start(): hw/i386/pc.c
In that sense, we didn't "diverge" from PC. Because, both i440fx and q35
received the exact same logic in 9fa99d2519cb. They both call the common
pc_pci_hole64_start() helper function as an internal / implementation
detail. (And both of them should be fixed.)
Current function call chains:
i440fx_pcihost_get_pci_hole64_start() |
q35_host_get_pci_hole64_start()
pc_pci_hole64_start() [good call] |
pc_pci_hole64_start() [good call]
|
i440fx_pcihost_get_pci_hole64_end() |
q35_host_get_pci_hole64_end()
pc_pci_hole64_start() [bug] |
pc_pci_hole64_start() [bug]
Proposed call chains:
i440fx_pcihost_get_pci_hole64_start() |
q35_host_get_pci_hole64_start()
pc_pci_hole64_start() [unchanged call] |
pc_pci_hole64_start() [unchanged call]
|
i440fx_pcihost_get_pci_hole64_end() |
q35_host_get_pci_hole64_end()
i440fx_pcihost_get_pci_hole64_start() [corrected call] |
q35_host_get_pci_hole64_start() [corrected call]
pc_pci_hole64_start() [unchanged call] |
pc_pci_hole64_start() [unchanged call]
>> The same would apply to i440fx too.
>
> I am lost here. The q35 PCI 64bit hole computation issue starts from
> the miss-use of the PC conterpart functions.
I disagree. If q35 called an i440fx-specific function, that would indeed
be mis-use. However, in this case, the callee -- that is,
pc_pci_hole64_start() -- is not an i440fx-specific function. It is a
common helper for both boards. Both boards can rightfully call it.
Instead, the issue here is that *both*
i440fx_pcihost_get_pci_hole64_end() and q35_host_get_pci_hole64_end()
need a "hole64_start" value that accounts for the BAR addresses that
were programmed by the firmware. pc_pci_hole64_start() simply doesn't do
that.
> What is the problem with the PC?
i440fx_pcihost_get_pci_hole64_end() currently calls
pc_pci_hole64_start() directly, so the "hole64_start" value does not
honor the BAR addresses set by the firmware.
Thanks,
Laszlo