Re: [RFC v12 31/65] target/arm: move exception code out of tcg/helper.c

[Claudio Fontana <cfontana@suse.de>]

On 3/28/21 6:40 PM, Richard Henderson wrote:
> On 3/26/21 1:36 PM, Claudio Fontana wrote:
>> +#ifdef CONFIG_TCG
>> +    arm_rebuild_hflags(env);
>> +#endif /* CONFIG_TCG */
> 
> No functional changes during code movement.
> Really.
> I cannot emphasize this enough.
we are asserting "not reached" for arm_rebuild_hflags in KVM-only, but the KVM-only build is not "active" yet at this point,
so will add separately ok.


> 
> Also, why is this an ifdef and not tcg_enabled()?

right, will change.

> 
>> +    aarch64_restore_sp(env, new_el);
>> +#ifdef CONFIG_TCG
>> +    arm_rebuild_hflags(env);
>> +#endif /* CONFIG_TCG */
> 
> Likewise.

Yes, will change.

> 
>> +#ifdef CONFIG_TCG
>> +    if (tcg_enabled()) {
> 
> Likewise.  And, why in the world do you need both?

Here both are needed because the prototypes of other functions in the block are not visile for non-tcg builds.

It is a struggle to balance making TCG-only symbols "invisible" to kvm-builds, so one developing for KVM only can forget about them,
and keeping TCG things contained,

and on the other side opening up the ability to use tcg_enabled(), which requires those prototypes to be visible.

Where do you see that balance?

Should I make arm_is_psci_call, arm_handle_psci_call, tcg_handle_semihosting visible as well?

Or can we give a meaningful name to this operation:

+    if (tcg_enabled()) {
+        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;
+        }
+        /*
+         * Semihosting semantics depend on the register width of the code
+         * that caused the exception, not the target exception level, so
+         * must be handled here.
+         */
+        if (cs->exception_index == EXCP_SEMIHOST) {
+            tcg_handle_semihosting(cs);
+            return;
+        }
+    }

and store it in a single tcg-specific function, whole prototype we could make visible?

Thanks,

Claudio



> 
> 
> r~
>