On Fri, 2011-05-27 at 09:05 -0700, Mike Travis wrote: > Hi David, > > Let me provide a short historyâ That's useful; thanks. > You're probably aware of the cycles Chris Wright and Mike Habeck and I > went through to clean up these patches. I think I was vaguely aware but I've been flat out with other things and if Chris was involved I'll have mostly ignored it â on the basis that I trust him and it will come to my attention when it needs toâ which it has. > The last patch is important as these types of systems require > "system management" controllers and since we want to set the options > "intel_iommu=on iommu=pt", we can only do that for kernels that have > these fixes. The special symbol (which the memory is released since > it's in the "__init" section) triggers that mgmt s/w to set those > options. If the kernel does not have the fixes, then it must use the > older "on,forcedac" options or the kernel will panic on bootup. That is a quite unspeakably vile interface between kernel and platform. I disapprove wholeheartedly. If I understand it correctly, we don't actually have to set the variable to 1; the mere *existence* of the variable is sufficient to signal to the SMC that the kernel has these bugs fixed. By the time the kernel is *running*, surely it's too late for the SMC to change its command line arguments anyway? So it *can't* be dependent on the *value* of the variable? Also, your 'flag' variable is "static", so can't the compiler just completely optimise it away? > If there is a better way, please let me know. The problem I found > is the SMC needs the info before booting the kernel, and using a > specially named symbol was the best I could think of. You're basically asking for a flag you can set in the kernel "metadata", which could live in a separate non-loadable ELF section or something, which indicates that a specific bug is fixed. Why not go the whole hog and do something generic, like a bitmap of all the bugzilla.kernel.org bugs that are fixed in this kernel image? Then it can be shot down in flames by someone other than me. :) Whatever the solution is, it *isn't* a problem which is specific to the intel-iommu code, and I'm fairly unhappy with a solution which exists only there. Most people get away with checking the version numbers. I know it's fairly icky to have a check which goes something like >= 2.6.40 || (2.6.32.x && >= 2.6.32.10) || (RHEL && >= 2.6.9-4.5.el5)... but at least it's icky somewhere *I* don't have to look at it :) Hell, even if you stick an int __initdata hey_bootloader_you_can_trust_that_bugzilla_11234_is_fixed; into init/main.c, that's still somewhere *I* don't have to look at it. And that'll do me just fine, because someone *else* can shout at you for it :) Alternatively, let your version check just be '>=2.6.40', and your backports to the archaeological trees can include a *new* kernel command line parameter like 'iommu=pt64' which unfixed kernels won't understand and will ignore. So your SCU will provide 'iommu=pt' for 2.6.40 or newer, and 'iommu=pt64' for older kernels. Then at least your hack doesn't affect the *current* kernels, and will eventually be lost in the mists of time. Really, anything but this :) > Thanks for looking into this. Please let me know when you have accepted > them and I will then push them to the 2.6.32 stable tree, and then on > to the distros. All six of your acceptable patches will want 'Cc: stable@xxxxxxxxxx' in the commit comment along with the signed-off-bys, if you want them to be backported to the stable tree. -- David Woodhouse Open Source Technology Centre David.Woodhouse@xxxxxxxxx Intel Corporation -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
