[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC 5/6] arm64: call enter_hypervisor_head only when it is needed

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 

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.

Andrii Anisov.

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.