[PATCH v5 1/8] i386: Add Intel RDT device and State to config.

Hendrik Wuethrich posted 8 patches 5 months ago
[PATCH v5 1/8] i386: Add Intel RDT device and State to config.
Posted by Hendrik Wuethrich 5 months ago
From: ‪Hendrik Wüthrich <whendrik@google.com>

Change config to show RDT, add minimal code to the rdt.c module to make
sure things still compile.

Signed-off-by: Hendrik Wüthrich <whendrik@google.com>
---
 hw/i386/Kconfig       |  4 ++
 hw/i386/meson.build   |  1 +
 hw/i386/rdt.c         | 99 +++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/rdt.h | 25 +++++++++++
 target/i386/cpu.h     |  3 ++
 5 files changed, 132 insertions(+)
 create mode 100644 hw/i386/rdt.c
 create mode 100644 include/hw/i386/rdt.h

diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 32818480d2..0186b85c3e 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -10,6 +10,9 @@ config SGX
     bool
     depends on KVM
 
+config RDT
+    bool
+
 config PC
     bool
     imply APPLESMC
@@ -26,6 +29,7 @@ config PC
     imply QXL
     imply SEV
     imply SGX
+    imply RDT
     imply TEST_DEVICES
     imply TPM_CRB
     imply TPM_TIS_ISA
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index 10bdfde27c..3a697dcc03 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -22,6 +22,7 @@ i386_ss.add(when: 'CONFIG_VMPORT', if_true: files('vmport.c'))
 i386_ss.add(when: 'CONFIG_VTD', if_true: files('intel_iommu.c'))
 i386_ss.add(when: 'CONFIG_SGX', if_true: files('sgx-epc.c','sgx.c'),
                                 if_false: files('sgx-stub.c'))
+i386_ss.add(when: 'CONFIG_RDT', if_true: files('rdt.c'))
 
 i386_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-common.c'))
 i386_ss.add(when: 'CONFIG_PC', if_true: files(
diff --git a/hw/i386/rdt.c b/hw/i386/rdt.c
new file mode 100644
index 0000000000..b2203197e3
--- /dev/null
+++ b/hw/i386/rdt.c
@@ -0,0 +1,99 @@
+/*
+ * Intel Resource Director Technology (RDT).
+ *
+ * Copyright 2024 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "hw/i386/rdt.h"
+#include "qemu/osdep.h" /* Needs to be included before isa.h */
+#include "hw/isa/isa.h"
+#include "hw/qdev-properties.h"
+#include "qom/object.h"
+
+/* Max counts for allocation masks or CBMs. In other words, the size of
+ * respective MSRs.
+ * L3_MASK and L3_mask are architectural limitations. THRTL_COUNT is just
+ * the space left until the next MSR.
+ * */
+#define RDT_MAX_L3_MASK_COUNT      127
+#define RDT_MAX_L2_MASK_COUNT      63
+#define RDT_MAX_MBA_THRTL_COUNT    63
+
+#define TYPE_RDT "rdt"
+#define RDT_NUM_RMID_PROP "rmids"
+
+OBJECT_DECLARE_TYPE(RDTState, RDTStateClass, RDT);
+
+struct RDTMonitor {
+    uint64_t count_local;
+    uint64_t count_remote;
+    uint64_t count_l3;
+};
+
+struct RDTAllocation {
+    uint32_t active_cos;
+};
+
+struct RDTStatePerCore {
+    uint32_t active_rmid;
+    GArray *monitors;
+
+    /*Parent RDTState*/
+    RDTState *rdtstate;
+};
+
+/*One instance of RDT-internal state to be shared by all cores*/
+struct RDTState {
+    ISADevice parent;
+
+    /*Max amount of RMIDs*/
+    uint32_t rmids;
+
+    /*Per core state*/
+    RDTStatePerCore *rdtInstances;
+    RDTAllocation *allocations;
+
+    /*RDT Allocation bitmask MSRs*/
+    uint32_t msr_L3_ia32_mask_n[RDT_MAX_L3_MASK_COUNT];
+    uint32_t msr_L2_ia32_mask_n[RDT_MAX_L2_MASK_COUNT];
+    uint32_t ia32_L2_qos_ext_bw_thrtl_n[RDT_MAX_MBA_THRTL_COUNT];
+};
+
+struct RDTStateClass {
+};
+
+OBJECT_DEFINE_TYPE(RDTState, rdt, RDT, ISA_DEVICE);
+
+static Property rdt_properties[] = {
+    DEFINE_PROP_UINT32(RDT_NUM_RMID_PROP, RDTState, rmids, 256),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void rdt_init(Object *obj)
+{
+}
+
+static void rdt_finalize(Object *obj)
+{
+}
+
+static void rdt_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->hotpluggable = false;
+    dc->desc = "RDT";
+    dc->user_creatable = true;
+
+    device_class_set_props(dc, rdt_properties);
+}
diff --git a/include/hw/i386/rdt.h b/include/hw/i386/rdt.h
new file mode 100644
index 0000000000..a21d95b265
--- /dev/null
+++ b/include/hw/i386/rdt.h
@@ -0,0 +1,25 @@
+/*
+ * Intel Resource Director Technology (RDT).
+ *
+ * Copyright 2024 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#ifndef HW_RDT_H
+#define HW_RDT_H
+
+typedef struct RDTState RDTState;
+typedef struct RDTStatePerCore RDTStatePerCore;
+typedef struct RDTMonitor RDTMonitor;
+typedef struct RDTAllocation RDTAllocation;
+
+#endif
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 4c239a6970..0f73c1244d 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2213,6 +2213,9 @@ struct ArchCPU {
     struct MemoryRegion *cpu_as_root, *cpu_as_mem, *smram;
     Notifier machine_done;
 
+    /* Help the RDT MSRs find the RDT device */
+    struct RDTStatePerCore *rdt;
+
     struct kvm_msrs *kvm_msr_buf;
 
     int32_t node_id; /* NUMA node this CPU belongs to */
-- 
2.47.1.613.gc27f4b7a9f-goog
Re: [PATCH v5 1/8] i386: Add Intel RDT device and State to config.
Posted by Jonathan Cameron via 2 months, 3 weeks ago
On Fri, 13 Dec 2024 17:26:38 +0000
Hendrik Wuethrich <whendrik@google.com> wrote:

> From: ‪Hendrik Wüthrich <whendrik@google.com>
> 
> Change config to show RDT, add minimal code to the rdt.c module to make
> sure things still compile.
> 
> Signed-off-by: Hendrik Wüthrich <whendrik@google.com>
Hi,

A few drive by comments.

> ---
>  hw/i386/Kconfig       |  4 ++
>  hw/i386/meson.build   |  1 +
>  hw/i386/rdt.c         | 99 +++++++++++++++++++++++++++++++++++++++++++
>  include/hw/i386/rdt.h | 25 +++++++++++
>  target/i386/cpu.h     |  3 ++
>  5 files changed, 132 insertions(+)
>  create mode 100644 hw/i386/rdt.c
>  create mode 100644 include/hw/i386/rdt.h

> diff --git a/hw/i386/rdt.c b/hw/i386/rdt.c
> new file mode 100644
> index 0000000000..b2203197e3
> --- /dev/null
> +++ b/hw/i386/rdt.c
> @@ -0,0 +1,99 @@
> +/*
> + * Intel Resource Director Technology (RDT).
> + *
> + * Copyright 2024 Google LLC
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * for more details.
> + */
> +
> +#include "hw/i386/rdt.h"
> +#include "qemu/osdep.h" /* Needs to be included before isa.h */
> +#include "hw/isa/isa.h"
> +#include "hw/qdev-properties.h"
> +#include "qom/object.h"
> +
> +/* Max counts for allocation masks or CBMs. In other words, the size of
> + * respective MSRs.
> + * L3_MASK and L3_mask are architectural limitations. THRTL_COUNT is just
> + * the space left until the next MSR.
> + * */


Should match multiline comment style in qemu style guide.

> +
> +/*One instance of RDT-internal state to be shared by all cores*/
> +struct RDTState {
> +    ISADevice parent;
> +
> +    /*Max amount of RMIDs*/

Spaces typically after * and before * I think are most common
syntax for comments in qEMU

> +    uint32_t rmids;
> +
> +    /*Per core state*/
> +    RDTStatePerCore *rdtInstances;
> +    RDTAllocation *allocations;
> +
> +    /*RDT Allocation bitmask MSRs*/
> +    uint32_t msr_L3_ia32_mask_n[RDT_MAX_L3_MASK_COUNT];
> +    uint32_t msr_L2_ia32_mask_n[RDT_MAX_L2_MASK_COUNT];
> +    uint32_t ia32_L2_qos_ext_bw_thrtl_n[RDT_MAX_MBA_THRTL_COUNT];
> +};
> +
> +struct RDTStateClass {
> +};
> +
> +OBJECT_DEFINE_TYPE(RDTState, rdt, RDT, ISA_DEVICE);
> +
> +static Property rdt_properties[] = {
> +    DEFINE_PROP_UINT32(RDT_NUM_RMID_PROP, RDTState, rmids, 256),
> +    DEFINE_PROP_END_OF_LIST(),

You'll see this in a rebase but this terminator is no longer needed
(or defined I think)
> +};
> +
> +static void rdt_init(Object *obj)
> +{
> +}
> +
> +static void rdt_finalize(Object *obj)
> +{
> +}

Why introduce this pair as empty and as far as I can see not called?
init is called in patch 2 so bring it in there.  I'm struggling to
spot finalize being called.


> +
> +static void rdt_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->hotpluggable = false;
> +    dc->desc = "RDT";
> +    dc->user_creatable = true;
> +
> +    device_class_set_props(dc, rdt_properties);
> +}

>      int32_t node_id; /* NUMA node this CPU belongs to */
Re: [PATCH v5 1/8] i386: Add Intel RDT device and State to config.
Posted by Hendrik Wüthrich 2 months, 2 weeks ago
On Thu, Feb 20, 2025 at 4:41 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Fri, 13 Dec 2024 17:26:38 +0000
> Hendrik Wuethrich <whendrik@google.com> wrote:
>
> > From: ‪Hendrik Wüthrich <whendrik@google.com>
> >
> > Change config to show RDT, add minimal code to the rdt.c module to make
> > sure things still compile.
> >
> > Signed-off-by: Hendrik Wüthrich <whendrik@google.com>
> Hi,
>
> A few drive by comments.
>
> > ---
> >  hw/i386/Kconfig       |  4 ++
> >  hw/i386/meson.build   |  1 +
> >  hw/i386/rdt.c         | 99 +++++++++++++++++++++++++++++++++++++++++++
> >  include/hw/i386/rdt.h | 25 +++++++++++
> >  target/i386/cpu.h     |  3 ++
> >  5 files changed, 132 insertions(+)
> >  create mode 100644 hw/i386/rdt.c
> >  create mode 100644 include/hw/i386/rdt.h
>
> > diff --git a/hw/i386/rdt.c b/hw/i386/rdt.c
> > new file mode 100644
> > index 0000000000..b2203197e3
> > --- /dev/null
> > +++ b/hw/i386/rdt.c
> > @@ -0,0 +1,99 @@
> > +/*
> > + * Intel Resource Director Technology (RDT).
> > + *
> > + * Copyright 2024 Google LLC
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by the
> > + * Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> > + * for more details.
> > + */
> > +
> > +#include "hw/i386/rdt.h"
> > +#include "qemu/osdep.h" /* Needs to be included before isa.h */
> > +#include "hw/isa/isa.h"
> > +#include "hw/qdev-properties.h"
> > +#include "qom/object.h"
> > +
> > +/* Max counts for allocation masks or CBMs. In other words, the size of
> > + * respective MSRs.
> > + * L3_MASK and L3_mask are architectural limitations. THRTL_COUNT is just
> > + * the space left until the next MSR.
> > + * */
>
>
> Should match multiline comment style in qemu style guide.
>
> > +
> > +/*One instance of RDT-internal state to be shared by all cores*/
> > +struct RDTState {
> > +    ISADevice parent;
> > +
> > +    /*Max amount of RMIDs*/
>
> Spaces typically after * and before * I think are most common
> syntax for comments in qEMU
>
> > +    uint32_t rmids;
> > +
> > +    /*Per core state*/
> > +    RDTStatePerCore *rdtInstances;
> > +    RDTAllocation *allocations;
> > +
> > +    /*RDT Allocation bitmask MSRs*/
> > +    uint32_t msr_L3_ia32_mask_n[RDT_MAX_L3_MASK_COUNT];
> > +    uint32_t msr_L2_ia32_mask_n[RDT_MAX_L2_MASK_COUNT];
> > +    uint32_t ia32_L2_qos_ext_bw_thrtl_n[RDT_MAX_MBA_THRTL_COUNT];
> > +};
> > +
> > +struct RDTStateClass {
> > +};
> > +
> > +OBJECT_DEFINE_TYPE(RDTState, rdt, RDT, ISA_DEVICE);
> > +
> > +static Property rdt_properties[] = {
> > +    DEFINE_PROP_UINT32(RDT_NUM_RMID_PROP, RDTState, rmids, 256),
> > +    DEFINE_PROP_END_OF_LIST(),
>
> You'll see this in a rebase but this terminator is no longer needed
> (or defined I think)
> > +};
> > +
> > +static void rdt_init(Object *obj)
> > +{
> > +}
> > +
> > +static void rdt_finalize(Object *obj)
> > +{
> > +}
>
> Why introduce this pair as empty and as far as I can see not called?
> init is called in patch 2 so bring it in there.  I'm struggling to
> spot finalize being called.

Because the init and finalize functions are needed by the macros
representing the RDT device, and the patch does not compile
if they are removed.
>
>
> > +
> > +static void rdt_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->hotpluggable = false;
> > +    dc->desc = "RDT";
> > +    dc->user_creatable = true;
> > +
> > +    device_class_set_props(dc, rdt_properties);
> > +}
>
> >      int32_t node_id; /* NUMA node this CPU belongs to */
>