Re: [RFC PATCH] iommu: make no-quarantine mean no-quarantine

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.04.2021 00:00, Scott Davis wrote:
> On 4/27/21, 2:56 AM, Jan Beulich wrote:
>> On 26.04.2021 19:25, Scott Davis wrote:
>>> This patch modifies Xen's behavior when making devices assignable while the
>>> iommu=no-quarantine command line option is in effect. Currently this option
>>> only affects device deassignment, causing devices to get immediately 
>>> assigned
>>> back to Dom0 instead of to the quarantine dom_io domain. This patch extends
>>> no-quarantine to device assignment as well, preventing devices from being
>>> assigned to dom_io when they are made assignable while no-quarantine is in
>>> effect.
>>
>> Well, the term "quarantine" to me means a safety action taken _after_
>> possible exposure to something "bad". Therefore I see this being specific
>> to device de-assignment as the logical thing. Hence if a mode like what
>> you describe was wanted, I don't think it should be the result of
>> "iommu=no-quarantine".
> 
> Sorry I'm a bit confused by this. Correct me if wrong, but my understanding 
> is  
> that the purpose of assigning a device to dom_io on de-assignment is to 
> protect  
> Dom0 from the effects of in-flight DMA operations initiated by a DomU. I 
> assumed  
> that the purpose of (temporarily) assigning to dom_io on assignment was the 
> same  
> but in reverse: protecting a DomU from Dom0-initiated ops. Is this not the 
> case?

Well, no, not really. Dom0 is considered fully trusted for a variety of
reasons.

> Also, documentation and code already refer to the operation in question as a 
> "quarantine" (see xl command line docs and libxl__device_pci_assignable_add) 
> and to devices that have undergone it as being "quarantined" (see 
> assign_device 
> in xen/drivers/passthrough/pci.c). So if that is not the correct term, there 
> may 
> be some additional changes needed for consistency. Is there another name that 
> would be more appropriate?

I don't see what's wrong with the term for how things are currently. If
you talk about an adjustment to terminology to accompany your proposed
change - not sure.

> I would also point out that, currently, there does not appear to be a way for 
> an 
> xl user to opt out of quarantine functionality in either direction other than 
> by 
> manually making devices assignable. There is no xl command line flag to 
> disable 
> it and iommu=no-quarantine will have no effect because any device that xl 
> itself
> makes assignable will have the struct pci_dev.quarantine flag set, which 
> overrides iommu=no-quarantine. Is that intentional?

Not sure here either: It may also have been that it was assumed to not
be of interest. Paul?

> If I misunderstood and your objection is simply that 
> "quarantine-on-assignment"  
> and "quarantine-on-deassignment" should be controllable by separate iommu  
> options, that's an easy enough change to make.

Yes, effectively it's that what I think things would want to be, if
"quarantine-on-assignment" is really something that we think is needed.
It would default to off imo.

> Although I think that might also 
> negate the need for/effect of struct pci_dev.quarantine as described above. 
> If 
> that's what is desired, any thoughts on what the new option(s) should be 
> called?

Following the extension to the command line option I'm putting in place
in "IOMMU: make DMA containment of quarantined devices optional" (which
I still need to get around to address review feedback for and resubmit),
I'd be inclined to suggest "iommu=quarantine=always" or
"iommu=quarantine=on-assign". Unless of course we'd prefer to have the
caller of the assignment operation have full control over the behavior
here anyway (in which case a command line option control simply is not
necessary).

Jan