Re: [PATCH 1/2] ppc/pnv: Add models for POWER9 PHB4 PCIe Host bridge

2020-01-28 Thread David Gibson
On Wed, Jan 29, 2020 at 02:54:19PM +1100, Oliver O'Halloran wrote:
> On Wed, Jan 29, 2020 at 2:09 PM David Gibson
>  wrote:
> >
> > On Mon, Jan 27, 2020 at 03:45:05PM +0100, Cédric Le Goater wrote:
> > > From: Benjamin Herrenschmidt 
> > >
> 
> *snip*
> 
> > > +
> > > +/*
> > > + * The CONFIG_DATA register expects little endian accesses, but as the
> > > + * region is big endian, we have to swap the value.
> > > + */
> > > +static void pnv_phb4_config_write(PnvPHB4 *phb, unsigned off,
> > > +  unsigned size, uint64_t val)
> > > +{
> > > +uint32_t cfg_addr, limit;
> > > +PCIDevice *pdev;
> > > +
> > > +pdev = pnv_phb4_find_cfg_dev(phb);
> > > +if (!pdev) {
> > > +return;
> > > +}
> > > +cfg_addr = (phb->regs[PHB_CONFIG_ADDRESS >> 3] >> 32) & 0xffc;
> > > +cfg_addr |= off;
> > > +limit = pci_config_size(pdev);
> > > +if (limit <= cfg_addr) {
> > > +/*
> > > + * conventional pci device can be behind pcie-to-pci bridge.
> > > + * 256 <= addr < 4K has no effects.
> > > + */
> > > +return;
> > > +}
> > > +switch (size) {
> > > +case 1:
> > > +break;
> > > +case 2:
> > > +val = bswap16(val);
> >
> > I'm a little confused by these byteswaps.  As I see below the device
> > is set to big endian, so the values passed in here should already be
> > in host-native endian.  Why do you need the swap?  Are some of the
> > registers in the bank BE and some LE?
> 
> All the registers are BE except for CONFIG_DATA, which isn't actually
> a register. It's really a window into the config space of the device
> specified in CONFIG_ADDR so it doesn't do any byte-swapping.

Ah, right, that makes sense.

> 
> > > +break;
> > > +case 4:
> > > +val = bswap32(val);
> > > +break;
> > > +default:
> > > +g_assert_not_reached();
> > > +}
> > > +pci_host_config_write_common(pdev, cfg_addr, limit, val, size);
> > > +}
> 

-- 
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


signature.asc
Description: PGP signature


Re: [PATCH 1/2] ppc/pnv: Add models for POWER9 PHB4 PCIe Host bridge

2020-01-28 Thread Oliver O'Halloran
On Wed, Jan 29, 2020 at 2:09 PM David Gibson
 wrote:
>
> On Mon, Jan 27, 2020 at 03:45:05PM +0100, Cédric Le Goater wrote:
> > From: Benjamin Herrenschmidt 
> >

*snip*

> > +
> > +/*
> > + * The CONFIG_DATA register expects little endian accesses, but as the
> > + * region is big endian, we have to swap the value.
> > + */
> > +static void pnv_phb4_config_write(PnvPHB4 *phb, unsigned off,
> > +  unsigned size, uint64_t val)
> > +{
> > +uint32_t cfg_addr, limit;
> > +PCIDevice *pdev;
> > +
> > +pdev = pnv_phb4_find_cfg_dev(phb);
> > +if (!pdev) {
> > +return;
> > +}
> > +cfg_addr = (phb->regs[PHB_CONFIG_ADDRESS >> 3] >> 32) & 0xffc;
> > +cfg_addr |= off;
> > +limit = pci_config_size(pdev);
> > +if (limit <= cfg_addr) {
> > +/*
> > + * conventional pci device can be behind pcie-to-pci bridge.
> > + * 256 <= addr < 4K has no effects.
> > + */
> > +return;
> > +}
> > +switch (size) {
> > +case 1:
> > +break;
> > +case 2:
> > +val = bswap16(val);
>
> I'm a little confused by these byteswaps.  As I see below the device
> is set to big endian, so the values passed in here should already be
> in host-native endian.  Why do you need the swap?  Are some of the
> registers in the bank BE and some LE?

All the registers are BE except for CONFIG_DATA, which isn't actually
a register. It's really a window into the config space of the device
specified in CONFIG_ADDR so it doesn't do any byte-swapping.

> > +break;
> > +case 4:
> > +val = bswap32(val);
> > +break;
> > +default:
> > +g_assert_not_reached();
> > +}
> > +pci_host_config_write_common(pdev, cfg_addr, limit, val, size);
> > +}



Re: [PATCH 1/2] ppc/pnv: Add models for POWER9 PHB4 PCIe Host bridge

2020-01-28 Thread David Gibson
On Mon, Jan 27, 2020 at 03:45:05PM +0100, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt 
> 
> These changes introduces models for the PCIe Host Bridge (PHB4) of the
> POWER9 processor. It includes the PowerBus logic interface (PBCQ),
> IOMMU support, a single PCIe Gen.4 Root Complex, and support for MSI
> and LSI interrupt sources as found on a POWER9 system using the XIVE
> interrupt controller.
> 
> POWER9 processor comes with 3 PHB4 PEC (PCI Express Controller) and
> each PEC can have several PHBs. By default,
> 
>   * PEC0 provides 1 PHB  (PHB0)
>   * PEC1 provides 2 PHBs (PHB1 and PHB2)
>   * PEC2 provides 3 PHBs (PHB3, PHB4 and PHB5)
> 
> Each PEC has a set  "global" registers and some "per-stack" (per-PHB)
> registers. Those are organized in two XSCOM ranges, the "Nest" range
> and the "PCI" range, each range contains both some "PEC" registers and
> some "per-stack" registers.
> 
> No default device layout is provided and PCI devices can be added on
> any of the available PCIe Root Port (pcie.0 .. 2 of a Power9 chip)
> with address 0x0 as the firwware (skiboot) only accepts a single
> device per root port. To run a simple system with a network and a
> storage adapters, use a command line options such as :
> 
>   -device e1000e,netdev=net0,mac=C0:FF:EE:00:00:02,bus=pcie.0,addr=0x0
>   -netdev 
> bridge,id=net0,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=hostnet0
> 
>   -device megasas,id=scsi0,bus=pcie.1,addr=0x0
>   -drive file=$disk,if=none,id=drive-scsi0-0-0-0,format=qcow2,cache=none
>   -device 
> scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=2
> 
> If more are needed, include a bridge.
> 
> Multi chip is supported, each chip adding its set of PHB4 controllers
> and its PCI busses. The model doesn't emulate the EEH error handling.
> 
> This model is not ready for hotplug yet.
> 
> Signed-off-by: Benjamin Herrenschmidt 

Mostly LGTM, one query below.

> [ clg: - numerous cleanups
>- commit log
>- fix for broken LSI support
>- PHB pic printinfo
>- large QOM rework ]
> Signed-off-by: Cédric Le Goater 
> ---
>  include/hw/pci-host/pnv_phb4.h  |  230 +
>  include/hw/pci-host/pnv_phb4_regs.h |  553 ++
>  include/hw/pci/pcie_port.h  |1 +
>  include/hw/ppc/pnv.h|7 +
>  include/hw/ppc/pnv_xscom.h  |   11 +
>  hw/pci-host/pnv_phb4.c  | 1438 +++
>  hw/pci-host/pnv_phb4_pec.c  |  593 +++
>  hw/ppc/pnv.c|  107 ++
>  hw/pci-host/Makefile.objs   |1 +
>  hw/ppc/Kconfig  |2 +
>  10 files changed, 2943 insertions(+)
>  create mode 100644 include/hw/pci-host/pnv_phb4.h
>  create mode 100644 include/hw/pci-host/pnv_phb4_regs.h
>  create mode 100644 hw/pci-host/pnv_phb4.c
>  create mode 100644 hw/pci-host/pnv_phb4_pec.c
> 
> diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
> new file mode 100644
> index ..c882bfd0aa23
> --- /dev/null
> +++ b/include/hw/pci-host/pnv_phb4.h
> @@ -0,0 +1,230 @@
> +/*
> + * QEMU PowerPC PowerNV (POWER9) PHB4 model
> + *
> + * Copyright (c) 2018-2020, IBM Corporation.
> + *
> + * This code is licensed under the GPL version 2 or later. See the
> + * COPYING file in the top-level directory.
> + */
> +
> +#ifndef PCI_HOST_PNV_PHB4_H
> +#define PCI_HOST_PNV_PHB4_H
> +
> +#include "hw/pci/pcie_host.h"
> +#include "hw/pci/pcie_port.h"
> +#include "hw/ppc/xive.h"
> +
> +typedef struct PnvPhb4PecState PnvPhb4PecState;
> +typedef struct PnvPhb4PecStack PnvPhb4PecStack;
> +typedef struct PnvPHB4 PnvPHB4;
> +typedef struct PnvChip PnvChip;
> +
> +/*
> + * We have one such address space wrapper per possible device under
> + * the PHB since they need to be assigned statically at qemu device
> + * creation time. The relationship to a PE is done later
> + * dynamically. This means we can potentially create a lot of these
> + * guys. Q35 stores them as some kind of radix tree but we never
> + * really need to do fast lookups so instead we simply keep a QLIST of
> + * them for now, we can add the radix if needed later on.
> + *
> + * We do cache the PE number to speed things up a bit though.
> + */
> +typedef struct PnvPhb4DMASpace {
> +PCIBus *bus;
> +uint8_t devfn;
> +int pe_num; /* Cached PE number */
> +#define PHB_INVALID_PE (-1)
> +PnvPHB4 *phb;
> +AddressSpace dma_as;
> +IOMMUMemoryRegion dma_mr;
> +MemoryRegion msi32_mr;
> +MemoryRegion msi64_mr;
> +QLIST_ENTRY(PnvPhb4DMASpace) list;
> +} PnvPhb4DMASpace;
> +
> +/*
> + * PHB4 PCIe Root port
> + */
> +#define TYPE_PNV_PHB4_ROOT_BUS "pnv-phb4-root-bus"
> +#define TYPE_PNV_PHB4_ROOT_PORT "pnv-phb4-root-port"
> +
> +typedef struct PnvPHB4RootPort {
> +PCIESlot parent_obj;
> +} PnvPHB4RootPort;
> +
> +/*
> + * PHB4 PCIe Host Bridge for PowerNV machines (POWER9)
> + */
> +#define TYPE_PNV_PHB4 "pnv-phb4"

[PATCH 1/2] ppc/pnv: Add models for POWER9 PHB4 PCIe Host bridge

2020-01-27 Thread Cédric Le Goater
From: Benjamin Herrenschmidt 

These changes introduces models for the PCIe Host Bridge (PHB4) of the
POWER9 processor. It includes the PowerBus logic interface (PBCQ),
IOMMU support, a single PCIe Gen.4 Root Complex, and support for MSI
and LSI interrupt sources as found on a POWER9 system using the XIVE
interrupt controller.

POWER9 processor comes with 3 PHB4 PEC (PCI Express Controller) and
each PEC can have several PHBs. By default,

  * PEC0 provides 1 PHB  (PHB0)
  * PEC1 provides 2 PHBs (PHB1 and PHB2)
  * PEC2 provides 3 PHBs (PHB3, PHB4 and PHB5)

Each PEC has a set  "global" registers and some "per-stack" (per-PHB)
registers. Those are organized in two XSCOM ranges, the "Nest" range
and the "PCI" range, each range contains both some "PEC" registers and
some "per-stack" registers.

No default device layout is provided and PCI devices can be added on
any of the available PCIe Root Port (pcie.0 .. 2 of a Power9 chip)
with address 0x0 as the firwware (skiboot) only accepts a single
device per root port. To run a simple system with a network and a
storage adapters, use a command line options such as :

  -device e1000e,netdev=net0,mac=C0:FF:EE:00:00:02,bus=pcie.0,addr=0x0
  -netdev 
bridge,id=net0,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=hostnet0

  -device megasas,id=scsi0,bus=pcie.1,addr=0x0
  -drive file=$disk,if=none,id=drive-scsi0-0-0-0,format=qcow2,cache=none
  -device 
scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=2

If more are needed, include a bridge.

Multi chip is supported, each chip adding its set of PHB4 controllers
and its PCI busses. The model doesn't emulate the EEH error handling.

This model is not ready for hotplug yet.

Signed-off-by: Benjamin Herrenschmidt 
[ clg: - numerous cleanups
   - commit log
   - fix for broken LSI support
   - PHB pic printinfo
   - large QOM rework ]
Signed-off-by: Cédric Le Goater 
---
 include/hw/pci-host/pnv_phb4.h  |  230 +
 include/hw/pci-host/pnv_phb4_regs.h |  553 ++
 include/hw/pci/pcie_port.h  |1 +
 include/hw/ppc/pnv.h|7 +
 include/hw/ppc/pnv_xscom.h  |   11 +
 hw/pci-host/pnv_phb4.c  | 1438 +++
 hw/pci-host/pnv_phb4_pec.c  |  593 +++
 hw/ppc/pnv.c|  107 ++
 hw/pci-host/Makefile.objs   |1 +
 hw/ppc/Kconfig  |2 +
 10 files changed, 2943 insertions(+)
 create mode 100644 include/hw/pci-host/pnv_phb4.h
 create mode 100644 include/hw/pci-host/pnv_phb4_regs.h
 create mode 100644 hw/pci-host/pnv_phb4.c
 create mode 100644 hw/pci-host/pnv_phb4_pec.c

diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
new file mode 100644
index ..c882bfd0aa23
--- /dev/null
+++ b/include/hw/pci-host/pnv_phb4.h
@@ -0,0 +1,230 @@
+/*
+ * QEMU PowerPC PowerNV (POWER9) PHB4 model
+ *
+ * Copyright (c) 2018-2020, IBM Corporation.
+ *
+ * This code is licensed under the GPL version 2 or later. See the
+ * COPYING file in the top-level directory.
+ */
+
+#ifndef PCI_HOST_PNV_PHB4_H
+#define PCI_HOST_PNV_PHB4_H
+
+#include "hw/pci/pcie_host.h"
+#include "hw/pci/pcie_port.h"
+#include "hw/ppc/xive.h"
+
+typedef struct PnvPhb4PecState PnvPhb4PecState;
+typedef struct PnvPhb4PecStack PnvPhb4PecStack;
+typedef struct PnvPHB4 PnvPHB4;
+typedef struct PnvChip PnvChip;
+
+/*
+ * We have one such address space wrapper per possible device under
+ * the PHB since they need to be assigned statically at qemu device
+ * creation time. The relationship to a PE is done later
+ * dynamically. This means we can potentially create a lot of these
+ * guys. Q35 stores them as some kind of radix tree but we never
+ * really need to do fast lookups so instead we simply keep a QLIST of
+ * them for now, we can add the radix if needed later on.
+ *
+ * We do cache the PE number to speed things up a bit though.
+ */
+typedef struct PnvPhb4DMASpace {
+PCIBus *bus;
+uint8_t devfn;
+int pe_num; /* Cached PE number */
+#define PHB_INVALID_PE (-1)
+PnvPHB4 *phb;
+AddressSpace dma_as;
+IOMMUMemoryRegion dma_mr;
+MemoryRegion msi32_mr;
+MemoryRegion msi64_mr;
+QLIST_ENTRY(PnvPhb4DMASpace) list;
+} PnvPhb4DMASpace;
+
+/*
+ * PHB4 PCIe Root port
+ */
+#define TYPE_PNV_PHB4_ROOT_BUS "pnv-phb4-root-bus"
+#define TYPE_PNV_PHB4_ROOT_PORT "pnv-phb4-root-port"
+
+typedef struct PnvPHB4RootPort {
+PCIESlot parent_obj;
+} PnvPHB4RootPort;
+
+/*
+ * PHB4 PCIe Host Bridge for PowerNV machines (POWER9)
+ */
+#define TYPE_PNV_PHB4 "pnv-phb4"
+#define PNV_PHB4(obj) OBJECT_CHECK(PnvPHB4, (obj), TYPE_PNV_PHB4)
+
+#define PNV_PHB4_MAX_LSIs  8
+#define PNV_PHB4_MAX_INTs  4096
+#define PNV_PHB4_MAX_MIST  (PNV_PHB4_MAX_INTs >> 2)
+#define PNV_PHB4_MAX_MMIO_WINDOWS  32
+#define PNV_PHB4_MIN_MMIO_WINDOWS  16
+#define PNV_PHB4_NUM_REGS  (0x3000 >> 3)
+#define