Re: [PATCH v4] IOMMU: make DMA containment of quarantined devices optional

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.11.2020 11:45, Paul Durrant wrote:
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: 27 November 2020 16:46
>>
>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -1278,7 +1278,7 @@ detection of systems known to misbehave
>>  > Default: `new` unless directed-EOI is supported
>>
>>  ### iommu
>> -    = List of [ <bool>, verbose, debug, force, required, quarantine,
>> +    = List of [ <bool>, verbose, debug, force, required, 
>> quarantine[=scratch-page],
>>                  sharept, intremap, intpost, crash-disable,
>>                  snoop, qinval, igfx, amd-iommu-perdev-intremap,
>>                  dom0-{passthrough,strict} ]
>> @@ -1316,11 +1316,32 @@ boolean (e.g. `iommu=no`) can override t
>>      will prevent Xen from booting if IOMMUs aren't discovered and enabled
>>      successfully.
>>
>> -*   The `quarantine` boolean can be used to control Xen's behavior when
>> -    de-assigning devices from guests.  If enabled (the default), Xen always
>> +*   The `quarantine` option can be used to control Xen's behavior when
>> +    de-assigning devices from guests.
>> +
>> +    When a PCI device is assigned to an untrusted domain, it is possible
>> +    for that domain to program the device to DMA to an arbitrary address.
>> +    The IOMMU is used to protect the host from malicious DMA by making
>> +    sure that the device addresses can only target memory assigned to the
>> +    guest.  However, when the guest domain is torn down, assigning the
>> +    device back to the hardware domain would allow any in-flight DMA to
>> +    potentially target critical host data.  To avoid this, quarantining
>> +    should be enabled.  Quarantining can be done in two ways: In its basic
>> +    form, all in-flight DMA will simply be forced to encounter IOMMU
>> +    faults.  Since there are systems where doing so can cause host lockup,
>> +    an alternative form is available where writes to memory will be made
>> +    fault, but reads will be directed to a dummy page.  The implication
>> +    here is that such reads will go unnoticed, i.e. an admin may not
>> +    become aware of the underlying problem.
>> +
>> +    Therefore, if this option is set to true (the default), Xen always
>>      quarantines such devices; they must be explicitly assigned back to Dom0
>> -    before they can be used there again.  If disabled, Xen will only
>> -    quarantine devices the toolstack hass arranged for getting quarantined.
>> +    before they can be used there again.  If set to "scratch-page", still
>> +    active DMA reads will additionally be directed to a "scratch" page.  If
> 
> There's inconsistency of terms here. We should choose either 'dummy page'
> or 'scratch page' (and my vote goes for the latter).

Oh, that wasn't intentional. I've replaced all "dummy" now.

> Also, rather than true or false, shouldn't we have 'off', 'basic', and
> 'scratch-page'?

I didn't want to break (or needlessly extend) the present boolean nature
of the option. Hence I only added "scratch-page". I wouldn't want to add
"basic" as an alias of "true", but if you think we really need this, then
I surely could do so. As to "off" vs "false" - both are permitted anyway
by the parsing functions. And to me (both as a programmer and as someone
who had been studying maths long ago) something that's boolean goes
rather with true/false than on/off; I can certainly change that wording
if you deem that more appropriate / helpful for the target audience.

Jan