> On 13-Mar-2024, at 6:19 PM, Andrew Jones <ajo...@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
