2021-03-18 02:31:17

by Tiezhu Yang

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

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 <[email protected]>
---

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


2021-03-22 04:48:38

by Maciej W. Rozycki

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

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

2021-03-22 07:15:13

by Tiezhu Yang

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

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

2021-03-22 07:16:27

by Christoph Hellwig

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

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.

2021-03-22 19:40:38

by Maciej W. Rozycki

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

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

2021-03-25 10:32:36

by Thomas Bogendoerfer

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

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 ]

2021-03-25 12:20:42

by Tiezhu Yang

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

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.
>

2021-03-28 21:08:26

by Maciej W. Rozycki

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

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