[PATCH v4 2/3] target/riscv: Expose sdtrig ISA extension

Himanshu Chauhan posted 3 patches 1 year, 11 months ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
There is a newer version of this series
[PATCH v4 2/3] target/riscv: Expose sdtrig ISA extension
Posted by Himanshu Chauhan 1 year, 11 months ago
This patch adds "sdtrig" in the ISA string when sdtrig extension is enabled.
The sdtrig extension may or may not be implemented in a system. Therefore, the
           -cpu rv64,sdtrig=<true/false>
option can be used to dynamically turn sdtrig extension on or off.

Since, the sdtrig ISA extension is a superset of debug specification, disable
the debug property when sdtrig is enabled. A warning is printed when this is
done.

By default, the sdtrig extension is disabled and debug property enabled as usual.

Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
---
 target/riscv/cpu.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 2602aae9f5..ab057a0926 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -175,6 +175,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(zvkt, PRIV_VERSION_1_12_0, ext_zvkt),
     ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
     ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
+    ISA_EXT_DATA_ENTRY(sdtrig, PRIV_VERSION_1_12_0, ext_sdtrig),
     ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
     ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
     ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
@@ -1008,6 +1009,12 @@ static void riscv_cpu_reset_hold(Object *obj)
     set_default_nan_mode(1, &env->fp_status);
 
 #ifndef CONFIG_USER_ONLY
+    if (cpu->cfg.debug && cpu->cfg.ext_sdtrig) {
+	    warn_report("Disabling debug property since sdtrig ISA extension "
+			"is enabled");
+	    cpu->cfg.debug = 0;
+    }
+
     if (cpu->cfg.debug || cpu->cfg.ext_sdtrig) {
         riscv_trigger_reset_hold(env);
     }
@@ -1480,6 +1487,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
     MULTI_EXT_CFG_BOOL("zvfhmin", ext_zvfhmin, false),
     MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
 
+    MULTI_EXT_CFG_BOOL("sdtrig", ext_sdtrig, false),
     MULTI_EXT_CFG_BOOL("smaia", ext_smaia, false),
     MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
     MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
-- 
2.34.1
Re: [PATCH v4 2/3] target/riscv: Expose sdtrig ISA extension
Posted by Andrew Jones 1 year, 11 months ago
On Wed, Mar 13, 2024 at 11:39:30AM +0530, Himanshu Chauhan wrote:
> This patch adds "sdtrig" in the ISA string when sdtrig extension is enabled.
> The sdtrig extension may or may not be implemented in a system. Therefore, the
>            -cpu rv64,sdtrig=<true/false>
> option can be used to dynamically turn sdtrig extension on or off.
> 
> Since, the sdtrig ISA extension is a superset of debug specification, disable
> the debug property when sdtrig is enabled. A warning is printed when this is
> done.
> 
> By default, the sdtrig extension is disabled and debug property enabled as usual.
> 
> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
> ---
>  target/riscv/cpu.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 2602aae9f5..ab057a0926 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -175,6 +175,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
>      ISA_EXT_DATA_ENTRY(zvkt, PRIV_VERSION_1_12_0, ext_zvkt),
>      ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
>      ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
> +    ISA_EXT_DATA_ENTRY(sdtrig, PRIV_VERSION_1_12_0, ext_sdtrig),
>      ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
>      ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
>      ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
> @@ -1008,6 +1009,12 @@ static void riscv_cpu_reset_hold(Object *obj)
>      set_default_nan_mode(1, &env->fp_status);
>  
>  #ifndef CONFIG_USER_ONLY
> +    if (cpu->cfg.debug && cpu->cfg.ext_sdtrig) {
> +	    warn_report("Disabling debug property since sdtrig ISA extension "
> +			"is enabled");
> +	    cpu->cfg.debug = 0;

If sdtrig is a superset of debug, then why do we need to disable debug?

Thanks,
drew

> +    }
> +
>      if (cpu->cfg.debug || cpu->cfg.ext_sdtrig) {
>          riscv_trigger_reset_hold(env);
>      }
> @@ -1480,6 +1487,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
>      MULTI_EXT_CFG_BOOL("zvfhmin", ext_zvfhmin, false),
>      MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
>  
> +    MULTI_EXT_CFG_BOOL("sdtrig", ext_sdtrig, false),
>      MULTI_EXT_CFG_BOOL("smaia", ext_smaia, false),
>      MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
>      MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
> -- 
> 2.34.1
> 
>
Re: [PATCH v4 2/3] target/riscv: Expose sdtrig ISA extension
Posted by Himanshu Chauhan 1 year, 11 months ago
On Wed, Mar 13, 2024 at 3:24 PM Andrew Jones <ajones@ventanamicro.com>
wrote:

> On Wed, Mar 13, 2024 at 11:39:30AM +0530, Himanshu Chauhan wrote:
> > This patch adds "sdtrig" in the ISA string when sdtrig extension is
> enabled.
> > The sdtrig extension may or may not be implemented in a system.
> Therefore, the
> >            -cpu rv64,sdtrig=<true/false>
> > option can be used to dynamically turn sdtrig extension on or off.
> >
> > Since, the sdtrig ISA extension is a superset of debug specification,
> disable
> > the debug property when sdtrig is enabled. A warning is printed when
> this is
> > done.
> >
> > By default, the sdtrig extension is disabled and debug property enabled
> as usual.
> >
> > Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
> > ---
> >  target/riscv/cpu.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 2602aae9f5..ab057a0926 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -175,6 +175,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> >      ISA_EXT_DATA_ENTRY(zvkt, PRIV_VERSION_1_12_0, ext_zvkt),
> >      ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
> >      ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
> > +    ISA_EXT_DATA_ENTRY(sdtrig, PRIV_VERSION_1_12_0, ext_sdtrig),
> >      ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
> >      ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
> >      ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
> > @@ -1008,6 +1009,12 @@ static void riscv_cpu_reset_hold(Object *obj)
> >      set_default_nan_mode(1, &env->fp_status);
> >
> >  #ifndef CONFIG_USER_ONLY
> > +    if (cpu->cfg.debug && cpu->cfg.ext_sdtrig) {
> > +         warn_report("Disabling debug property since sdtrig ISA
> extension "
> > +                     "is enabled");
> > +         cpu->cfg.debug = 0;
>
> If sdtrig is a superset of debug, then why do we need to disable debug?
>

"debug" is not disabled. The flag is turned off. This is for unambiguity
between which spec is in force currently.
May come handy (not now but later) in if conditions.


>
> Thanks,
> drew
>
> > +    }
> > +
> >      if (cpu->cfg.debug || cpu->cfg.ext_sdtrig) {
> >          riscv_trigger_reset_hold(env);
> >      }
> > @@ -1480,6 +1487,7 @@ const RISCVCPUMultiExtConfig
> riscv_cpu_extensions[] = {
> >      MULTI_EXT_CFG_BOOL("zvfhmin", ext_zvfhmin, false),
> >      MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
> >
> > +    MULTI_EXT_CFG_BOOL("sdtrig", ext_sdtrig, false),
> >      MULTI_EXT_CFG_BOOL("smaia", ext_smaia, false),
> >      MULTI_EXT_CFG_BOOL("smepmp", ext_smepmp, false),
> >      MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
> > --
> > 2.34.1
> >
> >
>
Re: [PATCH v4 2/3] target/riscv: Expose sdtrig ISA extension
Posted by Andrew Jones 1 year, 11 months ago
On Wed, Mar 13, 2024 at 03:50:16PM +0530, Himanshu Chauhan wrote:
> On Wed, Mar 13, 2024 at 3:24 PM Andrew Jones <ajones@ventanamicro.com>
> wrote:
> 
> > On Wed, Mar 13, 2024 at 11:39:30AM +0530, Himanshu Chauhan wrote:
> > > This patch adds "sdtrig" in the ISA string when sdtrig extension is
> > enabled.
> > > The sdtrig extension may or may not be implemented in a system.
> > Therefore, the
> > >            -cpu rv64,sdtrig=<true/false>
> > > option can be used to dynamically turn sdtrig extension on or off.
> > >
> > > Since, the sdtrig ISA extension is a superset of debug specification,
> > disable
> > > the debug property when sdtrig is enabled. A warning is printed when
> > this is
> > > done.
> > >
> > > By default, the sdtrig extension is disabled and debug property enabled
> > as usual.
> > >
> > > Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
> > > ---
> > >  target/riscv/cpu.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > index 2602aae9f5..ab057a0926 100644
> > > --- a/target/riscv/cpu.c
> > > +++ b/target/riscv/cpu.c
> > > @@ -175,6 +175,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> > >      ISA_EXT_DATA_ENTRY(zvkt, PRIV_VERSION_1_12_0, ext_zvkt),
> > >      ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
> > >      ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
> > > +    ISA_EXT_DATA_ENTRY(sdtrig, PRIV_VERSION_1_12_0, ext_sdtrig),
> > >      ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
> > >      ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
> > >      ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
> > > @@ -1008,6 +1009,12 @@ static void riscv_cpu_reset_hold(Object *obj)
> > >      set_default_nan_mode(1, &env->fp_status);
> > >
> > >  #ifndef CONFIG_USER_ONLY
> > > +    if (cpu->cfg.debug && cpu->cfg.ext_sdtrig) {
> > > +         warn_report("Disabling debug property since sdtrig ISA
> > extension "
> > > +                     "is enabled");
> > > +         cpu->cfg.debug = 0;
> >
> > If sdtrig is a superset of debug, then why do we need to disable debug?
> >
> 
> "debug" is not disabled. The flag is turned off. This is for unambiguity
> between which spec is in force currently.
> May come handy (not now but later) in if conditions.

I know sdtrig/debug functionality remains enabled, but why state that the
'debug' functionality is no longer enabled? If sdtrig is a superset, then,
when it's enabled, both the debug functionality and the sdtrig
functionality are enabled. Actually, sdtrig should do the opposite, it
should ensure debug=true. The warning would look odd to users that know
this and the debug=off would also look odd in the results of cpu model
expansion. Keeping debug=true would also avoid needing to change all the
if cpu->cfg.debug conditions to if cpu->cfg.debug || cpu->cfg.ext_sdtrig.

If we deprecate 'debug' someday, then we'll add a warning for when
there is 'debug' explicitly enabled by a user and also for 'debug'
configs which lack 'sdtrig', but we don't need to worry about that now.

Thanks,
drew

Re: [PATCH v4 2/3] target/riscv: Expose sdtrig ISA extension
Posted by Himanshu Chauhan 1 year, 11 months ago

> On 13-Mar-2024, at 4:28 PM, Andrew Jones <ajones@ventanamicro.com> wrote:
> 
> On Wed, Mar 13, 2024 at 03:50:16PM +0530, Himanshu Chauhan wrote:
>> On Wed, Mar 13, 2024 at 3:24 PM Andrew Jones <ajones@ventanamicro.com>
>> wrote:
>> 
>>> On Wed, Mar 13, 2024 at 11:39:30AM +0530, Himanshu Chauhan wrote:
>>>> This patch adds "sdtrig" in the ISA string when sdtrig extension is
>>> enabled.
>>>> The sdtrig extension may or may not be implemented in a system.
>>> Therefore, the
>>>>           -cpu rv64,sdtrig=<true/false>
>>>> option can be used to dynamically turn sdtrig extension on or off.
>>>> 
>>>> Since, the sdtrig ISA extension is a superset of debug specification,
>>> disable
>>>> the debug property when sdtrig is enabled. A warning is printed when
>>> this is
>>>> done.
>>>> 
>>>> By default, the sdtrig extension is disabled and debug property enabled
>>> as usual.
>>>> 
>>>> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
>>>> ---
>>>> target/riscv/cpu.c | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>> 
>>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>>> index 2602aae9f5..ab057a0926 100644
>>>> --- a/target/riscv/cpu.c
>>>> +++ b/target/riscv/cpu.c
>>>> @@ -175,6 +175,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
>>>>     ISA_EXT_DATA_ENTRY(zvkt, PRIV_VERSION_1_12_0, ext_zvkt),
>>>>     ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
>>>>     ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
>>>> +    ISA_EXT_DATA_ENTRY(sdtrig, PRIV_VERSION_1_12_0, ext_sdtrig),
>>>>     ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
>>>>     ISA_EXT_DATA_ENTRY(smepmp, PRIV_VERSION_1_12_0, ext_smepmp),
>>>>     ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
>>>> @@ -1008,6 +1009,12 @@ static void riscv_cpu_reset_hold(Object *obj)
>>>>     set_default_nan_mode(1, &env->fp_status);
>>>> 
>>>> #ifndef CONFIG_USER_ONLY
>>>> +    if (cpu->cfg.debug && cpu->cfg.ext_sdtrig) {
>>>> +         warn_report("Disabling debug property since sdtrig ISA
>>> extension "
>>>> +                     "is enabled");
>>>> +         cpu->cfg.debug = 0;
>>> 
>>> If sdtrig is a superset of debug, then why do we need to disable debug?
>>> 
>> 
>> "debug" is not disabled. The flag is turned off. This is for unambiguity
>> between which spec is in force currently.
>> May come handy (not now but later) in if conditions.
> 
> I know sdtrig/debug functionality remains enabled, but why state that the
> 'debug' functionality is no longer enabled?

Got it. The warning can be removed.

> If sdtrig is a superset, then,
> when it's enabled, both the debug functionality and the sdtrig
> functionality are enabled. Actually, sdtrig should do the opposite, it
> should ensure debug=true. The warning would look odd to users that know

I would disagree to set debug=true when sdtrig is selected. These are two different specifications and should be treated so. Sdtrig is a superset now but may have another revision not backward compatible. For two different specifications and flags should mirror the same. On the same lines, this patch (as it does) should keep (cfg->debug || cfg->sdtrig) which shows that you are dealing with two different specifications. They behave same for some triggers but are still different. Deprecation isn’t a problem now or later.

> this and the debug=off would also look odd in the results of cpu model
> expansion. Keeping debug=true would also avoid needing to change all the
> if cpu->cfg.debug conditions to if cpu->cfg.debug || cpu->cfg.ext_sdtrig.
> 
> If we deprecate 'debug' someday, then we'll add a warning for when
> there is 'debug' explicitly enabled by a user and also for 'debug'
> configs which lack 'sdtrig', but we don't need to worry about that now.
> 
> Thanks,
> drew
Re: [PATCH v4 2/3] target/riscv: Expose sdtrig ISA extension
Posted by Andrew Jones 1 year, 11 months ago
On Wed, Mar 13, 2024 at 05:48:16PM +0530, Himanshu Chauhan wrote:
...
> >>>> #ifndef CONFIG_USER_ONLY
> >>>> +    if (cpu->cfg.debug && cpu->cfg.ext_sdtrig) {
> >>>> +         warn_report("Disabling debug property since sdtrig ISA
> >>> extension "
> >>>> +                     "is enabled");
> >>>> +         cpu->cfg.debug = 0;
> >>> 
> >>> If sdtrig is a superset of debug, then why do we need to disable debug?
> >>> 
> >> 
> >> "debug" is not disabled. The flag is turned off. This is for unambiguity
> >> between which spec is in force currently.
> >> May come handy (not now but later) in if conditions.
> > 
> > I know sdtrig/debug functionality remains enabled, but why state that the
> > 'debug' functionality is no longer enabled?
> 
> Got it. The warning can be removed.
> 
> > If sdtrig is a superset, then,
> > when it's enabled, both the debug functionality and the sdtrig
> > functionality are enabled. Actually, sdtrig should do the opposite, it
> > should ensure debug=true. The warning would look odd to users that know
> 
> I would disagree to set debug=true when sdtrig is selected. These are two different specifications and should be treated so. Sdtrig is a superset now but may have another revision not backward compatible. For two different specifications and flags should mirror the same. On the same lines, this patch (as it does) should keep (cfg->debug || cfg->sdtrig) which shows that you are dealing with two different specifications. They behave same for some triggers but are still different. Deprecation isn’t a problem now or later.

sdtrig is frozen, otherwise it would require the 'x-' prefix, so it can
no longer change in a substantial way and therefore if it's a superset of
debug now then it always will be. If a change is made to "debug
functionality" then it will get a new extension name which may not be
compatible with either 'debug' or 'sdtrig', however that's not the case
here. If a platform supports 'sdtrig', then it supports 'debug', as
'sdtrig' is a superset of 'debug'. Pretending like they're mutually
exclusive doesn't make sense when they completely overlap. Indeed
platform's will likely *want* to advertise that they are compatible with
both because software that is only compatible with 'debug' will need to
know if it will work or not. A platform that says it's not compatible
with 'debug', when it is, because it supports sdtrig, is limiting the
software that will run on it for no reason.

Thanks,
drew

> 
> > this and the debug=off would also look odd in the results of cpu model
> > expansion. Keeping debug=true would also avoid needing to change all the
> > if cpu->cfg.debug conditions to if cpu->cfg.debug || cpu->cfg.ext_sdtrig.
> > 
> > If we deprecate 'debug' someday, then we'll add a warning for when
> > there is 'debug' explicitly enabled by a user and also for 'debug'
> > configs which lack 'sdtrig', but we don't need to worry about that now.
> > 
> > Thanks,
> > drew
> 
> 

Re: [PATCH v4 2/3] target/riscv: Expose sdtrig ISA extension
Posted by Himanshu Chauhan 1 year, 11 months ago

> On 13-Mar-2024, at 6:19 PM, Andrew Jones <ajones@ventanamicro.com> wrote:
> 
> On Wed, Mar 13, 2024 at 05:48:16PM +0530, Himanshu Chauhan wrote:
> ...
>>>>>> #ifndef CONFIG_USER_ONLY
>>>>>> +    if (cpu->cfg.debug && cpu->cfg.ext_sdtrig) {
>>>>>> +         warn_report("Disabling debug property since sdtrig ISA
>>>>> extension "
>>>>>> +                     "is enabled");
>>>>>> +         cpu->cfg.debug = 0;
>>>>> 
>>>>> If sdtrig is a superset of debug, then why do we need to disable debug?
>>>>> 
>>>> 
>>>> "debug" is not disabled. The flag is turned off. This is for unambiguity
>>>> between which spec is in force currently.
>>>> May come handy (not now but later) in if conditions.
>>> 
>>> I know sdtrig/debug functionality remains enabled, but why state that the
>>> 'debug' functionality is no longer enabled?
>> 
>> Got it. The warning can be removed.
>> 
>>> If sdtrig is a superset, then,
>>> when it's enabled, both the debug functionality and the sdtrig
>>> functionality are enabled. Actually, sdtrig should do the opposite, it
>>> should ensure debug=true. The warning would look odd to users that know
>> 
>> I would disagree to set debug=true when sdtrig is selected. These are two different specifications and should be treated so. Sdtrig is a superset now but may have another revision not backward compatible. For two different specifications and flags should mirror the same. On the same lines, this patch (as it does) should keep (cfg->debug || cfg->sdtrig) which shows that you are dealing with two different specifications. They behave same for some triggers but are still different. Deprecation isn’t a problem now or later.
> 
> sdtrig is frozen, otherwise it would require the 'x-' prefix, so it can
> no longer change in a substantial way and therefore if it's a superset of
> debug now then it always will be. If a change is made to "debug
> functionality" then it will get a new extension name which may not be
> compatible with either 'debug' or 'sdtrig', however that's not the case
> here. If a platform supports 'sdtrig', then it supports 'debug', as
> 'sdtrig' is a superset of 'debug'. Pretending like they're mutually
> exclusive doesn't make sense when they completely overlap. Indeed
> platform's will likely *want* to advertise that they are compatible with
> both because software that is only compatible with 'debug' will need to
> know if it will work or not. A platform that says it's not compatible
> with 'debug', when it is, because it supports sdtrig, is limiting the
> software that will run on it for no reason.

Got it. I will make the necessary changes.

> 
> Thanks,
> drew
> 
>> 
>>> this and the debug=off would also look odd in the results of cpu model
>>> expansion. Keeping debug=true would also avoid needing to change all the
>>> if cpu->cfg.debug conditions to if cpu->cfg.debug || cpu->cfg.ext_sdtrig.
>>> 
>>> If we deprecate 'debug' someday, then we'll add a warning for when
>>> there is 'debug' explicitly enabled by a user and also for 'debug'
>>> configs which lack 'sdtrig', but we don't need to worry about that now.
>>> 
>>> Thanks,
>>> drew