* [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.