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 > >