[Qemu-devel] [PATCH v3] spapr-pci: Enable huge BARs

[Qemu-devel] [PATCH v3] spapr-pci: Enable huge BARs

[Alexey Kardashevskiy]

At the moment sPAPR only supports 512MB window for MMIO BARs. However
modern devices might want bigger 64bit BARs.

This extends MMIO window from 512MB to 62GB (aligned to
SPAPR_PCI_WINDOW_SPACING) and advertises it in 2 records in
the PHB "ranges" property. 32bit gets the space from
SPAPR_PCI_MEM_WIN_BUS_OFFSET till the end of 4GB, 64bit gets the rest
of the space.

The approach changes the device tree which is a guest visible change, however
it won't break migration as:
1. we do not support migration to older QEMU versions
2. migration to newer QEMU will migrate the device tree as well and since
the new layout only extends the old one and does not change address mappigns,
no breakage is expected here too.

SLOF change is required to utilize this extension.

Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* used SPAPR_PCI_WINDOW_SPACING in SPAPR_PCI_MMIO_WIN_SIZE
* extended commit log

v2:
* do not change existing memory layout
* do not create another mmio window
---
 hw/ppc/spapr_pci.c          | 8 +++++++-
 include/hw/pci-host/spapr.h | 7 ++++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index c0d8c1c..7b73106 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -862,6 +862,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
     int bus_off, i, j;
     char nodename[256];
     uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) };
+    const uint64_t win32size = (1ULL << 32) - SPAPR_PCI_MEM_WIN_BUS_OFFSET;
     struct {
         uint32_t hi;
         uint64_t child;
@@ -876,7 +877,12 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
         {
             cpu_to_be32(b_ss(2)), cpu_to_be64(SPAPR_PCI_MEM_WIN_BUS_OFFSET),
             cpu_to_be64(phb->mem_win_addr),
-            cpu_to_be64(memory_region_size(&phb->memwindow)),
+            cpu_to_be64(win32size),
+        },
+        {
+            cpu_to_be32(b_ss(3)), cpu_to_be64(1ULL << 32),
+            cpu_to_be64(phb->mem_win_addr + win32size),
+            cpu_to_be64(memory_region_size(&phb->memwindow) - win32size)
         },
     };
     uint64_t bus_reg[] = { cpu_to_be64(phb->buid), 0 };
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index 4ea2a0d..92695b6 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -96,17 +96,18 @@ struct sPAPRPHBVFIOState {
 
 #define SPAPR_PCI_BASE_BUID          0x800000020000000ULL
 
+#define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL
+
 #define SPAPR_PCI_WINDOW_BASE        0x10000000000ULL
 #define SPAPR_PCI_WINDOW_SPACING     0x1000000000ULL
 #define SPAPR_PCI_MMIO_WIN_OFF       0xA0000000
-#define SPAPR_PCI_MMIO_WIN_SIZE      0x20000000
+#define SPAPR_PCI_MMIO_WIN_SIZE      (SPAPR_PCI_WINDOW_SPACING - \
+                                     SPAPR_PCI_MEM_WIN_BUS_OFFSET)
 #define SPAPR_PCI_IO_WIN_OFF         0x80000000
 #define SPAPR_PCI_IO_WIN_SIZE        0x10000
 
 #define SPAPR_PCI_MSI_WINDOW         0x40000000000ULL
 
-#define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL
-
 static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
 {
     return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq);
-- 
2.0.0

Re: [Qemu-devel] [PATCH v3] spapr-pci: Enable huge BARs

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4311 bytes --]

On Fri, Jan 09, 2015 at 12:11:14PM +1100, Alexey Kardashevskiy wrote:
> At the moment sPAPR only supports 512MB window for MMIO BARs. However
> modern devices might want bigger 64bit BARs.
> 
> This extends MMIO window from 512MB to 62GB (aligned to
> SPAPR_PCI_WINDOW_SPACING) and advertises it in 2 records in
> the PHB "ranges" property. 32bit gets the space from
> SPAPR_PCI_MEM_WIN_BUS_OFFSET till the end of 4GB, 64bit gets the rest
> of the space.
> 
> The approach changes the device tree which is a guest visible change, however
> it won't break migration as:
> 1. we do not support migration to older QEMU versions
> 2. migration to newer QEMU will migrate the device tree as well and since
> the new layout only extends the old one and does not change address mappigns,
> no breakage is expected here too.
> 
> SLOF change is required to utilize this extension.
> 
> Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v3:
> * used SPAPR_PCI_WINDOW_SPACING in SPAPR_PCI_MMIO_WIN_SIZE
> * extended commit log
> 
> v2:
> * do not change existing memory layout
> * do not create another mmio window
> ---
>  hw/ppc/spapr_pci.c          | 8 +++++++-
>  include/hw/pci-host/spapr.h | 7 ++++---
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index c0d8c1c..7b73106 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -862,6 +862,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>      int bus_off, i, j;
>      char nodename[256];
>      uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) };
> +    const uint64_t win32size = (1ULL << 32) - SPAPR_PCI_MEM_WIN_BUS_OFFSET;
>      struct {
>          uint32_t hi;
>          uint64_t child;
> @@ -876,7 +877,12 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>          {
>              cpu_to_be32(b_ss(2)), cpu_to_be64(SPAPR_PCI_MEM_WIN_BUS_OFFSET),
>              cpu_to_be64(phb->mem_win_addr),
> -            cpu_to_be64(memory_region_size(&phb->memwindow)),
> +            cpu_to_be64(win32size),
> +        },
> +        {
> +            cpu_to_be32(b_ss(3)), cpu_to_be64(1ULL << 32),
> +            cpu_to_be64(phb->mem_win_addr + win32size),
> +            cpu_to_be64(memory_region_size(&phb->memwindow) - win32size)
>          },

I think this needs to be a little fancier.  It will work in the case
of the normal PHB configuration.  But the user can override the size
of the memory window size, in which case you may need to omit the
second range, or even limit the size of the 32-bit range.

>      };
>      uint64_t bus_reg[] = { cpu_to_be64(phb->buid), 0 };
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 4ea2a0d..92695b6 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -96,17 +96,18 @@ struct sPAPRPHBVFIOState {
>  
>  #define SPAPR_PCI_BASE_BUID          0x800000020000000ULL
>  
> +#define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL
> +
>  #define SPAPR_PCI_WINDOW_BASE        0x10000000000ULL
>  #define SPAPR_PCI_WINDOW_SPACING     0x1000000000ULL
>  #define SPAPR_PCI_MMIO_WIN_OFF       0xA0000000
> -#define SPAPR_PCI_MMIO_WIN_SIZE      0x20000000
> +#define SPAPR_PCI_MMIO_WIN_SIZE      (SPAPR_PCI_WINDOW_SPACING - \
> +                                     SPAPR_PCI_MEM_WIN_BUS_OFFSET)
>  #define SPAPR_PCI_IO_WIN_OFF         0x80000000
>  #define SPAPR_PCI_IO_WIN_SIZE        0x10000
>  
>  #define SPAPR_PCI_MSI_WINDOW         0x40000000000ULL
>  
> -#define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL
> -
>  static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
>  {
>      return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq);

I think you also should disable the extended window for older machine
revisions.  It would probably work anyway, but I think it's going to
be safer for backwards compat if you keep the windows the same for a
given machine revision.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH v3] spapr-pci: Enable huge BARs

[Alexey Kardashevskiy]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/14/2015 03:51 PM, David Gibson wrote:
> On Fri, Jan 09, 2015 at 12:11:14PM +1100, Alexey Kardashevskiy wrote:
>> At the moment sPAPR only supports 512MB window for MMIO BARs.
>> However modern devices might want bigger 64bit BARs.
>> 
>> This extends MMIO window from 512MB to 62GB (aligned to 
>> SPAPR_PCI_WINDOW_SPACING) and advertises it in 2 records in the PHB
>> "ranges" property. 32bit gets the space from 
>> SPAPR_PCI_MEM_WIN_BUS_OFFSET till the end of 4GB, 64bit gets the
>> rest of the space.
>> 
>> The approach changes the device tree which is a guest visible
>> change, however it won't break migration as: 1. we do not support
>> migration to older QEMU versions 2. migration to newer QEMU will
>> migrate the device tree as well and since the new layout only
>> extends the old one and does not change address mappigns, no
>> breakage is expected here too.
>> 
>> SLOF change is required to utilize this extension.
>> 
>> Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> 
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- Changes: 
>> v3: * used SPAPR_PCI_WINDOW_SPACING in SPAPR_PCI_MMIO_WIN_SIZE *
>> extended commit log
>> 
>> v2: * do not change existing memory layout * do not create another
>> mmio window --- hw/ppc/spapr_pci.c          | 8 +++++++- 
>> include/hw/pci-host/spapr.h | 7 ++++--- 2 files changed, 11
>> insertions(+), 4 deletions(-)
>> 
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index
>> c0d8c1c..7b73106 100644 --- a/hw/ppc/spapr_pci.c +++
>> b/hw/ppc/spapr_pci.c @@ -862,6 +862,7 @@ int
>> spapr_populate_pci_dt(sPAPRPHBState *phb, int bus_off, i, j; char
>> nodename[256]; uint32_t bus_range[] = { cpu_to_be32(0),
>> cpu_to_be32(0xff) }; +    const uint64_t win32size = (1ULL << 32) -
>> SPAPR_PCI_MEM_WIN_BUS_OFFSET; struct { uint32_t hi; uint64_t child; 
>> @@ -876,7 +877,12 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, 
>> { cpu_to_be32(b_ss(2)), cpu_to_be64(SPAPR_PCI_MEM_WIN_BUS_OFFSET), 
>> cpu_to_be64(phb->mem_win_addr), -
>> cpu_to_be64(memory_region_size(&phb->memwindow)), +
>> cpu_to_be64(win32size), +        }, +        { +
>> cpu_to_be32(b_ss(3)), cpu_to_be64(1ULL << 32), +
>> cpu_to_be64(phb->mem_win_addr + win32size), +
>> cpu_to_be64(memory_region_size(&phb->memwindow) - win32size) },
> 
> I think this needs to be a little fancier.  It will work in the case 
> of the normal PHB configuration.  But the user can override the size 
> of the memory window size, in which case you may need to omit the 
> second range, or even limit the size of the 32-bit range.


Ah, right, the "memory_region_size(&phb->memwindow) > win32size" check is
missing here. Will fix, thanks.


>> }; uint64_t bus_reg[] = { cpu_to_be64(phb->buid), 0 }; diff --git
>> a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h index
>> 4ea2a0d..92695b6 100644 --- a/include/hw/pci-host/spapr.h +++
>> b/include/hw/pci-host/spapr.h @@ -96,17 +96,18 @@ struct
>> sPAPRPHBVFIOState {
>> 
>> #define SPAPR_PCI_BASE_BUID          0x800000020000000ULL
>> 
>> +#define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL + #define
>> SPAPR_PCI_WINDOW_BASE        0x10000000000ULL #define
>> SPAPR_PCI_WINDOW_SPACING     0x1000000000ULL #define
>> SPAPR_PCI_MMIO_WIN_OFF       0xA0000000 -#define
>> SPAPR_PCI_MMIO_WIN_SIZE      0x20000000 +#define
>> SPAPR_PCI_MMIO_WIN_SIZE      (SPAPR_PCI_WINDOW_SPACING - \ +
>> SPAPR_PCI_MEM_WIN_BUS_OFFSET) #define SPAPR_PCI_IO_WIN_OFF
>> 0x80000000 #define SPAPR_PCI_IO_WIN_SIZE        0x10000
>> 
>> #define SPAPR_PCI_MSI_WINDOW         0x40000000000ULL
>> 
>> -#define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL - static inline
>> qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin) { 
>> return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq);
> 
> I think you also should disable the extended window for older machine 
> revisions.  It would probably work anyway, but I think it's going to 
> be safer for backwards compat if you keep the windows the same for a 
> given machine revision.

Hos is it safer? What can possibly break it? I understand the intention
but in this particular case I cannot see any possible problems, can you?


- -- 
Alexey
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJUuF5hAAoJEIYTPdgrwSC5pPIP/iBfFOzM7NKC09Jir1TxeODg
dEAI4tBEWeF6yuoE5u6sN74SoT9kX3/FaUQoWAIT+6W4HVj7Sbd65J7O4yK2XTEa
XGsU3UN4qqp7rYP5OClHqXAiltN5saHe4MmPNLW5zqTl7bLkW1ExIZ18VdTrsZDI
5DNXqkhNdawTtQtu9URe1x5YmiHiaD1tKEn8v9yAXZwGMbjb+HWhkWXRuGER1WdO
V/+AuNTOsNGh+3PJE8WScOw/I1bHkCX4EQmevXVXcNQ1MLa5uI3h0cpCChDhppvV
Qj3CLk+PA8Ik3sq2uGEBnVI20jhRFb2o9LzO504EtEJMuqTbNbJrVX80WrgnzyWD
tqsE2tx6Pj/znhNRioHKBXqBYDcYaZ67Ut42PXbLOGm0Bha/1BrZEq7N0U+17xcT
diMsGcgMTEfBSfR+T3hCFOZVEBNYzvxeMfdTTIu2cysf6XYV3qMK7EopM4xMF1s6
dAUY6/TzTceOfBC6+6nfd3VWmxvC0CCd/upuV9x/a/0It6WOiYzwkMM4Tnu9AME+
gOEnY6GOwe1qw8/EQmUZcDqY0zC41KgTeuUyKk65juhmJjJuBUhn9p1YWfQ4xniD
Ra2CxoOkuO2BS3tAPxIdU0zPjm+NHOrwAOV5qC0MEmDzS2iixGEoG5ytbE1qNcg1
pwdxlDx2jOMbEfAny72s
=juwo
-----END PGP SIGNATURE-----