On Tue, 27 Feb 2024 17:36:21 +0000
Jonathan Cameron <jonathan.came...@huawei.com> wrote:
> On Tue, 27 Feb 2024 17:11:15 +0000
> Jonathan Cameron via <qemu-devel@nongnu.org> wrote:
>
> > On Tue, 27 Feb 2024 08:37:15 +0000
> > Ankit Agrawal <ank...@nvidia.com> wrote:
> >
> > > Thanks Jonathan for reviewing the change.
> > >
> > > Comments inline.
> > >
> > > >> The structure needs a PCI device handle [2] that consists of the
> > > >> device BDF.
> > > >> The vfio-pci device corresponding to the acpi-generic-initiator object
> > > >> is
> > > >> located to determine the BDF.
> > > >>
> > > >> [1] ACPI Spec 6.3, Section 5.2.16.6
> > > >> [2] ACPI Spec 6.3, Table 5.80
> > > >>
> > > >> Signed-off-by: Ankit Agrawal <ank...@nvidia.com>
> > > >Hi Ankit,
> > > >
> > > > As the code stands the use of a list seems overkill.
> > >
> > > Yeah, I will try out your suggestion.
> > >
> > > > Otherwise looks good to me. I need Generic Ports support for CXL
> > > > stuff so will copy your approach for that as it's ended up nice
> > > > and simple.
> > > >
> > > > Jonathan
> > >
> > > Nice, would be good to have uniform implementations.
> >
> > I've been messing around with this today.
> >
> > They differ only very trivially.
> > 2 Options.
> > 1) Have acpi-generic-port inherit from acpi-generic-initiator.
> > Works but implies a relationship that isn't really true.
> > 2) Add an abstract base class. I've called it acpi-generic-node
> > and have bother acpi-generic-initiator and acpi-generic-port
> > inherit from it.
> >
> > The second feels more natural but is a tiny bit more code (a few
> > more empty init / finalize functions.
> >
> > If we are going to end up with an abstract base 'object' it
> > will be cleaner to do this all as one series if you don't mind
> > carrying the generic port stuff as well? Or I can smash the
> > two series together and send out an updated version that hopefully
> > meets both our requirements (+ tests etc).
> >
> > I'm just running tests against the CXL qos / generic port code
> > but assuming all goes well can share my additional changes
> > in next day or two.
> >
> > Jonathan
>
> One more thing. Right now we can't use Generic Initiators as
> HMAT initiators. That also wants fixing given that's their
> normal usecase rather than what you are using them for so it
> should 'work'.
Something along the lines of this will do the job.
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index 4173ef2afa..825cfe86bc 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -41,6 +41,7 @@ struct NodeInfo {
struct HostMemoryBackend *node_memdev;
bool present;
bool has_cpu;
+ bool has_gi;
uint8_t lb_info_provided;
uint16_t initiator;
uint8_t distance[MAX_NODES];
diff --git a/hw/acpi/acpi-generic-initiator.c b/hw/acpi/acpi-generic-initiator.c
index 9179590a42..8a67300320 100644
--- a/hw/acpi/acpi-generic-initiator.c
+++ b/hw/acpi/acpi-generic-initiator.c
@@ -6,6 +6,7 @@
#include "qemu/osdep.h"
#include "hw/acpi/acpi-generic-initiator.h"
#include "hw/pci/pci_device.h"
+#include "hw/boards.h"
#include "qapi/error.h"
#include "qapi/qapi-builtin-visit.h"
#include "qapi/visitor.h"
@@ -58,6 +59,7 @@ static void acpi_generic_node_set_node(Object *obj, Visitor
*v,
const char *name, void *opaque,
Error **errp)
{
+ MachineState *ms = MACHINE(qdev_get_machine());
AcpiGenericNode *gn = ACPI_GENERIC_NODE(obj);
uint32_t value;
@@ -72,6 +74,10 @@ static void acpi_generic_node_set_node(Object *obj, Visitor
*v,
}
gn->node = value;
+
+ if (object_dynamic_cast(obj, TYPE_ACPI_GENERIC_INITIATOR)) {
+ ms->numa_state->nodes[gn->node].has_gi = true;
+ }
}
static void acpi_generic_node_class_init(ObjectClass *oc, void *data)
diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
index b933ae3c06..9b1662b6b8 100644
--- a/hw/acpi/hmat.c
+++ b/hw/acpi/hmat.c
@@ -225,7 +225,7 @@ static void hmat_build_table_structs(GArray *table_data,
NumaState *numa_state)
}
for (i = 0; i < numa_state->num_nodes; i++) {
- if (numa_state->nodes[i].has_cpu) {
+ if (numa_state->nodes[i].has_cpu || numa_state->nodes[i].has_gi) {
initiator_list[num_initiator++] = i;
}
}
diff --git a/hw/core/numa.c b/hw/core/numa.c
index f08956ddb0..58a32f1564 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -229,7 +229,8 @@ void parse_numa_hmat_lb(NumaState *numa_state,
NumaHmatLBOptions *node,
node->target, numa_state->num_nodes);
return;
}
- if (!numa_info[node->initiator].has_cpu) {
+ if (!numa_info[node->initiator].has_cpu &&
+ !numa_info[node->initiator].has_gi) {
error_setg(errp, "Invalid initiator=%d, it isn't an "
"initiator proximity domain", node->initiator);
return;
>
> Jonathan
>
> >
> >
> >
>
