From: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
To: Andrii Anisov <andrii.anisov@gmail.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
Julien Grall <julien.grall@arm.com>,
Stefano Stabellini <sstabellini@kernel.org>,
Andrii Anisov <Andrii_Anisov@epam.com>,
Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [Xen-devel] [RFC 4/9] arm64: utilize time accounting
Date: Wed, 11 Sep 2019 17:48:33 +0000 [thread overview]
Message-ID: <87pnk6g1vz.fsf@epam.com> (raw)
In-Reply-To: <1568197942-15374-5-git-send-email-andrii.anisov@gmail.com>
Hi Andrii,
Andrii Anisov writes:
> From: Andrii Anisov <andrii_anisov@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_anisov@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
next prev parent reply other threads:[~2019-09-11 17:49 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-11 10:32 [Xen-devel] [RFC 0/9] Changes to time accounting Andrii Anisov
2019-09-11 10:32 ` [Xen-devel] [RFC 1/9] schedule: Introduce per-pcpu " Andrii Anisov
2019-09-11 18:01 ` Volodymyr Babchuk
2019-09-12 10:26 ` Andrii Anisov
2019-10-28 14:28 ` Julien Grall
2019-11-06 11:24 ` Andrii Anisov
2020-05-26 2:27 ` Volodymyr Babchuk
2020-05-29 8:48 ` Dario Faggioli
2020-06-02 1:12 ` Volodymyr Babchuk
2020-06-03 15:22 ` Dario Faggioli
2019-09-11 10:32 ` [Xen-devel] [RFC 2/9] sysctl: extend XEN_SYSCTL_getcpuinfo interface Andrii Anisov
2019-10-28 14:52 ` Julien Grall
2019-11-06 11:25 ` Andrii Anisov
2019-09-11 10:32 ` [Xen-devel] [RFC 3/9] xentop: show CPU load information Andrii Anisov
2019-09-11 10:32 ` [Xen-devel] [RFC 4/9] arm64: utilize time accounting Andrii Anisov
2019-09-11 17:48 ` Volodymyr Babchuk [this message]
2019-09-12 12:09 ` Andrii Anisov
2019-09-12 12:17 ` Julien Grall
2019-09-12 12:29 ` Andrii Anisov
2019-10-28 14:47 ` Julien Grall
2019-11-06 11:31 ` Andrii Anisov
2019-09-11 10:32 ` [Xen-devel] [RFC 5/9] tacc: Introduce a lockless interface for guest time Andrii Anisov
2019-09-11 10:32 ` [Xen-devel] [RFC 6/9] sched:rtds: get guest time from time accounting code Andrii Anisov
2019-09-11 10:32 ` [Xen-devel] [RFC 7/9] tacc: Introduce a locked interface for guest time Andrii Anisov
2019-09-11 10:32 ` [Xen-devel] [RFC 8/9] sched:credit: get guest time from time accounting code Andrii Anisov
2019-09-11 10:32 ` [Xen-devel] [RFC 9/9] sched:credit2: " Andrii Anisov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87pnk6g1vz.fsf@epam.com \
--to=volodymyr_babchuk@epam.com \
--cc=Andrii_Anisov@epam.com \
--cc=andrii.anisov@gmail.com \
--cc=julien.grall@arm.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).