* [kvm-unit-tests PATCH 0/1] Don't access PMU when not present
@ 2020-12-17 12:00 ` Alexandru Elisei
0 siblings, 0 replies; 6+ messages in thread
From: Alexandru Elisei @ 2020-12-17 12:00 UTC (permalink / raw)
To: drjones, kvm, kvmarm
Found this when reviewing the PMU undef patches [1], which will get merged
for 5.11 (Paolo accepted the pull request [2]).
This only happens if we try to run the PMU tests when creating a VCPU
*without* the PMU feature. I think qemu creates the VCPU by default with a
PMU and I don't know if or how that could be disabled. On the other hand,
kvmtool doesn't enable the feature by default, and you need the specify the
--pmu knob on the command line to enable guest PMU. I think this should
only affect kvmtool users.
For reference, this is what happens without the patch when I try to run a
PMU test on a host built from the kvmarm-5.11 tag from the pull request
(the --pmu knob is missing from the kvmtool command line):
$ lkvm run -c1 -m64 -f arm/pmu.flat -p cycle-counter
# lkvm run --firmware arm/pmu.flat -m 64 -c 1 --name guest-821
Info: Placing fdt at 0x80200000 - 0x80210000
chr_testdev_init: chr-testdev: can't find a virtio-console
Unhandled exception ec=0 (UNKNOWN)
Vector: 4 (el1h_sync)
ESR_EL1: 02000000, ec=0 (UNKNOWN)
FAR_EL1: 1de7ec7edbadc0de (not valid)
Exception frame registers:
pc : [<000000008000b058>] lr : [<0000000080000084>] pstate: 00000000
sp : 000000008003ff60
x29: 0000000000000000 x28: 0000000000000000
x27: 0000000000000000 x26: 0000000000000000
x25: 0000000000000000 x24: 0000000000000000
x23: 0000000000000000 x22: 0000000000000000
x21: 0000000000000002 x20: 0000000080016a10
x19: 0000000000000000 x18: 0000000000000000
x17: 0000000000000000 x16: 0000000000000000
x15: 0000000080040000 x14: 0000000080040000
x13: 000000008003ff5c x12: 0000000000000058
x11: 0000000000000064 x10: 000000008003ff0c
x9 : 0000000000008400 x8 : 0000000000008008
x7 : 0000000000000001 x6 : 0000000000000004
x5 : 0000000000000000 x4 : 0000000080050003
x3 : 0000000000000001 x2 : 0000000080016d38
x1 : 0000000080016a10 x0 : 0000000000000002
With the patch applied:
$ lkvm run -c1 -m64 -f arm/pmu.flat -p cycle-counter
# lkvm run --firmware arm/pmu.flat -m 64 -c 1 --name guest-1240
Info: Placing fdt at 0x80200000 - 0x80210000
chr_testdev_init: chr-testdev: can't find a virtio-console
No PMU found, test skipped...
SUMMARY: 0 tests
[1] https://www.spinics.net/lists/kvm-arm/msg43347.html
[2] https://www.spinics.net/lists/kvm/msg231080.html
Alexandru Elisei (1):
arm: pmu: Don't read PMCR if PMU is not present
arm/pmu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--
2.29.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [kvm-unit-tests PATCH 0/1] Don't access PMU when not present
@ 2020-12-17 12:00 ` Alexandru Elisei
0 siblings, 0 replies; 6+ messages in thread
From: Alexandru Elisei @ 2020-12-17 12:00 UTC (permalink / raw)
To: drjones, kvm, kvmarm
Found this when reviewing the PMU undef patches [1], which will get merged
for 5.11 (Paolo accepted the pull request [2]).
This only happens if we try to run the PMU tests when creating a VCPU
*without* the PMU feature. I think qemu creates the VCPU by default with a
PMU and I don't know if or how that could be disabled. On the other hand,
kvmtool doesn't enable the feature by default, and you need the specify the
--pmu knob on the command line to enable guest PMU. I think this should
only affect kvmtool users.
For reference, this is what happens without the patch when I try to run a
PMU test on a host built from the kvmarm-5.11 tag from the pull request
(the --pmu knob is missing from the kvmtool command line):
$ lkvm run -c1 -m64 -f arm/pmu.flat -p cycle-counter
# lkvm run --firmware arm/pmu.flat -m 64 -c 1 --name guest-821
Info: Placing fdt at 0x80200000 - 0x80210000
chr_testdev_init: chr-testdev: can't find a virtio-console
Unhandled exception ec=0 (UNKNOWN)
Vector: 4 (el1h_sync)
ESR_EL1: 02000000, ec=0 (UNKNOWN)
FAR_EL1: 1de7ec7edbadc0de (not valid)
Exception frame registers:
pc : [<000000008000b058>] lr : [<0000000080000084>] pstate: 00000000
sp : 000000008003ff60
x29: 0000000000000000 x28: 0000000000000000
x27: 0000000000000000 x26: 0000000000000000
x25: 0000000000000000 x24: 0000000000000000
x23: 0000000000000000 x22: 0000000000000000
x21: 0000000000000002 x20: 0000000080016a10
x19: 0000000000000000 x18: 0000000000000000
x17: 0000000000000000 x16: 0000000000000000
x15: 0000000080040000 x14: 0000000080040000
x13: 000000008003ff5c x12: 0000000000000058
x11: 0000000000000064 x10: 000000008003ff0c
x9 : 0000000000008400 x8 : 0000000000008008
x7 : 0000000000000001 x6 : 0000000000000004
x5 : 0000000000000000 x4 : 0000000080050003
x3 : 0000000000000001 x2 : 0000000080016d38
x1 : 0000000080016a10 x0 : 0000000000000002
With the patch applied:
$ lkvm run -c1 -m64 -f arm/pmu.flat -p cycle-counter
# lkvm run --firmware arm/pmu.flat -m 64 -c 1 --name guest-1240
Info: Placing fdt at 0x80200000 - 0x80210000
chr_testdev_init: chr-testdev: can't find a virtio-console
No PMU found, test skipped...
SUMMARY: 0 tests
[1] https://www.spinics.net/lists/kvm-arm/msg43347.html
[2] https://www.spinics.net/lists/kvm/msg231080.html
Alexandru Elisei (1):
arm: pmu: Don't read PMCR if PMU is not present
arm/pmu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--
2.29.2
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 6+ messages in thread
* [kvm-unit-tests PATCH 1/1] arm: pmu: Don't read PMCR if PMU is not present
2020-12-17 12:00 ` Alexandru Elisei
@ 2020-12-17 12:00 ` Alexandru Elisei
-1 siblings, 0 replies; 6+ messages in thread
From: Alexandru Elisei @ 2020-12-17 12:00 UTC (permalink / raw)
To: drjones, kvm, kvmarm
For arm and arm64, the PMU is an optional part of the architecture.
According to ARM DDI 0487F.b, page D13-3683, accessing PMCR_EL0 when the
PMU is not present generates an undefined exception (one would assume that
this is also true for arm).
The pmu_probe() function reads the register before checking that a PMU is
present, so defer accessing PMCR_EL0 until after we know that it is safe.
This hasn't been a problem so far because there's no hardware in the wild
without a PMU and KVM, contrary to the architecture, has treated the PMU
registers as RAZ/WI if the VCPU doesn't have the PMU feature. However,
that's about to change as KVM will start treating the registers as
undefined when the guest doesn't have a PMU.
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
arm/pmu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arm/pmu.c b/arm/pmu.c
index cc959e6a5c76..15c542a230ea 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -988,7 +988,7 @@ static void pmccntr64_test(void)
/* Return FALSE if no PMU found, otherwise return TRUE */
static bool pmu_probe(void)
{
- uint32_t pmcr = get_pmcr();
+ uint32_t pmcr;
uint8_t implementer;
pmu.version = get_pmu_version();
@@ -997,6 +997,7 @@ static bool pmu_probe(void)
report_info("PMU version: 0x%x", pmu.version);
+ pmcr = get_pmcr();
implementer = (pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK;
report_info("PMU implementer/ID code: %#"PRIx32"(\"%c\")/%#"PRIx32,
(pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK,
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [kvm-unit-tests PATCH 1/1] arm: pmu: Don't read PMCR if PMU is not present
@ 2020-12-17 12:00 ` Alexandru Elisei
0 siblings, 0 replies; 6+ messages in thread
From: Alexandru Elisei @ 2020-12-17 12:00 UTC (permalink / raw)
To: drjones, kvm, kvmarm
For arm and arm64, the PMU is an optional part of the architecture.
According to ARM DDI 0487F.b, page D13-3683, accessing PMCR_EL0 when the
PMU is not present generates an undefined exception (one would assume that
this is also true for arm).
The pmu_probe() function reads the register before checking that a PMU is
present, so defer accessing PMCR_EL0 until after we know that it is safe.
This hasn't been a problem so far because there's no hardware in the wild
without a PMU and KVM, contrary to the architecture, has treated the PMU
registers as RAZ/WI if the VCPU doesn't have the PMU feature. However,
that's about to change as KVM will start treating the registers as
undefined when the guest doesn't have a PMU.
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
arm/pmu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arm/pmu.c b/arm/pmu.c
index cc959e6a5c76..15c542a230ea 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -988,7 +988,7 @@ static void pmccntr64_test(void)
/* Return FALSE if no PMU found, otherwise return TRUE */
static bool pmu_probe(void)
{
- uint32_t pmcr = get_pmcr();
+ uint32_t pmcr;
uint8_t implementer;
pmu.version = get_pmu_version();
@@ -997,6 +997,7 @@ static bool pmu_probe(void)
report_info("PMU version: 0x%x", pmu.version);
+ pmcr = get_pmcr();
implementer = (pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK;
report_info("PMU implementer/ID code: %#"PRIx32"(\"%c\")/%#"PRIx32,
(pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK,
--
2.29.2
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] arm: pmu: Don't read PMCR if PMU is not present
2020-12-17 12:00 ` Alexandru Elisei
@ 2020-12-17 12:17 ` Andrew Jones
-1 siblings, 0 replies; 6+ messages in thread
From: Andrew Jones @ 2020-12-17 12:17 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: kvm, kvmarm
On Thu, Dec 17, 2020 at 12:00:57PM +0000, Alexandru Elisei wrote:
> For arm and arm64, the PMU is an optional part of the architecture.
> According to ARM DDI 0487F.b, page D13-3683, accessing PMCR_EL0 when the
> PMU is not present generates an undefined exception (one would assume that
> this is also true for arm).
>
> The pmu_probe() function reads the register before checking that a PMU is
> present, so defer accessing PMCR_EL0 until after we know that it is safe.
>
> This hasn't been a problem so far because there's no hardware in the wild
> without a PMU and KVM, contrary to the architecture, has treated the PMU
> registers as RAZ/WI if the VCPU doesn't have the PMU feature. However,
> that's about to change as KVM will start treating the registers as
> undefined when the guest doesn't have a PMU.
>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> arm/pmu.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> index cc959e6a5c76..15c542a230ea 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -988,7 +988,7 @@ static void pmccntr64_test(void)
> /* Return FALSE if no PMU found, otherwise return TRUE */
> static bool pmu_probe(void)
> {
> - uint32_t pmcr = get_pmcr();
> + uint32_t pmcr;
> uint8_t implementer;
>
> pmu.version = get_pmu_version();
> @@ -997,6 +997,7 @@ static bool pmu_probe(void)
>
> report_info("PMU version: 0x%x", pmu.version);
>
> + pmcr = get_pmcr();
> implementer = (pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK;
> report_info("PMU implementer/ID code: %#"PRIx32"(\"%c\")/%#"PRIx32,
> (pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK,
> --
> 2.29.2
>
Queued, https://gitlab.com/rhdrjones/kvm-unit-tests/commits/arm/queue
Thanks,
drew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] arm: pmu: Don't read PMCR if PMU is not present
@ 2020-12-17 12:17 ` Andrew Jones
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Jones @ 2020-12-17 12:17 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: kvmarm, kvm
On Thu, Dec 17, 2020 at 12:00:57PM +0000, Alexandru Elisei wrote:
> For arm and arm64, the PMU is an optional part of the architecture.
> According to ARM DDI 0487F.b, page D13-3683, accessing PMCR_EL0 when the
> PMU is not present generates an undefined exception (one would assume that
> this is also true for arm).
>
> The pmu_probe() function reads the register before checking that a PMU is
> present, so defer accessing PMCR_EL0 until after we know that it is safe.
>
> This hasn't been a problem so far because there's no hardware in the wild
> without a PMU and KVM, contrary to the architecture, has treated the PMU
> registers as RAZ/WI if the VCPU doesn't have the PMU feature. However,
> that's about to change as KVM will start treating the registers as
> undefined when the guest doesn't have a PMU.
>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> arm/pmu.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arm/pmu.c b/arm/pmu.c
> index cc959e6a5c76..15c542a230ea 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -988,7 +988,7 @@ static void pmccntr64_test(void)
> /* Return FALSE if no PMU found, otherwise return TRUE */
> static bool pmu_probe(void)
> {
> - uint32_t pmcr = get_pmcr();
> + uint32_t pmcr;
> uint8_t implementer;
>
> pmu.version = get_pmu_version();
> @@ -997,6 +997,7 @@ static bool pmu_probe(void)
>
> report_info("PMU version: 0x%x", pmu.version);
>
> + pmcr = get_pmcr();
> implementer = (pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK;
> report_info("PMU implementer/ID code: %#"PRIx32"(\"%c\")/%#"PRIx32,
> (pmcr >> PMU_PMCR_IMP_SHIFT) & PMU_PMCR_IMP_MASK,
> --
> 2.29.2
>
Queued, https://gitlab.com/rhdrjones/kvm-unit-tests/commits/arm/queue
Thanks,
drew
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-17 12:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-17 12:00 [kvm-unit-tests PATCH 0/1] Don't access PMU when not present Alexandru Elisei
2020-12-17 12:00 ` Alexandru Elisei
2020-12-17 12:00 ` [kvm-unit-tests PATCH 1/1] arm: pmu: Don't read PMCR if PMU is " Alexandru Elisei
2020-12-17 12:00 ` Alexandru Elisei
2020-12-17 12:17 ` Andrew Jones
2020-12-17 12:17 ` Andrew Jones
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.