Re: [Xen-devel] [PATCH 5/7] public / x86: introduce __HYPERCALL_iommu_op

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Paul Durrant [mailto:Paul.Durrant@xxxxxxxxxx]
> Sent: Tuesday, February 13, 2018 5:23 PM
> 
> > -----Original Message-----
> > From: Tian, Kevin [mailto:kevin.tian@xxxxxxxxx]
> > Sent: 13 February 2018 06:43
> > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Wei Liu
> > <wei.liu2@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>;
> > Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Ian Jackson
> > <Ian.Jackson@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>; Jan Beulich
> > <jbeulich@xxxxxxxx>; Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> > Subject: RE: [Xen-devel] [PATCH 5/7] public / x86: introduce
> > __HYPERCALL_iommu_op
> >
> > > From: Paul Durrant
> > > Sent: Monday, February 12, 2018 6:47 PM
> > >
> > > This patch introduces the boilerplate for a new hypercall to allow a
> > > domain to control IOMMU mappings for its own pages.
> > > Whilst there is duplication of code between the native and compat
> entry
> > > points which appears ripe for some form of combination, I think it is
> > > better to maintain the separation as-is because the compat entry point
> > > will necessarily gain complexity in subsequent patches.
> > >
> > > NOTE: This hypercall is only implemented for x86 and is currently
> > >       restricted by XSM to dom0 since it could be used to cause IOMMU
> > >       faults which may bring down a host.
> > >
> > > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > [...]
> > > +
> > > +
> > > +static bool can_control_iommu(void)
> > > +{
> > > +    struct domain *currd = current->domain;
> > > +
> > > +    /*
> > > +     * IOMMU mappings cannot be manipulated if:
> > > +     * - the IOMMU is not enabled or,
> > > +     * - the IOMMU is passed through or,
> > > +     * - shared EPT configured or,
> > > +     * - Xen is maintaining an identity map.
> >
> > "for dom0"
> >
> > > +     */
> > > +    if ( !iommu_enabled || iommu_passthrough ||
> > > +         iommu_use_hap_pt(currd) || need_iommu(currd) )
> >
> > I guess it's clearer to directly check iommu_dom0_strict here
> 
> Well, the problem with that is that it totally ties this interface to dom0.
> Whilst, in practice, that is the case at the moment (because of the xsm
> check) I do want to leave the potential to allow other PV domains to control
> their IOMMU mappings, if that make sense in future.
> 

first it's inconsistent from the comments - "Xen is maintaining
an identity map" which only applies to dom0.

second I'm afraid !need_iommu is not an accurate condition to represent
PV domain. what about iommu also enabled for future PV domains?

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