diff mbox series

[v2,08/10] Documentation: admin-guide: Add riscv sysctl_perf_user_access

Message ID 20230512085321.13259-9-alexghiti@rivosinc.com
State New
Headers
Series riscv: Allow userspace to directly access perf counters |

Commit Message

Alexandre Ghiti May 12, 2023, 8:53 a.m. UTC 
  riscv now uses this sysctl so document its usage for this architecture.

Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 Documentation/admin-guide/sysctl/kernel.rst | 24 +++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

Comments

Andrew Jones May 31, 2023, 3:07 p.m. UTC | #1 
On Fri, May 12, 2023 at 10:53:19AM +0200, Alexandre Ghiti wrote:
> riscv now uses this sysctl so document its usage for this architecture.
> 
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
>  Documentation/admin-guide/sysctl/kernel.rst | 24 +++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
> index 4b7bfea28cd7..93cd518ca94b 100644
> --- a/Documentation/admin-guide/sysctl/kernel.rst
> +++ b/Documentation/admin-guide/sysctl/kernel.rst
> @@ -941,16 +941,32 @@ enabled, otherwise writing to this file will return ``-EBUSY``.
>  The default value is 8.
>  
>  
> -perf_user_access (arm64 only)
> -=================================
> +perf_user_access (arm64 and riscv only)
> +=======================================
> +
> +Controls user space access for reading perf event counters.
>  
> -Controls user space access for reading perf event counters. When set to 1,
> -user space can read performance monitor counter registers directly.
> +arm64
> +=====
>  
>  The default value is 0 (access disabled).
> +When set to 1, user space can read performance monitor counter registers
> +directly.
>  
>  See Documentation/arm64/perf.rst for more information.
>  
> +riscv
> +=====
> +
> +When set to 0, user access is disabled.
> +
> +When set to 1, user space can read performance monitor counter registers
> +directly only through perf, any direct access without perf intervention will
> +trigger an illegal instruction.
> +
> +The default value is 2, which enables legacy mode (user space has direct
> +access to cycle, time and insret CSRs only). Note that this legacy value
> +is deprecated and will be removed once all userspace applications are fixed.

All modes can access the time CSR so I'm not sure if it should be pointed
out here as if it's an exception. Maybe we shouldn't point it out at all
or we should point it out for all three?

Thanks,
drew
Atish Patra May 31, 2023, 5:08 p.m. UTC | #2 
On Wed, May 31, 2023 at 8:07 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Fri, May 12, 2023 at 10:53:19AM +0200, Alexandre Ghiti wrote:
> > riscv now uses this sysctl so document its usage for this architecture.
> >
> > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > ---
> >  Documentation/admin-guide/sysctl/kernel.rst | 24 +++++++++++++++++----
> >  1 file changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
> > index 4b7bfea28cd7..93cd518ca94b 100644
> > --- a/Documentation/admin-guide/sysctl/kernel.rst
> > +++ b/Documentation/admin-guide/sysctl/kernel.rst
> > @@ -941,16 +941,32 @@ enabled, otherwise writing to this file will return ``-EBUSY``.
> >  The default value is 8.
> >
> >
> > -perf_user_access (arm64 only)
> > -=================================
> > +perf_user_access (arm64 and riscv only)
> > +=======================================
> > +
> > +Controls user space access for reading perf event counters.
> >
> > -Controls user space access for reading perf event counters. When set to 1,
> > -user space can read performance monitor counter registers directly.
> > +arm64
> > +=====
> >
> >  The default value is 0 (access disabled).
> > +When set to 1, user space can read performance monitor counter registers
> > +directly.
> >
> >  See Documentation/arm64/perf.rst for more information.
> >
> > +riscv
> > +=====
> > +
> > +When set to 0, user access is disabled.
> > +
> > +When set to 1, user space can read performance monitor counter registers
> > +directly only through perf, any direct access without perf intervention will
> > +trigger an illegal instruction.
> > +
> > +The default value is 2, which enables legacy mode (user space has direct
> > +access to cycle, time and insret CSRs only). Note that this legacy value
> > +is deprecated and will be removed once all userspace applications are fixed.
>
> All modes can access the time CSR so I'm not sure if it should be pointed
> out here as if it's an exception. Maybe we shouldn't point it out at all
> or we should point it out for all three?
>

Valid point. Thanks Drew.
In the future, we probably want to support prctl
(PR_SET_TSC/SECCOMP_MODE_STRICT) to disable even
time CSR access. I don't think there is any use case for it right now.

> Thanks,
> drew
Alexandre Ghiti June 15, 2023, 10 a.m. UTC | #3 
On 31/05/2023 17:07, Andrew Jones wrote:
> On Fri, May 12, 2023 at 10:53:19AM +0200, Alexandre Ghiti wrote:
>> riscv now uses this sysctl so document its usage for this architecture.
>>
>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>> ---
>>   Documentation/admin-guide/sysctl/kernel.rst | 24 +++++++++++++++++----
>>   1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
>> index 4b7bfea28cd7..93cd518ca94b 100644
>> --- a/Documentation/admin-guide/sysctl/kernel.rst
>> +++ b/Documentation/admin-guide/sysctl/kernel.rst
>> @@ -941,16 +941,32 @@ enabled, otherwise writing to this file will return ``-EBUSY``.
>>   The default value is 8.
>>   
>>   
>> -perf_user_access (arm64 only)
>> -=================================
>> +perf_user_access (arm64 and riscv only)
>> +=======================================
>> +
>> +Controls user space access for reading perf event counters.
>>   
>> -Controls user space access for reading perf event counters. When set to 1,
>> -user space can read performance monitor counter registers directly.
>> +arm64
>> +=====
>>   
>>   The default value is 0 (access disabled).
>> +When set to 1, user space can read performance monitor counter registers
>> +directly.
>>   
>>   See Documentation/arm64/perf.rst for more information.
>>   
>> +riscv
>> +=====
>> +
>> +When set to 0, user access is disabled.
>> +
>> +When set to 1, user space can read performance monitor counter registers
>> +directly only through perf, any direct access without perf intervention will
>> +trigger an illegal instruction.
>> +
>> +The default value is 2, which enables legacy mode (user space has direct
>> +access to cycle, time and insret CSRs only). Note that this legacy value
>> +is deprecated and will be removed once all userspace applications are fixed.
> All modes can access the time CSR so I'm not sure if it should be pointed
> out here as if it's an exception. Maybe we shouldn't point it out at all
> or we should point it out for all three?


Ok I removed the reference to the time CSR for the legacy mode and 
instead added a note stating that this CSR is always accessible whatever 
the mode.

Thanks!


>
> Thanks,
> drew
diff mbox series

Patch

diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index 4b7bfea28cd7..93cd518ca94b 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -941,16 +941,32 @@  enabled, otherwise writing to this file will return ``-EBUSY``.
 The default value is 8.
 
 
-perf_user_access (arm64 only)
-=================================
+perf_user_access (arm64 and riscv only)
+=======================================
+
+Controls user space access for reading perf event counters.
 
-Controls user space access for reading perf event counters. When set to 1,
-user space can read performance monitor counter registers directly.
+arm64
+=====
 
 The default value is 0 (access disabled).
+When set to 1, user space can read performance monitor counter registers
+directly.
 
 See Documentation/arm64/perf.rst for more information.
 
+riscv
+=====
+
+When set to 0, user access is disabled.
+
+When set to 1, user space can read performance monitor counter registers
+directly only through perf, any direct access without perf intervention will
+trigger an illegal instruction.
+
+The default value is 2, which enables legacy mode (user space has direct
+access to cycle, time and insret CSRs only). Note that this legacy value
+is deprecated and will be removed once all userspace applications are fixed.
 
 pid_max
 =======