1. tech-privileged@lists.riscv.org
  2. Messages
   Date   
Re: RFC: Dedicated Clock Source and Clock Event Source for HS-mode and VS-mode

Andrew Waterman
 

Note that Section 3 of your proposal already exists.  The hypervisor spec says, "The htimedelta CSR is a read/write register that contains the delta
between the value of the time CSR and the value returned in VS-mode or VU-mode."  In other words, reading the time CSR when V=1 does what you propose.


On Tue, Aug 4, 2020 at 12:25 AM zhaosiqi (A) via lists.riscv.org <zhaosiqi3=huawei.com@...> wrote:

Hello Everyone,

 

We have come up with some ideas about improving the performance of virtual machines on the RISC-V architecture. Here’s the first piece which proposes a dedicated clock source and a clock event source for HS-mode and VS-mode, respectively. We extended the current idea of the mtime and mtimecmp CSRs and combined with the current hypervisor extension to come up with new CSRs and aliases. Evaluations on QEMU show that the proposed extension leads to performance improvement.

 

The attached document details the proposal. Any comments are welcome.

 

Regards,

Siqi
RFC: Dedicated Clock Source and Clock Event Source for HS-mode and VS-mode

Siqi Zhao
 

Hello Everyone,

 

We have come up with some ideas about improving the performance of virtual machines on the RISC-V architecture. Here’s the first piece which proposes a dedicated clock source and a clock event source for HS-mode and VS-mode, respectively. We extended the current idea of the mtime and mtimecmp CSRs and combined with the current hypervisor extension to come up with new CSRs and aliases. Evaluations on QEMU show that the proposed extension leads to performance improvement.

 

The attached document details the proposal. Any comments are welcome.

 

Regards,

Siqi

PDF
vstimer_RFC.PDF

Re: A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

alankao
 

Hi all,

I am for bit[28].  I tried to defend the idea but unfortunately my reply goes private to Anup only, and the system doesn't leave any backup.  Please wait and see my comment once Anup replies.
Re: A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

Greg Favor
 

On Mon, Aug 3, 2020 at 10:57 PM Anup Patel <Anup.Patel@...> wrote:

Hi Greg,

 

We have SBI_PMU_COUNTER_START/STOP calls where the SBI implementation will update the MCOUNTINHIBIT bits.


That reduces the argument for bit [31].  I won't remove it yet (until I write up an updated proposal), but I imagine that bit will be dropped if no one else speaks up in support of it.  (Although if/when someone (such as us) supports hardware events starting and stopping counters, then we'll have to deal with the fact that this is a change to the current arch definition of the mcountinhibit CSR.)
 

The SBI_PMU_COUNTER_START call also take parameter for initial value of counter so we don’t need HPMCOUNTER CSR to be writeable in S-mode and this will also avoid alias CSRs.

 

I think we can remove BIT[28] in your proposal.


I'm OK with this.  It is other people that have been more desirous of this feature.  I agree that it would be cleaner and simpler to not have this bit, but let's see who speaks up for keeping this feature.

Greg
 

 

Regards,

Anup

 

From: tech-privileged@... <tech-privileged@...> On Behalf Of Greg Favor
Sent: 04 August 2020 10:42
To: Anup Patel <Anup.Patel@...>
Cc: alankao <alankao@...>; tech-privileged@...
Subject: Re: [RISC-V] [tech-privileged] A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

 

Anup,

 

Thanks.  Comments below.

 

Greg

 

On Mon, Aug 3, 2020 at 9:25 PM Anup Patel <Anup.Patel@...> wrote:

Hi Greg,

 

Few comments on your proposal (https://lists.riscv.org/g/tech-privileged/message/205):

 

1. The BIT[31] is not required because we already have MCOUNTINHIBIT CSR

 

This is up in the air for inclusion or not in the proposal.  As solely a bit that software can set/clear to start/stop a counter, the argument for having this bit is weak.  Although SBI calls for writing to the mhpmevent CSR for a counter would need some way to recognize when the associated bit in mcountinhibit needs to be set or cleared.  But with this Active bit in mhpmevent itself, no special support is needed (i.e. the writing of event_info into the upper part of mhpmevent takes care of whatever all bits are there).

 

The argument for this bit in mhpmevent grows when one allows for hardware setting and clearing of the bit.  For example, in response to a cross-trigger from the debug Trigger Module, e.g. to start counting when a certain instruction executed and to stop counting when another address is executed.  Or to start/stop counting in response to another counter overflowing after N occurrences of some event.  In essence, for counting more complex types of event conditions, particularly in debug scenarios and less so in straight perf mon scenarios.

 

Currently cross-trigger capabilities like these aren't standardized but, irrespective of whether they get standardized or not, having a standard Active bit provides the framework for a design to have whatever mechanisms it desires.  And note that hardware manipulation of mcountinhibit bits would be a change to the architectural definition of mcountinhibit.  This isn't a forcing issue, but having this Active bit in mhpmevent sidesteps that issue.

 

But even with all this, it is still up in the air whether people want or don't want to standardize this separate counter control bit as part of a counter extension.  We'll see where people fall on this.

 

2. The BIT[28] contradicts CSR number semantics of HPMCOUNTER CSR because currently all HPMCOUNTER CSRs are “User-Read-Only”.

 

Good point.  To support this feature (which some others have also been requesting) will require defining an alias CSR for each hpmcounter CSR that is "User-Read-Write".

 

Having two User aliases of the same CSR is conceptually not pretty, but this is simple and seems like a necessary evil for supporting this feature.

 

Like above, we'll have to see if the interest in this feature is significant enough to warrant adding read/write hpmcounter aliases.

 

3. We need to align “event_info” definition in SBI PMU Extension to consider your prosed bits in MHPMEVENT CSRs.

 

In my mind event_info simply fills in all the higher bits of mhpmevent that are not written by event_idx - which I believe was to be the default code path in the SBI PMU code.  (This, of course, applies for future implementations that choose to organize their mhpmevent registers in this simple manner.  Implementations are free to organize their mhpmevent CSR differently and supply corresponding implementation-specific SBI code.)  In other words (for RV64):

 

mhpmevent[19:16] = event_idx.type

mhpmevent[15:  0] = event_idx.code

mhpmevent[63:20] = event_info[43:0]

 

Greg

 

 

Regards,

Anup

 
Re: A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

Anup Patel
 

Hi Greg,

 

No issues with Bit[31] of your proposed MHPMEVENT definition. The SBI_PMU_COUNTER_START/STOP calls can either update MCOUNTINHIBIT Bits or these calls can update Bit[31] of appropriate MHPMEVENT CSR.

 

Regarding Bit[28], I agree with you. Let’s wait for more comments.

 

Regards,

Anup

 

From: Greg Favor <gfavor@...>
Sent: 04 August 2020 11:40
To: Anup Patel <Anup.Patel@...>
Cc: alankao <alankao@...>; tech-privileged@...
Subject: Re: [RISC-V] [tech-privileged] A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

 

On Mon, Aug 3, 2020 at 10:57 PM Anup Patel <Anup.Patel@...> wrote:

Hi Greg,

 

We have SBI_PMU_COUNTER_START/STOP calls where the SBI implementation will update the MCOUNTINHIBIT bits.

 

That reduces the argument for bit [31].  I won't remove it yet (until I write up an updated proposal), but I imagine that bit will be dropped if no one else speaks up in support of it.  (Although if/when someone (such as us) supports hardware events starting and stopping counters, then we'll have to deal with the fact that this is a change to the current arch definition of the mcountinhibit CSR.)

 

The SBI_PMU_COUNTER_START call also take parameter for initial value of counter so we don’t need HPMCOUNTER CSR to be writeable in S-mode and this will also avoid alias CSRs.

 

I think we can remove BIT[28] in your proposal.

 

I'm OK with this.  It is other people that have been more desirous of this feature.  I agree that it would be cleaner and simpler to not have this bit, but let's see who speaks up for keeping this feature.

 

Greg

 

 

Regards,

Anup

 

From: tech-privileged@... <tech-privileged@...> On Behalf Of Greg Favor
Sent: 04 August 2020 10:42
To: Anup Patel <Anup.Patel@...>
Cc: alankao <alankao@...>; tech-privileged@...
Subject: Re: [RISC-V] [tech-privileged] A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

 

Anup,

 

Thanks.  Comments below.

 

Greg

 

On Mon, Aug 3, 2020 at 9:25 PM Anup Patel <Anup.Patel@...> wrote:

Hi Greg,

 

Few comments on your proposal (https://lists.riscv.org/g/tech-privileged/message/205):

 

1. The BIT[31] is not required because we already have MCOUNTINHIBIT CSR

 

This is up in the air for inclusion or not in the proposal.  As solely a bit that software can set/clear to start/stop a counter, the argument for having this bit is weak.  Although SBI calls for writing to the mhpmevent CSR for a counter would need some way to recognize when the associated bit in mcountinhibit needs to be set or cleared.  But with this Active bit in mhpmevent itself, no special support is needed (i.e. the writing of event_info into the upper part of mhpmevent takes care of whatever all bits are there).

 

The argument for this bit in mhpmevent grows when one allows for hardware setting and clearing of the bit.  For example, in response to a cross-trigger from the debug Trigger Module, e.g. to start counting when a certain instruction executed and to stop counting when another address is executed.  Or to start/stop counting in response to another counter overflowing after N occurrences of some event.  In essence, for counting more complex types of event conditions, particularly in debug scenarios and less so in straight perf mon scenarios.

 

Currently cross-trigger capabilities like these aren't standardized but, irrespective of whether they get standardized or not, having a standard Active bit provides the framework for a design to have whatever mechanisms it desires.  And note that hardware manipulation of mcountinhibit bits would be a change to the architectural definition of mcountinhibit.  This isn't a forcing issue, but having this Active bit in mhpmevent sidesteps that issue.

 

But even with all this, it is still up in the air whether people want or don't want to standardize this separate counter control bit as part of a counter extension.  We'll see where people fall on this.

 

2. The BIT[28] contradicts CSR number semantics of HPMCOUNTER CSR because currently all HPMCOUNTER CSRs are “User-Read-Only”.

 

Good point.  To support this feature (which some others have also been requesting) will require defining an alias CSR for each hpmcounter CSR that is "User-Read-Write".

 

Having two User aliases of the same CSR is conceptually not pretty, but this is simple and seems like a necessary evil for supporting this feature.

 

Like above, we'll have to see if the interest in this feature is significant enough to warrant adding read/write hpmcounter aliases.

 

3. We need to align “event_info” definition in SBI PMU Extension to consider your prosed bits in MHPMEVENT CSRs.

 

In my mind event_info simply fills in all the higher bits of mhpmevent that are not written by event_idx - which I believe was to be the default code path in the SBI PMU code.  (This, of course, applies for future implementations that choose to organize their mhpmevent registers in this simple manner.  Implementations are free to organize their mhpmevent CSR differently and supply corresponding implementation-specific SBI code.)  In other words (for RV64):

 

mhpmevent[19:16] = event_idx.type

mhpmevent[15:  0] = event_idx.code

mhpmevent[63:20] = event_info[43:0]

 

Greg

 

 

Regards,

Anup

 
Re: A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

Anup Patel
 

Hi Greg,

 

We have SBI_PMU_COUNTER_START/STOP calls where the SBI implementation will update the MCOUNTINHIBIT bits. The SBI_PMU_COUNTER_START call also take parameter for initial value of counter so we don’t need HPMCOUNTER CSR to be writeable in S-mode and this will also avoid alias CSRs.

 

I think we can remove BIT[28] in your proposal.

 

Regards,

Anup

 

From: tech-privileged@... <tech-privileged@...> On Behalf Of Greg Favor
Sent: 04 August 2020 10:42
To: Anup Patel <Anup.Patel@...>
Cc: alankao <alankao@...>; tech-privileged@...
Subject: Re: [RISC-V] [tech-privileged] A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

 

Anup,

 

Thanks.  Comments below.

 

Greg

 

On Mon, Aug 3, 2020 at 9:25 PM Anup Patel <Anup.Patel@...> wrote:

Hi Greg,

 

Few comments on your proposal (https://lists.riscv.org/g/tech-privileged/message/205):

 

1. The BIT[31] is not required because we already have MCOUNTINHIBIT CSR

 

This is up in the air for inclusion or not in the proposal.  As solely a bit that software can set/clear to start/stop a counter, the argument for having this bit is weak.  Although SBI calls for writing to the mhpmevent CSR for a counter would need some way to recognize when the associated bit in mcountinhibit needs to be set or cleared.  But with this Active bit in mhpmevent itself, no special support is needed (i.e. the writing of event_info into the upper part of mhpmevent takes care of whatever all bits are there).

 

The argument for this bit in mhpmevent grows when one allows for hardware setting and clearing of the bit.  For example, in response to a cross-trigger from the debug Trigger Module, e.g. to start counting when a certain instruction executed and to stop counting when another address is executed.  Or to start/stop counting in response to another counter overflowing after N occurrences of some event.  In essence, for counting more complex types of event conditions, particularly in debug scenarios and less so in straight perf mon scenarios.

 

Currently cross-trigger capabilities like these aren't standardized but, irrespective of whether they get standardized or not, having a standard Active bit provides the framework for a design to have whatever mechanisms it desires.  And note that hardware manipulation of mcountinhibit bits would be a change to the architectural definition of mcountinhibit.  This isn't a forcing issue, but having this Active bit in mhpmevent sidesteps that issue.

 

But even with all this, it is still up in the air whether people want or don't want to standardize this separate counter control bit as part of a counter extension.  We'll see where people fall on this.

 

2. The BIT[28] contradicts CSR number semantics of HPMCOUNTER CSR because currently all HPMCOUNTER CSRs are “User-Read-Only”.

 

Good point.  To support this feature (which some others have also been requesting) will require defining an alias CSR for each hpmcounter CSR that is "User-Read-Write".

 

Having two User aliases of the same CSR is conceptually not pretty, but this is simple and seems like a necessary evil for supporting this feature.

 

Like above, we'll have to see if the interest in this feature is significant enough to warrant adding read/write hpmcounter aliases.

 

3. We need to align “event_info” definition in SBI PMU Extension to consider your prosed bits in MHPMEVENT CSRs.

 

In my mind event_info simply fills in all the higher bits of mhpmevent that are not written by event_idx - which I believe was to be the default code path in the SBI PMU code.  (This, of course, applies for future implementations that choose to organize their mhpmevent registers in this simple manner.  Implementations are free to organize their mhpmevent CSR differently and supply corresponding implementation-specific SBI code.)  In other words (for RV64):

 

mhpmevent[19:16] = event_idx.type

mhpmevent[15:  0] = event_idx.code

mhpmevent[63:20] = event_info[43:0]

 

Greg

 

 

Regards,

Anup

 
Re: A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

Greg Favor
 

Anup,

Thanks.  Comments below.

Greg

On Mon, Aug 3, 2020 at 9:25 PM Anup Patel <Anup.Patel@...> wrote:

Hi Greg,

 

Few comments on your proposal (https://lists.riscv.org/g/tech-privileged/message/205):

 

1. The BIT[31] is not required because we already have MCOUNTINHIBIT CSR


This is up in the air for inclusion or not in the proposal.  As solely a bit that software can set/clear to start/stop a counter, the argument for having this bit is weak.  Although SBI calls for writing to the mhpmevent CSR for a counter would need some way to recognize when the associated bit in mcountinhibit needs to be set or cleared.  But with this Active bit in mhpmevent itself, no special support is needed (i.e. the writing of event_info into the upper part of mhpmevent takes care of whatever all bits are there).

The argument for this bit in mhpmevent grows when one allows for hardware setting and clearing of the bit.  For example, in response to a cross-trigger from the debug Trigger Module, e.g. to start counting when a certain instruction executed and to stop counting when another address is executed.  Or to start/stop counting in response to another counter overflowing after N occurrences of some event.  In essence, for counting more complex types of event conditions, particularly in debug scenarios and less so in straight perf mon scenarios.

Currently cross-trigger capabilities like these aren't standardized but, irrespective of whether they get standardized or not, having a standard Active bit provides the framework for a design to have whatever mechanisms it desires.  And note that hardware manipulation of mcountinhibit bits would be a change to the architectural definition of mcountinhibit.  This isn't a forcing issue, but having this Active bit in mhpmevent sidesteps that issue.

But even with all this, it is still up in the air whether people want or don't want to standardize this separate counter control bit as part of a counter extension.  We'll see where people fall on this.
 

2. The BIT[28] contradicts CSR number semantics of HPMCOUNTER CSR because currently all HPMCOUNTER CSRs are “User-Read-Only”.


Good point.  To support this feature (which some others have also been requesting) will require defining an alias CSR for each hpmcounter CSR that is "User-Read-Write".

Having two User aliases of the same CSR is conceptually not pretty, but this is simple and seems like a necessary evil for supporting this feature.

Like above, we'll have to see if the interest in this feature is significant enough to warrant adding read/write hpmcounter aliases.
 

3. We need to align “event_info” definition in SBI PMU Extension to consider your prosed bits in MHPMEVENT CSRs.


In my mind event_info simply fills in all the higher bits of mhpmevent that are not written by event_idx - which I believe was to be the default code path in the SBI PMU code.  (This, of course, applies for future implementations that choose to organize their mhpmevent registers in this simple manner.  Implementations are free to organize their mhpmevent CSR differently and supply corresponding implementation-specific SBI code.)  In other words (for RV64):

mhpmevent[19:16] = event_idx.type
mhpmevent[15:  0] = event_idx.code
mhpmevent[63:20] = event_info[43:0]

Greg
 

 

Regards,

Anup
Re: A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

Greg Favor
 

Email accidentally sent early.  Let me finish the email and then I'll send it again.

Greg


On Mon, Aug 3, 2020 at 9:41 PM Greg Favor via lists.riscv.org <gfavor=ventanamicro.com@...> wrote:
Anup,

Thanks.  Comments below.

Greg

On Mon, Aug 3, 2020 at 9:25 PM Anup Patel <Anup.Patel@...> wrote:

Hi Greg,

 

Few comments on your proposal (https://lists.riscv.org/g/tech-privileged/message/205):

 

1. The BIT[31] is not required because we already have MCOUNTINHIBIT CSR


This is up in the air for inclusion or not in the proposal.  As solely a bit that software can set/clear to start/stop a counter, the argument for having this bit is weak.  Although SBI calls for writing to the mhpmevent CSR for a counter would need some way to recognize when the associated bit in mcountinhibit needs to be set or cleared.  But with this bit in mhpmevent itself, no special support is needed (i.e. the writing of event_info into the upper part of mhpmevent takes care of whatever all bits are there).

The argument for this bit in mhpmevent grows when one allows for hardware setting and clearing of the bit.  For example, in response to a cross-trigger from the debug Trigger Module (e.g. to start counting when a certain instruction executed and to stop counting when another address is executed).  Or to start/stop counting in response to another counter overflowing after N occurrences of some event.  Currently cross-trigger capabilities like this aren't standardized but, irrespective of whether they get standardized or not, having a standard Active bit provides the framework for a design to have whatever mechanisms it desires. 


2. The BIT[28] contradicts CSR number semantics of HPMCOUNTER CSR because currently all HPMCOUNTER CSRs are “User-Read-Only”.

3. We need to align “event_info” definition in SBI PMU Extension to consider your prosed bits in MHPMEVENT CSRs.

 

Regards,

Anup

 

From: tech-privileged@... <tech-privileged@...> On Behalf Of Greg Favor
Sent: 30 July 2020 12:57
To: alankao <alankao@...>
Cc: tech-privileged@...
Subject: Re: [RISC-V] [tech-privileged] A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

 

Alan,

 

I'm fine with taking the lead on this architecture extension.  But it should follow a proper process as directed by the TSC.  Thus far this would mean getting a new TG created or doing something less formally under an existing TG.  But for smaller extension proposals like this there is need for a proper lighter weight and faster process.  Need for this is recognized and I suspect will probably be promulgated by the TSC some time soon.

 

So I suggest we pause for a short bit, and then see if we can follow that expedited process once it is available.  In the meantime I/we can prepare what we can in advance.  (I don't think this will represent a material slow down to getting to a frozen spec and then to ratification.)

 

Greg

 

On Wed, Jul 29, 2020 at 5:24 PM alankao <alankao@...> wrote:

Hi all,

Although there were some non-resolved discussions, it has little to do with what we should do for the next step.  I believe Greg's proposal is superior to the original one in the starting thread because

1.  It reuses `hpmevents` for most of the functions that we all agree that RISC-V needs, instead of adding a bunch of new registers.
2.  It is H-ext-aware

I suggest Greg take the lead to start a PR in the ISA Repo, I can help review and evaluate the effort to patch existing software.

Thanks,
Alan

Re: A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

Greg Favor
 

Anup,

Thanks.  Comments below.

Greg

On Mon, Aug 3, 2020 at 9:25 PM Anup Patel <Anup.Patel@...> wrote:

Hi Greg,

 

Few comments on your proposal (https://lists.riscv.org/g/tech-privileged/message/205):

 

1. The BIT[31] is not required because we already have MCOUNTINHIBIT CSR


This is up in the air for inclusion or not in the proposal.  As solely a bit that software can set/clear to start/stop a counter, the argument for having this bit is weak.  Although SBI calls for writing to the mhpmevent CSR for a counter would need some way to recognize when the associated bit in mcountinhibit needs to be set or cleared.  But with this bit in mhpmevent itself, no special support is needed (i.e. the writing of event_info into the upper part of mhpmevent takes care of whatever all bits are there).

The argument for this bit in mhpmevent grows when one allows for hardware setting and clearing of the bit.  For example, in response to a cross-trigger from the debug Trigger Module (e.g. to start counting when a certain instruction executed and to stop counting when another address is executed).  Or to start/stop counting in response to another counter overflowing after N occurrences of some event.  Currently cross-trigger capabilities like this aren't standardized but, irrespective of whether they get standardized or not, having a standard Active bit provides the framework for a design to have whatever mechanisms it desires. 


2. The BIT[28] contradicts CSR number semantics of HPMCOUNTER CSR because currently all HPMCOUNTER CSRs are “User-Read-Only”.

3. We need to align “event_info” definition in SBI PMU Extension to consider your prosed bits in MHPMEVENT CSRs.

 

Regards,

Anup

 

From: tech-privileged@... <tech-privileged@...> On Behalf Of Greg Favor
Sent: 30 July 2020 12:57
To: alankao <alankao@...>
Cc: tech-privileged@...
Subject: Re: [RISC-V] [tech-privileged] A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

 

Alan,

 

I'm fine with taking the lead on this architecture extension.  But it should follow a proper process as directed by the TSC.  Thus far this would mean getting a new TG created or doing something less formally under an existing TG.  But for smaller extension proposals like this there is need for a proper lighter weight and faster process.  Need for this is recognized and I suspect will probably be promulgated by the TSC some time soon.

 

So I suggest we pause for a short bit, and then see if we can follow that expedited process once it is available.  In the meantime I/we can prepare what we can in advance.  (I don't think this will represent a material slow down to getting to a frozen spec and then to ratification.)

 

Greg

 

On Wed, Jul 29, 2020 at 5:24 PM alankao <alankao@...> wrote:

Hi all,

Although there were some non-resolved discussions, it has little to do with what we should do for the next step.  I believe Greg's proposal is superior to the original one in the starting thread because

1.  It reuses `hpmevents` for most of the functions that we all agree that RISC-V needs, instead of adding a bunch of new registers.
2.  It is H-ext-aware

I suggest Greg take the lead to start a PR in the ISA Repo, I can help review and evaluate the effort to patch existing software.

Thanks,
Alan

Re: A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

Anup Patel
 

Hi Greg,

 

Few comments on your proposal (https://lists.riscv.org/g/tech-privileged/message/205):

 

1. The BIT[31] is not required because we already have MCOUNTINHIBIT CSR

2. The BIT[28] contradicts CSR number semantics of HPMCOUNTER CSR because currently all HPMCOUNTER CSRs are “User-Read-Only”.

3. We need to align “event_info” definition in SBI PMU Extension to consider your prosed bits in MHPMEVENT CSRs.

 

Regards,

Anup

 

From: tech-privileged@... <tech-privileged@...> On Behalf Of Greg Favor
Sent: 30 July 2020 12:57
To: alankao <alankao@...>
Cc: tech-privileged@...
Subject: Re: [RISC-V] [tech-privileged] A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

 

Alan,

 

I'm fine with taking the lead on this architecture extension.  But it should follow a proper process as directed by the TSC.  Thus far this would mean getting a new TG created or doing something less formally under an existing TG.  But for smaller extension proposals like this there is need for a proper lighter weight and faster process.  Need for this is recognized and I suspect will probably be promulgated by the TSC some time soon.

 

So I suggest we pause for a short bit, and then see if we can follow that expedited process once it is available.  In the meantime I/we can prepare what we can in advance.  (I don't think this will represent a material slow down to getting to a frozen spec and then to ratification.)

 

Greg

 

On Wed, Jul 29, 2020 at 5:24 PM alankao <alankao@...> wrote:

Hi all,

Although there were some non-resolved discussions, it has little to do with what we should do for the next step.  I believe Greg's proposal is superior to the original one in the starting thread because

1.  It reuses `hpmevents` for most of the functions that we all agree that RISC-V needs, instead of adding a bunch of new registers.
2.  It is H-ext-aware

I suggest Greg take the lead to start a PR in the ISA Repo, I can help review and evaluate the effort to patch existing software.

Thanks,
Alan

CSR address for debug scontext and hcontext

Ernie Edgar
 

Hello,

Background:

You may be aware that the RISC-V Debug Specification 0.13 defines two CSRs, mcontext and scontext, that can be used to qualify hardware breakpoints in a particular OS process or thread.  A modified S-mode OS kernel writes the process ID to scontext when switching processes.  Breakpoint hardware can be set to trigger only when the process ID in scontext matches the desired process.

Using ASID instead of scontext to qualify breakpoints has been suggested. However, many systems do not implement ASID or only implement a narrow field, forcing the OS to recycle ASID values.  This makes ASID useless for breakpoint qualification.

For those familiar with ARM, the equivalent registers in that architecture are CONTEXTIDR_EL1 and CONTEXTIDR_EL2.

Problem:

Scontext is defined in the ratified Debug Spec at CSR 0x7aa which is in the "Machine Standard read/write debug CSR" region and so is, by convention, inaccessible from S-mode.

The Debug Spec was ratified before work on the hypervisor had gotten very far, so Debug Spec 0.13 does not provide full support for hypervisor-based systems.  Among the missing items is a definition for an "hcontext" register to qualify breakpoints in a particular virtual machine.  An argument could be made to use VMID for this, but the discussion above about ASID qualification would also apply to VMID.

Proposed Solution:

The Debug Task Group would like to suggest allocating a range of CSR addresses in one of the Supervisor Standard read/write regions and in one of the Hypervisor Standard read/write regions to use for debug registers.  Our suggestion is 0x5A0-0x5AF for S-mode and 0x6A0-0x6AF for HS-mode, complementing the 0x7A0-0x7AF already defined for M-mode debug registers.  Allocating more than just one address gives the Debug TG flexibility for the future.

Thanks,
Ernie Edgar
RISC-V Debug Task Group
Re: Proposal for Custom Values in satp

Bill Huffman
 


On 8/3/20 7:01 AM, Jonathan Behrens wrote:
EXTERNAL MAIL

>For RV32, values with satp[31] clear and satp[30:0] non-zero are reserved.  I propose that values with satp[31] clear and satp[30:29]=0x3 be defined as Custom.

Are 7 bits enough , for ASID?


It depends on how you are using them. For x86-64, Linux actually only uses 4 ASID bits (out of the 12 available) because it assigns them per-core and recycles them aggressively. However, if you instead try to have globally unique ASIDs then you might need far more than 7 bits.

Jonathan

Agreed, but the proposal doesn't assume that a custom implementation will use the bits of satp in the same way the priv spec uses them.  The bits may well be used in a different fashion.

Of course, as with instruction opcodes, an implementation is free also to used reserved encodings if the implementers are willing to have a possible conflict with future standard extensions.

      Bill
Re: Proposal for Custom Values in satp

Jonathan Behrens <behrensj@...>
 

>For RV32, values with satp[31] clear and satp[30:0] non-zero are reserved.  I propose that values with satp[31] clear and satp[30:29]=0x3 be defined as Custom.

Are 7 bits enough , for ASID?


It depends on how you are using them. For x86-64, Linux actually only uses 4 ASID bits (out of the 12 available) because it assigns them per-core and recycles them aggressively. However, if you instead try to have globally unique ASIDs then you might need far more than 7 bits.

Jonathan
Re: Proposal for Custom Values in satp

Andrea Mondelli <andrea.mondelli@...>
 

>For RV32, values with satp[31] clear and satp[30:0] non-zero are reserved.  I propose that values with satp[31] clear and satp[30:29]=0x3 be defined as Custom.

Are 7 bits enough , for ASID?

 
Re: Proposal for Custom Values in satp

Bill Huffman
 

Looks appropriate to me.     Bill

On 7/31/20 4:22 PM, Andrew Waterman wrote:

EXTERNAL MAIL

I've written the idea up so it doesn't get lost, but others should still feel free to comment.  In the meantime, can you sanity-check my patch?  https://github.com/riscv/riscv-isa-manual/commit/f7710a02da497a721095a0252041122a6d0e0a6c

On Thu, Jul 30, 2020 at 10:26 AM Allen Baum <allen.baum@...> wrote:
That sounds like a no brainer good idea.

-Allen

On Jul 29, 2020, at 2:41 PM, Andrew Waterman <andrew@...> wrote:

I support this proposal.

On Wed, Jul 29, 2020 at 12:42 PM Bill Huffman <huffman@...> wrote:

The satp register has reserved values.  Some implementers will, no doubt, want to define non-standard behavior based on satp.  I would like to propose that we define some of the reserved values in satp as Custom now so that those who do so won't head in diverging directions.

For RV64, there are 11 reserved values of the Mode field.  I propose that the encodings 14 and 15 be defined as Custom.

For RV32, values with satp[31] clear and satp[30:0] non-zero are reserved.  I propose that values with satp[31] clear and satp[30:29]=0x3 be defined as Custom.

The RV32 encoding is not perfect.  To have the largest contiguous space free, we need to pick bits at the top or bottom.  Top bits encroach on what's now ASID space while bottom bits encroach on what's now PPN space.  The former seemed a bit better.

       Bill
Re: Proposal for Custom Values in satp

Andrew Waterman
 

I've written the idea up so it doesn't get lost, but others should still feel free to comment.  In the meantime, can you sanity-check my patch?  https://github.com/riscv/riscv-isa-manual/commit/f7710a02da497a721095a0252041122a6d0e0a6c


On Thu, Jul 30, 2020 at 10:26 AM Allen Baum <allen.baum@...> wrote:
That sounds like a no brainer good idea.

-Allen

On Jul 29, 2020, at 2:41 PM, Andrew Waterman <andrew@...> wrote:

I support this proposal.

On Wed, Jul 29, 2020 at 12:42 PM Bill Huffman <huffman@...> wrote:

The satp register has reserved values.  Some implementers will, no doubt, want to define non-standard behavior based on satp.  I would like to propose that we define some of the reserved values in satp as Custom now so that those who do so won't head in diverging directions.

For RV64, there are 11 reserved values of the Mode field.  I propose that the encodings 14 and 15 be defined as Custom.

For RV32, values with satp[31] clear and satp[30:0] non-zero are reserved.  I propose that values with satp[31] clear and satp[30:29]=0x3 be defined as Custom.

The RV32 encoding is not perfect.  To have the largest contiguous space free, we need to pick bits at the top or bottom.  Top bits encroach on what's now ASID space while bottom bits encroach on what's now PPN space.  The former seemed a bit better.

       Bill
CSR address for debug scontext and hcontext

Ernie Edgar
 

Hello,

Background:

You may be aware that the RISC-V Debug Specification 0.13 defines two CSRs, mcontext and scontext, that can be used to qualify hardware breakpoints in a particular OS process or thread.  A modified S-mode OS kernel writes the process ID to scontext when switching processes.  Breakpoint hardware can be set to trigger only when the process ID in scontext matches the desired process.

Using ASID instead of scontext to qualify breakpoints has been suggested. However, many systems do not implement ASID or only implement a narrow field, forcing the OS to recycle ASID values.  This makes ASID useless for breakpoint qualification.

For those familiar with ARM, the equivalent registers in that architecture are CONTEXTIDR_EL1 and CONTEXTIDR_EL2.

Problem:

Scontext is defined in the ratified Debug Spec at CSR 0x7aa which is in the "Machine Standard read/write debug CSR" region and so is, by convention, inaccessible from S-mode.

The Debug Spec was ratified before work on the hypervisor had gotten very far, so Debug Spec 0.13 does not provide full support for hypervisor-based systems.  Among the missing items is a definition for an "hcontext" register to qualify breakpoints in a particular virtual machine.  An argument could be made to use VMID for this, but the discussion above about ASID qualification would also apply to VMID.

Proposed Solution:

The Debug Task Group would like to suggest allocating a range of CSR addresses in one of the Supervisor Standard read/write regions and in one of the Hypervisor Standard read/write regions to use for debug registers.  Our suggestion is 0x5A0-0x5AF for S-mode and 0x6A0-0x6AF for HS-mode, complementing the 0x7A0-0x7AF already defined for M-mode debug registers.  Allocating more than just one address gives the Debug TG flexibility for the future.

Thanks,
Ernie Edgar
RISC-V Debug Task Group
Re: Proposal for Custom Values in satp

Allen Baum
 

That sounds like a no brainer good idea.

-Allen

On Jul 29, 2020, at 2:41 PM, Andrew Waterman <andrew@...> wrote:

I support this proposal.

On Wed, Jul 29, 2020 at 12:42 PM Bill Huffman <huffman@...> wrote:

The satp register has reserved values.  Some implementers will, no doubt, want to define non-standard behavior based on satp.  I would like to propose that we define some of the reserved values in satp as Custom now so that those who do so won't head in diverging directions.

For RV64, there are 11 reserved values of the Mode field.  I propose that the encodings 14 and 15 be defined as Custom.

For RV32, values with satp[31] clear and satp[30:0] non-zero are reserved.  I propose that values with satp[31] clear and satp[30:29]=0x3 be defined as Custom.

The RV32 encoding is not perfect.  To have the largest contiguous space free, we need to pick bits at the top or bottom.  Top bits encroach on what's now ASID space while bottom bits encroach on what's now PPN space.  The former seemed a bit better.

       Bill
Re: A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

Greg Favor
 

Alan,

I'm fine with taking the lead on this architecture extension.  But it should follow a proper process as directed by the TSC.  Thus far this would mean getting a new TG created or doing something less formally under an existing TG.  But for smaller extension proposals like this there is need for a proper lighter weight and faster process.  Need for this is recognized and I suspect will probably be promulgated by the TSC some time soon.

So I suggest we pause for a short bit, and then see if we can follow that expedited process once it is available.  In the meantime I/we can prepare what we can in advance.  (I don't think this will represent a material slow down to getting to a frozen spec and then to ratification.)

Greg


On Wed, Jul 29, 2020 at 5:24 PM alankao <alankao@...> wrote:
Hi all,

Although there were some non-resolved discussions, it has little to do with what we should do for the next step.  I believe Greg's proposal is superior to the original one in the starting thread because

1.  It reuses `hpmevents` for most of the functions that we all agree that RISC-V needs, instead of adding a bunch of new registers.
2.  It is H-ext-aware

I suggest Greg take the lead to start a PR in the ISA Repo, I can help review and evaluate the effort to patch existing software.

Thanks,
Alan

Re: A proposal to enhance RISC-V HPM (Hardware Performance Monitor)

alankao
 

Hi all,

Although there were some non-resolved discussions, it has little to do with what we should do for the next step.  I believe Greg's proposal is superior to the original one in the starting thread because

1.  It reuses `hpmevents` for most of the functions that we all agree that RISC-V needs, instead of adding a bunch of new registers.
2.  It is H-ext-aware

I suggest Greg take the lead to start a PR in the ISA Repo, I can help review and evaluate the effort to patch existing software.

Thanks,
Alan
1021 - 1040 of 1276