Message-ID: <70DD7215-76A0-493C-87CA-60A605842B44@ventanamicro.com>
Date: Fri, 16 May 2025 14:39:06 +0200
From: Andrew Jones <ajones@...tanamicro.com>
To: kvm-riscv@...ts.infradead.org, Anup Patel <anup@...infault.org>,
 Atish Patra <atishp@...osinc.com>
CC: Atish Patra <atishp@...shpatra.org>, Paul Walmsley <paul.walmsley@...ive.com>,
 Palmer Dabbelt <palmer@...belt.com>, Alexandre Ghiti <alex@...ti.fr>,
 kvm@...r.kernel.org, linux-riscv@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] RISC-V: KVM: Remove scounteren initialization

On May 16, 2025 2:18:18 PM GMT+02:00, Anup Patel <anup@...infault.org> wrote:
>On Fri, May 16, 2025 at 4:41 AM Atish Patra <atishp@...osinc.com> wrote:
>>
>> Scounteren CSR controls the direct access the hpmcounters and cycle/
>> instret/time from the userspace. It's the supervisor's responsibility
>> to set it up correctly for it's user space. They hypervisor doesn't
>> need to decide the policy on behalf of the supervisor.
>>
>> Signed-off-by: Atish Patra <atishp@...osinc.com>
>> ---
>> Changes in v3:
>> - Removed the redundant declaration
>> - Link to v2: https://lore.kernel.org/r/20250515-fix_scounteren_vs-v2-1-1fd8dc0693e8@rivosinc.com
>>
>> Changes in v2:
>> - Remove the scounteren initialization instead of just setting the TM bit.
>> - Link to v1: https://lore.kernel.org/r/20250513-fix_scounteren_vs-v1-1-c1f52af93c79@rivosinc.com
>> ---
>>  arch/riscv/kvm/vcpu.c | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
>> index 60d684c76c58..9bfaae9a11ea 100644
>> --- a/arch/riscv/kvm/vcpu.c
>> +++ b/arch/riscv/kvm/vcpu.c
>> @@ -111,7 +111,6 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>>  {
>>         int rc;
>>         struct kvm_cpu_context *cntx;
>> -       struct kvm_vcpu_csr *reset_csr = &vcpu->arch.guest_reset_csr;
>>
>>         spin_lock_init(&vcpu->arch.mp_state_lock);
>>
>> @@ -146,9 +145,6 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>>         if (kvm_riscv_vcpu_alloc_vector_context(vcpu, cntx))
>>                 return -ENOMEM;
>>
>> -       /* By default, make CY, TM, and IR counters accessible in VU mode */
>> -       reset_csr->scounteren = 0x7;
>> -
>>         /* Setup VCPU timer */
>>         kvm_riscv_vcpu_timer_init(vcpu);
>>
>>
>> ---
>> base-commit: 01f95500a162fca88cefab9ed64ceded5afabc12
>> change-id: 20250513-fix_scounteren_vs-fdd86255c7b7
>> --
>
>Overall, this looks good.
>
>Reviewed-by: Anup Patel <anup@...infault.org>
>
>Currently, the scounteren.TM bit is only set by the Linux SBI PMU
>driver but KVM RISC-V only provides SBI PMU for guest when
>Sscofpmf is available in host so we need the below hunk to
>completely get rid-off scounteren initialization in KVM RISC-V.
>
>diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>index 356d5397b2a2..bdf3352acf4c 100644
>--- a/arch/riscv/kernel/head.S
>+++ b/arch/riscv/kernel/head.S
>@@ -131,6 +131,12 @@ secondary_start_sbi:
>     csrw CSR_IE, zero
>     csrw CSR_IP, zero
>
>+#ifndef CONFIG_RISCV_M_MODE
>+    /* Enable time CSR */
>+    li t0, 0x2
>+    csrw CSR_SCOUNTEREN, t0
>+#endif
>+
>     /* Load the global pointer */
>     load_global_pointer
>
>@@ -226,6 +232,10 @@ SYM_CODE_START(_start_kernel)
>      * to hand it to us.
>      */
>     csrr a0, CSR_MHARTID
>+#else
>+    /* Enable time CSR */
>+    li t0, 0x2
>+    csrw CSR_SCOUNTEREN, t0
> #endif /* CONFIG_RISCV_M_MODE */
>
>     /* Load the global pointer */
>
>I have queued this patch for Linux-6.16 with the above hunk squashed.
>
>Thanks,
>Anup
>
>-- 
>kvm-riscv mailing list
>kvm-riscv@...ts.infradead.org
>http://lists.infradead.org/mailman/listinfo/kvm-riscv

Yup, I had planned to post something like that too, but didn't get to it.

Thanks,
drew

Powered by blists - more mailing lists