From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jean-Philippe Brucker Subject: Re: [PATCH 35/37] iommu/arm-smmu-v3: Add support for PRI Date: Wed, 14 Mar 2018 13:10:01 +0000 Message-ID: <2c3c608c-266c-b01f-819f-0c04d3d282f6@arm.com> References: <20180212183352.22730-1-jean-philippe.brucker@arm.com> <20180212183352.22730-36-jean-philippe.brucker@arm.com> <20180308172436.00006554@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20180308172436.00006554-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Jonathan Cameron Cc: Mark Rutland , "ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org" , "kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "xuzaibo-hv44wF8Li93QT0dZR+AlfA@public.gmane.org" , Will Deacon , "okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org" , "ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org" , "bharatku-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org" , "linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Catalin Marinas , "rfranz-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org" , "lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" , "bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , "dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org" , rjw@rjwyso List-Id: linux-acpi@vger.kernel.org On 08/03/18 16:24, Jonathan Cameron wrote: > On Mon, 12 Feb 2018 18:33:50 +0000 > Jean-Philippe Brucker wrote: > >> For PCI devices that support it, enable the PRI capability and handle >> PRI Page Requests with the generic fault handler. >> >> Signed-off-by: Jean-Philippe Brucker > A couple of nitpicks. > >> --- >> drivers/iommu/arm-smmu-v3.c | 174 ++++++++++++++++++++++++++++++-------------- >> 1 file changed, 119 insertions(+), 55 deletions(-) >> >> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c >> index 8d09615fab35..ace2f995b0c0 100644 >> --- a/drivers/iommu/arm-smmu-v3.c >> +++ b/drivers/iommu/arm-smmu-v3.c >> @@ -271,6 +271,7 @@ >> #define STRTAB_STE_1_S1COR_SHIFT 4 >> #define STRTAB_STE_1_S1CSH_SHIFT 6 >> >> +#define STRTAB_STE_1_PPAR (1UL << 18) >> #define STRTAB_STE_1_S1STALLD (1UL << 27) >> >> #define STRTAB_STE_1_EATS_ABT 0UL >> @@ -346,9 +347,9 @@ >> #define CMDQ_PRI_1_GRPID_SHIFT 0 >> #define CMDQ_PRI_1_GRPID_MASK 0x1ffUL >> #define CMDQ_PRI_1_RESP_SHIFT 12 >> -#define CMDQ_PRI_1_RESP_DENY (0UL << CMDQ_PRI_1_RESP_SHIFT) >> -#define CMDQ_PRI_1_RESP_FAIL (1UL << CMDQ_PRI_1_RESP_SHIFT) >> -#define CMDQ_PRI_1_RESP_SUCC (2UL << CMDQ_PRI_1_RESP_SHIFT) >> +#define CMDQ_PRI_1_RESP_FAILURE (0UL << CMDQ_PRI_1_RESP_SHIFT) >> +#define CMDQ_PRI_1_RESP_INVALID (1UL << CMDQ_PRI_1_RESP_SHIFT) >> +#define CMDQ_PRI_1_RESP_SUCCESS (2UL << CMDQ_PRI_1_RESP_SHIFT) > Mixing fixing up this naming with the rest of the patch does make things a > little harder to read than they would have been if done as separate patches. > Worth splitting? ok [...] > > The function ordering gets a bit random as you add all the new ones, > Might be better to keep each disable following each enable. Agreed Thanks, Jean From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: Subject: Re: [PATCH 35/37] iommu/arm-smmu-v3: Add support for PRI To: Jonathan Cameron References: <20180212183352.22730-1-jean-philippe.brucker@arm.com> <20180212183352.22730-36-jean-philippe.brucker@arm.com> <20180308172436.00006554@huawei.com> From: Jean-Philippe Brucker Message-ID: <2c3c608c-266c-b01f-819f-0c04d3d282f6@arm.com> Date: Wed, 14 Mar 2018 13:10:01 +0000 MIME-Version: 1.0 In-Reply-To: <20180308172436.00006554@huawei.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , "xieyisheng1@huawei.com" , "ilias.apalodimas@linaro.org" , "kvm@vger.kernel.org" , "linux-pci@vger.kernel.org" , "xuzaibo@huawei.com" , Will Deacon , "okaya@codeaurora.org" , "yi.l.liu@intel.com" , Lorenzo Pieralisi , "ashok.raj@intel.com" , "tn@semihalf.com" , "joro@8bytes.org" , "robdclark@gmail.com" , "bharatku@xilinx.com" , "linux-acpi@vger.kernel.org" , Catalin Marinas , "rfranz@cavium.com" , "lenb@kernel.org" , "devicetree@vger.kernel.org" , "jacob.jun.pan@linux.intel.com" , "alex.williamson@redhat.com" , "robh+dt@kernel.org" , "thunder.leizhen@huawei.com" , "bhelgaas@google.com" , "linux-arm-kernel@lists.infradead.org" , "shunyong.yang@hxt-semitech.com" , "dwmw2@infradead.org" , "liubo95@huawei.com" , "rjw@rjwysocki.net" , "jcrouse@codeaurora.org" , "iommu@lists.linux-foundation.org" , "hanjun.guo@linaro.org" , Sudeep Holla , Robin Murphy , "christian.koenig@amd.com" , "nwatters@codeaurora.org" Content-Type: text/plain; charset="us-ascii" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+bjorn=helgaas.com@lists.infradead.org List-ID: On 08/03/18 16:24, Jonathan Cameron wrote: > On Mon, 12 Feb 2018 18:33:50 +0000 > Jean-Philippe Brucker wrote: > >> For PCI devices that support it, enable the PRI capability and handle >> PRI Page Requests with the generic fault handler. >> >> Signed-off-by: Jean-Philippe Brucker > A couple of nitpicks. > >> --- >> drivers/iommu/arm-smmu-v3.c | 174 ++++++++++++++++++++++++++++++-------------- >> 1 file changed, 119 insertions(+), 55 deletions(-) >> >> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c >> index 8d09615fab35..ace2f995b0c0 100644 >> --- a/drivers/iommu/arm-smmu-v3.c >> +++ b/drivers/iommu/arm-smmu-v3.c >> @@ -271,6 +271,7 @@ >> #define STRTAB_STE_1_S1COR_SHIFT 4 >> #define STRTAB_STE_1_S1CSH_SHIFT 6 >> >> +#define STRTAB_STE_1_PPAR (1UL << 18) >> #define STRTAB_STE_1_S1STALLD (1UL << 27) >> >> #define STRTAB_STE_1_EATS_ABT 0UL >> @@ -346,9 +347,9 @@ >> #define CMDQ_PRI_1_GRPID_SHIFT 0 >> #define CMDQ_PRI_1_GRPID_MASK 0x1ffUL >> #define CMDQ_PRI_1_RESP_SHIFT 12 >> -#define CMDQ_PRI_1_RESP_DENY (0UL << CMDQ_PRI_1_RESP_SHIFT) >> -#define CMDQ_PRI_1_RESP_FAIL (1UL << CMDQ_PRI_1_RESP_SHIFT) >> -#define CMDQ_PRI_1_RESP_SUCC (2UL << CMDQ_PRI_1_RESP_SHIFT) >> +#define CMDQ_PRI_1_RESP_FAILURE (0UL << CMDQ_PRI_1_RESP_SHIFT) >> +#define CMDQ_PRI_1_RESP_INVALID (1UL << CMDQ_PRI_1_RESP_SHIFT) >> +#define CMDQ_PRI_1_RESP_SUCCESS (2UL << CMDQ_PRI_1_RESP_SHIFT) > Mixing fixing up this naming with the rest of the patch does make things a > little harder to read than they would have been if done as separate patches. > Worth splitting? ok [...] > > The function ordering gets a bit random as you add all the new ones, > Might be better to keep each disable following each enable. Agreed Thanks, Jean _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 From: jean-philippe.brucker@arm.com (Jean-Philippe Brucker) Date: Wed, 14 Mar 2018 13:10:01 +0000 Subject: [PATCH 35/37] iommu/arm-smmu-v3: Add support for PRI In-Reply-To: <20180308172436.00006554@huawei.com> References: <20180212183352.22730-1-jean-philippe.brucker@arm.com> <20180212183352.22730-36-jean-philippe.brucker@arm.com> <20180308172436.00006554@huawei.com> Message-ID: <2c3c608c-266c-b01f-819f-0c04d3d282f6@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 08/03/18 16:24, Jonathan Cameron wrote: > On Mon, 12 Feb 2018 18:33:50 +0000 > Jean-Philippe Brucker wrote: > >> For PCI devices that support it, enable the PRI capability and handle >> PRI Page Requests with the generic fault handler. >> >> Signed-off-by: Jean-Philippe Brucker > A couple of nitpicks. > >> --- >> drivers/iommu/arm-smmu-v3.c | 174 ++++++++++++++++++++++++++++++-------------- >> 1 file changed, 119 insertions(+), 55 deletions(-) >> >> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c >> index 8d09615fab35..ace2f995b0c0 100644 >> --- a/drivers/iommu/arm-smmu-v3.c >> +++ b/drivers/iommu/arm-smmu-v3.c >> @@ -271,6 +271,7 @@ >> #define STRTAB_STE_1_S1COR_SHIFT 4 >> #define STRTAB_STE_1_S1CSH_SHIFT 6 >> >> +#define STRTAB_STE_1_PPAR (1UL << 18) >> #define STRTAB_STE_1_S1STALLD (1UL << 27) >> >> #define STRTAB_STE_1_EATS_ABT 0UL >> @@ -346,9 +347,9 @@ >> #define CMDQ_PRI_1_GRPID_SHIFT 0 >> #define CMDQ_PRI_1_GRPID_MASK 0x1ffUL >> #define CMDQ_PRI_1_RESP_SHIFT 12 >> -#define CMDQ_PRI_1_RESP_DENY (0UL << CMDQ_PRI_1_RESP_SHIFT) >> -#define CMDQ_PRI_1_RESP_FAIL (1UL << CMDQ_PRI_1_RESP_SHIFT) >> -#define CMDQ_PRI_1_RESP_SUCC (2UL << CMDQ_PRI_1_RESP_SHIFT) >> +#define CMDQ_PRI_1_RESP_FAILURE (0UL << CMDQ_PRI_1_RESP_SHIFT) >> +#define CMDQ_PRI_1_RESP_INVALID (1UL << CMDQ_PRI_1_RESP_SHIFT) >> +#define CMDQ_PRI_1_RESP_SUCCESS (2UL << CMDQ_PRI_1_RESP_SHIFT) > Mixing fixing up this naming with the rest of the patch does make things a > little harder to read than they would have been if done as separate patches. > Worth splitting? ok [...] > > The function ordering gets a bit random as you add all the new ones, > Might be better to keep each disable following each enable. Agreed Thanks, Jean