* [Qemu-devel] [PATCH v4] Align PCI capabilities in pci_find_space
@ 2012-09-26 18:51 Matt Renzelmann
2012-09-26 19:12 ` Alex Williamson
2012-09-29 11:42 ` Blue Swirl
0 siblings, 2 replies; 3+ messages in thread
From: Matt Renzelmann @ 2012-09-26 18:51 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.williamson
The current implementation of pci_find_space does not correctly align
PCI capabilities in the PCI configuration space. It also does not
support PCI-Express devices. This patch fixes these issues.
Thanks to Alex Williamson for feedback.
Signed-off-by: Matt Renzelmann <mjr@cs.wisc.edu>
---
This version adds the assertions to pci_find_space and removes the
assertion from pci_find_express_space.
hw/pci.c | 35 +++++++++++++++++++++++++++--------
1 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index f855cf3..7050596 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1626,19 +1626,38 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)
return pci_create_simple_multifunction(bus, devfn, false, name);
}
-static int pci_find_space(PCIDevice *pdev, uint8_t size)
+static int pci_find_space(PCIDevice *pdev, uint32_t start,
+ uint32_t end, uint32_t size)
{
- int config_size = pci_config_size(pdev);
- int offset = PCI_CONFIG_HEADER_SIZE;
+ int offset = start;
int i;
- for (i = PCI_CONFIG_HEADER_SIZE; i < config_size; ++i)
- if (pdev->used[i])
- offset = i + 1;
- else if (i - offset + 1 == size)
+ uint32_t *dword_used = &pdev->used[start];
+
+ assert(pci_config_size(pdev) >= end);
+ assert(!(start & 0x3));
+
+ /* This approach ensures the capability is dword-aligned, as
+ required by the PCI and PCI-E specifications */
+ for (i = start; i < end; i += 4, dword_used++) {
+ if (*dword_used)
+ offset = i + 4;
+ else if (i - offset + 4 >= size)
return offset;
+ }
+
return 0;
}
+static int pci_find_legacy_space(PCIDevice *pdev, uint8_t size) {
+ return pci_find_space(pdev, PCI_CONFIG_HEADER_SIZE,
+ PCI_CONFIG_SPACE_SIZE, size);
+}
+
+static int pci_find_express_space(PCIDevice *pdev, uint16_t size) {
+ return pci_find_space(pdev, PCI_CONFIG_SPACE_SIZE,
+ PCIE_CONFIG_SPACE_SIZE, size);
+}
+
static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,
uint8_t *prev_p)
{
@@ -1826,7 +1845,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
int i, overlapping_cap;
if (!offset) {
- offset = pci_find_space(pdev, size);
+ offset = pci_find_legacy_space(pdev, size);
if (!offset) {
return -ENOSPC;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Align PCI capabilities in pci_find_space
2012-09-26 18:51 [Qemu-devel] [PATCH v4] Align PCI capabilities in pci_find_space Matt Renzelmann
@ 2012-09-26 19:12 ` Alex Williamson
2012-09-29 11:42 ` Blue Swirl
1 sibling, 0 replies; 3+ messages in thread
From: Alex Williamson @ 2012-09-26 19:12 UTC (permalink / raw)
To: Matt Renzelmann; +Cc: qemu-devel
On Wed, 2012-09-26 at 13:51 -0500, Matt Renzelmann wrote:
> The current implementation of pci_find_space does not correctly align
> PCI capabilities in the PCI configuration space. It also does not
> support PCI-Express devices. This patch fixes these issues.
>
> Thanks to Alex Williamson for feedback.
>
> Signed-off-by: Matt Renzelmann <mjr@cs.wisc.edu>
> ---
>
> This version adds the assertions to pci_find_space and removes the
> assertion from pci_find_express_space.
>
> hw/pci.c | 35 +++++++++++++++++++++++++++--------
> 1 files changed, 27 insertions(+), 8 deletions(-)
Acked-by: Alex Williamson <alex.williamson@redhat.com>
> diff --git a/hw/pci.c b/hw/pci.c
> index f855cf3..7050596 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1626,19 +1626,38 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)
> return pci_create_simple_multifunction(bus, devfn, false, name);
> }
>
> -static int pci_find_space(PCIDevice *pdev, uint8_t size)
> +static int pci_find_space(PCIDevice *pdev, uint32_t start,
> + uint32_t end, uint32_t size)
> {
> - int config_size = pci_config_size(pdev);
> - int offset = PCI_CONFIG_HEADER_SIZE;
> + int offset = start;
> int i;
> - for (i = PCI_CONFIG_HEADER_SIZE; i < config_size; ++i)
> - if (pdev->used[i])
> - offset = i + 1;
> - else if (i - offset + 1 == size)
> + uint32_t *dword_used = &pdev->used[start];
> +
> + assert(pci_config_size(pdev) >= end);
> + assert(!(start & 0x3));
> +
> + /* This approach ensures the capability is dword-aligned, as
> + required by the PCI and PCI-E specifications */
> + for (i = start; i < end; i += 4, dword_used++) {
> + if (*dword_used)
> + offset = i + 4;
> + else if (i - offset + 4 >= size)
> return offset;
> + }
> +
> return 0;
> }
>
> +static int pci_find_legacy_space(PCIDevice *pdev, uint8_t size) {
> + return pci_find_space(pdev, PCI_CONFIG_HEADER_SIZE,
> + PCI_CONFIG_SPACE_SIZE, size);
> +}
> +
> +static int pci_find_express_space(PCIDevice *pdev, uint16_t size) {
> + return pci_find_space(pdev, PCI_CONFIG_SPACE_SIZE,
> + PCIE_CONFIG_SPACE_SIZE, size);
> +}
> +
> static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,
> uint8_t *prev_p)
> {
> @@ -1826,7 +1845,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
> int i, overlapping_cap;
>
> if (!offset) {
> - offset = pci_find_space(pdev, size);
> + offset = pci_find_legacy_space(pdev, size);
> if (!offset) {
> return -ENOSPC;
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Align PCI capabilities in pci_find_space
2012-09-26 18:51 [Qemu-devel] [PATCH v4] Align PCI capabilities in pci_find_space Matt Renzelmann
2012-09-26 19:12 ` Alex Williamson
@ 2012-09-29 11:42 ` Blue Swirl
1 sibling, 0 replies; 3+ messages in thread
From: Blue Swirl @ 2012-09-29 11:42 UTC (permalink / raw)
To: Matt Renzelmann; +Cc: alex.williamson, qemu-devel
On Wed, Sep 26, 2012 at 6:51 PM, Matt Renzelmann <mjr@cs.wisc.edu> wrote:
> The current implementation of pci_find_space does not correctly align
> PCI capabilities in the PCI configuration space. It also does not
> support PCI-Express devices. This patch fixes these issues.
>
> Thanks to Alex Williamson for feedback.
>
> Signed-off-by: Matt Renzelmann <mjr@cs.wisc.edu>
> ---
>
> This version adds the assertions to pci_find_space and removes the
> assertion from pci_find_express_space.
>
> hw/pci.c | 35 +++++++++++++++++++++++++++--------
> 1 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index f855cf3..7050596 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1626,19 +1626,38 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)
> return pci_create_simple_multifunction(bus, devfn, false, name);
> }
>
> -static int pci_find_space(PCIDevice *pdev, uint8_t size)
> +static int pci_find_space(PCIDevice *pdev, uint32_t start,
> + uint32_t end, uint32_t size)
> {
> - int config_size = pci_config_size(pdev);
> - int offset = PCI_CONFIG_HEADER_SIZE;
> + int offset = start;
> int i;
> - for (i = PCI_CONFIG_HEADER_SIZE; i < config_size; ++i)
> - if (pdev->used[i])
> - offset = i + 1;
> - else if (i - offset + 1 == size)
> + uint32_t *dword_used = &pdev->used[start];
> +
> + assert(pci_config_size(pdev) >= end);
> + assert(!(start & 0x3));
> +
> + /* This approach ensures the capability is dword-aligned, as
> + required by the PCI and PCI-E specifications */
> + for (i = start; i < end; i += 4, dword_used++) {
> + if (*dword_used)
Please add braces.
> + offset = i + 4;
> + else if (i - offset + 4 >= size)
Also here to the 'if' part.
> return offset;
> + }
> +
> return 0;
> }
>
> +static int pci_find_legacy_space(PCIDevice *pdev, uint8_t size) {
> + return pci_find_space(pdev, PCI_CONFIG_HEADER_SIZE,
> + PCI_CONFIG_SPACE_SIZE, size);
> +}
> +
> +static int pci_find_express_space(PCIDevice *pdev, uint16_t size) {
> + return pci_find_space(pdev, PCI_CONFIG_SPACE_SIZE,
> + PCIE_CONFIG_SPACE_SIZE, size);
> +}
> +
> static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id,
> uint8_t *prev_p)
> {
> @@ -1826,7 +1845,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
> int i, overlapping_cap;
>
> if (!offset) {
> - offset = pci_find_space(pdev, size);
> + offset = pci_find_legacy_space(pdev, size);
> if (!offset) {
> return -ENOSPC;
> }
> --
> 1.7.5.4
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-09-29 11:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-26 18:51 [Qemu-devel] [PATCH v4] Align PCI capabilities in pci_find_space Matt Renzelmann
2012-09-26 19:12 ` Alex Williamson
2012-09-29 11:42 ` Blue Swirl
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).