On 31.07.19 14:02, Julien Grall wrote:
Everything is in the hot path here, yet there are a lot of other branches. So why this branch in particular?
This branch and function call is particular because I find this piece of code quite confusing:
I'm not seeing any benefits in calling `enter_hypervisor_head()` from functions named `do_trap_hyp_*`. That code is confusing for me. IMHO, dividing `do_trap_irq/fiq()` into guest and hyp specific functions is not a big deal. Even for ARM32. Moreover, it will make more obvious the fact that nothing from `enter_hypervisor_head()` is done for IRQ traps taken from hyp.
And I believe this patch balances patch "xen/arm: Re-enable interrupt later in the trap path" what you NAKed because of increased IRQ latency. While them together make the code more straight forward and clear, because: - you start all C-coded common trap handlers with IRQs locked, and free from asm code misunderstanding - all common trap handlers are distinct in their naming, purpose and action. In terms of calling `enter_hypervisor_head()` only from the traps taken from guest.
If you are really worry of the impact of branch then there are a few more important places (with a greater benefits) to look: 1) It seems the compiler use a jump table for the switch case in do_trap_guest_sync(), so it will result to multiple direct branch everytime. 2) Indirect branch have a non-negligible cost compare to direct branch. This is a lot used in the interrupt code (see gic_hw_ops->read_irq()). All of them are known at boot time, so they could be replace with direct branch. x86 recently introduced alternative_call() for this purpose. This could be re-used on Arm.
I remember this. But I'm not ready to code it. I admit that I have not yet good understanding about how alternatives work. -- Sincerely, Andrii Anisov. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
