Re: [PATCH v2 2/2] RISC-V: KVM: selftests: Add PMU event filter test

[email protected]
Newsgroups org.kernel.vger.kvm
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message falsely claims that the guest programs the 'instructions' SBI PMU event, but the implementation only ever programs the 'cycles' event.
--

--- Patch [2]: [PATCH v2 2/2] RISC-V: KVM: selftests: Add PMU event filter test ---
commit 828aa8f61ee14a213c9f6491df6c9b1b9ddbd337
Author: Yuhang.chen <[email protected]>

RISC-V: KVM: selftests: Add PMU event filter test
    
This patch adds a selftest to verify KVM_SET_PMU_EVENT_FILTER on RISC-V.
The host applies ALLOW and DENY filters, and asserts that the guest receives
the expected success or failure when configuring PMU counters.

> Add a selftest that exercises KVM_SET_PMU_EVENT_FILTER on RISC-V. The
> guest programs the CPU cycles and instructions SBI PMU events through
> SBI_EXT_PMU_COUNTER_CFG_MATCH while the host installs filters with the
> ALLOW and DENY actions, asserting that disallowed events return
> SBI_ERR_NOT_SUPPORTED and allowed events succeed.

[Severity: Medium]
Is this description accurate?

The commit message states that the guest programs both the CPU cycles and
instructions SBI PMU events.

However, looking at guest_code(), it appears to only ever program EV_CYCLES
via the SBI_EXT_PMU_COUNTER_CFG_MATCH call:

guest_code() {
        ...
        ret = sbi_ecall(SBI_EXT_PMU, SBI_EXT_PMU_COUNTER_CFG_MATCH,
                        0, 1, 0, EV_CYCLES, 0, 0);
        ...
}

An inline comment in the test code also explicitly notes that the
instructions event is never programmed by the guest: "Instructions itself is
never programmed, so host support for it is irrelevant."

Could the commit message be updated to reflect that only the CPU cycles
event is actually programmed by the guest code?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.