All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 1/1] lib: sbi: fix dbtr shmem address handling
@ 2024-06-24 21:13 Sergey Matyukevich
  2024-06-25  1:12 ` Xiang W
  0 siblings, 1 reply; 6+ messages in thread
From: Sergey Matyukevich @ 2024-06-24 21:13 UTC (permalink / raw)
  To: opensbi

Current debug triggers implementation in OpenSBI is accompanied by the
Linux kernel PoC code available on github, e.g. see the last two branches:

- https://github.com/hschauhan/riscv-linux/blob/linux-6.8-rc5-sdtrig
- https://github.com/hschauhan/riscv-linux/tree/sdtrig-virt

All implementation versions suggest a common way to pass shared memory
address to OpenSBI on RV32 and RV64 platforms. Shmem address is split
into two 32-bit parts and passed using two registers in SBI call.

This commit fixes shmem address handling for both RV32 and RV64. For
RV64 macro DBTR_SHMEM_MAKE_PHYS is updated to take into account both
high and low 32-bit parts of the passed shmem address. On RV32, M-mode
can only access the first 4GB of the physical address space. Macro
DBTR_SHMEM_MAKE_PHYS can be kept the same, though sbi_dbtr_setup_shmem
call should fail if the upper 32-bit part of the physical address is
not zero.

Suggested fix is for the kernel linux-6.8-rc5-sdtrig branch with the
modified sbi call in arch_smp_setup_sbi_shmem function:

simple snippet from the previous sdtrig-virt branch

:	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
:			MEM_LO(shmem_pa), MEM_HI(shmem_pa), 0, 0, 0, 0);

instead of later version from linux-6.8-rc5-sdtrig

:	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
:			(!MEM_LO(shmem_pa) ? 0xFFFFFFFFUL : MEM_LO(shmem_pa)),
:			(!MEM_HI(shmem_pa) ? 0xFFFFFFFFUL : MEM_HI(shmem_pa)),
:			 0, 0, 0, 0);

Fixes: 7f54527029d0 ("lib: sbi: fix DBTR_SHMEM_MAKE_PHYS for RV64")
Fixes: 97f234f15c96 ("lib: sbi: Introduce the SBI debug triggers extension support")

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 lib/sbi/sbi_dbtr.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/sbi/sbi_dbtr.c b/lib/sbi/sbi_dbtr.c
index 9c92af6..8aa835f 100644
--- a/lib/sbi/sbi_dbtr.c
+++ b/lib/sbi/sbi_dbtr.c
@@ -47,13 +47,7 @@ static unsigned long hart_state_ptr_offset;
 	     _idx < _max;						\
 	     _idx++, _entry = ((_etype *)_base + _idx))
 
-#if __riscv_xlen == 64
-#define DBTR_SHMEM_MAKE_PHYS(_p_hi, _p_lo) (_p_lo)
-#elif __riscv_xlen == 32
 #define DBTR_SHMEM_MAKE_PHYS(_p_hi, _p_lo) (((u64)(_p_hi) << 32) | (_p_lo))
-#else
-#error "Undefined XLEN"
-#endif
 
 /* must call with hs != NULL */
 static inline bool sbi_dbtr_shmem_disabled(
@@ -277,6 +271,12 @@ int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
 	if (shmem_phys_lo & SBI_DBTR_SHMEM_ALIGN_MASK)
 		return SBI_ERR_INVALID_PARAM;
 
+#if __riscv_xlen == 32
+	 /* M-mode on RV32 can only access the first 4GB */
+	if (shmem_phys_hi)
+		return SBI_EINVALID_ADDR;
+#endif
+
 	if (dom && !sbi_domain_check_addr(dom,
 		  DBTR_SHMEM_MAKE_PHYS(shmem_phys_hi, shmem_phys_lo), smode,
 		  SBI_DOMAIN_READ | SBI_DOMAIN_WRITE))
-- 
2.45.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [RFC PATCH 1/1] lib: sbi: fix dbtr shmem address handling
  2024-06-24 21:13 [RFC PATCH 1/1] lib: sbi: fix dbtr shmem address handling Sergey Matyukevich
@ 2024-06-25  1:12 ` Xiang W
  2024-06-25 20:26   ` Sergey Matyukevich
  0 siblings, 1 reply; 6+ messages in thread
From: Xiang W @ 2024-06-25  1:12 UTC (permalink / raw)
  To: opensbi

? 2024-06-25???? 00:13 +0300?Sergey Matyukevich???
> Current debug triggers implementation in OpenSBI is accompanied by the
> Linux kernel PoC code available on github, e.g. see the last two branches:
> 
> - https://github.com/hschauhan/riscv-linux/blob/linux-6.8-rc5-sdtrig
> - https://github.com/hschauhan/riscv-linux/tree/sdtrig-virt
> 
> All implementation versions suggest a common way to pass shared memory
> address to OpenSBI on RV32 and RV64 platforms. Shmem address is split
> into two 32-bit parts and passed using two registers in SBI call.
https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-debug-console.adoc?plain=1#L31C35-L33C53

The shmem address is not split into high 32 bits and low 32 bits, but high
XLEN bits and low XLEN bits

Regards,
Xiang W
> 
> This commit fixes shmem address handling for both RV32 and RV64. For
> RV64 macro DBTR_SHMEM_MAKE_PHYS is updated to take into account both
> high and low 32-bit parts of the passed shmem address. On RV32, M-mode
> can only access the first 4GB of the physical address space. Macro
> DBTR_SHMEM_MAKE_PHYS can be kept the same, though sbi_dbtr_setup_shmem
> call should fail if the upper 32-bit part of the physical address is
> not zero.
> 
> Suggested fix is for the kernel linux-6.8-rc5-sdtrig branch with the
> modified sbi call in arch_smp_setup_sbi_shmem function:
> 
> simple snippet from the previous sdtrig-virt branch
> 
> :	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
> :			MEM_LO(shmem_pa), MEM_HI(shmem_pa), 0, 0, 0, 0);
> 
> instead of later version from linux-6.8-rc5-sdtrig
> 
> :	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
> :			(!MEM_LO(shmem_pa) ? 0xFFFFFFFFUL : MEM_LO(shmem_pa)),
> :			(!MEM_HI(shmem_pa) ? 0xFFFFFFFFUL : MEM_HI(shmem_pa)),
> :			 0, 0, 0, 0);
> 
> Fixes: 7f54527029d0 ("lib: sbi: fix DBTR_SHMEM_MAKE_PHYS for RV64")
> Fixes: 97f234f15c96 ("lib: sbi: Introduce the SBI debug triggers extension support")
> 
> Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
> ---
> ?lib/sbi/sbi_dbtr.c | 12 ++++++------
> ?1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/sbi/sbi_dbtr.c b/lib/sbi/sbi_dbtr.c
> index 9c92af6..8aa835f 100644
> --- a/lib/sbi/sbi_dbtr.c
> +++ b/lib/sbi/sbi_dbtr.c
> @@ -47,13 +47,7 @@ static unsigned long hart_state_ptr_offset;
> ?	???? _idx < _max;						\
> ?	???? _idx++, _entry = ((_etype *)_base + _idx))
> ?
> -#if __riscv_xlen == 64
> -#define DBTR_SHMEM_MAKE_PHYS(_p_hi, _p_lo) (_p_lo)
> -#elif __riscv_xlen == 32
> ?#define DBTR_SHMEM_MAKE_PHYS(_p_hi, _p_lo) (((u64)(_p_hi) << 32) | (_p_lo))
> -#else
> -#error "Undefined XLEN"
> -#endif
> ?
> ?/* must call with hs != NULL */
> ?static inline bool sbi_dbtr_shmem_disabled(
> @@ -277,6 +271,12 @@ int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
> ?	if (shmem_phys_lo & SBI_DBTR_SHMEM_ALIGN_MASK)
> ?		return SBI_ERR_INVALID_PARAM;
> ?
> +#if __riscv_xlen == 32
> +	 /* M-mode on RV32 can only access the first 4GB */
> +	if (shmem_phys_hi)
> +		return SBI_EINVALID_ADDR;
> +#endif
> +
> ?	if (dom && !sbi_domain_check_addr(dom,
> ?		? DBTR_SHMEM_MAKE_PHYS(shmem_phys_hi, shmem_phys_lo), smode,
> ?		? SBI_DOMAIN_READ | SBI_DOMAIN_WRITE))
> -- 
> 2.45.2
> 
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH 1/1] lib: sbi: fix dbtr shmem address handling
  2024-06-25  1:12 ` Xiang W
@ 2024-06-25 20:26   ` Sergey Matyukevich
  2024-06-26  3:11     ` Xiang W
                       ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Sergey Matyukevich @ 2024-06-25 20:26 UTC (permalink / raw)
  To: opensbi

On Tue, Jun 25, 2024 at 09:12:05AM +0800, Xiang W wrote:
> ? 2024-06-25???? 00:13 +0300?Sergey Matyukevich???
> > Current debug triggers implementation in OpenSBI is accompanied by the
> > Linux kernel PoC code available on github, e.g. see the last two branches:
> > 
> > - https://github.com/hschauhan/riscv-linux/blob/linux-6.8-rc5-sdtrig
> > - https://github.com/hschauhan/riscv-linux/tree/sdtrig-virt
> > 
> > All implementation versions suggest a common way to pass shared memory
> > address to OpenSBI on RV32 and RV64 platforms. Shmem address is split
> > into two 32-bit parts and passed using two registers in SBI call.
> https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-debug-console.adoc?plain=1#L31C35-L33C53
> 
> The shmem address is not split into high 32 bits and low 32 bits, but high
> XLEN bits and low XLEN bits

This RFC is for DBTR (debug trigger) and not for DBCN (debug console).
In this case Linux kernel PoC implementation performs split, see:

- https://github.com/hschauhan/riscv-linux/blob/linux-6.8-rc5-sdtrig/arch/riscv/kernel/hw_breakpoint.c#L30-L58

But your remark regarding XLEN is applicable to DBTR case as well. Debug
Trigger Extension is not yet in SBI specification. Its proposal is
posted in riscv tech-debug mailing list:

- https://lists.riscv.org/g/tech-debug/message/1302

This proposal suggests to handle shmem address in sbi_debug_setup_shmem
in a similar way as addresses in DBCN read/write. So current Linux
kernel DBTR PoC code needs to be fixed along the following lines:

:	if (IS_ENABLED(CONFIG_32BIT))
:		ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
:				lower_32_bits(base_addr), upper_32_bits(base_addr);
:				0, 0, 0, 0);
:	else
:		ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
:				base_addr, 0, 0, 0, 0, 0);

Meanwhile OpenSBI fix is still needed to do the following two things:
- to check that upper address word is zero (makes sense for both RV32/RV64)
- to drop upper address word from DBTR_SHMEM_MAKE_PHYS (for RV32)

The updated RFC patch is as follows:

diff --git a/lib/sbi/sbi_dbtr.c b/lib/sbi/sbi_dbtr.c
index 9c92af6..9aab015 100644
--- a/lib/sbi/sbi_dbtr.c
+++ b/lib/sbi/sbi_dbtr.c
@@ -47,13 +47,7 @@ static unsigned long hart_state_ptr_offset;
 	     _idx < _max;						\
 	     _idx++, _entry = ((_etype *)_base + _idx))
 
-#if __riscv_xlen == 64
 #define DBTR_SHMEM_MAKE_PHYS(_p_hi, _p_lo) (_p_lo)
-#elif __riscv_xlen == 32
-#define DBTR_SHMEM_MAKE_PHYS(_p_hi, _p_lo) (((u64)(_p_hi) << 32) | (_p_lo))
-#else
-#error "Undefined XLEN"
-#endif
 
 /* must call with hs != NULL */
 static inline bool sbi_dbtr_shmem_disabled(
@@ -277,6 +271,9 @@ int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
 	if (shmem_phys_lo & SBI_DBTR_SHMEM_ALIGN_MASK)
 		return SBI_ERR_INVALID_PARAM;
 
+	if (shmem_phys_hi)
+		return SBI_EINVALID_ADDR;
+
 	if (dom && !sbi_domain_check_addr(dom,
 		  DBTR_SHMEM_MAKE_PHYS(shmem_phys_hi, shmem_phys_lo), smode,
 		  SBI_DOMAIN_READ | SBI_DOMAIN_WRITE))

Regards,
Sergey


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [RFC PATCH 1/1] lib: sbi: fix dbtr shmem address handling
  2024-06-25 20:26   ` Sergey Matyukevich
@ 2024-06-26  3:11     ` Xiang W
  2024-06-26 13:33     ` Himanshu Chauhan
  2024-06-26 13:46     ` Himanshu Chauhan
  2 siblings, 0 replies; 6+ messages in thread
From: Xiang W @ 2024-06-26  3:11 UTC (permalink / raw)
  To: opensbi

? 2024-06-25???? 23:26 +0300?Sergey Matyukevich???
> On Tue, Jun 25, 2024 at 09:12:05AM +0800, Xiang W wrote:
> > ? 2024-06-25???? 00:13 +0300?Sergey Matyukevich???
> > > Current debug triggers implementation in OpenSBI is accompanied by the
> > > Linux kernel PoC code available on github, e.g. see the last two branches:
> > > 
> > > - https://github.com/hschauhan/riscv-linux/blob/linux-6.8-rc5-sdtrig
> > > - https://github.com/hschauhan/riscv-linux/tree/sdtrig-virt
> > > 
> > > All implementation versions suggest a common way to pass shared memory
> > > address to OpenSBI on RV32 and RV64 platforms. Shmem address is split
> > > into two 32-bit parts and passed using two registers in SBI call.
> > https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-debug-console.adoc?plain=1#L31C35-L33C53
> > 
> > The shmem address is not split into high 32 bits and low 32 bits, but high
> > XLEN bits and low XLEN bits
> 
> This RFC is for DBTR (debug trigger) and not for DBCN (debug console).
> In this case Linux kernel PoC implementation performs split, see:
> 
> - https://github.com/hschauhan/riscv-linux/blob/linux-6.8-rc5-sdtrig/arch/riscv/kernel/hw_breakpoint.c#L30-L58
> 
> But your remark regarding XLEN is applicable to DBTR case as well. Debug
> Trigger Extension is not yet in SBI specification. Its proposal is
> posted in riscv tech-debug mailing list:
> 
> - https://lists.riscv.org/g/tech-debug/message/1302
> 
> This proposal suggests to handle shmem address in sbi_debug_setup_shmem
> in a similar way as addresses in DBCN read/write. So current Linux
> kernel DBTR PoC code needs to be fixed along the following lines:
> 
> :	if (IS_ENABLED(CONFIG_32BIT))
> :		ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
> :				lower_32_bits(base_addr), upper_32_bits(base_addr);
> :				0, 0, 0, 0);
> :	else
> :		ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
> :				base_addr, 0, 0, 0, 0, 0);
> 
> Meanwhile OpenSBI fix is still needed to do the following two things:
> - to check that upper address word is zero (makes sense for both RV32/RV64)
> - to drop upper address word from DBTR_SHMEM_MAKE_PHYS (for RV32)
> 
> The updated RFC patch is as follows:
> 
> diff --git a/lib/sbi/sbi_dbtr.c b/lib/sbi/sbi_dbtr.c
> index 9c92af6..9aab015 100644
> --- a/lib/sbi/sbi_dbtr.c
> +++ b/lib/sbi/sbi_dbtr.c
> @@ -47,13 +47,7 @@ static unsigned long hart_state_ptr_offset;
> ?	???? _idx < _max;						\
> ?	???? _idx++, _entry = ((_etype *)_base + _idx))
> ?
> -#if __riscv_xlen == 64
> ?#define DBTR_SHMEM_MAKE_PHYS(_p_hi, _p_lo) (_p_lo)
> -#elif __riscv_xlen == 32
> -#define DBTR_SHMEM_MAKE_PHYS(_p_hi, _p_lo) (((u64)(_p_hi) << 32) | (_p_lo))
> -#else
> -#error "Undefined XLEN"
> -#endif
> ?
> ?/* must call with hs != NULL */
> ?static inline bool sbi_dbtr_shmem_disabled(
> @@ -277,6 +271,9 @@ int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
> ?	if (shmem_phys_lo & SBI_DBTR_SHMEM_ALIGN_MASK)
> ?		return SBI_ERR_INVALID_PARAM;
> ?
> +	if (shmem_phys_hi)
> +		return SBI_EINVALID_ADDR;
> +
> ?	if (dom && !sbi_domain_check_addr(dom,
> ?		? DBTR_SHMEM_MAKE_PHYS(shmem_phys_hi, shmem_phys_lo), smode,
> ?		? SBI_DOMAIN_READ | SBI_DOMAIN_WRITE))
> 
> Regards,
> Sergey
New patch look good to me.

Regards,
Xiang W



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH 1/1] lib: sbi: fix dbtr shmem address handling
  2024-06-25 20:26   ` Sergey Matyukevich
  2024-06-26  3:11     ` Xiang W
@ 2024-06-26 13:33     ` Himanshu Chauhan
  2024-06-26 13:46     ` Himanshu Chauhan
  2 siblings, 0 replies; 6+ messages in thread
From: Himanshu Chauhan @ 2024-06-26 13:33 UTC (permalink / raw)
  To: opensbi

On Tue, Jun 25, 2024 at 11:26:38PM +0300, Sergey Matyukevich wrote:
> On Tue, Jun 25, 2024 at 09:12:05AM +0800, Xiang W wrote:
> > ? 2024-06-25???? 00:13 +0300?Sergey Matyukevich???
> > > Current debug triggers implementation in OpenSBI is accompanied by the
> > > Linux kernel PoC code available on github, e.g. see the last two branches:
> > > 
> > > - https://github.com/hschauhan/riscv-linux/blob/linux-6.8-rc5-sdtrig
> > > - https://github.com/hschauhan/riscv-linux/tree/sdtrig-virt
> > > 
> > > All implementation versions suggest a common way to pass shared memory
> > > address to OpenSBI on RV32 and RV64 platforms. Shmem address is split
> > > into two 32-bit parts and passed using two registers in SBI call.
> > https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-debug-console.adoc?plain=1#L31C35-L33C53
> > 
> > The shmem address is not split into high 32 bits and low 32 bits, but high
> > XLEN bits and low XLEN bits
> 
> This RFC is for DBTR (debug trigger) and not for DBCN (debug console).
> In this case Linux kernel PoC implementation performs split, see:
> 
> - https://github.com/hschauhan/riscv-linux/blob/linux-6.8-rc5-sdtrig/arch/riscv/kernel/hw_breakpoint.c#L30-L58
> 
> But your remark regarding XLEN is applicable to DBTR case as well. Debug
> Trigger Extension is not yet in SBI specification. Its proposal is
> posted in riscv tech-debug mailing list:
> 
> - https://lists.riscv.org/g/tech-debug/message/1302
> 
> This proposal suggests to handle shmem address in sbi_debug_setup_shmem
> in a similar way as addresses in DBCN read/write. So current Linux
> kernel DBTR PoC code needs to be fixed along the following lines:
> 
> :	if (IS_ENABLED(CONFIG_32BIT))
> :		ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
> :				lower_32_bits(base_addr), upper_32_bits(base_addr);
> :				0, 0, 0, 0);
> :	else
> :		ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
> :				base_addr, 0, 0, 0, 0, 0);
>

I will take care of this in kernel. Thanks for pointing out.

> Meanwhile OpenSBI fix is still needed to do the following two things:
> - to check that upper address word is zero (makes sense for both RV32/RV64)
> - to drop upper address word from DBTR_SHMEM_MAKE_PHYS (for RV32)
> 
> The updated RFC patch is as follows:
> 
> diff --git a/lib/sbi/sbi_dbtr.c b/lib/sbi/sbi_dbtr.c
> index 9c92af6..9aab015 100644
> --- a/lib/sbi/sbi_dbtr.c
> +++ b/lib/sbi/sbi_dbtr.c
> @@ -47,13 +47,7 @@ static unsigned long hart_state_ptr_offset;
>  	     _idx < _max;						\
>  	     _idx++, _entry = ((_etype *)_base + _idx))
>  
> -#if __riscv_xlen == 64
>  #define DBTR_SHMEM_MAKE_PHYS(_p_hi, _p_lo) (_p_lo)
> -#elif __riscv_xlen == 32
> -#define DBTR_SHMEM_MAKE_PHYS(_p_hi, _p_lo) (((u64)(_p_hi) << 32) | (_p_lo))
> -#else
> -#error "Undefined XLEN"
> -#endif
>  
>  /* must call with hs != NULL */
>  static inline bool sbi_dbtr_shmem_disabled(
> @@ -277,6 +271,9 @@ int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
>  	if (shmem_phys_lo & SBI_DBTR_SHMEM_ALIGN_MASK)
>  		return SBI_ERR_INVALID_PARAM;
>  
> +	if (shmem_phys_hi)
> +		return SBI_EINVALID_ADDR;
> +
>  	if (dom && !sbi_domain_check_addr(dom,
>  		  DBTR_SHMEM_MAKE_PHYS(shmem_phys_hi, shmem_phys_lo), smode,
>  		  SBI_DOMAIN_READ | SBI_DOMAIN_WRITE))
> 
> Regards,
> Sergey

Looks good to me!

Thanks & Regards
Himanshu


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH 1/1] lib: sbi: fix dbtr shmem address handling
  2024-06-25 20:26   ` Sergey Matyukevich
  2024-06-26  3:11     ` Xiang W
  2024-06-26 13:33     ` Himanshu Chauhan
@ 2024-06-26 13:46     ` Himanshu Chauhan
  2 siblings, 0 replies; 6+ messages in thread
From: Himanshu Chauhan @ 2024-06-26 13:46 UTC (permalink / raw)
  To: opensbi

On Tue, Jun 25, 2024 at 11:26:38PM +0300, Sergey Matyukevich wrote:
> On Tue, Jun 25, 2024 at 09:12:05AM +0800, Xiang W wrote:
> > ? 2024-06-25???? 00:13 +0300?Sergey Matyukevich???
> > > Current debug triggers implementation in OpenSBI is accompanied by the
> > > Linux kernel PoC code available on github, e.g. see the last two branches:
> > > 
> > > - https://github.com/hschauhan/riscv-linux/blob/linux-6.8-rc5-sdtrig
> > > - https://github.com/hschauhan/riscv-linux/tree/sdtrig-virt
> > > 
> > > All implementation versions suggest a common way to pass shared memory
> > > address to OpenSBI on RV32 and RV64 platforms. Shmem address is split
> > > into two 32-bit parts and passed using two registers in SBI call.
> > https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-debug-console.adoc?plain=1#L31C35-L33C53
> > 
> > The shmem address is not split into high 32 bits and low 32 bits, but high
> > XLEN bits and low XLEN bits
> 
> This RFC is for DBTR (debug trigger) and not for DBCN (debug console).
> In this case Linux kernel PoC implementation performs split, see:
> 
> - https://github.com/hschauhan/riscv-linux/blob/linux-6.8-rc5-sdtrig/arch/riscv/kernel/hw_breakpoint.c#L30-L58
> 
> But your remark regarding XLEN is applicable to DBTR case as well. Debug
> Trigger Extension is not yet in SBI specification. Its proposal is
> posted in riscv tech-debug mailing list:
> 
> - https://lists.riscv.org/g/tech-debug/message/1302
> 
> This proposal suggests to handle shmem address in sbi_debug_setup_shmem
> in a similar way as addresses in DBCN read/write. So current Linux
> kernel DBTR PoC code needs to be fixed along the following lines:
> 
> :	if (IS_ENABLED(CONFIG_32BIT))
> :		ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
> :				lower_32_bits(base_addr), upper_32_bits(base_addr);
> :				0, 0, 0, 0);
> :	else
> :		ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
> :				base_addr, 0, 0, 0, 0, 0);
> 
> Meanwhile OpenSBI fix is still needed to do the following two things:
> - to check that upper address word is zero (makes sense for both RV32/RV64)
> - to drop upper address word from DBTR_SHMEM_MAKE_PHYS (for RV32)
> 
> The updated RFC patch is as follows:
> 
> diff --git a/lib/sbi/sbi_dbtr.c b/lib/sbi/sbi_dbtr.c
> index 9c92af6..9aab015 100644
> --- a/lib/sbi/sbi_dbtr.c
> +++ b/lib/sbi/sbi_dbtr.c
> @@ -47,13 +47,7 @@ static unsigned long hart_state_ptr_offset;
>  	     _idx < _max;						\
>  	     _idx++, _entry = ((_etype *)_base + _idx))
>  
> -#if __riscv_xlen == 64
>  #define DBTR_SHMEM_MAKE_PHYS(_p_hi, _p_lo) (_p_lo)
> -#elif __riscv_xlen == 32
> -#define DBTR_SHMEM_MAKE_PHYS(_p_hi, _p_lo) (((u64)(_p_hi) << 32) | (_p_lo))
> -#else
> -#error "Undefined XLEN"
> -#endif
>  
>  /* must call with hs != NULL */
>  static inline bool sbi_dbtr_shmem_disabled(
> @@ -277,6 +271,9 @@ int sbi_dbtr_setup_shmem(const struct sbi_domain *dom, unsigned long smode,
>  	if (shmem_phys_lo & SBI_DBTR_SHMEM_ALIGN_MASK)
>  		return SBI_ERR_INVALID_PARAM;
>  
> +	if (shmem_phys_hi)
> +		return SBI_EINVALID_ADDR;
> +
>  	if (dom && !sbi_domain_check_addr(dom,
>  		  DBTR_SHMEM_MAKE_PHYS(shmem_phys_hi, shmem_phys_lo), smode,
>  		  SBI_DOMAIN_READ | SBI_DOMAIN_WRITE))
> 
> Regards,
> Sergey

Forgot to mention reviewed by tag in previous email. Sorry.

Reviewed-by: Himanshu Chauhan <hchauhan@ventanamicro.com>

Regards
Himanshu


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-06-26 13:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-24 21:13 [RFC PATCH 1/1] lib: sbi: fix dbtr shmem address handling Sergey Matyukevich
2024-06-25  1:12 ` Xiang W
2024-06-25 20:26   ` Sergey Matyukevich
2024-06-26  3:11     ` Xiang W
2024-06-26 13:33     ` Himanshu Chauhan
2024-06-26 13:46     ` Himanshu Chauhan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.