[PATCH v11 02/11] hw/fsi: Introduce IBM's scratchpad device

Ninad Palsule posted 11 patches 10 months ago
Maintainers: Ninad Palsule <ninad@linux.ibm.com>, "Cédric Le Goater" <clg@kaod.org>, Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[PATCH v11 02/11] hw/fsi: Introduce IBM's scratchpad device
Posted by Ninad Palsule 10 months ago
This is a part of patchset where IBM's Flexible Service Interface is
introduced.

The scratchpad provides a set of non-functional registers. The firmware
is free to use them, hardware does not support any special management
support. The scratchpad registers can be read or written from LBUS
slave. The scratch pad is managed under FSI CFAM state.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
[ clg: - moved object FSIScratchPad under FSICFAMState
       - moved FSIScratchPad code under cfam.c ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
---
 include/hw/fsi/lbus.h | 11 +++++++
 hw/fsi/lbus.c         | 69 +++++++++++++++++++++++++++++++++++++++++++
 hw/fsi/trace-events   |  2 ++
 3 files changed, 82 insertions(+)

diff --git a/include/hw/fsi/lbus.h b/include/hw/fsi/lbus.h
index e8a22e22a8..558268c013 100644
--- a/include/hw/fsi/lbus.h
+++ b/include/hw/fsi/lbus.h
@@ -29,4 +29,15 @@ typedef struct FSILBus {
     MemoryRegion mr;
 } FSILBus;
 
+#define TYPE_FSI_SCRATCHPAD "fsi.scratchpad"
+#define SCRATCHPAD(obj) OBJECT_CHECK(FSIScratchPad, (obj), TYPE_FSI_SCRATCHPAD)
+
+#define FSI_SCRATCHPAD_NR_REGS 4
+
+typedef struct FSIScratchPad {
+        FSILBusDevice parent;
+
+        uint32_t regs[FSI_SCRATCHPAD_NR_REGS];
+} FSIScratchPad;
+
 #endif /* FSI_LBUS_H */
diff --git a/hw/fsi/lbus.c b/hw/fsi/lbus.c
index 44d2319087..5ab7d0a741 100644
--- a/hw/fsi/lbus.c
+++ b/hw/fsi/lbus.c
@@ -13,6 +13,8 @@
 
 #include "trace.h"
 
+#define TO_REG(offset) ((offset) >> 2)
+
 static void fsi_lbus_init(Object *o)
 {
     FSILBus *lbus = FSI_LBUS(o);
@@ -34,10 +36,77 @@ static const TypeInfo fsi_lbus_device_type_info = {
     .abstract = true,
 };
 
+static uint64_t fsi_scratchpad_read(void *opaque, hwaddr addr, unsigned size)
+{
+    FSIScratchPad *s = SCRATCHPAD(opaque);
+    int reg = TO_REG(addr);
+
+    trace_fsi_scratchpad_read(addr, size);
+
+    if (reg >= FSI_SCRATCHPAD_NR_REGS) {
+        return 0;
+    }
+
+    return s->regs[reg];
+}
+
+static void fsi_scratchpad_write(void *opaque, hwaddr addr, uint64_t data,
+                                 unsigned size)
+{
+    FSIScratchPad *s = SCRATCHPAD(opaque);
+
+    trace_fsi_scratchpad_write(addr, size, data);
+    int reg = TO_REG(addr);
+
+    if (reg >= FSI_SCRATCHPAD_NR_REGS) {
+        return;
+    }
+
+    s->regs[reg] = data;
+}
+
+static const struct MemoryRegionOps scratchpad_ops = {
+    .read = fsi_scratchpad_read,
+    .write = fsi_scratchpad_write,
+    .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void fsi_scratchpad_realize(DeviceState *dev, Error **errp)
+{
+    FSILBusDevice *ldev = FSI_LBUS_DEVICE(dev);
+
+    memory_region_init_io(&ldev->iomem, OBJECT(ldev), &scratchpad_ops,
+                          ldev, TYPE_FSI_SCRATCHPAD, 0x400);
+}
+
+static void fsi_scratchpad_reset(DeviceState *dev)
+{
+    FSIScratchPad *s = SCRATCHPAD(dev);
+
+    memset(s->regs, 0, sizeof(s->regs));
+}
+
+static void fsi_scratchpad_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->bus_type = TYPE_FSI_LBUS;
+    dc->realize = fsi_scratchpad_realize;
+    dc->reset = fsi_scratchpad_reset;
+}
+
+static const TypeInfo fsi_scratchpad_info = {
+    .name = TYPE_FSI_SCRATCHPAD,
+    .parent = TYPE_FSI_LBUS_DEVICE,
+    .instance_size = sizeof(FSIScratchPad),
+    .class_init = fsi_scratchpad_class_init,
+};
+
 static void fsi_lbus_register_types(void)
 {
     type_register_static(&fsi_lbus_info);
     type_register_static(&fsi_lbus_device_type_info);
+    type_register_static(&fsi_scratchpad_info);
 }
 
 type_init(fsi_lbus_register_types);
diff --git a/hw/fsi/trace-events b/hw/fsi/trace-events
index e69de29bb2..c5753e2791 100644
--- a/hw/fsi/trace-events
+++ b/hw/fsi/trace-events
@@ -0,0 +1,2 @@
+fsi_scratchpad_read(uint64_t addr, uint32_t size) "@0x%" PRIx64 " size=%d"
+fsi_scratchpad_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size=%d value=0x%"PRIx64
-- 
2.39.2


Re: [PATCH v11 02/11] hw/fsi: Introduce IBM's scratchpad device
Posted by Cédric Le Goater 10 months ago
On 1/26/24 04:40, Ninad Palsule wrote:
> This is a part of patchset where IBM's Flexible Service Interface is
> introduced.
> 
> The scratchpad provides a set of non-functional registers. The firmware
> is free to use them, hardware does not support any special management
> support. The scratchpad registers can be read or written from LBUS
> slave. The scratch pad is managed under FSI CFAM state.
> 
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> [ clg: - moved object FSIScratchPad under FSICFAMState
>         - moved FSIScratchPad code under cfam.c ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
> ---
>   include/hw/fsi/lbus.h | 11 +++++++
>   hw/fsi/lbus.c         | 69 +++++++++++++++++++++++++++++++++++++++++++
>   hw/fsi/trace-events   |  2 ++
>   3 files changed, 82 insertions(+)
> 
> diff --git a/include/hw/fsi/lbus.h b/include/hw/fsi/lbus.h
> index e8a22e22a8..558268c013 100644
> --- a/include/hw/fsi/lbus.h
> +++ b/include/hw/fsi/lbus.h
> @@ -29,4 +29,15 @@ typedef struct FSILBus {
>       MemoryRegion mr;
>   } FSILBus;
>   
> +#define TYPE_FSI_SCRATCHPAD "fsi.scratchpad"
> +#define SCRATCHPAD(obj) OBJECT_CHECK(FSIScratchPad, (obj), TYPE_FSI_SCRATCHPAD)
> +
> +#define FSI_SCRATCHPAD_NR_REGS 4
> +
> +typedef struct FSIScratchPad {
> +        FSILBusDevice parent;
> +
> +        uint32_t regs[FSI_SCRATCHPAD_NR_REGS];
> +} FSIScratchPad;
> +
>   #endif /* FSI_LBUS_H */
> diff --git a/hw/fsi/lbus.c b/hw/fsi/lbus.c
> index 44d2319087..5ab7d0a741 100644
> --- a/hw/fsi/lbus.c
> +++ b/hw/fsi/lbus.c
> @@ -13,6 +13,8 @@
>   
>   #include "trace.h"
>   
> +#define TO_REG(offset) ((offset) >> 2)
> +
>   static void fsi_lbus_init(Object *o)
>   {
>       FSILBus *lbus = FSI_LBUS(o);
> @@ -34,10 +36,77 @@ static const TypeInfo fsi_lbus_device_type_info = {
>       .abstract = true,
>   };
>   
> +static uint64_t fsi_scratchpad_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    FSIScratchPad *s = SCRATCHPAD(opaque);
> +    int reg = TO_REG(addr);
> +
> +    trace_fsi_scratchpad_read(addr, size);
> +
> +    if (reg >= FSI_SCRATCHPAD_NR_REGS) {

usually, the model logs a GUEST_ERROR in such case, specially when the MMIO
window is larger than the register set. Same comment for the write memop.

with that,
  
Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.



> +        return 0;
> +    }
> +
> +    return s->regs[reg];
> +}
> +
> +static void fsi_scratchpad_write(void *opaque, hwaddr addr, uint64_t data,
> +                                 unsigned size)
> +{
> +    FSIScratchPad *s = SCRATCHPAD(opaque);
> +
> +    trace_fsi_scratchpad_write(addr, size, data);
> +    int reg = TO_REG(addr);
> +
> +    if (reg >= FSI_SCRATCHPAD_NR_REGS) {
> +        return;
> +    }
> +
> +    s->regs[reg] = data;
> +}
> +
> +static const struct MemoryRegionOps scratchpad_ops = {
> +    .read = fsi_scratchpad_read,
> +    .write = fsi_scratchpad_write,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> +static void fsi_scratchpad_realize(DeviceState *dev, Error **errp)
> +{
> +    FSILBusDevice *ldev = FSI_LBUS_DEVICE(dev);
> +
> +    memory_region_init_io(&ldev->iomem, OBJECT(ldev), &scratchpad_ops,
> +                          ldev, TYPE_FSI_SCRATCHPAD, 0x400);
> +}
> +
> +static void fsi_scratchpad_reset(DeviceState *dev)
> +{
> +    FSIScratchPad *s = SCRATCHPAD(dev);
> +
> +    memset(s->regs, 0, sizeof(s->regs));
> +}
> +
> +static void fsi_scratchpad_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->bus_type = TYPE_FSI_LBUS;
> +    dc->realize = fsi_scratchpad_realize;
> +    dc->reset = fsi_scratchpad_reset;
> +}
> +
> +static const TypeInfo fsi_scratchpad_info = {
> +    .name = TYPE_FSI_SCRATCHPAD,
> +    .parent = TYPE_FSI_LBUS_DEVICE,
> +    .instance_size = sizeof(FSIScratchPad),
> +    .class_init = fsi_scratchpad_class_init,
> +};
> +
>   static void fsi_lbus_register_types(void)
>   {
>       type_register_static(&fsi_lbus_info);
>       type_register_static(&fsi_lbus_device_type_info);
> +    type_register_static(&fsi_scratchpad_info);
>   }
>   
>   type_init(fsi_lbus_register_types);
> diff --git a/hw/fsi/trace-events b/hw/fsi/trace-events
> index e69de29bb2..c5753e2791 100644
> --- a/hw/fsi/trace-events
> +++ b/hw/fsi/trace-events
> @@ -0,0 +1,2 @@
> +fsi_scratchpad_read(uint64_t addr, uint32_t size) "@0x%" PRIx64 " size=%d"
> +fsi_scratchpad_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 " size=%d value=0x%"PRIx64


Re: [PATCH v11 02/11] hw/fsi: Introduce IBM's scratchpad device
Posted by Ninad Palsule 10 months ago
Hello Cedric,


>>   +static uint64_t fsi_scratchpad_read(void *opaque, hwaddr addr, 
>> unsigned size)
>> +{
>> +    FSIScratchPad *s = SCRATCHPAD(opaque);
>> +    int reg = TO_REG(addr);
>> +
>> +    trace_fsi_scratchpad_read(addr, size);
>> +
>> +    if (reg >= FSI_SCRATCHPAD_NR_REGS) {
>
> usually, the model logs a GUEST_ERROR in such case, specially when the 
> MMIO
> window is larger than the register set. Same comment for the write memop.
>
> with that,
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>

Added error log. Added tag.

Thanks for the review.

Regards,

Ninad