[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On 09/06/16 13:15, Jan Beulich wrote:
On 09.06.16 at 14:08, <julien.grall@xxxxxxx> wrote:


On 09/06/16 13:03, Julien Grall wrote:
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

I missed the bit "other dependent one". Which series are you talking
about? The cover letter does not give any dependent series...

"[Patch v11 0/3] VT-d Device-TLB flush issue"

To be honest, I am usually not going through x86 patch. In this case, the only call to domain_crash I can spot is in patch #2 which is Intel VTD specific.

So the behavior of iommu_iotlb_flush is up to the IOMMU driver. Whilst the behavior of iommu_{,un}map_page are defined by the common code.

This looks odd to me. If this is expected, then this needs to be documented somewhere.

Regards,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.