* [Qemu-devel] [PATCH 02/34] pci: handle BAR mapping at PCI level
@ 2010-07-22 21:54 Blue Swirl
2010-07-23 10:40 ` Isaku Yamahata
0 siblings, 1 reply; 4+ messages in thread
From: Blue Swirl @ 2010-07-22 21:54 UTC (permalink / raw)
To: qemu-devel
Move IOIO and MMIO BAR mapping to pci.c.
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
hw/pci.c | 167 ++++++++++++++++++++++++++++++++++++++++---------------------
hw/pci.h | 14 +++++-
2 files changed, 122 insertions(+), 59 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index a98d6f3..49f03fb 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -744,19 +744,28 @@ static target_phys_addr_t pci_to_cpu_addr(PCIBus *bus,
static void pci_unregister_io_regions(PCIDevice *pci_dev)
{
PCIIORegion *r;
- int i;
+ PCIIOSubRegion *s;
+ int i, j;
for(i = 0; i < PCI_NUM_REGIONS; i++) {
r = &pci_dev->io_regions[i];
if (!r->size || r->addr == PCI_BAR_UNMAPPED)
continue;
- if (r->type == PCI_BASE_ADDRESS_SPACE_IO) {
- isa_unassign_ioport(r->addr, r->filtered_size);
- } else {
- cpu_register_physical_memory(pci_to_cpu_addr(pci_dev->bus,
- r->addr),
- r->filtered_size,
- IO_MEM_UNASSIGNED);
+
+ for (j = 0; j < PCI_NUM_SUBREGIONS; j++) {
+ s = &r->subregions[j];
+
+ if (!s->size) {
+ continue;
+ }
+ if (r->type == PCI_BASE_ADDRESS_SPACE_IO) {
+ isa_unassign_ioport(r->addr + s->offset, s->filtered_size);
+ } else {
+ cpu_register_physical_memory(pci_to_cpu_addr(pci_dev->bus,
+ r->addr + s->offset),
+ s->filtered_size,
+ IO_MEM_UNASSIGNED);
+ }
}
}
}
@@ -798,7 +807,6 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
r = &pci_dev->io_regions[region_num];
r->addr = PCI_BAR_UNMAPPED;
r->size = size;
- r->filtered_size = size;
r->type = type;
r->map_func = map_func;
@@ -817,6 +825,25 @@ 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);
}
+ pci_bar_map(pci_dev, region_num, 0, 0, size, -1);
+}
+
+void pci_bar_map(PCIDevice *pci_dev, int region_num, int subregion_num,
+ pcibus_t offset, pcibus_t size, int ix)
+{
+ PCIIOSubRegion *s;
+
+ if ((unsigned int)region_num >= PCI_NUM_REGIONS ||
+ (unsigned int)subregion_num >= PCI_NUM_SUBREGIONS) {
+ return;
+ }
+
+ s = &pci_dev->io_regions[region_num].subregions[subregion_num];
+
+ s->offset = offset;
+ s->size = size;
+ s->filtered_size = size;
+ s->ix = ix;
}
static uint32_t pci_config_get_io_base(PCIDevice *d,
@@ -991,8 +1018,9 @@ static pcibus_t pci_bar_address(PCIDevice *d,
static void pci_update_mappings(PCIDevice *d)
{
PCIIORegion *r;
- int i;
- pcibus_t new_addr, filtered_size;
+ PCIIOSubRegion *s;
+ int i, j;
+ pcibus_t bar_addr, new_addr, filtered_size;
for(i = 0; i < PCI_NUM_REGIONS; i++) {
r = &d->io_regions[i];
@@ -1001,54 +1029,82 @@ static void pci_update_mappings(PCIDevice *d)
if (!r->size)
continue;
- new_addr = pci_bar_address(d, i, r->type, r->size);
+ bar_addr = pci_bar_address(d, i, r->type, r->size);
- /* bridge filtering */
- filtered_size = r->size;
- if (new_addr != PCI_BAR_UNMAPPED) {
- pci_bridge_filter(d, &new_addr, &filtered_size, r->type);
- }
+ for (j = 0; j < PCI_NUM_SUBREGIONS; j++) {
+ s = &r->subregions[j];
- /* This bar isn't changed */
- if (new_addr == r->addr && filtered_size == r->filtered_size)
- continue;
+ /* this subregion isn't registered */
+ if (!s->size) {
+ continue;
+ }
+
+ new_addr = bar_addr + s->offset;
- /* now do the real mapping */
- if (r->addr != PCI_BAR_UNMAPPED) {
- if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
- int class;
- /* NOTE: specific hack for IDE in PC case:
- only one byte must be mapped. */
- class = pci_get_word(d->config + PCI_CLASS_DEVICE);
- if (class == 0x0101 && r->size == 4) {
- isa_unassign_ioport(r->addr + 2, 1);
+ /* bridge filtering */
+ filtered_size = s->size;
+ if (bar_addr != PCI_BAR_UNMAPPED) {
+ pci_bridge_filter(d, &new_addr, &filtered_size, r->type);
+ }
+
+ /* this subregion hasn't changed */
+ if (bar_addr == r->addr && new_addr == bar_addr + s->offset &&
+ filtered_size == s->filtered_size) {
+ continue;
+ }
+ /* now do the real mapping */
+ if (r->addr != PCI_BAR_UNMAPPED) {
+ if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
+ int class;
+ /* NOTE: specific hack for IDE in PC case:
+ only one byte must be mapped. */
+ class = pci_get_word(d->config + PCI_CLASS_DEVICE);
+ if (class == 0x0101 && r->size == 4) {
+ isa_unassign_ioport(r->addr + s->offset + 2, 1);
+ } else {
+ isa_unassign_ioport(r->addr + s->offset,
+ s->filtered_size);
+ }
} else {
- isa_unassign_ioport(r->addr, r->filtered_size);
+ cpu_register_physical_memory(pci_to_cpu_addr(d->bus,
+ r->addr +
+ s->offset),
+ s->filtered_size,
+ IO_MEM_UNASSIGNED);
+ qemu_unregister_coalesced_mmio(r->addr + s->offset,
+ s->filtered_size);
}
- } else {
- cpu_register_physical_memory(pci_to_cpu_addr(d->bus, r->addr),
- r->filtered_size,
- IO_MEM_UNASSIGNED);
- qemu_unregister_coalesced_mmio(r->addr, r->filtered_size);
}
- }
- r->addr = new_addr;
- r->filtered_size = filtered_size;
- if (r->addr != PCI_BAR_UNMAPPED) {
- /*
- * TODO: currently almost all the map funcions assumes
- * filtered_size == size and addr & ~(size - 1) == addr.
- * However with bridge filtering, they aren't always true.
- * Teach them such cases, such that filtered_size < size and
- * addr & (size - 1) != 0.
- */
- if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
- r->map_func(d, i, r->addr, r->filtered_size, r->type);
- } else {
- r->map_func(d, i, pci_to_cpu_addr(d->bus, r->addr),
- r->filtered_size, r->type);
+ s->filtered_size = filtered_size;
+ if (bar_addr != PCI_BAR_UNMAPPED && new_addr != PCI_BAR_UNMAPPED &&
+ s->ix >= 0) {
+ /*
+ * TODO: currently almost all the map funcions assumes
+ * filtered_size == size and addr & ~(size - 1) == addr.
+ * However with bridge filtering, they aren't always true.
+ * Teach them such cases, such that filtered_size < size and
+ * addr & (size - 1) != 0.
+ */
+ if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
+ if (r->map_func) {
+ r->map_func(d, i, new_addr, s->filtered_size, r->type);
+ } else {
+ cpu_map_io(new_addr, s->ix);
+ }
+ } else {
+ if (r->map_func) {
+ r->map_func(d, i, pci_to_cpu_addr(d->bus, new_addr),
+ s->filtered_size, r->type);
+ } else {
+ cpu_register_physical_memory(pci_to_cpu_addr(d->bus,
+ new_addr),
+ s->filtered_size,
+ s->ix);
+ }
+ }
}
}
+ r->addr = bar_addr;
}
}
@@ -1791,11 +1847,6 @@ static uint8_t
pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,
return next;
}
-static void pci_map_option_rom(PCIDevice *pdev, int region_num,
pcibus_t addr, pcibus_t size, int type)
-{
- cpu_register_physical_memory(addr, size, pdev->rom_offset);
-}
-
/* Add an option rom for the device */
static int pci_add_option_rom(PCIDevice *pdev)
{
@@ -1848,8 +1899,8 @@ static int pci_add_option_rom(PCIDevice *pdev)
load_image(path, ptr);
qemu_free(path);
- pci_register_bar(pdev, PCI_ROM_SLOT, size,
- 0, pci_map_option_rom);
+ pci_register_bar(pdev, PCI_ROM_SLOT, size, 0, NULL);
+ pci_bar_map(pdev, PCI_ROM_SLOT, 0, 0, size, pdev->rom_offset);
return 0;
}
diff --git a/hw/pci.h b/hw/pci.h
index 1eab7e7..b518b3f 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -81,13 +81,22 @@ typedef void PCIMapIORegionFunc(PCIDevice
*pci_dev, int region_num,
pcibus_t addr, pcibus_t size, int type);
typedef int PCIUnregisterFunc(PCIDevice *pci_dev);
+typedef struct PCIIOSubRegion {
+ pcibus_t offset; /* offset to BAR start. -1 means not mapped */
+ pcibus_t size;
+ pcibus_t filtered_size;
+ int ix;
+} PCIIOSubRegion;
+
+#define PCI_NUM_SUBREGIONS 16
+
typedef struct PCIIORegion {
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;
+ PCIIOSubRegion subregions[PCI_NUM_SUBREGIONS];
} PCIIORegion;
#define PCI_ROM_SLOT 6
@@ -183,6 +192,9 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
pcibus_t size, int type,
PCIMapIORegionFunc *map_func);
+void pci_bar_map(PCIDevice *pci_dev, int region_num, int subregion_num,
+ pcibus_t offset, pcibus_t size, int ix);
+
int pci_add_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
int pci_add_capability_at_offset(PCIDevice *pci_dev, uint8_t cap_id,
uint8_t cap_offset, uint8_t cap_size);
--
1.6.2.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 02/34] pci: handle BAR mapping at PCI level
2010-07-22 21:54 [Qemu-devel] [PATCH 02/34] pci: handle BAR mapping at PCI level Blue Swirl
@ 2010-07-23 10:40 ` Isaku Yamahata
2010-07-23 20:15 ` Blue Swirl
0 siblings, 1 reply; 4+ messages in thread
From: Isaku Yamahata @ 2010-07-23 10:40 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel
On Thu, Jul 22, 2010 at 09:54:46PM +0000, Blue Swirl wrote:
> diff --git a/hw/pci.c b/hw/pci.c
> index a98d6f3..49f03fb 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
...
> @@ -817,6 +825,25 @@ 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);
> }
> + pci_bar_map(pci_dev, region_num, 0, 0, size, -1);
> +}
> +
> +void pci_bar_map(PCIDevice *pci_dev, int region_num, int subregion_num,
> + pcibus_t offset, pcibus_t size, int ix)
> +{
> + PCIIOSubRegion *s;
> +
> + if ((unsigned int)region_num >= PCI_NUM_REGIONS ||
> + (unsigned int)subregion_num >= PCI_NUM_SUBREGIONS) {
> + return;
> + }
abort() or assert()? It's caller's bug.
--
yamahata
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 02/34] pci: handle BAR mapping at PCI level
2010-07-23 10:40 ` Isaku Yamahata
@ 2010-07-23 20:15 ` Blue Swirl
2010-07-28 2:35 ` Isaku Yamahata
0 siblings, 1 reply; 4+ messages in thread
From: Blue Swirl @ 2010-07-23 20:15 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: qemu-devel
On Fri, Jul 23, 2010 at 10:40 AM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> On Thu, Jul 22, 2010 at 09:54:46PM +0000, Blue Swirl wrote:
>> diff --git a/hw/pci.c b/hw/pci.c
>> index a98d6f3..49f03fb 100644
>> --- a/hw/pci.c
>> +++ b/hw/pci.c
> ...
>> @@ -817,6 +825,25 @@ 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);
>> }
>> + pci_bar_map(pci_dev, region_num, 0, 0, size, -1);
>> +}
>> +
>> +void pci_bar_map(PCIDevice *pci_dev, int region_num, int subregion_num,
>> + pcibus_t offset, pcibus_t size, int ix)
>> +{
>> + PCIIOSubRegion *s;
>> +
>> + if ((unsigned int)region_num >= PCI_NUM_REGIONS ||
>> + (unsigned int)subregion_num >= PCI_NUM_SUBREGIONS) {
>> + return;
>> + }
>
> abort() or assert()? It's caller's bug.
Yes. I copied this from pci_register_bar(), which should also have
assert() or abort().
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 02/34] pci: handle BAR mapping at PCI level
2010-07-23 20:15 ` Blue Swirl
@ 2010-07-28 2:35 ` Isaku Yamahata
0 siblings, 0 replies; 4+ messages in thread
From: Isaku Yamahata @ 2010-07-28 2:35 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel, mst
On Fri, Jul 23, 2010 at 08:15:44PM +0000, Blue Swirl wrote:
> On Fri, Jul 23, 2010 at 10:40 AM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> > On Thu, Jul 22, 2010 at 09:54:46PM +0000, Blue Swirl wrote:
> >> diff --git a/hw/pci.c b/hw/pci.c
> >> index a98d6f3..49f03fb 100644
> >> --- a/hw/pci.c
> >> +++ b/hw/pci.c
> > ...
> >> @@ -817,6 +825,25 @@ 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);
> >> }
> >> + pci_bar_map(pci_dev, region_num, 0, 0, size, -1);
> >> +}
> >> +
> >> +void pci_bar_map(PCIDevice *pci_dev, int region_num, int subregion_num,
> >> + pcibus_t offset, pcibus_t size, int ix)
> >> +{
> >> + PCIIOSubRegion *s;
> >> +
> >> + if ((unsigned int)region_num >= PCI_NUM_REGIONS ||
> >> + (unsigned int)subregion_num >= PCI_NUM_SUBREGIONS) {
> >> + return;
> >> + }
> >
> > abort() or assert()? It's caller's bug.
>
> Yes. I copied this from pci_register_bar(), which should also have
> assert() or abort().
Here is the patch.
It would make sense to include this patch into the patch series.
>From 24ffedc781227e5f05ceb94f2690b207591b8929 Mon Sep 17 00:00:00 2001
Message-Id: <24ffedc781227e5f05ceb94f2690b207591b8929.1280284292.git.yamahata@valinux.co.jp>
In-Reply-To: <cover.1280284292.git.yamahata@valinux.co.jp>
References: <cover.1280284292.git.yamahata@valinux.co.jp>
From: Isaku Yamahata <yamahata@valinux.co.jp>
Date: Wed, 28 Jul 2010 11:15:45 +0900
Subject: [PATCH] pci: Improve pci_register_bar().
Improve pci_register_bar().
- Don't return silently when invalid region_num
is passed to pci_register_bar().
It's caller's bug, so use assert().
- make region_num argument unsigned int.
There is no point that it's signed int.
- make type argument uint8_t to match PCIIORegion::type
PCIMapIORegionFunc signature should be changed, but didn't change it
because it will be eliminated later.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pci.c | 9 ++++-----
hw/pci.h | 6 +++---
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index a98d6f3..8d581c4 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -778,16 +778,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)
+void pci_register_bar(PCIDevice *pci_dev, unsigned int region_num,
+ pcibus_t size, uint8_t type,
+ PCIMapIORegionFunc *map_func)
{
PCIIORegion *r;
uint32_t addr;
pcibus_t wmask;
- if ((unsigned int)region_num >= PCI_NUM_REGIONS)
- return;
+ assert(region_num < PCI_NUM_REGIONS);
if (size & (size-1)) {
fprintf(stderr, "ERROR: PCI region size must be pow2 "
diff --git a/hw/pci.h b/hw/pci.h
index 1eab7e7..df8fedd 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -179,9 +179,9 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
PCIConfigReadFunc *config_read,
PCIConfigWriteFunc *config_write);
-void pci_register_bar(PCIDevice *pci_dev, int region_num,
- pcibus_t size, int type,
- PCIMapIORegionFunc *map_func);
+void pci_register_bar(PCIDevice *pci_dev, unsigned int region_num,
+ pcibus_t size, uint8_t type,
+ PCIMapIORegionFunc *map_func);
int pci_add_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
int pci_add_capability_at_offset(PCIDevice *pci_dev, uint8_t cap_id,
--
1.7.1.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-07-28 2:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-22 21:54 [Qemu-devel] [PATCH 02/34] pci: handle BAR mapping at PCI level Blue Swirl
2010-07-23 10:40 ` Isaku Yamahata
2010-07-23 20:15 ` Blue Swirl
2010-07-28 2:35 ` Isaku Yamahata
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).