On 08.03.2012, at 01:41, David Gibson wrote: > In kvm-all.c we store an ioctl cmd number in the irqchip_inject_ioctl field > of KVMState, which has type 'int'. This seems to make sense since the > ioctl() man page says that the cmd parameter has type int. > > However, the kernel treats ioctl numbers as unsigned - sys_ioctl() takes an > unsigned int, and the macros which generate ioctl numbers expand to > unsigned expressions. Furthermore, some ioctls (IOC_READ ioctls on x86 > and IOC_WRITE ioctls on powerpc) have bit 31 set, and so would be negative > if interpreted as an int. This has the surprising and compile-breaking > consequence that in kvm_irqchip_set_irq() where we do: > return (s->irqchip_inject_ioctl == KVM_IRQ_LINE) ? 1 : event.status; > We will get a "comparison is always false due to limited range of data > type" warning from gcc if KVM_IRQ_LINE is one of the bit-31-set ioctls, > which it is on powerpc. > > So, despite the fact that the man page and posix say ioctl numbers are > signed, they're actually unsigned. The kernel uses unsigned, the glibc > header uses unsigned long, and FreeBSD, NetBSD and OSX also use unsigned > long ioctl numbers in the code. > > Therefore, this patch changes the variable to be unsigned, fixing the > compile. > > Cc: Avi Kivity <a...@redhat.com> > Cc: Marcelo Tossatti <mtossa...@redhat.com> > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
I picked that one into ppc-next, as without compilation on ppc breaks for me. I haven't touched the other 3 patches in the set though. Alex PS: These 4 patches don't target a single maintainer. Thus it's better to not send them as a patch set :).