From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1Kl7NW-0005hW-0x
	for qemu-devel@nongnu.org; Wed, 01 Oct 2008 15:28:54 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1Kl7NU-0005eF-KO
	for qemu-devel@nongnu.org; Wed, 01 Oct 2008 15:28:53 -0400
Received: from [199.232.76.173] (port=36195 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Kl7NU-0005e1-FG
	for qemu-devel@nongnu.org; Wed, 01 Oct 2008 15:28:52 -0400
Received: from yx-out-1718.google.com ([74.125.44.157]:23644)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <anthony@codemonkey.ws>) id 1Kl7NU-0006AZ-7C
	for qemu-devel@nongnu.org; Wed, 01 Oct 2008 15:28:52 -0400
Received: by yx-out-1718.google.com with SMTP id 3so120109yxi.82
	for <qemu-devel@nongnu.org>; Wed, 01 Oct 2008 12:28:51 -0700 (PDT)
Message-ID: <48E3CF33.3050702@codemonkey.ws>
Date: Wed, 01 Oct 2008 14:27:47 -0500
From: Anthony Liguori <anthony@codemonkey.ws>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 4/4] pci: use pci_config_header in pci.c
References: <1222870375-13489-1-git-send-email-kraxel@redhat.com>
	<1222870375-13489-4-git-send-email-kraxel@redhat.com>
	<48E3A5BD.8010000@codemonkey.ws> <48E3CAF9.5040207@redhat.com>
In-Reply-To: <48E3CAF9.5040207@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Reply-To: qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: qemu-devel@nongnu.org, Paul Brook <paul@codesourcery.com>

Gerd Hoffmann wrote:
> Anthony Liguori wrote:
>   
> I'm not sure what you are expecting here.  The idea to introduce
> gcc-checkable byteordered types (via struct trick) was rejected.  So I
> have le32 & friends just typedef'ed to the corrosponding int*_t types
> and use the existing cpu_*() byteorder macros.  The new types are
> basically documentation now.
>
> Endian swapping must still be done by the users.  Lots of casts can be
> dropped though, which IMHO improves the readability alot (random chunk
> picked from patch):
>
> -    cmd = le16_to_cpu(*(uint16_t *)(d->config + PCI_COMMAND));
> +    cmd = le16_to_cpu(conf->command);
>
> I'm not aware of any way to do the byteswapping here automatically.
>   

Because the config structure is loaded after pci_register device and 
because the MMIO callbacks into the config space?  If you look at 
pci_default_read_config(), it actually does conversion from le to host 
CPU because that's what MMIO callbacks expect.  If you simply removed 
this, it would work.  Of course, you'll have to audit each caller of 
pci_register_device() and make sure their config's get changed too.

Regards,

Anthony LIguori

> Except maybe introducing accessor functions/macros for each and every
> struct element and hide the byteswapping macro call therein.  Which in
> turn makes providing the struct to pci drivers sort of pointless ...
>
> cheers,
>   Gerd
>