On 3/18/2025 2:14 PM, ltaylorsimp...@gmail.com wrote:

-----Original Message-----
From: Sid Manning <sidn...@quicinc.com>
Sent: Tuesday, March 18, 2025 1:34 PM
To: ltaylorsimp...@gmail.com; 'Brian Cain'
<brian.c...@oss.qualcomm.com>; qemu-devel@nongnu.org
Cc: richard.hender...@linaro.org; phi...@linaro.org; Matheus Bernardino
(QUIC) <quic_mathb...@quicinc.com>; a...@rev.ng; a...@rev.ng; Marco
Liebel (QUIC) <quic_mlie...@quicinc.com>; alex.ben...@linaro.org; Mark
Burton (QUIC) <quic_mbur...@quicinc.com>; Brian Cain
<bc...@quicinc.com>
Subject: RE: [PATCH 05/39] target/hexagon: Implement modify SSR



-----Original Message-----
From: ltaylorsimp...@gmail.com <ltaylorsimp...@gmail.com>
Sent: Monday, March 17, 2025 12:37 PM
To: 'Brian Cain' <brian.c...@oss.qualcomm.com>; qemu-
de...@nongnu.org
Cc: richard.hender...@linaro.org; phi...@linaro.org; Matheus
Bernardino
(QUIC) <quic_mathb...@quicinc.com>; a...@rev.ng; a...@rev.ng; Marco
Liebel (QUIC) <quic_mlie...@quicinc.com>; alex.ben...@linaro.org; Mark
Burton (QUIC) <quic_mbur...@quicinc.com>; Sid Manning
<sidn...@quicinc.com>; Brian Cain <bc...@quicinc.com>
Subject: RE: [PATCH 05/39] target/hexagon: Implement modify SSR

-----Original Message-----
From: Brian Cain <brian.c...@oss.qualcomm.com>
Sent: Friday, February 28, 2025 11:28 PM
To: qemu-devel@nongnu.org
Cc: brian.c...@oss.qualcomm.com; richard.hender...@linaro.org;
phi...@linaro.org; quic_mathb...@quicinc.com; a...@rev.ng;
a...@rev.ng;
quic_mlie...@quicinc.com; ltaylorsimp...@gmail.com;
alex.ben...@linaro.org; quic_mbur...@quicinc.com;
sidn...@quicinc.com;
Brian Cain <bc...@quicinc.com>
Subject: [PATCH 05/39] target/hexagon: Implement modify SSR

From: Brian Cain <bc...@quicinc.com>

The per-vCPU System Status Register controls many modal behaviors of
the system architecture.  When the SSR is updated, we trigger the
necessary effects for interrupts, privilege/MMU, and HVX context
mapping.
Signed-off-by: Brian Cain <brian.c...@oss.qualcomm.com>
---
What does SSR_XE indicate?
[Sid Manning]
If SSR:XE isn't set this thread doesn't have the coproc enabled so discard
additional checking.  Any coproc insn issued when ssr:xe is 0 will trigger a,
"Illegal execution of coprocessor instruction." error.


+    if (old_XA != new_XA) {
+        int old_unit = parse_context_idx(env, old_XA);
+        int new_unit = parse_context_idx(env, new_XA);
Check that old_unit != new_unit.  Per the table above, different XA values
can point to the same unit.  For example, if cpu->hvx_contexts is 2, the
XA=0
and XA=2 both point to context 0.

+
+        /* Ownership exchange */
+        memcpy(VRegs[old_unit], env->VRegs, sizeof(env->VRegs));
+        memcpy(QRegs[old_unit], env->QRegs, sizeof(env->QRegs));
+        memcpy(env->VRegs, VRegs[new_unit], sizeof(env->VRegs));
+        memcpy(env->QRegs, QRegs[new_unit], sizeof(env->QRegs));
What does the hardware do?  Does it clear the context, or is that the OS'es
job?
Nothing would keep a single htid from taking hvx unit 0, doing some hvx-ops
, swapping to hvx unit 1 doing some more hvx-ops and so on.   We are doing
this because each thread has a private copy of the hvx register state.  Since
HVX units and threads are independent if one thread changes its ownership
from HVX context 0 to HVX context 1 we have to do this copy.  Instead of
memcpy should create a new object that represents the HVX units available
and change env->VRegs/QRegs to point to the one currently owned.

Refactoring this will be an improvement.


If the hardware leaves the context alone, the above should be
1) Copy env->{VQ}Regs to old_unit
2) Copy new_unit to env->{VQ}Regs

Should you check SSR_EX before doing these copies?

Do you need to do anything when SSR_EX changes?
I think you mean SSR:XE before doing the copies.  I think we have to do the
copy here regardless of ssr:xe since a thread could swap ownership, update
ssr:xa, without explicitly having ssr:xe set.  Maybe the OS disables SSR:XE
while changing hvx unit ownership?
Correct.  I meant SSR:XE.

Some refactoring is in order but need to understand the HW behavior more 
specifically.
- What will the HW do if more than one thread claims ownership of the same HVX 
context?
- What happens if a thread claims ownership without setting SSR:XE and then 
sets SSR:XE later?
- What about this example?
     1) Thread 0 claims context 1 and sets SSR:XE
     2) Thread 0 does some HVX computation
     3) Thread 0 is done with HVX for now so clears SSR:XE
     4) Thread 1 claims context 1 and sets SSR:XE, does some work, then clears 
SSR:XE
     5) Thread 0 wants to do more HVX, so it sets SSR:XE (still pointing to HVX 
context 1)

We should keep the copies for the contexts and local copies inside 
CPUHexagonState.  This makes TCG generation easier as well as having common 
code between system mode and linux-user mode.

Good point that linux-user will need their own exclusive HVX context.  But it doesn't sound right to me to have CPU state store both the system contexts *and* a local context for system emulation.  Our current design under review is better than that IMO.  Once we have some experience modeling the system registers as an independent QEMU Object, I think we'll be better prepared to model the HVX contexts similarly.  Ideally we can remap these via something along the lines of "object_property_set_link()" when the contexts change, without requiring any copies.  And ideally the existing TCG should work as-is, despite the remappable register files.

"What happens when ... " - multiple threads picking the same context is almost certainly explicitly or implicitly undefined architectural behavior, so whatever QEMU does should be appropriate.  However, we'll talk to the architecture team and get a definitive answer.



Also, since check_overcommitted_hvx only prints a log message, add an early 
exit if LOG_GUEST_ERROR is off.

Thanks,
Taylor



Reply via email to