Re: [PATCH v7 4/6] iommu: don't domain_crash() inside iommu_map/unmap_page()

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Roger Pau Monne
> Sent: 13 September 2018 09:22
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Kevin Tian <kevin.tian@intel.com>;
> Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wei.liu2@citrix.com>;
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; George Dunlap
> <George.Dunlap@citrix.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; Julien Grall <julien.grall@arm.com>; Jun Nakajima
> <jun.nakajima@intel.com>
> Subject: Re: [Xen-devel] [PATCH v7 4/6] iommu: don't domain_crash() inside
> iommu_map/unmap_page()
> 
> On Wed, Sep 12, 2018 at 12:30:26PM +0100, Paul Durrant wrote:
> > This patch removes the implicit domain_crash() from iommu_map(),
> > unmap_page() and iommu_iotlb_flush() and turns them into
> straightforward
> > wrappers that check the existence of the relevant iommu_op and call
> > through to it. This makes them usable by PV IOMMU code to be delivered
> in
> > future patches.
> > This patch adds a helper macro, domu_crash(), that will only invoke
> > domain_crash() if the domain is not the hardware domain and modifies
> > callers of iommu_map(), unmap_page() and iommu_iotlb_flush() to use
> this
> > should an operation fail.
> 
> I assume this is because future usages of iommu_map, unmap and flush
> will tolerate failure, and will be handled differently than crashing
> the domain.
> 

Correct. If they form part of the implementation of PV-IOMMU then there's no way we can allow guest supplied arguments to directly cause a domain crash.

> > NOTE: This patch includes one bit of clean-up in set_identity_p2m_entry()
> >       replacing use of p2m->domain with the domain pointer passed into the
> >       function.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 

Thanks,

  Paul

> > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> > index 0ba80cb1a8..f2c594d197 100644
> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -616,6 +616,11 @@ void __domain_crash(struct domain *d);
> >      __domain_crash(d);                                                    \
> >  } while (0)
> >
> > +#define domu_crash(d) do {                \
> > +    if ( !is_hardware_domain(d) )         \
> > +        domain_crash(d);                  \
> > +} while (false)
>             ^ missing spaces
> > +
> >  /*
> >   * Called from assembly code, with an optional address to help indicate
> why
> >   * the crash occured.  If addr is 0, look up address from last extable
> > --
> > 2.11.0
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xenproject.org
> > https://lists.xenproject.org/mailman/listinfo/xen-devel

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