Re: Re: [PATCH 0/2] Export debug triggers as an extension

[Andrew Jones]

On Mon, Jan 22, 2024 at 03:42:10PM +1000, Alistair Francis wrote:
> > > From memory the "debug" property is for the original debug spec:
> > > https://github.com/riscv/riscv-debug-spec/releases/tag/task_group_vote
> > >
> > > That was ratified and is an official extension. AFAIK this is what is
> > > in physical hardware as well.
> > >
> > > The actual PDF says draft though, I'm not sure what's going on there.
> > >
> > > The debug spec doesn't have a Z* name, so it's just "debug", at least
> > > AFAIK.
> > >
> > > "sdtrig" seems to be a new backwards-incompatible extension doing
> > > basically the same thing. What a mess
...
> >
> > I've done a bit of digging and I agree things are quite messy. Here are
> > my discoveries:
> >
> > The debug option and the code for triggers was added in these commits:
> >
> > c9711bd778 target/riscv: cpu: Enable native debug feature
> > 38b4e781a4 target/riscv: machine: Add debug state description
> > b6092544fc target/riscv: csr: Hook debug CSR read/write
> > 1acdb3b013 target/riscv: cpu: Add a config option for native debug
> > 95799e36c1 target/riscv: Add initial support for the Sdtrig extension
> >
> > In March 2022 - since the commit refers to the Sdtrig extension name
> > and from the date this was an implementation not of the ratified 0.13
> > debug spec (which did not have Sdtrig as a separate extension) but
> > rather a version of the in development 1.0 debug spec.
> 
> Yeah... We used the "stable" from master. That is our mistake there.
> 
> I'm pretty sure we targeted the 0.13. The "Sdtrig" was only added in
> the v4 as the changelog says: "mention Sdtrig extension in the commit"
> 
> >
> > It's not trivial to tell if it's closer to the ratified 0.13 version or
> > the (hopefully soon to be frozen) 1.0 version.
> >
> > As the only part of the debug specification to be implemented is the
> > triggers then effectively the debug option is x-sdtrig.
> >
> > I don't think there is any way for code running on the machine to
> > identify what version of the debug is implemented - the appropriate
> > register is only available for external debug. Once 1.0 is frozen then
> > the presence of Sdtrig isa string would indicate 1.0 trigger support is
> > available.
> >
> > According to JIRA - https://jira.riscv.org/browse/RVS-981 the debug
> > specification should freeze this month.
> >
> > How about considering this as a solution:
> >
> > - Add a new x-sdtrig option that defaults to false
> > - Deprecate debug option - but retain it with default on
> 
> We can't deprecate a ratified spec. The 0.13 just seems to call it
> "debug" so that's what we are stuck with
> 
> > - Add warning if triggers are used and x-sdtrig is not enabled
> > - Update the trigger implementation to match frozen spec
> 
> We will need to support two versions, as there are two ratified specs.
>

We'll likely want to be allowed to deprecate ratified extensions as riscv
evolves. Despite best intentions, extensions may be designed and ratified
which ultimately fail to be of much utility, and new extensions will
supersede old extensions. If QEMU keeps every extension it adds, then
we'll slow progress on new extensions by maintaining old extension code.
The old extensions will also bitrot or waste CI resources getting tested
for no reason.

I don't know the history of 'debug' and 'sdtrig', other than what I've
read above, but, to me, it looks like 'debug' might be one of the first
extensions which should be deprecated. Assuming we have a long enough
deprecation period, then I think it's always safe to attempt a
deprecation. If somebody shouts, then it can always be taken back off the
chopping block.

Thanks,
drew