RFC: extern illegal instruction trap and trap RDCYCLE

RFC: extern illegal instruction trap and trap RDCYCLE

[Ben Dooks]

This is a RFC series to change how the illegal instruction trap
is handled and then how to trap RDCYCLE and emulate it with RDTIME
instead.

I did this when we found multiple libraries using RDCYCLE and
upgrading multiple runners to newer kernels caused many problems



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 1/3] riscv: ptrace: add regs_set_register()

[Ben Dooks]

Since we have regs_get_register() and we could use the set counterpart
for things like fixing up traps, add regs_set_register() to set a pt_regs
value from offset.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 arch/riscv/include/asm/ptrace.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/riscv/include/asm/ptrace.h b/arch/riscv/include/asm/ptrace.h
index b5b0adcc85c1..66fc1795141d 100644
--- a/arch/riscv/include/asm/ptrace.h
+++ b/arch/riscv/include/asm/ptrace.h
@@ -143,6 +143,26 @@ static inline unsigned long regs_get_register(struct pt_regs *regs,
 	return *(unsigned long *)((unsigned long)regs + offset);
 }
 
+/**
+ * regs_set_register() - set register value from its offset
+ * @regs:	pt_regs from which register value is gotten
+ * @offset:	offset of the register.
+ * @to:		value to set register to
+ *
+ * regs_set_register sets the value @to to a register whose offset from @regs.
+ * The @offset is the offset of the register in struct pt_regs.
+ * If @offset is bigger than MAX_REG_OFFSET, this will ignore the write.
+ */
+static inline void regs_set_register(struct pt_regs *regs,
+				     unsigned int offset,
+				     unsigned long to)
+{
+	if (unlikely(offset > MAX_REG_OFFSET))
+		return;
+
+	*(unsigned long *)((unsigned long)regs + offset) = to;
+}
+
 /**
  * regs_get_kernel_argument() - get Nth function argument in kernel
  * @regs:       pt_regs of that context
-- 
2.37.2.352.g3c44437643


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 2/3] riscv: traps: make insn fetch common in unknown instruction

[Ben Dooks]

Add the insn as the second argument to riscv_v_first_use_handler() form
the trap handler so when we add more handlers we can do the fetch of the
instruction just once.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 arch/riscv/include/asm/vector.h |  4 ++--
 arch/riscv/kernel/traps.c       | 11 ++++++++++-
 arch/riscv/kernel/vector.c      | 11 +----------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index be7d309cca8a..c9f0b02cd975 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -21,7 +21,7 @@
 
 extern unsigned long riscv_v_vsize;
 int riscv_v_setup_vsize(void);
-bool riscv_v_first_use_handler(struct pt_regs *regs);
+bool riscv_v_first_use_handler(struct pt_regs *regs, u32 insn);
 void kernel_vector_begin(void);
 void kernel_vector_end(void);
 void get_cpu_vector_context(void);
@@ -268,7 +268,7 @@ struct pt_regs;
 
 static inline int riscv_v_setup_vsize(void) { return -EOPNOTSUPP; }
 static __always_inline bool has_vector(void) { return false; }
-static inline bool riscv_v_first_use_handler(struct pt_regs *regs) { return false; }
+static inline bool riscv_v_first_use_handler(struct pt_regs *regs, u32 insn) { return false; }
 static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; }
 static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; }
 #define riscv_v_vsize (0)
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 51ebfd23e007..1c3fab272fd1 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -172,11 +172,20 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re
 	bool handled;
 
 	if (user_mode(regs)) {
+		u32 __user *epc = (u32 __user *)regs->epc;
+		u32 insn = (u32)regs->badaddr;
+
 		irqentry_enter_from_user_mode(regs);
 
 		local_irq_enable();
 
-		handled = riscv_v_first_use_handler(regs);
+		if (!insn) {
+			if (__get_user(insn, epc)) {
+				/* todo */
+			}
+		}
+
+		handled = riscv_v_first_use_handler(regs, insn);
 
 		local_irq_disable();
 
diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
index 682b3feee451..b852648cb8d5 100644
--- a/arch/riscv/kernel/vector.c
+++ b/arch/riscv/kernel/vector.c
@@ -168,11 +168,8 @@ bool riscv_v_vstate_ctrl_user_allowed(void)
 }
 EXPORT_SYMBOL_GPL(riscv_v_vstate_ctrl_user_allowed);
 
-bool riscv_v_first_use_handler(struct pt_regs *regs)
+bool riscv_v_first_use_handler(struct pt_regs *regs, u32 insn)
 {
-	u32 __user *epc = (u32 __user *)regs->epc;
-	u32 insn = (u32)regs->badaddr;
-
 	if (!has_vector())
 		return false;
 
@@ -184,12 +181,6 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
 	if (riscv_v_vstate_query(regs))
 		return false;
 
-	/* Get the instruction */
-	if (!insn) {
-		if (__get_user(insn, epc))
-			return false;
-	}
-
 	/* Filter out non-V instructions */
 	if (!insn_is_vector(insn))
 		return false;
-- 
2.37.2.352.g3c44437643


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 3/3] riscv: add trap and emulation for RDCYCLE

[Ben Dooks]

Add a trap for RDCYCLE and emulate it as RDTIME instruciton.

This is an initial PoC and should probably be made more generic
way of trapping and dealing with bad instructions

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 arch/riscv/kernel/traps.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 1c3fab272fd1..51ea28ebf54d 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -167,6 +167,35 @@ DO_ERROR_INFO(do_trap_insn_misaligned,
 DO_ERROR_INFO(do_trap_insn_fault,
 	SIGSEGV, SEGV_ACCERR, "instruction access fault");
 
+#define is_system(__i) (((__i) & 0x7f) == RVG_OPCODE_SYSTEM)
+
+static bool riscv_try_csr_fixup_user(struct pt_regs *regs, u32 insn)
+{
+	/* expecting a 4 byte CSR instruction (*/
+	if (unlikely(GET_INSN_LENGTH(insn) != 4))
+		return false;
+
+	if (is_system(insn)) {
+		u32 csr = insn >> RVG_SYSTEM_CSR_OFF;
+		u32 rd = (insn >> RVG_RD_OPOFF) & RVG_RD_MASK;
+		u32 rs = (insn >> RVG_RS1_OPOFF) & RVG_RS1_MASK;
+		u32 funct3 = (insn >> RV_INSN_FUNCT3_OPOFF) & 0x7;
+
+		if (rs == 0 && funct3 == 2 && csr == CSR_CYCLE) {
+			u64 val = csr_read(CSR_TIME);
+			/* we've got a RDCCLYE, emulated it with CSR_TIME */
+
+			printk_ratelimited("PID %d: process using RDCYCLE, emulating with RDTIME\n", current->pid);
+
+			regs_set_register(regs, rd*sizeof(unsigned long), val);
+			regs->epc += 4;
+			return true;
+		}
+	}
+
+	return false;
+}
+
 asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *regs)
 {
 	bool handled;
@@ -186,6 +215,8 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re
 		}
 
 		handled = riscv_v_first_use_handler(regs, insn);
+		if (!handle)
+			handled = riscv_try_csr_fixup_user(regs, insn);
 
 		local_irq_disable();
 
-- 
2.37.2.352.g3c44437643


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 3/3] riscv: add trap and emulation for RDCYCLE

[Ben Dooks]

On 17/09/2024 14:08, Ben Dooks wrote:
> Add a trap for RDCYCLE and emulate it as RDTIME instruciton.
> 
> This is an initial PoC and should probably be made more generic
> way of trapping and dealing with bad instructions
> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>   arch/riscv/kernel/traps.c | 31 +++++++++++++++++++++++++++++++
>   1 file changed, 31 insertions(+)
> 
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index 1c3fab272fd1..51ea28ebf54d 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -167,6 +167,35 @@ DO_ERROR_INFO(do_trap_insn_misaligned,
>   DO_ERROR_INFO(do_trap_insn_fault,
>   	SIGSEGV, SEGV_ACCERR, "instruction access fault");
>   
> +#define is_system(__i) (((__i) & 0x7f) == RVG_OPCODE_SYSTEM)
> +
> +static bool riscv_try_csr_fixup_user(struct pt_regs *regs, u32 insn)
> +{
> +	/* expecting a 4 byte CSR instruction (*/
> +	if (unlikely(GET_INSN_LENGTH(insn) != 4))
> +		return false;
> +
> +	if (is_system(insn)) {
> +		u32 csr = insn >> RVG_SYSTEM_CSR_OFF;
> +		u32 rd = (insn >> RVG_RD_OPOFF) & RVG_RD_MASK;
> +		u32 rs = (insn >> RVG_RS1_OPOFF) & RVG_RS1_MASK;
> +		u32 funct3 = (insn >> RV_INSN_FUNCT3_OPOFF) & 0x7;
> +
> +		if (rs == 0 && funct3 == 2 && csr == CSR_CYCLE) {
> +			u64 val = csr_read(CSR_TIME);
> +			/* we've got a RDCCLYE, emulated it with CSR_TIME */
> +
> +			printk_ratelimited("PID %d: process using RDCYCLE, emulating with RDTIME\n", current->pid);
> +
> +			regs_set_register(regs, rd*sizeof(unsigned long), val);
> +			regs->epc += 4;
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
>   asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *regs)
>   {
>   	bool handled;
> @@ -186,6 +215,8 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re
>   		}
>   
>   		handled = riscv_v_first_use_handler(regs, insn);
> +		if (!handle)
> +			handled = riscv_try_csr_fixup_user(regs, insn);
>   
>   		local_irq_disable();
>   

oops, forgot to fold in a fix patch, this has 2 issues.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 3/3] riscv: add trap and emulation for RDCYCLE

[Andrew Jones]

On Tue, Sep 17, 2024 at 02:08:53PM GMT, Ben Dooks wrote:
> Add a trap for RDCYCLE and emulate it as RDTIME instruciton.
> 
> This is an initial PoC and should probably be made more generic
> way of trapping and dealing with bad instructions
> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>  arch/riscv/kernel/traps.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
> index 1c3fab272fd1..51ea28ebf54d 100644
> --- a/arch/riscv/kernel/traps.c
> +++ b/arch/riscv/kernel/traps.c
> @@ -167,6 +167,35 @@ DO_ERROR_INFO(do_trap_insn_misaligned,
>  DO_ERROR_INFO(do_trap_insn_fault,
>  	SIGSEGV, SEGV_ACCERR, "instruction access fault");
>  
> +#define is_system(__i) (((__i) & 0x7f) == RVG_OPCODE_SYSTEM)

We have riscv_insn_is_system()

> +
> +static bool riscv_try_csr_fixup_user(struct pt_regs *regs, u32 insn)
> +{
> +	/* expecting a 4 byte CSR instruction (*/
> +	if (unlikely(GET_INSN_LENGTH(insn) != 4))
> +		return false;
> +
> +	if (is_system(insn)) {
> +		u32 csr = insn >> RVG_SYSTEM_CSR_OFF;
> +		u32 rd = (insn >> RVG_RD_OPOFF) & RVG_RD_MASK;
> +		u32 rs = (insn >> RVG_RS1_OPOFF) & RVG_RS1_MASK;
> +		u32 funct3 = (insn >> RV_INSN_FUNCT3_OPOFF) & 0x7;

There are are a few other macros in asm/insn.h that can be applied, such
as RV_EXTRACT_RD_REG(), and more could be added if necessary.

> +
> +		if (rs == 0 && funct3 == 2 && csr == CSR_CYCLE) {

We could probably create a riscv_insn_is_csr_read() and RV_EXTRACT_CSR()
for this.

> +			u64 val = csr_read(CSR_TIME);
> +			/* we've got a RDCCLYE, emulated it with CSR_TIME */
> +
> +			printk_ratelimited("PID %d: process using RDCYCLE, emulating with RDTIME\n", current->pid);

If we add current->comm it may be easier to find applications that should
be converted to CSR_TIME.

> +
> +			regs_set_register(regs, rd*sizeof(unsigned long), val);

nit: spaces around the '*'

> +			regs->epc += 4;
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
>  asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *regs)
>  {
>  	bool handled;
> @@ -186,6 +215,8 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re
>  		}
>  
>  		handled = riscv_v_first_use_handler(regs, insn);
> +		if (!handle)
                     ^handled?

> +			handled = riscv_try_csr_fixup_user(regs, insn);
>  
>  		local_irq_disable();
>  
> -- 
> 2.37.2.352.g3c44437643
> 
>

Thanks,
drew

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 3/3] riscv: add trap and emulation for RDCYCLE

[Ben Dooks]

On 18/09/2024 07:45, Andrew Jones wrote:
> On Tue, Sep 17, 2024 at 02:08:53PM GMT, Ben Dooks wrote:
>> Add a trap for RDCYCLE and emulate it as RDTIME instruciton.
>>
>> This is an initial PoC and should probably be made more generic
>> way of trapping and dealing with bad instructions
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>>   arch/riscv/kernel/traps.c | 31 +++++++++++++++++++++++++++++++
>>   1 file changed, 31 insertions(+)
>>
>> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
>> index 1c3fab272fd1..51ea28ebf54d 100644
>> --- a/arch/riscv/kernel/traps.c
>> +++ b/arch/riscv/kernel/traps.c
>> @@ -167,6 +167,35 @@ DO_ERROR_INFO(do_trap_insn_misaligned,
>>   DO_ERROR_INFO(do_trap_insn_fault,
>>   	SIGSEGV, SEGV_ACCERR, "instruction access fault");
>>   
>> +#define is_system(__i) (((__i) & 0x7f) == RVG_OPCODE_SYSTEM)
> 
> We have riscv_insn_is_system()

Ah, didn't notice that.

>> +
>> +static bool riscv_try_csr_fixup_user(struct pt_regs *regs, u32 insn)
>> +{
>> +	/* expecting a 4 byte CSR instruction (*/
>> +	if (unlikely(GET_INSN_LENGTH(insn) != 4))
>> +		return false;
>> +
>> +	if (is_system(insn)) {
>> +		u32 csr = insn >> RVG_SYSTEM_CSR_OFF;
>> +		u32 rd = (insn >> RVG_RD_OPOFF) & RVG_RD_MASK;
>> +		u32 rs = (insn >> RVG_RS1_OPOFF) & RVG_RS1_MASK;
>> +		u32 funct3 = (insn >> RV_INSN_FUNCT3_OPOFF) & 0x7;
> 
> There are are a few other macros in asm/insn.h that can be applied, such
> as RV_EXTRACT_RD_REG(), and more could be added if necessary.

will have a look at that.

>> +
>> +		if (rs == 0 && funct3 == 2 && csr == CSR_CYCLE) {
> 
> We could probably create a riscv_insn_is_csr_read() and RV_EXTRACT_CSR()
> for this.

Not sure if this is totally necessary. I will check for the relevant 
read/write type from the CSR acces instructions

>> +			u64 val = csr_read(CSR_TIME);
>> +			/* we've got a RDCCLYE, emulated it with CSR_TIME */
>> +
>> +			printk_ratelimited("PID %d: process using RDCYCLE, emulating with RDTIME\n", current->pid);
> 
> If we add current->comm it may be easier to find applications that should
> be converted to CSR_TIME.

ok, will add.

>> +
>> +			regs_set_register(regs, rd*sizeof(unsigned long), val);
> 
> nit: spaces around the '*'

oops. will fix

>> +			regs->epc += 4;
>> +			return true;
>> +		}
>> +	}
>> +
>> +	return false;
>> +}
>> +
>>   asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *regs)
>>   {
>>   	bool handled;
>> @@ -186,6 +215,8 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re
>>   		}
>>   
>>   		handled = riscv_v_first_use_handler(regs, insn);
>> +		if (!handle)
>                       ^handled?
> 
>> +			handled = riscv_try_csr_fixup_user(regs, insn);
>>   
>>   		local_irq_disable();
>>   
>> -- 
>> 2.37.2.352.g3c44437643
>>
>>
> 
> Thanks,
> drew

Thanks.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 3/3] riscv: add trap and emulation for RDCYCLE

[Ben Dooks]

On 18/09/2024 07:45, Andrew Jones wrote:
> On Tue, Sep 17, 2024 at 02:08:53PM GMT, Ben Dooks wrote:
>> Add a trap for RDCYCLE and emulate it as RDTIME instruciton.
>>
>> This is an initial PoC and should probably be made more generic
>> way of trapping and dealing with bad instructions
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>>   arch/riscv/kernel/traps.c | 31 +++++++++++++++++++++++++++++++
>>   1 file changed, 31 insertions(+)
>>
>> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
>> index 1c3fab272fd1..51ea28ebf54d 100644
>> --- a/arch/riscv/kernel/traps.c
>> +++ b/arch/riscv/kernel/traps.c
>> @@ -167,6 +167,35 @@ DO_ERROR_INFO(do_trap_insn_misaligned,
>>   DO_ERROR_INFO(do_trap_insn_fault,
>>   	SIGSEGV, SEGV_ACCERR, "instruction access fault");
>>   
>> +#define is_system(__i) (((__i) & 0x7f) == RVG_OPCODE_SYSTEM)
> 
> We have riscv_insn_is_system()
> 
>> +
>> +static bool riscv_try_csr_fixup_user(struct pt_regs *regs, u32 insn)
>> +{
>> +	/* expecting a 4 byte CSR instruction (*/
>> +	if (unlikely(GET_INSN_LENGTH(insn) != 4))
>> +		return false;
>> +
>> +	if (is_system(insn)) {
>> +		u32 csr = insn >> RVG_SYSTEM_CSR_OFF;
>> +		u32 rd = (insn >> RVG_RD_OPOFF) & RVG_RD_MASK;
>> +		u32 rs = (insn >> RVG_RS1_OPOFF) & RVG_RS1_MASK;
>> +		u32 funct3 = (insn >> RV_INSN_FUNCT3_OPOFF) & 0x7;
> 
> There are are a few other macros in asm/insn.h that can be applied, such
> as RV_EXTRACT_RD_REG(), and more could be added if necessary.
> 
>> +
>> +		if (rs == 0 && funct3 == 2 && csr == CSR_CYCLE) {
> 
> We could probably create a riscv_insn_is_csr_read() and RV_EXTRACT_CSR()
> for this.
> 
>> +			u64 val = csr_read(CSR_TIME);
>> +			/* we've got a RDCCLYE, emulated it with CSR_TIME */
>> +
>> +			printk_ratelimited("PID %d: process using RDCYCLE, emulating with RDTIME\n", current->pid);
> 
> If we add current->comm it may be easier to find applications that should
> be converted to CSR_TIME.

I guess I should have also checked for the 32bit case too, but i think
those are slightly different encodings for RDCYCLE/RDCYCLEH calls.


> 
>> +
>> +			regs_set_register(regs, rd*sizeof(unsigned long), val);
> 
> nit: spaces around the '*'
> 
>> +			regs->epc += 4;
>> +			return true;
>> +		}
>> +	}
>> +
>> +	return false;
>> +}
>> +
>>   asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *regs)
>>   {
>>   	bool handled;
>> @@ -186,6 +215,8 @@ asmlinkage __visible __trap_section void do_trap_insn_illegal(struct pt_regs *re
>>   		}
>>   
>>   		handled = riscv_v_first_use_handler(regs, insn);
>> +		if (!handle)
>                       ^handled?
> 
>> +			handled = riscv_try_csr_fixup_user(regs, insn);
>>   
>>   		local_irq_disable();
>>   
>> -- 
>> 2.37.2.352.g3c44437643
>>
>>
> 
> Thanks,
> drew
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
> 


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: RFC: extern illegal instruction trap and trap RDCYCLE

[Palmer Dabbelt]

On Tue, 17 Sep 2024 06:08:50 PDT (-0700), ben.dooks@codethink.co.uk wrote:
> This is a RFC series to change how the illegal instruction trap
> is handled and then how to trap RDCYCLE and emulate it with RDTIME
> instead.

Only 1/3 made it to lore for me, not sure if it's just stuck somewhere.

>
> I did this when we found multiple libraries using RDCYCLE and
> upgrading multiple runners to newer kernels caused many problems

OK, so I think we're kind of just stuck with RDCYCLE then -- it was part 
of the base ISA when we merged the port, and every time it disappears 
we end up breaking userspace.

I'm not sure what exactly the right way to do this is: IIRC there's some 
perf-related hooks for this, but there's also systems that just don't 
implement the RDCYCLE instruction at all and thus we'll need some sort 
of emulation for those.

So hopefully the other two patches get through the lists at some point, 
but I think in general this is a reasonable thing to do -- or I guess 
maybe a completely unreasonable thing to be stuck needing to do, but no 
way around it ;)

>
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: RFC: extern illegal instruction trap and trap RDCYCLE

[Ben Dooks]

On 17/09/2024 14:46, Palmer Dabbelt wrote:
> On Tue, 17 Sep 2024 06:08:50 PDT (-0700), ben.dooks@codethink.co.uk wrote:
>> This is a RFC series to change how the illegal instruction trap
>> is handled and then how to trap RDCYCLE and emulate it with RDTIME
>> instead.
> 
> Only 1/3 made it to lore for me, not sure if it's just stuck somewhere.
> 
>>
>> I did this when we found multiple libraries using RDCYCLE and
>> upgrading multiple runners to newer kernels caused many problems
> 
> OK, so I think we're kind of just stuck with RDCYCLE then -- it was part 
> of the base ISA when we merged the port, and every time it disappears we 
> end up breaking userspace.

Yes, it was an annoyance, we managed to track down all the users and
patch out.

I thought this might be a useful idea for more generic type, so might
look at updating to have a table of instruction masks to call.

> I'm not sure what exactly the right way to do this is: IIRC there's some 
> perf-related hooks for this, but there's also systems that just don't 
> implement the RDCYCLE instruction at all and thus we'll need some sort 
> of emulation for those.

I couldn't get the PMU driver to allow it, not sure if there was an
issue higher up or some other issue?

> So hopefully the other two patches get through the lists at some point, 
> but I think in general this is a reasonable thing to do -- or I guess 
> maybe a completely unreasonable thing to be stuck needing to do, but no 
> way around it ;)

I'll check back later, currently as OSS-EU

>>
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
> 


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: RFC: extern illegal instruction trap and trap RDCYCLE

[Atish Patra]

On 9/17/24 7:01 AM, Ben Dooks wrote:
> On 17/09/2024 14:46, Palmer Dabbelt wrote:
>> On Tue, 17 Sep 2024 06:08:50 PDT (-0700), ben.dooks@codethink.co.uk 
>> wrote:
>>> This is a RFC series to change how the illegal instruction trap
>>> is handled and then how to trap RDCYCLE and emulate it with RDTIME
>>> instead.
>>
>> Only 1/3 made it to lore for me, not sure if it's just stuck somewhere.
>>
>>>
>>> I did this when we found multiple libraries using RDCYCLE and
>>> upgrading multiple runners to newer kernels caused many problems
>>
>> OK, so I think we're kind of just stuck with RDCYCLE then -- it was 
>> part of the base ISA when we merged the port, and every time it 
>> disappears we end up breaking userspace.
> 
> Yes, it was an annoyance, we managed to track down all the users and
> patch out.
> 
> I thought this might be a useful idea for more generic type, so might
> look at updating to have a table of instruction masks to call.
> 
>> I'm not sure what exactly the right way to do this is: IIRC there's 
>> some perf-related hooks for this, but there's also systems that just 
>> don't implement the RDCYCLE instruction at all and thus we'll need 
>> some sort of emulation for those.
> 
> I couldn't get the PMU driver to allow it, not sure if there was an
> issue higher up or some other issue?
> 

Here is the way to properly allow it via the driver.

https://www.kernel.org/doc/Documentation/admin-guide/sysctl/kernel.rst

Check the perf_user_access

As you noted, the user space application shouldn't use RDCYCLE for 
various reasons discussed in the past.

If RDTIME is really not an option, you can always set the legacy mode to 
enable access.

>> So hopefully the other two patches get through the lists at some 
>> point, but I think in general this is a reasonable thing to do -- or I 
>> guess maybe a completely unreasonable thing to be stuck needing to do, 
>> but no way around it ;)
> 
> I'll check back later, currently as OSS-EU
> 
>>>
>>>
>>>
>>> _______________________________________________
>>> linux-riscv mailing list
>>> linux-riscv@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
> 
> 


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: RFC: extern illegal instruction trap and trap RDCYCLE

[Ben Dooks]

On 18/09/2024 10:55, Atish Patra wrote:
> On 9/17/24 7:01 AM, Ben Dooks wrote:
>> On 17/09/2024 14:46, Palmer Dabbelt wrote:
>>> On Tue, 17 Sep 2024 06:08:50 PDT (-0700), ben.dooks@codethink.co.uk 
>>> wrote:
>>>> This is a RFC series to change how the illegal instruction trap
>>>> is handled and then how to trap RDCYCLE and emulate it with RDTIME
>>>> instead.
>>>
>>> Only 1/3 made it to lore for me, not sure if it's just stuck somewhere.
>>>
>>>>
>>>> I did this when we found multiple libraries using RDCYCLE and
>>>> upgrading multiple runners to newer kernels caused many problems
>>>
>>> OK, so I think we're kind of just stuck with RDCYCLE then -- it was 
>>> part of the base ISA when we merged the port, and every time it 
>>> disappears we end up breaking userspace.
>>
>> Yes, it was an annoyance, we managed to track down all the users and
>> patch out.
>>
>> I thought this might be a useful idea for more generic type, so might
>> look at updating to have a table of instruction masks to call.
>>
>>> I'm not sure what exactly the right way to do this is: IIRC there's 
>>> some perf-related hooks for this, but there's also systems that just 
>>> don't implement the RDCYCLE instruction at all and thus we'll need 
>>> some sort of emulation for those.
>>
>> I couldn't get the PMU driver to allow it, not sure if there was an
>> issue higher up or some other issue?
>>
> 
> Here is the way to properly allow it via the driver.
> 
> https://www.kernel.org/doc/Documentation/admin-guide/sysctl/kernel.rst
> 
> Check the perf_user_access

Didn't work for me, not sure why

> 
> As you noted, the user space application shouldn't use RDCYCLE for 
> various reasons discussed in the past.
> 
> If RDTIME is really not an option, you can always set the legacy mode to 
> enable access.
> 
>>> So hopefully the other two patches get through the lists at some 
>>> point, but I think in general this is a reasonable thing to do -- or 
>>> I guess maybe a completely unreasonable thing to be stuck needing to 
>>> do, but no way around it ;)
>>
>> I'll check back later, currently as OSS-EU
>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-riscv mailing list
>>>> linux-riscv@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>>
>>> _______________________________________________
>>> linux-riscv mailing list
>>> linux-riscv@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>>
>>
>>
> 
> 


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: RFC: extern illegal instruction trap and trap RDCYCLE

[Atish Kumar Patra]

On Wed, Sep 18, 2024 at 2:57 AM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>
> On 18/09/2024 10:55, Atish Patra wrote:
> > On 9/17/24 7:01 AM, Ben Dooks wrote:
> >> On 17/09/2024 14:46, Palmer Dabbelt wrote:
> >>> On Tue, 17 Sep 2024 06:08:50 PDT (-0700), ben.dooks@codethink.co.uk
> >>> wrote:
> >>>> This is a RFC series to change how the illegal instruction trap
> >>>> is handled and then how to trap RDCYCLE and emulate it with RDTIME
> >>>> instead.
> >>>
> >>> Only 1/3 made it to lore for me, not sure if it's just stuck somewhere.
> >>>
> >>>>
> >>>> I did this when we found multiple libraries using RDCYCLE and
> >>>> upgrading multiple runners to newer kernels caused many problems
> >>>
> >>> OK, so I think we're kind of just stuck with RDCYCLE then -- it was
> >>> part of the base ISA when we merged the port, and every time it
> >>> disappears we end up breaking userspace.
> >>
> >> Yes, it was an annoyance, we managed to track down all the users and
> >> patch out.
> >>
> >> I thought this might be a useful idea for more generic type, so might
> >> look at updating to have a table of instruction masks to call.
> >>
> >>> I'm not sure what exactly the right way to do this is: IIRC there's
> >>> some perf-related hooks for this, but there's also systems that just
> >>> don't implement the RDCYCLE instruction at all and thus we'll need
> >>> some sort of emulation for those.
> >>
> >> I couldn't get the PMU driver to allow it, not sure if there was an
> >> issue higher up or some other issue?
> >>
> >
> > Here is the way to properly allow it via the driver.
> >
> > https://www.kernel.org/doc/Documentation/admin-guide/sysctl/kernel.rst
> >
> > Check the perf_user_access
>
> Didn't work for me, not sure why
>

+Alexandre Ghiti

Hmm. That's weird. Maybe some bug crept in recently. Can you provide
some more details
on the platform, kernel version and procedure followed to change it ?

> >
> > As you noted, the user space application shouldn't use RDCYCLE for
> > various reasons discussed in the past.
> >
> > If RDTIME is really not an option, you can always set the legacy mode to
> > enable access.
> >
> >>> So hopefully the other two patches get through the lists at some
> >>> point, but I think in general this is a reasonable thing to do -- or
> >>> I guess maybe a completely unreasonable thing to be stuck needing to
> >>> do, but no way around it ;)
> >>
> >> I'll check back later, currently as OSS-EU
> >>
> >>>>
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> linux-riscv mailing list
> >>>> linux-riscv@lists.infradead.org
> >>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
> >>>
> >>> _______________________________________________
> >>> linux-riscv mailing list
> >>> linux-riscv@lists.infradead.org
> >>> http://lists.infradead.org/mailman/listinfo/linux-riscv
> >>>
> >>
> >>
> >
> >
>
>
> --
> Ben Dooks                               http://www.codethink.co.uk/
> Senior Engineer                         Codethink - Providing Genius
>
> https://www.codethink.co.uk/privacy.html

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: RFC: extern illegal instruction trap and trap RDCYCLE

[Alexandre Ghiti]

Hi Atish, Ben,

On 19/09/2024 01:23, Atish Kumar Patra wrote:
> On Wed, Sep 18, 2024 at 2:57 AM Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>> On 18/09/2024 10:55, Atish Patra wrote:
>>> On 9/17/24 7:01 AM, Ben Dooks wrote:
>>>> On 17/09/2024 14:46, Palmer Dabbelt wrote:
>>>>> On Tue, 17 Sep 2024 06:08:50 PDT (-0700), ben.dooks@codethink.co.uk
>>>>> wrote:
>>>>>> This is a RFC series to change how the illegal instruction trap
>>>>>> is handled and then how to trap RDCYCLE and emulate it with RDTIME
>>>>>> instead.
>>>>> Only 1/3 made it to lore for me, not sure if it's just stuck somewhere.
>>>>>
>>>>>> I did this when we found multiple libraries using RDCYCLE and
>>>>>> upgrading multiple runners to newer kernels caused many problems
>>>>> OK, so I think we're kind of just stuck with RDCYCLE then -- it was
>>>>> part of the base ISA when we merged the port, and every time it
>>>>> disappears we end up breaking userspace.
>>>> Yes, it was an annoyance, we managed to track down all the users and
>>>> patch out.
>>>>
>>>> I thought this might be a useful idea for more generic type, so might
>>>> look at updating to have a table of instruction masks to call.
>>>>
>>>>> I'm not sure what exactly the right way to do this is: IIRC there's
>>>>> some perf-related hooks for this, but there's also systems that just
>>>>> don't implement the RDCYCLE instruction at all and thus we'll need
>>>>> some sort of emulation for those.
>>>> I couldn't get the PMU driver to allow it, not sure if there was an
>>>> issue higher up or some other issue?
>>>>
>>> Here is the way to properly allow it via the driver.
>>>
>>> https://www.kernel.org/doc/Documentation/admin-guide/sysctl/kernel.rst
>>>
>>> Check the perf_user_access
>> Didn't work for me, not sure why
>>
> +Alexandre Ghiti
>
> Hmm. That's weird. Maybe some bug crept in recently. Can you provide
> some more details
> on the platform, kernel version and procedure followed to change it ?


So I have just tried perf_user_access on 6.11 and it works fine, meaning 
setting the "legacy" behaviour (ie 2) allows a userspace application to 
directly access RDCYCLE.

Can you provide more details Ben?

Thanks,

Alex


>
>>> As you noted, the user space application shouldn't use RDCYCLE for
>>> various reasons discussed in the past.
>>>
>>> If RDTIME is really not an option, you can always set the legacy mode to
>>> enable access.
>>>
>>>>> So hopefully the other two patches get through the lists at some
>>>>> point, but I think in general this is a reasonable thing to do -- or
>>>>> I guess maybe a completely unreasonable thing to be stuck needing to
>>>>> do, but no way around it ;)
>>>> I'll check back later, currently as OSS-EU
>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> linux-riscv mailing list
>>>>>> linux-riscv@lists.infradead.org
>>>>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>>>> _______________________________________________
>>>>> linux-riscv mailing list
>>>>> linux-riscv@lists.infradead.org
>>>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>>>>
>>>>
>>>
>>
>> --
>> Ben Dooks                               http://www.codethink.co.uk/
>> Senior Engineer                         Codethink - Providing Genius
>>
>> https://www.codethink.co.uk/privacy.html
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: RFC: extern illegal instruction trap and trap RDCYCLE

[Ben Dooks]

On 01/10/2024 10:41, Alexandre Ghiti wrote:
> Hi Atish, Ben,
> 
> On 19/09/2024 01:23, Atish Kumar Patra wrote:
>> On Wed, Sep 18, 2024 at 2:57 AM Ben Dooks <ben.dooks@codethink.co.uk> 
>> wrote:
>>> On 18/09/2024 10:55, Atish Patra wrote:
>>>> On 9/17/24 7:01 AM, Ben Dooks wrote:
>>>>> On 17/09/2024 14:46, Palmer Dabbelt wrote:
>>>>>> On Tue, 17 Sep 2024 06:08:50 PDT (-0700), ben.dooks@codethink.co.uk
>>>>>> wrote:
>>>>>>> This is a RFC series to change how the illegal instruction trap
>>>>>>> is handled and then how to trap RDCYCLE and emulate it with RDTIME
>>>>>>> instead.
>>>>>> Only 1/3 made it to lore for me, not sure if it's just stuck 
>>>>>> somewhere.
>>>>>>
>>>>>>> I did this when we found multiple libraries using RDCYCLE and
>>>>>>> upgrading multiple runners to newer kernels caused many problems
>>>>>> OK, so I think we're kind of just stuck with RDCYCLE then -- it was
>>>>>> part of the base ISA when we merged the port, and every time it
>>>>>> disappears we end up breaking userspace.
>>>>> Yes, it was an annoyance, we managed to track down all the users and
>>>>> patch out.
>>>>>
>>>>> I thought this might be a useful idea for more generic type, so might
>>>>> look at updating to have a table of instruction masks to call.
>>>>>
>>>>>> I'm not sure what exactly the right way to do this is: IIRC there's
>>>>>> some perf-related hooks for this, but there's also systems that just
>>>>>> don't implement the RDCYCLE instruction at all and thus we'll need
>>>>>> some sort of emulation for those.
>>>>> I couldn't get the PMU driver to allow it, not sure if there was an
>>>>> issue higher up or some other issue?
>>>>>
>>>> Here is the way to properly allow it via the driver.
>>>>
>>>> https://www.kernel.org/doc/Documentation/admin-guide/sysctl/kernel.rst
>>>>
>>>> Check the perf_user_access
>>> Didn't work for me, not sure why
>>>
>> +Alexandre Ghiti
>>
>> Hmm. That's weird. Maybe some bug crept in recently. Can you provide
>> some more details
>> on the platform, kernel version and procedure followed to change it ?
> 
> 
> So I have just tried perf_user_access on 6.11 and it works fine, meaning 
> setting the "legacy" behaviour (ie 2) allows a userspace application to 
> directly access RDCYCLE.
> 
> Can you provide more details Ben?

I've gone through and checked this again, I think it is very likely
that I got the enable as 1 instead of 2. It works on QEMU, I've not
got access to the runners that we upgraded and we've fixed the issue
by upgrading libabsl to a later version.

Thanks.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: RFC: extern illegal instruction trap and trap RDCYCLE

[Ben Dooks]

On 01/10/2024 22:00, Ben Dooks wrote:
> On 01/10/2024 10:41, Alexandre Ghiti wrote:
>> Hi Atish, Ben,
>>
>> On 19/09/2024 01:23, Atish Kumar Patra wrote:
>>> On Wed, Sep 18, 2024 at 2:57 AM Ben Dooks <ben.dooks@codethink.co.uk> 
>>> wrote:
>>>> On 18/09/2024 10:55, Atish Patra wrote:
>>>>> On 9/17/24 7:01 AM, Ben Dooks wrote:
>>>>>> On 17/09/2024 14:46, Palmer Dabbelt wrote:
>>>>>>> On Tue, 17 Sep 2024 06:08:50 PDT (-0700), ben.dooks@codethink.co.uk
>>>>>>> wrote:
>>>>>>>> This is a RFC series to change how the illegal instruction trap
>>>>>>>> is handled and then how to trap RDCYCLE and emulate it with RDTIME
>>>>>>>> instead.
>>>>>>> Only 1/3 made it to lore for me, not sure if it's just stuck 
>>>>>>> somewhere.
>>>>>>>
>>>>>>>> I did this when we found multiple libraries using RDCYCLE and
>>>>>>>> upgrading multiple runners to newer kernels caused many problems
>>>>>>> OK, so I think we're kind of just stuck with RDCYCLE then -- it was
>>>>>>> part of the base ISA when we merged the port, and every time it
>>>>>>> disappears we end up breaking userspace.
>>>>>> Yes, it was an annoyance, we managed to track down all the users and
>>>>>> patch out.
>>>>>>
>>>>>> I thought this might be a useful idea for more generic type, so might
>>>>>> look at updating to have a table of instruction masks to call.
>>>>>>
>>>>>>> I'm not sure what exactly the right way to do this is: IIRC there's
>>>>>>> some perf-related hooks for this, but there's also systems that just
>>>>>>> don't implement the RDCYCLE instruction at all and thus we'll need
>>>>>>> some sort of emulation for those.
>>>>>> I couldn't get the PMU driver to allow it, not sure if there was an
>>>>>> issue higher up or some other issue?
>>>>>>
>>>>> Here is the way to properly allow it via the driver.
>>>>>
>>>>> https://www.kernel.org/doc/Documentation/admin-guide/sysctl/kernel.rst
>>>>>
>>>>> Check the perf_user_access
>>>> Didn't work for me, not sure why
>>>>
>>> +Alexandre Ghiti
>>>
>>> Hmm. That's weird. Maybe some bug crept in recently. Can you provide
>>> some more details
>>> on the platform, kernel version and procedure followed to change it ?
>>
>>
>> So I have just tried perf_user_access on 6.11 and it works fine, 
>> meaning setting the "legacy" behaviour (ie 2) allows a userspace 
>> application to directly access RDCYCLE.
>>
>> Can you provide more details Ben?
> 
> I've gone through and checked this again, I think it is very likely
> that I got the enable as 1 instead of 2. It works on QEMU, I've not
> got access to the runners that we upgraded and we've fixed the issue
> by upgrading libabsl to a later version.
> 
> Thanks.

And I put some test code up at I used for testing my patch at:
https://gitlab.com/CodethinkLabs/riscv_rdcycle_test


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv