qemu-s390x
[Top][All Lists]
Advanced

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

Re: [PATCH v8 02/12] s390x/cpu_topology: CPU topology objects and struct


From: Pierre Morel
Subject: Re: [PATCH v8 02/12] s390x/cpu_topology: CPU topology objects and structures
Date: Thu, 14 Jul 2022 21:26:34 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0



On 7/14/22 14:50, Janis Schoetterl-Glausch wrote:
On 7/14/22 13:25, Pierre Morel wrote:

[...]


That is sure.
I thought about put a fatal error report during the initialization in the 
s390_topology_setup()

And you can set thread > 1 today, so we'd need to handle that. (increase the 
number of cpus instead and print a warning?)

[...]

this would introduce arch dependencies in the hw/core/
I think that the error report for Z is enough.

So once we support Multithreading in the guest we can adjust it easier without 
involving the common code.

Or we can introduce a thread_supported in SMPCompatProps, which would be good.
I would prefer to propose this outside of the series and suppress the fatal 
error once it is adopted.


Yeah, could be a separate series, but then the question remains what you in 
this one, that is
if you change the code so it would be correct if multithreading were supported.

I would like to first not support multi-thread and do a fatal error if threads are defined or implicitly defined as different of 1.

I prefer to keep multithreading for later, I did not have a look at all the implications for the moment.


+
+/*
+ * Setting the first topology: 1 book, 1 socket
+ * This is enough for 64 cores if the topology is flat (single socket)
+ */
+void s390_topology_setup(MachineState *ms)
+{
+    DeviceState *dev;
+
+    /* Create BOOK bridge device */
+    dev = qdev_new(TYPE_S390_TOPOLOGY_BOOK);
+    object_property_add_child(qdev_get_machine(),
+                              TYPE_S390_TOPOLOGY_BOOK, OBJECT(dev));

Why add it to the machine instead of directly using a static?

For my opinion it is a characteristic of the machine.

So it's visible to the user via info qtree or something?

It is already visible to the user on info qtree.

Would that even be the appropriate location to show that?

That is a very good question and I really appreciate if we discuss on the 
design before diving into details.

The idea is to have the architecture details being on qtree as object so we can 
plug new drawers/books/socket/cores and in the future when the infrastructure 
allows it unplug them.

Would it not be more accurate to say that we plug in new cpus only?
Since you need to specify the topology up front with -smp and it cannot change 
after.

smp specify the maximum we can have.
I thought we can add dynamically elements inside this maximum set.

So that all is static, books/sockets might be completely unpopulated, but they 
still exist in a way.
As far as I understand, STSI only allows for cpus to change, nothing above it.

I thought we want to plug new books or drawers but I may be wrong.

So you want to be able to plug in, for example, a socket without any cpus in it?
I'm not seeing anything in the description of STSI that forbids having empty 
containers
or containers with a cpu entry without any cpus. But I don't know why that 
would be useful.
And if you don't want empty containers, then the container will just show up 
when plugging in the cpu.

You already convinced me, it is a non sense and, anyway, building every container when a cpu is added is how it works with the current implementation.


--
Pierre Morel
IBM Lab Boeblingen



reply via email to

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