Re: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash() inside iommu_map/unmap_page()

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

Re-cc’ing xen-devel...

> On Oct 5, 2018, at 11:34 AM, George Dunlap <george.dunlap@xxxxxxxxxx> wrote:
> 
> 
> 
>> On Oct 5, 2018, at 11:27 AM, Paul Durrant <Paul.Durrant@xxxxxxxxxx> wrote:
>> 
>>> -----Original Message-----
>>> From: George Dunlap
>>> Sent: 05 October 2018 11:25
>>> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
>>> Subject: Re: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
>>> inside iommu_map/unmap_page()
>>> 
>>> [Sorry, my mail client crashed and I can’t figure out how to make it re-
>>> edit this draft, so I’m replying to it instead.]
>>> 
>>>> On Oct 5, 2018, at 11:22 AM, George Dunlap <George.Dunlap@xxxxxxxxxx>
>>> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On Oct 5, 2018, at 10:02 AM, Paul Durrant <Paul.Durrant@xxxxxxxxxx>
>>> wrote:
>>>>> 
>>>>>> -----Original Message-----
>>>>>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>>>>>> Sent: 05 October 2018 08:33
>>>>>> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
>>>>>> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
>>>>>> <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei
>>> Liu
>>>>>> <wei.liu2@xxxxxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>; Stefano
>>>>>> Stabellini <sstabellini@xxxxxxxxxx>; xen-devel <xen-
>>>>>> devel@xxxxxxxxxxxxxxxxxxxx>; Konrad Rzeszutek Wilk
>>>>>> <konrad.wilk@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>
>>>>>> Subject: Re: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
>>>>>> inside iommu_map/unmap_page()
>>>>>> 
>>>>>>>>> On 04.10.18 at 18:36, <Paul.Durrant@xxxxxxxxxx> wrote:
>>>>>>> I still think an implicit domain_crash() doesn't really belong in
>>>>>> something
>>>>>>> that looks like a straightforward wrapper around a per-implementation
>>>>>> jump
>>>>>>> table. How about iommu_map/unmap_may_crash() instead to highlight the
>>>>>>> semantic?
>>>>>> 
>>>>>> If anything then the other way around, i.e. iommu_unmap_no_crash(),
>>>>>> such that only callers who explicitly mean to deal with the crashing
>>>>>> themselves would use the otherwise insecure variant.
>>>>>> 
>>>>> 
>>>>> Ok. George, what is your preference?
>>>>> 
>>>>> At this point my proposal is to drop this patch and replace it with one
>>> that removes the implicit crash from from everything except the unmap. I
>>> can then introduce a 'nocrash' variant of unmap if I need it... although
>>> I'm no longer convinced I can really do anything else if a PV-IOMMU unmap
>>> fails.
>>>> 
>>>> Sorry, ‘mayfail’ was meant to be short for “may fail [without crashing
>>> the guest]”; as opposed to “must succeed [or crash the guest]”.  IOW, I
>>> agree with Jan that the default should be to crash the guest unless the
>>> caller explicitly opts to handle the failure themselves.  Don’t have a
>>> strong opinion on the name.
>> 
>> But for mapping too? It seems unnecessary to crash the domain in that case.
> 
> ISTR that the domain_crash() was added only a few years ago; I’d have to go 
> back and see the reasoning for it being added in the first place.  I’ll do 
> that Monday if Jan doesn’t beat me to it.
> 
> -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel