mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Yonghong Song <yhs@meta.com>
To: David Laight <David.Laight@ACULAB.COM>,
	"menglong8.dong@gmail.com" <menglong8.dong@gmail.com>,
	"alexei.starovoitov@gmail.com" <alexei.starovoitov@gmail.com>
Cc: "ast@kernel.org" <ast@kernel.org>,
	"daniel@iogearbox.net" <daniel@iogearbox.net>,
	"andrii@kernel.org" <andrii@kernel.org>,
	"martin.lau@linux.dev" <martin.lau@linux.dev>,
	"song@kernel.org" <song@kernel.org>, "yhs@fb.com" <yhs@fb.com>,
	"john.fastabend@gmail.com" <john.fastabend@gmail.com>,
	"kpsingh@kernel.org" <kpsingh@kernel.org>,
	"sdf@google.com" <sdf@google.com>,
	"haoluo@google.com" <haoluo@google.com>,
	"jolsa@kernel.org" <jolsa@kernel.org>,
	"benbjiang@tencent.com" <benbjiang@tencent.com>,
	"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Menglong Dong <imagedong@tencent.com>
Subject: Re: [PATCH bpf-next v5 2/3] bpf, x86: allow function arguments up to 12 for TRACING
Date: Thu, 22 Jun 2023 09:26:41 -0700	[thread overview]
Message-ID: <c59ab814-32d2-963d-e098-7965f6ff1441@meta.com> (raw)
In-Reply-To: <7a82744f454944778f55c36e8445762f@AcuMS.aculab.com>



On 6/22/23 2:06 AM, David Laight wrote:
> ...
>>> +	/* Generally speaking, the compiler will pass the arguments
>>> +	 * on-stack with "push" instruction, which will take 8-byte
>>> +	 * on the stack. On this case, there won't be garbage values
>>
>> On this case -> In this case. The same for below another case.
>>
>>> +	 * while we copy the arguments from origin stack frame to current
>>> +	 * in BPF_DW.
>>> +	 *
>>> +	 * However, sometimes the compiler will only allocate 4-byte on
>>> +	 * the stack for the arguments. For now, this case will only
>>> +	 * happen if there is only one argument on-stack and its size
>>> +	 * not more than 4 byte. On this case, there will be garbage
>>> +	 * values on the upper 4-byte where we store the argument on
>>> +	 * current stack frame.
> 
> Is that right for 86-64?

yes,

> 
> IIRC arguments always take (at least) 64bits.
> For any 32bit argument (register or stack) the high bits are undefined.
> (Maybe in kernel they are always zero?
>  From 32bit userspace they are definitely random.)
> 
> I think the called code is also responsible form masking 8 and 16bit
> values (in reality char/short args and return values just add code
> bloat).

yes, it does. For example, if an argument has type u8, so
x86_64 might only put a u8 value into 1-byte subregister
and rest of if is undefined. This is what happened to bpf program,
   (1). the whole register/stack is saved to 8-byte stack slot.
   (2). in bpf program, the 8-byte stack slot will be read
        and then cast to u8, so the compiler will do proper
        left shift and right shift to get proper value.

If the argument is u32/s32, the 32-bit subregister 'w*' could
be used without left/right shifting (similar to x86_64 subregister).

So we should be okay here.

> 
> A 128bit value is either passed in two registers or two stack
> slots. If the last register is skipped it will be used for the
> next argument.
> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)

  parent reply	other threads:[~2023-06-22 16:27 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-13  2:52 [PATCH bpf-next v5 0/3] " menglong8.dong
2023-06-13  2:52 ` [PATCH bpf-next v5 1/3] bpf, x86: clean garbage values when store args from regs into stack menglong8.dong
2023-06-18 22:52   ` Yonghong Song
2023-06-19  2:17     ` Menglong Dong
2023-06-13  2:52 ` [PATCH bpf-next v5 2/3] bpf, x86: allow function arguments up to 12 for TRACING menglong8.dong
2023-06-15  4:00   ` Menglong Dong
2023-06-18 23:10   ` Yonghong Song
2023-06-19  2:31     ` Menglong Dong
2023-06-22  9:06     ` David Laight
2023-06-22 13:05       ` Menglong Dong
2023-06-22 14:18         ` David Laight
2023-06-23 13:08           ` Menglong Dong
2023-06-22 16:26       ` Yonghong Song [this message]
2023-06-13  2:52 ` [PATCH bpf-next v5 3/3] selftests/bpf: add testcase for TRACING with 6+ arguments menglong8.dong

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=c59ab814-32d2-963d-e098-7965f6ff1441@meta.com \
    --to=yhs@meta.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=benbjiang@tencent.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=imagedong@tencent.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=menglong8.dong@gmail.com \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=yhs@fb.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®