* [Qemu-devel] [PATCH 0/3] pci: pci_register_bar() clean up
@ 2010-09-09 2:48 Isaku Yamahata
2010-09-09 2:48 ` [Qemu-devel] [PATCH 1/3] pci: sorting out type confusion in pci_register_bar() Isaku Yamahata
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Isaku Yamahata @ 2010-09-09 2:48 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata, mst
This patch set cleans up pci_register_bar() a bit.
Isaku Yamahata (3):
pci: sorting out type confusion in pci_register_bar().
pci: don't ignore invalid parameter for pci_register_bar().
pci: improve signature of pci_register_bar().
hw/pci.c | 11 +++++------
hw/pci.h | 2 +-
2 files changed, 6 insertions(+), 7 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 1/3] pci: sorting out type confusion in pci_register_bar().
2010-09-09 2:48 [Qemu-devel] [PATCH 0/3] pci: pci_register_bar() clean up Isaku Yamahata
@ 2010-09-09 2:48 ` Isaku Yamahata
2010-09-12 15:18 ` [Qemu-devel] " Michael S. Tsirkin
2010-09-09 2:48 ` [Qemu-devel] [PATCH 2/3] pci: don't ignore invalid parameter for pci_register_bar() Isaku Yamahata
2010-09-09 2:48 ` [Qemu-devel] [PATCH 3/3] pci: improve signature of pci_register_bar() Isaku Yamahata
2 siblings, 1 reply; 7+ messages in thread
From: Isaku Yamahata @ 2010-09-09 2:48 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata, mst
This patch sorts out invalid use of pcibus_t.
In pci_register_bar(), pcibus_t wmask is used.
It should, However, be uint64_t because it is used to set
pci configuration space value(PCIDevice::wmask) by pci_set_quad()
or pci_set_long().
So make its type uint64_t and cast from pcibus_t to int64_t where
necessary.
Maybe for strict type safety conversion functions would
be desirable. Something like pcibus_to_uintN(), uintN_to_pcibus().
But conversions are done only a few place, so it wouldn't be
worthwhile.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pci.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index f03b83e..8d6b299 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -763,7 +763,7 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
{
PCIIORegion *r;
uint32_t addr;
- pcibus_t wmask;
+ uint64_t wmask;
if ((unsigned int)region_num >= PCI_NUM_REGIONS)
return;
@@ -781,7 +781,7 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
r->type = type;
r->map_func = map_func;
- wmask = ~(size - 1);
+ wmask = (uint64_t)(~(size - 1));
addr = pci_bar(pci_dev, region_num);
if (region_num == PCI_ROM_SLOT) {
/* ROM enable bit is writeable */
--
1.7.1.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 2/3] pci: don't ignore invalid parameter for pci_register_bar().
2010-09-09 2:48 [Qemu-devel] [PATCH 0/3] pci: pci_register_bar() clean up Isaku Yamahata
2010-09-09 2:48 ` [Qemu-devel] [PATCH 1/3] pci: sorting out type confusion in pci_register_bar() Isaku Yamahata
@ 2010-09-09 2:48 ` Isaku Yamahata
2010-09-12 15:21 ` [Qemu-devel] " Michael S. Tsirkin
2010-09-09 2:48 ` [Qemu-devel] [PATCH 3/3] pci: improve signature of pci_register_bar() Isaku Yamahata
2 siblings, 1 reply; 7+ messages in thread
From: Isaku Yamahata @ 2010-09-09 2:48 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata, mst
Abort when invalid value for region_num is passed to pci_register_bar.
That is caller's bug. Abort instead of silently ignoring invalid value.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pci.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 8d6b299..31eba9a 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -765,9 +765,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
uint32_t addr;
uint64_t wmask;
- if ((unsigned int)region_num >= PCI_NUM_REGIONS)
- return;
-
+ assert(region_num >= 0);
+ assert(region_num < PCI_NUM_REGIONS);
if (size & (size-1)) {
fprintf(stderr, "ERROR: PCI region size must be pow2 "
"type=0x%x, size=0x%"FMT_PCIBUS"\n", type, size);
--
1.7.1.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 3/3] pci: improve signature of pci_register_bar().
2010-09-09 2:48 [Qemu-devel] [PATCH 0/3] pci: pci_register_bar() clean up Isaku Yamahata
2010-09-09 2:48 ` [Qemu-devel] [PATCH 1/3] pci: sorting out type confusion in pci_register_bar() Isaku Yamahata
2010-09-09 2:48 ` [Qemu-devel] [PATCH 2/3] pci: don't ignore invalid parameter for pci_register_bar() Isaku Yamahata
@ 2010-09-09 2:48 ` Isaku Yamahata
2010-09-12 15:21 ` [Qemu-devel] " Michael S. Tsirkin
2 siblings, 1 reply; 7+ messages in thread
From: Isaku Yamahata @ 2010-09-09 2:48 UTC (permalink / raw)
To: qemu-devel; +Cc: yamahata, mst
Make type uint8_t from int because PCIIORegion::type is uint8_t.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/pci.c | 2 +-
hw/pci.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 31eba9a..ceee291 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -758,7 +758,7 @@ static int pci_unregister_device(DeviceState *dev)
}
void pci_register_bar(PCIDevice *pci_dev, int region_num,
- pcibus_t size, int type,
+ pcibus_t size, uint8_t type,
PCIMapIORegionFunc *map_func)
{
PCIIORegion *r;
diff --git a/hw/pci.h b/hw/pci.h
index bccab3a..25179eb 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -192,7 +192,7 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
PCIConfigWriteFunc *config_write);
void pci_register_bar(PCIDevice *pci_dev, int region_num,
- pcibus_t size, int type,
+ pcibus_t size, uint8_t type,
PCIMapIORegionFunc *map_func);
int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
--
1.7.1.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH 1/3] pci: sorting out type confusion in pci_register_bar().
2010-09-09 2:48 ` [Qemu-devel] [PATCH 1/3] pci: sorting out type confusion in pci_register_bar() Isaku Yamahata
@ 2010-09-12 15:18 ` Michael S. Tsirkin
0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2010-09-12 15:18 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: qemu-devel
On Thu, Sep 09, 2010 at 11:48:55AM +0900, Isaku Yamahata wrote:
> This patch sorts out invalid use of pcibus_t.
>
> In pci_register_bar(), pcibus_t wmask is used.
> It should, However, be uint64_t because it is used to set
> pci configuration space value(PCIDevice::wmask) by pci_set_quad()
> or pci_set_long().
> So make its type uint64_t and cast from pcibus_t to int64_t where
> necessary.
I don't think we need the cast. Applied without the cast.
> Maybe for strict type safety conversion functions would
> be desirable. Something like pcibus_to_uintN(), uintN_to_pcibus().
> But conversions are done only a few place, so it wouldn't be
> worthwhile.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
> hw/pci.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index f03b83e..8d6b299 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -763,7 +763,7 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> {
> PCIIORegion *r;
> uint32_t addr;
> - pcibus_t wmask;
> + uint64_t wmask;
>
> if ((unsigned int)region_num >= PCI_NUM_REGIONS)
> return;
> @@ -781,7 +781,7 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> r->type = type;
> r->map_func = map_func;
>
> - wmask = ~(size - 1);
> + wmask = (uint64_t)(~(size - 1));
> addr = pci_bar(pci_dev, region_num);
> if (region_num == PCI_ROM_SLOT) {
> /* ROM enable bit is writeable */
> --
> 1.7.1.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH 2/3] pci: don't ignore invalid parameter for pci_register_bar().
2010-09-09 2:48 ` [Qemu-devel] [PATCH 2/3] pci: don't ignore invalid parameter for pci_register_bar() Isaku Yamahata
@ 2010-09-12 15:21 ` Michael S. Tsirkin
0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2010-09-12 15:21 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: qemu-devel
On Thu, Sep 09, 2010 at 11:48:56AM +0900, Isaku Yamahata wrote:
> Abort when invalid value for region_num is passed to pci_register_bar.
> That is caller's bug. Abort instead of silently ignoring invalid value.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
> hw/pci.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
Applied.
> diff --git a/hw/pci.c b/hw/pci.c
> index 8d6b299..31eba9a 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -765,9 +765,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> uint32_t addr;
> uint64_t wmask;
>
> - if ((unsigned int)region_num >= PCI_NUM_REGIONS)
> - return;
> -
> + assert(region_num >= 0);
> + assert(region_num < PCI_NUM_REGIONS);
> if (size & (size-1)) {
> fprintf(stderr, "ERROR: PCI region size must be pow2 "
> "type=0x%x, size=0x%"FMT_PCIBUS"\n", type, size);
> --
> 1.7.1.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] Re: [PATCH 3/3] pci: improve signature of pci_register_bar().
2010-09-09 2:48 ` [Qemu-devel] [PATCH 3/3] pci: improve signature of pci_register_bar() Isaku Yamahata
@ 2010-09-12 15:21 ` Michael S. Tsirkin
0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2010-09-12 15:21 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: qemu-devel
On Thu, Sep 09, 2010 at 11:48:57AM +0900, Isaku Yamahata wrote:
> Make type uint8_t from int because PCIIORegion::type is uint8_t.
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
> hw/pci.c | 2 +-
> hw/pci.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Applied, thanks.
> diff --git a/hw/pci.c b/hw/pci.c
> index 31eba9a..ceee291 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -758,7 +758,7 @@ static int pci_unregister_device(DeviceState *dev)
> }
>
> void pci_register_bar(PCIDevice *pci_dev, int region_num,
> - pcibus_t size, int type,
> + pcibus_t size, uint8_t type,
> PCIMapIORegionFunc *map_func)
> {
> PCIIORegion *r;
> diff --git a/hw/pci.h b/hw/pci.h
> index bccab3a..25179eb 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -192,7 +192,7 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
> PCIConfigWriteFunc *config_write);
>
> void pci_register_bar(PCIDevice *pci_dev, int region_num,
> - pcibus_t size, int type,
> + pcibus_t size, uint8_t type,
> PCIMapIORegionFunc *map_func);
>
> int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
> --
> 1.7.1.1
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-09-12 15:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-09 2:48 [Qemu-devel] [PATCH 0/3] pci: pci_register_bar() clean up Isaku Yamahata
2010-09-09 2:48 ` [Qemu-devel] [PATCH 1/3] pci: sorting out type confusion in pci_register_bar() Isaku Yamahata
2010-09-12 15:18 ` [Qemu-devel] " Michael S. Tsirkin
2010-09-09 2:48 ` [Qemu-devel] [PATCH 2/3] pci: don't ignore invalid parameter for pci_register_bar() Isaku Yamahata
2010-09-12 15:21 ` [Qemu-devel] " Michael S. Tsirkin
2010-09-09 2:48 ` [Qemu-devel] [PATCH 3/3] pci: improve signature of pci_register_bar() Isaku Yamahata
2010-09-12 15:21 ` [Qemu-devel] " Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.