Re: [Xen-devel] [PATCH v4 02/10] IOMMU: handle IOMMU mapping and unmapping failures

["Xu, Quan" <quan.xu@xxxxxxxxx>]

On May 10, 2016 12:14 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >>> On 06.05.16 at 10:54, <quan.xu@xxxxxxxxx> wrote:
> > --- a/xen/drivers/passthrough/iommu.c
> > +++ b/xen/drivers/passthrough/iommu.c
> > @@ -240,21 +240,47 @@ int iommu_map_page(struct domain *d,
> unsigned long gfn, unsigned long mfn,
> >                     unsigned int flags)  {
> >      const struct domain_iommu *hd = dom_iommu(d);
> > +    int rc;
> >
> >      if ( !iommu_enabled || !hd->platform_ops )
> >          return 0;
> >
> > -    return hd->platform_ops->map_page(d, gfn, mfn, flags);
> > +    rc = hd->platform_ops->map_page(d, gfn, mfn, flags);
> > +
> > +    if ( unlikely(rc) )
> > +    {
> > +        printk(XENLOG_ERR
> > +               "iommu_map_page: IOMMU mapping gfn %#lx mfn %#lx failed for
> dom%d.",
> > +               gfn, mfn, d->domain_id);
> > +
> > +        if ( !is_hardware_domain(d) )
> > +            domain_crash(d);
> > +    }
> 
> This still may spam the console in at least the case of Dom0.

I am afraid we may need a minor trade-off. What about:

       dprintk(XENLOG_ERR, "...");

to print out in debug mode.

>  For DomU I'd
> really expect you to state in the commit message why no spamming can occur
> (of course assuming it really can't, which I'm not convinced of).
>

In this v4, I think we will still spam the console in extreme cases :(:(..

For mapping:
+                ret = iommu_map_page();
+                if ( unlikely(ret) )
+                {
+                    while ( i-- )
+                        iommu_unmap_page();
+                }

We'll  stop map against any error and unmapping the previous mappings.  The 
extreme case is error for unmapping the previous mappings.

Again -- I think dprintk is a better solution. Any suggestion?

Quan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel