diff mbox series

[v1,1/9] Add Intel RDT device to config.

Message ID 20240719162929.1197154-2-whendrik@google.com (mailing list archive)
State New, archived
Headers show
Series target:386/ Emulate Intel RDT features needed to mount ResCtrl in Linux | expand

Commit Message

Hendrik Wuethrich July 19, 2024, 4:29 p.m. UTC 
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         | 49 +++++++++++++++++++++++++++++++++++++++++++
 include/hw/i386/rdt.h | 12 +++++++++++
 4 files changed, 66 insertions(+)
 create mode 100644 hw/i386/rdt.c
 create mode 100644 include/hw/i386/rdt.h

Comments

Jonathan Cameron July 26, 2024, 10:24 a.m. UTC | #1 
On Fri, 19 Jul 2024 16:29:21 +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 Hendrik

Great to see emulation of this. Will be handy for testing
kernel changes etc.

Not convinced it's worth a separate patch just to add stubs.
Why not at least bring some real code in with this?

> ---
>  hw/i386/Kconfig       |  4 ++++
>  hw/i386/meson.build   |  1 +
>  hw/i386/rdt.c         | 49 +++++++++++++++++++++++++++++++++++++++++++
>  include/hw/i386/rdt.h | 12 +++++++++++
>  4 files changed, 66 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 f4a33b6c08..4dd05ed6f2 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 03aad10df7..fdbf5962b5 100644
> --- a/hw/i386/meson.build
> +++ b/hw/i386/meson.build
> @@ -21,6 +21,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..0a5e95606b
> --- /dev/null
> +++ b/hw/i386/rdt.c

License etc missing.

> @@ -0,0 +1,49 @@
> +#include "qemu/osdep.h"
> +#include "hw/i386/rdt.h"
> +#include <stdint.h>
> +#include "hw/qdev-properties.h"
> +#include "qemu/typedefs.h"
> +#include "qom/object.h"
> +#include "target/i386/cpu.h"
> +#include "hw/isa/isa.h"

Ordering seems a bit random.  I don't really mind what order
they are in but it is easier to pick an option so it
becomes obvious where to put things later.

Also better to bring these in when they are needed so it
is obvious why they are here.


> +
> +#define TYPE_RDT "rdt"
> +
> +OBJECT_DECLARE_TYPE(RDTState, RDTStateClass, RDT);
> +
> +struct RDTState {
> +    ISADevice parent;
> +};
> +
> +struct RDTStateClass { };
I'd do
...Class {
};

As will reduce noise in later patches assuming this will have
content.


> +
> +OBJECT_DEFINE_TYPE(RDTState, rdt, RDT, ISA_DEVICE);
> +
> +static Property rdt_properties[] = {
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void rdt_init(Object *obj)

Not used?

> +{
> +}
> +
> +static void rdt_realize(DeviceState *dev, Error **errp)
> +{
> +}
> +
> +static void rdt_finalize(Object *obj)
> +{
> +}

Not used?

> +
> +static void rdt_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->hotpluggable = false;
> +    dc->desc = "RDT";
> +    dc->user_creatable = true;
> +    dc->realize = rdt_realize;
> +
> +    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..45e34d3103
> --- /dev/null
> +++ b/include/hw/i386/rdt.h
> @@ -0,0 +1,12 @@
> +#ifndef HW_RDT_H
> +#define HW_RDT_H
> +
> +#include <stdbool.h>
> +#include <stdint.h>

Not used so don't include them until needed.

> +
> +typedef struct RDTState RDTState;
> +typedef struct RDTStateInstance RDTStateInstance;
> +typedef struct RDTMonitor RDTMonitor;
> +typedef struct RDTAllocation RDTAllocation;
> +
> +#endif
Hendrik Wuethrich Sept. 4, 2024, 2:14 p.m. UTC | #2 
On Fri, Jul 26, 2024 at 12:24 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Fri, 19 Jul 2024 16:29:21 +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 Hendrik
>
> Great to see emulation of this. Will be handy for testing
> kernel changes etc.

Hi Jonathan, thank you very much for the review, and very sorry
for the late reply.

> Not convinced it's worth a separate patch just to add stubs.
> Why not at least bring some real code in with this?
>
> > ---
> >  hw/i386/Kconfig       |  4 ++++
> >  hw/i386/meson.build   |  1 +
> >  hw/i386/rdt.c         | 49 +++++++++++++++++++++++++++++++++++++++++++
> >  include/hw/i386/rdt.h | 12 +++++++++++
> >  4 files changed, 66 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 f4a33b6c08..4dd05ed6f2 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 03aad10df7..fdbf5962b5 100644
> > --- a/hw/i386/meson.build
> > +++ b/hw/i386/meson.build
> > @@ -21,6 +21,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..0a5e95606b
> > --- /dev/null
> > +++ b/hw/i386/rdt.c
>
> License etc missing.
>
> > @@ -0,0 +1,49 @@
> > +#include "qemu/osdep.h"
> > +#include "hw/i386/rdt.h"
> > +#include <stdint.h>
> > +#include "hw/qdev-properties.h"
> > +#include "qemu/typedefs.h"
> > +#include "qom/object.h"
> > +#include "target/i386/cpu.h"
> > +#include "hw/isa/isa.h"
>
> Ordering seems a bit random.  I don't really mind what order
> they are in but it is easier to pick an option so it
> becomes obvious where to put things later.
>
> Also better to bring these in when they are needed so it
> is obvious why they are here.
>
>
> > +
> > +#define TYPE_RDT "rdt"
> > +
> > +OBJECT_DECLARE_TYPE(RDTState, RDTStateClass, RDT);
> > +
> > +struct RDTState {
> > +    ISADevice parent;
> > +};
> > +
> > +struct RDTStateClass { };
> I'd do
> ...Class {
> };
>
> As will reduce noise in later patches assuming this will have
> content.
>
>
> > +
> > +OBJECT_DEFINE_TYPE(RDTState, rdt, RDT, ISA_DEVICE);
> > +
> > +static Property rdt_properties[] = {
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void rdt_init(Object *obj)
>
> Not used?
>
> > +{
> > +}
> > +
> > +static void rdt_realize(DeviceState *dev, Error **errp)
> > +{
> > +}
> > +
> > +static void rdt_finalize(Object *obj)
> > +{
> > +}
>
> Not used?

Realize will be removed in V2 until needed. init and
finalize are necessary to build the device, as far as I
can tell.
>
> > +
> > +static void rdt_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->hotpluggable = false;
> > +    dc->desc = "RDT";
> > +    dc->user_creatable = true;
> > +    dc->realize = rdt_realize;
> > +
> > +    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..45e34d3103
> > --- /dev/null
> > +++ b/include/hw/i386/rdt.h
> > @@ -0,0 +1,12 @@
> > +#ifndef HW_RDT_H
> > +#define HW_RDT_H
> > +
> > +#include <stdbool.h>
> > +#include <stdint.h>
>
> Not used so don't include them until needed.
>
> > +
> > +typedef struct RDTState RDTState;
> > +typedef struct RDTStateInstance RDTStateInstance;
> > +typedef struct RDTMonitor RDTMonitor;
> > +typedef struct RDTAllocation RDTAllocation;
> > +
> > +#endif
>
diff mbox series

Patch

diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index f4a33b6c08..4dd05ed6f2 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 03aad10df7..fdbf5962b5 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -21,6 +21,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..0a5e95606b
--- /dev/null
+++ b/hw/i386/rdt.c
@@ -0,0 +1,49 @@ 
+#include "qemu/osdep.h"
+#include "hw/i386/rdt.h"
+#include <stdint.h>
+#include "hw/qdev-properties.h"
+#include "qemu/typedefs.h"
+#include "qom/object.h"
+#include "target/i386/cpu.h"
+#include "hw/isa/isa.h"
+
+#define TYPE_RDT "rdt"
+
+OBJECT_DECLARE_TYPE(RDTState, RDTStateClass, RDT);
+
+struct RDTState {
+    ISADevice parent;
+};
+
+struct RDTStateClass { };
+
+OBJECT_DEFINE_TYPE(RDTState, rdt, RDT, ISA_DEVICE);
+
+static Property rdt_properties[] = {
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void rdt_init(Object *obj)
+{
+}
+
+static void rdt_realize(DeviceState *dev, Error **errp)
+{
+}
+
+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;
+    dc->realize = rdt_realize;
+
+    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..45e34d3103
--- /dev/null
+++ b/include/hw/i386/rdt.h
@@ -0,0 +1,12 @@ 
+#ifndef HW_RDT_H
+#define HW_RDT_H
+
+#include <stdbool.h>
+#include <stdint.h>
+
+typedef struct RDTState RDTState;
+typedef struct RDTStateInstance RDTStateInstance;
+typedef struct RDTMonitor RDTMonitor;
+typedef struct RDTAllocation RDTAllocation;
+
+#endif