LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH net-next 00/23] Clean driver, module and FW versions
From: Madalin Bucur (OSS) @ 2020-03-02 10:44 UTC (permalink / raw)
  To: Leon Romanovsky, David S . Miller, Jakub Kicinski
  Cc: Ajit Khaparde, Denis Kirjanov, Prashant Sreedharan,
	Govindarajulu Varadarajan, Somnath Kotur, Vishal Kulkarni,
	GR-everest-linux-l2@marvell.com, Casey Leedom, Doug Berger,
	bcm-kernel-feedback-list@broadcom.com, Linus Walleij,
	Sunil Goutham, Parvi Kaustubhi, Ariel Elior, Hans Ulli Kroll,
	Satanand Burla, Felix Manlunas, Leon Romanovsky, Claudiu Manoil,
	Florian Fainelli, Sathya Perla, Michael Chan,
	linux-arm-kernel@lists.infradead.org, Raghu Vatsavayi,
	GR-Linux-NIC-Dev@marvell.com, Andy Duan, Sriharsha Basavapatna,
	linux-parisc@vger.kernel.org, Siva Reddy Kallam, Rasesh Mody,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Leo Li,
	Hartley Sweeten, Robert Richter, Derek Chickles,
	linuxppc-dev@lists.ozlabs.org, Sudarsana Kalluru,
	Christian Benvenuti
In-Reply-To: <20200301144457.119795-1-leon@kernel.org>

> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Monday, March 2, 2020 5:02 AM
> To: leon@kernel.org
> Subject: Re: [PATCH net-next 00/23] Clean driver, module and FW versions
> 
> From: Leon Romanovsky <leon@kernel.org>
> Date: Sun,  1 Mar 2020 16:44:33 +0200
> 
> > This is second batch of the series which removes various static
> > versions in favour of globaly defined Linux kernel version.
> 
> This generally looks fine to me but I'll let it sit for a few days so
> that others can review.

Reviewed drivers/net/ethernet/freescale changes, thank you!

Reviewed-by: Madalin Bucur <madalin.bucur@oss.nxp.com>

^ permalink raw reply

* Re: [PATCH v18 00/24] selftests, powerpc, x86: Memory Protection Keys
From: Sandipan Das @ 2020-03-02 10:25 UTC (permalink / raw)
  To: shuah, skhan
  Cc: linux-arch, fweimer, linux-mm, aneesh.kumar, x86, linuxram,
	mhocko, Dave Hansen, mingo, linux-kselftest, akpm, msuchanek,
	linuxppc-dev, bauerman
In-Reply-To: <5887a858-b669-752e-b65e-ed7d7ded34aa@intel.com>

Hi Shuah,

On 31/01/20 3:21 am, Dave Hansen wrote:
> On 1/29/20 10:36 PM, Sandipan Das wrote:
>> v18:
>> 	(1) Fixed issues with x86 multilib builds based on
>> 	    feedback from Dave.
>> 	(2) Moved patch 2 to the end of the series.
> 
> These (finally) build and run successfully for me on an x86 system with
> protection keys.  Feel free to add my Tested-by, and Acked-by.
> 
> FWIW, I don't think look perfect, but my standards are lower for
> selftests/ than normal kernel code. :)
> 

Any updates on considering this for merging?

- Sandipan


^ permalink raw reply

* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: Peter Zijlstra @ 2020-03-02 10:13 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: mark.rutland, ak, maddy, alexander.shishkin, jolsa, linux-kernel,
	eranian, adrian.hunter, robert.richter, yao.jin, mingo, paulus,
	acme, namhyung, kim.phillips, linuxppc-dev, alexey.budankov,
	kan.liang
In-Reply-To: <20200302052355.36365-1-ravi.bangoria@linux.ibm.com>

On Mon, Mar 02, 2020 at 10:53:44AM +0530, Ravi Bangoria wrote:
> Modern processors export such hazard data in Performance
> Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
> Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
> AMD[3] provides similar information.
> 
> Implementation detail:
> 
> A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
> If it's set, kernel converts arch specific hazard information
> into generic format:
> 
>   struct perf_pipeline_haz_data {
>          /* Instruction/Opcode type: Load, Store, Branch .... */
>          __u8    itype;
>          /* Instruction Cache source */
>          __u8    icache;
>          /* Instruction suffered hazard in pipeline stage */
>          __u8    hazard_stage;
>          /* Hazard reason */
>          __u8    hazard_reason;
>          /* Instruction suffered stall in pipeline stage */
>          __u8    stall_stage;
>          /* Stall reason */
>          __u8    stall_reason;
>          __u16   pad;
>   };

Kim, does this format indeed work for AMD IBS?

^ permalink raw reply

* Re: [RFC 02/11] perf/core: Data structure to present hazard data
From: Peter Zijlstra @ 2020-03-02  9:55 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: mark.rutland, ak, maddy, alexander.shishkin, jolsa,
	Madhavan Srinivasan, linux-kernel, eranian, adrian.hunter,
	robert.richter, yao.jin, mingo, paulus, acme, namhyung,
	kim.phillips, linuxppc-dev, alexey.budankov, kan.liang
In-Reply-To: <20200302052355.36365-3-ravi.bangoria@linux.ibm.com>

On Mon, Mar 02, 2020 at 10:53:46AM +0530, Ravi Bangoria wrote:
> From: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> 
> Introduce new perf sample_type PERF_SAMPLE_PIPELINE_HAZ to request kernel
> to provide cpu pipeline hazard data. Also, introduce arch independent
> structure 'perf_pipeline_haz_data' to pass hazard data to userspace. This
> is generic structure and arch specific data needs to be converted to this
> format.
> 
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
>  include/linux/perf_event.h            |  7 ++++++
>  include/uapi/linux/perf_event.h       | 32 ++++++++++++++++++++++++++-
>  kernel/events/core.c                  |  6 +++++
>  tools/include/uapi/linux/perf_event.h | 32 ++++++++++++++++++++++++++-
>  4 files changed, 75 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 547773f5894e..d5b606e3c57d 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1001,6 +1001,7 @@ struct perf_sample_data {
>  	u64				stack_user_size;
>  
>  	u64				phys_addr;
> +	struct perf_pipeline_haz_data	pipeline_haz;
>  } ____cacheline_aligned;
>  
>  /* default value for data source */
> @@ -1021,6 +1022,12 @@ static inline void perf_sample_data_init(struct perf_sample_data *data,
>  	data->weight = 0;
>  	data->data_src.val = PERF_MEM_NA;
>  	data->txn = 0;
> +	data->pipeline_haz.itype = PERF_HAZ__ITYPE_NA;
> +	data->pipeline_haz.icache = PERF_HAZ__ICACHE_NA;
> +	data->pipeline_haz.hazard_stage = PERF_HAZ__PIPE_STAGE_NA;
> +	data->pipeline_haz.hazard_reason = PERF_HAZ__HREASON_NA;
> +	data->pipeline_haz.stall_stage = PERF_HAZ__PIPE_STAGE_NA;
> +	data->pipeline_haz.stall_reason = PERF_HAZ__SREASON_NA;
>  }

NAK, Don't touch anything outside of the first cacheline here.

^ permalink raw reply

* Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable
From: 王文虎 @ 2020-03-02  9:51 UTC (permalink / raw)
  To: Scott Wood
  Cc: Rai Harninder, trivial, linux-kernel, wangwenhu, Paul Mackerras,
	linuxppc-dev
In-Reply-To: <2a7cf26aa11adf43ec4f29ab733afc695039633c.camel@buserror.net>

发件人:Scott Wood <oss@buserror.net>
发送日期:2020-03-02 16:58:52
收件人:"王文虎" <wenhu.wang@vivo.com>
抄送人:wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <galak@kernel.crashing.org>,Benjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org,trivial@kernel.org,Rai Harninder <harninder.rai@nxp.com>
主题:Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On Mon, 2020-03-02 at 12:42 +0800, 王文虎 wrote:
>> 发件人:Scott Wood <oss@buserror.net>
>> 发送日期:2020-03-01 07:12:58
>> 收件人:"王文虎" <wenhu.wang@vivo.com>
>> 抄送人:wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <galak@kernel.crashing.org>,B
>> enjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <
>> paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,
>> linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org,
>> trivial@kernel.org,Rai Harninder <harninder.rai@nxp.com>
>> 主题:Re: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On
>> Tue, 2020-01-21 at 14:38 +0800, 王文虎 wrote:
>> > > 发件人:Scott Wood <oss@buserror.net>
>> > > 发送日期:2020-01-21 13:49:59
>> > > 收件人:"王文虎" <wenhu.wang@vivo.com>
>> > > 抄送人:wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <
>> > > galak@kernel.crashing.org>,B
>> > > enjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <
>> > > paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,
>> > > linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org,
>> > > trivial@kernel.org,Rai Harninder <harninder.rai@nxp.com>
>> > > 主题:Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On
>> > > Tue, 2020-01-21 at 13:20 +0800, 王文虎 wrote:
>> > > > > From: Scott Wood <oss@buserror.net>
>> > > > > Date: 2020-01-21 11:25:25
>> > > > > To:  wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <
>> > > > > galak@kernel.crashing.org>,
>> > > > > Benjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <
>> > > > > paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,
>> > > > > linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org
>> > > > > Cc:  trivial@kernel.org,wenhu.wang@vivo.com,Rai Harninder <
>> > > > > harninder.rai@nxp.com>
>> > > > > Subject: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM
>> > > > > configurable>On Mon, 2020-01-20 at 06:43 -0800, wangwenhu wrote:
>> > > > > > > From: wangwenhu <wenhu.wang@vivo.com>
>> > > > > > > 
>> > > > > > > When generating .config file with menuconfig on Freescale BOOKE
>> > > > > > > SOC, FSL_85XX_CACHE_SRAM is not configurable for the lack of
>> > > > > > > description in the Kconfig field, which makes it impossible
>> > > > > > > to support L2Cache-Sram driver. Add a description to make it
>> > > > > > > configurable.
>> > > > > > > 
>> > > > > > > Signed-off-by: wangwenhu <wenhu.wang@vivo.com>
>> > > > > > 
>> > > > > > The intent was that drivers using the SRAM API would select the
>> > > > > > symbol.  What
>> > > > > > is the use case for selecting it manually?
>> > > > > > 
>> > > > > 
>> > > > > With a repository of multiple products(meaning different defconfigs)
>> > > > > and
>> > > > > multiple
>> > > > > developers, the Kconfigs of the Kernel Source Tree change
>> > > > > frequently. So
>> > > > > the
>> > > > > "make menuconfig"
>> > > > > process is needed for defconfigs' re-generating or updating for the
>> > > > > complexity of dependencies
>> > > > > between different features defined in the Kconfigs.
>> > > > 
>> > > > That doesn't answer my question of how the SRAM code would be useful
>> > > > other
>> > > > than to some other driver that uses the API (which would use
>> > > > "select").  There
>> > > > is no userspace API.  You could use the kernel command line to
>> > > > configure
>> > > > the
>> > > > SRAM but you need to get the address of it for it to be useful.
>> > > > 
>> > > 
>> > > Like you've asked below, via /dev/mem or direct calling within the
>> > > Kernel.
>> > > And they are not submitted yes, under development.
>> > 
>> > If they are calling within the kernel, then whatever driver that is should
>> > select FSL_85XX_CACHE_SRAM.  Directly accessing /dev/mem without any way
>> > for
>> > the kernel to advertise where it is or which parts of SRAM are available
>> > for
>> > use sounds like a bad idea.
>> > 
>> 
>> Yes, definitely. So like we enable the moulde which should selet 
>> FSL_85XX_CACHE_SRAM to build vmlinux, FSL_85XX_CACHE_SRAM 
>> could not be seleted because of the Kconfig definition problem 
>> which I am trying to fix now.  So would you please merge the patch 
>> for the convenience of later works depending on the driver.
>
>Sorry, I don't think it's something that should be enabled by itself with
>nothing using the allocators.  Suppose we took this patch, and people enabled
>it and accessed it via /dev/mem.  Then suppose a driver is patched to allocate
>some sram and use it.  They'd be stepping on each others' toes undetected.
>
Right, and maybe i did not explain it clear: I mean that we are developing
modules both in kernel which call the interfaces of FSL_85XX_CACHE_SRAM 
directly, and in user space which is a further consideration upon the work
we have done. Cause we have not exported the code under developing, it 
seems like that nothing uses FSL_85XX_CACHE_SRAM.

>If you want to expose it to userspace, add code that allocates some or all of
>the sram and make it something userspace can mmap.  Or, if nothing's going to
>use them, remove the allocators and export the entire thing to userspace
>(again via an sram-specific mappable rather than /dev/mem).

As for /dev/mem, it was one of our choices ever but now a user-space 
driver is preferred for further consideration. But currently, the functionality 
that directly calls the insterfaces of FSL_85XX_CACHE_SRAM have been 
under developing for couples of days and would be exported in the future. 
They would be used on hw-platforms like PPCe500.

Just some time it takes. 

Do you mean the exporting is a pre-condition? If not, the merge would 
do a favor for the convenience.

Wenhu
 
>
>-Scott
>
>



^ permalink raw reply

* Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
From: Jason Yan @ 2020-03-02  9:37 UTC (permalink / raw)
  To: Scott Wood, Daniel Axtens, mpe, linuxppc-dev, diana.craciun,
	christophe.leroy, benh, paulus, npiggin, keescook,
	kernel-hardening, me
  Cc: linux-kernel, zhaohongjiang
In-Reply-To: <7f608c18250c509ff091990d4bb460846fae11a0.camel@buserror.net>



在 2020/3/2 16:47, Scott Wood 写道:
> On Mon, 2020-03-02 at 15:12 +0800, Jason Yan wrote:
>>
>> 在 2020/3/2 11:24, Scott Wood 写道:
>>> On Mon, 2020-03-02 at 10:17 +0800, Jason Yan wrote:
>>>>
>>>> 在 2020/3/1 6:54, Scott Wood 写道:
>>>>> On Sat, 2020-02-29 at 15:27 +0800, Jason Yan wrote:
>>>>>>
>>>>>> Turnning to %p may not be a good idea in this situation. So
>>>>>> for the REG logs printed when dumping stack, we can disable it when
>>>>>> KASLR is open. For the REG logs in other places like show_regs(),
>>>>>> only
>>>>>> privileged can trigger it, and they are not combind with a symbol,
>>>>>> so
>>>>>> I think it's ok to keep them.
>>>>>>
>>>>>> diff --git a/arch/powerpc/kernel/process.c
>>>>>> b/arch/powerpc/kernel/process.c
>>>>>> index fad50db9dcf2..659c51f0739a 100644
>>>>>> --- a/arch/powerpc/kernel/process.c
>>>>>> +++ b/arch/powerpc/kernel/process.c
>>>>>> @@ -2068,7 +2068,10 @@ void show_stack(struct task_struct *tsk,
>>>>>> unsigned
>>>>>> long *stack)
>>>>>>                     newsp = stack[0];
>>>>>>                     ip = stack[STACK_FRAME_LR_SAVE];
>>>>>>                     if (!firstframe || ip != lr) {
>>>>>> -                       printk("["REG"] ["REG"] %pS", sp, ip, (void
>>>>>> *)ip);
>>>>>> +                       if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
>>>>>> +                               printk("%pS", (void *)ip);
>>>>>> +                       else
>>>>>> +                               printk("["REG"] ["REG"] %pS", sp,
>>>>>> ip,
>>>>>> (void *)ip);
>>>>>
>>>>> This doesn't deal with "nokaslr" on the kernel command line.  It also
>>>>> doesn't
>>>>> seem like something that every callsite should have to opencode,
>>>>> versus
>>>>> having
>>>>> an appropriate format specifier behaves as I described above (and I
>>>>> still
>>>>> don't see why that format specifier should not be "%p").
>>>>>
>>>>
>>>> Actually I still do not understand why we should print the raw value
>>>> here. When KALLSYMS is enabled we have symbol name  and  offset like
>>>> put_cred_rcu+0x108/0x110, and when KALLSYMS is disabled we have the raw
>>>> address.
>>>
>>> I'm more concerned about the stack address for wading through a raw stack
>>> dump
>>> (to find function call arguments, etc).  The return address does help
>>> confirm
>>> that I'm on the right stack frame though, and also makes looking up a line
>>> number slightly easier than having to look up a symbol address and then
>>> add
>>> the offset (at least for non-module addresses).
>>>
>>> As a random aside, the mismatch between Linux printing a hex offset and
>>> GDB
>>> using decimal in disassembly is annoying...
>>>
>>
>> OK, I will send a RFC patch to add a new format specifier such as "%pk"
>> or change the exsiting "%pK" to print raw value of addresses when KASLR
>> is disabled and print hash value of addresses when KASLR is enabled.
>> Let's see what the printk guys would say :)
> 
> I'm not sure that a new format specifier is needed versus changing the
> behavior of "%p", and "%pK" definitely doesn't seem suitable given that it's
> intended to be more restricted than "%p" (see commit ef0010a30935de4).  The
> question is whether there is a legitimate reason to hash in the absence of
> kaslr.
> 

The problem is that if we change the behavior of "%p", we have to turn
all exsiting "%p" to "%pK". Hashing is still reasonable when there is no
kaslr because some architectures support randomize at build time such as 
arm64.


> -Scott
> 
> 
> 
> .
> 


^ permalink raw reply

* Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable
From: Scott Wood @ 2020-03-02  8:58 UTC (permalink / raw)
  To: 王文虎
  Cc: Rai Harninder, trivial, linux-kernel, wangwenhu, Paul Mackerras,
	linuxppc-dev
In-Reply-To: <AKMA9QAgCCx9CEivazbGIap-.3.1583124159966.Hmail.wenhu.wang@vivo.com>

On Mon, 2020-03-02 at 12:42 +0800, 王文虎 wrote:
> 发件人:Scott Wood <oss@buserror.net>
> 发送日期:2020-03-01 07:12:58
> 收件人:"王文虎" <wenhu.wang@vivo.com>
> 抄送人:wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <galak@kernel.crashing.org>,B
> enjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <
> paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,
> linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org,
> trivial@kernel.org,Rai Harninder <harninder.rai@nxp.com>
> 主题:Re: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On
> Tue, 2020-01-21 at 14:38 +0800, 王文虎 wrote:
> > > 发件人:Scott Wood <oss@buserror.net>
> > > 发送日期:2020-01-21 13:49:59
> > > 收件人:"王文虎" <wenhu.wang@vivo.com>
> > > 抄送人:wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <
> > > galak@kernel.crashing.org>,B
> > > enjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <
> > > paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,
> > > linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org,
> > > trivial@kernel.org,Rai Harninder <harninder.rai@nxp.com>
> > > 主题:Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On
> > > Tue, 2020-01-21 at 13:20 +0800, 王文虎 wrote:
> > > > > From: Scott Wood <oss@buserror.net>
> > > > > Date: 2020-01-21 11:25:25
> > > > > To:  wangwenhu <wenhu.pku@gmail.com>,Kumar Gala <
> > > > > galak@kernel.crashing.org>,
> > > > > Benjamin Herrenschmidt <benh@kernel.crashing.org>,Paul Mackerras <
> > > > > paulus@samba.org>,Michael Ellerman <mpe@ellerman.id.au>,
> > > > > linuxppc-dev@lists.ozlabs.org,linux-kernel@vger.kernel.org
> > > > > Cc:  trivial@kernel.org,wenhu.wang@vivo.com,Rai Harninder <
> > > > > harninder.rai@nxp.com>
> > > > > Subject: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM
> > > > > configurable>On Mon, 2020-01-20 at 06:43 -0800, wangwenhu wrote:
> > > > > > > From: wangwenhu <wenhu.wang@vivo.com>
> > > > > > > 
> > > > > > > When generating .config file with menuconfig on Freescale BOOKE
> > > > > > > SOC, FSL_85XX_CACHE_SRAM is not configurable for the lack of
> > > > > > > description in the Kconfig field, which makes it impossible
> > > > > > > to support L2Cache-Sram driver. Add a description to make it
> > > > > > > configurable.
> > > > > > > 
> > > > > > > Signed-off-by: wangwenhu <wenhu.wang@vivo.com>
> > > > > > 
> > > > > > The intent was that drivers using the SRAM API would select the
> > > > > > symbol.  What
> > > > > > is the use case for selecting it manually?
> > > > > > 
> > > > > 
> > > > > With a repository of multiple products(meaning different defconfigs)
> > > > > and
> > > > > multiple
> > > > > developers, the Kconfigs of the Kernel Source Tree change
> > > > > frequently. So
> > > > > the
> > > > > "make menuconfig"
> > > > > process is needed for defconfigs' re-generating or updating for the
> > > > > complexity of dependencies
> > > > > between different features defined in the Kconfigs.
> > > > 
> > > > That doesn't answer my question of how the SRAM code would be useful
> > > > other
> > > > than to some other driver that uses the API (which would use
> > > > "select").  There
> > > > is no userspace API.  You could use the kernel command line to
> > > > configure
> > > > the
> > > > SRAM but you need to get the address of it for it to be useful.
> > > > 
> > > 
> > > Like you've asked below, via /dev/mem or direct calling within the
> > > Kernel.
> > > And they are not submitted yes, under development.
> > 
> > If they are calling within the kernel, then whatever driver that is should
> > select FSL_85XX_CACHE_SRAM.  Directly accessing /dev/mem without any way
> > for
> > the kernel to advertise where it is or which parts of SRAM are available
> > for
> > use sounds like a bad idea.
> > 
> 
> Yes, definitely. So like we enable the moulde which should selet 
> FSL_85XX_CACHE_SRAM to build vmlinux, FSL_85XX_CACHE_SRAM 
> could not be seleted because of the Kconfig definition problem 
> which I am trying to fix now.  So would you please merge the patch 
> for the convenience of later works depending on the driver.

Sorry, I don't think it's something that should be enabled by itself with
nothing using the allocators.  Suppose we took this patch, and people enabled
it and accessed it via /dev/mem.  Then suppose a driver is patched to allocate
some sram and use it.  They'd be stepping on each others' toes undetected.

If you want to expose it to userspace, add code that allocates some or all of
the sram and make it something userspace can mmap.  Or, if nothing's going to
use them, remove the allocators and export the entire thing to userspace
(again via an sram-specific mappable rather than /dev/mem).

-Scott



^ permalink raw reply

* Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
From: Scott Wood @ 2020-03-02  8:47 UTC (permalink / raw)
  To: Jason Yan, Daniel Axtens, mpe, linuxppc-dev, diana.craciun,
	christophe.leroy, benh, paulus, npiggin, keescook,
	kernel-hardening, me
  Cc: linux-kernel, zhaohongjiang
In-Reply-To: <17658c2b-9eb8-cee9-e9a2-93d316a401b1@huawei.com>

On Mon, 2020-03-02 at 15:12 +0800, Jason Yan wrote:
> 
> 在 2020/3/2 11:24, Scott Wood 写道:
> > On Mon, 2020-03-02 at 10:17 +0800, Jason Yan wrote:
> > > 
> > > 在 2020/3/1 6:54, Scott Wood 写道:
> > > > On Sat, 2020-02-29 at 15:27 +0800, Jason Yan wrote:
> > > > > 
> > > > > Turnning to %p may not be a good idea in this situation. So
> > > > > for the REG logs printed when dumping stack, we can disable it when
> > > > > KASLR is open. For the REG logs in other places like show_regs(),
> > > > > only
> > > > > privileged can trigger it, and they are not combind with a symbol,
> > > > > so
> > > > > I think it's ok to keep them.
> > > > > 
> > > > > diff --git a/arch/powerpc/kernel/process.c
> > > > > b/arch/powerpc/kernel/process.c
> > > > > index fad50db9dcf2..659c51f0739a 100644
> > > > > --- a/arch/powerpc/kernel/process.c
> > > > > +++ b/arch/powerpc/kernel/process.c
> > > > > @@ -2068,7 +2068,10 @@ void show_stack(struct task_struct *tsk,
> > > > > unsigned
> > > > > long *stack)
> > > > >                    newsp = stack[0];
> > > > >                    ip = stack[STACK_FRAME_LR_SAVE];
> > > > >                    if (!firstframe || ip != lr) {
> > > > > -                       printk("["REG"] ["REG"] %pS", sp, ip, (void
> > > > > *)ip);
> > > > > +                       if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
> > > > > +                               printk("%pS", (void *)ip);
> > > > > +                       else
> > > > > +                               printk("["REG"] ["REG"] %pS", sp,
> > > > > ip,
> > > > > (void *)ip);
> > > > 
> > > > This doesn't deal with "nokaslr" on the kernel command line.  It also
> > > > doesn't
> > > > seem like something that every callsite should have to opencode,
> > > > versus
> > > > having
> > > > an appropriate format specifier behaves as I described above (and I
> > > > still
> > > > don't see why that format specifier should not be "%p").
> > > > 
> > > 
> > > Actually I still do not understand why we should print the raw value
> > > here. When KALLSYMS is enabled we have symbol name  and  offset like
> > > put_cred_rcu+0x108/0x110, and when KALLSYMS is disabled we have the raw
> > > address.
> > 
> > I'm more concerned about the stack address for wading through a raw stack
> > dump
> > (to find function call arguments, etc).  The return address does help
> > confirm
> > that I'm on the right stack frame though, and also makes looking up a line
> > number slightly easier than having to look up a symbol address and then
> > add
> > the offset (at least for non-module addresses).
> > 
> > As a random aside, the mismatch between Linux printing a hex offset and
> > GDB
> > using decimal in disassembly is annoying...
> > 
> 
> OK, I will send a RFC patch to add a new format specifier such as "%pk" 
> or change the exsiting "%pK" to print raw value of addresses when KASLR 
> is disabled and print hash value of addresses when KASLR is enabled. 
> Let's see what the printk guys would say :)

I'm not sure that a new format specifier is needed versus changing the
behavior of "%p", and "%pK" definitely doesn't seem suitable given that it's
intended to be more restricted than "%p" (see commit ef0010a30935de4).  The
question is whether there is a legitimate reason to hash in the absence of
kaslr.

-Scott



^ permalink raw reply

* Re: [RFC 1/3] mm/vma: Define a default value for VM_DATA_DEFAULT_FLAGS
From: Geert Uytterhoeven @ 2020-03-02  8:22 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Rich Felker, linux-ia64@vger.kernel.org, Linux-sh list,
	Catalin Marinas, Heiko Carstens, linux-mips, James E.J. Bottomley,
	Linux MM, Guo Ren, sparclinux, open list:QUALCOMM HEXAGON...,
	linux-riscv, Jonas Bonn, linux-s390, arcml, linux-c6x-dev,
	Brian Cain, Russell King, Ley Foon Tan, Parisc List, Mark Salter,
	Paul Burton, moderated list:H8/300 ARCHITECTURE,
	open list:TENSILICA XTENSA PORT (xtensa), Jeff Dike, linux-um,
	linux-m68k, Openrisc, Paul Walmsley, Thomas Gleixner, Guan Xuetao,
	Linux ARM, Richard Henderson, Chris Zankel, Michal Simek,
	Tony Luck, Yoshinori Sato, Nick Hu, Vineet Gupta,
	Linux Kernel Mailing List, Ralf Baechle, alpha, nios2-dev,
	Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <1583131666-15531-2-git-send-email-anshuman.khandual@arm.com>

On Mon, Mar 2, 2020 at 7:48 AM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
> There are many platforms with exact same value for VM_DATA_DEFAULT_FLAGS
> This creates a default value for VM_DATA_DEFAULT_FLAGS in line with the
> existing VM_STACK_DEFAULT_FLAGS. While here, also define some more macros
> with standard VMA access flag combinations that are used frequently across
> many platforms. Apart from simplification, this reduces code duplication
> as well.

> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>

>  arch/m68k/include/asm/page.h       |  3 ---

For m68k:
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
From: Hari Bathini @ 2020-03-02  8:07 UTC (permalink / raw)
  To: Pingfan Liu, linuxppc-dev; +Cc: Paul Mackerras, kexec
In-Reply-To: <1581389982-5701-2-git-send-email-kernelfans@gmail.com>



On 11/02/20 8:29 AM, Pingfan Liu wrote:
> A bug is observed on pseries by taking the following steps on rhel:
> -1. drmgr -c mem -r -q 5
> -2. echo c > /proc/sysrq-trigger
> 
> And then, the failure looks like:
> kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/
> kdump: saving vmcore-dmesg.txt
> kdump: saving vmcore-dmesg.txt complete
> kdump: saving vmcore
>  Checking for memory holes                         : [  0.0 %] /                   Checking for memory holes                         : [100.0 %] |                   Excluding unnecessary pages                       : [100.0 %] \                   Copying data                                      : [  0.3 %] -          eta: 38s[   44.337636] hash-mmu: mm: Hashing failure ! EA=0x7fffba400000 access=0x8000000000000004 current=makedumpfile
> [   44.337663] hash-mmu:     trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc000000050000504
> [   44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba400000 access=0x8000000000000004 current=makedumpfile
> [   44.337692] hash-mmu:     trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc000000050000504
> [   44.337708] makedumpfile[469]: unhandled signal 7 at 00007fffba400000 nip 00007fffbbc4d7fc lr 000000011356ca3c code 2
> [   44.338548] Core dump to |/bin/false pipe failed
> /lib/kdump-lib-initramfs.sh: line 98:   469 Bus error               $CORE_COLLECTOR /proc/vmcore $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete
> kdump: saving vmcore failed
> 
> * Root cause *
>   After analyzing, it turns out that in the current implementation,
> when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating as
> the code __remove_memory() comes before drmem_update_dt().
> 
> From a viewpoint of listener and publisher, the publisher notifies the
> listener before data is ready.  This introduces a problem where udev
> launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before
> updating. And in capture kernel, makedumpfile will access the memory based
> on the stale dt info, and hit a SIGBUS error due to an un-existed lmb.
> 
> * Fix *
>   In order to fix this issue, update dt before __remove_memory(), and
> accordingly the same rule in hot-add path.
> 
> This will introduce extra dt updating payload for each involved lmb when hotplug.
> But it should be fine since drmem_update_dt() is memory based operation and
> hotplug is not a hot path.
> 
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> To: linuxppc-dev@lists.ozlabs.org
> Cc: kexec@lists.infradead.org

KDump fails to capture vmcore as we end up looking at a stale elfcore hdr
with udev event happening before DT update. Resolved with these patches.
For the series:

Tested-by: Hari Bathini <hbathini@linux.ibm.com>


^ permalink raw reply

* Re: [PATCH v3 26/27] powerpc/powernv/pmem: Expose the firmware version in sysfs
From: Andrew Donnellan @ 2020-03-02  7:35 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
	Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
	Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
	Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
	Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
	Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-27-alastair@au1.ibm.com>

On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> This information will be used by ndctl in userspace to help users identify
> the device.

You should include the information from the subject line in the body of 
the commit message too.

I think this patch could probably be squashed in with the last one.

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited


^ permalink raw reply

* Reg: KASLR backporting to 4.9 kernels for ppc platform.
From: Bala murugan @ 2020-03-02  5:57 UTC (permalink / raw)
  To: linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 136 bytes --]

Hi,

Any plans to back port KASLR for 4.9 kernel. How feasible is to back port
to 4.9 kernel for PPC platforms.

Regards
S Balamurugan.

[-- Attachment #2: Type: text/html, Size: 235 bytes --]

^ permalink raw reply

* Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
From: Jason Yan @ 2020-03-02  7:12 UTC (permalink / raw)
  To: Scott Wood, Daniel Axtens, mpe, linuxppc-dev, diana.craciun,
	christophe.leroy, benh, paulus, npiggin, keescook,
	kernel-hardening, me
  Cc: linux-kernel, zhaohongjiang
In-Reply-To: <31b5966ba579ef246176a7d8ad18c2c02788dd27.camel@buserror.net>



在 2020/3/2 11:24, Scott Wood 写道:
> On Mon, 2020-03-02 at 10:17 +0800, Jason Yan wrote:
>>
>> 在 2020/3/1 6:54, Scott Wood 写道:
>>> On Sat, 2020-02-29 at 15:27 +0800, Jason Yan wrote:
>>>>
>>>> Turnning to %p may not be a good idea in this situation. So
>>>> for the REG logs printed when dumping stack, we can disable it when
>>>> KASLR is open. For the REG logs in other places like show_regs(), only
>>>> privileged can trigger it, and they are not combind with a symbol, so
>>>> I think it's ok to keep them.
>>>>
>>>> diff --git a/arch/powerpc/kernel/process.c
>>>> b/arch/powerpc/kernel/process.c
>>>> index fad50db9dcf2..659c51f0739a 100644
>>>> --- a/arch/powerpc/kernel/process.c
>>>> +++ b/arch/powerpc/kernel/process.c
>>>> @@ -2068,7 +2068,10 @@ void show_stack(struct task_struct *tsk, unsigned
>>>> long *stack)
>>>>                    newsp = stack[0];
>>>>                    ip = stack[STACK_FRAME_LR_SAVE];
>>>>                    if (!firstframe || ip != lr) {
>>>> -                       printk("["REG"] ["REG"] %pS", sp, ip, (void
>>>> *)ip);
>>>> +                       if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
>>>> +                               printk("%pS", (void *)ip);
>>>> +                       else
>>>> +                               printk("["REG"] ["REG"] %pS", sp, ip,
>>>> (void *)ip);
>>>
>>> This doesn't deal with "nokaslr" on the kernel command line.  It also
>>> doesn't
>>> seem like something that every callsite should have to opencode, versus
>>> having
>>> an appropriate format specifier behaves as I described above (and I still
>>> don't see why that format specifier should not be "%p").
>>>
>>
>> Actually I still do not understand why we should print the raw value
>> here. When KALLSYMS is enabled we have symbol name  and  offset like
>> put_cred_rcu+0x108/0x110, and when KALLSYMS is disabled we have the raw
>> address.
> 
> I'm more concerned about the stack address for wading through a raw stack dump
> (to find function call arguments, etc).  The return address does help confirm
> that I'm on the right stack frame though, and also makes looking up a line
> number slightly easier than having to look up a symbol address and then add
> the offset (at least for non-module addresses).
> 
> As a random aside, the mismatch between Linux printing a hex offset and GDB
> using decimal in disassembly is annoying...
> 

OK, I will send a RFC patch to add a new format specifier such as "%pk" 
or change the exsiting "%pK" to print raw value of addresses when KASLR 
is disabled and print hash value of addresses when KASLR is enabled. 
Let's see what the printk guys would say :)


> -Scott
> 
> 
> 
> .
> 


^ permalink raw reply

* [RFC 2/3] mm/vma: Introduce VM_ACCESS_FLAGS
From: Anshuman Khandual @ 2020-03-02  6:47 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-sh, Catalin Marinas, Dave Hansen, Heiko Carstens, devel,
	linux-s390, linux-c6x-dev, Yoshinori Sato, Russell King,
	Ley Foon Tan, Mark Salter, Anshuman Khandual, Rob Springer,
	Thomas Gleixner, Guan Xuetao, linux-arm-kernel, Nick Hu,
	Greg Kroah-Hartman, linux-kernel, nios2-dev, Andrew Morton,
	linuxppc-dev
In-Reply-To: <1583131666-15531-1-git-send-email-anshuman.khandual@arm.com>

There are many places where all basic VMA access flags (read, write, exec)
are initialized or checked against as a group. One such example is during
page fault. Existing vma_is_accessible() wrapper already creates the notion
of VMA accessibility as a group access permissions. Hence lets just create
VM_ACCESS_FLAGS (VM_READ|VM_WRITE|VM_EXEC) which will not only reduce code
duplication but also extend the VMA accessibility concept in general.

Cc: Russell King <linux@armlinux.org.uk>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Mark Salter <msalter@redhat.com>
Cc: Nick Hu <nickhu@andestech.com>
CC: Ley Foon Tan <ley.foon.tan@intel.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Guan Xuetao <gxt@pku.edu.cn>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Springer <rspringer@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-c6x-dev@linux-c6x.org
Cc: nios2-dev@lists.rocketboards.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: devel@driverdev.osuosl.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm/mm/fault.c                  | 2 +-
 arch/arm64/mm/fault.c                | 2 +-
 arch/c6x/include/asm/processor.h     | 2 +-
 arch/nds32/mm/fault.c                | 2 +-
 arch/nios2/include/asm/processor.h   | 2 +-
 arch/powerpc/mm/book3s64/pkeys.c     | 2 +-
 arch/s390/mm/fault.c                 | 2 +-
 arch/sh/include/asm/processor_64.h   | 2 +-
 arch/unicore32/mm/fault.c            | 2 +-
 arch/x86/mm/pkeys.c                  | 2 +-
 drivers/staging/gasket/gasket_core.c | 2 +-
 include/linux/mm.h                   | 4 +++-
 mm/mmap.c                            | 4 ++--
 mm/mprotect.c                        | 7 +++----
 14 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index bd0f4821f7e1..2c71028d9d6b 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -189,7 +189,7 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
  */
 static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma)
 {
-	unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+	unsigned int mask = VM_ACCESS_FLAGS;
 
 	if ((fsr & FSR_WRITE) && !(fsr & FSR_CM))
 		mask = VM_WRITE;
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 85566d32958f..63f31206a12e 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -445,7 +445,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	const struct fault_info *inf;
 	struct mm_struct *mm = current->mm;
 	vm_fault_t fault, major = 0;
-	unsigned long vm_flags = VM_READ | VM_WRITE | VM_EXEC;
+	unsigned long vm_flags = VM_ACCESS_FLAGS;
 	unsigned int mm_flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
 
 	if (kprobe_page_fault(regs, esr))
diff --git a/arch/c6x/include/asm/processor.h b/arch/c6x/include/asm/processor.h
index 1456f5e11de3..77372b8c28d7 100644
--- a/arch/c6x/include/asm/processor.h
+++ b/arch/c6x/include/asm/processor.h
@@ -57,7 +57,7 @@ struct thread_struct {
 }
 
 #define INIT_MMAP { \
-	&init_mm, 0, 0, NULL, PAGE_SHARED, VM_READ | VM_WRITE | VM_EXEC, 1, \
+	&init_mm, 0, 0, NULL, PAGE_SHARED, VM_ACCESS_FLAGS, 1, \
 	NULL, NULL }
 
 #define task_pt_regs(task) \
diff --git a/arch/nds32/mm/fault.c b/arch/nds32/mm/fault.c
index 906dfb25353c..55387a31bf42 100644
--- a/arch/nds32/mm/fault.c
+++ b/arch/nds32/mm/fault.c
@@ -79,7 +79,7 @@ void do_page_fault(unsigned long entry, unsigned long addr,
 	struct vm_area_struct *vma;
 	int si_code;
 	vm_fault_t fault;
-	unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+	unsigned int mask = VM_ACCESS_FLAGS;
 	unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
 
 	error_code = error_code & (ITYPE_mskINST | ITYPE_mskETYPE);
diff --git a/arch/nios2/include/asm/processor.h b/arch/nios2/include/asm/processor.h
index 94bcb86f679f..fbfb3ab14cfc 100644
--- a/arch/nios2/include/asm/processor.h
+++ b/arch/nios2/include/asm/processor.h
@@ -51,7 +51,7 @@ struct thread_struct {
 };
 
 #define INIT_MMAP \
-	{ &init_mm, (0), (0), __pgprot(0x0), VM_READ | VM_WRITE | VM_EXEC }
+	{ &init_mm, (0), (0), __pgprot(0x0), VM_ACCESS_FLAGS }
 
 # define INIT_THREAD {			\
 	.kregs	= NULL,			\
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 59e0ebbd8036..11fd52b24f68 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -315,7 +315,7 @@ int __execute_only_pkey(struct mm_struct *mm)
 static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma)
 {
 	/* Do this check first since the vm_flags should be hot */
-	if ((vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)) != VM_EXEC)
+	if ((vma->vm_flags & VM_ACCESS_FLAGS) != VM_EXEC)
 		return false;
 
 	return (vma_pkey(vma) == vma->vm_mm->context.execute_only_pkey);
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index 7b0bb475c166..b2cb3c0d0e1a 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -584,7 +584,7 @@ void do_dat_exception(struct pt_regs *regs)
 	int access;
 	vm_fault_t fault;
 
-	access = VM_READ | VM_EXEC | VM_WRITE;
+	access = VM_ACCESS_FLAGS;
 	fault = do_exception(regs, access);
 	if (unlikely(fault))
 		do_fault_error(regs, access, fault);
diff --git a/arch/sh/include/asm/processor_64.h b/arch/sh/include/asm/processor_64.h
index 53efc9f51ef1..3b8187284e3f 100644
--- a/arch/sh/include/asm/processor_64.h
+++ b/arch/sh/include/asm/processor_64.h
@@ -121,7 +121,7 @@ struct thread_struct {
 };
 
 #define INIT_MMAP \
-{ &init_mm, 0, 0, NULL, PAGE_SHARED, VM_READ | VM_WRITE | VM_EXEC, 1, NULL, NULL }
+{ &init_mm, 0, 0, NULL, PAGE_SHARED, VM_ACCESS_FLAGS, 1, NULL, NULL }
 
 #define INIT_THREAD  {				\
 	.sp		= sizeof(init_stack) +	\
diff --git a/arch/unicore32/mm/fault.c b/arch/unicore32/mm/fault.c
index 76342de9cf8c..fc27c274d358 100644
--- a/arch/unicore32/mm/fault.c
+++ b/arch/unicore32/mm/fault.c
@@ -149,7 +149,7 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
  */
 static inline bool access_error(unsigned int fsr, struct vm_area_struct *vma)
 {
-	unsigned int mask = VM_READ | VM_WRITE | VM_EXEC;
+	unsigned int mask = VM_ACCESS_FLAGS;
 
 	if (!(fsr ^ 0x12))	/* write? */
 		mask = VM_WRITE;
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index c6f84c0b5d7a..8873ed1438a9 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -63,7 +63,7 @@ int __execute_only_pkey(struct mm_struct *mm)
 static inline bool vma_is_pkey_exec_only(struct vm_area_struct *vma)
 {
 	/* Do this check first since the vm_flags should be hot */
-	if ((vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)) != VM_EXEC)
+	if ((vma->vm_flags & VM_ACCESS_FLAGS) != VM_EXEC)
 		return false;
 	if (vma_pkey(vma) != vma->vm_mm->context.execute_only_pkey)
 		return false;
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index be6b50f454b4..81bb7d58dc49 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -689,7 +689,7 @@ static bool gasket_mmap_has_permissions(struct gasket_dev *gasket_dev,
 
 	/* Make sure that no wrong flags are set. */
 	requested_permissions =
-		(vma->vm_flags & (VM_WRITE | VM_READ | VM_EXEC));
+		(vma->vm_flags & VM_ACCESS_FLAGS);
 	if (requested_permissions & ~(bar_permissions)) {
 		dev_dbg(gasket_dev->dev,
 			"Attempting to map a region with requested permissions "
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7a764ae6ab68..525026df1e58 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -368,6 +368,8 @@ extern unsigned int kobjsize(const void *objp);
 #endif
 
 #define VM_STACK_FLAGS	(VM_STACK | VM_STACK_DEFAULT_FLAGS | VM_ACCOUNT)
+#define VM_ACCESS_FLAGS (VM_READ | VM_WRITE | VM_EXEC)
+
 
 /*
  * Special vmas that are non-mergable, non-mlock()able.
@@ -558,7 +560,7 @@ static inline bool vma_is_anonymous(struct vm_area_struct *vma)
 
 static inline bool vma_is_accessible(struct vm_area_struct *vma)
 {
-	return vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
+	return vma->vm_flags & VM_ACCESS_FLAGS;
 }
 
 #ifdef CONFIG_SHMEM
diff --git a/mm/mmap.c b/mm/mmap.c
index 0d295f49b24d..f9a01763857b 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -106,7 +106,7 @@ static inline pgprot_t arch_filter_pgprot(pgprot_t prot)
 pgprot_t vm_get_page_prot(unsigned long vm_flags)
 {
 	pgprot_t ret = __pgprot(pgprot_val(protection_map[vm_flags &
-				(VM_READ|VM_WRITE|VM_EXEC|VM_SHARED)]) |
+				(VM_ACCESS_FLAGS | VM_SHARED)]) |
 			pgprot_val(arch_vm_get_page_prot(vm_flags)));
 
 	return arch_filter_pgprot(ret);
@@ -1221,7 +1221,7 @@ static int anon_vma_compatible(struct vm_area_struct *a, struct vm_area_struct *
 	return a->vm_end == b->vm_start &&
 		mpol_equal(vma_policy(a), vma_policy(b)) &&
 		a->vm_file == b->vm_file &&
-		!((a->vm_flags ^ b->vm_flags) & ~(VM_READ|VM_WRITE|VM_EXEC|VM_SOFTDIRTY)) &&
+		!((a->vm_flags ^ b->vm_flags) & ~(VM_ACCESS_FLAGS | VM_SOFTDIRTY)) &&
 		b->vm_pgoff == a->vm_pgoff + ((b->vm_start - a->vm_start) >> PAGE_SHIFT);
 }
 
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 7a8e84f86831..4921a4211c6b 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -359,7 +359,7 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 	 */
 	if (arch_has_pfn_modify_check() &&
 	    (vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)) &&
-	    (newflags & (VM_READ|VM_WRITE|VM_EXEC)) == 0) {
+	    (newflags & VM_ACCESS_FLAGS) == 0) {
 		pgprot_t new_pgprot = vm_get_page_prot(newflags);
 
 		error = walk_page_range(current->mm, start, end,
@@ -530,15 +530,14 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
 		 * If a permission is not passed to mprotect(), it must be
 		 * cleared from the VMA.
 		 */
-		mask_off_old_flags = VM_READ | VM_WRITE | VM_EXEC |
-					VM_FLAGS_CLEAR;
+		mask_off_old_flags = VM_ACCESS_FLAGS | VM_FLAGS_CLEAR;
 
 		new_vma_pkey = arch_override_mprotect_pkey(vma, prot, pkey);
 		newflags = calc_vm_prot_bits(prot, new_vma_pkey);
 		newflags |= (vma->vm_flags & ~mask_off_old_flags);
 
 		/* newflags >> 4 shift VM_MAY% in place of VM_% */
-		if ((newflags & ~(newflags >> 4)) & (VM_READ | VM_WRITE | VM_EXEC)) {
+		if ((newflags & ~(newflags >> 4)) & VM_ACCESS_FLAGS) {
 			error = -EACCES;
 			goto out;
 		}
-- 
2.20.1


^ permalink raw reply related

* [RFC 1/3] mm/vma: Define a default value for VM_DATA_DEFAULT_FLAGS
From: Anshuman Khandual @ 2020-03-02  6:47 UTC (permalink / raw)
  To: linux-mm
  Cc: Rich Felker, linux-ia64, linux-sh, Catalin Marinas,
	Heiko Carstens, linux-mips, James E.J. Bottomley, Guo Ren,
	sparclinux, linux-hexagon, linux-riscv, Jonas Bonn, linux-s390,
	linux-snps-arc, linux-c6x-dev, Brian Cain, Russell King,
	Ley Foon Tan, Geert Uytterhoeven, linux-parisc, Mark Salter,
	Paul Burton, uclinux-h8-devel, linux-xtensa, Anshuman Khandual,
	Jeff Dike, linux-um, linux-m68k, openrisc, Paul Walmsley,
	Thomas Gleixner, Guan Xuetao, linux-arm-kernel, Richard Henderson,
	Chris Zankel, Michal Simek, Tony Luck, Yoshinori Sato, Nick Hu,
	Vineet Gupta, linux-kernel, Ralf Baechle, linux-alpha, nios2-dev,
	Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <1583131666-15531-1-git-send-email-anshuman.khandual@arm.com>

There are many platforms with exact same value for VM_DATA_DEFAULT_FLAGS
This creates a default value for VM_DATA_DEFAULT_FLAGS in line with the
existing VM_STACK_DEFAULT_FLAGS. While here, also define some more macros
with standard VMA access flag combinations that are used frequently across
many platforms. Apart from simplification, this reduces code duplication
as well.

Cc: Richard Henderson <rth@twiddle.net>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Salter <msalter@redhat.com>
Cc: Guo Ren <guoren@kernel.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Brian Cain <bcain@codeaurora.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Michal Simek <monstr@monstr.eu>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Nick Hu <nickhu@andestech.com>
Cc: Ley Foon Tan <ley.foon.tan@intel.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Rich Felker <dalias@libc.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Guan Xuetao <gxt@pku.edu.cn>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: Chris Zankel <chris@zankel.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-alpha@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-c6x-dev@linux-c6x.org
Cc: uclinux-h8-devel@lists.sourceforge.jp
Cc: linux-hexagon@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: nios2-dev@lists.rocketboards.org
Cc: openrisc@lists.librecores.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-um@lists.infradead.org
Cc: linux-xtensa@linux-xtensa.org
Cc: linux-mm@kvack.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/alpha/include/asm/page.h      |  3 ---
 arch/arc/include/asm/page.h        |  2 +-
 arch/arm/include/asm/page.h        |  4 +---
 arch/arm64/include/asm/page.h      |  4 +---
 arch/c6x/include/asm/page.h        |  5 +----
 arch/csky/include/asm/page.h       |  3 ---
 arch/h8300/include/asm/page.h      |  2 --
 arch/hexagon/include/asm/page.h    |  3 +--
 arch/ia64/include/asm/page.h       |  5 +----
 arch/m68k/include/asm/page.h       |  3 ---
 arch/microblaze/include/asm/page.h |  2 --
 arch/mips/include/asm/page.h       |  5 +----
 arch/nds32/include/asm/page.h      |  3 ---
 arch/nios2/include/asm/page.h      |  3 +--
 arch/openrisc/include/asm/page.h   |  5 -----
 arch/parisc/include/asm/page.h     |  3 ---
 arch/powerpc/include/asm/page.h    |  9 ++-------
 arch/powerpc/include/asm/page_64.h |  7 ++-----
 arch/riscv/include/asm/page.h      |  3 +--
 arch/s390/include/asm/page.h       |  3 +--
 arch/sh/include/asm/page.h         |  3 ---
 arch/sparc/include/asm/page_32.h   |  3 ---
 arch/sparc/include/asm/page_64.h   |  3 ---
 arch/unicore32/include/asm/page.h  |  3 ---
 arch/x86/include/asm/page_types.h  |  4 +---
 arch/x86/um/asm/vm-flags.h         | 10 ++--------
 arch/xtensa/include/asm/page.h     |  3 ---
 include/linux/mm.h                 | 15 +++++++++++++++
 28 files changed, 32 insertions(+), 89 deletions(-)

diff --git a/arch/alpha/include/asm/page.h b/arch/alpha/include/asm/page.h
index f3fb2848470a..e241bd88880f 100644
--- a/arch/alpha/include/asm/page.h
+++ b/arch/alpha/include/asm/page.h
@@ -90,9 +90,6 @@ typedef struct page *pgtable_t;
 #define virt_addr_valid(kaddr)	pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
 #endif /* CONFIG_DISCONTIGMEM */
 
-#define VM_DATA_DEFAULT_FLAGS		(VM_READ | VM_WRITE | VM_EXEC | \
-					 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
 #include <asm-generic/memory_model.h>
 #include <asm-generic/getorder.h>
 
diff --git a/arch/arc/include/asm/page.h b/arch/arc/include/asm/page.h
index 0a32e8cfd074..b0dfed0f12be 100644
--- a/arch/arc/include/asm/page.h
+++ b/arch/arc/include/asm/page.h
@@ -102,7 +102,7 @@ typedef pte_t * pgtable_t;
 #define virt_addr_valid(kaddr)  pfn_valid(virt_to_pfn(kaddr))
 
 /* Default Permissions for stack/heaps pages (Non Executable) */
-#define VM_DATA_DEFAULT_FLAGS   (VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS	VM_DATA_FLAGS_NON_EXEC
 
 #define WANT_PAGE_VIRTUAL   1
 
diff --git a/arch/arm/include/asm/page.h b/arch/arm/include/asm/page.h
index c2b75cba26df..11b058a72a5b 100644
--- a/arch/arm/include/asm/page.h
+++ b/arch/arm/include/asm/page.h
@@ -161,9 +161,7 @@ extern int pfn_valid(unsigned long);
 
 #endif /* !__ASSEMBLY__ */
 
-#define VM_DATA_DEFAULT_FLAGS \
-	(((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
-	 VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS	VM_DATA_FLAGS_TSK_EXEC
 
 #include <asm-generic/getorder.h>
 
diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
index d39ddb258a04..cb4e1e6ca385 100644
--- a/arch/arm64/include/asm/page.h
+++ b/arch/arm64/include/asm/page.h
@@ -32,9 +32,7 @@ extern int pfn_valid(unsigned long);
 
 #endif /* !__ASSEMBLY__ */
 
-#define VM_DATA_DEFAULT_FLAGS \
-	(((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
-	 VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS	VM_DATA_FLAGS_TSK_EXEC
 
 #include <asm-generic/getorder.h>
 
diff --git a/arch/c6x/include/asm/page.h b/arch/c6x/include/asm/page.h
index 70db1e7632bc..40079899084d 100644
--- a/arch/c6x/include/asm/page.h
+++ b/arch/c6x/include/asm/page.h
@@ -2,10 +2,7 @@
 #ifndef _ASM_C6X_PAGE_H
 #define _ASM_C6X_PAGE_H
 
-#define VM_DATA_DEFAULT_FLAGS \
-	(VM_READ | VM_WRITE | \
-	((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
-		 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS	VM_DATA_FLAGS_TSK_EXEC
 
 #include <asm-generic/page.h>
 
diff --git a/arch/csky/include/asm/page.h b/arch/csky/include/asm/page.h
index 9738eacefdc7..9b98bf31d57c 100644
--- a/arch/csky/include/asm/page.h
+++ b/arch/csky/include/asm/page.h
@@ -85,9 +85,6 @@ extern unsigned long va_pa_offset;
 				 PHYS_OFFSET_OFFSET)
 #define virt_to_page(x)	(mem_map + MAP_NR(x))
 
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | VM_EXEC | \
-				VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
 #define pfn_to_kaddr(x)	__va(PFN_PHYS(x))
 
 #include <asm-generic/memory_model.h>
diff --git a/arch/h8300/include/asm/page.h b/arch/h8300/include/asm/page.h
index 8da5124ad344..53e037544239 100644
--- a/arch/h8300/include/asm/page.h
+++ b/arch/h8300/include/asm/page.h
@@ -6,8 +6,6 @@
 #include <linux/types.h>
 
 #define MAP_NR(addr) (((uintptr_t)(addr)-PAGE_OFFSET) >> PAGE_SHIFT)
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | VM_EXEC | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
 
 #ifndef __ASSEMBLY__
 extern unsigned long rom_length;
diff --git a/arch/hexagon/include/asm/page.h b/arch/hexagon/include/asm/page.h
index ee31f36f48f3..7cbf719c578e 100644
--- a/arch/hexagon/include/asm/page.h
+++ b/arch/hexagon/include/asm/page.h
@@ -93,8 +93,7 @@ struct page;
 #define virt_to_page(kaddr) pfn_to_page(PFN_DOWN(__pa(kaddr)))
 
 /* Default vm area behavior is non-executable.  */
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | \
-				VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS	VM_DATA_FLAGS_NON_EXEC
 
 #define pfn_valid(pfn) ((pfn) < max_mapnr)
 #define virt_addr_valid(kaddr) pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
diff --git a/arch/ia64/include/asm/page.h b/arch/ia64/include/asm/page.h
index 5798bd2b462c..b69a5499d75b 100644
--- a/arch/ia64/include/asm/page.h
+++ b/arch/ia64/include/asm/page.h
@@ -218,10 +218,7 @@ get_order (unsigned long size)
 
 #define PAGE_OFFSET			RGN_BASE(RGN_KERNEL)
 
-#define VM_DATA_DEFAULT_FLAGS		(VM_READ | VM_WRITE |					\
-					 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC |		\
-					 (((current->personality & READ_IMPLIES_EXEC) != 0)	\
-					  ? VM_EXEC : 0))
+#define VM_DATA_DEFAULT_FLAGS	VM_DATA_FLAGS_TSK_EXEC
 
 #define GATE_ADDR		RGN_BASE(RGN_GATE)
 
diff --git a/arch/m68k/include/asm/page.h b/arch/m68k/include/asm/page.h
index 05e1e1e77a9a..9d2ef0a51720 100644
--- a/arch/m68k/include/asm/page.h
+++ b/arch/m68k/include/asm/page.h
@@ -55,9 +55,6 @@ extern unsigned long _ramend;
 #define __phys_to_pfn(paddr)	((unsigned long)((paddr) >> PAGE_SHIFT))
 #define __pfn_to_phys(pfn)	PFN_PHYS(pfn)
 
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | VM_EXEC | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
 #include <asm-generic/getorder.h>
 
 #endif /* _M68K_PAGE_H */
diff --git a/arch/microblaze/include/asm/page.h b/arch/microblaze/include/asm/page.h
index f4b44b24b02e..43bd560b7b67 100644
--- a/arch/microblaze/include/asm/page.h
+++ b/arch/microblaze/include/asm/page.h
@@ -197,8 +197,6 @@ extern int page_is_ram(unsigned long pfn);
 
 #ifdef CONFIG_MMU
 
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | VM_EXEC | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
 #endif /* CONFIG_MMU */
 
 #endif /* __KERNEL__ */
diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index 0ba4ce6e2bf3..e2f503fc7a84 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -253,10 +253,7 @@ extern bool __virt_addr_valid(const volatile void *kaddr);
 #define virt_addr_valid(kaddr)						\
 	__virt_addr_valid((const volatile void *) (kaddr))
 
-#define VM_DATA_DEFAULT_FLAGS \
-	(VM_READ | VM_WRITE | \
-	 ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
-	 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS	VM_DATA_FLAGS_TSK_EXEC
 
 #include <asm-generic/memory_model.h>
 #include <asm-generic/getorder.h>
diff --git a/arch/nds32/include/asm/page.h b/arch/nds32/include/asm/page.h
index 86b32014c5f9..add33a7f02c8 100644
--- a/arch/nds32/include/asm/page.h
+++ b/arch/nds32/include/asm/page.h
@@ -59,9 +59,6 @@ typedef struct page *pgtable_t;
 
 #endif /* !__ASSEMBLY__ */
 
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | VM_EXEC | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
 #endif /* __KERNEL__ */
 
 #endif
diff --git a/arch/nios2/include/asm/page.h b/arch/nios2/include/asm/page.h
index 79fcac61f6ef..6a989819a7c1 100644
--- a/arch/nios2/include/asm/page.h
+++ b/arch/nios2/include/asm/page.h
@@ -98,8 +98,7 @@ static inline bool pfn_valid(unsigned long pfn)
 # define virt_to_page(vaddr)	pfn_to_page(PFN_DOWN(virt_to_phys(vaddr)))
 # define virt_addr_valid(vaddr)	pfn_valid(PFN_DOWN(virt_to_phys(vaddr)))
 
-# define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+# define VM_DATA_DEFAULT_FLAGS	VM_DATA_FLAGS_NON_EXEC
 
 #include <asm-generic/memory_model.h>
 
diff --git a/arch/openrisc/include/asm/page.h b/arch/openrisc/include/asm/page.h
index 01069db59454..aab6e64d6db4 100644
--- a/arch/openrisc/include/asm/page.h
+++ b/arch/openrisc/include/asm/page.h
@@ -86,11 +86,6 @@ typedef struct page *pgtable_t;
 
 #endif /* __ASSEMBLY__ */
 
-
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | VM_EXEC | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
-
 #include <asm-generic/memory_model.h>
 #include <asm-generic/getorder.h>
 
diff --git a/arch/parisc/include/asm/page.h b/arch/parisc/include/asm/page.h
index 796ae29e9b9a..6b3f6740a6a6 100644
--- a/arch/parisc/include/asm/page.h
+++ b/arch/parisc/include/asm/page.h
@@ -180,9 +180,6 @@ extern int npmem_ranges;
 #define page_to_phys(page)	(page_to_pfn(page) << PAGE_SHIFT)
 #define virt_to_page(kaddr)     pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
 
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | VM_EXEC | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
 #include <asm-generic/memory_model.h>
 #include <asm-generic/getorder.h>
 #include <asm/pdc.h>
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 080a0bf8e54b..3ee8df0f66e0 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -240,13 +240,8 @@ static inline bool pfn_valid(unsigned long pfn)
  * and needs to be executable.  This means the whole heap ends
  * up being executable.
  */
-#define VM_DATA_DEFAULT_FLAGS32 \
-	(((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
-				 VM_READ | VM_WRITE | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
-#define VM_DATA_DEFAULT_FLAGS64	(VM_READ | VM_WRITE | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS32	VM_DATA_FLAGS_TSK_EXEC
+#define VM_DATA_DEFAULT_FLAGS64	VM_DATA_FLAGS_NON_EXEC
 
 #ifdef __powerpc64__
 #include <asm/page_64.h>
diff --git a/arch/powerpc/include/asm/page_64.h b/arch/powerpc/include/asm/page_64.h
index 5962797f784a..79a9b7c6a132 100644
--- a/arch/powerpc/include/asm/page_64.h
+++ b/arch/powerpc/include/asm/page_64.h
@@ -94,11 +94,8 @@ extern u64 ppc64_pft_size;
  * stack by default, so in the absence of a PT_GNU_STACK program header
  * we turn execute permission off.
  */
-#define VM_STACK_DEFAULT_FLAGS32	(VM_READ | VM_WRITE | VM_EXEC | \
-					 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
-#define VM_STACK_DEFAULT_FLAGS64	(VM_READ | VM_WRITE | \
-					 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_STACK_DEFAULT_FLAGS32	VM_DATA_FLAGS_EXEC
+#define VM_STACK_DEFAULT_FLAGS64	VM_DATA_FLAGS_NON_EXEC
 
 #define VM_STACK_DEFAULT_FLAGS \
 	(is_32bit_task() ? \
diff --git a/arch/riscv/include/asm/page.h b/arch/riscv/include/asm/page.h
index 8ca1930caa44..2d50f76efe48 100644
--- a/arch/riscv/include/asm/page.h
+++ b/arch/riscv/include/asm/page.h
@@ -137,8 +137,7 @@ extern phys_addr_t __phys_addr_symbol(unsigned long x);
 
 #define virt_addr_valid(vaddr)	(pfn_valid(virt_to_pfn(vaddr)))
 
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS	VM_DATA_FLAGS_NON_EXEC
 
 #include <asm-generic/memory_model.h>
 #include <asm-generic/getorder.h>
diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h
index 1019efd85b9d..089f49230966 100644
--- a/arch/s390/include/asm/page.h
+++ b/arch/s390/include/asm/page.h
@@ -176,8 +176,7 @@ static inline int devmem_is_allowed(unsigned long pfn)
 
 #define virt_addr_valid(kaddr)	pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
 
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS	VM_DATA_FLAGS_NON_EXEC
 
 #include <asm-generic/memory_model.h>
 #include <asm-generic/getorder.h>
diff --git a/arch/sh/include/asm/page.h b/arch/sh/include/asm/page.h
index 5eef8be3e59f..ea8d68f58e39 100644
--- a/arch/sh/include/asm/page.h
+++ b/arch/sh/include/asm/page.h
@@ -182,9 +182,6 @@ typedef struct page *pgtable_t;
 #endif
 #define virt_addr_valid(kaddr)	pfn_valid(__pa(kaddr) >> PAGE_SHIFT)
 
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | VM_EXEC | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
 #include <asm-generic/memory_model.h>
 #include <asm-generic/getorder.h>
 
diff --git a/arch/sparc/include/asm/page_32.h b/arch/sparc/include/asm/page_32.h
index b76d59edec8c..478260002836 100644
--- a/arch/sparc/include/asm/page_32.h
+++ b/arch/sparc/include/asm/page_32.h
@@ -133,9 +133,6 @@ extern unsigned long pfn_base;
 #define pfn_valid(pfn)		(((pfn) >= (pfn_base)) && (((pfn)-(pfn_base)) < max_mapnr))
 #define virt_addr_valid(kaddr)	((((unsigned long)(kaddr)-PAGE_OFFSET)>>PAGE_SHIFT) < max_mapnr)
 
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | VM_EXEC | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
 #include <asm-generic/memory_model.h>
 #include <asm-generic/getorder.h>
 
diff --git a/arch/sparc/include/asm/page_64.h b/arch/sparc/include/asm/page_64.h
index e80f2d5bf62f..254dffd85fb1 100644
--- a/arch/sparc/include/asm/page_64.h
+++ b/arch/sparc/include/asm/page_64.h
@@ -158,9 +158,6 @@ extern unsigned long PAGE_OFFSET;
 
 #endif /* !(__ASSEMBLY__) */
 
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | VM_EXEC | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
 #include <asm-generic/getorder.h>
 
 #endif /* _SPARC64_PAGE_H */
diff --git a/arch/unicore32/include/asm/page.h b/arch/unicore32/include/asm/page.h
index 8a89335673f9..96d6bdf180bd 100644
--- a/arch/unicore32/include/asm/page.h
+++ b/arch/unicore32/include/asm/page.h
@@ -69,9 +69,6 @@ extern int pfn_valid(unsigned long);
 
 #endif /* !__ASSEMBLY__ */
 
-#define VM_DATA_DEFAULT_FLAGS \
-	(VM_READ | VM_WRITE | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
 #include <asm-generic/getorder.h>
 
 #endif
diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
index c85e15010f48..e27aa6be6320 100644
--- a/arch/x86/include/asm/page_types.h
+++ b/arch/x86/include/asm/page_types.h
@@ -35,9 +35,7 @@
 
 #define PAGE_OFFSET		((unsigned long)__PAGE_OFFSET)
 
-#define VM_DATA_DEFAULT_FLAGS \
-	(((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0 ) | \
-	 VM_READ | VM_WRITE | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS	VM_DATA_FLAGS_TSK_EXEC
 
 #define __PHYSICAL_START	ALIGN(CONFIG_PHYSICAL_START, \
 				      CONFIG_PHYSICAL_ALIGN)
diff --git a/arch/x86/um/asm/vm-flags.h b/arch/x86/um/asm/vm-flags.h
index 7c297e9e2413..df7a3896f5dd 100644
--- a/arch/x86/um/asm/vm-flags.h
+++ b/arch/x86/um/asm/vm-flags.h
@@ -9,17 +9,11 @@
 
 #ifdef CONFIG_X86_32
 
-#define VM_DATA_DEFAULT_FLAGS \
-	(VM_READ | VM_WRITE | \
-	((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0 ) | \
-		 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_DEFAULT_FLAGS	VM_DATA_FLAGS_TSK_EXEC
 
 #else
 
-#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | VM_EXEC | \
-	VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-#define VM_STACK_DEFAULT_FLAGS (VM_GROWSDOWN | VM_READ | VM_WRITE | \
-	VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_STACK_DEFAULT_FLAGS (VM_GROWSDOWN | VM_DATA_FLAGS_EXEC)
 
 #endif
 #endif
diff --git a/arch/xtensa/include/asm/page.h b/arch/xtensa/include/asm/page.h
index f4771c29c7e9..37ce25ef92d6 100644
--- a/arch/xtensa/include/asm/page.h
+++ b/arch/xtensa/include/asm/page.h
@@ -203,8 +203,5 @@ static inline unsigned long ___pa(unsigned long va)
 
 #endif /* __ASSEMBLY__ */
 
-#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | VM_EXEC | \
-				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-
 #include <asm-generic/memory_model.h>
 #endif /* _XTENSA_PAGE_H */
diff --git a/include/linux/mm.h b/include/linux/mm.h
index b0e53ef13ff1..7a764ae6ab68 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -342,6 +342,21 @@ extern unsigned int kobjsize(const void *objp);
 /* Bits set in the VMA until the stack is in its final location */
 #define VM_STACK_INCOMPLETE_SETUP	(VM_RAND_READ | VM_SEQ_READ)
 
+#define TASK_EXEC ((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0)
+
+/* Common data flag combinations */
+#define VM_DATA_FLAGS_TSK_EXEC	(VM_READ | VM_WRITE | TASK_EXEC | \
+				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_FLAGS_NON_EXEC	(VM_READ | VM_WRITE | VM_MAYREAD | \
+				 VM_MAYWRITE | VM_MAYEXEC)
+#define VM_DATA_FLAGS_EXEC	(VM_READ | VM_WRITE | VM_EXEC | \
+				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+
+#ifndef VM_DATA_DEFAULT_FLAGS		/* arch can override this */
+#define VM_DATA_DEFAULT_FLAGS	(VM_READ | VM_WRITE | VM_EXEC | \
+				 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+#endif
+
 #ifndef VM_STACK_DEFAULT_FLAGS		/* arch can override this */
 #define VM_STACK_DEFAULT_FLAGS VM_DATA_DEFAULT_FLAGS
 #endif
-- 
2.20.1


^ permalink raw reply related

* [RFC 0/3] mm/vma: some new flags and helpers
From: Anshuman Khandual @ 2020-03-02  6:47 UTC (permalink / raw)
  To: linux-mm
  Cc: uclinux-h8-devel, linux-s390, linux-ia64, linux-c6x-dev,
	linux-parisc, Anshuman Khandual, linux-hexagon, linux-xtensa,
	linux-sh, linux-um, linux-kernel, linux-mips, linux-m68k,
	openrisc, linux-alpha, sparclinux, nios2-dev, linux-riscv,
	linux-snps-arc, linuxppc-dev, linux-arm-kernel

The motivation here is to consolidate VMA flag combinations commonly used
across platforms and reduce code duplication while making it uncluttered
in general.

This first introduces a default VM_DATA_DEFAULT_FLAGS which platforms can
easily fall back on without requiring to define any similar data flag
combinations as they currently do. This also adds some more common data
flag combinations which are generally used when the platforms decide to
override the default.

The second patch consolidates VM_READ, VM_WRITE, VM_EXEC as VM_ACCESS_FLAGS
extending the existing VMA accessibility concept via vma_is_accessibility().
VM_ACCESS_FLAGS replaces many other instances which used check all three
VMA access flags simultaneously.

While here, this also adds some more special VMA flag based helpers which
wraps around similar checks at various places thus improving readability.
This series intentionally limits these new helpers which are applicable 
only for special purpose VM flags than the more common ones like VM_READ,
VM_WRITE, VM_EXEC, VM_SHARED etc just to limit code churn. But if there is
common agreement that every flag should have it's own wrapper here, we can
do that as well. Otherwise if this patch seems really unnecessary with much
code churn, will be happy to drop it.

Reviews, comments, suggestions and concerns welcome. Thank you.

This series is based on v5.6-r4 after applying these patches.

1. https://patchwork.kernel.org/cover/11399319/
2. https://patchwork.kernel.org/patch/11399379/

This series is build tested across multiple architectures but boot tested
only on arm64 and x86 platforms.

Cc: linux-alpha@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-c6x-dev@linux-c6x.org
Cc: uclinux-h8-devel@lists.sourceforge.jp
Cc: linux-hexagon@vger.kernel.org
Cc: linux-ia64@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-mips@vger.kernel.org
Cc: nios2-dev@lists.rocketboards.org
Cc: openrisc@lists.librecores.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-riscv@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-um@lists.infradead.org
Cc: linux-xtensa@linux-xtensa.org
Cc: linux-mm@kvack.org

Anshuman Khandual (3):
  mm/vma: Define a default value for VM_DATA_DEFAULT_FLAGS
  mm/vma: Introduce VM_ACCESS_FLAGS
  mm/vma: Introduce some more VMA flag wrappers

 arch/alpha/include/asm/page.h        |  3 --
 arch/arc/include/asm/page.h          |  2 +-
 arch/arm/include/asm/page.h          |  4 +-
 arch/arm/mm/fault.c                  |  2 +-
 arch/arm64/include/asm/page.h        |  4 +-
 arch/arm64/mm/fault.c                |  2 +-
 arch/c6x/include/asm/page.h          |  5 +--
 arch/c6x/include/asm/processor.h     |  2 +-
 arch/csky/include/asm/page.h         |  3 --
 arch/h8300/include/asm/page.h        |  2 -
 arch/hexagon/include/asm/page.h      |  3 +-
 arch/ia64/include/asm/page.h         |  5 +--
 arch/m68k/include/asm/page.h         |  3 --
 arch/microblaze/include/asm/page.h   |  2 -
 arch/mips/include/asm/page.h         |  5 +--
 arch/nds32/include/asm/page.h        |  3 --
 arch/nds32/mm/fault.c                |  2 +-
 arch/nios2/include/asm/page.h        |  3 +-
 arch/nios2/include/asm/processor.h   |  2 +-
 arch/openrisc/include/asm/page.h     |  5 ---
 arch/parisc/include/asm/page.h       |  3 --
 arch/powerpc/include/asm/page.h      |  9 +----
 arch/powerpc/include/asm/page_64.h   |  7 +---
 arch/powerpc/mm/book3s64/pkeys.c     |  2 +-
 arch/riscv/include/asm/page.h        |  3 +-
 arch/s390/include/asm/page.h         |  3 +-
 arch/s390/mm/fault.c                 |  2 +-
 arch/sh/include/asm/page.h           |  3 --
 arch/sh/include/asm/processor_64.h   |  2 +-
 arch/sparc/include/asm/mman.h        |  2 +-
 arch/sparc/include/asm/page_32.h     |  3 --
 arch/sparc/include/asm/page_64.h     |  3 --
 arch/unicore32/include/asm/page.h    |  3 --
 arch/unicore32/mm/fault.c            |  2 +-
 arch/x86/include/asm/page_types.h    |  4 +-
 arch/x86/mm/pkeys.c                  |  2 +-
 arch/x86/um/asm/vm-flags.h           | 10 +----
 arch/xtensa/include/asm/page.h       |  3 --
 drivers/staging/gasket/gasket_core.c |  2 +-
 fs/binfmt_elf.c                      |  2 +-
 fs/proc/task_mmu.c                   | 14 +++----
 include/linux/huge_mm.h              |  4 +-
 include/linux/mm.h                   | 58 +++++++++++++++++++++++++++-
 kernel/events/core.c                 |  2 +-
 kernel/events/uprobes.c              |  2 +-
 mm/gup.c                             |  2 +-
 mm/huge_memory.c                     |  6 +--
 mm/hugetlb.c                         |  4 +-
 mm/ksm.c                             |  8 ++--
 mm/madvise.c                         |  4 +-
 mm/memory.c                          |  4 +-
 mm/migrate.c                         |  4 +-
 mm/mlock.c                           |  4 +-
 mm/mmap.c                            | 20 +++++-----
 mm/mprotect.c                        |  9 ++---
 mm/mremap.c                          |  4 +-
 mm/msync.c                           |  3 +-
 mm/rmap.c                            |  6 +--
 mm/shmem.c                           |  8 ++--
 59 files changed, 140 insertions(+), 158 deletions(-)

-- 
2.20.1


^ permalink raw reply

* Re: [PATCH] powerpc/sysdev: fix compile errors
From: Christophe Leroy @ 2020-03-02  6:38 UTC (permalink / raw)
  To: WANG Wenhu, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Kate Stewart, Greg Kroah-Hartman,
	Allison Randal, Thomas Gleixner, linuxppc-dev, linux-kernel
  Cc: wenhu.pku, trivial
In-Reply-To: <20200302053801.26027-1-wenhu.wang@vivo.com>



Le 02/03/2020 à 06:37, WANG Wenhu a écrit :
> Include linux/io.h into fsl_85xx_cache_sram.c to fix the
> implicit-declaration compile errors when building Cache-Sram.
> 
> arch/powerpc/sysdev/fsl_85xx_cache_sram.c: In function ‘instantiate_cache_sram’:
> arch/powerpc/sysdev/fsl_85xx_cache_sram.c:97:26: error: implicit declaration of function ‘ioremap_coherent’; did you mean ‘bitmap_complement’? [-Werror=implicit-function-declaration]
>    cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
>                            ^~~~~~~~~~~~~~~~
>                            bitmap_complement
> arch/powerpc/sysdev/fsl_85xx_cache_sram.c:97:24: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
>    cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
>                          ^
> arch/powerpc/sysdev/fsl_85xx_cache_sram.c:123:2: error: implicit declaration of function ‘iounmap’; did you mean ‘roundup’? [-Werror=implicit-function-declaration]
>    iounmap(cache_sram->base_virt);
>    ^~~~~~~
>    roundup
> cc1: all warnings being treated as errors
> 
> Fixed: commit 6db92cc9d07d ("powerpc/85xx: add cache-sram support")
> Signed-off-by: WANG Wenhu <wenhu.wang@vivo.com>

Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>

> ---
>   arch/powerpc/sysdev/fsl_85xx_cache_sram.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/powerpc/sysdev/fsl_85xx_cache_sram.c b/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
> index f6c665dac725..be3aef4229d7 100644
> --- a/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
> +++ b/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
> @@ -17,6 +17,7 @@
>   #include <linux/of_platform.h>
>   #include <asm/pgtable.h>
>   #include <asm/fsl_85xx_cache_sram.h>
> +#include <linux/io.h>
>   
>   #include "fsl_85xx_cache_ctlr.h"
>   
> 

^ permalink raw reply

* Re: [PATCH v3 16/27] powerpc/powernv/pmem: Register a character device for userspace to interact with
From: Alastair D'Silva @ 2020-03-02  6:05 UTC (permalink / raw)
  To: Andrew Donnellan
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
	Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
	Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
	Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
	Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
	Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <1e980dc7-109a-d96f-1329-1c38918e2bba@linux.ibm.com>

On Mon, 2020-03-02 at 16:34 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> > 
> > This patch introduces a character device (/dev/ocxl-scmX) which
> > further
> > patches will use to interact with userspace.
> 
> As with the comments on other patches in this series, this commit 
> message is lacking in explanation. What's the purpose of this device?
> 

I'll reword this for v4.

> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> >   arch/powerpc/platforms/powernv/pmem/ocxl.c    | 116
> > +++++++++++++++++-
> >   .../platforms/powernv/pmem/ocxl_internal.h    |   2 +
> >   2 files changed, 116 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > index b8bd7e703b19..63109a870d2c 100644
> > --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> > @@ -10,6 +10,7 @@
> >   #include <misc/ocxl.h>
> >   #include <linux/delay.h>
> >   #include <linux/ndctl.h>
> > +#include <linux/fs.h>
> >   #include <linux/mm_types.h>
> >   #include <linux/memory_hotplug.h>
> >   #include "ocxl_internal.h"
> > @@ -339,6 +340,9 @@ static void free_ocxlpmem(struct ocxlpmem
> > *ocxlpmem)
> >   
> >   	free_minor(ocxlpmem);
> >   
> > +	if (ocxlpmem->cdev.owner)
> > +		cdev_del(&ocxlpmem->cdev);
> > +
> >   	if (ocxlpmem->metadata_addr)
> >   		devm_memunmap(&ocxlpmem->dev, ocxlpmem->metadata_addr);
> >   
> > @@ -396,6 +400,70 @@ static int ocxlpmem_register(struct ocxlpmem
> > *ocxlpmem)
> >   	return device_register(&ocxlpmem->dev);
> >   }
> >   
> > +static void ocxlpmem_put(struct ocxlpmem *ocxlpmem)
> > +{
> > +	put_device(&ocxlpmem->dev);
> > +}
> > +
> > +static struct ocxlpmem *ocxlpmem_get(struct ocxlpmem *ocxlpmem)
> > +{
> > +	return (get_device(&ocxlpmem->dev) == NULL) ? NULL : ocxlpmem;
> > +}
> > +
> > +static struct ocxlpmem *find_and_get_ocxlpmem(dev_t devno)
> > +{
> > +	struct ocxlpmem *ocxlpmem;
> > +	int minor = MINOR(devno);
> > +	/*
> > +	 * We don't declare an RCU critical section here, as our AFU
> > +	 * is protected by a re0ference counter on the device. By the
> > time the
> > +	 * minor number of a device is removed from the idr, the ref
> > count of
> > +	 * the device is already at 0, so no user API will access that
> > AFU and
> > +	 * this function can't return it.
> > +	 */
> > +	ocxlpmem = idr_find(&minors_idr, minor);
> > +	if (ocxlpmem)
> > +		ocxlpmem_get(ocxlpmem);
> > +	return ocxlpmem;
> > +}
> > +
> > +static int file_open(struct inode *inode, struct file *file)
> > +{
> > +	struct ocxlpmem *ocxlpmem;
> > +
> > +	ocxlpmem = find_and_get_ocxlpmem(inode->i_rdev);
> > +	if (!ocxlpmem)
> > +		return -ENODEV;
> > +
> > +	file->private_data = ocxlpmem;
> > +	return 0;
> > +}
> > +
> > +static int file_release(struct inode *inode, struct file *file)
> > +{
> > +	struct ocxlpmem *ocxlpmem = file->private_data;
> > +
> > +	ocxlpmem_put(ocxlpmem);
> > +	return 0;
> > +}
> > +
> > +static const struct file_operations fops = {
> > +	.owner		= THIS_MODULE,
> > +	.open		= file_open,
> > +	.release	= file_release,
> > +};
> > +
> > +/**
> > + * create_cdev() - Create the chardev in /dev for the device
> > + * @ocxlpmem: the SCM metadata
> > + * Return: 0 on success, negative on failure
> > + */
> > +static int create_cdev(struct ocxlpmem *ocxlpmem)
> > +{
> > +	cdev_init(&ocxlpmem->cdev, &fops);
> > +	return cdev_add(&ocxlpmem->cdev, ocxlpmem->dev.devt, 1);
> > +}
> > +
> >   /**
> >    * ocxlpmem_remove() - Free an OpenCAPI persistent memory device
> >    * @pdev: the PCI device information struct
> > @@ -572,6 +640,11 @@ static int probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> >   		goto err;
> >   	}
> >   
> > +	if (create_cdev(ocxlpmem)) {
> > +		dev_err(&pdev->dev, "Could not create character
> > device\n");
> > +		goto err;
> > +	}
> > +
> >   	elapsed = 0;
> >   	timeout = ocxlpmem->readiness_timeout + ocxlpmem-
> > >memory_available_timeout;
> >   	while (!is_usable(ocxlpmem, false)) {
> > @@ -613,20 +686,59 @@ static struct pci_driver pci_driver = {
> >   	.shutdown = ocxlpmem_remove,
> >   };
> >   
> > +static int file_init(void)
> > +{
> > +	int rc;
> > +
> > +	mutex_init(&minors_idr_lock);
> > +	idr_init(&minors_idr);
> > +
> > +	rc = alloc_chrdev_region(&ocxlpmem_dev, 0, NUM_MINORS, "ocxl-
> > pmem");
> 
> If the driver is going to be called "ocxlpmem" can we standardise on 
> that without the extra hyphen?

Ok

> > +	if (rc) {
> > +		idr_destroy(&minors_idr);
> > +		pr_err("Unable to allocate OpenCAPI persistent memory
> > major number: %d\n", rc);
> > +		return rc;
> > +	}
> > +
> > +	ocxlpmem_class = class_create(THIS_MODULE, "ocxl-pmem");
> > +	if (IS_ERR(ocxlpmem_class)) {
> > +		idr_destroy(&minors_idr);
> > +		pr_err("Unable to create ocxl-pmem class\n");
> > +		unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> > +		return PTR_ERR(ocxlpmem_class);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void file_exit(void)
> > +{
> > +	class_destroy(ocxlpmem_class);
> > +	unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> > +	idr_destroy(&minors_idr);
> > +}
> > +
> >   static int __init ocxlpmem_init(void)
> >   {
> > -	int rc = 0;
> > +	int rc;
> >   
> > -	rc = pci_register_driver(&pci_driver);
> > +	rc = file_init();
> >   	if (rc)
> >   		return rc;
> >   
> > +	rc = pci_register_driver(&pci_driver);
> > +	if (rc) {
> > +		file_exit();
> > +		return rc;
> > +	}
> > +
> >   	return 0;
> >   }
> >   
> >   static void ocxlpmem_exit(void)
> >   {
> >   	pci_unregister_driver(&pci_driver);
> > +	file_exit();
> >   }
> >   
> >   module_init(ocxlpmem_init);
-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819


^ permalink raw reply

* [PATCH] powerpc/sysdev: fix compile errors
From: WANG Wenhu @ 2020-03-02  5:37 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Kate Stewart, Greg Kroah-Hartman, Allison Randal, Thomas Gleixner,
	wangwenhu, linuxppc-dev, linux-kernel
  Cc: wenhu.pku, trivial

Include linux/io.h into fsl_85xx_cache_sram.c to fix the
implicit-declaration compile errors when building Cache-Sram.

arch/powerpc/sysdev/fsl_85xx_cache_sram.c: In function ‘instantiate_cache_sram’:
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:97:26: error: implicit declaration of function ‘ioremap_coherent’; did you mean ‘bitmap_complement’? [-Werror=implicit-function-declaration]
  cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
                          ^~~~~~~~~~~~~~~~
                          bitmap_complement
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:97:24: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
  cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
                        ^
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:123:2: error: implicit declaration of function ‘iounmap’; did you mean ‘roundup’? [-Werror=implicit-function-declaration]
  iounmap(cache_sram->base_virt);
  ^~~~~~~
  roundup
cc1: all warnings being treated as errors

Fixed: commit 6db92cc9d07d ("powerpc/85xx: add cache-sram support")
Signed-off-by: WANG Wenhu <wenhu.wang@vivo.com>
---
 arch/powerpc/sysdev/fsl_85xx_cache_sram.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/sysdev/fsl_85xx_cache_sram.c b/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
index f6c665dac725..be3aef4229d7 100644
--- a/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
+++ b/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
@@ -17,6 +17,7 @@
 #include <linux/of_platform.h>
 #include <asm/pgtable.h>
 #include <asm/fsl_85xx_cache_sram.h>
+#include <linux/io.h>
 
 #include "fsl_85xx_cache_ctlr.h"
 
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v3 21/27] powerpc/powernv/pmem: Add an IOCTL to request controller health & perf data
From: Alastair D'Silva @ 2020-03-02  5:40 UTC (permalink / raw)
  To: Andrew Donnellan
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
	Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
	Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
	Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
	Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
	Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <fdc5faec-d03d-3cba-4a9c-add7e522ad13@linux.ibm.com>

On Fri, 2020-02-28 at 17:12 +1100, Andrew Donnellan wrote:
> On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> > 
> > When health & performance data is requested from the controller,
> > it responds with an error log containing the requested information.
> > 
> > This patch allows the request to me issued via an IOCTL.
> 
> A better explanation would be good - this IOCTL triggers a request
> to 
> the controller to collect controller health/perf data, and the 
> controller will later respond with an error log that can be picked
> up 
> via the error log IOCTL that you've defined earlier.
> 
> 

Ok

-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819


^ permalink raw reply

* RE: [PATCH v3 25/27] powerpc/powernv/pmem: Expose the serial number in sysfs
From: Alastair D'Silva @ 2020-03-02  5:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Andrew Donnellan
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
	Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
	Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
	Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
	linux-kernel, Vishal Verma, Frederic Barrat, Paul Mackerras,
	Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <4075f48568fee61123579d4edea0e7939b4b2e6c.camel@au1.ibm.com>

On Mon, 2020-03-02 at 10:42 +1100, Alastair D'Silva wrote:
> On Fri, 2020-02-28 at 08:15 +0100, Greg Kroah-Hartman wrote:
> > On Fri, Feb 28, 2020 at 05:25:31PM +1100, Andrew Donnellan wrote:
> > > On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> > > > +int ocxlpmem_sysfs_add(struct ocxlpmem *ocxlpmem)
> > > > +{
> > > > +	int i, rc;
> > > > +
> > > > +	for (i = 0; i < ARRAY_SIZE(attrs); i++) {
> > > > +		rc = device_create_file(&ocxlpmem->dev,
> > > > &attrs[i]);
> > > > +		if (rc) {
> > > > +			for (; --i >= 0;)
> > > > +				device_remove_file(&ocxlpmem-
> > > > >dev,
> > > > &attrs[i]);
> > > 
> > > I'd rather avoid weird for loop constructs if possible.
> > > 
> > > Is it actually dangerous to call device_remove_file() on an attr
> > > that hasn't
> > > been added? If not then I'd rather define an err: label and loop
> > > over the
> > > whole array there.
> > 
> > None of this should be used at all, just use attribute groups
> > properly
> > and the driver core will handle this all for you.
> > 
> > device_create/remove_file should never be called by anyone anymore
> > if
> > at all
> > possible.
> > 
> > thanks,
> > 
> > greg k-h
> 
> Thanks, I'll rework it to use the .groups member of struct
> pci_driver.
> 

I ended up making these available as DIMM attributes instead.

-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819


^ permalink raw reply

* Re: [PATCH v3 16/27] powerpc/powernv/pmem: Register a character device for userspace to interact with
From: Andrew Donnellan @ 2020-03-02  5:34 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
	Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
	Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
	Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
	Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
	Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-17-alastair@au1.ibm.com>

On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> This patch introduces a character device (/dev/ocxl-scmX) which further
> patches will use to interact with userspace.

As with the comments on other patches in this series, this commit 
message is lacking in explanation. What's the purpose of this device?

> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>   arch/powerpc/platforms/powernv/pmem/ocxl.c    | 116 +++++++++++++++++-
>   .../platforms/powernv/pmem/ocxl_internal.h    |   2 +
>   2 files changed, 116 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> index b8bd7e703b19..63109a870d2c 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -10,6 +10,7 @@
>   #include <misc/ocxl.h>
>   #include <linux/delay.h>
>   #include <linux/ndctl.h>
> +#include <linux/fs.h>
>   #include <linux/mm_types.h>
>   #include <linux/memory_hotplug.h>
>   #include "ocxl_internal.h"
> @@ -339,6 +340,9 @@ static void free_ocxlpmem(struct ocxlpmem *ocxlpmem)
>   
>   	free_minor(ocxlpmem);
>   
> +	if (ocxlpmem->cdev.owner)
> +		cdev_del(&ocxlpmem->cdev);
> +
>   	if (ocxlpmem->metadata_addr)
>   		devm_memunmap(&ocxlpmem->dev, ocxlpmem->metadata_addr);
>   
> @@ -396,6 +400,70 @@ static int ocxlpmem_register(struct ocxlpmem *ocxlpmem)
>   	return device_register(&ocxlpmem->dev);
>   }
>   
> +static void ocxlpmem_put(struct ocxlpmem *ocxlpmem)
> +{
> +	put_device(&ocxlpmem->dev);
> +}
> +
> +static struct ocxlpmem *ocxlpmem_get(struct ocxlpmem *ocxlpmem)
> +{
> +	return (get_device(&ocxlpmem->dev) == NULL) ? NULL : ocxlpmem;
> +}
> +
> +static struct ocxlpmem *find_and_get_ocxlpmem(dev_t devno)
> +{
> +	struct ocxlpmem *ocxlpmem;
> +	int minor = MINOR(devno);
> +	/*
> +	 * We don't declare an RCU critical section here, as our AFU
> +	 * is protected by a re0ference counter on the device. By the time the
> +	 * minor number of a device is removed from the idr, the ref count of
> +	 * the device is already at 0, so no user API will access that AFU and
> +	 * this function can't return it.
> +	 */
> +	ocxlpmem = idr_find(&minors_idr, minor);
> +	if (ocxlpmem)
> +		ocxlpmem_get(ocxlpmem);
> +	return ocxlpmem;
> +}
> +
> +static int file_open(struct inode *inode, struct file *file)
> +{
> +	struct ocxlpmem *ocxlpmem;
> +
> +	ocxlpmem = find_and_get_ocxlpmem(inode->i_rdev);
> +	if (!ocxlpmem)
> +		return -ENODEV;
> +
> +	file->private_data = ocxlpmem;
> +	return 0;
> +}
> +
> +static int file_release(struct inode *inode, struct file *file)
> +{
> +	struct ocxlpmem *ocxlpmem = file->private_data;
> +
> +	ocxlpmem_put(ocxlpmem);
> +	return 0;
> +}
> +
> +static const struct file_operations fops = {
> +	.owner		= THIS_MODULE,
> +	.open		= file_open,
> +	.release	= file_release,
> +};
> +
> +/**
> + * create_cdev() - Create the chardev in /dev for the device
> + * @ocxlpmem: the SCM metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int create_cdev(struct ocxlpmem *ocxlpmem)
> +{
> +	cdev_init(&ocxlpmem->cdev, &fops);
> +	return cdev_add(&ocxlpmem->cdev, ocxlpmem->dev.devt, 1);
> +}
> +
>   /**
>    * ocxlpmem_remove() - Free an OpenCAPI persistent memory device
>    * @pdev: the PCI device information struct
> @@ -572,6 +640,11 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   		goto err;
>   	}
>   
> +	if (create_cdev(ocxlpmem)) {
> +		dev_err(&pdev->dev, "Could not create character device\n");
> +		goto err;
> +	}
> +
>   	elapsed = 0;
>   	timeout = ocxlpmem->readiness_timeout + ocxlpmem->memory_available_timeout;
>   	while (!is_usable(ocxlpmem, false)) {
> @@ -613,20 +686,59 @@ static struct pci_driver pci_driver = {
>   	.shutdown = ocxlpmem_remove,
>   };
>   
> +static int file_init(void)
> +{
> +	int rc;
> +
> +	mutex_init(&minors_idr_lock);
> +	idr_init(&minors_idr);
> +
> +	rc = alloc_chrdev_region(&ocxlpmem_dev, 0, NUM_MINORS, "ocxl-pmem");

If the driver is going to be called "ocxlpmem" can we standardise on 
that without the extra hyphen?

> +	if (rc) {
> +		idr_destroy(&minors_idr);
> +		pr_err("Unable to allocate OpenCAPI persistent memory major number: %d\n", rc);
> +		return rc;
> +	}
> +
> +	ocxlpmem_class = class_create(THIS_MODULE, "ocxl-pmem");
> +	if (IS_ERR(ocxlpmem_class)) {
> +		idr_destroy(&minors_idr);
> +		pr_err("Unable to create ocxl-pmem class\n");
> +		unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> +		return PTR_ERR(ocxlpmem_class);
> +	}
> +
> +	return 0;
> +}
> +
> +static void file_exit(void)
> +{
> +	class_destroy(ocxlpmem_class);
> +	unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> +	idr_destroy(&minors_idr);
> +}
> +
>   static int __init ocxlpmem_init(void)
>   {
> -	int rc = 0;
> +	int rc;
>   
> -	rc = pci_register_driver(&pci_driver);
> +	rc = file_init();
>   	if (rc)
>   		return rc;
>   
> +	rc = pci_register_driver(&pci_driver);
> +	if (rc) {
> +		file_exit();
> +		return rc;
> +	}
> +
>   	return 0;
>   }
>   
>   static void ocxlpmem_exit(void)
>   {
>   	pci_unregister_driver(&pci_driver);
> +	file_exit();
>   }
>   
>   module_init(ocxlpmem_init);
-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited


^ permalink raw reply

* [RFC 05/11] perf tools: Enable record and script to record and show hazard data
From: Ravi Bangoria @ 2020-03-02  5:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-kernel
  Cc: mark.rutland, ravi.bangoria, ak, maddy, peterz, alexey.budankov,
	Madhavan Srinivasan, adrian.hunter, acme, alexander.shishkin,
	yao.jin, mingo, paulus, eranian, robert.richter, namhyung,
	kim.phillips, jolsa, kan.liang
In-Reply-To: <20200302052355.36365-1-ravi.bangoria@linux.ibm.com>

From: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>

Introduce new perf record option "--hazard" to capture cpu pipeline
hazard data. Also enable perf script -D to dump raw values of it.
Sample o/p:

  $ ./perf record -e r4010e --hazard -- ls
  $ ./perf script -D
  ... PERF_RECORD_SAMPLE(IP, 0x2): ...
  hazard information:
  Inst Type 0x1
  Inst Cache 0x1
  Hazard Stage 0x4
  Hazard Reason 0x3
  Stall Stage 0x4
  Stall Reason 0x2

Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
 tools/perf/Documentation/perf-record.txt  |  3 +++
 tools/perf/builtin-record.c               |  1 +
 tools/perf/util/event.h                   |  1 +
 tools/perf/util/evsel.c                   | 10 ++++++++++
 tools/perf/util/perf_event_attr_fprintf.c |  1 +
 tools/perf/util/record.h                  |  1 +
 tools/perf/util/session.c                 | 16 ++++++++++++++++
 7 files changed, 33 insertions(+)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index b23a4012a606..e7bd1b6938ce 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -283,6 +283,9 @@ OPTIONS
 --phys-data::
 	Record the sample physical addresses.
 
+--hazard::
+	Record processor pipeline hazard and stall information.
+
 -T::
 --timestamp::
 	Record the sample timestamps. Use it with 'perf report -D' to see the
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 4c301466101b..6bd32d7bc4e9 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2301,6 +2301,7 @@ static struct option __record_options[] = {
 	OPT_BOOLEAN('s', "stat", &record.opts.inherit_stat,
 		    "per thread counts"),
 	OPT_BOOLEAN('d', "data", &record.opts.sample_address, "Record the sample addresses"),
+	OPT_BOOLEAN(0, "hazard", &record.opts.hazard, "Record processor pipeline hazard and stall information"),
 	OPT_BOOLEAN(0, "phys-data", &record.opts.sample_phys_addr,
 		    "Record the sample physical addresses"),
 	OPT_BOOLEAN(0, "sample-cpu", &record.opts.sample_cpu, "Record the sample cpu"),
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 85223159737c..ff0f03253a95 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -148,6 +148,7 @@ struct perf_sample {
 	struct stack_dump user_stack;
 	struct sample_read read;
 	struct aux_sample aux_sample;
+	struct perf_pipeline_haz_data *pipeline_haz;
 };
 
 #define PERF_MEM_DATA_SRC_NONE \
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index c8dc4450884c..e37ed7929c2c 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1080,6 +1080,9 @@ void perf_evsel__config(struct evsel *evsel, struct record_opts *opts,
 	if (opts->sample_phys_addr)
 		perf_evsel__set_sample_bit(evsel, PHYS_ADDR);
 
+	if (opts->hazard)
+		perf_evsel__set_sample_bit(evsel, PIPELINE_HAZ);
+
 	if (opts->no_buffering) {
 		attr->watermark = 0;
 		attr->wakeup_events = 1;
@@ -2265,6 +2268,13 @@ int perf_evsel__parse_sample(struct evsel *evsel, union perf_event *event,
 		array = (void *)array + sz;
 	}
 
+	if (type & PERF_SAMPLE_PIPELINE_HAZ) {
+		sz = sizeof(struct perf_pipeline_haz_data);
+		OVERFLOW_CHECK(array, sz, max_size);
+		data->pipeline_haz = (struct perf_pipeline_haz_data *)array;
+		array = (void *)array + sz;
+	}
+
 	return 0;
 }
 
diff --git a/tools/perf/util/perf_event_attr_fprintf.c b/tools/perf/util/perf_event_attr_fprintf.c
index 651203126c71..d97e755c886b 100644
--- a/tools/perf/util/perf_event_attr_fprintf.c
+++ b/tools/perf/util/perf_event_attr_fprintf.c
@@ -35,6 +35,7 @@ static void __p_sample_type(char *buf, size_t size, u64 value)
 		bit_name(BRANCH_STACK), bit_name(REGS_USER), bit_name(STACK_USER),
 		bit_name(IDENTIFIER), bit_name(REGS_INTR), bit_name(DATA_SRC),
 		bit_name(WEIGHT), bit_name(PHYS_ADDR), bit_name(AUX),
+		bit_name(PIPELINE_HAZ),
 		{ .name = NULL, }
 	};
 #undef bit_name
diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
index 5421fd2ad383..f1678a0bc8ce 100644
--- a/tools/perf/util/record.h
+++ b/tools/perf/util/record.h
@@ -67,6 +67,7 @@ struct record_opts {
 	int	      affinity;
 	int	      mmap_flush;
 	unsigned int  comp_level;
+	bool	      hazard;
 };
 
 extern const char * const *record_usage;
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index d0d7d25b23e3..834ca7df2349 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1153,6 +1153,19 @@ static void stack_user__printf(struct stack_dump *dump)
 	       dump->size, dump->offset);
 }
 
+static void pipeline_hazard__printf(struct perf_sample *sample)
+{
+	struct perf_pipeline_haz_data *haz = sample->pipeline_haz;
+
+	printf("... hazard information:\n");
+	printf(".... Inst Type 0x%" PRIx32 "\n", haz->itype);
+	printf(".... Inst Cache 0x%" PRIx32 "\n", haz->icache);
+	printf(".... Hazard Stage 0x%" PRIx32 "\n", haz->hazard_stage);
+	printf(".... Hazard Reason 0x%" PRIx32 "\n", haz->hazard_reason);
+	printf(".... Stall Stage 0x%" PRIx32 "\n", haz->stall_stage);
+	printf(".... Stall Reason 0x%" PRIx32 "\n", haz->stall_reason);
+}
+
 static void perf_evlist__print_tstamp(struct evlist *evlist,
 				       union perf_event *event,
 				       struct perf_sample *sample)
@@ -1251,6 +1264,9 @@ static void dump_sample(struct evsel *evsel, union perf_event *event,
 	if (sample_type & PERF_SAMPLE_STACK_USER)
 		stack_user__printf(&sample->user_stack);
 
+	if (sample_type & PERF_SAMPLE_PIPELINE_HAZ)
+		pipeline_hazard__printf(sample);
+
 	if (sample_type & PERF_SAMPLE_WEIGHT)
 		printf("... weight: %" PRIu64 "\n", sample->weight);
 
-- 
2.21.1


^ permalink raw reply related

* [RFC 11/11] perf annotate: Show hazard data in tui mode
From: Ravi Bangoria @ 2020-03-02  5:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-kernel
  Cc: mark.rutland, ravi.bangoria, ak, maddy, peterz, alexey.budankov,
	adrian.hunter, acme, alexander.shishkin, yao.jin, mingo, paulus,
	eranian, robert.richter, namhyung, kim.phillips, jolsa, kan.liang
In-Reply-To: <20200302052355.36365-1-ravi.bangoria@linux.ibm.com>

Enable perf report->annotate tui mode to show hazard information. By
default they are hidden, but user can unhide them by pressing hot key
'S'. Sample o/p:

         │    Disassembly of section .text:
         │
         │    0000000010001cf8 <compare>:
         │    compare():
         │    return NULL;
         │    }
         │
         │    static int
         │    compare(const void *p1, const void *p2)
         │    {
   33.23 │      std    r31,-8(r1)
         │       {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
         │       {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
         │       {haz_stage: LSU, haz_reason: Load Hit Store, stall_stage: LSU, stall_reason: -, icache: L3 hit}
         │       {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: -, stall_reason: -, icache: L1 hit}
         │       {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
         │       {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
    0.84 │      stdu   r1,-64(r1)
         │       {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: -, stall_reason: -, icache: L1 hit}
    0.24 │      mr     r31,r1
         │       {haz_stage: -, haz_reason: -, stall_stage: -, stall_reason: -, icache: L1 hit}
   21.18 │      std    r3,32(r31)
         │       {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
         │       {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}
         │       {haz_stage: LSU, haz_reason: ERAT Miss, stall_stage: LSU, stall_reason: Store, icache: L1 hit}

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
 tools/perf/builtin-annotate.c     |   5 ++
 tools/perf/ui/browsers/annotate.c | 124 ++++++++++++++++++++++++++----
 tools/perf/util/annotate.c        |  51 +++++++++++-
 tools/perf/util/annotate.h        |  18 ++++-
 4 files changed, 178 insertions(+), 20 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 78552a9428a6..a51313a6b019 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -472,6 +472,7 @@ static const char * const annotate_usage[] = {
 
 int cmd_annotate(int argc, const char **argv)
 {
+	bool annotate_haz = false;
 	struct perf_annotate annotate = {
 		.tool = {
 			.sample	= process_sample_event,
@@ -531,6 +532,8 @@ int cmd_annotate(int argc, const char **argv)
 		     symbol__config_symfs),
 	OPT_BOOLEAN(0, "source", &annotate.opts.annotate_src,
 		    "Interleave source code with assembly code (default)"),
+	OPT_BOOLEAN(0, "hazard", &annotate_haz,
+		    "Interleave CPU pileline hazard/stall data with assembly code"),
 	OPT_BOOLEAN(0, "asm-raw", &annotate.opts.show_asm_raw,
 		    "Display raw encoding of assembly instructions (default)"),
 	OPT_STRING('M', "disassembler-style", &annotate.opts.disassembler_style, "disassembler style",
@@ -583,6 +586,8 @@ int cmd_annotate(int argc, const char **argv)
 	if (annotate_check_args(&annotate.opts) < 0)
 		return -EINVAL;
 
+	annotate.opts.hide_haz_data = !annotate_haz;
+
 	if (symbol_conf.show_nr_samples && annotate.use_gtk) {
 		pr_err("--show-nr-samples is not available in --gtk mode at this time\n");
 		return ret;
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 2e4db8216b3b..b04d825cee50 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -190,9 +190,15 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
 		return;
 	}
 
-	if (notes->options->hide_src_code) {
+	if (notes->options->hide_src_code && notes->options->hide_haz_data) {
 		from = cursor->al.idx_asm;
 		to = target->idx_asm;
+	} else if (!notes->options->hide_src_code && notes->options->hide_haz_data) {
+		from = cursor->al.idx_asm + cursor->al.idx_src + 1;
+		to = target->idx_asm + target->idx_src + 1;
+	} else if (notes->options->hide_src_code && !notes->options->hide_haz_data) {
+		from = cursor->al.idx_asm + cursor->al.idx_haz + 1;
+		to = target->idx_asm + target->idx_haz + 1;
 	} else {
 		from = (u64)cursor->al.idx;
 		to = (u64)target->idx;
@@ -293,8 +299,13 @@ static void annotate_browser__set_rb_top(struct annotate_browser *browser,
 	struct annotation_line * pos = rb_entry(nd, struct annotation_line, rb_node);
 	u32 idx = pos->idx;
 
-	if (notes->options->hide_src_code)
+	if (notes->options->hide_src_code && notes->options->hide_haz_data)
 		idx = pos->idx_asm;
+	else if (!notes->options->hide_src_code && notes->options->hide_haz_data)
+		idx = pos->idx_asm + pos->idx_src + 1;
+	else if (notes->options->hide_src_code && !notes->options->hide_haz_data)
+		idx = pos->idx_asm + pos->idx_haz + 1;
+
 	annotate_browser__set_top(browser, pos, idx);
 	browser->curr_hot = nd;
 }
@@ -348,44 +359,117 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
 	struct annotation *notes = browser__annotation(&browser->b);
 	struct annotation_line *al;
 	off_t offset = browser->b.index - browser->b.top_idx;
+	u32 curr_idx;
 
 	browser->b.seek(&browser->b, offset, SEEK_CUR);
 	al = list_entry(browser->b.top, struct annotation_line, node);
 
 	if (notes->options->hide_src_code) {
-		if (al->idx_asm < offset)
-			offset = al->idx;
+		if (notes->options->hide_haz_data) {
+			curr_idx = al->idx_asm + al->idx_src + 1;
+			browser->b.nr_entries = notes->nr_asm_entries +
+						notes->nr_src_entries;
+		} else {
+			curr_idx = al->idx;
+			browser->b.nr_entries = notes->nr_entries;
+		}
 
-		browser->b.nr_entries = notes->nr_entries;
 		notes->options->hide_src_code = false;
 		browser->b.seek(&browser->b, -offset, SEEK_CUR);
-		browser->b.top_idx = al->idx - offset;
-		browser->b.index = al->idx;
+		browser->b.top_idx = curr_idx - offset;
+		browser->b.index = curr_idx;
 	} else {
-		if (al->idx_asm < 0) {
+		if (al->type != AL_TYPE_ASM) {
 			ui_helpline__puts("Only available for assembly lines.");
 			browser->b.seek(&browser->b, -offset, SEEK_CUR);
 			return false;
 		}
 
-		if (al->idx_asm < offset)
-			offset = al->idx_asm;
+		if (notes->options->hide_haz_data) {
+			curr_idx = al->idx_asm;
+			browser->b.nr_entries = notes->nr_asm_entries;
+		} else {
+			curr_idx = al->idx_asm + al->idx_haz + 1;
+			browser->b.nr_entries = notes->nr_asm_entries +
+						notes->nr_haz_entries;
+		}
+
+		if (curr_idx < offset)
+			offset = curr_idx;
 
-		browser->b.nr_entries = notes->nr_asm_entries;
 		notes->options->hide_src_code = true;
 		browser->b.seek(&browser->b, -offset, SEEK_CUR);
-		browser->b.top_idx = al->idx_asm - offset;
-		browser->b.index = al->idx_asm;
+		browser->b.top_idx = curr_idx - offset;
+		browser->b.index = curr_idx;
+	}
+
+	return true;
+}
+
+static bool annotate_browser__toggle_hazard(struct annotate_browser *browser)
+{
+	struct annotation *notes = browser__annotation(&browser->b);
+	struct annotation_line *al;
+	off_t offset = browser->b.index - browser->b.top_idx;
+	u32 curr_idx;
+
+	browser->b.seek(&browser->b, offset, SEEK_CUR);
+	al = list_entry(browser->b.top, struct annotation_line, node);
+
+	if (notes->options->hide_haz_data) {
+		if (notes->options->hide_src_code) {
+			curr_idx = al->idx_asm + al->idx_haz + 1;
+			browser->b.nr_entries = notes->nr_asm_entries +
+						notes->nr_haz_entries;
+		} else {
+			curr_idx = al->idx;
+			browser->b.nr_entries = notes->nr_entries;
+		}
+
+		notes->options->hide_haz_data = false;
+		browser->b.seek(&browser->b, -offset, SEEK_CUR);
+		browser->b.top_idx = curr_idx - offset;
+		browser->b.index = curr_idx;
+	} else {
+		if (al->type != AL_TYPE_ASM) {
+			ui_helpline__puts("Only available for assembly lines.");
+			browser->b.seek(&browser->b, -offset, SEEK_CUR);
+			return false;
+		}
+
+		if (notes->options->hide_src_code) {
+			curr_idx = al->idx_asm;
+			browser->b.nr_entries = notes->nr_asm_entries;
+		} else {
+			curr_idx = al->idx_asm + al->idx_src + 1;
+			browser->b.nr_entries = notes->nr_asm_entries +
+						notes->nr_src_entries;
+		}
+
+		if (curr_idx < offset)
+			offset = curr_idx;
+
+		notes->options->hide_haz_data = true;
+		browser->b.seek(&browser->b, -offset, SEEK_CUR);
+		browser->b.top_idx = curr_idx - offset;
+		browser->b.index = curr_idx;
 	}
 
 	return true;
 }
 
-static void ui_browser__init_asm_mode(struct ui_browser *browser)
+static void ui_browser__hide_src_code(struct ui_browser *browser)
+{
+	struct annotation *notes = browser__annotation(browser);
+	ui_browser__reset_index(browser);
+	browser->nr_entries -= notes->nr_src_entries;
+}
+
+static void ui_browser__hide_haz_data(struct ui_browser *browser)
 {
 	struct annotation *notes = browser__annotation(browser);
 	ui_browser__reset_index(browser);
-	browser->nr_entries = notes->nr_asm_entries;
+	browser->nr_entries -= notes->nr_haz_entries;
 }
 
 #define SYM_TITLE_MAX_SIZE (PATH_MAX + 64)
@@ -743,6 +827,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
 		"o             Toggle disassembler output/simplified view\n"
 		"O             Bump offset level (jump targets -> +call -> all -> cycle thru)\n"
 		"s             Toggle source code view\n"
+		"S             Toggle pipeline hazard/stall view\n"
 		"t             Circulate percent, total period, samples view\n"
 		"c             Show min/max cycle\n"
 		"/             Search string\n"
@@ -767,6 +852,10 @@ static int annotate_browser__run(struct annotate_browser *browser,
 			if (annotate_browser__toggle_source(browser))
 				ui_helpline__puts(help);
 			continue;
+		case 'S':
+			if (annotate_browser__toggle_hazard(browser))
+				ui_helpline__puts(help);
+			continue;
 		case 'o':
 			notes->options->use_offset = !notes->options->use_offset;
 			annotation__update_column_widths(notes);
@@ -932,7 +1021,10 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
 	browser.b.width += 18; /* Percentage */
 
 	if (notes->options->hide_src_code)
-		ui_browser__init_asm_mode(&browser.b);
+		ui_browser__hide_src_code(&browser.b);
+
+	if (notes->options->hide_haz_data)
+		ui_browser__hide_haz_data(&browser.b);
 
 	ret = annotate_browser__run(&browser, evsel, hbt);
 
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 766934b0f36d..d782ee193345 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -66,6 +66,7 @@ struct annotation_options annotation__default_options = {
 	.annotate_src	= true,
 	.offset_level	= ANNOTATION__OFFSET_JUMP_TARGETS,
 	.percent_type	= PERCENT_PERIOD_LOCAL,
+	.hide_haz_data	= true,
 };
 
 static regex_t	 file_lineno;
@@ -1268,7 +1269,7 @@ static struct disasm_line *disasm_line__new(struct annotate_args *args)
 	struct disasm_line *dl = NULL;
 	int nr = 1;
 
-	if (perf_evsel__is_group_event(args->evsel))
+	if (args->type != AL_TYPE_HAZ && perf_evsel__is_group_event(args->evsel))
 		nr = args->evsel->core.nr_members;
 
 	dl = zalloc(disasm_line_size(nr));
@@ -1509,6 +1510,7 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
 	} else if (max_lines && printed >= max_lines)
 		return 1;
 	else {
+		/* TODO: Hazard specific changes */
 		int width = symbol_conf.show_total_period ? 12 : 8;
 
 		if (queue)
@@ -1526,6 +1528,34 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
 	return 0;
 }
 
+static int annotation_add_hazard_detail(struct annotation *notes, s64 offset)
+{
+	int ret;
+	struct hazard_hist *hh;
+	struct disasm_line *dl;
+	struct annotate_args args = {
+		.type = AL_TYPE_HAZ,
+		.offset = -1,
+	};
+
+	list_for_each_entry(hh, &notes->src->haz_hist[offset].list, list) {
+		ret = asprintf(&(args.line), "       {haz_stage: %s, "
+			"haz_reason: %s, stall_stage: %s, stall_reason: %s, "
+			"icache: %s}", hh->haz_stage, hh->haz_reason,
+			hh->stall_stage, hh->stall_reason, hh->icache);
+		if (ret == -1)
+			return -ENOMEM;
+
+		dl = disasm_line__new(&args);
+		free(args.line);
+		if (!dl)
+			return -ENOMEM;
+
+		annotation_line__add(&dl->al, &notes->src->source);
+	}
+	return 0;
+}
+
 /*
  * symbol__parse_objdump_line() parses objdump output (with -d --no-show-raw)
  * which looks like following
@@ -1608,6 +1638,10 @@ static int symbol__parse_objdump_line(struct symbol *sym,
 
 	annotation_line__add(&dl->al, &notes->src->source);
 
+	if (offset != -1 && notes->src->haz_hist &&
+	    !list_empty(&notes->src->haz_hist[offset].list))
+		return annotation_add_hazard_detail(notes, offset);
+
 	return 0;
 }
 
@@ -2701,6 +2735,8 @@ static void annotation__set_offsets(struct annotation *notes, s64 size)
 	notes->max_line_len = 0;
 	notes->nr_entries = 0;
 	notes->nr_asm_entries = 0;
+	notes->nr_src_entries = 0;
+	notes->nr_haz_entries = 0;
 
 	list_for_each_entry(al, &notes->src->source, node) {
 		size_t line_len = strlen(al->line);
@@ -2710,6 +2746,8 @@ static void annotation__set_offsets(struct annotation *notes, s64 size)
 		al->idx = notes->nr_entries++;
 		if (al->type == AL_TYPE_ASM) {
 			al->idx_asm = notes->nr_asm_entries++;
+			al->idx_src = notes->nr_src_entries - 1;
+			al->idx_haz = notes->nr_haz_entries - 1;
 			/*
 			 * FIXME: short term bandaid to cope with assembly
 			 * routines that comes with labels in the same column
@@ -2719,8 +2757,15 @@ static void annotation__set_offsets(struct annotation *notes, s64 size)
  			 */
 			if (al->offset < size)
 				notes->offsets[al->offset] = al;
-		} else
+		} else if (al->type == AL_TYPE_SRC) {
 			al->idx_asm = -1;
+			al->idx_src = notes->nr_src_entries++;
+			al->idx_haz = notes->nr_haz_entries - 1;
+		} else if (al->type == AL_TYPE_HAZ) {
+			al->idx_asm = -1;
+			al->idx_haz = notes->nr_haz_entries++;
+			al->idx_src = notes->nr_src_entries - 1;
+		}
 	}
 }
 
@@ -3051,6 +3096,8 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
 			printed = scnprintf(bf, sizeof(bf), "%-*s  ", notes->widths.addr, " ");
 		obj__printf(obj, bf);
 		obj__printf(obj, "%-*s", width - printed - pcnt_width - cycles_width + 1, al->line);
+	} else if (al->type == AL_TYPE_HAZ) {
+		obj__printf(obj, "%-*s", width - pcnt_width - cycles_width, al->line);
 	} else {
 		u64 addr = al->offset;
 		int color = -1;
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index a3803f89b8fc..3a082c13bdad 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -85,7 +85,8 @@ struct annotation_options {
 	     show_nr_jumps,
 	     show_minmax_cycle,
 	     show_asm_raw,
-	     annotate_src;
+	     annotate_src,
+	     hide_haz_data;
 	u8   offset_level;
 	int  min_pcnt;
 	int  max_lines;
@@ -131,8 +132,16 @@ struct annotation_data {
 enum annotation_line_type {
 	AL_TYPE_ASM = 0,
 	AL_TYPE_SRC,
+	AL_TYPE_HAZ,
 };
 
+/*
+ * @idx_asm, @idx_src and @idx_haz starts from -1 and increments in
+ * chronological order. For _all_ non-assembly lines, @idx_asm is -1.
+ * OTOH, for non-source line, @idx_src contains prev value. Similarly
+ * for non-hazard lines @idx_haz also contains prev value.
+ * And @idx = @idx_asm + (@idx_src + 1) + (@idx_haz + 1);
+ */
 struct annotation_line {
 	struct list_head	 node;
 	struct rb_node		 rb_node;
@@ -148,6 +157,8 @@ struct annotation_line {
 	char			*path;
 	u32			 idx;
 	int			 idx_asm;
+	int			 idx_src;
+	int			 idx_haz;
 	int			 data_nr;
 	struct annotation_data	 data[0];
 };
@@ -298,6 +309,8 @@ struct annotation {
 	int			max_jump_sources;
 	int			nr_entries;
 	int			nr_asm_entries;
+	int			nr_src_entries;
+	int			nr_haz_entries;
 	u16			max_line_len;
 	struct {
 		u8		addr;
@@ -326,7 +339,8 @@ static inline int annotation__pcnt_width(struct annotation *notes)
 
 static inline bool annotation_line__filter(struct annotation_line *al, struct annotation *notes)
 {
-	return notes->options->hide_src_code && al->type == AL_TYPE_SRC;
+	return ((notes->options->hide_src_code && al->type == AL_TYPE_SRC) ||
+		(notes->options->hide_haz_data && al->type == AL_TYPE_HAZ));
 }
 
 void annotation__update_column_widths(struct annotation *notes);
-- 
2.21.1


^ permalink raw reply related

* [RFC 09/11] perf annotate: Introduce type for annotation_line
From: Ravi Bangoria @ 2020-03-02  5:23 UTC (permalink / raw)
  To: linuxppc-dev, linux-kernel
  Cc: mark.rutland, ravi.bangoria, ak, maddy, peterz, alexey.budankov,
	adrian.hunter, acme, alexander.shishkin, yao.jin, mingo, paulus,
	eranian, robert.richter, namhyung, kim.phillips, jolsa, kan.liang
In-Reply-To: <20200302052355.36365-1-ravi.bangoria@linux.ibm.com>

struct annotation_line can contain either assembly instruction or
a source code line. To distinguish between them we currently use
offset. If offset is -1, it's a source otherwise it's assembly.
This is bit cryptic when you first read the code. Introduce new
field 'type' that denotes type of the data the annotation_line
object contains.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
 tools/perf/ui/browsers/annotate.c |  4 ++--
 tools/perf/ui/gtk/annotate.c      |  6 +++---
 tools/perf/util/annotate.c        | 27 ++++++++++++++++-----------
 tools/perf/util/annotate.h        |  8 +++++++-
 4 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 9023267e5643..2e4db8216b3b 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -317,7 +317,7 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
 		double max_percent = 0.0;
 		int i;
 
-		if (pos->al.offset == -1) {
+		if (pos->al.type != AL_TYPE_ASM) {
 			RB_CLEAR_NODE(&pos->al.rb_node);
 			continue;
 		}
@@ -816,7 +816,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
 
 			if (browser->selection == NULL)
 				ui_helpline__puts("Huh? No selection. Report to linux-kernel@vger.kernel.org");
-			else if (browser->selection->offset == -1)
+			else if (browser->selection->type != AL_TYPE_ASM)
 				ui_helpline__puts("Actions are only available for assembly lines.");
 			else if (!dl->ins.ops)
 				goto show_sup_ins;
diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index 35f9641bf670..71c792a0b17d 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -35,7 +35,7 @@ static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym,
 
 	strcpy(buf, "");
 
-	if (dl->al.offset == (s64) -1)
+	if (dl->al.type != AL_TYPE_ASM)
 		return 0;
 
 	symhist = annotation__histogram(symbol__annotation(sym), evidx);
@@ -61,7 +61,7 @@ static int perf_gtk__get_offset(char *buf, size_t size, struct map_symbol *ms,
 
 	strcpy(buf, "");
 
-	if (dl->al.offset == (s64) -1)
+	if (dl->al.type != AL_TYPE_ASM)
 		return 0;
 
 	return scnprintf(buf, size, "%"PRIx64, start + dl->al.offset);
@@ -78,7 +78,7 @@ static int perf_gtk__get_line(char *buf, size_t size, struct disasm_line *dl)
 	if (!line)
 		return 0;
 
-	if (dl->al.offset != (s64) -1)
+	if (dl->al.type == AL_TYPE_ASM)
 		markup = NULL;
 
 	if (markup)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 4e2706274d85..8aef60a6ffea 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1148,6 +1148,7 @@ struct annotate_args {
 	struct evsel		  *evsel;
 	struct annotation_options *options;
 	s64			  offset;
+	u8			  type;
 	char			  *line;
 	int			  line_nr;
 };
@@ -1156,6 +1157,7 @@ static void annotation_line__init(struct annotation_line *al,
 				  struct annotate_args *args,
 				  int nr)
 {
+	al->type = args->type;
 	al->offset = args->offset;
 	al->line = strdup(args->line);
 	al->line_nr = args->line_nr;
@@ -1202,7 +1204,7 @@ static struct disasm_line *disasm_line__new(struct annotate_args *args)
 	if (dl->al.line == NULL)
 		goto out_delete;
 
-	if (args->offset != -1) {
+	if (dl->al.type == AL_TYPE_ASM) {
 		if (disasm_line__parse(dl->al.line, &dl->ins.name, &dl->ops.raw) < 0)
 			goto out_free_line;
 
@@ -1246,7 +1248,7 @@ struct annotation_line *
 annotation_line__next(struct annotation_line *pos, struct list_head *head)
 {
 	list_for_each_entry_continue(pos, head, node)
-		if (pos->offset >= 0)
+		if (pos->type == AL_TYPE_ASM)
 			return pos;
 
 	return NULL;
@@ -1357,7 +1359,7 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
 	static const char *prev_line;
 	static const char *prev_color;
 
-	if (al->offset != -1) {
+	if (al->type == AL_TYPE_ASM) {
 		double max_percent = 0.0;
 		int i, nr_percent = 1;
 		const char *color;
@@ -1500,6 +1502,7 @@ static int symbol__parse_objdump_line(struct symbol *sym,
 	}
 
 	args->offset  = offset;
+	args->type    = (offset != -1) ? AL_TYPE_ASM : AL_TYPE_SRC;
 	args->line    = parsed_line;
 	args->line_nr = *line_nr;
 	args->ms.sym  = sym;
@@ -1783,6 +1786,7 @@ static int symbol__disassemble_bpf(struct symbol *sym,
 		fflush(s);
 
 		if (!opts->hide_src_code && srcline) {
+			args->type = AL_TYPE_SRC;
 			args->offset = -1;
 			args->line = strdup(srcline);
 			args->line_nr = 0;
@@ -1794,6 +1798,7 @@ static int symbol__disassemble_bpf(struct symbol *sym,
 			}
 		}
 
+		args->type = AL_TYPE_ASM;
 		args->offset = pc;
 		args->line = buf + prev_buf_size;
 		args->line_nr = 0;
@@ -2099,7 +2104,7 @@ static void annotation__calc_percent(struct annotation *notes,
 		s64 end;
 		int i = 0;
 
-		if (al->offset == -1)
+		if (al->type != AL_TYPE_ASM)
 			continue;
 
 		next = annotation_line__next(al, &notes->src->source);
@@ -2309,7 +2314,7 @@ static int annotated_source__addr_fmt_width(struct list_head *lines, u64 start)
 	struct annotation_line *line;
 
 	list_for_each_entry_reverse(line, lines, node) {
-		if (line->offset != -1)
+		if (line->type == AL_TYPE_ASM)
 			return scnprintf(bf, sizeof(bf), "%" PRIx64, start + line->offset);
 	}
 
@@ -2549,7 +2554,7 @@ static size_t disasm_line__fprintf(struct disasm_line *dl, FILE *fp)
 {
 	size_t printed;
 
-	if (dl->al.offset == -1)
+	if (dl->al.type == AL_TYPE_SRC)
 		return fprintf(fp, "%s\n", dl->al.line);
 
 	printed = fprintf(fp, "%#" PRIx64 " %s", dl->al.offset, dl->ins.name);
@@ -2628,7 +2633,7 @@ static void annotation__set_offsets(struct annotation *notes, s64 size)
 		if (notes->max_line_len < line_len)
 			notes->max_line_len = line_len;
 		al->idx = notes->nr_entries++;
-		if (al->offset != -1) {
+		if (al->type == AL_TYPE_ASM) {
 			al->idx_asm = notes->nr_asm_entries++;
 			/*
 			 * FIXME: short term bandaid to cope with assembly
@@ -2659,7 +2664,7 @@ static int annotation__max_ins_name(struct annotation *notes)
 	struct annotation_line *al;
 
         list_for_each_entry(al, &notes->src->source, node) {
-		if (al->offset == -1)
+		if (al->type != AL_TYPE_ASM)
 			continue;
 
 		len = strlen(disasm_line(al)->ins.name);
@@ -2875,7 +2880,7 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
 	char bf[256];
 	int printed;
 
-	if (first_line && (al->offset == -1 || percent_max == 0.0)) {
+	if (first_line && (al->type != AL_TYPE_ASM || percent_max == 0.0)) {
 		if (notes->have_cycles) {
 			if (al->ipc == 0.0 && al->cycles == 0)
 				show_title = true;
@@ -2883,7 +2888,7 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
 			show_title = true;
 	}
 
-	if (al->offset != -1 && percent_max != 0.0) {
+	if (al->type == AL_TYPE_ASM && percent_max != 0.0) {
 		int i;
 
 		for (i = 0; i < notes->nr_events; i++) {
@@ -2964,7 +2969,7 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
 
 	if (!*al->line)
 		obj__printf(obj, "%-*s", width - pcnt_width - cycles_width, " ");
-	else if (al->offset == -1) {
+	else if (al->type == AL_TYPE_SRC) {
 		if (al->line_nr && notes->options->show_linenr)
 			printed = scnprintf(bf, sizeof(bf), "%-*d ", notes->widths.addr + 1, al->line_nr);
 		else
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index d9b5bb105056..89839713d242 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -128,9 +128,15 @@ struct annotation_data {
 	struct sym_hist_entry	 he;
 };
 
+enum annotation_line_type {
+	AL_TYPE_ASM = 0,
+	AL_TYPE_SRC,
+};
+
 struct annotation_line {
 	struct list_head	 node;
 	struct rb_node		 rb_node;
+	u8			 type;
 	s64			 offset;
 	char			*line;
 	int			 line_nr;
@@ -310,7 +316,7 @@ static inline int annotation__pcnt_width(struct annotation *notes)
 
 static inline bool annotation_line__filter(struct annotation_line *al, struct annotation *notes)
 {
-	return notes->options->hide_src_code && al->offset == -1;
+	return notes->options->hide_src_code && al->type == AL_TYPE_SRC;
 }
 
 void annotation__update_column_widths(struct annotation *notes);
-- 
2.21.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox