On Wed, Feb 23, 2011 at 2:56 PM, Sakari Ailus
<sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> David Cohen wrote:
>> On Wed, Feb 23, 2011 at 3:39 PM, Guzman Lugo, Fernando
>> <fernando.lugo@xxxxxx> wrote:
>>> On Wed, Feb 23, 2011 at 3:45 AM, David Cohen <dacohen@xxxxxxxxx> wrote:
>>>> On Wed, Feb 23, 2011 at 3:17 AM, Guzman Lugo, Fernando
>>>> <fernando.lugo@xxxxxx> wrote:
>>>>> On Wed, Feb 16, 2011 at 1:35 PM, David Cohen <dacohen@xxxxxxxxx> wrote:
>>>>>> Add support to register an isr for IOMMU fault situations and adapt it
>>>>>> to allow such (*isr)() to be used as fault callback. Drivers using IOMMU
>>>>>> module might want to be informed when errors happen in order to debug it
>>>>>> or react.
>>>>>>
>>>>>> Signed-off-by: David Cohen <dacohen@xxxxxxxxx>
>>>>>> ---
>>>>>> arch/arm/mach-omap2/iommu2.c | 17 +++++++++-
>>>>>> arch/arm/plat-omap/include/plat/iommu.h | 14 ++++++++-
>>>>>> arch/arm/plat-omap/iommu.c | 52 ++++++++++++++++++++++---------
>>>>>> 3 files changed, 65 insertions(+), 18 deletions(-)
>>>>>>
>>>>> ....
>>>>>
>>>>>> @@ -917,6 +912,33 @@ void iommu_put(struct iommu *obj)
>>>>>> }
>>>>>> EXPORT_SYMBOL_GPL(iommu_put);
>>>>>>
>>>>>> +int iommu_set_isr(const char *name,
>>>>>> + int (*isr)(struct iommu *obj, u32 da, u32 iommu_errs,
>>>>>> + void *priv),
>>>>>> + void *isr_priv)
>>>>>> +{
>>>>>> + struct device *dev;
>>>>>> + struct iommu *obj;
>>>>>> +
>>>>>
>>>>> if the driver support multiple user for the same iommu why can only
>>>>> one callback be registered? should it support register multiple
>>>>> callback function (one per user)?
>>>>
>>>> Can you define a scenario for that?
>>>> On OMAP3 ISP the multiple users are the multiple ISP submodule, but I
>>>> don't think it's necessary all submodule to have a specific callback.
>>>> ISP core layer should handle.
>>>
>>> Hi,
>>>
>>> In OMAP4 the cortex M3 is a double core processor and as each core is
>>> running they own version of the RTOS we threat them independently. So
>>> our driver which controls the remote processor sees two processor but
>>> both use the same iommu hw. When a iommu fault happens, at this
>>> moment, it is consider as a faltal error and it is no managed to
>>> recover and continue, instead a restart of the processor is needed, if
>>> the fault happens in core0 we need to reset core1 too and vice versa.
>>> if the iommu would support several user callbacks, we can register the
>>> callback which resets core0 and also the callback which resets core1
>>> and treat them as totally independent processors. Also we have an
>>> error event notifier driver, which is only in charge of notifying
>>> error events to userspace, so we would have multiple callbacks we
>>> could do this
>>
>> I understood your point. In this case, I may not disagree about having
>> more than one callback per obj, although it doesn't seem a nice
>> scenario.
>> We can have a list of callbacks and call the entire list when a fault
>> happens. But it's necessary to pay attention it will happen in atomic
>> context and users should not abuse and register many callbacks. The
>> callback should *NOT* print useless messages and must verify the error
>> code to not execute useless steps.
>> In this context, callback and ISR cannot share a same pointer anymore.
>
> I think this is outside of the scope of the patch but...
yes, the same behaviour was before the patches, but as the patches are
changing the isr, I think it is a good time to modify, not in patch 2,
but in a new patch to be added to the serie between patch 1 and 2, so
that we dont need to change ISR part again after this set of patches.
>
> To efficiently debug iommu faults (with a driver using iommu page
> walking), besides the actual fault address the list of existing mappings
> and the information which driver created them and for which purpose is
> useful.
>
> The list of mappings is already available in the iommu structure. It'd
> be nice if there was a function a driver could call to print them.
>
> I can only think of ugly ways to implement the other.
>
> Just my 5 cents (as we have no 2 cent coins here).
>
> Regards,
>
> --
> Sakari Ailus
> sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
