linux-kernel - Re: [GIT PULL] x86/asm for 2.6.36

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 06 Aug 2010 10:45:07 -0700
From:	"H. Peter Anvin" <hpa@...or.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	"H. Peter Anvin" <hpa@...ux.intel.com>, stable@...nel.org,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Avi Kivity <avi@...hat.com>, Brian Gerst <brgerst@...il.com>,
	Glauber Costa <glommer@...hat.com>, Greg KH <gregkh@...e.de>,
	Ingo Molnar <mingo@...e.hu>, Jeff Dike <jdike@...toit.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Marcelo Tosatti <mtosatti@...hat.com>,
	Pekka Enberg <penberg@...helsinki.fi>,
	Peter Palfrader <peter@...frader.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Zachary Amsden <zamsden@...hat.com>
Subject: Re: [GIT PULL] x86/asm for 2.6.36

On 08/06/2010 10:17 AM, Linus Torvalds wrote:
> On Thu, Aug 5, 2010 at 1:37 PM, H. Peter Anvin <hpa@...ux.intel.com> wrote:
>>
>>  git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git x86-asm-for-linus
>>
>> H. Peter Anvin (7):
>>      x86, asm: Clean up and simplify set_64bit()
> 
> Hmm. This seems to cause compiler warnings for me on x86-64:
> 
>   drivers/pci/intr_remapping.c: In function ‘modify_irte’:
>   drivers/pci/intr_remapping.c:314: warning: passing argument 1 of
> ‘set_64bit’ from incompatible pointer type
>   /home/torvalds/v2.6/linux/arch/x86/include/asm/cmpxchg_64.h:6: note:
> expected ‘volatile u64 *’ but argument is of type ‘long unsigned int
> *’
> 
> and I'm not clear on the reason for that function prototype change.
> 
>                           Linus

On 64 bits the intent was to be consistent with the tightened-up 32-bit
code.  Perhaps that was a mistake, but it seems a lot more consistent to
me to have "u64" be the type

On the 32-bit side, it's not so much a function prototype change as the
previous version of the code actually explicitly cast it to an array of
unsigned ints... it has the advantage that it can accept any type, but
the disadvantage that is can accept *ANY* type.  The problem of course
is the above, since gcc will issue warnings for u64 (unsigned long long)
version the otherwise-identical "unsigned long".

It's worth noting that in this particular case the code itself looks
like this:

	set_64bit((unsigned long *)&irte->low, irte_modified->low);
	set_64bit((unsigned long *)&irte->high, irte_modified->high);

... where the existing cast is there because irte->low and irte->high
are types __u64.  In other words, with the "more logical" u64 prototype
the casts should just get removed.

On the other hand I should have seen this sooner, obviously.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/