[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH V5 19/29] pci: split out ioport address parsing
From: |
Isaku Yamahata |
Subject: |
[Qemu-devel] Re: [PATCH V5 19/29] pci: split out ioport address parsing from pci configuration access logic. |
Date: |
Tue, 13 Oct 2009 23:14:11 +0900 |
User-agent: |
Mutt/1.5.6i |
On Sat, Oct 10, 2009 at 09:45:00PM +0200, Michael S. Tsirkin wrote:
> On Fri, Oct 09, 2009 at 03:28:52PM +0900, Isaku Yamahata wrote:
> > Split out pci_data_{write, read} into two part, one is the code which
> > convert ioport address into pci device and the other is the code
> > which access PCI configuration space.
> >
> > Later PCI express code will use the access code.
> > PCI express addressing scheme is different so that the address
> > must be parsed differently.
> >
> > PCI:
> > 0- 7 bit: offset in the configuration space (256bytes)
> > 7-15 bit: devfn
> > 16-24 bit: bus
> >
> > PCI express:
> > 0-11 bit: offset in the configuration space (4KBytes)
> > 12-19 bit: devfn
> > 20-28 bit: bus
> >
> > Signed-off-by: Isaku Yamahata <address@hidden>
>
> So I think I said this already: the only useful thing that
> pci_data_read/write does is address conversion which is different for
> express. PCI Express should do its own thing and then call
> pci_dev->config_read / pci_dev->config_write directly.
>
> I think this will even make it possible to put express code in a
> separate file from pci.c.
Yes, I did it. And with the next patch, they will moved to pci_host.c
too as you noticed it.
Hmm, you seem to be saying that pci_dev_{write, read}_config() aren't
needed, however they are used by both pci_host.c and pcie_host.c
as follows. So introducing pci_dev_{write, read}_config() makes sense.
void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len)
{
pci_dev_write_config(pci_addr_to_dev(s, addr), pci_addr_to_config(addr),
val, len);
}
It parses pci address and pass it to lower function.
With the patch of "pci: pcie host and mmcfg support.",
pcie counter part is as follows.
static void pcie_mmcfg_data_write(PCIBus *s,
uint32_t mmcfg_addr, uint32_t val, int len)
{
pci_dev_write_config(pcie_mmcfg_addr_to_dev(s, mmcfg_addr),
PCIE_MMCFG_CONFOFFSET(mmcfg_addr),
val, len);
}
It parses pcie mmconfig address and pass it to lower function.
>
> > ---
> > hw/pci.c | 80
> > ++++++++++++++++++++++++++++++-------------------------------
> > 1 files changed, 39 insertions(+), 41 deletions(-)
> >
> > diff --git a/hw/pci.c b/hw/pci.c
> > index 0021c96..d472b58 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -654,46 +654,24 @@ void pci_default_write_config(PCIDevice *d, uint32_t
> > addr, uint32_t val, int l)
> > pci_update_mappings(d);
> > }
> >
> > -static inline PCIDevice *pci_addr_to_dev(PCIBus *bus, uint32_t addr)
> > -{
> > - uint8_t bus_num = (addr >> 16) & 0xff;
> > - uint8_t devfn = (addr >> 8) & 0xff;
> > - return pci_find_device(bus, bus_num, PCI_SLOT(devfn), PCI_FUNC(devfn));
> > -}
> > -
> > -static inline int pci_addr_to_config(uint32_t addr)
> > -{
> > - return addr & 0xff;
> > -}
> > -
> > -void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len)
> > +static void pci_dev_write_config(PCIDevice *pci_dev,
> > + uint32_t config_addr, uint32_t val, int
> > len)
> > {
> > - PCIBus *s = opaque;
> > - PCIDevice *pci_dev;
> > - int config_addr;
> > -
> > -#if 0
> > - PCI_DPRINTF("pci_data_write: addr=%08x val=%08x len=%d\n",
> > - addr, val, len);
> > -#endif
> > - pci_dev = pci_addr_to_dev(s, addr);
> > + assert(len == 1 || len == 2 || len == 4);
> > if (!pci_dev)
> > return;
> > - config_addr = addr & 0xff;
> > - config_addr = pci_addr_to_config(addr);
> > - PCI_DPRINTF("pci_config_write: %s: addr=%02x val=%08x len=%d\n",
> > - pci_dev->name, config_addr, val, len);
> > +
> > + PCI_DPRINTF("%s: %s: addr=%02"PRIx32" val=%08"PRI32x" len=%d\n",
> > + __func__, pci_dev->name, config_addr, val, len);
> > pci_dev->config_write(pci_dev, config_addr, val, len);
> > }
> >
> > -uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
> > +static uint32_t pci_dev_read_config(PCIDevice *pci_dev,
> > + uint32_t config_addr, int len)
> > {
> > - PCIBus *s = opaque;
> > - PCIDevice *pci_dev;
> > - int config_addr;
> > uint32_t val;
> >
> > - pci_dev = pci_addr_to_dev(s, addr);
> > + assert(len == 1 || len == 2 || len == 4);
> > if (!pci_dev) {
> > switch(len) {
> > case 1:
> > @@ -707,20 +685,40 @@ uint32_t pci_data_read(void *opaque, uint32_t addr,
> > int len)
> > val = 0xffffffff;
> > break;
> > }
> > - goto the_end;
> > + } else {
> > + val = pci_dev->config_read(pci_dev, config_addr, len);
> > + PCI_DPRINTF("%s: %s: addr=%02"PRIx32" val=%08"PRIx32" len=%d\n",
> > + __func__, pci_dev->name, config_addr, val, len);
> > }
> > - config_addr = pci_addr_to_config(addr);
> > - val = pci_dev->config_read(pci_dev, config_addr, len);
> > - PCI_DPRINTF("pci_config_read: %s: addr=%02x val=%08x len=%d\n",
> > - pci_dev->name, config_addr, val, len);
> > - the_end:
> > -#if 0
> > - PCI_DPRINTF("pci_data_read: addr=%08x val=%08x len=%d\n",
> > - addr, val, len);
> > -#endif
> > return val;
> > }
> >
> > +static inline PCIDevice *pci_addr_to_dev(PCIBus *bus, uint32_t addr)
> > +{
> > + uint8_t bus_num = (addr >> 16) & 0xff;
> > + uint8_t devfn = (addr >> 8) & 0xff;
> > + return pci_find_device(bus, bus_num, PCI_SLOT(devfn), PCI_FUNC(devfn));
> > +}
> > +
> > +static inline int pci_addr_to_config(uint32_t addr)
> > +{
> > + return addr & 0xff;
> > +}
> > +
> > +void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len)
> > +{
> > + PCIBus *s = opaque;
> > + pci_dev_write_config(pci_addr_to_dev(s, addr),
> > pci_addr_to_config(addr),
> > + val, len);
> > +}
> > +
> > +uint32_t pci_data_read(void *opaque, uint32_t addr, int len)
> > +{
> > + PCIBus *s = opaque;
> > + return pci_dev_read_config(pci_addr_to_dev(s, addr),
> > + pci_addr_to_config(addr), len);
> > +}
> > +
> > /***********************************************************/
> > /* generic PCI irq support */
> >
>
--
yamahata
- [Qemu-devel] [PATCH V5 21/29] pci_host: change the signature of pci_data_{read, write}., (continued)
- [Qemu-devel] [PATCH V5 21/29] pci_host: change the signature of pci_data_{read, write}., Isaku Yamahata, 2009/10/09
- [Qemu-devel] [PATCH V5 24/29] pci: fix pci_default_write_config(), Isaku Yamahata, 2009/10/09
- [Qemu-devel] [PATCH V5 25/29] pci: add helper functions for pci config write function., Isaku Yamahata, 2009/10/09
- [Qemu-devel] [PATCH V5 20/29] pci: move pci host stuff from pci.c to pci_host.c, Isaku Yamahata, 2009/10/09
- [Qemu-devel] [PATCH V5 06/29] pci: use helper functions to access pci config space., Isaku Yamahata, 2009/10/09
- [Qemu-devel] [PATCH V5 19/29] pci: split out ioport address parsing from pci configuration access logic., Isaku Yamahata, 2009/10/09
- [Qemu-devel] [PATCH V5 26/29] pci: use helper function in pci_default_write_config(), Isaku Yamahata, 2009/10/09
- [Qemu-devel] [PATCH V5 13/29] pci: introduce pcibus_t to represent pci bus address/size instead of uint32_t, Isaku Yamahata, 2009/10/09
- [Qemu-devel] [PATCH V5 16/29] pci: 64bit bar support., Isaku Yamahata, 2009/10/09
[Qemu-devel] [PATCH V5 22/29] vmstate: add VMSTATE_ARRAY_POINTER for pointer to array., Isaku Yamahata, 2009/10/09
[Qemu-devel] [PATCH V5 23/29] pci: pcie host and mmcfg support., Isaku Yamahata, 2009/10/09