Re: [Xen-devel] [PATCH v7 05/11] IOMMU/MMU: propagate IOMMU Device-TLB flush error up to iommu_iotlb_flush{, _all} (top level ones)

[Julien Grall <julien.grall@xxxxxxx>]

On 09/06/16 12:45, Jan Beulich wrote:
> > > > On 09.06.16 at 13:12, <julien.grall@xxxxxxx> wrote:
> > On 08/06/16 09:58, Xu, Quan wrote:
> > > From: Quan Xu <quan.xu@xxxxxxxxx>
> > >
> > > Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx>
> > > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > >
> > > CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > CC: Julien Grall <julien.grall@xxxxxxx>
> > > CC: Jan Beulich <jbeulich@xxxxxxxx>
> > > CC: Kevin Tian <kevin.tian@xxxxxxxxx>
> > > ---
> > >    xen/arch/arm/p2m.c                  |  4 +++-
> > >    xen/common/memory.c                 | 12 ++++++++++--
> > >    xen/drivers/passthrough/iommu.c     | 13 +++++++++----
> > >    xen/drivers/passthrough/x86/iommu.c |  5 +++--
> > >    xen/include/xen/iommu.h             |  5 +++--
> > >    5 files changed, 28 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > > index 6a19c57..65d8f1a 100644
> > > --- a/xen/arch/arm/p2m.c
> > > +++ b/xen/arch/arm/p2m.c
> > > @@ -1178,7 +1178,9 @@ out:
> > >        if ( flush )
> > >        {
> > >            flush_tlb_domain(d);
> > > -        iommu_iotlb_flush(d, sgfn, egfn - sgfn);
> > > +        ret = iommu_iotlb_flush(d, sgfn, egfn - sgfn);
> > Sorry for coming late in the discussion. What kind of error do you
> > expect to return with iommu_tlb_flush?
> >
> > Today the ARM SMMU will always return 0 if the TLB flush timeout (see
> > arm_smmu_tlb_inv_context).
> >
> > We may want in the future to return an error when it has timeout,
> > however only returning an error is not safe at all. The TLB may contain
> > entries which are invalid (because we remove the mapping earlier) and a
> > device/domain could take advantage of that.
> >
> > So I am not sure if we should let running the guest when a flush has
> > timeout. Any thoughts?
> Well, did you look at the rest of this series, and the other dependent
> one? Guests (other than Dom0) get crashed when a flush times out. I
> guess that's what you will want on ARM then too.
I have found a call to domain_crash in iommu_map_page and 
iommu_unmap_page. However, I have not found any for iommu_tlb_flush.
iommu_unmap_page and iommu_map_page are not called in the p2m code 
because the page table are shared with the SMMU. So guest will not be 
crashed if the timeout has failed.
Regards,

--
Julien Grall

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