* [PATCH v2 0/1] OMAP: IOMMU fault callback support @ 2011-02-15 14:36 David Cohen 2011-02-15 14:36 ` [PATCH v2 1/1] OMAP: IOMMU: add support to callback during fault handling David Cohen 0 siblings, 1 reply; 5+ messages in thread From: David Cohen @ 2011-02-15 14:36 UTC (permalink / raw) To: linux-arm-kernel Hi, v2: Previous patch 1/2 on v1 was dropped. --- v1: This patch set adds fault callback support to allow IOMMU users to debug or react when a fault happens. IOMMU faults might be very difficult to reproduce and then to figure out the source of the problem. Currently IOMMU driver prints not so useful debug message and does not notice user about such issue. With a fault callback, IOMMU user may debug much more useful information and/or react to go back to a valid state. Br, David --- David Cohen (1): OMAP: IOMMU: add support to callback during fault handling arch/arm/mach-omap2/iommu2.c | 21 +++++++++++++-- arch/arm/plat-omap/include/plat/iommu.h | 15 ++++++++++- arch/arm/plat-omap/iommu.c | 41 ++++++++++++++++++++++++++++--- 3 files changed, 69 insertions(+), 8 deletions(-) -- 1.7.2.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/1] OMAP: IOMMU: add support to callback during fault handling 2011-02-15 14:36 [PATCH v2 0/1] OMAP: IOMMU fault callback support David Cohen @ 2011-02-15 14:36 ` David Cohen 2011-02-15 14:48 ` Hiroshi DOYU 0 siblings, 1 reply; 5+ messages in thread From: David Cohen @ 2011-02-15 14:36 UTC (permalink / raw) To: linux-arm-kernel Add support to register a callback for IOMMU fault situations. Drivers using IOMMU module might want to be informed when such errors happen in order to debug it or react. Signed-off-by: David Cohen <dacohen@gmail.com> --- arch/arm/mach-omap2/iommu2.c | 21 +++++++++++++-- arch/arm/plat-omap/include/plat/iommu.h | 15 ++++++++++- arch/arm/plat-omap/iommu.c | 41 ++++++++++++++++++++++++++++--- 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c index 14ee686..504310d 100644 --- a/arch/arm/mach-omap2/iommu2.c +++ b/arch/arm/mach-omap2/iommu2.c @@ -143,10 +143,10 @@ static void omap2_iommu_set_twl(struct iommu *obj, bool on) __iommu_set_twl(obj, false); } -static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) +static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra, u32 *iommu_errs) { int i; - u32 stat, da; + u32 stat, da, errs; const char *err_msg[] = { "tlb miss", "translation fault", @@ -157,8 +157,10 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) stat = iommu_read_reg(obj, MMU_IRQSTATUS); stat &= MMU_IRQ_MASK; - if (!stat) + if (!stat) { + *iommu_errs = 0; return 0; + } da = iommu_read_reg(obj, MMU_FAULT_AD); *ra = da; @@ -171,6 +173,19 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) } printk("\n"); + errs = 0; + if (stat & MMU_IRQ_TLBMISS) + errs |= OMAP_IOMMU_ERR_TLB_MISS; + if (stat & MMU_IRQ_TRANSLATIONFAULT) + errs |= OMAP_IOMMU_ERR_TRANS_FAULT; + if (stat & MMU_IRQ_EMUMISS) + errs |= OMAP_IOMMU_ERR_EMU_MISS; + if (stat & MMU_IRQ_TABLEWALKFAULT) + errs |= OMAP_IOMMU_ERR_TBLWALK_FAULT; + if (stat & MMU_IRQ_MULTIHITFAULT) + errs |= OMAP_IOMMU_ERR_MULTIHIT_FAULT; + *iommu_errs = errs; + iommu_write_reg(obj, stat, MMU_IRQSTATUS); return stat; diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h index 19cbb5e..5a2475f 100644 --- a/arch/arm/plat-omap/include/plat/iommu.h +++ b/arch/arm/plat-omap/include/plat/iommu.h @@ -31,6 +31,7 @@ struct iommu { struct clk *clk; void __iomem *regbase; struct device *dev; + void *fault_cb_priv; unsigned int refcount; struct mutex iommu_lock; /* global for this whole object */ @@ -48,6 +49,7 @@ struct iommu { struct mutex mmap_lock; /* protect mmap */ int (*isr)(struct iommu *obj); + void (*fault_cb)(struct iommu *obj, u32 da, u32 iommu_errs, void *priv); void *ctx; /* iommu context: registres saved area */ u32 da_start; @@ -83,7 +85,7 @@ struct iommu_functions { int (*enable)(struct iommu *obj); void (*disable)(struct iommu *obj); void (*set_twl)(struct iommu *obj, bool on); - u32 (*fault_isr)(struct iommu *obj, u32 *ra); + u32 (*fault_isr)(struct iommu *obj, u32 *ra, u32 *iommu_errs); void (*tlb_read_cr)(struct iommu *obj, struct cr_regs *cr); void (*tlb_load_cr)(struct iommu *obj, struct cr_regs *cr); @@ -109,6 +111,13 @@ struct iommu_platform_data { u32 da_end; }; +/* IOMMU errors */ +#define OMAP_IOMMU_ERR_TLB_MISS (1 << 0) +#define OMAP_IOMMU_ERR_TRANS_FAULT (1 << 1) +#define OMAP_IOMMU_ERR_EMU_MISS (1 << 2) +#define OMAP_IOMMU_ERR_TBLWALK_FAULT (1 << 3) +#define OMAP_IOMMU_ERR_MULTIHIT_FAULT (1 << 4) + #if defined(CONFIG_ARCH_OMAP1) #error "iommu for this processor not implemented yet" #else @@ -161,6 +170,10 @@ extern size_t iopgtable_clear_entry(struct iommu *obj, u32 iova); extern int iommu_set_da_range(struct iommu *obj, u32 start, u32 end); extern struct iommu *iommu_get(const char *name); extern void iommu_put(struct iommu *obj); +extern int iommu_set_fault_callback(const char *name, + void (*fault_cb)(struct iommu *obj, u32 da, + u32 errs, void *priv), + void *fault_cb_priv); extern void iommu_save_ctx(struct iommu *obj); extern void iommu_restore_ctx(struct iommu *obj); diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c index b1107c0..7f780ee 100644 --- a/arch/arm/plat-omap/iommu.c +++ b/arch/arm/plat-omap/iommu.c @@ -163,9 +163,9 @@ static u32 get_iopte_attr(struct iotlb_entry *e) return arch_iommu->get_pte_attr(e); } -static u32 iommu_report_fault(struct iommu *obj, u32 *da) +static u32 iommu_report_fault(struct iommu *obj, u32 *da, u32 *iommu_errs) { - return arch_iommu->fault_isr(obj, da); + return arch_iommu->fault_isr(obj, da, iommu_errs); } static void iotlb_lock_get(struct iommu *obj, struct iotlb_lock *l) @@ -780,7 +780,7 @@ static void iopgtable_clear_entry_all(struct iommu *obj) */ static irqreturn_t iommu_fault_handler(int irq, void *data) { - u32 stat, da; + u32 stat, da, errs; u32 *iopgd, *iopte; int err = -EIO; struct iommu *obj = data; @@ -796,13 +796,19 @@ static irqreturn_t iommu_fault_handler(int irq, void *data) return IRQ_HANDLED; clk_enable(obj->clk); - stat = iommu_report_fault(obj, &da); + stat = iommu_report_fault(obj, &da, &errs); clk_disable(obj->clk); if (!stat) return IRQ_HANDLED; iommu_disable(obj); + if (obj->fault_cb) { + obj->fault_cb(obj, da, errs, obj->fault_cb_priv); + /* No need to print error message as callback is called */ + return IRQ_NONE; + } + iopgd = iopgd_offset(obj, da); if (!iopgd_is_table(*iopgd)) { @@ -917,6 +923,33 @@ void iommu_put(struct iommu *obj) } EXPORT_SYMBOL_GPL(iommu_put); +int iommu_set_fault_callback(const char *name, + void (*fault_cb)(struct iommu *obj, u32 da, + u32 iommu_errs, void *priv), + void *fault_cb_priv) +{ + struct device *dev; + struct iommu *obj; + + dev = driver_find_device(&omap_iommu_driver.driver, NULL, (void *)name, + device_match_by_alias); + if (!dev) + return -ENODEV; + + obj = to_iommu(dev); + mutex_lock(&obj->iommu_lock); + if (obj->refcount != 0) { + mutex_unlock(&obj->iommu_lock); + return -EBUSY; + } + obj->fault_cb = fault_cb; + obj->fault_cb_priv = fault_cb_priv; + mutex_unlock(&obj->iommu_lock); + + return 0; +} +EXPORT_SYMBOL_GPL(iommu_set_fault_callback); + /* * OMAP Device MMU(IOMMU) detection */ -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 1/1] OMAP: IOMMU: add support to callback during fault handling 2011-02-15 14:36 ` [PATCH v2 1/1] OMAP: IOMMU: add support to callback during fault handling David Cohen @ 2011-02-15 14:48 ` Hiroshi DOYU 2011-02-15 15:10 ` David Cohen 0 siblings, 1 reply; 5+ messages in thread From: Hiroshi DOYU @ 2011-02-15 14:48 UTC (permalink / raw) To: linux-arm-kernel Hi David, Sorry for a bit late reply.... From: David Cohen <dacohen@gmail.com> Subject: [PATCH v2 1/1] OMAP: IOMMU: add support to callback during fault handling Date: Tue, 15 Feb 2011 16:36:31 +0200 > Add support to register a callback for IOMMU fault situations. Drivers using > IOMMU module might want to be informed when such errors happen in order to > debug it or react. > > Signed-off-by: David Cohen <dacohen@gmail.com> > --- > arch/arm/mach-omap2/iommu2.c | 21 +++++++++++++-- > arch/arm/plat-omap/include/plat/iommu.h | 15 ++++++++++- > arch/arm/plat-omap/iommu.c | 41 ++++++++++++++++++++++++++++--- > 3 files changed, 69 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c > index 14ee686..504310d 100644 > --- a/arch/arm/mach-omap2/iommu2.c > +++ b/arch/arm/mach-omap2/iommu2.c > @@ -143,10 +143,10 @@ static void omap2_iommu_set_twl(struct iommu *obj, bool on) > __iommu_set_twl(obj, false); > } > > -static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) > +static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra, u32 *iommu_errs) > { > int i; > - u32 stat, da; > + u32 stat, da, errs; > const char *err_msg[] = { > "tlb miss", > "translation fault", > @@ -157,8 +157,10 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) > > stat = iommu_read_reg(obj, MMU_IRQSTATUS); > stat &= MMU_IRQ_MASK; > - if (!stat) > + if (!stat) { > + *iommu_errs = 0; > return 0; > + } > > da = iommu_read_reg(obj, MMU_FAULT_AD); > *ra = da; > @@ -171,6 +173,19 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) > } > printk("\n"); > > + errs = 0; > + if (stat & MMU_IRQ_TLBMISS) > + errs |= OMAP_IOMMU_ERR_TLB_MISS; > + if (stat & MMU_IRQ_TRANSLATIONFAULT) > + errs |= OMAP_IOMMU_ERR_TRANS_FAULT; > + if (stat & MMU_IRQ_EMUMISS) > + errs |= OMAP_IOMMU_ERR_EMU_MISS; > + if (stat & MMU_IRQ_TABLEWALKFAULT) > + errs |= OMAP_IOMMU_ERR_TBLWALK_FAULT; > + if (stat & MMU_IRQ_MULTIHITFAULT) > + errs |= OMAP_IOMMU_ERR_MULTIHIT_FAULT; > + *iommu_errs = errs; > + > iommu_write_reg(obj, stat, MMU_IRQSTATUS); > > return stat; > diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h > index 19cbb5e..5a2475f 100644 > --- a/arch/arm/plat-omap/include/plat/iommu.h > +++ b/arch/arm/plat-omap/include/plat/iommu.h > @@ -31,6 +31,7 @@ struct iommu { > struct clk *clk; > void __iomem *regbase; > struct device *dev; > + void *fault_cb_priv; > > unsigned int refcount; > struct mutex iommu_lock; /* global for this whole object */ > @@ -48,6 +49,7 @@ struct iommu { > struct mutex mmap_lock; /* protect mmap */ > > int (*isr)(struct iommu *obj); > + void (*fault_cb)(struct iommu *obj, u32 da, u32 iommu_errs, void *priv); What about making use of (*isr)() for fault call back as well? Basic concept is that, client can decide how to deal with iommu fault. For example, for advanced case, client wants daynamic loading of TLB(or PTE), or for ISP case, clients just want more appropriate fault reporting. This (*isr)() could be used in such flexibility. 785 * Device IOMMU generic operations 786 */ 787 static irqreturn_t iommu_fault_handler(int irq, void *data) 788 { 789 u32 stat, da; 790 u32 *iopgd, *iopte; 791 int err = -EIO; 792 struct iommu *obj = data; 793 794 if (!obj->refcount) 795 return IRQ_NONE; 796 797 /* Dynamic loading TLB or PTE */ 798 if (obj->isr) 799 err = obj->isr(obj); 800 801 if (!err) 802 return IRQ_HANDLED; 803 804 clk_enable(obj->clk); 805 stat = iommu_report_fault(obj, &da); 806 clk_disable(obj->clk); 807 if (!stat) 808 return IRQ_HANDLED; 809 810 iommu_disable(obj); I guess that this modifying the above code could make (*isr)() flexible to be used for any purpose for clients? For me, having both following may be a bit residual. > int (*isr)(struct iommu *obj); > + void (*fault_cb)(struct iommu *obj, u32 da, u32 iommu_errs, void *priv); ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/1] OMAP: IOMMU: add support to callback during fault handling 2011-02-15 14:48 ` Hiroshi DOYU @ 2011-02-15 15:10 ` David Cohen 2011-02-16 16:28 ` David Cohen 0 siblings, 1 reply; 5+ messages in thread From: David Cohen @ 2011-02-15 15:10 UTC (permalink / raw) To: linux-arm-kernel On Tue, Feb 15, 2011 at 4:48 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote: > Hi David, Hi Hiroshi, > > Sorry for a bit late reply.... You're just in time. :) > > From: David Cohen <dacohen@gmail.com> > Subject: [PATCH v2 1/1] OMAP: IOMMU: add support to callback during fault handling > Date: Tue, 15 Feb 2011 16:36:31 +0200 > >> Add support to register a callback for IOMMU fault situations. Drivers using >> IOMMU module might want to be informed when such errors happen in order to >> debug it or react. >> >> Signed-off-by: David Cohen <dacohen@gmail.com> >> --- >> ?arch/arm/mach-omap2/iommu2.c ? ? ? ? ? ?| ? 21 +++++++++++++-- >> ?arch/arm/plat-omap/include/plat/iommu.h | ? 15 ++++++++++- >> ?arch/arm/plat-omap/iommu.c ? ? ? ? ? ? ?| ? 41 ++++++++++++++++++++++++++++--- >> ?3 files changed, 69 insertions(+), 8 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c >> index 14ee686..504310d 100644 >> --- a/arch/arm/mach-omap2/iommu2.c >> +++ b/arch/arm/mach-omap2/iommu2.c >> @@ -143,10 +143,10 @@ static void omap2_iommu_set_twl(struct iommu *obj, bool on) >> ? ? ? __iommu_set_twl(obj, false); >> ?} >> >> -static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) >> +static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra, u32 *iommu_errs) >> ?{ >> ? ? ? int i; >> - ? ? u32 stat, da; >> + ? ? u32 stat, da, errs; >> ? ? ? const char *err_msg[] = { >> ? ? ? ? ? ? ? "tlb miss", >> ? ? ? ? ? ? ? "translation fault", >> @@ -157,8 +157,10 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) >> >> ? ? ? stat = iommu_read_reg(obj, MMU_IRQSTATUS); >> ? ? ? stat &= MMU_IRQ_MASK; >> - ? ? if (!stat) >> + ? ? if (!stat) { >> + ? ? ? ? ? ? *iommu_errs = 0; >> ? ? ? ? ? ? ? return 0; >> + ? ? } >> >> ? ? ? da = iommu_read_reg(obj, MMU_FAULT_AD); >> ? ? ? *ra = da; >> @@ -171,6 +173,19 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) >> ? ? ? } >> ? ? ? printk("\n"); >> >> + ? ? errs = 0; >> + ? ? if (stat & MMU_IRQ_TLBMISS) >> + ? ? ? ? ? ? errs |= OMAP_IOMMU_ERR_TLB_MISS; >> + ? ? if (stat & MMU_IRQ_TRANSLATIONFAULT) >> + ? ? ? ? ? ? errs |= OMAP_IOMMU_ERR_TRANS_FAULT; >> + ? ? if (stat & MMU_IRQ_EMUMISS) >> + ? ? ? ? ? ? errs |= OMAP_IOMMU_ERR_EMU_MISS; >> + ? ? if (stat & MMU_IRQ_TABLEWALKFAULT) >> + ? ? ? ? ? ? errs |= OMAP_IOMMU_ERR_TBLWALK_FAULT; >> + ? ? if (stat & MMU_IRQ_MULTIHITFAULT) >> + ? ? ? ? ? ? errs |= OMAP_IOMMU_ERR_MULTIHIT_FAULT; >> + ? ? *iommu_errs = errs; >> + >> ? ? ? iommu_write_reg(obj, stat, MMU_IRQSTATUS); >> >> ? ? ? return stat; >> diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h >> index 19cbb5e..5a2475f 100644 >> --- a/arch/arm/plat-omap/include/plat/iommu.h >> +++ b/arch/arm/plat-omap/include/plat/iommu.h >> @@ -31,6 +31,7 @@ struct iommu { >> ? ? ? struct clk ? ? ?*clk; >> ? ? ? void __iomem ? ?*regbase; >> ? ? ? struct device ? *dev; >> + ? ? void ? ? ? ? ? ?*fault_cb_priv; >> >> ? ? ? unsigned int ? ?refcount; >> ? ? ? struct mutex ? ?iommu_lock; ? ? /* global for this whole object */ >> @@ -48,6 +49,7 @@ struct iommu { >> ? ? ? struct mutex ? ? ? ? ? ?mmap_lock; /* protect mmap */ >> >> ? ? ? int (*isr)(struct iommu *obj); >> + ? ? void (*fault_cb)(struct iommu *obj, u32 da, u32 iommu_errs, void *priv); > > What about making use of (*isr)() for fault call back as well? > > Basic concept is that, client can decide how to deal with iommu > fault. For example, for advanced case, client wants daynamic loading of > TLB(or PTE), or for ISP case, clients just want more appropriate fault > reporting. This (*isr)() could be used in such flexibility. In this case, it seems we can re-use it. > > ? 785 ? * ? ? ?Device IOMMU generic operations > ? 786 ? */ > ? 787 ?static irqreturn_t iommu_fault_handler(int irq, void *data) > ? 788 ?{ > ? 789 ? ? ? ? ?u32 stat, da; > ? 790 ? ? ? ? ?u32 *iopgd, *iopte; > ? 791 ? ? ? ? ?int err = -EIO; > ? 792 ? ? ? ? ?struct iommu *obj = data; > ? 793 > ? 794 ? ? ? ? ?if (!obj->refcount) > ? 795 ? ? ? ? ? ? ? ? ?return IRQ_NONE; > ? 796 > ? 797 ? ? ? ? ?/* Dynamic loading TLB or PTE */ > ? 798 ? ? ? ? ?if (obj->isr) > ? 799 ? ? ? ? ? ? ? ? ?err = obj->isr(obj); > ? 800 > ? 801 ? ? ? ? ?if (!err) > ? 802 ? ? ? ? ? ? ? ? ?return IRQ_HANDLED; > ? 803 > ? 804 ? ? ? ? ?clk_enable(obj->clk); > ? 805 ? ? ? ? ?stat = iommu_report_fault(obj, &da); > ? 806 ? ? ? ? ?clk_disable(obj->clk); > ? 807 ? ? ? ? ?if (!stat) > ? 808 ? ? ? ? ? ? ? ? ?return IRQ_HANDLED; > ? 809 > ? 810 ? ? ? ? ?iommu_disable(obj); > > I guess that this modifying the above code could make (*isr)() > flexible to be used for any purpose for clients? For me, having both > following may be a bit residual. I agree. We need to add 'void *isr_priv' to iommu struct and the function to register isr. I'll send a v3 soon. Regards, David > >> ? ? ? int (*isr)(struct iommu *obj); >> + ? ? void (*fault_cb)(struct iommu *obj, u32 da, u32 iommu_errs, void *priv); > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/1] OMAP: IOMMU: add support to callback during fault handling 2011-02-15 15:10 ` David Cohen @ 2011-02-16 16:28 ` David Cohen 0 siblings, 0 replies; 5+ messages in thread From: David Cohen @ 2011-02-16 16:28 UTC (permalink / raw) To: linux-arm-kernel On Tue, Feb 15, 2011 at 5:10 PM, David Cohen <dacohen@gmail.com> wrote: > On Tue, Feb 15, 2011 at 4:48 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote: >> Hi David, > > Hi Hiroshi, > >> >> Sorry for a bit late reply.... > > You're just in time. :) > >> >> From: David Cohen <dacohen@gmail.com> >> Subject: [PATCH v2 1/1] OMAP: IOMMU: add support to callback during fault handling >> Date: Tue, 15 Feb 2011 16:36:31 +0200 >> >>> Add support to register a callback for IOMMU fault situations. Drivers using >>> IOMMU module might want to be informed when such errors happen in order to >>> debug it or react. >>> >>> Signed-off-by: David Cohen <dacohen@gmail.com> >>> --- >>> ?arch/arm/mach-omap2/iommu2.c ? ? ? ? ? ?| ? 21 +++++++++++++-- >>> ?arch/arm/plat-omap/include/plat/iommu.h | ? 15 ++++++++++- >>> ?arch/arm/plat-omap/iommu.c ? ? ? ? ? ? ?| ? 41 ++++++++++++++++++++++++++++--- >>> ?3 files changed, 69 insertions(+), 8 deletions(-) >>> >>> diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c >>> index 14ee686..504310d 100644 >>> --- a/arch/arm/mach-omap2/iommu2.c >>> +++ b/arch/arm/mach-omap2/iommu2.c >>> @@ -143,10 +143,10 @@ static void omap2_iommu_set_twl(struct iommu *obj, bool on) >>> ? ? ? __iommu_set_twl(obj, false); >>> ?} >>> >>> -static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) >>> +static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra, u32 *iommu_errs) >>> ?{ >>> ? ? ? int i; >>> - ? ? u32 stat, da; >>> + ? ? u32 stat, da, errs; >>> ? ? ? const char *err_msg[] = { >>> ? ? ? ? ? ? ? "tlb miss", >>> ? ? ? ? ? ? ? "translation fault", >>> @@ -157,8 +157,10 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) >>> >>> ? ? ? stat = iommu_read_reg(obj, MMU_IRQSTATUS); >>> ? ? ? stat &= MMU_IRQ_MASK; >>> - ? ? if (!stat) >>> + ? ? if (!stat) { >>> + ? ? ? ? ? ? *iommu_errs = 0; >>> ? ? ? ? ? ? ? return 0; >>> + ? ? } >>> >>> ? ? ? da = iommu_read_reg(obj, MMU_FAULT_AD); >>> ? ? ? *ra = da; >>> @@ -171,6 +173,19 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) >>> ? ? ? } >>> ? ? ? printk("\n"); >>> >>> + ? ? errs = 0; >>> + ? ? if (stat & MMU_IRQ_TLBMISS) >>> + ? ? ? ? ? ? errs |= OMAP_IOMMU_ERR_TLB_MISS; >>> + ? ? if (stat & MMU_IRQ_TRANSLATIONFAULT) >>> + ? ? ? ? ? ? errs |= OMAP_IOMMU_ERR_TRANS_FAULT; >>> + ? ? if (stat & MMU_IRQ_EMUMISS) >>> + ? ? ? ? ? ? errs |= OMAP_IOMMU_ERR_EMU_MISS; >>> + ? ? if (stat & MMU_IRQ_TABLEWALKFAULT) >>> + ? ? ? ? ? ? errs |= OMAP_IOMMU_ERR_TBLWALK_FAULT; >>> + ? ? if (stat & MMU_IRQ_MULTIHITFAULT) >>> + ? ? ? ? ? ? errs |= OMAP_IOMMU_ERR_MULTIHIT_FAULT; >>> + ? ? *iommu_errs = errs; >>> + >>> ? ? ? iommu_write_reg(obj, stat, MMU_IRQSTATUS); >>> >>> ? ? ? return stat; >>> diff --git a/arch/arm/plat-omap/include/plat/iommu.h b/arch/arm/plat-omap/include/plat/iommu.h >>> index 19cbb5e..5a2475f 100644 >>> --- a/arch/arm/plat-omap/include/plat/iommu.h >>> +++ b/arch/arm/plat-omap/include/plat/iommu.h >>> @@ -31,6 +31,7 @@ struct iommu { >>> ? ? ? struct clk ? ? ?*clk; >>> ? ? ? void __iomem ? ?*regbase; >>> ? ? ? struct device ? *dev; >>> + ? ? void ? ? ? ? ? ?*fault_cb_priv; >>> >>> ? ? ? unsigned int ? ?refcount; >>> ? ? ? struct mutex ? ?iommu_lock; ? ? /* global for this whole object */ >>> @@ -48,6 +49,7 @@ struct iommu { >>> ? ? ? struct mutex ? ? ? ? ? ?mmap_lock; /* protect mmap */ >>> >>> ? ? ? int (*isr)(struct iommu *obj); >>> + ? ? void (*fault_cb)(struct iommu *obj, u32 da, u32 iommu_errs, void *priv); >> >> What about making use of (*isr)() for fault call back as well? >> >> Basic concept is that, client can decide how to deal with iommu >> fault. For example, for advanced case, client wants daynamic loading of >> TLB(or PTE), or for ISP case, clients just want more appropriate fault >> reporting. This (*isr)() could be used in such flexibility. > > In this case, it seems we can re-use it. > >> >> ? 785 ? * ? ? ?Device IOMMU generic operations >> ? 786 ? */ >> ? 787 ?static irqreturn_t iommu_fault_handler(int irq, void *data) >> ? 788 ?{ >> ? 789 ? ? ? ? ?u32 stat, da; >> ? 790 ? ? ? ? ?u32 *iopgd, *iopte; >> ? 791 ? ? ? ? ?int err = -EIO; >> ? 792 ? ? ? ? ?struct iommu *obj = data; >> ? 793 >> ? 794 ? ? ? ? ?if (!obj->refcount) >> ? 795 ? ? ? ? ? ? ? ? ?return IRQ_NONE; >> ? 796 >> ? 797 ? ? ? ? ?/* Dynamic loading TLB or PTE */ >> ? 798 ? ? ? ? ?if (obj->isr) >> ? 799 ? ? ? ? ? ? ? ? ?err = obj->isr(obj); >> ? 800 >> ? 801 ? ? ? ? ?if (!err) >> ? 802 ? ? ? ? ? ? ? ? ?return IRQ_HANDLED; >> ? 803 >> ? 804 ? ? ? ? ?clk_enable(obj->clk); >> ? 805 ? ? ? ? ?stat = iommu_report_fault(obj, &da); >> ? 806 ? ? ? ? ?clk_disable(obj->clk); >> ? 807 ? ? ? ? ?if (!stat) >> ? 808 ? ? ? ? ? ? ? ? ?return IRQ_HANDLED; >> ? 809 >> ? 810 ? ? ? ? ?iommu_disable(obj); >> >> I guess that this modifying the above code could make (*isr)() >> flexible to be used for any purpose for clients? For me, having both >> following may be a bit residual. > There are few possible issues in this case. (*isr)() is meant to replace the generic fault handler on iommu, but the fault callback isn't. Basically fault callback needs to call specific iommu fault report to know faulty 'da' and iommu errors, so part of the generic fault handler must be called. One possible solution is always call specific fault report before (*isr)(). IMO it makes sense as (*isr)() might want to know at least the current faults. But in this case, specific fault report should not print anything else as only the upper layer and user will know when a fault is actually an error or not. So, they're responsible for error messages. I'm sending a v3 with these changes. Comments will be welcome. Br, David > I agree. We need to add 'void *isr_priv' to iommu struct and the > function to register isr. > I'll send a v3 soon. > > Regards, > > David > >> >>> ? ? ? int (*isr)(struct iommu *obj); >>> + ? ? void (*fault_cb)(struct iommu *obj, u32 da, u32 iommu_errs, void *priv); >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-02-16 16:28 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-15 14:36 [PATCH v2 0/1] OMAP: IOMMU fault callback support David Cohen 2011-02-15 14:36 ` [PATCH v2 1/1] OMAP: IOMMU: add support to callback during fault handling David Cohen 2011-02-15 14:48 ` Hiroshi DOYU 2011-02-15 15:10 ` David Cohen 2011-02-16 16:28 ` David Cohen
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).