> -----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>
> ---
> target/hexagon/cpu_helper.c | 100
> +++++++++++++++++++++++++++++++++++-
> 1 file changed, 99 insertions(+), 1 deletion(-)
>
> diff --git a/target/hexagon/cpu_helper.c b/target/hexagon/cpu_helper.c
> index e151c6335a..3e2364a7b0 100644
> --- a/target/hexagon/cpu_helper.c
> +++ b/target/hexagon/cpu_helper.c
> @@ -14,6 +14,8 @@
> #else
> #include "hw/boards.h"
> #include "hw/hexagon/hexagon.h"
> +#include "hex_interrupts.h"
> +#include "hex_mmu.h"
> #endif
> #include "exec/exec-all.h"
> #include "exec/cpu_ldst.h"
> @@ -69,9 +71,105 @@ void
> hexagon_set_sys_pcycle_count(CPUHexagonState *env, uint64_t cycles)
> g_assert_not_reached();
> }
>
> +static MMVector VRegs[VECTOR_UNIT_MAX][NUM_VREGS];
> +static MMQReg QRegs[VECTOR_UNIT_MAX][NUM_QREGS];
This won't scale for a system with multiple Hexagon instances. See discussion
on how to handle shared resources.
> +
> +/*
> + * EXT_CONTEXTS
> + * SSR.XA 2 4 6 8
> + * 000 HVX Context 0 HVX Context 0 HVX Context 0 HVX Context 0
> + * 001 HVX Context 1 HVX Context 1 HVX Context 1 HVX Context 1
> + * 010 HVX Context 0 HVX Context 2 HVX Context 2 HVX Context 2
> + * 011 HVX Context 1 HVX Context 3 HVX Context 3 HVX Context 3
> + * 100 HVX Context 0 HVX Context 0 HVX Context 4 HVX Context 4
> + * 101 HVX Context 1 HVX Context 1 HVX Context 5 HVX Context 5
> + * 110 HVX Context 0 HVX Context 2 HVX Context 2 HVX Context 6
> + * 111 HVX Context 1 HVX Context 3 HVX Context 3 HVX Context 7
> + */
This is different from what the HVX PRM says. It only specifies what XA values
4, 5, 6, 7 mean.
Here is what it says:
The HVX scalar core can contain any number of hardware threads greater or equal
to the number
of vector contexts. The scalar hardware thread is assignable to a vector
context through per-
thread SSR:XA programming, as follows:
- SSR:XA = 4: HVX instructions use vector context 0.
- SSR:XA = 5: HVX instructions use vector context 1, if it is available.
- SSR:XA = 6: HVX instructions use vector context 2, if it is available.
- SSR:XA = 7: HVX instructions use vector context 3, if it is available.
> +static int parse_context_idx(CPUHexagonState *env, uint8_t XA) {
> + int ret;
> + HexagonCPU *cpu = env_archcpu(env);
You should assert that cpu->hvx_contexts is in { 2, 4, 6, 8 }. This will
future proof against changes to the hardware as well as protect against bad
command-line settings.
> + if (cpu->hvx_contexts == 6 && XA >= 6) {
> + ret = XA - 6 + 2;
> + } else {
> + ret = XA % cpu->hvx_contexts;
> + }
> + g_assert(ret >= 0 && ret < VECTOR_UNIT_MAX);
> + return ret;
> +}
> +
> +static void check_overcommitted_hvx(CPUHexagonState *env, uint32_t
> ssr)
> +{
> + if (!GET_FIELD(SSR_XE, ssr)) {
> + return;
> + }
What does SSR_XE indicate?
> +
> + uint8_t XA = GET_SSR_FIELD(SSR_XA, ssr);
> +
> + CPUState *cs;
> + CPU_FOREACH(cs) {
> + CPUHexagonState *env_ = cpu_env(cs);
This underscore is confusing. Use a full name such as thread_env.
> + if (env_ == env) {
> + continue;
> + }
> + /* Check if another thread has the XE bit set and same XA */
> + uint32_t ssr_ = arch_get_system_reg(env_, HEX_SREG_SSR);
Ditto
> + if (GET_SSR_FIELD(SSR_XE2, ssr_) && GET_FIELD(SSR_XA, ssr_) == XA) {
The comment says check the XE bit but the code checks XE2. Also, note the XE
check on the current thread above.
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "setting SSR.XA '%d' on thread %d but thread"
> + " %d has same extension active\n", XA, env->threadId,
> + env_->threadId);
> + }
> + }
> +}
> +
> void hexagon_modify_ssr(CPUHexagonState *env, uint32_t new, uint32_t
> old) {
> - g_assert_not_reached();
> + g_assert(bql_locked());
> +
> + bool old_EX = GET_SSR_FIELD(SSR_EX, old);
> + bool old_UM = GET_SSR_FIELD(SSR_UM, old);
> + bool old_GM = GET_SSR_FIELD(SSR_GM, old);
> + bool old_IE = GET_SSR_FIELD(SSR_IE, old);
> + uint8_t old_XA = GET_SSR_FIELD(SSR_XA, old);
> + bool new_EX = GET_SSR_FIELD(SSR_EX, new);
> + bool new_UM = GET_SSR_FIELD(SSR_UM, new);
> + bool new_GM = GET_SSR_FIELD(SSR_GM, new);
> + bool new_IE = GET_SSR_FIELD(SSR_IE, new);
> + uint8_t new_XA = GET_SSR_FIELD(SSR_XA, new);
> +
> + if ((old_EX != new_EX) ||
> + (old_UM != new_UM) ||
> + (old_GM != new_GM)) {
> + hex_mmu_mode_change(env);
> + }
> +
> + uint8_t old_asid = GET_SSR_FIELD(SSR_ASID, old);
> + uint8_t new_asid = GET_SSR_FIELD(SSR_ASID, new);
> + if (new_asid != old_asid) {
> + CPUState *cs = env_cpu(env);
> + tlb_flush(cs);
> + }
> +
> + 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?
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?
> +
> + check_overcommitted_hvx(env, new);
> + }
> +
> + /* See if the interrupts have been enabled or we have exited EX mode */
> + if ((new_IE && !old_IE) ||
> + (!new_EX && old_EX)) {
> + hex_interrupt_update(env);
> + }
> }
>
> void clear_wait_mode(CPUHexagonState *env)
> --
> 2.34.1
