On Mon, Feb 09, 2009 at 04:33:25PM +0800, Liu Yu-B13201 wrote: > > -----Original Message----- > > From: Aurelien Jarno [mailto:aurelien@xxxxxxxxxxx] > > Sent: Sunday, January 25, 2009 12:03 AM > > To: Liu Yu-B13201 > > Cc: hollisb@xxxxxxxxxx; qemu-devel@xxxxxxxxxx; kvm-ppc@xxxxxxxxxxxxxxx > > Subject: Re: [Qemu-devel] [PATCH 2/6] kvm/powerpc: Add > > freescale pcicontroller's support > > > > On Thu, Jan 22, 2009 at 06:14:12PM +0800, Liu Yu wrote: > > > This patch add the emulation of freescale's pci controller > > for MPC85xx platform. > > > > > > Signed-off-by: Liu Yu <yu.liu@xxxxxxxxxxxxx> > > > > I have one general comment for this patch, QEMU (mostly) uses > > spaces for > > indentation, while this patch uses spaces + tab. Would it be > > possible to > > convert it to spaces only, using 4 spaces? > > Fixed. > > > > > I also have one minor comment inside. Otherwise the patch looks ok. > > > > > Makefile.target | 2 + > > > hw/ppce500_pci.c | 370 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 372 insertions(+), 0 deletions(-) > > > create mode 100644 hw/ppce500_pci.c > > > > > > diff --git a/Makefile.target b/Makefile.target > > > index a091ce9..5cae257 100644 > > > --- a/Makefile.target > > > +++ b/Makefile.target > > > @@ -598,6 +598,8 @@ OBJS+= unin_pci.o ppc_chrp.o > > > # PowerPC 4xx boards > > > OBJS+= pflash_cfi02.o ppc4xx_devs.o ppc4xx_pci.o > > ppc405_uc.o ppc405_boards.o > > > OBJS+= ppc440.o ppc440_bamboo.o > > > +# PowerPC E500 boards > > > +OBJS+= ppce500_pci.o > > > ifdef FDT_LIBS > > > OBJS+= device_tree.o > > > LIBS+= $(FDT_LIBS) > > > diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c > > > new file mode 100644 > > > index 0000000..6325555 > > > --- /dev/null > > > +++ b/hw/ppce500_pci.c > > > @@ -0,0 +1,370 @@ > > > +/* > > > + * QEMU PowerPC E500 embedded processors pci controller emulation > > > + * > > > + * Copyright (C) 2009 Freescale Semiconductor, Inc. All > > rights reserved. > > > + * > > > + * Author: Yu Liu, <yu.liu@xxxxxxxxxxxxx> > > > + * > > > + * This file is derived from hw/ppc4xx_pci.c, > > > + * the copyright for that material belongs to the original owners. > > > + * > > > + * This is free software; you can redistribute it and/or modify > > > + * it under the terms of the GNU General Public License > > as published by > > > + * the Free Software Foundation; either version 2 of the > > License, or > > > + * (at your option) any later version. > > > + */ > > > + > > > +#include "hw.h" > > > +#include "ppc.h" > > > +#include "ppce500.h" > > > +typedef target_phys_addr_t pci_addr_t; > > > +#include "pci.h" > > > +#include "pci_host.h" > > > +#include "bswap.h" > > > +#include "qemu-log.h" > > > + > > > +#ifdef DEBUG_PCI > > > +#define pci_debug(fmt, arg...) fprintf(stderr, fmt, ##arg) > > > +#else > > > +#define pci_debug(fmt, arg...) > > > +#endif > > > + > > > +#define PCIE500_CFGADDR 0x0 > > > +#define PCIE500_CFGDATA 0x4 > > > +#define PCIE500_REG_BASE 0xC00 > > > +#define PCIE500_REG_SIZE (0x1000 - PCIE500_REG_BASE) > > > + > > > +#define PPCE500_PCI_CONFIG_ADDR 0x0 > > > +#define PPCE500_PCI_CONFIG_DATA 0x4 > > > +#define PPCE500_PCI_INTACK 0x8 > > > + > > > +#define PPCE500_PCI_OW1 (0xC20 - > > PCIE500_REG_BASE) > > > +#define PPCE500_PCI_OW2 (0xC40 - > > PCIE500_REG_BASE) > > > +#define PPCE500_PCI_OW3 (0xC60 - > > PCIE500_REG_BASE) > > > +#define PPCE500_PCI_OW4 (0xC80 - > > PCIE500_REG_BASE) > > > +#define PPCE500_PCI_IW3 (0xDA0 - > > PCIE500_REG_BASE) > > > +#define PPCE500_PCI_IW2 (0xDC0 - > > PCIE500_REG_BASE) > > > +#define PPCE500_PCI_IW1 (0xDE0 - > > PCIE500_REG_BASE) > > > + > > > +#define PPCE500_PCI_GASKET_TIMR (0xE20 - > > PCIE500_REG_BASE) > > > + > > > +#define PCI_POTAR 0x0 > > > +#define PCI_POTEAR 0x4 > > > +#define PCI_POWBAR 0x8 > > > +#define PCI_POWAR 0x10 > > > + > > > +#define PCI_PITAR 0x0 > > > +#define PCI_PIWBAR 0x8 > > > +#define PCI_PIWBEAR 0xC > > > +#define PCI_PIWAR 0x10 > > > + > > > +#define PPCE500_PCI_NR_POBS 5 > > > +#define PPCE500_PCI_NR_PIBS 3 > > > + > > > +struct pci_outbound { > > > + uint32_t potar; > > > + uint32_t potear; > > > + uint32_t powbar; > > > + uint32_t powar; > > > +}; > > > + > > > +struct pci_inbound { > > > + uint32_t pitar; > > > + uint32_t piwbar; > > > + uint32_t piwbear; > > > + uint32_t piwar; > > > +}; > > > + > > > +struct PPCE500PCIState { > > > + struct pci_outbound pob[PPCE500_PCI_NR_POBS]; > > > + struct pci_inbound pib[PPCE500_PCI_NR_PIBS]; > > > + uint32_t gasket_time; > > > + PCIHostState pci_state; > > > + PCIDevice *pci_dev; > > > +}; > > > + > > > +typedef struct PPCE500PCIState PPCE500PCIState; > > > + > > > +static uint32_t pcie500_cfgaddr_readl(void *opaque, > > target_phys_addr_t addr) > > > +{ > > > + PPCE500PCIState *pci = opaque; > > > + > > > + pci_debug("%s: (addr:%Lx) -> value:%x\n", __func__, addr, > > > + pci->pci_state.config_reg); > > > + return pci->pci_state.config_reg; > > > +} > > > + > > > +static CPUReadMemoryFunc *pcie500_cfgaddr_read[] = { > > > + &pcie500_cfgaddr_readl, > > > + &pcie500_cfgaddr_readl, > > > + &pcie500_cfgaddr_readl, > > > +}; > > > + > > > +static void pcie500_cfgaddr_writel(void *opaque, > > target_phys_addr_t addr, > > > + uint32_t value) > > > +{ > > > + PPCE500PCIState *controller = opaque; > > > + > > > + pci_debug("%s: value:%x -> (addr%Lx)\n", __func__, > > value, addr); > > > + controller->pci_state.config_reg = value & ~0x3; > > > +} > > > + > > > +static CPUWriteMemoryFunc *pcie500_cfgaddr_write[] = { > > > + &pcie500_cfgaddr_writel, > > > + &pcie500_cfgaddr_writel, > > > + &pcie500_cfgaddr_writel, > > > +}; > > > + > > > +static CPUReadMemoryFunc *pcie500_cfgdata_read[] = { > > > + &pci_host_data_readb, > > > + &pci_host_data_readw, > > > + &pci_host_data_readl, > > > +}; > > > + > > > +static CPUWriteMemoryFunc *pcie500_cfgdata_write[] = { > > > + &pci_host_data_writeb, > > > + &pci_host_data_writew, > > > + &pci_host_data_writel, > > > +}; > > > + > > > +static uint32_t pci_reg_read4(void *opaque, > > target_phys_addr_t addr) > > > +{ > > > + PPCE500PCIState *pci = opaque; > > > + unsigned long win; > > > + uint32_t value = 0; > > > + > > > + win = addr & 0xfe0; > > > + > > > + switch (win) { > > > + case PPCE500_PCI_OW1: > > > + case PPCE500_PCI_OW2: > > > + case PPCE500_PCI_OW3: > > > + case PPCE500_PCI_OW4: > > > + switch (addr & 0xC) { > > > + case PCI_POTAR: value = pci->pob[(addr >> 5) & > > 0x7].potar; break; > > > + case PCI_POTEAR: value = pci->pob[(addr >> 5) & > > 0x7].potear; break; > > > + case PCI_POWBAR: value = pci->pob[(addr >> 5) & > > 0x7].powbar; break; > > > + case PCI_POWAR: value = pci->pob[(addr >> 5) & > > 0x7].powar; break; > > > + default: break; > > > + } > > > + break; > > > + > > > + case PPCE500_PCI_IW3: > > > + case PPCE500_PCI_IW2: > > > + case PPCE500_PCI_IW1: > > > + switch (addr & 0xC) { > > > + case PCI_PITAR: value = pci->pib[(addr >> 5) & > > 0x3].pitar; break; > > > + case PCI_PIWBAR: value = pci->pib[(addr >> 5) & > > 0x3].piwbar; break; > > > + case PCI_PIWBEAR: value = pci->pib[(addr >> 5) & > > 0x3].piwbear; break; > > > + case PCI_PIWAR: value = pci->pib[(addr >> 5) & > > 0x3].piwar; break; > > > + default: break; > > > + }; > > > + break; > > > + > > > + case PPCE500_PCI_GASKET_TIMR: > > > + value = pci->gasket_time; > > > + break; > > > + > > > + default: > > > + break; > > > + } > > > + > > > + pci_debug("%s: win:%lx(addr:%Lx) -> > > value:%x\n",__func__,win,addr,value); > > > + return value; > > > +} > > > + > > > +static CPUReadMemoryFunc *e500_pci_reg_read[] = { > > > + &pci_reg_read4, > > > + &pci_reg_read4, > > > + &pci_reg_read4, > > > +}; > > > + > > > +static void pci_reg_write4(void *opaque, target_phys_addr_t addr, > > > + uint32_t value) > > > +{ > > > + PPCE500PCIState *pci = opaque; > > > + unsigned long win; > > > + > > > + win = addr & 0xfe0; > > > + > > > + pci_debug("%s: value:%x -> > > win:%lx(addr:%Lx)\n",__func__,value,win,addr); > > > + > > > + switch (win) { > > > + case PPCE500_PCI_OW1: > > > + case PPCE500_PCI_OW2: > > > + case PPCE500_PCI_OW3: > > > + case PPCE500_PCI_OW4: > > > + switch (addr & 0xC) { > > > + case PCI_POTAR: pci->pob[(addr >> 5) & 0x7].potar = > > value; break; > > > + case PCI_POTEAR: pci->pob[(addr >> 5) & 0x7].potear = > > value; break; > > > + case PCI_POWBAR: pci->pob[(addr >> 5) & 0x7].powbar = > > value; break; > > > + case PCI_POWAR: pci->pob[(addr >> 5) & 0x7].powar = > > value; break; > > > + default: break; > > > + }; > > > + break; > > > + > > > + case PPCE500_PCI_IW3: > > > + case PPCE500_PCI_IW2: > > > + case PPCE500_PCI_IW1: > > > + switch (addr & 0xC) { > > > + case PCI_PITAR: pci->pib[(addr >> 5) & 0x3].pitar = > > value; break; > > > + case PCI_PIWBAR: pci->pib[(addr >> 5) & 0x3].piwbar = > > value; break; > > > + case PCI_PIWBEAR: pci->pib[(addr >> 5) & 0x3].piwbear = > > value; break; > > > + case PCI_PIWAR: pci->pib[(addr >> 5) & 0x3].piwar = > > value; break; > > > + default: break; > > > + }; > > > + break; > > > + > > > + case PPCE500_PCI_GASKET_TIMR: > > > + pci->gasket_time = value; > > > + break; > > > + > > > + default: > > > + break; > > > + }; > > > +} > > > + > > > +static CPUWriteMemoryFunc *e500_pci_reg_write[] = { > > > + &pci_reg_write4, > > > + &pci_reg_write4, > > > + &pci_reg_write4, > > > +}; > > > + > > > +static int mpc85xx_pci_map_irq(PCIDevice *pci_dev, int irq_num) > > > +{ > > > + int devno = pci_dev->devfn >> 3, ret = 0; > > > + > > > + switch (devno) { > > > + /* Two PCI slot */ > > > + case 0x11: > > > + case 0x12: > > > + ret = (irq_num + devno - 0x10) % 4; > > > + break; > > > + default: > > > + printf("Error:%s:unknow dev number\n", __func__); > > > + } > > > > While I understand that the physical board only supports two PCI > > devices, I wonder if we shouldn't actually support more in QEMU. Think > > for example to virtio which can quickly takes two slots. > > Certainly it could support more. > Here only support two is because MPC8544.dts says so, kernel will access pci device according to dts file. > > The dts file in this patchset is copied from MPC8544's dts file, so we must keep it consistent here. > > If we need more pci devices, I'd prefer to do it in a isolated patch, as we need modify both here and dts file. > I see, then it's fine with me. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@xxxxxxxxxxx http://www.aurel32.net -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html