All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shaoqin Huang <shahuang@redhat.com>
To: Oliver Upton <oliver.upton@linux.dev>,
	Marc Zyngier <maz@kernel.org>,
	kvmarm@lists.linux.dev
Cc: Eric Auger <eauger@redhat.com>,
	Shaoqin Huang <shahuang@redhat.com>,
	Eric Auger <eric.auger@redhat.com>,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Shuah Khan <shuah@kernel.org>,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v4 3/5] KVM: selftests: aarch64: Fix the buggy [enable|disable]_counter
Date: Thu,  1 Feb 2024 21:56:52 -0500	[thread overview]
Message-ID: <20240202025659.5065-4-shahuang@redhat.com> (raw)
In-Reply-To: <20240202025659.5065-1-shahuang@redhat.com>

In general, the set/clr registers should always be used in their
write form, never in a RMW form (imagine an interrupt disabling
a counter between the read and the write...).

The current implementation of [enable|disable]_counter both use the RMW
form, fix them by directly write to the set/clr registers.

At the same time, it also fix the buggy disable_counter() which would
end up disabling all the counters.

Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
 tools/testing/selftests/kvm/include/aarch64/vpmu.h | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/aarch64/vpmu.h b/tools/testing/selftests/kvm/include/aarch64/vpmu.h
index f78c93a08bff..92715021f892 100644
--- a/tools/testing/selftests/kvm/include/aarch64/vpmu.h
+++ b/tools/testing/selftests/kvm/include/aarch64/vpmu.h
@@ -77,17 +77,13 @@ static inline void write_sel_evtyper(int sel, unsigned long val)
 
 static inline void enable_counter(int idx)
 {
-	uint64_t v = read_sysreg(pmcntenset_el0);
-
-	write_sysreg(BIT(idx) | v, pmcntenset_el0);
+	write_sysreg(BIT(idx), pmcntenset_el0);
 	isb();
 }
 
 static inline void disable_counter(int idx)
 {
-	uint64_t v = read_sysreg(pmcntenset_el0);
-
-	write_sysreg(BIT(idx) | v, pmcntenclr_el0);
+	write_sysreg(BIT(idx), pmcntenclr_el0);
 	isb();
 }
 
-- 
2.40.1


WARNING: multiple messages have this Message-ID (diff)
From: Shaoqin Huang <shahuang@redhat.com>
To: Oliver Upton <oliver.upton@linux.dev>,
	Marc Zyngier <maz@kernel.org>,
	kvmarm@lists.linux.dev
Cc: Eric Auger <eauger@redhat.com>,
	Shaoqin Huang <shahuang@redhat.com>,
	Eric Auger <eric.auger@redhat.com>,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Shuah Khan <shuah@kernel.org>,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v4 3/5] KVM: selftests: aarch64: Fix the buggy [enable|disable]_counter
Date: Thu,  1 Feb 2024 21:56:52 -0500	[thread overview]
Message-ID: <20240202025659.5065-4-shahuang@redhat.com> (raw)
In-Reply-To: <20240202025659.5065-1-shahuang@redhat.com>

In general, the set/clr registers should always be used in their
write form, never in a RMW form (imagine an interrupt disabling
a counter between the read and the write...).

The current implementation of [enable|disable]_counter both use the RMW
form, fix them by directly write to the set/clr registers.

At the same time, it also fix the buggy disable_counter() which would
end up disabling all the counters.

Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
 tools/testing/selftests/kvm/include/aarch64/vpmu.h | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/aarch64/vpmu.h b/tools/testing/selftests/kvm/include/aarch64/vpmu.h
index f78c93a08bff..92715021f892 100644
--- a/tools/testing/selftests/kvm/include/aarch64/vpmu.h
+++ b/tools/testing/selftests/kvm/include/aarch64/vpmu.h
@@ -77,17 +77,13 @@ static inline void write_sel_evtyper(int sel, unsigned long val)
 
 static inline void enable_counter(int idx)
 {
-	uint64_t v = read_sysreg(pmcntenset_el0);
-
-	write_sysreg(BIT(idx) | v, pmcntenset_el0);
+	write_sysreg(BIT(idx), pmcntenset_el0);
 	isb();
 }
 
 static inline void disable_counter(int idx)
 {
-	uint64_t v = read_sysreg(pmcntenset_el0);
-
-	write_sysreg(BIT(idx) | v, pmcntenclr_el0);
+	write_sysreg(BIT(idx), pmcntenclr_el0);
 	isb();
 }
 
-- 
2.40.1


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

  parent reply	other threads:[~2024-02-02  2:57 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-02  2:56 [PATCH v4 0/5] KVM: selftests: aarch64: Introduce pmu_event_filter_test Shaoqin Huang
2024-02-02  2:56 ` Shaoqin Huang
2024-02-02  2:56 ` [PATCH v4 1/5] KVM: selftests: aarch64: Make the [create|destroy]_vpmu_vm() public Shaoqin Huang
2024-02-02  2:56   ` Shaoqin Huang
2024-02-02  7:36   ` Oliver Upton
2024-02-02  7:36     ` Oliver Upton
2024-02-27  3:10     ` Shaoqin Huang
2024-02-27  3:10       ` Shaoqin Huang
2024-02-02  2:56 ` [PATCH v4 2/5] KVM: selftests: aarch64: Move pmu helper functions into vpmu.h Shaoqin Huang
2024-02-02  2:56   ` Shaoqin Huang
2024-02-02  7:44   ` Oliver Upton
2024-02-02  7:44     ` Oliver Upton
2024-02-02  2:56 ` Shaoqin Huang [this message]
2024-02-02  2:56   ` [PATCH v4 3/5] KVM: selftests: aarch64: Fix the buggy [enable|disable]_counter Shaoqin Huang
2024-02-02  2:56 ` [PATCH v4 4/5] KVM: selftests: aarch64: Introduce pmu_event_filter_test Shaoqin Huang
2024-02-02  2:56   ` Shaoqin Huang
2024-02-02  8:34   ` Oliver Upton
2024-02-02  8:34     ` Oliver Upton
2024-02-15 14:42     ` Eric Auger
2024-02-15 14:42       ` Eric Auger
2024-02-27  4:24     ` Shaoqin Huang
2024-02-27  4:24       ` Shaoqin Huang
2024-02-02  2:56 ` [PATCH v4 5/5] KVM: selftests: aarch64: Add invalid filter test in pmu_event_filter_test Shaoqin Huang
2024-02-02  2:56   ` Shaoqin Huang
2024-02-15 18:27   ` Eric Auger
2024-02-15 18:27     ` Eric Auger

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=20240202025659.5065-4-shahuang@redhat.com \
    --to=shahuang@redhat.com \
    --cc=eauger@redhat.com \
    --cc=eric.auger@redhat.com \
    --cc=james.morse@arm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=oliver.upton@linux.dev \
    --cc=pbonzini@redhat.com \
    --cc=shuah@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=yuzenghui@huawei.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.