Message ID | 1263286378-10398-7-git-send-email-yamahata@valinux.co.jp |
---|---|
State | New |
Headers | show |
On Tue, Jan 12, 2010 at 05:52:58PM +0900, Isaku Yamahata wrote: > Move out pci address decoding logic from pci_data_{write, read}() > by making pci_data_{write, read}() get PCIConfigAddress. > > Cc: Blue Swirl <blauwirbel@gmail.com> > Cc: Aurelien Jarno <aurelien@aurel32.net> > Cc: Paul Brook <paul@codesourcery.com> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> > --- > hw/apb_pci.c | 12 ++++++++---- > hw/gt64xxx.c | 20 ++++++++++++-------- > hw/pci_host.c | 25 ++++++++++++++----------- > hw/pci_host.h | 5 +++-- > hw/pci_host_template.h | 15 +++++++-------- > hw/prep_pci.c | 28 ++++++++++++++++++++++------ > hw/sh_pci.c | 24 ++++++++++++++++++++---- > hw/versatile_pci.c | 30 ++++++++++++++++++++++++------ > 8 files changed, 110 insertions(+), 49 deletions(-) > > diff --git a/hw/apb_pci.c b/hw/apb_pci.c > index c14e1c0..3c098d2 100644 > --- a/hw/apb_pci.c > +++ b/hw/apb_pci.c > @@ -107,18 +107,22 @@ static CPUReadMemoryFunc * const apb_config_read[] = { > static void apb_pci_config_write(APBState *s, target_phys_addr_t addr, > uint32_t val, int size) > { > + PCIConfigAddress conf_addr; > APB_DPRINTF("%s: addr " TARGET_FMT_lx " val %x\n", __func__, addr, val); > - pci_data_write(s->host_state.bus, (addr & 0x00ffffff) | (1u << 31), val, > - size); > + pci_host_decode_config_addr_valid(&s->host_state, (addr & 0x00ffffff), > + &conf_addr); > + pci_data_write(&conf_addr, 0, val, size); > } > > static uint32_t apb_pci_config_read(APBState *s, target_phys_addr_t addr, > int size) > { > + PCIConfigAddress conf_addr; > uint32_t ret; > > - ret = pci_data_read(s->host_state.bus, (addr & 0x00ffffff) | (1u << 31), > - size); > + pci_host_decode_config_addr_valid(&s->host_state, (addr & 0x00ffffff), > + &conf_addr); > + ret = pci_data_read(&conf_addr, 0, size); > APB_DPRINTF("%s: addr " TARGET_FMT_lx " -> %x\n", __func__, addr, ret); > return ret; > } > diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c > index c8034e2..2e135dc 100644 > --- a/hw/gt64xxx.c > +++ b/hw/gt64xxx.c > @@ -527,12 +527,15 @@ static void gt64120_writel (void *opaque, target_phys_addr_t addr, > case GT_PCI0_CFGADDR: > s->pci->config_reg = val & 0x80fffffc; > break; > - case GT_PCI0_CFGDATA: > + case GT_PCI0_CFGDATA: { > + PCIConfigAddress conf_addr; > if (!(s->regs[GT_PCI0_CMD] & 1) && (s->pci->config_reg & 0x00fff800)) > val = bswap32(val); > - if (s->pci->config_reg & (1u << 31)) > - pci_data_write(s->pci->bus, s->pci->config_reg, val, 4); > + pci_host_decode_config_addr_cfge(s->pci, s->pci->config_reg, > + &conf_addr); > + pci_data_write(&conf_addr, 0, val, 4); > break; > + } > > /* Interrupts */ > case GT_INTRCAUSE: > @@ -767,14 +770,15 @@ static uint32_t gt64120_readl (void *opaque, > case GT_PCI0_CFGADDR: > val = s->pci->config_reg; > break; > - case GT_PCI0_CFGDATA: > - if (!(s->pci->config_reg & (1 << 31))) > - val = 0xffffffff; > - else > - val = pci_data_read(s->pci->bus, s->pci->config_reg, 4); > + case GT_PCI0_CFGDATA: { > + PCIConfigAddress conf_addr; > + pci_host_decode_config_addr_cfge(s->pci, s->pci->config_reg, > + &conf_addr); > + val = pci_data_read(&conf_addr, 0, 4); > if (!(s->regs[GT_PCI0_CMD] & 1) && (s->pci->config_reg & 0x00fff800)) > val = bswap32(val); > break; > + } > > case GT_PCI0_CMD: > case GT_PCI0_TOR: > diff --git a/hw/pci_host.c b/hw/pci_host.c > index fa194e2..c837110 100644 > --- a/hw/pci_host.c > +++ b/hw/pci_host.c > @@ -72,18 +72,21 @@ void pci_host_decode_config_addr_valid(const PCIHostState *s, > } > > /* the helper functio to get a PCIDeice* for a given pci address */ > -static inline PCIDevice *pci_dev_find_by_addr(PCIBus *bus, uint32_t addr) > +static inline PCIDevice *pci_dev_find_by_addr( > + const PCIConfigAddress *conf_addr) > { > - uint8_t bus_num = addr >> 16; > - uint8_t devfn = addr >> 8; > - > - return pci_find_device(bus, bus_num, PCI_SLOT(devfn), PCI_FUNC(devfn)); > + const PCIAddress *addr = &conf_addr->addr; > + if (!conf_addr->valid) { > + return NULL; > + } > + return pci_find_device(addr->domain, addr->bus, addr->slot, addr->fn); > } > > -void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len) > +void pci_data_write(PCIConfigAddress *conf_addr, > + uint32_t addr, uint32_t val, int len) > { > - PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr); > - uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1); > + PCIDevice *pci_dev = pci_dev_find_by_addr(conf_addr); > + uint32_t config_addr = conf_addr->offset | (conf_addr->addr_mask & addr); > > if (!pci_dev) > return; > @@ -93,10 +96,10 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len) > pci_dev->config_write(pci_dev, config_addr, val, len); > } > > -uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len) > +uint32_t pci_data_read(PCIConfigAddress *conf_addr, uint32_t addr, int len) > { > - PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr); > - uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1); > + PCIDevice *pci_dev = pci_dev_find_by_addr(conf_addr); > + uint32_t config_addr = conf_addr->offset | (conf_addr->addr_mask & addr); > uint32_t val; > > assert(len == 1 || len == 2 || len == 4); > diff --git a/hw/pci_host.h b/hw/pci_host.h > index ebc95f2..d2d558d 100644 > --- a/hw/pci_host.h > +++ b/hw/pci_host.h > @@ -52,8 +52,9 @@ void pci_host_decode_config_addr_valid(const PCIHostState *s, > uint32_t config_reg, > PCIConfigAddress *decoded); > > -void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len); > -uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len); > +void pci_data_write(PCIConfigAddress *conf_addr, > + uint32_t addr, uint32_t val, int len); > +uint32_t pci_data_read(PCIConfigAddress *conf_addr, uint32_t addr, int len); > > /* for mmio */ > int pci_host_conf_register_mmio(PCIHostState *s); > diff --git a/hw/pci_host_template.h b/hw/pci_host_template.h > index 2c508bf..02f0ff1 100644 > --- a/hw/pci_host_template.h > +++ b/hw/pci_host_template.h > @@ -29,28 +29,27 @@ static void glue(glue(pci_host_data_write, PCI_HOST_BWL), PCI_HOST_SUFFIX)( > void* opaque, PCI_ADDR_T addr, uint32_t val) > { > PCIHostState *s = opaque; > + PCIConfigAddress conf_addr; > > #if defined(TARGET_WORDS_BIGENDIAN) && (PCI_HOST_BITS > 8) > val = glue(bswap, PCI_HOST_BITS)(val); > #endif > PCI_DPRINTF("%s: addr " TARGET_FMT_plx " val %x\n", > __func__, (target_phys_addr_t)addr, val); > - if (s->config_reg & (1u << 31)) > - pci_data_write(s->bus, s->config_reg | (addr & 3), val, > - sizeof(glue(glue(uint, PCI_HOST_BITS), _t))); > + pci_host_decode_config_addr_cfge(s, s->config_reg, &conf_addr); > + pci_data_write(&conf_addr, addr, val, > + sizeof(glue(glue(uint, PCI_HOST_BITS), _t))); > } > > static uint32_t glue(glue(pci_host_data_read, PCI_HOST_BWL), PCI_HOST_SUFFIX)( > void* opaque, PCI_ADDR_T addr) > { > PCIHostState *s = opaque; > + PCIConfigAddress conf_addr; > uint32_t val; > > - if (!(s->config_reg & (1 << 31))) { > - return ( 1ULL << PCI_HOST_BITS ) - 1; > - } > - > - val = pci_data_read(s->bus, s->config_reg | (addr & 3), > + pci_host_decode_config_addr_cfge(s, s->config_reg, &conf_addr); > + val = pci_data_read(&conf_addr, addr, > sizeof(glue(glue(uint, PCI_HOST_BITS), _t))); > PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %x\n", > __func__, (target_phys_addr_t)addr, val); > diff --git a/hw/prep_pci.c b/hw/prep_pci.c > index 19f028c..53862d7 100644 > --- a/hw/prep_pci.c > +++ b/hw/prep_pci.c > @@ -40,10 +40,26 @@ static inline uint32_t PPC_PCIIO_config(target_phys_addr_t addr) > return (addr & 0x7ff) | (i << 11); > } > > +static void PPC_pci_data_write(PREPPCIState *s, target_phys_addr_t addr, > + uint32_t val, int len) > +{ > + PCIConfigAddress conf_addr; > + pci_host_decode_config_addr_valid(s, PPC_PCIIO_config(addr), &conf_addr); > + pci_data_write(&conf_addr, 0, val, len); > +} > + > +static uint32_t PPC_pci_data_read(PREPPCIState *s, target_phys_addr_t addr, > + int len) > +{ > + PCIConfigAddress conf_addr; > + pci_host_decode_config_addr_valid(s, PPC_PCIIO_config(addr), &conf_addr); > + return pci_data_read(&conf_addr, 0, len); > +} > + > static void PPC_PCIIO_writeb (void *opaque, target_phys_addr_t addr, uint32_t val) > { > PREPPCIState *s = opaque; > - pci_data_write(s->bus, PPC_PCIIO_config(addr), val, 1); > + PPC_pci_data_write(s, addr, val, 1); > } > > static void PPC_PCIIO_writew (void *opaque, target_phys_addr_t addr, uint32_t val) > @@ -52,7 +68,7 @@ static void PPC_PCIIO_writew (void *opaque, target_phys_addr_t addr, uint32_t va > #ifdef TARGET_WORDS_BIGENDIAN > val = bswap16(val); > #endif > - pci_data_write(s->bus, PPC_PCIIO_config(addr), val, 2); > + PPC_pci_data_write(s, addr, val, 2); > } > > static void PPC_PCIIO_writel (void *opaque, target_phys_addr_t addr, uint32_t val) > @@ -61,14 +77,14 @@ static void PPC_PCIIO_writel (void *opaque, target_phys_addr_t addr, uint32_t va > #ifdef TARGET_WORDS_BIGENDIAN > val = bswap32(val); > #endif > - pci_data_write(s->bus, PPC_PCIIO_config(addr), val, 4); > + PPC_pci_data_write(s, addr, val, 4); > } > > static uint32_t PPC_PCIIO_readb (void *opaque, target_phys_addr_t addr) > { > PREPPCIState *s = opaque; > uint32_t val; > - val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 1); > + val = PPC_pci_data_read(s, addr, 1); > return val; > } > > @@ -76,7 +92,7 @@ static uint32_t PPC_PCIIO_readw (void *opaque, target_phys_addr_t addr) > { > PREPPCIState *s = opaque; > uint32_t val; > - val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 2); > + val = PPC_pci_data_read(s, addr, 2); > #ifdef TARGET_WORDS_BIGENDIAN > val = bswap16(val); > #endif > @@ -87,7 +103,7 @@ static uint32_t PPC_PCIIO_readl (void *opaque, target_phys_addr_t addr) > { > PREPPCIState *s = opaque; > uint32_t val; > - val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 4); > + val = PPC_pci_data_read(s, addr, 4); > #ifdef TARGET_WORDS_BIGENDIAN > val = bswap32(val); > #endif > diff --git a/hw/sh_pci.c b/hw/sh_pci.c > index 046db2e..2a8f132 100644 > --- a/hw/sh_pci.c > +++ b/hw/sh_pci.c > @@ -36,6 +36,22 @@ typedef struct { > uint32_t iobr; > } SHPCIC; > > +static void sh_pci_data_write(SHPCIC *pcic, target_phys_addr_t addr, > + uint32_t val, int size) > +{ > + PCIConfigAddress conf_addr; > + pci_host_decode_config_addr_valid(&pcic->s, addr, &conf_addr); > + pci_data_write(&conf_addr, 0, val, size); > +} > + > +static uint32_t sh_pci_data_read(SHPCIC *pcic, > + target_phys_addr_t addr, int size) > +{ > + PCIConfigAddress conf_addr; > + pci_host_decode_config_addr_valid(&pcic->s, addr, &conf_addr); > + return pci_data_read(&conf_addr, 0, size); > +} > + > static void sh_pci_reg_write (void *p, target_phys_addr_t addr, uint32_t val) > { > SHPCIC *pcic = p; > @@ -53,7 +69,7 @@ static void sh_pci_reg_write (void *p, target_phys_addr_t addr, uint32_t val) > pcic->iobr = val; > break; > case 0x220: > - pci_data_write(pcic->s.bus, pcic->par, val, 4); > + sh_pci_data_write(pcic, pcic->par, val, 4); > break; > } > } > @@ -67,7 +83,7 @@ static uint32_t sh_pci_reg_read (void *p, target_phys_addr_t addr) > case 0x1c0: > return pcic->par; > case 0x220: > - return pci_data_read(pcic->s.bus, pcic->par, 4); > + return sh_pci_data_read(pcic, pcic->par, 4); > } > return 0; > } > @@ -75,13 +91,13 @@ static uint32_t sh_pci_reg_read (void *p, target_phys_addr_t addr) > static void sh_pci_mem_write (SHPCIC *pcic, target_phys_addr_t addr, > uint32_t val, int size) > { > - pci_data_write(pcic->s.bus, addr + pcic->mbr, val, size); > + sh_pci_data_write(pcic, addr + pcic->mbr, val, size); > } > > static uint32_t sh_pci_mem_read (SHPCIC *pcic, target_phys_addr_t addr, > int size) > { > - return pci_data_read(pcic->s.bus, addr + pcic->mbr, size); > + return sh_pci_data_read(pcic, addr + pcic->mbr, size); > } > > static void sh_pci_writeb (void *p, target_phys_addr_t addr, uint32_t val) > diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c > index d99b7fa..14a728a 100644 > --- a/hw/versatile_pci.c > +++ b/hw/versatile_pci.c > @@ -23,11 +23,29 @@ static inline uint32_t vpb_pci_config_addr(target_phys_addr_t addr) > return addr & 0xffffff; > } > > +static void vpb_pci_data_write(PCIHostState *s, target_phys_addr_t addr, > + uint32_t val, int len) > +{ > + PCIConfigAddress conf_addr; > + pci_host_decode_config_addr_valid(s, vpb_pci_config_addr(addr), > + &conf_addr); > + pci_data_write(&conf_addr, 0, val, len); > +} > + > +static uint32_t vpb_pci_data_read(PCIHostState *s, target_phys_addr_t addr, > + int len) > +{ > + PCIConfigAddress conf_addr; > + pci_host_decode_config_addr_valid(s, vpb_pci_config_addr(addr), > + &conf_addr); > + return pci_data_read(&conf_addr, 0, len); > +} > + I thought we will get rid of vpb_pci_config_addr, and fill in fields in PCIConfigAddress directly. If we don't, and still recode into PC format, this is not making code any prettier so I don't really see what this buys us. > static void pci_vpb_config_writeb (void *opaque, target_phys_addr_t addr, > uint32_t val) > { > PCIHostState *s = opaque; > - pci_data_write(s->bus, vpb_pci_config_addr (addr), val, 1); > + vpb_pci_data_write(s, addr, val, 1); > } > > static void pci_vpb_config_writew (void *opaque, target_phys_addr_t addr, > @@ -37,7 +55,7 @@ static void pci_vpb_config_writew (void *opaque, target_phys_addr_t addr, > #ifdef TARGET_WORDS_BIGENDIAN > val = bswap16(val); > #endif > - pci_data_write(s->bus, vpb_pci_config_addr (addr), val, 2); > + vpb_pci_data_write(s, addr, val, 2); > } > > static void pci_vpb_config_writel (void *opaque, target_phys_addr_t addr, > @@ -47,14 +65,14 @@ static void pci_vpb_config_writel (void *opaque, target_phys_addr_t addr, > #ifdef TARGET_WORDS_BIGENDIAN > val = bswap32(val); > #endif > - pci_data_write(s->bus, vpb_pci_config_addr (addr), val, 4); > + vpb_pci_data_write(s, addr, val, 4); > } > > static uint32_t pci_vpb_config_readb (void *opaque, target_phys_addr_t addr) > { > PCIHostState *s = opaque; > uint32_t val; > - val = pci_data_read(s->bus, vpb_pci_config_addr (addr), 1); > + val = vpb_pci_data_read(s, addr, 1); > return val; > } > > @@ -62,7 +80,7 @@ static uint32_t pci_vpb_config_readw (void *opaque, target_phys_addr_t addr) > { > PCIHostState *s = opaque; > uint32_t val; > - val = pci_data_read(s->bus, vpb_pci_config_addr (addr), 2); > + val = vpb_pci_data_read(s, addr, 2); > #ifdef TARGET_WORDS_BIGENDIAN > val = bswap16(val); > #endif > @@ -73,7 +91,7 @@ static uint32_t pci_vpb_config_readl (void *opaque, target_phys_addr_t addr) > { > PCIHostState *s = opaque; > uint32_t val; > - val = pci_data_read(s->bus, vpb_pci_config_addr (addr), 4); > + val = vpb_pci_data_read(s, addr, 4); > #ifdef TARGET_WORDS_BIGENDIAN > val = bswap32(val); > #endif > -- > 1.6.5.4
On Tue, Jan 12, 2010 at 12:12:36PM +0200, Michael S. Tsirkin wrote: > On Tue, Jan 12, 2010 at 05:52:58PM +0900, Isaku Yamahata wrote: > > diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c > > index d99b7fa..14a728a 100644 > > --- a/hw/versatile_pci.c > > +++ b/hw/versatile_pci.c > > @@ -23,11 +23,29 @@ static inline uint32_t vpb_pci_config_addr(target_phys_addr_t addr) > > return addr & 0xffffff; > > } > > > > +static void vpb_pci_data_write(PCIHostState *s, target_phys_addr_t addr, > > + uint32_t val, int len) > > +{ > > + PCIConfigAddress conf_addr; > > + pci_host_decode_config_addr_valid(s, vpb_pci_config_addr(addr), > > + &conf_addr); > > + pci_data_write(&conf_addr, 0, val, len); > > +} > > + > > +static uint32_t vpb_pci_data_read(PCIHostState *s, target_phys_addr_t addr, > > + int len) > > +{ > > + PCIConfigAddress conf_addr; > > + pci_host_decode_config_addr_valid(s, vpb_pci_config_addr(addr), > > + &conf_addr); > > + return pci_data_read(&conf_addr, 0, len); > > +} > > + > > > I thought we will get rid of vpb_pci_config_addr, and fill in > fields in PCIConfigAddress directly. If we don't, and still > recode into PC format, this is not making code any prettier > so I don't really see what this buys us. I should have explain my plan. After introducing decode callback by Alexander patch, I'd like to remove them. I was waiting for his respin, but he seems to be busy. So I created this patch series to make his respin easier (hopefully). For that sake, I kept the original structures. I'm fine with either ways. Waiting for his patch merged or make this patch series into shape.
Isaku Yamahata wrote: > Move out pci address decoding logic from pci_data_{write, read}() > by making pci_data_{write, read}() get PCIConfigAddress. > I think this is going in the wrong direction. You are keeping the device specific MMIO/PIO based config space access handlers and just move the address decoding into a separate function. The whole idea of my patch was to only use pci_host to handle the MMIOs and have a simple callback into a bus specific decoder. That way we could probably remove most of the functions you're patching in this patch. Or am I wrong here? Alex
Isaku Yamahata wrote: > On Tue, Jan 12, 2010 at 12:12:36PM +0200, Michael S. Tsirkin wrote: > >> On Tue, Jan 12, 2010 at 05:52:58PM +0900, Isaku Yamahata wrote: >> >>> diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c >>> index d99b7fa..14a728a 100644 >>> --- a/hw/versatile_pci.c >>> +++ b/hw/versatile_pci.c >>> @@ -23,11 +23,29 @@ static inline uint32_t vpb_pci_config_addr(target_phys_addr_t addr) >>> return addr & 0xffffff; >>> } >>> >>> +static void vpb_pci_data_write(PCIHostState *s, target_phys_addr_t addr, >>> + uint32_t val, int len) >>> +{ >>> + PCIConfigAddress conf_addr; >>> + pci_host_decode_config_addr_valid(s, vpb_pci_config_addr(addr), >>> + &conf_addr); >>> + pci_data_write(&conf_addr, 0, val, len); >>> +} >>> + >>> +static uint32_t vpb_pci_data_read(PCIHostState *s, target_phys_addr_t addr, >>> + int len) >>> +{ >>> + PCIConfigAddress conf_addr; >>> + pci_host_decode_config_addr_valid(s, vpb_pci_config_addr(addr), >>> + &conf_addr); >>> + return pci_data_read(&conf_addr, 0, len); >>> +} >>> + >>> >> I thought we will get rid of vpb_pci_config_addr, and fill in >> fields in PCIConfigAddress directly. If we don't, and still >> recode into PC format, this is not making code any prettier >> so I don't really see what this buys us. >> > > I should have explain my plan. > > After introducing decode callback by Alexander patch, > I'd like to remove them. > I was waiting for his respin, but he seems to be busy. > So I created this patch series to make his respin easier (hopefully). > For that sake, I kept the original structures. > > I'm fine with either ways. Waiting for his patch merged or > make this patch series into shape. > Oh, I guess I read this mail too late. Sorry about that :-). Alex
> I thought we will get rid of vpb_pci_config_addr, and fill in > fields in PCIConfigAddress directly. If we don't, and still > recode into PC format, this is not making code any prettier > so I don't really see what this buys us. I agree. This patch seems to be introducing churn for no benefit. See also comments about direct v.s. indirect access to PCI config space. I suspect you're trying to use a common implementation for two fundamentally different things. Paul
diff --git a/hw/apb_pci.c b/hw/apb_pci.c index c14e1c0..3c098d2 100644 --- a/hw/apb_pci.c +++ b/hw/apb_pci.c @@ -107,18 +107,22 @@ static CPUReadMemoryFunc * const apb_config_read[] = { static void apb_pci_config_write(APBState *s, target_phys_addr_t addr, uint32_t val, int size) { + PCIConfigAddress conf_addr; APB_DPRINTF("%s: addr " TARGET_FMT_lx " val %x\n", __func__, addr, val); - pci_data_write(s->host_state.bus, (addr & 0x00ffffff) | (1u << 31), val, - size); + pci_host_decode_config_addr_valid(&s->host_state, (addr & 0x00ffffff), + &conf_addr); + pci_data_write(&conf_addr, 0, val, size); } static uint32_t apb_pci_config_read(APBState *s, target_phys_addr_t addr, int size) { + PCIConfigAddress conf_addr; uint32_t ret; - ret = pci_data_read(s->host_state.bus, (addr & 0x00ffffff) | (1u << 31), - size); + pci_host_decode_config_addr_valid(&s->host_state, (addr & 0x00ffffff), + &conf_addr); + ret = pci_data_read(&conf_addr, 0, size); APB_DPRINTF("%s: addr " TARGET_FMT_lx " -> %x\n", __func__, addr, ret); return ret; } diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c index c8034e2..2e135dc 100644 --- a/hw/gt64xxx.c +++ b/hw/gt64xxx.c @@ -527,12 +527,15 @@ static void gt64120_writel (void *opaque, target_phys_addr_t addr, case GT_PCI0_CFGADDR: s->pci->config_reg = val & 0x80fffffc; break; - case GT_PCI0_CFGDATA: + case GT_PCI0_CFGDATA: { + PCIConfigAddress conf_addr; if (!(s->regs[GT_PCI0_CMD] & 1) && (s->pci->config_reg & 0x00fff800)) val = bswap32(val); - if (s->pci->config_reg & (1u << 31)) - pci_data_write(s->pci->bus, s->pci->config_reg, val, 4); + pci_host_decode_config_addr_cfge(s->pci, s->pci->config_reg, + &conf_addr); + pci_data_write(&conf_addr, 0, val, 4); break; + } /* Interrupts */ case GT_INTRCAUSE: @@ -767,14 +770,15 @@ static uint32_t gt64120_readl (void *opaque, case GT_PCI0_CFGADDR: val = s->pci->config_reg; break; - case GT_PCI0_CFGDATA: - if (!(s->pci->config_reg & (1 << 31))) - val = 0xffffffff; - else - val = pci_data_read(s->pci->bus, s->pci->config_reg, 4); + case GT_PCI0_CFGDATA: { + PCIConfigAddress conf_addr; + pci_host_decode_config_addr_cfge(s->pci, s->pci->config_reg, + &conf_addr); + val = pci_data_read(&conf_addr, 0, 4); if (!(s->regs[GT_PCI0_CMD] & 1) && (s->pci->config_reg & 0x00fff800)) val = bswap32(val); break; + } case GT_PCI0_CMD: case GT_PCI0_TOR: diff --git a/hw/pci_host.c b/hw/pci_host.c index fa194e2..c837110 100644 --- a/hw/pci_host.c +++ b/hw/pci_host.c @@ -72,18 +72,21 @@ void pci_host_decode_config_addr_valid(const PCIHostState *s, } /* the helper functio to get a PCIDeice* for a given pci address */ -static inline PCIDevice *pci_dev_find_by_addr(PCIBus *bus, uint32_t addr) +static inline PCIDevice *pci_dev_find_by_addr( + const PCIConfigAddress *conf_addr) { - uint8_t bus_num = addr >> 16; - uint8_t devfn = addr >> 8; - - return pci_find_device(bus, bus_num, PCI_SLOT(devfn), PCI_FUNC(devfn)); + const PCIAddress *addr = &conf_addr->addr; + if (!conf_addr->valid) { + return NULL; + } + return pci_find_device(addr->domain, addr->bus, addr->slot, addr->fn); } -void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len) +void pci_data_write(PCIConfigAddress *conf_addr, + uint32_t addr, uint32_t val, int len) { - PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr); - uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1); + PCIDevice *pci_dev = pci_dev_find_by_addr(conf_addr); + uint32_t config_addr = conf_addr->offset | (conf_addr->addr_mask & addr); if (!pci_dev) return; @@ -93,10 +96,10 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len) pci_dev->config_write(pci_dev, config_addr, val, len); } -uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len) +uint32_t pci_data_read(PCIConfigAddress *conf_addr, uint32_t addr, int len) { - PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr); - uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1); + PCIDevice *pci_dev = pci_dev_find_by_addr(conf_addr); + uint32_t config_addr = conf_addr->offset | (conf_addr->addr_mask & addr); uint32_t val; assert(len == 1 || len == 2 || len == 4); diff --git a/hw/pci_host.h b/hw/pci_host.h index ebc95f2..d2d558d 100644 --- a/hw/pci_host.h +++ b/hw/pci_host.h @@ -52,8 +52,9 @@ void pci_host_decode_config_addr_valid(const PCIHostState *s, uint32_t config_reg, PCIConfigAddress *decoded); -void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len); -uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len); +void pci_data_write(PCIConfigAddress *conf_addr, + uint32_t addr, uint32_t val, int len); +uint32_t pci_data_read(PCIConfigAddress *conf_addr, uint32_t addr, int len); /* for mmio */ int pci_host_conf_register_mmio(PCIHostState *s); diff --git a/hw/pci_host_template.h b/hw/pci_host_template.h index 2c508bf..02f0ff1 100644 --- a/hw/pci_host_template.h +++ b/hw/pci_host_template.h @@ -29,28 +29,27 @@ static void glue(glue(pci_host_data_write, PCI_HOST_BWL), PCI_HOST_SUFFIX)( void* opaque, PCI_ADDR_T addr, uint32_t val) { PCIHostState *s = opaque; + PCIConfigAddress conf_addr; #if defined(TARGET_WORDS_BIGENDIAN) && (PCI_HOST_BITS > 8) val = glue(bswap, PCI_HOST_BITS)(val); #endif PCI_DPRINTF("%s: addr " TARGET_FMT_plx " val %x\n", __func__, (target_phys_addr_t)addr, val); - if (s->config_reg & (1u << 31)) - pci_data_write(s->bus, s->config_reg | (addr & 3), val, - sizeof(glue(glue(uint, PCI_HOST_BITS), _t))); + pci_host_decode_config_addr_cfge(s, s->config_reg, &conf_addr); + pci_data_write(&conf_addr, addr, val, + sizeof(glue(glue(uint, PCI_HOST_BITS), _t))); } static uint32_t glue(glue(pci_host_data_read, PCI_HOST_BWL), PCI_HOST_SUFFIX)( void* opaque, PCI_ADDR_T addr) { PCIHostState *s = opaque; + PCIConfigAddress conf_addr; uint32_t val; - if (!(s->config_reg & (1 << 31))) { - return ( 1ULL << PCI_HOST_BITS ) - 1; - } - - val = pci_data_read(s->bus, s->config_reg | (addr & 3), + pci_host_decode_config_addr_cfge(s, s->config_reg, &conf_addr); + val = pci_data_read(&conf_addr, addr, sizeof(glue(glue(uint, PCI_HOST_BITS), _t))); PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %x\n", __func__, (target_phys_addr_t)addr, val); diff --git a/hw/prep_pci.c b/hw/prep_pci.c index 19f028c..53862d7 100644 --- a/hw/prep_pci.c +++ b/hw/prep_pci.c @@ -40,10 +40,26 @@ static inline uint32_t PPC_PCIIO_config(target_phys_addr_t addr) return (addr & 0x7ff) | (i << 11); } +static void PPC_pci_data_write(PREPPCIState *s, target_phys_addr_t addr, + uint32_t val, int len) +{ + PCIConfigAddress conf_addr; + pci_host_decode_config_addr_valid(s, PPC_PCIIO_config(addr), &conf_addr); + pci_data_write(&conf_addr, 0, val, len); +} + +static uint32_t PPC_pci_data_read(PREPPCIState *s, target_phys_addr_t addr, + int len) +{ + PCIConfigAddress conf_addr; + pci_host_decode_config_addr_valid(s, PPC_PCIIO_config(addr), &conf_addr); + return pci_data_read(&conf_addr, 0, len); +} + static void PPC_PCIIO_writeb (void *opaque, target_phys_addr_t addr, uint32_t val) { PREPPCIState *s = opaque; - pci_data_write(s->bus, PPC_PCIIO_config(addr), val, 1); + PPC_pci_data_write(s, addr, val, 1); } static void PPC_PCIIO_writew (void *opaque, target_phys_addr_t addr, uint32_t val) @@ -52,7 +68,7 @@ static void PPC_PCIIO_writew (void *opaque, target_phys_addr_t addr, uint32_t va #ifdef TARGET_WORDS_BIGENDIAN val = bswap16(val); #endif - pci_data_write(s->bus, PPC_PCIIO_config(addr), val, 2); + PPC_pci_data_write(s, addr, val, 2); } static void PPC_PCIIO_writel (void *opaque, target_phys_addr_t addr, uint32_t val) @@ -61,14 +77,14 @@ static void PPC_PCIIO_writel (void *opaque, target_phys_addr_t addr, uint32_t va #ifdef TARGET_WORDS_BIGENDIAN val = bswap32(val); #endif - pci_data_write(s->bus, PPC_PCIIO_config(addr), val, 4); + PPC_pci_data_write(s, addr, val, 4); } static uint32_t PPC_PCIIO_readb (void *opaque, target_phys_addr_t addr) { PREPPCIState *s = opaque; uint32_t val; - val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 1); + val = PPC_pci_data_read(s, addr, 1); return val; } @@ -76,7 +92,7 @@ static uint32_t PPC_PCIIO_readw (void *opaque, target_phys_addr_t addr) { PREPPCIState *s = opaque; uint32_t val; - val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 2); + val = PPC_pci_data_read(s, addr, 2); #ifdef TARGET_WORDS_BIGENDIAN val = bswap16(val); #endif @@ -87,7 +103,7 @@ static uint32_t PPC_PCIIO_readl (void *opaque, target_phys_addr_t addr) { PREPPCIState *s = opaque; uint32_t val; - val = pci_data_read(s->bus, PPC_PCIIO_config(addr), 4); + val = PPC_pci_data_read(s, addr, 4); #ifdef TARGET_WORDS_BIGENDIAN val = bswap32(val); #endif diff --git a/hw/sh_pci.c b/hw/sh_pci.c index 046db2e..2a8f132 100644 --- a/hw/sh_pci.c +++ b/hw/sh_pci.c @@ -36,6 +36,22 @@ typedef struct { uint32_t iobr; } SHPCIC; +static void sh_pci_data_write(SHPCIC *pcic, target_phys_addr_t addr, + uint32_t val, int size) +{ + PCIConfigAddress conf_addr; + pci_host_decode_config_addr_valid(&pcic->s, addr, &conf_addr); + pci_data_write(&conf_addr, 0, val, size); +} + +static uint32_t sh_pci_data_read(SHPCIC *pcic, + target_phys_addr_t addr, int size) +{ + PCIConfigAddress conf_addr; + pci_host_decode_config_addr_valid(&pcic->s, addr, &conf_addr); + return pci_data_read(&conf_addr, 0, size); +} + static void sh_pci_reg_write (void *p, target_phys_addr_t addr, uint32_t val) { SHPCIC *pcic = p; @@ -53,7 +69,7 @@ static void sh_pci_reg_write (void *p, target_phys_addr_t addr, uint32_t val) pcic->iobr = val; break; case 0x220: - pci_data_write(pcic->s.bus, pcic->par, val, 4); + sh_pci_data_write(pcic, pcic->par, val, 4); break; } } @@ -67,7 +83,7 @@ static uint32_t sh_pci_reg_read (void *p, target_phys_addr_t addr) case 0x1c0: return pcic->par; case 0x220: - return pci_data_read(pcic->s.bus, pcic->par, 4); + return sh_pci_data_read(pcic, pcic->par, 4); } return 0; } @@ -75,13 +91,13 @@ static uint32_t sh_pci_reg_read (void *p, target_phys_addr_t addr) static void sh_pci_mem_write (SHPCIC *pcic, target_phys_addr_t addr, uint32_t val, int size) { - pci_data_write(pcic->s.bus, addr + pcic->mbr, val, size); + sh_pci_data_write(pcic, addr + pcic->mbr, val, size); } static uint32_t sh_pci_mem_read (SHPCIC *pcic, target_phys_addr_t addr, int size) { - return pci_data_read(pcic->s.bus, addr + pcic->mbr, size); + return sh_pci_data_read(pcic, addr + pcic->mbr, size); } static void sh_pci_writeb (void *p, target_phys_addr_t addr, uint32_t val) diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c index d99b7fa..14a728a 100644 --- a/hw/versatile_pci.c +++ b/hw/versatile_pci.c @@ -23,11 +23,29 @@ static inline uint32_t vpb_pci_config_addr(target_phys_addr_t addr) return addr & 0xffffff; } +static void vpb_pci_data_write(PCIHostState *s, target_phys_addr_t addr, + uint32_t val, int len) +{ + PCIConfigAddress conf_addr; + pci_host_decode_config_addr_valid(s, vpb_pci_config_addr(addr), + &conf_addr); + pci_data_write(&conf_addr, 0, val, len); +} + +static uint32_t vpb_pci_data_read(PCIHostState *s, target_phys_addr_t addr, + int len) +{ + PCIConfigAddress conf_addr; + pci_host_decode_config_addr_valid(s, vpb_pci_config_addr(addr), + &conf_addr); + return pci_data_read(&conf_addr, 0, len); +} + static void pci_vpb_config_writeb (void *opaque, target_phys_addr_t addr, uint32_t val) { PCIHostState *s = opaque; - pci_data_write(s->bus, vpb_pci_config_addr (addr), val, 1); + vpb_pci_data_write(s, addr, val, 1); } static void pci_vpb_config_writew (void *opaque, target_phys_addr_t addr, @@ -37,7 +55,7 @@ static void pci_vpb_config_writew (void *opaque, target_phys_addr_t addr, #ifdef TARGET_WORDS_BIGENDIAN val = bswap16(val); #endif - pci_data_write(s->bus, vpb_pci_config_addr (addr), val, 2); + vpb_pci_data_write(s, addr, val, 2); } static void pci_vpb_config_writel (void *opaque, target_phys_addr_t addr, @@ -47,14 +65,14 @@ static void pci_vpb_config_writel (void *opaque, target_phys_addr_t addr, #ifdef TARGET_WORDS_BIGENDIAN val = bswap32(val); #endif - pci_data_write(s->bus, vpb_pci_config_addr (addr), val, 4); + vpb_pci_data_write(s, addr, val, 4); } static uint32_t pci_vpb_config_readb (void *opaque, target_phys_addr_t addr) { PCIHostState *s = opaque; uint32_t val; - val = pci_data_read(s->bus, vpb_pci_config_addr (addr), 1); + val = vpb_pci_data_read(s, addr, 1); return val; } @@ -62,7 +80,7 @@ static uint32_t pci_vpb_config_readw (void *opaque, target_phys_addr_t addr) { PCIHostState *s = opaque; uint32_t val; - val = pci_data_read(s->bus, vpb_pci_config_addr (addr), 2); + val = vpb_pci_data_read(s, addr, 2); #ifdef TARGET_WORDS_BIGENDIAN val = bswap16(val); #endif @@ -73,7 +91,7 @@ static uint32_t pci_vpb_config_readl (void *opaque, target_phys_addr_t addr) { PCIHostState *s = opaque; uint32_t val; - val = pci_data_read(s->bus, vpb_pci_config_addr (addr), 4); + val = vpb_pci_data_read(s, addr, 4); #ifdef TARGET_WORDS_BIGENDIAN val = bswap32(val); #endif
Move out pci address decoding logic from pci_data_{write, read}() by making pci_data_{write, read}() get PCIConfigAddress. Cc: Blue Swirl <blauwirbel@gmail.com> Cc: Aurelien Jarno <aurelien@aurel32.net> Cc: Paul Brook <paul@codesourcery.com> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> --- hw/apb_pci.c | 12 ++++++++---- hw/gt64xxx.c | 20 ++++++++++++-------- hw/pci_host.c | 25 ++++++++++++++----------- hw/pci_host.h | 5 +++-- hw/pci_host_template.h | 15 +++++++-------- hw/prep_pci.c | 28 ++++++++++++++++++++++------ hw/sh_pci.c | 24 ++++++++++++++++++++---- hw/versatile_pci.c | 30 ++++++++++++++++++++++++------ 8 files changed, 110 insertions(+), 49 deletions(-)