Hi Alex, On Tue, Apr 26, 2022 at 2:01 AM Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote: > > > The odroid-c4 is somewhat of a special case, because Linux doesn't probe > > > the PMU. But the above errors can easily be reproduced on any hardware, > > > with or without a PMU driver, as long as userspace doesn't set the PMU > > > feature. > > > > This note has me wondering if we could do more negative testing with > > kvm-unit-tests just by selectively turning on/off features, with the > > expectation that tests either skip or pass. > > I'm not sure that that can be accomplished right now. kvm-unit-tests > supports only qemu as an automated test runner, and qemu enables the PMU by > default. I don't know if it can be disabled, it would be nice if it could. > I stumbled upon this by mistake, when I ran kvmtool without enabling the > PMU (the default in kvmtool is to not have it enabled). > > If it is possible to disable PMU emulation from the qemu's command line, > then it should be as simple as writing a test that expects all PMU register > accesses to trigger an undefined exception (and adding a new test > definition). You can disable the PMU with QEMU by specifying pmu=off in the -cpu argument, among other things. > If it is not possible to do this with qemu, then we would have to wait > until kvm-unit-tests can use kvmtool as the test runner. I have an RFC sent > for that [1], I need to get back to it. > > Another option would be to have this as a kselftest, although I don't know > how easy it is to register an exception handler in a kselftest. The test > could be further expanded to other registers gated by a VCPU feature being > set. We definitely have the plumbing for exception handlers in selftests, aarch64/debug-exceptions.c is an example. My thought was more general + rather lazy. For any combination of CPU features, expect that kvm-unit-tests should either pass or skip. If they fail or blow up the host then probably a good indicator of a KVM bug. > [1] https://lore.kernel.org/kvmarm/20210702163122.96110-1-alexandru.elisei@xxxxxxx/ Thanks for the link, I'll have a peek. > > > > > Work around the fact that KVM advertises a PMU even when the VCPU feature > > > is not set by gating all PMU emulation on the feature. The guest can still > > > access the registers without KVM injecting an undefined exception. > > > > We're going to need something similar even after KVM conditionally > > advertises the PMU. > > > > WDYT about wiring up sys_reg_desc::visibility for the AArch32 PMU > > registers? For now just treat them as REG_RAZ (probably extend this to > > imply WI too) then promote to REG_HIDDEN in a later patch. > > I was thinking you can simply use .visibility = pmu_visibility, like it's > done with the PMU_SYS_REG macro: Right -- I completely agree this is where we should be when AArch32 feature registers are trapped. Seems to me all the AArch32 feature register trap logic should come later on as there's a nonzero chance I introduced a bug :) Shall we stop the bleeding w/ your originally proposed patch? Doesn't seem any more objectionable than what we're already doing. [...] > I've renamed AA32_ZEROHIGH -> AA32_DIRECT. Feel free to use the snippet as > you see fit (or not at all). To avoid shamelessly plagiarizing: may I package up what you have below as a commit coming from you? -- Thanks, Oliver _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm