Re: [PATCH 3/5] target/arm: wrap semihosting and psci calls with tcg_enabled

[Fabiano Rosas <farosas@suse.de>]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 12/16/22 13:29, Fabiano Rosas wrote:
>> -    if (arm_is_psci_call(cpu, cs->exception_index)) {
>> -        arm_handle_psci_call(cpu);
>> -        qemu_log_mask(CPU_LOG_INT, "...handled as PSCI call\n");
>> -        return;
>> -    }
>> +    if (tcg_enabled()) {
>> +        if (arm_is_psci_call(cpu, cs->exception_index)) {
>
> This could be
>
>      if (tcg_enabled() && arm_is_psci_call(...))
>
> because...
>
>> -    /*
>> -     * Semihosting semantics depend on the register width of the code
>> -     * that caused the exception, not the target exception level, so
>> -     * must be handled here.
>> -     */
>> +        /*
>> +         * Semihosting semantics depend on the register width of the code
>> +         * that caused the exception, not the target exception level, so
>> +         * must be handled here.
>> +         */
>>   #ifdef CONFIG_TCG
>> -    if (cs->exception_index == EXCP_SEMIHOST) {
>> -        tcg_handle_semihosting(cs);
>> -        return;
>> -    }
>> +        if (cs->exception_index == EXCP_SEMIHOST) {
>
> If you were able to replace the ifdef, that would be one thing, but since you aren't I 
> don't think this requires a separate check.  There is no way to generate EXCP_SEMIHOST 
> except via TCG.

Right, I had to keep the ifdef that was being removed in the original
patch because tcg_handle_semihosting had moved elsewhere in Claudio's
series.

>
> I guess I don't insist, since you're working toward Claudio's much larger patch set, so
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

If you don't mind I'll leave like this then, unless this comes to a v2.

Thank you