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~
>
