* [PATCH 0/2] iommu/exynos: Add fault handling on SysMMU v7
@ 2022-07-26 20:07 Sam Protsenko
2022-07-26 20:07 ` [PATCH 1/2] iommu/exynos: Abstract getting the fault info Sam Protsenko
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Sam Protsenko @ 2022-07-26 20:07 UTC (permalink / raw)
To: Marek Szyprowski, Krzysztof Kozlowski
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Janghyuck Kim,
Cho KyongHo, Daniel Mentz, David Virag, Sumit Semwal, iommu,
linux-arm-kernel, linux-samsung-soc, linux-kernel
Exynos IOMMU driver implements fault handling for SysMMU v1..v5. But the
abstraction currently used is not suited for SysMMU v7, as it has quite
different fault related register set.
This patch series reworks the mentioned fault handling abstraction and
adds fault handling support for SysMMU v7.
Sam Protsenko (2):
iommu/exynos: Abstract getting the fault info
iommu/exynos: Implement fault handling on SysMMU v7
drivers/iommu/exynos-iommu.c | 208 ++++++++++++++++++++++++-----------
1 file changed, 143 insertions(+), 65 deletions(-)
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/2] iommu/exynos: Abstract getting the fault info 2022-07-26 20:07 [PATCH 0/2] iommu/exynos: Add fault handling on SysMMU v7 Sam Protsenko @ 2022-07-26 20:07 ` Sam Protsenko 2022-08-12 12:25 ` Marek Szyprowski 2022-07-26 20:07 ` [PATCH 2/2] iommu/exynos: Implement fault handling on SysMMU v7 Sam Protsenko ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Sam Protsenko @ 2022-07-26 20:07 UTC (permalink / raw) To: Marek Szyprowski, Krzysztof Kozlowski Cc: Joerg Roedel, Will Deacon, Robin Murphy, Janghyuck Kim, Cho KyongHo, Daniel Mentz, David Virag, Sumit Semwal, iommu, linux-arm-kernel, linux-samsung-soc, linux-kernel Fault info obtaining is implemented for SysMMU v1..v5 in a very hardware specific way, as it relies on: - interrupt bits being tied to read or write access - having separate registers for the fault address w.r.t. AR/AW ops Newer SysMMU versions (like SysMMU v7) have different way of providing the fault info via registers: - the transaction type (read or write) should be read from the register (instead of hard-coding it w.r.t. corresponding interrupt status bit) - there is only one single register for storing the fault address Because of that, it is not possible to add newer SysMMU support into existing paradigm. Also it's not very effective performance-wise: - checking SysMMU version in ISR each time is not necessary - performing linear search to find the fault info by interrupt bit can be replaced with a single lookup operation Pave the way for adding support for new SysMMU versions by abstracting the getting of fault info in ISR. While at it, do some related style cleanups as well. This is mostly a refactoring patch, but there are some minor functional changes: - fault message format is a bit different; now instead of AR/AW prefixes for the fault's name, the request direction is printed as [READ]/[WRITE]. It has to be done to prepare an abstraction for SysMMU v7 support - don't panic on unknown interrupts; print corresponding message and continue - if fault wasn't recovered, panic with some sane message instead of just doing BUG_ON() The whole fault message looks like this now: [READ] PAGE FAULT occurred at 0x12341000 Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> --- drivers/iommu/exynos-iommu.c | 162 +++++++++++++++++++++-------------- 1 file changed, 100 insertions(+), 62 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 8e18984a0c4f..766d409e084a 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -185,38 +185,36 @@ static sysmmu_pte_t *page_entry(sysmmu_pte_t *sent, sysmmu_iova_t iova) lv2table_base(sent)) + lv2ent_offset(iova); } -/* - * IOMMU fault information register - */ -struct sysmmu_fault_info { - unsigned int bit; /* bit number in STATUS register */ - unsigned short addr_reg; /* register to read VA fault address */ +struct sysmmu_fault { + sysmmu_iova_t addr; /* IOVA address that caused fault */ + const char *name; /* human readable fault name */ + unsigned int type; /* fault type for report_iommu_fault() */ +}; + +struct sysmmu_v1_fault_info { + unsigned short addr_reg; /* register to read IOVA fault address */ const char *name; /* human readable fault name */ unsigned int type; /* fault type for report_iommu_fault */ }; -static const struct sysmmu_fault_info sysmmu_faults[] = { - { 0, REG_PAGE_FAULT_ADDR, "PAGE", IOMMU_FAULT_READ }, - { 1, REG_AR_FAULT_ADDR, "AR MULTI-HIT", IOMMU_FAULT_READ }, - { 2, REG_AW_FAULT_ADDR, "AW MULTI-HIT", IOMMU_FAULT_WRITE }, - { 3, REG_DEFAULT_SLAVE_ADDR, "BUS ERROR", IOMMU_FAULT_READ }, - { 4, REG_AR_FAULT_ADDR, "AR SECURITY PROTECTION", IOMMU_FAULT_READ }, - { 5, REG_AR_FAULT_ADDR, "AR ACCESS PROTECTION", IOMMU_FAULT_READ }, - { 6, REG_AW_FAULT_ADDR, "AW SECURITY PROTECTION", IOMMU_FAULT_WRITE }, - { 7, REG_AW_FAULT_ADDR, "AW ACCESS PROTECTION", IOMMU_FAULT_WRITE }, +static const struct sysmmu_v1_fault_info sysmmu_v1_faults[] = { + { REG_PAGE_FAULT_ADDR, "PAGE", IOMMU_FAULT_READ }, + { REG_AR_FAULT_ADDR, "MULTI-HIT", IOMMU_FAULT_READ }, + { REG_AW_FAULT_ADDR, "MULTI-HIT", IOMMU_FAULT_WRITE }, + { REG_DEFAULT_SLAVE_ADDR, "BUS ERROR", IOMMU_FAULT_READ }, + { REG_AR_FAULT_ADDR, "SECURITY PROTECTION", IOMMU_FAULT_READ }, + { REG_AR_FAULT_ADDR, "ACCESS PROTECTION", IOMMU_FAULT_READ }, + { REG_AW_FAULT_ADDR, "SECURITY PROTECTION", IOMMU_FAULT_WRITE }, + { REG_AW_FAULT_ADDR, "ACCESS PROTECTION", IOMMU_FAULT_WRITE }, }; -static const struct sysmmu_fault_info sysmmu_v5_faults[] = { - { 0, REG_V5_FAULT_AR_VA, "AR PTW", IOMMU_FAULT_READ }, - { 1, REG_V5_FAULT_AR_VA, "AR PAGE", IOMMU_FAULT_READ }, - { 2, REG_V5_FAULT_AR_VA, "AR MULTI-HIT", IOMMU_FAULT_READ }, - { 3, REG_V5_FAULT_AR_VA, "AR ACCESS PROTECTION", IOMMU_FAULT_READ }, - { 4, REG_V5_FAULT_AR_VA, "AR SECURITY PROTECTION", IOMMU_FAULT_READ }, - { 16, REG_V5_FAULT_AW_VA, "AW PTW", IOMMU_FAULT_WRITE }, - { 17, REG_V5_FAULT_AW_VA, "AW PAGE", IOMMU_FAULT_WRITE }, - { 18, REG_V5_FAULT_AW_VA, "AW MULTI-HIT", IOMMU_FAULT_WRITE }, - { 19, REG_V5_FAULT_AW_VA, "AW ACCESS PROTECTION", IOMMU_FAULT_WRITE }, - { 20, REG_V5_FAULT_AW_VA, "AW SECURITY PROTECTION", IOMMU_FAULT_WRITE }, +/* SysMMU v5 has the same faults for AR (0..4 bits) and AW (16..20 bits) */ +static const char * const sysmmu_v5_fault_names[] = { + "PTW", + "PAGE", + "MULTI-HIT", + "ACCESS PROTECTION", + "SECURITY PROTECTION" }; /* @@ -246,9 +244,12 @@ struct exynos_iommu_domain { struct iommu_domain domain; /* generic domain data structure */ }; +struct sysmmu_drvdata; + /* * SysMMU version specific data. Contains offsets for the registers which can * be found in different SysMMU variants, but have different offset values. + * Also contains version specific callbacks to abstract the hardware. */ struct sysmmu_variant { u32 pt_base; /* page table base address (physical) */ @@ -259,6 +260,9 @@ struct sysmmu_variant { u32 flush_end; /* end address of range invalidation */ u32 int_status; /* interrupt status information */ u32 int_clear; /* clear the interrupt */ + + int (*get_fault_info)(struct sysmmu_drvdata *data, unsigned int itype, + struct sysmmu_fault *fault); }; /* @@ -293,6 +297,46 @@ struct sysmmu_drvdata { #define SYSMMU_REG(data, reg) ((data)->sfrbase + (data)->variant->reg) +static int exynos_sysmmu_v1_get_fault_info(struct sysmmu_drvdata *data, + unsigned int itype, + struct sysmmu_fault *fault) +{ + const struct sysmmu_v1_fault_info *finfo; + + if (itype >= ARRAY_SIZE(sysmmu_v1_faults)) + return -ENXIO; + + finfo = &sysmmu_v1_faults[itype]; + fault->addr = readl(data->sfrbase + finfo->addr_reg); + fault->name = finfo->name; + fault->type = finfo->type; + + return 0; +} + +static int exynos_sysmmu_v5_get_fault_info(struct sysmmu_drvdata *data, + unsigned int itype, + struct sysmmu_fault *fault) +{ + unsigned int addr_reg; + + if (itype < ARRAY_SIZE(sysmmu_v5_fault_names)) { + fault->type = IOMMU_FAULT_READ; + addr_reg = REG_V5_FAULT_AR_VA; + } else if (itype >= 16 && itype <= 20) { + fault->type = IOMMU_FAULT_WRITE; + addr_reg = REG_V5_FAULT_AW_VA; + itype -= 16; + } else { + return -ENXIO; + } + + fault->name = sysmmu_v5_fault_names[itype]; + fault->addr = readl(data->sfrbase + addr_reg); + + return 0; +} + /* SysMMU v1..v3 */ static const struct sysmmu_variant sysmmu_v1_variant = { .flush_all = 0x0c, @@ -300,6 +344,8 @@ static const struct sysmmu_variant sysmmu_v1_variant = { .pt_base = 0x14, .int_status = 0x18, .int_clear = 0x1c, + + .get_fault_info = exynos_sysmmu_v1_get_fault_info, }; /* SysMMU v5 and v7 (non-VM capable) */ @@ -312,6 +358,8 @@ static const struct sysmmu_variant sysmmu_v5_variant = { .flush_end = 0x24, .int_status = 0x60, .int_clear = 0x64, + + .get_fault_info = exynos_sysmmu_v5_get_fault_info, }; /* SysMMU v7: VM capable register set */ @@ -324,6 +372,8 @@ static const struct sysmmu_variant sysmmu_v7_vm_variant = { .flush_end = 0x8024, .int_status = 0x60, .int_clear = 0x64, + + .get_fault_info = exynos_sysmmu_v5_get_fault_info, }; static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom) @@ -453,68 +503,56 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data) } static void show_fault_information(struct sysmmu_drvdata *data, - const struct sysmmu_fault_info *finfo, - sysmmu_iova_t fault_addr) + const struct sysmmu_fault *fault) { sysmmu_pte_t *ent; - dev_err(data->sysmmu, "%s: %s FAULT occurred at %#x\n", - dev_name(data->master), finfo->name, fault_addr); + dev_err(data->sysmmu, "%s: [%s] %s FAULT occurred at %#x\n", + dev_name(data->master), + fault->type == IOMMU_FAULT_READ ? "READ" : "WRITE", + fault->name, fault->addr); dev_dbg(data->sysmmu, "Page table base: %pa\n", &data->pgtable); - ent = section_entry(phys_to_virt(data->pgtable), fault_addr); + ent = section_entry(phys_to_virt(data->pgtable), fault->addr); dev_dbg(data->sysmmu, "\tLv1 entry: %#x\n", *ent); if (lv1ent_page(ent)) { - ent = page_entry(ent, fault_addr); + ent = page_entry(ent, fault->addr); dev_dbg(data->sysmmu, "\t Lv2 entry: %#x\n", *ent); } } static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) { - /* SYSMMU is in blocked state when interrupt occurred. */ struct sysmmu_drvdata *data = dev_id; - const struct sysmmu_fault_info *finfo; - unsigned int i, n, itype; - sysmmu_iova_t fault_addr; + unsigned int itype; + struct sysmmu_fault fault; int ret = -ENOSYS; WARN_ON(!data->active); - if (MMU_MAJ_VER(data->version) < 5) { - finfo = sysmmu_faults; - n = ARRAY_SIZE(sysmmu_faults); - } else { - finfo = sysmmu_v5_faults; - n = ARRAY_SIZE(sysmmu_v5_faults); - } - spin_lock(&data->lock); - clk_enable(data->clk_master); itype = __ffs(readl(SYSMMU_REG(data, int_status))); - for (i = 0; i < n; i++, finfo++) - if (finfo->bit == itype) - break; - /* unknown/unsupported fault */ - BUG_ON(i == n); - - /* print debug message */ - fault_addr = readl(data->sfrbase + finfo->addr_reg); - show_fault_information(data, finfo, fault_addr); - - if (data->domain) - ret = report_iommu_fault(&data->domain->domain, - data->master, fault_addr, finfo->type); - /* fault is not recovered by fault handler */ - BUG_ON(ret != 0); + ret = data->variant->get_fault_info(data, itype, &fault); + if (ret) { + dev_err(data->sysmmu, "Unhandled interrupt bit %u\n", itype); + goto out; + } + show_fault_information(data, &fault); + if (data->domain) { + ret = report_iommu_fault(&data->domain->domain, data->master, + fault.addr, fault.type); + } + if (ret) + panic("Unrecoverable System MMU Fault!"); + +out: writel(1 << itype, SYSMMU_REG(data, int_clear)); + /* SysMMU is in blocked state when interrupt occurred */ sysmmu_unblock(data); - clk_disable(data->clk_master); - spin_unlock(&data->lock); return IRQ_HANDLED; -- 2.30.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] iommu/exynos: Abstract getting the fault info 2022-07-26 20:07 ` [PATCH 1/2] iommu/exynos: Abstract getting the fault info Sam Protsenko @ 2022-08-12 12:25 ` Marek Szyprowski 2022-10-24 14:43 ` Sam Protsenko 0 siblings, 1 reply; 12+ messages in thread From: Marek Szyprowski @ 2022-08-12 12:25 UTC (permalink / raw) To: Sam Protsenko, Krzysztof Kozlowski Cc: Joerg Roedel, Will Deacon, Robin Murphy, Janghyuck Kim, Cho KyongHo, Daniel Mentz, David Virag, Sumit Semwal, iommu, linux-arm-kernel, linux-samsung-soc, linux-kernel Hi Sam, On 26.07.2022 22:07, Sam Protsenko wrote: > Fault info obtaining is implemented for SysMMU v1..v5 in a very hardware > specific way, as it relies on: > - interrupt bits being tied to read or write access > - having separate registers for the fault address w.r.t. AR/AW ops > > Newer SysMMU versions (like SysMMU v7) have different way of providing > the fault info via registers: > - the transaction type (read or write) should be read from the > register (instead of hard-coding it w.r.t. corresponding interrupt > status bit) > - there is only one single register for storing the fault address > > Because of that, it is not possible to add newer SysMMU support into > existing paradigm. Also it's not very effective performance-wise: > - checking SysMMU version in ISR each time is not necessary > - performing linear search to find the fault info by interrupt bit can > be replaced with a single lookup operation > > Pave the way for adding support for new SysMMU versions by abstracting > the getting of fault info in ISR. While at it, do some related style > cleanups as well. > > This is mostly a refactoring patch, but there are some minor functional > changes: > - fault message format is a bit different; now instead of AR/AW > prefixes for the fault's name, the request direction is printed as > [READ]/[WRITE]. It has to be done to prepare an abstraction for > SysMMU v7 support > - don't panic on unknown interrupts; print corresponding message and > continue > - if fault wasn't recovered, panic with some sane message instead of > just doing BUG_ON() > > The whole fault message looks like this now: > > [READ] PAGE FAULT occurred at 0x12341000 > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> I'm not very happy with converting the sysmmu_fault_info arrays into the decoding functions. If I got the code right, adding v7 is still possible with the current approach. The main advantage of the array-based approach is readability and keeping all the information together in a single place. I agree for the items listed above as 'minor functional changes', though. Those sysmmu_fault_info arrays might be a part of sysmmu hw variant to avoid decoding hw version for each fault. I'm not sure that the linear scan is so problematic with regards to the performance. You really don't want your drivers to trigger IOMMU fault so often during normal operation. It is just a way to get some debugging information or handle some exception. You mentioned that the transaction type is read from the separate register in case of v7, but your code (here and in second patch) still relies on the reported interrupt bits. Could you try to rework all your changes in a such way, that the sysmmu_fault_info arrays are still used? V7 is really very similar to the v5 already supported by the current driver. > --- > drivers/iommu/exynos-iommu.c | 162 +++++++++++++++++++++-------------- > 1 file changed, 100 insertions(+), 62 deletions(-) > > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c > index 8e18984a0c4f..766d409e084a 100644 > --- a/drivers/iommu/exynos-iommu.c > +++ b/drivers/iommu/exynos-iommu.c > @@ -185,38 +185,36 @@ static sysmmu_pte_t *page_entry(sysmmu_pte_t *sent, sysmmu_iova_t iova) > lv2table_base(sent)) + lv2ent_offset(iova); > } > > -/* > - * IOMMU fault information register > - */ > -struct sysmmu_fault_info { > - unsigned int bit; /* bit number in STATUS register */ > - unsigned short addr_reg; /* register to read VA fault address */ > +struct sysmmu_fault { > + sysmmu_iova_t addr; /* IOVA address that caused fault */ > + const char *name; /* human readable fault name */ > + unsigned int type; /* fault type for report_iommu_fault() */ > +}; > + > +struct sysmmu_v1_fault_info { > + unsigned short addr_reg; /* register to read IOVA fault address */ > const char *name; /* human readable fault name */ > unsigned int type; /* fault type for report_iommu_fault */ > }; > > -static const struct sysmmu_fault_info sysmmu_faults[] = { > - { 0, REG_PAGE_FAULT_ADDR, "PAGE", IOMMU_FAULT_READ }, > - { 1, REG_AR_FAULT_ADDR, "AR MULTI-HIT", IOMMU_FAULT_READ }, > - { 2, REG_AW_FAULT_ADDR, "AW MULTI-HIT", IOMMU_FAULT_WRITE }, > - { 3, REG_DEFAULT_SLAVE_ADDR, "BUS ERROR", IOMMU_FAULT_READ }, > - { 4, REG_AR_FAULT_ADDR, "AR SECURITY PROTECTION", IOMMU_FAULT_READ }, > - { 5, REG_AR_FAULT_ADDR, "AR ACCESS PROTECTION", IOMMU_FAULT_READ }, > - { 6, REG_AW_FAULT_ADDR, "AW SECURITY PROTECTION", IOMMU_FAULT_WRITE }, > - { 7, REG_AW_FAULT_ADDR, "AW ACCESS PROTECTION", IOMMU_FAULT_WRITE }, > +static const struct sysmmu_v1_fault_info sysmmu_v1_faults[] = { > + { REG_PAGE_FAULT_ADDR, "PAGE", IOMMU_FAULT_READ }, > + { REG_AR_FAULT_ADDR, "MULTI-HIT", IOMMU_FAULT_READ }, > + { REG_AW_FAULT_ADDR, "MULTI-HIT", IOMMU_FAULT_WRITE }, > + { REG_DEFAULT_SLAVE_ADDR, "BUS ERROR", IOMMU_FAULT_READ }, > + { REG_AR_FAULT_ADDR, "SECURITY PROTECTION", IOMMU_FAULT_READ }, > + { REG_AR_FAULT_ADDR, "ACCESS PROTECTION", IOMMU_FAULT_READ }, > + { REG_AW_FAULT_ADDR, "SECURITY PROTECTION", IOMMU_FAULT_WRITE }, > + { REG_AW_FAULT_ADDR, "ACCESS PROTECTION", IOMMU_FAULT_WRITE }, > }; > > -static const struct sysmmu_fault_info sysmmu_v5_faults[] = { > - { 0, REG_V5_FAULT_AR_VA, "AR PTW", IOMMU_FAULT_READ }, > - { 1, REG_V5_FAULT_AR_VA, "AR PAGE", IOMMU_FAULT_READ }, > - { 2, REG_V5_FAULT_AR_VA, "AR MULTI-HIT", IOMMU_FAULT_READ }, > - { 3, REG_V5_FAULT_AR_VA, "AR ACCESS PROTECTION", IOMMU_FAULT_READ }, > - { 4, REG_V5_FAULT_AR_VA, "AR SECURITY PROTECTION", IOMMU_FAULT_READ }, > - { 16, REG_V5_FAULT_AW_VA, "AW PTW", IOMMU_FAULT_WRITE }, > - { 17, REG_V5_FAULT_AW_VA, "AW PAGE", IOMMU_FAULT_WRITE }, > - { 18, REG_V5_FAULT_AW_VA, "AW MULTI-HIT", IOMMU_FAULT_WRITE }, > - { 19, REG_V5_FAULT_AW_VA, "AW ACCESS PROTECTION", IOMMU_FAULT_WRITE }, > - { 20, REG_V5_FAULT_AW_VA, "AW SECURITY PROTECTION", IOMMU_FAULT_WRITE }, > +/* SysMMU v5 has the same faults for AR (0..4 bits) and AW (16..20 bits) */ > +static const char * const sysmmu_v5_fault_names[] = { > + "PTW", > + "PAGE", > + "MULTI-HIT", > + "ACCESS PROTECTION", > + "SECURITY PROTECTION" > }; > > /* > @@ -246,9 +244,12 @@ struct exynos_iommu_domain { > struct iommu_domain domain; /* generic domain data structure */ > }; > > +struct sysmmu_drvdata; > + > /* > * SysMMU version specific data. Contains offsets for the registers which can > * be found in different SysMMU variants, but have different offset values. > + * Also contains version specific callbacks to abstract the hardware. > */ > struct sysmmu_variant { > u32 pt_base; /* page table base address (physical) */ > @@ -259,6 +260,9 @@ struct sysmmu_variant { > u32 flush_end; /* end address of range invalidation */ > u32 int_status; /* interrupt status information */ > u32 int_clear; /* clear the interrupt */ > + > + int (*get_fault_info)(struct sysmmu_drvdata *data, unsigned int itype, > + struct sysmmu_fault *fault); > }; > > /* > @@ -293,6 +297,46 @@ struct sysmmu_drvdata { > > #define SYSMMU_REG(data, reg) ((data)->sfrbase + (data)->variant->reg) > > +static int exynos_sysmmu_v1_get_fault_info(struct sysmmu_drvdata *data, > + unsigned int itype, > + struct sysmmu_fault *fault) > +{ > + const struct sysmmu_v1_fault_info *finfo; > + > + if (itype >= ARRAY_SIZE(sysmmu_v1_faults)) > + return -ENXIO; > + > + finfo = &sysmmu_v1_faults[itype]; > + fault->addr = readl(data->sfrbase + finfo->addr_reg); > + fault->name = finfo->name; > + fault->type = finfo->type; > + > + return 0; > +} > + > +static int exynos_sysmmu_v5_get_fault_info(struct sysmmu_drvdata *data, > + unsigned int itype, > + struct sysmmu_fault *fault) > +{ > + unsigned int addr_reg; > + > + if (itype < ARRAY_SIZE(sysmmu_v5_fault_names)) { > + fault->type = IOMMU_FAULT_READ; > + addr_reg = REG_V5_FAULT_AR_VA; > + } else if (itype >= 16 && itype <= 20) { > + fault->type = IOMMU_FAULT_WRITE; > + addr_reg = REG_V5_FAULT_AW_VA; > + itype -= 16; > + } else { > + return -ENXIO; > + } > + > + fault->name = sysmmu_v5_fault_names[itype]; > + fault->addr = readl(data->sfrbase + addr_reg); > + > + return 0; > +} > + > /* SysMMU v1..v3 */ > static const struct sysmmu_variant sysmmu_v1_variant = { > .flush_all = 0x0c, > @@ -300,6 +344,8 @@ static const struct sysmmu_variant sysmmu_v1_variant = { > .pt_base = 0x14, > .int_status = 0x18, > .int_clear = 0x1c, > + > + .get_fault_info = exynos_sysmmu_v1_get_fault_info, > }; > > /* SysMMU v5 and v7 (non-VM capable) */ > @@ -312,6 +358,8 @@ static const struct sysmmu_variant sysmmu_v5_variant = { > .flush_end = 0x24, > .int_status = 0x60, > .int_clear = 0x64, > + > + .get_fault_info = exynos_sysmmu_v5_get_fault_info, > }; > > /* SysMMU v7: VM capable register set */ > @@ -324,6 +372,8 @@ static const struct sysmmu_variant sysmmu_v7_vm_variant = { > .flush_end = 0x8024, > .int_status = 0x60, > .int_clear = 0x64, > + > + .get_fault_info = exynos_sysmmu_v5_get_fault_info, > }; > > static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom) > @@ -453,68 +503,56 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data) > } > > static void show_fault_information(struct sysmmu_drvdata *data, > - const struct sysmmu_fault_info *finfo, > - sysmmu_iova_t fault_addr) > + const struct sysmmu_fault *fault) > { > sysmmu_pte_t *ent; > > - dev_err(data->sysmmu, "%s: %s FAULT occurred at %#x\n", > - dev_name(data->master), finfo->name, fault_addr); > + dev_err(data->sysmmu, "%s: [%s] %s FAULT occurred at %#x\n", > + dev_name(data->master), > + fault->type == IOMMU_FAULT_READ ? "READ" : "WRITE", > + fault->name, fault->addr); > dev_dbg(data->sysmmu, "Page table base: %pa\n", &data->pgtable); > - ent = section_entry(phys_to_virt(data->pgtable), fault_addr); > + ent = section_entry(phys_to_virt(data->pgtable), fault->addr); > dev_dbg(data->sysmmu, "\tLv1 entry: %#x\n", *ent); > if (lv1ent_page(ent)) { > - ent = page_entry(ent, fault_addr); > + ent = page_entry(ent, fault->addr); > dev_dbg(data->sysmmu, "\t Lv2 entry: %#x\n", *ent); > } > } > > static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id) > { > - /* SYSMMU is in blocked state when interrupt occurred. */ > struct sysmmu_drvdata *data = dev_id; > - const struct sysmmu_fault_info *finfo; > - unsigned int i, n, itype; > - sysmmu_iova_t fault_addr; > + unsigned int itype; > + struct sysmmu_fault fault; > int ret = -ENOSYS; > > WARN_ON(!data->active); > > - if (MMU_MAJ_VER(data->version) < 5) { > - finfo = sysmmu_faults; > - n = ARRAY_SIZE(sysmmu_faults); > - } else { > - finfo = sysmmu_v5_faults; > - n = ARRAY_SIZE(sysmmu_v5_faults); > - } > - > spin_lock(&data->lock); > - > clk_enable(data->clk_master); > > itype = __ffs(readl(SYSMMU_REG(data, int_status))); > - for (i = 0; i < n; i++, finfo++) > - if (finfo->bit == itype) > - break; > - /* unknown/unsupported fault */ > - BUG_ON(i == n); > - > - /* print debug message */ > - fault_addr = readl(data->sfrbase + finfo->addr_reg); > - show_fault_information(data, finfo, fault_addr); > - > - if (data->domain) > - ret = report_iommu_fault(&data->domain->domain, > - data->master, fault_addr, finfo->type); > - /* fault is not recovered by fault handler */ > - BUG_ON(ret != 0); > + ret = data->variant->get_fault_info(data, itype, &fault); > + if (ret) { > + dev_err(data->sysmmu, "Unhandled interrupt bit %u\n", itype); > + goto out; > + } > + show_fault_information(data, &fault); > > + if (data->domain) { > + ret = report_iommu_fault(&data->domain->domain, data->master, > + fault.addr, fault.type); > + } > + if (ret) > + panic("Unrecoverable System MMU Fault!"); > + > +out: > writel(1 << itype, SYSMMU_REG(data, int_clear)); > > + /* SysMMU is in blocked state when interrupt occurred */ > sysmmu_unblock(data); > - > clk_disable(data->clk_master); > - > spin_unlock(&data->lock); > > return IRQ_HANDLED; Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] iommu/exynos: Abstract getting the fault info 2022-08-12 12:25 ` Marek Szyprowski @ 2022-10-24 14:43 ` Sam Protsenko 2022-12-21 21:32 ` Sam Protsenko 0 siblings, 1 reply; 12+ messages in thread From: Sam Protsenko @ 2022-10-24 14:43 UTC (permalink / raw) To: Marek Szyprowski Cc: Krzysztof Kozlowski, Joerg Roedel, Will Deacon, Robin Murphy, Janghyuck Kim, Cho KyongHo, Daniel Mentz, David Virag, Sumit Semwal, iommu, linux-arm-kernel, linux-samsung-soc, linux-kernel Hi Marek, On Fri, 12 Aug 2022 at 14:25, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Hi Sam, > [snip] > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > > I'm not very happy with converting the sysmmu_fault_info arrays into the > decoding functions. If I got the code right, adding v7 is still possible > with the current approach. The main advantage of the array-based > approach is readability and keeping all the information together in a > single place. > > I agree for the items listed above as 'minor functional changes', > though. Those sysmmu_fault_info arrays might be a part of sysmmu hw > variant to avoid decoding hw version for each fault. > > I'm not sure that the linear scan is so problematic with regards to the > performance. You really don't want your drivers to trigger IOMMU fault > so often during normal operation. It is just a way to get some debugging > information or handle some exception. > > You mentioned that the transaction type is read from the separate > register in case of v7, but your code (here and in second patch) still > relies on the reported interrupt bits. > > Could you try to rework all your changes in a such way, that the > sysmmu_fault_info arrays are still used? V7 is really very similar to > the v5 already supported by the current driver. > That's actually how I implemented this patch on my first attempt. Really didn't like it, because a half of existing sysmmu_fault_info structure doesn't make sense for v7, and some functionality of v7 has to be implemented separately from that structure. I'd argue that previous abstraction is just broken, and doesn't work for all SysMMU versions anymore. It's easy to see how much difference between v5 and v7, just by looking at corresponding get_fault_info() functions I implemented. For example, the transaction type is probed from different registers using different version, etc. There is also the need to handle new VM/non-VM registers on v7. Also there is some extra functionality that will be added later, like multiple translation domains support, which is also quite different from how things done for v5. I'd show more specifics to demonstrate my statements above, but alas I already deleted my initial implementation (which was exactly what you suggest). This callback-style HAL seems to be a perfect choice, and I spent several days just experimenting with different approaches and seeing all pros and cons. And from my point of view, this way is the best for providing actual solid abstraction, which doesn't require adding any workarounds on top of that. I understand that my patch changes the very conception of how IRQ is handled in this driver, but I'm still convinced it's a proper way to do that for all v1/v5/v7, especially w.r.t. further v7 additions, to keep the abstraction solid. Not that I'm lazy and don't want to rework things :) But in this particular case I'd go with unchanged patches. Do you think it's reasonable to take this series as is? I can try and collect more particular code snippets to demonstrate my point, if you like. Thanks! [snip] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] iommu/exynos: Abstract getting the fault info 2022-10-24 14:43 ` Sam Protsenko @ 2022-12-21 21:32 ` Sam Protsenko 2022-12-22 13:20 ` Marek Szyprowski 0 siblings, 1 reply; 12+ messages in thread From: Sam Protsenko @ 2022-12-21 21:32 UTC (permalink / raw) To: Marek Szyprowski Cc: Krzysztof Kozlowski, Joerg Roedel, Will Deacon, Robin Murphy, Janghyuck Kim, Cho KyongHo, Daniel Mentz, David Virag, Sumit Semwal, iommu, linux-arm-kernel, linux-samsung-soc, linux-kernel Hi Marek, On Mon, 24 Oct 2022 at 09:43, Sam Protsenko <semen.protsenko@linaro.org> wrote: > > Hi Marek, > > On Fri, 12 Aug 2022 at 14:25, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > > > Hi Sam, > > > > [snip] > > > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > > > > I'm not very happy with converting the sysmmu_fault_info arrays into the > > decoding functions. If I got the code right, adding v7 is still possible > > with the current approach. The main advantage of the array-based > > approach is readability and keeping all the information together in a > > single place. > > > > I agree for the items listed above as 'minor functional changes', > > though. Those sysmmu_fault_info arrays might be a part of sysmmu hw > > variant to avoid decoding hw version for each fault. > > > > I'm not sure that the linear scan is so problematic with regards to the > > performance. You really don't want your drivers to trigger IOMMU fault > > so often during normal operation. It is just a way to get some debugging > > information or handle some exception. > > > > You mentioned that the transaction type is read from the separate > > register in case of v7, but your code (here and in second patch) still > > relies on the reported interrupt bits. > > > > Could you try to rework all your changes in a such way, that the > > sysmmu_fault_info arrays are still used? V7 is really very similar to > > the v5 already supported by the current driver. > > > > That's actually how I implemented this patch on my first attempt. > Really didn't like it, because a half of existing sysmmu_fault_info > structure doesn't make sense for v7, and some functionality of v7 has > to be implemented separately from that structure. I'd argue that > previous abstraction is just broken, and doesn't work for all SysMMU > versions anymore. It's easy to see how much difference between v5 and > v7, just by looking at corresponding get_fault_info() functions I > implemented. For example, the transaction type is probed from > different registers using different version, etc. There is also the > need to handle new VM/non-VM registers on v7. Also there is some extra > functionality that will be added later, like multiple translation > domains support, which is also quite different from how things done > for v5. > > I'd show more specifics to demonstrate my statements above, but alas I > already deleted my initial implementation (which was exactly what you > suggest). This callback-style HAL seems to be a perfect choice, and I > spent several days just experimenting with different approaches and > seeing all pros and cons. And from my point of view, this way is the > best for providing actual solid abstraction, which doesn't require > adding any workarounds on top of that. I understand that my patch > changes the very conception of how IRQ is handled in this driver, but > I'm still convinced it's a proper way to do that for all v1/v5/v7, > especially w.r.t. further v7 additions, to keep the abstraction solid. > Not that I'm lazy and don't want to rework things :) But in this > particular case I'd go with unchanged patches. > > Do you think it's reasonable to take this series as is? I can try and > collect more particular code snippets to demonstrate my point, if you > like. > > Thanks! > So, what do you think about this? > [snip] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] iommu/exynos: Abstract getting the fault info 2022-12-21 21:32 ` Sam Protsenko @ 2022-12-22 13:20 ` Marek Szyprowski 2022-12-22 15:23 ` Sam Protsenko 0 siblings, 1 reply; 12+ messages in thread From: Marek Szyprowski @ 2022-12-22 13:20 UTC (permalink / raw) To: Sam Protsenko Cc: Krzysztof Kozlowski, Joerg Roedel, Will Deacon, Robin Murphy, Janghyuck Kim, Cho KyongHo, Daniel Mentz, David Virag, Sumit Semwal, iommu, linux-arm-kernel, linux-samsung-soc, linux-kernel On 21.12.2022 22:32, Sam Protsenko wrote: > Hi Marek, > > On Mon, 24 Oct 2022 at 09:43, Sam Protsenko <semen.protsenko@linaro.org> wrote: >> Hi Marek, >> >> On Fri, 12 Aug 2022 at 14:25, Marek Szyprowski <m.szyprowski@samsung.com> wrote: >>> Hi Sam, >> [snip] >> >>>> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> >>> I'm not very happy with converting the sysmmu_fault_info arrays into the >>> decoding functions. If I got the code right, adding v7 is still possible >>> with the current approach. The main advantage of the array-based >>> approach is readability and keeping all the information together in a >>> single place. >>> >>> I agree for the items listed above as 'minor functional changes', >>> though. Those sysmmu_fault_info arrays might be a part of sysmmu hw >>> variant to avoid decoding hw version for each fault. >>> >>> I'm not sure that the linear scan is so problematic with regards to the >>> performance. You really don't want your drivers to trigger IOMMU fault >>> so often during normal operation. It is just a way to get some debugging >>> information or handle some exception. >>> >>> You mentioned that the transaction type is read from the separate >>> register in case of v7, but your code (here and in second patch) still >>> relies on the reported interrupt bits. >>> >>> Could you try to rework all your changes in a such way, that the >>> sysmmu_fault_info arrays are still used? V7 is really very similar to >>> the v5 already supported by the current driver. >>> >> That's actually how I implemented this patch on my first attempt. >> Really didn't like it, because a half of existing sysmmu_fault_info >> structure doesn't make sense for v7, and some functionality of v7 has >> to be implemented separately from that structure. I'd argue that >> previous abstraction is just broken, and doesn't work for all SysMMU >> versions anymore. It's easy to see how much difference between v5 and >> v7, just by looking at corresponding get_fault_info() functions I >> implemented. For example, the transaction type is probed from >> different registers using different version, etc. There is also the >> need to handle new VM/non-VM registers on v7. Also there is some extra >> functionality that will be added later, like multiple translation >> domains support, which is also quite different from how things done >> for v5. >> >> I'd show more specifics to demonstrate my statements above, but alas I >> already deleted my initial implementation (which was exactly what you >> suggest). This callback-style HAL seems to be a perfect choice, and I >> spent several days just experimenting with different approaches and >> seeing all pros and cons. And from my point of view, this way is the >> best for providing actual solid abstraction, which doesn't require >> adding any workarounds on top of that. I understand that my patch >> changes the very conception of how IRQ is handled in this driver, but >> I'm still convinced it's a proper way to do that for all v1/v5/v7, >> especially w.r.t. further v7 additions, to keep the abstraction solid. >> Not that I'm lazy and don't want to rework things :) But in this >> particular case I'd go with unchanged patches. >> >> Do you think it's reasonable to take this series as is? I can try and >> collect more particular code snippets to demonstrate my point, if you >> like. >> >> Thanks! > So, what do you think about this? Okay, go ahead with your approach. If I find a better way, I will rework it then. I would just like to have the code for fault handling for hw v1, v5 and v7 similar as much as possible. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] iommu/exynos: Abstract getting the fault info 2022-12-22 13:20 ` Marek Szyprowski @ 2022-12-22 15:23 ` Sam Protsenko 2022-12-22 15:34 ` Marek Szyprowski 0 siblings, 1 reply; 12+ messages in thread From: Sam Protsenko @ 2022-12-22 15:23 UTC (permalink / raw) To: Marek Szyprowski, Joerg Roedel Cc: Krzysztof Kozlowski, Will Deacon, Robin Murphy, Janghyuck Kim, Cho KyongHo, Daniel Mentz, David Virag, Sumit Semwal, iommu, linux-arm-kernel, linux-samsung-soc, linux-kernel On Thu, 22 Dec 2022 at 07:20, Marek Szyprowski <m.szyprowski@samsung.com> wrote: [snip] > > So, what do you think about this? > > Okay, go ahead with your approach. If I find a better way, I will rework > it then. I would just like to have the code for fault handling for hw > v1, v5 and v7 similar as much as possible. > Thanks, Marek! Joerg, can you please apply this series? Please let me know if I need to rebase it first, but I guess there shouldn't be any issues, the SysMMU driver doesn't seem to get updated often. > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] iommu/exynos: Abstract getting the fault info 2022-12-22 15:23 ` Sam Protsenko @ 2022-12-22 15:34 ` Marek Szyprowski 2023-01-21 20:28 ` Sam Protsenko 0 siblings, 1 reply; 12+ messages in thread From: Marek Szyprowski @ 2022-12-22 15:34 UTC (permalink / raw) To: Sam Protsenko, Joerg Roedel Cc: Krzysztof Kozlowski, Will Deacon, Robin Murphy, Janghyuck Kim, Cho KyongHo, Daniel Mentz, David Virag, Sumit Semwal, iommu, linux-arm-kernel, linux-samsung-soc, linux-kernel On 22.12.2022 16:23, Sam Protsenko wrote: > On Thu, 22 Dec 2022 at 07:20, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > [snip] > >>> So, what do you think about this? >> Okay, go ahead with your approach. If I find a better way, I will rework >> it then. I would just like to have the code for fault handling for hw >> v1, v5 and v7 similar as much as possible. >> > Thanks, Marek! > > Joerg, can you please apply this series? Please let me know if I need > to rebase it first, but I guess there shouldn't be any issues, the > SysMMU driver doesn't seem to get updated often. Just to makes things a bit more formal - feel free to add: Acked-by: Marek Szyprowski <m.szyprowski@samsung.com> Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] iommu/exynos: Abstract getting the fault info 2022-12-22 15:34 ` Marek Szyprowski @ 2023-01-21 20:28 ` Sam Protsenko 0 siblings, 0 replies; 12+ messages in thread From: Sam Protsenko @ 2023-01-21 20:28 UTC (permalink / raw) To: Joerg Roedel, Joerg Roedel Cc: Krzysztof Kozlowski, Will Deacon, Robin Murphy, Janghyuck Kim, Cho KyongHo, Daniel Mentz, David Virag, Sumit Semwal, iommu, linux-arm-kernel, linux-samsung-soc, linux-kernel, Marek Szyprowski Hi Joerg, On Thu, 22 Dec 2022 at 09:34, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > On 22.12.2022 16:23, Sam Protsenko wrote: > > On Thu, 22 Dec 2022 at 07:20, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > > > [snip] > > > >>> So, what do you think about this? > >> Okay, go ahead with your approach. If I find a better way, I will rework > >> it then. I would just like to have the code for fault handling for hw > >> v1, v5 and v7 similar as much as possible. > >> > > Thanks, Marek! > > > > Joerg, can you please apply this series? Please let me know if I need > > to rebase it first, but I guess there shouldn't be any issues, the > > SysMMU driver doesn't seem to get updated often. > > Just to makes things a bit more formal - feel free to add: > > Acked-by: Marek Szyprowski <m.szyprowski@samsung.com> > Can you please apply this series? Thanks! > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] iommu/exynos: Implement fault handling on SysMMU v7 2022-07-26 20:07 [PATCH 0/2] iommu/exynos: Add fault handling on SysMMU v7 Sam Protsenko 2022-07-26 20:07 ` [PATCH 1/2] iommu/exynos: Abstract getting the fault info Sam Protsenko @ 2022-07-26 20:07 ` Sam Protsenko 2022-08-08 17:18 ` [PATCH 0/2] iommu/exynos: Add " Sam Protsenko 2023-01-25 11:05 ` Joerg Roedel 3 siblings, 0 replies; 12+ messages in thread From: Sam Protsenko @ 2022-07-26 20:07 UTC (permalink / raw) To: Marek Szyprowski, Krzysztof Kozlowski Cc: Joerg Roedel, Will Deacon, Robin Murphy, Janghyuck Kim, Cho KyongHo, Daniel Mentz, David Virag, Sumit Semwal, iommu, linux-arm-kernel, linux-samsung-soc, linux-kernel SysMMU v7 has a bit different registers for getting the fault info: - there is one single register (MMU_FAULT_VA) to get the fault address - fault access type (R/W) can be read from MMU_FAULT_TRANS_INFO register now - interrupt status register has different bits w.r.t. previous SysMMU versions - VM and non-VM layouts have different register addresses Add correct fault handling implementation for SysMMU v7, according to all mentioned differences. Only VID #0 (default) is handled, as VM domains support is not implemented yet. Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> --- drivers/iommu/exynos-iommu.c | 48 +++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c index 766d409e084a..ac47c796741b 100644 --- a/drivers/iommu/exynos-iommu.c +++ b/drivers/iommu/exynos-iommu.c @@ -217,6 +217,13 @@ static const char * const sysmmu_v5_fault_names[] = { "SECURITY PROTECTION" }; +static const char * const sysmmu_v7_fault_names[] = { + "PTW", + "PAGE", + "ACCESS PROTECTION", + "RESERVED" +}; + /* * This structure is attached to dev->iommu->priv of the master device * on device add, contains a list of SYSMMU controllers defined by device tree, @@ -260,6 +267,8 @@ struct sysmmu_variant { u32 flush_end; /* end address of range invalidation */ u32 int_status; /* interrupt status information */ u32 int_clear; /* clear the interrupt */ + u32 fault_va; /* IOVA address that caused fault */ + u32 fault_info; /* fault transaction info */ int (*get_fault_info)(struct sysmmu_drvdata *data, unsigned int itype, struct sysmmu_fault *fault); @@ -337,6 +346,19 @@ static int exynos_sysmmu_v5_get_fault_info(struct sysmmu_drvdata *data, return 0; } +static int exynos_sysmmu_v7_get_fault_info(struct sysmmu_drvdata *data, + unsigned int itype, + struct sysmmu_fault *fault) +{ + u32 info = readl(SYSMMU_REG(data, fault_info)); + + fault->addr = readl(SYSMMU_REG(data, fault_va)); + fault->name = sysmmu_v7_fault_names[itype % 4]; + fault->type = (info & BIT(20)) ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ; + + return 0; +} + /* SysMMU v1..v3 */ static const struct sysmmu_variant sysmmu_v1_variant = { .flush_all = 0x0c, @@ -348,7 +370,7 @@ static const struct sysmmu_variant sysmmu_v1_variant = { .get_fault_info = exynos_sysmmu_v1_get_fault_info, }; -/* SysMMU v5 and v7 (non-VM capable) */ +/* SysMMU v5 */ static const struct sysmmu_variant sysmmu_v5_variant = { .pt_base = 0x0c, .flush_all = 0x10, @@ -362,7 +384,23 @@ static const struct sysmmu_variant sysmmu_v5_variant = { .get_fault_info = exynos_sysmmu_v5_get_fault_info, }; -/* SysMMU v7: VM capable register set */ +/* SysMMU v7: non-VM capable register layout */ +static const struct sysmmu_variant sysmmu_v7_variant = { + .pt_base = 0x0c, + .flush_all = 0x10, + .flush_entry = 0x14, + .flush_range = 0x18, + .flush_start = 0x20, + .flush_end = 0x24, + .int_status = 0x60, + .int_clear = 0x64, + .fault_va = 0x70, + .fault_info = 0x78, + + .get_fault_info = exynos_sysmmu_v7_get_fault_info, +}; + +/* SysMMU v7: VM capable register layout */ static const struct sysmmu_variant sysmmu_v7_vm_variant = { .pt_base = 0x800c, .flush_all = 0x8010, @@ -372,8 +410,10 @@ static const struct sysmmu_variant sysmmu_v7_vm_variant = { .flush_end = 0x8024, .int_status = 0x60, .int_clear = 0x64, + .fault_va = 0x1000, + .fault_info = 0x1004, - .get_fault_info = exynos_sysmmu_v5_get_fault_info, + .get_fault_info = exynos_sysmmu_v7_get_fault_info, }; static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom) @@ -496,7 +536,7 @@ static void __sysmmu_get_version(struct sysmmu_drvdata *data) if (data->has_vcr) data->variant = &sysmmu_v7_vm_variant; else - data->variant = &sysmmu_v5_variant; + data->variant = &sysmmu_v7_variant; } __sysmmu_disable_clocks(data); -- 2.30.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] iommu/exynos: Add fault handling on SysMMU v7 2022-07-26 20:07 [PATCH 0/2] iommu/exynos: Add fault handling on SysMMU v7 Sam Protsenko 2022-07-26 20:07 ` [PATCH 1/2] iommu/exynos: Abstract getting the fault info Sam Protsenko 2022-07-26 20:07 ` [PATCH 2/2] iommu/exynos: Implement fault handling on SysMMU v7 Sam Protsenko @ 2022-08-08 17:18 ` Sam Protsenko 2023-01-25 11:05 ` Joerg Roedel 3 siblings, 0 replies; 12+ messages in thread From: Sam Protsenko @ 2022-08-08 17:18 UTC (permalink / raw) To: Marek Szyprowski Cc: Joerg Roedel, Will Deacon, Robin Murphy, Janghyuck Kim, Cho KyongHo, Daniel Mentz, David Virag, Sumit Semwal, iommu, linux-arm-kernel, linux-samsung-soc, linux-kernel, Krzysztof Kozlowski On Tue, 26 Jul 2022 at 23:07, Sam Protsenko <semen.protsenko@linaro.org> wrote: > > Exynos IOMMU driver implements fault handling for SysMMU v1..v5. But the > abstraction currently used is not suited for SysMMU v7, as it has quite > different fault related register set. > > This patch series reworks the mentioned fault handling abstraction and > adds fault handling support for SysMMU v7. > > Sam Protsenko (2): > iommu/exynos: Abstract getting the fault info > iommu/exynos: Implement fault handling on SysMMU v7 > > drivers/iommu/exynos-iommu.c | 208 ++++++++++++++++++++++++----------- > 1 file changed, 143 insertions(+), 65 deletions(-) > > -- > 2.30.2 > Hi Marek, Can you please review and test this series? I only have my E850 (which has VM-capable SysMMU register layout). So it would be nice to check if it works fine with non-VM SysMMU v7 and older versions. Thanks! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] iommu/exynos: Add fault handling on SysMMU v7 2022-07-26 20:07 [PATCH 0/2] iommu/exynos: Add fault handling on SysMMU v7 Sam Protsenko ` (2 preceding siblings ...) 2022-08-08 17:18 ` [PATCH 0/2] iommu/exynos: Add " Sam Protsenko @ 2023-01-25 11:05 ` Joerg Roedel 3 siblings, 0 replies; 12+ messages in thread From: Joerg Roedel @ 2023-01-25 11:05 UTC (permalink / raw) To: Sam Protsenko Cc: Marek Szyprowski, Krzysztof Kozlowski, Will Deacon, Robin Murphy, Janghyuck Kim, Cho KyongHo, Daniel Mentz, David Virag, Sumit Semwal, iommu, linux-arm-kernel, linux-samsung-soc, linux-kernel On Tue, Jul 26, 2022 at 11:07:37PM +0300, Sam Protsenko wrote: > Sam Protsenko (2): > iommu/exynos: Abstract getting the fault info > iommu/exynos: Implement fault handling on SysMMU v7 Applied, thanks. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-01-25 11:06 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-07-26 20:07 [PATCH 0/2] iommu/exynos: Add fault handling on SysMMU v7 Sam Protsenko 2022-07-26 20:07 ` [PATCH 1/2] iommu/exynos: Abstract getting the fault info Sam Protsenko 2022-08-12 12:25 ` Marek Szyprowski 2022-10-24 14:43 ` Sam Protsenko 2022-12-21 21:32 ` Sam Protsenko 2022-12-22 13:20 ` Marek Szyprowski 2022-12-22 15:23 ` Sam Protsenko 2022-12-22 15:34 ` Marek Szyprowski 2023-01-21 20:28 ` Sam Protsenko 2022-07-26 20:07 ` [PATCH 2/2] iommu/exynos: Implement fault handling on SysMMU v7 Sam Protsenko 2022-08-08 17:18 ` [PATCH 0/2] iommu/exynos: Add " Sam Protsenko 2023-01-25 11:05 ` Joerg Roedel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).