From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Jones Subject: Re: [kvm-unit-tests PATCH v2 01/10] asm-generic: add portio accessors to io.h Date: Mon, 18 Jan 2016 14:52:34 +0100 Message-ID: <20160118135234.GC4075@hawk.localdomain> References: <1452876695-9240-1-git-send-email-drjones@redhat.com> <1452876695-9240-2-git-send-email-drjones@redhat.com> <20160115213409.GA12949@potion.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, pbonzini@redhat.com, mst@redhat.com, agordeev@redhat.com To: Radim =?utf-8?B?S3LEjW3DocWZ?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:38044 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754902AbcARNwi (ORCPT ); Mon, 18 Jan 2016 08:52:38 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 9DDDAA852 for ; Mon, 18 Jan 2016 13:52:38 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20160115213409.GA12949@potion.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, Jan 15, 2016 at 10:34:09PM +0100, Radim Kr=C4=8Dm=C3=A1=C5=99 w= rote: > 2016-01-15 17:51+0100, Andrew Jones: > > Signed-off-by: Andrew Jones > > --- > > diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h > > @@ -152,6 +152,58 @@ static inline u64 __bswap64(u64 x) > > +#ifndef PCI_IOBASE > > +#define PCI_IOBASE ((void *)0) > > +#endif > > + > > +#ifndef inb > > +#define inb inb >=20 > (I consider this repeated pattern to be very ugly, even by C standard= s.) >=20 > > +static inline u8 inb(unsigned long addr) > > +{ > > + return readb(PCI_IOBASE + addr); > > +} >=20 > My first reaction was "throw this abomination out!", but Drew explain= ed > that in*/out* is here because we'll also generalize the x86 PCI code > (which uses PIO and MMIO) and that Linux has the same code. >=20 > arm, arm64, and unicore32 define PCI_IOBASE in Linux. I didn't figur= e > out why they want to use a PIO based abstraction for MMIO, so the > interface is fine with me as long as Having these aliases for read*/write* are less evil than attempting to share code between arm and x86 with a bunch of #ifdefs. As I didn't yet write a shared pci-testdev driver though, then this patch can be droppe= d for now, if you'd prefer. However at some point it may need to come bac= k. > - functions are hidden behind a single #ifndef, like ARCH_HAS_PORT_I= O. > (Ideally defined as part of global configuration, because it's har= der > to fail that way.) I prefer to stay consistent with Linux. It's ugly, but we already have the ugliness for read* and write*. > - "unsigned long addr" is changed to "u16 port"; > x86 ought to have that and we should use different names if we nee= d > different types, because behavior couldn't be the same then. in*/out* should be portio for x86, and just aliases for read*/write* mmio for other architectures. For mmio to work we may need 'unsigned lo= ng addr', but that shouldn't stop x86 from defining its version as 'u16 po= rt'. (Although Linux defines it as 'int' actually). It does appear that in Linux arch/x86/include/asm/io.h doesn't include asm-generic/io.h. So, if you'd prefer x86 to not include it here either= , then we can drop the inclusion and the '#define inb inb' type stuff fro= m lib/x86/asm/io.h. Thanks, drew