On Mon, Dec 20, 2021 at 3:19 PM Beeman Strong <beeman@...> wrote:

Thanks, I definitely misunderstood the intent.  So the expectation is that, in Debug Mode, reads to mtime will see time continue to progress, but reads to the time CSR will see a frozen value.  Reads of the time CSR by software running outside debug mode should not be impacted, and will see a value synchronized with mtime.

I suppose I can imagine usages where keeping the time CSR frozen has value to a debugger, but it does add complexity and latency in requiring a resync with mtime on debug mode exit.  Does the value really rise to the level of being a platform requirement?  Is there some important debug functionality that breaks if we keep it simple and let the time CSR keep running in debug mode?

On Mon, Dec 20, 2021 at 2:05 PM Andrew Waterman <andrew@...> wrote:

On Mon, Dec 20, 2021 at 3:42 PM Greg Favor <gfavor@...> wrote:

I think there's a little bit of confusion going on.  The 'stoptime' bit is defined as "Don’t increment any hart-local timers while in Debug Mode."  I take this to clearly not be referring to MTIME, but to the local time CSR.

I fully agree that expecting a debug action on a core to have to reach out to wherever in a system MTIME may be, is inappropriate.  Which also affects other still active harts - which is probably very inappropriate (i.e. debugging just one hart shouldn't inherently affect operation of all harts).

Oops, it has been a while since I've read this spec.  I withdraw my comment, if it's indeed the case that shared implementations of mtime need not be affected by stoptime.

Whereas stopping the local time CSR for the duration of being in Debug mode would be easy to implement, i.e. in_debug_mode inhibits the time CSR from advancing.  Presumably, once the hart exits Debug mode, the time CSR effectively immediately catches back up with the current time value that has been broadcast to it from MTIME.

Greg

On Mon, Dec 20, 2021 at 1:19 PM Andrew Waterman <andrew@...> wrote:

On Mon, Dec 20, 2021 at 12:11 PM Beeman Strong <beeman@...> wrote:

Hi there,

In the OS-A platform spec I see the following requirement:

• dcsr.stopcount and dcsr.stoptime must be supported and the reset value of each must be 1

◦ Rationale: The architecture has strict requirements on minstret which may be perturbed by an external debugger in a way that’s visible to software. The default should allow code that’s sensitive to these requirements to be debugged.

The rationale justifies the requirement for stopcount=1, but I don't see any rationale for stoptime=1.

The debug spec refers to stoptime=1 stopping "timers", which I interpret to mean the mtime counter.  This timer is expected to by synchronized across harts in a system ("The real-time clocks of all harts in a single user application should be synchronized to within one tick of the real-time clock.")  In a system with multiple harts, where a subset of harts may be halted at a given time, this stoptime=1 requirement risks violating this ISA requirement and confusing software by causing wall-clock time to get out of sync.

Can we remove "and dcsr.stoptime" from this platform requirement?

FWIW, although I appreciate the motivation behind this requirement, I also support removing it.  For the case that mtime is centrally implemented, this requirement is quite onerous to implement.  For the case that mtime is decentralized, this requirement is easy to satisfy, but is differently problematic, as the spec mentions ("risks violating this ISA requirement").  I dislike disadvantaging the centralized-mtime implementations for a feature we've already admitted is problematic at the ISA level.

 

thanks,

beeman