[AMD Official Use Only - AMD Internal Distribution Only]
Hi,
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Tuesday, February 18, 2025 10:56 PM
> To: Penny, Zheng <penny.zh...@amd.com>
> Cc: Huang, Ray <ray.hu...@amd.com>; Andryuk, Jason
> <jason.andr...@amd.com>; Andrew Cooper <andrew.coop...@citrix.com>;
> Anthony PERARD <anthony.per...@vates.tech>; Orzel, Michal
> <michal.or...@amd.com>; Julien Grall <jul...@xen.org>; Roger Pau Monné
> <roger....@citrix.com>; Stefano Stabellini <sstabell...@kernel.org>; xen-
> de...@lists.xenproject.org
> Subject: Re: [PATCH v2 03/11] xen/x86: introduce "cpufreq=amd-cppc" xen
> cmdline
>
> On 18.02.2025 05:24, Penny, Zheng wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeul...@suse.com>
> >> Sent: Monday, February 17, 2025 6:34 PM
> >>
> >> On 17.02.2025 11:17, Penny, Zheng wrote:
> >>>> -----Original Message-----
> >>>> From: Jan Beulich <jbeul...@suse.com>
> >>>> Sent: Tuesday, February 11, 2025 8:09 PM
> >>>>
> >>>> On 06.02.2025 09:32, Penny Zheng wrote:
> >>>>> @@ -131,6 +131,15 @@ static int __init cf_check
> >>>>> setup_cpufreq_option(const
> >>>> char *str)
> >>>>> if ( arg[0] && arg[1] )
> >>>>> ret = hwp_cmdline_parse(arg + 1, end);
> >>>>> }
> >>>>> + else if ( choice < 0 && !cmdline_strcmp(str, "amd-cppc") )
> >>>>> + {
> >>>>> + xen_processor_pmbits |= XEN_PROCESSOR_PM_CPPC;
> >>>>> + cpufreq_controller = FREQCTL_xen;
> >>>>> + cpufreq_xen_opts[cpufreq_xen_cnt++] =
> >>>>> + CPUFREQ_amd_cppc;
> >>>>
> >>>> While apparently again a pre-existing problem, the risk of array
> >>>> overrun will become more manifest with this addition: People may
> >>>> plausibly want to pass a universal option to Xen on all their instances:
> >>>> "cpufreq=hwp,amd-cppc,xen". I think this wants taking care of in a
> >>>> prereq patch,
> >> i.e.
> >>>> before you further extend it. Here you will then further want to
> >>>> bump cpufreq_xen_opts[]'es dimension, to account for the now
> >>>> sensible three-fold
> >> option.
> >>>>
> >>>
> >>> Correct me if I'm wrong, We are missing dealing the scenario which
> >>> looks like the
> >> following:
> >>> "cpufreq=amd-cppc,hwp,verbose".
> >>
> >> Not so much this one (can it even overflow). It's
> >> "cpufreq=amd-cppc,hwp,xen"
> >> that I'm concerned about (or, prior to your change something
> >> redundant like "cpufreq=hwp,hwp,xen").
> >
> > I misunderstood before, sorry
> > What is the appropriate behavior when user passes an option to Xen, like
> "cpufreq=amd-cppc,hwp,xen" ?
> > FWIT, amd-cppc and hwp are incompatible options.
>
> Sure, but as said people may want to use something like this uniformly on all
> their
> systems, be them AMD or Intel ones. IOW ...
>
> > Send the error info to tell them you shall choose either of them, amd-cppc,
> > or hwp,
> or xen?
>
> ... no, I don't think this should be an error.
>
> > If user wants to add fall back scheme, when amd-cppc is hardware
> > unavailable, we fall back to xen. user shall use ";", not "," to add, like
> "cpufreq=amd-cppc;xen"
>
> Well, I didn't closely check whether the separator is to be semicolon or
> comma.
> Things is that people may want to use one single command line option across
> all
> their systems, old or new, Intel or AMD. Hence they may want to ask to use
> HWP is
> available, CPPC is available, or fall back to what we have had for ages. Yet
> they will
> also need to have a way to express that they want only HWP and CPPC to be
> tried,
> without falling back to the legacy driver. Hence we may not automatically
> fall back to
> that if "amp-cppc" was passed, but is unavailable.
>
Then I suggest we use the semicolon to separate all options user would like to
try, but in the
priority order, like "cpufreq=hwp;amd-cppc;xen", if hwp and amd-cppc are both
tried and found
not supported,legacy xen will be considered.
If it's only "cpufreq=hwp;amd-cppc", and when hwp and amd-cppc are both not
supported, we
will not automatically fall back to any.
For specific driver, like "amd-cppc", sub-features like active mode will be
separated by ",", like
"cpufreq=hwp;amd-cppc,active;xen"
> Jan
