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

From: Andrii Anisov <andrii.anisov@gmail.com>
To: Julien Grall <julien.grall@arm.com>, xen-devel@lists.xenproject.org
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Andrii Anisov <andrii_anisov@epam.com>,
	Andre Przywara <andre.przywara@arm.com>
Subject: Re: [Xen-devel] [RFC 5/6] arm64: call enter_hypervisor_head only when it is needed
Date: Thu, 1 Aug 2019 10:33:01 +0300	[thread overview]
Message-ID: <c2a220ad-553f-272f-606f-2c569e299e10@gmail.com> (raw)
In-Reply-To: <8c7bc6d1-3482-ec5b-b3d9-c6562caf5711@arm.com>

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