diff mbox series

[v6,02/10] hw/fsi: Introduce IBM's scratchpad

Message ID 20231021211720.3571082-3-ninad@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series Introduce model for IBM's FSI | expand

Commit Message

Ninad Palsule Oct. 21, 2023, 9:17 p.m. UTC 
This is a part of patchset where scratchpad 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.

In this model, The LBUS device is parent for the scratchpad.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
---
v2:
- Incorporated Joel's review comments.
v5:
- Incorporated review comments by Cedric.
v6:
- Incorporated review comments by Daniel.
---
 meson.build                        |  1 +
 hw/fsi/trace.h                     |  1 +
 include/hw/fsi/engine-scratchpad.h | 32 ++++++++++
 include/hw/fsi/fsi.h               | 16 +++++
 hw/fsi/engine-scratchpad.c         | 93 ++++++++++++++++++++++++++++++
 hw/fsi/Kconfig                     |  4 ++
 hw/fsi/meson.build                 |  1 +
 hw/fsi/trace-events                |  2 +
 8 files changed, 150 insertions(+)
 create mode 100644 hw/fsi/trace.h
 create mode 100644 include/hw/fsi/engine-scratchpad.h
 create mode 100644 include/hw/fsi/fsi.h
 create mode 100644 hw/fsi/engine-scratchpad.c
 create mode 100644 hw/fsi/trace-events

Comments

Philippe Mathieu-Daudé Oct. 23, 2023, 3 p.m. UTC | #1 
On 21/10/23 23:17, Ninad Palsule wrote:
> This is a part of patchset where scratchpad 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.
> 
> In this model, The LBUS device is parent for the scratchpad.
> 
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
> ---
> v2:
> - Incorporated Joel's review comments.
> v5:
> - Incorporated review comments by Cedric.
> v6:
> - Incorporated review comments by Daniel.
> ---
>   meson.build                        |  1 +
>   hw/fsi/trace.h                     |  1 +
>   include/hw/fsi/engine-scratchpad.h | 32 ++++++++++
>   include/hw/fsi/fsi.h               | 16 +++++
>   hw/fsi/engine-scratchpad.c         | 93 ++++++++++++++++++++++++++++++
>   hw/fsi/Kconfig                     |  4 ++
>   hw/fsi/meson.build                 |  1 +
>   hw/fsi/trace-events                |  2 +
>   8 files changed, 150 insertions(+)
>   create mode 100644 hw/fsi/trace.h
>   create mode 100644 include/hw/fsi/engine-scratchpad.h
>   create mode 100644 include/hw/fsi/fsi.h
>   create mode 100644 hw/fsi/engine-scratchpad.c
>   create mode 100644 hw/fsi/trace-events


> diff --git a/include/hw/fsi/fsi.h b/include/hw/fsi/fsi.h
> new file mode 100644
> index 0000000000..e65f26f17b
> --- /dev/null
> +++ b/include/hw/fsi/fsi.h
> @@ -0,0 +1,16 @@
> +/*
> + * 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
> +
> +/* Bitwise operations at the word level. */
> +#define BE_BIT(x)                          BIT(31 - (x))
> +#define GENMASK(t, b) \
> +    (((1ULL << ((t) + 1)) - 1) & ~((1ULL << (b)) - 1))

Please use MAKE_64BIT_MASK() from "qemu/bitops.h".

> +#define BE_GENMASK(t, b)                   GENMASK(BE_BIT(t), BE_BIT(b))
> +
> +#endif
Ninad Palsule Oct. 23, 2023, 5:08 p.m. UTC | #2 
Hello Philippe,

On 10/23/23 10:00, Philippe Mathieu-Daudé wrote:
> On 21/10/23 23:17, Ninad Palsule wrote:
>> This is a part of patchset where scratchpad 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.
>>
>> In this model, The LBUS device is parent for the scratchpad.
>>
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
>> ---
>> v2:
>> - Incorporated Joel's review comments.
>> v5:
>> - Incorporated review comments by Cedric.
>> v6:
>> - Incorporated review comments by Daniel.
>> ---
>>   meson.build                        |  1 +
>>   hw/fsi/trace.h                     |  1 +
>>   include/hw/fsi/engine-scratchpad.h | 32 ++++++++++
>>   include/hw/fsi/fsi.h               | 16 +++++
>>   hw/fsi/engine-scratchpad.c         | 93 ++++++++++++++++++++++++++++++
>>   hw/fsi/Kconfig                     |  4 ++
>>   hw/fsi/meson.build                 |  1 +
>>   hw/fsi/trace-events                |  2 +
>>   8 files changed, 150 insertions(+)
>>   create mode 100644 hw/fsi/trace.h
>>   create mode 100644 include/hw/fsi/engine-scratchpad.h
>>   create mode 100644 include/hw/fsi/fsi.h
>>   create mode 100644 hw/fsi/engine-scratchpad.c
>>   create mode 100644 hw/fsi/trace-events
>
>
>> diff --git a/include/hw/fsi/fsi.h b/include/hw/fsi/fsi.h
>> new file mode 100644
>> index 0000000000..e65f26f17b
>> --- /dev/null
>> +++ b/include/hw/fsi/fsi.h
>> @@ -0,0 +1,16 @@
>> +/*
>> + * 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
>> +
>> +/* Bitwise operations at the word level. */
>> +#define BE_BIT(x)                          BIT(31 - (x))
>> +#define GENMASK(t, b) \
>> +    (((1ULL << ((t) + 1)) - 1) & ~((1ULL << (b)) - 1))
>
> Please use MAKE_64BIT_MASK() from "qemu/bitops.h".

The GENMASK and MAKE_64BIT_MASK macros are invoke differently.

GENMASK is invoked with bit t and bit b (t:b) and it provides the mask and

MAKE_64BIT_MASK uses shift and length.

Thanks for the review.

Regards,

Ninad


>> +#define BE_GENMASK(t, b)                   GENMASK(BE_BIT(t), 
>> BE_BIT(b))
>> +
>> +#endif
>
Philippe Mathieu-Daudé Oct. 24, 2023, 7:08 a.m. UTC | #3 
On 23/10/23 19:08, Ninad Palsule wrote:
> Hello Philippe,
> 
> On 10/23/23 10:00, Philippe Mathieu-Daudé wrote:
>> On 21/10/23 23:17, Ninad Palsule wrote:
>>> This is a part of patchset where scratchpad 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.
>>>
>>> In this model, The LBUS device is parent for the scratchpad.
>>>
>>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
>>> ---
>>> v2:
>>> - Incorporated Joel's review comments.
>>> v5:
>>> - Incorporated review comments by Cedric.
>>> v6:
>>> - Incorporated review comments by Daniel.
>>> ---
>>>   meson.build                        |  1 +
>>>   hw/fsi/trace.h                     |  1 +
>>>   include/hw/fsi/engine-scratchpad.h | 32 ++++++++++
>>>   include/hw/fsi/fsi.h               | 16 +++++
>>>   hw/fsi/engine-scratchpad.c         | 93 ++++++++++++++++++++++++++++++
>>>   hw/fsi/Kconfig                     |  4 ++
>>>   hw/fsi/meson.build                 |  1 +
>>>   hw/fsi/trace-events                |  2 +
>>>   8 files changed, 150 insertions(+)
>>>   create mode 100644 hw/fsi/trace.h
>>>   create mode 100644 include/hw/fsi/engine-scratchpad.h
>>>   create mode 100644 include/hw/fsi/fsi.h
>>>   create mode 100644 hw/fsi/engine-scratchpad.c
>>>   create mode 100644 hw/fsi/trace-events
>>
>>
>>> diff --git a/include/hw/fsi/fsi.h b/include/hw/fsi/fsi.h
>>> new file mode 100644
>>> index 0000000000..e65f26f17b
>>> --- /dev/null
>>> +++ b/include/hw/fsi/fsi.h
>>> @@ -0,0 +1,16 @@
>>> +/*
>>> + * 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
>>> +
>>> +/* Bitwise operations at the word level. */
>>> +#define BE_BIT(x)                          BIT(31 - (x))
>>> +#define GENMASK(t, b) \
>>> +    (((1ULL << ((t) + 1)) - 1) & ~((1ULL << (b)) - 1))
>>
>> Please use MAKE_64BIT_MASK() from "qemu/bitops.h".
> 
> The GENMASK and MAKE_64BIT_MASK macros are invoke differently.
> 
> GENMASK is invoked with bit t and bit b (t:b) and it provides the mask and
> 
> MAKE_64BIT_MASK uses shift and length.

Don't we have:

#define GENMASK(t, b) MAKE_64BIT_MASK(t, b - t + 1)

?

> Thanks for the review.
> 
> Regards,
> 
> Ninad
> 
> 
>>> +#define BE_GENMASK(t, b)                   GENMASK(BE_BIT(t), 
>>> BE_BIT(b))
>>> +
>>> +#endif
>>
Ninad Palsule Oct. 26, 2023, 3:24 p.m. UTC | #4 
On 10/24/23 02:08, Philippe Mathieu-Daudé wrote:
> On 23/10/23 19:08, Ninad Palsule wrote:
>> Hello Philippe,
>>
>> On 10/23/23 10:00, Philippe Mathieu-Daudé wrote:
>>> On 21/10/23 23:17, Ninad Palsule wrote:
>>>> This is a part of patchset where scratchpad 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.
>>>>
>>>> In this model, The LBUS device is parent for the scratchpad.
>>>>
>>>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>>>> Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
>>>> ---
>>>> v2:
>>>> - Incorporated Joel's review comments.
>>>> v5:
>>>> - Incorporated review comments by Cedric.
>>>> v6:
>>>> - Incorporated review comments by Daniel.
>>>> ---
>>>>   meson.build                        |  1 +
>>>>   hw/fsi/trace.h                     |  1 +
>>>>   include/hw/fsi/engine-scratchpad.h | 32 ++++++++++
>>>>   include/hw/fsi/fsi.h               | 16 +++++
>>>>   hw/fsi/engine-scratchpad.c         | 93 
>>>> ++++++++++++++++++++++++++++++
>>>>   hw/fsi/Kconfig                     |  4 ++
>>>>   hw/fsi/meson.build                 |  1 +
>>>>   hw/fsi/trace-events                |  2 +
>>>>   8 files changed, 150 insertions(+)
>>>>   create mode 100644 hw/fsi/trace.h
>>>>   create mode 100644 include/hw/fsi/engine-scratchpad.h
>>>>   create mode 100644 include/hw/fsi/fsi.h
>>>>   create mode 100644 hw/fsi/engine-scratchpad.c
>>>>   create mode 100644 hw/fsi/trace-events
>>>
>>>
>>>> diff --git a/include/hw/fsi/fsi.h b/include/hw/fsi/fsi.h
>>>> new file mode 100644
>>>> index 0000000000..e65f26f17b
>>>> --- /dev/null
>>>> +++ b/include/hw/fsi/fsi.h
>>>> @@ -0,0 +1,16 @@
>>>> +/*
>>>> + * 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
>>>> +
>>>> +/* Bitwise operations at the word level. */
>>>> +#define BE_BIT(x)                          BIT(31 - (x))
>>>> +#define GENMASK(t, b) \
>>>> +    (((1ULL << ((t) + 1)) - 1) & ~((1ULL << (b)) - 1))
>>>
>>> Please use MAKE_64BIT_MASK() from "qemu/bitops.h".
>>
>> The GENMASK and MAKE_64BIT_MASK macros are invoke differently.
>>
>> GENMASK is invoked with bit t and bit b (t:b) and it provides the 
>> mask and
>>
>> MAKE_64BIT_MASK uses shift and length.
>
> Don't we have:
>
> #define GENMASK(t, b) MAKE_64BIT_MASK(t, b - t + 1)
>
> ?

You are right. I am able to use this macro. I have removed some unused 
macros.

Thanks for the review.

Regards,

Ninad

>
>> Thanks for the review.
>>
>> Regards,
>>
>> Ninad
>>
>>
>>>> +#define BE_GENMASK(t, b)                   GENMASK(BE_BIT(t), 
>>>> BE_BIT(b))
>>>> +
>>>> +#endif
>>>
>
diff mbox series

Patch

diff --git a/meson.build b/meson.build
index 4961c82a6b..31078f0bb3 100644
--- a/meson.build
+++ b/meson.build
@@ -3256,6 +3256,7 @@  if have_system
     'hw/char',
     'hw/display',
     'hw/dma',
+    'hw/fsi',
     'hw/hyperv',
     'hw/i2c',
     'hw/i386',
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/engine-scratchpad.h b/include/hw/fsi/engine-scratchpad.h
new file mode 100644
index 0000000000..959a8a6677
--- /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/fsi.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_FSI_SCRATCHPAD "fsi.scratchpad"
+#define SCRATCHPAD(obj) OBJECT_CHECK(FSIScratchPad, (obj), TYPE_FSI_SCRATCHPAD)
+
+typedef struct FSIScratchPad {
+        FSILBusDevice parent;
+
+        uint32_t reg;
+} FSIScratchPad;
+
+#endif /* FSI_ENGINE_SCRATCHPAD_H */
diff --git a/include/hw/fsi/fsi.h b/include/hw/fsi/fsi.h
new file mode 100644
index 0000000000..e65f26f17b
--- /dev/null
+++ b/include/hw/fsi/fsi.h
@@ -0,0 +1,16 @@ 
+/*
+ * 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
+
+/* Bitwise operations at the word level. */
+#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
diff --git a/hw/fsi/engine-scratchpad.c b/hw/fsi/engine-scratchpad.c
new file mode 100644
index 0000000000..a8887cd613
--- /dev/null
+++ b/hw/fsi/engine-scratchpad.c
@@ -0,0 +1,93 @@ 
+/*
+ * 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 "trace.h"
+
+#include "hw/fsi/engine-scratchpad.h"
+
+static uint64_t fsi_scratchpad_read(void *opaque, hwaddr addr, unsigned size)
+{
+    FSIScratchPad *s = SCRATCHPAD(opaque);
+
+    trace_fsi_scratchpad_read(addr, size);
+
+    if (addr) {
+        return 0;
+    }
+
+    return s->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);
+
+    if (addr) {
+        return;
+    }
+
+    s->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);
+
+    s->reg = 0;
+}
+
+static void fsi_scratchpad_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    FSILBusDeviceClass *ldc = FSI_LBUS_DEVICE_CLASS(klass);
+
+    dc->realize = fsi_scratchpad_realize;
+    dc->reset = fsi_scratchpad_reset;
+
+    ldc->config =
+          ENGINE_CONFIG_NEXT            | /* valid */
+          0x00010000                    | /* slots */
+          0x00001000                    | /* version */
+          ENGINE_CONFIG_TYPE_SCRATCHPAD | /* type */
+          0x00000007;                     /* crc */
+}
+
+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,
+    .class_size = sizeof(FSILBusDeviceClass),
+};
+
+static void fsi_scratchpad_register_types(void)
+{
+    type_register_static(&fsi_scratchpad_info);
+}
+
+type_init(fsi_scratchpad_register_types);
diff --git a/hw/fsi/Kconfig b/hw/fsi/Kconfig
index e650c660f0..f7c7fd1b28 100644
--- a/hw/fsi/Kconfig
+++ b/hw/fsi/Kconfig
@@ -1,2 +1,6 @@ 
+config FSI_SCRATCHPAD
+    bool
+    select FSI_LBUS
+
 config FSI_LBUS
     bool
diff --git a/hw/fsi/meson.build b/hw/fsi/meson.build
index 4074d3a7d2..d45a98c223 100644
--- a/hw/fsi/meson.build
+++ b/hw/fsi/meson.build
@@ -1 +1,2 @@ 
 system_ss.add(when: 'CONFIG_FSI_LBUS', if_true: files('lbus.c'))
+system_ss.add(when: 'CONFIG_FSI_SCRATCHPAD', if_true: files('engine-scratchpad.c'))
diff --git a/hw/fsi/trace-events b/hw/fsi/trace-events
new file mode 100644
index 0000000000..c5753e2791
--- /dev/null
+++ 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