From: Will Deacon <will.deacon@arm.com>
To: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Hoan Tran <hotran@apm.com>,
Itaru Kitayama <itaru.kitayama@riken.jp>,
kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org,
Tai Tri Nguyen <ttnguyen@apm.com>
Subject: Re: [PATCH v2] arm64: KVM: pmu: Reset PMSELR_EL0.SEL to a sane value before entering the guest
Date: Tue, 6 Dec 2016 16:37:07 +0000 [thread overview]
Message-ID: <20161206163706.GR2498@arm.com> (raw)
In-Reply-To: <20161206162918.GE4816@cbox>
On Tue, Dec 06, 2016 at 05:29:18PM +0100, Christoffer Dall wrote:
> On Tue, Dec 06, 2016 at 02:56:50PM +0000, Marc Zyngier wrote:
> > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > index 83037cd..3b7cfbd 100644
> > --- a/arch/arm64/kvm/hyp/switch.c
> > +++ b/arch/arm64/kvm/hyp/switch.c
> > @@ -85,7 +85,13 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
> > write_sysreg(val, hcr_el2);
> > /* Trap on AArch32 cp15 c15 accesses (EL1 or EL0) */
> > write_sysreg(1 << 15, hstr_el2);
> > - /* Make sure we trap PMU access from EL0 to EL2 */
> > + /*
> > + * Make sure we trap PMU access from EL0 to EL2. Also sanitize
> > + * PMSELR_EL0 to make sure it never contains the cycle
> > + * counter, which could make a PMXEVCNTR_EL0 access UNDEF.
> > + */
> > + if (vcpu->arch.mdcr_el2 & MDCR_EL2_HPMN_MASK)
> > + write_sysreg(0, pmselr_el0);
>
> I'm a bit confused about how we use the HPMN field. This value is
> always set to the full number of counters available on the system and
> never modified by the guest, right? So this is in essence a check that
> says 'do you have any performance counters, then make sure accesses
> don't undef to el1 instead of trapping to el2', but then my question is,
> why not just set pmselr_el0 to zero unconditionally, because in the case
> where (vcpu->arch.mdcr_el2 & MDCR_EL2_HPMN_MASK) == 0, it means we have
> no counters, which we'll have exposed to the guest anyhow, and we should
> undef at el1 in the guest, or did I get this completely wrong (like
> everything else today)?
I think Marc and I came to the same conclusion a few minutes ago. The check
you might want is "Have I instantiated a virtual PMU for this device?",
but that's probably a micro-optimisation.
Will
WARNING: multiple messages have this Message-ID (diff)
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] arm64: KVM: pmu: Reset PMSELR_EL0.SEL to a sane value before entering the guest
Date: Tue, 6 Dec 2016 16:37:07 +0000 [thread overview]
Message-ID: <20161206163706.GR2498@arm.com> (raw)
In-Reply-To: <20161206162918.GE4816@cbox>
On Tue, Dec 06, 2016 at 05:29:18PM +0100, Christoffer Dall wrote:
> On Tue, Dec 06, 2016 at 02:56:50PM +0000, Marc Zyngier wrote:
> > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > index 83037cd..3b7cfbd 100644
> > --- a/arch/arm64/kvm/hyp/switch.c
> > +++ b/arch/arm64/kvm/hyp/switch.c
> > @@ -85,7 +85,13 @@ static void __hyp_text __activate_traps(struct kvm_vcpu *vcpu)
> > write_sysreg(val, hcr_el2);
> > /* Trap on AArch32 cp15 c15 accesses (EL1 or EL0) */
> > write_sysreg(1 << 15, hstr_el2);
> > - /* Make sure we trap PMU access from EL0 to EL2 */
> > + /*
> > + * Make sure we trap PMU access from EL0 to EL2. Also sanitize
> > + * PMSELR_EL0 to make sure it never contains the cycle
> > + * counter, which could make a PMXEVCNTR_EL0 access UNDEF.
> > + */
> > + if (vcpu->arch.mdcr_el2 & MDCR_EL2_HPMN_MASK)
> > + write_sysreg(0, pmselr_el0);
>
> I'm a bit confused about how we use the HPMN field. This value is
> always set to the full number of counters available on the system and
> never modified by the guest, right? So this is in essence a check that
> says 'do you have any performance counters, then make sure accesses
> don't undef to el1 instead of trapping to el2', but then my question is,
> why not just set pmselr_el0 to zero unconditionally, because in the case
> where (vcpu->arch.mdcr_el2 & MDCR_EL2_HPMN_MASK) == 0, it means we have
> no counters, which we'll have exposed to the guest anyhow, and we should
> undef at el1 in the guest, or did I get this completely wrong (like
> everything else today)?
I think Marc and I came to the same conclusion a few minutes ago. The check
you might want is "Have I instantiated a virtual PMU for this device?",
but that's probably a micro-optimisation.
Will
next prev parent reply other threads:[~2016-12-06 16:36 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-06 14:56 [PATCH v2] arm64: KVM: pmu: Reset PMSELR_EL0.SEL to a sane value before entering the guest Marc Zyngier
2016-12-06 14:56 ` Marc Zyngier
2016-12-06 15:27 ` Will Deacon
2016-12-06 15:27 ` Will Deacon
2016-12-06 15:42 ` Marc Zyngier
2016-12-06 15:42 ` Marc Zyngier
2016-12-06 16:29 ` Christoffer Dall
2016-12-06 16:29 ` Christoffer Dall
2016-12-06 16:37 ` Will Deacon [this message]
2016-12-06 16:37 ` Will Deacon
2016-12-06 17:17 ` Marc Zyngier
2016-12-06 17:17 ` Marc Zyngier
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=20161206163706.GR2498@arm.com \
--to=will.deacon@arm.com \
--cc=catalin.marinas@arm.com \
--cc=christoffer.dall@linaro.org \
--cc=hotran@apm.com \
--cc=itaru.kitayama@riken.jp \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=marc.zyngier@arm.com \
--cc=ttnguyen@apm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.