> -----Original Message----- > From: Roger Pau Monné <roger....@citrix.com> > Sent: 07 April 2020 09:07 > To: p...@xen.org > Cc: 'Harsha Shamsundara Havanur' <hava...@amazon.com>; > xen-devel@lists.xenproject.org; 'Wei Liu' > <w...@xen.org>; 'Andrew Cooper' <andrew.coop...@citrix.com>; 'Ian Jackson' > <ian.jack...@eu.citrix.com>; > 'Jan Beulich' <jbeul...@suse.com> > Subject: Re: [XEN PATCH] hvmloader: Enable MMIO and I/O decode, after all > resource allocation > > On Tue, Apr 07, 2020 at 08:48:42AM +0100, Paul Durrant wrote: > > > -----Original Message----- > > > From: Xen-devel <xen-devel-boun...@lists.xenproject.org> On Behalf Of > > > Harsha Shamsundara Havanur > > > Sent: 06 April 2020 18:47 > > > To: xen-devel@lists.xenproject.org > > > Cc: Wei Liu <w...@xen.org>; Andrew Cooper <andrew.coop...@citrix.com>; > > > Ian Jackson > > > <ian.jack...@eu.citrix.com>; Jan Beulich <jbeul...@suse.com>; Harsha > > > Shamsundara Havanur > > > <hava...@amazon.com>; Roger Pau Monné <roger....@citrix.com> > > > Subject: [XEN PATCH] hvmloader: Enable MMIO and I/O decode, after all > > > resource allocation > > > > > > It was observed that PCI MMIO and/or IO BARs were programmed with > > > BUS master, memory and I/O decodes (bits 0,1 and 2 of PCI COMMAND > > > register) enabled, during PCI setup phase. This resulted in > > > spurious and premature bus transactions as soon as the lower bar of > > > the 64 bit bar is programmed. It is highly recommended that BARs be > > > programmed whilst decode bits are cleared to avoid spurious bus > > > transactions. > > > > > > > It's not so much spurious transactions that are the issue. I think > > "spurious and premature bus > transactions" should be replaced with "incorrect mappings being created". > > > > I believe the PCI spec says all three bits should be clear after reset > > anyway, and BAR programming > whilst decodes are enabled causes problems for emulators such as QEMU which > need to create and destroy > mappings between the gaddr being programming into the virtual BAR and the > maddr programmed into the > physical BAR. > > Specifically the case we see is that a 64-bit memory BAR is programmed by > > writing the lower half and > then the upper half. After the first write the BAR is mapped to an address > under 4G that happens to > contain RAM, which is displaced by the mapping. After the second write the > BAR is re-mapped to the > intended location but the RAM displaced by the other mapping is not restored. > The OS then continues to > boot and function until at some point it happens to try to use that RAM at > which point it suffers a > page fault and crashes. It was only by noticing that the faulting address lay > within the transient BAR > mapping that we figured out what was happening. > > In order to fix this isn't it enough to just disable memory and IO > decodes, leaving bus mastering as it is? > > I assume there is (or was) some reason why bus master is enabled by > hvmloader in the first place? >
I admit it is a while since I went mining for the reason BME was being set but IIRC the commit that added the original code did not state why it was done. In the past I have run with local hacks disabling it whilst playing with GPU pass-through and not noticed it causing any problems. There is an argument to say that hvmloader should perhaps leave it alone but there is no good reason I can think of why it ought to be enabling it. Paul > Thanks, Roger.
