diff mbox

[3/4] libqos: fix spapr qpci_map()

Message ID 1481585316-31612-4-git-send-email-lvivier@redhat.com
State New
Headers show

Commit Message

Laurent Vivier Dec. 12, 2016, 11:28 p.m. UTC
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 tests/libqos/pci-spapr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Thomas Huth Dec. 13, 2016, 8:46 a.m. UTC | #1
A short patch description would be nice here.

On 13.12.2016 00:28, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  tests/libqos/pci-spapr.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/libqos/pci-spapr.c b/tests/libqos/pci-spapr.c
> index 1e5d015..2043f1e 100644
> --- a/tests/libqos/pci-spapr.c
> +++ b/tests/libqos/pci-spapr.c
> @@ -193,8 +193,8 @@ QPCIBus *qpci_init_spapr(QGuestAllocator *alloc)
>      ret->pio.size = SPAPR_PCI_IO_WIN_SIZE;
>  
>      /* 32-bit portion of the MMIO window is at PCI address 2..4 GiB */
> -    ret->mmio32_cpu_base = SPAPR_PCI_BASE + SPAPR_PCI_MMIO32_WIN_SIZE;
> -    ret->mmio32.pci_base = 0x80000000; /* 2 GiB */
> +    ret->mmio32_cpu_base = SPAPR_PCI_BASE;
> +    ret->mmio32.pci_base = SPAPR_PCI_MMIO32_WIN_SIZE;
>      ret->mmio32.size = SPAPR_PCI_MMIO32_WIN_SIZE;

Not sure whether it is really right to use SPAPR_PCI_MMIO32_WIN_SIZE for
the pci_base? I mean, the WIN_SIZE could also be 1G for example, but the
base could still be at 2G ? IMHO it's better to keep the old value for
pci_base here.

 Thomas
Laurent Vivier Dec. 13, 2016, 11:58 a.m. UTC | #2
On 13/12/2016 09:46, Thomas Huth wrote:
> A short patch description would be nice here.
> 
> On 13.12.2016 00:28, Laurent Vivier wrote:
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>  tests/libqos/pci-spapr.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/libqos/pci-spapr.c b/tests/libqos/pci-spapr.c
>> index 1e5d015..2043f1e 100644
>> --- a/tests/libqos/pci-spapr.c
>> +++ b/tests/libqos/pci-spapr.c
>> @@ -193,8 +193,8 @@ QPCIBus *qpci_init_spapr(QGuestAllocator *alloc)
>>      ret->pio.size = SPAPR_PCI_IO_WIN_SIZE;
>>  
>>      /* 32-bit portion of the MMIO window is at PCI address 2..4 GiB */
>> -    ret->mmio32_cpu_base = SPAPR_PCI_BASE + SPAPR_PCI_MMIO32_WIN_SIZE;
>> -    ret->mmio32.pci_base = 0x80000000; /* 2 GiB */
>> +    ret->mmio32_cpu_base = SPAPR_PCI_BASE;
>> +    ret->mmio32.pci_base = SPAPR_PCI_MMIO32_WIN_SIZE;
>>      ret->mmio32.size = SPAPR_PCI_MMIO32_WIN_SIZE;
> 
> Not sure whether it is really right to use SPAPR_PCI_MMIO32_WIN_SIZE for
> the pci_base? I mean, the WIN_SIZE could also be 1G for example, but the
> base could still be at 2G ? IMHO it's better to keep the old value for
> pci_base here.

in hw/ppc/spapr.c, we have:

    *mmio32 = SPAPR_PCI_BASE + (index + 1) * SPAPR_PCI_MEM32_WIN_SIZE;

So I think it's logic to have the PCI base at  SPAPR_PCI_MMIO32_WIN_SIZE.

Thanks for your review,
Laurent
diff mbox

Patch

diff --git a/tests/libqos/pci-spapr.c b/tests/libqos/pci-spapr.c
index 1e5d015..2043f1e 100644
--- a/tests/libqos/pci-spapr.c
+++ b/tests/libqos/pci-spapr.c
@@ -193,8 +193,8 @@  QPCIBus *qpci_init_spapr(QGuestAllocator *alloc)
     ret->pio.size = SPAPR_PCI_IO_WIN_SIZE;
 
     /* 32-bit portion of the MMIO window is at PCI address 2..4 GiB */
-    ret->mmio32_cpu_base = SPAPR_PCI_BASE + SPAPR_PCI_MMIO32_WIN_SIZE;
-    ret->mmio32.pci_base = 0x80000000; /* 2 GiB */
+    ret->mmio32_cpu_base = SPAPR_PCI_BASE;
+    ret->mmio32.pci_base = SPAPR_PCI_MMIO32_WIN_SIZE;
     ret->mmio32.size = SPAPR_PCI_MMIO32_WIN_SIZE;
 
     ret->bus.pio_alloc_ptr = 0xc000;