Re: [PATCH v2] Hexagon: add PC alignment check and exception

[Richard Henderson <richard.henderson@linaro.org>]

On 4/29/24 12:40, Matheus Tavares Bernardino wrote:
> @@ -144,6 +148,9 @@ static inline void cpu_get_tb_cpu_state(CPUHexagonState *env, vaddr *pc,
>           hex_flags = FIELD_DP32(hex_flags, TB_FLAGS, IS_TIGHT_LOOP, 1);
>       }
>       *flags = hex_flags;
> +    if (*pc & PCALIGN_MASK) {
> +        hexagon_raise_exception_err(env, HEX_EXCP_PC_NOT_ALIGNED, 0);
> +    }
>   }

With the test here ...

> diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
> index 47a870f42d..26e6809976 100644
> --- a/target/hexagon/translate.c
> +++ b/target/hexagon/translate.c
> @@ -346,8 +346,9 @@ static void mark_implicit_pred_writes(DisasContext *ctx)
>   static bool pkt_raises_exception(Packet *pkt)
>   {
>       if (check_for_attrib(pkt, A_LOAD) ||
> -        check_for_attrib(pkt, A_STORE)) {
> -        return true;
> +        check_for_attrib(pkt, A_STORE) ||
> +        check_for_attrib(pkt, A_COF)) {
> +            return true;
>       }

... you don't need a change here, because we don't raise the exception from within the packet.


> +run-unaligned_pc run-unaligned_pc_endloop run-unaligned_pc_multi_cof:
> +	$(call run-test, $<, $(QEMU) $< 2> $<.stderr,"$< on $(TARGET_NAME)"); \
> +	if [ $$? -ne 1 ] ; then \
> +		return 1; \
> +	fi
> +	$(call quiet-command, \
> +		grep -q "exception 0x1e" $<.stderr, \
> +		"GREP", "exception 0x1e");

What's missing in the patch set is handling this exception in 
linux-user/hexagon/cpu_loop.c.  This would correspond to misaligned_instruction() in the 
kernel.

At which point you could improve the test case to set up a signal handler in C, generate 
the exception, and test for correct values in the signal frame.


r~


PS: I think the kernel could be improved to use

    force_sig_fault(SIGBUS, BUS_ADRALN, regs->r31);
instead of
    force_sig(SIGBUS);

and similarly for misaligned_data_{load,store}.