From: Christoffer Dall <christoffer.dall@linaro.org>
To: Marc Zyngier <marc.zyngier@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH v2 02/21] arm64: Allow the arch timer to use the HYP timer
Date: Mon, 1 Feb 2016 16:37:03 +0100 [thread overview]
Message-ID: <20160201153703.GB1478@cbox> (raw)
In-Reply-To: <56AF60CA.6040709@arm.com>
On Mon, Feb 01, 2016 at 01:42:34PM +0000, Marc Zyngier wrote:
> On 01/02/16 12:29, Christoffer Dall wrote:
> > On Mon, Jan 25, 2016 at 03:53:36PM +0000, Marc Zyngier wrote:
> >> With the ARMv8.1 VHE, the kernel can run in HYP mode, and thus
> >> use the HYP timer instead of the normal guest timer in a mostly
> >> transparent way, except for the interrupt line.
> >>
> >> This patch reworks the arch timer code to allow the selection of
> >> the HYP PPI, possibly falling back to the guest timer if not
> >> available.
> >>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >> ---
> >> drivers/clocksource/arm_arch_timer.c | 96 ++++++++++++++++++++++--------------
> >> 1 file changed, 59 insertions(+), 37 deletions(-)
> >>
> >> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> >> index c64d543..ffe9d1c 100644
> >> --- a/drivers/clocksource/arm_arch_timer.c
> >> +++ b/drivers/clocksource/arm_arch_timer.c
> >> @@ -67,7 +67,7 @@ static int arch_timer_ppi[MAX_TIMER_PPI];
> >>
> >> static struct clock_event_device __percpu *arch_timer_evt;
> >>
> >> -static bool arch_timer_use_virtual = true;
> >> +static enum ppi_nr arch_timer_uses_ppi = VIRT_PPI;
> >> static bool arch_timer_c3stop;
> >> static bool arch_timer_mem_use_virtual;
> >>
> >> @@ -263,14 +263,20 @@ static void __arch_timer_setup(unsigned type,
> >> clk->name = "arch_sys_timer";
> >> clk->rating = 450;
> >> clk->cpumask = cpumask_of(smp_processor_id());
> >> - if (arch_timer_use_virtual) {
> >> - clk->irq = arch_timer_ppi[VIRT_PPI];
> >> + clk->irq = arch_timer_ppi[arch_timer_uses_ppi];
> >> + switch (arch_timer_uses_ppi) {
> >> + case VIRT_PPI:
> >> clk->set_state_shutdown = arch_timer_shutdown_virt;
> >> clk->set_next_event = arch_timer_set_next_event_virt;
> >> - } else {
> >> - clk->irq = arch_timer_ppi[PHYS_SECURE_PPI];
> >> + break;
> >> + case PHYS_SECURE_PPI:
> >> + case PHYS_NONSECURE_PPI:
> >> + case HYP_PPI:
> >> clk->set_state_shutdown = arch_timer_shutdown_phys;
> >> clk->set_next_event = arch_timer_set_next_event_phys;
> >> + break;
> >> + default:
> >> + BUG();
> >> }
> >> } else {
> >> clk->features |= CLOCK_EVT_FEAT_DYNIRQ;
> >> @@ -338,17 +344,20 @@ static void arch_counter_set_user_access(void)
> >> arch_timer_set_cntkctl(cntkctl);
> >> }
> >>
> >> +static bool arch_timer_has_nonsecure_ppi(void)
> >> +{
> >> + return (arch_timer_uses_ppi == PHYS_SECURE_PPI &&
> >> + arch_timer_ppi[PHYS_NONSECURE_PPI]);
> >> +}
> >> +
> >> static int arch_timer_setup(struct clock_event_device *clk)
> >> {
> >> __arch_timer_setup(ARCH_CP15_TIMER, clk);
> >>
> >> - if (arch_timer_use_virtual)
> >> - enable_percpu_irq(arch_timer_ppi[VIRT_PPI], 0);
> >> - else {
> >> - enable_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI], 0);
> >> - if (arch_timer_ppi[PHYS_NONSECURE_PPI])
> >> - enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
> >> - }
> >> + enable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], 0);
> >> +
> >> + if (arch_timer_has_nonsecure_ppi())
> >> + enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
> >>
> >> arch_counter_set_user_access();
> >> if (IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM))
> >> @@ -390,7 +399,7 @@ static void arch_timer_banner(unsigned type)
> >> (unsigned long)arch_timer_rate / 1000000,
> >> (unsigned long)(arch_timer_rate / 10000) % 100,
> >> type & ARCH_CP15_TIMER ?
> >> - arch_timer_use_virtual ? "virt" : "phys" :
> >> + (arch_timer_uses_ppi == VIRT_PPI) ? "virt" : "phys" :
> >> "",
> >> type == (ARCH_CP15_TIMER | ARCH_MEM_TIMER) ? "/" : "",
> >> type & ARCH_MEM_TIMER ?
> >> @@ -460,7 +469,7 @@ static void __init arch_counter_register(unsigned type)
> >>
> >> /* Register the CP15 based counter if we have one */
> >> if (type & ARCH_CP15_TIMER) {
> >> - if (IS_ENABLED(CONFIG_ARM64) || arch_timer_use_virtual)
> >> + if (IS_ENABLED(CONFIG_ARM64) || arch_timer_uses_ppi == VIRT_PPI)
> >> arch_timer_read_counter = arch_counter_get_cntvct;
> >> else
> >> arch_timer_read_counter = arch_counter_get_cntpct;
> >> @@ -490,13 +499,9 @@ static void arch_timer_stop(struct clock_event_device *clk)
> >> pr_debug("arch_timer_teardown disable IRQ%d cpu #%d\n",
> >> clk->irq, smp_processor_id());
> >>
> >> - if (arch_timer_use_virtual)
> >> - disable_percpu_irq(arch_timer_ppi[VIRT_PPI]);
> >> - else {
> >> - disable_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI]);
> >> - if (arch_timer_ppi[PHYS_NONSECURE_PPI])
> >> - disable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI]);
> >> - }
> >> + disable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi]);
> >> + if (arch_timer_has_nonsecure_ppi())
> >> + disable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI]);
> >>
> >> clk->set_state_shutdown(clk);
> >> }
> >> @@ -562,12 +567,14 @@ static int __init arch_timer_register(void)
> >> goto out;
> >> }
> >>
> >> - if (arch_timer_use_virtual) {
> >> - ppi = arch_timer_ppi[VIRT_PPI];
> >> + ppi = arch_timer_ppi[arch_timer_uses_ppi];
> >> + switch (arch_timer_uses_ppi) {
> >> + case VIRT_PPI:
> >> err = request_percpu_irq(ppi, arch_timer_handler_virt,
> >> "arch_timer", arch_timer_evt);
> >> - } else {
> >> - ppi = arch_timer_ppi[PHYS_SECURE_PPI];
> >> + break;
> >> + case PHYS_SECURE_PPI:
> >> + case PHYS_NONSECURE_PPI:
> >> err = request_percpu_irq(ppi, arch_timer_handler_phys,
> >> "arch_timer", arch_timer_evt);
> >> if (!err && arch_timer_ppi[PHYS_NONSECURE_PPI]) {
> >> @@ -578,6 +585,13 @@ static int __init arch_timer_register(void)
> >> free_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI],
> >> arch_timer_evt);
> >> }
> >> + break;
> >> + case HYP_PPI:
> >> + err = request_percpu_irq(ppi, arch_timer_handler_phys,
> >> + "arch_timer", arch_timer_evt);
> >> + break;
> >> + default:
> >> + BUG();
> >> }
> >>
> >> if (err) {
> >> @@ -602,15 +616,10 @@ static int __init arch_timer_register(void)
> >> out_unreg_notify:
> >> unregister_cpu_notifier(&arch_timer_cpu_nb);
> >> out_free_irq:
> >> - if (arch_timer_use_virtual)
> >> - free_percpu_irq(arch_timer_ppi[VIRT_PPI], arch_timer_evt);
> >> - else {
> >> - free_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI],
> >> + free_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], arch_timer_evt);
> >> + if (arch_timer_has_nonsecure_ppi())
> >> + free_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI],
> >> arch_timer_evt);
> >> - if (arch_timer_ppi[PHYS_NONSECURE_PPI])
> >> - free_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI],
> >> - arch_timer_evt);
> >> - }
> >>
> >> out_free:
> >> free_percpu(arch_timer_evt);
> >> @@ -697,12 +706,25 @@ static void __init arch_timer_init(void)
> >> *
> >> * If no interrupt provided for virtual timer, we'll have to
> >> * stick to the physical timer. It'd better be accessible...
> >> + *
> >> + * On ARMv8.1 with VH extensions, the kernel runs in HYP. VHE
> >> + * accesses to CNTP_*_EL1 registers are silently redirected to
> >> + * their CNTHP_*_EL2 counterparts, and use a different PPI
> >> + * number.
> >> */
> >> if (is_hyp_mode_available() || !arch_timer_ppi[VIRT_PPI]) {
> >> - arch_timer_use_virtual = false;
> >> + bool has_ppi;
> >> +
> >> + if (is_kernel_in_hyp_mode()) {
> >> + arch_timer_uses_ppi = HYP_PPI;
> >> + has_ppi = !!arch_timer_ppi[HYP_PPI];
> >> + } else {
> >> + arch_timer_uses_ppi = PHYS_SECURE_PPI;
> >> + has_ppi = (!!arch_timer_ppi[PHYS_SECURE_PPI] ||
> >> + !!arch_timer_ppi[PHYS_NONSECURE_PPI]);
> >
> > shouldn't this simply test the PHYS_SECURE_PPI since otherwise you could
> > potentially have the PHYS_NONSECURE_PPI but not PHYS_SECURE_PPI and
> > you'll try to request IRQ 0 for this later... ?
>
> I don't really see how you could have the non-secure PPI, but not the
> secure one, as the binding doesn't give you opportunity to do so (the
> first interrupt is the secure one, then the non-secure one...).
>
I didn't bring the DT-binding-by-heart part of my brain to work today.
You're right, thanks.
-Christoffer
next prev parent reply other threads:[~2016-02-01 15:36 UTC|newest]
Thread overview: 83+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-25 15:53 [PATCH v2 00/21] arm64: Virtualization Host Extension support Marc Zyngier
2016-01-25 15:53 ` [PATCH v2 01/21] arm/arm64: Add new is_kernel_in_hyp_mode predicate Marc Zyngier
2016-02-01 13:59 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 02/21] arm64: Allow the arch timer to use the HYP timer Marc Zyngier
2016-02-01 12:29 ` Christoffer Dall
2016-02-01 13:42 ` Marc Zyngier
2016-02-01 15:37 ` Christoffer Dall [this message]
2016-01-25 15:53 ` [PATCH v2 03/21] arm64: Add ARM64_HAS_VIRT_HOST_EXTN feature Marc Zyngier
2016-02-01 13:59 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 04/21] arm64: KVM: Skip HYP setup when already running in HYP Marc Zyngier
2016-02-01 13:59 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 05/21] arm64: KVM: VHE: Turn VTCR_EL2 setup into a reusable macro Marc Zyngier
2016-02-01 13:13 ` Christoffer Dall
2016-02-01 14:21 ` Marc Zyngier
2016-02-01 15:38 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 06/21] arm64: KVM: VHE: Patch out use of HVC Marc Zyngier
2016-02-01 13:16 ` Christoffer Dall
2016-02-01 13:34 ` Marc Zyngier
2016-02-01 15:36 ` Catalin Marinas
2016-02-01 16:20 ` Marc Zyngier
2016-02-01 17:08 ` Ard Biesheuvel
2016-02-01 17:28 ` Marc Zyngier
2016-02-02 15:42 ` Christoffer Dall
2016-02-01 15:39 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 07/21] arm64: KVM: VHE: Patch out kern_hyp_va Marc Zyngier
2016-02-01 13:20 ` Christoffer Dall
2016-02-01 13:38 ` Marc Zyngier
2016-02-01 15:40 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 08/21] arm64: KVM: VHE: Introduce unified system register accessors Marc Zyngier
2016-02-01 13:47 ` Christoffer Dall
2016-02-01 14:04 ` Marc Zyngier
2016-02-01 15:43 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 09/21] arm64: KVM: VHE: Differenciate host/guest sysreg save/restore Marc Zyngier
2016-02-01 13:59 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 10/21] arm64: KVM: VHE: Split save/restore of sysregs shared between EL1 and EL2 Marc Zyngier
2016-02-01 13:54 ` Christoffer Dall
2016-02-02 9:46 ` Marc Zyngier
2016-02-02 15:46 ` Christoffer Dall
2016-02-02 16:19 ` Marc Zyngier
2016-02-02 20:07 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 11/21] arm64: KVM: VHE: Use unified system register accessors Marc Zyngier
2016-02-01 13:59 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 12/21] arm64: KVM: VHE: Enable minimal sysreg save/restore Marc Zyngier
2016-02-01 14:02 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 13/21] arm64: KVM: VHE: Make __fpsimd_enabled VHE aware Marc Zyngier
2016-02-01 14:17 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 14/21] arm64: KVM: VHE: Implement VHE activate/deactivate_traps Marc Zyngier
2016-02-01 14:20 ` Christoffer Dall
2016-02-02 11:27 ` Marc Zyngier
2016-01-25 15:53 ` [PATCH v2 15/21] arm64: KVM: VHE: Use unified sysreg accessors for timer Marc Zyngier
2016-02-01 14:23 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 16/21] arm64: KVM: VHE: Add fpsimd enabling on guest access Marc Zyngier
2016-02-01 14:24 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 17/21] arm64: KVM: VHE: Add alternative panic handling Marc Zyngier
2016-02-01 14:26 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 18/21] arm64: KVM: Introduce hyp_alternate_value helper Marc Zyngier
2016-02-01 14:41 ` Christoffer Dall
2016-02-02 13:42 ` Marc Zyngier
2016-02-02 15:47 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 19/21] arm64: KVM: Move most of the fault decoding to C Marc Zyngier
2016-02-01 15:21 ` Christoffer Dall
2016-02-02 14:24 ` Marc Zyngier
2016-02-02 15:50 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 20/21] arm64: VHE: Add support for running Linux in EL2 mode Marc Zyngier
2016-01-26 14:04 ` Suzuki K. Poulose
2016-01-26 14:30 ` Suzuki K. Poulose
2016-02-01 15:26 ` Christoffer Dall
2016-01-25 15:53 ` [PATCH v2 21/21] arm64: Panic when VHE and non VHE CPUs coexist Marc Zyngier
2016-01-26 14:25 ` Suzuki K. Poulose
2016-01-26 14:34 ` Marc Zyngier
2016-02-01 15:36 ` Christoffer Dall
2016-02-02 15:32 ` Marc Zyngier
2016-02-03 8:49 ` Christoffer Dall
2016-02-03 17:45 ` Marc Zyngier
2016-02-03 19:12 ` Christoffer Dall
2016-01-25 16:15 ` [PATCH v2 00/21] arm64: Virtualization Host Extension support Arnd Bergmann
2016-01-25 16:23 ` Marc Zyngier
2016-01-25 16:26 ` Arnd Bergmann
2016-01-25 16:26 ` Will Deacon
2016-01-25 16:37 ` Marc Zyngier
2016-01-25 16:44 ` Will Deacon
2016-01-25 19:16 ` Marc Zyngier
2016-02-01 16:25 ` Christoffer Dall
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=20160201153703.GB1478@cbox \
--to=christoffer.dall@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=will.deacon@arm.com \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).