On Mon, Nov 15, 2010 at 3:16 AM, Jakub Jelinek <ja...@redhat.com> wrote: > > I don't see any problems on the assembly level. i8k_smm is > not inlined in this case and checks all 3 conditions.
If it really is related to gcc not understanding that "*regs" has changed due to the memory being an automatic variable, and passing in "regs" itself as a pointer to that automatic variable together with the "memory" clobber not being sufficient, than I think it's the lack of inlining that will automatically hide the bug. (Side note: and I think this does show how much of a gcc bug it is not to consider "memory" together with passing in a pointer to an asm to always be a clobber). Because if it isn't inlined, then "regs" will be seen a a real pointer to some external memory (the caller) rather than being optimized to just be the auto structure on the stack. Because *mem is auto only within the context of the caller. Which actually points to a possible simpler: - remove the "+m" since it adds too much register pressure - mark the i8k_smm() as "noinline" instead. Quite frankly, I'd hate to add even more crud to that inline asm (to save/restore the registers manually). It's already not the prettiest thing around. So does the attached patch work for everybody? Linus
drivers/char/i8k.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c index f0863be..101011e 100644 --- a/drivers/char/i8k.c +++ b/drivers/char/i8k.c @@ -114,7 +114,7 @@ static inline const char *i8k_get_dmi_data(int field) /* * Call the System Management Mode BIOS. Code provided by Jonathan Buzzard. */ -static int i8k_smm(struct smm_regs *regs) +static noinline int i8k_smm(struct smm_regs *regs) { int rc; int eax = regs->eax; @@ -142,7 +142,7 @@ static int i8k_smm(struct smm_regs *regs) "lahf\n\t" "shrl $8,%%eax\n\t" "andl $1,%%eax\n" - :"=a"(rc), "+m" (*regs) + :"=a"(rc) : "a"(regs) : "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"); #else @@ -168,7 +168,7 @@ static int i8k_smm(struct smm_regs *regs) "lahf\n\t" "shrl $8,%%eax\n\t" "andl $1,%%eax\n" - :"=a"(rc), "+m" (*regs) + :"=a"(rc) : "a"(regs) : "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory"); #endif