Re: [PATCH v1 5/7] hw/fsi: IBM's On-chip Peripheral Bus

2023-08-28 Thread Ninad Palsule

Thanks for the review Joel.

On 8/28/23 23:59, Joel Stanley wrote:

On Fri, 25 Aug 2023 at 20:35, Ninad Palsule  wrote:

This is a part of patchset where IBM's Flexible Service Interface is
introduced.

The On-Chip Peripheral Bus (OPB): A low-speed bus typically found in
POWER processors. This now makes an appearance in the ASPEED SoC due
to tight integration of the FSI master IP with the OPB, mainly the
existence of an MMIO-mapping of the CFAM address straight onto a
sub-region of the OPB address space.

Signed-off-by: Andrew Jeffery 
Signed-off-by: Cédric Le Goater 
Signed-off-by: Ninad Palsule 

Reviewed-by: Joel Stanley 


---
  hw/fsi/Kconfig   |   4 +
  hw/fsi/fsi-master.c  |   3 +-
  hw/fsi/meson.build   |   1 +
  hw/fsi/opb.c | 194 +++
  include/hw/fsi/opb.h |  45 ++
  5 files changed, 245 insertions(+), 2 deletions(-)
  create mode 100644 hw/fsi/opb.c
  create mode 100644 include/hw/fsi/opb.h

diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
index 087980be22..560ce536db 100644
--- a/hw/fsi/Kconfig
+++ b/hw/fsi/Kconfig
@@ -1,3 +1,7 @@
+config OPB
+bool
+select CFAM
+
  config CFAM
  bool
  select FSI
diff --git a/hw/fsi/fsi-master.c b/hw/fsi/fsi-master.c
index fe1693539a..ba00e2bb7d 100644
--- a/hw/fsi/fsi-master.c
+++ b/hw/fsi/fsi-master.c
@@ -13,8 +13,7 @@

  #include "hw/fsi/bits.h"
  #include "hw/fsi/fsi-master.h"
-
-#define TYPE_OP_BUS "opb"
+#include "hw/fsi/opb.h"

  #define TO_REG(x)   ((x) >> 2)

diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
index ca80d11cb9..cab645f4ea 100644
--- a/hw/fsi/meson.build
+++ b/hw/fsi/meson.build
@@ -2,3 +2,4 @@ system_ss.add(when: 'CONFIG_LBUS', if_true: files('lbus.c'))
  system_ss.add(when: 'CONFIG_SCRATCHPAD', if_true: 
files('engine-scratchpad.c'))
  system_ss.add(when: 'CONFIG_CFAM', if_true: files('cfam.c'))
  system_ss.add(when: 'CONFIG_FSI', if_true: 
files('fsi.c','fsi-master.c','fsi-slave.c'))
+system_ss.add(when: 'CONFIG_OPB', if_true: files('opb.c'))
diff --git a/hw/fsi/opb.c b/hw/fsi/opb.c
new file mode 100644
index 00..ac7693c001
--- /dev/null
+++ b/hw/fsi/opb.c
@@ -0,0 +1,194 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM On-chip Peripheral Bus
+ */
+
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+#include "qemu/log.h"
+
+#include "hw/fsi/opb.h"
+
+static MemTxResult opb_read(OPBus *opb, hwaddr addr, void *data, size_t len)
+{
+return address_space_read(>as, addr, MEMTXATTRS_UNSPECIFIED, data,
+  len);
+}
+
+uint8_t opb_read8(OPBus *opb, hwaddr addr)
+{
+MemTxResult tx;
+uint8_t data;
+
+tx = opb_read(opb, addr, , sizeof(data));
+/* FIXME: improve error handling */
+assert(!tx);
+
+return data;
+}
+
+uint16_t opb_read16(OPBus *opb, hwaddr addr)
+{
+MemTxResult tx;
+uint16_t data;
+
+tx = opb_read(opb, addr, , sizeof(data));
+/* FIXME: improve error handling */
+assert(!tx);
+
+return data;
+}
+
+uint32_t opb_read32(OPBus *opb, hwaddr addr)
+{
+MemTxResult tx;
+uint32_t data;
+
+tx = opb_read(opb, addr, , sizeof(data));
+/* FIXME: improve error handling */
+assert(!tx);
+
+return data;
+}
+
+static MemTxResult opb_write(OPBus *opb, hwaddr addr, void *data, size_t len)
+{
+return address_space_write(>as, addr, MEMTXATTRS_UNSPECIFIED, data,
+   len);
+}
+
+void opb_write8(OPBus *opb, hwaddr addr, uint8_t data)
+{
+MemTxResult tx;
+
+tx = opb_write(opb, addr, , sizeof(data));
+/* FIXME: improve error handling */
+assert(!tx);
+}
+
+void opb_write16(OPBus *opb, hwaddr addr, uint16_t data)
+{
+MemTxResult tx;
+
+tx = opb_write(opb, addr, , sizeof(data));
+/* FIXME: improve error handling */
+assert(!tx);
+}
+
+void opb_write32(OPBus *opb, hwaddr addr, uint32_t data)
+{
+MemTxResult tx;
+
+tx = opb_write(opb, addr, , sizeof(data));
+/* FIXME: improve error handling */
+assert(!tx);
+}
+
+void opb_fsi_master_address(OPBus *opb, hwaddr addr)
+{
+memory_region_transaction_begin();
+memory_region_set_address(>fsi.iomem, addr);
+memory_region_transaction_commit();
+}
+
+void opb_opb2fsi_address(OPBus *opb, hwaddr addr)
+{
+memory_region_transaction_begin();
+memory_region_set_address(>fsi.opb2fsi, addr);
+memory_region_transaction_commit();
+}
+
+static uint64_t opb_unimplemented_read(void *opaque, hwaddr addr, unsigned 
size)
+{
+qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx " size=%d\n",
+  __func__, addr, size);
+
+return 0;
+}
+
+static void opb_unimplemented_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned size)
+{
+qemu_log_mask(LOG_UNIMP, "%s: write @0x%" HWADDR_PRIx " size=%d "
+  "value=%"PRIx64"\n", __func__, addr, size, data);
+}
+
+static const struct MemoryRegionOps 

Re: [PATCH v1 5/7] hw/fsi: IBM's On-chip Peripheral Bus

2023-08-28 Thread Joel Stanley
On Fri, 25 Aug 2023 at 20:35, Ninad Palsule  wrote:
>
> This is a part of patchset where IBM's Flexible Service Interface is
> introduced.
>
> The On-Chip Peripheral Bus (OPB): A low-speed bus typically found in
> POWER processors. This now makes an appearance in the ASPEED SoC due
> to tight integration of the FSI master IP with the OPB, mainly the
> existence of an MMIO-mapping of the CFAM address straight onto a
> sub-region of the OPB address space.
>
> Signed-off-by: Andrew Jeffery 
> Signed-off-by: Cédric Le Goater 
> Signed-off-by: Ninad Palsule 

Reviewed-by: Joel Stanley 

> ---
>  hw/fsi/Kconfig   |   4 +
>  hw/fsi/fsi-master.c  |   3 +-
>  hw/fsi/meson.build   |   1 +
>  hw/fsi/opb.c | 194 +++
>  include/hw/fsi/opb.h |  45 ++
>  5 files changed, 245 insertions(+), 2 deletions(-)
>  create mode 100644 hw/fsi/opb.c
>  create mode 100644 include/hw/fsi/opb.h
>
> diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
> index 087980be22..560ce536db 100644
> --- a/hw/fsi/Kconfig
> +++ b/hw/fsi/Kconfig
> @@ -1,3 +1,7 @@
> +config OPB
> +bool
> +select CFAM
> +
>  config CFAM
>  bool
>  select FSI
> diff --git a/hw/fsi/fsi-master.c b/hw/fsi/fsi-master.c
> index fe1693539a..ba00e2bb7d 100644
> --- a/hw/fsi/fsi-master.c
> +++ b/hw/fsi/fsi-master.c
> @@ -13,8 +13,7 @@
>
>  #include "hw/fsi/bits.h"
>  #include "hw/fsi/fsi-master.h"
> -
> -#define TYPE_OP_BUS "opb"
> +#include "hw/fsi/opb.h"
>
>  #define TO_REG(x)   ((x) >> 2)
>
> diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
> index ca80d11cb9..cab645f4ea 100644
> --- a/hw/fsi/meson.build
> +++ b/hw/fsi/meson.build
> @@ -2,3 +2,4 @@ system_ss.add(when: 'CONFIG_LBUS', if_true: files('lbus.c'))
>  system_ss.add(when: 'CONFIG_SCRATCHPAD', if_true: 
> files('engine-scratchpad.c'))
>  system_ss.add(when: 'CONFIG_CFAM', if_true: files('cfam.c'))
>  system_ss.add(when: 'CONFIG_FSI', if_true: 
> files('fsi.c','fsi-master.c','fsi-slave.c'))
> +system_ss.add(when: 'CONFIG_OPB', if_true: files('opb.c'))
> diff --git a/hw/fsi/opb.c b/hw/fsi/opb.c
> new file mode 100644
> index 00..ac7693c001
> --- /dev/null
> +++ b/hw/fsi/opb.c
> @@ -0,0 +1,194 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2023 IBM Corp.
> + *
> + * IBM On-chip Peripheral Bus
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "qapi/error.h"
> +#include "qemu/log.h"
> +
> +#include "hw/fsi/opb.h"
> +
> +static MemTxResult opb_read(OPBus *opb, hwaddr addr, void *data, size_t len)
> +{
> +return address_space_read(>as, addr, MEMTXATTRS_UNSPECIFIED, data,
> +  len);
> +}
> +
> +uint8_t opb_read8(OPBus *opb, hwaddr addr)
> +{
> +MemTxResult tx;
> +uint8_t data;
> +
> +tx = opb_read(opb, addr, , sizeof(data));
> +/* FIXME: improve error handling */
> +assert(!tx);
> +
> +return data;
> +}
> +
> +uint16_t opb_read16(OPBus *opb, hwaddr addr)
> +{
> +MemTxResult tx;
> +uint16_t data;
> +
> +tx = opb_read(opb, addr, , sizeof(data));
> +/* FIXME: improve error handling */
> +assert(!tx);
> +
> +return data;
> +}
> +
> +uint32_t opb_read32(OPBus *opb, hwaddr addr)
> +{
> +MemTxResult tx;
> +uint32_t data;
> +
> +tx = opb_read(opb, addr, , sizeof(data));
> +/* FIXME: improve error handling */
> +assert(!tx);
> +
> +return data;
> +}
> +
> +static MemTxResult opb_write(OPBus *opb, hwaddr addr, void *data, size_t len)
> +{
> +return address_space_write(>as, addr, MEMTXATTRS_UNSPECIFIED, data,
> +   len);
> +}
> +
> +void opb_write8(OPBus *opb, hwaddr addr, uint8_t data)
> +{
> +MemTxResult tx;
> +
> +tx = opb_write(opb, addr, , sizeof(data));
> +/* FIXME: improve error handling */
> +assert(!tx);
> +}
> +
> +void opb_write16(OPBus *opb, hwaddr addr, uint16_t data)
> +{
> +MemTxResult tx;
> +
> +tx = opb_write(opb, addr, , sizeof(data));
> +/* FIXME: improve error handling */
> +assert(!tx);
> +}
> +
> +void opb_write32(OPBus *opb, hwaddr addr, uint32_t data)
> +{
> +MemTxResult tx;
> +
> +tx = opb_write(opb, addr, , sizeof(data));
> +/* FIXME: improve error handling */
> +assert(!tx);
> +}
> +
> +void opb_fsi_master_address(OPBus *opb, hwaddr addr)
> +{
> +memory_region_transaction_begin();
> +memory_region_set_address(>fsi.iomem, addr);
> +memory_region_transaction_commit();
> +}
> +
> +void opb_opb2fsi_address(OPBus *opb, hwaddr addr)
> +{
> +memory_region_transaction_begin();
> +memory_region_set_address(>fsi.opb2fsi, addr);
> +memory_region_transaction_commit();
> +}
> +
> +static uint64_t opb_unimplemented_read(void *opaque, hwaddr addr, unsigned 
> size)
> +{
> +qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx " size=%d\n",
> +  __func__, addr, size);
> +
> +return 0;
> +}
> +
> +static void opb_unimplemented_write(void *opaque, hwaddr 

[PATCH v1 5/7] hw/fsi: IBM's On-chip Peripheral Bus

2023-08-25 Thread Ninad Palsule
This is a part of patchset where IBM's Flexible Service Interface is
introduced.

The On-Chip Peripheral Bus (OPB): A low-speed bus typically found in
POWER processors. This now makes an appearance in the ASPEED SoC due
to tight integration of the FSI master IP with the OPB, mainly the
existence of an MMIO-mapping of the CFAM address straight onto a
sub-region of the OPB address space.

Signed-off-by: Andrew Jeffery 
Signed-off-by: Cédric Le Goater 
Signed-off-by: Ninad Palsule 
---
 hw/fsi/Kconfig   |   4 +
 hw/fsi/fsi-master.c  |   3 +-
 hw/fsi/meson.build   |   1 +
 hw/fsi/opb.c | 194 +++
 include/hw/fsi/opb.h |  45 ++
 5 files changed, 245 insertions(+), 2 deletions(-)
 create mode 100644 hw/fsi/opb.c
 create mode 100644 include/hw/fsi/opb.h

diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
index 087980be22..560ce536db 100644
--- a/hw/fsi/Kconfig
+++ b/hw/fsi/Kconfig
@@ -1,3 +1,7 @@
+config OPB
+bool
+select CFAM
+
 config CFAM
 bool
 select FSI
diff --git a/hw/fsi/fsi-master.c b/hw/fsi/fsi-master.c
index fe1693539a..ba00e2bb7d 100644
--- a/hw/fsi/fsi-master.c
+++ b/hw/fsi/fsi-master.c
@@ -13,8 +13,7 @@
 
 #include "hw/fsi/bits.h"
 #include "hw/fsi/fsi-master.h"
-
-#define TYPE_OP_BUS "opb"
+#include "hw/fsi/opb.h"
 
 #define TO_REG(x)   ((x) >> 2)
 
diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
index ca80d11cb9..cab645f4ea 100644
--- a/hw/fsi/meson.build
+++ b/hw/fsi/meson.build
@@ -2,3 +2,4 @@ system_ss.add(when: 'CONFIG_LBUS', if_true: files('lbus.c'))
 system_ss.add(when: 'CONFIG_SCRATCHPAD', if_true: files('engine-scratchpad.c'))
 system_ss.add(when: 'CONFIG_CFAM', if_true: files('cfam.c'))
 system_ss.add(when: 'CONFIG_FSI', if_true: 
files('fsi.c','fsi-master.c','fsi-slave.c'))
+system_ss.add(when: 'CONFIG_OPB', if_true: files('opb.c'))
diff --git a/hw/fsi/opb.c b/hw/fsi/opb.c
new file mode 100644
index 00..ac7693c001
--- /dev/null
+++ b/hw/fsi/opb.c
@@ -0,0 +1,194 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM On-chip Peripheral Bus
+ */
+
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+#include "qemu/log.h"
+
+#include "hw/fsi/opb.h"
+
+static MemTxResult opb_read(OPBus *opb, hwaddr addr, void *data, size_t len)
+{
+return address_space_read(>as, addr, MEMTXATTRS_UNSPECIFIED, data,
+  len);
+}
+
+uint8_t opb_read8(OPBus *opb, hwaddr addr)
+{
+MemTxResult tx;
+uint8_t data;
+
+tx = opb_read(opb, addr, , sizeof(data));
+/* FIXME: improve error handling */
+assert(!tx);
+
+return data;
+}
+
+uint16_t opb_read16(OPBus *opb, hwaddr addr)
+{
+MemTxResult tx;
+uint16_t data;
+
+tx = opb_read(opb, addr, , sizeof(data));
+/* FIXME: improve error handling */
+assert(!tx);
+
+return data;
+}
+
+uint32_t opb_read32(OPBus *opb, hwaddr addr)
+{
+MemTxResult tx;
+uint32_t data;
+
+tx = opb_read(opb, addr, , sizeof(data));
+/* FIXME: improve error handling */
+assert(!tx);
+
+return data;
+}
+
+static MemTxResult opb_write(OPBus *opb, hwaddr addr, void *data, size_t len)
+{
+return address_space_write(>as, addr, MEMTXATTRS_UNSPECIFIED, data,
+   len);
+}
+
+void opb_write8(OPBus *opb, hwaddr addr, uint8_t data)
+{
+MemTxResult tx;
+
+tx = opb_write(opb, addr, , sizeof(data));
+/* FIXME: improve error handling */
+assert(!tx);
+}
+
+void opb_write16(OPBus *opb, hwaddr addr, uint16_t data)
+{
+MemTxResult tx;
+
+tx = opb_write(opb, addr, , sizeof(data));
+/* FIXME: improve error handling */
+assert(!tx);
+}
+
+void opb_write32(OPBus *opb, hwaddr addr, uint32_t data)
+{
+MemTxResult tx;
+
+tx = opb_write(opb, addr, , sizeof(data));
+/* FIXME: improve error handling */
+assert(!tx);
+}
+
+void opb_fsi_master_address(OPBus *opb, hwaddr addr)
+{
+memory_region_transaction_begin();
+memory_region_set_address(>fsi.iomem, addr);
+memory_region_transaction_commit();
+}
+
+void opb_opb2fsi_address(OPBus *opb, hwaddr addr)
+{
+memory_region_transaction_begin();
+memory_region_set_address(>fsi.opb2fsi, addr);
+memory_region_transaction_commit();
+}
+
+static uint64_t opb_unimplemented_read(void *opaque, hwaddr addr, unsigned 
size)
+{
+qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx " size=%d\n",
+  __func__, addr, size);
+
+return 0;
+}
+
+static void opb_unimplemented_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned size)
+{
+qemu_log_mask(LOG_UNIMP, "%s: write @0x%" HWADDR_PRIx " size=%d "
+  "value=%"PRIx64"\n", __func__, addr, size, data);
+}
+
+static const struct MemoryRegionOps opb_unimplemented_ops = {
+.read = opb_unimplemented_read,
+.write = opb_unimplemented_write,
+.endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void