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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: George Dunlap
> Sent: 05 October 2018 11:35
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH v14 4/9] iommu: don't domain_crash()
> inside iommu_map/unmap_page()
> 
> 
> 
> > 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.
> 

I was added by the following commit:

commit 834c97baebb3743c54bcae228e984ae1b9692e6a
Author: Quan Xu <quan.xu@xxxxxxxxx>
Date:   Tue Jun 14 15:10:57 2016 +0200

    IOMMU: handle IOMMU mapping and unmapping failures

    Treat IOMMU mapping and unmapping failures as a fatal to the DomU
    If IOMMU mapping and unmapping failed, crash the DomU and propagate
    the error up to the call trees.

    No spamming of the log can occur. For DomU, we avoid logging any
    message for already dying domains. For Dom0, that'll still be more
    verbose than we'd really like, but it at least wouldn't outright
    flood the console.

    Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx>
    Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

So the justification appears to be to avoid log spam.

  Paul

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