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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.04.2020 15:51, Paul Durrant wrote:
>> -----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.

Well, I'd agree if only we knew the reason for the introduction of
this, many years ago.

Jan