[{"id":1804010,"web_url":"http://patchwork.ozlabs.org/comment/1804010/","msgid":"<8c58fcaa-dedc-e6be-44bf-2cb349ef9e38@ozlabs.ru>","list_archive_url":null,"date":"2017-11-13T23:47:12","subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","submitter":{"id":7621,"url":"http://patchwork.ozlabs.org/api/people/7621/","name":"Alexey Kardashevskiy","email":"aik@ozlabs.ru"},"content":"On 27/10/17 14:00, Alexey Kardashevskiy wrote:\n> This adds trace_map/trace_unmap tracepoints to spapr driver. Type1 already\n> uses these via the IOMMU API (iommu_map/__iommu_unmap).\n> \n> Signed-off-by: Alexey Kardashevskiy \n\nPing?\n\n\n\n> ---\n> \n> Example:\n> qemu-system-ppc-8655 [096] 724.662740: unmap: IOMMU: iova=0x000000003ffff000 size=4096 unmapped_size=4096\n> qemu-system-ppc-8656 [104] 724.970912: map: IOMMU: iova=0x0800000000000000 paddr=0x00007ffef7ff0000 size=65536\n> ---\n> drivers/vfio/vfio_iommu_spapr_tce.c | 12 ++++++++++--\n> 1 file changed, 10 insertions(+), 2 deletions(-)\n> \n> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c\n> index 63112c36ab2d..4531486c77c6 100644\n> --- a/drivers/vfio/vfio_iommu_spapr_tce.c\n> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c\n> @@ -22,6 +22,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -502,17 +503,19 @@ static int tce_iommu_clear(struct tce_container *container,\n> \t\tstruct iommu_table *tbl,\n> \t\tunsigned long entry, unsigned long pages)\n> {\n> -\tunsigned long oldhpa;\n> +\tunsigned long oldhpa, unmapped, firstentry = entry, totalpages = pages;\n> \tlong ret;\n> \tenum dma_data_direction direction;\n> \n> -\tfor ( ; pages; --pages, ++entry) {\n> +\tfor (unmapped = 0; pages; --pages, ++entry) {\n> \t\tdirection = DMA_NONE;\n> \t\toldhpa = 0;\n> \t\tret = iommu_tce_xchg(tbl, entry, &oldhpa, &direction);\n> \t\tif (ret)\n> \t\t\tcontinue;\n> \n> +\t\t++unmapped;\n> +\n> \t\tif (direction == DMA_NONE)\n> \t\t\tcontinue;\n> \n> @@ -523,6 +526,9 @@ static int tce_iommu_clear(struct tce_container *container,\n> \n> \t\ttce_iommu_unuse_page(container, oldhpa);\n> \t}\n> +\ttrace_unmap(firstentry << tbl->it_page_shift,\n> +\t\t\ttotalpages << tbl->it_page_shift,\n> +\t\t\tunmapped << tbl->it_page_shift);\n> \n> \treturn 0;\n> }\n> @@ -965,6 +971,8 @@ static long tce_iommu_ioctl(void *iommu_data,\n> \t\t\t\t\tdirection);\n> \n> \t\tiommu_flush_tce(tbl);\n> +\t\tif (!ret)\n> +\t\t\ttrace_map(param.iova, param.vaddr, param.size);\n> \n> \t\treturn ret;\n> \t}\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=kvm-ppc-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ozlabs-ru.20150623.gappssmtp.com\n\theader.i=@ozlabs-ru.20150623.gappssmtp.com\n\theader.b=\"JjKJ0MEW\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3ybS3r4FfCz9s8J\n\tfor ;\n\tTue, 14 Nov 2017 10:47:20 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753315AbdKMXrT (ORCPT );\n\tMon, 13 Nov 2017 18:47:19 -0500","from mail-pf0-f196.google.com ([209.85.192.196]:43742 \"EHLO\n\tmail-pf0-f196.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751593AbdKMXrS (ORCPT\n\t); Mon, 13 Nov 2017 18:47:18 -0500","by mail-pf0-f196.google.com with SMTP id a84so8100403pfl.0\n\tfor ; Mon, 13 Nov 2017 15:47:18 -0800 (PST)","from [192.168.10.22] (124-171-205-99.dyn.iinet.net.au.\n\t[124.171.205.99]) by smtp.googlemail.com with ESMTPSA id\n\tq13sm38245873pfi.166.2017.11.13.15.47.15\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tMon, 13 Nov 2017 15:47:17 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ozlabs-ru.20150623.gappssmtp.com; s=20150623;\n\th=subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=Hl76ccwwQRoIPBHUbPKWXUJNW75HQK9Yimn14UEprlU=;\n\tb=JjKJ0MEWjtOiPSkaulrd1o6kWU4c/vCealrSDU5aM5Fg0tc+M5n2PPbv9cgo1/H+Ey\n\taGfFZ3hjY/hC1LRhPayNjlXkfHL1Qe81Snw8/kro5WSEBhkhxJ58RU/xw9iYkC9/g2hm\n\trdJX5bK0b4WDsV27BV75nJR0EKJvC3Ty27A15vJGaVww4D7E0aJEtFgJVi+KZGekzLKX\n\t8Yh1KpcN12jCwE2qNSlhhAXxxmN7IaASzGJeEfDpQ53KYVPmvLBhqXmHU5++vNzI5zkx\n\tqHpogk5GtmHpzEcAsrcIR08cRghS5Hb9Rs6ar5YR6mO7ci8nMP/D0hJ/s9la2+qTsex1\n\tv+QQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:cc:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=Hl76ccwwQRoIPBHUbPKWXUJNW75HQK9Yimn14UEprlU=;\n\tb=G2gv1xF7glU9WxwHrtA5CNBYuC8B/D5SRH5rbU8C4yT19b41UVIk1LsvSC+rjzPDK6\n\thAuMxNg+SoTOXowYtMcDrZ1ar9prHaCUqFqv+++Z2Id5VIHCusN2iSFsOSHKorS/Z+Lk\n\t/9nW55fiNQcwAbHjKK2Ko77LdGiwJ8fKOwHYyqglW+UQAmjpkfYF/LGYJ+BqFcQxj66Q\n\tWw2zd2KcB8Q5Ng0Rcr2+gdJ5/8q5pvfzwOuQEezbKL1eZ5RNblluUCco8tCkqJatx5Qb\n\taIu1dcFMSYI4ynfxrPeZbbUB5jzLc7BAVP4vcC6iwjvUt2GEUpfsigt9gOiG8umtvC8z\n\tjE/g==","X-Gm-Message-State":"AJaThX6Th0IhPiIp6eNeHT8o/KX1dADiBVOka0LdW8UtBME6Mfs3FsXf\n\t2QO0w/xKv5KTSCm/WPiJIM5mTw==","X-Google-Smtp-Source":"AGs4zMY4GXPHgcnmy51pGJr4cJMfPEq7vZzypfCVnxVrDMvyI/WhyCfSSGOq2+QnA3l+/j2Clav2Jw==","X-Received":"by 10.99.159.25 with SMTP id g25mr6005776pge.176.1510616837935; \n\tMon, 13 Nov 2017 15:47:17 -0800 (PST)","Subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","To":"linuxppc-dev@lists.ozlabs.org","Cc":"Alex Williamson ,\n\tDavid Gibson ,\n\tkvm-ppc@vger.kernel.org, kvm@vger.kernel.org","References":"<20171027030008.17200-1-aik@ozlabs.ru>","From":"Alexey Kardashevskiy ","Message-ID":"<8c58fcaa-dedc-e6be-44bf-2cb349ef9e38@ozlabs.ru>","Date":"Tue, 14 Nov 2017 10:47:12 +1100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<20171027030008.17200-1-aik@ozlabs.ru>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-AU","Content-Transfer-Encoding":"7bit","Sender":"kvm-ppc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"kvm-ppc@vger.kernel.org"}},{"id":1806039,"web_url":"http://patchwork.ozlabs.org/comment/1806039/","msgid":"<20171116171301.6552869b@t450s.home>","list_archive_url":null,"date":"2017-11-17T00:13:01","subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","submitter":{"id":4123,"url":"http://patchwork.ozlabs.org/api/people/4123/","name":"Alex Williamson","email":"alex.williamson@redhat.com"},"content":"On Tue, 14 Nov 2017 10:47:12 +1100\nAlexey Kardashevskiy wrote:\n\n> On 27/10/17 14:00, Alexey Kardashevskiy wrote:\n> > This adds trace_map/trace_unmap tracepoints to spapr driver. Type1 already\n> > uses these via the IOMMU API (iommu_map/__iommu_unmap).\n> > \n> > Signed-off-by: Alexey Kardashevskiy \n\nIs this really legitimate to include tracepoints from a different\nsubsystem? The vfio type1 backend gets these trace points by virtue of\nit actually using the IOMMU API, it doesn't call them itself. I'm kind\nof surprised these are actually available to be called from a module.\nI suspect the way to do this is probably to define our own tracepoints\nin the vfio/spapr backend or insert tracepoints into the IOMMU layers\nthat that code calls into rather than masquerading as tracepoints from\na different subsystem. Right? Thanks,\n\nAlex\n\n> > ---\n> > \n> > Example:\n> > qemu-system-ppc-8655 [096] 724.662740: unmap: IOMMU: iova=0x000000003ffff000 size=4096 unmapped_size=4096\n> > qemu-system-ppc-8656 [104] 724.970912: map: IOMMU: iova=0x0800000000000000 paddr=0x00007ffef7ff0000 size=65536\n> > ---\n> > drivers/vfio/vfio_iommu_spapr_tce.c | 12 ++++++++++--\n> > 1 file changed, 10 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c\n> > index 63112c36ab2d..4531486c77c6 100644\n> > --- a/drivers/vfio/vfio_iommu_spapr_tce.c\n> > +++ b/drivers/vfio/vfio_iommu_spapr_tce.c\n> > @@ -22,6 +22,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> > \n> > #include \n> > #include \n> > @@ -502,17 +503,19 @@ static int tce_iommu_clear(struct tce_container *container,\n> > \t\tstruct iommu_table *tbl,\n> > \t\tunsigned long entry, unsigned long pages)\n> > {\n> > -\tunsigned long oldhpa;\n> > +\tunsigned long oldhpa, unmapped, firstentry = entry, totalpages = pages;\n> > \tlong ret;\n> > \tenum dma_data_direction direction;\n> > \n> > -\tfor ( ; pages; --pages, ++entry) {\n> > +\tfor (unmapped = 0; pages; --pages, ++entry) {\n> > \t\tdirection = DMA_NONE;\n> > \t\toldhpa = 0;\n> > \t\tret = iommu_tce_xchg(tbl, entry, &oldhpa, &direction);\n> > \t\tif (ret)\n> > \t\t\tcontinue;\n> > \n> > +\t\t++unmapped;\n> > +\n> > \t\tif (direction == DMA_NONE)\n> > \t\t\tcontinue;\n> > \n> > @@ -523,6 +526,9 @@ static int tce_iommu_clear(struct tce_container *container,\n> > \n> > \t\ttce_iommu_unuse_page(container, oldhpa);\n> > \t}\n> > +\ttrace_unmap(firstentry << tbl->it_page_shift,\n> > +\t\t\ttotalpages << tbl->it_page_shift,\n> > +\t\t\tunmapped << tbl->it_page_shift);\n> > \n> > \treturn 0;\n> > }\n> > @@ -965,6 +971,8 @@ static long tce_iommu_ioctl(void *iommu_data,\n> > \t\t\t\t\tdirection);\n> > \n> > \t\tiommu_flush_tce(tbl);\n> > +\t\tif (!ret)\n> > +\t\t\ttrace_map(param.iova, param.vaddr, param.size);\n> > \n> > \t\treturn ret;\n> > \t}\n> > \n> \n> \n\n--\nTo unsubscribe from this list: send the line \"unsubscribe kvm-ppc\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=kvm-ppc-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3ydJVC6Qcmz9s1h\n\tfor ;\n\tFri, 17 Nov 2017 11:13:07 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753572AbdKQANG (ORCPT );\n\tThu, 16 Nov 2017 19:13:06 -0500","from mx1.redhat.com ([209.132.183.28]:51048 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1753559AbdKQANF (ORCPT );\n\tThu, 16 Nov 2017 19:13:05 -0500","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id DDB3A80F7A;\n\tFri, 17 Nov 2017 00:13:04 +0000 (UTC)","from t450s.home (ovpn-116-16.phx2.redhat.com [10.3.116.16])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id B2B115C3FD;\n\tFri, 17 Nov 2017 00:13:02 +0000 (UTC)"],"Date":"Thu, 16 Nov 2017 17:13:01 -0700","From":"Alex Williamson ","To":"Alexey Kardashevskiy ","Cc":"linuxppc-dev@lists.ozlabs.org,\n\tDavid Gibson ,\n\tkvm-ppc@vger.kernel.org, kvm@vger.kernel.org,\n\tShuah Khan , ","Subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","Message-ID":"<20171116171301.6552869b@t450s.home>","In-Reply-To":"<8c58fcaa-dedc-e6be-44bf-2cb349ef9e38@ozlabs.ru>","References":"<20171027030008.17200-1-aik@ozlabs.ru>\n\t<8c58fcaa-dedc-e6be-44bf-2cb349ef9e38@ozlabs.ru>","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.27]);\n\tFri, 17 Nov 2017 00:13:05 +0000 (UTC)","Sender":"kvm-ppc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"kvm-ppc@vger.kernel.org"}},{"id":1806138,"web_url":"http://patchwork.ozlabs.org/comment/1806138/","msgid":"<52a19cfb-431f-522b-11de-c1c47f09a4c8@ozlabs.ru>","list_archive_url":null,"date":"2017-11-17T06:58:32","subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","submitter":{"id":7621,"url":"http://patchwork.ozlabs.org/api/people/7621/","name":"Alexey Kardashevskiy","email":"aik@ozlabs.ru"},"content":"On 17/11/17 11:13, Alex Williamson wrote:\n> On Tue, 14 Nov 2017 10:47:12 +1100\n> Alexey Kardashevskiy wrote:\n> \n>> On 27/10/17 14:00, Alexey Kardashevskiy wrote:\n>>> This adds trace_map/trace_unmap tracepoints to spapr driver. Type1 already\n>>> uses these via the IOMMU API (iommu_map/__iommu_unmap).\n>>>\n>>> Signed-off-by: Alexey Kardashevskiy \n> \n> Is this really legitimate to include tracepoints from a different\n> subsystem?> The vfio type1 backend gets these trace points by virtue of\n> it actually using the IOMMU API, it doesn't call them itself. I'm kind\n> of surprised these are actually available to be called from a module.\n\nThey are explicitly exported:\n\nEXPORT_TRACEPOINT_SYMBOL_GPL(map);\nEXPORT_TRACEPOINT_SYMBOL_GPL(unmap);\n\nI would think this is for things like drivers/vfio/vfio_iommu_spapr_tce.c ,\nwhy else?...\n\n\n> I suspect the way to do this is probably to define our own tracepoints\n> in the vfio/spapr backend or insert tracepoints into the IOMMU layers\n> that that code calls into rather than masquerading as tracepoints from\n> a different subsystem. Right?\n\nThis makes sense too. But it is going to be just cut-n-paste and some\nconfusion -\n/sys/kernel/debug/tracing/events/iommu/map will still be present but\nwon't work and\n/sys/kernel/debug/tracing/events/vfio/vfio_iommu_spapr_tce/map will.\n\nJudges? :)\n\n\n\n\n> Thanks,\n> \n> Alex\n> \n>>> ---\n>>>\n>>> Example:\n>>> qemu-system-ppc-8655 [096] 724.662740: unmap: IOMMU: iova=0x000000003ffff000 size=4096 unmapped_size=4096\n>>> qemu-system-ppc-8656 [104] 724.970912: map: IOMMU: iova=0x0800000000000000 paddr=0x00007ffef7ff0000 size=65536\n>>> ---\n>>> drivers/vfio/vfio_iommu_spapr_tce.c | 12 ++++++++++--\n>>> 1 file changed, 10 insertions(+), 2 deletions(-)\n>>>\n>>> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c\n>>> index 63112c36ab2d..4531486c77c6 100644\n>>> --- a/drivers/vfio/vfio_iommu_spapr_tce.c\n>>> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c\n>>> @@ -22,6 +22,7 @@\n>>> #include \n>>> #include \n>>> #include \n>>> +#include \n>>> \n>>> #include \n>>> #include \n>>> @@ -502,17 +503,19 @@ static int tce_iommu_clear(struct tce_container *container,\n>>> \t\tstruct iommu_table *tbl,\n>>> \t\tunsigned long entry, unsigned long pages)\n>>> {\n>>> -\tunsigned long oldhpa;\n>>> +\tunsigned long oldhpa, unmapped, firstentry = entry, totalpages = pages;\n>>> \tlong ret;\n>>> \tenum dma_data_direction direction;\n>>> \n>>> -\tfor ( ; pages; --pages, ++entry) {\n>>> +\tfor (unmapped = 0; pages; --pages, ++entry) {\n>>> \t\tdirection = DMA_NONE;\n>>> \t\toldhpa = 0;\n>>> \t\tret = iommu_tce_xchg(tbl, entry, &oldhpa, &direction);\n>>> \t\tif (ret)\n>>> \t\t\tcontinue;\n>>> \n>>> +\t\t++unmapped;\n>>> +\n>>> \t\tif (direction == DMA_NONE)\n>>> \t\t\tcontinue;\n>>> \n>>> @@ -523,6 +526,9 @@ static int tce_iommu_clear(struct tce_container *container,\n>>> \n>>> \t\ttce_iommu_unuse_page(container, oldhpa);\n>>> \t}\n>>> +\ttrace_unmap(firstentry << tbl->it_page_shift,\n>>> +\t\t\ttotalpages << tbl->it_page_shift,\n>>> +\t\t\tunmapped << tbl->it_page_shift);\n>>> \n>>> \treturn 0;\n>>> }\n>>> @@ -965,6 +971,8 @@ static long tce_iommu_ioctl(void *iommu_data,\n>>> \t\t\t\t\tdirection);\n>>> \n>>> \t\tiommu_flush_tce(tbl);\n>>> +\t\tif (!ret)\n>>> +\t\t\ttrace_map(param.iova, param.vaddr, param.size);\n>>> \n>>> \t\treturn ret;\n>>> \t}\n>>> \n>>\n>>\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=kvm-ppc-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ozlabs-ru.20150623.gappssmtp.com\n\theader.i=@ozlabs-ru.20150623.gappssmtp.com\n\theader.b=\"EfdcdYsp\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3ydTV96jpJz9s4s\n\tfor ;\n\tFri, 17 Nov 2017 17:58:41 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S933695AbdKQG6l (ORCPT );\n\tFri, 17 Nov 2017 01:58:41 -0500","from mail-it0-f65.google.com ([209.85.214.65]:41454 \"EHLO\n\tmail-it0-f65.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S933608AbdKQG6k (ORCPT\n\t); Fri, 17 Nov 2017 01:58:40 -0500","by mail-it0-f65.google.com with SMTP id x28so2951408ita.0\n\tfor ; Thu, 16 Nov 2017 22:58:39 -0800 (PST)","from [192.168.10.22] (124-171-65-237.dyn.iinet.net.au.\n\t[124.171.65.237]) by smtp.googlemail.com with ESMTPSA id\n\tm72sm1143541ioe.40.2017.11.16.22.58.35\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 16 Nov 2017 22:58:38 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ozlabs-ru.20150623.gappssmtp.com; s=20150623;\n\th=subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=O6MHq0ClggkK3s1Z4YgnCxnuSB4q6ouxDDTLGmaRfAE=;\n\tb=EfdcdYsp+J4G4LhttJ99SH/Jgh9Zedd10QPH0T+/LwvQ3kBAbsdMmhntXd95GTWx8r\n\tSt+2HS1tNSOpXYBOWRnh/7UuP/dOep1vfHy/VUAyPJraKauqFROEExb2IJKLMImy3FiW\n\ttqDTokCieNP9mQdGJFhjluPidOFROx9Nf3VRS5evIfpzAYjdnCD7gIu9OTpI935x7865\n\t/G6/tgZynhKEnQaBEEI5OqVB56fCffZcxpqM0xjpRjl5qAbxULhKFtZDdeuO5JzL2b4i\n\toW74JgQABmQUGxPuIKb0nxToBx3YWL3BamzDxJv6IlVUAO1EsRSn7nLqeFK1NEXGyWRn\n\tFpgw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:cc:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=O6MHq0ClggkK3s1Z4YgnCxnuSB4q6ouxDDTLGmaRfAE=;\n\tb=Bo/m01QSr4q7AiDUAPv3kEJIRk9AJFJsO+OGcjiHdcDARNjhIMv7m5Dw3KmOMH+7e3\n\t1yar51xXzweNr/9iVuNvpFfkNmZNrJmGtolSM7fm4Sw5vCOYQTrZojPljLNmNCFy3L7+\n\tdNNN2+3WV7FBEsTTntkMCPgeS+eeUz2ZEDPV3CEFhp5VKwmkDey98W2GYod3ViOLubad\n\tpPrG0FoJozyq5OCZrIjNzoTZ4qUfYIlfanmUGP88yyUNUUdnAP8bjdm8oO8uimSJfWX2\n\tTPEybXm6itZ9H4rsiGQ8RFmdNUunKb/7UqW95MgyvVYwmEaCTlI+Rfe3Wm2/uFXoLLvZ\n\tQn2A==","X-Gm-Message-State":"AJaThX7MSO2pThKO+XzB4yxydN+Vq0XlOXBR3IYGySXQbZpYuvi2v8iw\n\tnuD071WWLFvV53DDyOWslYcA2zm1","X-Google-Smtp-Source":"AGs4zMYWzGVLX8CUcQ4dSYEsvAn2GjlJbJq7KJt4huyzR5qcJYYymYnqTTuyGWoi59aZDxDuAp4Pcg==","X-Received":"by 10.36.86.10 with SMTP id o10mr458773itb.86.1510901919543;\n\tThu, 16 Nov 2017 22:58:39 -0800 (PST)","Subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","To":"Alex Williamson ","Cc":"linuxppc-dev@lists.ozlabs.org,\n\tDavid Gibson ,\n\tkvm-ppc@vger.kernel.org, kvm@vger.kernel.org,\n\tShuah Khan ,\n\tiommu@lists.linux-foundation.org, Joerg Roedel ","References":"<20171027030008.17200-1-aik@ozlabs.ru>\n\t<8c58fcaa-dedc-e6be-44bf-2cb349ef9e38@ozlabs.ru>\n\t<20171116171301.6552869b@t450s.home>","From":"Alexey Kardashevskiy ","Message-ID":"<52a19cfb-431f-522b-11de-c1c47f09a4c8@ozlabs.ru>","Date":"Fri, 17 Nov 2017 17:58:32 +1100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<20171116171301.6552869b@t450s.home>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-AU","Content-Transfer-Encoding":"7bit","Sender":"kvm-ppc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"kvm-ppc@vger.kernel.org"}},{"id":1809035,"web_url":"http://patchwork.ozlabs.org/comment/1809035/","msgid":"","list_archive_url":null,"date":"2017-11-23T04:13:37","subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","submitter":{"id":7621,"url":"http://patchwork.ozlabs.org/api/people/7621/","name":"Alexey Kardashevskiy","email":"aik@ozlabs.ru"},"content":"On 17/11/17 17:58, Alexey Kardashevskiy wrote:\n> On 17/11/17 11:13, Alex Williamson wrote:\n>> On Tue, 14 Nov 2017 10:47:12 +1100\n>> Alexey Kardashevskiy wrote:\n>>\n>>> On 27/10/17 14:00, Alexey Kardashevskiy wrote:\n>>>> This adds trace_map/trace_unmap tracepoints to spapr driver. Type1 already\n>>>> uses these via the IOMMU API (iommu_map/__iommu_unmap).\n>>>>\n>>>> Signed-off-by: Alexey Kardashevskiy \n>>\n>> Is this really legitimate to include tracepoints from a different\n>> subsystem?> The vfio type1 backend gets these trace points by virtue of\n>> it actually using the IOMMU API, it doesn't call them itself. I'm kind\n>> of surprised these are actually available to be called from a module.\n> \n> They are explicitly exported:\n> \n> EXPORT_TRACEPOINT_SYMBOL_GPL(map);\n> EXPORT_TRACEPOINT_SYMBOL_GPL(unmap);\n> \n> I would think this is for things like drivers/vfio/vfio_iommu_spapr_tce.c ,\n> why else?...\n> \n> \n>> I suspect the way to do this is probably to define our own tracepoints\n>> in the vfio/spapr backend or insert tracepoints into the IOMMU layers\n>> that that code calls into rather than masquerading as tracepoints from\n>> a different subsystem. Right?\n> \n> This makes sense too. But it is going to be just cut-n-paste and some\n> confusion -\n> /sys/kernel/debug/tracing/events/iommu/map will still be present but\n> won't work and\n> /sys/kernel/debug/tracing/events/vfio/vfio_iommu_spapr_tce/map will.\n> \n> Judges? :)\n\n\nStill nak? I discussed this locally, the conclusion was it is a matter of\ntaste and this proposal is not that disgusting. Thanks.\n\n\n> \n> \n> \n>> Thanks,\n>>\n>> Alex\n>>\n>>>> ---\n>>>>\n>>>> Example:\n>>>> qemu-system-ppc-8655 [096] 724.662740: unmap: IOMMU: iova=0x000000003ffff000 size=4096 unmapped_size=4096\n>>>> qemu-system-ppc-8656 [104] 724.970912: map: IOMMU: iova=0x0800000000000000 paddr=0x00007ffef7ff0000 size=65536\n>>>> ---\n>>>> drivers/vfio/vfio_iommu_spapr_tce.c | 12 ++++++++++--\n>>>> 1 file changed, 10 insertions(+), 2 deletions(-)\n>>>>\n>>>> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c\n>>>> index 63112c36ab2d..4531486c77c6 100644\n>>>> --- a/drivers/vfio/vfio_iommu_spapr_tce.c\n>>>> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c\n>>>> @@ -22,6 +22,7 @@\n>>>> #include \n>>>> #include \n>>>> #include \n>>>> +#include \n>>>> \n>>>> #include \n>>>> #include \n>>>> @@ -502,17 +503,19 @@ static int tce_iommu_clear(struct tce_container *container,\n>>>> \t\tstruct iommu_table *tbl,\n>>>> \t\tunsigned long entry, unsigned long pages)\n>>>> {\n>>>> -\tunsigned long oldhpa;\n>>>> +\tunsigned long oldhpa, unmapped, firstentry = entry, totalpages = pages;\n>>>> \tlong ret;\n>>>> \tenum dma_data_direction direction;\n>>>> \n>>>> -\tfor ( ; pages; --pages, ++entry) {\n>>>> +\tfor (unmapped = 0; pages; --pages, ++entry) {\n>>>> \t\tdirection = DMA_NONE;\n>>>> \t\toldhpa = 0;\n>>>> \t\tret = iommu_tce_xchg(tbl, entry, &oldhpa, &direction);\n>>>> \t\tif (ret)\n>>>> \t\t\tcontinue;\n>>>> \n>>>> +\t\t++unmapped;\n>>>> +\n>>>> \t\tif (direction == DMA_NONE)\n>>>> \t\t\tcontinue;\n>>>> \n>>>> @@ -523,6 +526,9 @@ static int tce_iommu_clear(struct tce_container *container,\n>>>> \n>>>> \t\ttce_iommu_unuse_page(container, oldhpa);\n>>>> \t}\n>>>> +\ttrace_unmap(firstentry << tbl->it_page_shift,\n>>>> +\t\t\ttotalpages << tbl->it_page_shift,\n>>>> +\t\t\tunmapped << tbl->it_page_shift);\n>>>> \n>>>> \treturn 0;\n>>>> }\n>>>> @@ -965,6 +971,8 @@ static long tce_iommu_ioctl(void *iommu_data,\n>>>> \t\t\t\t\tdirection);\n>>>> \n>>>> \t\tiommu_flush_tce(tbl);\n>>>> +\t\tif (!ret)\n>>>> +\t\t\ttrace_map(param.iova, param.vaddr, param.size);\n>>>> \n>>>> \t\treturn ret;\n>>>> \t}\n>>>> \n>>>\n>>>\n>>\n> \n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=kvm-ppc-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ozlabs-ru.20150623.gappssmtp.com\n\theader.i=@ozlabs-ru.20150623.gappssmtp.com\n\theader.b=\"F4Xfml5a\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yj5Y70C15z9t2S\n\tfor ;\n\tThu, 23 Nov 2017 15:13:46 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752148AbdKWENp (ORCPT );\n\tWed, 22 Nov 2017 23:13:45 -0500","from mail-pl0-f68.google.com ([209.85.160.68]:33636 \"EHLO\n\tmail-pl0-f68.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752142AbdKWENn (ORCPT\n\t); Wed, 22 Nov 2017 23:13:43 -0500","by mail-pl0-f68.google.com with SMTP id a12so1991298pll.0\n\tfor ; Wed, 22 Nov 2017 20:13:43 -0800 (PST)","from [10.61.2.175] ([122.99.82.10])\n\tby smtp.googlemail.com with ESMTPSA id\n\tk8sm25126076pgt.22.2017.11.22.20.13.39\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 22 Nov 2017 20:13:42 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ozlabs-ru.20150623.gappssmtp.com; s=20150623;\n\th=subject:from:to:cc:references:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=ifr2FLjRMQrlhaobRrSsMmHkz8QQ+YgfzYXzTsjI1EE=;\n\tb=F4Xfml5a64PU+pk5hvrcQBsdj6L1IgPRjBedMwMZReRN6BhB8UyevHI0WggU20tcV5\n\tQlACHCLajlAkWquRXtl4PFutiRk2glbbtwKgSRw6flFPWTHAh9DITzu+FVRFuiLQHJxR\n\tGeVuKCzmHJMUsXEi25cDczmBkhkN3kP6hXX5on/HPyUTeVUlnkVg5mjtHgogCwJnLbUs\n\tM0zNkR4QvZMnumIMLT0iSyMupbdYdvZwfNVPwbsUQNvuhVU4UgrYs2vMuMNNPKz/dB8Z\n\tvuoTF6m/EQkCbII6B2EpT9QxEF7mgDv+8rYuxGOw7AN9KWZ1yEBBSDP//E63MclLN8xr\n\tOyFw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:from:to:cc:references:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=ifr2FLjRMQrlhaobRrSsMmHkz8QQ+YgfzYXzTsjI1EE=;\n\tb=OeEFaCx+f4AY/kXG7UFf6F9KMfKX8ZAZ4TXOAYGz0LfkMHXIN8o679J00zbFJaSkIg\n\t/aWzApcJoSxDli8um4ih31W5fTxWmlj86v9ng67Z2jh5sW3bRs+fYYtH1fujCdVWtfuP\n\tf8qSjpDS4fq4g5h7NnSiEbbGwUBDxIJXJDXpXSpDb5h4b7QfMiKr/gPXE7t3xHkbHu9j\n\t002cpm5sGo5w9Q5eFKpHPH8frxFSvdcD0+vOAi7CklCguyLH0f6Z5S0PATZfca4l4e7i\n\t7fBbz5qgA+0nI/UgjiBQy4DXVc/06rhfPiUcIVTIQQcxIjzgf4lv0k1ZXoQJj2Dzj0n6\n\tBI2A==","X-Gm-Message-State":"AJaThX4uzXz7m4Bz6jvY4q3jZlWWUcrGkPQZfBqDTmzRJZdLKYEnNY2k\n\tKGZHTtP3jfSHBHopRsK5KQPoUQ==","X-Google-Smtp-Source":"AGs4zMaeEd/yaUp/JW1s/Jscw4aIf/HZ8Z8eQT11mwhJoDaAVBFdRssX/zSuwmvcIfvok8y+FBiFVA==","X-Received":"by 10.84.160.233 with SMTP id v38mr453863plg.179.1511410423301; \n\tWed, 22 Nov 2017 20:13:43 -0800 (PST)","Subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","From":"Alexey Kardashevskiy ","To":"Alex Williamson ","Cc":"linuxppc-dev@lists.ozlabs.org,\n\tDavid Gibson ,\n\tkvm-ppc@vger.kernel.org, kvm@vger.kernel.org,\n\tShuah Khan ,\n\tiommu@lists.linux-foundation.org, Joerg Roedel ","References":"<20171027030008.17200-1-aik@ozlabs.ru>\n\t<8c58fcaa-dedc-e6be-44bf-2cb349ef9e38@ozlabs.ru>\n\t<20171116171301.6552869b@t450s.home>\n\t<52a19cfb-431f-522b-11de-c1c47f09a4c8@ozlabs.ru>","Message-ID":"","Date":"Thu, 23 Nov 2017 15:13:37 +1100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<52a19cfb-431f-522b-11de-c1c47f09a4c8@ozlabs.ru>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-AU","Content-Transfer-Encoding":"7bit","Sender":"kvm-ppc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"kvm-ppc@vger.kernel.org"}},{"id":1812435,"web_url":"http://patchwork.ozlabs.org/comment/1812435/","msgid":"<20171129093244.0849fd7c@t450s.home>","list_archive_url":null,"date":"2017-11-29T16:32:44","subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","submitter":{"id":4123,"url":"http://patchwork.ozlabs.org/api/people/4123/","name":"Alex Williamson","email":"alex.williamson@redhat.com"},"content":"On Thu, 23 Nov 2017 15:13:37 +1100\nAlexey Kardashevskiy wrote:\n\n> On 17/11/17 17:58, Alexey Kardashevskiy wrote:\n> > On 17/11/17 11:13, Alex Williamson wrote: \n> >> On Tue, 14 Nov 2017 10:47:12 +1100\n> >> Alexey Kardashevskiy wrote:\n> >> \n> >>> On 27/10/17 14:00, Alexey Kardashevskiy wrote: \n> >>>> This adds trace_map/trace_unmap tracepoints to spapr driver. Type1 already\n> >>>> uses these via the IOMMU API (iommu_map/__iommu_unmap).\n> >>>>\n> >>>> Signed-off-by: Alexey Kardashevskiy \n> >>\n> >> Is this really legitimate to include tracepoints from a different \n> >> subsystem?> The vfio type1 backend gets these trace points by virtue of \n> >> it actually using the IOMMU API, it doesn't call them itself. I'm kind\n> >> of surprised these are actually available to be called from a module. \n> > \n> > They are explicitly exported:\n> > \n> > EXPORT_TRACEPOINT_SYMBOL_GPL(map);\n> > EXPORT_TRACEPOINT_SYMBOL_GPL(unmap);\n> > \n> > I would think this is for things like drivers/vfio/vfio_iommu_spapr_tce.c ,\n> > why else?...\n> > \n> > \n> >> I suspect the way to do this is probably to define our own tracepoints\n> >> in the vfio/spapr backend or insert tracepoints into the IOMMU layers\n> >> that that code calls into rather than masquerading as tracepoints from\n> >> a different subsystem. Right? \n> > \n> > This makes sense too. But it is going to be just cut-n-paste and some\n> > confusion -\n> > /sys/kernel/debug/tracing/events/iommu/map will still be present but\n> > won't work and\n> > /sys/kernel/debug/tracing/events/vfio/vfio_iommu_spapr_tce/map will.\n\nBut iommu/map does work, it's just not called by the vfio spapr tce\nbackend, which is absolutely correct. In fact, that's part of my\nreservation about this approach is that it could be interpreted as\nimplying a call path that doesn't exist on this arch.\n\n> > Judges? :) \n> \n> \n> Still nak? I discussed this locally, the conclusion was it is a matter of\n> taste and this proposal is not that disgusting. Thanks.\n\nIs that our goal now, proposals that aren't too disgusting? I think\nit's an opportunity to introduce our own tracing infrastructure and it\nfeels a lot more correct to do that than to generalize existing trace\npoints from other subsystems. Thanks,\n\nAlex\n\n\n> >>>> ---\n> >>>>\n> >>>> Example:\n> >>>> qemu-system-ppc-8655 [096] 724.662740: unmap: IOMMU: iova=0x000000003ffff000 size=4096 unmapped_size=4096\n> >>>> qemu-system-ppc-8656 [104] 724.970912: map: IOMMU: iova=0x0800000000000000 paddr=0x00007ffef7ff0000 size=65536\n> >>>> ---\n> >>>> drivers/vfio/vfio_iommu_spapr_tce.c | 12 ++++++++++--\n> >>>> 1 file changed, 10 insertions(+), 2 deletions(-)\n> >>>>\n> >>>> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c\n> >>>> index 63112c36ab2d..4531486c77c6 100644\n> >>>> --- a/drivers/vfio/vfio_iommu_spapr_tce.c\n> >>>> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c\n> >>>> @@ -22,6 +22,7 @@\n> >>>> #include \n> >>>> #include \n> >>>> #include \n> >>>> +#include \n> >>>> \n> >>>> #include \n> >>>> #include \n> >>>> @@ -502,17 +503,19 @@ static int tce_iommu_clear(struct tce_container *container,\n> >>>> \t\tstruct iommu_table *tbl,\n> >>>> \t\tunsigned long entry, unsigned long pages)\n> >>>> {\n> >>>> -\tunsigned long oldhpa;\n> >>>> +\tunsigned long oldhpa, unmapped, firstentry = entry, totalpages = pages;\n> >>>> \tlong ret;\n> >>>> \tenum dma_data_direction direction;\n> >>>> \n> >>>> -\tfor ( ; pages; --pages, ++entry) {\n> >>>> +\tfor (unmapped = 0; pages; --pages, ++entry) {\n> >>>> \t\tdirection = DMA_NONE;\n> >>>> \t\toldhpa = 0;\n> >>>> \t\tret = iommu_tce_xchg(tbl, entry, &oldhpa, &direction);\n> >>>> \t\tif (ret)\n> >>>> \t\t\tcontinue;\n> >>>> \n> >>>> +\t\t++unmapped;\n> >>>> +\n> >>>> \t\tif (direction == DMA_NONE)\n> >>>> \t\t\tcontinue;\n> >>>> \n> >>>> @@ -523,6 +526,9 @@ static int tce_iommu_clear(struct tce_container *container,\n> >>>> \n> >>>> \t\ttce_iommu_unuse_page(container, oldhpa);\n> >>>> \t}\n> >>>> +\ttrace_unmap(firstentry << tbl->it_page_shift,\n> >>>> +\t\t\ttotalpages << tbl->it_page_shift,\n> >>>> +\t\t\tunmapped << tbl->it_page_shift);\n> >>>> \n> >>>> \treturn 0;\n> >>>> }\n> >>>> @@ -965,6 +971,8 @@ static long tce_iommu_ioctl(void *iommu_data,\n> >>>> \t\t\t\t\tdirection);\n> >>>> \n> >>>> \t\tiommu_flush_tce(tbl);\n> >>>> +\t\tif (!ret)\n> >>>> +\t\t\ttrace_map(param.iova, param.vaddr, param.size);\n> >>>> \n> >>>> \t\treturn ret;\n> >>>> \t}\n> >>>> \n> >>>\n> >>> \n> >> \n> > \n> > \n> \n> \n\n--\nTo unsubscribe from this list: send the line \"unsubscribe kvm-ppc\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=kvm-ppc-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yn5g42QqSz9t3Z\n\tfor ;\n\tThu, 30 Nov 2017 03:32:48 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S932234AbdK2Qcr (ORCPT );\n\tWed, 29 Nov 2017 11:32:47 -0500","from mx1.redhat.com ([209.132.183.28]:41744 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S932094AbdK2Qcq (ORCPT );\n\tWed, 29 Nov 2017 11:32:46 -0500","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 2C32813A53;\n\tWed, 29 Nov 2017 16:32:46 +0000 (UTC)","from t450s.home (ovpn-116-16.phx2.redhat.com [10.3.116.16])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id F3FC85D753;\n\tWed, 29 Nov 2017 16:32:44 +0000 (UTC)"],"Date":"Wed, 29 Nov 2017 09:32:44 -0700","From":"Alex Williamson ","To":"Alexey Kardashevskiy ","Cc":"linuxppc-dev@lists.ozlabs.org,\n\tDavid Gibson ,\n\tkvm-ppc@vger.kernel.org, kvm@vger.kernel.org,\n\tShuah Khan ,\n\tiommu@lists.linux-foundation.org, Joerg Roedel ","Subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","Message-ID":"<20171129093244.0849fd7c@t450s.home>","In-Reply-To":"","References":"<20171027030008.17200-1-aik@ozlabs.ru>\n\t<8c58fcaa-dedc-e6be-44bf-2cb349ef9e38@ozlabs.ru>\n\t<20171116171301.6552869b@t450s.home>\n\t<52a19cfb-431f-522b-11de-c1c47f09a4c8@ozlabs.ru>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.29]);\n\tWed, 29 Nov 2017 16:32:46 +0000 (UTC)","Sender":"kvm-ppc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"kvm-ppc@vger.kernel.org"}},{"id":1812448,"web_url":"http://patchwork.ozlabs.org/comment/1812448/","msgid":"<24f7ea0f-8a18-752f-6841-7001f88d1e00@arm.com>","list_archive_url":null,"date":"2017-11-29T16:43:46","subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","submitter":{"id":65641,"url":"http://patchwork.ozlabs.org/api/people/65641/","name":"Robin Murphy","email":"robin.murphy@arm.com"},"content":"On 29/11/17 16:32, Alex Williamson wrote:\n> On Thu, 23 Nov 2017 15:13:37 +1100\n> Alexey Kardashevskiy wrote:\n> \n>> On 17/11/17 17:58, Alexey Kardashevskiy wrote:\n>>> On 17/11/17 11:13, Alex Williamson wrote:\n>>>> On Tue, 14 Nov 2017 10:47:12 +1100\n>>>> Alexey Kardashevskiy wrote:\n>>>> \n>>>>> On 27/10/17 14:00, Alexey Kardashevskiy wrote:\n>>>>>> This adds trace_map/trace_unmap tracepoints to spapr driver. Type1 already\n>>>>>> uses these via the IOMMU API (iommu_map/__iommu_unmap).\n>>>>>>\n>>>>>> Signed-off-by: Alexey Kardashevskiy \n>>>>\n>>>> Is this really legitimate to include tracepoints from a different\n>>>> subsystem?> The vfio type1 backend gets these trace points by virtue of\n>>>> it actually using the IOMMU API, it doesn't call them itself. I'm kind\n>>>> of surprised these are actually available to be called from a module.\n>>>\n>>> They are explicitly exported:\n>>>\n>>> EXPORT_TRACEPOINT_SYMBOL_GPL(map);\n>>> EXPORT_TRACEPOINT_SYMBOL_GPL(unmap);\n>>>\n>>> I would think this is for things like drivers/vfio/vfio_iommu_spapr_tce.c ,\n>>> why else?...\n>>>\n>>> \n>>>> I suspect the way to do this is probably to define our own tracepoints\n>>>> in the vfio/spapr backend or insert tracepoints into the IOMMU layers\n>>>> that that code calls into rather than masquerading as tracepoints from\n>>>> a different subsystem. Right?\n>>>\n>>> This makes sense too. But it is going to be just cut-n-paste and some\n>>> confusion -\n>>> /sys/kernel/debug/tracing/events/iommu/map will still be present but\n>>> won't work and\n>>> /sys/kernel/debug/tracing/events/vfio/vfio_iommu_spapr_tce/map will.\n> \n> But iommu/map does work, it's just not called by the vfio spapr tce\n> backend, which is absolutely correct. In fact, that's part of my\n> reservation about this approach is that it could be interpreted as\n> implying a call path that doesn't exist on this arch.\n\nCome to think of it, the inverse also applies - if users are led to \nexpect x86-like behaviour where the iommu/map tracepoint just happens to \nreflect VFIO calls, they'll likely also get a nasty surprise on \nplatforms like arm64 where regular DMA API calls by drivers may be \nhammering iommu_map() in the background.\n\nRobin.\n\n>>> Judges? :)\n>>\n>>\n>> Still nak? I discussed this locally, the conclusion was it is a matter of\n>> taste and this proposal is not that disgusting. Thanks.\n> \n> Is that our goal now, proposals that aren't too disgusting? I think\n> it's an opportunity to introduce our own tracing infrastructure and it\n> feels a lot more correct to do that than to generalize existing trace\n> points from other subsystems. Thanks,\n> \n> Alex\n> \n> \n>>>>>> ---\n>>>>>>\n>>>>>> Example:\n>>>>>> qemu-system-ppc-8655 [096] 724.662740: unmap: IOMMU: iova=0x000000003ffff000 size=4096 unmapped_size=4096\n>>>>>> qemu-system-ppc-8656 [104] 724.970912: map: IOMMU: iova=0x0800000000000000 paddr=0x00007ffef7ff0000 size=65536\n>>>>>> ---\n>>>>>> drivers/vfio/vfio_iommu_spapr_tce.c | 12 ++++++++++--\n>>>>>> 1 file changed, 10 insertions(+), 2 deletions(-)\n>>>>>>\n>>>>>> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c\n>>>>>> index 63112c36ab2d..4531486c77c6 100644\n>>>>>> --- a/drivers/vfio/vfio_iommu_spapr_tce.c\n>>>>>> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c\n>>>>>> @@ -22,6 +22,7 @@\n>>>>>> #include \n>>>>>> #include \n>>>>>> #include \n>>>>>> +#include \n>>>>>> \n>>>>>> #include \n>>>>>> #include \n>>>>>> @@ -502,17 +503,19 @@ static int tce_iommu_clear(struct tce_container *container,\n>>>>>> \t\tstruct iommu_table *tbl,\n>>>>>> \t\tunsigned long entry, unsigned long pages)\n>>>>>> {\n>>>>>> -\tunsigned long oldhpa;\n>>>>>> +\tunsigned long oldhpa, unmapped, firstentry = entry, totalpages = pages;\n>>>>>> \tlong ret;\n>>>>>> \tenum dma_data_direction direction;\n>>>>>> \n>>>>>> -\tfor ( ; pages; --pages, ++entry) {\n>>>>>> +\tfor (unmapped = 0; pages; --pages, ++entry) {\n>>>>>> \t\tdirection = DMA_NONE;\n>>>>>> \t\toldhpa = 0;\n>>>>>> \t\tret = iommu_tce_xchg(tbl, entry, &oldhpa, &direction);\n>>>>>> \t\tif (ret)\n>>>>>> \t\t\tcontinue;\n>>>>>> \n>>>>>> +\t\t++unmapped;\n>>>>>> +\n>>>>>> \t\tif (direction == DMA_NONE)\n>>>>>> \t\t\tcontinue;\n>>>>>> \n>>>>>> @@ -523,6 +526,9 @@ static int tce_iommu_clear(struct tce_container *container,\n>>>>>> \n>>>>>> \t\ttce_iommu_unuse_page(container, oldhpa);\n>>>>>> \t}\n>>>>>> +\ttrace_unmap(firstentry << tbl->it_page_shift,\n>>>>>> +\t\t\ttotalpages << tbl->it_page_shift,\n>>>>>> +\t\t\tunmapped << tbl->it_page_shift);\n>>>>>> \n>>>>>> \treturn 0;\n>>>>>> }\n>>>>>> @@ -965,6 +971,8 @@ static long tce_iommu_ioctl(void *iommu_data,\n>>>>>> \t\t\t\t\tdirection);\n>>>>>> \n>>>>>> \t\tiommu_flush_tce(tbl);\n>>>>>> +\t\tif (!ret)\n>>>>>> +\t\t\ttrace_map(param.iova, param.vaddr, param.size);\n>>>>>> \n>>>>>> \t\treturn ret;\n>>>>>> \t}\n>>>>>> \n>>>>>\n>>>>> \n>>>> \n>>>\n>>> \n>>\n>>\n> \n> _______________________________________________\n> iommu mailing list\n> iommu@lists.linux-foundation.org\n> https://lists.linuxfoundation.org/mailman/listinfo/iommu\n> \n--\nTo unsubscribe from this list: send the line \"unsubscribe kvm-ppc\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=kvm-ppc-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yn5vq2MVfz9t2V\n\tfor ;\n\tThu, 30 Nov 2017 03:43:51 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753706AbdK2Qnu (ORCPT );\n\tWed, 29 Nov 2017 11:43:50 -0500","from foss.arm.com ([217.140.101.70]:41554 \"EHLO foss.arm.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1752314AbdK2Qnu (ORCPT );\n\tWed, 29 Nov 2017 11:43:50 -0500","from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A344E1435;\n\tWed, 29 Nov 2017 08:43:49 -0800 (PST)","from [10.1.210.88] (e110467-lin.cambridge.arm.com [10.1.210.88])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t462F83F236; Wed, 29 Nov 2017 08:43:48 -0800 (PST)"],"Subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","To":"Alex Williamson ,\n\tAlexey Kardashevskiy ","Cc":"kvm@vger.kernel.org, Shuah Khan ,\n\tkvm-ppc@vger.kernel.org, iommu@lists.linux-foundation.org,\n\tlinuxppc-dev@lists.ozlabs.org, David Gibson ","References":"<20171027030008.17200-1-aik@ozlabs.ru>\n\t<8c58fcaa-dedc-e6be-44bf-2cb349ef9e38@ozlabs.ru>\n\t<20171116171301.6552869b@t450s.home>\n\t<52a19cfb-431f-522b-11de-c1c47f09a4c8@ozlabs.ru>\n\t\n\t<20171129093244.0849fd7c@t450s.home>","From":"Robin Murphy ","Message-ID":"<24f7ea0f-8a18-752f-6841-7001f88d1e00@arm.com>","Date":"Wed, 29 Nov 2017 16:43:46 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<20171129093244.0849fd7c@t450s.home>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Sender":"kvm-ppc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"kvm-ppc@vger.kernel.org"}},{"id":1812450,"web_url":"http://patchwork.ozlabs.org/comment/1812450/","msgid":"","list_archive_url":null,"date":"2017-11-29T16:44:13","subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","submitter":{"id":65093,"url":"http://patchwork.ozlabs.org/api/people/65093/","name":"Shuah Khan","email":"shuahkh@osg.samsung.com"},"content":"On 11/29/2017 09:32 AM, Alex Williamson wrote:\n> On Thu, 23 Nov 2017 15:13:37 +1100\n> Alexey Kardashevskiy wrote:\n> \n>> On 17/11/17 17:58, Alexey Kardashevskiy wrote:\n>>> On 17/11/17 11:13, Alex Williamson wrote: \n>>>> On Tue, 14 Nov 2017 10:47:12 +1100\n>>>> Alexey Kardashevskiy wrote:\n>>>> \n>>>>> On 27/10/17 14:00, Alexey Kardashevskiy wrote: \n>>>>>> This adds trace_map/trace_unmap tracepoints to spapr driver. Type1 already\n>>>>>> uses these via the IOMMU API (iommu_map/__iommu_unmap).\n>>>>>>\n>>>>>> Signed-off-by: Alexey Kardashevskiy \n>>>>\n>>>> Is this really legitimate to include tracepoints from a different \n>>>> subsystem?> The vfio type1 backend gets these trace points by virtue of \n>>>> it actually using the IOMMU API, it doesn't call them itself. I'm kind\n>>>> of surprised these are actually available to be called from a module. \n>>>\n>>> They are explicitly exported:\n>>>\n>>> EXPORT_TRACEPOINT_SYMBOL_GPL(map);\n>>> EXPORT_TRACEPOINT_SYMBOL_GPL(unmap);\n\nTracepoints need to be exported as per the tracepoint sub-system\nrequirements.\n\n>>>\n>>> I would think this is for things like drivers/vfio/vfio_iommu_spapr_tce.c ,\n>>> why else?...\n>>>\n>>> \n>>>> I suspect the way to do this is probably to define our own tracepoints\n>>>> in the vfio/spapr backend or insert tracepoints into the IOMMU layers\n>>>> that that code calls into rather than masquerading as tracepoints from\n>>>> a different subsystem. Right? \n>>>\n>>> This makes sense too. But it is going to be just cut-n-paste and some\n>>> confusion -\n>>> /sys/kernel/debug/tracing/events/iommu/map will still be present but\n>>> won't work and\n>>> /sys/kernel/debug/tracing/events/vfio/vfio_iommu_spapr_tce/map will.\n> \n> But iommu/map does work, it's just not called by the vfio spapr tce\n> backend, which is absolutely correct. In fact, that's part of my\n> reservation about this approach is that it could be interpreted as\n> implying a call path that doesn't exist on this arch.\n\nThese tracepoints are intended to be called after a iommo/map/unmap\ncalls from IOMMU API. It doesn't make any sense to call them from\nrandom places. It is totally useless to do so and makes it difficult\nto debug problems.\n\nAs an author of these tracepoints - that is my opinion.\n\nthanks,\n-- Shuah\n--\nTo unsubscribe from this list: send the line \"unsubscribe kvm-ppc\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=kvm-ppc-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yn5wf6Tv6z9t2h\n\tfor ;\n\tThu, 30 Nov 2017 03:44:34 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753706AbdK2Qoe (ORCPT );\n\tWed, 29 Nov 2017 11:44:34 -0500","from osg.samsung.com ([64.30.133.232]:52284 \"EHLO osg.samsung.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1753343AbdK2Qod (ORCPT );\n\tWed, 29 Nov 2017 11:44:33 -0500","from localhost (localhost [127.0.0.1])\n\tby osg.samsung.com (Postfix) with ESMTP id F0CCB2E462;\n\tWed, 29 Nov 2017 08:44:32 -0800 (PST)","from osg.samsung.com ([127.0.0.1])\n\tby localhost (localhost [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id bLfjYA27fIaw; Wed, 29 Nov 2017 08:44:21 -0800 (PST)","from [192.168.1.87] (c-24-9-64-241.hsd1.co.comcast.net\n\t[24.9.64.241])\n\tby osg.samsung.com (Postfix) with ESMTPSA id 774002E457;\n\tWed, 29 Nov 2017 08:44:21 -0800 (PST)"],"X-Virus-Scanned":"Debian amavisd-new at dev.s-opensource.com","Subject":"Re: [PATCH kernel] vfio/spapr: Add trace points for map/unmap","To":"Alex Williamson ,\n\tAlexey Kardashevskiy ","Cc":"linuxppc-dev@lists.ozlabs.org,\n\tDavid Gibson ,\n\tkvm-ppc@vger.kernel.org, kvm@vger.kernel.org,\n\tiommu@lists.linux-foundation.org, Joerg Roedel ,\n\tShuah Khan ","References":"<20171027030008.17200-1-aik@ozlabs.ru>\n\t<8c58fcaa-dedc-e6be-44bf-2cb349ef9e38@ozlabs.ru>\n\t<20171116171301.6552869b@t450s.home>\n\t<52a19cfb-431f-522b-11de-c1c47f09a4c8@ozlabs.ru>\n\t\n\t<20171129093244.0849fd7c@t450s.home>","From":"Shuah Khan ","Message-ID":"","Date":"Wed, 29 Nov 2017 09:44:13 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.4.0","MIME-Version":"1.0","In-Reply-To":"<20171129093244.0849fd7c@t450s.home>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Sender":"kvm-ppc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"kvm-ppc@vger.kernel.org"}}]