[vbox-dev] Patch for PCI prefetch alignment issue

[Klaus Espenlaub]

Hi Ric,

On 03.10.2017 20:36, Vilbig, Ric wrote:
> Hi,
> 
> I found a problem in the ICH9 fake bios prefetch space address 
> management, and I would like to submit my patch in hopes you will merge 
> it into the mainstream.  I am submitting the patch under the MIT license 
> terms.  Please find attached the diff file showing the patch as well as 
> the actual (one) source file that was changed.  These are based on the 
> 5.1.28 source code tarball.

Thanks (next time please use "diff -u", but since you were so precise we 
can apply it nevertheless).

Could you give an example of a setup where the incorrect behavior is 
triggered using the original code? I'm aware that you can only provide 
it with this change, but I hope I can figure out what the actual problem 
with the existing code is. Your analysis below makes only some sense, 
not complete sense.

Ideally the output of "lspci -v" showing at least the bridge(s) and the 
relevant BAR information for the device behind the bridge. Feel free to 
censor out whatever we're not supposed to know, because in the end I'm 
really only after the bridge/BAR config in the VM, and nothing else.

> The problem arises when the first prefetchable BAR has a size greater 
> than the 1M alignment that is applied to the Prefetch Base register.  In 
> this case, the Prefetch Base register is aligned differently than the 
> BAR register.  This leads to VERR_MM_HYPER_NO_MEMORY being returned when 
> the range is registered with PDMDevHlpMMIORegister().  Specifically, the 
> error originates in mmHyperAllocInternal().

Scratching my head. The BAR address of a device should be always 
correctly aligned. The address decoding programmed into the bridge(s) 
connecting the device to the root bus may have a lower alignment, but it 
shouldn't be incorrect as such. If I read our code correctly the address 
decoding of bridges will have no impact on hyper heap allocation.

> This patch resolves the problem by changing the hard-coded 1M alignment 
> applied to the Prefetch Base register into a variable which is the 
> greater of 1M or the alignment requirement indicated in the BAR 
> registers. The result is that the Prefetch Base register and BAR are 
> aligned the same way, and the memory registration is successful.

Again not really plausible. I'm aware that the bridge might end up with 
a start address below the actual start of the resources behind the 
bridge, but there will be no decoding overlaps, so this should be 
entirely harmless. I've tested this months ago with a rather complicated 
device using big BARs, where the alignment issues should have also shown 
up, but there was no issue.

Wondering if you're not observing some issue with slightly varying 
amount of hyper heap usage depending on some alignment, pushing this 
over the edge relatively randomly. How big are these MMIO BARs of your 
custom device? Devices aren't meant to allocate giant MMIO regions using 
PDMDevHlpMMIORegister. It's inefficient for many reasons, and most 
importantly it wastes space on the hyper heap.

Klaus

> 
> //  RicV