All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jean-Philippe Brucker <jean-philippe@linaro.org>
To: Xu Kuohai <xukuohai@huawei.com>
Cc: bpf@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Will Deacon <will@kernel.org>, KP Singh <kpsingh@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Zi Shen Lim <zlim.lnx@gmail.com>, Martin KaFai Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	David Ahern <dsahern@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H . Peter Anvin" <hpa@zytor.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	James Morse <james.morse@arm.com>, Hou Tao <houtao1@huawei.com>,
	Jason Wang <wangborong@cdjrlc.com>
Subject: Re: [PATCH bpf-next v7 4/4] bpf, arm64: bpf trampoline for arm64
Date: Mon, 11 Jul 2022 15:37:17 +0100	[thread overview]
Message-ID: <Ysw1nSxQUghvY/eY@myrica> (raw)
In-Reply-To: <4852eba8-9fd0-6894-934c-ab89c0c7cea9@huawei.com>

On Mon, Jul 11, 2022 at 10:16:00PM +0800, Xu Kuohai wrote:
> >> +	if (save_ret)
> >> +		emit(A64_STR64I(p->jited ? r0 : A64_R(0), A64_SP, retval_off),
> >> +		     ctx);
> > 
> > This should be only A64_R(0), not r0. r0 happens to equal A64_R(0) when
> > jitted due to the way build_epilogue() builds the function at the moment,
> > but we shouldn't rely on that.
> > 
> 
> looks like I misunderstood something, will change it to:
> 
> /* store return value, which is held in x0 for interpreter and in
>  * bpf register r0 for JIT,

It's simpler than that: in both cases the return value is in x0 because
the function follows the procedure call standard. You could drop the
comment to avoid confusion and only do the change to A64_R(0)

Thanks,
Jean

>
>
>  but r0 happens to equal x0 due to the
>  * way build_epilogue() builds the JIT image.
>  */
> if (save_ret)
>         emit(A64_STR64I(A64_R(0), A64_SP, retval_off), ctx);
> 
> > Apart from that, for the series
> > 
> > Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > 
> > .

WARNING: multiple messages have this Message-ID (diff)
From: Jean-Philippe Brucker <jean-philippe@linaro.org>
To: Xu Kuohai <xukuohai@huawei.com>
Cc: bpf@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Will Deacon <will@kernel.org>, KP Singh <kpsingh@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Zi Shen Lim <zlim.lnx@gmail.com>, Martin KaFai Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	David Ahern <dsahern@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H . Peter Anvin" <hpa@zytor.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	James Morse <james.morse@arm.com>, Hou Tao <houtao1@huawei.com>,
	Jason Wang <wangborong@cdjrlc.com>
Subject: Re: [PATCH bpf-next v7 4/4] bpf, arm64: bpf trampoline for arm64
Date: Mon, 11 Jul 2022 15:37:17 +0100	[thread overview]
Message-ID: <Ysw1nSxQUghvY/eY@myrica> (raw)
In-Reply-To: <4852eba8-9fd0-6894-934c-ab89c0c7cea9@huawei.com>

On Mon, Jul 11, 2022 at 10:16:00PM +0800, Xu Kuohai wrote:
> >> +	if (save_ret)
> >> +		emit(A64_STR64I(p->jited ? r0 : A64_R(0), A64_SP, retval_off),
> >> +		     ctx);
> > 
> > This should be only A64_R(0), not r0. r0 happens to equal A64_R(0) when
> > jitted due to the way build_epilogue() builds the function at the moment,
> > but we shouldn't rely on that.
> > 
> 
> looks like I misunderstood something, will change it to:
> 
> /* store return value, which is held in x0 for interpreter and in
>  * bpf register r0 for JIT,

It's simpler than that: in both cases the return value is in x0 because
the function follows the procedure call standard. You could drop the
comment to avoid confusion and only do the change to A64_R(0)

Thanks,
Jean

>
>
>  but r0 happens to equal x0 due to the
>  * way build_epilogue() builds the JIT image.
>  */
> if (save_ret)
>         emit(A64_STR64I(A64_R(0), A64_SP, retval_off), ctx);
> 
> > Apart from that, for the series
> > 
> > Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > 
> > .

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-07-11 14:37 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-08  9:30 [PATCH bpf-next v7 0/4] bpf trampoline for arm64 Xu Kuohai
2022-07-08  9:30 ` Xu Kuohai
2022-07-08  9:30 ` [PATCH bpf-next v7 1/4] bpf: Remove is_valid_bpf_tramp_flags() Xu Kuohai
2022-07-08  9:30   ` Xu Kuohai
2022-07-08  9:30 ` [PATCH bpf-next v7 2/4] arm64: Add LDR (literal) instruction Xu Kuohai
2022-07-08  9:30   ` Xu Kuohai
2022-07-08  9:30 ` [PATCH bpf-next v7 3/4] bpf, arm64: Implement bpf_arch_text_poke() for arm64 Xu Kuohai
2022-07-08  9:30   ` Xu Kuohai
2022-07-08  9:30 ` [PATCH bpf-next v7 4/4] bpf, arm64: bpf trampoline " Xu Kuohai
2022-07-08  9:30   ` Xu Kuohai
2022-07-11 11:57   ` Jean-Philippe Brucker
2022-07-11 11:57     ` Jean-Philippe Brucker
2022-07-11 14:16     ` Xu Kuohai
2022-07-11 14:16       ` Xu Kuohai
2022-07-11 14:37       ` Jean-Philippe Brucker [this message]
2022-07-11 14:37         ` Jean-Philippe Brucker
2022-07-11 14:40         ` Xu Kuohai
2022-07-11 14:40           ` Xu Kuohai
2022-07-11 14:48           ` Jean-Philippe Brucker
2022-07-11 14:48             ` Jean-Philippe Brucker

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=Ysw1nSxQUghvY/eY@myrica \
    --to=jean-philippe@linaro.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bp@alien8.de \
    --cc=bpf@vger.kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=daniel@iogearbox.net \
    --cc=dave.hansen@linux.intel.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=hawk@kernel.org \
    --cc=houtao1@huawei.com \
    --cc=hpa@zytor.com \
    --cc=james.morse@arm.com \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=songliubraving@fb.com \
    --cc=tglx@linutronix.de \
    --cc=wangborong@cdjrlc.com \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    --cc=xukuohai@huawei.com \
    --cc=yhs@fb.com \
    --cc=yoshfuji@linux-ipv6.org \
    --cc=zlim.lnx@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.