On 4/28/25 8:31 AM, Jan Beulich wrote:
On 25.04.2025 19:07, Oleksii Kurochko wrote:
On 4/15/25 3:45 PM, Jan Beulich wrote:
On 15.04.2025 15:39, Oleksii Kurochko wrote:
On 4/10/25 5:53 PM, Jan Beulich wrote:
On 08.04.2025 17:57, Oleksii Kurochko wrote:
+{
+ const __be32 *cell;
+ int ac;
+ uint32_t len;
+
+ ac = dt_n_addr_cells(cpun);
+ cell = dt_get_property(cpun, "reg", &len);
+ if ( !cell || !ac || ((sizeof(*cell) * ac * (thread + 1)) > len) )
+ return ~0ULL;
I'm sorry for my lack of DT knowledge, but what's "thread" representing here?
You only pass in 0 below, so it's unclear whether it's what one might expect
(the thread number on a multi-threaded core).
Based on the DT specification alone, the|`reg`| value could refer to either a
CPU or a thread ID:
```
The value of reg is a <prop-encoded-array> that defines a unique CPU/thread id
for
the CPU/threads represented by the CPU node. If a CPU supports more than one
thread
(i.e. multiple streams of execution) the reg prop-erty is an array with 1
element
per thread.
```
My understanding is that the term/thread/ was used in the Linux kernel to cover
both
cases.
When SMT isn't supported, the CPU can be considered to have a single thread.
For example, RISC-V uses the term/hardware thread/ to describe a hart (i.e., a
CPU).
Note the terminology ("CPU") you used here.
Interestingly, the Linux kernel always uses|thread = 0|.
We could potentially drop this ambiguity and introduce an|ASSERT()| to check
that
the|`reg`| property contains only one entry, representing the HART (CPU) ID:
```
Software can determine the number of threads by dividing the size of reg
by the parent
node’s #address-cells. If `|reg`| has more than one entry, it would simply
SMT support
is required.
```
Does that approach make sense, or should we stick with the current
implementation?
If extra enabling is required to make multi-thread CPUs work, then panic()ing
(not so much ASSERT()ing) may make sense, for the time being. Better would be
if we could use all threads in a system right away.
Actually, this function is ready to be used for multi-thread CPUs. A caller can
request hardware id
by passing `thread` argument (`thread` -> the local thread number to get the
hardware ID for).
So by calling:
dt_get_cpu_hwid(cpu0, 0) -> it will return hardware id of thread 0 of cpu0
dt_get_cpu_hwid(cpu0, 1) -> it will return hardware id of thread 1 of cpu0
...
In our case we assume that SMP isn't supported so that is why it is used only
dt_get_cpu_hwid(cpu0, 0).
If one day, SMP will be enabled then it will be needed to change a callers of
dt_get_cpu_hwid().
I assume you meant SMT in both places you wrote SMP?
Yes, it should be SMT.
But my main point here is:
If enumeration gives you "thread <N> of core <M>" (using x86 terminology), you
need to be quite careful with what you call "CPU". Things need to be entirely
unambiguous, taking into account what internally in (common code) Xen we call a
"CPU". You certainly may call "CPU" what is a collection of threads / harts,
but you then need to clarify this in a prominent comment somewhere, and you
need to be entirely consistent throughout the RISC-V sub-tree.
╭────────────────────╮
│ CPU │ ← 1 physical processor (chip)
│ ┌───────┬─────────┐ │
│ │ Core 0│ Core 1 │ │ ← 2 cores (for example)
│ │ ┌──┬──┐ ┌──┬──┐ │ │
│ │Thr0 Thr1 Thr0 Thr1│ ← 2 threads on each core (SMT)
│ └───────┴─────────┘ │
╰────────────────────╯
I want to double check what Xen call a "CPU". I thought that Xen uses word
CPU to describe a core, right?
What you wrote above "thread <N> of core <M> (using x86 terminology)" is also
correlated
with RISC-V terminology:
A component is termed a core if it contains an independent instruction fetch
unit.
A RISC-V-compatible core might support multiple RISC-V-compatible hardware
threads,
or harts, through multithreading
I checked RISC-V's DTS binding and it seems it is a little bit contradictory to
DTS spec,
where it is mentioned that reg property is used to describe how many threads a
cpu has
when SMP is used, but in RISC-V's dts binding they are describing a hardware
execution
context:
This document uses some terminology common to the RISC-V community
that is not widely used, the definitions of which are listed here:
hart: A hardware execution context, which contains all the state
mandated by the RISC-V ISA: a PC and some registers. This
terminology is designed to disambiguate software's view of execution
contexts from any particular microarchitectural implementation
strategy. For example, an Intel laptop containing one socket with
two cores, each of which has two hyperthreads, could be described as
having four harts.
So in RISC-V's DTS binding they are describing only hardware threads what makes
things more
confusing in terms what kind terminology from Xen point of view should be used.
And based on what is written in RISC-V's dts binding:
For example, an Intel laptop containing one socket with
two cores, each of which has two hyperthreads, could be described as
having four harts.
It would be more logical to drop 'thread' argument of
riscv_of_get_cpu_hwid(const struct dt_device_node *cpun).
And then the question is what to do with the name of variable cpun? As it could
be still confusing. Or, at least,
I can add the comment that CPUn in terms of RISC-V means hart (hardware
thread). And then will it be needed to
add such comment for each usage of word "CPU"?
~ Oleksii
