linux-kernel - RE: [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and duplicated headers declaration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DM4PR12MB5278152B572E53554FFD59069C7B9@DM4PR12MB5278.namprd12.prod.outlook.com>
Date:   Thu, 1 Sep 2022 05:34:49 +0000
From:   "Yuan, Perry" <Perry.Yuan@....com>
To:     Punit Agrawal <punit.agrawal@...edance.com>
CC:     "rafael.j.wysocki@...el.com" <rafael.j.wysocki@...el.com>,
        "Huang, Ray" <Ray.Huang@....com>,
        "viresh.kumar@...aro.org" <viresh.kumar@...aro.org>,
        "Sharma, Deepak" <Deepak.Sharma@....com>,
        "Limonciello, Mario" <Mario.Limonciello@....com>,
        "Fontenot, Nathan" <Nathan.Fontenot@....com>,
        "Deucher, Alexander" <Alexander.Deucher@....com>,
        "Su, Jinzhou (Joe)" <Jinzhou.Su@....com>,
        "Huang, Shimmer" <Shimmer.Huang@....com>,
        "Du, Xiaojian" <Xiaojian.Du@....com>,
        "Meng, Li (Jassmine)" <Li.Meng@....com>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and
 duplicated headers declaration

[AMD Official Use Only - General]

Hi Punit, 

> -----Original Message-----
> From: Punit Agrawal <punit.agrawal@...edance.com>
> Sent: Monday, August 15, 2022 11:05 PM
> To: Yuan, Perry <Perry.Yuan@....com>
> Cc: rafael.j.wysocki@...el.com; Huang, Ray <Ray.Huang@....com>;
> viresh.kumar@...aro.org; Sharma, Deepak <Deepak.Sharma@....com>;
> Limonciello, Mario <Mario.Limonciello@....com>; Fontenot, Nathan
> <Nathan.Fontenot@....com>; Deucher, Alexander
> <Alexander.Deucher@....com>; Su, Jinzhou (Joe) <Jinzhou.Su@....com>;
> Huang, Shimmer <Shimmer.Huang@....com>; Du, Xiaojian
> <Xiaojian.Du@....com>; Meng, Li (Jassmine) <Li.Meng@....com>; linux-
> pm@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v5 1/7] cpufreq: amd-pstate: cleanup the unused and
> duplicated headers declaration
> 
> [CAUTION: External Email]
> 
> Hi Perry,
> 
> Perry Yuan <Perry.Yuan@....com> writes:
> 
> > Cleanup the headers declaration which are not used actually and some
> > duplicated declaration which is declarated in some other headers
> > already, it will help to simplify the header part.
> 
> We usually don't get rid of indirectly included headers as long as definitions from
> header are used in the code. This avoids problems if for some reason the
> included header gets dropped - it'll leave the code in an uncompilable state.
> 
> More below.
> 
> >
> > Reviewed-by: Huang Rui <ray.huang@....com>
> > Acked-by: Viresh Kumar <viresh.kumar@...aro.org>
> > Signed-off-by: Perry Yuan <Perry.Yuan@....com>
> > ---
> >  drivers/cpufreq/amd-pstate.c | 5 -----
> >  1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/cpufreq/amd-pstate.c
> > b/drivers/cpufreq/amd-pstate.c index 9ac75c1cde9c..19a078e232dd 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -31,19 +31,14 @@
> >  #include <linux/compiler.h>
> >  #include <linux/dmi.h>
> >  #include <linux/slab.h>
> > -#include <linux/acpi.h>
> >  #include <linux/io.h>
> >  #include <linux/delay.h>
> >  #include <linux/uaccess.h>
> >  #include <linux/static_call.h>
> >
> > -#include <acpi/processor.h>
> >  #include <acpi/cppc_acpi.h>
> >
> >  #include <asm/msr.h>
> > -#include <asm/processor.h>
> 
> On a quick scan, I noticed that "boot_cpu_data" and "boot_cpu_has()" in the
> module init function are defined in "asm/processor.h" that is being removed
> here. It may compile for now but makes the code more fragile as explained
> above.
> 
> Please ensure that only the header files that have no definitions used in this file
> (amd-pstate.c) are dropped.

Here is the part of the headers we are working on.

==============================================
#include <linux/static_call.h>

#include <acpi/cppc_acpi.h>

#include <asm/msr.h>
#include "amd-pstate-trace.h"

============================================

I have tested the latest Linux-pm/bleeding-edge branch to verify the changes working.
As I know  the "asm/processor.h" is included  by the  "acpi/cppc_acpi.h" subpath

#include <acpi/cppc_acpi.h> 
   #include <linux/acpi.h>
         #include <acpi/acpi.h> 
              #include <asm/acpi.h>  
                       #include <asm/processor.h>
 
To make it comfortable for us, I will send V6 for this patch (1/7)

Perry.

> 
> > -#include <asm/cpufeature.h>
> > -#include <asm/cpu_device_id.h>
> >  #include "amd-pstate-trace.h"
> >
> >  #define AMD_PSTATE_TRANSITION_LATENCY        0x20000