Re: [PATCH v1 1/5] ACPI: CPPC: Check _OSC for flexible address space
by Sudeep Holla
On Wed, May 11, 2022 at 03:45:55PM +0200, Pierre Gondois wrote:
> ACPI 6.2 Section 6.2.11.2 'Platform-Wide OSPM Capabilities':
> Starting with ACPI Specification 6.2, all _CPC registers can be in
> PCC, System Memory, System IO, or Functional Fixed Hardware address
> spaces. OSPM support for this more flexible register space scheme is
> indicated by the “Flexible Address Space for CPPC Registers” _OSC bit
>
> Otherwise (cf ACPI 6.1, s8.4.7.1.1.X), _CPC registers must be in:
> - PCC or Functional Fixed Hardware address space if defined
> - SystemMemory address space (NULL register) if not defined
>
> Add the corresponding _OSC bit and check it when parsing _CPC objects.
>
Looks good, other than a minor nit below. Feel free to ignore that or
check what is Rafael's preference. Otherwise,
Reviewed-by: Sudeep Holla <sudeep.holla(a)arm.com>
> Signed-off-by: Pierre Gondois <pierre.gondois(a)arm.com>
> ---
> drivers/acpi/bus.c | 18 ++++++++++++++++++
> drivers/acpi/cppc_acpi.c | 9 +++++++++
> include/linux/acpi.h | 2 ++
> 3 files changed, 29 insertions(+)
>
[...]
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index d7136d13aa44..977d74d0465b 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -574,6 +574,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);
> #define OSC_SB_OSLPI_SUPPORT 0x00000100
> #define OSC_SB_CPC_DIVERSE_HIGH_SUPPORT 0x00001000
> #define OSC_SB_GENERIC_INITIATOR_SUPPORT 0x00002000
> +#define OSC_SB_CPC_FLEXIBLE_ADR_SP 0x00004000
I would prefer ADR_SPACE instead of ADR_SP.
--
Regards,
Sudeep
3 months
Re: [PATCH v1 5/5] cpufreq: CPPC: Enable dvfs_possible_from_any_cpu
by Sudeep Holla
On Wed, May 11, 2022 at 03:45:59PM +0200, Pierre Gondois wrote:
> From: Pierre Gondois <Pierre.Gondois(a)arm.com>
>
> The communication mean of the _CPC desired performance can be
> PCC, System Memory, System IO, or Functional Fixed Hardware (FFH).
>
> PCC, SystemMemory and SystemIo address spaces are available from any
> CPU. Thus, dvfs_possible_from_any_cpu should be enabled in such case.
> For FFH, let the FFH implementation do smp_call_function_*() calls.
>
Fair enough. I just thought it would be good to check if this is already
taken care for Arm platforms and found that we don't support it yet. So all
is fine :).
> Signed-off-by: Pierre Gondois <pierre.gondois(a)arm.com>
> ---
> drivers/cpufreq/cppc_cpufreq.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 000a0c610c30..ad1535fbf389 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -558,6 +558,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
> }
>
> policy->fast_switch_possible = cppc_allow_fast_switch();
> + policy->dvfs_possible_from_any_cpu = true;
>
Reviewed-by: Sudeep Holla <sudeep.holla(a)arm.com>
--
Regards,
Sudeep
3 months
Re: [PATCH v1 4/5] cpufreq: CPPC: Enable fast_switch
by Sudeep Holla
On Wed, May 11, 2022 at 03:45:58PM +0200, Pierre Gondois wrote:
> From: Pierre Gondois <Pierre.Gondois(a)arm.com>
>
> The communication mean of the _CPC desired performance can be
> PCC, System Memory, System IO, or Functional Fixed Hardware.
>
> commit b7898fda5bc7 ("cpufreq: Support for fast frequency switching")
> fast_switching is 'for switching CPU frequencies from interrupt
> context'.
> Writes to SystemMemory and SystemIo are fast and suitable this.
> This is not the case for PCC and might not be the case for FFH.
>
> Enable fast_switching for the cppc_cpufreq driver in above cases.
>
> Add cppc_allow_fast_switch() to check the desired performance
> register address space and set fast_switching accordingly.
>
Reviewed-by: Sudeep Holla <sudeep.holla(a)arm.com>
--
Regards,
Sudeep
3 months
Re: [PATCH v1 2/5] ACPI: bus: Set CPPC _OSC bits for all and when CPPC_LIB is supported
by Sudeep Holla
On Wed, May 11, 2022 at 03:45:56PM +0200, Pierre Gondois wrote:
> The _OSC method allows the OS and firmware to communicate about
> supported features/capabitlities. It also allows the OS to take
> control of some features.
>
> In ACPI 6.4, s6.2.11.2 Platform-Wide OSPM Capabilities, the CPPC
> (resp. v2) bit should be set by the OS if it 'supports controlling
> processor performance via the interfaces described in the _CPC
> object'.
>
> The OS supports CPPC and parses the _CPC object only if
> CONFIG_ACPI_CPPC_LIB is set. Replace the x86 specific
> boot_cpu_has(X86_FEATURE_HWP) dynamic check with an arch
> generic CONFIG_ACPI_CPPC_LIB build-time check.
>
> Note:
> CONFIG_X86_INTEL_PSTATE selects CONFIG_ACPI_CPPC_LIB.
While this is work as per the spec, by not sure what kind of ACPI firmware are
in the wild. So be prepared to relax/constrain to original feature check
for x86, unfortunate but may be needed.
Anyways,
Reviewed-by: Sudeep Holla <sudeep.holla(a)arm.com>
--
Regards,
Sudeep
3 months
[rafael-pm:bleeding-edge 4/16] WARNING: modpost: vmlinux.o(.text+0xae6ba8): Section mismatch in reference from the function img_ir_isr_raw() to the variable .init.text:.LBB2360
by kernel test robot
tree: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git bleeding-edge
head: 85595c9b5107be01157871392e1aa1ce065a7ca7
commit: 8dd2434c1ab5239533382fe988c57e8269a0988c [4/16] Merge branch 'devprop' into linux-next
config: riscv-randconfig-r042-20220509 (https://download.01.org/0day-ci/archive/20220509/202205091949.BeE39vLd-lk...)
compiler: riscv32-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commi...
git remote add rafael-pm https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
git fetch --no-tags rafael-pm bleeding-edge
git checkout 8dd2434c1ab5239533382fe988c57e8269a0988c
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp(a)intel.com>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> WARNING: modpost: vmlinux.o(.text+0xae6ba8): Section mismatch in reference from the function img_ir_isr_raw() to the variable .init.text:.LBB2360
The function img_ir_isr_raw() references
the variable __init .LBB2360.
This is often because img_ir_isr_raw lacks a __init
annotation or the annotation of .LBB2360 is wrong.
--
>> WARNING: modpost: vmlinux.o(.text+0xae6bdc): Section mismatch in reference from the function img_ir_setup_raw() to the variable .init.text:.LBE2360
The function img_ir_setup_raw() references
the variable __init .LBE2360.
This is often because img_ir_setup_raw lacks a __init
annotation or the annotation of .LBE2360 is wrong.
--
>> WARNING: modpost: vmlinux.o(.text+0xc031fe): Section mismatch in reference from the function nvmem_cell_read_u32() to the variable .init.text:.L0
The function nvmem_cell_read_u32() references
the variable __init .L0 .
This is often because nvmem_cell_read_u32 lacks a __init
annotation or the annotation of .L0 is wrong.
--
>> WARNING: modpost: vmlinux.o(.text+0xc03208): Section mismatch in reference from the function nvmem_cell_read_u64() to the variable .init.text:.L0
The function nvmem_cell_read_u64() references
the variable __init .L0 .
This is often because nvmem_cell_read_u64 lacks a __init
annotation or the annotation of .L0 is wrong.
--
>> WARNING: modpost: vmlinux.o(__ex_table+0x1434): Section mismatch in reference from the variable .L0 to the variable .debug_str:.LASF1599
FATAL: modpost: extable_entry size hasn't been discovered!
--
>> WARNING: modpost: vmlinux.o(.text+0xc02b22): Section mismatch in reference from the function nvmem_device_find() to the variable .init.text:.L0
The function nvmem_device_find() references
the variable __init .L0 .
This is often because nvmem_device_find lacks a __init
annotation or the annotation of .L0 is wrong.
--
>> WARNING: modpost: vmlinux.o(.text+0xc031ea): Section mismatch in reference from the function nvmem_cell_read_u8() to the variable .init.text:.L0
The function nvmem_cell_read_u8() references
the variable __init .L0 .
This is often because nvmem_cell_read_u8 lacks a __init
annotation or the annotation of .L0 is wrong.
--
>> WARNING: modpost: vmlinux.o(.text+0xc031f4): Section mismatch in reference from the function nvmem_cell_read_u16() to the variable .init.text:.L0
The function nvmem_cell_read_u16() references
the variable __init .L0 .
This is often because nvmem_cell_read_u16 lacks a __init
annotation or the annotation of .L0 is wrong.
Note: the below error/warnings can be found in parent commit:
<< WARNING: modpost: vmlinux.o(.text+0xa740cc): Section mismatch in reference from the function rtc_month_days() to the variable .init.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xb34f2e): Section mismatch in reference from the function btintel_enter_mfg() to the variable .init.text:.LBB18632
<< WARNING: modpost: vmlinux.o(.text+0xc2a6ce): Section mismatch in reference from the function __sys_recvmsg_sock() to the variable .exit.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xd02eec): Section mismatch in reference from the function dn_fib_lookup() to the variable .init.text:.LBB1596
<< WARNING: modpost: vmlinux.o(__ex_table+0x141c): Section mismatch in reference from the variable .L0 to the variable .debug_str:.LASF864
<< WARNING: modpost: vmlinux.o(.text+0xa740cc): Section mismatch in reference from the function rtc_month_days() to the variable .init.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xb34f2e): Section mismatch in reference from the function btintel_enter_mfg() to the variable .init.text:.LBB18632
<< WARNING: modpost: vmlinux.o(.text+0xc2a6ce): Section mismatch in reference from the function __sys_recvmsg_sock() to the variable .exit.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xd02eec): Section mismatch in reference from the function dn_fib_lookup() to the variable .init.text:.LBB1596
<< WARNING: modpost: vmlinux.o(__ex_table+0x141c): Section mismatch in reference from the variable .L0 to the variable .debug_str:.LASF864
<< WARNING: modpost: vmlinux.o(.text+0xa740cc): Section mismatch in reference from the function rtc_month_days() to the variable .init.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xb34f2e): Section mismatch in reference from the function btintel_enter_mfg() to the variable .init.text:.LBB18632
<< WARNING: modpost: vmlinux.o(.text+0xc2a6ce): Section mismatch in reference from the function __sys_recvmsg_sock() to the variable .exit.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xd02eec): Section mismatch in reference from the function dn_fib_lookup() to the variable .init.text:.LBB1596
<< WARNING: modpost: vmlinux.o(__ex_table+0x141c): Section mismatch in reference from the variable .L0 to the variable .debug_str:.LASF864
<< WARNING: modpost: vmlinux.o(.text+0xa740cc): Section mismatch in reference from the function rtc_month_days() to the variable .init.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xb34f2e): Section mismatch in reference from the function btintel_enter_mfg() to the variable .init.text:.LBB18632
<< WARNING: modpost: vmlinux.o(.text+0xc2a6ce): Section mismatch in reference from the function __sys_recvmsg_sock() to the variable .exit.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xd02eec): Section mismatch in reference from the function dn_fib_lookup() to the variable .init.text:.LBB1596
<< WARNING: modpost: vmlinux.o(__ex_table+0x141c): Section mismatch in reference from the variable .L0 to the variable .debug_str:.LASF864
<< WARNING: modpost: vmlinux.o(.text+0xa740cc): Section mismatch in reference from the function rtc_month_days() to the variable .init.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xb34f2e): Section mismatch in reference from the function btintel_enter_mfg() to the variable .init.text:.LBB18632
<< WARNING: modpost: vmlinux.o(.text+0xc2a6ce): Section mismatch in reference from the function __sys_recvmsg_sock() to the variable .exit.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xd02eec): Section mismatch in reference from the function dn_fib_lookup() to the variable .init.text:.LBB1596
<< WARNING: modpost: vmlinux.o(__ex_table+0x141c): Section mismatch in reference from the variable .L0 to the variable .debug_str:.LASF864
<< WARNING: modpost: vmlinux.o(.text+0xa740cc): Section mismatch in reference from the function rtc_month_days() to the variable .init.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xb34f2e): Section mismatch in reference from the function btintel_enter_mfg() to the variable .init.text:.LBB18632
<< WARNING: modpost: vmlinux.o(.text+0xc2a6ce): Section mismatch in reference from the function __sys_recvmsg_sock() to the variable .exit.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xd02eec): Section mismatch in reference from the function dn_fib_lookup() to the variable .init.text:.LBB1596
<< WARNING: modpost: vmlinux.o(__ex_table+0x141c): Section mismatch in reference from the variable .L0 to the variable .debug_str:.LASF864
<< WARNING: modpost: vmlinux.o(.text+0xa740cc): Section mismatch in reference from the function rtc_month_days() to the variable .init.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xb34f2e): Section mismatch in reference from the function btintel_enter_mfg() to the variable .init.text:.LBB18632
<< WARNING: modpost: vmlinux.o(.text+0xc2a6ce): Section mismatch in reference from the function __sys_recvmsg_sock() to the variable .exit.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xd02eec): Section mismatch in reference from the function dn_fib_lookup() to the variable .init.text:.LBB1596
<< WARNING: modpost: vmlinux.o(__ex_table+0x141c): Section mismatch in reference from the variable .L0 to the variable .debug_str:.LASF864
<< WARNING: modpost: vmlinux.o(.text+0xa740cc): Section mismatch in reference from the function rtc_month_days() to the variable .init.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xb34f2e): Section mismatch in reference from the function btintel_enter_mfg() to the variable .init.text:.LBB18632
<< WARNING: modpost: vmlinux.o(.text+0xc2a6ce): Section mismatch in reference from the function __sys_recvmsg_sock() to the variable .exit.text:.L0
<< WARNING: modpost: vmlinux.o(.text+0xd02eec): Section mismatch in reference from the function dn_fib_lookup() to the variable .init.text:.LBB1596
<< WARNING: modpost: vmlinux.o(__ex_table+0x141c): Section mismatch in reference from the variable .L0 to the variable .debug_str:.LASF864
--
0-DAY CI Kernel Test Service
https://01.org/lkp
3 months, 1 week
Re: [PATCH 2/2] ACPI: ARM Performance Monitoring Unit Table (APMT) initial support
by Sudeep Holla
On Wed, May 04, 2022 at 10:08:39PM +0000, Besar Wicaksono wrote:
> Hi Sudeep,
>
> > Any particular reason why you would like to rush and push this without
> > the actual driver to probe the device being added here ?
>
> I plan to have two patch series, one for ACPI patch (this patch) and one for
> driver patch. My understanding is the driver patch will depend on this
> patch, but not the other way. So, I thought it would be better to get this
> patch approved first. However, if it helps the review of this patch, I am
> hoping to post the driver patch by end of the week and will CC you on that
> one.
Sure please do that. IMO, the driver is usually upstreamed first along with
the DT bindings(in ACPI case either the spec change or the std namespace node)
The actual addition of the device happens via DT. ACPI APMT needs creation
of device here but I prefer to see the driver first.
>
> > I really don't prefer this name:
> > 1. arm-coresight-pmu is much better than "csite". I see the short form
> > used elsewhere in the kernel is just "cs" as in cs_etm,...etc
> > 2. Since APMT is more generic than just coresight(I understand coresight
> > was the initial motivation for the generic specification) and also
> > the type list seem to cover memory controller, SMMU,..etc, does
> > it make sense to call it "arm-generic-pmu" or something similar.
>
> Between these two, I would prefer arm-coresight-pmu just to anticipate
> another standard in the future from ARM. The APMT, to my understanding, is
> applicable only to CoreSight based PMUs. Using "coresight" as part of the
> device name is reasonable.
I read the APMT spec again and it has very little reference to coresight
though it is weirdly labelled as Coresight PMU for no sane reasons(Sorry I
know it's Arm to blame here and I bet something to do with marketing).
Anyways the APMT spec on its own covers all types of PMUs as I stated earlier.
So I prefer "arm-generic-pmu" or something better if you can come up with. I
am fine if you would like to retain arm-coresight-pmu when you post driver as
that's what the spec is labelled as.
Once you post the driver we can debate on that and come up with better name
with all the concerned parties involved.
>
> > Not sure if the same device name will be re-used or PMUs can be registered
> > with different name under perf subsystem, but the name matters for the user
> > space tools and decoders. They may use the name or type information to analyse
> > the data samples.
> >
> > So it is better to wait for all those discussion as part of the driver
> > upstreaming before you use this device name unless we are absolutely sure
> > the PMUs can be registered with different names in the driver(which could
> > be possible, I just don't know)
> >
> > Apart from this name, I am OK with the changes here and happy to ack if it
> > is OK to merge this without any driver to probe this device yet.
>
> I believe using a different name to register the PMU is possible.
> In the current driver patch, we use a different name format to register the
> PMU: arm_csite_pmu<numeric id>. Certainly the "csite" needs to change as
> well 😊. Another example like ARM CCI PMU uses device name "ARM-CCI PMU",
> but it is registered to perf subsystem as "CCI_400" or "CCI_500".
>
Agreed, those are details we can discuss once you post with all the
maintainers involved.
> If there is no objection, I can post update to this patch and go ahead with
> "arm-coresight-pmu" for the device name.
Sure as I mentioned above that should be fine. I will then raise it with
the maintainers how we managed to labelled the spec to confuse it with other
components. I prefer whatever we add must be easy to identify and doesn't
conflict with existing PMUs(like ARM CPU PMUs, or the coresight ETM PMUs,
..etc)
In short, just post the driver the way you prefer and let us start the
discussion there.
--
Regards,
Sudeep
3 months, 1 week
