mbox series

[v2,0/4] RISC-V: Modularize common match conditions for trigger

Message ID 20240223022119.41255-1-alvinga@andestech.com (mailing list archive)
Headers show
Series RISC-V: Modularize common match conditions for trigger | expand

Message

Alvin Chang Feb. 23, 2024, 2:21 a.m. UTC
According to RISC-V Debug specification, the enabled privilege levels of
the trigger is common match conditions for all the types of the trigger.
This series modularize the code for checking the privilege levels of
type 2/3/6 triggers by implementing functions trigger_common_match()
and trigger_priv_match().

Additional match conditions, such as CSR tcontrol and textra, can be
further implemented into trigger_common_match() in the future.

Changes from v1:
- Fix typo
- Add commit description for changing behavior of looping the triggers
  when we check type 2 triggers.

Alvin Chang (4):
  target/riscv: Add functions for common matching conditions of trigger
  target/riscv: Apply modularized matching conditions for breakpoint
  target/riscv: Apply modularized matching conditions for watchpoint
  target/riscv: Apply modularized matching conditions for icount trigger

 target/riscv/debug.c | 124 +++++++++++++++++++++++++++++--------------
 1 file changed, 83 insertions(+), 41 deletions(-)

Comments

Alistair Francis Feb. 26, 2024, 12:25 a.m. UTC | #1
On Fri, Feb 23, 2024 at 12:22 PM Alvin Chang via <qemu-devel@nongnu.org> wrote:
>
> According to RISC-V Debug specification, the enabled privilege levels of

Can you specify what version of the debug spec?

Ideally if you can link directly to the PDF that would be very useful.
There are multiple versions so it's hard to keep track of.

Alistair

> the trigger is common match conditions for all the types of the trigger.
> This series modularize the code for checking the privilege levels of
> type 2/3/6 triggers by implementing functions trigger_common_match()
> and trigger_priv_match().
>
> Additional match conditions, such as CSR tcontrol and textra, can be
> further implemented into trigger_common_match() in the future.
>
> Changes from v1:
> - Fix typo
> - Add commit description for changing behavior of looping the triggers
>   when we check type 2 triggers.
>
> Alvin Chang (4):
>   target/riscv: Add functions for common matching conditions of trigger
>   target/riscv: Apply modularized matching conditions for breakpoint
>   target/riscv: Apply modularized matching conditions for watchpoint
>   target/riscv: Apply modularized matching conditions for icount trigger
>
>  target/riscv/debug.c | 124 +++++++++++++++++++++++++++++--------------
>  1 file changed, 83 insertions(+), 41 deletions(-)
>
> --
> 2.34.1
>
>
Alvin Chang Feb. 26, 2024, 1:16 a.m. UTC | #2
Hi Alistair,

> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Monday, February 26, 2024 8:25 AM
> To: Alvin Che-Chia Chang(張哲嘉) <alvinga@andestech.com>
> Cc: qemu-riscv@nongnu.org; qemu-devel@nongnu.org;
> alistair.francis@wdc.com; bin.meng@windriver.com; liwei1518@gmail.com;
> dbarboza@ventanamicro.com; zhiwei_liu@linux.alibaba.com
> Subject: Re: [PATCH v2 0/4] RISC-V: Modularize common match conditions for
> trigger
>
> [EXTERNAL MAIL 外部信件]
>
> On Fri, Feb 23, 2024 at 12:22 PM Alvin Chang via <qemu-devel@nongnu.org>
> wrote:
> >
> > According to RISC-V Debug specification, the enabled privilege levels
> > of
>
> Can you specify what version of the debug spec?

In general, this series does not add any new functionalities.
The original implementation has duplicated code in type 2/3/6 triggers.
I just eliminated those code and modularized them to be trigger_common_match().
Besides, we may want to check other conditions in the future, so this function can be used for those purposes.

When I track the commit history, it seems the code is submitted in the following commits two years ago:
https://github.com/qemu/qemu/commit/95799e36c15a9ab602a388491c40f6860f6ae8bf
https://github.com/qemu/qemu/commit/b5f6379d134bd201d52380c73ff73565e6a4321e
https://github.com/qemu/qemu/commit/c32461d8eeb17490b1b1e969e2ce8f1ecd83bfbb
https://github.com/qemu/qemu/commit/c472c142a7552f5b0e40378d5643a2810ef1b111

Since they mentioned the "type 6" trigger and "Sdtrig" extension, I assume current implementation is based on Debug Spec version 1.0
There is no type 6 trigger and Sdtrig extension in Debug Spec version 0.13

Sincerely,
Alvin Chang

>
> Ideally if you can link directly to the PDF that would be very useful.
> There are multiple versions so it's hard to keep track of.
>
> Alistair
>
> > the trigger is common match conditions for all the types of the trigger.
> > This series modularize the code for checking the privilege levels of
> > type 2/3/6 triggers by implementing functions trigger_common_match()
> > and trigger_priv_match().
> >
> > Additional match conditions, such as CSR tcontrol and textra, can be
> > further implemented into trigger_common_match() in the future.
> >
> > Changes from v1:
> > - Fix typo
> > - Add commit description for changing behavior of looping the triggers
> >   when we check type 2 triggers.
> >
> > Alvin Chang (4):
> >   target/riscv: Add functions for common matching conditions of trigger
> >   target/riscv: Apply modularized matching conditions for breakpoint
> >   target/riscv: Apply modularized matching conditions for watchpoint
> >   target/riscv: Apply modularized matching conditions for icount
> > trigger
> >
> >  target/riscv/debug.c | 124
> > +++++++++++++++++++++++++++++--------------
> >  1 file changed, 83 insertions(+), 41 deletions(-)
> >
> > --
> > 2.34.1
> >
> >
CONFIDENTIALITY NOTICE:

This e-mail (and its attachments) may contain confidential and legally privileged information or information protected from disclosure. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the information contained herein is strictly prohibited. In this case, please immediately notify the sender by return e-mail, delete the message (and any accompanying documents) and destroy all printed hard copies. Thank you for your cooperation.

Copyright ANDES TECHNOLOGY CORPORATION - All Rights Reserved.
Alistair Francis Feb. 26, 2024, 6:44 a.m. UTC | #3
On Mon, Feb 26, 2024 at 11:16 AM Alvin Che-Chia Chang(張哲嘉)
<alvinga@andestech.com> wrote:
>
> Hi Alistair,
>
> > -----Original Message-----
> > From: Alistair Francis <alistair23@gmail.com>
> > Sent: Monday, February 26, 2024 8:25 AM
> > To: Alvin Che-Chia Chang(張哲嘉) <alvinga@andestech.com>
> > Cc: qemu-riscv@nongnu.org; qemu-devel@nongnu.org;
> > alistair.francis@wdc.com; bin.meng@windriver.com; liwei1518@gmail.com;
> > dbarboza@ventanamicro.com; zhiwei_liu@linux.alibaba.com
> > Subject: Re: [PATCH v2 0/4] RISC-V: Modularize common match conditions for
> > trigger
> >
> > [EXTERNAL MAIL 外部信件]
> >
> > On Fri, Feb 23, 2024 at 12:22 PM Alvin Chang via <qemu-devel@nongnu.org>
> > wrote:
> > >
> > > According to RISC-V Debug specification, the enabled privilege levels
> > > of
> >
> > Can you specify what version of the debug spec?
>
> In general, this series does not add any new functionalities.
> The original implementation has duplicated code in type 2/3/6 triggers.
> I just eliminated those code and modularized them to be trigger_common_match().
> Besides, we may want to check other conditions in the future, so this function can be used for those purposes.

Ah, you are right. I just skimmed the message

>
> When I track the commit history, it seems the code is submitted in the following commits two years ago:
> https://github.com/qemu/qemu/commit/95799e36c15a9ab602a388491c40f6860f6ae8bf
> https://github.com/qemu/qemu/commit/b5f6379d134bd201d52380c73ff73565e6a4321e
> https://github.com/qemu/qemu/commit/c32461d8eeb17490b1b1e969e2ce8f1ecd83bfbb
> https://github.com/qemu/qemu/commit/c472c142a7552f5b0e40378d5643a2810ef1b111
>
> Since they mentioned the "type 6" trigger and "Sdtrig" extension, I assume current implementation is based on Debug Spec version 1.0
> There is no type 6 trigger and Sdtrig extension in Debug Spec version 0.13

Yeah, we are a weird mix-match of the two unfortunately. Which is why
I wanted to be explicit about which debug spec version you are
targeting.

>
> Sincerely,
> Alvin Chang
>
> >
> > Ideally if you can link directly to the PDF that would be very useful.
> > There are multiple versions so it's hard to keep track of.
> >
> > Alistair
> >
> > > the trigger is common match conditions for all the types of the trigger.
> > > This series modularize the code for checking the privilege levels of
> > > type 2/3/6 triggers by implementing functions trigger_common_match()
> > > and trigger_priv_match().
> > >
> > > Additional match conditions, such as CSR tcontrol and textra, can be
> > > further implemented into trigger_common_match() in the future.
> > >
> > > Changes from v1:
> > > - Fix typo
> > > - Add commit description for changing behavior of looping the triggers
> > >   when we check type 2 triggers.
> > >
> > > Alvin Chang (4):
> > >   target/riscv: Add functions for common matching conditions of trigger
> > >   target/riscv: Apply modularized matching conditions for breakpoint
> > >   target/riscv: Apply modularized matching conditions for watchpoint
> > >   target/riscv: Apply modularized matching conditions for icount
> > > trigger
> > >
> > >  target/riscv/debug.c | 124
> > > +++++++++++++++++++++++++++++--------------
> > >  1 file changed, 83 insertions(+), 41 deletions(-)
> > >
> > > --
> > > 2.34.1
> > >
> > >
> CONFIDENTIALITY NOTICE:
>
> This e-mail (and its attachments) may contain confidential and legally privileged information or information protected from disclosure. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the information contained herein is strictly prohibited. In this case, please immediately notify the sender by return e-mail, delete the message (and any accompanying documents) and destroy all printed hard copies. Thank you for your cooperation.
>
> Copyright ANDES TECHNOLOGY CORPORATION - All Rights Reserved.

I'm not sure what you want me to do here

Alistair
Alvin Chang Feb. 26, 2024, 7:21 a.m. UTC | #4
Hi Alistair,

> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Monday, February 26, 2024 2:45 PM
> To: Alvin Che-Chia Chang(張哲嘉) <alvinga@andestech.com>
> Cc: qemu-riscv@nongnu.org; qemu-devel@nongnu.org;
> alistair.francis@wdc.com; bin.meng@windriver.com; liwei1518@gmail.com;
> dbarboza@ventanamicro.com; zhiwei_liu@linux.alibaba.com
> Subject: Re: [PATCH v2 0/4] RISC-V: Modularize common match conditions for
> trigger
>
> [EXTERNAL MAIL 外部信件]
>
> On Mon, Feb 26, 2024 at 11:16 AM Alvin Che-Chia Chang(張哲嘉)
> <alvinga@andestech.com> wrote:
> >
> > Hi Alistair,
> >
> > > -----Original Message-----
> > > From: Alistair Francis <alistair23@gmail.com>
> > > Sent: Monday, February 26, 2024 8:25 AM
> > > To: Alvin Che-Chia Chang(張哲嘉) <alvinga@andestech.com>
> > > Cc: qemu-riscv@nongnu.org; qemu-devel@nongnu.org;
> > > alistair.francis@wdc.com; bin.meng@windriver.com;
> > > liwei1518@gmail.com; dbarboza@ventanamicro.com;
> > > zhiwei_liu@linux.alibaba.com
> > > Subject: Re: [PATCH v2 0/4] RISC-V: Modularize common match
> > > conditions for trigger
> > >
> > > [EXTERNAL MAIL 外部信件]
> > >
> > > On Fri, Feb 23, 2024 at 12:22 PM Alvin Chang via
> > > <qemu-devel@nongnu.org>
> > > wrote:
> > > >
> > > > According to RISC-V Debug specification, the enabled privilege
> > > > levels of
> > >
> > > Can you specify what version of the debug spec?
> >
> > In general, this series does not add any new functionalities.
> > The original implementation has duplicated code in type 2/3/6 triggers.
> > I just eliminated those code and modularized them to be
> trigger_common_match().
> > Besides, we may want to check other conditions in the future, so this function
> can be used for those purposes.
>
> Ah, you are right. I just skimmed the message
>
> >
> > When I track the commit history, it seems the code is submitted in the
> following commits two years ago:
> >
> https://github.com/qemu/qemu/commit/95799e36c15a9ab602a388491c40f68
> 60f
> > 6ae8bf
> >
> https://github.com/qemu/qemu/commit/b5f6379d134bd201d52380c73ff7356
> 5e6
> > a4321e
> >
> https://github.com/qemu/qemu/commit/c32461d8eeb17490b1b1e969e2ce8f1
> ecd
> > 83bfbb
> >
> https://github.com/qemu/qemu/commit/c472c142a7552f5b0e40378d5643a28
> 10e
> > f1b111
> >
> > Since they mentioned the "type 6" trigger and "Sdtrig" extension, I
> > assume current implementation is based on Debug Spec version 1.0 There
> > is no type 6 trigger and Sdtrig extension in Debug Spec version 0.13
>
> Yeah, we are a weird mix-match of the two unfortunately. Which is why I
> wanted to be explicit about which debug spec version you are targeting.

Done, I explicitly mention the version of Debug Spec in the commit message.
Please check patch v3.

>
> >
> > Sincerely,
> > Alvin Chang
> >
> > >
> > > Ideally if you can link directly to the PDF that would be very useful.
> > > There are multiple versions so it's hard to keep track of.
> > >
> > > Alistair
> > >
> > > > the trigger is common match conditions for all the types of the trigger.
> > > > This series modularize the code for checking the privilege levels
> > > > of type 2/3/6 triggers by implementing functions
> > > > trigger_common_match() and trigger_priv_match().
> > > >
> > > > Additional match conditions, such as CSR tcontrol and textra, can
> > > > be further implemented into trigger_common_match() in the future.
> > > >
> > > > Changes from v1:
> > > > - Fix typo
> > > > - Add commit description for changing behavior of looping the triggers
> > > >   when we check type 2 triggers.
> > > >
> > > > Alvin Chang (4):
> > > >   target/riscv: Add functions for common matching conditions of trigger
> > > >   target/riscv: Apply modularized matching conditions for breakpoint
> > > >   target/riscv: Apply modularized matching conditions for watchpoint
> > > >   target/riscv: Apply modularized matching conditions for icount
> > > > trigger
> > > >
> > > >  target/riscv/debug.c | 124
> > > > +++++++++++++++++++++++++++++--------------
> > > >  1 file changed, 83 insertions(+), 41 deletions(-)
> > > >
> > > > --
> > > > 2.34.1
> > > >
> > > >
> > CONFIDENTIALITY NOTICE:
> >
> > This e-mail (and its attachments) may contain confidential and legally
> privileged information or information protected from disclosure. If you are not
> the intended recipient, you are hereby notified that any disclosure, copying,
> distribution, or use of the information contained herein is strictly prohibited. In
> this case, please immediately notify the sender by return e-mail, delete the
> message (and any accompanying documents) and destroy all printed hard
> copies. Thank you for your cooperation.
> >
> > Copyright ANDES TECHNOLOGY CORPORATION - All Rights Reserved.
>
> I'm not sure what you want me to do here

Oh.. It automatically shows up when I send email out of our company server.
Please ignore it here.

>
> Alistair
CONFIDENTIALITY NOTICE:

This e-mail (and its attachments) may contain confidential and legally privileged information or information protected from disclosure. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the information contained herein is strictly prohibited. In this case, please immediately notify the sender by return e-mail, delete the message (and any accompanying documents) and destroy all printed hard copies. Thank you for your cooperation.

Copyright ANDES TECHNOLOGY CORPORATION - All Rights Reserved.