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

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 14 April 2020 14:43
> To: paul@xxxxxxx
> Cc: 'Shamsundara Havanur, Harsha' <havanur@xxxxxxxxxx>; 
> xen-devel@xxxxxxxxxxxxxxxxxxxx;
> andrew.cooper3@xxxxxxxxxx; ian.jackson@xxxxxxxxxxxxx; wl@xxxxxxx; 
> roger.pau@xxxxxxxxxx
> Subject: Re: [XEN PATCH v2] hvmloader: Enable MMIO and I/O decode, after all 
> resource allocation
> 
> On 14.04.2020 13:58, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: 14 April 2020 12:40
> >> To: paul@xxxxxxx
> >> Cc: 'Shamsundara Havanur, Harsha' <havanur@xxxxxxxxxx>; 
> >> xen-devel@xxxxxxxxxxxxxxxxxxxx;
> >> andrew.cooper3@xxxxxxxxxx; ian.jackson@xxxxxxxxxxxxx; wl@xxxxxxx; 
> >> roger.pau@xxxxxxxxxx
> >> Subject: Re: [XEN PATCH v2] hvmloader: Enable MMIO and I/O decode, after 
> >> all resource allocation
> >>
> >> On 14.04.2020 13:01, Paul Durrant wrote:
> >>>> -----Original Message-----
> >>>>>
> >>>>> Previous commit enabled MASTER for all functions. I am bit confused
> >>>>> here on the consensus on enabling/disabling/retaining BME.
> >>>>> Should we even care about MASTER?
> >>>>
> >>>> With the commit introducing its universal setting, I'm afraid to
> >>>> avoid regressions we can't sensibly alter the behavior unless it
> >>>> can be explained clearly why the original change must have been
> >>>> outright wrong.
> >>>>
> >>>
> >>> Well the original code IIRC had no justification for setting BME
> >>> and doing it unconditionally does seem dangerous.
> >>
> >> I'm not viewing this as dangerous, merely as (typically) pointless.
> >> A well behaved device won't start issuing DMA requests merely
> >> because it had its bus mastering capability enabled. (And in the
> >> context of some IOMMU work of yours you actually stated there are
> >> devices where clearing of this bit won't stop them from doing so.)
> >>
> >
> > It's a line of defence against some devices at least,
> 
> What defence? Once we're past hvmloader, the guest can do whatever it
> wants anyway.
> 

My observation is that it is typically the device function driver that will 
enable BME, and that may come after a device-specific reset has been done. So, 
the chances of the VM surviving in the face of buggy h/w is higher if we don't 
blindly enable BME early on.

  Paul