updates for instruction trapping and ptrace

updates for instruction trapping and ptrace

[Ben Dooks]

After looking at handling the RDCYCLE as RDTIME, the following
patches are probably still useful (even if the actual in-kernel
fixup of RDCYCLE isn't).



_______________________________________________
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

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

[Andrew Jones]

On Wed, Nov 13, 2024 at 09:17:01AM +0000, Ben Dooks wrote:
> 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

s/from/at/

> + * @regs:	pt_regs from which register value is gotten

s/gotten/set/

> + * @offset:	offset of the register.
> + * @to:		value to set register to

Can we name this 'value' instead of '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
>

Thanks,
drew

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

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

[Ben Dooks]

On 13/11/2024 10:12, Andrew Jones wrote:
> On Wed, Nov 13, 2024 at 09:17:01AM +0000, Ben Dooks wrote:
>> 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
> 
> s/from/at/
> 
>> + * @regs:	pt_regs from which register value is gotten
> 
> s/gotten/set/
> 
>> + * @offset:	offset of the register.
>> + * @to:		value to set register to
> 
> Can we name this 'value' instead of 'to'?

Thank you, I think these are all good comments.

-- 
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

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

[Ben Dooks]

Add the trapped instruction (insn) as the second argument to
riscv_v_first_use_handler() from 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>
---
- fixed wording of patch from rfc
---
 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

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

[Ben Dooks]

On 13/11/2024 09:17, Ben Dooks wrote:
> Add the trapped instruction (insn) as the second argument to
> riscv_v_first_use_handler() from 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>
> ---
> - fixed wording of patch from rfc
> ---
>   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 */
> +			}

grr, of course as soon as it is sent I notice the /todo/ here

Not sure if we should print something if __get_user() fails for
what /should/ be an good instruction addrss.... should probably
at-least bail out of do_trap_insn_illegal() if not also print a
warning.

> +		}
> +
> +		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;


-- 
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 2/3] riscv: traps: make insn fetch common in unknown instruction

[Ben Dooks]

On 13/11/2024 09:21, Ben Dooks wrote:
> On 13/11/2024 09:17, Ben Dooks wrote:
>> Add the trapped instruction (insn) as the second argument to
>> riscv_v_first_use_handler() from 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>
>> ---
>> - fixed wording of patch from rfc
>> ---
>>   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 */
>> +            }
> 
> grr, of course as soon as it is sent I notice the /todo/ here
> 
> Not sure if we should print something if __get_user() fails for
> what /should/ be an good instruction addrss.... should probably
> at-least bail out of do_trap_insn_illegal() if not also print a
> warning.
>

i think doing:

	if (___get_user(insn, epr)) {
		handled = false;
		insn = 0;
	}

then just checking for "if (insn)" before hand handlers.

-- 
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

[PATCH 3/3] riscv: insn: add RV_EXTRACT_FUNCT3()

[Ben Dooks]

Add extraction for the func3 field of most instructions
for use with anyone who needs it.

Note, added this for decoding of CSR accesses for
work we did looking at the RDCYCLE v RDTIME calls.

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

diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
index 09fde95a5e8f..c67f44ff2066 100644
--- a/arch/riscv/include/asm/insn.h
+++ b/arch/riscv/include/asm/insn.h
@@ -299,6 +299,10 @@ static __always_inline bool riscv_insn_is_c_jalr(u32 code)
 	({typeof(x) x_ = (x); \
 	(RV_X(x_, RVG_RD_OPOFF, RVG_RD_MASK)); })
 
+#define RV_EXTRACT_FUNCT3(x) \
+	({typeof(x) x_ = (x); \
+	(RV_X(x_, RV_INSN_FUNCT3_OPOFF, RV_INSN_FUNCT3_MASK)); })
+
 #define RV_EXTRACT_UTYPE_IMM(x) \
 	({typeof(x) x_ = (x); \
 	(RV_X(x_, RV_U_IMM_31_12_OPOFF, RV_U_IMM_31_12_MASK)); })
-- 
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: insn: add RV_EXTRACT_FUNCT3()

[Andrew Jones]

On Wed, Nov 13, 2024 at 09:17:03AM +0000, Ben Dooks wrote:
> Add extraction for the func3 field of most instructions
> for use with anyone who needs it.
> 
> Note, added this for decoding of CSR accesses for
> work we did looking at the RDCYCLE v RDTIME calls.
> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>  arch/riscv/include/asm/insn.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
> index 09fde95a5e8f..c67f44ff2066 100644
> --- a/arch/riscv/include/asm/insn.h
> +++ b/arch/riscv/include/asm/insn.h
> @@ -299,6 +299,10 @@ static __always_inline bool riscv_insn_is_c_jalr(u32 code)
>  	({typeof(x) x_ = (x); \
>  	(RV_X(x_, RVG_RD_OPOFF, RVG_RD_MASK)); })
>  
> +#define RV_EXTRACT_FUNCT3(x) \
> +	({typeof(x) x_ = (x); \
> +	(RV_X(x_, RV_INSN_FUNCT3_OPOFF, RV_INSN_FUNCT3_MASK)); })
> +
>  #define RV_EXTRACT_UTYPE_IMM(x) \
>  	({typeof(x) x_ = (x); \
>  	(RV_X(x_, RV_U_IMM_31_12_OPOFF, RV_U_IMM_31_12_MASK)); })
> -- 
> 2.37.2.352.g3c44437643
>

It doesn't hurt to have an unused macro and there's a reasonable chance
there will be a user someday, but I'd typically wait for a user before
adding stuff. Anyway,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

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

Re: [PATCH 3/3] riscv: insn: add RV_EXTRACT_FUNCT3()

[Ben Dooks]

On 13/11/2024 10:17, Andrew Jones wrote:
> On Wed, Nov 13, 2024 at 09:17:03AM +0000, Ben Dooks wrote:
>> Add extraction for the func3 field of most instructions
>> for use with anyone who needs it.
>>
>> Note, added this for decoding of CSR accesses for
>> work we did looking at the RDCYCLE v RDTIME calls.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>>   arch/riscv/include/asm/insn.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
>> index 09fde95a5e8f..c67f44ff2066 100644
>> --- a/arch/riscv/include/asm/insn.h
>> +++ b/arch/riscv/include/asm/insn.h
>> @@ -299,6 +299,10 @@ static __always_inline bool riscv_insn_is_c_jalr(u32 code)
>>   	({typeof(x) x_ = (x); \
>>   	(RV_X(x_, RVG_RD_OPOFF, RVG_RD_MASK)); })
>>   
>> +#define RV_EXTRACT_FUNCT3(x) \
>> +	({typeof(x) x_ = (x); \
>> +	(RV_X(x_, RV_INSN_FUNCT3_OPOFF, RV_INSN_FUNCT3_MASK)); })
>> +
>>   #define RV_EXTRACT_UTYPE_IMM(x) \
>>   	({typeof(x) x_ = (x); \
>>   	(RV_X(x_, RV_U_IMM_31_12_OPOFF, RV_U_IMM_31_12_MASK)); })
>> -- 
>> 2.37.2.352.g3c44437643
>>
> 
> It doesn't hurt to have an unused macro and there's a reasonable chance
> there will be a user someday, but I'd typically wait for a user before
> adding stuff. Anyway,
> 
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

I am wondering if we add a check for RDCYCLE and print a warning
even if we didn't try an fixup.

-- 
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

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 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