* [PATCH] iommu/vt-d: Remove unnecassary qi clflushes @ 2016-06-24 13:13 Nadav Amit [not found] ` <1466773994-242872-1-git-send-email-namit-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Nadav Amit @ 2016-06-24 13:13 UTC (permalink / raw) To: joro-zLv9SwRftAIdnm+yROfE0A Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Nadav Amit According to the manual: "Hardware access to ... invalidation queue ... are always coherent." Remove unnecassary clflushes accordingly. Signed-off-by: Nadav Amit <namit-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org> --- Build-tested since I do not have an IOMMU that does not support coherency. --- drivers/iommu/dmar.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c index 6a86b5d..ca56ec3 100644 --- a/drivers/iommu/dmar.c +++ b/drivers/iommu/dmar.c @@ -1155,8 +1155,6 @@ static int qi_check_fault(struct intel_iommu *iommu, int index) (unsigned long long)qi->desc[index].high); memcpy(&qi->desc[index], &qi->desc[wait_index], sizeof(struct qi_desc)); - __iommu_flush_cache(iommu, &qi->desc[index], - sizeof(struct qi_desc)); writel(DMA_FSTS_IQE, iommu->reg + DMAR_FSTS_REG); return -EINVAL; } @@ -1231,9 +1229,6 @@ restart: hw[wait_index] = wait_desc; - __iommu_flush_cache(iommu, &hw[index], sizeof(struct qi_desc)); - __iommu_flush_cache(iommu, &hw[wait_index], sizeof(struct qi_desc)); - qi->free_head = (qi->free_head + 2) % QI_LENGTH; qi->free_cnt -= 2; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
[parent not found: <1466773994-242872-1-git-send-email-namit-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH] iommu/vt-d: Remove unnecassary qi clflushes [not found] ` <1466773994-242872-1-git-send-email-namit-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org> @ 2016-06-27 11:30 ` Joerg Roedel [not found] ` <20160627113014.GL11432-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> 2016-07-13 10:07 ` Joerg Roedel 1 sibling, 1 reply; 8+ messages in thread From: Joerg Roedel @ 2016-06-27 11:30 UTC (permalink / raw) To: Nadav Amit; +Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA On Fri, Jun 24, 2016 at 06:13:14AM -0700, Nadav Amit wrote: > According to the manual: "Hardware access to ... invalidation queue ... > are always coherent." > > Remove unnecassary clflushes accordingly. It is one thing what the spec says and another how hardware really behaves. Have you tested this on (potentially really old) VT-d machines to make sure the spec is _always_ right here? Joerg ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20160627113014.GL11432-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH] iommu/vt-d: Remove unnecassary qi clflushes [not found] ` <20160627113014.GL11432-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> @ 2016-06-27 16:17 ` Nadav Amit [not found] ` <C78095BC-3BED-48DE-90CE-601DE40AA613-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Nadav Amit @ 2016-06-27 16:17 UTC (permalink / raw) To: Joerg Roedel Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Nadav Amit Joerg Roedel <joro@8bytes.org> wrote: > On Fri, Jun 24, 2016 at 06:13:14AM -0700, Nadav Amit wrote: >> According to the manual: "Hardware access to ... invalidation queue ... >> are always coherent." >> >> Remove unnecassary clflushes accordingly. > > It is one thing what the spec says and another how hardware really > behaves. Have you tested this on (potentially really old) VT-d machines > to make sure the spec is _always_ right here? No I didn’t as the commit message says. I would be happy for someones’ tested-by. Having said that - FreeBSD does not do these (unnecessary) invalidations [1], and their code comment clearly says it. Since this code is not new, I would assume FreeBSD would crash by now if the code was buggy. Although such hardware is old, there are some hypervisors that do not set the ecap.coherency of emulated IOMMUs. Yes, it is unwise, but there is no reason to further punish these hypervisors. Thanks, Nadav [1] http://web.mit.edu/freebsd/head/sys/x86/iommu/intel_qi.c _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <C78095BC-3BED-48DE-90CE-601DE40AA613-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] iommu/vt-d: Remove unnecassary qi clflushes [not found] ` <C78095BC-3BED-48DE-90CE-601DE40AA613-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2016-07-05 16:27 ` Nadav Amit [not found] ` <926A8CFB-D22D-4DC6-9EE0-D912451365BB-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Nadav Amit @ 2016-07-05 16:27 UTC (permalink / raw) To: Joerg Roedel Cc: Paolo Bonzini, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Nadav Amit Nadav Amit <nadav.amit@gmail.com> wrote: > Joerg Roedel <joro@8bytes.org> wrote: > >> On Fri, Jun 24, 2016 at 06:13:14AM -0700, Nadav Amit wrote: >>> According to the manual: "Hardware access to ... invalidation queue ... >>> are always coherent." >>> >>> Remove unnecassary clflushes accordingly. >> >> It is one thing what the spec says and another how hardware really >> behaves. Have you tested this on (potentially really old) VT-d machines >> to make sure the spec is _always_ right here? > > No I didn’t as the commit message says. I would be happy for someones’ > tested-by. > > Having said that - FreeBSD does not do these (unnecessary) > invalidations [1], and their code comment clearly says it. Since this code > is not new, I would assume FreeBSD would crash by now if the code was > buggy. > > Although such hardware is old, there are some hypervisors that do not set > the ecap.coherency of emulated IOMMUs. Yes, it is unwise, but there is no > reason to further punish these hypervisors. Ping? To clarify - this behavior applies to the guest of VMware and KVM (which uses QEMU for IOMMU emulation). Regardless, please respond to the other patch I sent as well. Thanks, Nadav _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <926A8CFB-D22D-4DC6-9EE0-D912451365BB-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] iommu/vt-d: Remove unnecassary qi clflushes [not found] ` <926A8CFB-D22D-4DC6-9EE0-D912451365BB-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2016-07-05 16:40 ` Paolo Bonzini [not found] ` <ac3ff8d4-e47b-e4d8-40ed-02811732526a-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Paolo Bonzini @ 2016-07-05 16:40 UTC (permalink / raw) To: Nadav Amit, Joerg Roedel Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Nadav Amit On 05/07/2016 18:27, Nadav Amit wrote: >> Although such hardware is old, there are some hypervisors that do not set >> the ecap.coherency of emulated IOMMUs. Yes, it is unwise, but there is no >> reason to further punish these hypervisors. QEMU will need the kernel to respect ecap.coherency in order to support nested VFIO, for example. Paolo ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <ac3ff8d4-e47b-e4d8-40ed-02811732526a-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH] iommu/vt-d: Remove unnecassary qi clflushes [not found] ` <ac3ff8d4-e47b-e4d8-40ed-02811732526a-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2016-07-05 16:50 ` Nadav Amit [not found] ` <5F48A3E6-99EE-4DC5-BF38-4F0338E9E8D0-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Nadav Amit @ 2016-07-05 16:50 UTC (permalink / raw) To: Paolo Bonzini Cc: Nadav Amit, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Paolo Bonzini <pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > > > On 05/07/2016 18:27, Nadav Amit wrote: >>> Although such hardware is old, there are some hypervisors that do not set >>> the ecap.coherency of emulated IOMMUs. Yes, it is unwise, but there is no >>> reason to further punish these hypervisors. > > QEMU will need the kernel to respect ecap.coherency in order to support > nested VFIO, for example. To clarify - the kernel respects the coherency, but performs more clflushes than necessary. It has no functional impact, but induces performance degradation (which I did not measure, but is likely to be several hundreds of cycles per flush). Regards, Nadav ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <5F48A3E6-99EE-4DC5-BF38-4F0338E9E8D0-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] iommu/vt-d: Remove unnecassary qi clflushes [not found] ` <5F48A3E6-99EE-4DC5-BF38-4F0338E9E8D0-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2016-07-05 16:57 ` Paolo Bonzini 0 siblings, 0 replies; 8+ messages in thread From: Paolo Bonzini @ 2016-07-05 16:57 UTC (permalink / raw) To: Nadav Amit; +Cc: Nadav Amit, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA On 05/07/2016 18:50, Nadav Amit wrote: > Paolo Bonzini <pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > >> >> >> On 05/07/2016 18:27, Nadav Amit wrote: >>>> Although such hardware is old, there are some hypervisors that do not set >>>> the ecap.coherency of emulated IOMMUs. Yes, it is unwise, but there is no >>>> reason to further punish these hypervisors. >> >> QEMU will need the kernel to respect ecap.coherency in order to support >> nested VFIO, for example. > > To clarify - the kernel respects the coherency, but performs more clflushes > than necessary. It has no functional impact, but induces performance > degradation (which I did not measure, but is likely to be several hundreds > of cycles per flush). Oh, then QEMU indeed doesn't care about flushing the invalidation queue. Paolo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/vt-d: Remove unnecassary qi clflushes [not found] ` <1466773994-242872-1-git-send-email-namit-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org> 2016-06-27 11:30 ` Joerg Roedel @ 2016-07-13 10:07 ` Joerg Roedel 1 sibling, 0 replies; 8+ messages in thread From: Joerg Roedel @ 2016-07-13 10:07 UTC (permalink / raw) To: Nadav Amit; +Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA On Fri, Jun 24, 2016 at 06:13:14AM -0700, Nadav Amit wrote: > According to the manual: "Hardware access to ... invalidation queue ... > are always coherent." > > Remove unnecassary clflushes accordingly. > > Signed-off-by: Nadav Amit <namit-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org> Applied, thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-07-13 10:07 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-24 13:13 [PATCH] iommu/vt-d: Remove unnecassary qi clflushes Nadav Amit [not found] ` <1466773994-242872-1-git-send-email-namit-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org> 2016-06-27 11:30 ` Joerg Roedel [not found] ` <20160627113014.GL11432-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> 2016-06-27 16:17 ` Nadav Amit [not found] ` <C78095BC-3BED-48DE-90CE-601DE40AA613-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2016-07-05 16:27 ` Nadav Amit [not found] ` <926A8CFB-D22D-4DC6-9EE0-D912451365BB-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2016-07-05 16:40 ` Paolo Bonzini [not found] ` <ac3ff8d4-e47b-e4d8-40ed-02811732526a-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2016-07-05 16:50 ` Nadav Amit [not found] ` <5F48A3E6-99EE-4DC5-BF38-4F0338E9E8D0-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2016-07-05 16:57 ` Paolo Bonzini 2016-07-13 10:07 ` Joerg Roedel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.