From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1L7c5z-0004Ex-6g for qemu-devel@nongnu.org; Tue, 02 Dec 2008 15:43:47 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1L7c5x-0004Ef-Kn for qemu-devel@nongnu.org; Tue, 02 Dec 2008 15:43:46 -0500 Received: from [199.232.76.173] (port=52529 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1L7c5x-0004Ec-EN for qemu-devel@nongnu.org; Tue, 02 Dec 2008 15:43:45 -0500 Received: from e2.ny.us.ibm.com ([32.97.182.142]:41499) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1L7c5w-0000FH-UB for qemu-devel@nongnu.org; Tue, 02 Dec 2008 15:43:45 -0500 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by e2.ny.us.ibm.com (8.13.1/8.13.1) with ESMTP id mB2Kh7de012245 for ; Tue, 2 Dec 2008 15:43:07 -0500 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id mB2KhdaI189842 for ; Tue, 2 Dec 2008 15:43:39 -0500 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id mB2KhCQA029607 for ; Tue, 2 Dec 2008 15:43:13 -0500 Subject: Re: [Qemu-devel] [PATCH 1/1] IBM PowerPC 4xx 32-bit PCI controller emulation From: Hollis Blanchard In-Reply-To: <49359908.5040407@codemonkey.ws> References: <1228248154-15208-1-git-send-email-hollisb@us.ibm.com> <49359908.5040407@codemonkey.ws> Content-Type: text/plain Date: Tue, 02 Dec 2008 14:43:38 -0600 Message-Id: <1228250618.8128.24.camel@localhost.localdomain> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: plagnioj@jcrosoft.com, qemu-devel@nongnu.org, aurelien@aurel32.net On Tue, 2008-12-02 at 14:22 -0600, Anthony Liguori wrote: > Hollis Blanchard wrote: > > > +#include "pci.h" > > +#include "pci_host.h" > > +#include "bswap.h" > > + > > +#undef DEBUG > > +#ifdef DEBUG > > +#define DPRINTF(fmt, args...) do { printf(fmt, ##args); } while (0) > > +#else > > +#define DPRINTF(fmt, args...) > > +#endif /* DEBUG */ > > > > This is a GCC-ism that's deprecated. The proper syntax (C99) is: > > #define DPRINTF(fmt, ...) do { printf(fmt, ## __VA_ARGS__); } while (0) Will do. > > +struct ppc4xx_pci_t { > > + struct pci_master_map pmm[PPC44x_PCI_NR_PMMS]; > > + struct pci_target_map ptm[PPC44x_PCI_NR_PTMS]; > > + > > + PCIHostState pci_state; > > +}; > > +typedef struct ppc4xx_pci_t ppc4xx_pci_t; > > > > It would be better to use QEMU style type naming. I copied this style from ppc405_uc.c, but I will ChangeItLikeThis. > > +static void pci4xx_cfgaddr_writel(void *opaque, target_phys_addr_t addr, > > + uint32_t value) > > +{ > > + ppc4xx_pci_t *ppc4xx_pci = opaque; > > + > > +#ifdef TARGET_WORDS_BIGENDIAN > > + value = bswap32(value); > > +#endif > > > > > > Is this byte swapping correct? I hate this byte swapping, because as we've discussed at great length in the past, "TARGET_WORDS_BIGENDIAN" never should have existed. That said, as far as I can tell it is correct for the current state of qemu. I changed it to this style due to Aurelien's previous request. As I noted in the patch description, I can only test on big-endian/big-endian guest/host. > > + > > + /* XXX register_savevm() */ > > > > Should be easy enough to add a register_savevm() function, no? It should be, but since exactly 0 other PPC 4xx devices implement this, I won't be able to test it. Should I implement it and leave a comment in there that says "untested"? -- Hollis Blanchard IBM Linux Technology Center