Hi Andrii,
Andrii Anisov writes:
> From: Andrii Anisov <andrii_ani...@epam.com>
>
> Call time accounting hooks from appropriate transition points
> of the ARM64 code.
I'd prefer more elaborate commit message. For example - what are
appropriate transition points? I mean - how you chose ones?
> Signed-off-by: Andrii Anisov <andrii_ani...@epam.com>
> ---
> xen/arch/arm/arm64/entry.S | 39 ++++++++++++++++++++++++++++++++++++---
> xen/arch/arm/domain.c | 2 ++
> 2 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 2d9a271..6fb2fa9 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -143,12 +143,21 @@
>
> .endm
>
> - .macro exit, hyp, compat
> + .macro exit, hyp, compat, tacc=1
>
> .if \hyp == 0 /* Guest mode */
>
> + .if \tacc == 1
There is a hard tab, instead of 8 spaces.
Also, while it is easy to guess what 'hyp' and 'compat' mean, it is hard
to tell what 'tacc' stands for. I think, you need either better
name for this or a comment, which explains all parameters.
> +
> + mov x0, #1
> + bl tacc_hyp
> +
> + .endif
The same about hard tabs. Probably, there are more of them in this patch.
> +
> bl leave_hypervisor_tail /* Disables interrupts on return */
>
> + mov x0, #1
> + bl tacc_guest
> exit_guest \compat
>
> .endif
> @@ -205,9 +214,15 @@ hyp_sync:
>
> hyp_irq:
> entry hyp=1
> + mov x0,#5
Space is missing before #5
> + bl tacc_irq_enter
> msr daifclr, #4
> mov x0, sp
> bl do_trap_irq
> +
> + mov x0,#5
Space is missing before #5
> + bl tacc_irq_exit
> +
> exit hyp=1
>
> guest_sync:
> @@ -291,6 +306,9 @@ guest_sync_slowpath:
> * to save them.
> */
> entry hyp=0, compat=0, save_x0_x1=0
> +
There are trailing whitespaces. I sure that 'git diff' highlights
such mistakes...
> + mov x0,#1
Space is missing before #1
> + bl tacc_gsync
> /*
> * The vSError will be checked while
> SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> * is not set. If a vSError took place, the initial exception will be
> @@ -307,6 +325,10 @@ guest_sync_slowpath:
>
> guest_irq:
> entry hyp=0, compat=0
> +
> + mov x0,#6
Space is missing before #6
> + bl tacc_irq_enter
> +
> /*
> * The vSError will be checked while
> SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> * is not set. If a vSError took place, the initial exception will be
> @@ -319,6 +341,8 @@ guest_irq:
> mov x0, sp
> bl do_trap_irq
> 1:
> + mov x0,#6
Space is missing before #6, also looks like there is a hard tab character.
> + bl tacc_irq_exit
> exit hyp=0, compat=0
>
> guest_fiq_invalid:
> @@ -334,6 +358,9 @@ guest_error:
>
> guest_sync_compat:
> entry hyp=0, compat=1
> +
> + mov x0,#2
Space is missing before #2.
> + bl tacc_gsync
> /*
> * The vSError will be checked while
> SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> * is not set. If a vSError took place, the initial exception will be
> @@ -350,6 +377,10 @@ guest_sync_compat:
>
> guest_irq_compat:
> entry hyp=0, compat=1
> +
> + mov x0,#7
Space is missing before #7.
> + bl tacc_irq_enter
> +
> /*
> * The vSError will be checked while
> SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> * is not set. If a vSError took place, the initial exception will be
> @@ -362,6 +393,8 @@ guest_irq_compat:
> mov x0, sp
> bl do_trap_irq
> 1:
> + mov x0,#7
Space is missing before #7...
> + bl tacc_irq_exit
> exit hyp=0, compat=1
>
> guest_fiq_invalid_compat:
> @@ -376,9 +409,9 @@ guest_error_compat:
> exit hyp=0, compat=1
>
> ENTRY(return_to_new_vcpu32)
> - exit hyp=0, compat=1
> + exit hyp=0, compat=1, tacc=0
> ENTRY(return_to_new_vcpu64)
> - exit hyp=0, compat=0
> + exit hyp=0, compat=0, tacc=0
>
> return_from_trap:
> msr daifset, #2 /* Mask interrupts */
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index a9c4113..53ef630 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -51,11 +51,13 @@ static void do_idle(void)
> process_pending_softirqs();
>
> local_irq_disable();
> + tacc_idle(1);
1 and 2 (below) look like some magical values to me.
I believe, you need to define some enumeration.
> if ( cpu_is_haltable(cpu) )
> {
> dsb(sy);
> wfi();
> }
> + tacc_hyp(2);
> local_irq_enable();
>
> sched_tick_resume();
--
Volodymyr Babchuk at EPAM
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
