On Thu, 7 Nov 2024 22:00:17 +0800 Zhao Liu <zhao1....@intel.com> wrote:
> Hi Igor, > > > > What's the difference between arch_id and CPU index (CPUState.cpu_index)? > > > > > > > They might be the same but not necessarily. > > arch_id is unique cpu identifier from architecture point of view > > (which easily could be sparse and without specific order). > > (ex: for x86 it's apic_id, for spapr it's core_id) > > Yes, I was previously puzzled as to why the core_id of spapr is global, > which is completely different from the meaning of core_id in x86. Now, > your analogy has made it very clear to me. Thanks! > > > while cpu_index is internal QEMU, that existed before possible_cpus[] > > and non-sparse range and depends on order of cpus are created. > > For machines that support possible_cpus[], we override default > > cpu_index assignment to fit possible_cpus[]. > > > > It might be nice to get rid of cpu_index in favor of possible_cpus[], > > but that would be a lot work probably with no huge benefit > > when it comes majority of machines that don't need variable > > cpu count. > > Thank you! Now I see. > > > > In include/hw/boards.h, the doc of CPUArchId said: > > > > > > vcpus_count - number of threads provided by @cpu object > > > > > > And I undersatnd each element in possible_cpus->cpus[] is mapped to a > > > CPU object, so that here vcpus_count should be 1. > > > > it's arch specific, CPU object in possible_cpus was meant to point to > > thread/core/..higher layers in future../ > > > > For example spapr put's there CPUCore, where vcpus_count can be > 1 > > > > That is a bit broken though, since CPUCore is not CPUState by any means, > > spapr_core_plug() gets away with it only because > > core_slot->cpu = CPU(dev) > > CPU() is dumb pointer cast. > > Is it also because of architectural reasons that the smallest granularity > supported by spapr can only be the core? Yes, I think so if I recall it correctly. > > Ideally CPUArchId::cpu should be (Object*) to accommodate various > > levels of granularity correctly (with dumb cast above it's just > > cosmetics though as long as we treat it as Object in non arch > > specific code). > > Thank you. So, I would like to ask, should the elements in possible_cpus > be understood as the smallest granularity supported by hotplug? not necessarily, eventually we might want to plug sockets someday, machine would expose only have_socket in hotpluggable CPUs interface and hotplugging a CPU would look like (device_add xeon_E5_2630,socket-id=X), which resembles real hw use-case. And that xeon device would create internally all necessary vCPUs, and configure internal parameters topo/caches/whatnot. > I want to understand that this reason is unrelated to the loongarch patch, > instead I mainly want to continue thinking about my previous qom-topo[*] > proposal. > > I remember your hotplug slides also mentioned larger granularity hotplug, > which I understand, for example, allows x86 to support core/socket, etc. > (this of course requires core/socket object abstraction). > > If possible_cpus[] only needs to correspond to the smallest granularity > topo object, then it matches my qom-topo proposal quite well, essentially > mapping one layer of a complete topology tree (which is built from socket > to thread, layer by layer) to possible_cpus[] (actually, this is my design: > mapping the thread layer of the x86 topology tree to possible_cpus[]. :) ) > > Although many years have passed, I still believe that larger granularity > hotplug is valuable, especially as hardware includes more and more CPUs. that wasn't the case in the past for virt world, as main goal of it there is dynamic scalability, while in real hw (in x86 world) it's mainly RAS feature. > [*]: > https://lore.kernel.org/qemu-devel/20240919015533.766754-1-zhao1....@intel.com/ It looks to me as too complicated approach, I'll comment there. > > [snip] > > > > IIUC, the phy_id is kind of like the x86 apic_id, but the field is > > > completely > > > derived from topology, so why do you need to define it as a property and > > > then > > > expose it to the user? > > > > for x86 we do expose apic_id as a property as well, partly from historical > > pov > > but also it's better to access cpu fields via properties from outside of > > CPU object than directly poke inside of CPU structure from outside of CPU > > (especially if it comes to generic code) > > Thank you for your guidance. Similar to Bibo’s question, I also wonder > if there is the need for a property that won't be exposed to users. > > Regards, > Zhao >