linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paul Burton <paulburton@kernel.org>
To: Paul Burton <paulburton@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Paul Burton <paulburton@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Hassan Naveed <hnaveed@wavecomp.com>,
	Tony Ambardar <itugrok@yahoo.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org,
	stable@vger.kernel.org
Cc: linux-mips@vger.kernel.org
Subject: Re: [PATCH] MIPS: BPF: Restore MIPS32 cBPF JIT; disable MIPS32 eBPF JIT
Date: Fri, 06 Dec 2019 11:18:44 -0800	[thread overview]
Message-ID: <5deaa994.1c69fb81.97561.647e@mx.google.com> (raw)
In-Reply-To: <20191205182318.2761605-1-paulburton@kernel.org>

Hello,

Paul Burton wrote:
> Commit 716850ab104d ("MIPS: eBPF: Initial eBPF support for MIPS32
> architecture.") enabled our eBPF JIT for MIPS32 kernels, whereas it has
> previously only been availailable for MIPS64. It was my understanding at
> the time that the BPF test suite was passing & JITing a comparable
> number of tests to our cBPF JIT [1], but it turns out that was not the
> case.
> 
> The eBPF JIT has a number of problems on MIPS32:
> 
> - Most notably various code paths still result in emission of MIPS64
>   instructions which will cause reserved instruction exceptions & kernel
>   panics when run on MIPS32 CPUs.
> 
> - The eBPF JIT doesn't account for differences between the O32 ABI used
>   by MIPS32 kernels versus the N64 ABI used by MIPS64 kernels. Notably
>   arguments beyond the first 4 are passed on the stack in O32, and this
>   is entirely unhandled when JITing a BPF_CALL instruction. Stack space
>   must be reserved for arguments even if they all fit in registers, and
>   the callee is free to assume that stack space has been reserved for
>   its use - with the eBPF JIT this is not the case, so calling any
>   function can result in clobbering values on the stack & unpredictable
>   behaviour. Function arguments in eBPF are always 64-bit values which
>   is also entirely unhandled - the JIT still uses a single (32-bit)
>   register per argument. As a result all function arguments are always
>   passed incorrectly when JITing a BPF_CALL instruction, leading to
>   kernel crashes or strange behavior.
> 
> - The JIT attempts to bail our on use of ALU64 instructions or 64-bit
>   memory access instructions. The code doing this at the start of
>   build_one_insn() incorrectly checks whether BPF_OP() equals BPF_DW,
>   when it should really be checking BPF_SIZE() & only doing so when
>   BPF_CLASS() is one of BPF_{LD,LDX,ST,STX}. This results in false
>   positives that cause more bailouts than intended, and that in turns
>   hides some of the problems described above.
> 
> - The kernel's cBPF->eBPF translation makes heavy use of 64-bit eBPF
>   instructions that the MIPS32 eBPF JIT bails out on, leading to most
>   cBPF programs not being JITed at all.
> 
> Until these problems are resolved, revert the removal of the cBPF JIT &
> the enabling of the eBPF JIT on MIPS32 done by commit 716850ab104d
> ("MIPS: eBPF: Initial eBPF support for MIPS32 architecture.").
> 
> Note that this does not undo the changes made to the eBPF JIT by that
> commit, since they are a useful starting point to providing MIPS32
> support - they're just not nearly complete.
> 
> [1] https://lore.kernel.org/linux-mips/MWHPR2201MB13583388481F01A422CE7D66D4410@MWHPR2201MB1358.namprd22.prod.outlook.com/

Applied to mips-fixes.

> commit c409cd05ab7f
> https://git.kernel.org/mips/c/c409cd05ab7f
> 
> Signed-off-by: Paul Burton <paulburton@kernel.org>
> Fixes: 716850ab104d ("MIPS: eBPF: Initial eBPF support for MIPS32 architecture.")

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paulburton@kernel.org to report it. ]

      parent reply	other threads:[~2019-12-06 19:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-05 18:23 [PATCH] MIPS: BPF: Restore MIPS32 cBPF JIT; disable MIPS32 eBPF JIT Paul Burton
2019-12-06 11:16 ` Alexander Lobakin
2019-12-06 19:18 ` Paul Burton [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5deaa994.1c69fb81.97561.647e@mx.google.com \
    --to=paulburton@kernel.org \
    --cc=linux-mips@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).