| Message ID | 20230308110948.1820163-1-ryan.roberts@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: selftests: Fixes for broken tests | expand |
Hi Oliver, Just a polite nudge on this: I was originally hoping to get these into 6.3 since I thought they were fairly uncontroversial and clearly fixing bugs. What are my chances? Thanks, Ryan On 08/03/2023 11:09, Ryan Roberts wrote: > Hi Oliver, > > Here is a respin of the KVM selftests fixes, with your nits addressed; I've > fixed the footer whitespace issue and I'm now using FIELD_GET() in the place > where you suggested and a couple of other places too. I've also included the 3rd > patch in this series (the ttbr0_el1 fix), which I originally sent separately - > this is now using FIELD_GET() too. > > So this series superceeds [1] and [2]. > > Thanks, > Ryan > > [1] https://lore.kernel.org/kvmarm/e8ed178a-0c67-3e00-a085-1d88fb3cb41f@arm.com/ > [2] https://lore.kernel.org/kvmarm/20230302152033.242073-1-ryan.roberts@arm.com/ > > Ryan Roberts (3): > KVM: selftests: Fixup config fragment for access_tracking_perf_test > KVM: selftests: arm64: Fix pte encode/decode for PA bits > 48 > KVM: selftests: arm64: Fix ttbr0_el1 encoding for PA bits > 48 > > tools/testing/selftests/kvm/config | 1 + > .../selftests/kvm/lib/aarch64/processor.c | 39 ++++++++++++++----- > 2 files changed, 30 insertions(+), 10 deletions(-) > > -- > 2.25.1 > >
Hey Ryan, On Thu, Mar 23, 2023 at 12:56:18PM +0000, Ryan Roberts wrote: > Hi Oliver, > > Just a polite nudge on this: I was originally hoping to get these into 6.3 since > I thought they were fairly uncontroversial and clearly fixing bugs. What are my > chances? Yes, your changes are indeed uncontroversial :) At least for me, fixes to selftests take a strictly lower priority than fixes to the kernel outside of a merge window. AFAICT, only LPA systems are affected by the changes here and I'm not aware of any of those out in the wild. So, unless there is a burning issue, I'd like to defer these patches to the 6.4 merge window. Nonetheless, it all looks good to me: Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
On 2023-03-23 17:38, Oliver Upton wrote: > Hey Ryan, > > On Thu, Mar 23, 2023 at 12:56:18PM +0000, Ryan Roberts wrote: >> Hi Oliver, >> >> Just a polite nudge on this: I was originally hoping to get these into >> 6.3 since >> I thought they were fairly uncontroversial and clearly fixing bugs. >> What are my >> chances? > > Yes, your changes are indeed uncontroversial :) At least for me, fixes > to > selftests take a strictly lower priority than fixes to the kernel > outside of > a merge window. AFAICT, only LPA systems are affected by the changes > here and > I'm not aware of any of those out in the wild. Agreed. My usual take on fixing tests is that unless the test has been broken in the current cycle, we can safely delay merging the fix until the following cycle. And yes, LPA-capable HW is essentially vapourware at this stage. > > So, unless there is a burning issue, I'd like to defer these patches to > the > 6.4 merge window. Nonetheless, it all looks good to me: > > Reviewed-by: Oliver Upton <oliver.upton@linux.dev> Thanks for that. I'll start queuing 6.4 material once I'm back to my usual time zone, beginning of next week. Cheers, M.
On 23/03/2023 18:56, Marc Zyngier wrote: > On 2023-03-23 17:38, Oliver Upton wrote: >> Hey Ryan, >> >> On Thu, Mar 23, 2023 at 12:56:18PM +0000, Ryan Roberts wrote: >>> Hi Oliver, >>> >>> Just a polite nudge on this: I was originally hoping to get these into 6.3 since >>> I thought they were fairly uncontroversial and clearly fixing bugs. What are my >>> chances? >> >> Yes, your changes are indeed uncontroversial :) At least for me, fixes to >> selftests take a strictly lower priority than fixes to the kernel outside of >> a merge window. AFAICT, only LPA systems are affected by the changes here and >> I'm not aware of any of those out in the wild. The first patch is not related to LPA, it adds a missing kernel config to the config fragment so that one of the tests (access_tracking_perf_test) is not skipped. This change will mean our CI system starts actually running the test. > > Agreed. My usual take on fixing tests is that unless the test has been > broken in the current cycle, we can safely delay merging the fix until > the following cycle. Thanks for the explanation. I have a slightly different opinion though (please bare with me through the rant): Being fairly new to Linux development, I'd like to be able to run (all) the selftests as a matter of course to be able to quickly answer the "did I obviously break anything?" question. But there is a lot of friction to even being able to compile, let alone run, the things - undocumented dependencies on libraries (even more difficult when needing to cross compile), undocumented dependencies on kernel configs, test code that is broken and fails to compile, tests that silently skip for difficult to determine reasons, tests that fail even when run against the unmodified kernel, and results buried in copious amounts of logs. These are all paper cuts that make them difficult to use and trust. Or perhaps I'm just doing it wrong... I would love to live in a world where I could confidently take a mainline release, compile and run tests at close-to-zero effort and see all tests running and passing... one day, perhaps. But only if we give more priority to the test code ;-) > > And yes, LPA-capable HW is essentially vapourware at this stage. > >> >> So, unless there is a burning issue, I'd like to defer these patches to the >> 6.4 merge window. Nonetheless, it all looks good to me: >> >> Reviewed-by: Oliver Upton <oliver.upton@linux.dev> Thanks for that. > > Thanks for that. I'll start queuing 6.4 material once I'm back to my > usual time zone, beginning of next week. Appreciated. Thanks for taking the patches. If you have a rule-of-thumb about the best time to post different types of patches (and what it's best to base them on), I'll try to follow it in future. Thanks, Ryan > > Cheers, > > M.
On Wed, 8 Mar 2023 11:09:45 +0000, Ryan Roberts wrote: > Here is a respin of the KVM selftests fixes, with your nits addressed; I've > fixed the footer whitespace issue and I'm now using FIELD_GET() in the place > where you suggested and a couple of other places too. I've also included the 3rd > patch in this series (the ttbr0_el1 fix), which I originally sent separately - > this is now using FIELD_GET() too. > > So this series superceeds [1] and [2]. > > [...] Applied to next, thanks! [1/3] KVM: selftests: Fixup config fragment for access_tracking_perf_test commit: a2bed39057b434c4fd816005d1b950fefc61569d [2/3] KVM: selftests: arm64: Fix pte encode/decode for PA bits > 48 commit: e659babfc5a693553cf9473470840464f0ed5d77 [3/3] KVM: selftests: arm64: Fix ttbr0_el1 encoding for PA bits > 48 commit: e17071754cf50e5f6ff8ebee077e0e4114d3bea5 Cheers, M.