* [PATCH v1 1/7] hw/fsi: Introduce IBM's Local bus
2023-08-25 20:30 [PATCH v1 0/7] Introduce model for IBM's FSP Ninad Palsule
@ 2023-08-25 20:30 ` Ninad Palsule
2023-08-28 8:34 ` Joel Stanley
2023-08-25 20:30 ` [PATCH v1 2/7] hw/fsi: Introduce IBM's scratchpad Ninad Palsule
` (7 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Ninad Palsule @ 2023-08-25 20:30 UTC (permalink / raw)
To: qemu-devel, clg, peter.maydell, andrew, joel, pbonzini,
marcandre.lureau, berrange, thuth, philmd
Cc: Ninad Palsule, qemu-arm
This is a part of patchset where IBM's Flexible Service Interface is
introduced.
The LBUS is modelled to maintain the qdev bus hierarchy and to take
advantage of the object model to automatically generate the CFAM
configuration block. The configuration block presents engines in the
order they are attached to the CFAM's LBUS. Engine implementations
should subclass the LBusDevice and set the 'config' member of
LBusDeviceClass to match the engine's type.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
---
hw/Kconfig | 1 +
hw/fsi/Kconfig | 2 +
hw/fsi/lbus.c | 94 +++++++++++++++++++++++++++++++++++++++++++
hw/fsi/meson.build | 1 +
hw/meson.build | 1 +
include/hw/fsi/bits.h | 15 +++++++
include/hw/fsi/lbus.h | 57 ++++++++++++++++++++++++++
7 files changed, 171 insertions(+)
create mode 100644 hw/fsi/Kconfig
create mode 100644 hw/fsi/lbus.c
create mode 100644 hw/fsi/meson.build
create mode 100644 include/hw/fsi/bits.h
create mode 100644 include/hw/fsi/lbus.h
diff --git a/hw/Kconfig b/hw/Kconfig
index ba62ff6417..2ccb73add5 100644
--- a/hw/Kconfig
+++ b/hw/Kconfig
@@ -9,6 +9,7 @@ source core/Kconfig
source cxl/Kconfig
source display/Kconfig
source dma/Kconfig
+source fsi/Kconfig
source gpio/Kconfig
source hyperv/Kconfig
source i2c/Kconfig
diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
new file mode 100644
index 0000000000..687449e14e
--- /dev/null
+++ b/hw/fsi/Kconfig
@@ -0,0 +1,2 @@
+config LBUS
+ bool
diff --git a/hw/fsi/lbus.c b/hw/fsi/lbus.c
new file mode 100644
index 0000000000..afb26ef7ea
--- /dev/null
+++ b/hw/fsi/lbus.c
@@ -0,0 +1,94 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Local bus where FSI slaves are connected
+ */
+
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+#include "qemu/log.h"
+
+#include "hw/fsi/lbus.h"
+
+#include "hw/qdev-properties.h"
+
+static void lbus_realize(BusState *bus, Error **errp)
+{
+ LBusNode *node;
+ LBus *lbus = LBUS(bus);
+
+ memory_region_init(&lbus->mr, OBJECT(lbus), TYPE_LBUS,
+ (2 * 1024 * 1024) - 0x400);
+
+ QLIST_FOREACH(node, &lbus->devices, next) {
+ memory_region_add_subregion(&lbus->mr, node->ldev->address,
+ &node->ldev->iomem);
+ }
+}
+
+static void lbus_init(Object *o)
+{
+}
+
+static void lbus_class_init(ObjectClass *klass, void *data)
+{
+ BusClass *k = BUS_CLASS(klass);
+ k->realize = lbus_realize;
+}
+
+static const TypeInfo lbus_info = {
+ .name = TYPE_LBUS,
+ .parent = TYPE_BUS,
+ .instance_init = lbus_init,
+ .instance_size = sizeof(LBus),
+ .class_init = lbus_class_init,
+};
+
+static Property lbus_device_props[] = {
+ DEFINE_PROP_UINT32("address", LBusDevice, address, 0),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+DeviceState *lbus_create_device(LBus *bus, const char *type, uint32_t addr)
+{
+ DeviceState *dev;
+ LBusNode *node;
+
+ dev = qdev_new(type);
+ qdev_prop_set_uint8(dev, "address", addr);
+ qdev_realize_and_unref(dev, &bus->bus, &error_fatal);
+
+ /* Move to post_load */
+ node = g_malloc(sizeof(struct LBusNode));
+ node->ldev = LBUS_DEVICE(dev);
+ QLIST_INSERT_HEAD(&bus->devices, node, next);
+
+ return dev;
+}
+
+static void lbus_device_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->bus_type = TYPE_LBUS;
+ device_class_set_props(dc, lbus_device_props);
+}
+
+static const TypeInfo lbus_device_type_info = {
+ .name = TYPE_LBUS_DEVICE,
+ .parent = TYPE_DEVICE,
+ .instance_size = sizeof(LBusDevice),
+ .abstract = true,
+ .class_init = lbus_device_class_init,
+ .class_size = sizeof(LBusDeviceClass),
+};
+
+static void lbus_register_types(void)
+{
+ type_register_static(&lbus_info);
+ type_register_static(&lbus_device_type_info);
+}
+
+type_init(lbus_register_types);
diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
new file mode 100644
index 0000000000..e1007d5fea
--- /dev/null
+++ b/hw/fsi/meson.build
@@ -0,0 +1 @@
+system_ss.add(when: 'CONFIG_LBUS', if_true: files('lbus.c'))
diff --git a/hw/meson.build b/hw/meson.build
index c7ac7d3d75..6c71ee9cfa 100644
--- a/hw/meson.build
+++ b/hw/meson.build
@@ -43,6 +43,7 @@ subdir('virtio')
subdir('watchdog')
subdir('xen')
subdir('xenpv')
+subdir('fsi')
subdir('alpha')
subdir('arm')
diff --git a/include/hw/fsi/bits.h b/include/hw/fsi/bits.h
new file mode 100644
index 0000000000..338ae483cf
--- /dev/null
+++ b/include/hw/fsi/bits.h
@@ -0,0 +1,15 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * Bit operation macros
+ */
+#ifndef FSI_BITS_H
+#define FSI_BITS_H
+
+#define BE_BIT(x) BIT(31 - (x))
+#define GENMASK(t, b) \
+ (((1ULL << ((t) + 1)) - 1) & ~((1ULL << (b)) - 1))
+#define BE_GENMASK(t, b) GENMASK(BE_BIT(t), BE_BIT(b))
+
+#endif /* FSI_BITS_H */
diff --git a/include/hw/fsi/lbus.h b/include/hw/fsi/lbus.h
new file mode 100644
index 0000000000..00ed7e5250
--- /dev/null
+++ b/include/hw/fsi/lbus.h
@@ -0,0 +1,57 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Local bus and connected device structures.
+ */
+#ifndef FSI_LBUS_H
+#define FSI_LBUS_H
+
+#include "exec/memory.h"
+#include "hw/qdev-core.h"
+
+#define TYPE_LBUS_DEVICE "lbus.device"
+#define LBUS_DEVICE(obj) \
+ OBJECT_CHECK(LBusDevice, (obj), TYPE_LBUS_DEVICE)
+#define LBUS_DEVICE_CLASS(klass) \
+ OBJECT_CLASS_CHECK(LBusDeviceClass, (klass), TYPE_LBUS_DEVICE)
+#define LBUS_DEVICE_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(LBusDeviceClass, (obj), TYPE_LBUS_DEVICE)
+
+typedef struct LBusDevice {
+ DeviceState parent;
+
+ MemoryRegion iomem;
+ uint32_t address;
+} LBusDevice;
+
+typedef struct LBusDeviceClass {
+ DeviceClass parent;
+
+ uint32_t config;
+} LBusDeviceClass;
+
+typedef struct LBusNode {
+ LBusDevice *ldev;
+
+ QLIST_ENTRY(LBusNode) next;
+} LBusNode;
+
+#define TYPE_LBUS "lbus"
+#define LBUS(obj) OBJECT_CHECK(LBus, (obj), TYPE_LBUS)
+#define LBUS_CLASS(klass) \
+ OBJECT_CLASS_CHECK(LBusClass, (klass), TYPE_LBUS)
+#define LBUS_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(LBusClass, (obj), TYPE_LBUS)
+
+typedef struct LBus {
+ BusState bus;
+
+ MemoryRegion mr;
+
+ QLIST_HEAD(, LBusNode) devices;
+} LBus;
+
+DeviceState *lbus_create_device(LBus *bus, const char *type, uint32_t addr);
+int lbus_add_device(LBus *bus, LBusDevice *dev);
+#endif /* FSI_LBUS_H */
--
2.39.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v1 1/7] hw/fsi: Introduce IBM's Local bus
2023-08-25 20:30 ` [PATCH v1 1/7] hw/fsi: Introduce IBM's Local bus Ninad Palsule
@ 2023-08-28 8:34 ` Joel Stanley
2023-08-28 22:55 ` Ninad Palsule
0 siblings, 1 reply; 28+ messages in thread
From: Joel Stanley @ 2023-08-28 8:34 UTC (permalink / raw)
To: Ninad Palsule
Cc: qemu-devel, clg, peter.maydell, andrew, pbonzini,
marcandre.lureau, berrange, thuth, philmd, qemu-arm
On Fri, 25 Aug 2023 at 20:31, Ninad Palsule <ninad@linux.ibm.com> wrote:
>
> This is a part of patchset where IBM's Flexible Service Interface is
> introduced.
>
> The LBUS is modelled to maintain the qdev bus hierarchy and to take
> advantage of the object model to automatically generate the CFAM
> configuration block. The configuration block presents engines in the
> order they are attached to the CFAM's LBUS. Engine implementations
> should subclass the LBusDevice and set the 'config' member of
> LBusDeviceClass to match the engine's type.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
> ---
> hw/Kconfig | 1 +
> hw/fsi/Kconfig | 2 +
> hw/fsi/lbus.c | 94 +++++++++++++++++++++++++++++++++++++++++++
> hw/fsi/meson.build | 1 +
> hw/meson.build | 1 +
> include/hw/fsi/bits.h | 15 +++++++
> include/hw/fsi/lbus.h | 57 ++++++++++++++++++++++++++
> 7 files changed, 171 insertions(+)
> create mode 100644 hw/fsi/Kconfig
> create mode 100644 hw/fsi/lbus.c
> create mode 100644 hw/fsi/meson.build
> create mode 100644 include/hw/fsi/bits.h
> create mode 100644 include/hw/fsi/lbus.h
>
> diff --git a/hw/Kconfig b/hw/Kconfig
> index ba62ff6417..2ccb73add5 100644
> --- a/hw/Kconfig
> +++ b/hw/Kconfig
> @@ -9,6 +9,7 @@ source core/Kconfig
> source cxl/Kconfig
> source display/Kconfig
> source dma/Kconfig
> +source fsi/Kconfig
> source gpio/Kconfig
> source hyperv/Kconfig
> source i2c/Kconfig
> diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
> new file mode 100644
> index 0000000000..687449e14e
> --- /dev/null
> +++ b/hw/fsi/Kconfig
> @@ -0,0 +1,2 @@
> +config LBUS
> + bool
> diff --git a/hw/fsi/lbus.c b/hw/fsi/lbus.c
> new file mode 100644
> index 0000000000..afb26ef7ea
> --- /dev/null
> +++ b/hw/fsi/lbus.c
> @@ -0,0 +1,94 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2023 IBM Corp.
> + *
> + * IBM Local bus where FSI slaves are connected
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "qapi/error.h"
> +#include "qemu/log.h"
> +
> +#include "hw/fsi/lbus.h"
> +
> +#include "hw/qdev-properties.h"
> +
> +static void lbus_realize(BusState *bus, Error **errp)
> +{
> + LBusNode *node;
> + LBus *lbus = LBUS(bus);
> +
> + memory_region_init(&lbus->mr, OBJECT(lbus), TYPE_LBUS,
> + (2 * 1024 * 1024) - 0x400);
> +
> + QLIST_FOREACH(node, &lbus->devices, next) {
> + memory_region_add_subregion(&lbus->mr, node->ldev->address,
> + &node->ldev->iomem);
> + }
> +}
> +
> +static void lbus_init(Object *o)
> +{
> +}
> +
> +static void lbus_class_init(ObjectClass *klass, void *data)
> +{
> + BusClass *k = BUS_CLASS(klass);
> + k->realize = lbus_realize;
> +}
> +
> +static const TypeInfo lbus_info = {
> + .name = TYPE_LBUS,
> + .parent = TYPE_BUS,
> + .instance_init = lbus_init,
> + .instance_size = sizeof(LBus),
> + .class_init = lbus_class_init,
> +};
> +
> +static Property lbus_device_props[] = {
> + DEFINE_PROP_UINT32("address", LBusDevice, address, 0),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +DeviceState *lbus_create_device(LBus *bus, const char *type, uint32_t addr)
> +{
> + DeviceState *dev;
> + LBusNode *node;
> +
> + dev = qdev_new(type);
> + qdev_prop_set_uint8(dev, "address", addr);
> + qdev_realize_and_unref(dev, &bus->bus, &error_fatal);
> +
> + /* Move to post_load */
> + node = g_malloc(sizeof(struct LBusNode));
> + node->ldev = LBUS_DEVICE(dev);
> + QLIST_INSERT_HEAD(&bus->devices, node, next);
> +
> + return dev;
> +}
> +
> +static void lbus_device_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->bus_type = TYPE_LBUS;
> + device_class_set_props(dc, lbus_device_props);
> +}
> +
> +static const TypeInfo lbus_device_type_info = {
> + .name = TYPE_LBUS_DEVICE,
> + .parent = TYPE_DEVICE,
> + .instance_size = sizeof(LBusDevice),
> + .abstract = true,
> + .class_init = lbus_device_class_init,
> + .class_size = sizeof(LBusDeviceClass),
> +};
> +
> +static void lbus_register_types(void)
> +{
> + type_register_static(&lbus_info);
> + type_register_static(&lbus_device_type_info);
> +}
> +
> +type_init(lbus_register_types);
> diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
> new file mode 100644
> index 0000000000..e1007d5fea
> --- /dev/null
> +++ b/hw/fsi/meson.build
> @@ -0,0 +1 @@
> +system_ss.add(when: 'CONFIG_LBUS', if_true: files('lbus.c'))
> diff --git a/hw/meson.build b/hw/meson.build
> index c7ac7d3d75..6c71ee9cfa 100644
> --- a/hw/meson.build
> +++ b/hw/meson.build
> @@ -43,6 +43,7 @@ subdir('virtio')
> subdir('watchdog')
> subdir('xen')
> subdir('xenpv')
> +subdir('fsi')
>
> subdir('alpha')
> subdir('arm')
> diff --git a/include/hw/fsi/bits.h b/include/hw/fsi/bits.h
> new file mode 100644
> index 0000000000..338ae483cf
> --- /dev/null
> +++ b/include/hw/fsi/bits.h
> @@ -0,0 +1,15 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2023 IBM Corp.
> + *
> + * Bit operation macros
> + */
> +#ifndef FSI_BITS_H
> +#define FSI_BITS_H
> +
> +#define BE_BIT(x) BIT(31 - (x))
> +#define GENMASK(t, b) \
> + (((1ULL << ((t) + 1)) - 1) & ~((1ULL << (b)) - 1))
> +#define BE_GENMASK(t, b) GENMASK(BE_BIT(t), BE_BIT(b))
These are similar to the PPC_BIT definitions. We have a copy of them
in taget/ppc/cpu.h, perhaps they could be moved into a common header.
> +
> +#endif /* FSI_BITS_H */
> diff --git a/include/hw/fsi/lbus.h b/include/hw/fsi/lbus.h
> new file mode 100644
> index 0000000000..00ed7e5250
> --- /dev/null
> +++ b/include/hw/fsi/lbus.h
> @@ -0,0 +1,57 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2023 IBM Corp.
> + *
> + * IBM Local bus and connected device structures.
> + */
> +#ifndef FSI_LBUS_H
> +#define FSI_LBUS_H
> +
> +#include "exec/memory.h"
> +#include "hw/qdev-core.h"
> +
> +#define TYPE_LBUS_DEVICE "lbus.device"
> +#define LBUS_DEVICE(obj) \
> + OBJECT_CHECK(LBusDevice, (obj), TYPE_LBUS_DEVICE)
> +#define LBUS_DEVICE_CLASS(klass) \
> + OBJECT_CLASS_CHECK(LBusDeviceClass, (klass), TYPE_LBUS_DEVICE)
> +#define LBUS_DEVICE_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(LBusDeviceClass, (obj), TYPE_LBUS_DEVICE)
I think we can use OBJECT_DECLARE_SIMPLE_TYPE here to create the macros for us.
> +
> +typedef struct LBusDevice {
> + DeviceState parent;
> +
> + MemoryRegion iomem;
> + uint32_t address;
> +} LBusDevice;
> +
> +typedef struct LBusDeviceClass {
> + DeviceClass parent;
> +
> + uint32_t config;
> +} LBusDeviceClass;
> +
> +typedef struct LBusNode {
> + LBusDevice *ldev;
> +
> + QLIST_ENTRY(LBusNode) next;
> +} LBusNode;
> +
> +#define TYPE_LBUS "lbus"
> +#define LBUS(obj) OBJECT_CHECK(LBus, (obj), TYPE_LBUS)
> +#define LBUS_CLASS(klass) \
> + OBJECT_CLASS_CHECK(LBusClass, (klass), TYPE_LBUS)
> +#define LBUS_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(LBusClass, (obj), TYPE_LBUS)
Same here.
> +
> +typedef struct LBus {
> + BusState bus;
> +
> + MemoryRegion mr;
> +
> + QLIST_HEAD(, LBusNode) devices;
> +} LBus;
> +
> +DeviceState *lbus_create_device(LBus *bus, const char *type, uint32_t addr);
> +int lbus_add_device(LBus *bus, LBusDevice *dev);
> +#endif /* FSI_LBUS_H */
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 1/7] hw/fsi: Introduce IBM's Local bus
2023-08-28 8:34 ` Joel Stanley
@ 2023-08-28 22:55 ` Ninad Palsule
0 siblings, 0 replies; 28+ messages in thread
From: Ninad Palsule @ 2023-08-28 22:55 UTC (permalink / raw)
To: Joel Stanley
Cc: qemu-devel, clg, peter.maydell, andrew, pbonzini,
marcandre.lureau, berrange, thuth, philmd, qemu-arm
Hi Joel,
On 8/28/23 23:34, Joel Stanley wrote:
> On Fri, 25 Aug 2023 at 20:31, Ninad Palsule <ninad@linux.ibm.com> wrote:
>> This is a part of patchset where IBM's Flexible Service Interface is
>> introduced.
>>
>> The LBUS is modelled to maintain the qdev bus hierarchy and to take
>> advantage of the object model to automatically generate the CFAM
>> configuration block. The configuration block presents engines in the
>> order they are attached to the CFAM's LBUS. Engine implementations
>> should subclass the LBusDevice and set the 'config' member of
>> LBusDeviceClass to match the engine's type.
>>
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
>> ---
>> hw/Kconfig | 1 +
>> hw/fsi/Kconfig | 2 +
>> hw/fsi/lbus.c | 94 +++++++++++++++++++++++++++++++++++++++++++
>> hw/fsi/meson.build | 1 +
>> hw/meson.build | 1 +
>> include/hw/fsi/bits.h | 15 +++++++
>> include/hw/fsi/lbus.h | 57 ++++++++++++++++++++++++++
>> 7 files changed, 171 insertions(+)
>> create mode 100644 hw/fsi/Kconfig
>> create mode 100644 hw/fsi/lbus.c
>> create mode 100644 hw/fsi/meson.build
>> create mode 100644 include/hw/fsi/bits.h
>> create mode 100644 include/hw/fsi/lbus.h
>>
>> diff --git a/hw/Kconfig b/hw/Kconfig
>> index ba62ff6417..2ccb73add5 100644
>> --- a/hw/Kconfig
>> +++ b/hw/Kconfig
>> @@ -9,6 +9,7 @@ source core/Kconfig
>> source cxl/Kconfig
>> source display/Kconfig
>> source dma/Kconfig
>> +source fsi/Kconfig
>> source gpio/Kconfig
>> source hyperv/Kconfig
>> source i2c/Kconfig
>> diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
>> new file mode 100644
>> index 0000000000..687449e14e
>> --- /dev/null
>> +++ b/hw/fsi/Kconfig
>> @@ -0,0 +1,2 @@
>> +config LBUS
>> + bool
>> diff --git a/hw/fsi/lbus.c b/hw/fsi/lbus.c
>> new file mode 100644
>> index 0000000000..afb26ef7ea
>> --- /dev/null
>> +++ b/hw/fsi/lbus.c
>> @@ -0,0 +1,94 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + * Copyright (C) 2023 IBM Corp.
>> + *
>> + * IBM Local bus where FSI slaves are connected
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +
>> +#include "qapi/error.h"
>> +#include "qemu/log.h"
>> +
>> +#include "hw/fsi/lbus.h"
>> +
>> +#include "hw/qdev-properties.h"
>> +
>> +static void lbus_realize(BusState *bus, Error **errp)
>> +{
>> + LBusNode *node;
>> + LBus *lbus = LBUS(bus);
>> +
>> + memory_region_init(&lbus->mr, OBJECT(lbus), TYPE_LBUS,
>> + (2 * 1024 * 1024) - 0x400);
>> +
>> + QLIST_FOREACH(node, &lbus->devices, next) {
>> + memory_region_add_subregion(&lbus->mr, node->ldev->address,
>> + &node->ldev->iomem);
>> + }
>> +}
>> +
>> +static void lbus_init(Object *o)
>> +{
>> +}
>> +
>> +static void lbus_class_init(ObjectClass *klass, void *data)
>> +{
>> + BusClass *k = BUS_CLASS(klass);
>> + k->realize = lbus_realize;
>> +}
>> +
>> +static const TypeInfo lbus_info = {
>> + .name = TYPE_LBUS,
>> + .parent = TYPE_BUS,
>> + .instance_init = lbus_init,
>> + .instance_size = sizeof(LBus),
>> + .class_init = lbus_class_init,
>> +};
>> +
>> +static Property lbus_device_props[] = {
>> + DEFINE_PROP_UINT32("address", LBusDevice, address, 0),
>> + DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +DeviceState *lbus_create_device(LBus *bus, const char *type, uint32_t addr)
>> +{
>> + DeviceState *dev;
>> + LBusNode *node;
>> +
>> + dev = qdev_new(type);
>> + qdev_prop_set_uint8(dev, "address", addr);
>> + qdev_realize_and_unref(dev, &bus->bus, &error_fatal);
>> +
>> + /* Move to post_load */
>> + node = g_malloc(sizeof(struct LBusNode));
>> + node->ldev = LBUS_DEVICE(dev);
>> + QLIST_INSERT_HEAD(&bus->devices, node, next);
>> +
>> + return dev;
>> +}
>> +
>> +static void lbus_device_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + dc->bus_type = TYPE_LBUS;
>> + device_class_set_props(dc, lbus_device_props);
>> +}
>> +
>> +static const TypeInfo lbus_device_type_info = {
>> + .name = TYPE_LBUS_DEVICE,
>> + .parent = TYPE_DEVICE,
>> + .instance_size = sizeof(LBusDevice),
>> + .abstract = true,
>> + .class_init = lbus_device_class_init,
>> + .class_size = sizeof(LBusDeviceClass),
>> +};
>> +
>> +static void lbus_register_types(void)
>> +{
>> + type_register_static(&lbus_info);
>> + type_register_static(&lbus_device_type_info);
>> +}
>> +
>> +type_init(lbus_register_types);
>> diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
>> new file mode 100644
>> index 0000000000..e1007d5fea
>> --- /dev/null
>> +++ b/hw/fsi/meson.build
>> @@ -0,0 +1 @@
>> +system_ss.add(when: 'CONFIG_LBUS', if_true: files('lbus.c'))
>> diff --git a/hw/meson.build b/hw/meson.build
>> index c7ac7d3d75..6c71ee9cfa 100644
>> --- a/hw/meson.build
>> +++ b/hw/meson.build
>> @@ -43,6 +43,7 @@ subdir('virtio')
>> subdir('watchdog')
>> subdir('xen')
>> subdir('xenpv')
>> +subdir('fsi')
>>
>> subdir('alpha')
>> subdir('arm')
>> diff --git a/include/hw/fsi/bits.h b/include/hw/fsi/bits.h
>> new file mode 100644
>> index 0000000000..338ae483cf
>> --- /dev/null
>> +++ b/include/hw/fsi/bits.h
>> @@ -0,0 +1,15 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + * Copyright (C) 2023 IBM Corp.
>> + *
>> + * Bit operation macros
>> + */
>> +#ifndef FSI_BITS_H
>> +#define FSI_BITS_H
>> +
>> +#define BE_BIT(x) BIT(31 - (x))
>> +#define GENMASK(t, b) \
>> + (((1ULL << ((t) + 1)) - 1) & ~((1ULL << (b)) - 1))
>> +#define BE_GENMASK(t, b) GENMASK(BE_BIT(t), BE_BIT(b))
> These are similar to the PPC_BIT definitions. We have a copy of them
> in taget/ppc/cpu.h, perhaps they could be moved into a common header.
Moved it to bitopts.h and removed a bits.h file.
>
>> +
>> +#endif /* FSI_BITS_H */
>> diff --git a/include/hw/fsi/lbus.h b/include/hw/fsi/lbus.h
>> new file mode 100644
>> index 0000000000..00ed7e5250
>> --- /dev/null
>> +++ b/include/hw/fsi/lbus.h
>> @@ -0,0 +1,57 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + * Copyright (C) 2023 IBM Corp.
>> + *
>> + * IBM Local bus and connected device structures.
>> + */
>> +#ifndef FSI_LBUS_H
>> +#define FSI_LBUS_H
>> +
>> +#include "exec/memory.h"
>> +#include "hw/qdev-core.h"
>> +
>> +#define TYPE_LBUS_DEVICE "lbus.device"
>> +#define LBUS_DEVICE(obj) \
>> + OBJECT_CHECK(LBusDevice, (obj), TYPE_LBUS_DEVICE)
>> +#define LBUS_DEVICE_CLASS(klass) \
>> + OBJECT_CLASS_CHECK(LBusDeviceClass, (klass), TYPE_LBUS_DEVICE)
>> +#define LBUS_DEVICE_GET_CLASS(obj) \
>> + OBJECT_GET_CLASS(LBusDeviceClass, (obj), TYPE_LBUS_DEVICE)
> I think we can use OBJECT_DECLARE_SIMPLE_TYPE here to create the macros for us.
Changed it various places.
>
>> +
>> +typedef struct LBusDevice {
>> + DeviceState parent;
>> +
>> + MemoryRegion iomem;
>> + uint32_t address;
>> +} LBusDevice;
>> +
>> +typedef struct LBusDeviceClass {
>> + DeviceClass parent;
>> +
>> + uint32_t config;
>> +} LBusDeviceClass;
>> +
>> +typedef struct LBusNode {
>> + LBusDevice *ldev;
>> +
>> + QLIST_ENTRY(LBusNode) next;
>> +} LBusNode;
>> +
>> +#define TYPE_LBUS "lbus"
>> +#define LBUS(obj) OBJECT_CHECK(LBus, (obj), TYPE_LBUS)
>> +#define LBUS_CLASS(klass) \
>> + OBJECT_CLASS_CHECK(LBusClass, (klass), TYPE_LBUS)
>> +#define LBUS_GET_CLASS(obj) \
>> + OBJECT_GET_CLASS(LBusClass, (obj), TYPE_LBUS)
> Same here.
Fixed.
>
>> +
>> +typedef struct LBus {
>> + BusState bus;
>> +
>> + MemoryRegion mr;
>> +
>> + QLIST_HEAD(, LBusNode) devices;
>> +} LBus;
>> +
>> +DeviceState *lbus_create_device(LBus *bus, const char *type, uint32_t addr);
>> +int lbus_add_device(LBus *bus, LBusDevice *dev);
>> +#endif /* FSI_LBUS_H */
>> --
>> 2.39.2
>>
Thanks for the review
Ninad
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v1 2/7] hw/fsi: Introduce IBM's scratchpad
2023-08-25 20:30 [PATCH v1 0/7] Introduce model for IBM's FSP Ninad Palsule
2023-08-25 20:30 ` [PATCH v1 1/7] hw/fsi: Introduce IBM's Local bus Ninad Palsule
@ 2023-08-25 20:30 ` Ninad Palsule
2023-08-28 5:52 ` Thomas Huth
2023-08-25 20:30 ` [PATCH v1 3/7] hw/fsi: Introduce IBM's cfam,fsi-slave Ninad Palsule
` (6 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Ninad Palsule @ 2023-08-25 20:30 UTC (permalink / raw)
To: qemu-devel, clg, peter.maydell, andrew, joel, pbonzini,
marcandre.lureau, berrange, thuth, philmd
Cc: Ninad Palsule, qemu-arm
This is a part of patchset where IBM's Flexible Service Interface is
introduced.
The LBUS device is embeded inside the scratchpad. The scratchpad
provides a non-functional registers. There is a 1-1 relation between
scratchpad and LBUS devices. Each LBUS device has 1K memory mapped in
the LBUS.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
---
hw/fsi/Kconfig | 4 ++
hw/fsi/engine-scratchpad.c | 100 +++++++++++++++++++++++++++++
hw/fsi/meson.build | 1 +
include/hw/fsi/engine-scratchpad.h | 32 +++++++++
4 files changed, 137 insertions(+)
create mode 100644 hw/fsi/engine-scratchpad.c
create mode 100644 include/hw/fsi/engine-scratchpad.h
diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
index 687449e14e..2a9c49f2c9 100644
--- a/hw/fsi/Kconfig
+++ b/hw/fsi/Kconfig
@@ -1,2 +1,6 @@
+config SCRATCHPAD
+ bool
+ select LBUS
+
config LBUS
bool
diff --git a/hw/fsi/engine-scratchpad.c b/hw/fsi/engine-scratchpad.c
new file mode 100644
index 0000000000..15a8f8cc66
--- /dev/null
+++ b/hw/fsi/engine-scratchpad.c
@@ -0,0 +1,100 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM scratchpad engine
+ */
+
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+#include "qemu/log.h"
+
+#include "hw/fsi/engine-scratchpad.h"
+
+static uint64_t scratchpad_read(void *opaque, hwaddr addr, unsigned size)
+{
+ ScratchPad *s = SCRATCHPAD(opaque);
+
+ qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx " size=%d\n",
+ __func__, addr, size);
+
+ if (addr) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Out of bounds read: 0x%"HWADDR_PRIx" for %u\n",
+ __func__, addr, size);
+ return 0;
+ }
+
+ return s->reg;
+}
+
+static void scratchpad_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned size)
+{
+ ScratchPad *s = SCRATCHPAD(opaque);
+
+ qemu_log_mask(LOG_UNIMP, "%s: write @0x%" HWADDR_PRIx " size=%d "
+ "value=%"PRIx64"\n", __func__, addr, size, data);
+
+ if (addr) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Out of bounds write: 0x%"HWADDR_PRIx" for %u\n",
+ __func__, addr, size);
+ return;
+ }
+
+ s->reg = data;
+}
+
+static const struct MemoryRegionOps scratchpad_ops = {
+ .read = scratchpad_read,
+ .write = scratchpad_write,
+ .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void scratchpad_realize(DeviceState *dev, Error **errp)
+{
+ LBusDevice *ldev = LBUS_DEVICE(dev);
+
+ memory_region_init_io(&ldev->iomem, OBJECT(ldev), &scratchpad_ops,
+ ldev, TYPE_SCRATCHPAD, 0x400);
+}
+
+static void scratchpad_reset(DeviceState *dev)
+{
+ ScratchPad *s = SCRATCHPAD(dev);
+
+ s->reg = 0;
+}
+
+static void scratchpad_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ LBusDeviceClass *ldc = LBUS_DEVICE_CLASS(klass);
+
+ dc->realize = scratchpad_realize;
+ dc->reset = scratchpad_reset;
+
+ ldc->config =
+ ENGINE_CONFIG_NEXT /* valid */
+ | 0x00010000 /* slots */
+ | 0x00001000 /* version */
+ | ENGINE_CONFIG_TYPE_SCRATCHPAD /* type */
+ | 0x00000007; /* crc */
+}
+
+static const TypeInfo scratchpad_info = {
+ .name = TYPE_SCRATCHPAD,
+ .parent = TYPE_LBUS_DEVICE,
+ .instance_size = sizeof(ScratchPad),
+ .class_init = scratchpad_class_init,
+ .class_size = sizeof(LBusDeviceClass),
+};
+
+static void scratchpad_register_types(void)
+{
+ type_register_static(&scratchpad_info);
+}
+
+type_init(scratchpad_register_types);
diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
index e1007d5fea..f90e09ddab 100644
--- a/hw/fsi/meson.build
+++ b/hw/fsi/meson.build
@@ -1 +1,2 @@
system_ss.add(when: 'CONFIG_LBUS', if_true: files('lbus.c'))
+system_ss.add(when: 'CONFIG_SCRATCHPAD', if_true: files('engine-scratchpad.c'))
diff --git a/include/hw/fsi/engine-scratchpad.h b/include/hw/fsi/engine-scratchpad.h
new file mode 100644
index 0000000000..c2c8aa0b7e
--- /dev/null
+++ b/include/hw/fsi/engine-scratchpad.h
@@ -0,0 +1,32 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM scratchpad engne
+ */
+#ifndef FSI_ENGINE_SCRATCHPAD_H
+#define FSI_ENGINE_SCRATCHPAD_H
+
+#include "hw/fsi/lbus.h"
+#include "hw/fsi/bits.h"
+
+#define ENGINE_CONFIG_NEXT BE_BIT(0)
+#define ENGINE_CONFIG_VPD BE_BIT(1)
+#define ENGINE_CONFIG_SLOTS BE_GENMASK(8, 15)
+#define ENGINE_CONFIG_VERSION BE_GENMASK(16, 19)
+#define ENGINE_CONFIG_TYPE BE_GENMASK(20, 27)
+#define ENGINE_CONFIG_TYPE_PEEK (0x02 << 4)
+#define ENGINE_CONFIG_TYPE_FSI (0x03 << 4)
+#define ENGINE_CONFIG_TYPE_SCRATCHPAD (0x06 << 4)
+#define ENGINE_CONFIG_CRC BE_GENMASK(28, 31)
+
+#define TYPE_SCRATCHPAD "scratchpad"
+#define SCRATCHPAD(obj) OBJECT_CHECK(ScratchPad, (obj), TYPE_SCRATCHPAD)
+
+typedef struct ScratchPad {
+ LBusDevice parent;
+
+ uint32_t reg;
+} ScratchPad;
+
+#endif /* FSI_ENGINE_SCRATCHPAD_H */
--
2.39.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v1 2/7] hw/fsi: Introduce IBM's scratchpad
2023-08-25 20:30 ` [PATCH v1 2/7] hw/fsi: Introduce IBM's scratchpad Ninad Palsule
@ 2023-08-28 5:52 ` Thomas Huth
2023-08-29 13:21 ` Ninad Palsule
0 siblings, 1 reply; 28+ messages in thread
From: Thomas Huth @ 2023-08-28 5:52 UTC (permalink / raw)
To: Ninad Palsule, qemu-devel, clg, peter.maydell, andrew, joel,
pbonzini, marcandre.lureau, berrange, philmd
Cc: qemu-arm
On 25/08/2023 22.30, Ninad Palsule wrote:
> This is a part of patchset where IBM's Flexible Service Interface is
> introduced.
>
> The LBUS device is embeded inside the scratchpad. The scratchpad
> provides a non-functional registers. There is a 1-1 relation between
> scratchpad and LBUS devices. Each LBUS device has 1K memory mapped in
> the LBUS.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
> ---
> hw/fsi/Kconfig | 4 ++
> hw/fsi/engine-scratchpad.c | 100 +++++++++++++++++++++++++++++
> hw/fsi/meson.build | 1 +
> include/hw/fsi/engine-scratchpad.h | 32 +++++++++
> 4 files changed, 137 insertions(+)
> create mode 100644 hw/fsi/engine-scratchpad.c
> create mode 100644 include/hw/fsi/engine-scratchpad.h
>
> diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
> index 687449e14e..2a9c49f2c9 100644
> --- a/hw/fsi/Kconfig
> +++ b/hw/fsi/Kconfig
> @@ -1,2 +1,6 @@
> +config SCRATCHPAD
> + bool
> + select LBUS
> +
> config LBUS
> bool
The config names sound pretty generic ... could you maybe name them
FSI_SCRATCHPAD and FSI_LBUS instead?
Thomas
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 2/7] hw/fsi: Introduce IBM's scratchpad
2023-08-28 5:52 ` Thomas Huth
@ 2023-08-29 13:21 ` Ninad Palsule
0 siblings, 0 replies; 28+ messages in thread
From: Ninad Palsule @ 2023-08-29 13:21 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, clg, peter.maydell, andrew, joel,
pbonzini, marcandre.lureau, berrange, philmd
Cc: qemu-arm
Hello Thomas,
On 8/28/23 20:52, Thomas Huth wrote:
> On 25/08/2023 22.30, Ninad Palsule wrote:
>> This is a part of patchset where IBM's Flexible Service Interface is
>> introduced.
>>
>> The LBUS device is embeded inside the scratchpad. The scratchpad
>> provides a non-functional registers. There is a 1-1 relation between
>> scratchpad and LBUS devices. Each LBUS device has 1K memory mapped in
>> the LBUS.
>>
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
>> ---
>> hw/fsi/Kconfig | 4 ++
>> hw/fsi/engine-scratchpad.c | 100 +++++++++++++++++++++++++++++
>> hw/fsi/meson.build | 1 +
>> include/hw/fsi/engine-scratchpad.h | 32 +++++++++
>> 4 files changed, 137 insertions(+)
>> create mode 100644 hw/fsi/engine-scratchpad.c
>> create mode 100644 include/hw/fsi/engine-scratchpad.h
>>
>> diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
>> index 687449e14e..2a9c49f2c9 100644
>> --- a/hw/fsi/Kconfig
>> +++ b/hw/fsi/Kconfig
>> @@ -1,2 +1,6 @@
>> +config SCRATCHPAD
>> + bool
>> + select LBUS
>> +
>> config LBUS
>> bool
>
> The config names sound pretty generic ... could you maybe name them
> FSI_SCRATCHPAD and FSI_LBUS instead?
OK, I fixed it by adding the FSI prefix.
Thank you for the review.
Ninad
>
> Thomas
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v1 3/7] hw/fsi: Introduce IBM's cfam,fsi-slave
2023-08-25 20:30 [PATCH v1 0/7] Introduce model for IBM's FSP Ninad Palsule
2023-08-25 20:30 ` [PATCH v1 1/7] hw/fsi: Introduce IBM's Local bus Ninad Palsule
2023-08-25 20:30 ` [PATCH v1 2/7] hw/fsi: Introduce IBM's scratchpad Ninad Palsule
@ 2023-08-25 20:30 ` Ninad Palsule
2023-08-28 6:03 ` Thomas Huth
2023-08-25 20:30 ` [PATCH v1 4/7] hw/fsi: Introduce IBM's FSI Ninad Palsule
` (5 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Ninad Palsule @ 2023-08-25 20:30 UTC (permalink / raw)
To: qemu-devel, clg, peter.maydell, andrew, joel, pbonzini,
marcandre.lureau, berrange, thuth, philmd
Cc: Ninad Palsule, qemu-arm
This is a part of patchset where IBM's Flexible Service Interface is
introduced.
The Common FRU Access Macro (CFAM), an address space containing
various "engines" that drive accesses on busses internal and external
to the POWER chip. Examples include the SBEFIFO and I2C masters. The
engines hang off of an internal Local Bus (LBUS) which is described
by the CFAM configuration block.
The FSI slave: The slave is the terminal point of the FSI bus for
FSI symbols addressed to it. Slaves can be cascaded off of one
another. The slave's configuration registers appear in address space
of the CFAM to which it is attached.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
---
hw/fsi/Kconfig | 9 ++
hw/fsi/cfam.c | 235 +++++++++++++++++++++++++++++++++++++
hw/fsi/fsi-slave.c | 109 +++++++++++++++++
hw/fsi/meson.build | 2 +
include/hw/fsi/cfam.h | 61 ++++++++++
include/hw/fsi/fsi-slave.h | 29 +++++
6 files changed, 445 insertions(+)
create mode 100644 hw/fsi/cfam.c
create mode 100644 hw/fsi/fsi-slave.c
create mode 100644 include/hw/fsi/cfam.h
create mode 100644 include/hw/fsi/fsi-slave.h
diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
index 2a9c49f2c9..087980be22 100644
--- a/hw/fsi/Kconfig
+++ b/hw/fsi/Kconfig
@@ -1,3 +1,12 @@
+config CFAM
+ bool
+ select FSI
+ select SCRATCHPAD
+ select LBUS
+
+config FSI
+ bool
+
config SCRATCHPAD
bool
select LBUS
diff --git a/hw/fsi/cfam.c b/hw/fsi/cfam.c
new file mode 100644
index 0000000000..19256050bd
--- /dev/null
+++ b/hw/fsi/cfam.c
@@ -0,0 +1,235 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Common FRU Access Macro
+ */
+
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+#include "qemu/log.h"
+
+#include "hw/fsi/bits.h"
+#include "hw/fsi/cfam.h"
+#include "hw/fsi/engine-scratchpad.h"
+
+#include "hw/qdev-properties.h"
+
+#define TO_REG(x) ((x) >> 2)
+
+#define CFAM_ENGINE_CONFIG TO_REG(0x04)
+
+#define CFAM_CONFIG_CHIP_ID TO_REG(0x00)
+#define CFAM_CONFIG_CHIP_ID_P9 0xc0022d15
+#define CFAM_CONFIG_CHIP_ID_BREAK 0xc0de0000
+
+static uint64_t cfam_config_read(void *opaque, hwaddr addr, unsigned size)
+{
+ CFAMConfig *config;
+ CFAMState *cfam;
+ LBusNode *node;
+ int i;
+
+ config = CFAM_CONFIG(opaque);
+ cfam = container_of(config, CFAMState, config);
+
+ qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx " size=%d\n",
+ __func__, addr, size);
+
+ assert(size == 4);
+ assert(!(addr & 3));
+
+ switch (addr) {
+ case 0x00:
+ return CFAM_CONFIG_CHIP_ID_P9;
+ case 0x04:
+ return ENGINE_CONFIG_NEXT
+ | 0x00010000 /* slots */
+ | 0x00001000 /* version */
+ | ENGINE_CONFIG_TYPE_PEEK /* type */
+ | 0x0000000c; /* crc */
+ case 0x08:
+ return ENGINE_CONFIG_NEXT
+ | 0x00010000 /* slots */
+ | 0x00005000 /* version */
+ | ENGINE_CONFIG_TYPE_FSI /* type */
+ | 0x0000000a; /* crc */
+ break;
+ default:
+ /* FIXME: Improve this */
+ i = 0xc;
+ QLIST_FOREACH(node, &cfam->lbus.devices, next) {
+ if (i == addr) {
+ return LBUS_DEVICE_GET_CLASS(node->ldev)->config;
+ }
+ i += size;
+ }
+
+ if (i == addr) {
+ return 0;
+ }
+
+ return 0xc0de0000;
+ }
+}
+
+static void cfam_config_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned size)
+{
+ CFAMConfig *s = CFAM_CONFIG(opaque);
+
+ qemu_log_mask(LOG_UNIMP, "%s: write @0x%" HWADDR_PRIx " size=%d "
+ "value=%"PRIx64"\n", __func__, addr, size, data);
+
+ assert(size == 4);
+ assert(!(addr & 3));
+
+ switch (TO_REG(addr)) {
+ case CFAM_CONFIG_CHIP_ID:
+ case CFAM_CONFIG_CHIP_ID + 4:
+ if (data == CFAM_CONFIG_CHIP_ID_BREAK) {
+ bus_cold_reset(qdev_get_parent_bus(DEVICE(s)));
+ }
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Not implemented: 0x%"
+ HWADDR_PRIx" for %u\n",
+ __func__, addr, size);
+ }
+}
+
+static const struct MemoryRegionOps cfam_config_ops = {
+ .read = cfam_config_read,
+ .write = cfam_config_write,
+ .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void cfam_config_realize(DeviceState *dev, Error **errp)
+{
+ CFAMConfig *s = CFAM_CONFIG(dev);
+
+ memory_region_init_io(&s->iomem, OBJECT(s), &cfam_config_ops, s,
+ TYPE_CFAM_CONFIG, 0x400);
+}
+
+static void cfam_config_reset(DeviceState *dev)
+{
+ /* Config is read-only */
+}
+
+static void cfam_config_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ dc->bus_type = TYPE_LBUS;
+ dc->realize = cfam_config_realize;
+ dc->reset = cfam_config_reset;
+}
+
+static const TypeInfo cfam_config_info = {
+ .name = TYPE_CFAM_CONFIG,
+ .parent = TYPE_DEVICE,
+ .instance_size = sizeof(CFAMConfig),
+ .class_init = cfam_config_class_init,
+};
+
+static uint64_t cfam_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 cfam_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 cfam_unimplemented_ops = {
+ .read = cfam_unimplemented_read,
+ .write = cfam_unimplemented_write,
+ .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void cfam_realize(DeviceState *dev, Error **errp)
+{
+ CFAMState *cfam = CFAM(dev);
+ FSISlaveState *slave = FSI_SLAVE(dev);
+ Error *err = NULL;
+
+ /* Each slave has a 2MiB address space */
+ memory_region_init_io(&cfam->mr, OBJECT(cfam), &cfam_unimplemented_ops,
+ cfam, TYPE_CFAM, 2 * 1024 * 1024);
+ address_space_init(&cfam->as, &cfam->mr, TYPE_CFAM);
+
+ qbus_init(&cfam->lbus, sizeof(cfam->lbus), TYPE_LBUS,
+ DEVICE(cfam), NULL);
+
+ lbus_create_device(&cfam->lbus, TYPE_SCRATCHPAD, 0);
+
+ object_property_set_bool(OBJECT(&cfam->config), "realized", true, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+ qdev_set_parent_bus(DEVICE(&cfam->config), BUS(&cfam->lbus), &error_abort);
+
+ object_property_set_bool(OBJECT(&cfam->lbus), "realized", true, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ memory_region_add_subregion(&cfam->mr, 0, &cfam->config.iomem);
+ /* memory_region_add_subregion(&cfam->mr, 0x800, &cfam->lbus.peek.iomem); */
+ memory_region_add_subregion(&cfam->mr, 0x800, &slave->iomem);
+ memory_region_add_subregion(&cfam->mr, 0xc00, &cfam->lbus.mr);
+}
+
+static void cfam_reset(DeviceState *dev)
+{
+ /* TODO: Figure out how inherited reset works */
+}
+
+static void cfam_init(Object *o)
+{
+ CFAMState *s = CFAM(o);
+
+ object_initialize_child(o, TYPE_CFAM_CONFIG, &s->config, TYPE_CFAM_CONFIG);
+}
+
+static void cfam_finalize(Object *o)
+{
+ CFAMState *s = CFAM(o);
+
+ address_space_destroy(&s->as);
+}
+
+static void cfam_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ dc->bus_type = TYPE_FSI_BUS;
+ dc->realize = cfam_realize;
+ dc->reset = cfam_reset;
+}
+
+static const TypeInfo cfam_info = {
+ .name = TYPE_CFAM,
+ .parent = TYPE_FSI_SLAVE,
+ .instance_init = cfam_init,
+ .instance_finalize = cfam_finalize,
+ .instance_size = sizeof(CFAMState),
+ .class_init = cfam_class_init,
+};
+
+static void cfam_register_types(void)
+{
+ type_register_static(&cfam_config_info);
+ type_register_static(&cfam_info);
+}
+
+type_init(cfam_register_types);
diff --git a/hw/fsi/fsi-slave.c b/hw/fsi/fsi-slave.c
new file mode 100644
index 0000000000..612e587a47
--- /dev/null
+++ b/hw/fsi/fsi-slave.c
@@ -0,0 +1,109 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Flexible Service Interface slave
+ */
+
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+#include "qemu/log.h"
+
+#include "hw/fsi/bits.h"
+#include "hw/fsi/fsi-slave.h"
+
+#define TO_REG(x) ((x) >> 2)
+
+#define FSI_SMODE TO_REG(0x00)
+#define FSI_SMODE_WSTART BE_BIT(0)
+#define FSI_SMODE_AUX_EN BE_BIT(1)
+#define FSI_SMODE_SLAVE_ID BE_GENMASK(6, 7)
+#define FSI_SMODE_ECHO_DELAY BE_GENMASK(8, 11)
+#define FSI_SMODE_SEND_DELAY BE_GENMASK(12, 15)
+#define FSI_SMODE_LBUS_DIV BE_GENMASK(20, 23)
+#define FSI_SMODE_BRIEF_LEFT BE_GENMASK(24, 27)
+#define FSI_SMODE_BRIEF_RIGHT BE_GENMASK(28, 31)
+
+#define FSI_SDMA TO_REG(0x04)
+#define FSI_SISC TO_REG(0x08)
+#define FSI_SCISC TO_REG(0x08)
+#define FSI_SISM TO_REG(0x0c)
+#define FSI_SISS TO_REG(0x10)
+#define FSI_SSISM TO_REG(0x10)
+#define FSI_SCISM TO_REG(0x14)
+
+static uint64_t fsi_slave_read(void *opaque, hwaddr addr, unsigned size)
+{
+ FSISlaveState *s = FSI_SLAVE(opaque);
+
+ qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx " size=%d\n",
+ __func__, addr, size);
+
+ if (addr + size > sizeof(s->regs)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Out of bounds read: 0x%"HWADDR_PRIx" for %u\n",
+ __func__, addr, size);
+ return 0;
+ }
+
+ return 0;
+}
+
+static void fsi_slave_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned size)
+{
+ FSISlaveState *s = FSI_SLAVE(opaque);
+
+ qemu_log_mask(LOG_UNIMP, "%s: write @0x%" HWADDR_PRIx " size=%d "
+ "value=%"PRIx64"\n", __func__, addr, size, data);
+
+ if (addr + size > sizeof(s->regs)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Out of bounds write: 0x%"HWADDR_PRIx" for %u\n",
+ __func__, addr, size);
+ return;
+ }
+
+ s->regs[TO_REG(addr)] = data;
+}
+
+static const struct MemoryRegionOps fsi_slave_ops = {
+ .read = fsi_slave_read,
+ .write = fsi_slave_write,
+ .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void fsi_slave_init(Object *o)
+{
+ FSISlaveState *s = FSI_SLAVE(o);
+
+ memory_region_init_io(&s->iomem, OBJECT(s), &fsi_slave_ops,
+ s, TYPE_FSI_SLAVE, 0x400);
+}
+
+static void fsi_slave_reset(DeviceState *dev)
+{
+ /* FIXME */
+}
+
+static void fsi_slave_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ dc->reset = fsi_slave_reset;
+}
+
+static const TypeInfo fsi_slave_info = {
+ .name = TYPE_FSI_SLAVE,
+ .parent = TYPE_DEVICE,
+ .instance_init = fsi_slave_init,
+ .instance_size = sizeof(FSISlaveState),
+ .class_init = fsi_slave_class_init,
+};
+
+static void fsi_slave_register_types(void)
+{
+ type_register_static(&fsi_slave_info);
+}
+
+type_init(fsi_slave_register_types);
diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
index f90e09ddab..5600502b33 100644
--- a/hw/fsi/meson.build
+++ b/hw/fsi/meson.build
@@ -1,2 +1,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-slave.c'))
diff --git a/include/hw/fsi/cfam.h b/include/hw/fsi/cfam.h
new file mode 100644
index 0000000000..af9f88cb22
--- /dev/null
+++ b/include/hw/fsi/cfam.h
@@ -0,0 +1,61 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Common FRU Access Macro
+ */
+#ifndef FSI_CFAM_H
+#define FSI_CFAM_H
+
+#include "exec/memory.h"
+
+#include "hw/fsi/fsi-slave.h"
+#include "hw/fsi/lbus.h"
+
+#define TYPE_FSI_BUS "fsi.bus"
+
+#define TYPE_CFAM "cfam"
+#define CFAM(obj) OBJECT_CHECK(CFAMState, (obj), TYPE_CFAM)
+
+#define CFAM_NR_REGS ((0x2e0 >> 2) + 1)
+
+#define TYPE_CFAM_CONFIG "cfam.config"
+#define CFAM_CONFIG(obj) \
+ OBJECT_CHECK(CFAMConfig, (obj), TYPE_CFAM_CONFIG)
+/* P9-ism */
+#define CFAM_CONFIG_NR_REGS 0x28
+
+typedef struct CFAMState CFAMState;
+
+/* TODO: Generalise this accommodate different CFAM configurations */
+typedef struct CFAMConfig {
+ DeviceState parent;
+
+ MemoryRegion iomem;
+} CFAMConfig;
+
+#define TYPE_CFAM_PEEK "cfam.peek"
+#define CFAM_PEEK(obj) \
+ OBJECT_CHECK(CFAMPeek, (obj), TYPE_CFAM_PEEK)
+#define CFAM_PEEK_NR_REGS ((0x130 >> 2) + 1)
+
+typedef struct CFAMPeek {
+ DeviceState parent;
+
+ MemoryRegion iomem;
+} CFAMPeek;
+
+struct CFAMState {
+ /* < private > */
+ FSISlaveState parent;
+
+ MemoryRegion mr;
+ AddressSpace as;
+
+ CFAMConfig config;
+ CFAMPeek peek;
+
+ LBus lbus;
+};
+
+#endif /* FSI_CFAM_H */
diff --git a/include/hw/fsi/fsi-slave.h b/include/hw/fsi/fsi-slave.h
new file mode 100644
index 0000000000..bff807ff20
--- /dev/null
+++ b/include/hw/fsi/fsi-slave.h
@@ -0,0 +1,29 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Flexible Service Interface slave
+ */
+#ifndef FSI_FSI_SLAVE_H
+#define FSI_FSI_SLAVE_H
+
+#include "exec/memory.h"
+#include "hw/qdev-core.h"
+
+#include "hw/fsi/lbus.h"
+
+#include <stdint.h>
+
+#define TYPE_FSI_SLAVE "fsi.slave"
+#define FSI_SLAVE(obj) \
+ OBJECT_CHECK(FSISlaveState, (obj), TYPE_FSI_SLAVE)
+#define FSI_SLAVE_CONTROL_NR_REGS ((0x40 >> 2) + 1)
+
+typedef struct FSISlaveState {
+ DeviceState parent;
+
+ MemoryRegion iomem;
+ uint32_t regs[FSI_SLAVE_CONTROL_NR_REGS];
+} FSISlaveState;
+
+#endif /* FSI_FSI_H */
--
2.39.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v1 3/7] hw/fsi: Introduce IBM's cfam,fsi-slave
2023-08-25 20:30 ` [PATCH v1 3/7] hw/fsi: Introduce IBM's cfam,fsi-slave Ninad Palsule
@ 2023-08-28 6:03 ` Thomas Huth
2023-08-29 13:39 ` Ninad Palsule
0 siblings, 1 reply; 28+ messages in thread
From: Thomas Huth @ 2023-08-28 6:03 UTC (permalink / raw)
To: Ninad Palsule, qemu-devel, clg, peter.maydell, andrew, joel,
pbonzini, marcandre.lureau, berrange, philmd
Cc: qemu-arm
On 25/08/2023 22.30, Ninad Palsule wrote:
> This is a part of patchset where IBM's Flexible Service Interface is
> introduced.
>
> The Common FRU Access Macro (CFAM), an address space containing
> various "engines" that drive accesses on busses internal and external
> to the POWER chip. Examples include the SBEFIFO and I2C masters. The
> engines hang off of an internal Local Bus (LBUS) which is described
> by the CFAM configuration block.
>
> The FSI slave: The slave is the terminal point of the FSI bus for
> FSI symbols addressed to it. Slaves can be cascaded off of one
> another. The slave's configuration registers appear in address space
> of the CFAM to which it is attached.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
> ---
...
> diff --git a/hw/fsi/cfam.c b/hw/fsi/cfam.c
> new file mode 100644
> index 0000000000..19256050bd
> --- /dev/null
> +++ b/hw/fsi/cfam.c
> @@ -0,0 +1,235 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2023 IBM Corp.
> + *
> + * IBM Common FRU Access Macro
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "qapi/error.h"
> +#include "qemu/log.h"
> +
> +#include "hw/fsi/bits.h"
> +#include "hw/fsi/cfam.h"
> +#include "hw/fsi/engine-scratchpad.h"
> +
> +#include "hw/qdev-properties.h"
> +
> +#define TO_REG(x) ((x) >> 2)
> +
> +#define CFAM_ENGINE_CONFIG TO_REG(0x04)
> +
> +#define CFAM_CONFIG_CHIP_ID TO_REG(0x00)
> +#define CFAM_CONFIG_CHIP_ID_P9 0xc0022d15
> +#define CFAM_CONFIG_CHIP_ID_BREAK 0xc0de0000
> +
> +static uint64_t cfam_config_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + CFAMConfig *config;
> + CFAMState *cfam;
> + LBusNode *node;
> + int i;
> +
> + config = CFAM_CONFIG(opaque);
> + cfam = container_of(config, CFAMState, config);
> +
> + qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx " size=%d\n",
> + __func__, addr, size);
> +
> + assert(size == 4);
> + assert(!(addr & 3));
> +
> + switch (addr) {
> + case 0x00:
> + return CFAM_CONFIG_CHIP_ID_P9;
> + case 0x04:
> + return ENGINE_CONFIG_NEXT
> + | 0x00010000 /* slots */
> + | 0x00001000 /* version */
> + | ENGINE_CONFIG_TYPE_PEEK /* type */
> + | 0x0000000c; /* crc */
> + case 0x08:
> + return ENGINE_CONFIG_NEXT
> + | 0x00010000 /* slots */
> + | 0x00005000 /* version */
> + | ENGINE_CONFIG_TYPE_FSI /* type */
> + | 0x0000000a; /* crc */
> + break;
> + default:
> + /* FIXME: Improve this */
> + i = 0xc;
> + QLIST_FOREACH(node, &cfam->lbus.devices, next) {
> + if (i == addr) {
> + return LBUS_DEVICE_GET_CLASS(node->ldev)->config;
> + }
> + i += size;
> + }
> +
> + if (i == addr) {
> + return 0;
> + }
> +
> + return 0xc0de0000;
Can you explain the magic number at least with a comment?
Maybe it would also make sense to add a qemu_log_mask(LOG_GUEST_ERROR, ...)
or qemu_log_mask(LOG_UNIMP, ...) statement here?
Thomas
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 3/7] hw/fsi: Introduce IBM's cfam,fsi-slave
2023-08-28 6:03 ` Thomas Huth
@ 2023-08-29 13:39 ` Ninad Palsule
2023-08-29 13:43 ` Cédric Le Goater
0 siblings, 1 reply; 28+ messages in thread
From: Ninad Palsule @ 2023-08-29 13:39 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, clg, peter.maydell, andrew, joel,
pbonzini, marcandre.lureau, berrange, philmd
Cc: qemu-arm
Hello Thomas,
On 8/28/23 21:03, Thomas Huth wrote:
> On 25/08/2023 22.30, Ninad Palsule wrote:
>> This is a part of patchset where IBM's Flexible Service Interface is
>> introduced.
>>
>> The Common FRU Access Macro (CFAM), an address space containing
>> various "engines" that drive accesses on busses internal and external
>> to the POWER chip. Examples include the SBEFIFO and I2C masters. The
>> engines hang off of an internal Local Bus (LBUS) which is described
>> by the CFAM configuration block.
>>
>> The FSI slave: The slave is the terminal point of the FSI bus for
>> FSI symbols addressed to it. Slaves can be cascaded off of one
>> another. The slave's configuration registers appear in address space
>> of the CFAM to which it is attached.
>>
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
>> ---
> ...
>> diff --git a/hw/fsi/cfam.c b/hw/fsi/cfam.c
>> new file mode 100644
>> index 0000000000..19256050bd
>> --- /dev/null
>> +++ b/hw/fsi/cfam.c
>> @@ -0,0 +1,235 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + * Copyright (C) 2023 IBM Corp.
>> + *
>> + * IBM Common FRU Access Macro
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +
>> +#include "qapi/error.h"
>> +#include "qemu/log.h"
>> +
>> +#include "hw/fsi/bits.h"
>> +#include "hw/fsi/cfam.h"
>> +#include "hw/fsi/engine-scratchpad.h"
>> +
>> +#include "hw/qdev-properties.h"
>> +
>> +#define TO_REG(x) ((x) >> 2)
>> +
>> +#define CFAM_ENGINE_CONFIG TO_REG(0x04)
>> +
>> +#define CFAM_CONFIG_CHIP_ID TO_REG(0x00)
>> +#define CFAM_CONFIG_CHIP_ID_P9 0xc0022d15
>> +#define CFAM_CONFIG_CHIP_ID_BREAK 0xc0de0000
>> +
>> +static uint64_t cfam_config_read(void *opaque, hwaddr addr, unsigned
>> size)
>> +{
>> + CFAMConfig *config;
>> + CFAMState *cfam;
>> + LBusNode *node;
>> + int i;
>> +
>> + config = CFAM_CONFIG(opaque);
>> + cfam = container_of(config, CFAMState, config);
>> +
>> + qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx " size=%d\n",
>> + __func__, addr, size);
>> +
>> + assert(size == 4);
>> + assert(!(addr & 3));
>> +
>> + switch (addr) {
>> + case 0x00:
>> + return CFAM_CONFIG_CHIP_ID_P9;
>> + case 0x04:
>> + return ENGINE_CONFIG_NEXT
>> + | 0x00010000 /* slots */
>> + | 0x00001000 /* version */
>> + | ENGINE_CONFIG_TYPE_PEEK /* type */
>> + | 0x0000000c; /* crc */
>> + case 0x08:
>> + return ENGINE_CONFIG_NEXT
>> + | 0x00010000 /* slots */
>> + | 0x00005000 /* version */
>> + | ENGINE_CONFIG_TYPE_FSI /* type */
>> + | 0x0000000a; /* crc */
>> + break;
>> + default:
>> + /* FIXME: Improve this */
>> + i = 0xc;
>> + QLIST_FOREACH(node, &cfam->lbus.devices, next) {
>> + if (i == addr) {
>> + return LBUS_DEVICE_GET_CLASS(node->ldev)->config;
>> + }
>> + i += size;
>> + }
>> +
>> + if (i == addr) {
>> + return 0;
>> + }
>> +
>> + return 0xc0de0000;
>
> Can you explain the magic number at least with a comment?
Added comment for the magic number 0xc0de0000
>
> Maybe it would also make sense to add a qemu_log_mask(LOG_GUEST_ERROR,
> ...) or qemu_log_mask(LOG_UNIMP, ...) statement here?
There is LOG_UNIMP most of the function. I added it in the reset function.
>
> Thomas
>
Thank you for the review.
Ninad
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 3/7] hw/fsi: Introduce IBM's cfam,fsi-slave
2023-08-29 13:39 ` Ninad Palsule
@ 2023-08-29 13:43 ` Cédric Le Goater
2023-08-30 2:31 ` Ninad Palsule
0 siblings, 1 reply; 28+ messages in thread
From: Cédric Le Goater @ 2023-08-29 13:43 UTC (permalink / raw)
To: Ninad Palsule, Thomas Huth, qemu-devel, peter.maydell, andrew,
joel, pbonzini, marcandre.lureau, berrange, philmd
Cc: qemu-arm
On 8/29/23 15:39, Ninad Palsule wrote:
> Hello Thomas,
>
> On 8/28/23 21:03, Thomas Huth wrote:
>> On 25/08/2023 22.30, Ninad Palsule wrote:
>>> This is a part of patchset where IBM's Flexible Service Interface is
>>> introduced.
>>>
>>> The Common FRU Access Macro (CFAM), an address space containing
>>> various "engines" that drive accesses on busses internal and external
>>> to the POWER chip. Examples include the SBEFIFO and I2C masters. The
>>> engines hang off of an internal Local Bus (LBUS) which is described
>>> by the CFAM configuration block.
>>>
>>> The FSI slave: The slave is the terminal point of the FSI bus for
>>> FSI symbols addressed to it. Slaves can be cascaded off of one
>>> another. The slave's configuration registers appear in address space
>>> of the CFAM to which it is attached.
>>>
>>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
>>> ---
>> ...
>>> diff --git a/hw/fsi/cfam.c b/hw/fsi/cfam.c
>>> new file mode 100644
>>> index 0000000000..19256050bd
>>> --- /dev/null
>>> +++ b/hw/fsi/cfam.c
>>> @@ -0,0 +1,235 @@
>>> +/*
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + * Copyright (C) 2023 IBM Corp.
>>> + *
>>> + * IBM Common FRU Access Macro
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +
>>> +#include "qapi/error.h"
>>> +#include "qemu/log.h"
>>> +
>>> +#include "hw/fsi/bits.h"
>>> +#include "hw/fsi/cfam.h"
>>> +#include "hw/fsi/engine-scratchpad.h"
>>> +
>>> +#include "hw/qdev-properties.h"
>>> +
>>> +#define TO_REG(x) ((x) >> 2)
>>> +
>>> +#define CFAM_ENGINE_CONFIG TO_REG(0x04)
>>> +
>>> +#define CFAM_CONFIG_CHIP_ID TO_REG(0x00)
>>> +#define CFAM_CONFIG_CHIP_ID_P9 0xc0022d15
>>> +#define CFAM_CONFIG_CHIP_ID_BREAK 0xc0de0000
>>> +
>>> +static uint64_t cfam_config_read(void *opaque, hwaddr addr, unsigned size)
>>> +{
>>> + CFAMConfig *config;
>>> + CFAMState *cfam;
>>> + LBusNode *node;
>>> + int i;
>>> +
>>> + config = CFAM_CONFIG(opaque);
>>> + cfam = container_of(config, CFAMState, config);
>>> +
>>> + qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx " size=%d\n",
>>> + __func__, addr, size);
>>> +
>>> + assert(size == 4);
>>> + assert(!(addr & 3));
>>> +
>>> + switch (addr) {
>>> + case 0x00:
>>> + return CFAM_CONFIG_CHIP_ID_P9;
>>> + case 0x04:
>>> + return ENGINE_CONFIG_NEXT
>>> + | 0x00010000 /* slots */
>>> + | 0x00001000 /* version */
>>> + | ENGINE_CONFIG_TYPE_PEEK /* type */
>>> + | 0x0000000c; /* crc */
>>> + case 0x08:
>>> + return ENGINE_CONFIG_NEXT
>>> + | 0x00010000 /* slots */
>>> + | 0x00005000 /* version */
>>> + | ENGINE_CONFIG_TYPE_FSI /* type */
>>> + | 0x0000000a; /* crc */
>>> + break;
>>> + default:
>>> + /* FIXME: Improve this */
>>> + i = 0xc;
>>> + QLIST_FOREACH(node, &cfam->lbus.devices, next) {
>>> + if (i == addr) {
>>> + return LBUS_DEVICE_GET_CLASS(node->ldev)->config;
>>> + }
>>> + i += size;
>>> + }
>>> +
>>> + if (i == addr) {
>>> + return 0;
>>> + }
>>> +
>>> + return 0xc0de0000;
>>
>> Can you explain the magic number at least with a comment?
> Added comment for the magic number 0xc0de0000
>>
>> Maybe it would also make sense to add a qemu_log_mask(LOG_GUEST_ERROR, ...) or qemu_log_mask(LOG_UNIMP, ...) statement here?
> There is LOG_UNIMP most of the function. I added it in the reset function.
I took a quick look at the series and I think that all the
qemu_log_mask(LOG_UNIMP, ..) as the one above should be replaced
by trace events instead.
Thanks,
C.
>>
>> Thomas
>>
>
> Thank you for the review.
>
> Ninad
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 3/7] hw/fsi: Introduce IBM's cfam,fsi-slave
2023-08-29 13:43 ` Cédric Le Goater
@ 2023-08-30 2:31 ` Ninad Palsule
0 siblings, 0 replies; 28+ messages in thread
From: Ninad Palsule @ 2023-08-30 2:31 UTC (permalink / raw)
To: Cédric Le Goater, Thomas Huth, qemu-devel, peter.maydell,
andrew, joel, pbonzini, marcandre.lureau, berrange, philmd
Cc: qemu-arm
Hello Cedric,
On 8/29/23 08:43, Cédric Le Goater wrote:
> On 8/29/23 15:39, Ninad Palsule wrote:
>> Hello Thomas,
>>
>> On 8/28/23 21:03, Thomas Huth wrote:
>>> On 25/08/2023 22.30, Ninad Palsule wrote:
>>>> This is a part of patchset where IBM's Flexible Service Interface is
>>>> introduced.
>>>>
>>>> The Common FRU Access Macro (CFAM), an address space containing
>>>> various "engines" that drive accesses on busses internal and external
>>>> to the POWER chip. Examples include the SBEFIFO and I2C masters. The
>>>> engines hang off of an internal Local Bus (LBUS) which is described
>>>> by the CFAM configuration block.
>>>>
>>>> The FSI slave: The slave is the terminal point of the FSI bus for
>>>> FSI symbols addressed to it. Slaves can be cascaded off of one
>>>> another. The slave's configuration registers appear in address space
>>>> of the CFAM to which it is attached.
>>>>
>>>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
>>>> ---
>>> ...
>>>> diff --git a/hw/fsi/cfam.c b/hw/fsi/cfam.c
>>>> new file mode 100644
>>>> index 0000000000..19256050bd
>>>> --- /dev/null
>>>> +++ b/hw/fsi/cfam.c
>>>> @@ -0,0 +1,235 @@
>>>> +/*
>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>> + * Copyright (C) 2023 IBM Corp.
>>>> + *
>>>> + * IBM Common FRU Access Macro
>>>> + */
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +
>>>> +#include "qapi/error.h"
>>>> +#include "qemu/log.h"
>>>> +
>>>> +#include "hw/fsi/bits.h"
>>>> +#include "hw/fsi/cfam.h"
>>>> +#include "hw/fsi/engine-scratchpad.h"
>>>> +
>>>> +#include "hw/qdev-properties.h"
>>>> +
>>>> +#define TO_REG(x) ((x) >> 2)
>>>> +
>>>> +#define CFAM_ENGINE_CONFIG TO_REG(0x04)
>>>> +
>>>> +#define CFAM_CONFIG_CHIP_ID TO_REG(0x00)
>>>> +#define CFAM_CONFIG_CHIP_ID_P9 0xc0022d15
>>>> +#define CFAM_CONFIG_CHIP_ID_BREAK 0xc0de0000
>>>> +
>>>> +static uint64_t cfam_config_read(void *opaque, hwaddr addr,
>>>> unsigned size)
>>>> +{
>>>> + CFAMConfig *config;
>>>> + CFAMState *cfam;
>>>> + LBusNode *node;
>>>> + int i;
>>>> +
>>>> + config = CFAM_CONFIG(opaque);
>>>> + cfam = container_of(config, CFAMState, config);
>>>> +
>>>> + qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx "
>>>> size=%d\n",
>>>> + __func__, addr, size);
>>>> +
>>>> + assert(size == 4);
>>>> + assert(!(addr & 3));
>>>> +
>>>> + switch (addr) {
>>>> + case 0x00:
>>>> + return CFAM_CONFIG_CHIP_ID_P9;
>>>> + case 0x04:
>>>> + return ENGINE_CONFIG_NEXT
>>>> + | 0x00010000 /* slots */
>>>> + | 0x00001000 /* version */
>>>> + | ENGINE_CONFIG_TYPE_PEEK /* type */
>>>> + | 0x0000000c; /* crc */
>>>> + case 0x08:
>>>> + return ENGINE_CONFIG_NEXT
>>>> + | 0x00010000 /* slots */
>>>> + | 0x00005000 /* version */
>>>> + | ENGINE_CONFIG_TYPE_FSI /* type */
>>>> + | 0x0000000a; /* crc */
>>>> + break;
>>>> + default:
>>>> + /* FIXME: Improve this */
>>>> + i = 0xc;
>>>> + QLIST_FOREACH(node, &cfam->lbus.devices, next) {
>>>> + if (i == addr) {
>>>> + return LBUS_DEVICE_GET_CLASS(node->ldev)->config;
>>>> + }
>>>> + i += size;
>>>> + }
>>>> +
>>>> + if (i == addr) {
>>>> + return 0;
>>>> + }
>>>> +
>>>> + return 0xc0de0000;
>>>
>>> Can you explain the magic number at least with a comment?
>> Added comment for the magic number 0xc0de0000
>>>
>>> Maybe it would also make sense to add a
>>> qemu_log_mask(LOG_GUEST_ERROR, ...) or qemu_log_mask(LOG_UNIMP, ...)
>>> statement here?
>> There is LOG_UNIMP most of the function. I added it in the reset
>> function.
>
> I took a quick look at the series and I think that all the
> qemu_log_mask(LOG_UNIMP, ..) as the one above should be replaced
> by trace events instead.
I have converted 4 logs into trace.
Thank you for the review.
~Ninad
>
> Thanks,
>
> C.
>
>
>>>
>>> Thomas
>>>
>>
>> Thank you for the review.
>>
>> Ninad
>>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v1 4/7] hw/fsi: Introduce IBM's FSI
2023-08-25 20:30 [PATCH v1 0/7] Introduce model for IBM's FSP Ninad Palsule
` (2 preceding siblings ...)
2023-08-25 20:30 ` [PATCH v1 3/7] hw/fsi: Introduce IBM's cfam,fsi-slave Ninad Palsule
@ 2023-08-25 20:30 ` Ninad Palsule
2023-08-28 8:57 ` Joel Stanley
2023-08-25 20:30 ` [PATCH v1 5/7] hw/fsi: IBM's On-chip Peripheral Bus Ninad Palsule
` (4 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Ninad Palsule @ 2023-08-25 20:30 UTC (permalink / raw)
To: qemu-devel, clg, peter.maydell, andrew, joel, pbonzini,
marcandre.lureau, berrange, thuth, philmd
Cc: Ninad Palsule, qemu-arm
This is a part of patchset where IBM's Flexible Service Interface is
introduced.
This commit models the FSI bus. CFAM is hanging out of FSI bus. The bus
is model such a way that it is embeded inside the FSI master which is a
bus controller.
The FSI master: A controller in the platform service processor (e.g.
BMC) driving CFAM engine accesses into the POWER chip. At the
hardware level FSI is a bit-based protocol supporting synchronous and
DMA-driven accesses of engines in a CFAM.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
---
hw/fsi/cfam.c | 1 +
hw/fsi/fsi-master.c | 203 ++++++++++++++++++++++++++++++++++++
hw/fsi/fsi.c | 54 ++++++++++
hw/fsi/meson.build | 2 +-
include/hw/fsi/cfam.h | 2 -
include/hw/fsi/fsi-master.h | 30 ++++++
include/hw/fsi/fsi.h | 35 +++++++
7 files changed, 324 insertions(+), 3 deletions(-)
create mode 100644 hw/fsi/fsi-master.c
create mode 100644 hw/fsi/fsi.c
create mode 100644 include/hw/fsi/fsi-master.h
create mode 100644 include/hw/fsi/fsi.h
diff --git a/hw/fsi/cfam.c b/hw/fsi/cfam.c
index 19256050bd..12ce31cac4 100644
--- a/hw/fsi/cfam.c
+++ b/hw/fsi/cfam.c
@@ -12,6 +12,7 @@
#include "hw/fsi/bits.h"
#include "hw/fsi/cfam.h"
+#include "hw/fsi/fsi.h"
#include "hw/fsi/engine-scratchpad.h"
#include "hw/qdev-properties.h"
diff --git a/hw/fsi/fsi-master.c b/hw/fsi/fsi-master.c
new file mode 100644
index 0000000000..fe1693539a
--- /dev/null
+++ b/hw/fsi/fsi-master.c
@@ -0,0 +1,203 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Flexible Service Interface master
+ */
+
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+
+#include "qemu/log.h"
+
+#include "hw/fsi/bits.h"
+#include "hw/fsi/fsi-master.h"
+
+#define TYPE_OP_BUS "opb"
+
+#define TO_REG(x) ((x) >> 2)
+
+#define FSI_MMODE TO_REG(0x000)
+#define FSI_MMODE_IPOLL_DMA_EN BE_BIT(0)
+#define FSI_MMODE_HW_ERROR_RECOVERY_EN BE_BIT(1)
+#define FSI_MMODE_RELATIVE_ADDRESS_EN BE_BIT(2)
+#define FSI_MMODE_PARITY_CHECK_EN BE_BIT(3)
+#define FSI_MMODE_CLOCK_DIVIDER_0 BE_GENMASK(4, 13)
+#define FSI_MMODE_CLOCK_DIVIDER_1 BE_GENMASK(14, 23)
+#define FSI_MMODE_DEBUG_EN BE_BIT(24)
+
+#define FSI_MDELAY TO_REG(0x004)
+#define FSI_MDELAY_ECHO_0 BE_GENMASK(0, 3)
+#define FSI_MDELAY_SEND_0 BE_GENMASK(4, 7)
+#define FSI_MDELAY_ECHO_1 BE_GENMASK(8, 11)
+#define FSI_MDELAY_SEND_1 BE_GENMASK(12, 15)
+
+#define FSI_MENP0 TO_REG(0x010)
+#define FSI_MENP32 TO_REG(0x014)
+#define FSI_MSENP0 TO_REG(0x018)
+#define FSI_MLEVP0 TO_REG(0x018)
+#define FSI_MSENP32 TO_REG(0x01c)
+#define FSI_MLEVP32 TO_REG(0x01c)
+#define FSI_MCENP0 TO_REG(0x020)
+#define FSI_MREFP0 TO_REG(0x020)
+#define FSI_MCENP32 TO_REG(0x024)
+#define FSI_MREFP32 TO_REG(0x024)
+
+#define FSI_MAEB TO_REG(0x070)
+#define FSI_MAEB_ANY_CPU_ERROR BE_BIT(0)
+#define FSI_MAEB_ANY_DMA_ERROR BE_GENMASK(1, 16)
+#define FSI_MAEB_ANY_PARITY_ERROR BE_BIT(17)
+
+#define FSI_MVER TO_REG(0x074)
+#define FSI_MVER_VERSION BE_GENMASK(0, 7)
+#define FSI_MVER_BRIDGES BE_GENMASK(8, 15)
+#define FSI_MVER_PORTS BE_GENMASK(16, 23)
+
+#define FSI_MRESP0 TO_REG(0x0d0)
+#define FSI_MRESP0_RESET_PORT_GENERAL BE_BIT(0)
+#define FSI_MRESP0_RESET_PORT_ERROR BE_BIT(1)
+#define FSI_MRESP0_RESET_ALL_BRIDGES_GENERAL BE_BIT(2)
+#define FSI_MRESP0_RESET_ALL_PORTS_GENERAL BE_BIT(3)
+#define FSI_MRESP0_RESET_MASTER BE_BIT(4)
+#define FSI_MRESP0_RESET_PARITY_ERROR_LATCH BE_BIT(5)
+
+#define FSI_MRESB0 TO_REG(0x1d0)
+#define FSI_MRESB0_RESET_GENERAL BE_BIT(0)
+#define FSI_MRESB0_RESET_ERROR BE_BIT(1)
+#define FSI_MRESB0_SET_DMA_SUSPEND BE_BIT(5)
+#define FSI_MRESB0_CLEAR_DMA_SUSPEND BE_BIT(6)
+#define FSI_MRESB0_SET_DELAY_MEASURE BE_BIT(7)
+
+#define FSI_MECTRL TO_REG(0x2e0)
+#define FSI_MECTRL_TEST_PULSE BE_GENMASK(0, 7)
+#define FSI_MECTRL_INHIBIT_PARITY_ERROR BE_GENMASK(8, 15)
+#define FSI_MECTRL_ENABLE_OPB_ERR_ACK BE_BIT(16)
+#define FSI_MECTRL_AUTO_TERMINATE BE_BIT(17)
+#define FSI_MECTRL_PORT_ERROR_FREEZE BE_BIT(18)
+
+static uint64_t fsi_master_read(void *opaque, hwaddr addr, unsigned size)
+{
+ FSIMasterState *s = FSI_MASTER(opaque);
+
+ qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx " size=%d\n",
+ __func__, addr, size);
+
+ if (addr + size > sizeof(s->regs)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Out of bounds read: 0x%"HWADDR_PRIx" for %u\n",
+ __func__, addr, size);
+ return 0;
+ }
+
+ return s->regs[TO_REG(addr)];
+}
+
+static void fsi_master_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned size)
+{
+ FSIMasterState *s = FSI_MASTER(opaque);
+
+ qemu_log_mask(LOG_UNIMP, "%s: write @0x%" HWADDR_PRIx " size=%d "
+ "value=%"PRIx64"\n", __func__, addr, size, data);
+
+ if (addr + size > sizeof(s->regs)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Out of bounds write: %"HWADDR_PRIx" for %u\n",
+ __func__, addr, size);
+ return;
+ }
+
+ switch (TO_REG(addr)) {
+ case FSI_MENP0:
+ s->regs[FSI_MENP0] = data;
+ break;
+ case FSI_MENP32:
+ s->regs[FSI_MENP32] = data;
+ break;
+ case FSI_MSENP0:
+ s->regs[FSI_MENP0] |= data;
+ break;
+ case FSI_MSENP32:
+ s->regs[FSI_MENP32] |= data;
+ break;
+ case FSI_MCENP0:
+ s->regs[FSI_MENP0] &= ~data;
+ break;
+ case FSI_MCENP32:
+ s->regs[FSI_MENP32] &= ~data;
+ break;
+ case FSI_MRESP0:
+ /* Perform necessary resets leave register 0 to indicate no errors */
+ break;
+ case FSI_MRESB0:
+ if (data & FSI_MRESB0_RESET_GENERAL) {
+ device_cold_reset(DEVICE(opaque));
+ }
+ if (data & FSI_MRESB0_RESET_ERROR) {
+ /* FIXME: this seems dubious */
+ device_cold_reset(DEVICE(opaque));
+ }
+ break;
+ default:
+ s->regs[TO_REG(addr)] = data;
+ }
+}
+
+static const struct MemoryRegionOps fsi_master_ops = {
+ .read = fsi_master_read,
+ .write = fsi_master_write,
+ .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void fsi_master_realize(DeviceState *dev, Error **errp)
+{
+ FSIMasterState *s = FSI_MASTER(dev);
+ Error *err = NULL;
+
+ qbus_init(&s->bus, sizeof(s->bus), TYPE_FSI_BUS, DEVICE(s), NULL);
+
+ memory_region_init_io(&s->iomem, OBJECT(s), &fsi_master_ops, s,
+ TYPE_FSI_MASTER, 0x10000000);
+ memory_region_init(&s->opb2fsi, OBJECT(s), "fsi.opb2fsi", 0x10000000);
+
+ object_property_set_bool(OBJECT(&s->bus), "realized", true, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+
+ memory_region_add_subregion(&s->opb2fsi, 0, &s->bus.slave.mr);
+}
+
+static void fsi_master_reset(DeviceState *dev)
+{
+ FSIMasterState *s = FSI_MASTER(dev);
+
+ /* ASPEED default */
+ s->regs[FSI_MVER] = 0xe0050101;
+}
+
+static void fsi_master_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->bus_type = TYPE_OP_BUS;
+ dc->desc = "FSI Master";
+ dc->realize = fsi_master_realize;
+ dc->reset = fsi_master_reset;
+}
+
+static const TypeInfo fsi_master_info = {
+ .name = TYPE_FSI_MASTER,
+ .parent = TYPE_DEVICE,
+ .instance_size = sizeof(FSIMasterState),
+ .class_init = fsi_master_class_init,
+};
+
+static void fsi_register_types(void)
+{
+ type_register_static(&fsi_master_info);
+}
+
+type_init(fsi_register_types);
diff --git a/hw/fsi/fsi.c b/hw/fsi/fsi.c
new file mode 100644
index 0000000000..43f889fee9
--- /dev/null
+++ b/hw/fsi/fsi.c
@@ -0,0 +1,54 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Flexible Service Interface
+ */
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+
+#include "hw/fsi/fsi.h"
+#include "hw/fsi/cfam.h"
+
+static void fsi_bus_realize(BusState *bus, Error **errp)
+{
+ FSIBus *s = FSI_BUS(bus);
+ Error *err = NULL;
+
+ /* FIXME: Should be realised elsewhere and added to the bus */
+ object_property_set_bool(OBJECT(&s->slave), "realized", true, &err);
+ if (err) {
+ error_propagate(errp, err);
+ }
+}
+
+static void fsi_bus_init(Object *o)
+{
+ FSIBus *s = FSI_BUS(o);
+
+ /* FIXME: Move this elsewhere */
+ object_initialize_child(o, TYPE_CFAM, &s->slave, TYPE_CFAM);
+ qdev_set_parent_bus(DEVICE(&s->slave), BUS(o), &error_abort);
+}
+
+static void fsi_bus_class_init(ObjectClass *klass, void *data)
+{
+ BusClass *bc = BUS_CLASS(klass);
+ bc->realize = fsi_bus_realize;
+}
+
+static const TypeInfo fsi_bus_info = {
+ .name = TYPE_FSI_BUS,
+ .parent = TYPE_BUS,
+ .instance_init = fsi_bus_init,
+ .instance_size = sizeof(FSIBus),
+ .class_init = fsi_bus_class_init,
+};
+
+static void fsi_bus_register_types(void)
+{
+ type_register_static(&fsi_bus_info);
+}
+
+type_init(fsi_bus_register_types);
diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
index 5600502b33..ca80d11cb9 100644
--- a/hw/fsi/meson.build
+++ b/hw/fsi/meson.build
@@ -1,4 +1,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-slave.c'))
+system_ss.add(when: 'CONFIG_FSI', if_true: files('fsi.c','fsi-master.c','fsi-slave.c'))
diff --git a/include/hw/fsi/cfam.h b/include/hw/fsi/cfam.h
index af9f88cb22..83bd29b0c3 100644
--- a/include/hw/fsi/cfam.h
+++ b/include/hw/fsi/cfam.h
@@ -12,8 +12,6 @@
#include "hw/fsi/fsi-slave.h"
#include "hw/fsi/lbus.h"
-#define TYPE_FSI_BUS "fsi.bus"
-
#define TYPE_CFAM "cfam"
#define CFAM(obj) OBJECT_CHECK(CFAMState, (obj), TYPE_CFAM)
diff --git a/include/hw/fsi/fsi-master.h b/include/hw/fsi/fsi-master.h
new file mode 100644
index 0000000000..5341d4c873
--- /dev/null
+++ b/include/hw/fsi/fsi-master.h
@@ -0,0 +1,30 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2019 IBM Corp.
+ *
+ * IBM Flexible Service Interface Master
+ */
+#ifndef FSI_FSI_MASTER_H
+#define FSI_FSI_MASTER_H
+
+#include "exec/memory.h"
+#include "hw/qdev-core.h"
+#include "hw/fsi/fsi.h"
+
+#define TYPE_FSI_MASTER "fsi.master"
+#define FSI_MASTER(obj) OBJECT_CHECK(FSIMasterState, (obj), TYPE_FSI_MASTER)
+
+#define FSI_MASTER_NR_REGS ((0x2e0 >> 2) + 1)
+
+typedef struct FSIMasterState {
+ DeviceState parent;
+ MemoryRegion iomem;
+ MemoryRegion opb2fsi;
+
+ FSIBus bus;
+
+ uint32_t regs[FSI_MASTER_NR_REGS];
+} FSIMasterState;
+
+
+#endif /* FSI_FSI_H */
diff --git a/include/hw/fsi/fsi.h b/include/hw/fsi/fsi.h
new file mode 100644
index 0000000000..e88c5432ad
--- /dev/null
+++ b/include/hw/fsi/fsi.h
@@ -0,0 +1,35 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Flexible Service Interface
+ */
+#ifndef FSI_FSI_H
+#define FSI_FSI_H
+
+#include "hw/qdev-core.h"
+
+/*
+ * TODO: Maybe unwind this dependency with const links? Store a
+ * pointer in FSIBus?
+ */
+#include "hw/fsi/cfam.h"
+
+#define TYPE_FSI_BUS "fsi.bus"
+#define FSI_BUS(obj) OBJECT_CHECK(FSIBus, (obj), TYPE_FSI_BUS)
+#define FSI_BUS_CLASS(klass) \
+ OBJECT_CLASS_CHECK(FSIBusClass, (klass), TYPE_FSI_BUS)
+#define FSI_BUS_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(FSIBusClass, (obj), TYPE_FSI_BUS)
+
+/* TODO: Figure out what's best with a point-to-point bus */
+typedef struct FSISlaveState FSISlaveState;
+
+typedef struct FSIBus {
+ BusState bus;
+
+ /* XXX: It's point-to-point, just instantiate the slave directly for now */
+ CFAMState slave;
+} FSIBus;
+
+#endif
--
2.39.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v1 4/7] hw/fsi: Introduce IBM's FSI
2023-08-25 20:30 ` [PATCH v1 4/7] hw/fsi: Introduce IBM's FSI Ninad Palsule
@ 2023-08-28 8:57 ` Joel Stanley
2023-08-28 22:58 ` Ninad Palsule
0 siblings, 1 reply; 28+ messages in thread
From: Joel Stanley @ 2023-08-28 8:57 UTC (permalink / raw)
To: Ninad Palsule
Cc: qemu-devel, clg, peter.maydell, andrew, pbonzini,
marcandre.lureau, berrange, thuth, philmd, qemu-arm
On Fri, 25 Aug 2023 at 20:44, Ninad Palsule <ninad@linux.ibm.com> wrote:
>
> This is a part of patchset where IBM's Flexible Service Interface is
> introduced.
>
> This commit models the FSI bus. CFAM is hanging out of FSI bus. The bus
> is model such a way that it is embeded inside the FSI master which is a
> bus controller.
>
> The FSI master: A controller in the platform service processor (e.g.
> BMC) driving CFAM engine accesses into the POWER chip. At the
> hardware level FSI is a bit-based protocol supporting synchronous and
> DMA-driven accesses of engines in a CFAM.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> hw/fsi/cfam.c | 1 +
> hw/fsi/fsi-master.c | 203 ++++++++++++++++++++++++++++++++++++
> hw/fsi/fsi.c | 54 ++++++++++
> hw/fsi/meson.build | 2 +-
> include/hw/fsi/cfam.h | 2 -
> include/hw/fsi/fsi-master.h | 30 ++++++
> include/hw/fsi/fsi.h | 35 +++++++
> 7 files changed, 324 insertions(+), 3 deletions(-)
> create mode 100644 hw/fsi/fsi-master.c
> create mode 100644 hw/fsi/fsi.c
> create mode 100644 include/hw/fsi/fsi-master.h
> create mode 100644 include/hw/fsi/fsi.h
>
> diff --git a/hw/fsi/cfam.c b/hw/fsi/cfam.c
> index 19256050bd..12ce31cac4 100644
> --- a/hw/fsi/cfam.c
> +++ b/hw/fsi/cfam.c
> @@ -12,6 +12,7 @@
>
> #include "hw/fsi/bits.h"
> #include "hw/fsi/cfam.h"
> +#include "hw/fsi/fsi.h"
> #include "hw/fsi/engine-scratchpad.h"
>
> #include "hw/qdev-properties.h"
> diff --git a/hw/fsi/fsi-master.c b/hw/fsi/fsi-master.c
> new file mode 100644
> index 0000000000..fe1693539a
> --- /dev/null
> +++ b/hw/fsi/fsi-master.c
> @@ -0,0 +1,203 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2023 IBM Corp.
> + *
> + * IBM Flexible Service Interface master
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "qapi/error.h"
> +
> +#include "qemu/log.h"
> +
> +#include "hw/fsi/bits.h"
> +#include "hw/fsi/fsi-master.h"
> +
> +#define TYPE_OP_BUS "opb"
> +
> +#define TO_REG(x) ((x) >> 2)
> +
> +#define FSI_MMODE TO_REG(0x000)
> +#define FSI_MMODE_IPOLL_DMA_EN BE_BIT(0)
> +#define FSI_MMODE_HW_ERROR_RECOVERY_EN BE_BIT(1)
> +#define FSI_MMODE_RELATIVE_ADDRESS_EN BE_BIT(2)
> +#define FSI_MMODE_PARITY_CHECK_EN BE_BIT(3)
> +#define FSI_MMODE_CLOCK_DIVIDER_0 BE_GENMASK(4, 13)
> +#define FSI_MMODE_CLOCK_DIVIDER_1 BE_GENMASK(14, 23)
> +#define FSI_MMODE_DEBUG_EN BE_BIT(24)
> +
> +#define FSI_MDELAY TO_REG(0x004)
> +#define FSI_MDELAY_ECHO_0 BE_GENMASK(0, 3)
> +#define FSI_MDELAY_SEND_0 BE_GENMASK(4, 7)
> +#define FSI_MDELAY_ECHO_1 BE_GENMASK(8, 11)
> +#define FSI_MDELAY_SEND_1 BE_GENMASK(12, 15)
> +
> +#define FSI_MENP0 TO_REG(0x010)
> +#define FSI_MENP32 TO_REG(0x014)
> +#define FSI_MSENP0 TO_REG(0x018)
> +#define FSI_MLEVP0 TO_REG(0x018)
> +#define FSI_MSENP32 TO_REG(0x01c)
> +#define FSI_MLEVP32 TO_REG(0x01c)
> +#define FSI_MCENP0 TO_REG(0x020)
> +#define FSI_MREFP0 TO_REG(0x020)
> +#define FSI_MCENP32 TO_REG(0x024)
> +#define FSI_MREFP32 TO_REG(0x024)
> +
> +#define FSI_MAEB TO_REG(0x070)
> +#define FSI_MAEB_ANY_CPU_ERROR BE_BIT(0)
> +#define FSI_MAEB_ANY_DMA_ERROR BE_GENMASK(1, 16)
> +#define FSI_MAEB_ANY_PARITY_ERROR BE_BIT(17)
> +
> +#define FSI_MVER TO_REG(0x074)
> +#define FSI_MVER_VERSION BE_GENMASK(0, 7)
> +#define FSI_MVER_BRIDGES BE_GENMASK(8, 15)
> +#define FSI_MVER_PORTS BE_GENMASK(16, 23)
> +
> +#define FSI_MRESP0 TO_REG(0x0d0)
> +#define FSI_MRESP0_RESET_PORT_GENERAL BE_BIT(0)
> +#define FSI_MRESP0_RESET_PORT_ERROR BE_BIT(1)
> +#define FSI_MRESP0_RESET_ALL_BRIDGES_GENERAL BE_BIT(2)
> +#define FSI_MRESP0_RESET_ALL_PORTS_GENERAL BE_BIT(3)
> +#define FSI_MRESP0_RESET_MASTER BE_BIT(4)
> +#define FSI_MRESP0_RESET_PARITY_ERROR_LATCH BE_BIT(5)
> +
> +#define FSI_MRESB0 TO_REG(0x1d0)
> +#define FSI_MRESB0_RESET_GENERAL BE_BIT(0)
> +#define FSI_MRESB0_RESET_ERROR BE_BIT(1)
> +#define FSI_MRESB0_SET_DMA_SUSPEND BE_BIT(5)
> +#define FSI_MRESB0_CLEAR_DMA_SUSPEND BE_BIT(6)
> +#define FSI_MRESB0_SET_DELAY_MEASURE BE_BIT(7)
> +
> +#define FSI_MECTRL TO_REG(0x2e0)
> +#define FSI_MECTRL_TEST_PULSE BE_GENMASK(0, 7)
> +#define FSI_MECTRL_INHIBIT_PARITY_ERROR BE_GENMASK(8, 15)
> +#define FSI_MECTRL_ENABLE_OPB_ERR_ACK BE_BIT(16)
> +#define FSI_MECTRL_AUTO_TERMINATE BE_BIT(17)
> +#define FSI_MECTRL_PORT_ERROR_FREEZE BE_BIT(18)
> +
> +static uint64_t fsi_master_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + FSIMasterState *s = FSI_MASTER(opaque);
> +
> + qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx " size=%d\n",
> + __func__, addr, size);
> +
> + if (addr + size > sizeof(s->regs)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Out of bounds read: 0x%"HWADDR_PRIx" for %u\n",
> + __func__, addr, size);
> + return 0;
> + }
> +
> + return s->regs[TO_REG(addr)];
> +}
> +
> +static void fsi_master_write(void *opaque, hwaddr addr, uint64_t data,
> + unsigned size)
> +{
> + FSIMasterState *s = FSI_MASTER(opaque);
> +
> + qemu_log_mask(LOG_UNIMP, "%s: write @0x%" HWADDR_PRIx " size=%d "
> + "value=%"PRIx64"\n", __func__, addr, size, data);
> +
> + if (addr + size > sizeof(s->regs)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Out of bounds write: %"HWADDR_PRIx" for %u\n",
> + __func__, addr, size);
> + return;
> + }
> +
> + switch (TO_REG(addr)) {
> + case FSI_MENP0:
> + s->regs[FSI_MENP0] = data;
> + break;
> + case FSI_MENP32:
> + s->regs[FSI_MENP32] = data;
> + break;
> + case FSI_MSENP0:
> + s->regs[FSI_MENP0] |= data;
> + break;
> + case FSI_MSENP32:
> + s->regs[FSI_MENP32] |= data;
> + break;
> + case FSI_MCENP0:
> + s->regs[FSI_MENP0] &= ~data;
> + break;
> + case FSI_MCENP32:
> + s->regs[FSI_MENP32] &= ~data;
> + break;
> + case FSI_MRESP0:
> + /* Perform necessary resets leave register 0 to indicate no errors */
> + break;
> + case FSI_MRESB0:
> + if (data & FSI_MRESB0_RESET_GENERAL) {
> + device_cold_reset(DEVICE(opaque));
> + }
> + if (data & FSI_MRESB0_RESET_ERROR) {
> + /* FIXME: this seems dubious */
> + device_cold_reset(DEVICE(opaque));
> + }
> + break;
> + default:
> + s->regs[TO_REG(addr)] = data;
> + }
> +}
> +
> +static const struct MemoryRegionOps fsi_master_ops = {
> + .read = fsi_master_read,
> + .write = fsi_master_write,
> + .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> +static void fsi_master_realize(DeviceState *dev, Error **errp)
> +{
> + FSIMasterState *s = FSI_MASTER(dev);
> + Error *err = NULL;
> +
> + qbus_init(&s->bus, sizeof(s->bus), TYPE_FSI_BUS, DEVICE(s), NULL);
> +
> + memory_region_init_io(&s->iomem, OBJECT(s), &fsi_master_ops, s,
> + TYPE_FSI_MASTER, 0x10000000);
> + memory_region_init(&s->opb2fsi, OBJECT(s), "fsi.opb2fsi", 0x10000000);
> +
> + object_property_set_bool(OBJECT(&s->bus), "realized", true, &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> +
> + memory_region_add_subregion(&s->opb2fsi, 0, &s->bus.slave.mr);
> +}
> +
> +static void fsi_master_reset(DeviceState *dev)
> +{
> + FSIMasterState *s = FSI_MASTER(dev);
> +
> + /* ASPEED default */
> + s->regs[FSI_MVER] = 0xe0050101;
> +}
> +
> +static void fsi_master_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->bus_type = TYPE_OP_BUS;
> + dc->desc = "FSI Master";
> + dc->realize = fsi_master_realize;
> + dc->reset = fsi_master_reset;
> +}
> +
> +static const TypeInfo fsi_master_info = {
> + .name = TYPE_FSI_MASTER,
> + .parent = TYPE_DEVICE,
> + .instance_size = sizeof(FSIMasterState),
> + .class_init = fsi_master_class_init,
> +};
> +
> +static void fsi_register_types(void)
> +{
> + type_register_static(&fsi_master_info);
> +}
> +
> +type_init(fsi_register_types);
> diff --git a/hw/fsi/fsi.c b/hw/fsi/fsi.c
> new file mode 100644
> index 0000000000..43f889fee9
> --- /dev/null
> +++ b/hw/fsi/fsi.c
> @@ -0,0 +1,54 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2023 IBM Corp.
> + *
> + * IBM Flexible Service Interface
> + */
> +#include "qemu/osdep.h"
> +
> +#include "qapi/error.h"
> +
> +#include "hw/fsi/fsi.h"
> +#include "hw/fsi/cfam.h"
> +
> +static void fsi_bus_realize(BusState *bus, Error **errp)
> +{
> + FSIBus *s = FSI_BUS(bus);
> + Error *err = NULL;
> +
> + /* FIXME: Should be realised elsewhere and added to the bus */
> + object_property_set_bool(OBJECT(&s->slave), "realized", true, &err);
> + if (err) {
> + error_propagate(errp, err);
> + }
> +}
> +
> +static void fsi_bus_init(Object *o)
> +{
> + FSIBus *s = FSI_BUS(o);
> +
> + /* FIXME: Move this elsewhere */
> + object_initialize_child(o, TYPE_CFAM, &s->slave, TYPE_CFAM);
> + qdev_set_parent_bus(DEVICE(&s->slave), BUS(o), &error_abort);
> +}
> +
> +static void fsi_bus_class_init(ObjectClass *klass, void *data)
> +{
> + BusClass *bc = BUS_CLASS(klass);
> + bc->realize = fsi_bus_realize;
> +}
> +
> +static const TypeInfo fsi_bus_info = {
> + .name = TYPE_FSI_BUS,
> + .parent = TYPE_BUS,
> + .instance_init = fsi_bus_init,
> + .instance_size = sizeof(FSIBus),
> + .class_init = fsi_bus_class_init,
> +};
> +
> +static void fsi_bus_register_types(void)
> +{
> + type_register_static(&fsi_bus_info);
> +}
> +
> +type_init(fsi_bus_register_types);
> diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
> index 5600502b33..ca80d11cb9 100644
> --- a/hw/fsi/meson.build
> +++ b/hw/fsi/meson.build
> @@ -1,4 +1,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-slave.c'))
> +system_ss.add(when: 'CONFIG_FSI', if_true: files('fsi.c','fsi-master.c','fsi-slave.c'))
> diff --git a/include/hw/fsi/cfam.h b/include/hw/fsi/cfam.h
> index af9f88cb22..83bd29b0c3 100644
> --- a/include/hw/fsi/cfam.h
> +++ b/include/hw/fsi/cfam.h
> @@ -12,8 +12,6 @@
> #include "hw/fsi/fsi-slave.h"
> #include "hw/fsi/lbus.h"
>
> -#define TYPE_FSI_BUS "fsi.bus"
> -
> #define TYPE_CFAM "cfam"
> #define CFAM(obj) OBJECT_CHECK(CFAMState, (obj), TYPE_CFAM)
>
> diff --git a/include/hw/fsi/fsi-master.h b/include/hw/fsi/fsi-master.h
> new file mode 100644
> index 0000000000..5341d4c873
> --- /dev/null
> +++ b/include/hw/fsi/fsi-master.h
> @@ -0,0 +1,30 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2019 IBM Corp.
> + *
> + * IBM Flexible Service Interface Master
> + */
> +#ifndef FSI_FSI_MASTER_H
> +#define FSI_FSI_MASTER_H
> +
> +#include "exec/memory.h"
> +#include "hw/qdev-core.h"
> +#include "hw/fsi/fsi.h"
> +
> +#define TYPE_FSI_MASTER "fsi.master"
> +#define FSI_MASTER(obj) OBJECT_CHECK(FSIMasterState, (obj), TYPE_FSI_MASTER)
> +
> +#define FSI_MASTER_NR_REGS ((0x2e0 >> 2) + 1)
> +
> +typedef struct FSIMasterState {
> + DeviceState parent;
> + MemoryRegion iomem;
> + MemoryRegion opb2fsi;
> +
> + FSIBus bus;
> +
> + uint32_t regs[FSI_MASTER_NR_REGS];
> +} FSIMasterState;
> +
> +
> +#endif /* FSI_FSI_H */
> diff --git a/include/hw/fsi/fsi.h b/include/hw/fsi/fsi.h
> new file mode 100644
> index 0000000000..e88c5432ad
> --- /dev/null
> +++ b/include/hw/fsi/fsi.h
> @@ -0,0 +1,35 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2023 IBM Corp.
> + *
> + * IBM Flexible Service Interface
> + */
> +#ifndef FSI_FSI_H
> +#define FSI_FSI_H
> +
> +#include "hw/qdev-core.h"
> +
> +/*
> + * TODO: Maybe unwind this dependency with const links? Store a
> + * pointer in FSIBus?
> + */
> +#include "hw/fsi/cfam.h"
> +
> +#define TYPE_FSI_BUS "fsi.bus"
> +#define FSI_BUS(obj) OBJECT_CHECK(FSIBus, (obj), TYPE_FSI_BUS)
> +#define FSI_BUS_CLASS(klass) \
> + OBJECT_CLASS_CHECK(FSIBusClass, (klass), TYPE_FSI_BUS)
> +#define FSI_BUS_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(FSIBusClass, (obj), TYPE_FSI_BUS)
> +
> +/* TODO: Figure out what's best with a point-to-point bus */
> +typedef struct FSISlaveState FSISlaveState;
> +
> +typedef struct FSIBus {
> + BusState bus;
> +
> + /* XXX: It's point-to-point, just instantiate the slave directly for now */
> + CFAMState slave;
> +} FSIBus;
> +
> +#endif
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 4/7] hw/fsi: Introduce IBM's FSI
2023-08-28 8:57 ` Joel Stanley
@ 2023-08-28 22:58 ` Ninad Palsule
0 siblings, 0 replies; 28+ messages in thread
From: Ninad Palsule @ 2023-08-28 22:58 UTC (permalink / raw)
To: Joel Stanley
Cc: qemu-devel, clg, peter.maydell, andrew, pbonzini,
marcandre.lureau, berrange, thuth, philmd, qemu-arm
Thanks for the review, Joel.
On 8/28/23 23:57, Joel Stanley wrote:
> On Fri, 25 Aug 2023 at 20:44, Ninad Palsule <ninad@linux.ibm.com> wrote:
>> This is a part of patchset where IBM's Flexible Service Interface is
>> introduced.
>>
>> This commit models the FSI bus. CFAM is hanging out of FSI bus. The bus
>> is model such a way that it is embeded inside the FSI master which is a
>> bus controller.
>>
>> The FSI master: A controller in the platform service processor (e.g.
>> BMC) driving CFAM engine accesses into the POWER chip. At the
>> hardware level FSI is a bit-based protocol supporting synchronous and
>> DMA-driven accesses of engines in a CFAM.
>>
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
>
>> ---
>> hw/fsi/cfam.c | 1 +
>> hw/fsi/fsi-master.c | 203 ++++++++++++++++++++++++++++++++++++
>> hw/fsi/fsi.c | 54 ++++++++++
>> hw/fsi/meson.build | 2 +-
>> include/hw/fsi/cfam.h | 2 -
>> include/hw/fsi/fsi-master.h | 30 ++++++
>> include/hw/fsi/fsi.h | 35 +++++++
>> 7 files changed, 324 insertions(+), 3 deletions(-)
>> create mode 100644 hw/fsi/fsi-master.c
>> create mode 100644 hw/fsi/fsi.c
>> create mode 100644 include/hw/fsi/fsi-master.h
>> create mode 100644 include/hw/fsi/fsi.h
>>
>> diff --git a/hw/fsi/cfam.c b/hw/fsi/cfam.c
>> index 19256050bd..12ce31cac4 100644
>> --- a/hw/fsi/cfam.c
>> +++ b/hw/fsi/cfam.c
>> @@ -12,6 +12,7 @@
>>
>> #include "hw/fsi/bits.h"
>> #include "hw/fsi/cfam.h"
>> +#include "hw/fsi/fsi.h"
>> #include "hw/fsi/engine-scratchpad.h"
>>
>> #include "hw/qdev-properties.h"
>> diff --git a/hw/fsi/fsi-master.c b/hw/fsi/fsi-master.c
>> new file mode 100644
>> index 0000000000..fe1693539a
>> --- /dev/null
>> +++ b/hw/fsi/fsi-master.c
>> @@ -0,0 +1,203 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + * Copyright (C) 2023 IBM Corp.
>> + *
>> + * IBM Flexible Service Interface master
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +
>> +#include "qapi/error.h"
>> +
>> +#include "qemu/log.h"
>> +
>> +#include "hw/fsi/bits.h"
>> +#include "hw/fsi/fsi-master.h"
>> +
>> +#define TYPE_OP_BUS "opb"
>> +
>> +#define TO_REG(x) ((x) >> 2)
>> +
>> +#define FSI_MMODE TO_REG(0x000)
>> +#define FSI_MMODE_IPOLL_DMA_EN BE_BIT(0)
>> +#define FSI_MMODE_HW_ERROR_RECOVERY_EN BE_BIT(1)
>> +#define FSI_MMODE_RELATIVE_ADDRESS_EN BE_BIT(2)
>> +#define FSI_MMODE_PARITY_CHECK_EN BE_BIT(3)
>> +#define FSI_MMODE_CLOCK_DIVIDER_0 BE_GENMASK(4, 13)
>> +#define FSI_MMODE_CLOCK_DIVIDER_1 BE_GENMASK(14, 23)
>> +#define FSI_MMODE_DEBUG_EN BE_BIT(24)
>> +
>> +#define FSI_MDELAY TO_REG(0x004)
>> +#define FSI_MDELAY_ECHO_0 BE_GENMASK(0, 3)
>> +#define FSI_MDELAY_SEND_0 BE_GENMASK(4, 7)
>> +#define FSI_MDELAY_ECHO_1 BE_GENMASK(8, 11)
>> +#define FSI_MDELAY_SEND_1 BE_GENMASK(12, 15)
>> +
>> +#define FSI_MENP0 TO_REG(0x010)
>> +#define FSI_MENP32 TO_REG(0x014)
>> +#define FSI_MSENP0 TO_REG(0x018)
>> +#define FSI_MLEVP0 TO_REG(0x018)
>> +#define FSI_MSENP32 TO_REG(0x01c)
>> +#define FSI_MLEVP32 TO_REG(0x01c)
>> +#define FSI_MCENP0 TO_REG(0x020)
>> +#define FSI_MREFP0 TO_REG(0x020)
>> +#define FSI_MCENP32 TO_REG(0x024)
>> +#define FSI_MREFP32 TO_REG(0x024)
>> +
>> +#define FSI_MAEB TO_REG(0x070)
>> +#define FSI_MAEB_ANY_CPU_ERROR BE_BIT(0)
>> +#define FSI_MAEB_ANY_DMA_ERROR BE_GENMASK(1, 16)
>> +#define FSI_MAEB_ANY_PARITY_ERROR BE_BIT(17)
>> +
>> +#define FSI_MVER TO_REG(0x074)
>> +#define FSI_MVER_VERSION BE_GENMASK(0, 7)
>> +#define FSI_MVER_BRIDGES BE_GENMASK(8, 15)
>> +#define FSI_MVER_PORTS BE_GENMASK(16, 23)
>> +
>> +#define FSI_MRESP0 TO_REG(0x0d0)
>> +#define FSI_MRESP0_RESET_PORT_GENERAL BE_BIT(0)
>> +#define FSI_MRESP0_RESET_PORT_ERROR BE_BIT(1)
>> +#define FSI_MRESP0_RESET_ALL_BRIDGES_GENERAL BE_BIT(2)
>> +#define FSI_MRESP0_RESET_ALL_PORTS_GENERAL BE_BIT(3)
>> +#define FSI_MRESP0_RESET_MASTER BE_BIT(4)
>> +#define FSI_MRESP0_RESET_PARITY_ERROR_LATCH BE_BIT(5)
>> +
>> +#define FSI_MRESB0 TO_REG(0x1d0)
>> +#define FSI_MRESB0_RESET_GENERAL BE_BIT(0)
>> +#define FSI_MRESB0_RESET_ERROR BE_BIT(1)
>> +#define FSI_MRESB0_SET_DMA_SUSPEND BE_BIT(5)
>> +#define FSI_MRESB0_CLEAR_DMA_SUSPEND BE_BIT(6)
>> +#define FSI_MRESB0_SET_DELAY_MEASURE BE_BIT(7)
>> +
>> +#define FSI_MECTRL TO_REG(0x2e0)
>> +#define FSI_MECTRL_TEST_PULSE BE_GENMASK(0, 7)
>> +#define FSI_MECTRL_INHIBIT_PARITY_ERROR BE_GENMASK(8, 15)
>> +#define FSI_MECTRL_ENABLE_OPB_ERR_ACK BE_BIT(16)
>> +#define FSI_MECTRL_AUTO_TERMINATE BE_BIT(17)
>> +#define FSI_MECTRL_PORT_ERROR_FREEZE BE_BIT(18)
>> +
>> +static uint64_t fsi_master_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> + FSIMasterState *s = FSI_MASTER(opaque);
>> +
>> + qemu_log_mask(LOG_UNIMP, "%s: read @0x%" HWADDR_PRIx " size=%d\n",
>> + __func__, addr, size);
>> +
>> + if (addr + size > sizeof(s->regs)) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Out of bounds read: 0x%"HWADDR_PRIx" for %u\n",
>> + __func__, addr, size);
>> + return 0;
>> + }
>> +
>> + return s->regs[TO_REG(addr)];
>> +}
>> +
>> +static void fsi_master_write(void *opaque, hwaddr addr, uint64_t data,
>> + unsigned size)
>> +{
>> + FSIMasterState *s = FSI_MASTER(opaque);
>> +
>> + qemu_log_mask(LOG_UNIMP, "%s: write @0x%" HWADDR_PRIx " size=%d "
>> + "value=%"PRIx64"\n", __func__, addr, size, data);
>> +
>> + if (addr + size > sizeof(s->regs)) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Out of bounds write: %"HWADDR_PRIx" for %u\n",
>> + __func__, addr, size);
>> + return;
>> + }
>> +
>> + switch (TO_REG(addr)) {
>> + case FSI_MENP0:
>> + s->regs[FSI_MENP0] = data;
>> + break;
>> + case FSI_MENP32:
>> + s->regs[FSI_MENP32] = data;
>> + break;
>> + case FSI_MSENP0:
>> + s->regs[FSI_MENP0] |= data;
>> + break;
>> + case FSI_MSENP32:
>> + s->regs[FSI_MENP32] |= data;
>> + break;
>> + case FSI_MCENP0:
>> + s->regs[FSI_MENP0] &= ~data;
>> + break;
>> + case FSI_MCENP32:
>> + s->regs[FSI_MENP32] &= ~data;
>> + break;
>> + case FSI_MRESP0:
>> + /* Perform necessary resets leave register 0 to indicate no errors */
>> + break;
>> + case FSI_MRESB0:
>> + if (data & FSI_MRESB0_RESET_GENERAL) {
>> + device_cold_reset(DEVICE(opaque));
>> + }
>> + if (data & FSI_MRESB0_RESET_ERROR) {
>> + /* FIXME: this seems dubious */
>> + device_cold_reset(DEVICE(opaque));
>> + }
>> + break;
>> + default:
>> + s->regs[TO_REG(addr)] = data;
>> + }
>> +}
>> +
>> +static const struct MemoryRegionOps fsi_master_ops = {
>> + .read = fsi_master_read,
>> + .write = fsi_master_write,
>> + .endianness = DEVICE_BIG_ENDIAN,
>> +};
>> +
>> +static void fsi_master_realize(DeviceState *dev, Error **errp)
>> +{
>> + FSIMasterState *s = FSI_MASTER(dev);
>> + Error *err = NULL;
>> +
>> + qbus_init(&s->bus, sizeof(s->bus), TYPE_FSI_BUS, DEVICE(s), NULL);
>> +
>> + memory_region_init_io(&s->iomem, OBJECT(s), &fsi_master_ops, s,
>> + TYPE_FSI_MASTER, 0x10000000);
>> + memory_region_init(&s->opb2fsi, OBJECT(s), "fsi.opb2fsi", 0x10000000);
>> +
>> + object_property_set_bool(OBJECT(&s->bus), "realized", true, &err);
>> + if (err) {
>> + error_propagate(errp, err);
>> + return;
>> + }
>> +
>> + memory_region_add_subregion(&s->opb2fsi, 0, &s->bus.slave.mr);
>> +}
>> +
>> +static void fsi_master_reset(DeviceState *dev)
>> +{
>> + FSIMasterState *s = FSI_MASTER(dev);
>> +
>> + /* ASPEED default */
>> + s->regs[FSI_MVER] = 0xe0050101;
>> +}
>> +
>> +static void fsi_master_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + dc->bus_type = TYPE_OP_BUS;
>> + dc->desc = "FSI Master";
>> + dc->realize = fsi_master_realize;
>> + dc->reset = fsi_master_reset;
>> +}
>> +
>> +static const TypeInfo fsi_master_info = {
>> + .name = TYPE_FSI_MASTER,
>> + .parent = TYPE_DEVICE,
>> + .instance_size = sizeof(FSIMasterState),
>> + .class_init = fsi_master_class_init,
>> +};
>> +
>> +static void fsi_register_types(void)
>> +{
>> + type_register_static(&fsi_master_info);
>> +}
>> +
>> +type_init(fsi_register_types);
>> diff --git a/hw/fsi/fsi.c b/hw/fsi/fsi.c
>> new file mode 100644
>> index 0000000000..43f889fee9
>> --- /dev/null
>> +++ b/hw/fsi/fsi.c
>> @@ -0,0 +1,54 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + * Copyright (C) 2023 IBM Corp.
>> + *
>> + * IBM Flexible Service Interface
>> + */
>> +#include "qemu/osdep.h"
>> +
>> +#include "qapi/error.h"
>> +
>> +#include "hw/fsi/fsi.h"
>> +#include "hw/fsi/cfam.h"
>> +
>> +static void fsi_bus_realize(BusState *bus, Error **errp)
>> +{
>> + FSIBus *s = FSI_BUS(bus);
>> + Error *err = NULL;
>> +
>> + /* FIXME: Should be realised elsewhere and added to the bus */
>> + object_property_set_bool(OBJECT(&s->slave), "realized", true, &err);
>> + if (err) {
>> + error_propagate(errp, err);
>> + }
>> +}
>> +
>> +static void fsi_bus_init(Object *o)
>> +{
>> + FSIBus *s = FSI_BUS(o);
>> +
>> + /* FIXME: Move this elsewhere */
>> + object_initialize_child(o, TYPE_CFAM, &s->slave, TYPE_CFAM);
>> + qdev_set_parent_bus(DEVICE(&s->slave), BUS(o), &error_abort);
>> +}
>> +
>> +static void fsi_bus_class_init(ObjectClass *klass, void *data)
>> +{
>> + BusClass *bc = BUS_CLASS(klass);
>> + bc->realize = fsi_bus_realize;
>> +}
>> +
>> +static const TypeInfo fsi_bus_info = {
>> + .name = TYPE_FSI_BUS,
>> + .parent = TYPE_BUS,
>> + .instance_init = fsi_bus_init,
>> + .instance_size = sizeof(FSIBus),
>> + .class_init = fsi_bus_class_init,
>> +};
>> +
>> +static void fsi_bus_register_types(void)
>> +{
>> + type_register_static(&fsi_bus_info);
>> +}
>> +
>> +type_init(fsi_bus_register_types);
>> diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
>> index 5600502b33..ca80d11cb9 100644
>> --- a/hw/fsi/meson.build
>> +++ b/hw/fsi/meson.build
>> @@ -1,4 +1,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-slave.c'))
>> +system_ss.add(when: 'CONFIG_FSI', if_true: files('fsi.c','fsi-master.c','fsi-slave.c'))
>> diff --git a/include/hw/fsi/cfam.h b/include/hw/fsi/cfam.h
>> index af9f88cb22..83bd29b0c3 100644
>> --- a/include/hw/fsi/cfam.h
>> +++ b/include/hw/fsi/cfam.h
>> @@ -12,8 +12,6 @@
>> #include "hw/fsi/fsi-slave.h"
>> #include "hw/fsi/lbus.h"
>>
>> -#define TYPE_FSI_BUS "fsi.bus"
>> -
>> #define TYPE_CFAM "cfam"
>> #define CFAM(obj) OBJECT_CHECK(CFAMState, (obj), TYPE_CFAM)
>>
>> diff --git a/include/hw/fsi/fsi-master.h b/include/hw/fsi/fsi-master.h
>> new file mode 100644
>> index 0000000000..5341d4c873
>> --- /dev/null
>> +++ b/include/hw/fsi/fsi-master.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + * Copyright (C) 2019 IBM Corp.
>> + *
>> + * IBM Flexible Service Interface Master
>> + */
>> +#ifndef FSI_FSI_MASTER_H
>> +#define FSI_FSI_MASTER_H
>> +
>> +#include "exec/memory.h"
>> +#include "hw/qdev-core.h"
>> +#include "hw/fsi/fsi.h"
>> +
>> +#define TYPE_FSI_MASTER "fsi.master"
>> +#define FSI_MASTER(obj) OBJECT_CHECK(FSIMasterState, (obj), TYPE_FSI_MASTER)
>> +
>> +#define FSI_MASTER_NR_REGS ((0x2e0 >> 2) + 1)
>> +
>> +typedef struct FSIMasterState {
>> + DeviceState parent;
>> + MemoryRegion iomem;
>> + MemoryRegion opb2fsi;
>> +
>> + FSIBus bus;
>> +
>> + uint32_t regs[FSI_MASTER_NR_REGS];
>> +} FSIMasterState;
>> +
>> +
>> +#endif /* FSI_FSI_H */
>> diff --git a/include/hw/fsi/fsi.h b/include/hw/fsi/fsi.h
>> new file mode 100644
>> index 0000000000..e88c5432ad
>> --- /dev/null
>> +++ b/include/hw/fsi/fsi.h
>> @@ -0,0 +1,35 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + * Copyright (C) 2023 IBM Corp.
>> + *
>> + * IBM Flexible Service Interface
>> + */
>> +#ifndef FSI_FSI_H
>> +#define FSI_FSI_H
>> +
>> +#include "hw/qdev-core.h"
>> +
>> +/*
>> + * TODO: Maybe unwind this dependency with const links? Store a
>> + * pointer in FSIBus?
>> + */
>> +#include "hw/fsi/cfam.h"
>> +
>> +#define TYPE_FSI_BUS "fsi.bus"
>> +#define FSI_BUS(obj) OBJECT_CHECK(FSIBus, (obj), TYPE_FSI_BUS)
>> +#define FSI_BUS_CLASS(klass) \
>> + OBJECT_CLASS_CHECK(FSIBusClass, (klass), TYPE_FSI_BUS)
>> +#define FSI_BUS_GET_CLASS(obj) \
>> + OBJECT_GET_CLASS(FSIBusClass, (obj), TYPE_FSI_BUS)
>> +
>> +/* TODO: Figure out what's best with a point-to-point bus */
>> +typedef struct FSISlaveState FSISlaveState;
>> +
>> +typedef struct FSIBus {
>> + BusState bus;
>> +
>> + /* XXX: It's point-to-point, just instantiate the slave directly for now */
>> + CFAMState slave;
>> +} FSIBus;
>> +
>> +#endif
>> --
>> 2.39.2
>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v1 5/7] hw/fsi: IBM's On-chip Peripheral Bus
2023-08-25 20:30 [PATCH v1 0/7] Introduce model for IBM's FSP Ninad Palsule
` (3 preceding siblings ...)
2023-08-25 20:30 ` [PATCH v1 4/7] hw/fsi: Introduce IBM's FSI Ninad Palsule
@ 2023-08-25 20:30 ` Ninad Palsule
2023-08-28 8:59 ` Joel Stanley
2023-08-25 20:30 ` [PATCH v1 6/7] hw/fsi: Aspeed APB2OPB interface Ninad Palsule
` (3 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Ninad Palsule @ 2023-08-25 20:30 UTC (permalink / raw)
To: qemu-devel, clg, peter.maydell, andrew, joel, pbonzini,
marcandre.lureau, berrange, thuth, philmd
Cc: Ninad Palsule, qemu-arm
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 <andrew@aj.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
---
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 0000000000..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(&opb->as, addr, MEMTXATTRS_UNSPECIFIED, data,
+ len);
+}
+
+uint8_t opb_read8(OPBus *opb, hwaddr addr)
+{
+ MemTxResult tx;
+ uint8_t data;
+
+ tx = opb_read(opb, addr, &data, 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, &data, 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, &data, 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(&opb->as, addr, MEMTXATTRS_UNSPECIFIED, data,
+ len);
+}
+
+void opb_write8(OPBus *opb, hwaddr addr, uint8_t data)
+{
+ MemTxResult tx;
+
+ tx = opb_write(opb, addr, &data, 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, &data, 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, &data, 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(&opb->fsi.iomem, addr);
+ memory_region_transaction_commit();
+}
+
+void opb_opb2fsi_address(OPBus *opb, hwaddr addr)
+{
+ memory_region_transaction_begin();
+ memory_region_set_address(&opb->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 opb_realize(BusState *bus, Error **errp)
+{
+ OPBus *opb = OP_BUS(bus);
+ Error *err = NULL;
+
+ memory_region_init_io(&opb->mr, OBJECT(opb), &opb_unimplemented_ops, opb,
+ NULL, UINT32_MAX);
+ address_space_init(&opb->as, &opb->mr, "opb");
+
+ object_property_set_bool(OBJECT(&opb->fsi), "realized", true, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+ memory_region_add_subregion(&opb->mr, 0x80000000, &opb->fsi.iomem);
+
+ /* OPB2FSI region */
+ /*
+ * Avoid endianness issues by mapping each slave's memory region directly.
+ * Manually bridging multiple address-spaces causes endian swapping
+ * headaches as memory_region_dispatch_read() and
+ * memory_region_dispatch_write() correct the endianness based on the
+ * target machine endianness and not relative to the device endianness on
+ * either side of the bridge.
+ */
+ /*
+ * XXX: This is a bit hairy and will need to be fixed when I sort out the
+ * bus/slave relationship and any changes to the CFAM modelling (multiple
+ * slaves, LBUS)
+ */
+ memory_region_add_subregion(&opb->mr, 0xa0000000, &opb->fsi.opb2fsi);
+}
+
+static void opb_init(Object *o)
+{
+ OPBus *opb = OP_BUS(o);
+
+ object_initialize_child(o, "fsi-master", &opb->fsi, TYPE_FSI_MASTER);
+ qdev_set_parent_bus(DEVICE(&opb->fsi), BUS(o), &error_abort);
+}
+
+static void opb_finalize(Object *o)
+{
+ OPBus *opb = OP_BUS(o);
+
+ address_space_destroy(&opb->as);
+}
+
+static void opb_class_init(ObjectClass *klass, void *data)
+{
+ BusClass *bc = BUS_CLASS(klass);
+ bc->realize = opb_realize;
+}
+
+static const TypeInfo opb_info = {
+ .name = TYPE_OP_BUS,
+ .parent = TYPE_BUS,
+ .instance_init = opb_init,
+ .instance_finalize = opb_finalize,
+ .instance_size = sizeof(OPBus),
+ .class_init = opb_class_init,
+ .class_size = sizeof(OPBusClass),
+};
+
+static void opb_register_types(void)
+{
+ type_register_static(&opb_info);
+}
+
+type_init(opb_register_types);
diff --git a/include/hw/fsi/opb.h b/include/hw/fsi/opb.h
new file mode 100644
index 0000000000..fad40eed79
--- /dev/null
+++ b/include/hw/fsi/opb.h
@@ -0,0 +1,45 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM On-Chip Peripheral Bus
+ */
+#ifndef FSI_OPB_H
+#define FSI_OPB_H
+
+#include "exec/memory.h"
+#include "hw/fsi/fsi-master.h"
+
+#define TYPE_OP_BUS "opb"
+#define OP_BUS(obj) OBJECT_CHECK(OPBus, (obj), TYPE_OP_BUS)
+#define OP_BUS_CLASS(klass) OBJECT_CLASS_CHECK(OPBusClass, (klass), TYPE_OP_BUS)
+#define OP_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(OPBusClass, (obj), TYPE_OP_BUS)
+
+typedef struct OPBus {
+ /*< private >*/
+ BusState bus;
+
+ /*< public >*/
+ MemoryRegion mr;
+ AddressSpace as;
+
+ /* Model OPB as dumb enough just to provide an address-space */
+ /* TODO: Maybe don't store device state in the bus? */
+ FSIMasterState fsi;
+} OPBus;
+
+typedef struct OPBusClass {
+ BusClass parent_class;
+} OPBusClass;
+
+uint8_t opb_read8(OPBus *opb, hwaddr addr);
+uint16_t opb_read16(OPBus *opb, hwaddr addr);
+uint32_t opb_read32(OPBus *opb, hwaddr addr);
+void opb_write8(OPBus *opb, hwaddr addr, uint8_t data);
+void opb_write16(OPBus *opb, hwaddr addr, uint16_t data);
+void opb_write32(OPBus *opb, hwaddr addr, uint32_t data);
+
+void opb_fsi_master_address(OPBus *opb, hwaddr addr);
+void opb_opb2fsi_address(OPBus *opb, hwaddr addr);
+
+#endif /* FSI_OPB_H */
--
2.39.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v1 5/7] hw/fsi: IBM's On-chip Peripheral Bus
2023-08-25 20:30 ` [PATCH v1 5/7] hw/fsi: IBM's On-chip Peripheral Bus Ninad Palsule
@ 2023-08-28 8:59 ` Joel Stanley
2023-08-28 22:58 ` Ninad Palsule
0 siblings, 1 reply; 28+ messages in thread
From: Joel Stanley @ 2023-08-28 8:59 UTC (permalink / raw)
To: Ninad Palsule
Cc: qemu-devel, clg, peter.maydell, andrew, pbonzini,
marcandre.lureau, berrange, thuth, philmd, qemu-arm
On Fri, 25 Aug 2023 at 20:35, Ninad Palsule <ninad@linux.ibm.com> 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 <andrew@aj.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> 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 0000000000..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(&opb->as, addr, MEMTXATTRS_UNSPECIFIED, data,
> + len);
> +}
> +
> +uint8_t opb_read8(OPBus *opb, hwaddr addr)
> +{
> + MemTxResult tx;
> + uint8_t data;
> +
> + tx = opb_read(opb, addr, &data, 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, &data, 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, &data, 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(&opb->as, addr, MEMTXATTRS_UNSPECIFIED, data,
> + len);
> +}
> +
> +void opb_write8(OPBus *opb, hwaddr addr, uint8_t data)
> +{
> + MemTxResult tx;
> +
> + tx = opb_write(opb, addr, &data, 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, &data, 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, &data, 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(&opb->fsi.iomem, addr);
> + memory_region_transaction_commit();
> +}
> +
> +void opb_opb2fsi_address(OPBus *opb, hwaddr addr)
> +{
> + memory_region_transaction_begin();
> + memory_region_set_address(&opb->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 opb_realize(BusState *bus, Error **errp)
> +{
> + OPBus *opb = OP_BUS(bus);
> + Error *err = NULL;
> +
> + memory_region_init_io(&opb->mr, OBJECT(opb), &opb_unimplemented_ops, opb,
> + NULL, UINT32_MAX);
> + address_space_init(&opb->as, &opb->mr, "opb");
> +
> + object_property_set_bool(OBJECT(&opb->fsi), "realized", true, &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> + memory_region_add_subregion(&opb->mr, 0x80000000, &opb->fsi.iomem);
> +
> + /* OPB2FSI region */
> + /*
> + * Avoid endianness issues by mapping each slave's memory region directly.
> + * Manually bridging multiple address-spaces causes endian swapping
> + * headaches as memory_region_dispatch_read() and
> + * memory_region_dispatch_write() correct the endianness based on the
> + * target machine endianness and not relative to the device endianness on
> + * either side of the bridge.
> + */
> + /*
> + * XXX: This is a bit hairy and will need to be fixed when I sort out the
> + * bus/slave relationship and any changes to the CFAM modelling (multiple
> + * slaves, LBUS)
> + */
> + memory_region_add_subregion(&opb->mr, 0xa0000000, &opb->fsi.opb2fsi);
> +}
> +
> +static void opb_init(Object *o)
> +{
> + OPBus *opb = OP_BUS(o);
> +
> + object_initialize_child(o, "fsi-master", &opb->fsi, TYPE_FSI_MASTER);
> + qdev_set_parent_bus(DEVICE(&opb->fsi), BUS(o), &error_abort);
> +}
> +
> +static void opb_finalize(Object *o)
> +{
> + OPBus *opb = OP_BUS(o);
> +
> + address_space_destroy(&opb->as);
> +}
> +
> +static void opb_class_init(ObjectClass *klass, void *data)
> +{
> + BusClass *bc = BUS_CLASS(klass);
> + bc->realize = opb_realize;
> +}
> +
> +static const TypeInfo opb_info = {
> + .name = TYPE_OP_BUS,
> + .parent = TYPE_BUS,
> + .instance_init = opb_init,
> + .instance_finalize = opb_finalize,
> + .instance_size = sizeof(OPBus),
> + .class_init = opb_class_init,
> + .class_size = sizeof(OPBusClass),
> +};
> +
> +static void opb_register_types(void)
> +{
> + type_register_static(&opb_info);
> +}
> +
> +type_init(opb_register_types);
> diff --git a/include/hw/fsi/opb.h b/include/hw/fsi/opb.h
> new file mode 100644
> index 0000000000..fad40eed79
> --- /dev/null
> +++ b/include/hw/fsi/opb.h
> @@ -0,0 +1,45 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2023 IBM Corp.
> + *
> + * IBM On-Chip Peripheral Bus
> + */
> +#ifndef FSI_OPB_H
> +#define FSI_OPB_H
> +
> +#include "exec/memory.h"
> +#include "hw/fsi/fsi-master.h"
> +
> +#define TYPE_OP_BUS "opb"
> +#define OP_BUS(obj) OBJECT_CHECK(OPBus, (obj), TYPE_OP_BUS)
> +#define OP_BUS_CLASS(klass) OBJECT_CLASS_CHECK(OPBusClass, (klass), TYPE_OP_BUS)
> +#define OP_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(OPBusClass, (obj), TYPE_OP_BUS)
> +
> +typedef struct OPBus {
> + /*< private >*/
> + BusState bus;
> +
> + /*< public >*/
> + MemoryRegion mr;
> + AddressSpace as;
> +
> + /* Model OPB as dumb enough just to provide an address-space */
> + /* TODO: Maybe don't store device state in the bus? */
> + FSIMasterState fsi;
> +} OPBus;
> +
> +typedef struct OPBusClass {
> + BusClass parent_class;
> +} OPBusClass;
> +
> +uint8_t opb_read8(OPBus *opb, hwaddr addr);
> +uint16_t opb_read16(OPBus *opb, hwaddr addr);
> +uint32_t opb_read32(OPBus *opb, hwaddr addr);
> +void opb_write8(OPBus *opb, hwaddr addr, uint8_t data);
> +void opb_write16(OPBus *opb, hwaddr addr, uint16_t data);
> +void opb_write32(OPBus *opb, hwaddr addr, uint32_t data);
> +
> +void opb_fsi_master_address(OPBus *opb, hwaddr addr);
> +void opb_opb2fsi_address(OPBus *opb, hwaddr addr);
> +
> +#endif /* FSI_OPB_H */
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 5/7] hw/fsi: IBM's On-chip Peripheral Bus
2023-08-28 8:59 ` Joel Stanley
@ 2023-08-28 22:58 ` Ninad Palsule
0 siblings, 0 replies; 28+ messages in thread
From: Ninad Palsule @ 2023-08-28 22:58 UTC (permalink / raw)
To: Joel Stanley
Cc: qemu-devel, clg, peter.maydell, andrew, pbonzini,
marcandre.lureau, berrange, thuth, philmd, qemu-arm
Thanks for the review Joel.
On 8/28/23 23:59, Joel Stanley wrote:
> On Fri, 25 Aug 2023 at 20:35, Ninad Palsule <ninad@linux.ibm.com> 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 <andrew@aj.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
>
>> ---
>> 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 0000000000..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(&opb->as, addr, MEMTXATTRS_UNSPECIFIED, data,
>> + len);
>> +}
>> +
>> +uint8_t opb_read8(OPBus *opb, hwaddr addr)
>> +{
>> + MemTxResult tx;
>> + uint8_t data;
>> +
>> + tx = opb_read(opb, addr, &data, 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, &data, 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, &data, 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(&opb->as, addr, MEMTXATTRS_UNSPECIFIED, data,
>> + len);
>> +}
>> +
>> +void opb_write8(OPBus *opb, hwaddr addr, uint8_t data)
>> +{
>> + MemTxResult tx;
>> +
>> + tx = opb_write(opb, addr, &data, 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, &data, 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, &data, 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(&opb->fsi.iomem, addr);
>> + memory_region_transaction_commit();
>> +}
>> +
>> +void opb_opb2fsi_address(OPBus *opb, hwaddr addr)
>> +{
>> + memory_region_transaction_begin();
>> + memory_region_set_address(&opb->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 opb_realize(BusState *bus, Error **errp)
>> +{
>> + OPBus *opb = OP_BUS(bus);
>> + Error *err = NULL;
>> +
>> + memory_region_init_io(&opb->mr, OBJECT(opb), &opb_unimplemented_ops, opb,
>> + NULL, UINT32_MAX);
>> + address_space_init(&opb->as, &opb->mr, "opb");
>> +
>> + object_property_set_bool(OBJECT(&opb->fsi), "realized", true, &err);
>> + if (err) {
>> + error_propagate(errp, err);
>> + return;
>> + }
>> + memory_region_add_subregion(&opb->mr, 0x80000000, &opb->fsi.iomem);
>> +
>> + /* OPB2FSI region */
>> + /*
>> + * Avoid endianness issues by mapping each slave's memory region directly.
>> + * Manually bridging multiple address-spaces causes endian swapping
>> + * headaches as memory_region_dispatch_read() and
>> + * memory_region_dispatch_write() correct the endianness based on the
>> + * target machine endianness and not relative to the device endianness on
>> + * either side of the bridge.
>> + */
>> + /*
>> + * XXX: This is a bit hairy and will need to be fixed when I sort out the
>> + * bus/slave relationship and any changes to the CFAM modelling (multiple
>> + * slaves, LBUS)
>> + */
>> + memory_region_add_subregion(&opb->mr, 0xa0000000, &opb->fsi.opb2fsi);
>> +}
>> +
>> +static void opb_init(Object *o)
>> +{
>> + OPBus *opb = OP_BUS(o);
>> +
>> + object_initialize_child(o, "fsi-master", &opb->fsi, TYPE_FSI_MASTER);
>> + qdev_set_parent_bus(DEVICE(&opb->fsi), BUS(o), &error_abort);
>> +}
>> +
>> +static void opb_finalize(Object *o)
>> +{
>> + OPBus *opb = OP_BUS(o);
>> +
>> + address_space_destroy(&opb->as);
>> +}
>> +
>> +static void opb_class_init(ObjectClass *klass, void *data)
>> +{
>> + BusClass *bc = BUS_CLASS(klass);
>> + bc->realize = opb_realize;
>> +}
>> +
>> +static const TypeInfo opb_info = {
>> + .name = TYPE_OP_BUS,
>> + .parent = TYPE_BUS,
>> + .instance_init = opb_init,
>> + .instance_finalize = opb_finalize,
>> + .instance_size = sizeof(OPBus),
>> + .class_init = opb_class_init,
>> + .class_size = sizeof(OPBusClass),
>> +};
>> +
>> +static void opb_register_types(void)
>> +{
>> + type_register_static(&opb_info);
>> +}
>> +
>> +type_init(opb_register_types);
>> diff --git a/include/hw/fsi/opb.h b/include/hw/fsi/opb.h
>> new file mode 100644
>> index 0000000000..fad40eed79
>> --- /dev/null
>> +++ b/include/hw/fsi/opb.h
>> @@ -0,0 +1,45 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + * Copyright (C) 2023 IBM Corp.
>> + *
>> + * IBM On-Chip Peripheral Bus
>> + */
>> +#ifndef FSI_OPB_H
>> +#define FSI_OPB_H
>> +
>> +#include "exec/memory.h"
>> +#include "hw/fsi/fsi-master.h"
>> +
>> +#define TYPE_OP_BUS "opb"
>> +#define OP_BUS(obj) OBJECT_CHECK(OPBus, (obj), TYPE_OP_BUS)
>> +#define OP_BUS_CLASS(klass) OBJECT_CLASS_CHECK(OPBusClass, (klass), TYPE_OP_BUS)
>> +#define OP_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(OPBusClass, (obj), TYPE_OP_BUS)
>> +
>> +typedef struct OPBus {
>> + /*< private >*/
>> + BusState bus;
>> +
>> + /*< public >*/
>> + MemoryRegion mr;
>> + AddressSpace as;
>> +
>> + /* Model OPB as dumb enough just to provide an address-space */
>> + /* TODO: Maybe don't store device state in the bus? */
>> + FSIMasterState fsi;
>> +} OPBus;
>> +
>> +typedef struct OPBusClass {
>> + BusClass parent_class;
>> +} OPBusClass;
>> +
>> +uint8_t opb_read8(OPBus *opb, hwaddr addr);
>> +uint16_t opb_read16(OPBus *opb, hwaddr addr);
>> +uint32_t opb_read32(OPBus *opb, hwaddr addr);
>> +void opb_write8(OPBus *opb, hwaddr addr, uint8_t data);
>> +void opb_write16(OPBus *opb, hwaddr addr, uint16_t data);
>> +void opb_write32(OPBus *opb, hwaddr addr, uint32_t data);
>> +
>> +void opb_fsi_master_address(OPBus *opb, hwaddr addr);
>> +void opb_opb2fsi_address(OPBus *opb, hwaddr addr);
>> +
>> +#endif /* FSI_OPB_H */
>> --
>> 2.39.2
>>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v1 6/7] hw/fsi: Aspeed APB2OPB interface
2023-08-25 20:30 [PATCH v1 0/7] Introduce model for IBM's FSP Ninad Palsule
` (4 preceding siblings ...)
2023-08-25 20:30 ` [PATCH v1 5/7] hw/fsi: IBM's On-chip Peripheral Bus Ninad Palsule
@ 2023-08-25 20:30 ` Ninad Palsule
2023-08-28 8:55 ` Joel Stanley
2023-08-25 20:30 ` [PATCH v1 7/7] hw/arm: Hook up FSI module in AST2600 Ninad Palsule
` (2 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Ninad Palsule @ 2023-08-25 20:30 UTC (permalink / raw)
To: qemu-devel, clg, peter.maydell, andrew, joel, pbonzini,
marcandre.lureau, berrange, thuth, philmd
Cc: Ninad Palsule, qemu-arm
This is a part of patchset where IBM's Flexible Service Interface is
introduced.
An APB-to-OPB bridge enabling access to the OPB from the ARM core in
the AST2600. Hardware limitations prevent the OPB from being directly
mapped into APB, so all accesses are indirect through the bridge.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
---
hw/arm/Kconfig | 1 +
hw/fsi/Kconfig | 4 +
hw/fsi/aspeed-apb2opb.c | 346 ++++++++++++++++++++++++++++++++
hw/fsi/meson.build | 1 +
hw/fsi/trace-events | 2 +
hw/fsi/trace.h | 1 +
include/hw/fsi/aspeed-apb2opb.h | 32 +++
meson.build | 1 +
8 files changed, 388 insertions(+)
create mode 100644 hw/fsi/aspeed-apb2opb.c
create mode 100644 hw/fsi/trace-events
create mode 100644 hw/fsi/trace.h
create mode 100644 include/hw/fsi/aspeed-apb2opb.h
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 7e68348440..a6994cd9d7 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -589,6 +589,7 @@ config FSL_IMX7
select PCI_EXPRESS_DESIGNWARE
select SDHCI
select UNIMP
+ select APB2OPB_ASPEED
config ARM_SMMUV3
bool
diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
index 560ce536db..fbb021658d 100644
--- a/hw/fsi/Kconfig
+++ b/hw/fsi/Kconfig
@@ -1,3 +1,7 @@
+config APB2OPB_ASPEED
+ bool
+ select OPB
+
config OPB
bool
select CFAM
diff --git a/hw/fsi/aspeed-apb2opb.c b/hw/fsi/aspeed-apb2opb.c
new file mode 100644
index 0000000000..bbc63f2eb3
--- /dev/null
+++ b/hw/fsi/aspeed-apb2opb.c
@@ -0,0 +1,346 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * ASPEED APB-OPB FSI interface
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qom/object.h"
+#include "qapi/error.h"
+#include "trace.h"
+
+#include "hw/fsi/aspeed-apb2opb.h"
+#include "hw/qdev-core.h"
+
+#define TO_REG(x) (x >> 2)
+#define GENMASK(t, b) (((1ULL << ((t) + 1)) - 1) & ~((1ULL << (b)) - 1))
+
+#define APB2OPB_VERSION TO_REG(0x00)
+#define APB2OPB_VERSION_VER GENMASK(7, 0)
+
+#define APB2OPB_TRIGGER TO_REG(0x04)
+#define APB2OPB_TRIGGER_EN BIT(0)
+
+#define APB2OPB_CONTROL TO_REG(0x08)
+#define APB2OPB_CONTROL_OFF GENMASK(31, 13)
+
+#define APB2OPB_OPB2FSI TO_REG(0x0c)
+#define APB2OPB_OPB2FSI_OFF GENMASK(31, 22)
+
+#define APB2OPB_OPB0_SEL TO_REG(0x10)
+#define APB2OPB_OPB1_SEL TO_REG(0x28)
+#define APB2OPB_OPB_SEL_EN BIT(0)
+
+#define APB2OPB_OPB0_MODE TO_REG(0x14)
+#define APB2OPB_OPB1_MODE TO_REG(0x2c)
+#define APB2OPB_OPB_MODE_RD BIT(0)
+
+#define APB2OPB_OPB0_XFER TO_REG(0x18)
+#define APB2OPB_OPB1_XFER TO_REG(0x30)
+#define APB2OPB_OPB_XFER_FULL BIT(1)
+#define APB2OPB_OPB_XFER_HALF BIT(0)
+
+#define APB2OPB_OPB0_ADDR TO_REG(0x1c)
+#define APB2OPB_OPB0_WRITE_DATA TO_REG(0x20)
+
+#define APB2OPB_OPB1_DMA_EN TO_REG(0x24)
+#define APB2OPB_OPB1_DMA_EN_3 BIT(3)
+#define APB2OPB_OPB1_DMA_EN_2 BIT(2)
+#define APB2OPB_OPB1_DMA_EN_1 BIT(1)
+#define APB2OPB_OPB1_DMA_EN_0 BIT(0)
+
+#define APB2OPB_OPB1_ADDR TO_REG(0x34)
+#define APB2OPB_OPB1_WRITE_DATA TO_REG(0x38)
+
+#define APB2OPB_OPB_CLK TO_REG(0x3c)
+#define APB2OPB_OPB_CLK_SYNC BIT(0)
+
+#define APB2OPB_IRQ_CLEAR TO_REG(0x40)
+#define APB2OPB_IRQ_CLEAR_EN BIT(0)
+
+#define APB2OPB_IRQ_MASK TO_REG(0x44)
+#define APB2OPB_IRQ_MASK_OPB1_TX_ACK BIT(17)
+#define APB2OPB_IRQ_MASK_OPB0_TX_ACK BIT(16)
+#define APB2OPB_IRQ_MASK_CH3_TCONT BIT(15)
+#define APB2OPB_IRQ_MASK_CH2_TCONT BIT(14)
+#define APB2OPB_IRQ_MASK_CH1_TCONT BIT(13)
+#define APB2OPB_IRQ_MASK_CH0_TCONT BIT(12)
+#define APB2OPB_IRQ_MASK_CH3_FIFO_EMPTY BIT(11)
+#define APB2OPB_IRQ_MASK_CH2_FIFO_EMPTY BIT(10)
+#define APB2OPB_IRQ_MASK_CH1_FIFO_EMPTY BIT(9)
+#define APB2OPB_IRQ_MASK_CH0_FIFO_EMPTY BIT(8)
+#define APB2OPB_IRQ_MASK_CH3_FIFO_FULL BIT(7)
+#define APB2OPB_IRQ_MASK_CH2_FIFO_FULL BIT(6)
+#define APB2OPB_IRQ_MASK_CH1_FIFO_FULL BIT(5)
+#define APB2OPB_IRQ_MASK_CH0_FIFO_FULL BIT(4)
+#define APB2OPB_IRQ_MASK_CH3_DMA_EOT BIT(3)
+#define APB2OPB_IRQ_MASK_CH2_DMA_EOT BIT(2)
+#define APB2OPB_IRQ_MASK_CH1_DMA_EOT BIT(1)
+#define APB2OPB_IRQ_MASK_CH0_DMA_EOT BIT(0)
+
+#define APB2OPB_IRQ_STS TO_REG(0x48)
+#define APB2OPB_IRQ_STS_MASTER_ERROR BIT(28)
+#define APB2OPB_IRQ_STS_PORT_ERROR BIT(27)
+#define APB2OPB_IRQ_STS_HOTPLUG BIT(26)
+#define APB2OPB_IRQ_STS_SLAVE_7 BIT(25)
+#define APB2OPB_IRQ_STS_SLAVE_6 BIT(24)
+#define APB2OPB_IRQ_STS_SLAVE_5 BIT(23)
+#define APB2OPB_IRQ_STS_SLAVE_4 BIT(22)
+#define APB2OPB_IRQ_STS_SLAVE_3 BIT(21)
+#define APB2OPB_IRQ_STS_SLAVE_2 BIT(20)
+#define APB2OPB_IRQ_STS_SLAVE_1 BIT(19)
+#define APB2OPB_IRQ_STS_SLAVE_0 BIT(18)
+#define APB2OPB_IRQ_STS_OPB1_TX_ACK BIT(17)
+#define APB2OPB_IRQ_STS_OPB0_TX_ACK BIT(16)
+#define APB2OPB_IRQ_STS_CH3_TCONT BIT(15)
+#define APB2OPB_IRQ_STS_CH2_TCONT BIT(14)
+#define APB2OPB_IRQ_STS_CH1_TCONT BIT(13)
+#define APB2OPB_IRQ_STS_CH0_TCONT BIT(12)
+#define APB2OPB_IRQ_STS_CH3_FIFO_EMPTY BIT(11)
+#define APB2OPB_IRQ_STS_CH2_FIFO_EMPTY BIT(10)
+#define APB2OPB_IRQ_STS_CH1_FIFO_EMPTY BIT(9)
+#define APB2OPB_IRQ_STS_CH0_FIFO_EMPTY BIT(8)
+#define APB2OPB_IRQ_STS_CH3_FIFO_FULL BIT(7)
+#define APB2OPB_IRQ_STS_CH2_FIFO_FULL BIT(6)
+#define APB2OPB_IRQ_STS_CH1_FIFO_FULL BIT(5)
+#define APB2OPB_IRQ_STS_CH0_FIFO_FULL BIT(4)
+#define APB2OPB_IRQ_STS_CH3_DMA_EOT BIT(3)
+#define APB2OPB_IRQ_STS_CH2_DMA_EOT BIT(2)
+#define APB2OPB_IRQ_STS_CH1_DMA_EOT BIT(1)
+#define APB2OPB_IRQ_STS_CH0_DMA_EOT BIT(0)
+
+#define APB2OPB_OPB0_WRITE_WORD_ENDIAN TO_REG(0x4c)
+#define APB2OPB_OPB0_WRITE_WORD_ENDIAN_BE 0x0011101b
+#define APB2OPB_OPB0_WRITE_BYTE_ENDIAN TO_REG(0x50)
+#define APB2OPB_OPB0_WRITE_BYTE_ENDIAN_BE 0x0c330f3f
+#define APB2OPB_OPB1_WRITE_WORD_ENDIAN TO_REG(0x54)
+#define APB2OPB_OPB1_WRITE_BYTE_ENDIAN TO_REG(0x58)
+#define APB2OPB_OPB0_READ_BYTE_ENDIAN TO_REG(0x5c)
+#define APB2OPB_OPB0_READ_WORD_ENDIAN_BE 0x00030b1b
+#define APB2OPB_OPB1_READ_BYTE_ENDIAN TO_REG(0x60)
+
+#define APB2OPB_RETRY TO_REG(0x64)
+#define APB2OPB_RETRY_COUNTER GENMASK(15, 0)
+
+#define APB2OPB_OPB0_STATUS TO_REG(0x80)
+#define APB2OPB_OPB1_STATUS TO_REG(0x8c)
+#define APB2OPB_OPB_STATUS_TIMEOUT BIT(4)
+#define APB2OPB_OPB_STATUS_RETRY BIT(3)
+#define APB2OPB_OPB_STATUS_ERROR_ACK BIT(2)
+#define APB2OPB_OPB_STATUS_FW_ACK BIT(1)
+#define APB2OPB_OPB_STATUS_HW_ACK BIT(0)
+
+#define APB2OPB_OPB0_READ_DATA TO_REG(0x84)
+
+#define APB2OPB_OPB1_DMA_STATUS TO_REG(0x88)
+#define APB2OPB_OPB1_DMA_STATUS_CH3_EOT BIT(7)
+#define APB2OPB_OPB1_DMA_STATUS_CH2_EOT BIT(6)
+#define APB2OPB_OPB1_DMA_STATUS_CH1_EOT BIT(5)
+#define APB2OPB_OPB1_DMA_STATUS_CH0_EOT BIT(4)
+#define APB2OPB_OPB1_DMA_STATUS_CH3_REQ BIT(3)
+#define APB2OPB_OPB1_DMA_STATUS_CH2_REQ BIT(2)
+#define APB2OPB_OPB1_DMA_STATUS_CH1_REQ BIT(1)
+#define APB2OPB_OPB1_DMA_STATUS_CH0_REQ BIT(0)
+
+#define APB2OPB_OPB1_READ_DATA TO_REG(0x90)
+
+static uint64_t aspeed_apb2opb_read(void *opaque, hwaddr addr, unsigned size)
+{
+ AspeedAPB2OPBState *s = ASPEED_APB2OPB(opaque);
+
+ trace_aspeed_apb2opb_read(addr, size);
+
+ assert(!(addr & 3));
+ assert(size == 4);
+
+ if (addr + size > sizeof(s->regs)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Out of bounds read: 0x%"HWADDR_PRIx" for %u\n",
+ __func__, addr, size);
+ return 0;
+ }
+
+ return s->regs[TO_REG(addr)];
+}
+
+static void aspeed_apb2opb_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned size)
+{
+ AspeedAPB2OPBState *s = ASPEED_APB2OPB(opaque);
+
+ trace_aspeed_apb2opb_write(addr, size, data);
+
+ assert(!(addr & 3));
+ assert(size == 4);
+
+ if (addr + size > sizeof(s->regs)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Out of bounds write: %"HWADDR_PRIx" for %u\n",
+ __func__, addr, size);
+ return;
+ }
+
+ switch (TO_REG(addr)) {
+ case APB2OPB_CONTROL:
+ opb_fsi_master_address(&s->opb[0], data & APB2OPB_CONTROL_OFF);
+ break;
+ case APB2OPB_OPB2FSI:
+ opb_opb2fsi_address(&s->opb[0], data & APB2OPB_OPB2FSI_OFF);
+ break;
+ case APB2OPB_OPB0_WRITE_WORD_ENDIAN:
+ if (data != APB2OPB_OPB0_WRITE_WORD_ENDIAN_BE) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Bridge needs to be driven as BE (0x%x)\n",
+ __func__, APB2OPB_OPB0_WRITE_WORD_ENDIAN_BE);
+ }
+ break;
+ case APB2OPB_OPB0_WRITE_BYTE_ENDIAN:
+ if (data != APB2OPB_OPB0_WRITE_BYTE_ENDIAN_BE) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Bridge needs to be driven as BE (0x%x)\n",
+ __func__, APB2OPB_OPB0_WRITE_BYTE_ENDIAN_BE);
+ }
+ break;
+ case APB2OPB_OPB0_READ_BYTE_ENDIAN:
+ if (data != APB2OPB_OPB0_READ_WORD_ENDIAN_BE) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Bridge needs to be driven as BE (0x%x)\n",
+ __func__, APB2OPB_OPB0_READ_WORD_ENDIAN_BE);
+ }
+ break;
+ case APB2OPB_TRIGGER:
+ {
+ uint32_t opb, op_mode, op_size, op_addr, op_data;
+
+ assert((s->regs[APB2OPB_OPB0_SEL] & APB2OPB_OPB_SEL_EN) ^
+ (s->regs[APB2OPB_OPB1_SEL] & APB2OPB_OPB_SEL_EN));
+
+ if (s->regs[APB2OPB_OPB0_SEL] & APB2OPB_OPB_SEL_EN) {
+ opb = 0;
+ op_mode = s->regs[APB2OPB_OPB0_MODE];
+ op_size = s->regs[APB2OPB_OPB0_XFER];
+ op_addr = s->regs[APB2OPB_OPB0_ADDR];
+ op_data = s->regs[APB2OPB_OPB0_WRITE_DATA];
+ } else if (s->regs[APB2OPB_OPB1_SEL] & APB2OPB_OPB_SEL_EN) {
+ opb = 1;
+ op_mode = s->regs[APB2OPB_OPB1_MODE];
+ op_size = s->regs[APB2OPB_OPB1_XFER];
+ op_addr = s->regs[APB2OPB_OPB1_ADDR];
+ op_data = s->regs[APB2OPB_OPB1_WRITE_DATA];
+ } else {
+ g_assert_not_reached();
+ }
+
+ if (op_size & ~(APB2OPB_OPB_XFER_HALF | APB2OPB_OPB_XFER_FULL)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "OPB transaction failed: Unrecognised access width: %d\n",
+ op_size);
+ return;
+ }
+
+ op_size += 1;
+
+ if (op_mode & APB2OPB_OPB_MODE_RD) {
+ int index = opb ? APB2OPB_OPB1_READ_DATA
+ : APB2OPB_OPB0_READ_DATA;
+
+ switch (op_size) {
+ case 1:
+ s->regs[index] = opb_read8(&s->opb[opb], op_addr);
+ break;
+ case 2:
+ s->regs[index] = opb_read16(&s->opb[opb], op_addr);
+ break;
+ case 4:
+ s->regs[index] = opb_read32(&s->opb[opb], op_addr);
+ break;
+ default:
+ g_assert_not_reached(); /* should have bailed above */
+ }
+ } else {
+ /* FIXME: Endian swizzling */
+ switch (op_size) {
+ case 1:
+ opb_write8(&s->opb[opb], op_addr, op_data);
+ break;
+ case 2:
+ opb_write16(&s->opb[opb], op_addr, op_data);
+ break;
+ case 4:
+ opb_write32(&s->opb[opb], op_addr, op_data);
+ break;
+ default:
+ g_assert_not_reached(); /* should have bailed above */
+ }
+ }
+ s->regs[APB2OPB_IRQ_STS] |= opb ? APB2OPB_IRQ_STS_OPB1_TX_ACK
+ : APB2OPB_IRQ_STS_OPB0_TX_ACK;
+ break;
+ }
+ }
+
+ s->regs[TO_REG(addr)] = data;
+}
+
+static const struct MemoryRegionOps aspeed_apb2opb_ops = {
+ .read = aspeed_apb2opb_read,
+ .write = aspeed_apb2opb_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void aspeed_apb2opb_realize(DeviceState *dev, Error **errp)
+{
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+ AspeedAPB2OPBState *s = ASPEED_APB2OPB(dev);
+
+ qbus_init(&s->opb[0], sizeof(s->opb[0]), TYPE_OP_BUS,
+ DEVICE(s), NULL);
+ qbus_init(&s->opb[1], sizeof(s->opb[1]), TYPE_OP_BUS,
+ DEVICE(s), NULL);
+
+ sysbus_init_irq(sbd, &s->irq);
+
+ memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_apb2opb_ops, s,
+ TYPE_ASPEED_APB2OPB, 0x1000);
+ sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static void aspeed_apb2opb_reset(DeviceState *dev)
+{
+ AspeedAPB2OPBState *s = ASPEED_APB2OPB(dev);
+
+ memset(s->regs, 0, sizeof(s->regs));
+
+ s->regs[APB2OPB_VERSION] = 0x000000a1;
+ s->regs[APB2OPB_OPB0_WRITE_WORD_ENDIAN] = 0x0044eee4;
+ s->regs[APB2OPB_OPB0_WRITE_BYTE_ENDIAN] = 0x0055aaff;
+ s->regs[APB2OPB_OPB1_WRITE_WORD_ENDIAN] = 0x00117717;
+ s->regs[APB2OPB_OPB1_WRITE_BYTE_ENDIAN] = 0xffaa5500;
+ s->regs[APB2OPB_OPB0_READ_BYTE_ENDIAN] = 0x0044eee4;
+ s->regs[APB2OPB_OPB0_READ_BYTE_ENDIAN] = 0x00117717;
+}
+
+static void aspeed_apb2opb_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->desc = "ASPEED APB2OPB Bridge";
+ dc->realize = aspeed_apb2opb_realize;
+ dc->reset = aspeed_apb2opb_reset;
+}
+
+static const TypeInfo aspeed_apb2opb_info = {
+ .name = TYPE_ASPEED_APB2OPB,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(AspeedAPB2OPBState),
+ .class_init = aspeed_apb2opb_class_init,
+};
+
+static void aspeed_apb2opb_register_types(void)
+{
+ type_register_static(&aspeed_apb2opb_info);
+}
+
+type_init(aspeed_apb2opb_register_types);
diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
index cab645f4ea..ce8b9555ce 100644
--- a/hw/fsi/meson.build
+++ b/hw/fsi/meson.build
@@ -3,3 +3,4 @@ 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'))
+system_ss.add(when: 'CONFIG_APB2OPB_ASPEED', if_true: files('aspeed-apb2opb.c'))
diff --git a/hw/fsi/trace-events b/hw/fsi/trace-events
new file mode 100644
index 0000000000..c64245f7f6
--- /dev/null
+++ b/hw/fsi/trace-events
@@ -0,0 +1,2 @@
+aspeed_apb2opb_read(uint64_t addr, uint32_t size) "@0x%" PRIx64 " size=%d"
+aspeed_apb2opb_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size=%d value=0x%"PRIx64
diff --git a/hw/fsi/trace.h b/hw/fsi/trace.h
new file mode 100644
index 0000000000..ee67c7fb04
--- /dev/null
+++ b/hw/fsi/trace.h
@@ -0,0 +1 @@
+#include "trace/trace-hw_fsi.h"
diff --git a/include/hw/fsi/aspeed-apb2opb.h b/include/hw/fsi/aspeed-apb2opb.h
new file mode 100644
index 0000000000..4acc980e7d
--- /dev/null
+++ b/include/hw/fsi/aspeed-apb2opb.h
@@ -0,0 +1,32 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * ASPEED APB2OPB Bridge
+ */
+#ifndef FSI_ASPEED_APB2OPB_H
+#define FSI_ASPEED_APB2OPB_H
+
+#include "hw/sysbus.h"
+#include "hw/fsi/opb.h"
+
+#define TYPE_ASPEED_APB2OPB "aspeed.apb2opb"
+#define ASPEED_APB2OPB(obj) \
+ OBJECT_CHECK(AspeedAPB2OPBState, (obj), TYPE_ASPEED_APB2OPB)
+
+#define ASPEED_APB2OPB_NR_REGS ((0xe8 >> 2) + 1)
+
+typedef struct AspeedAPB2OPBState {
+ /*< private >*/
+ SysBusDevice parent_obj;
+
+ /*< public >*/
+ MemoryRegion iomem;
+
+ uint32_t regs[ASPEED_APB2OPB_NR_REGS];
+ qemu_irq irq;
+
+ OPBus opb[2];
+} AspeedAPB2OPBState;
+
+#endif /* FSI_ASPEED_APB2OPB_H */
diff --git a/meson.build b/meson.build
index 98e68ef0b1..1a722693a6 100644
--- a/meson.build
+++ b/meson.build
@@ -3244,6 +3244,7 @@ if have_system
'hw/char',
'hw/display',
'hw/dma',
+ 'hw/fsi',
'hw/hyperv',
'hw/i2c',
'hw/i386',
--
2.39.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v1 6/7] hw/fsi: Aspeed APB2OPB interface
2023-08-25 20:30 ` [PATCH v1 6/7] hw/fsi: Aspeed APB2OPB interface Ninad Palsule
@ 2023-08-28 8:55 ` Joel Stanley
2023-08-28 22:57 ` Ninad Palsule
0 siblings, 1 reply; 28+ messages in thread
From: Joel Stanley @ 2023-08-28 8:55 UTC (permalink / raw)
To: Ninad Palsule
Cc: qemu-devel, clg, peter.maydell, andrew, pbonzini,
marcandre.lureau, berrange, thuth, philmd, qemu-arm
On Fri, 25 Aug 2023 at 20:31, Ninad Palsule <ninad@linux.ibm.com> wrote:
>
> This is a part of patchset where IBM's Flexible Service Interface is
> introduced.
>
> An APB-to-OPB bridge enabling access to the OPB from the ARM core in
> the AST2600. Hardware limitations prevent the OPB from being directly
> mapped into APB, so all accesses are indirect through the bridge.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
> ---
> hw/arm/Kconfig | 1 +
> hw/fsi/Kconfig | 4 +
> hw/fsi/aspeed-apb2opb.c | 346 ++++++++++++++++++++++++++++++++
> hw/fsi/meson.build | 1 +
> hw/fsi/trace-events | 2 +
> hw/fsi/trace.h | 1 +
> include/hw/fsi/aspeed-apb2opb.h | 32 +++
> meson.build | 1 +
> 8 files changed, 388 insertions(+)
> create mode 100644 hw/fsi/aspeed-apb2opb.c
> create mode 100644 hw/fsi/trace-events
> create mode 100644 hw/fsi/trace.h
> create mode 100644 include/hw/fsi/aspeed-apb2opb.h
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 7e68348440..a6994cd9d7 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -589,6 +589,7 @@ config FSL_IMX7
> select PCI_EXPRESS_DESIGNWARE
> select SDHCI
> select UNIMP
> + select APB2OPB_ASPEED
>
> config ARM_SMMUV3
> bool
> diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
> index 560ce536db..fbb021658d 100644
> --- a/hw/fsi/Kconfig
> +++ b/hw/fsi/Kconfig
> @@ -1,3 +1,7 @@
> +config APB2OPB_ASPEED
> + bool
> + select OPB
> +
> config OPB
> bool
> select CFAM
> diff --git a/hw/fsi/aspeed-apb2opb.c b/hw/fsi/aspeed-apb2opb.c
> new file mode 100644
> index 0000000000..bbc63f2eb3
> --- /dev/null
> +++ b/hw/fsi/aspeed-apb2opb.c
> @@ -0,0 +1,346 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2023 IBM Corp.
> + *
> + * ASPEED APB-OPB FSI interface
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qom/object.h"
> +#include "qapi/error.h"
> +#include "trace.h"
> +
> +#include "hw/fsi/aspeed-apb2opb.h"
> +#include "hw/qdev-core.h"
> +
> +#define TO_REG(x) (x >> 2)
> +#define GENMASK(t, b) (((1ULL << ((t) + 1)) - 1) & ~((1ULL << (b)) - 1))
We should put this in a common header.
> +
> +#define APB2OPB_VERSION TO_REG(0x00)
> +#define APB2OPB_VERSION_VER GENMASK(7, 0)
> +
> +#define APB2OPB_TRIGGER TO_REG(0x04)
> +#define APB2OPB_TRIGGER_EN BIT(0)
> +
> +#define APB2OPB_CONTROL TO_REG(0x08)
> +#define APB2OPB_CONTROL_OFF GENMASK(31, 13)
> +
> +#define APB2OPB_OPB2FSI TO_REG(0x0c)
> +#define APB2OPB_OPB2FSI_OFF GENMASK(31, 22)
> +
> +#define APB2OPB_OPB0_SEL TO_REG(0x10)
> +#define APB2OPB_OPB1_SEL TO_REG(0x28)
> +#define APB2OPB_OPB_SEL_EN BIT(0)
> +
> +#define APB2OPB_OPB0_MODE TO_REG(0x14)
> +#define APB2OPB_OPB1_MODE TO_REG(0x2c)
> +#define APB2OPB_OPB_MODE_RD BIT(0)
> +
> +#define APB2OPB_OPB0_XFER TO_REG(0x18)
> +#define APB2OPB_OPB1_XFER TO_REG(0x30)
> +#define APB2OPB_OPB_XFER_FULL BIT(1)
> +#define APB2OPB_OPB_XFER_HALF BIT(0)
> +
> +#define APB2OPB_OPB0_ADDR TO_REG(0x1c)
> +#define APB2OPB_OPB0_WRITE_DATA TO_REG(0x20)
> +
> +#define APB2OPB_OPB1_DMA_EN TO_REG(0x24)
> +#define APB2OPB_OPB1_DMA_EN_3 BIT(3)
> +#define APB2OPB_OPB1_DMA_EN_2 BIT(2)
> +#define APB2OPB_OPB1_DMA_EN_1 BIT(1)
> +#define APB2OPB_OPB1_DMA_EN_0 BIT(0)
> +
> +#define APB2OPB_OPB1_ADDR TO_REG(0x34)
> +#define APB2OPB_OPB1_WRITE_DATA TO_REG(0x38)
> +
> +#define APB2OPB_OPB_CLK TO_REG(0x3c)
> +#define APB2OPB_OPB_CLK_SYNC BIT(0)
> +
> +#define APB2OPB_IRQ_CLEAR TO_REG(0x40)
> +#define APB2OPB_IRQ_CLEAR_EN BIT(0)
> +
> +#define APB2OPB_IRQ_MASK TO_REG(0x44)
> +#define APB2OPB_IRQ_MASK_OPB1_TX_ACK BIT(17)
> +#define APB2OPB_IRQ_MASK_OPB0_TX_ACK BIT(16)
> +#define APB2OPB_IRQ_MASK_CH3_TCONT BIT(15)
> +#define APB2OPB_IRQ_MASK_CH2_TCONT BIT(14)
> +#define APB2OPB_IRQ_MASK_CH1_TCONT BIT(13)
> +#define APB2OPB_IRQ_MASK_CH0_TCONT BIT(12)
> +#define APB2OPB_IRQ_MASK_CH3_FIFO_EMPTY BIT(11)
> +#define APB2OPB_IRQ_MASK_CH2_FIFO_EMPTY BIT(10)
> +#define APB2OPB_IRQ_MASK_CH1_FIFO_EMPTY BIT(9)
> +#define APB2OPB_IRQ_MASK_CH0_FIFO_EMPTY BIT(8)
> +#define APB2OPB_IRQ_MASK_CH3_FIFO_FULL BIT(7)
> +#define APB2OPB_IRQ_MASK_CH2_FIFO_FULL BIT(6)
> +#define APB2OPB_IRQ_MASK_CH1_FIFO_FULL BIT(5)
> +#define APB2OPB_IRQ_MASK_CH0_FIFO_FULL BIT(4)
> +#define APB2OPB_IRQ_MASK_CH3_DMA_EOT BIT(3)
> +#define APB2OPB_IRQ_MASK_CH2_DMA_EOT BIT(2)
> +#define APB2OPB_IRQ_MASK_CH1_DMA_EOT BIT(1)
> +#define APB2OPB_IRQ_MASK_CH0_DMA_EOT BIT(0)
> +
> +#define APB2OPB_IRQ_STS TO_REG(0x48)
> +#define APB2OPB_IRQ_STS_MASTER_ERROR BIT(28)
> +#define APB2OPB_IRQ_STS_PORT_ERROR BIT(27)
> +#define APB2OPB_IRQ_STS_HOTPLUG BIT(26)
> +#define APB2OPB_IRQ_STS_SLAVE_7 BIT(25)
> +#define APB2OPB_IRQ_STS_SLAVE_6 BIT(24)
> +#define APB2OPB_IRQ_STS_SLAVE_5 BIT(23)
> +#define APB2OPB_IRQ_STS_SLAVE_4 BIT(22)
> +#define APB2OPB_IRQ_STS_SLAVE_3 BIT(21)
> +#define APB2OPB_IRQ_STS_SLAVE_2 BIT(20)
> +#define APB2OPB_IRQ_STS_SLAVE_1 BIT(19)
> +#define APB2OPB_IRQ_STS_SLAVE_0 BIT(18)
> +#define APB2OPB_IRQ_STS_OPB1_TX_ACK BIT(17)
> +#define APB2OPB_IRQ_STS_OPB0_TX_ACK BIT(16)
> +#define APB2OPB_IRQ_STS_CH3_TCONT BIT(15)
> +#define APB2OPB_IRQ_STS_CH2_TCONT BIT(14)
> +#define APB2OPB_IRQ_STS_CH1_TCONT BIT(13)
> +#define APB2OPB_IRQ_STS_CH0_TCONT BIT(12)
> +#define APB2OPB_IRQ_STS_CH3_FIFO_EMPTY BIT(11)
> +#define APB2OPB_IRQ_STS_CH2_FIFO_EMPTY BIT(10)
> +#define APB2OPB_IRQ_STS_CH1_FIFO_EMPTY BIT(9)
> +#define APB2OPB_IRQ_STS_CH0_FIFO_EMPTY BIT(8)
> +#define APB2OPB_IRQ_STS_CH3_FIFO_FULL BIT(7)
> +#define APB2OPB_IRQ_STS_CH2_FIFO_FULL BIT(6)
> +#define APB2OPB_IRQ_STS_CH1_FIFO_FULL BIT(5)
> +#define APB2OPB_IRQ_STS_CH0_FIFO_FULL BIT(4)
> +#define APB2OPB_IRQ_STS_CH3_DMA_EOT BIT(3)
> +#define APB2OPB_IRQ_STS_CH2_DMA_EOT BIT(2)
> +#define APB2OPB_IRQ_STS_CH1_DMA_EOT BIT(1)
> +#define APB2OPB_IRQ_STS_CH0_DMA_EOT BIT(0)
> +
> +#define APB2OPB_OPB0_WRITE_WORD_ENDIAN TO_REG(0x4c)
> +#define APB2OPB_OPB0_WRITE_WORD_ENDIAN_BE 0x0011101b
> +#define APB2OPB_OPB0_WRITE_BYTE_ENDIAN TO_REG(0x50)
> +#define APB2OPB_OPB0_WRITE_BYTE_ENDIAN_BE 0x0c330f3f
> +#define APB2OPB_OPB1_WRITE_WORD_ENDIAN TO_REG(0x54)
> +#define APB2OPB_OPB1_WRITE_BYTE_ENDIAN TO_REG(0x58)
> +#define APB2OPB_OPB0_READ_BYTE_ENDIAN TO_REG(0x5c)
> +#define APB2OPB_OPB0_READ_WORD_ENDIAN_BE 0x00030b1b
> +#define APB2OPB_OPB1_READ_BYTE_ENDIAN TO_REG(0x60)
> +
> +#define APB2OPB_RETRY TO_REG(0x64)
> +#define APB2OPB_RETRY_COUNTER GENMASK(15, 0)
> +
> +#define APB2OPB_OPB0_STATUS TO_REG(0x80)
> +#define APB2OPB_OPB1_STATUS TO_REG(0x8c)
> +#define APB2OPB_OPB_STATUS_TIMEOUT BIT(4)
> +#define APB2OPB_OPB_STATUS_RETRY BIT(3)
> +#define APB2OPB_OPB_STATUS_ERROR_ACK BIT(2)
> +#define APB2OPB_OPB_STATUS_FW_ACK BIT(1)
> +#define APB2OPB_OPB_STATUS_HW_ACK BIT(0)
> +
> +#define APB2OPB_OPB0_READ_DATA TO_REG(0x84)
> +
> +#define APB2OPB_OPB1_DMA_STATUS TO_REG(0x88)
> +#define APB2OPB_OPB1_DMA_STATUS_CH3_EOT BIT(7)
> +#define APB2OPB_OPB1_DMA_STATUS_CH2_EOT BIT(6)
> +#define APB2OPB_OPB1_DMA_STATUS_CH1_EOT BIT(5)
> +#define APB2OPB_OPB1_DMA_STATUS_CH0_EOT BIT(4)
> +#define APB2OPB_OPB1_DMA_STATUS_CH3_REQ BIT(3)
> +#define APB2OPB_OPB1_DMA_STATUS_CH2_REQ BIT(2)
> +#define APB2OPB_OPB1_DMA_STATUS_CH1_REQ BIT(1)
> +#define APB2OPB_OPB1_DMA_STATUS_CH0_REQ BIT(0)
> +
> +#define APB2OPB_OPB1_READ_DATA TO_REG(0x90)
> +
> +static uint64_t aspeed_apb2opb_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + AspeedAPB2OPBState *s = ASPEED_APB2OPB(opaque);
> +
> + trace_aspeed_apb2opb_read(addr, size);
> +
> + assert(!(addr & 3));
> + assert(size == 4);
> +
> + if (addr + size > sizeof(s->regs)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Out of bounds read: 0x%"HWADDR_PRIx" for %u\n",
> + __func__, addr, size);
> + return 0;
> + }
> +
> + return s->regs[TO_REG(addr)];
> +}
> +
> +static void aspeed_apb2opb_write(void *opaque, hwaddr addr, uint64_t data,
> + unsigned size)
> +{
> + AspeedAPB2OPBState *s = ASPEED_APB2OPB(opaque);
> +
> + trace_aspeed_apb2opb_write(addr, size, data);
> +
> + assert(!(addr & 3));
> + assert(size == 4);
> +
> + if (addr + size > sizeof(s->regs)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Out of bounds write: %"HWADDR_PRIx" for %u\n",
> + __func__, addr, size);
> + return;
> + }
> +
> + switch (TO_REG(addr)) {
> + case APB2OPB_CONTROL:
> + opb_fsi_master_address(&s->opb[0], data & APB2OPB_CONTROL_OFF);
> + break;
> + case APB2OPB_OPB2FSI:
> + opb_opb2fsi_address(&s->opb[0], data & APB2OPB_OPB2FSI_OFF);
> + break;
> + case APB2OPB_OPB0_WRITE_WORD_ENDIAN:
> + if (data != APB2OPB_OPB0_WRITE_WORD_ENDIAN_BE) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Bridge needs to be driven as BE (0x%x)\n",
> + __func__, APB2OPB_OPB0_WRITE_WORD_ENDIAN_BE);
> + }
> + break;
> + case APB2OPB_OPB0_WRITE_BYTE_ENDIAN:
> + if (data != APB2OPB_OPB0_WRITE_BYTE_ENDIAN_BE) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Bridge needs to be driven as BE (0x%x)\n",
> + __func__, APB2OPB_OPB0_WRITE_BYTE_ENDIAN_BE);
> + }
> + break;
> + case APB2OPB_OPB0_READ_BYTE_ENDIAN:
> + if (data != APB2OPB_OPB0_READ_WORD_ENDIAN_BE) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Bridge needs to be driven as BE (0x%x)\n",
> + __func__, APB2OPB_OPB0_READ_WORD_ENDIAN_BE);
> + }
> + break;
> + case APB2OPB_TRIGGER:
> + {
> + uint32_t opb, op_mode, op_size, op_addr, op_data;
> +
> + assert((s->regs[APB2OPB_OPB0_SEL] & APB2OPB_OPB_SEL_EN) ^
> + (s->regs[APB2OPB_OPB1_SEL] & APB2OPB_OPB_SEL_EN));
> +
> + if (s->regs[APB2OPB_OPB0_SEL] & APB2OPB_OPB_SEL_EN) {
> + opb = 0;
> + op_mode = s->regs[APB2OPB_OPB0_MODE];
> + op_size = s->regs[APB2OPB_OPB0_XFER];
> + op_addr = s->regs[APB2OPB_OPB0_ADDR];
> + op_data = s->regs[APB2OPB_OPB0_WRITE_DATA];
> + } else if (s->regs[APB2OPB_OPB1_SEL] & APB2OPB_OPB_SEL_EN) {
> + opb = 1;
> + op_mode = s->regs[APB2OPB_OPB1_MODE];
> + op_size = s->regs[APB2OPB_OPB1_XFER];
> + op_addr = s->regs[APB2OPB_OPB1_ADDR];
> + op_data = s->regs[APB2OPB_OPB1_WRITE_DATA];
> + } else {
> + g_assert_not_reached();
> + }
> +
> + if (op_size & ~(APB2OPB_OPB_XFER_HALF | APB2OPB_OPB_XFER_FULL)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "OPB transaction failed: Unrecognised access width: %d\n",
> + op_size);
> + return;
> + }
> +
> + op_size += 1;
> +
> + if (op_mode & APB2OPB_OPB_MODE_RD) {
> + int index = opb ? APB2OPB_OPB1_READ_DATA
> + : APB2OPB_OPB0_READ_DATA;
> +
> + switch (op_size) {
> + case 1:
> + s->regs[index] = opb_read8(&s->opb[opb], op_addr);
> + break;
> + case 2:
> + s->regs[index] = opb_read16(&s->opb[opb], op_addr);
> + break;
> + case 4:
> + s->regs[index] = opb_read32(&s->opb[opb], op_addr);
> + break;
> + default:
> + g_assert_not_reached(); /* should have bailed above */
> + }
> + } else {
> + /* FIXME: Endian swizzling */
> + switch (op_size) {
> + case 1:
> + opb_write8(&s->opb[opb], op_addr, op_data);
> + break;
> + case 2:
> + opb_write16(&s->opb[opb], op_addr, op_data);
> + break;
> + case 4:
> + opb_write32(&s->opb[opb], op_addr, op_data);
> + break;
> + default:
> + g_assert_not_reached(); /* should have bailed above */
> + }
> + }
> + s->regs[APB2OPB_IRQ_STS] |= opb ? APB2OPB_IRQ_STS_OPB1_TX_ACK
> + : APB2OPB_IRQ_STS_OPB0_TX_ACK;
> + break;
> + }
> + }
> +
> + s->regs[TO_REG(addr)] = data;
> +}
> +
> +static const struct MemoryRegionOps aspeed_apb2opb_ops = {
> + .read = aspeed_apb2opb_read,
> + .write = aspeed_apb2opb_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static void aspeed_apb2opb_realize(DeviceState *dev, Error **errp)
> +{
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + AspeedAPB2OPBState *s = ASPEED_APB2OPB(dev);
> +
> + qbus_init(&s->opb[0], sizeof(s->opb[0]), TYPE_OP_BUS,
> + DEVICE(s), NULL);
> + qbus_init(&s->opb[1], sizeof(s->opb[1]), TYPE_OP_BUS,
> + DEVICE(s), NULL);
> +
> + sysbus_init_irq(sbd, &s->irq);
> +
> + memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_apb2opb_ops, s,
> + TYPE_ASPEED_APB2OPB, 0x1000);
> + sysbus_init_mmio(sbd, &s->iomem);
> +}
> +
> +static void aspeed_apb2opb_reset(DeviceState *dev)
> +{
> + AspeedAPB2OPBState *s = ASPEED_APB2OPB(dev);
> +
> + memset(s->regs, 0, sizeof(s->regs));
> +
> + s->regs[APB2OPB_VERSION] = 0x000000a1;
> + s->regs[APB2OPB_OPB0_WRITE_WORD_ENDIAN] = 0x0044eee4;
> + s->regs[APB2OPB_OPB0_WRITE_BYTE_ENDIAN] = 0x0055aaff;
> + s->regs[APB2OPB_OPB1_WRITE_WORD_ENDIAN] = 0x00117717;
> + s->regs[APB2OPB_OPB1_WRITE_BYTE_ENDIAN] = 0xffaa5500;
> + s->regs[APB2OPB_OPB0_READ_BYTE_ENDIAN] = 0x0044eee4;
> + s->regs[APB2OPB_OPB0_READ_BYTE_ENDIAN] = 0x00117717;
Add a comment about the source of these magic numbers. I think this is
the best reference we have:
https://git.kernel.org/torvalds/c/5e5022995683539609b3904213b7d72ab71188b7
I'm not sure if the reset values were taken from the hardware, or the
kernel driver.
> +}
> +
> +static void aspeed_apb2opb_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->desc = "ASPEED APB2OPB Bridge";
> + dc->realize = aspeed_apb2opb_realize;
> + dc->reset = aspeed_apb2opb_reset;
> +}
> +
> +static const TypeInfo aspeed_apb2opb_info = {
> + .name = TYPE_ASPEED_APB2OPB,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(AspeedAPB2OPBState),
> + .class_init = aspeed_apb2opb_class_init,
> +};
> +
> +static void aspeed_apb2opb_register_types(void)
> +{
> + type_register_static(&aspeed_apb2opb_info);
> +}
> +
> +type_init(aspeed_apb2opb_register_types);
> diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
> index cab645f4ea..ce8b9555ce 100644
> --- a/hw/fsi/meson.build
> +++ b/hw/fsi/meson.build
> @@ -3,3 +3,4 @@ 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'))
> +system_ss.add(when: 'CONFIG_APB2OPB_ASPEED', if_true: files('aspeed-apb2opb.c'))
> diff --git a/hw/fsi/trace-events b/hw/fsi/trace-events
> new file mode 100644
> index 0000000000..c64245f7f6
> --- /dev/null
> +++ b/hw/fsi/trace-events
> @@ -0,0 +1,2 @@
> +aspeed_apb2opb_read(uint64_t addr, uint32_t size) "@0x%" PRIx64 " size=%d"
> +aspeed_apb2opb_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size=%d value=0x%"PRIx64
> diff --git a/hw/fsi/trace.h b/hw/fsi/trace.h
> new file mode 100644
> index 0000000000..ee67c7fb04
> --- /dev/null
> +++ b/hw/fsi/trace.h
> @@ -0,0 +1 @@
> +#include "trace/trace-hw_fsi.h"
> diff --git a/include/hw/fsi/aspeed-apb2opb.h b/include/hw/fsi/aspeed-apb2opb.h
> new file mode 100644
> index 0000000000..4acc980e7d
> --- /dev/null
> +++ b/include/hw/fsi/aspeed-apb2opb.h
> @@ -0,0 +1,32 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2023 IBM Corp.
> + *
> + * ASPEED APB2OPB Bridge
> + */
> +#ifndef FSI_ASPEED_APB2OPB_H
> +#define FSI_ASPEED_APB2OPB_H
> +
> +#include "hw/sysbus.h"
> +#include "hw/fsi/opb.h"
> +
> +#define TYPE_ASPEED_APB2OPB "aspeed.apb2opb"
> +#define ASPEED_APB2OPB(obj) \
> + OBJECT_CHECK(AspeedAPB2OPBState, (obj), TYPE_ASPEED_APB2OPB)
> +
> +#define ASPEED_APB2OPB_NR_REGS ((0xe8 >> 2) + 1)
> +
> +typedef struct AspeedAPB2OPBState {
> + /*< private >*/
> + SysBusDevice parent_obj;
> +
> + /*< public >*/
> + MemoryRegion iomem;
> +
> + uint32_t regs[ASPEED_APB2OPB_NR_REGS];
> + qemu_irq irq;
> +
> + OPBus opb[2];
> +} AspeedAPB2OPBState;
> +
> +#endif /* FSI_ASPEED_APB2OPB_H */
> diff --git a/meson.build b/meson.build
> index 98e68ef0b1..1a722693a6 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3244,6 +3244,7 @@ if have_system
> 'hw/char',
> 'hw/display',
> 'hw/dma',
> + 'hw/fsi',
> 'hw/hyperv',
> 'hw/i2c',
> 'hw/i386',
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 6/7] hw/fsi: Aspeed APB2OPB interface
2023-08-28 8:55 ` Joel Stanley
@ 2023-08-28 22:57 ` Ninad Palsule
0 siblings, 0 replies; 28+ messages in thread
From: Ninad Palsule @ 2023-08-28 22:57 UTC (permalink / raw)
To: Joel Stanley
Cc: qemu-devel, clg, peter.maydell, andrew, pbonzini,
marcandre.lureau, berrange, thuth, philmd, qemu-arm
Hi Joel,
On 8/28/23 23:55, Joel Stanley wrote:
> On Fri, 25 Aug 2023 at 20:31, Ninad Palsule <ninad@linux.ibm.com> wrote:
>> This is a part of patchset where IBM's Flexible Service Interface is
>> introduced.
>>
>> An APB-to-OPB bridge enabling access to the OPB from the ARM core in
>> the AST2600. Hardware limitations prevent the OPB from being directly
>> mapped into APB, so all accesses are indirect through the bridge.
>>
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
>> ---
>> hw/arm/Kconfig | 1 +
>> hw/fsi/Kconfig | 4 +
>> hw/fsi/aspeed-apb2opb.c | 346 ++++++++++++++++++++++++++++++++
>> hw/fsi/meson.build | 1 +
>> hw/fsi/trace-events | 2 +
>> hw/fsi/trace.h | 1 +
>> include/hw/fsi/aspeed-apb2opb.h | 32 +++
>> meson.build | 1 +
>> 8 files changed, 388 insertions(+)
>> create mode 100644 hw/fsi/aspeed-apb2opb.c
>> create mode 100644 hw/fsi/trace-events
>> create mode 100644 hw/fsi/trace.h
>> create mode 100644 include/hw/fsi/aspeed-apb2opb.h
>>
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index 7e68348440..a6994cd9d7 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -589,6 +589,7 @@ config FSL_IMX7
>> select PCI_EXPRESS_DESIGNWARE
>> select SDHCI
>> select UNIMP
>> + select APB2OPB_ASPEED
>>
>> config ARM_SMMUV3
>> bool
>> diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
>> index 560ce536db..fbb021658d 100644
>> --- a/hw/fsi/Kconfig
>> +++ b/hw/fsi/Kconfig
>> @@ -1,3 +1,7 @@
>> +config APB2OPB_ASPEED
>> + bool
>> + select OPB
>> +
>> config OPB
>> bool
>> select CFAM
>> diff --git a/hw/fsi/aspeed-apb2opb.c b/hw/fsi/aspeed-apb2opb.c
>> new file mode 100644
>> index 0000000000..bbc63f2eb3
>> --- /dev/null
>> +++ b/hw/fsi/aspeed-apb2opb.c
>> @@ -0,0 +1,346 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + * Copyright (C) 2023 IBM Corp.
>> + *
>> + * ASPEED APB-OPB FSI interface
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/log.h"
>> +#include "qom/object.h"
>> +#include "qapi/error.h"
>> +#include "trace.h"
>> +
>> +#include "hw/fsi/aspeed-apb2opb.h"
>> +#include "hw/qdev-core.h"
>> +
>> +#define TO_REG(x) (x >> 2)
>> +#define GENMASK(t, b) (((1ULL << ((t) + 1)) - 1) & ~((1ULL << (b)) - 1))
> We should put this in a common header.
Moved it to bitops.h
>
>> +
>> +#define APB2OPB_VERSION TO_REG(0x00)
>> +#define APB2OPB_VERSION_VER GENMASK(7, 0)
>> +
>> +#define APB2OPB_TRIGGER TO_REG(0x04)
>> +#define APB2OPB_TRIGGER_EN BIT(0)
>> +
>> +#define APB2OPB_CONTROL TO_REG(0x08)
>> +#define APB2OPB_CONTROL_OFF GENMASK(31, 13)
>> +
>> +#define APB2OPB_OPB2FSI TO_REG(0x0c)
>> +#define APB2OPB_OPB2FSI_OFF GENMASK(31, 22)
>> +
>> +#define APB2OPB_OPB0_SEL TO_REG(0x10)
>> +#define APB2OPB_OPB1_SEL TO_REG(0x28)
>> +#define APB2OPB_OPB_SEL_EN BIT(0)
>> +
>> +#define APB2OPB_OPB0_MODE TO_REG(0x14)
>> +#define APB2OPB_OPB1_MODE TO_REG(0x2c)
>> +#define APB2OPB_OPB_MODE_RD BIT(0)
>> +
>> +#define APB2OPB_OPB0_XFER TO_REG(0x18)
>> +#define APB2OPB_OPB1_XFER TO_REG(0x30)
>> +#define APB2OPB_OPB_XFER_FULL BIT(1)
>> +#define APB2OPB_OPB_XFER_HALF BIT(0)
>> +
>> +#define APB2OPB_OPB0_ADDR TO_REG(0x1c)
>> +#define APB2OPB_OPB0_WRITE_DATA TO_REG(0x20)
>> +
>> +#define APB2OPB_OPB1_DMA_EN TO_REG(0x24)
>> +#define APB2OPB_OPB1_DMA_EN_3 BIT(3)
>> +#define APB2OPB_OPB1_DMA_EN_2 BIT(2)
>> +#define APB2OPB_OPB1_DMA_EN_1 BIT(1)
>> +#define APB2OPB_OPB1_DMA_EN_0 BIT(0)
>> +
>> +#define APB2OPB_OPB1_ADDR TO_REG(0x34)
>> +#define APB2OPB_OPB1_WRITE_DATA TO_REG(0x38)
>> +
>> +#define APB2OPB_OPB_CLK TO_REG(0x3c)
>> +#define APB2OPB_OPB_CLK_SYNC BIT(0)
>> +
>> +#define APB2OPB_IRQ_CLEAR TO_REG(0x40)
>> +#define APB2OPB_IRQ_CLEAR_EN BIT(0)
>> +
>> +#define APB2OPB_IRQ_MASK TO_REG(0x44)
>> +#define APB2OPB_IRQ_MASK_OPB1_TX_ACK BIT(17)
>> +#define APB2OPB_IRQ_MASK_OPB0_TX_ACK BIT(16)
>> +#define APB2OPB_IRQ_MASK_CH3_TCONT BIT(15)
>> +#define APB2OPB_IRQ_MASK_CH2_TCONT BIT(14)
>> +#define APB2OPB_IRQ_MASK_CH1_TCONT BIT(13)
>> +#define APB2OPB_IRQ_MASK_CH0_TCONT BIT(12)
>> +#define APB2OPB_IRQ_MASK_CH3_FIFO_EMPTY BIT(11)
>> +#define APB2OPB_IRQ_MASK_CH2_FIFO_EMPTY BIT(10)
>> +#define APB2OPB_IRQ_MASK_CH1_FIFO_EMPTY BIT(9)
>> +#define APB2OPB_IRQ_MASK_CH0_FIFO_EMPTY BIT(8)
>> +#define APB2OPB_IRQ_MASK_CH3_FIFO_FULL BIT(7)
>> +#define APB2OPB_IRQ_MASK_CH2_FIFO_FULL BIT(6)
>> +#define APB2OPB_IRQ_MASK_CH1_FIFO_FULL BIT(5)
>> +#define APB2OPB_IRQ_MASK_CH0_FIFO_FULL BIT(4)
>> +#define APB2OPB_IRQ_MASK_CH3_DMA_EOT BIT(3)
>> +#define APB2OPB_IRQ_MASK_CH2_DMA_EOT BIT(2)
>> +#define APB2OPB_IRQ_MASK_CH1_DMA_EOT BIT(1)
>> +#define APB2OPB_IRQ_MASK_CH0_DMA_EOT BIT(0)
>> +
>> +#define APB2OPB_IRQ_STS TO_REG(0x48)
>> +#define APB2OPB_IRQ_STS_MASTER_ERROR BIT(28)
>> +#define APB2OPB_IRQ_STS_PORT_ERROR BIT(27)
>> +#define APB2OPB_IRQ_STS_HOTPLUG BIT(26)
>> +#define APB2OPB_IRQ_STS_SLAVE_7 BIT(25)
>> +#define APB2OPB_IRQ_STS_SLAVE_6 BIT(24)
>> +#define APB2OPB_IRQ_STS_SLAVE_5 BIT(23)
>> +#define APB2OPB_IRQ_STS_SLAVE_4 BIT(22)
>> +#define APB2OPB_IRQ_STS_SLAVE_3 BIT(21)
>> +#define APB2OPB_IRQ_STS_SLAVE_2 BIT(20)
>> +#define APB2OPB_IRQ_STS_SLAVE_1 BIT(19)
>> +#define APB2OPB_IRQ_STS_SLAVE_0 BIT(18)
>> +#define APB2OPB_IRQ_STS_OPB1_TX_ACK BIT(17)
>> +#define APB2OPB_IRQ_STS_OPB0_TX_ACK BIT(16)
>> +#define APB2OPB_IRQ_STS_CH3_TCONT BIT(15)
>> +#define APB2OPB_IRQ_STS_CH2_TCONT BIT(14)
>> +#define APB2OPB_IRQ_STS_CH1_TCONT BIT(13)
>> +#define APB2OPB_IRQ_STS_CH0_TCONT BIT(12)
>> +#define APB2OPB_IRQ_STS_CH3_FIFO_EMPTY BIT(11)
>> +#define APB2OPB_IRQ_STS_CH2_FIFO_EMPTY BIT(10)
>> +#define APB2OPB_IRQ_STS_CH1_FIFO_EMPTY BIT(9)
>> +#define APB2OPB_IRQ_STS_CH0_FIFO_EMPTY BIT(8)
>> +#define APB2OPB_IRQ_STS_CH3_FIFO_FULL BIT(7)
>> +#define APB2OPB_IRQ_STS_CH2_FIFO_FULL BIT(6)
>> +#define APB2OPB_IRQ_STS_CH1_FIFO_FULL BIT(5)
>> +#define APB2OPB_IRQ_STS_CH0_FIFO_FULL BIT(4)
>> +#define APB2OPB_IRQ_STS_CH3_DMA_EOT BIT(3)
>> +#define APB2OPB_IRQ_STS_CH2_DMA_EOT BIT(2)
>> +#define APB2OPB_IRQ_STS_CH1_DMA_EOT BIT(1)
>> +#define APB2OPB_IRQ_STS_CH0_DMA_EOT BIT(0)
>> +
>> +#define APB2OPB_OPB0_WRITE_WORD_ENDIAN TO_REG(0x4c)
>> +#define APB2OPB_OPB0_WRITE_WORD_ENDIAN_BE 0x0011101b
>> +#define APB2OPB_OPB0_WRITE_BYTE_ENDIAN TO_REG(0x50)
>> +#define APB2OPB_OPB0_WRITE_BYTE_ENDIAN_BE 0x0c330f3f
>> +#define APB2OPB_OPB1_WRITE_WORD_ENDIAN TO_REG(0x54)
>> +#define APB2OPB_OPB1_WRITE_BYTE_ENDIAN TO_REG(0x58)
>> +#define APB2OPB_OPB0_READ_BYTE_ENDIAN TO_REG(0x5c)
>> +#define APB2OPB_OPB0_READ_WORD_ENDIAN_BE 0x00030b1b
>> +#define APB2OPB_OPB1_READ_BYTE_ENDIAN TO_REG(0x60)
>> +
>> +#define APB2OPB_RETRY TO_REG(0x64)
>> +#define APB2OPB_RETRY_COUNTER GENMASK(15, 0)
>> +
>> +#define APB2OPB_OPB0_STATUS TO_REG(0x80)
>> +#define APB2OPB_OPB1_STATUS TO_REG(0x8c)
>> +#define APB2OPB_OPB_STATUS_TIMEOUT BIT(4)
>> +#define APB2OPB_OPB_STATUS_RETRY BIT(3)
>> +#define APB2OPB_OPB_STATUS_ERROR_ACK BIT(2)
>> +#define APB2OPB_OPB_STATUS_FW_ACK BIT(1)
>> +#define APB2OPB_OPB_STATUS_HW_ACK BIT(0)
>> +
>> +#define APB2OPB_OPB0_READ_DATA TO_REG(0x84)
>> +
>> +#define APB2OPB_OPB1_DMA_STATUS TO_REG(0x88)
>> +#define APB2OPB_OPB1_DMA_STATUS_CH3_EOT BIT(7)
>> +#define APB2OPB_OPB1_DMA_STATUS_CH2_EOT BIT(6)
>> +#define APB2OPB_OPB1_DMA_STATUS_CH1_EOT BIT(5)
>> +#define APB2OPB_OPB1_DMA_STATUS_CH0_EOT BIT(4)
>> +#define APB2OPB_OPB1_DMA_STATUS_CH3_REQ BIT(3)
>> +#define APB2OPB_OPB1_DMA_STATUS_CH2_REQ BIT(2)
>> +#define APB2OPB_OPB1_DMA_STATUS_CH1_REQ BIT(1)
>> +#define APB2OPB_OPB1_DMA_STATUS_CH0_REQ BIT(0)
>> +
>> +#define APB2OPB_OPB1_READ_DATA TO_REG(0x90)
>> +
>> +static uint64_t aspeed_apb2opb_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> + AspeedAPB2OPBState *s = ASPEED_APB2OPB(opaque);
>> +
>> + trace_aspeed_apb2opb_read(addr, size);
>> +
>> + assert(!(addr & 3));
>> + assert(size == 4);
>> +
>> + if (addr + size > sizeof(s->regs)) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Out of bounds read: 0x%"HWADDR_PRIx" for %u\n",
>> + __func__, addr, size);
>> + return 0;
>> + }
>> +
>> + return s->regs[TO_REG(addr)];
>> +}
>> +
>> +static void aspeed_apb2opb_write(void *opaque, hwaddr addr, uint64_t data,
>> + unsigned size)
>> +{
>> + AspeedAPB2OPBState *s = ASPEED_APB2OPB(opaque);
>> +
>> + trace_aspeed_apb2opb_write(addr, size, data);
>> +
>> + assert(!(addr & 3));
>> + assert(size == 4);
>> +
>> + if (addr + size > sizeof(s->regs)) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Out of bounds write: %"HWADDR_PRIx" for %u\n",
>> + __func__, addr, size);
>> + return;
>> + }
>> +
>> + switch (TO_REG(addr)) {
>> + case APB2OPB_CONTROL:
>> + opb_fsi_master_address(&s->opb[0], data & APB2OPB_CONTROL_OFF);
>> + break;
>> + case APB2OPB_OPB2FSI:
>> + opb_opb2fsi_address(&s->opb[0], data & APB2OPB_OPB2FSI_OFF);
>> + break;
>> + case APB2OPB_OPB0_WRITE_WORD_ENDIAN:
>> + if (data != APB2OPB_OPB0_WRITE_WORD_ENDIAN_BE) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Bridge needs to be driven as BE (0x%x)\n",
>> + __func__, APB2OPB_OPB0_WRITE_WORD_ENDIAN_BE);
>> + }
>> + break;
>> + case APB2OPB_OPB0_WRITE_BYTE_ENDIAN:
>> + if (data != APB2OPB_OPB0_WRITE_BYTE_ENDIAN_BE) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Bridge needs to be driven as BE (0x%x)\n",
>> + __func__, APB2OPB_OPB0_WRITE_BYTE_ENDIAN_BE);
>> + }
>> + break;
>> + case APB2OPB_OPB0_READ_BYTE_ENDIAN:
>> + if (data != APB2OPB_OPB0_READ_WORD_ENDIAN_BE) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "%s: Bridge needs to be driven as BE (0x%x)\n",
>> + __func__, APB2OPB_OPB0_READ_WORD_ENDIAN_BE);
>> + }
>> + break;
>> + case APB2OPB_TRIGGER:
>> + {
>> + uint32_t opb, op_mode, op_size, op_addr, op_data;
>> +
>> + assert((s->regs[APB2OPB_OPB0_SEL] & APB2OPB_OPB_SEL_EN) ^
>> + (s->regs[APB2OPB_OPB1_SEL] & APB2OPB_OPB_SEL_EN));
>> +
>> + if (s->regs[APB2OPB_OPB0_SEL] & APB2OPB_OPB_SEL_EN) {
>> + opb = 0;
>> + op_mode = s->regs[APB2OPB_OPB0_MODE];
>> + op_size = s->regs[APB2OPB_OPB0_XFER];
>> + op_addr = s->regs[APB2OPB_OPB0_ADDR];
>> + op_data = s->regs[APB2OPB_OPB0_WRITE_DATA];
>> + } else if (s->regs[APB2OPB_OPB1_SEL] & APB2OPB_OPB_SEL_EN) {
>> + opb = 1;
>> + op_mode = s->regs[APB2OPB_OPB1_MODE];
>> + op_size = s->regs[APB2OPB_OPB1_XFER];
>> + op_addr = s->regs[APB2OPB_OPB1_ADDR];
>> + op_data = s->regs[APB2OPB_OPB1_WRITE_DATA];
>> + } else {
>> + g_assert_not_reached();
>> + }
>> +
>> + if (op_size & ~(APB2OPB_OPB_XFER_HALF | APB2OPB_OPB_XFER_FULL)) {
>> + qemu_log_mask(LOG_GUEST_ERROR,
>> + "OPB transaction failed: Unrecognised access width: %d\n",
>> + op_size);
>> + return;
>> + }
>> +
>> + op_size += 1;
>> +
>> + if (op_mode & APB2OPB_OPB_MODE_RD) {
>> + int index = opb ? APB2OPB_OPB1_READ_DATA
>> + : APB2OPB_OPB0_READ_DATA;
>> +
>> + switch (op_size) {
>> + case 1:
>> + s->regs[index] = opb_read8(&s->opb[opb], op_addr);
>> + break;
>> + case 2:
>> + s->regs[index] = opb_read16(&s->opb[opb], op_addr);
>> + break;
>> + case 4:
>> + s->regs[index] = opb_read32(&s->opb[opb], op_addr);
>> + break;
>> + default:
>> + g_assert_not_reached(); /* should have bailed above */
>> + }
>> + } else {
>> + /* FIXME: Endian swizzling */
>> + switch (op_size) {
>> + case 1:
>> + opb_write8(&s->opb[opb], op_addr, op_data);
>> + break;
>> + case 2:
>> + opb_write16(&s->opb[opb], op_addr, op_data);
>> + break;
>> + case 4:
>> + opb_write32(&s->opb[opb], op_addr, op_data);
>> + break;
>> + default:
>> + g_assert_not_reached(); /* should have bailed above */
>> + }
>> + }
>> + s->regs[APB2OPB_IRQ_STS] |= opb ? APB2OPB_IRQ_STS_OPB1_TX_ACK
>> + : APB2OPB_IRQ_STS_OPB0_TX_ACK;
>> + break;
>> + }
>> + }
>> +
>> + s->regs[TO_REG(addr)] = data;
>> +}
>> +
>> +static const struct MemoryRegionOps aspeed_apb2opb_ops = {
>> + .read = aspeed_apb2opb_read,
>> + .write = aspeed_apb2opb_write,
>> + .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>> +
>> +static void aspeed_apb2opb_realize(DeviceState *dev, Error **errp)
>> +{
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> + AspeedAPB2OPBState *s = ASPEED_APB2OPB(dev);
>> +
>> + qbus_init(&s->opb[0], sizeof(s->opb[0]), TYPE_OP_BUS,
>> + DEVICE(s), NULL);
>> + qbus_init(&s->opb[1], sizeof(s->opb[1]), TYPE_OP_BUS,
>> + DEVICE(s), NULL);
>> +
>> + sysbus_init_irq(sbd, &s->irq);
>> +
>> + memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_apb2opb_ops, s,
>> + TYPE_ASPEED_APB2OPB, 0x1000);
>> + sysbus_init_mmio(sbd, &s->iomem);
>> +}
>> +
>> +static void aspeed_apb2opb_reset(DeviceState *dev)
>> +{
>> + AspeedAPB2OPBState *s = ASPEED_APB2OPB(dev);
>> +
>> + memset(s->regs, 0, sizeof(s->regs));
>> +
>> + s->regs[APB2OPB_VERSION] = 0x000000a1;
>> + s->regs[APB2OPB_OPB0_WRITE_WORD_ENDIAN] = 0x0044eee4;
>> + s->regs[APB2OPB_OPB0_WRITE_BYTE_ENDIAN] = 0x0055aaff;
>> + s->regs[APB2OPB_OPB1_WRITE_WORD_ENDIAN] = 0x00117717;
>> + s->regs[APB2OPB_OPB1_WRITE_BYTE_ENDIAN] = 0xffaa5500;
>> + s->regs[APB2OPB_OPB0_READ_BYTE_ENDIAN] = 0x0044eee4;
>> + s->regs[APB2OPB_OPB0_READ_BYTE_ENDIAN] = 0x00117717;
> Add a comment about the source of these magic numbers. I think this is
> the best reference we have:
>
> https://git.kernel.org/torvalds/c/5e5022995683539609b3904213b7d72ab71188b7
>
> I'm not sure if the reset values were taken from the hardware, or the
> kernel driver.
Added comment. These magic numbers are from AST2600 data sheet.
>
>> +}
>> +
>> +static void aspeed_apb2opb_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + dc->desc = "ASPEED APB2OPB Bridge";
>> + dc->realize = aspeed_apb2opb_realize;
>> + dc->reset = aspeed_apb2opb_reset;
>> +}
>> +
>> +static const TypeInfo aspeed_apb2opb_info = {
>> + .name = TYPE_ASPEED_APB2OPB,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(AspeedAPB2OPBState),
>> + .class_init = aspeed_apb2opb_class_init,
>> +};
>> +
>> +static void aspeed_apb2opb_register_types(void)
>> +{
>> + type_register_static(&aspeed_apb2opb_info);
>> +}
>> +
>> +type_init(aspeed_apb2opb_register_types);
>> diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
>> index cab645f4ea..ce8b9555ce 100644
>> --- a/hw/fsi/meson.build
>> +++ b/hw/fsi/meson.build
>> @@ -3,3 +3,4 @@ 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'))
>> +system_ss.add(when: 'CONFIG_APB2OPB_ASPEED', if_true: files('aspeed-apb2opb.c'))
>> diff --git a/hw/fsi/trace-events b/hw/fsi/trace-events
>> new file mode 100644
>> index 0000000000..c64245f7f6
>> --- /dev/null
>> +++ b/hw/fsi/trace-events
>> @@ -0,0 +1,2 @@
>> +aspeed_apb2opb_read(uint64_t addr, uint32_t size) "@0x%" PRIx64 " size=%d"
>> +aspeed_apb2opb_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size=%d value=0x%"PRIx64
>> diff --git a/hw/fsi/trace.h b/hw/fsi/trace.h
>> new file mode 100644
>> index 0000000000..ee67c7fb04
>> --- /dev/null
>> +++ b/hw/fsi/trace.h
>> @@ -0,0 +1 @@
>> +#include "trace/trace-hw_fsi.h"
>> diff --git a/include/hw/fsi/aspeed-apb2opb.h b/include/hw/fsi/aspeed-apb2opb.h
>> new file mode 100644
>> index 0000000000..4acc980e7d
>> --- /dev/null
>> +++ b/include/hw/fsi/aspeed-apb2opb.h
>> @@ -0,0 +1,32 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + * Copyright (C) 2023 IBM Corp.
>> + *
>> + * ASPEED APB2OPB Bridge
>> + */
>> +#ifndef FSI_ASPEED_APB2OPB_H
>> +#define FSI_ASPEED_APB2OPB_H
>> +
>> +#include "hw/sysbus.h"
>> +#include "hw/fsi/opb.h"
>> +
>> +#define TYPE_ASPEED_APB2OPB "aspeed.apb2opb"
>> +#define ASPEED_APB2OPB(obj) \
>> + OBJECT_CHECK(AspeedAPB2OPBState, (obj), TYPE_ASPEED_APB2OPB)
>> +
>> +#define ASPEED_APB2OPB_NR_REGS ((0xe8 >> 2) + 1)
>> +
>> +typedef struct AspeedAPB2OPBState {
>> + /*< private >*/
>> + SysBusDevice parent_obj;
>> +
>> + /*< public >*/
>> + MemoryRegion iomem;
>> +
>> + uint32_t regs[ASPEED_APB2OPB_NR_REGS];
>> + qemu_irq irq;
>> +
>> + OPBus opb[2];
>> +} AspeedAPB2OPBState;
>> +
>> +#endif /* FSI_ASPEED_APB2OPB_H */
>> diff --git a/meson.build b/meson.build
>> index 98e68ef0b1..1a722693a6 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -3244,6 +3244,7 @@ if have_system
>> 'hw/char',
>> 'hw/display',
>> 'hw/dma',
>> + 'hw/fsi',
>> 'hw/hyperv',
>> 'hw/i2c',
>> 'hw/i386',
>> --
>> 2.39.2
>>
Thanks for the review.
Ninad
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v1 7/7] hw/arm: Hook up FSI module in AST2600
2023-08-25 20:30 [PATCH v1 0/7] Introduce model for IBM's FSP Ninad Palsule
` (5 preceding siblings ...)
2023-08-25 20:30 ` [PATCH v1 6/7] hw/fsi: Aspeed APB2OPB interface Ninad Palsule
@ 2023-08-25 20:30 ` Ninad Palsule
2023-08-28 8:48 ` Joel Stanley
2023-08-28 8:25 ` [PATCH v1 0/7] Introduce model for IBM's FSP Joel Stanley
2023-08-28 8:49 ` Cédric Le Goater
8 siblings, 1 reply; 28+ messages in thread
From: Ninad Palsule @ 2023-08-25 20:30 UTC (permalink / raw)
To: qemu-devel, clg, peter.maydell, andrew, joel, pbonzini,
marcandre.lureau, berrange, thuth, philmd
Cc: Ninad Palsule, qemu-arm
This patchset introduces IBM's Flexible Service Interface(FSI).
Time for some fun with inter-processor buses. FSI allows a service
processor access to the internal buses of a host POWER processor to
perform configuration or debugging.
FSI has long existed in POWER processes and so comes with some baggage,
including how it has been integrated into the ASPEED SoC.
Working backwards from the POWER processor, the fundamental pieces of
interest for the implementation are:
1. The Common FRU Access Macro (CFAM), an address space containing
various "engines" that drive accesses on buses internal and external
to the POWER chip. Examples include the SBEFIFO and I2C masters. The
engines hang off of an internal Local Bus (LBUS) which is described
by the CFAM configuration block.
2. The FSI slave: The slave is the terminal point of the FSI bus for
FSI symbols addressed to it. Slaves can be cascaded off of one
another. The slave's configuration registers appear in address space
of the CFAM to which it is attached.
3. The FSI master: A controller in the platform service processor (e.g.
BMC) driving CFAM engine accesses into the POWER chip. At the
hardware level FSI is a bit-based protocol supporting synchronous and
DMA-driven accesses of engines in a CFAM.
4. 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.
5. An APB-to-OPB bridge enabling access to the OPB from the ARM core in
the AST2600. Hardware limitations prevent the OPB from being directly
mapped into APB, so all accesses are indirect through the bridge.
The implementation appears as following in the qemu device tree:
(qemu) info qtree
bus: main-system-bus
type System
...
dev: aspeed.apb2opb, id ""
gpio-out "sysbus-irq" 1
mmio 000000001e79b000/0000000000001000
bus: opb.1
type opb
dev: fsi.master, id ""
bus: fsi.bus.1
type fsi.bus
dev: cfam.config, id ""
dev: cfam, id ""
bus: lbus.1
type lbus
dev: scratchpad, id ""
address = 0 (0x0)
bus: opb.0
type opb
dev: fsi.master, id ""
bus: fsi.bus.0
type fsi.bus
dev: cfam.config, id ""
dev: cfam, id ""
bus: lbus.0
type lbus
dev: scratchpad, id ""
address = 0 (0x0)
The LBUS is modelled to maintain the qdev bus hierarchy and to take
advantage of the object model to automatically generate the CFAM
configuration block. The configuration block presents engines in the
order they are attached to the CFAM's LBUS. Engine implementations
should subclass the LBusDevice and set the 'config' member of
LBusDeviceClass to match the engine's type.
CFAM designs offer a lot of flexibility, for instance it is possible for
a CFAM to be simultaneously driven from multiple FSI links. The modeling
is not so complete; it's assumed that each CFAM is attached to a single
FSI slave (as a consequence the CFAM subclasses the FSI slave).
As for FSI, its symbols and wire-protocol are not modelled at all. This
is not necessary to get FSI off the ground thanks to the mapping of the
CFAM address space onto the OPB address space - the models follow this
directly and map the CFAM memory region into the OPB's memory region.
Future work includes supporting more advanced accesses that drive the
FSI master directly rather than indirectly via the CFAM mapping, which
will require implementing the FSI state machine and methods for each of
the FSI symbols on the slave. Further down the track we can also look at
supporting the bitbanged SoftFSI drivers in Linux by extending the FSI
slave model to resolve sequences of GPIO IRQs into FSI symbols, and
calling the associated symbol method on the slave to map the access onto
the CFAM.
Testing:
Tested by reading cfam config address 0 on rainier machine. We can
ignore the error line as it is not related.
root@p10bmc:~# pdbg -a getcfam 0x0
Unable to open dtb file '/var/lib/phosphor-software-manager/pnor/rw/DEVTREE'
p0: 0x0 = 0xc0022d15
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
---
hw/arm/aspeed_ast2600.c | 15 +++++++++++++++
include/hw/arm/aspeed_soc.h | 4 ++++
2 files changed, 19 insertions(+)
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index a8b3a8065a..e239487c16 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -75,6 +75,8 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
[ASPEED_DEV_UART12] = 0x1E790600,
[ASPEED_DEV_UART13] = 0x1E790700,
[ASPEED_DEV_VUART] = 0x1E787000,
+ [ASPEED_DEV_FSI1] = 0x1E79B000,
+ [ASPEED_DEV_FSI2] = 0x1E79B100,
[ASPEED_DEV_I3C] = 0x1E7A0000,
[ASPEED_DEV_SDRAM] = 0x80000000,
};
@@ -132,6 +134,8 @@ static const int aspeed_soc_ast2600_irqmap[] = {
[ASPEED_DEV_ETH4] = 33,
[ASPEED_DEV_KCS] = 138, /* 138 -> 142 */
[ASPEED_DEV_DP] = 62,
+ [ASPEED_DEV_FSI1] = 100,
+ [ASPEED_DEV_FSI2] = 101,
[ASPEED_DEV_I3C] = 102, /* 102 -> 107 */
};
@@ -262,6 +266,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
object_initialize_child(obj, "emmc-boot-controller",
&s->emmc_boot_controller,
TYPE_UNIMPLEMENTED_DEVICE);
+
+ object_initialize_child(obj, "fsi[*]", &s->fsi[0], TYPE_ASPEED_APB2OPB);
}
/*
@@ -622,6 +628,15 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
return;
}
aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
+
+ /* FSI */
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->fsi[0]), errp)) {
+ return;
+ }
+ aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fsi[0]), 0,
+ sc->memmap[ASPEED_DEV_FSI1]);
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->fsi[0]), 0,
+ aspeed_soc_get_irq(s, ASPEED_DEV_FSI1));
}
static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 8adff70072..db3ba3abc7 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -36,6 +36,7 @@
#include "hw/misc/aspeed_lpc.h"
#include "hw/misc/unimp.h"
#include "hw/misc/aspeed_peci.h"
+#include "hw/fsi/aspeed-apb2opb.h"
#include "hw/char/serial.h"
#define ASPEED_SPIS_NUM 2
@@ -96,6 +97,7 @@ struct AspeedSoCState {
UnimplementedDeviceState udc;
UnimplementedDeviceState sgpiom;
UnimplementedDeviceState jtag[ASPEED_JTAG_NUM];
+ AspeedAPB2OPBState fsi[2];
};
#define TYPE_ASPEED_SOC "aspeed-soc"
@@ -191,6 +193,8 @@ enum {
ASPEED_DEV_SGPIOM,
ASPEED_DEV_JTAG0,
ASPEED_DEV_JTAG1,
+ ASPEED_DEV_FSI1,
+ ASPEED_DEV_FSI2,
};
#define ASPEED_SOC_SPI_BOOT_ADDR 0x0
--
2.39.2
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v1 7/7] hw/arm: Hook up FSI module in AST2600
2023-08-25 20:30 ` [PATCH v1 7/7] hw/arm: Hook up FSI module in AST2600 Ninad Palsule
@ 2023-08-28 8:48 ` Joel Stanley
2023-08-28 22:56 ` Ninad Palsule
0 siblings, 1 reply; 28+ messages in thread
From: Joel Stanley @ 2023-08-28 8:48 UTC (permalink / raw)
To: Ninad Palsule
Cc: qemu-devel, clg, peter.maydell, andrew, pbonzini,
marcandre.lureau, berrange, thuth, philmd, qemu-arm
On Fri, 25 Aug 2023 at 20:35, Ninad Palsule <ninad@linux.ibm.com> wrote:
>
> This patchset introduces IBM's Flexible Service Interface(FSI).
>
> Time for some fun with inter-processor buses. FSI allows a service
> processor access to the internal buses of a host POWER processor to
> perform configuration or debugging.
>
> FSI has long existed in POWER processes and so comes with some baggage,
> including how it has been integrated into the ASPEED SoC.
>
> Working backwards from the POWER processor, the fundamental pieces of
> interest for the implementation are:
>
> 1. The Common FRU Access Macro (CFAM), an address space containing
> various "engines" that drive accesses on buses internal and external
> to the POWER chip. Examples include the SBEFIFO and I2C masters. The
> engines hang off of an internal Local Bus (LBUS) which is described
> by the CFAM configuration block.
>
> 2. The FSI slave: The slave is the terminal point of the FSI bus for
> FSI symbols addressed to it. Slaves can be cascaded off of one
> another. The slave's configuration registers appear in address space
> of the CFAM to which it is attached.
>
> 3. The FSI master: A controller in the platform service processor (e.g.
> BMC) driving CFAM engine accesses into the POWER chip. At the
> hardware level FSI is a bit-based protocol supporting synchronous and
> DMA-driven accesses of engines in a CFAM.
>
> 4. 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.
>
> 5. An APB-to-OPB bridge enabling access to the OPB from the ARM core in
> the AST2600. Hardware limitations prevent the OPB from being directly
> mapped into APB, so all accesses are indirect through the bridge.
>
> The implementation appears as following in the qemu device tree:
>
> (qemu) info qtree
> bus: main-system-bus
> type System
> ...
> dev: aspeed.apb2opb, id ""
> gpio-out "sysbus-irq" 1
> mmio 000000001e79b000/0000000000001000
> bus: opb.1
> type opb
> dev: fsi.master, id ""
> bus: fsi.bus.1
> type fsi.bus
> dev: cfam.config, id ""
> dev: cfam, id ""
> bus: lbus.1
> type lbus
> dev: scratchpad, id ""
> address = 0 (0x0)
> bus: opb.0
> type opb
> dev: fsi.master, id ""
> bus: fsi.bus.0
> type fsi.bus
> dev: cfam.config, id ""
> dev: cfam, id ""
> bus: lbus.0
> type lbus
> dev: scratchpad, id ""
> address = 0 (0x0)
>
> The LBUS is modelled to maintain the qdev bus hierarchy and to take
> advantage of the object model to automatically generate the CFAM
> configuration block. The configuration block presents engines in the
> order they are attached to the CFAM's LBUS. Engine implementations
> should subclass the LBusDevice and set the 'config' member of
> LBusDeviceClass to match the engine's type.
>
> CFAM designs offer a lot of flexibility, for instance it is possible for
> a CFAM to be simultaneously driven from multiple FSI links. The modeling
> is not so complete; it's assumed that each CFAM is attached to a single
> FSI slave (as a consequence the CFAM subclasses the FSI slave).
>
> As for FSI, its symbols and wire-protocol are not modelled at all. This
> is not necessary to get FSI off the ground thanks to the mapping of the
> CFAM address space onto the OPB address space - the models follow this
> directly and map the CFAM memory region into the OPB's memory region.
> Future work includes supporting more advanced accesses that drive the
> FSI master directly rather than indirectly via the CFAM mapping, which
> will require implementing the FSI state machine and methods for each of
> the FSI symbols on the slave. Further down the track we can also look at
> supporting the bitbanged SoftFSI drivers in Linux by extending the FSI
> slave model to resolve sequences of GPIO IRQs into FSI symbols, and
> calling the associated symbol method on the slave to map the access onto
> the CFAM.
>
> Testing:
> Tested by reading cfam config address 0 on rainier machine. We can
> ignore the error line as it is not related.
> root@p10bmc:~# pdbg -a getcfam 0x0
> Unable to open dtb file '/var/lib/phosphor-software-manager/pnor/rw/DEVTREE'
Delete this line (and the explanation). It's something to follow up
with the version of pdbg that openbmc has, but unrelated to this
patch.
> p0: 0x0 = 0xc0022d15
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
> ---
> hw/arm/aspeed_ast2600.c | 15 +++++++++++++++
> include/hw/arm/aspeed_soc.h | 4 ++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index a8b3a8065a..e239487c16 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -75,6 +75,8 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
> [ASPEED_DEV_UART12] = 0x1E790600,
> [ASPEED_DEV_UART13] = 0x1E790700,
> [ASPEED_DEV_VUART] = 0x1E787000,
> + [ASPEED_DEV_FSI1] = 0x1E79B000,
> + [ASPEED_DEV_FSI2] = 0x1E79B100,
> [ASPEED_DEV_I3C] = 0x1E7A0000,
> [ASPEED_DEV_SDRAM] = 0x80000000,
> };
> @@ -132,6 +134,8 @@ static const int aspeed_soc_ast2600_irqmap[] = {
> [ASPEED_DEV_ETH4] = 33,
> [ASPEED_DEV_KCS] = 138, /* 138 -> 142 */
> [ASPEED_DEV_DP] = 62,
> + [ASPEED_DEV_FSI1] = 100,
> + [ASPEED_DEV_FSI2] = 101,
> [ASPEED_DEV_I3C] = 102, /* 102 -> 107 */
> };
>
> @@ -262,6 +266,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
> object_initialize_child(obj, "emmc-boot-controller",
> &s->emmc_boot_controller,
> TYPE_UNIMPLEMENTED_DEVICE);
> +
> + object_initialize_child(obj, "fsi[*]", &s->fsi[0], TYPE_ASPEED_APB2OPB);
> }
>
> /*
> @@ -622,6 +628,15 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> return;
> }
> aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
> +
> + /* FSI */
> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->fsi[0]), errp)) {
> + return;
> + }
> + aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fsi[0]), 0,
> + sc->memmap[ASPEED_DEV_FSI1]);
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->fsi[0]), 0,
> + aspeed_soc_get_irq(s, ASPEED_DEV_FSI1));
You only attach the first FSI device. You should attach the second, or
if you decide to not model the second, remove the array and just have
one AspeedAPB2OPBState in AspeedSoCState.
> }
>
> static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 8adff70072..db3ba3abc7 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -36,6 +36,7 @@
> #include "hw/misc/aspeed_lpc.h"
> #include "hw/misc/unimp.h"
> #include "hw/misc/aspeed_peci.h"
> +#include "hw/fsi/aspeed-apb2opb.h"
> #include "hw/char/serial.h"
>
> #define ASPEED_SPIS_NUM 2
> @@ -96,6 +97,7 @@ struct AspeedSoCState {
> UnimplementedDeviceState udc;
> UnimplementedDeviceState sgpiom;
> UnimplementedDeviceState jtag[ASPEED_JTAG_NUM];
> + AspeedAPB2OPBState fsi[2];
> };
>
> #define TYPE_ASPEED_SOC "aspeed-soc"
> @@ -191,6 +193,8 @@ enum {
> ASPEED_DEV_SGPIOM,
> ASPEED_DEV_JTAG0,
> ASPEED_DEV_JTAG1,
> + ASPEED_DEV_FSI1,
> + ASPEED_DEV_FSI2,
> };
>
> #define ASPEED_SOC_SPI_BOOT_ADDR 0x0
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 7/7] hw/arm: Hook up FSI module in AST2600
2023-08-28 8:48 ` Joel Stanley
@ 2023-08-28 22:56 ` Ninad Palsule
0 siblings, 0 replies; 28+ messages in thread
From: Ninad Palsule @ 2023-08-28 22:56 UTC (permalink / raw)
To: Joel Stanley
Cc: qemu-devel, clg, peter.maydell, andrew, pbonzini,
marcandre.lureau, berrange, thuth, philmd, qemu-arm
Hi Joel,
On 8/28/23 23:48, Joel Stanley wrote:
> On Fri, 25 Aug 2023 at 20:35, Ninad Palsule <ninad@linux.ibm.com> wrote:
>> This patchset introduces IBM's Flexible Service Interface(FSI).
>>
>> Time for some fun with inter-processor buses. FSI allows a service
>> processor access to the internal buses of a host POWER processor to
>> perform configuration or debugging.
>>
>> FSI has long existed in POWER processes and so comes with some baggage,
>> including how it has been integrated into the ASPEED SoC.
>>
>> Working backwards from the POWER processor, the fundamental pieces of
>> interest for the implementation are:
>>
>> 1. The Common FRU Access Macro (CFAM), an address space containing
>> various "engines" that drive accesses on buses internal and external
>> to the POWER chip. Examples include the SBEFIFO and I2C masters. The
>> engines hang off of an internal Local Bus (LBUS) which is described
>> by the CFAM configuration block.
>>
>> 2. The FSI slave: The slave is the terminal point of the FSI bus for
>> FSI symbols addressed to it. Slaves can be cascaded off of one
>> another. The slave's configuration registers appear in address space
>> of the CFAM to which it is attached.
>>
>> 3. The FSI master: A controller in the platform service processor (e.g.
>> BMC) driving CFAM engine accesses into the POWER chip. At the
>> hardware level FSI is a bit-based protocol supporting synchronous and
>> DMA-driven accesses of engines in a CFAM.
>>
>> 4. 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.
>>
>> 5. An APB-to-OPB bridge enabling access to the OPB from the ARM core in
>> the AST2600. Hardware limitations prevent the OPB from being directly
>> mapped into APB, so all accesses are indirect through the bridge.
>>
>> The implementation appears as following in the qemu device tree:
>>
>> (qemu) info qtree
>> bus: main-system-bus
>> type System
>> ...
>> dev: aspeed.apb2opb, id ""
>> gpio-out "sysbus-irq" 1
>> mmio 000000001e79b000/0000000000001000
>> bus: opb.1
>> type opb
>> dev: fsi.master, id ""
>> bus: fsi.bus.1
>> type fsi.bus
>> dev: cfam.config, id ""
>> dev: cfam, id ""
>> bus: lbus.1
>> type lbus
>> dev: scratchpad, id ""
>> address = 0 (0x0)
>> bus: opb.0
>> type opb
>> dev: fsi.master, id ""
>> bus: fsi.bus.0
>> type fsi.bus
>> dev: cfam.config, id ""
>> dev: cfam, id ""
>> bus: lbus.0
>> type lbus
>> dev: scratchpad, id ""
>> address = 0 (0x0)
>>
>> The LBUS is modelled to maintain the qdev bus hierarchy and to take
>> advantage of the object model to automatically generate the CFAM
>> configuration block. The configuration block presents engines in the
>> order they are attached to the CFAM's LBUS. Engine implementations
>> should subclass the LBusDevice and set the 'config' member of
>> LBusDeviceClass to match the engine's type.
>>
>> CFAM designs offer a lot of flexibility, for instance it is possible for
>> a CFAM to be simultaneously driven from multiple FSI links. The modeling
>> is not so complete; it's assumed that each CFAM is attached to a single
>> FSI slave (as a consequence the CFAM subclasses the FSI slave).
>>
>> As for FSI, its symbols and wire-protocol are not modelled at all. This
>> is not necessary to get FSI off the ground thanks to the mapping of the
>> CFAM address space onto the OPB address space - the models follow this
>> directly and map the CFAM memory region into the OPB's memory region.
>> Future work includes supporting more advanced accesses that drive the
>> FSI master directly rather than indirectly via the CFAM mapping, which
>> will require implementing the FSI state machine and methods for each of
>> the FSI symbols on the slave. Further down the track we can also look at
>> supporting the bitbanged SoftFSI drivers in Linux by extending the FSI
>> slave model to resolve sequences of GPIO IRQs into FSI symbols, and
>> calling the associated symbol method on the slave to map the access onto
>> the CFAM.
>>
>> Testing:
>> Tested by reading cfam config address 0 on rainier machine. We can
>> ignore the error line as it is not related.
>> root@p10bmc:~# pdbg -a getcfam 0x0
>> Unable to open dtb file '/var/lib/phosphor-software-manager/pnor/rw/DEVTREE'
> Delete this line (and the explanation). It's something to follow up
> with the version of pdbg that openbmc has, but unrelated to this
> patch.
Done.
>
>> p0: 0x0 = 0xc0022d15
>>
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
>> ---
>> hw/arm/aspeed_ast2600.c | 15 +++++++++++++++
>> include/hw/arm/aspeed_soc.h | 4 ++++
>> 2 files changed, 19 insertions(+)
>>
>> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
>> index a8b3a8065a..e239487c16 100644
>> --- a/hw/arm/aspeed_ast2600.c
>> +++ b/hw/arm/aspeed_ast2600.c
>> @@ -75,6 +75,8 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
>> [ASPEED_DEV_UART12] = 0x1E790600,
>> [ASPEED_DEV_UART13] = 0x1E790700,
>> [ASPEED_DEV_VUART] = 0x1E787000,
>> + [ASPEED_DEV_FSI1] = 0x1E79B000,
>> + [ASPEED_DEV_FSI2] = 0x1E79B100,
>> [ASPEED_DEV_I3C] = 0x1E7A0000,
>> [ASPEED_DEV_SDRAM] = 0x80000000,
>> };
>> @@ -132,6 +134,8 @@ static const int aspeed_soc_ast2600_irqmap[] = {
>> [ASPEED_DEV_ETH4] = 33,
>> [ASPEED_DEV_KCS] = 138, /* 138 -> 142 */
>> [ASPEED_DEV_DP] = 62,
>> + [ASPEED_DEV_FSI1] = 100,
>> + [ASPEED_DEV_FSI2] = 101,
>> [ASPEED_DEV_I3C] = 102, /* 102 -> 107 */
>> };
>>
>> @@ -262,6 +266,8 @@ static void aspeed_soc_ast2600_init(Object *obj)
>> object_initialize_child(obj, "emmc-boot-controller",
>> &s->emmc_boot_controller,
>> TYPE_UNIMPLEMENTED_DEVICE);
>> +
>> + object_initialize_child(obj, "fsi[*]", &s->fsi[0], TYPE_ASPEED_APB2OPB);
>> }
>>
>> /*
>> @@ -622,6 +628,15 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
>> return;
>> }
>> aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->sbc), 0, sc->memmap[ASPEED_DEV_SBC]);
>> +
>> + /* FSI */
>> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->fsi[0]), errp)) {
>> + return;
>> + }
>> + aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->fsi[0]), 0,
>> + sc->memmap[ASPEED_DEV_FSI1]);
>> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->fsi[0]), 0,
>> + aspeed_soc_get_irq(s, ASPEED_DEV_FSI1));
> You only attach the first FSI device. You should attach the second, or
> if you decide to not model the second, remove the array and just have
> one AspeedAPB2OPBState in AspeedSoCState.
Registered both the FSI devices.
>
>> }
>>
>> static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
>> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
>> index 8adff70072..db3ba3abc7 100644
>> --- a/include/hw/arm/aspeed_soc.h
>> +++ b/include/hw/arm/aspeed_soc.h
>> @@ -36,6 +36,7 @@
>> #include "hw/misc/aspeed_lpc.h"
>> #include "hw/misc/unimp.h"
>> #include "hw/misc/aspeed_peci.h"
>> +#include "hw/fsi/aspeed-apb2opb.h"
>> #include "hw/char/serial.h"
>>
>> #define ASPEED_SPIS_NUM 2
>> @@ -96,6 +97,7 @@ struct AspeedSoCState {
>> UnimplementedDeviceState udc;
>> UnimplementedDeviceState sgpiom;
>> UnimplementedDeviceState jtag[ASPEED_JTAG_NUM];
>> + AspeedAPB2OPBState fsi[2];
>> };
>>
>> #define TYPE_ASPEED_SOC "aspeed-soc"
>> @@ -191,6 +193,8 @@ enum {
>> ASPEED_DEV_SGPIOM,
>> ASPEED_DEV_JTAG0,
>> ASPEED_DEV_JTAG1,
>> + ASPEED_DEV_FSI1,
>> + ASPEED_DEV_FSI2,
>> };
>>
>> #define ASPEED_SOC_SPI_BOOT_ADDR 0x0
>> --
>> 2.39.2
>>
Thanks for the review.
Ninad
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 0/7] Introduce model for IBM's FSP
2023-08-25 20:30 [PATCH v1 0/7] Introduce model for IBM's FSP Ninad Palsule
` (6 preceding siblings ...)
2023-08-25 20:30 ` [PATCH v1 7/7] hw/arm: Hook up FSI module in AST2600 Ninad Palsule
@ 2023-08-28 8:25 ` Joel Stanley
2023-08-28 21:13 ` Ninad Palsule
2023-08-28 8:49 ` Cédric Le Goater
8 siblings, 1 reply; 28+ messages in thread
From: Joel Stanley @ 2023-08-28 8:25 UTC (permalink / raw)
To: Ninad Palsule
Cc: qemu-devel, clg, peter.maydell, andrew, pbonzini,
marcandre.lureau, berrange, thuth, philmd, qemu-arm
Hi Ninad,
On Fri, 25 Aug 2023 at 20:51, Ninad Palsule <ninad@linux.ibm.com> wrote:
>
> Hello,
>
> Please review the patch-set.
>
> This is a first step towards introducing model for IBM's Flexible
> Service Interface. The full functionality will be implemented over the
> time.
You have a typo in the subject, I think you meant to write FSI instead of FSP.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 0/7] Introduce model for IBM's FSP
2023-08-28 8:25 ` [PATCH v1 0/7] Introduce model for IBM's FSP Joel Stanley
@ 2023-08-28 21:13 ` Ninad Palsule
0 siblings, 0 replies; 28+ messages in thread
From: Ninad Palsule @ 2023-08-28 21:13 UTC (permalink / raw)
To: Joel Stanley
Cc: qemu-devel, clg, peter.maydell, andrew, pbonzini,
marcandre.lureau, berrange, thuth, philmd, qemu-arm
Hi Joel,
On 8/28/23 23:25, Joel Stanley wrote:
> Hi Ninad,
>
> On Fri, 25 Aug 2023 at 20:51, Ninad Palsule <ninad@linux.ibm.com> wrote:
>> Hello,
>>
>> Please review the patch-set.
>>
>> This is a first step towards introducing model for IBM's Flexible
>> Service Interface. The full functionality will be implemented over the
>> time.
> You have a typo in the subject, I think you meant to write FSI instead of FSP.
Good catch. Fixed the typo.
Thank you for the review.
Ninad
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v1 0/7] Introduce model for IBM's FSP
2023-08-25 20:30 [PATCH v1 0/7] Introduce model for IBM's FSP Ninad Palsule
` (7 preceding siblings ...)
2023-08-28 8:25 ` [PATCH v1 0/7] Introduce model for IBM's FSP Joel Stanley
@ 2023-08-28 8:49 ` Cédric Le Goater
2023-08-30 2:30 ` Ninad Palsule
8 siblings, 1 reply; 28+ messages in thread
From: Cédric Le Goater @ 2023-08-28 8:49 UTC (permalink / raw)
To: Ninad Palsule, qemu-devel, peter.maydell, andrew, joel, pbonzini,
marcandre.lureau, berrange, thuth, philmd
Cc: qemu-arm
Hello Ninad,
On 8/25/23 22:30, Ninad Palsule wrote:
> Hello,
>
> Please review the patch-set.
>
> This is a first step towards introducing model for IBM's Flexible
> Service Interface. The full functionality will be implemented over the
> time.
>
> Ninad Palsule (7):
> hw/fsi: Introduce IBM's Local bus
> hw/fsi: Introduce IBM's scratchpad
> hw/fsi: Introduce IBM's cfam,fsi-slave
> hw/fsi: Introduce IBM's FSI
> hw/fsi: IBM's On-chip Peripheral Bus
> hw/fsi: Aspeed APB2OPB interface
> hw/arm: Hook up FSI module in AST2600
>
> hw/Kconfig | 1 +
> hw/arm/Kconfig | 1 +
> hw/arm/aspeed_ast2600.c | 15 ++
> hw/fsi/Kconfig | 23 ++
> hw/fsi/aspeed-apb2opb.c | 346 +++++++++++++++++++++++++++++
> hw/fsi/cfam.c | 236 ++++++++++++++++++++
> hw/fsi/engine-scratchpad.c | 100 +++++++++
> hw/fsi/fsi-master.c | 202 +++++++++++++++++
> hw/fsi/fsi-slave.c | 109 +++++++++
> hw/fsi/fsi.c | 54 +++++
> hw/fsi/lbus.c | 94 ++++++++
> hw/fsi/meson.build | 6 +
> hw/fsi/opb.c | 194 ++++++++++++++++
> hw/fsi/trace-events | 2 +
> hw/fsi/trace.h | 1 +
> hw/meson.build | 1 +
> include/hw/arm/aspeed_soc.h | 4 +
> include/hw/fsi/aspeed-apb2opb.h | 32 +++
> include/hw/fsi/bits.h | 15 ++
> include/hw/fsi/cfam.h | 59 +++++
> include/hw/fsi/engine-scratchpad.h | 32 +++
> include/hw/fsi/fsi-master.h | 30 +++
> include/hw/fsi/fsi-slave.h | 29 +++
> include/hw/fsi/fsi.h | 35 +++
> include/hw/fsi/lbus.h | 57 +++++
> include/hw/fsi/opb.h | 45 ++++
> meson.build | 1 +
Thanks for creating a series for these models.
I think the commit logs have a lot of useful information which would
be good for some docs/specs/ file. Please add a need a entry in
MAINTAINERS for this new bus model.
Tests are *very* much welcome also. Ideally we should have a couple
in qtest. A minimum would be an avocado test running pdbg.
Thanks,
C.
> 27 files changed, 1724 insertions(+)
> create mode 100644 hw/fsi/Kconfig
> create mode 100644 hw/fsi/aspeed-apb2opb.c
> create mode 100644 hw/fsi/cfam.c
> create mode 100644 hw/fsi/engine-scratchpad.c
> create mode 100644 hw/fsi/fsi-master.c
> create mode 100644 hw/fsi/fsi-slave.c
> create mode 100644 hw/fsi/fsi.c
> create mode 100644 hw/fsi/lbus.c
> create mode 100644 hw/fsi/meson.build
> create mode 100644 hw/fsi/opb.c
> create mode 100644 hw/fsi/trace-events
> create mode 100644 hw/fsi/trace.h
> create mode 100644 include/hw/fsi/aspeed-apb2opb.h
> create mode 100644 include/hw/fsi/bits.h
> create mode 100644 include/hw/fsi/cfam.h
> create mode 100644 include/hw/fsi/engine-scratchpad.h
> create mode 100644 include/hw/fsi/fsi-master.h
> create mode 100644 include/hw/fsi/fsi-slave.h
> create mode 100644 include/hw/fsi/fsi.h
> create mode 100644 include/hw/fsi/lbus.h
> create mode 100644 include/hw/fsi/opb.h
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v1 0/7] Introduce model for IBM's FSP
2023-08-28 8:49 ` Cédric Le Goater
@ 2023-08-30 2:30 ` Ninad Palsule
0 siblings, 0 replies; 28+ messages in thread
From: Ninad Palsule @ 2023-08-30 2:30 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel, peter.maydell, andrew, joel,
pbonzini, marcandre.lureau, berrange, thuth, philmd
Cc: qemu-arm
Hello Cedric,
On 8/28/23 03:49, Cédric Le Goater wrote:
> Hello Ninad,
>
> On 8/25/23 22:30, Ninad Palsule wrote:
>> Hello,
>>
>> Please review the patch-set.
>>
>> This is a first step towards introducing model for IBM's Flexible
>> Service Interface. The full functionality will be implemented over the
>> time.
>>
>> Ninad Palsule (7):
>> hw/fsi: Introduce IBM's Local bus
>> hw/fsi: Introduce IBM's scratchpad
>> hw/fsi: Introduce IBM's cfam,fsi-slave
>> hw/fsi: Introduce IBM's FSI
>> hw/fsi: IBM's On-chip Peripheral Bus
>> hw/fsi: Aspeed APB2OPB interface
>> hw/arm: Hook up FSI module in AST2600
>>
>> hw/Kconfig | 1 +
>> hw/arm/Kconfig | 1 +
>> hw/arm/aspeed_ast2600.c | 15 ++
>> hw/fsi/Kconfig | 23 ++
>> hw/fsi/aspeed-apb2opb.c | 346 +++++++++++++++++++++++++++++
>> hw/fsi/cfam.c | 236 ++++++++++++++++++++
>> hw/fsi/engine-scratchpad.c | 100 +++++++++
>> hw/fsi/fsi-master.c | 202 +++++++++++++++++
>> hw/fsi/fsi-slave.c | 109 +++++++++
>> hw/fsi/fsi.c | 54 +++++
>> hw/fsi/lbus.c | 94 ++++++++
>> hw/fsi/meson.build | 6 +
>> hw/fsi/opb.c | 194 ++++++++++++++++
>> hw/fsi/trace-events | 2 +
>> hw/fsi/trace.h | 1 +
>> hw/meson.build | 1 +
>> include/hw/arm/aspeed_soc.h | 4 +
>> include/hw/fsi/aspeed-apb2opb.h | 32 +++
>> include/hw/fsi/bits.h | 15 ++
>> include/hw/fsi/cfam.h | 59 +++++
>> include/hw/fsi/engine-scratchpad.h | 32 +++
>> include/hw/fsi/fsi-master.h | 30 +++
>> include/hw/fsi/fsi-slave.h | 29 +++
>> include/hw/fsi/fsi.h | 35 +++
>> include/hw/fsi/lbus.h | 57 +++++
>> include/hw/fsi/opb.h | 45 ++++
>> meson.build | 1 +
>
> Thanks for creating a series for these models.
>
> I think the commit logs have a lot of useful information which would
> be good for some docs/specs/ file. Please add a need a entry in
> MAINTAINERS for this new bus model.
>
> Tests are *very* much welcome also. Ideally we should have a couple
> in qtest. A minimum would be an avocado test running pdbg.
I have added document, MAINTAINERS and qtest.
Thank you for the review.
~Ninad
>
> Thanks,
>
> C.
>
>
>
>> 27 files changed, 1724 insertions(+)
>> create mode 100644 hw/fsi/Kconfig
>> create mode 100644 hw/fsi/aspeed-apb2opb.c
>> create mode 100644 hw/fsi/cfam.c
>> create mode 100644 hw/fsi/engine-scratchpad.c
>> create mode 100644 hw/fsi/fsi-master.c
>> create mode 100644 hw/fsi/fsi-slave.c
>> create mode 100644 hw/fsi/fsi.c
>> create mode 100644 hw/fsi/lbus.c
>> create mode 100644 hw/fsi/meson.build
>> create mode 100644 hw/fsi/opb.c
>> create mode 100644 hw/fsi/trace-events
>> create mode 100644 hw/fsi/trace.h
>> create mode 100644 include/hw/fsi/aspeed-apb2opb.h
>> create mode 100644 include/hw/fsi/bits.h
>> create mode 100644 include/hw/fsi/cfam.h
>> create mode 100644 include/hw/fsi/engine-scratchpad.h
>> create mode 100644 include/hw/fsi/fsi-master.h
>> create mode 100644 include/hw/fsi/fsi-slave.h
>> create mode 100644 include/hw/fsi/fsi.h
>> create mode 100644 include/hw/fsi/lbus.h
>> create mode 100644 include/hw/fsi/opb.h
>>
>
^ permalink raw reply [flat|nested] 28+ messages in thread