On Mon, Mar 28, 2011 at 4:42 AM, Ramirez Luna, Omar <omar.ramirez@xxxxxx> wrote: > Hi, > > On Sun, Mar 27, 2011 at 12:27 PM, Sakari Ailus > <sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx> wrote: >> Ramirez Luna, Omar wrote: >>> On Fri, Mar 25, 2011 at 10:13 AM, Sakari Ailus >>> <sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx> wrote: >>>> This patchset is aimed to fix a problem in arch_iommu implementation >>>> references. When an actual arch_iommu implementation is not loaded while >>>> iommu_get() is being called results to a kernel oops, as well as >>>> removing an arch_iommu implementation which is in use. >>> >>> How about fixing the dependency instead? Right now iommu2 depends on >>> iommu because of the calls to >>> install_iommu_arch/uninstall_iommu_arch... we should change that >>> dependency to iommu depend on iommu2. Something like iommu (plat) >>> querying iommu2 (mach) for devices to install. >> >> There is no direct dependency from a driver using the generic API to a >> particular implementation of the iommu. This comes from the design of >> the iommu framework. The generic layer shouldn't depend on particular >> implementation(s). > > IMHO there is, take as an example bridgedriver (it is arm/omap > dependent), so it depends on iommu providing the mach-omap2 > implementation. I imagine isp for omap imposes the same dependency, > even more your patchset enforces that dependency. The generic layer can exist without a specific implementation. It should accept later arch install/uninstall without any problem. > > Basically, if there is no "arch_iommu" the iommu driver does not work, > and if there was an "arch_iommu" but it was removed then the driver > crashes. The loaded module *must* uninstall arch when existing, then kernel will see no crash. And yes, generic layer won't work anymore but that's an expected and correct situation. > > Now, there could be architectures that does not depend on a particular > implementation but this iommu driver doesn't support them because if > there is no arch_iommu operations, it does nothing or crashes. > >> What comes to the patch, it works as long as there's only one iommu >> implementation loaded / compiled to the kernel. I wonder if this kind of >> limitation can be accepted. > > Which is the way iommu choose to work, like I said if there is no > arch_iommu nothing works, most of APIs in iommu depend on an machine > specific implementation. To fix that it is not the scope of my > proposal. > > If indeed iommu can function without a machine specific implementation > then a redesign needs to be made, but to me the same approach as I did > needs to be followed: if there is mach implementation (e.g.: iommu2.c) > the generic API needs to depend on it, otherwise the module can be > removed and crash the kernel; OTOH if there is no mach implementation, > then iommu should not depend on it to be installed as you point out, > this could be handled in plat/iommu.h among with: > > #if defined(CONFIG_ARCH_OMAP1) > #error "iommu for this processor not implemented yet" > #else > #include <plat/iommu2.h> > #endif So, every new specific implementation should modify this piece of code? Are you sure it's a good idea? Regards, David Cohen > > A new else defining the install/uninstall_arch_iommu functions or > simply reversing the check to be OMAP2+ and error on anything else. > > Regards, > > Omar > -- > 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 > -- 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
