On Wed, 11 May 2011 16:28:04 +0200, Luca Tettamanti wrote: > On Wed, May 11, 2011 at 04:15:45PM +0200, Jean Delvare wrote: > > On Wed, 11 May 2011 15:48:59 +0200, Luca Tettamanti wrote: > > > The driver uses lahf to copy the lowest byte of EFLAGS to eax, then > > > shifts it to right and test the lowest bit (CF). > > > Using pushf instead we get the whole EFLAGS and test the lowest bit. > > > Does it make sense? Someone should double check the patch, I don't trust > > > my assembly skills ;) > > > > > > diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c > > > index d72433f..3554d10 100644 > > > --- a/drivers/char/i8k.c > > > +++ b/drivers/char/i8k.c > > > @@ -139,8 +139,8 @@ static int i8k_smm(struct smm_regs *regs) > > > "movl %%edi,20(%%rax)\n\t" > > > "popq %%rdx\n\t" > > > "movl %%edx,0(%%rax)\n\t" > > > - "lahf\n\t" > > > - "shrl $8,%%eax\n\t" > > > + "pusfh\n\t" > > > > I think you meant pushf, not pusfh, right? > > Yeah, right... hand me that brown paper bag ;) > > > > > > + "popl %%eax\n\t" > > > "andl $1,%%eax\n" > > > :"=a"(rc) > > > : "a"(regs) > > > > I am no asm expert, but I don't get why you removed the shrl > > instruction. As I understand it, your pushf+popl replaces only lahf, > > the bit shifting is still needed (I assume the code wants to test bit 8 > > in the flag register.) > > AFAIK lahf loads the lowest byte of EFLAGS into AH which is the upper > half of AX, hence the shift. Yes, you're right, of course. So much for my asm skills. Please return the brown paper bag to me when you're done ;) > The popl is wrong though, pushf only pushes 2 bytes... so let's try > again: > > diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c > index d72433f..f71f266 100644 > --- a/drivers/char/i8k.c > +++ b/drivers/char/i8k.c > @@ -139,8 +139,8 @@ static int i8k_smm(struct smm_regs *regs) > "movl %%edi,20(%%rax)\n\t" > "popq %%rdx\n\t" > "movl %%edx,0(%%rax)\n\t" > - "lahf\n\t" > - "shrl $8,%%eax\n\t" > + "pushf\n\t" > + "pop %%ax\n\t" > "andl $1,%%eax\n" > :"=a"(rc) > : "a"(regs) Looks reasonable. I can't test it as I don't have the hardware, but I have updated the driver at: http://khali.linux-fr.org/devel/misc/i8k/ with the above fix. Jeff, Harry, can you test? Thanks, -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
