[PATCH v2] MIPS/bpf: Enable bpf_probe_read{, str}() on MIPS again

[PATCH v2] MIPS/bpf: Enable bpf_probe_read{, str}() on MIPS again

[Tiezhu Yang]

After commit 0ebeea8ca8a4 ("bpf: Restrict bpf_probe_read{, str}() only to
archs where they work"), bpf_probe_read{, str}() functions were no longer
available on MIPS, so there exist some errors when running bpf program:

root@linux:/home/loongson/bcc# python examples/tracing/task_switch.py
bpf: Failed to load program: Invalid argument
[...]
11: (85) call bpf_probe_read#4
unknown func bpf_probe_read#4
[...]
Exception: Failed to load BPF program count_sched: Invalid argument

So select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE in arch/mips/Kconfig,
otherwise the bpf old helper bpf_probe_read() will not be available.

This is similar with the commit d195b1d1d119 ("powerpc/bpf: Enable
bpf_probe_read{, str}() on powerpc again").

Fixes: 0ebeea8ca8a4 ("bpf: Restrict bpf_probe_read{, str}() only to archs where they work")
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---

v2: update the commit message to fix typos found by
    Sergei Shtylyov, thank you!

    not longer --> no longer
    there exists --> there exist

 arch/mips/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 160b3a8..4b94ec7 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -6,6 +6,7 @@ config MIPS
 	select ARCH_BINFMT_ELF_STATE if MIPS_FP_SUPPORT
 	select ARCH_HAS_FORTIFY_SOURCE
 	select ARCH_HAS_KCOV
+	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
 	select ARCH_HAS_PTE_SPECIAL if !(32BIT && CPU_HAS_RIXI)
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
-- 
2.1.0

Re: [PATCH v2] MIPS/bpf: Enable bpf_probe_read{, str}() on MIPS again

[Maciej W. Rozycki]

On Thu, 18 Mar 2021, Tiezhu Yang wrote:

> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 160b3a8..4b94ec7 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -6,6 +6,7 @@ config MIPS
>  	select ARCH_BINFMT_ELF_STATE if MIPS_FP_SUPPORT
>  	select ARCH_HAS_FORTIFY_SOURCE
>  	select ARCH_HAS_KCOV
> +	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE

 Hmm, documentation on ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE seems rather 
scarce, but based on my guess shouldn't this be "if !EVA"?

  Maciej

Re: [PATCH v2] MIPS/bpf: Enable bpf_probe_read{, str}() on MIPS again

[Christoph Hellwig]

On Mon, Mar 22, 2021 at 05:46:56AM +0100, Maciej W. Rozycki wrote:
> On Thu, 18 Mar 2021, Tiezhu Yang wrote:
> 
> > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > index 160b3a8..4b94ec7 100644
> > --- a/arch/mips/Kconfig
> > +++ b/arch/mips/Kconfig
> > @@ -6,6 +6,7 @@ config MIPS
> >  	select ARCH_BINFMT_ELF_STATE if MIPS_FP_SUPPORT
> >  	select ARCH_HAS_FORTIFY_SOURCE
> >  	select ARCH_HAS_KCOV
> > +	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
> 
>  Hmm, documentation on ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE seems rather 
> scarce, but based on my guess shouldn't this be "if !EVA"?

ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE means the any given virtual
address in a task context can either be a valid kernel or user address
space, but not both.

Note that the bpf probe really is a legacy use case and should not be
used in new code independent of that.

Re: [PATCH v2] MIPS/bpf: Enable bpf_probe_read{, str}() on MIPS again

[Tiezhu Yang]

On 03/22/2021 12:46 PM, Maciej W. Rozycki wrote:
> On Thu, 18 Mar 2021, Tiezhu Yang wrote:
>
>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>> index 160b3a8..4b94ec7 100644
>> --- a/arch/mips/Kconfig
>> +++ b/arch/mips/Kconfig
>> @@ -6,6 +6,7 @@ config MIPS
>>   	select ARCH_BINFMT_ELF_STATE if MIPS_FP_SUPPORT
>>   	select ARCH_HAS_FORTIFY_SOURCE
>>   	select ARCH_HAS_KCOV
>> +	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>   Hmm, documentation on ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE seems rather
> scarce, but based on my guess shouldn't this be "if !EVA"?
>
>    Maciej

I do not quite know what the effect if MIPS EVA (Enhanced Virtual 
Addressing)
is set, I saw that ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE should be 
restricted
to archs with non-overlapping address ranges.

I wonder whether MIPS EVA will generate overlapping address ranges?
If yes, it is better to make ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE depend
on !EVA on MIPS.

Thanks,
Tiezhu

Re: [PATCH v2] MIPS/bpf: Enable bpf_probe_read{, str}() on MIPS again

[Maciej W. Rozycki]

On Mon, 22 Mar 2021, Tiezhu Yang wrote:

> > > --- a/arch/mips/Kconfig
> > > +++ b/arch/mips/Kconfig
> > > @@ -6,6 +6,7 @@ config MIPS
> > >   	select ARCH_BINFMT_ELF_STATE if MIPS_FP_SUPPORT
> > >   	select ARCH_HAS_FORTIFY_SOURCE
> > >   	select ARCH_HAS_KCOV
> > > +	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
> >   Hmm, documentation on ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE seems rather
> > scarce, but based on my guess shouldn't this be "if !EVA"?
> 
> I do not quite know what the effect if MIPS EVA (Enhanced Virtual Addressing)
> is set, I saw that ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE should be restricted
> to archs with non-overlapping address ranges.
> 
> I wonder whether MIPS EVA will generate overlapping address ranges?

 The architecture specification has it all.

  Maciej

Re: [PATCH v2] MIPS/bpf: Enable bpf_probe_read{, str}() on MIPS again

[Thomas Bogendoerfer]

On Mon, Mar 22, 2021 at 03:12:59PM +0800, Tiezhu Yang wrote:
> On 03/22/2021 12:46 PM, Maciej W. Rozycki wrote:
> > On Thu, 18 Mar 2021, Tiezhu Yang wrote:
> > 
> > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> > > index 160b3a8..4b94ec7 100644
> > > --- a/arch/mips/Kconfig
> > > +++ b/arch/mips/Kconfig
> > > @@ -6,6 +6,7 @@ config MIPS
> > >   	select ARCH_BINFMT_ELF_STATE if MIPS_FP_SUPPORT
> > >   	select ARCH_HAS_FORTIFY_SOURCE
> > >   	select ARCH_HAS_KCOV
> > > +	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
> >   Hmm, documentation on ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE seems rather
> > scarce, but based on my guess shouldn't this be "if !EVA"?
> > 
> >    Maciej
> 
> I do not quite know what the effect if MIPS EVA (Enhanced Virtual
> Addressing)
> is set, I saw that ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE should be
> restricted
> to archs with non-overlapping address ranges.
> 
> I wonder whether MIPS EVA will generate overlapping address ranges?

they can overlap in EVA mode.

> If yes, it is better to make ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE depend
> on !EVA on MIPS.

Could please add the change ?

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH v2] MIPS/bpf: Enable bpf_probe_read{, str}() on MIPS again

[Tiezhu Yang]

On 03/25/2021 06:17 PM, Thomas Bogendoerfer wrote:
> On Mon, Mar 22, 2021 at 03:12:59PM +0800, Tiezhu Yang wrote:
>> On 03/22/2021 12:46 PM, Maciej W. Rozycki wrote:
>>> On Thu, 18 Mar 2021, Tiezhu Yang wrote:
>>>
>>>> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
>>>> index 160b3a8..4b94ec7 100644
>>>> --- a/arch/mips/Kconfig
>>>> +++ b/arch/mips/Kconfig
>>>> @@ -6,6 +6,7 @@ config MIPS
>>>>    	select ARCH_BINFMT_ELF_STATE if MIPS_FP_SUPPORT
>>>>    	select ARCH_HAS_FORTIFY_SOURCE
>>>>    	select ARCH_HAS_KCOV
>>>> +	select ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>>>    Hmm, documentation on ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE seems rather
>>> scarce, but based on my guess shouldn't this be "if !EVA"?
>>>
>>>     Maciej
>> I do not quite know what the effect if MIPS EVA (Enhanced Virtual
>> Addressing)
>> is set, I saw that ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE should be
>> restricted
>> to archs with non-overlapping address ranges.
>>
>> I wonder whether MIPS EVA will generate overlapping address ranges?
> they can overlap in EVA mode.
>
>> If yes, it is better to make ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE depend
>> on !EVA on MIPS.
> Could please add the change ?

OK, thank you, I will do it soon.

>
> Thomas.
>

Re: [PATCH v2] MIPS/bpf: Enable bpf_probe_read{, str}() on MIPS again

[Maciej W. Rozycki]

On Thu, 25 Mar 2021, Tiezhu Yang wrote:

> > > I wonder whether MIPS EVA will generate overlapping address ranges?
> > they can overlap in EVA mode.
> > 
> > > If yes, it is better to make ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE depend
> > > on !EVA on MIPS.
> > Could please add the change ?
> 
> OK, thank you, I will do it soon.

 For the record this is clearly described and accompanied with a drawing 
[1][2] in the architecture specification.  I do encourage you and anyone 
serious about contributing to the MIPS/Linux project to make yourselves 
familiar with the architecture beyond the area of your immediate interest 
so as to offload the maintainers who are often overloaded and sometimes do 
their work in their precious free time.  There are so many contributors 
and so few maintainers, so please help everyone and spread the work.

 Also please pay attention to quality change descriptions.  It's your task 
to convince the maintainer your work is worth including, and in your best 
interest to make the decision easy to make for the maintainer.  Think in 
terms of an exam at the university and what you would do to persuade your 
professor to give you a good score.  This is what the change description 
is for, beyond the quality of the change itself of course.

 This general rule of course applies to any community-maintained projects 
and not only MIPS/Linux.

References:

[1] "MIPS Architecture For Programmers, Vol. III: MIPS32/microMIPS32
    Privileged Resource Architecture", Document Number: MD00090, Revision 
    5.05, November 14, 2014, Figure 4.5 "EVA addressability", p. 51, 
    <https://wavecomp.ai/mips-technology/>

[2] "MIPS Architecture For Programmers, Volume III: The MIPS64 and 
    microMIPS64 Privileged Resource Architecture", Document Number: 
    MD00091, Revision 5.04, January 15, 2014, Figure 4.5 "EVA 
    addressability", p. 58, <https://wavecomp.ai/mips-technology/>

  Maciej