| Message ID | 20210303151634.3421880-1-marcin.juszkiewicz@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm: bump amount of PMU counters to pass SBSA ACS | expand |
On Wed, Mar 03, 2021 at 16:16:34 +0100, Marcin Juszkiewicz wrote: > Arm BSA (Base System Architecture) specification says: > > B_PE_09: PEs must implement the FEAT_PMUv3p1 extension, and the base > system must expose a minimum of four programmable PMU counters to the > operating system. > > B_PE_21: The base system must expose a minimum of two programmable PMU > counters to a hypervisor. > > It is then repeated in SBSA (Server Base System Architecture) > specification in level 3 requirements: > > Each PE must implement a minimum of six programmable PMU counters. > > So let make QEMU provide those 6 PMU counters. > > SBSA-ACS says now: > > 12 : Check number of PMU counters : Result: PASS > > Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org> Reviewed-by: Leif Lindholm <leif@nuviainc.com> It would be good if we could get 6.0 closer to SBSA compliance. Would it be worth the effort to make this controllable per cpu model? / Leif > --- > target/arm/helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 0e1a3b9421..02e25b5c22 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -38,7 +38,7 @@ > #endif > > #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */ > -#define PMCR_NUM_COUNTERS 4 /* QEMU IMPDEF choice */ > +#define PMCR_NUM_COUNTERS 6 /* QEMU IMPDEF choice */ > > #ifndef CONFIG_USER_ONLY > > -- > 2.29.2 >
On Wed, 3 Mar 2021 at 17:48, Leif Lindholm <leif@nuviainc.com> wrote: > It would be good if we could get 6.0 closer to SBSA compliance. How far away are we at the moment ? > Would it be worth the effort to make this controllable per cpu model? I don't have a strong opinion on whether we should, but if we do then the right way to implement that would be to have the PMCR reset value as a reset_pmcr_el0 field in struct ARMCPU (like the existing reset_fpsid, reset_sctlr, etc) that gets set per-CPU to whatever the CPU's value for it is; and then instead of using a PMCR_NUM_COUNTERS value, extract the PMCR.N field when needed. The hardest part would be going through all the CPU TRMs to find out the correct reset value. thanks -- PMM
W dniu 03.03.2021 o 19:06, Peter Maydell pisze: > On Wed, 3 Mar 2021 at 17:48, Leif Lindholm <leif@nuviainc.com> wrote: >> It would be good if we could get 6.0 closer to SBSA compliance. > > How far away are we at the moment ? Hard to tell me how many of those things are missing in QEMU and how many in EDK2 we use as firmware. SBSA-ACS failures: GIC ITS is missing (Shashi Mallela works on it): 102 : If PCIe, then GIC implements ITS : Result: --FAIL-- 1 104 : GIC Maintenance Interrupt : Result: --FAIL-- 1 System timers are not present in GTDT so few more tests are not run: 206 : SYS Timer if PE Timer not ON : Result: --FAIL-- 1 PE Timers are not always-on. 207 : CNTCTLBase & CNTBaseN access : Result: -SKIPPED- 1 No System timers are defined 505 : Wake from System Timer Interrupt : Result: -SKIPPED- 1 No system timers implemented There is no SMMU present so SMMU and IO virtualization tests are not run. This one is probably related: 605 : Memory Access to Un-Populated addr: Result: --FAIL-- 1 Memory access check fails at address = 0x104C0000
On Wed, Mar 03, 2021 at 18:06:46 +0000, Peter Maydell wrote: > On Wed, 3 Mar 2021 at 17:48, Leif Lindholm <leif@nuviainc.com> wrote: > > It would be good if we could get 6.0 closer to SBSA compliance. > > How far away are we at the moment ? > > > Would it be worth the effort to make this controllable per cpu model? > > I don't have a strong opinion on whether we should, but if we do then the > right way to implement that would be to have the PMCR reset value > as a reset_pmcr_el0 field in struct ARMCPU (like the existing reset_fpsid, > reset_sctlr, etc) that gets set per-CPU to whatever the CPU's value for > it is; and then instead of using a PMCR_NUM_COUNTERS value, > extract the PMCR.N field when needed. The hardest part would be > going through all the CPU TRMs to find out the correct reset value. That makes sense. I guess we could also phase the transition by using the default value if zero? Regards, Leif
On Thu, 4 Mar 2021 at 13:53, Leif Lindholm <leif@nuviainc.com> wrote: > > On Wed, Mar 03, 2021 at 18:06:46 +0000, Peter Maydell wrote: > > On Wed, 3 Mar 2021 at 17:48, Leif Lindholm <leif@nuviainc.com> wrote: > > > It would be good if we could get 6.0 closer to SBSA compliance. > > > > How far away are we at the moment ? > > > > > Would it be worth the effort to make this controllable per cpu model? > > > > I don't have a strong opinion on whether we should, but if we do then the > > right way to implement that would be to have the PMCR reset value > > as a reset_pmcr_el0 field in struct ARMCPU (like the existing reset_fpsid, > > reset_sctlr, etc) that gets set per-CPU to whatever the CPU's value for > > it is; and then instead of using a PMCR_NUM_COUNTERS value, > > extract the PMCR.N field when needed. The hardest part would be > > going through all the CPU TRMs to find out the correct reset value. > > That makes sense. > > I guess we could also phase the transition by using the default value > if zero? I tend to prefer to avoid that kind of transitional thing, because as a project we have a tendency to never complete transitions. The PMU stuff only applies to the v7 and v8 cores, and we don't implement that many of them, so it's better to just make the effort to find out the correct PMCR reset value for them and be done with it. -- PMM
On Thu, Mar 04, 2021 at 15:14:36 +0000, Peter Maydell wrote: > On Thu, 4 Mar 2021 at 13:53, Leif Lindholm <leif@nuviainc.com> wrote: > > > > On Wed, Mar 03, 2021 at 18:06:46 +0000, Peter Maydell wrote: > > > On Wed, 3 Mar 2021 at 17:48, Leif Lindholm <leif@nuviainc.com> wrote: > > > > It would be good if we could get 6.0 closer to SBSA compliance. > > > > > > How far away are we at the moment ? > > > > > > > Would it be worth the effort to make this controllable per cpu model? > > > > > > I don't have a strong opinion on whether we should, but if we do then the > > > right way to implement that would be to have the PMCR reset value > > > as a reset_pmcr_el0 field in struct ARMCPU (like the existing reset_fpsid, > > > reset_sctlr, etc) that gets set per-CPU to whatever the CPU's value for > > > it is; and then instead of using a PMCR_NUM_COUNTERS value, > > > extract the PMCR.N field when needed. The hardest part would be > > > going through all the CPU TRMs to find out the correct reset value. > > > > That makes sense. > > > > I guess we could also phase the transition by using the default value > > if zero? > > I tend to prefer to avoid that kind of transitional thing, because > as a project we have a tendency to never complete transitions. The > PMU stuff only applies to the v7 and v8 cores, and we don't implement > that many of them, so it's better to just make the effort to find out > the correct PMCR reset value for them and be done with it. Understood. I'll throw this on my never-shrinking pile of things I hope to get around to at some point. / Leif
On Thu, 4 Mar 2021 at 15:25, Leif Lindholm <leif@nuviainc.com> wrote: > > On Thu, Mar 04, 2021 at 15:14:36 +0000, Peter Maydell wrote: > > On Thu, 4 Mar 2021 at 13:53, Leif Lindholm <leif@nuviainc.com> wrote: > > > > > > On Wed, Mar 03, 2021 at 18:06:46 +0000, Peter Maydell wrote: > > > > On Wed, 3 Mar 2021 at 17:48, Leif Lindholm <leif@nuviainc.com> wrote: > > > > > It would be good if we could get 6.0 closer to SBSA compliance. > > > > > > > > How far away are we at the moment ? > > > > > > > > > Would it be worth the effort to make this controllable per cpu model? > > > > > > > > I don't have a strong opinion on whether we should, but if we do then the > > > > right way to implement that would be to have the PMCR reset value > > > > as a reset_pmcr_el0 field in struct ARMCPU (like the existing reset_fpsid, > > > > reset_sctlr, etc) that gets set per-CPU to whatever the CPU's value for > > > > it is; and then instead of using a PMCR_NUM_COUNTERS value, > > > > extract the PMCR.N field when needed. The hardest part would be > > > > going through all the CPU TRMs to find out the correct reset value. > > > > > > That makes sense. > I'll throw this on my never-shrinking pile of things I hope to get > around to at some point. I just sent a patch that does it that way, though it is desperately in need of more testing. I'm currently on the fence about whether to put this patch or that one in for 6.0. thanks -- PMM
diff --git a/target/arm/helper.c b/target/arm/helper.c index 0e1a3b9421..02e25b5c22 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -38,7 +38,7 @@ #endif #define ARM_CPU_FREQ 1000000000 /* FIXME: 1 GHz, should be configurable */ -#define PMCR_NUM_COUNTERS 4 /* QEMU IMPDEF choice */ +#define PMCR_NUM_COUNTERS 6 /* QEMU IMPDEF choice */ #ifndef CONFIG_USER_ONLY
Arm BSA (Base System Architecture) specification says: B_PE_09: PEs must implement the FEAT_PMUv3p1 extension, and the base system must expose a minimum of four programmable PMU counters to the operating system. B_PE_21: The base system must expose a minimum of two programmable PMU counters to a hypervisor. It is then repeated in SBSA (Server Base System Architecture) specification in level 3 requirements: Each PE must implement a minimum of six programmable PMU counters. So let make QEMU provide those 6 PMU counters. SBSA-ACS says now: 12 : Check number of PMU counters : Result: PASS Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org> --- target/arm/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)