[PATCH v3 2/2] OMAP: IOMMU: add support to callback during fault handling

Hi,

On Mon, Feb 21, 2011 at 3:12 PM, David Cohen [off-list ref] wrote:

Generic errors codes make easier to threat possible IOMMU users which
have (partially or totally) common drivers for different OMAP
versions.

Agree then.

quoted

quoted

+ ? ? ? /* Fault callback or TLB/PTE Dynamic loading */
+ ? ? ? if (obj->isr && !obj->isr(obj, da, errs, obj->isr_priv))
? ? ? ? ? ? ? ?return IRQ_HANDLED;

BTW, why not changing the name isr for cb, it is confusing since there
is another fault_isr called by mmu, AFAIK nobody uses obj->isr

The main purpose of this function is to be an ISR, not only callback.

Yep, I just thought it was a bit too much of:
(1) The real isr: iommu_fault_handler, set on request_irq
(2) A plugged fault_isr for mach specific code: omap2_iommu_fault_isr,
for error reporting.
(3) And then a plugged custom isr, to be set by the user.

Feel free to ignore.

quoted

So I think the following will be bad practice:

? ? ? ?mmu = iommu_get("iva2");
? ? ? ?if (!IS_ERR(mmu))
? ? ? ? ? ? ? ?mmu->isr = mmu_fault_callback;

Shall we think anything to prevent such mis-usage?

Well, the IOMMU user has access to IOMMU obj, so it can not only
change the (*isr)() but to mess with a lot of other stuff. The only
way to prevent it is to avoid user to have obj. But then, this fix (or
issue) does not belong to this patch.

Agree, just pointing out.

Regards,

Omar