Re: [PATCH] KVM: arm64: Don't eagerly teardown the vgic on init error

[Marc Zyngier <maz@kernel.org>]

On Fri, 25 Oct 2024 11:54:38 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (7bit)>]
> On Thu, Oct 24, 2024 at 07:05:10PM +0100, Marc Zyngier wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> 
> > > # ==== Test Assertion Failure ====
> > > #   lib/kvm_util.c:724: false
> > > #   pid=1947 tid=1947 errno=5 - Input/output error
> > > #      1	0x0000000000404edb: __vm_mem_region_delete at kvm_util.c:724 (discriminator 5)
> > > #      2	0x0000000000405d0b: kvm_vm_free at kvm_util.c:762 (discriminator 12)
> > > #      3	0x0000000000402d5f: vm_gic_destroy at vgic_init.c:101
> > > #      4	 (inlined by) test_vcpus_then_vgic at vgic_init.c:368
> > > #      5	 (inlined by) run_tests at vgic_init.c:720
> > > #      6	0x0000000000401a6f: main at vgic_init.c:748
> > > #      7	0x0000ffffa7b37543: ?? ??:0
> > > #      8	0x0000ffffa7b37617: ?? ??:0
> > > #      9	0x0000000000401b6f: _start at ??:?
> > > #   KVM killed/bugged the VM, check the kernel log for clues
> > > not ok 10 selftests: kvm: vgic_init # exit=254
> 
> > > which does rather look like a test bug rather than a problem in the
> > > change itself.
> 
> > Well, the test tries to do braindead things, and then the test
> > infrastructure seems surprised that KVM tells it to bugger off...
> 
> > I can paper over it with this (see below), but frankly, someone who
> > actually cares about this crap should take a look (and ownership).
> 
> I'm not even sure that's a terrible fix, looking at the changelog I get
> the impression the test is deliberately looking to do problematic things
> with the goal of making sure that the kernel handles them appropriately.
> That's not interacting well with the KVM selftest framework's general
> assert early assert often approach but it's a reasonable thing to want
> to test so relaxing the asserts like this is one way of squaring the
> circile.

It *is* a terrible fix, since it makes no effort in finding out
whether the VM is be dead for a good or a bad reason. In a way, the
fix is worse than the current error, because it silently hide the
crap.

So this test will continue to explode until someone fixes it
*properly*. And if that means rewriting the selftest harness, so be
it.

	M.

-- 
Without deviation from the norm, progress is not possible.