Re: [RFC PATCH 2/3] perf tools: Add support for "report" for some spe events

[Tan Xiaojun]

On 2019/10/17 9:51, Tan Xiaojun wrote:
> On 2019/10/16 18:12, James Clark wrote:
>> Hi Xiaojun,
>>
>>>>
>>>> What do you mean when the user specifies "event:pp", if the SPE is 
>>>> available, configure and record the spe data directly via the perf event 
>>>> open syscall?
>>>> (perf.data itself is the same as using -e arm_spe_0//xxx?)
>>>
>>> I mean, for the perf record, if the user does not add ":pp" to these 
>>> events, the original process is taken, and if ":pp" is added, the spe 
>>> process is taken.
>>>
>>
>> Yes we think this is the best way to do it considering that SPE has been 
>> implemented as a separate PMU and it will be very difficult to do it in the 
>> Kernel when the precise_ip attribute is set.
>>
>> I think doing everything in userspace is easiest. This will at least mean 
>> that users of Perf don't have to be aware of the details of SPE to get 
>> precise sample data.
>>
>> So if the user specifies "event:p" when SPE is available, the SPE PMU is 
>> automatically configured data is recorded. If the user also specifies -e 
>> arm_spe_0//xxx and wants to do some manual configuration, then that could 
>> override the automatic configuration.
>>
>>
>> James
>>
>>
>>
> 
> OK. I got it.
> 
> I found a bug in the test. If I specify cpu_list(use -a or -C) when logging 
> spe data, some events with "pid:0 tid:0" is logged. This is obviously wrong.
> 
> I want to solve this problem, but I haven't found out what went wrong.
> 
> --
> [root@server121 perf]# perf record -e 
> arm_spe_0/branch_filter=1,ts_enable=1,pa_enable=1,load_filter=1,jitter=0,store_filter=1,min_latency=0/
>  -a

Sorry, it should add "--all-user" here, and finally there will still be some 
"pid:0" events in spe_dump.out. 
(And if kernel event is included, then "pid:0" is not a problem)

This causes the pc address of some spe sampled data to be untranslated because 
the wrong pid/tid is obtained from here.

Thanks.
Xiaojun.

> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 7.925 MB perf.data ]
> [root@server121 perf]# perf report -D > spe_dump.out
> [root@server121 perf]# vim spe_dump.out
> 
> --
> ...
> 0xd0330 [0x30]: event: 12
> .
> . ... raw event: size 48 bytes
> .  :  0c 00 00 00 00 00 30 00 00 00 00 00 00 00 00 00  ..0.
> .  0010:  00 00 00 00 00 00 00 00 f8 d9 fe bd f7 08 02 00  
> .  0020:  00 00 00 00 00 00 00 00 4c bc 14 00 00 00 00 00  L...
> 
> 0 572810090961400 0xd0330 [0x30]: PERF_RECORD_ITRACE_START pid: 0 tid: 0
> 
> 0xd0438 [0x30]: event: 12
> .
> . ... raw event: size 48 bytes
> .  :  0c 00 00 00 00 00 30 00 00 00 00 00 00 00 00 00  ..0.
> .  0010:  00 00 00 00 00 00 00 00 d8 ef fe bd f7 08 02 00  
> .  0020:  01 00 00 00 00 00 00 00 4d bc 14 00 00 00 00 00  M...
> 
> 1 572810090967000 0xd0438 [0x30]: PERF_RECORD_ITRACE_START pid: 0 tid: 0
> ...
> --
> 
> Thanks.
> Xiaojun.
> 
> 
> .
> 

Re: [RFC PATCH 2/3] perf tools: Add support for "report" for some spe events

[Tan Xiaojun]

On 2019/10/16 18:12, James Clark wrote:
> Hi Xiaojun,
> 
>>>
>>> What do you mean when the user specifies "event:pp", if the SPE is 
>>> available, configure and record the spe data directly via the perf event 
>>> open syscall?
>>> (perf.data itself is the same as using -e arm_spe_0//xxx?)
>>
>> I mean, for the perf record, if the user does not add ":pp" to these events, 
>> the original process is taken, and if ":pp" is added, the spe process is 
>> taken.
>>
> 
> Yes we think this is the best way to do it considering that SPE has been 
> implemented as a separate PMU and it will be very difficult to do it in the 
> Kernel when the precise_ip attribute is set.
> 
> I think doing everything in userspace is easiest. This will at least mean 
> that users of Perf don't have to be aware of the details of SPE to get 
> precise sample data.
> 
> So if the user specifies "event:p" when SPE is available, the SPE PMU is 
> automatically configured data is recorded. If the user also specifies -e 
> arm_spe_0//xxx and wants to do some manual configuration, then that could 
> override the automatic configuration.
> 
> 
> James
> 
> 
> 

OK. I got it.

I found a bug in the test. If I specify cpu_list(use -a or -C) when logging spe 
data, some events with "pid:0 tid:0" is logged. This is obviously wrong.

I want to solve this problem, but I haven't found out what went wrong.

--
[root@server121 perf]# perf record -e 
arm_spe_0/branch_filter=1,ts_enable=1,pa_enable=1,load_filter=1,jitter=0,store_filter=1,min_latency=0/
 -a
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 7.925 MB perf.data ]
[root@server121 perf]# perf report -D > spe_dump.out
[root@server121 perf]# vim spe_dump.out

--
...
0xd0330 [0x30]: event: 12
.
. ... raw event: size 48 bytes
.  :  0c 00 00 00 00 00 30 00 00 00 00 00 00 00 00 00  ..0.
.  0010:  00 00 00 00 00 00 00 00 f8 d9 fe bd f7 08 02 00  
.  0020:  00 00 00 00 00 00 00 00 4c bc 14 00 00 00 00 00  L...

0 572810090961400 0xd0330 [0x30]: PERF_RECORD_ITRACE_START pid: 0 tid: 0

0xd0438 [0x30]: event: 12
.
. ... raw event: size 48 bytes
.  :  0c 00 00 00 00 00 30 00 00 00 00 00 00 00 00 00  ..0.
.  0010:  00 00 00 00 00 00 00 00 d8 ef fe bd f7 08 02 00  
.  0020:  01 00 00 00 00 00 00 00 4d bc 14 00 00 00 00 00  M...

1 572810090967000 0xd0438 [0x30]: PERF_RECORD_ITRACE_START pid: 0 tid: 0
...
--

Thanks.
Xiaojun.

Re: [RFC PATCH 2/3] perf tools: Add support for "report" for some spe events

[Tan Xiaojun]

On 2019/10/9 19:09, Tan Xiaojun wrote:
> On 2019/10/9 17:48, James Clark wrote:
>> Hi Xiaojun,
>>
>>> By the way, you mentioned before that you want the spe event to be in the 
>>> form of "event:pp" like pebs. Is that the whole framework should be made 
>>> similar to pebs? Or is it just a modification to the command format? 
>>
>> We're currently still investigating if it makes sense to modify the Perf 
>> event open syscall to use SPE when the "precise_ip" attribute is set. And 
>> then synthesize samples using the SPE data when available. This would keep 
>> the syscall interface more consistent between architectures.
>>
>> And if tools other than Perf want more precise data, they don't have to be 
>> aware of SPE or any of the implementation defined details of it. For example 
>> the 'data source' encoding can be different from one micro architecture to 
>> the next. The kernel is probably the best place to handle this.
>>
>> At the moment, every tool that wants to use the Perf syscall to get precise 
>> data on ARM would have to be aware of SPE and implement their own decoding.
>>
> 
> Hi James,
> 
> What do you mean when the user specifies "event:pp", if the SPE is available, 
> configure and record the spe data directly via the perf event open syscall?
> (perf.data itself is the same as using -e arm_spe_0//xxx?)

I mean, for the perf record, if the user does not add ":pp" to these events, 
the original process is taken, and if ":pp" is added, the spe process is taken.

Xiaojun.

> 
> OK. If I have not misunderstood, I think I know how to do it.
> Thank you.
> 
>>> For the former, this may be a bit difficult. For the latter, there is 
>>> currently no modification to the record part, so "-c -F, etc." is only for 
>>> instructions rather than events, so it may be misunderstood by users.
>>>
>>> So I haven't figured out how to do. What do you think of this?
>>
>> I think the patch at the moment is a good start to make SPE more accessible. 
>> And the changes I mentioned above wouldn't change the fact that the raw SPE 
>> data would still be available via the SPE PMU. So I think continuing with 
>> the patch as-is for now is the best idea.
>>
> 
> Yes. I agree.
> 
> Xiaojun.
> 
>>
>> James
>>
>>
> 

Re: [RFC PATCH 2/3] perf tools: Add support for "report" for some spe events

[Tan Xiaojun]

On 2019/10/9 17:48, James Clark wrote:
> Hi Xiaojun,
> 
>> By the way, you mentioned before that you want the spe event to be in the 
>> form of "event:pp" like pebs. Is that the whole framework should be made 
>> similar to pebs? Or is it just a modification to the command format? 
> 
> We're currently still investigating if it makes sense to modify the Perf 
> event open syscall to use SPE when the "precise_ip" attribute is set. And 
> then synthesize samples using the SPE data when available. This would keep 
> the syscall interface more consistent between architectures.
> 
> And if tools other than Perf want more precise data, they don't have to be 
> aware of SPE or any of the implementation defined details of it. For example 
> the 'data source' encoding can be different from one micro architecture to 
> the next. The kernel is probably the best place to handle this.
> 
> At the moment, every tool that wants to use the Perf syscall to get precise 
> data on ARM would have to be aware of SPE and implement their own decoding.
> 

Hi James,

What do you mean when the user specifies "event:pp", if the SPE is available, 
configure and record the spe data directly via the perf event open syscall?
(perf.data itself is the same as using -e arm_spe_0//xxx?)

OK. If I have not misunderstood, I think I know how to do it.
Thank you.

>> For the former, this may be a bit difficult. For the latter, there is 
>> currently no modification to the record part, so "-c -F, etc." is only for 
>> instructions rather than events, so it may be misunderstood by users.
>>
>> So I haven't figured out how to do. What do you think of this?
> 
> I think the patch at the moment is a good start to make SPE more accessible. 
> And the changes I mentioned above wouldn't change the fact that the raw SPE 
> data would still be available via the SPE PMU. So I think continuing with the 
> patch as-is for now is the best idea.
> 

Yes. I agree.

Xiaojun.

> 
> James
> 
> 

Re: [RFC PATCH 2/3] perf tools: Add support for "report" for some spe events

[Tan Xiaojun]

On 2019/10/4 21:46, James Clark wrote:
> Hi Xiaojun,
> 
> I wanted to ask if you are still working on this?
> 
> I've noticed that it doesn't apply cleanly to perf/core anymore and I was 
> working on re-basing it.
> Would you be interested in me posting my progress?
> 
> I was also interested in decoding the "data source" of events and displaying 
> that information. Does this
> clash with any of your current work?
> 
> 
> Thanks
> James
> 

(Sorry, you may have received a lot of this email because I am suddenly not on 
the mail-list, I need to confirm it.)

Hi, James,

Sorry, I did not respond in time because of the National Day holiday in China.

I am still doing this, but I have been scheduled for other tasks some time ago, 
so that there is no obvious progress on spe.

By the way, you mentioned before that you want the spe event to be in the form 
of "event:pp" like pebs. Is that the whole framework should be made similar to 
pebs? Or is it just a modification to the command format? For the former, this 
may be a bit difficult. For the latter, there is currently no modification to 
the record part, so "-c -F, etc." is only for instructions rather than events, 
so it may be misunderstood by users.

So I haven't figured out how to do. What do you think of this?

Thanks.
Xiaojun.

> On 09/08/2019 07:12, Tan Xiaojun wrote:
>> On 2019/8/9 5:00, Jeremy Linton wrote:
>>> Hi,
>>>
>>> First thanks for posting this!
>>>
>>> I ran this on our DAWN platform and it does what it says. Its a pretty 
>>> reasonable start, but I get -1's in the command row rather than "dd" (or 
>>> similar) and this also results in [unknown] for the shared object and most 
>>> userspace addresses. This is quite possibly something I'm not doing right, 
>>> but I didn't spend a lot of time testing/debugging it.
>>>
>>> I did a quick glance at the code to, and had a couple comments, although 
>>> I'm not a perf tool expert.
>>>
>>
>> Hi,
>>
>> Thank you for your reply.
>>
>> I have only recently started working on this aspect of the perf tool, so 
>> your reply is very important to me.
>>
>> I need to be sorry, my example here is not complete, until you said that I 
>> found that I only posted a part of the example. The complete example is as 
>> follows:
>>
>> Example usage:
>>
>> # perf record -e arm_spe/ts_enable=1,pa_enable=1/ dd if=/dev/zero 
>> of=/dev/null count=1
>> # perf report
>>
>> 
>> ...
>> # Samples: 37  of event 'llc-miss'
>> # Event count (approx.): 37
>> #
>> # Children  Self  Command  Shared Object  Symbol
>> #     ...  .  
>> 
>> #
>> 37.84%37.84%  dd   [kernel.kallsyms]  [k] 
>> perf_iterate_ctx.constprop.64
>> 16.22%16.22%  dd   [kernel.kallsyms]  [k] copy_page
>>  5.41% 5.41%  dd   [kernel.kallsyms]  [k] find_vma
>>  5.41% 5.41%  dd   [kernel.kallsyms]  [k] perf_event_mmap
>>  5.41% 5.41%  dd   [kernel.kallsyms]  [k] zap_pte_range
>>  5.41% 5.41%  dd   ld-2.28.so [.] _dl_lookup_symbol_x
>>  5.41% 5.41%  dd   libc-2.28.so   [.] _nl_intern_locale_data
>>  2.70% 2.70%  dd   [kernel.kallsyms]  [k] 
>> __remove_shared_vm_struct.isra.1
>>  2.70% 2.70%  dd   [kernel.kallsyms]  [k] kmem_cache_free
>>  2.70% 2.70%  dd   [kernel.kallsyms]  [k] ttwu_do_wakeup.isra.19
>>  2.70% 2.70%  dd   dd [.] 0xd9d8
>>  2.70% 2.70%  dd   ld-2.28.so [.] _dl_relocate_object
>>  2.70% 2.70%  dd   libc-2.28.so   [.] __unregister_atfork
>>  2.70% 2.70%  dd   libc-2.28.so   [.] _dl_addr
>>
>>
>> # Samples: 8  of event 'tlb-miss'
>> # Event count (approx.): 8
>> #
>> # Children  Self  Command  Shared Object  Symbol
>> #     ...  .  
>> .
>> #
>> 12.50%12.50%  dd   [kernel.kallsyms]  [k] __audit_syscall_entry
>> 12.50%12.50%  dd   [kernel.kallsyms]  [k] kmem_cache_free
>> 12.50%12.50%  dd   [kernel.kallsyms]  [k] 
>> perf_iterate_ctx.constprop.64
>> 12.50%12.50%  dd   [kernel.kallsyms]  [k] ttwu_do_wakeup.isra.19
>> 12.50%12.50%  dd   dd [.] 0xd9d8
>> 12.5

Re: [RFC PATCH 2/3] perf tools: Add support for "report" for some spe events

[Tan Xiaojun]

On 2019/10/4 21:46, James Clark wrote:
> Hi Xiaojun,
> 
> I wanted to ask if you are still working on this?
> 
> I've noticed that it doesn't apply cleanly to perf/core anymore and I was 
> working on re-basing it.
> Would you be interested in me posting my progress?
> 
> I was also interested in decoding the "data source" of events and displaying 
> that information. Does this
> clash with any of your current work?
> 
> 
> Thanks
> James
> 

Hi, James,

Sorry, I did not respond in time because of the National Day holiday in China.

I am still doing this, but I have been scheduled for other tasks some time ago, 
so that there is no obvious progress on spe.

By the way, you mentioned before that you want the spe event to be in the form 
of "event:pp" like pebs. Is that the whole framework should be made similar to 
pebs? Or is it just a modification to the command format? For the former, this 
may be a bit difficult. For the latter, there is currently no modification to 
the record part, so "-c -F, etc." is only for instructions rather than events, 
so it may be misunderstood by users.

So I haven't figured out how to do. What do you think of this?

Thanks.
Xiaojun.

> On 09/08/2019 07:12, Tan Xiaojun wrote:
>> On 2019/8/9 5:00, Jeremy Linton wrote:
>>> Hi,
>>>
>>> First thanks for posting this!
>>>
>>> I ran this on our DAWN platform and it does what it says. Its a pretty 
>>> reasonable start, but I get -1's in the command row rather than "dd" (or 
>>> similar) and this also results in [unknown] for the shared object and most 
>>> userspace addresses. This is quite possibly something I'm not doing right, 
>>> but I didn't spend a lot of time testing/debugging it.
>>>
>>> I did a quick glance at the code to, and had a couple comments, although 
>>> I'm not a perf tool expert.
>>>
>>
>> Hi,
>>
>> Thank you for your reply.
>>
>> I have only recently started working on this aspect of the perf tool, so 
>> your reply is very important to me.
>>
>> I need to be sorry, my example here is not complete, until you said that I 
>> found that I only posted a part of the example. The complete example is as 
>> follows:
>>
>> Example usage:
>>
>> # perf record -e arm_spe/ts_enable=1,pa_enable=1/ dd if=/dev/zero 
>> of=/dev/null count=1
>> # perf report
>>
>> 
>> ...
>> # Samples: 37  of event 'llc-miss'
>> # Event count (approx.): 37
>> #
>> # Children  Self  Command  Shared Object  Symbol
>> #     ...  .  
>> 
>> #
>> 37.84%37.84%  dd   [kernel.kallsyms]  [k] 
>> perf_iterate_ctx.constprop.64
>> 16.22%16.22%  dd   [kernel.kallsyms]  [k] copy_page
>>  5.41% 5.41%  dd   [kernel.kallsyms]  [k] find_vma
>>  5.41% 5.41%  dd   [kernel.kallsyms]  [k] perf_event_mmap
>>  5.41% 5.41%  dd   [kernel.kallsyms]  [k] zap_pte_range
>>  5.41% 5.41%  dd   ld-2.28.so [.] _dl_lookup_symbol_x
>>  5.41% 5.41%  dd   libc-2.28.so   [.] _nl_intern_locale_data
>>  2.70% 2.70%  dd   [kernel.kallsyms]  [k] 
>> __remove_shared_vm_struct.isra.1
>>  2.70% 2.70%  dd   [kernel.kallsyms]  [k] kmem_cache_free
>>  2.70% 2.70%  dd   [kernel.kallsyms]  [k] ttwu_do_wakeup.isra.19
>>  2.70% 2.70%  dd   dd [.] 0xd9d8
>>  2.70% 2.70%  dd   ld-2.28.so [.] _dl_relocate_object
>>  2.70% 2.70%  dd   libc-2.28.so   [.] __unregister_atfork
>>  2.70% 2.70%  dd   libc-2.28.so   [.] _dl_addr
>>
>>
>> # Samples: 8  of event 'tlb-miss'
>> # Event count (approx.): 8
>> #
>> # Children  Self  Command  Shared Object  Symbol
>> #     ...  .  
>> .
>> #
>> 12.50%12.50%  dd   [kernel.kallsyms]  [k] __audit_syscall_entry
>> 12.50%12.50%  dd   [kernel.kallsyms]  [k] kmem_cache_free
>> 12.50%12.50%  dd   [kernel.kallsyms]  [k] 
>> perf_iterate_ctx.constprop.64
>> 12.50%12.50%  dd   [kernel.kallsyms]  [k] ttwu_do_wakeup.isra.19
>> 12.50%12.50%  dd   dd [.] 0xd9d8
>> 12.50%12.50%  dd   libc-2.28.so   [.] __unregister_atfork
>> 12.50%12.50%  dd   libc-2.

Re: [RFC PATCH 2/3] perf tools: Add support for "report" for some spe events

[Tan Xiaojun]

On 2019/10/4 21:46, James Clark wrote:
> Hi Xiaojun,
> 
> I wanted to ask if you are still working on this?
> 
> I've noticed that it doesn't apply cleanly to perf/core anymore and I was 
> working on re-basing it.
> Would you be interested in me posting my progress?
> 
> I was also interested in decoding the "data source" of events and displaying 
> that information. Does this
> clash with any of your current work?
> 
> 
> Thanks
> James
> 

Hi, James,

Sorry, I did not respond in time because of the National Day holiday in China.

I am still doing this, but I have been scheduled for other tasks some time ago, 
so that there is no obvious progress on spe.

By the way, you mentioned before that you want the spe event to be in the form 
of "event:pp" like pebs. Is that the whole framework should be made similar to 
pebs? Or is it just a modification to the command format? For the former, this 
may be a bit difficult. For the latter, there is currently no modification to 
the record part, so "-c -F, etc." is only for instructions rather than events, 
so it may be misunderstood by users.

So I haven't figured out how to do. What do you think of this?

Thanks.
Xiaojun.

> On 09/08/2019 07:12, Tan Xiaojun wrote:
>> On 2019/8/9 5:00, Jeremy Linton wrote:
>>> Hi,
>>>
>>> First thanks for posting this!
>>>
>>> I ran this on our DAWN platform and it does what it says. Its a pretty 
>>> reasonable start, but I get -1's in the command row rather than "dd" (or 
>>> similar) and this also results in [unknown] for the shared object and most 
>>> userspace addresses. This is quite possibly something I'm not doing right, 
>>> but I didn't spend a lot of time testing/debugging it.
>>>
>>> I did a quick glance at the code to, and had a couple comments, although 
>>> I'm not a perf tool expert.
>>>
>>
>> Hi,
>>
>> Thank you for your reply.
>>
>> I have only recently started working on this aspect of the perf tool, so 
>> your reply is very important to me.
>>
>> I need to be sorry, my example here is not complete, until you said that I 
>> found that I only posted a part of the example. The complete example is as 
>> follows:
>>
>> Example usage:
>>
>> # perf record -e arm_spe/ts_enable=1,pa_enable=1/ dd if=/dev/zero 
>> of=/dev/null count=1
>> # perf report
>>
>> 
>> ...
>> # Samples: 37  of event 'llc-miss'
>> # Event count (approx.): 37
>> #
>> # Children  Self  Command  Shared Object  Symbol
>> #     ...  .  
>> 
>> #
>> 37.84%37.84%  dd   [kernel.kallsyms]  [k] 
>> perf_iterate_ctx.constprop.64
>> 16.22%16.22%  dd   [kernel.kallsyms]  [k] copy_page
>>  5.41% 5.41%  dd   [kernel.kallsyms]  [k] find_vma
>>  5.41% 5.41%  dd   [kernel.kallsyms]  [k] perf_event_mmap
>>  5.41% 5.41%  dd   [kernel.kallsyms]  [k] zap_pte_range
>>  5.41% 5.41%  dd   ld-2.28.so [.] _dl_lookup_symbol_x
>>  5.41% 5.41%  dd   libc-2.28.so   [.] _nl_intern_locale_data
>>  2.70% 2.70%  dd   [kernel.kallsyms]  [k] 
>> __remove_shared_vm_struct.isra.1
>>  2.70% 2.70%  dd   [kernel.kallsyms]  [k] kmem_cache_free
>>  2.70% 2.70%  dd   [kernel.kallsyms]  [k] ttwu_do_wakeup.isra.19
>>  2.70% 2.70%  dd   dd [.] 0xd9d8
>>  2.70% 2.70%  dd   ld-2.28.so [.] _dl_relocate_object
>>  2.70% 2.70%  dd   libc-2.28.so   [.] __unregister_atfork
>>  2.70% 2.70%  dd   libc-2.28.so   [.] _dl_addr
>>
>>
>> # Samples: 8  of event 'tlb-miss'
>> # Event count (approx.): 8
>> #
>> # Children  Self  Command  Shared Object  Symbol
>> #     ...  .  
>> .
>> #
>> 12.50%12.50%  dd   [kernel.kallsyms]  [k] __audit_syscall_entry
>> 12.50%12.50%  dd   [kernel.kallsyms]  [k] kmem_cache_free
>> 12.50%12.50%  dd   [kernel.kallsyms]  [k] 
>> perf_iterate_ctx.constprop.64
>> 12.50%12.50%  dd   [kernel.kallsyms]  [k] ttwu_do_wakeup.isra.19
>> 12.50%12.50%  dd   dd [.] 0xd9d8
>> 12.50%12.50%  dd   libc-2.28.so   [.] __unregister_atfork
>> 12.50%12.50%  dd   libc-2.

Re: [RFC PATCH 3/3] perf report: add --spe options for arm-spe

[Tan Xiaojun]

On 2019/8/21 20:38, James Clark wrote:
> Hi,
> 
> I also had a look at this and had a question about the --spe option.
> It seems that whatever options I give it, the output is the same:
> 
>   perf report 
> And
>   perf report --spe=t
> 
> Both give the same result:
> 
>   # Samples: 4  of event 'llc-miss'
>   # Event count (approx.): 4
>   #
>   # Children  Self  Command  Shared Object  Symbol
> 
>   #     ...  .  
> ..
>   #
>   ...
>   # Samples: 0  of event 'tlb-miss'
>   # Event count (approx.): 0
>   #
>   # Children  Self  Command  Shared Object  Symbol
>   #     ...  .  ..
>   #
> 
>   # Samples: 83  of event 'branch-miss'
>   # Event count (approx.): 83
>   #
>   # Children  Self  Command  Shared Object  Symbol
>
>   #     ...  .  
> .
>   #
>   ...
> 
> I would have expected it to not include the branch and LLC sections for the 
> second
> command with --spe=t.
> 

Hi,

Sorry, this should be a bug in my code.

> And that leads me to another point. Does it make sense to have this option as 
> a post
> processing step? SPE already has support for filtering events at collection 
> time with
> the PMSFCR_EL1 register.
> 
> Should we try to make the interface more like PEBS, where you specify which 
> events you
> are interested in doing precise tracing on like this?
> 
>   perf record -e branch-misses:pp
> 
> And then perf could use the modifier to configure SPE so that it only records 
> branch
> misses? The benefits of this would be keeping the user interface for precise 
> tracing
> similar between platforms.
> 

Good suggestion. And I need to spend some time thinking about how to implement 
it.

Thank you for your reply.
Xiaojun.

> Thanks
> James
> 
> On 02/08/2019 10:40, Tan Xiaojun wrote:
>> The previous patch added support in "perf report" for some arm-spe
>> events(llc-miss, tlb-miss, branch-miss). This patch adds their help
>> instructions.
>>
>> Signed-off-by: Tan Xiaojun 
>> ---
>>  tools/perf/Documentation/perf-report.txt | 9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/tools/perf/Documentation/perf-report.txt 
>> b/tools/perf/Documentation/perf-report.txt
>> index 987261d..d998d4b 100644
>> --- a/tools/perf/Documentation/perf-report.txt
>> +++ b/tools/perf/Documentation/perf-report.txt
>> @@ -445,6 +445,15 @@ include::itrace.txt[]
>>  
>>  To disable decoding entirely, use --no-itrace.
>>  
>> +--spe::
>> +Options for decoding arm-spe tracing data. The options are:
>> +
>> +l   synthesize llc miss events
>> +t   synthesize tlb miss events
>> +b   synthesize branch miss events
>> +
>> +The default is all events i.e. the same as --spe=ltb
>> +
>>  --full-source-path::
>>  Show the full path for source files for srcline output.
>>  
>>

[tip:perf/core] perf record: Support aarch64 random socket_id assignment

[tip-bot for Tan Xiaojun]

Commit-ID:  0a4d8fb229dd78f9e0752817339e19e903b37a60
Gitweb: https://git.kernel.org/tip/0a4d8fb229dd78f9e0752817339e19e903b37a60
Author: Tan Xiaojun 
AuthorDate: Fri, 2 Aug 2019 11:48:57 +0800
Committer:  Arnaldo Carvalho de Melo 
CommitDate: Wed, 14 Aug 2019 11:00:00 -0300

perf record: Support aarch64 random socket_id assignment

Same as in the commit 01766229533f ("perf record: Support s390 random
socket_id assignment"), aarch64 also have this problem.

Without this fix:

  [root@localhost perf]# ./perf report --header -I -v
  ...
  socket_id number is too big.You may need to upgrade the perf tool.

  # 
  # captured on: Thu Aug  1 22:58:38 2019
  # header version : 1
  ...
  # Core ID and Socket ID information is not available
  ...

With this fix:
  [root@localhost perf]# ./perf report --header -I -v
  ...
  cpumask list: 0-31
  cpumask list: 32-63
  cpumask list: 64-95
  cpumask list: 96-127

  # 
  # captured on: Thu Aug  1 22:58:38 2019
  # header version : 1
  ...
  # CPU 0: Core ID 0, Socket ID 36
  # CPU 1: Core ID 1, Socket ID 36
  ...
  # CPU 126: Core ID 126, Socket ID 8442
  # CPU 127: Core ID 127, Socket ID 8442
  ...

Signed-off-by: Tan Xiaojun 
Acked-by: Jiri Olsa 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: Kan Liang 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Song Liu 
Cc: Steven Rostedt (VMware) 
Cc: Tzvetomir Stoyanov (VMware) 
Link: 
http://lkml.kernel.org/r/1564717737-21602-1-git-send-email-tanxiao...@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/header.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index b04c2b6b28b3..1f2965a07bef 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2252,8 +2252,10 @@ static int process_cpu_topology(struct feat_fd *ff, void 
*data __maybe_unused)
/* On s390 the socket_id number is not related to the numbers of cpus.
 * The socket_id number might be higher than the numbers of cpus.
 * This depends on the configuration.
+* AArch64 is the same.
 */
-   if (ph->env.arch && !strncmp(ph->env.arch, "s390", 4))
+   if (ph->env.arch && (!strncmp(ph->env.arch, "s390", 4)
+ || !strncmp(ph->env.arch, "aarch64", 7)))
do_core_id_test = false;
 
for (i = 0; i < (u32)cpu_nr; i++) {

Re: [RFC PATCH 2/3] perf tools: Add support for "report" for some spe events

[Tan Xiaojun]

On 2019/8/9 5:00, Jeremy Linton wrote:
> Hi,
> 
> First thanks for posting this!
> 
> I ran this on our DAWN platform and it does what it says. Its a pretty 
> reasonable start, but I get -1's in the command row rather than "dd" (or 
> similar) and this also results in [unknown] for the shared object and most 
> userspace addresses. This is quite possibly something I'm not doing right, 
> but I didn't spend a lot of time testing/debugging it.
> 
> I did a quick glance at the code to, and had a couple comments, although I'm 
> not a perf tool expert.
> 

Hi,

Thank you for your reply.

I have only recently started working on this aspect of the perf tool, so your 
reply is very important to me.

I need to be sorry, my example here is not complete, until you said that I 
found that I only posted a part of the example. The complete example is as 
follows:

Example usage:

# perf record -e arm_spe/ts_enable=1,pa_enable=1/ dd if=/dev/zero of=/dev/null 
count=1
# perf report


...
# Samples: 37  of event 'llc-miss'
# Event count (approx.): 37
#
# Children  Self  Command  Shared Object  Symbol
#     ...  .  

#
37.84%37.84%  dd   [kernel.kallsyms]  [k] 
perf_iterate_ctx.constprop.64
16.22%16.22%  dd   [kernel.kallsyms]  [k] copy_page
 5.41% 5.41%  dd   [kernel.kallsyms]  [k] find_vma
 5.41% 5.41%  dd   [kernel.kallsyms]  [k] perf_event_mmap
 5.41% 5.41%  dd   [kernel.kallsyms]  [k] zap_pte_range
 5.41% 5.41%  dd   ld-2.28.so [.] _dl_lookup_symbol_x
 5.41% 5.41%  dd   libc-2.28.so   [.] _nl_intern_locale_data
 2.70% 2.70%  dd   [kernel.kallsyms]  [k] 
__remove_shared_vm_struct.isra.1
 2.70% 2.70%  dd   [kernel.kallsyms]  [k] kmem_cache_free
 2.70% 2.70%  dd   [kernel.kallsyms]  [k] ttwu_do_wakeup.isra.19
 2.70% 2.70%  dd   dd [.] 0xd9d8
 2.70% 2.70%  dd   ld-2.28.so [.] _dl_relocate_object
 2.70% 2.70%  dd   libc-2.28.so   [.] __unregister_atfork
 2.70% 2.70%  dd   libc-2.28.so   [.] _dl_addr


# Samples: 8  of event 'tlb-miss'
# Event count (approx.): 8
#
# Children  Self  Command  Shared Object  Symbol
#     ...  .  
.
#
12.50%12.50%  dd   [kernel.kallsyms]  [k] __audit_syscall_entry
12.50%12.50%  dd   [kernel.kallsyms]  [k] kmem_cache_free
12.50%12.50%  dd   [kernel.kallsyms]  [k] 
perf_iterate_ctx.constprop.64
12.50%12.50%  dd   [kernel.kallsyms]  [k] ttwu_do_wakeup.isra.19
12.50%12.50%  dd   dd [.] 0xd9d8
12.50%12.50%  dd   libc-2.28.so   [.] __unregister_atfork
12.50%12.50%  dd   libc-2.28.so   [.] _nl_intern_locale_data
12.50%12.50%  dd   libc-2.28.so   [.] vfprintf


# Samples: 12  of event 'branch-miss'
# Event count (approx.): 12
#
# Children  Self  Command  Shared Object  Symbol
#     ...  .  ..
#
16.67%16.67%  dd   libc-2.28.so   [.] read_alias_file
 8.33% 8.33%  dd   [kernel.kallsyms]  [k] __arch_copy_from_user
 8.33% 8.33%  dd   [kernel.kallsyms]  [k] __arch_copy_to_user
 8.33% 8.33%  dd   [kernel.kallsyms]  [k] lookup_fast
 8.33% 8.33%  dd   [kernel.kallsyms]  [k] strncpy_from_user
 8.33% 8.33%  dd   ld-2.28.so [.] _dl_lookup_symbol_x
 8.33% 8.33%  dd   ld-2.28.so [.] check_match
 8.33% 8.33%  dd   libc-2.28.so   [.] __GI___printf_fp_l
 8.33% 8.33%  dd   libc-2.28.so   [.] _dl_addr
 8.33% 8.33%  dd   libc-2.28.so   [.] _int_malloc
 8.33% 8.33%  dd   libc-2.28.so   [.] _nl_intern_locale_data



> 
> On 8/2/19 4:40 AM, Tan Xiaojun wrote:
>> After the commit ffd3d18c20b8 ("perf tools: Add ARM Statistical
>> Profiling Extensions (SPE) support") is merged, "perf record" and
>> "perf report --dump-raw-trace" have been supported. However, the
>> raw data that is dumped cannot be used without parsing.
>>
>> This patch is to improve the "perf report" support for spe, and
>> further process the data. Currently, support for the three events
>> of llc-miss, tlb-miss, and branch-miss is added.
>>
>> Example usage:
>>
>> 
>> ...
>>  37.84%    37.84%  dd   [kernel.kallsyms]  [k] 
>> perf_iterate_ctx.constprop.64
>>  16.22%    16.22%  dd   

[RFC PATCH 2/3] perf tools: Add support for "report" for some spe events

[Tan Xiaojun]

After the commit ffd3d18c20b8 ("perf tools: Add ARM Statistical
Profiling Extensions (SPE) support") is merged, "perf record" and
"perf report --dump-raw-trace" have been supported. However, the
raw data that is dumped cannot be used without parsing.

This patch is to improve the "perf report" support for spe, and
further process the data. Currently, support for the three events
of llc-miss, tlb-miss, and branch-miss is added.

Example usage:


...
37.84%37.84%  dd   [kernel.kallsyms]  [k] 
perf_iterate_ctx.constprop.64
16.22%16.22%  dd   [kernel.kallsyms]  [k] copy_page
 5.41% 5.41%  dd   [kernel.kallsyms]  [k] find_vma
 5.41% 5.41%  dd   [kernel.kallsyms]  [k] perf_event_mmap
 5.41% 5.41%  dd   [kernel.kallsyms]  [k] zap_pte_range
 5.41% 5.41%  dd   ld-2.28.so [.] _dl_lookup_symbol_x
 5.41% 5.41%  dd   libc-2.28.so   [.] _nl_intern_locale_data
 2.70% 2.70%  dd   [kernel.kallsyms]  [k] 
__remove_shared_vm_struct.isra.1
 2.70% 2.70%  dd   [kernel.kallsyms]  [k] kmem_cache_free
 2.70% 2.70%  dd   [kernel.kallsyms]  [k] ttwu_do_wakeup.isra.19
 2.70% 2.70%  dd   dd [.] 0xd9d8
 2.70% 2.70%  dd   ld-2.28.so [.] _dl_relocate_object
 2.70% 2.70%  dd   libc-2.28.so   [.] __unregister_atfork
 2.70% 2.70%  dd   libc-2.28.so   [.] _dl_addr

12.50%12.50%  dd   [kernel.kallsyms]  [k] __audit_syscall_entry
12.50%12.50%  dd   [kernel.kallsyms]  [k] kmem_cache_free
12.50%12.50%  dd   [kernel.kallsyms]  [k] 
perf_iterate_ctx.constprop.64
12.50%12.50%  dd   [kernel.kallsyms]  [k] ttwu_do_wakeup.isra.19
12.50%12.50%  dd   dd [.] 0xd9d8
12.50%12.50%  dd   libc-2.28.so   [.] __unregister_atfork
12.50%12.50%  dd   libc-2.28.so   [.] _nl_intern_locale_data
12.50%12.50%  dd   libc-2.28.so   [.] vfprintf

16.67%16.67%  dd   libc-2.28.so   [.] read_alias_file
 8.33% 8.33%  dd   [kernel.kallsyms]  [k] __arch_copy_from_user
 8.33% 8.33%  dd   [kernel.kallsyms]  [k] __arch_copy_to_user
 8.33% 8.33%  dd   [kernel.kallsyms]  [k] lookup_fast
 8.33% 8.33%  dd   [kernel.kallsyms]  [k] strncpy_from_user
 8.33% 8.33%  dd   ld-2.28.so [.] _dl_lookup_symbol_x
 8.33% 8.33%  dd   ld-2.28.so [.] check_match
 8.33% 8.33%  dd   libc-2.28.so   [.] __GI___printf_fp_l
 8.33% 8.33%  dd   libc-2.28.so   [.] _dl_addr
 8.33% 8.33%  dd   libc-2.28.so   [.] _int_malloc
 8.33% 8.33%  dd   libc-2.28.so   [.] _nl_intern_locale_data



After that, more analysis and processing of the raw data of spe
will be done.

Signed-off-by: Tan Xiaojun 
---
 tools/perf/builtin-report.c|   5 +
 tools/perf/util/arm-spe-decoder/Build  |   2 +-
 tools/perf/util/arm-spe-decoder/arm-spe-decoder.c  | 214 ++
 tools/perf/util/arm-spe-decoder/arm-spe-decoder.h  |  51 ++
 .../util/arm-spe-decoder/arm-spe-pkt-decoder.h |   2 +
 tools/perf/util/arm-spe.c  | 715 -
 tools/perf/util/auxtrace.c |  45 ++
 tools/perf/util/auxtrace.h |  27 +
 tools/perf/util/session.h  |   2 +
 9 files changed, 1028 insertions(+), 35 deletions(-)
 create mode 100644 tools/perf/util/arm-spe-decoder/arm-spe-decoder.c
 create mode 100644 tools/perf/util/arm-spe-decoder/arm-spe-decoder.h

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index abf0b9b..fadc8eb 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1007,6 +1007,7 @@ int cmd_report(int argc, const char **argv)
 {
struct perf_session *session;
struct itrace_synth_opts itrace_synth_opts = { .set = 0, };
+   struct arm_spe_synth_opts arm_spe_synth_opts;
struct stat st;
bool has_br_stack = false;
int branch_mode = -1;
@@ -1165,6 +1166,9 @@ int cmd_report(int argc, const char **argv)
OPT_CALLBACK_OPTARG(0, "itrace", _synth_opts, NULL, "opts",
"Instruction Tracing options\n" ITRACE_HELP,
itrace_parse_synth_opts),
+   OPT_CALLBACK_OPTARG(0, "spe", _spe_synth_opts, NULL, "spe opts",
+   "ARM SPE Tracing options",
+   arm_spe_parse_synth_opts),
OPT_BOOLEAN(0, "full-source-path", _full_filename,
"Show fu

[RFC PATCH 0/3] perf tools: Add support for "report" for some spe events

[Tan Xiaojun]

After the commit ffd3d18c20b8 ("perf tools: Add ARM Statistical
Profiling Extensions (SPE) support") is merged, "perf record" and
"perf report --dump-raw-trace" have been supported. However, the
raw data that is dumped cannot be used without parsing.

This patchset is to improve the "perf report" support for spe, and
further process the data. Currently, support for the three events
of llc-miss, tlb-miss, and branch-miss is added.

More details in [2/3].

Tan Xiaojun (3):
  perf tools: Move arm-spe-pkt-decoder.h/c to the new dir
  perf tools: Add support for "report" for some spe events
  perf report: add --spe options for arm-spe

 tools/perf/Documentation/perf-report.txt   |   9 +
 tools/perf/builtin-report.c|   5 +
 tools/perf/util/Build  |   2 +-
 tools/perf/util/arm-spe-decoder/Build  |   1 +
 tools/perf/util/arm-spe-decoder/arm-spe-decoder.c  | 214 ++
 tools/perf/util/arm-spe-decoder/arm-spe-decoder.h  |  51 ++
 .../util/arm-spe-decoder/arm-spe-pkt-decoder.c | 462 +
 .../util/arm-spe-decoder/arm-spe-pkt-decoder.h |  45 ++
 tools/perf/util/arm-spe-pkt-decoder.c  | 462 -
 tools/perf/util/arm-spe-pkt-decoder.h  |  43 --
 tools/perf/util/arm-spe.c  | 717 -
 tools/perf/util/auxtrace.c |  45 ++
 tools/perf/util/auxtrace.h |  27 +
 tools/perf/util/session.h  |   2 +
 14 files changed, 1544 insertions(+), 541 deletions(-)
 create mode 100644 tools/perf/util/arm-spe-decoder/Build
 create mode 100644 tools/perf/util/arm-spe-decoder/arm-spe-decoder.c
 create mode 100644 tools/perf/util/arm-spe-decoder/arm-spe-decoder.h
 create mode 100644 tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
 create mode 100644 tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.h
 delete mode 100644 tools/perf/util/arm-spe-pkt-decoder.c
 delete mode 100644 tools/perf/util/arm-spe-pkt-decoder.h

-- 
2.7.4

[RFC PATCH 3/3] perf report: add --spe options for arm-spe

[Tan Xiaojun]

The previous patch added support in "perf report" for some arm-spe
events(llc-miss, tlb-miss, branch-miss). This patch adds their help
instructions.

Signed-off-by: Tan Xiaojun 
---
 tools/perf/Documentation/perf-report.txt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tools/perf/Documentation/perf-report.txt 
b/tools/perf/Documentation/perf-report.txt
index 987261d..d998d4b 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -445,6 +445,15 @@ include::itrace.txt[]
 
To disable decoding entirely, use --no-itrace.
 
+--spe::
+   Options for decoding arm-spe tracing data. The options are:
+
+   l   synthesize llc miss events
+   t   synthesize tlb miss events
+   b   synthesize branch miss events
+
+   The default is all events i.e. the same as --spe=ltb
+
 --full-source-path::
Show the full path for source files for srcline output.
 
-- 
2.7.4

[RFC PATCH 1/3] perf tools: Move arm-spe-pkt-decoder.h/c to the new dir

[Tan Xiaojun]

Create a new arm-spe-decoder directory for subsequent extensions and
move arm-spe-pkt-decoder.h/c to this directory. No code changes.

Signed-off-by: Tan Xiaojun 
---
 tools/perf/util/Build  |   2 +-
 tools/perf/util/arm-spe-decoder/Build  |   1 +
 .../util/arm-spe-decoder/arm-spe-pkt-decoder.c | 462 +
 .../util/arm-spe-decoder/arm-spe-pkt-decoder.h |  43 ++
 tools/perf/util/arm-spe-pkt-decoder.c  | 462 -
 tools/perf/util/arm-spe-pkt-decoder.h  |  43 --
 tools/perf/util/arm-spe.c  |   2 +-
 7 files changed, 508 insertions(+), 507 deletions(-)
 create mode 100644 tools/perf/util/arm-spe-decoder/Build
 create mode 100644 tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
 create mode 100644 tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.h
 delete mode 100644 tools/perf/util/arm-spe-pkt-decoder.c
 delete mode 100644 tools/perf/util/arm-spe-pkt-decoder.h

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 14f812b..762625c 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -96,7 +96,7 @@ perf-$(CONFIG_AUXTRACE) += intel-pt-decoder/
 perf-$(CONFIG_AUXTRACE) += intel-pt.o
 perf-$(CONFIG_AUXTRACE) += intel-bts.o
 perf-$(CONFIG_AUXTRACE) += arm-spe.o
-perf-$(CONFIG_AUXTRACE) += arm-spe-pkt-decoder.o
+perf-$(CONFIG_AUXTRACE) += arm-spe-decoder/
 perf-$(CONFIG_AUXTRACE) += s390-cpumsf.o
 
 ifdef CONFIG_LIBOPENCSD
diff --git a/tools/perf/util/arm-spe-decoder/Build 
b/tools/perf/util/arm-spe-decoder/Build
new file mode 100644
index 000..16efbc2
--- /dev/null
+++ b/tools/perf/util/arm-spe-decoder/Build
@@ -0,0 +1 @@
+perf-$(CONFIG_AUXTRACE) += arm-spe-pkt-decoder.o
diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c 
b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
new file mode 100644
index 000..b94001b
--- /dev/null
+++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
@@ -0,0 +1,462 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Arm Statistical Profiling Extensions (SPE) support
+ * Copyright (c) 2017-2018, Arm Ltd.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "arm-spe-pkt-decoder.h"
+
+#define BIT(n) (1ULL << (n))
+
+#define NS_FLAGBIT(63)
+#define EL_FLAG(BIT(62) | BIT(61))
+
+#define SPE_HEADER0_PAD0x0
+#define SPE_HEADER0_END0x1
+#define SPE_HEADER0_ADDRESS0x30 /* address packet (short) */
+#define SPE_HEADER0_ADDRESS_MASK   0x38
+#define SPE_HEADER0_COUNTER0x18 /* counter packet (short) */
+#define SPE_HEADER0_COUNTER_MASK   0x38
+#define SPE_HEADER0_TIMESTAMP  0x71
+#define SPE_HEADER0_TIMESTAMP  0x71
+#define SPE_HEADER0_EVENTS 0x2
+#define SPE_HEADER0_EVENTS_MASK0xf
+#define SPE_HEADER0_SOURCE 0x3
+#define SPE_HEADER0_SOURCE_MASK0xf
+#define SPE_HEADER0_CONTEXT0x24
+#define SPE_HEADER0_CONTEXT_MASK   0x3c
+#define SPE_HEADER0_OP_TYPE0x8
+#define SPE_HEADER0_OP_TYPE_MASK   0x3c
+#define SPE_HEADER1_ALIGNMENT  0x0
+#define SPE_HEADER1_ADDRESS0xb0 /* address packet (extended) */
+#define SPE_HEADER1_ADDRESS_MASK   0xf8
+#define SPE_HEADER1_COUNTER0x98 /* counter packet (extended) */
+#define SPE_HEADER1_COUNTER_MASK   0xf8
+
+#if __BYTE_ORDER == __BIG_ENDIAN
+#define le16_to_cpu bswap_16
+#define le32_to_cpu bswap_32
+#define le64_to_cpu bswap_64
+#define memcpy_le64(d, s, n) do { \
+   memcpy((d), (s), (n));\
+   *(d) = le64_to_cpu(*(d)); \
+} while (0)
+#else
+#define le16_to_cpu
+#define le32_to_cpu
+#define le64_to_cpu
+#define memcpy_le64 memcpy
+#endif
+
+static const char * const arm_spe_packet_name[] = {
+   [ARM_SPE_PAD]   = "PAD",
+   [ARM_SPE_END]   = "END",
+   [ARM_SPE_TIMESTAMP] = "TS",
+   [ARM_SPE_ADDRESS]   = "ADDR",
+   [ARM_SPE_COUNTER]   = "LAT",
+   [ARM_SPE_CONTEXT]   = "CONTEXT",
+   [ARM_SPE_OP_TYPE]   = "OP-TYPE",
+   [ARM_SPE_EVENTS]= "EVENTS",
+   [ARM_SPE_DATA_SOURCE]   = "DATA-SOURCE",
+};
+
+const char *arm_spe_pkt_name(enum arm_spe_pkt_type type)
+{
+   return arm_spe_packet_name[type];
+}
+
+/* return ARM SPE payload size from its encoding,
+ * which is in bits 5:4 of the byte.
+ * 00 : byte
+ * 01 : halfword (2)
+ * 10 : word (4)
+ * 11 : doubleword (8)
+ */
+static int payloadlen(unsigned char byte)
+{
+   return 1 << ((byte & 0x30) >> 4);
+}
+
+static int arm_spe_get_payload(const unsigned char *buf, size_t len,
+  struct arm_spe_pkt *packet)
+{
+   size_t payload_len = payloadlen(buf[0]);
+
+   if (len < 1 + payload_len)
+   

[PATCH] perf record: Support aarch64 random socket_id assignment

[Tan Xiaojun]

Same as the commit 01766229533f ("perf record: Support s390 random
socket_id assignment"), aarch64 also have this problem.

Without this fix:
  [root@localhost perf]# ./perf report --header -I -v
  ...
  socket_id number is too big.You may need to upgrade the perf tool.

  # 
  # captured on: Thu Aug  1 22:58:38 2019
  # header version : 1
  ...
  # Core ID and Socket ID information is not available
  ...

With this fix:
  [root@localhost perf]# ./perf report --header -I -v
  ...
  cpumask list: 0-31
  cpumask list: 32-63
  cpumask list: 64-95
  cpumask list: 96-127

  # 
  # captured on: Thu Aug  1 22:58:38 2019
  # header version : 1
  ...
  # CPU 0: Core ID 0, Socket ID 36
  # CPU 1: Core ID 1, Socket ID 36
  ...
  # CPU 126: Core ID 126, Socket ID 8442
  # CPU 127: Core ID 127, Socket ID 8442
  ...

Signed-off-by: Tan Xiaojun 
---
 tools/perf/util/header.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 20111f8..d57fb74 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2251,8 +2251,10 @@ static int process_cpu_topology(struct feat_fd *ff, void 
*data __maybe_unused)
/* On s390 the socket_id number is not related to the numbers of cpus.
 * The socket_id number might be higher than the numbers of cpus.
 * This depends on the configuration.
+* AArch64 is the same.
 */
-   if (ph->env.arch && !strncmp(ph->env.arch, "s390", 4))
+   if (ph->env.arch && (!strncmp(ph->env.arch, "s390", 4)
+ || !strncmp(ph->env.arch, "aarch64", 7)))
do_core_id_test = false;
 
for (i = 0; i < (u32)cpu_nr; i++) {
-- 
2.7.4

Re: [PATCH v2] aio: add check for timeout to aviod invalid value

[Tan Xiaojun]

Any comments?
Anything is fine to me.

Thanks.
Xiaojun.

On 2019/2/25 18:09, Tan Xiaojun wrote:
> Syzkaller reported a UBSAN bug below, which was mainly caused by a large
> negative number passed to the timeout of the io_getevents system call.
>
> ==
> UBSAN: Undefined behaviour in ./include/linux/ktime.h:42:14
> signed integer overflow:
> -8427032702788048137 * 10 cannot be represented in type 'long long 
> int'
> CPU: 3 PID: 11668 Comm: syz-executor0 Not tainted 4.19.18-514.55.6.9.x86_64+ 
> #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 
> 04/01/2014
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0xca/0x13e lib/dump_stack.c:113
>  ubsan_epilogue+0xe/0x81 lib/ubsan.c:159
>  handle_overflow+0x193/0x1e2 lib/ubsan.c:190
>  ktime_set include/linux/ktime.h:42 [inline]
>  timespec64_to_ktime include/linux/ktime.h:78 [inline]
>  do_io_getevents+0x307/0x390 fs/aio.c:2043
>  __do_sys_io_getevents fs/aio.c:2080 [inline]
>  __se_sys_io_getevents fs/aio.c:2068 [inline]
>  __x64_sys_io_getevents+0x163/0x250 fs/aio.c:2068
>  do_syscall_64+0xc8/0x580 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x462589
> Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48 89 f7 48 
> 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 
> 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
> RSP: 002b:7fde9b04ec58 EFLAGS: 0246 ORIG_RAX: 00d0
> RAX: ffda RBX: 0072bf00 RCX: 00462589
> RDX: 0006 RSI:  RDI: 
> RBP: 0005 R08: 2100 R09: 
> R10: 2040 R11: 0246 R12: 7fde9b04f6bc
> R13: 004bd1f0 R14: 006f6b60 R15: 
> ==
> bond0 (unregistering): Released all slaves
>
> The timeout described in "man io_getevents" does not say whether it
> can be negative or not, but as a waiting time, a negative number has
> no meaning. So I add check to avoid this case.
>
> This issue was previously discussed at:
> https://lore.kernel.org/lkml/cact4y+bbxvylq6ltokrktnlthqlhcw-bmp3aqp3mjdavr9f...@mail.gmail.com/
>
> Signed-off-by: Tan Xiaojun 
> Reviewed-by: Jeff Moyer 
> ---
>  fs/aio.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index f4d..3a39673 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -2078,10 +2078,16 @@ static long do_io_getevents(aio_context_t ctx_id,
>   struct io_event __user *events,
>   struct timespec64 *ts)
>  {
> - ktime_t until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
> - struct kioctx *ioctx = lookup_ioctx(ctx_id);
> + ktime_t until;
> + struct kioctx *ioctx;
>   long ret = -EINVAL;
>  
> + if (ts && !timespec64_valid(ts))
> + return -EINVAL;
> +
> + until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
> + ioctx = lookup_ioctx(ctx_id);
> +
>   if (likely(ioctx)) {
>   if (likely(min_nr <= nr && min_nr >= 0))
>   ret = read_events(ioctx, min_nr, nr, events, until);

[PATCH v2] aio: add check for timeout to aviod invalid value

[Tan Xiaojun]

Syzkaller reported a UBSAN bug below, which was mainly caused by a large
negative number passed to the timeout of the io_getevents system call.

==
UBSAN: Undefined behaviour in ./include/linux/ktime.h:42:14
signed integer overflow:
-8427032702788048137 * 10 cannot be represented in type 'long long int'
CPU: 3 PID: 11668 Comm: syz-executor0 Not tainted 4.19.18-514.55.6.9.x86_64+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 
04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xca/0x13e lib/dump_stack.c:113
 ubsan_epilogue+0xe/0x81 lib/ubsan.c:159
 handle_overflow+0x193/0x1e2 lib/ubsan.c:190
 ktime_set include/linux/ktime.h:42 [inline]
 timespec64_to_ktime include/linux/ktime.h:78 [inline]
 do_io_getevents+0x307/0x390 fs/aio.c:2043
 __do_sys_io_getevents fs/aio.c:2080 [inline]
 __se_sys_io_getevents fs/aio.c:2068 [inline]
 __x64_sys_io_getevents+0x163/0x250 fs/aio.c:2068
 do_syscall_64+0xc8/0x580 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x462589
Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48 89 f7 48 
89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 
c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:7fde9b04ec58 EFLAGS: 0246 ORIG_RAX: 00d0
RAX: ffda RBX: 0072bf00 RCX: 00462589
RDX: 0006 RSI:  RDI: 
RBP: 0005 R08: 2100 R09: 
R10: 2040 R11: 0246 R12: 7fde9b04f6bc
R13: 004bd1f0 R14: 006f6b60 R15: 
==
bond0 (unregistering): Released all slaves

The timeout described in "man io_getevents" does not say whether it
can be negative or not, but as a waiting time, a negative number has
no meaning. So I add check to avoid this case.

This issue was previously discussed at:
https://lore.kernel.org/lkml/cact4y+bbxvylq6ltokrktnlthqlhcw-bmp3aqp3mjdavr9f...@mail.gmail.com/

Signed-off-by: Tan Xiaojun 
Reviewed-by: Jeff Moyer 
---
 fs/aio.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index f4d..3a39673 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -2078,10 +2078,16 @@ static long do_io_getevents(aio_context_t ctx_id,
struct io_event __user *events,
struct timespec64 *ts)
 {
-   ktime_t until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
-   struct kioctx *ioctx = lookup_ioctx(ctx_id);
+   ktime_t until;
+   struct kioctx *ioctx;
long ret = -EINVAL;
 
+   if (ts && !timespec64_valid(ts))
+   return -EINVAL;
+
+   until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
+   ioctx = lookup_ioctx(ctx_id);
+
if (likely(ioctx)) {
if (likely(min_nr <= nr && min_nr >= 0))
ret = read_events(ioctx, min_nr, nr, events, until);
-- 
2.7.4

Re: [PATCH] aio: add check for timeout to aviod invalid value

[Tan Xiaojun]

I made a mistake. I will fix it and send v2.

Thanks.
Xiaojun.

On 2019/2/25 17:42, Tan Xiaojun wrote:
> Syzkaller reported a UBSAN bug below, which was mainly caused by a large
> negative number passed to the timeout of the io_getevents system call.
> 
> 
> UBSAN: Undefined behaviour in ./include/linux/ktime.h:42:14
> signed integer overflow:
> -8427032702788048137 * 10 cannot be represented in type 'long long 
> int'
> CPU: 3 PID: 11668 Comm: syz-executor0 Not tainted 4.19.18-514.55.6.9.x86_64+ 
> #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 
> 04/01/2014
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0xca/0x13e lib/dump_stack.c:113
>  ubsan_epilogue+0xe/0x81 lib/ubsan.c:159
>  handle_overflow+0x193/0x1e2 lib/ubsan.c:190
>  ktime_set include/linux/ktime.h:42 [inline]
>  timespec64_to_ktime include/linux/ktime.h:78 [inline]
>  do_io_getevents+0x307/0x390 fs/aio.c:2043
>  __do_sys_io_getevents fs/aio.c:2080 [inline]
>  __se_sys_io_getevents fs/aio.c:2068 [inline]
>  __x64_sys_io_getevents+0x163/0x250 fs/aio.c:2068
>  do_syscall_64+0xc8/0x580 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x462589
> Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48 89 f7 48 
> 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 
> 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
> RSP: 002b:7fde9b04ec58 EFLAGS: 0246 ORIG_RAX: 00d0
> RAX: ffda RBX: 0072bf00 RCX: 00462589
> RDX: 0006 RSI:  RDI: 
> RBP: 0005 R08: 2100 R09: 
> R10: 2040 R11: 0246 R12: 7fde9b04f6bc
> R13: 004bd1f0 R14: 006f6b60 R15: 
> 
> bond0 (unregistering): Released all slaves
> 
> The timeout described in "man io_getevents" does not say whether it
> can be negative or not, but as a waiting time, a negative number has
> no meaning. So I add check to avoid this case.
> 
> This issue was previously discussed at:
> https://lore.kernel.org/lkml/cact4y+bbxvylq6ltokrktnlthqlhcw-bmp3aqp3mjdavr9f...@mail.gmail.com/
> 
> Signed-off-by: Tan Xiaojun 
> Reviewed-by: Jeff Moyer 
> ---
>  fs/aio.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/aio.c b/fs/aio.c
> index f4d..28e0fa6 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -2078,10 +2078,15 @@ static long do_io_getevents(aio_context_t ctx_id,
>   struct io_event __user *events,
>   struct timespec64 *ts)
>  {
> - ktime_t until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
> + ktime_t until;
>   struct kioctx *ioctx = lookup_ioctx(ctx_id);
>   long ret = -EINVAL;
>  
> + if (ts && !timespec64_valid(ts))
> + return -EINVAL;
> +
> + until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
> +
>   if (likely(ioctx)) {
>   if (likely(min_nr <= nr && min_nr >= 0))
>   ret = read_events(ioctx, min_nr, nr, events, until);
> 

[PATCH] aio: add check for timeout to aviod invalid value

[Tan Xiaojun]

Syzkaller reported a UBSAN bug below, which was mainly caused by a large
negative number passed to the timeout of the io_getevents system call.


UBSAN: Undefined behaviour in ./include/linux/ktime.h:42:14
signed integer overflow:
-8427032702788048137 * 10 cannot be represented in type 'long long int'
CPU: 3 PID: 11668 Comm: syz-executor0 Not tainted 4.19.18-514.55.6.9.x86_64+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 
04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xca/0x13e lib/dump_stack.c:113
 ubsan_epilogue+0xe/0x81 lib/ubsan.c:159
 handle_overflow+0x193/0x1e2 lib/ubsan.c:190
 ktime_set include/linux/ktime.h:42 [inline]
 timespec64_to_ktime include/linux/ktime.h:78 [inline]
 do_io_getevents+0x307/0x390 fs/aio.c:2043
 __do_sys_io_getevents fs/aio.c:2080 [inline]
 __se_sys_io_getevents fs/aio.c:2068 [inline]
 __x64_sys_io_getevents+0x163/0x250 fs/aio.c:2068
 do_syscall_64+0xc8/0x580 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x462589
Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48 89 f7 48 
89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 
c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:7fde9b04ec58 EFLAGS: 0246 ORIG_RAX: 00d0
RAX: ffda RBX: 0072bf00 RCX: 00462589
RDX: 0006 RSI:  RDI: 
RBP: 0005 R08: 2100 R09: 
R10: 2040 R11: 0246 R12: 7fde9b04f6bc
R13: 004bd1f0 R14: 006f6b60 R15: 

bond0 (unregistering): Released all slaves

The timeout described in "man io_getevents" does not say whether it
can be negative or not, but as a waiting time, a negative number has
no meaning. So I add check to avoid this case.

This issue was previously discussed at:
https://lore.kernel.org/lkml/cact4y+bbxvylq6ltokrktnlthqlhcw-bmp3aqp3mjdavr9f...@mail.gmail.com/

Signed-off-by: Tan Xiaojun 
Reviewed-by: Jeff Moyer 
---
 fs/aio.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/aio.c b/fs/aio.c
index f4d..28e0fa6 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -2078,10 +2078,15 @@ static long do_io_getevents(aio_context_t ctx_id,
struct io_event __user *events,
struct timespec64 *ts)
 {
-   ktime_t until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
+   ktime_t until;
struct kioctx *ioctx = lookup_ioctx(ctx_id);
long ret = -EINVAL;
 
+   if (ts && !timespec64_valid(ts))
+   return -EINVAL;
+
+   until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
+
if (likely(ioctx)) {
if (likely(min_nr <= nr && min_nr >= 0))
ret = read_events(ioctx, min_nr, nr, events, until);
-- 
2.7.4

Re: [RFC PATCH] aio: add check for timeout to aviod invalid value

[Tan Xiaojun]

On 2019/2/19 4:33, Jeff Moyer wrote:
> Tan Xiaojun  writes:
> 
>> (When I was testing with syzkaller, I found a lot of ubsan problems. Here 
>> is one of them. I am not sure if it needs to be fixed and how it will be 
>> fixed. So I sent this patch to ask your opinion.)
>>
>> Syzkaller reported a UBSAN bug below, which was mainly caused by a large
>> negative number passed to the timeout of the io_getevents system call.
>>
>> 
>> UBSAN: Undefined behaviour in ./include/linux/ktime.h:42:14
>> signed integer overflow:
>> -8427032702788048137 * 10 cannot be represented in type 'long long 
>> int'
>> CPU: 3 PID: 11668 Comm: syz-executor0 Not tainted 4.19.18-514.55.6.9.x86_64+ 
>> #1
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 
>> 04/01/2014
>> Call Trace:
>>  __dump_stack lib/dump_stack.c:77 [inline]
>>  dump_stack+0xca/0x13e lib/dump_stack.c:113
>>  ubsan_epilogue+0xe/0x81 lib/ubsan.c:159
>>  handle_overflow+0x193/0x1e2 lib/ubsan.c:190
>>  ktime_set include/linux/ktime.h:42 [inline]
>>  timespec64_to_ktime include/linux/ktime.h:78 [inline]
>>  do_io_getevents+0x307/0x390 fs/aio.c:2043
>>  __do_sys_io_getevents fs/aio.c:2080 [inline]
>>  __se_sys_io_getevents fs/aio.c:2068 [inline]
>>  __x64_sys_io_getevents+0x163/0x250 fs/aio.c:2068
>>  do_syscall_64+0xc8/0x580 arch/x86/entry/common.c:290
>>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
>> RIP: 0033:0x462589
>> Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48 89 f7 
>> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 
>> 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
>> RSP: 002b:7fde9b04ec58 EFLAGS: 0246 ORIG_RAX: 00d0
>> RAX: ffda RBX: 0072bf00 RCX: 00462589
>> RDX: 0006 RSI:  RDI: 
>> RBP: 0005 R08: 2100 R09: 
>> R10: 2040 R11: 0246 R12: 7fde9b04f6bc
>> R13: 004bd1f0 R14: 006f6b60 R15: 
>> 
>> bond0 (unregistering): Released all slaves
>>
>> The timeout described in "man io_getevents" does not say whether it
>> can be negative or not, but as a waiting time, a negative number has
>> no meaning. So I add check to avoid this case.
> 
> It's embarrassing that this bug is still present.  See, for example,
> this discussion, started in 2015:
>   
> https://lore.kernel.org/lkml/cact4y+bbxvylq6ltokrktnlthqlhcw-bmp3aqp3mjdavr9f...@mail.gmail.com/
> 
> I could swear it was brought up again since then, but I can't find
> records of that.
> 

Yes. I will add this, thank you.

>> Signed-off-by: Tan Xiaojun 
>> ---
>>  fs/aio.c | 7 ++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/aio.c b/fs/aio.c
>> index f4d..28e0fa6 100644
>> --- a/fs/aio.c
>> +++ b/fs/aio.c
>> @@ -2078,10 +2078,15 @@ static long do_io_getevents(aio_context_t ctx_id,
>>  struct io_event __user *events,
>>  struct timespec64 *ts)
>>  {
>> -ktime_t until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
>> +ktime_t until;
>>  struct kioctx *ioctx = lookup_ioctx(ctx_id);
>>  long ret = -EINVAL;
>>  
>> +if (ts && !timespec64_valid(ts))
>> +return -EINVAL;
>> +
>> +until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
>> +
>>  if (likely(ioctx)) {
>>  if (likely(min_nr <= nr && min_nr >= 0))
>>  ret = read_events(ioctx, min_nr, nr, events, until);
> 
> Looks good to me.  Thanks for fixing this.
> 
> Reviewed-by: Jeff Moyer 

Thank you for your reply, I went out for a trip last week. I will send it 
officially immediately.

Xiaojun. 

> 
> .
> 

[RFC PATCH] aio: add check for timeout to aviod invalid value

[Tan Xiaojun]

(When I was testing with syzkaller, I found a lot of ubsan problems. Here 
is one of them. I am not sure if it needs to be fixed and how it will be 
fixed. So I sent this patch to ask your opinion.)

Syzkaller reported a UBSAN bug below, which was mainly caused by a large
negative number passed to the timeout of the io_getevents system call.


UBSAN: Undefined behaviour in ./include/linux/ktime.h:42:14
signed integer overflow:
-8427032702788048137 * 10 cannot be represented in type 'long long int'
CPU: 3 PID: 11668 Comm: syz-executor0 Not tainted 4.19.18-514.55.6.9.x86_64+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 
04/01/2014
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xca/0x13e lib/dump_stack.c:113
 ubsan_epilogue+0xe/0x81 lib/ubsan.c:159
 handle_overflow+0x193/0x1e2 lib/ubsan.c:190
 ktime_set include/linux/ktime.h:42 [inline]
 timespec64_to_ktime include/linux/ktime.h:78 [inline]
 do_io_getevents+0x307/0x390 fs/aio.c:2043
 __do_sys_io_getevents fs/aio.c:2080 [inline]
 __se_sys_io_getevents fs/aio.c:2068 [inline]
 __x64_sys_io_getevents+0x163/0x250 fs/aio.c:2068
 do_syscall_64+0xc8/0x580 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x462589
Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48 89 f7 48 
89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 
c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:7fde9b04ec58 EFLAGS: 0246 ORIG_RAX: 00d0
RAX: ffda RBX: 0072bf00 RCX: 00462589
RDX: 0006 RSI:  RDI: 
RBP: 0005 R08: 2100 R09: 
R10: 2040 R11: 0246 R12: 7fde9b04f6bc
R13: 004bd1f0 R14: 006f6b60 R15: 

bond0 (unregistering): Released all slaves

The timeout described in "man io_getevents" does not say whether it
can be negative or not, but as a waiting time, a negative number has
no meaning. So I add check to avoid this case.

Signed-off-by: Tan Xiaojun 
---
 fs/aio.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/aio.c b/fs/aio.c
index f4d..28e0fa6 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -2078,10 +2078,15 @@ static long do_io_getevents(aio_context_t ctx_id,
struct io_event __user *events,
struct timespec64 *ts)
 {
-   ktime_t until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
+   ktime_t until;
struct kioctx *ioctx = lookup_ioctx(ctx_id);
long ret = -EINVAL;
 
+   if (ts && !timespec64_valid(ts))
+   return -EINVAL;
+
+   until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
+
if (likely(ioctx)) {
if (likely(min_nr <= nr && min_nr >= 0))
ret = read_events(ioctx, min_nr, nr, events, until);
-- 
2.7.4

[PATCH net-next] net: hns3: add common validation in hclge_dcb

[Tan Xiaojun]

From: Yunsheng Lin 

Before setting tm related configuration to hardware, driver
needs to check the configuration provided by user is valid.
Currently hclge_ieee_setets and hclge_setup_tc both implement
their own checking, which has a lot in common.

This patch addes hclge_dcb_common_validate to do the common
checking. The checking in hclge_tm_prio_tc_info_update
and hclge_tm_schd_info_update is unnecessary now, so change
the return type to void, which removes the need to do error
handling when one of the checking fails.

Also, ets->prio_tc is indexed by user prio and ets->tc_tsa is
indexed by tc num, so this patch changes them to use different
index.

Signed-off-by: Yunsheng Lin 
Signed-off-by: Tan Xiaojun 
---
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 70 +++---
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c  | 14 +
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h  |  4 +-
 3 files changed, 53 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
index e72f724..f6323b2 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
@@ -35,7 +35,9 @@ static int hclge_ieee_ets_to_tm_info(struct hclge_dev *hdev,
}
}
 
-   return hclge_tm_prio_tc_info_update(hdev, ets->prio_tc);
+   hclge_tm_prio_tc_info_update(hdev, ets->prio_tc);
+
+   return 0;
 }
 
 static void hclge_tm_info_to_ieee_ets(struct hclge_dev *hdev,
@@ -70,25 +72,61 @@ static int hclge_ieee_getets(struct hnae3_handle *h, struct 
ieee_ets *ets)
return 0;
 }
 
+static int hclge_dcb_common_validate(struct hclge_dev *hdev, u8 num_tc,
+u8 *prio_tc)
+{
+   int i;
+
+   if (num_tc > hdev->tc_max) {
+   dev_err(>pdev->dev,
+   "tc num checking failed, %u > tc_max(%u)\n",
+   num_tc, hdev->tc_max);
+   return -EINVAL;
+   }
+
+   for (i = 0; i < HNAE3_MAX_USER_PRIO; i++) {
+   if (prio_tc[i] >= num_tc) {
+   dev_err(>pdev->dev,
+   "prio_tc[%u] checking failed, %u >= 
num_tc(%u)\n",
+   i, prio_tc[i], num_tc);
+   return -EINVAL;
+   }
+   }
+
+   for (i = 0; i < hdev->num_alloc_vport; i++) {
+   if (num_tc > hdev->vport[i].alloc_tqps) {
+   dev_err(>pdev->dev,
+   "allocated tqp(%u) checking failed, %u > 
tqp(%u)\n",
+   i, num_tc, hdev->vport[i].alloc_tqps);
+   return -EINVAL;
+   }
+   }
+
+   return 0;
+}
+
 static int hclge_ets_validate(struct hclge_dev *hdev, struct ieee_ets *ets,
  u8 *tc, bool *changed)
 {
bool has_ets_tc = false;
u32 total_ets_bw = 0;
u8 max_tc = 0;
+   int ret;
u8 i;
 
-   for (i = 0; i < HNAE3_MAX_TC; i++) {
-   if (ets->prio_tc[i] >= hdev->tc_max ||
-   i >= hdev->tc_max)
-   return -EINVAL;
-
+   for (i = 0; i < HNAE3_MAX_USER_PRIO; i++) {
if (ets->prio_tc[i] != hdev->tm_info.prio_tc[i])
*changed = true;
 
if (ets->prio_tc[i] > max_tc)
max_tc = ets->prio_tc[i];
+   }
 
+   ret = hclge_dcb_common_validate(hdev, max_tc + 1, ets->prio_tc);
+   if (ret)
+   return ret;
+
+   for (i = 0; i < HNAE3_MAX_TC; i++) {
switch (ets->tc_tsa[i]) {
case IEEE_8021QAZ_TSA_STRICT:
if (hdev->tm_info.tc_info[i].tc_sch_mode !=
@@ -184,9 +222,7 @@ static int hclge_ieee_setets(struct hnae3_handle *h, struct 
ieee_ets *ets)
if (ret)
return ret;
 
-   ret = hclge_tm_schd_info_update(hdev, num_tc);
-   if (ret)
-   return ret;
+   hclge_tm_schd_info_update(hdev, num_tc);
 
ret = hclge_ieee_ets_to_tm_info(hdev, ets);
if (ret)
@@ -305,20 +341,12 @@ static int hclge_setup_tc(struct hnae3_handle *h, u8 tc, 
u8 *prio_tc)
if (hdev->flag & HCLGE_FLAG_DCB_ENABLE)
return -EINVAL;
 
-   if (tc > hdev->tc_max) {
-   dev_err(>pdev->dev,
-   "setup tc failed, tc(%u) > tc_max(%u)\n",
-   tc, hdev->tc_max);
-   return -EINVAL;
-   }
-
-   ret = hclge_tm_schd_info_update(hdev, tc);
+   ret = hclge_dcb_common_validate(hdev, tc, prio_tc);
if (ret)
-   return ret;
+   return -EINVAL;
 
-   ret = hcl

[PATCH net-next] net: hns3: add common validation in hclge_dcb

[Tan Xiaojun]

From: Yunsheng Lin 

Before setting tm related configuration to hardware, driver
needs to check the configuration provided by user is valid.
Currently hclge_ieee_setets and hclge_setup_tc both implement
their own checking, which has a lot in common.

This patch addes hclge_dcb_common_validate to do the common
checking. The checking in hclge_tm_prio_tc_info_update
and hclge_tm_schd_info_update is unnecessary now, so change
the return type to void, which removes the need to do error
handling when one of the checking fails.

Also, ets->prio_tc is indexed by user prio and ets->tc_tsa is
indexed by tc num, so this patch changes them to use different
index.

Signed-off-by: Yunsheng Lin 
Signed-off-by: Tan Xiaojun 
---
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 70 +++---
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c  | 14 +
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h  |  4 +-
 3 files changed, 53 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
index e72f724..f6323b2 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
@@ -35,7 +35,9 @@ static int hclge_ieee_ets_to_tm_info(struct hclge_dev *hdev,
}
}
 
-   return hclge_tm_prio_tc_info_update(hdev, ets->prio_tc);
+   hclge_tm_prio_tc_info_update(hdev, ets->prio_tc);
+
+   return 0;
 }
 
 static void hclge_tm_info_to_ieee_ets(struct hclge_dev *hdev,
@@ -70,25 +72,61 @@ static int hclge_ieee_getets(struct hnae3_handle *h, struct 
ieee_ets *ets)
return 0;
 }
 
+static int hclge_dcb_common_validate(struct hclge_dev *hdev, u8 num_tc,
+u8 *prio_tc)
+{
+   int i;
+
+   if (num_tc > hdev->tc_max) {
+   dev_err(>pdev->dev,
+   "tc num checking failed, %u > tc_max(%u)\n",
+   num_tc, hdev->tc_max);
+   return -EINVAL;
+   }
+
+   for (i = 0; i < HNAE3_MAX_USER_PRIO; i++) {
+   if (prio_tc[i] >= num_tc) {
+   dev_err(>pdev->dev,
+   "prio_tc[%u] checking failed, %u >= 
num_tc(%u)\n",
+   i, prio_tc[i], num_tc);
+   return -EINVAL;
+   }
+   }
+
+   for (i = 0; i < hdev->num_alloc_vport; i++) {
+   if (num_tc > hdev->vport[i].alloc_tqps) {
+   dev_err(>pdev->dev,
+   "allocated tqp(%u) checking failed, %u > 
tqp(%u)\n",
+   i, num_tc, hdev->vport[i].alloc_tqps);
+   return -EINVAL;
+   }
+   }
+
+   return 0;
+}
+
 static int hclge_ets_validate(struct hclge_dev *hdev, struct ieee_ets *ets,
  u8 *tc, bool *changed)
 {
bool has_ets_tc = false;
u32 total_ets_bw = 0;
u8 max_tc = 0;
+   int ret;
u8 i;
 
-   for (i = 0; i < HNAE3_MAX_TC; i++) {
-   if (ets->prio_tc[i] >= hdev->tc_max ||
-   i >= hdev->tc_max)
-   return -EINVAL;
-
+   for (i = 0; i < HNAE3_MAX_USER_PRIO; i++) {
if (ets->prio_tc[i] != hdev->tm_info.prio_tc[i])
*changed = true;
 
if (ets->prio_tc[i] > max_tc)
max_tc = ets->prio_tc[i];
+   }
 
+   ret = hclge_dcb_common_validate(hdev, max_tc + 1, ets->prio_tc);
+   if (ret)
+   return ret;
+
+   for (i = 0; i < HNAE3_MAX_TC; i++) {
switch (ets->tc_tsa[i]) {
case IEEE_8021QAZ_TSA_STRICT:
if (hdev->tm_info.tc_info[i].tc_sch_mode !=
@@ -184,9 +222,7 @@ static int hclge_ieee_setets(struct hnae3_handle *h, struct 
ieee_ets *ets)
if (ret)
return ret;
 
-   ret = hclge_tm_schd_info_update(hdev, num_tc);
-   if (ret)
-   return ret;
+   hclge_tm_schd_info_update(hdev, num_tc);
 
ret = hclge_ieee_ets_to_tm_info(hdev, ets);
if (ret)
@@ -305,20 +341,12 @@ static int hclge_setup_tc(struct hnae3_handle *h, u8 tc, 
u8 *prio_tc)
if (hdev->flag & HCLGE_FLAG_DCB_ENABLE)
return -EINVAL;
 
-   if (tc > hdev->tc_max) {
-   dev_err(>pdev->dev,
-   "setup tc failed, tc(%u) > tc_max(%u)\n",
-   tc, hdev->tc_max);
-   return -EINVAL;
-   }
-
-   ret = hclge_tm_schd_info_update(hdev, tc);
+   ret = hclge_dcb_common_validate(hdev, tc, prio_tc);
if (ret)
-   return ret;
+   return -EINVAL;
 
-   ret = hcl

[Question] A UBSAN problem in stable-4.4

[Tan Xiaojun]

Hi, all,

I found the following problem (attached to the end) when testing stable-4.4 with
Syzkaller. This is not an easy-to-trigger problem, so the tool does not generate
code for recurring problems.

>From the call stack, it is because the first parameter in ktime_sub is large, 
>and
the second parameter offset is a negative number, causing the final result to
overflow into the sign bit and become a large negative number.

--
...
ktime_t expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
...
--

But I don't know how to fix this problem. The mainline code is also different 
from
stable-4.4, and I have not found a patch to fix this problem in the mainline
repository.

So I am a bit confused about how to fix it. Can anyone give me some advice?

Thanks.
Xiaojun.


UBSAN: Undefined behaviour in kernel/time/hrtimer.c:615:20
signed integer overflow:
9223372036854775807 - -495588161 cannot be represented in type 'long long int'
CPU: 0 PID: 4542 Comm: syz-executor0 Not tainted 4.4.156-514.55.6.9.x86_64+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
 1100391dbf45 ad071d3307b76e03 8801c8edfab0 81c9f586
 41b58ab3 831fd4e6 81c9f478 8801c8edfad8
 8801c8edfa78 14a9 ad071d3307b76e03 837fd660
Call Trace:
 [] __dump_stack lib/dump_stack.c:15 [inline]
 [] dump_stack+0x10e/0x1a8 lib/dump_stack.c:51
 [] ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
 [] handle_overflow+0x23e/0x299 lib/ubsan.c:195
 [] __ubsan_handle_sub_overflow+0x2a/0x31 lib/ubsan.c:211
 [] hrtimer_reprogram kernel/time/hrtimer.c:615 [inline]
 [] hrtimer_start_range_ns+0x1083/0x1580 
kernel/time/hrtimer.c:1024
 [] hrtimer_start include/linux/hrtimer.h:393 [inline]
 [] alarm_start+0xcf/0x130 kernel/time/alarmtimer.c:328
 [] alarm_timer_set+0x296/0x4a0 kernel/time/alarmtimer.c:632
 [] SYSC_timer_settime kernel/time/posix-timers.c:914 [inline]
 [] SyS_timer_settime+0x2be/0x3d0 
kernel/time/posix-timers.c:885
 [] entry_SYSCALL_64_fastpath+0x1e/0x9e


UBSAN: Undefined behaviour in kernel/time/hrtimer.c:490:13
signed integer overflow:
9223372036854775807 - -495588161 cannot be represented in type 'long long int'
CPU: 0 PID: 4542 Comm: syz-executor0 Not tainted 4.4.156-514.55.6.9.x86_64+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
 11003ed40f8b ad071d3307b76e03 8801f6a07ce0 81c9f586
 41b58ab3 831fd4e6 81c9f478 8801f6a07d08
 8801f6a07ca8 000a ad071d3307b76e03 837fd660
Call Trace:
   [] __dump_stack lib/dump_stack.c:15 [inline]
   [] dump_stack+0x10e/0x1a8 lib/dump_stack.c:51
 [] ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
 [] handle_overflow+0x23e/0x299 lib/ubsan.c:195
 [] __ubsan_handle_sub_overflow+0x2a/0x31 lib/ubsan.c:211
 [] __hrtimer_get_next_event+0x1da/0x2b0 
kernel/time/hrtimer.c:490
 [] hrtimer_interrupt+0x202/0x580 kernel/time/hrtimer.c:1361
 [] local_apic_timer_interrupt+0x9d/0x150 
arch/x86/kernel/apic/apic.c:901
 [] smp_apic_timer_interrupt+0x80/0xb0 
arch/x86/kernel/apic/apic.c:925
 [] apic_timer_interrupt+0xa5/0xb0 
arch/x86/entry/entry_64.S:563
   [] ? arch_local_irq_restore 
arch/x86/include/asm/paravirt.h:812 [inline]
   [] ? __raw_spin_unlock_irqrestore 
include/linux/spinlock_api_smp.h:162 [inline]
   [] ? _raw_spin_unlock_irqrestore+0x3b/0x60 
kernel/locking/spinlock.c:191
 [] unlock_timer include/linux/spinlock.h:362 [inline]
 [] SYSC_timer_settime kernel/time/posix-timers.c:916 [inline]
 [] SyS_timer_settime+0x2cf/0x3d0 
kernel/time/posix-timers.c:885
 [] entry_SYSCALL_64_fastpath+0x1e/0x9e

[Question] A UBSAN problem in stable-4.4

[Tan Xiaojun]

Hi, all,

I found the following problem (attached to the end) when testing stable-4.4 with
Syzkaller. This is not an easy-to-trigger problem, so the tool does not generate
code for recurring problems.

>From the call stack, it is because the first parameter in ktime_sub is large, 
>and
the second parameter offset is a negative number, causing the final result to
overflow into the sign bit and become a large negative number.

--
...
ktime_t expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
...
--

But I don't know how to fix this problem. The mainline code is also different 
from
stable-4.4, and I have not found a patch to fix this problem in the mainline
repository.

So I am a bit confused about how to fix it. Can anyone give me some advice?

Thanks.
Xiaojun.


UBSAN: Undefined behaviour in kernel/time/hrtimer.c:615:20
signed integer overflow:
9223372036854775807 - -495588161 cannot be represented in type 'long long int'
CPU: 0 PID: 4542 Comm: syz-executor0 Not tainted 4.4.156-514.55.6.9.x86_64+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
 1100391dbf45 ad071d3307b76e03 8801c8edfab0 81c9f586
 41b58ab3 831fd4e6 81c9f478 8801c8edfad8
 8801c8edfa78 14a9 ad071d3307b76e03 837fd660
Call Trace:
 [] __dump_stack lib/dump_stack.c:15 [inline]
 [] dump_stack+0x10e/0x1a8 lib/dump_stack.c:51
 [] ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
 [] handle_overflow+0x23e/0x299 lib/ubsan.c:195
 [] __ubsan_handle_sub_overflow+0x2a/0x31 lib/ubsan.c:211
 [] hrtimer_reprogram kernel/time/hrtimer.c:615 [inline]
 [] hrtimer_start_range_ns+0x1083/0x1580 
kernel/time/hrtimer.c:1024
 [] hrtimer_start include/linux/hrtimer.h:393 [inline]
 [] alarm_start+0xcf/0x130 kernel/time/alarmtimer.c:328
 [] alarm_timer_set+0x296/0x4a0 kernel/time/alarmtimer.c:632
 [] SYSC_timer_settime kernel/time/posix-timers.c:914 [inline]
 [] SyS_timer_settime+0x2be/0x3d0 
kernel/time/posix-timers.c:885
 [] entry_SYSCALL_64_fastpath+0x1e/0x9e


UBSAN: Undefined behaviour in kernel/time/hrtimer.c:490:13
signed integer overflow:
9223372036854775807 - -495588161 cannot be represented in type 'long long int'
CPU: 0 PID: 4542 Comm: syz-executor0 Not tainted 4.4.156-514.55.6.9.x86_64+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
 11003ed40f8b ad071d3307b76e03 8801f6a07ce0 81c9f586
 41b58ab3 831fd4e6 81c9f478 8801f6a07d08
 8801f6a07ca8 000a ad071d3307b76e03 837fd660
Call Trace:
   [] __dump_stack lib/dump_stack.c:15 [inline]
   [] dump_stack+0x10e/0x1a8 lib/dump_stack.c:51
 [] ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
 [] handle_overflow+0x23e/0x299 lib/ubsan.c:195
 [] __ubsan_handle_sub_overflow+0x2a/0x31 lib/ubsan.c:211
 [] __hrtimer_get_next_event+0x1da/0x2b0 
kernel/time/hrtimer.c:490
 [] hrtimer_interrupt+0x202/0x580 kernel/time/hrtimer.c:1361
 [] local_apic_timer_interrupt+0x9d/0x150 
arch/x86/kernel/apic/apic.c:901
 [] smp_apic_timer_interrupt+0x80/0xb0 
arch/x86/kernel/apic/apic.c:925
 [] apic_timer_interrupt+0xa5/0xb0 
arch/x86/entry/entry_64.S:563
   [] ? arch_local_irq_restore 
arch/x86/include/asm/paravirt.h:812 [inline]
   [] ? __raw_spin_unlock_irqrestore 
include/linux/spinlock_api_smp.h:162 [inline]
   [] ? _raw_spin_unlock_irqrestore+0x3b/0x60 
kernel/locking/spinlock.c:191
 [] unlock_timer include/linux/spinlock.h:362 [inline]
 [] SYSC_timer_settime kernel/time/posix-timers.c:916 [inline]
 [] SyS_timer_settime+0x2cf/0x3d0 
kernel/time/posix-timers.c:885
 [] entry_SYSCALL_64_fastpath+0x1e/0x9e

[Question] A UBSAN problem in stable-4.4

[Tan Xiaojun]

Hi, all,

I found the following problem (attached to the end) when testing stable-4.4 with
Syzkaller. This is not an easy-to-trigger problem, so the tool does not generate
code for recurring problems.

>From the call stack, it is because the first parameter in ktime_sub is large, 
>and
the second parameter offset is a negative number, causing the final result to
overflow into the sign bit and become a large negative number.

--
...
ktime_t expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
...
--

But I don't know how to fix this problem. The mainline code is also different 
from
stable-4.4, and I have not found a patch to fix this problem in the mainline
repository.

So I am a bit confused about how to fix it. Can anyone give me some advice?

Thanks.
Xiaojun.


UBSAN: Undefined behaviour in kernel/time/hrtimer.c:615:20
signed integer overflow:
9223372036854775807 - -495588161 cannot be represented in type 'long long int'
CPU: 0 PID: 4542 Comm: syz-executor0 Not tainted 4.4.156-514.55.6.9.x86_64+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
 1100391dbf45 ad071d3307b76e03 8801c8edfab0 81c9f586
 41b58ab3 831fd4e6 81c9f478 8801c8edfad8
 8801c8edfa78 14a9 ad071d3307b76e03 837fd660
Call Trace:
 [] __dump_stack lib/dump_stack.c:15 [inline]
 [] dump_stack+0x10e/0x1a8 lib/dump_stack.c:51
 [] ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
 [] handle_overflow+0x23e/0x299 lib/ubsan.c:195
 [] __ubsan_handle_sub_overflow+0x2a/0x31 lib/ubsan.c:211
 [] hrtimer_reprogram kernel/time/hrtimer.c:615 [inline]
 [] hrtimer_start_range_ns+0x1083/0x1580 
kernel/time/hrtimer.c:1024
 [] hrtimer_start include/linux/hrtimer.h:393 [inline]
 [] alarm_start+0xcf/0x130 kernel/time/alarmtimer.c:328
 [] alarm_timer_set+0x296/0x4a0 kernel/time/alarmtimer.c:632
 [] SYSC_timer_settime kernel/time/posix-timers.c:914 [inline]
 [] SyS_timer_settime+0x2be/0x3d0 
kernel/time/posix-timers.c:885
 [] entry_SYSCALL_64_fastpath+0x1e/0x9e


UBSAN: Undefined behaviour in kernel/time/hrtimer.c:490:13
signed integer overflow:
9223372036854775807 - -495588161 cannot be represented in type 'long long int'
CPU: 0 PID: 4542 Comm: syz-executor0 Not tainted 4.4.156-514.55.6.9.x86_64+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
 11003ed40f8b ad071d3307b76e03 8801f6a07ce0 81c9f586
 41b58ab3 831fd4e6 81c9f478 8801f6a07d08
 8801f6a07ca8 000a ad071d3307b76e03 837fd660
Call Trace:
   [] __dump_stack lib/dump_stack.c:15 [inline]
   [] dump_stack+0x10e/0x1a8 lib/dump_stack.c:51
 [] ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
 [] handle_overflow+0x23e/0x299 lib/ubsan.c:195
 [] __ubsan_handle_sub_overflow+0x2a/0x31 lib/ubsan.c:211
 [] __hrtimer_get_next_event+0x1da/0x2b0 
kernel/time/hrtimer.c:490
 [] hrtimer_interrupt+0x202/0x580 kernel/time/hrtimer.c:1361
 [] local_apic_timer_interrupt+0x9d/0x150 
arch/x86/kernel/apic/apic.c:901
 [] smp_apic_timer_interrupt+0x80/0xb0 
arch/x86/kernel/apic/apic.c:925
 [] apic_timer_interrupt+0xa5/0xb0 
arch/x86/entry/entry_64.S:563
   [] ? arch_local_irq_restore 
arch/x86/include/asm/paravirt.h:812 [inline]
   [] ? __raw_spin_unlock_irqrestore 
include/linux/spinlock_api_smp.h:162 [inline]
   [] ? _raw_spin_unlock_irqrestore+0x3b/0x60 
kernel/locking/spinlock.c:191
 [] unlock_timer include/linux/spinlock.h:362 [inline]
 [] SYSC_timer_settime kernel/time/posix-timers.c:916 [inline]
 [] SyS_timer_settime+0x2cf/0x3d0 
kernel/time/posix-timers.c:885
 [] entry_SYSCALL_64_fastpath+0x1e/0x9e

[Question] A UBSAN problem in stable-4.4

[Tan Xiaojun]

Hi, all,

I found the following problem (attached to the end) when testing stable-4.4 with
Syzkaller. This is not an easy-to-trigger problem, so the tool does not generate
code for recurring problems.

>From the call stack, it is because the first parameter in ktime_sub is large, 
>and
the second parameter offset is a negative number, causing the final result to
overflow into the sign bit and become a large negative number.

--
...
ktime_t expires = ktime_sub(hrtimer_get_expires(timer), base->offset);
...
--

But I don't know how to fix this problem. The mainline code is also different 
from
stable-4.4, and I have not found a patch to fix this problem in the mainline
repository.

So I am a bit confused about how to fix it. Can anyone give me some advice?

Thanks.
Xiaojun.


UBSAN: Undefined behaviour in kernel/time/hrtimer.c:615:20
signed integer overflow:
9223372036854775807 - -495588161 cannot be represented in type 'long long int'
CPU: 0 PID: 4542 Comm: syz-executor0 Not tainted 4.4.156-514.55.6.9.x86_64+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
 1100391dbf45 ad071d3307b76e03 8801c8edfab0 81c9f586
 41b58ab3 831fd4e6 81c9f478 8801c8edfad8
 8801c8edfa78 14a9 ad071d3307b76e03 837fd660
Call Trace:
 [] __dump_stack lib/dump_stack.c:15 [inline]
 [] dump_stack+0x10e/0x1a8 lib/dump_stack.c:51
 [] ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
 [] handle_overflow+0x23e/0x299 lib/ubsan.c:195
 [] __ubsan_handle_sub_overflow+0x2a/0x31 lib/ubsan.c:211
 [] hrtimer_reprogram kernel/time/hrtimer.c:615 [inline]
 [] hrtimer_start_range_ns+0x1083/0x1580 
kernel/time/hrtimer.c:1024
 [] hrtimer_start include/linux/hrtimer.h:393 [inline]
 [] alarm_start+0xcf/0x130 kernel/time/alarmtimer.c:328
 [] alarm_timer_set+0x296/0x4a0 kernel/time/alarmtimer.c:632
 [] SYSC_timer_settime kernel/time/posix-timers.c:914 [inline]
 [] SyS_timer_settime+0x2be/0x3d0 
kernel/time/posix-timers.c:885
 [] entry_SYSCALL_64_fastpath+0x1e/0x9e


UBSAN: Undefined behaviour in kernel/time/hrtimer.c:490:13
signed integer overflow:
9223372036854775807 - -495588161 cannot be represented in type 'long long int'
CPU: 0 PID: 4542 Comm: syz-executor0 Not tainted 4.4.156-514.55.6.9.x86_64+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
 11003ed40f8b ad071d3307b76e03 8801f6a07ce0 81c9f586
 41b58ab3 831fd4e6 81c9f478 8801f6a07d08
 8801f6a07ca8 000a ad071d3307b76e03 837fd660
Call Trace:
   [] __dump_stack lib/dump_stack.c:15 [inline]
   [] dump_stack+0x10e/0x1a8 lib/dump_stack.c:51
 [] ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
 [] handle_overflow+0x23e/0x299 lib/ubsan.c:195
 [] __ubsan_handle_sub_overflow+0x2a/0x31 lib/ubsan.c:211
 [] __hrtimer_get_next_event+0x1da/0x2b0 
kernel/time/hrtimer.c:490
 [] hrtimer_interrupt+0x202/0x580 kernel/time/hrtimer.c:1361
 [] local_apic_timer_interrupt+0x9d/0x150 
arch/x86/kernel/apic/apic.c:901
 [] smp_apic_timer_interrupt+0x80/0xb0 
arch/x86/kernel/apic/apic.c:925
 [] apic_timer_interrupt+0xa5/0xb0 
arch/x86/entry/entry_64.S:563
   [] ? arch_local_irq_restore 
arch/x86/include/asm/paravirt.h:812 [inline]
   [] ? __raw_spin_unlock_irqrestore 
include/linux/spinlock_api_smp.h:162 [inline]
   [] ? _raw_spin_unlock_irqrestore+0x3b/0x60 
kernel/locking/spinlock.c:191
 [] unlock_timer include/linux/spinlock.h:362 [inline]
 [] SYSC_timer_settime kernel/time/posix-timers.c:916 [inline]
 [] SyS_timer_settime+0x2cf/0x3d0 
kernel/time/posix-timers.c:885
 [] entry_SYSCALL_64_fastpath+0x1e/0x9e

[PATCH] net: hns3: fix length overflow when CONFIG_ARM64_64K_PAGES

[Tan Xiaojun]

When enable the config item "CONFIG_ARM64_64K_PAGES", the size of PAGE_SIZE
is 65536(64K). But the type of length is u16, it will overflow. So change it
to u32.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h 
b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index 9e4cfbb..98cdbd3 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -288,7 +288,7 @@ struct hns3_desc_cb {
u16 page_offset;
u16 reuse_flag;
 
-   u16 length; /* length of the buffer */
+   u32 length; /* length of the buffer */
 
/* desc type, used by the ring user to mark the type of the priv data */
u16 type;
-- 
2.7.4

[PATCH] net: hns3: fix length overflow when CONFIG_ARM64_64K_PAGES

[Tan Xiaojun]

When enable the config item "CONFIG_ARM64_64K_PAGES", the size of PAGE_SIZE
is 65536(64K). But the type of length is u16, it will overflow. So change it
to u32.

Signed-off-by: Tan Xiaojun 
---
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h 
b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index 9e4cfbb..98cdbd3 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -288,7 +288,7 @@ struct hns3_desc_cb {
u16 page_offset;
u16 reuse_flag;
 
-   u16 length; /* length of the buffer */
+   u32 length; /* length of the buffer */
 
/* desc type, used by the ring user to mark the type of the priv data */
u16 type;
-- 
2.7.4

Re: [PATCH v2 00/12] drm: add check if io_mem_pfn is NULL and cleanup

[Tan Xiaojun]

On 2017/12/25 16:18, Christian König wrote:
> Series is Reviewed-by: Christian König <christian.koe...@amd.com>.
> 
> I'm going to pick that up for 4.16.
> 
> Thanks for the cleanup,
> Christian.
> 

Thank you very much.
Xiaojun.

> Am 25.12.2017 um 04:43 schrieb Tan Xiaojun:
>> I found an OOPS when I used the mainline kernel for graphical tests in 
>> Hisilicon
>> D05, I do not know how to solve this problem until I saw your discussion on 
>> this
>> issue a month ago:
>>
>> https://lists.freedesktop.org/archives/dri-devel/2017-November/159046.html
>>
>> And my problem can be solved perfectly by your solution.
>>
>> This is important for me, I want to solve this problem as soon as possible. 
>> So
>> I follow the result of your discussion, make and send these patches below.
>>
>> If anything is not good, please point it out, thanks.
>>
>> Change logs of v2:
>>   * add new function to instead of ttm_bo_default_io_mem_pfn() and
>> do some cleanup.
>>
>> Tan Xiaojun (12):
>>drm/ttm: add ttm_bo_io_mem_pfn to check io_mem_pfn
>>drm/ast: remove the default io_mem_pfn set
>>drm/bochs: remove the default io_mem_pfn set
>>drm/cirrus: remove the default io_mem_pfn set
>>drm/mgag200: remove the default io_mem_pfn set
>>drm/nouveau: remove the default io_mem_pfn set
>>drm/qxl: remove the default io_mem_pfn set
>>drm/radeon: remove the default io_mem_pfn set
>>drm/virtio: remove the default io_mem_pfn set
>>drm/vmwgfx: remove the default io_mem_pfn set
>>staging: remove the default io_mem_pfn set
>>drm/ttm: remove ttm_bo_default_io_mem_pfn
>>
>>   drivers/gpu/drm/ast/ast_ttm.c  |  1 -
>>   drivers/gpu/drm/bochs/bochs_mm.c   |  1 -
>>   drivers/gpu/drm/cirrus/cirrus_ttm.c|  1 -
>>   drivers/gpu/drm/mgag200/mgag200_ttm.c  |  1 -
>>   drivers/gpu/drm/nouveau/nouveau_bo.c   |  1 -
>>   drivers/gpu/drm/qxl/qxl_ttm.c  |  1 -
>>   drivers/gpu/drm/radeon/radeon_ttm.c|  1 -
>>   drivers/gpu/drm/ttm/ttm_bo_vm.c| 22 +-
>>   drivers/gpu/drm/virtio/virtgpu_ttm.c   |  1 -
>>   drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c |  1 -
>>   drivers/staging/vboxvideo/vbox_ttm.c   |  1 -
>>   include/drm/ttm/ttm_bo_api.h   | 11 ---
>>   12 files changed, 13 insertions(+), 30 deletions(-)
>>
> 
> 
> 

Re: [PATCH v2 00/12] drm: add check if io_mem_pfn is NULL and cleanup

[Tan Xiaojun]

On 2017/12/25 16:18, Christian König wrote:
> Series is Reviewed-by: Christian König .
> 
> I'm going to pick that up for 4.16.
> 
> Thanks for the cleanup,
> Christian.
> 

Thank you very much.
Xiaojun.

> Am 25.12.2017 um 04:43 schrieb Tan Xiaojun:
>> I found an OOPS when I used the mainline kernel for graphical tests in 
>> Hisilicon
>> D05, I do not know how to solve this problem until I saw your discussion on 
>> this
>> issue a month ago:
>>
>> https://lists.freedesktop.org/archives/dri-devel/2017-November/159046.html
>>
>> And my problem can be solved perfectly by your solution.
>>
>> This is important for me, I want to solve this problem as soon as possible. 
>> So
>> I follow the result of your discussion, make and send these patches below.
>>
>> If anything is not good, please point it out, thanks.
>>
>> Change logs of v2:
>>   * add new function to instead of ttm_bo_default_io_mem_pfn() and
>> do some cleanup.
>>
>> Tan Xiaojun (12):
>>drm/ttm: add ttm_bo_io_mem_pfn to check io_mem_pfn
>>drm/ast: remove the default io_mem_pfn set
>>drm/bochs: remove the default io_mem_pfn set
>>drm/cirrus: remove the default io_mem_pfn set
>>drm/mgag200: remove the default io_mem_pfn set
>>drm/nouveau: remove the default io_mem_pfn set
>>drm/qxl: remove the default io_mem_pfn set
>>drm/radeon: remove the default io_mem_pfn set
>>drm/virtio: remove the default io_mem_pfn set
>>drm/vmwgfx: remove the default io_mem_pfn set
>>staging: remove the default io_mem_pfn set
>>drm/ttm: remove ttm_bo_default_io_mem_pfn
>>
>>   drivers/gpu/drm/ast/ast_ttm.c  |  1 -
>>   drivers/gpu/drm/bochs/bochs_mm.c   |  1 -
>>   drivers/gpu/drm/cirrus/cirrus_ttm.c|  1 -
>>   drivers/gpu/drm/mgag200/mgag200_ttm.c  |  1 -
>>   drivers/gpu/drm/nouveau/nouveau_bo.c   |  1 -
>>   drivers/gpu/drm/qxl/qxl_ttm.c  |  1 -
>>   drivers/gpu/drm/radeon/radeon_ttm.c|  1 -
>>   drivers/gpu/drm/ttm/ttm_bo_vm.c| 22 +-
>>   drivers/gpu/drm/virtio/virtgpu_ttm.c   |  1 -
>>   drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c |  1 -
>>   drivers/staging/vboxvideo/vbox_ttm.c   |  1 -
>>   include/drm/ttm/ttm_bo_api.h   | 11 ---
>>   12 files changed, 13 insertions(+), 30 deletions(-)
>>
> 
> 
> 

[PATCH v2 07/12] drm/qxl: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index ab48238..a86eaf9 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -393,7 +393,6 @@ static struct ttm_bo_driver qxl_bo_driver = {
.verify_access = _verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
.move_notify = _bo_move_notify,
 };
 
-- 
2.7.4

[PATCH v2 08/12] drm/radeon: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 6ada64d..8595c76 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -844,7 +844,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
.fault_reserve_notify = _bo_fault_reserve_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int radeon_ttm_init(struct radeon_device *rdev)
-- 
2.7.4

[PATCH v2 07/12] drm/qxl: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index ab48238..a86eaf9 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -393,7 +393,6 @@ static struct ttm_bo_driver qxl_bo_driver = {
.verify_access = _verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
.move_notify = _bo_move_notify,
 };
 
-- 
2.7.4

[PATCH v2 08/12] drm/radeon: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 6ada64d..8595c76 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -844,7 +844,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
.fault_reserve_notify = _bo_fault_reserve_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int radeon_ttm_init(struct radeon_device *rdev)
-- 
2.7.4

[PATCH v2 01/12] drm/ttm: add ttm_bo_io_mem_pfn to check io_mem_pfn

[Tan Xiaojun]

The io_mem_pfn field was added in commit ea642c3216cb ("drm/ttm: add
io_mem_pfn callback") and is called unconditionally. However, not all
drivers were updated to set it.

Use the ttm_bo_default_io_mem_pfn function if a driver did not set its
own. And add new function ttm_bo_io_mem_pfn() as wrapper.

Signed-off-by: Michal Srb <m...@suse.com>
Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index c8ebb75..292d157 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -92,6 +92,17 @@ static int ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
return ret;
 }
 
+static unsigned long ttm_bo_io_mem_pfn(struct ttm_buffer_object *bo,
+  unsigned long page_offset)
+{
+   struct ttm_bo_device *bdev = bo->bdev;
+
+   if (bdev->driver->io_mem_pfn)
+   return bdev->driver->io_mem_pfn(bo, page_offset);
+
+   return ttm_bo_default_io_mem_pfn(bo, page_offset);
+}
+
 static int ttm_bo_vm_fault(struct vm_fault *vmf)
 {
struct vm_area_struct *vma = vmf->vma;
@@ -234,7 +245,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
if (bo->mem.bus.is_iomem) {
/* Iomem should not be marked encrypted */
cvma.vm_page_prot = pgprot_decrypted(cvma.vm_page_prot);
-   pfn = bdev->driver->io_mem_pfn(bo, page_offset);
+   pfn = ttm_bo_io_mem_pfn(bo, page_offset);
} else {
page = ttm->pages[page_offset];
if (unlikely(!page && i == 0)) {
-- 
2.7.4

[PATCH v2 01/12] drm/ttm: add ttm_bo_io_mem_pfn to check io_mem_pfn

[Tan Xiaojun]

The io_mem_pfn field was added in commit ea642c3216cb ("drm/ttm: add
io_mem_pfn callback") and is called unconditionally. However, not all
drivers were updated to set it.

Use the ttm_bo_default_io_mem_pfn function if a driver did not set its
own. And add new function ttm_bo_io_mem_pfn() as wrapper.

Signed-off-by: Michal Srb 
Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index c8ebb75..292d157 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -92,6 +92,17 @@ static int ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
return ret;
 }
 
+static unsigned long ttm_bo_io_mem_pfn(struct ttm_buffer_object *bo,
+  unsigned long page_offset)
+{
+   struct ttm_bo_device *bdev = bo->bdev;
+
+   if (bdev->driver->io_mem_pfn)
+   return bdev->driver->io_mem_pfn(bo, page_offset);
+
+   return ttm_bo_default_io_mem_pfn(bo, page_offset);
+}
+
 static int ttm_bo_vm_fault(struct vm_fault *vmf)
 {
struct vm_area_struct *vma = vmf->vma;
@@ -234,7 +245,7 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
if (bo->mem.bus.is_iomem) {
/* Iomem should not be marked encrypted */
cvma.vm_page_prot = pgprot_decrypted(cvma.vm_page_prot);
-   pfn = bdev->driver->io_mem_pfn(bo, page_offset);
+   pfn = ttm_bo_io_mem_pfn(bo, page_offset);
} else {
page = ttm->pages[page_offset];
if (unlikely(!page && i == 0)) {
-- 
2.7.4

[PATCH v2 09/12] drm/virtio: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/virtio/virtgpu_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_ttm.c 
b/drivers/gpu/drm/virtio/virtgpu_ttm.c
index cd389c5..4a12434 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ttm.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ttm.c
@@ -431,7 +431,6 @@ static struct ttm_bo_driver virtio_gpu_bo_driver = {
.verify_access = _gpu_verify_access,
.io_mem_reserve = _gpu_ttm_io_mem_reserve,
.io_mem_free = _gpu_ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
.move_notify = _gpu_bo_move_notify,
.swap_notify = _gpu_bo_swap_notify,
 };
-- 
2.7.4

[PATCH v2 09/12] drm/virtio: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/virtio/virtgpu_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_ttm.c 
b/drivers/gpu/drm/virtio/virtgpu_ttm.c
index cd389c5..4a12434 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ttm.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ttm.c
@@ -431,7 +431,6 @@ static struct ttm_bo_driver virtio_gpu_bo_driver = {
.verify_access = _gpu_verify_access,
.io_mem_reserve = _gpu_ttm_io_mem_reserve,
.io_mem_free = _gpu_ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
.move_notify = _gpu_bo_move_notify,
.swap_notify = _gpu_bo_swap_notify,
 };
-- 
2.7.4

[PATCH v2 12/12] drm/ttm: remove ttm_bo_default_io_mem_pfn

[Tan Xiaojun]

No one will use this function except ttm_bo_io_mem_pfn() now, so move
the calculation of ttm_bo_default_io_mem_pfn() into ttm_bo_io_mem_pfn()
and do some cleanup.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 11 ++-
 include/drm/ttm/ttm_bo_api.h| 11 ---
 2 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 292d157..6edc19f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -100,7 +100,8 @@ static unsigned long ttm_bo_io_mem_pfn(struct 
ttm_buffer_object *bo,
if (bdev->driver->io_mem_pfn)
return bdev->driver->io_mem_pfn(bo, page_offset);
 
-   return ttm_bo_default_io_mem_pfn(bo, page_offset);
+   return ((bo->mem.bus.base + bo->mem.bus.offset) >> PAGE_SHIFT)
+   + page_offset;
 }
 
 static int ttm_bo_vm_fault(struct vm_fault *vmf)
@@ -415,14 +416,6 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct 
ttm_bo_device *bdev,
return bo;
 }
 
-unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
-   unsigned long page_offset)
-{
-   return ((bo->mem.bus.base + bo->mem.bus.offset) >> PAGE_SHIFT)
-   + page_offset;
-}
-EXPORT_SYMBOL(ttm_bo_default_io_mem_pfn);
-
 int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
struct ttm_bo_device *bdev)
 {
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index fa07be1..0b1ce05 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -711,17 +711,6 @@ extern int ttm_fbdev_mmap(struct vm_area_struct *vma,
  struct ttm_buffer_object *bo);
 
 /**
- * ttm_bo_default_iomem_pfn - get a pfn for a page offset
- *
- * @bo: the BO we need to look up the pfn for
- * @page_offset: offset inside the BO to look up.
- *
- * Calculate the PFN for iomem based mappings during page fault
- */
-unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
-   unsigned long page_offset);
-
-/**
  * ttm_bo_mmap - mmap out of the ttm device address space.
  *
  * @filp:  filp as input from the mmap method.
-- 
2.7.4

[PATCH v2 11/12] staging: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/staging/vboxvideo/vbox_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/vboxvideo/vbox_ttm.c 
b/drivers/staging/vboxvideo/vbox_ttm.c
index 4eb410a..4da1723 100644
--- a/drivers/staging/vboxvideo/vbox_ttm.c
+++ b/drivers/staging/vboxvideo/vbox_ttm.c
@@ -241,7 +241,6 @@ static struct ttm_bo_driver vbox_bo_driver = {
.verify_access = vbox_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int vbox_mm_init(struct vbox_private *vbox)
-- 
2.7.4

[PATCH v2 12/12] drm/ttm: remove ttm_bo_default_io_mem_pfn

[Tan Xiaojun]

No one will use this function except ttm_bo_io_mem_pfn() now, so move
the calculation of ttm_bo_default_io_mem_pfn() into ttm_bo_io_mem_pfn()
and do some cleanup.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 11 ++-
 include/drm/ttm/ttm_bo_api.h| 11 ---
 2 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 292d157..6edc19f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -100,7 +100,8 @@ static unsigned long ttm_bo_io_mem_pfn(struct 
ttm_buffer_object *bo,
if (bdev->driver->io_mem_pfn)
return bdev->driver->io_mem_pfn(bo, page_offset);
 
-   return ttm_bo_default_io_mem_pfn(bo, page_offset);
+   return ((bo->mem.bus.base + bo->mem.bus.offset) >> PAGE_SHIFT)
+   + page_offset;
 }
 
 static int ttm_bo_vm_fault(struct vm_fault *vmf)
@@ -415,14 +416,6 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct 
ttm_bo_device *bdev,
return bo;
 }
 
-unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
-   unsigned long page_offset)
-{
-   return ((bo->mem.bus.base + bo->mem.bus.offset) >> PAGE_SHIFT)
-   + page_offset;
-}
-EXPORT_SYMBOL(ttm_bo_default_io_mem_pfn);
-
 int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
struct ttm_bo_device *bdev)
 {
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index fa07be1..0b1ce05 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -711,17 +711,6 @@ extern int ttm_fbdev_mmap(struct vm_area_struct *vma,
  struct ttm_buffer_object *bo);
 
 /**
- * ttm_bo_default_iomem_pfn - get a pfn for a page offset
- *
- * @bo: the BO we need to look up the pfn for
- * @page_offset: offset inside the BO to look up.
- *
- * Calculate the PFN for iomem based mappings during page fault
- */
-unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
-   unsigned long page_offset);
-
-/**
  * ttm_bo_mmap - mmap out of the ttm device address space.
  *
  * @filp:  filp as input from the mmap method.
-- 
2.7.4

[PATCH v2 11/12] staging: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/staging/vboxvideo/vbox_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/vboxvideo/vbox_ttm.c 
b/drivers/staging/vboxvideo/vbox_ttm.c
index 4eb410a..4da1723 100644
--- a/drivers/staging/vboxvideo/vbox_ttm.c
+++ b/drivers/staging/vboxvideo/vbox_ttm.c
@@ -241,7 +241,6 @@ static struct ttm_bo_driver vbox_bo_driver = {
.verify_access = vbox_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int vbox_mm_init(struct vbox_private *vbox)
-- 
2.7.4

[PATCH v2 00/12] drm: add check if io_mem_pfn is NULL and cleanup

[Tan Xiaojun]

I found an OOPS when I used the mainline kernel for graphical tests in Hisilicon
D05, I do not know how to solve this problem until I saw your discussion on this
issue a month ago:

https://lists.freedesktop.org/archives/dri-devel/2017-November/159046.html

And my problem can be solved perfectly by your solution.

This is important for me, I want to solve this problem as soon as possible. So
I follow the result of your discussion, make and send these patches below.

If anything is not good, please point it out, thanks.

Change logs of v2:
 * add new function to instead of ttm_bo_default_io_mem_pfn() and
   do some cleanup.

Tan Xiaojun (12):
  drm/ttm: add ttm_bo_io_mem_pfn to check io_mem_pfn
  drm/ast: remove the default io_mem_pfn set
  drm/bochs: remove the default io_mem_pfn set
  drm/cirrus: remove the default io_mem_pfn set
  drm/mgag200: remove the default io_mem_pfn set
  drm/nouveau: remove the default io_mem_pfn set
  drm/qxl: remove the default io_mem_pfn set
  drm/radeon: remove the default io_mem_pfn set
  drm/virtio: remove the default io_mem_pfn set
  drm/vmwgfx: remove the default io_mem_pfn set
  staging: remove the default io_mem_pfn set
  drm/ttm: remove ttm_bo_default_io_mem_pfn

 drivers/gpu/drm/ast/ast_ttm.c  |  1 -
 drivers/gpu/drm/bochs/bochs_mm.c   |  1 -
 drivers/gpu/drm/cirrus/cirrus_ttm.c|  1 -
 drivers/gpu/drm/mgag200/mgag200_ttm.c  |  1 -
 drivers/gpu/drm/nouveau/nouveau_bo.c   |  1 -
 drivers/gpu/drm/qxl/qxl_ttm.c  |  1 -
 drivers/gpu/drm/radeon/radeon_ttm.c|  1 -
 drivers/gpu/drm/ttm/ttm_bo_vm.c| 22 +-
 drivers/gpu/drm/virtio/virtgpu_ttm.c   |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c |  1 -
 drivers/staging/vboxvideo/vbox_ttm.c   |  1 -
 include/drm/ttm/ttm_bo_api.h   | 11 ---
 12 files changed, 13 insertions(+), 30 deletions(-)

-- 
2.7.4

[PATCH v2 00/12] drm: add check if io_mem_pfn is NULL and cleanup

[Tan Xiaojun]

I found an OOPS when I used the mainline kernel for graphical tests in Hisilicon
D05, I do not know how to solve this problem until I saw your discussion on this
issue a month ago:

https://lists.freedesktop.org/archives/dri-devel/2017-November/159046.html

And my problem can be solved perfectly by your solution.

This is important for me, I want to solve this problem as soon as possible. So
I follow the result of your discussion, make and send these patches below.

If anything is not good, please point it out, thanks.

Change logs of v2:
 * add new function to instead of ttm_bo_default_io_mem_pfn() and
   do some cleanup.

Tan Xiaojun (12):
  drm/ttm: add ttm_bo_io_mem_pfn to check io_mem_pfn
  drm/ast: remove the default io_mem_pfn set
  drm/bochs: remove the default io_mem_pfn set
  drm/cirrus: remove the default io_mem_pfn set
  drm/mgag200: remove the default io_mem_pfn set
  drm/nouveau: remove the default io_mem_pfn set
  drm/qxl: remove the default io_mem_pfn set
  drm/radeon: remove the default io_mem_pfn set
  drm/virtio: remove the default io_mem_pfn set
  drm/vmwgfx: remove the default io_mem_pfn set
  staging: remove the default io_mem_pfn set
  drm/ttm: remove ttm_bo_default_io_mem_pfn

 drivers/gpu/drm/ast/ast_ttm.c  |  1 -
 drivers/gpu/drm/bochs/bochs_mm.c   |  1 -
 drivers/gpu/drm/cirrus/cirrus_ttm.c|  1 -
 drivers/gpu/drm/mgag200/mgag200_ttm.c  |  1 -
 drivers/gpu/drm/nouveau/nouveau_bo.c   |  1 -
 drivers/gpu/drm/qxl/qxl_ttm.c  |  1 -
 drivers/gpu/drm/radeon/radeon_ttm.c|  1 -
 drivers/gpu/drm/ttm/ttm_bo_vm.c| 22 +-
 drivers/gpu/drm/virtio/virtgpu_ttm.c   |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c |  1 -
 drivers/staging/vboxvideo/vbox_ttm.c   |  1 -
 include/drm/ttm/ttm_bo_api.h   | 11 ---
 12 files changed, 13 insertions(+), 30 deletions(-)

-- 
2.7.4

[PATCH v2 04/12] drm/cirrus: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/cirrus/cirrus_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c 
b/drivers/gpu/drm/cirrus/cirrus_ttm.c
index 1ff1838..2c652af 100644
--- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
+++ b/drivers/gpu/drm/cirrus/cirrus_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver cirrus_bo_driver = {
.verify_access = cirrus_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int cirrus_mm_init(struct cirrus_device *cirrus)
-- 
2.7.4

[PATCH v2 04/12] drm/cirrus: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/cirrus/cirrus_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c 
b/drivers/gpu/drm/cirrus/cirrus_ttm.c
index 1ff1838..2c652af 100644
--- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
+++ b/drivers/gpu/drm/cirrus/cirrus_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver cirrus_bo_driver = {
.verify_access = cirrus_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int cirrus_mm_init(struct cirrus_device *cirrus)
-- 
2.7.4

[PATCH v2 05/12] drm/mgag200: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/mgag200/mgag200_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c 
b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index 3e7e1cd..89b550f 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver mgag200_bo_driver = {
.verify_access = mgag200_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int mgag200_mm_init(struct mga_device *mdev)
-- 
2.7.4

[PATCH v2 05/12] drm/mgag200: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/mgag200/mgag200_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c 
b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index 3e7e1cd..89b550f 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver mgag200_bo_driver = {
.verify_access = mgag200_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int mgag200_mm_init(struct mga_device *mdev)
-- 
2.7.4

[PATCH v2 03/12] drm/bochs: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/bochs/bochs_mm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
index c4cadb6..857755a 100644
--- a/drivers/gpu/drm/bochs/bochs_mm.c
+++ b/drivers/gpu/drm/bochs/bochs_mm.c
@@ -205,7 +205,6 @@ struct ttm_bo_driver bochs_bo_driver = {
.verify_access = bochs_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int bochs_mm_init(struct bochs_device *bochs)
-- 
2.7.4

[PATCH v2 06/12] drm/nouveau: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 435ff86..8de82a3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1667,5 +1667,4 @@ struct ttm_bo_driver nouveau_bo_driver = {
.fault_reserve_notify = _ttm_fault_reserve_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
-- 
2.7.4

[PATCH v2 03/12] drm/bochs: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/bochs/bochs_mm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
index c4cadb6..857755a 100644
--- a/drivers/gpu/drm/bochs/bochs_mm.c
+++ b/drivers/gpu/drm/bochs/bochs_mm.c
@@ -205,7 +205,6 @@ struct ttm_bo_driver bochs_bo_driver = {
.verify_access = bochs_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int bochs_mm_init(struct bochs_device *bochs)
-- 
2.7.4

[PATCH v2 06/12] drm/nouveau: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 435ff86..8de82a3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1667,5 +1667,4 @@ struct ttm_bo_driver nouveau_bo_driver = {
.fault_reserve_notify = _ttm_fault_reserve_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
-- 
2.7.4

[PATCH v2 10/12] drm/vmwgfx: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
index c705632..828dd59 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
@@ -859,5 +859,4 @@ struct ttm_bo_driver vmw_bo_driver = {
.fault_reserve_notify = _ttm_fault_reserve_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
-- 
2.7.4

[PATCH v2 10/12] drm/vmwgfx: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
index c705632..828dd59 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
@@ -859,5 +859,4 @@ struct ttm_bo_driver vmw_bo_driver = {
.fault_reserve_notify = _ttm_fault_reserve_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
-- 
2.7.4

[PATCH v2 02/12] drm/ast: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/ast/ast_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
index 696a15d..fdd521d 100644
--- a/drivers/gpu/drm/ast/ast_ttm.c
+++ b/drivers/gpu/drm/ast/ast_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver ast_bo_driver = {
.verify_access = ast_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int ast_mm_init(struct ast_private *ast)
-- 
2.7.4

[PATCH v2 02/12] drm/ast: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/ast/ast_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
index 696a15d..fdd521d 100644
--- a/drivers/gpu/drm/ast/ast_ttm.c
+++ b/drivers/gpu/drm/ast/ast_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver ast_bo_driver = {
.verify_access = ast_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int ast_mm_init(struct ast_private *ast)
-- 
2.7.4

Re: [PATCH 01/12] drm/ttm: Use ttm_bo_default_io_mem_pfn if io_mem_pfn is NULL

[Tan Xiaojun]

On 2017/12/24 17:27, Christian König wrote:
> Am 24.12.2017 um 07:14 schrieb Tan Xiaojun:
>> From: Michal Srb <m...@suse.com>
>>
>> The io_mem_pfn field was added in commit 
>> ea642c3216cb2a60d1c0e760ae47ee85c9c16447
>> and is called unconditionally. However, not all drivers were updated to set 
>> it.
>>
>> Use the ttm_bo_default_io_mem_pfn function if a driver did not set its own.
>>
>> Signed-off-by: Michal Srb <m...@suse.com>
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo_vm.c | 5 -
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c 
>> b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> index c8ebb75..e25a99b 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> @@ -234,7 +234,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
>>   if (bo->mem.bus.is_iomem) {
>>   /* Iomem should not be marked encrypted */
>>   cvma.vm_page_prot = pgprot_decrypted(cvma.vm_page_prot);
>> -pfn = bdev->driver->io_mem_pfn(bo, page_offset);
>> +if (bdev->driver->io_mem_pfn)
>> +pfn = bdev->driver->io_mem_pfn(bo, page_offset);
>> +else
>> +pfn = ttm_bo_default_io_mem_pfn(bo, page_offset);
> 
> Please move this check into a new function ttm_bo_io_mem_pfn().
> 
> You can then move the calculation of ttm_bo_default_io_mem_pfn() into this 
> new function in patch #12 as well.
> 
> Regards,
> Christian.
> 

OK. Thank you for your reply. I will modify it and send v2.

Thanks.
Xiaojun.

>>   } else {
>>   page = ttm->pages[page_offset];
>>   if (unlikely(!page && i == 0)) {
> 
> 
> .
> 

Re: [PATCH 01/12] drm/ttm: Use ttm_bo_default_io_mem_pfn if io_mem_pfn is NULL

[Tan Xiaojun]

On 2017/12/24 17:27, Christian König wrote:
> Am 24.12.2017 um 07:14 schrieb Tan Xiaojun:
>> From: Michal Srb 
>>
>> The io_mem_pfn field was added in commit 
>> ea642c3216cb2a60d1c0e760ae47ee85c9c16447
>> and is called unconditionally. However, not all drivers were updated to set 
>> it.
>>
>> Use the ttm_bo_default_io_mem_pfn function if a driver did not set its own.
>>
>> Signed-off-by: Michal Srb 
>> ---
>>   drivers/gpu/drm/ttm/ttm_bo_vm.c | 5 -
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c 
>> b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> index c8ebb75..e25a99b 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>> @@ -234,7 +234,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
>>   if (bo->mem.bus.is_iomem) {
>>   /* Iomem should not be marked encrypted */
>>   cvma.vm_page_prot = pgprot_decrypted(cvma.vm_page_prot);
>> -pfn = bdev->driver->io_mem_pfn(bo, page_offset);
>> +if (bdev->driver->io_mem_pfn)
>> +pfn = bdev->driver->io_mem_pfn(bo, page_offset);
>> +else
>> +pfn = ttm_bo_default_io_mem_pfn(bo, page_offset);
> 
> Please move this check into a new function ttm_bo_io_mem_pfn().
> 
> You can then move the calculation of ttm_bo_default_io_mem_pfn() into this 
> new function in patch #12 as well.
> 
> Regards,
> Christian.
> 

OK. Thank you for your reply. I will modify it and send v2.

Thanks.
Xiaojun.

>>   } else {
>>   page = ttm->pages[page_offset];
>>   if (unlikely(!page && i == 0)) {
> 
> 
> .
> 

[PATCH 11/12] staging: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/staging/vboxvideo/vbox_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/vboxvideo/vbox_ttm.c 
b/drivers/staging/vboxvideo/vbox_ttm.c
index 4eb410a..4da1723 100644
--- a/drivers/staging/vboxvideo/vbox_ttm.c
+++ b/drivers/staging/vboxvideo/vbox_ttm.c
@@ -241,7 +241,6 @@ static struct ttm_bo_driver vbox_bo_driver = {
.verify_access = vbox_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int vbox_mm_init(struct vbox_private *vbox)
-- 
2.7.4

[PATCH 03/12] drm/bochs: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/bochs/bochs_mm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
index c4cadb6..857755a 100644
--- a/drivers/gpu/drm/bochs/bochs_mm.c
+++ b/drivers/gpu/drm/bochs/bochs_mm.c
@@ -205,7 +205,6 @@ struct ttm_bo_driver bochs_bo_driver = {
.verify_access = bochs_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int bochs_mm_init(struct bochs_device *bochs)
-- 
2.7.4

[PATCH 01/12] drm/ttm: Use ttm_bo_default_io_mem_pfn if io_mem_pfn is NULL

[Tan Xiaojun]

From: Michal Srb 

The io_mem_pfn field was added in commit 
ea642c3216cb2a60d1c0e760ae47ee85c9c16447
and is called unconditionally. However, not all drivers were updated to set it.

Use the ttm_bo_default_io_mem_pfn function if a driver did not set its own.

Signed-off-by: Michal Srb 
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index c8ebb75..e25a99b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -234,7 +234,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
if (bo->mem.bus.is_iomem) {
/* Iomem should not be marked encrypted */
cvma.vm_page_prot = pgprot_decrypted(cvma.vm_page_prot);
-   pfn = bdev->driver->io_mem_pfn(bo, page_offset);
+   if (bdev->driver->io_mem_pfn)
+   pfn = bdev->driver->io_mem_pfn(bo, page_offset);
+   else
+   pfn = ttm_bo_default_io_mem_pfn(bo, 
page_offset);
} else {
page = ttm->pages[page_offset];
if (unlikely(!page && i == 0)) {
-- 
2.7.4

[PATCH 11/12] staging: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/staging/vboxvideo/vbox_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/vboxvideo/vbox_ttm.c 
b/drivers/staging/vboxvideo/vbox_ttm.c
index 4eb410a..4da1723 100644
--- a/drivers/staging/vboxvideo/vbox_ttm.c
+++ b/drivers/staging/vboxvideo/vbox_ttm.c
@@ -241,7 +241,6 @@ static struct ttm_bo_driver vbox_bo_driver = {
.verify_access = vbox_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int vbox_mm_init(struct vbox_private *vbox)
-- 
2.7.4

[PATCH 03/12] drm/bochs: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/bochs/bochs_mm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
index c4cadb6..857755a 100644
--- a/drivers/gpu/drm/bochs/bochs_mm.c
+++ b/drivers/gpu/drm/bochs/bochs_mm.c
@@ -205,7 +205,6 @@ struct ttm_bo_driver bochs_bo_driver = {
.verify_access = bochs_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int bochs_mm_init(struct bochs_device *bochs)
-- 
2.7.4

[PATCH 01/12] drm/ttm: Use ttm_bo_default_io_mem_pfn if io_mem_pfn is NULL

[Tan Xiaojun]

From: Michal Srb 

The io_mem_pfn field was added in commit 
ea642c3216cb2a60d1c0e760ae47ee85c9c16447
and is called unconditionally. However, not all drivers were updated to set it.

Use the ttm_bo_default_io_mem_pfn function if a driver did not set its own.

Signed-off-by: Michal Srb 
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index c8ebb75..e25a99b 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -234,7 +234,10 @@ static int ttm_bo_vm_fault(struct vm_fault *vmf)
if (bo->mem.bus.is_iomem) {
/* Iomem should not be marked encrypted */
cvma.vm_page_prot = pgprot_decrypted(cvma.vm_page_prot);
-   pfn = bdev->driver->io_mem_pfn(bo, page_offset);
+   if (bdev->driver->io_mem_pfn)
+   pfn = bdev->driver->io_mem_pfn(bo, page_offset);
+   else
+   pfn = ttm_bo_default_io_mem_pfn(bo, 
page_offset);
} else {
page = ttm->pages[page_offset];
if (unlikely(!page && i == 0)) {
-- 
2.7.4

[PATCH 12/12] drm/ttm: unexport ttm_bo_default_io_mem_pfn and make it static

[Tan Xiaojun]

No one will use this function except in ttm_bo_vm.c now. So unexport it
and make it static.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 23 +++
 include/drm/ttm/ttm_bo_api.h| 11 ---
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index e25a99b..ff70fc96 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -92,6 +92,21 @@ static int ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
return ret;
 }
 
+/**
+ * ttm_bo_default_iomem_pfn - get a pfn for a page offset
+ *
+ * @bo: the BO we need to look up the pfn for
+ * @page_offset: offset inside the BO to look up.
+ *
+ * Calculate the PFN for iomem based mappings during page fault
+ */
+static unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
+  unsigned long page_offset)
+{
+   return ((bo->mem.bus.base + bo->mem.bus.offset) >> PAGE_SHIFT)
+   + page_offset;
+}
+
 static int ttm_bo_vm_fault(struct vm_fault *vmf)
 {
struct vm_area_struct *vma = vmf->vma;
@@ -407,14 +422,6 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct 
ttm_bo_device *bdev,
return bo;
 }
 
-unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
-   unsigned long page_offset)
-{
-   return ((bo->mem.bus.base + bo->mem.bus.offset) >> PAGE_SHIFT)
-   + page_offset;
-}
-EXPORT_SYMBOL(ttm_bo_default_io_mem_pfn);
-
 int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
struct ttm_bo_device *bdev)
 {
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index fa07be1..0b1ce05 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -711,17 +711,6 @@ extern int ttm_fbdev_mmap(struct vm_area_struct *vma,
  struct ttm_buffer_object *bo);
 
 /**
- * ttm_bo_default_iomem_pfn - get a pfn for a page offset
- *
- * @bo: the BO we need to look up the pfn for
- * @page_offset: offset inside the BO to look up.
- *
- * Calculate the PFN for iomem based mappings during page fault
- */
-unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
-   unsigned long page_offset);
-
-/**
  * ttm_bo_mmap - mmap out of the ttm device address space.
  *
  * @filp:  filp as input from the mmap method.
-- 
2.7.4

[PATCH 10/12] drm/vmwgfx: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
index c705632..828dd59 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
@@ -859,5 +859,4 @@ struct ttm_bo_driver vmw_bo_driver = {
.fault_reserve_notify = _ttm_fault_reserve_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
-- 
2.7.4

[PATCH 10/12] drm/vmwgfx: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
index c705632..828dd59 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
@@ -859,5 +859,4 @@ struct ttm_bo_driver vmw_bo_driver = {
.fault_reserve_notify = _ttm_fault_reserve_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
-- 
2.7.4

[PATCH 12/12] drm/ttm: unexport ttm_bo_default_io_mem_pfn and make it static

[Tan Xiaojun]

No one will use this function except in ttm_bo_vm.c now. So unexport it
and make it static.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 23 +++
 include/drm/ttm/ttm_bo_api.h| 11 ---
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index e25a99b..ff70fc96 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -92,6 +92,21 @@ static int ttm_bo_vm_fault_idle(struct ttm_buffer_object *bo,
return ret;
 }
 
+/**
+ * ttm_bo_default_iomem_pfn - get a pfn for a page offset
+ *
+ * @bo: the BO we need to look up the pfn for
+ * @page_offset: offset inside the BO to look up.
+ *
+ * Calculate the PFN for iomem based mappings during page fault
+ */
+static unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
+  unsigned long page_offset)
+{
+   return ((bo->mem.bus.base + bo->mem.bus.offset) >> PAGE_SHIFT)
+   + page_offset;
+}
+
 static int ttm_bo_vm_fault(struct vm_fault *vmf)
 {
struct vm_area_struct *vma = vmf->vma;
@@ -407,14 +422,6 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct 
ttm_bo_device *bdev,
return bo;
 }
 
-unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
-   unsigned long page_offset)
-{
-   return ((bo->mem.bus.base + bo->mem.bus.offset) >> PAGE_SHIFT)
-   + page_offset;
-}
-EXPORT_SYMBOL(ttm_bo_default_io_mem_pfn);
-
 int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
struct ttm_bo_device *bdev)
 {
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index fa07be1..0b1ce05 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -711,17 +711,6 @@ extern int ttm_fbdev_mmap(struct vm_area_struct *vma,
  struct ttm_buffer_object *bo);
 
 /**
- * ttm_bo_default_iomem_pfn - get a pfn for a page offset
- *
- * @bo: the BO we need to look up the pfn for
- * @page_offset: offset inside the BO to look up.
- *
- * Calculate the PFN for iomem based mappings during page fault
- */
-unsigned long ttm_bo_default_io_mem_pfn(struct ttm_buffer_object *bo,
-   unsigned long page_offset);
-
-/**
  * ttm_bo_mmap - mmap out of the ttm device address space.
  *
  * @filp:  filp as input from the mmap method.
-- 
2.7.4

[PATCH 08/12] drm/radeon: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 6ada64d..8595c76 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -844,7 +844,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
.fault_reserve_notify = _bo_fault_reserve_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int radeon_ttm_init(struct radeon_device *rdev)
-- 
2.7.4

[PATCH 08/12] drm/radeon: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 6ada64d..8595c76 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -844,7 +844,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
.fault_reserve_notify = _bo_fault_reserve_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int radeon_ttm_init(struct radeon_device *rdev)
-- 
2.7.4

[PATCH 06/12] drm/nouveau: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 435ff86..8de82a3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1667,5 +1667,4 @@ struct ttm_bo_driver nouveau_bo_driver = {
.fault_reserve_notify = _ttm_fault_reserve_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
-- 
2.7.4

[PATCH 06/12] drm/nouveau: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 435ff86..8de82a3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1667,5 +1667,4 @@ struct ttm_bo_driver nouveau_bo_driver = {
.fault_reserve_notify = _ttm_fault_reserve_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
-- 
2.7.4

[PATCH 07/12] drm/qxl: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index ab48238..a86eaf9 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -393,7 +393,6 @@ static struct ttm_bo_driver qxl_bo_driver = {
.verify_access = _verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
.move_notify = _bo_move_notify,
 };
 
-- 
2.7.4

[PATCH 07/12] drm/qxl: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index ab48238..a86eaf9 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -393,7 +393,6 @@ static struct ttm_bo_driver qxl_bo_driver = {
.verify_access = _verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
.move_notify = _bo_move_notify,
 };
 
-- 
2.7.4

[PATCH 00/12] drm: add check if io_mem_pfn is NULL and cleanup

[Tan Xiaojun]

I found an OOPS when I used the mainline kernel for graphical tests in Hisilicon
D05, I do not know how to solve this problem until I saw your discussion on this
issue a month ago:

https://lists.freedesktop.org/archives/dri-devel/2017-November/159046.html

And my problem can be solved perfectly by your solution.

This is important for me, I want to solve this problem as soon as possible. So
I follow the result of your discussion, make and send these patches below.

If anything is not good, please point it out, thanks.

Michal Srb (1):
  drm/ttm: Use ttm_bo_default_io_mem_pfn if io_mem_pfn is NULL

Tan Xiaojun (11):
  drm/ast: remove the default io_mem_pfn set
  drm/bochs: remove the default io_mem_pfn set
  drm/cirrus: remove the default io_mem_pfn set
  drm/mgag200: remove the default io_mem_pfn set
  drm/nouveau: remove the default io_mem_pfn set
  drm/qxl: remove the default io_mem_pfn set
  drm/radeon: remove the default io_mem_pfn set
  drm/virtio: remove the default io_mem_pfn set
  drm/vmwgfx: remove the default io_mem_pfn set
  staging: remove the default io_mem_pfn set
  drm/ttm: unexport ttm_bo_default_io_mem_pfn and make it static

 drivers/gpu/drm/ast/ast_ttm.c  |  1 -
 drivers/gpu/drm/bochs/bochs_mm.c   |  1 -
 drivers/gpu/drm/cirrus/cirrus_ttm.c|  1 -
 drivers/gpu/drm/mgag200/mgag200_ttm.c  |  1 -
 drivers/gpu/drm/nouveau/nouveau_bo.c   |  1 -
 drivers/gpu/drm/qxl/qxl_ttm.c  |  1 -
 drivers/gpu/drm/radeon/radeon_ttm.c|  1 -
 drivers/gpu/drm/ttm/ttm_bo_vm.c| 28 +++-
 drivers/gpu/drm/virtio/virtgpu_ttm.c   |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c |  1 -
 drivers/staging/vboxvideo/vbox_ttm.c   |  1 -
 include/drm/ttm/ttm_bo_api.h   | 11 ---
 12 files changed, 19 insertions(+), 30 deletions(-)

-- 
2.7.4

[PATCH 00/12] drm: add check if io_mem_pfn is NULL and cleanup

[Tan Xiaojun]

I found an OOPS when I used the mainline kernel for graphical tests in Hisilicon
D05, I do not know how to solve this problem until I saw your discussion on this
issue a month ago:

https://lists.freedesktop.org/archives/dri-devel/2017-November/159046.html

And my problem can be solved perfectly by your solution.

This is important for me, I want to solve this problem as soon as possible. So
I follow the result of your discussion, make and send these patches below.

If anything is not good, please point it out, thanks.

Michal Srb (1):
  drm/ttm: Use ttm_bo_default_io_mem_pfn if io_mem_pfn is NULL

Tan Xiaojun (11):
  drm/ast: remove the default io_mem_pfn set
  drm/bochs: remove the default io_mem_pfn set
  drm/cirrus: remove the default io_mem_pfn set
  drm/mgag200: remove the default io_mem_pfn set
  drm/nouveau: remove the default io_mem_pfn set
  drm/qxl: remove the default io_mem_pfn set
  drm/radeon: remove the default io_mem_pfn set
  drm/virtio: remove the default io_mem_pfn set
  drm/vmwgfx: remove the default io_mem_pfn set
  staging: remove the default io_mem_pfn set
  drm/ttm: unexport ttm_bo_default_io_mem_pfn and make it static

 drivers/gpu/drm/ast/ast_ttm.c  |  1 -
 drivers/gpu/drm/bochs/bochs_mm.c   |  1 -
 drivers/gpu/drm/cirrus/cirrus_ttm.c|  1 -
 drivers/gpu/drm/mgag200/mgag200_ttm.c  |  1 -
 drivers/gpu/drm/nouveau/nouveau_bo.c   |  1 -
 drivers/gpu/drm/qxl/qxl_ttm.c  |  1 -
 drivers/gpu/drm/radeon/radeon_ttm.c|  1 -
 drivers/gpu/drm/ttm/ttm_bo_vm.c| 28 +++-
 drivers/gpu/drm/virtio/virtgpu_ttm.c   |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c |  1 -
 drivers/staging/vboxvideo/vbox_ttm.c   |  1 -
 include/drm/ttm/ttm_bo_api.h   | 11 ---
 12 files changed, 19 insertions(+), 30 deletions(-)

-- 
2.7.4

[PATCH 05/12] drm/mgag200: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/mgag200/mgag200_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c 
b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index 3e7e1cd..89b550f 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver mgag200_bo_driver = {
.verify_access = mgag200_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int mgag200_mm_init(struct mga_device *mdev)
-- 
2.7.4

[PATCH 09/12] drm/virtio: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/virtio/virtgpu_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_ttm.c 
b/drivers/gpu/drm/virtio/virtgpu_ttm.c
index cd389c5..4a12434 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ttm.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ttm.c
@@ -431,7 +431,6 @@ static struct ttm_bo_driver virtio_gpu_bo_driver = {
.verify_access = _gpu_verify_access,
.io_mem_reserve = _gpu_ttm_io_mem_reserve,
.io_mem_free = _gpu_ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
.move_notify = _gpu_bo_move_notify,
.swap_notify = _gpu_bo_swap_notify,
 };
-- 
2.7.4

[PATCH 05/12] drm/mgag200: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/mgag200/mgag200_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c 
b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index 3e7e1cd..89b550f 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver mgag200_bo_driver = {
.verify_access = mgag200_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int mgag200_mm_init(struct mga_device *mdev)
-- 
2.7.4

[PATCH 09/12] drm/virtio: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/virtio/virtgpu_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_ttm.c 
b/drivers/gpu/drm/virtio/virtgpu_ttm.c
index cd389c5..4a12434 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ttm.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ttm.c
@@ -431,7 +431,6 @@ static struct ttm_bo_driver virtio_gpu_bo_driver = {
.verify_access = _gpu_verify_access,
.io_mem_reserve = _gpu_ttm_io_mem_reserve,
.io_mem_free = _gpu_ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
.move_notify = _gpu_bo_move_notify,
.swap_notify = _gpu_bo_swap_notify,
 };
-- 
2.7.4

[PATCH 04/12] drm/cirrus: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/cirrus/cirrus_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c 
b/drivers/gpu/drm/cirrus/cirrus_ttm.c
index 1ff1838..2c652af 100644
--- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
+++ b/drivers/gpu/drm/cirrus/cirrus_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver cirrus_bo_driver = {
.verify_access = cirrus_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int cirrus_mm_init(struct cirrus_device *cirrus)
-- 
2.7.4

[PATCH 04/12] drm/cirrus: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/cirrus/cirrus_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c 
b/drivers/gpu/drm/cirrus/cirrus_ttm.c
index 1ff1838..2c652af 100644
--- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
+++ b/drivers/gpu/drm/cirrus/cirrus_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver cirrus_bo_driver = {
.verify_access = cirrus_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int cirrus_mm_init(struct cirrus_device *cirrus)
-- 
2.7.4

[PATCH 02/12] drm/ast: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/gpu/drm/ast/ast_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
index 696a15d..fdd521d 100644
--- a/drivers/gpu/drm/ast/ast_ttm.c
+++ b/drivers/gpu/drm/ast/ast_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver ast_bo_driver = {
.verify_access = ast_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int ast_mm_init(struct ast_private *ast)
-- 
2.7.4

[PATCH 02/12] drm/ast: remove the default io_mem_pfn set

[Tan Xiaojun]

The default interface situation has been taken into the framework, so
remove the default set of each module.

Signed-off-by: Tan Xiaojun 
---
 drivers/gpu/drm/ast/ast_ttm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
index 696a15d..fdd521d 100644
--- a/drivers/gpu/drm/ast/ast_ttm.c
+++ b/drivers/gpu/drm/ast/ast_ttm.c
@@ -237,7 +237,6 @@ struct ttm_bo_driver ast_bo_driver = {
.verify_access = ast_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
-   .io_mem_pfn = ttm_bo_default_io_mem_pfn,
 };
 
 int ast_mm_init(struct ast_private *ast)
-- 
2.7.4

Re: [Question] does there have another fix for ldisc crash instead of "tty: Fix ldisc crash on reopened tty" ?

[Tan Xiaojun]

On 2017/11/29 16:02, Greg KH wrote:
> On Wed, Nov 29, 2017 at 03:57:51PM +0800, Tan Xiaojun wrote:
>> Hi, all,
>>
>>  I test in a Arm64 fpga board (Hisilicon D06 fpga) with 
>> kernel-stable-4.14.2, then I get a panic log attached in the end.
>>
>>  I found this problem try to be fixed before by the patch "tty: Fix 
>> ldisc crash on reopened tty", and it was reverted
>>  shortly thereafter because of conflicting with another patch and may 
>> not fix completely.
>>
>>  But this problem is still here. Are there any better fixed patch now ?
> 
> Nope, care to work on one?
> 
> thanks,
> 
> greg k-h
> 

Thank you for your reply so soon.
I am not familiar with this codes, so ... I can try ...

Thanks.
Xiaojun.

> .
> 

Re: [Question] does there have another fix for ldisc crash instead of "tty: Fix ldisc crash on reopened tty" ?

[Tan Xiaojun]

On 2017/11/29 16:02, Greg KH wrote:
> On Wed, Nov 29, 2017 at 03:57:51PM +0800, Tan Xiaojun wrote:
>> Hi, all,
>>
>>  I test in a Arm64 fpga board (Hisilicon D06 fpga) with 
>> kernel-stable-4.14.2, then I get a panic log attached in the end.
>>
>>  I found this problem try to be fixed before by the patch "tty: Fix 
>> ldisc crash on reopened tty", and it was reverted
>>  shortly thereafter because of conflicting with another patch and may 
>> not fix completely.
>>
>>  But this problem is still here. Are there any better fixed patch now ?
> 
> Nope, care to work on one?
> 
> thanks,
> 
> greg k-h
> 

Thank you for your reply so soon.
I am not familiar with this codes, so ... I can try ...

Thanks.
Xiaojun.

> .
> 

[Question] does there have another fix for ldisc crash instead of "tty: Fix ldisc crash on reopened tty" ?

[Tan Xiaojun]

Hi, all,

I test in a Arm64 fpga board (Hisilicon D06 fpga) with 
kernel-stable-4.14.2, then I get a panic log attached in the end.

I found this problem try to be fixed before by the patch "tty: Fix 
ldisc crash on reopened tty", and it was reverted
shortly thereafter because of conflicting with another patch and may 
not fix completely.

But this problem is still here. Are there any better fixed patch now ?


Thanks.
Xiaojun.
[44100.745418] Unable to handle kernel paging request at virtual address 
2260   
[44100.758109] Mem abort info:  

[44100.761956]   Exception class = DABT (current EL), IL = 32 bits  

[44100.768677]   SET = 0, FnV = 0   

[44100.772039]   EA = 0, S1PTW = 0  

[44100.775736] Data abort info: 

[44100.779766]   ISV = 0, ISS = 0x0004  

[44100.784425]   CM = 0, WnR = 0

[44100.788179] user pgtable: 4k pages, 48-bit VAs, pgd = 8021a8b4   

[44100.795815] [2260] *pgd= 

[44100.802390] Internal error: Oops: 9604 [#1] SMP  

[44100.808278] Modules linked in: rdma_test(OE) hns_roce_pci(OE) hns_roce(OE) 
debug_nic_mii(OE) rdma_ucm(OE) rdma_cm(OE) ib_cm(OE) i
w_cm(OE) ib_uverbs(OE) ib_core(OE) hns3_enet_ut(OE) hclge(OE) hnae3(OE) [last 
unloaded: rdma_test]  
[44100.833620] CPU: 0 PID: 4680 Comm: kworker/u2:0 Tainted: G   OE   
4.14.0 #1  
[44100.845099] Workqueue: events_unbound flush_to_ldisc 

[44100.850594] task: 8021f266c200 task.stack: 3bd28000  

[44100.857556] PC is at n_tty_receive_buf_common+0x68/0xa08 

[44100.863382] LR is at n_tty_receive_buf_common+0x54/0xa08 

[44100.869091] pc : [] lr : [] pstate: 
80c00149 
[44100.876787] sp : 3bd2bc40

[44100.880411] x29: 3bd2bc40 x28: 8021a3c47400  

[44100.886332] x27: 3c533cc8 x26: 08e0c000  

[44100.892170] x25:  x24: 8021f266c200  

[44100.897990] x23:  x22: 8021a3c46c20  

[44100.903841] x21: 8021a3c474c0 x20: 0003  

[44100.909654] x19:  x18: 0003  

[44100.915482] x17: a0e0dca8 x16: 00438170  

[44100.921318] x15: 3d27a000 x14: 3d09  

[44100.927167] x13: 61a8 x12: ac42  

[44100.932979] x11: 0001 x10: 0c80  

[44100.938788] x9 : 3bd2bd40 x8 : 8021f266cee0  

[44100.944592] x7 : 8021ad587e10 x6 : 0001  

[44100.950446] x5 : 094d0ab0 x4 : 0001  

[44100.956274] x3 : 0003 x2 :   

[44100.962056] 

[Question] does there have another fix for ldisc crash instead of "tty: Fix ldisc crash on reopened tty" ?

[Tan Xiaojun]

Hi, all,

I test in a Arm64 fpga board (Hisilicon D06 fpga) with 
kernel-stable-4.14.2, then I get a panic log attached in the end.

I found this problem try to be fixed before by the patch "tty: Fix 
ldisc crash on reopened tty", and it was reverted
shortly thereafter because of conflicting with another patch and may 
not fix completely.

But this problem is still here. Are there any better fixed patch now ?


Thanks.
Xiaojun.
[44100.745418] Unable to handle kernel paging request at virtual address 
2260   
[44100.758109] Mem abort info:  

[44100.761956]   Exception class = DABT (current EL), IL = 32 bits  

[44100.768677]   SET = 0, FnV = 0   

[44100.772039]   EA = 0, S1PTW = 0  

[44100.775736] Data abort info: 

[44100.779766]   ISV = 0, ISS = 0x0004  

[44100.784425]   CM = 0, WnR = 0

[44100.788179] user pgtable: 4k pages, 48-bit VAs, pgd = 8021a8b4   

[44100.795815] [2260] *pgd= 

[44100.802390] Internal error: Oops: 9604 [#1] SMP  

[44100.808278] Modules linked in: rdma_test(OE) hns_roce_pci(OE) hns_roce(OE) 
debug_nic_mii(OE) rdma_ucm(OE) rdma_cm(OE) ib_cm(OE) i
w_cm(OE) ib_uverbs(OE) ib_core(OE) hns3_enet_ut(OE) hclge(OE) hnae3(OE) [last 
unloaded: rdma_test]  
[44100.833620] CPU: 0 PID: 4680 Comm: kworker/u2:0 Tainted: G   OE   
4.14.0 #1  
[44100.845099] Workqueue: events_unbound flush_to_ldisc 

[44100.850594] task: 8021f266c200 task.stack: 3bd28000  

[44100.857556] PC is at n_tty_receive_buf_common+0x68/0xa08 

[44100.863382] LR is at n_tty_receive_buf_common+0x54/0xa08 

[44100.869091] pc : [] lr : [] pstate: 
80c00149 
[44100.876787] sp : 3bd2bc40

[44100.880411] x29: 3bd2bc40 x28: 8021a3c47400  

[44100.886332] x27: 3c533cc8 x26: 08e0c000  

[44100.892170] x25:  x24: 8021f266c200  

[44100.897990] x23:  x22: 8021a3c46c20  

[44100.903841] x21: 8021a3c474c0 x20: 0003  

[44100.909654] x19:  x18: 0003  

[44100.915482] x17: a0e0dca8 x16: 00438170  

[44100.921318] x15: 3d27a000 x14: 3d09  

[44100.927167] x13: 61a8 x12: ac42  

[44100.932979] x11: 0001 x10: 0c80  

[44100.938788] x9 : 3bd2bd40 x8 : 8021f266cee0  

[44100.944592] x7 : 8021ad587e10 x6 : 0001  

[44100.950446] x5 : 094d0ab0 x4 : 0001  

[44100.956274] x3 : 0003 x2 :   

[44100.962056] 

Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type

[Tan Xiaojun]

On 2017/11/17 19:13, Sudeep Holla wrote:
> On Fri, Nov 17, 2017 at 10:13:39AM +0800, Tan Xiaojun wrote:
>> On 2017/11/16 23:23, Sudeep Holla wrote:
>>>
>>> I assume L3 is outer non-architected system cache.
>>>
>>
>> Yes.
>>
> 
> Good.
> 
>> OK. That's fine. I will test this.
> 
> Thanks.
> 
>> By the way, Arm64 tend to use acpi way to boot now. Is there some
>> suitable solution for acpi?
>>
> 
> Yes Jeremy is currently working on it[1] and IIRC Xiongfeng Wang from Huawei
> was testing them[2]. You need to talk to him ;)
> 
> --
> Regards,
> Sudeep
> 
> [1] https://marc.info/?l=linux-pm=151026155606677=2
> [2] https://marc.info/?l=linux-pm=151073816218307=2
> 
> 
> .
> 

OK. Thank you for telling me this.

Xiaojun.

Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type

[Tan Xiaojun]

On 2017/11/17 19:13, Sudeep Holla wrote:
> On Fri, Nov 17, 2017 at 10:13:39AM +0800, Tan Xiaojun wrote:
>> On 2017/11/16 23:23, Sudeep Holla wrote:
>>>
>>> I assume L3 is outer non-architected system cache.
>>>
>>
>> Yes.
>>
> 
> Good.
> 
>> OK. That's fine. I will test this.
> 
> Thanks.
> 
>> By the way, Arm64 tend to use acpi way to boot now. Is there some
>> suitable solution for acpi?
>>
> 
> Yes Jeremy is currently working on it[1] and IIRC Xiongfeng Wang from Huawei
> was testing them[2]. You need to talk to him ;)
> 
> --
> Regards,
> Sudeep
> 
> [1] https://marc.info/?l=linux-pm=151026155606677=2
> [2] https://marc.info/?l=linux-pm=151073816218307=2
> 
> 
> .
> 

OK. Thank you for telling me this.

Xiaojun.

Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type

[Tan Xiaojun]

On 2017/11/17 10:13, Tan Xiaojun wrote:
> On 2017/11/16 23:23, Sudeep Holla wrote:
>>
>>
>> On 16/11/17 12:58, Tan Xiaojun wrote:
>>> Since commit dfea747d2aba ("drivers: base: cacheinfo: support DT
>>> overrides for cache properties"), we can set the correct cacheinfo
>>> via DT. But the cache type can't be set in the same way.
>>>
>>> I found this may be a problem in recent tests. I tested L3 cache
>>> node setting in DT in Hisilicon D03/D05 board. And I got cacheinfo
>>> via sysfs below:
>>
>> I assume L3 is outer non-architected system cache.
>>
> 
> Yes.
> 
>>>
>>> $ cat /sys/devices/system/cpu/cpu*/cache/index3/
>>> allocation_policy  level  power/
>>> shared_cpu_map uevent write_policy
>>> coherency_line_sizenumber_of_sets shared_cpu_list
>>> size   ways_of_associativity
>>>
>>> This is incomplete, no type file to display type info. Because L3
>>> cache is uncore, we can't get correct type info from system
>>> register, and will get a default type "CACHE_TYPE_NOCACHE". Then
>>> use "lscpu" will print an error like below:
>>>
>>> $ lscpu
>>> lscpu: cannot open /sys/devices/system/cpu/cpu0/cache/index3/type:
>>> No such file or directory
>>>
>>> So I think maybe we can set correct cache type via DT too.
>>>
>>> Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
>>> ---
>>>  drivers/base/cacheinfo.c | 10 ++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
>>> index eb3af27..3e650dc 100644
>>> --- a/drivers/base/cacheinfo.c
>>> +++ b/drivers/base/cacheinfo.c
>>> @@ -122,6 +122,15 @@ static inline int get_cacheinfo_idx(enum cache_type 
>>> type)
>>> return type;
>>>  }
>>>  
>>> +static void cache_type(struct cacheinfo *this_leaf)
>>> +{
>>> +   const __be32 *cache_type;
>>> +
>>> +   cache_type = of_get_property(this_leaf->of_node, "type", NULL);
>>
>> NACK for this:
>>
>> 1. We don't have any DT binding for the "type"
>> 2. Even if we had, we will never have such a binding that magical
>>returns the enum used in Linux implementation. That's not how DT
>>bindings are designed.
>>
>> Please refer ePAPR or Devicetree Specification Release 0.1 from
>> devicetree.org, we have cache-unified boolean for type.
>>
>> Let me know if the below patch works for you, I will submit it
>> preferably with your tested-by.
>>
>> Regards,
>> Sudeep
>>
> 
> OK. That's fine. I will test this.
> By the way, Arm64 tend to use acpi way to boot now. Is there some
> suitable solution for acpi?
> 
> Thanks.
> Xiaojun.
> 

Test passed, this is indeed a better solution.

Thanks.
Xiaojun.

>> -->8
>>
>> diff --git i/drivers/base/cacheinfo.c w/drivers/base/cacheinfo.c
>> index eb3af2739537..07532d83be0b 100644
>> --- i/drivers/base/cacheinfo.c
>> +++ w/drivers/base/cacheinfo.c
>> @@ -186,6 +186,11 @@ static void cache_associativity(struct cacheinfo
>> *this_leaf)
>> this_leaf->ways_of_associativity = (size / nr_sets) /
>> line_size;
>>  }
>>
>> +static bool cache_node_is_unified(struct cacheinfo *this_leaf)
>> +{
>> +   return of_property_read_bool(this_leaf->of_node, "cache-unified");
>> +}
>> +
>>  static void cache_of_override_properties(unsigned int cpu)
>>  {
>> int index;
>> @@ -194,6 +199,14 @@ static void cache_of_override_properties(unsigned
>> int cpu)
>>
>> for (index = 0; index < cache_leaves(cpu); index++) {
>> this_leaf = this_cpu_ci->info_list + index;
>> +   /*
>> +* init_cache_level must setup the cache level correctly
>> +* overriding the architecturally specified levels, so
>> +* if type is NONE at this stage, it should be unified
>> +*/
>> +   if (this_leaf->type == CACHE_TYPE_NOCACHE &&
>> +   cache_node_is_unified(this_leaf))
>> +   this_leaf->type = CACHE_TYPE_UNIFIED;
>> cache_size(this_leaf);
>> cache_get_line_size(this_leaf);
>> cache_nr_sets(this_leaf);
>>
>>
>> .
>>
> 
> 
> 
> .
> 

Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type

[Tan Xiaojun]

On 2017/11/17 10:13, Tan Xiaojun wrote:
> On 2017/11/16 23:23, Sudeep Holla wrote:
>>
>>
>> On 16/11/17 12:58, Tan Xiaojun wrote:
>>> Since commit dfea747d2aba ("drivers: base: cacheinfo: support DT
>>> overrides for cache properties"), we can set the correct cacheinfo
>>> via DT. But the cache type can't be set in the same way.
>>>
>>> I found this may be a problem in recent tests. I tested L3 cache
>>> node setting in DT in Hisilicon D03/D05 board. And I got cacheinfo
>>> via sysfs below:
>>
>> I assume L3 is outer non-architected system cache.
>>
> 
> Yes.
> 
>>>
>>> $ cat /sys/devices/system/cpu/cpu*/cache/index3/
>>> allocation_policy  level  power/
>>> shared_cpu_map uevent write_policy
>>> coherency_line_sizenumber_of_sets shared_cpu_list
>>> size   ways_of_associativity
>>>
>>> This is incomplete, no type file to display type info. Because L3
>>> cache is uncore, we can't get correct type info from system
>>> register, and will get a default type "CACHE_TYPE_NOCACHE". Then
>>> use "lscpu" will print an error like below:
>>>
>>> $ lscpu
>>> lscpu: cannot open /sys/devices/system/cpu/cpu0/cache/index3/type:
>>> No such file or directory
>>>
>>> So I think maybe we can set correct cache type via DT too.
>>>
>>> Signed-off-by: Tan Xiaojun 
>>> ---
>>>  drivers/base/cacheinfo.c | 10 ++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
>>> index eb3af27..3e650dc 100644
>>> --- a/drivers/base/cacheinfo.c
>>> +++ b/drivers/base/cacheinfo.c
>>> @@ -122,6 +122,15 @@ static inline int get_cacheinfo_idx(enum cache_type 
>>> type)
>>> return type;
>>>  }
>>>  
>>> +static void cache_type(struct cacheinfo *this_leaf)
>>> +{
>>> +   const __be32 *cache_type;
>>> +
>>> +   cache_type = of_get_property(this_leaf->of_node, "type", NULL);
>>
>> NACK for this:
>>
>> 1. We don't have any DT binding for the "type"
>> 2. Even if we had, we will never have such a binding that magical
>>returns the enum used in Linux implementation. That's not how DT
>>bindings are designed.
>>
>> Please refer ePAPR or Devicetree Specification Release 0.1 from
>> devicetree.org, we have cache-unified boolean for type.
>>
>> Let me know if the below patch works for you, I will submit it
>> preferably with your tested-by.
>>
>> Regards,
>> Sudeep
>>
> 
> OK. That's fine. I will test this.
> By the way, Arm64 tend to use acpi way to boot now. Is there some
> suitable solution for acpi?
> 
> Thanks.
> Xiaojun.
> 

Test passed, this is indeed a better solution.

Thanks.
Xiaojun.

>> -->8
>>
>> diff --git i/drivers/base/cacheinfo.c w/drivers/base/cacheinfo.c
>> index eb3af2739537..07532d83be0b 100644
>> --- i/drivers/base/cacheinfo.c
>> +++ w/drivers/base/cacheinfo.c
>> @@ -186,6 +186,11 @@ static void cache_associativity(struct cacheinfo
>> *this_leaf)
>> this_leaf->ways_of_associativity = (size / nr_sets) /
>> line_size;
>>  }
>>
>> +static bool cache_node_is_unified(struct cacheinfo *this_leaf)
>> +{
>> +   return of_property_read_bool(this_leaf->of_node, "cache-unified");
>> +}
>> +
>>  static void cache_of_override_properties(unsigned int cpu)
>>  {
>> int index;
>> @@ -194,6 +199,14 @@ static void cache_of_override_properties(unsigned
>> int cpu)
>>
>> for (index = 0; index < cache_leaves(cpu); index++) {
>> this_leaf = this_cpu_ci->info_list + index;
>> +   /*
>> +* init_cache_level must setup the cache level correctly
>> +* overriding the architecturally specified levels, so
>> +* if type is NONE at this stage, it should be unified
>> +*/
>> +   if (this_leaf->type == CACHE_TYPE_NOCACHE &&
>> +   cache_node_is_unified(this_leaf))
>> +   this_leaf->type = CACHE_TYPE_UNIFIED;
>> cache_size(this_leaf);
>> cache_get_line_size(this_leaf);
>> cache_nr_sets(this_leaf);
>>
>>
>> .
>>
> 
> 
> 
> .
> 

Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type

[Tan Xiaojun]

On 2017/11/16 23:23, Sudeep Holla wrote:
> 
> 
> On 16/11/17 12:58, Tan Xiaojun wrote:
>> Since commit dfea747d2aba ("drivers: base: cacheinfo: support DT
>> overrides for cache properties"), we can set the correct cacheinfo
>> via DT. But the cache type can't be set in the same way.
>>
>> I found this may be a problem in recent tests. I tested L3 cache
>> node setting in DT in Hisilicon D03/D05 board. And I got cacheinfo
>> via sysfs below:
> 
> I assume L3 is outer non-architected system cache.
> 

Yes.

>>
>> $ cat /sys/devices/system/cpu/cpu*/cache/index3/
>> allocation_policy  level  power/
>> shared_cpu_map uevent write_policy
>> coherency_line_sizenumber_of_sets shared_cpu_list
>> size   ways_of_associativity
>>
>> This is incomplete, no type file to display type info. Because L3
>> cache is uncore, we can't get correct type info from system
>> register, and will get a default type "CACHE_TYPE_NOCACHE". Then
>> use "lscpu" will print an error like below:
>>
>> $ lscpu
>> lscpu: cannot open /sys/devices/system/cpu/cpu0/cache/index3/type:
>> No such file or directory
>>
>> So I think maybe we can set correct cache type via DT too.
>>
>> Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
>> ---
>>  drivers/base/cacheinfo.c | 10 ++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
>> index eb3af27..3e650dc 100644
>> --- a/drivers/base/cacheinfo.c
>> +++ b/drivers/base/cacheinfo.c
>> @@ -122,6 +122,15 @@ static inline int get_cacheinfo_idx(enum cache_type 
>> type)
>>  return type;
>>  }
>>  
>> +static void cache_type(struct cacheinfo *this_leaf)
>> +{
>> +const __be32 *cache_type;
>> +
>> +cache_type = of_get_property(this_leaf->of_node, "type", NULL);
> 
> NACK for this:
> 
> 1. We don't have any DT binding for the "type"
> 2. Even if we had, we will never have such a binding that magical
>returns the enum used in Linux implementation. That's not how DT
>bindings are designed.
> 
> Please refer ePAPR or Devicetree Specification Release 0.1 from
> devicetree.org, we have cache-unified boolean for type.
> 
> Let me know if the below patch works for you, I will submit it
> preferably with your tested-by.
> 
> Regards,
> Sudeep
> 

OK. That's fine. I will test this.
By the way, Arm64 tend to use acpi way to boot now. Is there some
suitable solution for acpi?

Thanks.
Xiaojun.

> -->8
> 
> diff --git i/drivers/base/cacheinfo.c w/drivers/base/cacheinfo.c
> index eb3af2739537..07532d83be0b 100644
> --- i/drivers/base/cacheinfo.c
> +++ w/drivers/base/cacheinfo.c
> @@ -186,6 +186,11 @@ static void cache_associativity(struct cacheinfo
> *this_leaf)
> this_leaf->ways_of_associativity = (size / nr_sets) /
> line_size;
>  }
> 
> +static bool cache_node_is_unified(struct cacheinfo *this_leaf)
> +{
> +   return of_property_read_bool(this_leaf->of_node, "cache-unified");
> +}
> +
>  static void cache_of_override_properties(unsigned int cpu)
>  {
> int index;
> @@ -194,6 +199,14 @@ static void cache_of_override_properties(unsigned
> int cpu)
> 
> for (index = 0; index < cache_leaves(cpu); index++) {
> this_leaf = this_cpu_ci->info_list + index;
> +   /*
> +* init_cache_level must setup the cache level correctly
> +* overriding the architecturally specified levels, so
> +* if type is NONE at this stage, it should be unified
> +*/
> +   if (this_leaf->type == CACHE_TYPE_NOCACHE &&
> +   cache_node_is_unified(this_leaf))
> +   this_leaf->type = CACHE_TYPE_UNIFIED;
> cache_size(this_leaf);
> cache_get_line_size(this_leaf);
> cache_nr_sets(this_leaf);
> 
> 
> .
> 

Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type

[Tan Xiaojun]

On 2017/11/16 23:23, Sudeep Holla wrote:
> 
> 
> On 16/11/17 12:58, Tan Xiaojun wrote:
>> Since commit dfea747d2aba ("drivers: base: cacheinfo: support DT
>> overrides for cache properties"), we can set the correct cacheinfo
>> via DT. But the cache type can't be set in the same way.
>>
>> I found this may be a problem in recent tests. I tested L3 cache
>> node setting in DT in Hisilicon D03/D05 board. And I got cacheinfo
>> via sysfs below:
> 
> I assume L3 is outer non-architected system cache.
> 

Yes.

>>
>> $ cat /sys/devices/system/cpu/cpu*/cache/index3/
>> allocation_policy  level  power/
>> shared_cpu_map uevent write_policy
>> coherency_line_sizenumber_of_sets shared_cpu_list
>> size   ways_of_associativity
>>
>> This is incomplete, no type file to display type info. Because L3
>> cache is uncore, we can't get correct type info from system
>> register, and will get a default type "CACHE_TYPE_NOCACHE". Then
>> use "lscpu" will print an error like below:
>>
>> $ lscpu
>> lscpu: cannot open /sys/devices/system/cpu/cpu0/cache/index3/type:
>> No such file or directory
>>
>> So I think maybe we can set correct cache type via DT too.
>>
>> Signed-off-by: Tan Xiaojun 
>> ---
>>  drivers/base/cacheinfo.c | 10 ++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
>> index eb3af27..3e650dc 100644
>> --- a/drivers/base/cacheinfo.c
>> +++ b/drivers/base/cacheinfo.c
>> @@ -122,6 +122,15 @@ static inline int get_cacheinfo_idx(enum cache_type 
>> type)
>>  return type;
>>  }
>>  
>> +static void cache_type(struct cacheinfo *this_leaf)
>> +{
>> +const __be32 *cache_type;
>> +
>> +cache_type = of_get_property(this_leaf->of_node, "type", NULL);
> 
> NACK for this:
> 
> 1. We don't have any DT binding for the "type"
> 2. Even if we had, we will never have such a binding that magical
>returns the enum used in Linux implementation. That's not how DT
>bindings are designed.
> 
> Please refer ePAPR or Devicetree Specification Release 0.1 from
> devicetree.org, we have cache-unified boolean for type.
> 
> Let me know if the below patch works for you, I will submit it
> preferably with your tested-by.
> 
> Regards,
> Sudeep
> 

OK. That's fine. I will test this.
By the way, Arm64 tend to use acpi way to boot now. Is there some
suitable solution for acpi?

Thanks.
Xiaojun.

> -->8
> 
> diff --git i/drivers/base/cacheinfo.c w/drivers/base/cacheinfo.c
> index eb3af2739537..07532d83be0b 100644
> --- i/drivers/base/cacheinfo.c
> +++ w/drivers/base/cacheinfo.c
> @@ -186,6 +186,11 @@ static void cache_associativity(struct cacheinfo
> *this_leaf)
> this_leaf->ways_of_associativity = (size / nr_sets) /
> line_size;
>  }
> 
> +static bool cache_node_is_unified(struct cacheinfo *this_leaf)
> +{
> +   return of_property_read_bool(this_leaf->of_node, "cache-unified");
> +}
> +
>  static void cache_of_override_properties(unsigned int cpu)
>  {
> int index;
> @@ -194,6 +199,14 @@ static void cache_of_override_properties(unsigned
> int cpu)
> 
> for (index = 0; index < cache_leaves(cpu); index++) {
> this_leaf = this_cpu_ci->info_list + index;
> +   /*
> +* init_cache_level must setup the cache level correctly
> +* overriding the architecturally specified levels, so
> +* if type is NONE at this stage, it should be unified
> +*/
> +   if (this_leaf->type == CACHE_TYPE_NOCACHE &&
> +   cache_node_is_unified(this_leaf))
> +   this_leaf->type = CACHE_TYPE_UNIFIED;
> cache_size(this_leaf);
> cache_get_line_size(this_leaf);
> cache_nr_sets(this_leaf);
> 
> 
> .
> 

[PATCH] drivers: base: cacheinfo: support DT overrides for cache type

[Tan Xiaojun]

Since commit dfea747d2aba ("drivers: base: cacheinfo: support DT
overrides for cache properties"), we can set the correct cacheinfo
via DT. But the cache type can't be set in the same way.

I found this may be a problem in recent tests. I tested L3 cache
node setting in DT in Hisilicon D03/D05 board. And I got cacheinfo
via sysfs below:

$ cat /sys/devices/system/cpu/cpu*/cache/index3/
allocation_policy  level  power/
shared_cpu_map uevent write_policy
coherency_line_sizenumber_of_sets shared_cpu_list
size   ways_of_associativity

This is incomplete, no type file to display type info. Because L3
cache is uncore, we can't get correct type info from system
register, and will get a default type "CACHE_TYPE_NOCACHE". Then
use "lscpu" will print an error like below:

$ lscpu
lscpu: cannot open /sys/devices/system/cpu/cpu0/cache/index3/type:
No such file or directory

So I think maybe we can set correct cache type via DT too.

Signed-off-by: Tan Xiaojun <tanxiao...@huawei.com>
---
 drivers/base/cacheinfo.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index eb3af27..3e650dc 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -122,6 +122,15 @@ static inline int get_cacheinfo_idx(enum cache_type type)
return type;
 }
 
+static void cache_type(struct cacheinfo *this_leaf)
+{
+   const __be32 *cache_type;
+
+   cache_type = of_get_property(this_leaf->of_node, "type", NULL);
+   if (cache_type)
+   this_leaf->type = of_read_number(cache_type, 1);
+}
+
 static void cache_size(struct cacheinfo *this_leaf)
 {
const char *propname;
@@ -194,6 +203,7 @@ static void cache_of_override_properties(unsigned int cpu)
 
for (index = 0; index < cache_leaves(cpu); index++) {
this_leaf = this_cpu_ci->info_list + index;
+   cache_type(this_leaf);
cache_size(this_leaf);
cache_get_line_size(this_leaf);
cache_nr_sets(this_leaf);
-- 
2.7.4

[PATCH] drivers: base: cacheinfo: support DT overrides for cache type

[Tan Xiaojun]

Since commit dfea747d2aba ("drivers: base: cacheinfo: support DT
overrides for cache properties"), we can set the correct cacheinfo
via DT. But the cache type can't be set in the same way.

I found this may be a problem in recent tests. I tested L3 cache
node setting in DT in Hisilicon D03/D05 board. And I got cacheinfo
via sysfs below:

$ cat /sys/devices/system/cpu/cpu*/cache/index3/
allocation_policy  level  power/
shared_cpu_map uevent write_policy
coherency_line_sizenumber_of_sets shared_cpu_list
size   ways_of_associativity

This is incomplete, no type file to display type info. Because L3
cache is uncore, we can't get correct type info from system
register, and will get a default type "CACHE_TYPE_NOCACHE". Then
use "lscpu" will print an error like below:

$ lscpu
lscpu: cannot open /sys/devices/system/cpu/cpu0/cache/index3/type:
No such file or directory

So I think maybe we can set correct cache type via DT too.

Signed-off-by: Tan Xiaojun 
---
 drivers/base/cacheinfo.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index eb3af27..3e650dc 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -122,6 +122,15 @@ static inline int get_cacheinfo_idx(enum cache_type type)
return type;
 }
 
+static void cache_type(struct cacheinfo *this_leaf)
+{
+   const __be32 *cache_type;
+
+   cache_type = of_get_property(this_leaf->of_node, "type", NULL);
+   if (cache_type)
+   this_leaf->type = of_read_number(cache_type, 1);
+}
+
 static void cache_size(struct cacheinfo *this_leaf)
 {
const char *propname;
@@ -194,6 +203,7 @@ static void cache_of_override_properties(unsigned int cpu)
 
for (index = 0; index < cache_leaves(cpu); index++) {
this_leaf = this_cpu_ci->info_list + index;
+   cache_type(this_leaf);
cache_size(this_leaf);
cache_get_line_size(this_leaf);
cache_nr_sets(this_leaf);
-- 
2.7.4

Re: [bug report & help] arm64: ltp testcase "migrate_pages01" failed

[Tan Xiaojun]

On 2017/10/17 17:23, Will Deacon wrote:
> On Tue, Oct 17, 2017 at 10:58:53AM +0800, Tan Xiaojun wrote:
>> I'm not sure if this is the problem on arm64 numa. What do you think ?
>> By the way, this testcase can be successful in any case on x86.
> 
> To be honest, this isn't a particularly helpful bug report. I appreciate
> that a test is reporting failure, but it doesn't look like you've spent
> very much effort to understand what the test is trying to do and why it
> thinks it's failed to do it. All I can sensibly do with your bug report
> is run the test myself, and it passes on the systems I have available.
> 
> So, you need to:
> 
> 1. Understand what the test is doing.
> 2. Figure out which bit isn't doing what it's supposed to
> 3. See if that part can be isolated to trigger the problem
> 
> At that point, it should be possible to describe the unexpected behaviour
> at a level which we can actually investigate if necessary.
> 
> Will
> 

OK. I will do these things. And can you tell me the model of your test board?
If possible, run "numactl -H" to make sure all nodes have memory.

Thanks.
Xiaojun.

>> On 2017/10/16 19:42, Tan Xiaojun wrote:
>>> Hi all,
>>>
>>> I test ltp in Hisilicon D05 board and get a failed result about the 
>>> testcase "migrate_pages01".
>>>
>>> In fact, The sub testcase "test_invalid_nodes" failed. The testcase is to 
>>> find a invalid numa node and migrate memory pages to this node via syscall 
>>> of "migrate_pages".
>>> The expected result of this case is returning "-1", but it actually return 
>>> "0".
>>>
>>> 
>>> # ./migrate_pages01
>>> migrate_pages010  TINFO  :  test_empty_mask
>>> migrate_pages011  TPASS  :  expected ret success: returned value = 0
>>> migrate_pages010  TINFO  :  test_invalid_pid -1
>>> migrate_pages012  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages013  TPASS  :  expected failure: TEST_ERRNO=ESRCH(3): No 
>>> such process
>>> migrate_pages010  TINFO  :  test_invalid_pid unused pid
>>> migrate_pages014  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages015  TPASS  :  expected failure: TEST_ERRNO=ESRCH(3): No 
>>> such process
>>> migrate_pages010  TINFO  :  test_invalid_masksize
>>> migrate_pages016  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages017  TPASS  :  expected failure: TEST_ERRNO=EINVAL(22): 
>>> Invalid argument
>>> migrate_pages010  TINFO  :  test_invalid_mem -1
>>> migrate_pages018  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages019  TPASS  :  expected failure: TEST_ERRNO=EFAULT(14): 
>>> Bad address
>>> migrate_pages010  TINFO  :  test_invalid_mem invalid prot
>>> migrate_pages01   10  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages01   11  TPASS  :  expected failure: TEST_ERRNO=EFAULT(14): 
>>> Bad address
>>> migrate_pages010  TINFO  :  test_invalid_mem unmmaped
>>> migrate_pages01   12  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages01   13  TPASS  :  expected failure: TEST_ERRNO=EFAULT(14): 
>>> Bad address
>>> migrate_pages010  TINFO  :  test_invalid_nodes
>>> migrate_pages01   14  TFAIL  :  migrate_pages_common.c:45: unexpected 
>>> failure - returned value = 0, expected: -1
>>> migrate_pages01   15  TFAIL  :  migrate_pages_common.c:55: call succeeded 
>>> unexpectedly
>>> migrate_pages010  TINFO  :  test_invalid_perm
>>> migrate_pages01   16  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages01   17  TPASS  :  expected failure: TEST_ERRNO=EPERM(1): 
>>> Operation not permitted
>>> 
>>>
>>> I debug and find a interesting thing, this case does not always fail.
>>>
>>> 1) If one or several numa nodes have no memory, this case will run 
>>> successfully like below:
>>>
>>> 
>>> available: 4 nodes (0-3)
>>> node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
>>> node 0 size: 65309 MB
>>> node 0 free: 61650 MB
>>> node 1 cpus: 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
>>> node 1 size: 65404 MB
>>> node 1 free: 61377 MB
>>> node 2 cpus: 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
&g

Re: [bug report & help] arm64: ltp testcase "migrate_pages01" failed

[Tan Xiaojun]

On 2017/10/17 17:23, Will Deacon wrote:
> On Tue, Oct 17, 2017 at 10:58:53AM +0800, Tan Xiaojun wrote:
>> I'm not sure if this is the problem on arm64 numa. What do you think ?
>> By the way, this testcase can be successful in any case on x86.
> 
> To be honest, this isn't a particularly helpful bug report. I appreciate
> that a test is reporting failure, but it doesn't look like you've spent
> very much effort to understand what the test is trying to do and why it
> thinks it's failed to do it. All I can sensibly do with your bug report
> is run the test myself, and it passes on the systems I have available.
> 
> So, you need to:
> 
> 1. Understand what the test is doing.
> 2. Figure out which bit isn't doing what it's supposed to
> 3. See if that part can be isolated to trigger the problem
> 
> At that point, it should be possible to describe the unexpected behaviour
> at a level which we can actually investigate if necessary.
> 
> Will
> 

OK. I will do these things. And can you tell me the model of your test board?
If possible, run "numactl -H" to make sure all nodes have memory.

Thanks.
Xiaojun.

>> On 2017/10/16 19:42, Tan Xiaojun wrote:
>>> Hi all,
>>>
>>> I test ltp in Hisilicon D05 board and get a failed result about the 
>>> testcase "migrate_pages01".
>>>
>>> In fact, The sub testcase "test_invalid_nodes" failed. The testcase is to 
>>> find a invalid numa node and migrate memory pages to this node via syscall 
>>> of "migrate_pages".
>>> The expected result of this case is returning "-1", but it actually return 
>>> "0".
>>>
>>> 
>>> # ./migrate_pages01
>>> migrate_pages010  TINFO  :  test_empty_mask
>>> migrate_pages011  TPASS  :  expected ret success: returned value = 0
>>> migrate_pages010  TINFO  :  test_invalid_pid -1
>>> migrate_pages012  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages013  TPASS  :  expected failure: TEST_ERRNO=ESRCH(3): No 
>>> such process
>>> migrate_pages010  TINFO  :  test_invalid_pid unused pid
>>> migrate_pages014  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages015  TPASS  :  expected failure: TEST_ERRNO=ESRCH(3): No 
>>> such process
>>> migrate_pages010  TINFO  :  test_invalid_masksize
>>> migrate_pages016  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages017  TPASS  :  expected failure: TEST_ERRNO=EINVAL(22): 
>>> Invalid argument
>>> migrate_pages010  TINFO  :  test_invalid_mem -1
>>> migrate_pages018  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages019  TPASS  :  expected failure: TEST_ERRNO=EFAULT(14): 
>>> Bad address
>>> migrate_pages010  TINFO  :  test_invalid_mem invalid prot
>>> migrate_pages01   10  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages01   11  TPASS  :  expected failure: TEST_ERRNO=EFAULT(14): 
>>> Bad address
>>> migrate_pages010  TINFO  :  test_invalid_mem unmmaped
>>> migrate_pages01   12  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages01   13  TPASS  :  expected failure: TEST_ERRNO=EFAULT(14): 
>>> Bad address
>>> migrate_pages010  TINFO  :  test_invalid_nodes
>>> migrate_pages01   14  TFAIL  :  migrate_pages_common.c:45: unexpected 
>>> failure - returned value = 0, expected: -1
>>> migrate_pages01   15  TFAIL  :  migrate_pages_common.c:55: call succeeded 
>>> unexpectedly
>>> migrate_pages010  TINFO  :  test_invalid_perm
>>> migrate_pages01   16  TPASS  :  expected ret success: returned value = -1
>>> migrate_pages01   17  TPASS  :  expected failure: TEST_ERRNO=EPERM(1): 
>>> Operation not permitted
>>> 
>>>
>>> I debug and find a interesting thing, this case does not always fail.
>>>
>>> 1) If one or several numa nodes have no memory, this case will run 
>>> successfully like below:
>>>
>>> 
>>> available: 4 nodes (0-3)
>>> node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
>>> node 0 size: 65309 MB
>>> node 0 free: 61650 MB
>>> node 1 cpus: 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
>>> node 1 size: 65404 MB
>>> node 1 free: 61377 MB
>>> node 2 cpus: 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
&g

Re: [bug report & help] arm64: ltp testcase "migrate_pages01" failed

[Tan Xiaojun]

Hi all,

I'm not sure if this is the problem on arm64 numa. What do you think ?
By the way, this testcase can be successful in any case on x86.

Thanks.
Xiaojun.

On 2017/10/16 19:42, Tan Xiaojun wrote:
> Hi all,
> 
> I test ltp in Hisilicon D05 board and get a failed result about the testcase 
> "migrate_pages01".
> 
> In fact, The sub testcase "test_invalid_nodes" failed. The testcase is to 
> find a invalid numa node and migrate memory pages to this node via syscall of 
> "migrate_pages".
> The expected result of this case is returning "-1", but it actually return 
> "0".
> 
> 
> # ./migrate_pages01
> migrate_pages010  TINFO  :  test_empty_mask
> migrate_pages011  TPASS  :  expected ret success: returned value = 0
> migrate_pages010  TINFO  :  test_invalid_pid -1
> migrate_pages012  TPASS  :  expected ret success: returned value = -1
> migrate_pages013  TPASS  :  expected failure: TEST_ERRNO=ESRCH(3): No 
> such process
> migrate_pages010  TINFO  :  test_invalid_pid unused pid
> migrate_pages014  TPASS  :  expected ret success: returned value = -1
> migrate_pages015  TPASS  :  expected failure: TEST_ERRNO=ESRCH(3): No 
> such process
> migrate_pages010  TINFO  :  test_invalid_masksize
> migrate_pages016  TPASS  :  expected ret success: returned value = -1
> migrate_pages017  TPASS  :  expected failure: TEST_ERRNO=EINVAL(22): 
> Invalid argument
> migrate_pages010  TINFO  :  test_invalid_mem -1
> migrate_pages018  TPASS  :  expected ret success: returned value = -1
> migrate_pages019  TPASS  :  expected failure: TEST_ERRNO=EFAULT(14): Bad 
> address
> migrate_pages010  TINFO  :  test_invalid_mem invalid prot
> migrate_pages01   10  TPASS  :  expected ret success: returned value = -1
> migrate_pages01   11  TPASS  :  expected failure: TEST_ERRNO=EFAULT(14): Bad 
> address
> migrate_pages010  TINFO  :  test_invalid_mem unmmaped
> migrate_pages01   12  TPASS  :  expected ret success: returned value = -1
> migrate_pages01   13  TPASS  :  expected failure: TEST_ERRNO=EFAULT(14): Bad 
> address
> migrate_pages010  TINFO  :  test_invalid_nodes
> migrate_pages01   14  TFAIL  :  migrate_pages_common.c:45: unexpected failure 
> - returned value = 0, expected: -1
> migrate_pages01   15  TFAIL  :  migrate_pages_common.c:55: call succeeded 
> unexpectedly
> migrate_pages010  TINFO  :  test_invalid_perm
> migrate_pages01   16  TPASS  :  expected ret success: returned value = -1
> migrate_pages01   17  TPASS  :  expected failure: TEST_ERRNO=EPERM(1): 
> Operation not permitted
> 
> 
> I debug and find a interesting thing, this case does not always fail.
> 
> 1) If one or several numa nodes have no memory, this case will run 
> successfully like below:
> 
> 
> available: 4 nodes (0-3)
> node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
> node 0 size: 65309 MB
> node 0 free: 61650 MB
> node 1 cpus: 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
> node 1 size: 65404 MB
> node 1 free: 61377 MB
> node 2 cpus: 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
> node 2 size: 65401 MB
> node 2 free: 62316 MB
> node 3 cpus: 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63
> node 3 size: 0 MB
> node 3 free: 0 MB
> node distances:
> node   0   1   2   3
>   0:  10  15  20  20
>   1:  15  10  20  20
>   2:  20  20  10  15
>   3:  20  20  15  10
> -
> 
> This testcase will find node number 3 and migrate pages to node 3. And 
> syscall of "migrate_pages" return -1, test succeeded.
> 
> 2) In most cases, all nodes have memory, and the testcase will get 
> non-existent node like node number 4. The syscall of "migrate_pages" should 
> also return -1, but return 0 actually.
> So the testcase failed.
> 
> I think it is a problem in arm64. But I am not familiar with numa, so I ask 
> for help from you.
> 
> Thanks.
> Xiaojun.
> 
> 
> .
>