From mboxrd@z Thu Jan  1 00:00:00 1970
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
Subject: Re: [kvm-unit-tests PATCH v2 01/10] asm-generic: add portio
 accessors to io.h
Date: Fri, 15 Jan 2016 22:34:09 +0100
Message-ID: <20160115213409.GA12949@potion.brq.redhat.com>
References: <1452876695-9240-1-git-send-email-drjones@redhat.com>
 <1452876695-9240-2-git-send-email-drjones@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, pbonzini@redhat.com, mst@redhat.com,
	agordeev@redhat.com
To: Andrew Jones <drjones@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:48870 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751314AbcAOVeN (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 15 Jan 2016 16:34:13 -0500
Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27])
	by mx1.redhat.com (Postfix) with ESMTPS id 0D6AF18B319
	for <kvm@vger.kernel.org>; Fri, 15 Jan 2016 21:34:13 +0000 (UTC)
Content-Disposition: inline
In-Reply-To: <1452876695-9240-2-git-send-email-drjones@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

2016-01-15 17:51+0100, Andrew Jones:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> 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

(I consider this repeated pattern to be very ugly, even by C standards.)

> +static inline u8 inb(unsigned long addr)
> +{
> +	return readb(PCI_IOBASE + addr);
> +}

My first reaction was "throw this abomination out!", but Drew explained
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.

arm, arm64, and unicore32 define PCI_IOBASE in Linux.  I didn't figure
out why they want to use a PIO based abstraction for MMIO, so the
interface is fine with me as long as
 - functions are hidden behind a single #ifndef, like ARCH_HAS_PORT_IO.
   (Ideally defined as part of global configuration, because it's harder
    to fail that way.)
 - "unsigned long addr" is changed to "u16 port";
   x86 ought to have that and we should use different names if we need
   different types, because behavior couldn't be the same then.