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: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 05 October 2018 12:18
> To: George Dunlap <George.Dunlap@xxxxxxxxxx>; Paul Durrant
> <Paul.Durrant@xxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Ian Jackson
> <Ian.Jackson@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; 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 05.10.18 at 12:38, <Paul.Durrant@xxxxxxxxxx> wrote:
> >> From: George Dunlap
> >> Sent: 05 October 2018 11:35
> >>
> >> > On Oct 5, 2018, at 11:27 AM, Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> >> wrote:
> >> > 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.
> 
> Iirc that part of the description only exists because early version of
> that patch did introduce log spam.
> 
> The problem iirc is mainly proper error handling, in particular proper
> unwinding of earlier mappings that may have got installed
> successfully in the context of the same hypercall (or whatever).
> 

Ok. In the interest of making progress let's just drop this patch altogether. 
I'll add a patch to introduce a no-crash variant for map into my series 
implementing PV-IOMMU.

  Paul

> Jan

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