1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] spapr_pci: allow control of BAR alignment through SLOF

Michael Roth posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1488329807-11193-1-git-send-email-mdroth@linux.vnet.ibm.com
Test checkpatch passed
Test docker passed
Test s390x passed
There is a newer version of this series
hw/ppc/spapr.c              | 7 ++++++-
hw/ppc/spapr_pci.c          | 8 ++++++++
include/hw/pci-host/spapr.h | 1 +
3 files changed, 15 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] spapr_pci: allow control of BAR alignment through SLOF
Posted by Michael Roth 7 years, 1 month ago 
In certain cases, such as PCI-passthrough with VFIO, we cannot offload
MMIO accesses to KVM unless the BAR alignment matches the host. This
patch, in conjunction with a separately submitted patch for SLOF
which allows for control of this via the device-tree, allows us to
set this alignment via QEMU.

Cc: qemu-ppc@nongnu.org
Cc: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c              | 7 ++++++-
 hw/ppc/spapr_pci.c          | 8 ++++++++
 include/hw/pci-host/spapr.h | 1 +
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 81c6c1c..321951c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3121,7 +3121,12 @@ DEFINE_SPAPR_MACHINE(2_9, "2.9", true);
  * pseries-2.8
  */
 #define SPAPR_COMPAT_2_8                            \
-    HW_COMPAT_2_8
+    HW_COMPAT_2_8                                   \
+    {                                               \
+        .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,     \
+        .property = "mem_bar_min_align",            \
+        .value    = "0",                            \
+    },                                              \
 
 static void spapr_machine_2_8_instance_options(MachineState *machine)
 {
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 2a3499e..fdfd494 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1660,6 +1660,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (sphb->mem_bar_min_align == (uint64_t)-1) {
+        sphb->mem_bar_min_align = qemu_real_host_page_size;
+    }
+
     sphb->dtbusname = g_strdup_printf("pci@%" PRIx64, sphb->buid);
 
     namebuf = alloca(strlen(sphb->dtbusname) + 32);
@@ -1854,6 +1858,8 @@ static Property spapr_phb_properties[] = {
     DEFINE_PROP_UINT32("numa_node", sPAPRPHBState, numa_node, -1),
     DEFINE_PROP_BOOL("pre-2.8-migration", sPAPRPHBState,
                      pre_2_8_migration, false),
+    DEFINE_PROP_UINT64("mem_bar_min_align", sPAPRPHBState, mem_bar_min_align,
+                       -1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2224,6 +2230,8 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
     if (ret) {
         return ret;
     }
+    _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,mem-bar-min-align",
+                          phb->mem_bar_min_align));
 
     return 0;
 }
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index dfa7614..fa33346 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -79,6 +79,7 @@ struct sPAPRPHBState {
     uint64_t dma64_win_addr;
 
     uint32_t numa_node;
+    uint64_t mem_bar_min_align;
 
     /* Fields for migration compatibility hacks */
     bool pre_2_8_migration;
-- 
2.7.4
Re: [Qemu-devel] [PATCH] spapr_pci: allow control of BAR alignment through SLOF
Posted by David Gibson 7 years, 1 month ago 
On Tue, Feb 28, 2017 at 06:56:47PM -0600, Michael Roth wrote:
> In certain cases, such as PCI-passthrough with VFIO, we cannot offload
> MMIO accesses to KVM unless the BAR alignment matches the host. This
> patch, in conjunction with a separately submitted patch for SLOF
> which allows for control of this via the device-tree, allows us to
> set this alignment via QEMU.
> 
> Cc: qemu-ppc@nongnu.org
> Cc: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  hw/ppc/spapr.c              | 7 ++++++-
>  hw/ppc/spapr_pci.c          | 8 ++++++++
>  include/hw/pci-host/spapr.h | 1 +
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 81c6c1c..321951c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3121,7 +3121,12 @@ DEFINE_SPAPR_MACHINE(2_9, "2.9", true);
>   * pseries-2.8
>   */
>  #define SPAPR_COMPAT_2_8                            \
> -    HW_COMPAT_2_8
> +    HW_COMPAT_2_8                                   \
> +    {                                               \
> +        .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,     \
> +        .property = "mem_bar_min_align",            \
> +        .value    = "0",                            \
> +    },                                              \
>  
>  static void spapr_machine_2_8_instance_options(MachineState *machine)
>  {
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 2a3499e..fdfd494 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1660,6 +1660,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>  
> +    if (sphb->mem_bar_min_align == (uint64_t)-1) {
> +        sphb->mem_bar_min_align = qemu_real_host_page_size;
> +    }
> +
>      sphb->dtbusname = g_strdup_printf("pci@%" PRIx64, sphb->buid);
>  
>      namebuf = alloca(strlen(sphb->dtbusname) + 32);
> @@ -1854,6 +1858,8 @@ static Property spapr_phb_properties[] = {
>      DEFINE_PROP_UINT32("numa_node", sPAPRPHBState, numa_node, -1),
>      DEFINE_PROP_BOOL("pre-2.8-migration", sPAPRPHBState,
>                       pre_2_8_migration, false),
> +    DEFINE_PROP_UINT64("mem_bar_min_align", sPAPRPHBState, mem_bar_min_align,
> +                       -1),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -2224,6 +2230,8 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>      if (ret) {
>          return ret;
>      }
> +    _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,mem-bar-min-align",
> +                          phb->mem_bar_min_align));

AFAICT the sensible thing to do here is to leave the property out on
older machine types.  In which case you need an if
(phb->mem_bar_min_align) here.
>  
>      return 0;
>  }
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index dfa7614..fa33346 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -79,6 +79,7 @@ struct sPAPRPHBState {
>      uint64_t dma64_win_addr;
>  
>      uint32_t numa_node;
> +    uint64_t mem_bar_min_align;
>  
>      /* Fields for migration compatibility hacks */
>      bool pre_2_8_migration;

-- 
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
Re: [Qemu-devel] [PATCH] spapr_pci: allow control of BAR alignment through SLOF
Posted by Michael Roth 7 years, 1 month ago 
Quoting David Gibson (2017-02-28 20:56:44)
> On Tue, Feb 28, 2017 at 06:56:47PM -0600, Michael Roth wrote:
> > In certain cases, such as PCI-passthrough with VFIO, we cannot offload
> > MMIO accesses to KVM unless the BAR alignment matches the host. This
> > patch, in conjunction with a separately submitted patch for SLOF
> > which allows for control of this via the device-tree, allows us to
> > set this alignment via QEMU.
> > 
> > Cc: qemu-ppc@nongnu.org
> > Cc: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> > Cc: David Gibson <david@gibson.dropbear.id.au>
> > Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> >  hw/ppc/spapr.c              | 7 ++++++-
> >  hw/ppc/spapr_pci.c          | 8 ++++++++
> >  include/hw/pci-host/spapr.h | 1 +
> >  3 files changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 81c6c1c..321951c 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3121,7 +3121,12 @@ DEFINE_SPAPR_MACHINE(2_9, "2.9", true);
> >   * pseries-2.8
> >   */
> >  #define SPAPR_COMPAT_2_8                            \
> > -    HW_COMPAT_2_8
> > +    HW_COMPAT_2_8                                   \
> > +    {                                               \
> > +        .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,     \
> > +        .property = "mem_bar_min_align",            \
> > +        .value    = "0",                            \
> > +    },                                              \
> >  
> >  static void spapr_machine_2_8_instance_options(MachineState *machine)
> >  {
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 2a3499e..fdfd494 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1660,6 +1660,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> >          return;
> >      }
> >  
> > +    if (sphb->mem_bar_min_align == (uint64_t)-1) {
> > +        sphb->mem_bar_min_align = qemu_real_host_page_size;
> > +    }
> > +
> >      sphb->dtbusname = g_strdup_printf("pci@%" PRIx64, sphb->buid);
> >  
> >      namebuf = alloca(strlen(sphb->dtbusname) + 32);
> > @@ -1854,6 +1858,8 @@ static Property spapr_phb_properties[] = {
> >      DEFINE_PROP_UINT32("numa_node", sPAPRPHBState, numa_node, -1),
> >      DEFINE_PROP_BOOL("pre-2.8-migration", sPAPRPHBState,
> >                       pre_2_8_migration, false),
> > +    DEFINE_PROP_UINT64("mem_bar_min_align", sPAPRPHBState, mem_bar_min_align,
> > +                       -1),
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >  
> > @@ -2224,6 +2230,8 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
> >      if (ret) {
> >          return ret;
> >      }
> > +    _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,mem-bar-min-align",
> > +                          phb->mem_bar_min_align));
> 
> AFAICT the sensible thing to do here is to leave the property out on
> older machine types.  In which case you need an if
> (phb->mem_bar_min_align) here.

Yah. On the SLOF side it seemed like 64K was the safer default, which
made this more of an override. But that doesn't make much sense unless
you're mix-and-matching SLOFs/QEMUs. Will change it for v2.

> >  
> >      return 0;
> >  }
> > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> > index dfa7614..fa33346 100644
> > --- a/include/hw/pci-host/spapr.h
> > +++ b/include/hw/pci-host/spapr.h
> > @@ -79,6 +79,7 @@ struct sPAPRPHBState {
> >      uint64_t dma64_win_addr;
> >  
> >      uint32_t numa_node;
> > +    uint64_t mem_bar_min_align;
> >  
> >      /* Fields for migration compatibility hacks */
> >      bool pre_2_8_migration;
> 
> -- 
> 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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.