On Fri, Jul 23, 2010 at 08:15:44PM +0000, Blue Swirl wrote:
> On Fri, Jul 23, 2010 at 10:40 AM, Isaku Yamahata <yamah...@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.yamah...@valinux.co.jp>
In-Reply-To: <cover.1280284292.git.yamah...@valinux.co.jp>
References: <cover.1280284292.git.yamah...@valinux.co.jp>
From: Isaku Yamahata <yamah...@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 <yamah...@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
