linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value
@ 2022-12-09 16:44 James Clark
  2022-12-09 16:44 ` [PATCH 1/1] " James Clark
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: James Clark @ 2022-12-09 16:44 UTC (permalink / raw)
  To: kvmarm
  Cc: James Clark, Marc Zyngier, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Oliver Upton, Catalin Marinas, Will Deacon,
	linux-arm-kernel, kvmarm, linux-kernel

Hi,

We noticed qemu failing to run because of an assert on our CI. I don't see the issue anymore with
this fix.

Applies to kvmarm/next (753d734f3f34)

Thanks

James Clark (1):
  KVM: arm64: PMU: Fix PMCR_EL0 reset value

 arch/arm64/kvm/sys_regs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value
  2022-12-09 16:44 [PATCH 0/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value James Clark
@ 2022-12-09 16:44 ` James Clark
  2022-12-09 17:58   ` Oliver Upton
  2022-12-12  9:08 ` [PATCH 0/1] " Marc Zyngier
  2023-01-03 17:57 ` Marc Zyngier
  2 siblings, 1 reply; 8+ messages in thread
From: James Clark @ 2022-12-09 16:44 UTC (permalink / raw)
  To: kvmarm
  Cc: James Clark, Marc Zyngier, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Oliver Upton, Catalin Marinas, Will Deacon,
	linux-arm-kernel, kvmarm, linux-kernel

ARMV8_PMU_PMCR_N_MASK is an unshifted value which results in the wrong
reset value for PMCR_EL0, so shift it to fix it.

This fixes the following error when running qemu:

  $ qemu-system-aarch64 -cpu host -machine type=virt,accel=kvm -kernel ...

  target/arm/helper.c:1813: pmevcntr_rawwrite: Assertion `counter < pmu_num_counters(env)' failed.

Fixes: 292e8f149476 ("KVM: arm64: PMU: Simplify PMCR_EL0 reset handling")
Signed-off-by: James Clark <james.clark@arm.com>
---
 arch/arm64/kvm/sys_regs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index d5ee52d6bf73..c6cbfe6b854b 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -646,7 +646,7 @@ static void reset_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 		return;
 
 	/* Only preserve PMCR_EL0.N, and reset the rest to 0 */
-	pmcr = read_sysreg(pmcr_el0) & ARMV8_PMU_PMCR_N_MASK;
+	pmcr = read_sysreg(pmcr_el0) & (ARMV8_PMU_PMCR_N_MASK << ARMV8_PMU_PMCR_N_SHIFT);
 	if (!kvm_supports_32bit_el0())
 		pmcr |= ARMV8_PMU_PMCR_LC;
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value
  2022-12-09 16:44 ` [PATCH 1/1] " James Clark
@ 2022-12-09 17:58   ` Oliver Upton
  2022-12-10 11:16     ` Marc Zyngier
                       ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Oliver Upton @ 2022-12-09 17:58 UTC (permalink / raw)
  To: James Clark
  Cc: kvmarm, Marc Zyngier, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Catalin Marinas, Will Deacon, linux-arm-kernel,
	kvmarm, linux-kernel

On Fri, Dec 09, 2022 at 04:44:46PM +0000, James Clark wrote:
> ARMV8_PMU_PMCR_N_MASK is an unshifted value which results in the wrong
> reset value for PMCR_EL0, so shift it to fix it.

That's just mean. *_MASK tends to be a shifted mask, although it would
appear that asm/perf_event.h does not follow this convention. Fixing
that would be nice (as I'm sure somebody else will get burned by this),
but for the sake of an immediate fix:

> This fixes the following error when running qemu:
> 
>   $ qemu-system-aarch64 -cpu host -machine type=virt,accel=kvm -kernel ...
> 
>   target/arm/helper.c:1813: pmevcntr_rawwrite: Assertion `counter < pmu_num_counters(env)' failed.
> 
> Fixes: 292e8f149476 ("KVM: arm64: PMU: Simplify PMCR_EL0 reset handling")
> Signed-off-by: James Clark <james.clark@arm.com>

Reviewed-by: Oliver Upton <oliver.upton@linux.dev>

--
Thanks,
Oliver

> ---
>  arch/arm64/kvm/sys_regs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index d5ee52d6bf73..c6cbfe6b854b 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -646,7 +646,7 @@ static void reset_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>  		return;
>  
>  	/* Only preserve PMCR_EL0.N, and reset the rest to 0 */
> -	pmcr = read_sysreg(pmcr_el0) & ARMV8_PMU_PMCR_N_MASK;
> +	pmcr = read_sysreg(pmcr_el0) & (ARMV8_PMU_PMCR_N_MASK << ARMV8_PMU_PMCR_N_SHIFT);
>  	if (!kvm_supports_32bit_el0())
>  		pmcr |= ARMV8_PMU_PMCR_LC;
>  
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value
  2022-12-09 17:58   ` Oliver Upton
@ 2022-12-10 11:16     ` Marc Zyngier
  2022-12-12  6:08     ` Anshuman Khandual
  2022-12-13 18:22     ` Ricardo Koller
  2 siblings, 0 replies; 8+ messages in thread
From: Marc Zyngier @ 2022-12-10 11:16 UTC (permalink / raw)
  To: James Clark, Oliver Upton
  Cc: kvmarm, James Morse, Alexandru Elisei, Suzuki K Poulose,
	Catalin Marinas, Will Deacon, linux-arm-kernel, kvmarm,
	linux-kernel

On Fri, 09 Dec 2022 17:58:31 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> On Fri, Dec 09, 2022 at 04:44:46PM +0000, James Clark wrote:
> > ARMV8_PMU_PMCR_N_MASK is an unshifted value which results in the wrong
> > reset value for PMCR_EL0, so shift it to fix it.
> 
> That's just mean. *_MASK tends to be a shifted mask, although it would
> appear that asm/perf_event.h does not follow this convention. Fixing
> that would be nice (as I'm sure somebody else will get burned by this),
> but for the sake of an immediate fix:

Well, that'll teach me the usual lesson: last minute changes without
full non-regression testing are bound to end in disaster.

> 
> > This fixes the following error when running qemu:
> > 
> >   $ qemu-system-aarch64 -cpu host -machine type=virt,accel=kvm -kernel ...
> > 
> >   target/arm/helper.c:1813: pmevcntr_rawwrite: Assertion `counter < pmu_num_counters(env)' failed.
> > 
> > Fixes: 292e8f149476 ("KVM: arm64: PMU: Simplify PMCR_EL0 reset handling")
> > Signed-off-by: James Clark <james.clark@arm.com>
> 
> Reviewed-by: Oliver Upton <oliver.upton@linux.dev>

Thanks both. I'll queue that ASAP as a fix.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value
  2022-12-09 17:58   ` Oliver Upton
  2022-12-10 11:16     ` Marc Zyngier
@ 2022-12-12  6:08     ` Anshuman Khandual
  2022-12-13 18:22     ` Ricardo Koller
  2 siblings, 0 replies; 8+ messages in thread
From: Anshuman Khandual @ 2022-12-12  6:08 UTC (permalink / raw)
  To: Oliver Upton, James Clark
  Cc: kvmarm, Marc Zyngier, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Catalin Marinas, Will Deacon, linux-arm-kernel,
	kvmarm, linux-kernel



On 12/9/22 23:28, Oliver Upton wrote:
> On Fri, Dec 09, 2022 at 04:44:46PM +0000, James Clark wrote:
>> ARMV8_PMU_PMCR_N_MASK is an unshifted value which results in the wrong
>> reset value for PMCR_EL0, so shift it to fix it.
> 
> That's just mean. *_MASK tends to be a shifted mask, although it would
> appear that asm/perf_event.h does not follow this convention. Fixing
> that would be nice (as I'm sure somebody else will get burned by this),
> but for the sake of an immediate fix:

New arch/arm64/tools/sysreg generates shifted i.e in place masks for register
fields. Once all these PMU registers move into arch/arm64/tools/sysreg, this
problem will be solved.

> 
>> This fixes the following error when running qemu:
>>
>>   $ qemu-system-aarch64 -cpu host -machine type=virt,accel=kvm -kernel ...
>>
>>   target/arm/helper.c:1813: pmevcntr_rawwrite: Assertion `counter < pmu_num_counters(env)' failed.
>>
>> Fixes: 292e8f149476 ("KVM: arm64: PMU: Simplify PMCR_EL0 reset handling")
>> Signed-off-by: James Clark <james.clark@arm.com>
> 
> Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
> 
> --
> Thanks,
> Oliver
> 
>> ---
>>  arch/arm64/kvm/sys_regs.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index d5ee52d6bf73..c6cbfe6b854b 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -646,7 +646,7 @@ static void reset_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>>  		return;
>>  
>>  	/* Only preserve PMCR_EL0.N, and reset the rest to 0 */
>> -	pmcr = read_sysreg(pmcr_el0) & ARMV8_PMU_PMCR_N_MASK;
>> +	pmcr = read_sysreg(pmcr_el0) & (ARMV8_PMU_PMCR_N_MASK << ARMV8_PMU_PMCR_N_SHIFT);
>>  	if (!kvm_supports_32bit_el0())
>>  		pmcr |= ARMV8_PMU_PMCR_LC;
>>  
>> -- 
>> 2.25.1
>>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value
  2022-12-09 16:44 [PATCH 0/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value James Clark
  2022-12-09 16:44 ` [PATCH 1/1] " James Clark
@ 2022-12-12  9:08 ` Marc Zyngier
  2023-01-03 17:57 ` Marc Zyngier
  2 siblings, 0 replies; 8+ messages in thread
From: Marc Zyngier @ 2022-12-12  9:08 UTC (permalink / raw)
  To: kvmarm, James Clark
  Cc: Catalin Marinas, linux-kernel, Will Deacon, linux-arm-kernel,
	kvmarm

On Fri, 9 Dec 2022 16:44:45 +0000, James Clark wrote:
> We noticed qemu failing to run because of an assert on our CI. I don't see the issue anymore with
> this fix.
> 
> Applies to kvmarm/next (753d734f3f34)
> 
> Thanks
> 
> [...]

Applied to fixes, thanks!

[1/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value
      commit: aff234839f8b80ac101e6c2f14d0e44b236efa48

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value
  2022-12-09 17:58   ` Oliver Upton
  2022-12-10 11:16     ` Marc Zyngier
  2022-12-12  6:08     ` Anshuman Khandual
@ 2022-12-13 18:22     ` Ricardo Koller
  2 siblings, 0 replies; 8+ messages in thread
From: Ricardo Koller @ 2022-12-13 18:22 UTC (permalink / raw)
  To: Oliver Upton
  Cc: James Clark, Will Deacon, Marc Zyngier, linux-kernel,
	Catalin Marinas, kvmarm, kvmarm, linux-arm-kernel

On Fri, Dec 09, 2022 at 05:58:31PM +0000, Oliver Upton wrote:
> On Fri, Dec 09, 2022 at 04:44:46PM +0000, James Clark wrote:
> > ARMV8_PMU_PMCR_N_MASK is an unshifted value which results in the wrong
> > reset value for PMCR_EL0, so shift it to fix it.
> 
> That's just mean. *_MASK tends to be a shifted mask, although it would
> appear that asm/perf_event.h does not follow this convention. Fixing
> that would be nice (as I'm sure somebody else will get burned by this),
> but for the sake of an immediate fix:
>

Even kvm-unit-tests does this:

arm/pmu.c:
#define PMU_PMCR_N_SHIFT   11
#define PMU_PMCR_N_MASK    0x1f


> > This fixes the following error when running qemu:
> > 
> >   $ qemu-system-aarch64 -cpu host -machine type=virt,accel=kvm -kernel ...
> > 
> >   target/arm/helper.c:1813: pmevcntr_rawwrite: Assertion `counter < pmu_num_counters(env)' failed.
> > 
> > Fixes: 292e8f149476 ("KVM: arm64: PMU: Simplify PMCR_EL0 reset handling")
> > Signed-off-by: James Clark <james.clark@arm.com>
> 
> Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
> 
> --
> Thanks,
> Oliver
> 
> > ---
> >  arch/arm64/kvm/sys_regs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index d5ee52d6bf73..c6cbfe6b854b 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -646,7 +646,7 @@ static void reset_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> >  		return;
> >  
> >  	/* Only preserve PMCR_EL0.N, and reset the rest to 0 */
> > -	pmcr = read_sysreg(pmcr_el0) & ARMV8_PMU_PMCR_N_MASK;
> > +	pmcr = read_sysreg(pmcr_el0) & (ARMV8_PMU_PMCR_N_MASK << ARMV8_PMU_PMCR_N_SHIFT);
> >  	if (!kvm_supports_32bit_el0())
> >  		pmcr |= ARMV8_PMU_PMCR_LC;
> >  
> > -- 
> > 2.25.1
> > 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value
  2022-12-09 16:44 [PATCH 0/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value James Clark
  2022-12-09 16:44 ` [PATCH 1/1] " James Clark
  2022-12-12  9:08 ` [PATCH 0/1] " Marc Zyngier
@ 2023-01-03 17:57 ` Marc Zyngier
  2 siblings, 0 replies; 8+ messages in thread
From: Marc Zyngier @ 2023-01-03 17:57 UTC (permalink / raw)
  To: James Clark, kvmarm
  Cc: kvmarm, linux-arm-kernel, Will Deacon, Catalin Marinas,
	linux-kernel

On Fri, 9 Dec 2022 16:44:45 +0000, James Clark wrote:
> We noticed qemu failing to run because of an assert on our CI. I don't see the issue anymore with
> this fix.
> 
> Applies to kvmarm/next (753d734f3f34)
> 
> Thanks
> 
> [...]

Applied to fixes, thanks!

[1/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value
      commit: edb4929ea9ba48cd91e3867041f49e4c34d729ed

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-01-03 19:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-09 16:44 [PATCH 0/1] KVM: arm64: PMU: Fix PMCR_EL0 reset value James Clark
2022-12-09 16:44 ` [PATCH 1/1] " James Clark
2022-12-09 17:58   ` Oliver Upton
2022-12-10 11:16     ` Marc Zyngier
2022-12-12  6:08     ` Anshuman Khandual
2022-12-13 18:22     ` Ricardo Koller
2022-12-12  9:08 ` [PATCH 0/1] " Marc Zyngier
2023-01-03 17:57 ` Marc Zyngier

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).