Re: [PATCH v4 7/9] iommu/vt-d: Add trace events for domain map/unmap

2019-06-11 Thread Lu Baolu

Hi,

On 6/11/19 12:08 AM, Konrad Rzeszutek Wilk wrote:

On Mon, Jun 03, 2019 at 09:16:18AM +0800, Lu Baolu wrote:

This adds trace support for the Intel IOMMU driver. It
also declares some events which could be used to trace
the events when an IOVA is being mapped or unmapped in
a domain.


Is that even needed considering SWIOTLB also has tracing events?



Currently there isn't any trace point in swiotlb_tbl_map_single().
If we want to add trace point there, I hope we can distinguish the
bounce page events from other use cases (such as bounce buffer for
direct dma), so that we could calculate how many percents of DMA buffers
used by a specific device driver needs to use bounce page.

Best regards,
Baolu


Re: [PATCH v4 7/9] iommu/vt-d: Add trace events for domain map/unmap

2019-06-10 Thread Konrad Rzeszutek Wilk
On Mon, Jun 03, 2019 at 09:16:18AM +0800, Lu Baolu wrote:
> This adds trace support for the Intel IOMMU driver. It
> also declares some events which could be used to trace
> the events when an IOVA is being mapped or unmapped in
> a domain.

Is that even needed considering SWIOTLB also has tracing events?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 7/9] iommu/vt-d: Add trace events for domain map/unmap

2019-06-05 Thread Lu Baolu

Hi Steve,

On 6/4/19 5:01 PM, Steven Rostedt wrote:

On Mon,  3 Jun 2019 09:16:18 +0800
Lu Baolu  wrote:



+TRACE_EVENT(bounce_unmap_single,
+   TP_PROTO(struct device *dev, dma_addr_t dev_addr, size_t size),
+
+   TP_ARGS(dev, dev_addr, size),
+
+   TP_STRUCT__entry(
+   __string(dev_name, dev_name(dev))
+   __field(dma_addr_t, dev_addr)
+   __field(size_t, size)
+   ),
+
+   TP_fast_assign(
+   __assign_str(dev_name, dev_name(dev));
+   __entry->dev_addr = dev_addr;
+   __entry->size = size;
+   ),
+
+   TP_printk("dev=%s dev_addr=0x%llx size=%zu",
+ __get_str(dev_name),
+ (unsigned long long)__entry->dev_addr,
+ __entry->size)
+);
+
+TRACE_EVENT(bounce_map_sg,
+   TP_PROTO(struct device *dev, unsigned int i, unsigned int nelems,
+dma_addr_t dev_addr, phys_addr_t phys_addr, size_t size),
+
+   TP_ARGS(dev, i, nelems, dev_addr, phys_addr, size),
+
+   TP_STRUCT__entry(
+   __string(dev_name, dev_name(dev))
+   __field(unsigned int, i)
+   __field(unsigned int, last)
+   __field(dma_addr_t, dev_addr)
+   __field(phys_addr_t, phys_addr)
+   __field(size_t, size)
+   ),
+
+   TP_fast_assign(
+   __assign_str(dev_name, dev_name(dev));
+   __entry->i = i;
+   __entry->last = nelems - 1;
+   __entry->dev_addr = dev_addr;
+   __entry->phys_addr = phys_addr;
+   __entry->size = size;
+   ),
+
+   TP_printk("dev=%s elem=%u/%u dev_addr=0x%llx phys_addr=0x%llx size=%zu",
+ __get_str(dev_name), __entry->i, __entry->last,
+ (unsigned long long)__entry->dev_addr,
+ (unsigned long long)__entry->phys_addr,
+ __entry->size)
+);
+
+TRACE_EVENT(bounce_unmap_sg,
+   TP_PROTO(struct device *dev, unsigned int i, unsigned int nelems,
+dma_addr_t dev_addr, phys_addr_t phys_addr, size_t size),
+
+   TP_ARGS(dev, i, nelems, dev_addr, phys_addr, size),
+
+   TP_STRUCT__entry(
+   __string(dev_name, dev_name(dev))
+   __field(unsigned int, i)
+   __field(unsigned int, last)
+   __field(dma_addr_t, dev_addr)
+   __field(phys_addr_t, phys_addr)
+   __field(size_t, size)
+   ),
+
+   TP_fast_assign(
+   __assign_str(dev_name, dev_name(dev));
+   __entry->i = i;
+   __entry->last = nelems - 1;
+   __entry->dev_addr = dev_addr;
+   __entry->phys_addr = phys_addr;
+   __entry->size = size;
+   ),
+
+   TP_printk("dev=%s elem=%u/%u dev_addr=0x%llx phys_addr=0x%llx size=%zu",
+ __get_str(dev_name), __entry->i, __entry->last,
+ (unsigned long long)__entry->dev_addr,
+ (unsigned long long)__entry->phys_addr,
+ __entry->size)
+);


These last two events look identical. Please use the
DECLARE_EVENT_CLASS() to describe the event and then DEFINE_EVENT() for
the two events.

Each TRACE_EVENT() can add up to 5k of data/text, where as a
DEFINE_EVENT() just adds around 250 bytes.

(Note, a TRACE_EVENT() is defined as a
DECLARE_EVENT_CLASS()/DEFINE_EVENT() pair)


Thanks for reviewing. I will rework this patch according to your
comments here.



-- Steve



Best regards,
Baolu




+#endif /* _TRACE_INTEL_IOMMU_H */
+
+/* This part must be outside protection */
+#include 




___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 7/9] iommu/vt-d: Add trace events for domain map/unmap

2019-06-04 Thread Steven Rostedt
On Mon,  3 Jun 2019 09:16:18 +0800
Lu Baolu  wrote:


> +TRACE_EVENT(bounce_unmap_single,
> + TP_PROTO(struct device *dev, dma_addr_t dev_addr, size_t size),
> +
> + TP_ARGS(dev, dev_addr, size),
> +
> + TP_STRUCT__entry(
> + __string(dev_name, dev_name(dev))
> + __field(dma_addr_t, dev_addr)
> + __field(size_t, size)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(dev_name, dev_name(dev));
> + __entry->dev_addr = dev_addr;
> + __entry->size = size;
> + ),
> +
> + TP_printk("dev=%s dev_addr=0x%llx size=%zu",
> +   __get_str(dev_name),
> +   (unsigned long long)__entry->dev_addr,
> +   __entry->size)
> +);
> +
> +TRACE_EVENT(bounce_map_sg,
> + TP_PROTO(struct device *dev, unsigned int i, unsigned int nelems,
> +  dma_addr_t dev_addr, phys_addr_t phys_addr, size_t size),
> +
> + TP_ARGS(dev, i, nelems, dev_addr, phys_addr, size),
> +
> + TP_STRUCT__entry(
> + __string(dev_name, dev_name(dev))
> + __field(unsigned int, i)
> + __field(unsigned int, last)
> + __field(dma_addr_t, dev_addr)
> + __field(phys_addr_t, phys_addr)
> + __field(size_t, size)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(dev_name, dev_name(dev));
> + __entry->i = i;
> + __entry->last = nelems - 1;
> + __entry->dev_addr = dev_addr;
> + __entry->phys_addr = phys_addr;
> + __entry->size = size;
> + ),
> +
> + TP_printk("dev=%s elem=%u/%u dev_addr=0x%llx phys_addr=0x%llx size=%zu",
> +   __get_str(dev_name), __entry->i, __entry->last,
> +   (unsigned long long)__entry->dev_addr,
> +   (unsigned long long)__entry->phys_addr,
> +   __entry->size)
> +);
> +
> +TRACE_EVENT(bounce_unmap_sg,
> + TP_PROTO(struct device *dev, unsigned int i, unsigned int nelems,
> +  dma_addr_t dev_addr, phys_addr_t phys_addr, size_t size),
> +
> + TP_ARGS(dev, i, nelems, dev_addr, phys_addr, size),
> +
> + TP_STRUCT__entry(
> + __string(dev_name, dev_name(dev))
> + __field(unsigned int, i)
> + __field(unsigned int, last)
> + __field(dma_addr_t, dev_addr)
> + __field(phys_addr_t, phys_addr)
> + __field(size_t, size)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(dev_name, dev_name(dev));
> + __entry->i = i;
> + __entry->last = nelems - 1;
> + __entry->dev_addr = dev_addr;
> + __entry->phys_addr = phys_addr;
> + __entry->size = size;
> + ),
> +
> + TP_printk("dev=%s elem=%u/%u dev_addr=0x%llx phys_addr=0x%llx size=%zu",
> +   __get_str(dev_name), __entry->i, __entry->last,
> +   (unsigned long long)__entry->dev_addr,
> +   (unsigned long long)__entry->phys_addr,
> +   __entry->size)
> +);

These last two events look identical. Please use the
DECLARE_EVENT_CLASS() to describe the event and then DEFINE_EVENT() for
the two events.

Each TRACE_EVENT() can add up to 5k of data/text, where as a
DEFINE_EVENT() just adds around 250 bytes.

(Note, a TRACE_EVENT() is defined as a
DECLARE_EVENT_CLASS()/DEFINE_EVENT() pair)

-- Steve


> +#endif /* _TRACE_INTEL_IOMMU_H */
> +
> +/* This part must be outside protection */
> +#include 

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v4 7/9] iommu/vt-d: Add trace events for domain map/unmap

2019-06-02 Thread Lu Baolu
This adds trace support for the Intel IOMMU driver. It
also declares some events which could be used to trace
the events when an IOVA is being mapped or unmapped in
a domain.

Cc: Ashok Raj 
Cc: Jacob Pan 
Cc: Kevin Tian 
Signed-off-by: Mika Westerberg 
Signed-off-by: Lu Baolu 
---
 drivers/iommu/Makefile |   1 +
 drivers/iommu/intel-trace.c|  14 +++
 include/trace/events/intel_iommu.h | 132 +
 3 files changed, 147 insertions(+)
 create mode 100644 drivers/iommu/intel-trace.c
 create mode 100644 include/trace/events/intel_iommu.h

diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 8c71a15e986b..8b5fb8051281 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
 obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
 obj-$(CONFIG_DMAR_TABLE) += dmar.o
 obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
+obj-$(CONFIG_INTEL_IOMMU) += intel-trace.o
 obj-$(CONFIG_INTEL_IOMMU_DEBUGFS) += intel-iommu-debugfs.o
 obj-$(CONFIG_INTEL_IOMMU_SVM) += intel-svm.o
 obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o
diff --git a/drivers/iommu/intel-trace.c b/drivers/iommu/intel-trace.c
new file mode 100644
index ..bfb6a6e37a88
--- /dev/null
+++ b/drivers/iommu/intel-trace.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Intel IOMMU trace support
+ *
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Lu Baolu 
+ */
+
+#include 
+#include 
+
+#define CREATE_TRACE_POINTS
+#include 
diff --git a/include/trace/events/intel_iommu.h 
b/include/trace/events/intel_iommu.h
new file mode 100644
index ..49ca57a90079
--- /dev/null
+++ b/include/trace/events/intel_iommu.h
@@ -0,0 +1,132 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Intel IOMMU trace support
+ *
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Lu Baolu 
+ */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM intel_iommu
+
+#if !defined(_TRACE_INTEL_IOMMU_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_INTEL_IOMMU_H
+
+#include 
+#include 
+
+TRACE_EVENT(bounce_map_single,
+   TP_PROTO(struct device *dev, dma_addr_t dev_addr, phys_addr_t phys_addr,
+size_t size),
+
+   TP_ARGS(dev, dev_addr, phys_addr, size),
+
+   TP_STRUCT__entry(
+   __string(dev_name, dev_name(dev))
+   __field(dma_addr_t, dev_addr)
+   __field(phys_addr_t, phys_addr)
+   __field(size_t, size)
+   ),
+
+   TP_fast_assign(
+   __assign_str(dev_name, dev_name(dev));
+   __entry->dev_addr = dev_addr;
+   __entry->phys_addr = phys_addr;
+   __entry->size = size;
+   ),
+
+   TP_printk("dev=%s dev_addr=0x%llx phys_addr=0x%llx size=%zu",
+ __get_str(dev_name),
+ (unsigned long long)__entry->dev_addr,
+ (unsigned long long)__entry->phys_addr,
+ __entry->size)
+);
+
+TRACE_EVENT(bounce_unmap_single,
+   TP_PROTO(struct device *dev, dma_addr_t dev_addr, size_t size),
+
+   TP_ARGS(dev, dev_addr, size),
+
+   TP_STRUCT__entry(
+   __string(dev_name, dev_name(dev))
+   __field(dma_addr_t, dev_addr)
+   __field(size_t, size)
+   ),
+
+   TP_fast_assign(
+   __assign_str(dev_name, dev_name(dev));
+   __entry->dev_addr = dev_addr;
+   __entry->size = size;
+   ),
+
+   TP_printk("dev=%s dev_addr=0x%llx size=%zu",
+ __get_str(dev_name),
+ (unsigned long long)__entry->dev_addr,
+ __entry->size)
+);
+
+TRACE_EVENT(bounce_map_sg,
+   TP_PROTO(struct device *dev, unsigned int i, unsigned int nelems,
+dma_addr_t dev_addr, phys_addr_t phys_addr, size_t size),
+
+   TP_ARGS(dev, i, nelems, dev_addr, phys_addr, size),
+
+   TP_STRUCT__entry(
+   __string(dev_name, dev_name(dev))
+   __field(unsigned int, i)
+   __field(unsigned int, last)
+   __field(dma_addr_t, dev_addr)
+   __field(phys_addr_t, phys_addr)
+   __field(size_t, size)
+   ),
+
+   TP_fast_assign(
+   __assign_str(dev_name, dev_name(dev));
+   __entry->i = i;
+   __entry->last = nelems - 1;
+   __entry->dev_addr = dev_addr;
+   __entry->phys_addr = phys_addr;
+   __entry->size = size;
+   ),
+
+   TP_printk("dev=%s elem=%u/%u dev_addr=0x%llx phys_addr=0x%llx size=%zu",
+ __get_str(dev_name), __entry->i, __entry->last,
+ (unsigned long long)__entry->dev_addr,
+ (unsigned long long)__entry->phys_addr,
+ __entry->size)
+);
+
+TRACE_EVENT(bounce_unmap_sg,
+   TP_PROTO(struct device *dev, unsigned int i, unsigned int nelems,
+dma_addr_t dev_addr, phys_addr_t phys_addr, size_t size),
+