Re: [PATCH v3] RISC-V: KVM: Remove scounteren initialization

[Atish Patra]

On 5/16/25 5:18 AM, Anup Patel wrote:
> On Fri, May 16, 2025 at 4:41 AM Atish Patra <atishp@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> > ---
> > Changes in v3:
> > - Removed the redundant declaration
> > - Link to v2: https://lore.kernel.org/r/20250515-fix_scounteren_vs-v2-1-1fd8dc0693e8@xxxxxxxxxxxx
> >
> > 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@xxxxxxxxxxxx
> > ---
> >   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@xxxxxxxxxxxxxx>
>
> 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.

Yes. Thanks for adding this.

> Thanks,
> Anup