qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 2/3] hw/ppc: Add nest1 chiplet model


From: Chalapathi V
Subject: Re: [PATCH v5 2/3] hw/ppc: Add nest1 chiplet model
Date: Sun, 26 Nov 2023 14:59:24 +0530
User-agent: Mozilla Thunderbird


On 24-11-2023 18:31, Nicholas Piggin wrote:
On Fri Nov 24, 2023 at 10:19 PM AEST, Cédric Le Goater wrote:
On 11/24/23 12:26, Nicholas Piggin wrote:
For this and actually the last patch too, it would be good to mention
(possibly in a header comment in the file too) what actual functionality
is being provided/modeled. It looks like it's just modeling behaviour of
reads and writes for some registers.

Oh, and sorry I didn't follow development and comments on this too
closely, so forgive me if I've missed things already said. I'll go
back and read through the series.

On Fri Nov 24, 2023 at 8:15 PM AEST, Chalapathi V wrote:
The nest1 chiplet handle the high speed i/o traffic over PCIe and others.
The nest1 chiplet consists of PowerBus Fabric controller,
nest Memory Management Unit, chiplet control unit and more.

This commit creates a nest1 chiplet model and initialize and realize the
pervasive chiplet model where chiplet control registers are implemented.

This commit also implement the read/write method for the powerbus scom
registers
The powerbus scom registers, are those specifically for the PowerBus
Fabric controller mentioned in the first paragraph, or is it a more
general set of registers for the chiplet?
Yes, They are for the PowerBus racetrack unit.
Signed-off-by: Chalapathi V <chalapathi.v@linux.ibm.com>
---
   include/hw/ppc/pnv_nest_chiplet.h |  36 ++++++
   include/hw/ppc/pnv_xscom.h        |   6 +
   hw/ppc/pnv_nest1_chiplet.c        | 197 ++++++++++++++++++++++++++++++
   hw/ppc/meson.build                |   1 +
   4 files changed, 240 insertions(+)
   create mode 100644 include/hw/ppc/pnv_nest_chiplet.h
   create mode 100644 hw/ppc/pnv_nest1_chiplet.c

diff --git a/include/hw/ppc/pnv_nest_chiplet.h 
b/include/hw/ppc/pnv_nest_chiplet.h
new file mode 100644
index 0000000000..845030fb1a
--- /dev/null
+++ b/include/hw/ppc/pnv_nest_chiplet.h
@@ -0,0 +1,36 @@
+/*
+ * QEMU PowerPC nest chiplet model
+ *
+ * Copyright (c) 2023, IBM Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This code is licensed under the GPL version 2 or later. See the
+ * COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef PPC_PNV_NEST1_CHIPLET_H
+#define PPC_PNV_NEST1_CHIPLET_H
+
+#include "hw/ppc/pnv_pervasive.h"
+
+#define TYPE_PNV_NEST1 "pnv-nest1-chiplet"
+#define PNV_NEST1(obj) OBJECT_CHECK(PnvNest1, (obj), TYPE_PNV_NEST1)
+
+typedef struct pb_scom {
+    uint64_t mode;
+    uint64_t hp_mode2_curr;
+} pb_scom;
+
+typedef struct PnvNest1 {
Naming nitpicking again...

The main ifndef guard for header files should match the file name, so
the file should be called pnv_nest1_chiplet.h (and that matches the .c
file too).

I think this struct should be called Nest1Chiplet too.
I asked Chalapathi to do the exact opposit :)
Oops :)

I don't mind really, my argument was that most models represent HW logic
units or subunits of a bigger unit. I don't see the point in adding a
chip/chiplet suffix apart from PnvChip since it represents a socket or
processor.

You choose. I will keep quiet :)
Ah. I can see that side of it. And for many of the nest chiplets (MC,
PAU, PCI) that makes sense. For Nest0 and Nest1... it's a bit
overloaded. First of all, all the nest chiplets are "nest". Then
there is also some nest units inside the processor chiplets (L2, L3,
NCU are considered to be nest). And then the nest also has a Pervasive
Chiplet itself, and we also have these pervasive registers in each
chiplet, etc., etc.

So my worry is we'll run into confusion if we shorten names too much.

We can always rename things, so it won't be the end of the world, but
thinking about the pervasive chiplet, I think we can already see that
"PnvPervasive" would not be a good name for it.

The chiplets have short names actually if that would help. Nest 1 is
called N1, so we could call it PnvN1Chiplet. That seems the usual
way to refer to them in docs, so I think a better name.

Thanks,
Nick
Sure. Will rename this to PnvN1Chiplet.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]