[vbox-dev] Patch for PCI prefetch alignment issue

[Vilbig, Ric]

Hi Klaus,

Sorry for the delayed response, I got pulled on to something else for a few days.  Please find attached the information you asked for, and other details, specifically:

diff-u.log  = diff -u

lspci-tt.log  = PCI bus hierarchy.  The device at 0:1c:0 is my PDM library.  Everything below it is the end user's device model.

lspci-v_scrubbed.log  = output of lspci -v with user's information removed.  This shows the memory requirements of their device model.

RemDev.log  = summary of PCI config and memory assignments after Fake BIOS is done, emitted by my PDM library.

VirtualBox_abridged.log  = VirtualBox log file; VirtualBox was built with LOG_ENABLED set to 1, and was run with VBOX_LOG=all.  Then everything unrelated to this issue was removed.  The VERR_MM_HYPER_NO_MEMORY at the end is the result of my PDM library trying to register the address ranges it's responsible for, based on Base/Limit register pairs.

RicV.log  = some log messages I inserted deep in the memory allocation code, showing call/return values -- note this one file was from my analysis in September, with 5.1.24.


With my patch enabled, the PrfBase register is set to 3001 instead of 2401, so the prefetch range I register is 230000000 - 0x2401fffff instead of 224000000 - 0x2401fffff.  Because of the reduced size, the registration succeeds.

You said "Devices aren't meant to allocate giant MMIO regions using PDMDevHlpMMIORegister", but how else can I make sure accesses to my memory ranges are passed to my PDM library?  And what do you consider to be giant?

Let me know if you need anything more.

//  RicV


| -----Original Message-----
| From: vbox-dev-bounces at virtualbox.org [mailto:vbox-dev-
| bounces at virtualbox.org] On Behalf Of Klaus Espenlaub
| Sent: Wednesday, October 04, 2017 04:52
| To: vbox-dev at virtualbox.org
| Subject: Re: [vbox-dev] Patch for PCI prefetch alignment issue
| 
| 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
| _______________________________________________
| vbox-dev mailing list
| vbox-dev at virtualbox.org
| https://www.virtualbox.org/mailman/listinfo/vbox-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: RicV.tar.gz
Type: application/x-gzip
Size: 4673 bytes
Desc: RicV.tar.gz
URL: <http://www.virtualbox.org/pipermail/vbox-dev/attachments/20171011/db18f843/attachment.bin>