RE: [XEN PATCH] hvmloader: Enable MMIO and I/O decode, after all resource allocation

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Andrew 
> Cooper
> Sent: 06 April 2020 19:07
> To: Harsha Shamsundara Havanur <havanur@xxxxxxxxxx>; 
> xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Jan 
> Beulich <jbeulich@xxxxxxxx>;
> Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Subject: Re: [XEN PATCH] hvmloader: Enable MMIO and I/O decode, after all 
> resource allocation
> 
> On 06/04/2020 18:46, Harsha Shamsundara Havanur wrote:
> > 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.
> 
> What kinds of spurious transactions?
> 
> Keeping memory and I/O decoding disabled until the BARs are set up is a
> no-brainer, but busmastering is a more complicated subject.  Therefore,
> it would be helpful to know exactly what you've seen in the way of
> spurious transactions.
> 

I think you know of some GPU h/w that doesn't necessarily stop DMAing after an 
FLR. There is no reason why hvmloader, or anything else until the function 
driver loads, needs BME to be on. As you say mem and io decodes are 
no-brainers, yet without this patch hvmloader enables them after the first BAR 
on the device is programmed, thus causing much fun for device models when the 
subsequent BARs are programmed.

> >
> > This patch address the issue by deferring enablement of memory and
> > I/O decode in command register until all the resources, like interrupts
> > I/O and/or MMIO BARs for all the PCI device functions are programmed.
> > PCI bus memory and I/O space is enabled in command register after
> > all the resources like interrupts, I/O and/or MMIO BARs are
> > programmed for all valid device functions. PCI BUS MASTER is kept
> > disabled in the bootloader as this needs to be enabled by the guest
> > OS driver once it initializes and takes control of the device.
> 
> Has this been tested with an Intel integrated graphics card?  These have
> a habit of hitting a platform reset line if busmaster is ever disabled.
> 

No, we don't have suitable h/w for that AFAIK. If that is the case then we 
ought to quirk it.

> A lot of this will depend on what Qemu does behind the scenes, and
> whether disabling busmastering gets reflected in the real device.
> 

When I last looked at upstream QEMU modifications to the BME bit were echoed 
through.

> >
> > Signed-off-by: Harsha Shamsundara Havanur <havanur@xxxxxxxxxx>
> > Ack-by: Paul Durrant <pdurrant@xxxxxxxxxx>
> 
> Acked-by

This was a little premature as I have not yet looked at the re-based code.

  Paul

> 
> ~Andrew