From: Isaku Yamahata <yamahata@valinux.co.jp>
To: Anthony Liguori <aliguori@us.ibm.com>
Cc: Michael Tsirkin <mstirkin@redhat.com>,
qemu-devel@nongnu.org, Alex Graf <agraf@suse.de>
Subject: Re: [Qemu-devel] [PATCH 01/15] pci: add new bus functions
Date: Wed, 10 Feb 2010 17:09:35 +0900 [thread overview]
Message-ID: <20100210080935.GL22624@valinux.co.jp> (raw)
In-Reply-To: <1265752899-26980-2-git-send-email-aliguori@us.ibm.com>
On Tue, Feb 09, 2010 at 04:01:25PM -0600, Anthony Liguori wrote:
> - mapping and managing io regions
> - reading and writing physical memory
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> hw/pci.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> hw/pci.h | 18 ++++++-
> 2 files changed, 181 insertions(+), 9 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 9ad63dd..5460f27 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -664,6 +664,9 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev)
> r->addr),
> r->filtered_size,
> IO_MEM_UNASSIGNED);
> + if (!r->map_func && r->read && r->write) {
> + cpu_unregister_io_memory(r->io_memory_addr);
> + }
> }
> }
> }
> @@ -687,16 +690,15 @@ static int pci_unregister_device(DeviceState *dev)
> return 0;
> }
>
> -void pci_register_bar(PCIDevice *pci_dev, int region_num,
> - pcibus_t size, int type,
> - PCIMapIORegionFunc *map_func)
> +static PCIIORegion *do_pci_register_bar(PCIDevice *pci_dev, int region_num,
> + pcibus_t size, int type)
> {
> PCIIORegion *r;
> uint32_t addr;
> pcibus_t wmask;
>
> if ((unsigned int)region_num >= PCI_NUM_REGIONS)
> - return;
> + return NULL;
>
> if (size & (size-1)) {
> fprintf(stderr, "ERROR: PCI region size must be pow2 "
> @@ -709,7 +711,6 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> r->size = size;
> r->filtered_size = size;
> r->type = type;
> - r->map_func = map_func;
>
> wmask = ~(size - 1);
> addr = pci_bar(pci_dev, region_num);
> @@ -726,6 +727,100 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> pci_set_long(pci_dev->wmask + addr, wmask & 0xffffffff);
> pci_set_long(pci_dev->cmask + addr, 0xffffffff);
> }
> +
> + return r;
> +}
> +
> +void pci_register_bar(PCIDevice *pci_dev, int region_num,
> + pcibus_t size, int type,
> + PCIMapIORegionFunc *map_func)
> +{
> + PCIIORegion *r;
> + r = do_pci_register_bar(pci_dev, region_num, size, type);
> + if (r) {
> + r->map_func = map_func;
> + }
> +}
> +
> +void pci_memory_read(PCIDevice *pci_dev, pcibus_t addr, void *buf, int len)
> +{
> + cpu_physical_memory_read(addr, buf, len);
> +}
> +
> +void pci_memory_write(PCIDevice *pci_dev, pcibus_t addr,
> + const void *buf, int len)
> +{
> + cpu_physical_memory_write(addr, buf, len);
> +}
> +
Isn't something like cpu_to_pci_addr() needed in theory?
> +static void pci_io_region_writeb(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, addr, 1, value);
> +}
> +
> +static void pci_io_region_writew(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, addr, 2, value);
> +}
> +
> +static void pci_io_region_writel(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, addr, 4, value);
> +}
> +
> +static uint32_t pci_io_region_readb(void *opaque, target_phys_addr_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, addr, 1);
> +}
> +
> +static uint32_t pci_io_region_readw(void *opaque, target_phys_addr_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, addr, 2);
> +}
> +
> +static uint32_t pci_io_region_readl(void *opaque, target_phys_addr_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, addr, 4);
> +}
They pass addr, not offset from base address.
pci_io_region_ioport_ {read, write}[bwl]() pass offset.
It's inconsistent. offset should be passed.
Isn't the inverse of pci_to_cpu_addr() necessary in theory?
> +
> +static CPUReadMemoryFunc * const pci_io_region_readfn[3] = {
> + pci_io_region_readb,
> + pci_io_region_readw,
> + pci_io_region_readl,
> +};
> +
> +static CPUWriteMemoryFunc * const pci_io_region_writefn[3] = {
> + pci_io_region_writeb,
> + pci_io_region_writew,
> + pci_io_region_writel,
> +};
> +
> +void pci_register_io_region(PCIDevice *d, int region_num,
> + pcibus_t size, int type,
> + PCIIOReadFunc *readcb, PCIIOWriteFunc *writecb)
> +{
> + PCIIORegion *r;
> + r = do_pci_register_bar(d, region_num, size, type);
> + if (r) {
> + r->map_func = NULL;
> + r->dev = d;
> + r->read = readcb;
> + r->write = writecb;
> + if (r->type != PCI_BASE_ADDRESS_SPACE_IO && r->read && r->write) {
> + r->io_memory_addr = cpu_register_io_memory(pci_io_region_readfn,
> + pci_io_region_writefn,
> + r);
> + }
> + }
> }
>
> static uint32_t pci_config_get_io_base(PCIDevice *d,
> @@ -897,6 +992,45 @@ static pcibus_t pci_bar_address(PCIDevice *d,
> return new_addr;
> }
>
> +static void pci_io_region_ioport_writeb(void *opaque, uint32_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, (addr - r->addr), 1, value);
> +}
> +
> +static void pci_io_region_ioport_writew(void *opaque, uint32_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, (addr - r->addr), 2, value);
> +}
> +
> +static void pci_io_region_ioport_writel(void *opaque, uint32_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, (addr - r->addr), 4, value);
> +}
> +
> +static uint32_t pci_io_region_ioport_readb(void *opaque, uint32_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, (addr - r->addr), 1);
> +}
> +
> +static uint32_t pci_io_region_ioport_readw(void *opaque, uint32_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, (addr - r->addr), 2);
> +}
> +
> +static uint32_t pci_io_region_ioport_readl(void *opaque, uint32_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, (addr - r->addr), 4);
> +}
> +
addr - (r->addr & ~(r->size - 1))
r->addr isn't always base address because of bridge filtering.
> static void pci_update_mappings(PCIDevice *d)
> {
> PCIIORegion *r;
> @@ -952,10 +1086,32 @@ static void pci_update_mappings(PCIDevice *d)
> * addr & (size - 1) != 0.
> */
> if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
> - r->map_func(d, i, r->addr, r->filtered_size, r->type);
> + if (r->map_func) {
> + r->map_func(d, i, r->addr, r->filtered_size, r->type);
> + } else {
> + register_ioport_write(r->addr, r->filtered_size, 1,
> + pci_io_region_ioport_writeb, r);
> + register_ioport_write(r->addr, r->filtered_size, 2,
> + pci_io_region_ioport_writew, r);
> + register_ioport_write(r->addr, r->filtered_size, 4,
> + pci_io_region_ioport_writel, r);
> + register_ioport_read(r->addr, r->filtered_size, 1,
> + pci_io_region_ioport_readb, r);
> + register_ioport_read(r->addr, r->filtered_size, 2,
> + pci_io_region_ioport_readw, r);
> + register_ioport_read(r->addr, r->filtered_size, 4,
> + pci_io_region_ioport_readl, r);
> + }
> } else {
> - r->map_func(d, i, pci_to_cpu_addr(d->bus, r->addr),
> - r->filtered_size, r->type);
> + if (r->map_func) {
> + r->map_func(d, i, pci_to_cpu_addr(d->bus, r->addr),
> + r->filtered_size, r->type);
> + } else if (r->read && r->write) {
> + cpu_register_physical_memory(pci_to_cpu_addr(d->bus,
> + r->addr),
> + r->filtered_size,
> + r->io_memory_addr);
> + }
> }
> }
> }
> diff --git a/hw/pci.h b/hw/pci.h
> index 8b511d2..3edf28f 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -81,13 +81,21 @@ typedef void PCIMapIORegionFunc(PCIDevice *pci_dev, int region_num,
> pcibus_t addr, pcibus_t size, int type);
> typedef int PCIUnregisterFunc(PCIDevice *pci_dev);
>
> +typedef uint32_t (PCIIOReadFunc)(PCIDevice *pci_dev, pcibus_t addr, int size);
> +typedef void (PCIIOWriteFunc)(PCIDevice *pci_dev, pcibus_t addr, int size,
> + uint32_t value);
> +
> typedef struct PCIIORegion {
> + PCIDevice *dev;
> pcibus_t addr; /* current PCI mapping address. -1 means not mapped */
> #define PCI_BAR_UNMAPPED (~(pcibus_t)0)
> pcibus_t size;
> pcibus_t filtered_size;
> uint8_t type;
> - PCIMapIORegionFunc *map_func;
> + PCIMapIORegionFunc *map_func; /* legacy mapping function */
> + PCIIOReadFunc *read;
> + PCIIOWriteFunc *write;
> + int io_memory_addr;
> } PCIIORegion;
>
> #define PCI_ROM_SLOT 6
> @@ -190,6 +198,14 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> pcibus_t size, int type,
> PCIMapIORegionFunc *map_func);
>
> +void pci_register_io_region(PCIDevice *d, int region_num,
> + pcibus_t size, int type,
> + PCIIOReadFunc *readcb, PCIIOWriteFunc *writecb);
> +
> +void pci_memory_read(PCIDevice *pci_dev, pcibus_t addr, void *buf, int len);
> +void pci_memory_write(PCIDevice *pci_dev, pcibus_t addr,
> + const void *buf, int len);
> +
> int pci_add_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
>
> void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
> --
> 1.6.5.2
>
>
>
--
yamahata
next prev parent reply other threads:[~2010-02-10 8:09 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 01/15] pci: add new bus functions Anthony Liguori
2010-02-10 8:09 ` Isaku Yamahata [this message]
2010-02-10 8:57 ` [Qemu-devel] " Michael S. Tsirkin
2010-02-09 22:01 ` [Qemu-devel] [PATCH 02/15] rtl8139: convert to new PCI interfaces Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 03/15] lsi53c895a: convert to new pci interfaces Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 04/15] e1000: convert to new pci interface Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 05/15] wdt_i6300esb: fix io type leakage Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 06/15] wdt_i6300esb: convert to new pci inteface Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API Anthony Liguori
2010-02-09 22:45 ` malc
2010-02-09 22:52 ` Anthony Liguori
2010-02-09 23:06 ` malc
2010-02-09 23:10 ` Anthony Liguori
2010-02-09 23:36 ` malc
2010-02-09 23:52 ` Anthony Liguori
2010-02-10 0:24 ` malc
2010-02-11 14:20 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 08/15] es1370: convert to new pci interface Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 09/15] eepro100: " Anthony Liguori
2010-02-10 6:32 ` Stefan Weil
2010-02-10 9:49 ` [Qemu-devel] " Michael S. Tsirkin
2010-02-10 10:43 ` Markus Armbruster
2010-02-10 10:48 ` Michael S. Tsirkin
2010-02-10 14:13 ` [Qemu-devel] " Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 10/15] virtio-pci: " Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 11/15] pci: add pci_register_msix_region Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 12/15] ne2000: convert to new pci interface Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 13/15] pcnet: " Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 14/15] usb-uhci: " Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 15/15] pci: byte swap as PCI interface layer Anthony Liguori
2010-02-10 9:31 ` [Qemu-devel] Re: [PATCH 0/15][RFC] New PCI interfaces Michael S. Tsirkin
2010-02-10 18:34 ` [Qemu-devel] " Blue Swirl
2010-02-10 19:29 ` Anthony Liguori
2010-02-10 20:41 ` Richard Henderson
2010-02-10 21:13 ` Anthony Liguori
2010-02-12 17:40 ` Blue Swirl
2010-03-01 2:51 ` Paul Brook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100210080935.GL22624@valinux.co.jp \
--to=yamahata@valinux.co.jp \
--cc=agraf@suse.de \
--cc=aliguori@us.ibm.com \
--cc=mstirkin@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).