Looks like the subject would warmly welcome a "kvm:" tag. On 2012-02-09 05:22, 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. > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > --- > kvm-all.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index 0b87658..681ad15 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -78,7 +78,10 @@ struct KVMState > int pit_in_kernel; > int xsave, xcrs; > int many_ioeventfds; > - int irqchip_inject_ioctl; > + /* The man page (and posix) say ioctl numbers are signed int, but > + * they're not. Linux, glibc and *BSD all treat ioctl numbers as > + * unsigned, and treating them as signed here can break things */
What about naming the problem instead: /* Comparison with IOCTL macros on 32-bit hosts requires unsigned. */ > + unsigned irqchip_inject_ioctl; > #ifdef KVM_CAP_IRQ_ROUTING > struct kvm_irq_routing *irq_routes; > int nr_allocated_irq_routes; Jan
signature.asc
Description: OpenPGP digital signature
