* [Qemu-devel] [PATCH V6 0/2] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
@ 2014-04-10 18:55 Marcel Apfelbaum
2014-04-10 18:55 ` [Qemu-devel] [PATCH V6 1/2] " Marcel Apfelbaum
2014-04-10 18:55 ` [Qemu-devel] [PATCH V6 2/2] hw/pci: check if pci2pci bridges implement optional limit registers Marcel Apfelbaum
0 siblings, 2 replies; 5+ messages in thread
From: Marcel Apfelbaum @ 2014-04-10 18:55 UTC (permalink / raw)
To: seabios; +Cc: kevin, kraxel, qemu-devel, mst
v5 -> v6:
- Minor, fixed a typo in the subject
v4 -> v5
- Addressed Michael S. Tsirkin's comments (patch 2/2):
- Open-coded pci_config_is_reserved() method.
v3 -> v4:
- Addressed Kevin O'Connor's comments:
- Refactored a for loop in patch 1/2.
- Addressed Michael S. Tsirkin's comments (patch 2/2):
- Removed not needed method
- Test only base registers (dropped limits tests)
- Renamed a helper method
- Used 0xFF to test if the memory is reserved
- Simplified code in pci_bridge_has_region
- I did keep the code that restores base's address as I don't want
to modify the registers in a 'query' method. (as replied on the mail thread)
v2 -> v3:
- Addressed Michael S. Tsirkin's comments:
- I/O and Prefetchable Memory are optional. Do not allocate ranges
if they are not implemented (2/2).
- Note that 2/2 patch can be seen as a separate fix. However, it
is related to ranges reservation.
v1 -> v2:
- Thanks Gerd Hoffmann for the review.
- Addressed Michael S. Tsirkin's comments:
- Limit capabilities query to 256 iterations, to make sure we
don't get into an infinite loop with a broken device.
If a pci-2-pci bridge supports hot-plug functionality but there are no devices
connected to it, reserve IO/mem in order to be able to attach devices
later. Do not waste space, use minimum allowed.
Marcel Apfelbaum (2):
hw/pci: reserve IO and mem for pci-2-pci bridges with no devices
attached
hw/pci: check if pci2pci bridges implement optional limit registers
src/fw/pciinit.c | 12 +++++-------
src/hw/pci.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
src/hw/pci.h | 10 ++++++++++
3 files changed, 60 insertions(+), 7 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH V6 1/2] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
2014-04-10 18:55 [Qemu-devel] [PATCH V6 0/2] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached Marcel Apfelbaum
@ 2014-04-10 18:55 ` Marcel Apfelbaum
2014-04-10 19:05 ` Michael S. Tsirkin
2014-04-10 18:55 ` [Qemu-devel] [PATCH V6 2/2] hw/pci: check if pci2pci bridges implement optional limit registers Marcel Apfelbaum
1 sibling, 1 reply; 5+ messages in thread
From: Marcel Apfelbaum @ 2014-04-10 18:55 UTC (permalink / raw)
To: seabios; +Cc: kevin, kraxel, qemu-devel, mst
If a pci-2-pci bridge supports hot-plug functionality but there are no devices
connected to it, reserve IO/mem in order to be able to attach devices
later. Do not waste space, use minimum allowed.
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
src/fw/pciinit.c | 3 +++
src/hw/pci.c | 19 +++++++++++++++++++
src/hw/pci.h | 1 +
3 files changed, 23 insertions(+)
diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index 64f1d41..9b5d7ad 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -677,12 +677,15 @@ static int pci_bios_check_devices(struct pci_bus *busses)
continue;
struct pci_bus *parent = &busses[pci_bdf_to_bus(s->bus_dev->bdf)];
int type;
+ u8 shpc_cap = pci_find_capability(s->bus_dev, PCI_CAP_ID_SHPC);
for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
u64 align = (type == PCI_REGION_TYPE_IO) ?
PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
if (pci_region_align(&s->r[type]) > align)
align = pci_region_align(&s->r[type]);
u64 sum = pci_region_sum(&s->r[type]);
+ if (!sum && shpc_cap)
+ sum = align; /* reserve min size for hot-plug */
u64 size = ALIGN(sum, align);
int is64 = pci_bios_bridge_region_is64(&s->r[type],
s->bus_dev, type);
diff --git a/src/hw/pci.c b/src/hw/pci.c
index caf9265..77cdba2 100644
--- a/src/hw/pci.c
+++ b/src/hw/pci.c
@@ -225,6 +225,25 @@ pci_find_init_device(const struct pci_device_id *ids, void *arg)
return NULL;
}
+u8 pci_find_capability(struct pci_device *pci, u8 cap_id)
+{
+ int i;
+ u8 cap;
+ u16 status = pci_config_readw(pci->bdf, PCI_STATUS);
+
+ if (!(status & PCI_STATUS_CAP_LIST))
+ return 0;
+
+ cap = pci_config_readb(pci->bdf, PCI_CAPABILITY_LIST);
+ for (i = 0; cap && i <= 0xff; i++) {
+ if (pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_ID) == cap_id)
+ return cap;
+ cap = pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_NEXT);
+ }
+
+ return 0;
+}
+
void
pci_reboot(void)
{
diff --git a/src/hw/pci.h b/src/hw/pci.h
index 167a027..e828225 100644
--- a/src/hw/pci.h
+++ b/src/hw/pci.h
@@ -116,6 +116,7 @@ int pci_init_device(const struct pci_device_id *ids
, struct pci_device *pci, void *arg);
struct pci_device *pci_find_init_device(const struct pci_device_id *ids
, void *arg);
+u8 pci_find_capability(struct pci_device *pci, u8 cap_id);
void pci_reboot(void);
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH V6 2/2] hw/pci: check if pci2pci bridges implement optional limit registers
2014-04-10 18:55 [Qemu-devel] [PATCH V6 0/2] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached Marcel Apfelbaum
2014-04-10 18:55 ` [Qemu-devel] [PATCH V6 1/2] " Marcel Apfelbaum
@ 2014-04-10 18:55 ` Marcel Apfelbaum
2014-04-10 19:05 ` Michael S. Tsirkin
1 sibling, 1 reply; 5+ messages in thread
From: Marcel Apfelbaum @ 2014-04-10 18:55 UTC (permalink / raw)
To: seabios; +Cc: kevin, kraxel, qemu-devel, mst
<I/O Base Register, I/O Limit Register> pair and
<Prefetchable Memory Base Register, Prefetchable Memory Limit Register> pair
are both optional.
Do not reserve ranges if the above registers are not implemented.
Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
src/fw/pciinit.c | 9 ++-------
src/hw/pci.c | 26 ++++++++++++++++++++++++++
src/hw/pci.h | 9 +++++++++
3 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index 9b5d7ad..bbaecd6 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -26,13 +26,6 @@
#define PCI_BRIDGE_MEM_MIN (1<<21) // 2M == hugepage size
#define PCI_BRIDGE_IO_MIN 0x1000 // mandated by pci bridge spec
-enum pci_region_type {
- PCI_REGION_TYPE_IO,
- PCI_REGION_TYPE_MEM,
- PCI_REGION_TYPE_PREFMEM,
- PCI_REGION_TYPE_COUNT,
-};
-
static const char *region_type_name[] = {
[ PCI_REGION_TYPE_IO ] = "io",
[ PCI_REGION_TYPE_MEM ] = "mem",
@@ -681,6 +674,8 @@ static int pci_bios_check_devices(struct pci_bus *busses)
for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
u64 align = (type == PCI_REGION_TYPE_IO) ?
PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
+ if (!pci_bridge_has_region(s->bus_dev, type))
+ continue;
if (pci_region_align(&s->r[type]) > align)
align = pci_region_align(&s->r[type]);
u64 sum = pci_region_sum(&s->r[type]);
diff --git a/src/hw/pci.c b/src/hw/pci.c
index 77cdba2..390b4dd 100644
--- a/src/hw/pci.c
+++ b/src/hw/pci.c
@@ -244,6 +244,32 @@ u8 pci_find_capability(struct pci_device *pci, u8 cap_id)
return 0;
}
+/* Test whether bridge support forwarding of transactions
+ * of a specific type.
+ * Note: disables bridge's window registers as a side effect.
+ */
+int pci_bridge_has_region(struct pci_device *pci,
+ enum pci_region_type region_type)
+{
+ u8 base;
+
+ switch (region_type) {
+ case PCI_REGION_TYPE_IO:
+ base = PCI_IO_BASE;
+ break;
+ case PCI_REGION_TYPE_PREFMEM:
+ base = PCI_PREF_MEMORY_BASE;
+ break;
+ default:
+ /* Regular memory support is mandatory */
+ return 1;
+ }
+
+ pci_config_writeb(pci->bdf, base, 0xFF);
+
+ return pci_config_readb(pci->bdf, base) != 0;
+}
+
void
pci_reboot(void)
{
diff --git a/src/hw/pci.h b/src/hw/pci.h
index e828225..0aaa84c 100644
--- a/src/hw/pci.h
+++ b/src/hw/pci.h
@@ -12,6 +12,13 @@
#define PCI_NUM_REGIONS 7
#define PCI_BRIDGE_NUM_REGIONS 2
+enum pci_region_type {
+ PCI_REGION_TYPE_IO,
+ PCI_REGION_TYPE_MEM,
+ PCI_REGION_TYPE_PREFMEM,
+ PCI_REGION_TYPE_COUNT,
+};
+
static inline u8 pci_bdf_to_bus(u16 bdf) {
return bdf >> 8;
}
@@ -117,6 +124,8 @@ int pci_init_device(const struct pci_device_id *ids
struct pci_device *pci_find_init_device(const struct pci_device_id *ids
, void *arg);
u8 pci_find_capability(struct pci_device *pci, u8 cap_id);
+int pci_bridge_has_region(struct pci_device *pci,
+ enum pci_region_type region_type);
void pci_reboot(void);
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH V6 2/2] hw/pci: check if pci2pci bridges implement optional limit registers
2014-04-10 18:55 ` [Qemu-devel] [PATCH V6 2/2] hw/pci: check if pci2pci bridges implement optional limit registers Marcel Apfelbaum
@ 2014-04-10 19:05 ` Michael S. Tsirkin
0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2014-04-10 19:05 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: kevin, seabios, qemu-devel, kraxel
On Thu, Apr 10, 2014 at 09:55:22PM +0300, Marcel Apfelbaum wrote:
> <I/O Base Register, I/O Limit Register> pair and
> <Prefetchable Memory Base Register, Prefetchable Memory Limit Register> pair
> are both optional.
> Do not reserve ranges if the above registers are not implemented.
>
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> src/fw/pciinit.c | 9 ++-------
> src/hw/pci.c | 26 ++++++++++++++++++++++++++
> src/hw/pci.h | 9 +++++++++
> 3 files changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index 9b5d7ad..bbaecd6 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -26,13 +26,6 @@
> #define PCI_BRIDGE_MEM_MIN (1<<21) // 2M == hugepage size
> #define PCI_BRIDGE_IO_MIN 0x1000 // mandated by pci bridge spec
>
> -enum pci_region_type {
> - PCI_REGION_TYPE_IO,
> - PCI_REGION_TYPE_MEM,
> - PCI_REGION_TYPE_PREFMEM,
> - PCI_REGION_TYPE_COUNT,
> -};
> -
> static const char *region_type_name[] = {
> [ PCI_REGION_TYPE_IO ] = "io",
> [ PCI_REGION_TYPE_MEM ] = "mem",
> @@ -681,6 +674,8 @@ static int pci_bios_check_devices(struct pci_bus *busses)
> for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
> u64 align = (type == PCI_REGION_TYPE_IO) ?
> PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
> + if (!pci_bridge_has_region(s->bus_dev, type))
> + continue;
> if (pci_region_align(&s->r[type]) > align)
> align = pci_region_align(&s->r[type]);
> u64 sum = pci_region_sum(&s->r[type]);
> diff --git a/src/hw/pci.c b/src/hw/pci.c
> index 77cdba2..390b4dd 100644
> --- a/src/hw/pci.c
> +++ b/src/hw/pci.c
> @@ -244,6 +244,32 @@ u8 pci_find_capability(struct pci_device *pci, u8 cap_id)
> return 0;
> }
>
> +/* Test whether bridge support forwarding of transactions
> + * of a specific type.
> + * Note: disables bridge's window registers as a side effect.
> + */
> +int pci_bridge_has_region(struct pci_device *pci,
> + enum pci_region_type region_type)
> +{
> + u8 base;
> +
> + switch (region_type) {
> + case PCI_REGION_TYPE_IO:
> + base = PCI_IO_BASE;
> + break;
> + case PCI_REGION_TYPE_PREFMEM:
> + base = PCI_PREF_MEMORY_BASE;
> + break;
> + default:
> + /* Regular memory support is mandatory */
> + return 1;
> + }
> +
> + pci_config_writeb(pci->bdf, base, 0xFF);
> +
> + return pci_config_readb(pci->bdf, base) != 0;
> +}
> +
> void
> pci_reboot(void)
> {
> diff --git a/src/hw/pci.h b/src/hw/pci.h
> index e828225..0aaa84c 100644
> --- a/src/hw/pci.h
> +++ b/src/hw/pci.h
> @@ -12,6 +12,13 @@
> #define PCI_NUM_REGIONS 7
> #define PCI_BRIDGE_NUM_REGIONS 2
>
> +enum pci_region_type {
> + PCI_REGION_TYPE_IO,
> + PCI_REGION_TYPE_MEM,
> + PCI_REGION_TYPE_PREFMEM,
> + PCI_REGION_TYPE_COUNT,
> +};
> +
> static inline u8 pci_bdf_to_bus(u16 bdf) {
> return bdf >> 8;
> }
> @@ -117,6 +124,8 @@ int pci_init_device(const struct pci_device_id *ids
> struct pci_device *pci_find_init_device(const struct pci_device_id *ids
> , void *arg);
> u8 pci_find_capability(struct pci_device *pci, u8 cap_id);
> +int pci_bridge_has_region(struct pci_device *pci,
> + enum pci_region_type region_type);
> void pci_reboot(void);
>
> #endif
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH V6 1/2] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
2014-04-10 18:55 ` [Qemu-devel] [PATCH V6 1/2] " Marcel Apfelbaum
@ 2014-04-10 19:05 ` Michael S. Tsirkin
0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2014-04-10 19:05 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: kevin, seabios, qemu-devel, kraxel
On Thu, Apr 10, 2014 at 09:55:21PM +0300, Marcel Apfelbaum wrote:
> If a pci-2-pci bridge supports hot-plug functionality but there are no devices
> connected to it, reserve IO/mem in order to be able to attach devices
> later. Do not waste space, use minimum allowed.
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> src/fw/pciinit.c | 3 +++
> src/hw/pci.c | 19 +++++++++++++++++++
> src/hw/pci.h | 1 +
> 3 files changed, 23 insertions(+)
>
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index 64f1d41..9b5d7ad 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -677,12 +677,15 @@ static int pci_bios_check_devices(struct pci_bus *busses)
> continue;
> struct pci_bus *parent = &busses[pci_bdf_to_bus(s->bus_dev->bdf)];
> int type;
> + u8 shpc_cap = pci_find_capability(s->bus_dev, PCI_CAP_ID_SHPC);
> for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
> u64 align = (type == PCI_REGION_TYPE_IO) ?
> PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
> if (pci_region_align(&s->r[type]) > align)
> align = pci_region_align(&s->r[type]);
> u64 sum = pci_region_sum(&s->r[type]);
> + if (!sum && shpc_cap)
> + sum = align; /* reserve min size for hot-plug */
> u64 size = ALIGN(sum, align);
> int is64 = pci_bios_bridge_region_is64(&s->r[type],
> s->bus_dev, type);
> diff --git a/src/hw/pci.c b/src/hw/pci.c
> index caf9265..77cdba2 100644
> --- a/src/hw/pci.c
> +++ b/src/hw/pci.c
> @@ -225,6 +225,25 @@ pci_find_init_device(const struct pci_device_id *ids, void *arg)
> return NULL;
> }
>
> +u8 pci_find_capability(struct pci_device *pci, u8 cap_id)
> +{
> + int i;
> + u8 cap;
> + u16 status = pci_config_readw(pci->bdf, PCI_STATUS);
> +
> + if (!(status & PCI_STATUS_CAP_LIST))
> + return 0;
> +
> + cap = pci_config_readb(pci->bdf, PCI_CAPABILITY_LIST);
> + for (i = 0; cap && i <= 0xff; i++) {
> + if (pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_ID) == cap_id)
> + return cap;
> + cap = pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_NEXT);
> + }
> +
> + return 0;
> +}
> +
> void
> pci_reboot(void)
> {
> diff --git a/src/hw/pci.h b/src/hw/pci.h
> index 167a027..e828225 100644
> --- a/src/hw/pci.h
> +++ b/src/hw/pci.h
> @@ -116,6 +116,7 @@ int pci_init_device(const struct pci_device_id *ids
> , struct pci_device *pci, void *arg);
> struct pci_device *pci_find_init_device(const struct pci_device_id *ids
> , void *arg);
> +u8 pci_find_capability(struct pci_device *pci, u8 cap_id);
> void pci_reboot(void);
>
> #endif
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-04-10 19:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-10 18:55 [Qemu-devel] [PATCH V6 0/2] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached Marcel Apfelbaum
2014-04-10 18:55 ` [Qemu-devel] [PATCH V6 1/2] " Marcel Apfelbaum
2014-04-10 19:05 ` Michael S. Tsirkin
2014-04-10 18:55 ` [Qemu-devel] [PATCH V6 2/2] hw/pci: check if pci2pci bridges implement optional limit registers Marcel Apfelbaum
2014-04-10 19:05 ` Michael S. Tsirkin
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).