| Message ID | 20220811185210.234711-1-ricarkol@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm: pmu: Fixes for bare metal | expand |
On Thu, Aug 11, 2022 at 11:52:06AM -0700, Ricardo Koller wrote: > There are some tests that fail when running on bare metal (including a > passthrough prototype). There are three issues with the tests. The > first one is that there are some missing isb()'s between enabling event > counting and the actual counting. This wasn't an issue on KVM as > trapping on registers served as context synchronization events. The > second issue is that some tests assume that registers reset to 0. And > finally, the third issue is that overflowing the low counter of a > chained event sets the overflow flag in PMVOS and some tests fail by > checking for it not being set. > > Addressed all comments from the previous version: > https://lore.kernel.org/kvmarm/YvPsBKGbHHQP+0oS@google.com/T/#mb077998e2eb9fb3e15930b3412fd7ba2fb4103ca > - add pmu_reset() for 32-bit arm [Andrew] > - collect r-b from Alexandru You forgot to pick up Oliver's r-b's and his Link suggestion. I can do that again, though. Thanks, drew
On Fri, Aug 12, 2022 at 08:33:00AM +0200, Andrew Jones wrote: > On Thu, Aug 11, 2022 at 11:52:06AM -0700, Ricardo Koller wrote: > > There are some tests that fail when running on bare metal (including a > > passthrough prototype). There are three issues with the tests. The > > first one is that there are some missing isb()'s between enabling event > > counting and the actual counting. This wasn't an issue on KVM as > > trapping on registers served as context synchronization events. The > > second issue is that some tests assume that registers reset to 0. And > > finally, the third issue is that overflowing the low counter of a > > chained event sets the overflow flag in PMVOS and some tests fail by > > checking for it not being set. > > > > Addressed all comments from the previous version: > > https://lore.kernel.org/kvmarm/YvPsBKGbHHQP+0oS@google.com/T/#mb077998e2eb9fb3e15930b3412fd7ba2fb4103ca > > - add pmu_reset() for 32-bit arm [Andrew] > > - collect r-b from Alexandru > > You forgot to pick up Oliver's r-b's and his Link suggestion. Ahh, yes, sorry Oliver. > I can do that again, though. Thanks Andrew > > Thanks, > drew
On 2022-08-11 19:52, Ricardo Koller wrote: > There are some tests that fail when running on bare metal (including a > passthrough prototype). There are three issues with the tests. The > first one is that there are some missing isb()'s between enabling event > counting and the actual counting. This wasn't an issue on KVM as > trapping on registers served as context synchronization events. The > second issue is that some tests assume that registers reset to 0. And > finally, the third issue is that overflowing the low counter of a > chained event sets the overflow flag in PMVOS and some tests fail by > checking for it not being set. > > Addressed all comments from the previous version: > https://lore.kernel.org/kvmarm/YvPsBKGbHHQP+0oS@google.com/T/#mb077998e2eb9fb3e15930b3412fd7ba2fb4103ca > - add pmu_reset() for 32-bit arm [Andrew] > - collect r-b from Alexandru > > Thanks! > Ricardo > > Ricardo Koller (4): > arm: pmu: Add missing isb()'s after sys register writing > arm: pmu: Add reset_pmu() for 32-bit arm > arm: pmu: Reset the pmu registers before starting some tests > arm: pmu: Check for overflow in the low counter in chained counters > tests > > arm/pmu.c | 72 ++++++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 55 insertions(+), 17 deletions(-) For the series: Acked-by: Marc Zyngier <maz@kernel.org> M.
On Thu, Aug 11, 2022 at 11:52:06AM -0700, Ricardo Koller wrote: > There are some tests that fail when running on bare metal (including a > passthrough prototype). There are three issues with the tests. The > first one is that there are some missing isb()'s between enabling event > counting and the actual counting. This wasn't an issue on KVM as > trapping on registers served as context synchronization events. The > second issue is that some tests assume that registers reset to 0. And > finally, the third issue is that overflowing the low counter of a > chained event sets the overflow flag in PMVOS and some tests fail by > checking for it not being set. > > Addressed all comments from the previous version: > https://lore.kernel.org/kvmarm/YvPsBKGbHHQP+0oS@google.com/T/#mb077998e2eb9fb3e15930b3412fd7ba2fb4103ca > - add pmu_reset() for 32-bit arm [Andrew] > - collect r-b from Alexandru > > Thanks! > Ricardo > > Ricardo Koller (4): > arm: pmu: Add missing isb()'s after sys register writing > arm: pmu: Add reset_pmu() for 32-bit arm > arm: pmu: Reset the pmu registers before starting some tests > arm: pmu: Check for overflow in the low counter in chained counters > tests > > arm/pmu.c | 72 ++++++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 55 insertions(+), 17 deletions(-) > Hi all, Please refresh my memory. Does this series work on current platforms? Or was it introducing new test failures which may be in the test, as opposed to KVM? If they work on most platforms, but not on every platform, then have we identified what triggers them to fail and whether that should be fixed or just worked-around? I'm sorry I still can't help out with the testing as I haven't yet had time to setup the Rpi that Mark Rutland gave me in Dublin. I know this series has been rotting on arm/queue for months, so I'll be happy to merge it if the consensus is to do so. I can also drop it, or some of the patches, if that's the consensus. Thanks, drew
Hi Drew, On Fri, 28 Oct 2022 12:40:41 +0100, Andrew Jones <andrew.jones@linux.dev> wrote: > > On Thu, Aug 11, 2022 at 11:52:06AM -0700, Ricardo Koller wrote: > > There are some tests that fail when running on bare metal (including a > > passthrough prototype). There are three issues with the tests. The > > first one is that there are some missing isb()'s between enabling event > > counting and the actual counting. This wasn't an issue on KVM as > > trapping on registers served as context synchronization events. The > > second issue is that some tests assume that registers reset to 0. And > > finally, the third issue is that overflowing the low counter of a > > chained event sets the overflow flag in PMVOS and some tests fail by > > checking for it not being set. > > > > Addressed all comments from the previous version: > > https://lore.kernel.org/kvmarm/YvPsBKGbHHQP+0oS@google.com/T/#mb077998e2eb9fb3e15930b3412fd7ba2fb4103ca > > - add pmu_reset() for 32-bit arm [Andrew] > > - collect r-b from Alexandru > > > > Thanks! > > Ricardo > > > > Ricardo Koller (4): > > arm: pmu: Add missing isb()'s after sys register writing > > arm: pmu: Add reset_pmu() for 32-bit arm > > arm: pmu: Reset the pmu registers before starting some tests > > arm: pmu: Check for overflow in the low counter in chained counters > > tests > > > > arm/pmu.c | 72 ++++++++++++++++++++++++++++++++++++++++++------------- > > 1 file changed, 55 insertions(+), 17 deletions(-) > > > > Hi all, > > Please refresh my memory. Does this series work on current platforms? Or > was it introducing new test failures which may be in the test, as opposed > to KVM? If they work on most platforms, but not on every platform, then > have we identified what triggers them to fail and whether that should be > fixed or just worked-around? I'm sorry I still can't help out with the > testing as I haven't yet had time to setup the Rpi that Mark Rutland gave > me in Dublin. This series does show that KVM is buggy, and I have patches out to fix it [1]. The patches should work on anything, really. > I know this series has been rotting on arm/queue for months, so I'll be > happy to merge it if the consensus is to do so. I can also drop it, or > some of the patches, if that's the consensus. I'd be very happy to see these patches being merged. Thanks, M. [1] https://lore.kernel.org/r/20221028105402.2030192-1-maz@kernel.org
On Fri, Oct 28, 2022 at 04:01:50PM +0100, Marc Zyngier wrote: > Hi Drew, > > On Fri, 28 Oct 2022 12:40:41 +0100, > Andrew Jones <andrew.jones@linux.dev> wrote: > > > > On Thu, Aug 11, 2022 at 11:52:06AM -0700, Ricardo Koller wrote: > > > There are some tests that fail when running on bare metal (including a > > > passthrough prototype). There are three issues with the tests. The > > > first one is that there are some missing isb()'s between enabling event > > > counting and the actual counting. This wasn't an issue on KVM as > > > trapping on registers served as context synchronization events. The > > > second issue is that some tests assume that registers reset to 0. And > > > finally, the third issue is that overflowing the low counter of a > > > chained event sets the overflow flag in PMVOS and some tests fail by > > > checking for it not being set. > > > > > > Addressed all comments from the previous version: > > > https://lore.kernel.org/kvmarm/YvPsBKGbHHQP+0oS@google.com/T/#mb077998e2eb9fb3e15930b3412fd7ba2fb4103ca > > > - add pmu_reset() for 32-bit arm [Andrew] > > > - collect r-b from Alexandru > > > > > > Thanks! > > > Ricardo > > > > > > Ricardo Koller (4): > > > arm: pmu: Add missing isb()'s after sys register writing > > > arm: pmu: Add reset_pmu() for 32-bit arm > > > arm: pmu: Reset the pmu registers before starting some tests > > > arm: pmu: Check for overflow in the low counter in chained counters > > > tests > > > > > > arm/pmu.c | 72 ++++++++++++++++++++++++++++++++++++++++++------------- > > > 1 file changed, 55 insertions(+), 17 deletions(-) > > > > > > > Hi all, > > > > Please refresh my memory. Does this series work on current platforms? Or > > was it introducing new test failures which may be in the test, as opposed > > to KVM? If they work on most platforms, but not on every platform, then > > have we identified what triggers them to fail and whether that should be > > fixed or just worked-around? I'm sorry I still can't help out with the > > testing as I haven't yet had time to setup the Rpi that Mark Rutland gave > > me in Dublin. > > This series does show that KVM is buggy, and I have patches out to fix > it [1]. The patches should work on anything, really. > > > I know this series has been rotting on arm/queue for months, so I'll be > > happy to merge it if the consensus is to do so. I can also drop it, or > > some of the patches, if that's the consensus. > > I'd be very happy to see these patches being merged. Thanks for the information, Marc. I've gone ahead and merged the tests. What's the worst than can happen :-) Anyway, I agree that when the tests start failing in CIs, then they're doing their job. If your pending series can't be applied right away, then the CIs can likely be taught to temporarily ignore the known failures. Thanks, drew > > Thanks, > > M. > > [1] https://lore.kernel.org/r/20221028105402.2030192-1-maz@kernel.org > > -- > Without deviation from the norm, progress is not possible.
There are some tests that fail when running on bare metal (including a passthrough prototype). There are three issues with the tests. The first one is that there are some missing isb()'s between enabling event counting and the actual counting. This wasn't an issue on KVM as trapping on registers served as context synchronization events. The second issue is that some tests assume that registers reset to 0. And finally, the third issue is that overflowing the low counter of a chained event sets the overflow flag in PMVOS and some tests fail by checking for it not being set. Addressed all comments from the previous version: https://lore.kernel.org/kvmarm/YvPsBKGbHHQP+0oS@google.com/T/#mb077998e2eb9fb3e15930b3412fd7ba2fb4103ca - add pmu_reset() for 32-bit arm [Andrew] - collect r-b from Alexandru Thanks! Ricardo Ricardo Koller (4): arm: pmu: Add missing isb()'s after sys register writing arm: pmu: Add reset_pmu() for 32-bit arm arm: pmu: Reset the pmu registers before starting some tests arm: pmu: Check for overflow in the low counter in chained counters tests arm/pmu.c | 72 ++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 17 deletions(-)