Re: [Bpf] Review of draft-thaler-bpf-isa-01

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Dave Thaler]

[-- Attachment #1.1: Type: text/plain, Size: 753 bytes --]

I am forwarding the email below to the bpf@vger.kernel.org<mailto:bpf@vger.kernel.org> list
so replies can go to both lists.

Thanks,
Dave
From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
Sent: Monday, July 24, 2023 10:05 PM
To: bpf@ietf.org
Subject: [Bpf] Review of draft-thaler-bpf-isa-01

Dear BPF wg,

I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is the specification seems to use an underspecified C pseudo code for operations vs defining them mathematically.

The good news is I think this is very fixable although tedious.

The other thornier issues are memory model etc. But the overall structure seems good and the document overall makes sense.

Sincerely,
Watson Ladd

[-- Attachment #1.2: Type: text/html, Size: 3656 bytes --]

[-- Attachment #2: Type: text/plain, Size: 76 bytes --]

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

RE: [Bpf] Review of draft-thaler-bpf-isa-01

[Dave Thaler]

I am forwarding the email below (after converting HTML to plain text)
to the mailto:bpf@vger.kernel.org list so replies can go to both lists.

Please use this one for any replies.

Thanks,
Dave

> From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> Sent: Monday, July 24, 2023 10:05 PM
> To: bpf@ietf.org
> Subject: [Bpf] Review of draft-thaler-bpf-isa-01
>
> Dear BPF wg,
>
> I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> the specification seems to use an underspecified C pseudo code for operations vs
> defining them mathematically.
>
> The good news is I think this is very fixable although tedious.
>
> The other thornier issues are memory model etc. But the overall structure seems good
> and the document overall makes sense.
>
> Sincerely,
> Watson Ladd

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Dave Thaler]

I am forwarding the email below (after converting HTML to plain text)
to the mailto:bpf@vger.kernel.org list so replies can go to both lists.

Please use this one for any replies.

Thanks,
Dave

> From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> Sent: Monday, July 24, 2023 10:05 PM
> To: bpf@ietf.org
> Subject: [Bpf] Review of draft-thaler-bpf-isa-01
>
> Dear BPF wg,
>
> I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> the specification seems to use an underspecified C pseudo code for operations vs
> defining them mathematically.
>
> The good news is I think this is very fixable although tedious.
>
> The other thornier issues are memory model etc. But the overall structure seems good
> and the document overall makes sense.
>
> Sincerely,
> Watson Ladd
-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
>
> I am forwarding the email below (after converting HTML to plain text)
> to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
>
> Please use this one for any replies.
>
> Thanks,
> Dave
>
> > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > Sent: Monday, July 24, 2023 10:05 PM
> > To: bpf@ietf.org
> > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> >
> > Dear BPF wg,
> >
> > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > the specification seems to use an underspecified C pseudo code for operations vs
> > defining them mathematically.

Hi Watson,

This is not "underspecified C" pseudo code.
This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.

> > The good news is I think this is very fixable although tedious.
> >
> > The other thornier issues are memory model etc. But the overall structure seems good
> > and the document overall makes sense.

What do you mean by "memory model" ?
Do you see a reference to it ? Please be specific.

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
>
> I am forwarding the email below (after converting HTML to plain text)
> to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
>
> Please use this one for any replies.
>
> Thanks,
> Dave
>
> > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > Sent: Monday, July 24, 2023 10:05 PM
> > To: bpf@ietf.org
> > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> >
> > Dear BPF wg,
> >
> > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > the specification seems to use an underspecified C pseudo code for operations vs
> > defining them mathematically.

Hi Watson,

This is not "underspecified C" pseudo code.
This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.

> > The good news is I think this is very fixable although tedious.
> >
> > The other thornier issues are memory model etc. But the overall structure seems good
> > and the document overall makes sense.

What do you mean by "memory model" ?
Do you see a reference to it ? Please be specific.

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Watson Ladd]

On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> >
> > I am forwarding the email below (after converting HTML to plain text)
> > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> >
> > Please use this one for any replies.
> >
> > Thanks,
> > Dave
> >
> > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > Sent: Monday, July 24, 2023 10:05 PM
> > > To: bpf@ietf.org
> > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > >
> > > Dear BPF wg,
> > >
> > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > the specification seems to use an underspecified C pseudo code for operations vs
> > > defining them mathematically.
>
> Hi Watson,
>
> This is not "underspecified C" pseudo code.
> This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.

I don't see a reference to any description of that in section 4.1.
It's possible I've overlooked this, and if people think this style of
definition is good enough that works for me. But I found table 4
pretty scanty on what exactly happens.
>
> > > The good news is I think this is very fixable although tedious.
> > >
> > > The other thornier issues are memory model etc. But the overall structure seems good
> > > and the document overall makes sense.
>
> What do you mean by "memory model" ?
> Do you see a reference to it ? Please be specific.

No, and that's the problem. Section 5.2 talks about atomic operations.
I'd expect that to be paired with a description of barriers so that
these work, or a big warning about when you need to use them. For
clarity I'm pretty unfamiliar with bpf as a technology, and it's
possible that with more knowledge this would make sense. On looking
back on that I don't even know if the memory space is flat, or
segmented: can I access maps through a value set to dst+offset, or
must I always used index? I'm just very confused.

Sincerely,
Watson

-- 
Astra mortemque praestare gradatim

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Watson Ladd]

On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> >
> > I am forwarding the email below (after converting HTML to plain text)
> > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> >
> > Please use this one for any replies.
> >
> > Thanks,
> > Dave
> >
> > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > Sent: Monday, July 24, 2023 10:05 PM
> > > To: bpf@ietf.org
> > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > >
> > > Dear BPF wg,
> > >
> > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > the specification seems to use an underspecified C pseudo code for operations vs
> > > defining them mathematically.
>
> Hi Watson,
>
> This is not "underspecified C" pseudo code.
> This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.

I don't see a reference to any description of that in section 4.1.
It's possible I've overlooked this, and if people think this style of
definition is good enough that works for me. But I found table 4
pretty scanty on what exactly happens.
>
> > > The good news is I think this is very fixable although tedious.
> > >
> > > The other thornier issues are memory model etc. But the overall structure seems good
> > > and the document overall makes sense.
>
> What do you mean by "memory model" ?
> Do you see a reference to it ? Please be specific.

No, and that's the problem. Section 5.2 talks about atomic operations.
I'd expect that to be paired with a description of barriers so that
these work, or a big warning about when you need to use them. For
clarity I'm pretty unfamiliar with bpf as a technology, and it's
possible that with more knowledge this would make sense. On looking
back on that I don't even know if the memory space is flat, or
segmented: can I access maps through a value set to dst+offset, or
must I always used index? I'm just very confused.

Sincerely,
Watson

-- 
Astra mortemque praestare gradatim

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
>
> On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > >
> > > I am forwarding the email below (after converting HTML to plain text)
> > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > >
> > > Please use this one for any replies.
> > >
> > > Thanks,
> > > Dave
> > >
> > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > To: bpf@ietf.org
> > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > >
> > > > Dear BPF wg,
> > > >
> > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > defining them mathematically.
> >
> > Hi Watson,
> >
> > This is not "underspecified C" pseudo code.
> > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
>
> I don't see a reference to any description of that in section 4.1.
> It's possible I've overlooked this, and if people think this style of
> definition is good enough that works for me. But I found table 4
> pretty scanty on what exactly happens.

Hello! Based on Watson's post, I have done some research and would
potentially like to offer a path forward. There are several different
ways that ISAs specify the semantics of their operations:

1. Intel has a section in their manual that describes the pseudocode
they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
IA-32 Architectures Software Developer’s Manual at
https://cdrdv2.intel.com/v1/dl/getContent/671199
2. ARM has an equivalent for their variety of pseudocode: Chapter J1
of Arm Architecture Reference Manual for A-profile architecture at
https://developer.arm.com/documentation/ddi0487/latest/
3. Sail "is a language for describing the instruction-set architecture
(ISA) semantics of processors."
(https://www.cl.cam.ac.uk/~pes20/sail/)

Given the commercial nature of (1) and (2), perhaps Sail is a way to
proceed. If people are interested, I would be happy to lead an effort
to encode the eBPF ISA semantics in Sail (or find someone who already
has) and incorporate them in the draft.

Sincerely,
Will

> >
> > > > The good news is I think this is very fixable although tedious.
> > > >
> > > > The other thornier issues are memory model etc. But the overall structure seems good
> > > > and the document overall makes sense.
> >
> > What do you mean by "memory model" ?
> > Do you see a reference to it ? Please be specific.
>
> No, and that's the problem. Section 5.2 talks about atomic operations.
> I'd expect that to be paired with a description of barriers so that
> these work, or a big warning about when you need to use them. For
> clarity I'm pretty unfamiliar with bpf as a technology, and it's
> possible that with more knowledge this would make sense. On looking
> back on that I don't even know if the memory space is flat, or
> segmented: can I access maps through a value set to dst+offset, or
> must I always used index? I'm just very confused.
>
> Sincerely,
> Watson
>
> --
> Astra mortemque praestare gradatim
>
> --
> Bpf mailing list
> Bpf@ietf.org
> https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
>
> On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > >
> > > I am forwarding the email below (after converting HTML to plain text)
> > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > >
> > > Please use this one for any replies.
> > >
> > > Thanks,
> > > Dave
> > >
> > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > To: bpf@ietf.org
> > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > >
> > > > Dear BPF wg,
> > > >
> > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > defining them mathematically.
> >
> > Hi Watson,
> >
> > This is not "underspecified C" pseudo code.
> > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
>
> I don't see a reference to any description of that in section 4.1.
> It's possible I've overlooked this, and if people think this style of
> definition is good enough that works for me. But I found table 4
> pretty scanty on what exactly happens.

Hello! Based on Watson's post, I have done some research and would
potentially like to offer a path forward. There are several different
ways that ISAs specify the semantics of their operations:

1. Intel has a section in their manual that describes the pseudocode
they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
IA-32 Architectures Software Developer’s Manual at
https://cdrdv2.intel.com/v1/dl/getContent/671199
2. ARM has an equivalent for their variety of pseudocode: Chapter J1
of Arm Architecture Reference Manual for A-profile architecture at
https://developer.arm.com/documentation/ddi0487/latest/
3. Sail "is a language for describing the instruction-set architecture
(ISA) semantics of processors."
(https://www.cl.cam.ac.uk/~pes20/sail/)

Given the commercial nature of (1) and (2), perhaps Sail is a way to
proceed. If people are interested, I would be happy to lead an effort
to encode the eBPF ISA semantics in Sail (or find someone who already
has) and incorporate them in the draft.

Sincerely,
Will

> >
> > > > The good news is I think this is very fixable although tedious.
> > > >
> > > > The other thornier issues are memory model etc. But the overall structure seems good
> > > > and the document overall makes sense.
> >
> > What do you mean by "memory model" ?
> > Do you see a reference to it ? Please be specific.
>
> No, and that's the problem. Section 5.2 talks about atomic operations.
> I'd expect that to be paired with a description of barriers so that
> these work, or a big warning about when you need to use them. For
> clarity I'm pretty unfamiliar with bpf as a technology, and it's
> possible that with more knowledge this would make sense. On looking
> back on that I don't even know if the memory space is flat, or
> segmented: can I access maps through a value set to dst+offset, or
> must I always used index? I'm just very confused.
>
> Sincerely,
> Watson
>
> --
> Astra mortemque praestare gradatim
>
> --
> Bpf mailing list
> Bpf@ietf.org
> https://www.ietf.org/mailman/listinfo/bpf

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> >
> > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > >
> > > > I am forwarding the email below (after converting HTML to plain text)
> > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > >
> > > > Please use this one for any replies.
> > > >
> > > > Thanks,
> > > > Dave
> > > >
> > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > To: bpf@ietf.org
> > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > >
> > > > > Dear BPF wg,
> > > > >
> > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > defining them mathematically.
> > >
> > > Hi Watson,
> > >
> > > This is not "underspecified C" pseudo code.
> > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> >
> > I don't see a reference to any description of that in section 4.1.
> > It's possible I've overlooked this, and if people think this style of
> > definition is good enough that works for me. But I found table 4
> > pretty scanty on what exactly happens.
>
> Hello! Based on Watson's post, I have done some research and would
> potentially like to offer a path forward. There are several different
> ways that ISAs specify the semantics of their operations:
>
> 1. Intel has a section in their manual that describes the pseudocode
> they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> IA-32 Architectures Software Developer’s Manual at
> https://cdrdv2.intel.com/v1/dl/getContent/671199
> 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> of Arm Architecture Reference Manual for A-profile architecture at
> https://developer.arm.com/documentation/ddi0487/latest/
> 3. Sail "is a language for describing the instruction-set architecture
> (ISA) semantics of processors."
> (https://www.cl.cam.ac.uk/~pes20/sail/)
>
> Given the commercial nature of (1) and (2), perhaps Sail is a way to
> proceed. If people are interested, I would be happy to lead an effort
> to encode the eBPF ISA semantics in Sail (or find someone who already
> has) and incorporate them in the draft.

imo Sail is too researchy to have practical use.
Looking at arm64 or x86 Sail description I really don't see how
it would map to an IETF standard.
It's done in a "sail" language that people need to learn first to be
able to read it.
Say we had bpf.sail somewhere on github. What value does it bring to
BPF ISA standard? I don't see an immediate benefit to standardization.
There could be other use cases, no doubt, but standardization is our goal.

As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
to add a paragraph to describe it. We are using C to describe BPF ISA
semantics. I don't think we need to explain C in the BPF ISA doc.
The only exception is "s>=", but it is explained in the doc already.

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> >
> > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > >
> > > > I am forwarding the email below (after converting HTML to plain text)
> > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > >
> > > > Please use this one for any replies.
> > > >
> > > > Thanks,
> > > > Dave
> > > >
> > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > To: bpf@ietf.org
> > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > >
> > > > > Dear BPF wg,
> > > > >
> > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > defining them mathematically.
> > >
> > > Hi Watson,
> > >
> > > This is not "underspecified C" pseudo code.
> > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> >
> > I don't see a reference to any description of that in section 4.1.
> > It's possible I've overlooked this, and if people think this style of
> > definition is good enough that works for me. But I found table 4
> > pretty scanty on what exactly happens.
>
> Hello! Based on Watson's post, I have done some research and would
> potentially like to offer a path forward. There are several different
> ways that ISAs specify the semantics of their operations:
>
> 1. Intel has a section in their manual that describes the pseudocode
> they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> IA-32 Architectures Software Developer’s Manual at
> https://cdrdv2.intel.com/v1/dl/getContent/671199
> 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> of Arm Architecture Reference Manual for A-profile architecture at
> https://developer.arm.com/documentation/ddi0487/latest/
> 3. Sail "is a language for describing the instruction-set architecture
> (ISA) semantics of processors."
> (https://www.cl.cam.ac.uk/~pes20/sail/)
>
> Given the commercial nature of (1) and (2), perhaps Sail is a way to
> proceed. If people are interested, I would be happy to lead an effort
> to encode the eBPF ISA semantics in Sail (or find someone who already
> has) and incorporate them in the draft.

imo Sail is too researchy to have practical use.
Looking at arm64 or x86 Sail description I really don't see how
it would map to an IETF standard.
It's done in a "sail" language that people need to learn first to be
able to read it.
Say we had bpf.sail somewhere on github. What value does it bring to
BPF ISA standard? I don't see an immediate benefit to standardization.
There could be other use cases, no doubt, but standardization is our goal.

As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
to add a paragraph to describe it. We are using C to describe BPF ISA
semantics. I don't think we need to explain C in the BPF ISA doc.
The only exception is "s>=", but it is explained in the doc already.

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > >
> > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > >
> > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > >
> > > > > Please use this one for any replies.
> > > > >
> > > > > Thanks,
> > > > > Dave
> > > > >
> > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > To: bpf@ietf.org
> > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > >
> > > > > > Dear BPF wg,
> > > > > >
> > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > defining them mathematically.
> > > >
> > > > Hi Watson,
> > > >
> > > > This is not "underspecified C" pseudo code.
> > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > >
> > > I don't see a reference to any description of that in section 4.1.
> > > It's possible I've overlooked this, and if people think this style of
> > > definition is good enough that works for me. But I found table 4
> > > pretty scanty on what exactly happens.
> >
> > Hello! Based on Watson's post, I have done some research and would
> > potentially like to offer a path forward. There are several different
> > ways that ISAs specify the semantics of their operations:
> >
> > 1. Intel has a section in their manual that describes the pseudocode
> > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > IA-32 Architectures Software Developer’s Manual at
> > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > of Arm Architecture Reference Manual for A-profile architecture at
> > https://developer.arm.com/documentation/ddi0487/latest/
> > 3. Sail "is a language for describing the instruction-set architecture
> > (ISA) semantics of processors."
> > (https://www.cl.cam.ac.uk/~pes20/sail/)
> >
> > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > proceed. If people are interested, I would be happy to lead an effort
> > to encode the eBPF ISA semantics in Sail (or find someone who already
> > has) and incorporate them in the draft.
>
> imo Sail is too researchy to have practical use.
> Looking at arm64 or x86 Sail description I really don't see how
> it would map to an IETF standard.
> It's done in a "sail" language that people need to learn first to be
> able to read it.
> Say we had bpf.sail somewhere on github. What value does it bring to
> BPF ISA standard? I don't see an immediate benefit to standardization.
> There could be other use cases, no doubt, but standardization is our goal.
>
> As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> to add a paragraph to describe it. We are using C to describe BPF ISA


I cannot find a reference in the current version that specifies what
we are using to describe the operations. I'd like to add that, but
want to make sure that I clarify two statements that seem to be at
odds.

Immediately above you say that we are using "C to describe the BPF
ISA" and further above you say "This is assembly syntax parsed and
emitted by GCC, LLVM, gas, Linux Kernel, etc."

My own reading is that it is the former, and not the latter. But, I
want to double check before adding the appropriate statements to the
Convention section.

Will

> semantics. I don't think we need to explain C in the BPF ISA doc.
> The only exception is "s>=", but it is explained in the doc already.

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > >
> > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > >
> > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > >
> > > > > Please use this one for any replies.
> > > > >
> > > > > Thanks,
> > > > > Dave
> > > > >
> > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > To: bpf@ietf.org
> > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > >
> > > > > > Dear BPF wg,
> > > > > >
> > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > defining them mathematically.
> > > >
> > > > Hi Watson,
> > > >
> > > > This is not "underspecified C" pseudo code.
> > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > >
> > > I don't see a reference to any description of that in section 4.1.
> > > It's possible I've overlooked this, and if people think this style of
> > > definition is good enough that works for me. But I found table 4
> > > pretty scanty on what exactly happens.
> >
> > Hello! Based on Watson's post, I have done some research and would
> > potentially like to offer a path forward. There are several different
> > ways that ISAs specify the semantics of their operations:
> >
> > 1. Intel has a section in their manual that describes the pseudocode
> > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > IA-32 Architectures Software Developer’s Manual at
> > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > of Arm Architecture Reference Manual for A-profile architecture at
> > https://developer.arm.com/documentation/ddi0487/latest/
> > 3. Sail "is a language for describing the instruction-set architecture
> > (ISA) semantics of processors."
> > (https://www.cl.cam.ac.uk/~pes20/sail/)
> >
> > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > proceed. If people are interested, I would be happy to lead an effort
> > to encode the eBPF ISA semantics in Sail (or find someone who already
> > has) and incorporate them in the draft.
>
> imo Sail is too researchy to have practical use.
> Looking at arm64 or x86 Sail description I really don't see how
> it would map to an IETF standard.
> It's done in a "sail" language that people need to learn first to be
> able to read it.
> Say we had bpf.sail somewhere on github. What value does it bring to
> BPF ISA standard? I don't see an immediate benefit to standardization.
> There could be other use cases, no doubt, but standardization is our goal.
>
> As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> to add a paragraph to describe it. We are using C to describe BPF ISA


I cannot find a reference in the current version that specifies what
we are using to describe the operations. I'd like to add that, but
want to make sure that I clarify two statements that seem to be at
odds.

Immediately above you say that we are using "C to describe the BPF
ISA" and further above you say "This is assembly syntax parsed and
emitted by GCC, LLVM, gas, Linux Kernel, etc."

My own reading is that it is the former, and not the latter. But, I
want to double check before adding the appropriate statements to the
Convention section.

Will

> semantics. I don't think we need to explain C in the BPF ISA doc.
> The only exception is "s>=", but it is explained in the doc already.

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > >
> > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > >
> > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > >
> > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > >
> > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > >
> > > > > > Please use this one for any replies.
> > > > > >
> > > > > > Thanks,
> > > > > > Dave
> > > > > >
> > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > To: bpf@ietf.org
> > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > >
> > > > > > > Dear BPF wg,
> > > > > > >
> > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > defining them mathematically.
> > > > >
> > > > > Hi Watson,
> > > > >
> > > > > This is not "underspecified C" pseudo code.
> > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > >
> > > > I don't see a reference to any description of that in section 4.1.
> > > > It's possible I've overlooked this, and if people think this style of
> > > > definition is good enough that works for me. But I found table 4
> > > > pretty scanty on what exactly happens.
> > >
> > > Hello! Based on Watson's post, I have done some research and would
> > > potentially like to offer a path forward. There are several different
> > > ways that ISAs specify the semantics of their operations:
> > >
> > > 1. Intel has a section in their manual that describes the pseudocode
> > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > IA-32 Architectures Software Developer’s Manual at
> > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > of Arm Architecture Reference Manual for A-profile architecture at
> > > https://developer.arm.com/documentation/ddi0487/latest/
> > > 3. Sail "is a language for describing the instruction-set architecture
> > > (ISA) semantics of processors."
> > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > >
> > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > proceed. If people are interested, I would be happy to lead an effort
> > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > has) and incorporate them in the draft.
> >
> > imo Sail is too researchy to have practical use.
> > Looking at arm64 or x86 Sail description I really don't see how
> > it would map to an IETF standard.
> > It's done in a "sail" language that people need to learn first to be
> > able to read it.
> > Say we had bpf.sail somewhere on github. What value does it bring to
> > BPF ISA standard? I don't see an immediate benefit to standardization.
> > There could be other use cases, no doubt, but standardization is our goal.
> >
> > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > to add a paragraph to describe it. We are using C to describe BPF ISA
>
>
> I cannot find a reference in the current version that specifies what
> we are using to describe the operations. I'd like to add that, but
> want to make sure that I clarify two statements that seem to be at
> odds.
>
> Immediately above you say that we are using "C to describe the BPF
> ISA" and further above you say "This is assembly syntax parsed and
> emitted by GCC, LLVM, gas, Linux Kernel, etc."
>
> My own reading is that it is the former, and not the latter. But, I
> want to double check before adding the appropriate statements to the
> Convention section.

It's both. I'm not sure where you see a contradiction.
It's a normal C syntax and it's emitted by the kernel verifier,
parsed by clang/gcc assemblers and emitted by compilers.

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > >
> > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > >
> > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > >
> > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > >
> > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > >
> > > > > > Please use this one for any replies.
> > > > > >
> > > > > > Thanks,
> > > > > > Dave
> > > > > >
> > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > To: bpf@ietf.org
> > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > >
> > > > > > > Dear BPF wg,
> > > > > > >
> > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > defining them mathematically.
> > > > >
> > > > > Hi Watson,
> > > > >
> > > > > This is not "underspecified C" pseudo code.
> > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > >
> > > > I don't see a reference to any description of that in section 4.1.
> > > > It's possible I've overlooked this, and if people think this style of
> > > > definition is good enough that works for me. But I found table 4
> > > > pretty scanty on what exactly happens.
> > >
> > > Hello! Based on Watson's post, I have done some research and would
> > > potentially like to offer a path forward. There are several different
> > > ways that ISAs specify the semantics of their operations:
> > >
> > > 1. Intel has a section in their manual that describes the pseudocode
> > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > IA-32 Architectures Software Developer’s Manual at
> > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > of Arm Architecture Reference Manual for A-profile architecture at
> > > https://developer.arm.com/documentation/ddi0487/latest/
> > > 3. Sail "is a language for describing the instruction-set architecture
> > > (ISA) semantics of processors."
> > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > >
> > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > proceed. If people are interested, I would be happy to lead an effort
> > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > has) and incorporate them in the draft.
> >
> > imo Sail is too researchy to have practical use.
> > Looking at arm64 or x86 Sail description I really don't see how
> > it would map to an IETF standard.
> > It's done in a "sail" language that people need to learn first to be
> > able to read it.
> > Say we had bpf.sail somewhere on github. What value does it bring to
> > BPF ISA standard? I don't see an immediate benefit to standardization.
> > There could be other use cases, no doubt, but standardization is our goal.
> >
> > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > to add a paragraph to describe it. We are using C to describe BPF ISA
>
>
> I cannot find a reference in the current version that specifies what
> we are using to describe the operations. I'd like to add that, but
> want to make sure that I clarify two statements that seem to be at
> odds.
>
> Immediately above you say that we are using "C to describe the BPF
> ISA" and further above you say "This is assembly syntax parsed and
> emitted by GCC, LLVM, gas, Linux Kernel, etc."
>
> My own reading is that it is the former, and not the latter. But, I
> want to double check before adding the appropriate statements to the
> Convention section.

It's both. I'm not sure where you see a contradiction.
It's a normal C syntax and it's emitted by the kernel verifier,
parsed by clang/gcc assemblers and emitted by compilers.

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > >
> > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > >
> > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > >
> > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > >
> > > > > > > Please use this one for any replies.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Dave
> > > > > > >
> > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > To: bpf@ietf.org
> > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > >
> > > > > > > > Dear BPF wg,
> > > > > > > >
> > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > defining them mathematically.
> > > > > >
> > > > > > Hi Watson,
> > > > > >
> > > > > > This is not "underspecified C" pseudo code.
> > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > >
> > > > > I don't see a reference to any description of that in section 4.1.
> > > > > It's possible I've overlooked this, and if people think this style of
> > > > > definition is good enough that works for me. But I found table 4
> > > > > pretty scanty on what exactly happens.
> > > >
> > > > Hello! Based on Watson's post, I have done some research and would
> > > > potentially like to offer a path forward. There are several different
> > > > ways that ISAs specify the semantics of their operations:
> > > >
> > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > IA-32 Architectures Software Developer’s Manual at
> > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > (ISA) semantics of processors."
> > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > >
> > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > proceed. If people are interested, I would be happy to lead an effort
> > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > has) and incorporate them in the draft.
> > >
> > > imo Sail is too researchy to have practical use.
> > > Looking at arm64 or x86 Sail description I really don't see how
> > > it would map to an IETF standard.
> > > It's done in a "sail" language that people need to learn first to be
> > > able to read it.
> > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > There could be other use cases, no doubt, but standardization is our goal.
> > >
> > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > to add a paragraph to describe it. We are using C to describe BPF ISA
> >
> >
> > I cannot find a reference in the current version that specifies what
> > we are using to describe the operations. I'd like to add that, but
> > want to make sure that I clarify two statements that seem to be at
> > odds.
> >
> > Immediately above you say that we are using "C to describe the BPF
> > ISA" and further above you say "This is assembly syntax parsed and
> > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> >
> > My own reading is that it is the former, and not the latter. But, I
> > want to double check before adding the appropriate statements to the
> > Convention section.
>
> It's both. I'm not sure where you see a contradiction.
> It's a normal C syntax and it's emitted by the kernel verifier,
> parsed by clang/gcc assemblers and emitted by compilers.


Okay. I apologize. I am sincerely confused. For instance,

if (u32)dst >= (u32)src goto +offset

Looks like nothing that I have ever seen in "normal C syntax".

There also appear to be a few other places where things might be a bit wonky:

1. Address arithmetic in the description of the load/store
instructions will depend on the type of the target: E.g.,

*(u64 *)(dst + offset) = imm

The address to which the store is done will be offset*sizeof(X) bytes
from dst where X is the type of the target of dst. If we are assuming
that dst (or its equivalent in similar instructions) is being treated
simply as an unsigned integer, I believe that we will have to say that
explicitly, especially given that we describe offset as "signed
integer offset used with pointer arithmetic" in the Instruction
encoding section.

2. hto[bl]eN functions are not specified by standard C and, while
"obvious" what they do, are not defined in the document anywhere.

Again, I am really sorry to be causing so much confusion. I hope that
at least some of this discussion is helpful.

Will

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > >
> > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > >
> > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > >
> > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > >
> > > > > > > Please use this one for any replies.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Dave
> > > > > > >
> > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > To: bpf@ietf.org
> > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > >
> > > > > > > > Dear BPF wg,
> > > > > > > >
> > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > defining them mathematically.
> > > > > >
> > > > > > Hi Watson,
> > > > > >
> > > > > > This is not "underspecified C" pseudo code.
> > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > >
> > > > > I don't see a reference to any description of that in section 4.1.
> > > > > It's possible I've overlooked this, and if people think this style of
> > > > > definition is good enough that works for me. But I found table 4
> > > > > pretty scanty on what exactly happens.
> > > >
> > > > Hello! Based on Watson's post, I have done some research and would
> > > > potentially like to offer a path forward. There are several different
> > > > ways that ISAs specify the semantics of their operations:
> > > >
> > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > IA-32 Architectures Software Developer’s Manual at
> > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > (ISA) semantics of processors."
> > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > >
> > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > proceed. If people are interested, I would be happy to lead an effort
> > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > has) and incorporate them in the draft.
> > >
> > > imo Sail is too researchy to have practical use.
> > > Looking at arm64 or x86 Sail description I really don't see how
> > > it would map to an IETF standard.
> > > It's done in a "sail" language that people need to learn first to be
> > > able to read it.
> > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > There could be other use cases, no doubt, but standardization is our goal.
> > >
> > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > to add a paragraph to describe it. We are using C to describe BPF ISA
> >
> >
> > I cannot find a reference in the current version that specifies what
> > we are using to describe the operations. I'd like to add that, but
> > want to make sure that I clarify two statements that seem to be at
> > odds.
> >
> > Immediately above you say that we are using "C to describe the BPF
> > ISA" and further above you say "This is assembly syntax parsed and
> > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> >
> > My own reading is that it is the former, and not the latter. But, I
> > want to double check before adding the appropriate statements to the
> > Convention section.
>
> It's both. I'm not sure where you see a contradiction.
> It's a normal C syntax and it's emitted by the kernel verifier,
> parsed by clang/gcc assemblers and emitted by compilers.


Okay. I apologize. I am sincerely confused. For instance,

if (u32)dst >= (u32)src goto +offset

Looks like nothing that I have ever seen in "normal C syntax".

There also appear to be a few other places where things might be a bit wonky:

1. Address arithmetic in the description of the load/store
instructions will depend on the type of the target: E.g.,

*(u64 *)(dst + offset) = imm

The address to which the store is done will be offset*sizeof(X) bytes
from dst where X is the type of the target of dst. If we are assuming
that dst (or its equivalent in similar instructions) is being treated
simply as an unsigned integer, I believe that we will have to say that
explicitly, especially given that we describe offset as "signed
integer offset used with pointer arithmetic" in the Instruction
encoding section.

2. hto[bl]eN functions are not specified by standard C and, while
"obvious" what they do, are not defined in the document anywhere.

Again, I am really sorry to be causing so much confusion. I hope that
at least some of this discussion is helpful.

Will

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Fri, Jul 28, 2023 at 5:19 PM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > >
> > > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > >
> > > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > >
> > > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > > >
> > > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > > >
> > > > > > > > Please use this one for any replies.
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Dave
> > > > > > > >
> > > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > > To: bpf@ietf.org
> > > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > > >
> > > > > > > > > Dear BPF wg,
> > > > > > > > >
> > > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > > defining them mathematically.
> > > > > > >
> > > > > > > Hi Watson,
> > > > > > >
> > > > > > > This is not "underspecified C" pseudo code.
> > > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > > >
> > > > > > I don't see a reference to any description of that in section 4.1.
> > > > > > It's possible I've overlooked this, and if people think this style of
> > > > > > definition is good enough that works for me. But I found table 4
> > > > > > pretty scanty on what exactly happens.
> > > > >
> > > > > Hello! Based on Watson's post, I have done some research and would
> > > > > potentially like to offer a path forward. There are several different
> > > > > ways that ISAs specify the semantics of their operations:
> > > > >
> > > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > > IA-32 Architectures Software Developer’s Manual at
> > > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > > (ISA) semantics of processors."
> > > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > > >
> > > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > > proceed. If people are interested, I would be happy to lead an effort
> > > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > > has) and incorporate them in the draft.
> > > >
> > > > imo Sail is too researchy to have practical use.
> > > > Looking at arm64 or x86 Sail description I really don't see how
> > > > it would map to an IETF standard.
> > > > It's done in a "sail" language that people need to learn first to be
> > > > able to read it.
> > > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > > There could be other use cases, no doubt, but standardization is our goal.
> > > >
> > > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > > to add a paragraph to describe it. We are using C to describe BPF ISA
> > >
> > >
> > > I cannot find a reference in the current version that specifies what
> > > we are using to describe the operations. I'd like to add that, but
> > > want to make sure that I clarify two statements that seem to be at
> > > odds.
> > >
> > > Immediately above you say that we are using "C to describe the BPF
> > > ISA" and further above you say "This is assembly syntax parsed and
> > > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> > >
> > > My own reading is that it is the former, and not the latter. But, I
> > > want to double check before adding the appropriate statements to the
> > > Convention section.
> >
> > It's both. I'm not sure where you see a contradiction.
> > It's a normal C syntax and it's emitted by the kernel verifier,
> > parsed by clang/gcc assemblers and emitted by compilers.
>
>
> Okay. I apologize. I am sincerely confused. For instance,
>
> if (u32)dst >= (u32)src goto +offset
>
> Looks like nothing that I have ever seen in "normal C syntax".

I thought we're talking about table 4 and ALU ops.
Above is not a pure C, but it's obvious enough without explanation, no?
Also I don't see above anywhere in the doc.
We describe conditionals like:
BPF_JGE   0x3    any  PC += offset if dst >= src

> There also appear to be a few other places where things might be a bit wonky:
>
> 1. Address arithmetic in the description of the load/store
> instructions will depend on the type of the target: E.g.,
>
> *(u64 *)(dst + offset) = imm
>
> The address to which the store is done will be offset*sizeof(X) bytes
> from dst where X is the type of the target of dst. If we are assuming
> that dst (or its equivalent in similar instructions) is being treated
> simply as an unsigned integer, I believe that we will have to say that
> explicitly, especially given that we describe offset as "signed
> integer offset used with pointer arithmetic" in the Instruction
> encoding section.

It's not:
*((u64 *)(dst) + offset) = imm

The doc doesn't say that 'dst' is a pointer 'u64 *dst' type.
Instead it says:
--
The 'code' field encodes the operation as below, where 'src' and 'dst' refer
to the values of the source and destination registers, respectively.
--

so dst + offset is a plain addition of two values and then type cast.

>
> 2. hto[bl]eN functions are not specified by standard C and, while
> "obvious" what they do, are not defined in the document anywhere.

yeah. we can add a short sentence about htoln.

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Fri, Jul 28, 2023 at 5:19 PM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > >
> > > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > >
> > > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > >
> > > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > > >
> > > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > > >
> > > > > > > > Please use this one for any replies.
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Dave
> > > > > > > >
> > > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > > To: bpf@ietf.org
> > > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > > >
> > > > > > > > > Dear BPF wg,
> > > > > > > > >
> > > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > > defining them mathematically.
> > > > > > >
> > > > > > > Hi Watson,
> > > > > > >
> > > > > > > This is not "underspecified C" pseudo code.
> > > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > > >
> > > > > > I don't see a reference to any description of that in section 4.1.
> > > > > > It's possible I've overlooked this, and if people think this style of
> > > > > > definition is good enough that works for me. But I found table 4
> > > > > > pretty scanty on what exactly happens.
> > > > >
> > > > > Hello! Based on Watson's post, I have done some research and would
> > > > > potentially like to offer a path forward. There are several different
> > > > > ways that ISAs specify the semantics of their operations:
> > > > >
> > > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > > IA-32 Architectures Software Developer’s Manual at
> > > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > > (ISA) semantics of processors."
> > > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > > >
> > > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > > proceed. If people are interested, I would be happy to lead an effort
> > > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > > has) and incorporate them in the draft.
> > > >
> > > > imo Sail is too researchy to have practical use.
> > > > Looking at arm64 or x86 Sail description I really don't see how
> > > > it would map to an IETF standard.
> > > > It's done in a "sail" language that people need to learn first to be
> > > > able to read it.
> > > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > > There could be other use cases, no doubt, but standardization is our goal.
> > > >
> > > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > > to add a paragraph to describe it. We are using C to describe BPF ISA
> > >
> > >
> > > I cannot find a reference in the current version that specifies what
> > > we are using to describe the operations. I'd like to add that, but
> > > want to make sure that I clarify two statements that seem to be at
> > > odds.
> > >
> > > Immediately above you say that we are using "C to describe the BPF
> > > ISA" and further above you say "This is assembly syntax parsed and
> > > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> > >
> > > My own reading is that it is the former, and not the latter. But, I
> > > want to double check before adding the appropriate statements to the
> > > Convention section.
> >
> > It's both. I'm not sure where you see a contradiction.
> > It's a normal C syntax and it's emitted by the kernel verifier,
> > parsed by clang/gcc assemblers and emitted by compilers.
>
>
> Okay. I apologize. I am sincerely confused. For instance,
>
> if (u32)dst >= (u32)src goto +offset
>
> Looks like nothing that I have ever seen in "normal C syntax".

I thought we're talking about table 4 and ALU ops.
Above is not a pure C, but it's obvious enough without explanation, no?
Also I don't see above anywhere in the doc.
We describe conditionals like:
BPF_JGE   0x3    any  PC += offset if dst >= src

> There also appear to be a few other places where things might be a bit wonky:
>
> 1. Address arithmetic in the description of the load/store
> instructions will depend on the type of the target: E.g.,
>
> *(u64 *)(dst + offset) = imm
>
> The address to which the store is done will be offset*sizeof(X) bytes
> from dst where X is the type of the target of dst. If we are assuming
> that dst (or its equivalent in similar instructions) is being treated
> simply as an unsigned integer, I believe that we will have to say that
> explicitly, especially given that we describe offset as "signed
> integer offset used with pointer arithmetic" in the Instruction
> encoding section.

It's not:
*((u64 *)(dst) + offset) = imm

The doc doesn't say that 'dst' is a pointer 'u64 *dst' type.
Instead it says:
--
The 'code' field encodes the operation as below, where 'src' and 'dst' refer
to the values of the source and destination registers, respectively.
--

so dst + offset is a plain addition of two values and then type cast.

>
> 2. hto[bl]eN functions are not specified by standard C and, while
> "obvious" what they do, are not defined in the document anywhere.

yeah. we can add a short sentence about htoln.

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

On Fri, Jul 28, 2023 at 8:35 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Jul 28, 2023 at 5:19 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > >
> > > > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > >
> > > > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > >
> > > > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > > > >
> > > > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > > > >
> > > > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > > > >
> > > > > > > > > Please use this one for any replies.
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > Dave
> > > > > > > > >
> > > > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > > > To: bpf@ietf.org
> > > > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > > > >
> > > > > > > > > > Dear BPF wg,
> > > > > > > > > >
> > > > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > > > defining them mathematically.
> > > > > > > >
> > > > > > > > Hi Watson,
> > > > > > > >
> > > > > > > > This is not "underspecified C" pseudo code.
> > > > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > > > >
> > > > > > > I don't see a reference to any description of that in section 4.1.
> > > > > > > It's possible I've overlooked this, and if people think this style of
> > > > > > > definition is good enough that works for me. But I found table 4
> > > > > > > pretty scanty on what exactly happens.
> > > > > >
> > > > > > Hello! Based on Watson's post, I have done some research and would
> > > > > > potentially like to offer a path forward. There are several different
> > > > > > ways that ISAs specify the semantics of their operations:
> > > > > >
> > > > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > > > IA-32 Architectures Software Developer’s Manual at
> > > > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > > > (ISA) semantics of processors."
> > > > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > > > >
> > > > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > > > proceed. If people are interested, I would be happy to lead an effort
> > > > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > > > has) and incorporate them in the draft.
> > > > >
> > > > > imo Sail is too researchy to have practical use.
> > > > > Looking at arm64 or x86 Sail description I really don't see how
> > > > > it would map to an IETF standard.
> > > > > It's done in a "sail" language that people need to learn first to be
> > > > > able to read it.
> > > > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > > > There could be other use cases, no doubt, but standardization is our goal.
> > > > >
> > > > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > > > to add a paragraph to describe it. We are using C to describe BPF ISA
> > > >
> > > >
> > > > I cannot find a reference in the current version that specifies what
> > > > we are using to describe the operations. I'd like to add that, but
> > > > want to make sure that I clarify two statements that seem to be at
> > > > odds.
> > > >
> > > > Immediately above you say that we are using "C to describe the BPF
> > > > ISA" and further above you say "This is assembly syntax parsed and
> > > > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> > > >
> > > > My own reading is that it is the former, and not the latter. But, I
> > > > want to double check before adding the appropriate statements to the
> > > > Convention section.
> > >
> > > It's both. I'm not sure where you see a contradiction.
> > > It's a normal C syntax and it's emitted by the kernel verifier,
> > > parsed by clang/gcc assemblers and emitted by compilers.
> >
> >
> > Okay. I apologize. I am sincerely confused. For instance,
> >
> > if (u32)dst >= (u32)src goto +offset
> >
> > Looks like nothing that I have ever seen in "normal C syntax".
>
> I thought we're talking about table 4 and ALU ops.
> Above is not a pure C, but it's obvious enough without explanation, no?

To "us", yes. Although I am not an expert, it seems like being
explicit is important when it comes to writing a spec. I suppose we
should leave that to Dave and the chairs.

> Also I don't see above anywhere in the doc.

That is from the Appendix. It is currently in Dave's tree and gets
amalgamated with other files to build the final draft.

https://datatracker.ietf.org/doc/draft-thaler-bpf-isa/

> We describe conditionals like:
> BPF_JGE   0x3    any  PC += offset if dst >= src
>
> > There also appear to be a few other places where things might be a bit wonky:
> >
> > 1. Address arithmetic in the description of the load/store
> > instructions will depend on the type of the target: E.g.,
> >
> > *(u64 *)(dst + offset) = imm
> >
> > The address to which the store is done will be offset*sizeof(X) bytes
> > from dst where X is the type of the target of dst. If we are assuming
> > that dst (or its equivalent in similar instructions) is being treated
> > simply as an unsigned integer, I believe that we will have to say that
> > explicitly, especially given that we describe offset as "signed
> > integer offset used with pointer arithmetic" in the Instruction
> > encoding section.
>
> It's not:
> *((u64 *)(dst) + offset) = imm
>
> The doc doesn't say that 'dst' is a pointer 'u64 *dst' type.
> Instead it says:
> --
> The 'code' field encodes the operation as below, where 'src' and 'dst' refer
> to the values of the source and destination registers, respectively.
> --
>
> so dst + offset is a plain addition of two values and then type cast.

Again I of course understand and "we" know what that means. However,
it seems to me that an earlier description of offset as "signed
integer offset used with pointer arithmetic" might signal something
else to an unfamiliar reader.

Will

>
> >
> > 2. hto[bl]eN functions are not specified by standard C and, while
> > "obvious" what they do, are not defined in the document anywhere.
>
> yeah. we can add a short sentence about htoln.

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

On Fri, Jul 28, 2023 at 8:35 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Jul 28, 2023 at 5:19 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > >
> > > > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > >
> > > > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > >
> > > > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > > > >
> > > > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > > > >
> > > > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > > > >
> > > > > > > > > Please use this one for any replies.
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > Dave
> > > > > > > > >
> > > > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > > > To: bpf@ietf.org
> > > > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > > > >
> > > > > > > > > > Dear BPF wg,
> > > > > > > > > >
> > > > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > > > defining them mathematically.
> > > > > > > >
> > > > > > > > Hi Watson,
> > > > > > > >
> > > > > > > > This is not "underspecified C" pseudo code.
> > > > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > > > >
> > > > > > > I don't see a reference to any description of that in section 4.1.
> > > > > > > It's possible I've overlooked this, and if people think this style of
> > > > > > > definition is good enough that works for me. But I found table 4
> > > > > > > pretty scanty on what exactly happens.
> > > > > >
> > > > > > Hello! Based on Watson's post, I have done some research and would
> > > > > > potentially like to offer a path forward. There are several different
> > > > > > ways that ISAs specify the semantics of their operations:
> > > > > >
> > > > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > > > IA-32 Architectures Software Developer’s Manual at
> > > > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > > > (ISA) semantics of processors."
> > > > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > > > >
> > > > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > > > proceed. If people are interested, I would be happy to lead an effort
> > > > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > > > has) and incorporate them in the draft.
> > > > >
> > > > > imo Sail is too researchy to have practical use.
> > > > > Looking at arm64 or x86 Sail description I really don't see how
> > > > > it would map to an IETF standard.
> > > > > It's done in a "sail" language that people need to learn first to be
> > > > > able to read it.
> > > > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > > > There could be other use cases, no doubt, but standardization is our goal.
> > > > >
> > > > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > > > to add a paragraph to describe it. We are using C to describe BPF ISA
> > > >
> > > >
> > > > I cannot find a reference in the current version that specifies what
> > > > we are using to describe the operations. I'd like to add that, but
> > > > want to make sure that I clarify two statements that seem to be at
> > > > odds.
> > > >
> > > > Immediately above you say that we are using "C to describe the BPF
> > > > ISA" and further above you say "This is assembly syntax parsed and
> > > > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> > > >
> > > > My own reading is that it is the former, and not the latter. But, I
> > > > want to double check before adding the appropriate statements to the
> > > > Convention section.
> > >
> > > It's both. I'm not sure where you see a contradiction.
> > > It's a normal C syntax and it's emitted by the kernel verifier,
> > > parsed by clang/gcc assemblers and emitted by compilers.
> >
> >
> > Okay. I apologize. I am sincerely confused. For instance,
> >
> > if (u32)dst >= (u32)src goto +offset
> >
> > Looks like nothing that I have ever seen in "normal C syntax".
>
> I thought we're talking about table 4 and ALU ops.
> Above is not a pure C, but it's obvious enough without explanation, no?

To "us", yes. Although I am not an expert, it seems like being
explicit is important when it comes to writing a spec. I suppose we
should leave that to Dave and the chairs.

> Also I don't see above anywhere in the doc.

That is from the Appendix. It is currently in Dave's tree and gets
amalgamated with other files to build the final draft.

https://datatracker.ietf.org/doc/draft-thaler-bpf-isa/

> We describe conditionals like:
> BPF_JGE   0x3    any  PC += offset if dst >= src
>
> > There also appear to be a few other places where things might be a bit wonky:
> >
> > 1. Address arithmetic in the description of the load/store
> > instructions will depend on the type of the target: E.g.,
> >
> > *(u64 *)(dst + offset) = imm
> >
> > The address to which the store is done will be offset*sizeof(X) bytes
> > from dst where X is the type of the target of dst. If we are assuming
> > that dst (or its equivalent in similar instructions) is being treated
> > simply as an unsigned integer, I believe that we will have to say that
> > explicitly, especially given that we describe offset as "signed
> > integer offset used with pointer arithmetic" in the Instruction
> > encoding section.
>
> It's not:
> *((u64 *)(dst) + offset) = imm
>
> The doc doesn't say that 'dst' is a pointer 'u64 *dst' type.
> Instead it says:
> --
> The 'code' field encodes the operation as below, where 'src' and 'dst' refer
> to the values of the source and destination registers, respectively.
> --
>
> so dst + offset is a plain addition of two values and then type cast.

Again I of course understand and "we" know what that means. However,
it seems to me that an earlier description of offset as "signed
integer offset used with pointer arithmetic" might signal something
else to an unfamiliar reader.

Will

>
> >
> > 2. hto[bl]eN functions are not specified by standard C and, while
> > "obvious" what they do, are not defined in the document anywhere.
>
> yeah. we can add a short sentence about htoln.

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Fri, Jul 28, 2023 at 5:46 PM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> On Fri, Jul 28, 2023 at 8:35 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Jul 28, 2023 at 5:19 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > >
> > > On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > >
> > > > > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > >
> > > > > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > > >
> > > > > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > > > > >
> > > > > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > > > > >
> > > > > > > > > > Please use this one for any replies.
> > > > > > > > > >
> > > > > > > > > > Thanks,
> > > > > > > > > > Dave
> > > > > > > > > >
> > > > > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > > > > To: bpf@ietf.org
> > > > > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > > > > >
> > > > > > > > > > > Dear BPF wg,
> > > > > > > > > > >
> > > > > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > > > > defining them mathematically.
> > > > > > > > >
> > > > > > > > > Hi Watson,
> > > > > > > > >
> > > > > > > > > This is not "underspecified C" pseudo code.
> > > > > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > > > > >
> > > > > > > > I don't see a reference to any description of that in section 4.1.
> > > > > > > > It's possible I've overlooked this, and if people think this style of
> > > > > > > > definition is good enough that works for me. But I found table 4
> > > > > > > > pretty scanty on what exactly happens.
> > > > > > >
> > > > > > > Hello! Based on Watson's post, I have done some research and would
> > > > > > > potentially like to offer a path forward. There are several different
> > > > > > > ways that ISAs specify the semantics of their operations:
> > > > > > >
> > > > > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > > > > IA-32 Architectures Software Developer’s Manual at
> > > > > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > > > > (ISA) semantics of processors."
> > > > > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > > > > >
> > > > > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > > > > proceed. If people are interested, I would be happy to lead an effort
> > > > > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > > > > has) and incorporate them in the draft.
> > > > > >
> > > > > > imo Sail is too researchy to have practical use.
> > > > > > Looking at arm64 or x86 Sail description I really don't see how
> > > > > > it would map to an IETF standard.
> > > > > > It's done in a "sail" language that people need to learn first to be
> > > > > > able to read it.
> > > > > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > > > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > > > > There could be other use cases, no doubt, but standardization is our goal.
> > > > > >
> > > > > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > > > > to add a paragraph to describe it. We are using C to describe BPF ISA
> > > > >
> > > > >
> > > > > I cannot find a reference in the current version that specifies what
> > > > > we are using to describe the operations. I'd like to add that, but
> > > > > want to make sure that I clarify two statements that seem to be at
> > > > > odds.
> > > > >
> > > > > Immediately above you say that we are using "C to describe the BPF
> > > > > ISA" and further above you say "This is assembly syntax parsed and
> > > > > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> > > > >
> > > > > My own reading is that it is the former, and not the latter. But, I
> > > > > want to double check before adding the appropriate statements to the
> > > > > Convention section.
> > > >
> > > > It's both. I'm not sure where you see a contradiction.
> > > > It's a normal C syntax and it's emitted by the kernel verifier,
> > > > parsed by clang/gcc assemblers and emitted by compilers.
> > >
> > >
> > > Okay. I apologize. I am sincerely confused. For instance,
> > >
> > > if (u32)dst >= (u32)src goto +offset
> > >
> > > Looks like nothing that I have ever seen in "normal C syntax".
> >
> > I thought we're talking about table 4 and ALU ops.
> > Above is not a pure C, but it's obvious enough without explanation, no?
>
> To "us", yes. Although I am not an expert, it seems like being
> explicit is important when it comes to writing a spec. I suppose we
> should leave that to Dave and the chairs.
>
> > Also I don't see above anywhere in the doc.
>
> That is from the Appendix. It is currently in Dave's tree and gets
> amalgamated with other files to build the final draft.
>
> https://datatracker.ietf.org/doc/draft-thaler-bpf-isa/

This is a mirror and it's already outdated.
You should look at the source. Which is git kernel tree.

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Fri, Jul 28, 2023 at 5:46 PM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> On Fri, Jul 28, 2023 at 8:35 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Jul 28, 2023 at 5:19 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > >
> > > On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > >
> > > > > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > >
> > > > > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > > >
> > > > > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > > > > >
> > > > > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > > > > >
> > > > > > > > > > Please use this one for any replies.
> > > > > > > > > >
> > > > > > > > > > Thanks,
> > > > > > > > > > Dave
> > > > > > > > > >
> > > > > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > > > > To: bpf@ietf.org
> > > > > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > > > > >
> > > > > > > > > > > Dear BPF wg,
> > > > > > > > > > >
> > > > > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > > > > defining them mathematically.
> > > > > > > > >
> > > > > > > > > Hi Watson,
> > > > > > > > >
> > > > > > > > > This is not "underspecified C" pseudo code.
> > > > > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > > > > >
> > > > > > > > I don't see a reference to any description of that in section 4.1.
> > > > > > > > It's possible I've overlooked this, and if people think this style of
> > > > > > > > definition is good enough that works for me. But I found table 4
> > > > > > > > pretty scanty on what exactly happens.
> > > > > > >
> > > > > > > Hello! Based on Watson's post, I have done some research and would
> > > > > > > potentially like to offer a path forward. There are several different
> > > > > > > ways that ISAs specify the semantics of their operations:
> > > > > > >
> > > > > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > > > > IA-32 Architectures Software Developer’s Manual at
> > > > > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > > > > (ISA) semantics of processors."
> > > > > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > > > > >
> > > > > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > > > > proceed. If people are interested, I would be happy to lead an effort
> > > > > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > > > > has) and incorporate them in the draft.
> > > > > >
> > > > > > imo Sail is too researchy to have practical use.
> > > > > > Looking at arm64 or x86 Sail description I really don't see how
> > > > > > it would map to an IETF standard.
> > > > > > It's done in a "sail" language that people need to learn first to be
> > > > > > able to read it.
> > > > > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > > > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > > > > There could be other use cases, no doubt, but standardization is our goal.
> > > > > >
> > > > > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > > > > to add a paragraph to describe it. We are using C to describe BPF ISA
> > > > >
> > > > >
> > > > > I cannot find a reference in the current version that specifies what
> > > > > we are using to describe the operations. I'd like to add that, but
> > > > > want to make sure that I clarify two statements that seem to be at
> > > > > odds.
> > > > >
> > > > > Immediately above you say that we are using "C to describe the BPF
> > > > > ISA" and further above you say "This is assembly syntax parsed and
> > > > > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> > > > >
> > > > > My own reading is that it is the former, and not the latter. But, I
> > > > > want to double check before adding the appropriate statements to the
> > > > > Convention section.
> > > >
> > > > It's both. I'm not sure where you see a contradiction.
> > > > It's a normal C syntax and it's emitted by the kernel verifier,
> > > > parsed by clang/gcc assemblers and emitted by compilers.
> > >
> > >
> > > Okay. I apologize. I am sincerely confused. For instance,
> > >
> > > if (u32)dst >= (u32)src goto +offset
> > >
> > > Looks like nothing that I have ever seen in "normal C syntax".
> >
> > I thought we're talking about table 4 and ALU ops.
> > Above is not a pure C, but it's obvious enough without explanation, no?
>
> To "us", yes. Although I am not an expert, it seems like being
> explicit is important when it comes to writing a spec. I suppose we
> should leave that to Dave and the chairs.
>
> > Also I don't see above anywhere in the doc.
>
> That is from the Appendix. It is currently in Dave's tree and gets
> amalgamated with other files to build the final draft.
>
> https://datatracker.ietf.org/doc/draft-thaler-bpf-isa/

This is a mirror and it's already outdated.
You should look at the source. Which is git kernel tree.

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

On Fri, Jul 28, 2023 at 8:52 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Jul 28, 2023 at 5:46 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > On Fri, Jul 28, 2023 at 8:35 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Fri, Jul 28, 2023 at 5:19 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > >
> > > > On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > >
> > > > > On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > >
> > > > > > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > >
> > > > > > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > > > >
> > > > > > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > > > > > >
> > > > > > > > > > > Please use this one for any replies.
> > > > > > > > > > >
> > > > > > > > > > > Thanks,
> > > > > > > > > > > Dave
> > > > > > > > > > >
> > > > > > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > > > > > To: bpf@ietf.org
> > > > > > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > > > > > >
> > > > > > > > > > > > Dear BPF wg,
> > > > > > > > > > > >
> > > > > > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > > > > > defining them mathematically.
> > > > > > > > > >
> > > > > > > > > > Hi Watson,
> > > > > > > > > >
> > > > > > > > > > This is not "underspecified C" pseudo code.
> > > > > > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > > > > > >
> > > > > > > > > I don't see a reference to any description of that in section 4.1.
> > > > > > > > > It's possible I've overlooked this, and if people think this style of
> > > > > > > > > definition is good enough that works for me. But I found table 4
> > > > > > > > > pretty scanty on what exactly happens.
> > > > > > > >
> > > > > > > > Hello! Based on Watson's post, I have done some research and would
> > > > > > > > potentially like to offer a path forward. There are several different
> > > > > > > > ways that ISAs specify the semantics of their operations:
> > > > > > > >
> > > > > > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > > > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > > > > > IA-32 Architectures Software Developer’s Manual at
> > > > > > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > > > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > > > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > > > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > > > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > > > > > (ISA) semantics of processors."
> > > > > > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > > > > > >
> > > > > > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > > > > > proceed. If people are interested, I would be happy to lead an effort
> > > > > > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > > > > > has) and incorporate them in the draft.
> > > > > > >
> > > > > > > imo Sail is too researchy to have practical use.
> > > > > > > Looking at arm64 or x86 Sail description I really don't see how
> > > > > > > it would map to an IETF standard.
> > > > > > > It's done in a "sail" language that people need to learn first to be
> > > > > > > able to read it.
> > > > > > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > > > > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > > > > > There could be other use cases, no doubt, but standardization is our goal.
> > > > > > >
> > > > > > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > > > > > to add a paragraph to describe it. We are using C to describe BPF ISA
> > > > > >
> > > > > >
> > > > > > I cannot find a reference in the current version that specifies what
> > > > > > we are using to describe the operations. I'd like to add that, but
> > > > > > want to make sure that I clarify two statements that seem to be at
> > > > > > odds.
> > > > > >
> > > > > > Immediately above you say that we are using "C to describe the BPF
> > > > > > ISA" and further above you say "This is assembly syntax parsed and
> > > > > > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> > > > > >
> > > > > > My own reading is that it is the former, and not the latter. But, I
> > > > > > want to double check before adding the appropriate statements to the
> > > > > > Convention section.
> > > > >
> > > > > It's both. I'm not sure where you see a contradiction.
> > > > > It's a normal C syntax and it's emitted by the kernel verifier,
> > > > > parsed by clang/gcc assemblers and emitted by compilers.
> > > >
> > > >
> > > > Okay. I apologize. I am sincerely confused. For instance,
> > > >
> > > > if (u32)dst >= (u32)src goto +offset
> > > >
> > > > Looks like nothing that I have ever seen in "normal C syntax".
> > >
> > > I thought we're talking about table 4 and ALU ops.
> > > Above is not a pure C, but it's obvious enough without explanation, no?
> >
> > To "us", yes. Although I am not an expert, it seems like being
> > explicit is important when it comes to writing a spec. I suppose we
> > should leave that to Dave and the chairs.
> >
> > > Also I don't see above anywhere in the doc.
> >
> > That is from the Appendix. It is currently in Dave's tree and gets
> > amalgamated with other files to build the final draft.
> >
> > https://datatracker.ietf.org/doc/draft-thaler-bpf-isa/
>
> This is a mirror and it's already outdated.
> You should look at the source. Which is git kernel tree.

As he discussed at the meeting, he has the github workflow that
produces a version of the draft RFC that he will submit to the WG:

https://github.com/ietf-wg-bpf/ebpf-docs/blob/update/.github/workflows/build.yml

That uses

https://github.com/ietf-wg-bpf/ebpf-docs/blob/main/rst/instruction-set-skeleton.rst

to build in the acknowledgements and subsequently brings in that
Appendix. If he plans to take that out, then that's great. I was just
trying to help. Sorry.

Will

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

On Fri, Jul 28, 2023 at 8:52 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Jul 28, 2023 at 5:46 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > On Fri, Jul 28, 2023 at 8:35 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Fri, Jul 28, 2023 at 5:19 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > >
> > > > On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > >
> > > > > On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > >
> > > > > > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > >
> > > > > > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > > > >
> > > > > > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > > > > > >
> > > > > > > > > > > Please use this one for any replies.
> > > > > > > > > > >
> > > > > > > > > > > Thanks,
> > > > > > > > > > > Dave
> > > > > > > > > > >
> > > > > > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > > > > > To: bpf@ietf.org
> > > > > > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > > > > > >
> > > > > > > > > > > > Dear BPF wg,
> > > > > > > > > > > >
> > > > > > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > > > > > defining them mathematically.
> > > > > > > > > >
> > > > > > > > > > Hi Watson,
> > > > > > > > > >
> > > > > > > > > > This is not "underspecified C" pseudo code.
> > > > > > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > > > > > >
> > > > > > > > > I don't see a reference to any description of that in section 4.1.
> > > > > > > > > It's possible I've overlooked this, and if people think this style of
> > > > > > > > > definition is good enough that works for me. But I found table 4
> > > > > > > > > pretty scanty on what exactly happens.
> > > > > > > >
> > > > > > > > Hello! Based on Watson's post, I have done some research and would
> > > > > > > > potentially like to offer a path forward. There are several different
> > > > > > > > ways that ISAs specify the semantics of their operations:
> > > > > > > >
> > > > > > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > > > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > > > > > IA-32 Architectures Software Developer’s Manual at
> > > > > > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > > > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > > > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > > > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > > > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > > > > > (ISA) semantics of processors."
> > > > > > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > > > > > >
> > > > > > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > > > > > proceed. If people are interested, I would be happy to lead an effort
> > > > > > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > > > > > has) and incorporate them in the draft.
> > > > > > >
> > > > > > > imo Sail is too researchy to have practical use.
> > > > > > > Looking at arm64 or x86 Sail description I really don't see how
> > > > > > > it would map to an IETF standard.
> > > > > > > It's done in a "sail" language that people need to learn first to be
> > > > > > > able to read it.
> > > > > > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > > > > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > > > > > There could be other use cases, no doubt, but standardization is our goal.
> > > > > > >
> > > > > > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > > > > > to add a paragraph to describe it. We are using C to describe BPF ISA
> > > > > >
> > > > > >
> > > > > > I cannot find a reference in the current version that specifies what
> > > > > > we are using to describe the operations. I'd like to add that, but
> > > > > > want to make sure that I clarify two statements that seem to be at
> > > > > > odds.
> > > > > >
> > > > > > Immediately above you say that we are using "C to describe the BPF
> > > > > > ISA" and further above you say "This is assembly syntax parsed and
> > > > > > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> > > > > >
> > > > > > My own reading is that it is the former, and not the latter. But, I
> > > > > > want to double check before adding the appropriate statements to the
> > > > > > Convention section.
> > > > >
> > > > > It's both. I'm not sure where you see a contradiction.
> > > > > It's a normal C syntax and it's emitted by the kernel verifier,
> > > > > parsed by clang/gcc assemblers and emitted by compilers.
> > > >
> > > >
> > > > Okay. I apologize. I am sincerely confused. For instance,
> > > >
> > > > if (u32)dst >= (u32)src goto +offset
> > > >
> > > > Looks like nothing that I have ever seen in "normal C syntax".
> > >
> > > I thought we're talking about table 4 and ALU ops.
> > > Above is not a pure C, but it's obvious enough without explanation, no?
> >
> > To "us", yes. Although I am not an expert, it seems like being
> > explicit is important when it comes to writing a spec. I suppose we
> > should leave that to Dave and the chairs.
> >
> > > Also I don't see above anywhere in the doc.
> >
> > That is from the Appendix. It is currently in Dave's tree and gets
> > amalgamated with other files to build the final draft.
> >
> > https://datatracker.ietf.org/doc/draft-thaler-bpf-isa/
>
> This is a mirror and it's already outdated.
> You should look at the source. Which is git kernel tree.

As he discussed at the meeting, he has the github workflow that
produces a version of the draft RFC that he will submit to the WG:

https://github.com/ietf-wg-bpf/ebpf-docs/blob/update/.github/workflows/build.yml

That uses

https://github.com/ietf-wg-bpf/ebpf-docs/blob/main/rst/instruction-set-skeleton.rst

to build in the acknowledgements and subsequently brings in that
Appendix. If he plans to take that out, then that's great. I was just
trying to help. Sorry.

Will

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Fri, Jul 28, 2023 at 6:07 PM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> On Fri, Jul 28, 2023 at 8:52 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Jul 28, 2023 at 5:46 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > >
> > > On Fri, Jul 28, 2023 at 8:35 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Fri, Jul 28, 2023 at 5:19 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > >
> > > > > On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
> > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > >
> > > > > > On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > > >
> > > > > > > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > > > > > > >
> > > > > > > > > > > > Please use this one for any replies.
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks,
> > > > > > > > > > > > Dave
> > > > > > > > > > > >
> > > > > > > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > > > > > > To: bpf@ietf.org
> > > > > > > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > > > > > > >
> > > > > > > > > > > > > Dear BPF wg,
> > > > > > > > > > > > >
> > > > > > > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > > > > > > defining them mathematically.
> > > > > > > > > > >
> > > > > > > > > > > Hi Watson,
> > > > > > > > > > >
> > > > > > > > > > > This is not "underspecified C" pseudo code.
> > > > > > > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > > > > > > >
> > > > > > > > > > I don't see a reference to any description of that in section 4.1.
> > > > > > > > > > It's possible I've overlooked this, and if people think this style of
> > > > > > > > > > definition is good enough that works for me. But I found table 4
> > > > > > > > > > pretty scanty on what exactly happens.
> > > > > > > > >
> > > > > > > > > Hello! Based on Watson's post, I have done some research and would
> > > > > > > > > potentially like to offer a path forward. There are several different
> > > > > > > > > ways that ISAs specify the semantics of their operations:
> > > > > > > > >
> > > > > > > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > > > > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > > > > > > IA-32 Architectures Software Developer’s Manual at
> > > > > > > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > > > > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > > > > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > > > > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > > > > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > > > > > > (ISA) semantics of processors."
> > > > > > > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > > > > > > >
> > > > > > > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > > > > > > proceed. If people are interested, I would be happy to lead an effort
> > > > > > > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > > > > > > has) and incorporate them in the draft.
> > > > > > > >
> > > > > > > > imo Sail is too researchy to have practical use.
> > > > > > > > Looking at arm64 or x86 Sail description I really don't see how
> > > > > > > > it would map to an IETF standard.
> > > > > > > > It's done in a "sail" language that people need to learn first to be
> > > > > > > > able to read it.
> > > > > > > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > > > > > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > > > > > > There could be other use cases, no doubt, but standardization is our goal.
> > > > > > > >
> > > > > > > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > > > > > > to add a paragraph to describe it. We are using C to describe BPF ISA
> > > > > > >
> > > > > > >
> > > > > > > I cannot find a reference in the current version that specifies what
> > > > > > > we are using to describe the operations. I'd like to add that, but
> > > > > > > want to make sure that I clarify two statements that seem to be at
> > > > > > > odds.
> > > > > > >
> > > > > > > Immediately above you say that we are using "C to describe the BPF
> > > > > > > ISA" and further above you say "This is assembly syntax parsed and
> > > > > > > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> > > > > > >
> > > > > > > My own reading is that it is the former, and not the latter. But, I
> > > > > > > want to double check before adding the appropriate statements to the
> > > > > > > Convention section.
> > > > > >
> > > > > > It's both. I'm not sure where you see a contradiction.
> > > > > > It's a normal C syntax and it's emitted by the kernel verifier,
> > > > > > parsed by clang/gcc assemblers and emitted by compilers.
> > > > >
> > > > >
> > > > > Okay. I apologize. I am sincerely confused. For instance,
> > > > >
> > > > > if (u32)dst >= (u32)src goto +offset
> > > > >
> > > > > Looks like nothing that I have ever seen in "normal C syntax".
> > > >
> > > > I thought we're talking about table 4 and ALU ops.
> > > > Above is not a pure C, but it's obvious enough without explanation, no?
> > >
> > > To "us", yes. Although I am not an expert, it seems like being
> > > explicit is important when it comes to writing a spec. I suppose we
> > > should leave that to Dave and the chairs.
> > >
> > > > Also I don't see above anywhere in the doc.
> > >
> > > That is from the Appendix. It is currently in Dave's tree and gets
> > > amalgamated with other files to build the final draft.
> > >
> > > https://datatracker.ietf.org/doc/draft-thaler-bpf-isa/
> >
> > This is a mirror and it's already outdated.
> > You should look at the source. Which is git kernel tree.
>
> As he discussed at the meeting, he has the github workflow that
> produces a version of the draft RFC that he will submit to the WG:
>
> https://github.com/ietf-wg-bpf/ebpf-docs/blob/update/.github/workflows/build.yml
>
> That uses
>
> https://github.com/ietf-wg-bpf/ebpf-docs/blob/main/rst/instruction-set-skeleton.rst

correct.

> to build in the acknowledgements and subsequently brings in that
> Appendix.

correct.

> If he plans to take that out, then that's great. I was just
> trying to help. Sorry.

No. That workflow will stay.
The future changes to RFC will be in the form of patches to
instruction-set-skeleton.rst. Once they land the RFC will be
regenerated.
We can regenerate RFC as often as we like.

All I'm saying is that RFC has bugs that were already fixed in
instruction-set-skeleton.rst. Hence it's outdated.

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Fri, Jul 28, 2023 at 6:07 PM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> On Fri, Jul 28, 2023 at 8:52 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Jul 28, 2023 at 5:46 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > >
> > > On Fri, Jul 28, 2023 at 8:35 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > >
> > > > On Fri, Jul 28, 2023 at 5:19 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > >
> > > > > On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
> > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > >
> > > > > > On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > > >
> > > > > > > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > > > > > > >
> > > > > > > > > > > > Please use this one for any replies.
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks,
> > > > > > > > > > > > Dave
> > > > > > > > > > > >
> > > > > > > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > > > > > > To: bpf@ietf.org
> > > > > > > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > > > > > > >
> > > > > > > > > > > > > Dear BPF wg,
> > > > > > > > > > > > >
> > > > > > > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > > > > > > defining them mathematically.
> > > > > > > > > > >
> > > > > > > > > > > Hi Watson,
> > > > > > > > > > >
> > > > > > > > > > > This is not "underspecified C" pseudo code.
> > > > > > > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > > > > > > >
> > > > > > > > > > I don't see a reference to any description of that in section 4.1.
> > > > > > > > > > It's possible I've overlooked this, and if people think this style of
> > > > > > > > > > definition is good enough that works for me. But I found table 4
> > > > > > > > > > pretty scanty on what exactly happens.
> > > > > > > > >
> > > > > > > > > Hello! Based on Watson's post, I have done some research and would
> > > > > > > > > potentially like to offer a path forward. There are several different
> > > > > > > > > ways that ISAs specify the semantics of their operations:
> > > > > > > > >
> > > > > > > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > > > > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > > > > > > IA-32 Architectures Software Developer’s Manual at
> > > > > > > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > > > > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > > > > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > > > > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > > > > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > > > > > > (ISA) semantics of processors."
> > > > > > > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > > > > > > >
> > > > > > > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > > > > > > proceed. If people are interested, I would be happy to lead an effort
> > > > > > > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > > > > > > has) and incorporate them in the draft.
> > > > > > > >
> > > > > > > > imo Sail is too researchy to have practical use.
> > > > > > > > Looking at arm64 or x86 Sail description I really don't see how
> > > > > > > > it would map to an IETF standard.
> > > > > > > > It's done in a "sail" language that people need to learn first to be
> > > > > > > > able to read it.
> > > > > > > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > > > > > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > > > > > > There could be other use cases, no doubt, but standardization is our goal.
> > > > > > > >
> > > > > > > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > > > > > > to add a paragraph to describe it. We are using C to describe BPF ISA
> > > > > > >
> > > > > > >
> > > > > > > I cannot find a reference in the current version that specifies what
> > > > > > > we are using to describe the operations. I'd like to add that, but
> > > > > > > want to make sure that I clarify two statements that seem to be at
> > > > > > > odds.
> > > > > > >
> > > > > > > Immediately above you say that we are using "C to describe the BPF
> > > > > > > ISA" and further above you say "This is assembly syntax parsed and
> > > > > > > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> > > > > > >
> > > > > > > My own reading is that it is the former, and not the latter. But, I
> > > > > > > want to double check before adding the appropriate statements to the
> > > > > > > Convention section.
> > > > > >
> > > > > > It's both. I'm not sure where you see a contradiction.
> > > > > > It's a normal C syntax and it's emitted by the kernel verifier,
> > > > > > parsed by clang/gcc assemblers and emitted by compilers.
> > > > >
> > > > >
> > > > > Okay. I apologize. I am sincerely confused. For instance,
> > > > >
> > > > > if (u32)dst >= (u32)src goto +offset
> > > > >
> > > > > Looks like nothing that I have ever seen in "normal C syntax".
> > > >
> > > > I thought we're talking about table 4 and ALU ops.
> > > > Above is not a pure C, but it's obvious enough without explanation, no?
> > >
> > > To "us", yes. Although I am not an expert, it seems like being
> > > explicit is important when it comes to writing a spec. I suppose we
> > > should leave that to Dave and the chairs.
> > >
> > > > Also I don't see above anywhere in the doc.
> > >
> > > That is from the Appendix. It is currently in Dave's tree and gets
> > > amalgamated with other files to build the final draft.
> > >
> > > https://datatracker.ietf.org/doc/draft-thaler-bpf-isa/
> >
> > This is a mirror and it's already outdated.
> > You should look at the source. Which is git kernel tree.
>
> As he discussed at the meeting, he has the github workflow that
> produces a version of the draft RFC that he will submit to the WG:
>
> https://github.com/ietf-wg-bpf/ebpf-docs/blob/update/.github/workflows/build.yml
>
> That uses
>
> https://github.com/ietf-wg-bpf/ebpf-docs/blob/main/rst/instruction-set-skeleton.rst

correct.

> to build in the acknowledgements and subsequently brings in that
> Appendix.

correct.

> If he plans to take that out, then that's great. I was just
> trying to help. Sorry.

No. That workflow will stay.
The future changes to RFC will be in the form of patches to
instruction-set-skeleton.rst. Once they land the RFC will be
regenerated.
We can regenerate RFC as often as we like.

All I'm saying is that RFC has bugs that were already fixed in
instruction-set-skeleton.rst. Hence it's outdated.

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

On Fri, Jul 28, 2023 at 10:32 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Jul 28, 2023 at 6:07 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > On Fri, Jul 28, 2023 at 8:52 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Fri, Jul 28, 2023 at 5:46 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > >
> > > > On Fri, Jul 28, 2023 at 8:35 PM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > >
> > > > > On Fri, Jul 28, 2023 at 5:19 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > >
> > > > > > On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
> > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > >
> > > > > > > On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > > > >
> > > > > > > > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Please use this one for any replies.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > Dave
> > > > > > > > > > > > >
> > > > > > > > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > > > > > > > To: bpf@ietf.org
> > > > > > > > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Dear BPF wg,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > > > > > > > defining them mathematically.
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Watson,
> > > > > > > > > > > >
> > > > > > > > > > > > This is not "underspecified C" pseudo code.
> > > > > > > > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > > > > > > > >
> > > > > > > > > > > I don't see a reference to any description of that in section 4.1.
> > > > > > > > > > > It's possible I've overlooked this, and if people think this style of
> > > > > > > > > > > definition is good enough that works for me. But I found table 4
> > > > > > > > > > > pretty scanty on what exactly happens.
> > > > > > > > > >
> > > > > > > > > > Hello! Based on Watson's post, I have done some research and would
> > > > > > > > > > potentially like to offer a path forward. There are several different
> > > > > > > > > > ways that ISAs specify the semantics of their operations:
> > > > > > > > > >
> > > > > > > > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > > > > > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > > > > > > > IA-32 Architectures Software Developer’s Manual at
> > > > > > > > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > > > > > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > > > > > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > > > > > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > > > > > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > > > > > > > (ISA) semantics of processors."
> > > > > > > > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > > > > > > > >
> > > > > > > > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > > > > > > > proceed. If people are interested, I would be happy to lead an effort
> > > > > > > > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > > > > > > > has) and incorporate them in the draft.
> > > > > > > > >
> > > > > > > > > imo Sail is too researchy to have practical use.
> > > > > > > > > Looking at arm64 or x86 Sail description I really don't see how
> > > > > > > > > it would map to an IETF standard.
> > > > > > > > > It's done in a "sail" language that people need to learn first to be
> > > > > > > > > able to read it.
> > > > > > > > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > > > > > > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > > > > > > > There could be other use cases, no doubt, but standardization is our goal.
> > > > > > > > >
> > > > > > > > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > > > > > > > to add a paragraph to describe it. We are using C to describe BPF ISA
> > > > > > > >
> > > > > > > >
> > > > > > > > I cannot find a reference in the current version that specifies what
> > > > > > > > we are using to describe the operations. I'd like to add that, but
> > > > > > > > want to make sure that I clarify two statements that seem to be at
> > > > > > > > odds.
> > > > > > > >
> > > > > > > > Immediately above you say that we are using "C to describe the BPF
> > > > > > > > ISA" and further above you say "This is assembly syntax parsed and
> > > > > > > > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> > > > > > > >
> > > > > > > > My own reading is that it is the former, and not the latter. But, I
> > > > > > > > want to double check before adding the appropriate statements to the
> > > > > > > > Convention section.
> > > > > > >
> > > > > > > It's both. I'm not sure where you see a contradiction.
> > > > > > > It's a normal C syntax and it's emitted by the kernel verifier,
> > > > > > > parsed by clang/gcc assemblers and emitted by compilers.
> > > > > >
> > > > > >
> > > > > > Okay. I apologize. I am sincerely confused. For instance,
> > > > > >
> > > > > > if (u32)dst >= (u32)src goto +offset
> > > > > >
> > > > > > Looks like nothing that I have ever seen in "normal C syntax".
> > > > >
> > > > > I thought we're talking about table 4 and ALU ops.
> > > > > Above is not a pure C, but it's obvious enough without explanation, no?
> > > >
> > > > To "us", yes. Although I am not an expert, it seems like being
> > > > explicit is important when it comes to writing a spec. I suppose we
> > > > should leave that to Dave and the chairs.
> > > >
> > > > > Also I don't see above anywhere in the doc.
> > > >
> > > > That is from the Appendix. It is currently in Dave's tree and gets
> > > > amalgamated with other files to build the final draft.
> > > >
> > > > https://datatracker.ietf.org/doc/draft-thaler-bpf-isa/
> > >
> > > This is a mirror and it's already outdated.
> > > You should look at the source. Which is git kernel tree.
> >
> > As he discussed at the meeting, he has the github workflow that
> > produces a version of the draft RFC that he will submit to the WG:
> >
> > https://github.com/ietf-wg-bpf/ebpf-docs/blob/update/.github/workflows/build.yml
> >
> > That uses
> >
> > https://github.com/ietf-wg-bpf/ebpf-docs/blob/main/rst/instruction-set-skeleton.rst
>
> correct.
>
> > to build in the acknowledgements and subsequently brings in that
> > Appendix.
>
> correct.
>
> > If he plans to take that out, then that's great. I was just
> > trying to help. Sorry.
>
> No. That workflow will stay.
> The future changes to RFC will be in the form of patches to
> instruction-set-skeleton.rst. Once they land the RFC will be
> regenerated.
> We can regenerate RFC as often as we like.
>
> All I'm saying is that RFC has bugs that were already fixed in
> instruction-set-skeleton.rst. Hence it's outdated.

The Appendix (the opcode table) is not in the kernel repo now and
still has the issues that I outlined above. Will that make it in to
the kernel?

https://github.com/ietf-wg-bpf/ebpf-docs/blob/main/rst/instruction-set-opcodes.rst

Will

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

On Fri, Jul 28, 2023 at 10:32 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Jul 28, 2023 at 6:07 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > On Fri, Jul 28, 2023 at 8:52 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Fri, Jul 28, 2023 at 5:46 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > >
> > > > On Fri, Jul 28, 2023 at 8:35 PM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > >
> > > > > On Fri, Jul 28, 2023 at 5:19 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > >
> > > > > > On Fri, Jul 28, 2023 at 8:05 PM Alexei Starovoitov
> > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > >
> > > > > > > On Fri, Jul 28, 2023 at 4:32 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > > > >
> > > > > > > > On Thu, Jul 27, 2023 at 9:05 PM Alexei Starovoitov
> > > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > On Wed, Jul 26, 2023 at 12:16 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, Jul 25, 2023 at 2:37 PM Watson Ladd <watsonbladd@gmail.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> > > > > > > > > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > I am forwarding the email below (after converting HTML to plain text)
> > > > > > > > > > > > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Please use this one for any replies.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > Dave
> > > > > > > > > > > > >
> > > > > > > > > > > > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > > > > > > > > > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > > > > > > > > > > > To: bpf@ietf.org
> > > > > > > > > > > > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Dear BPF wg,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > > > > > > > > > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > > > > > > > > > > > defining them mathematically.
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Watson,
> > > > > > > > > > > >
> > > > > > > > > > > > This is not "underspecified C" pseudo code.
> > > > > > > > > > > > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
> > > > > > > > > > >
> > > > > > > > > > > I don't see a reference to any description of that in section 4.1.
> > > > > > > > > > > It's possible I've overlooked this, and if people think this style of
> > > > > > > > > > > definition is good enough that works for me. But I found table 4
> > > > > > > > > > > pretty scanty on what exactly happens.
> > > > > > > > > >
> > > > > > > > > > Hello! Based on Watson's post, I have done some research and would
> > > > > > > > > > potentially like to offer a path forward. There are several different
> > > > > > > > > > ways that ISAs specify the semantics of their operations:
> > > > > > > > > >
> > > > > > > > > > 1. Intel has a section in their manual that describes the pseudocode
> > > > > > > > > > they use to specify their ISA: Section 3.1.1.9 of The Intel® 64 and
> > > > > > > > > > IA-32 Architectures Software Developer’s Manual at
> > > > > > > > > > https://cdrdv2.intel.com/v1/dl/getContent/671199
> > > > > > > > > > 2. ARM has an equivalent for their variety of pseudocode: Chapter J1
> > > > > > > > > > of Arm Architecture Reference Manual for A-profile architecture at
> > > > > > > > > > https://developer.arm.com/documentation/ddi0487/latest/
> > > > > > > > > > 3. Sail "is a language for describing the instruction-set architecture
> > > > > > > > > > (ISA) semantics of processors."
> > > > > > > > > > (https://www.cl.cam.ac.uk/~pes20/sail/)
> > > > > > > > > >
> > > > > > > > > > Given the commercial nature of (1) and (2), perhaps Sail is a way to
> > > > > > > > > > proceed. If people are interested, I would be happy to lead an effort
> > > > > > > > > > to encode the eBPF ISA semantics in Sail (or find someone who already
> > > > > > > > > > has) and incorporate them in the draft.
> > > > > > > > >
> > > > > > > > > imo Sail is too researchy to have practical use.
> > > > > > > > > Looking at arm64 or x86 Sail description I really don't see how
> > > > > > > > > it would map to an IETF standard.
> > > > > > > > > It's done in a "sail" language that people need to learn first to be
> > > > > > > > > able to read it.
> > > > > > > > > Say we had bpf.sail somewhere on github. What value does it bring to
> > > > > > > > > BPF ISA standard? I don't see an immediate benefit to standardization.
> > > > > > > > > There could be other use cases, no doubt, but standardization is our goal.
> > > > > > > > >
> > > > > > > > > As far as 1 and 2. Intel and Arm use their own pseudocode, so they had
> > > > > > > > > to add a paragraph to describe it. We are using C to describe BPF ISA
> > > > > > > >
> > > > > > > >
> > > > > > > > I cannot find a reference in the current version that specifies what
> > > > > > > > we are using to describe the operations. I'd like to add that, but
> > > > > > > > want to make sure that I clarify two statements that seem to be at
> > > > > > > > odds.
> > > > > > > >
> > > > > > > > Immediately above you say that we are using "C to describe the BPF
> > > > > > > > ISA" and further above you say "This is assembly syntax parsed and
> > > > > > > > emitted by GCC, LLVM, gas, Linux Kernel, etc."
> > > > > > > >
> > > > > > > > My own reading is that it is the former, and not the latter. But, I
> > > > > > > > want to double check before adding the appropriate statements to the
> > > > > > > > Convention section.
> > > > > > >
> > > > > > > It's both. I'm not sure where you see a contradiction.
> > > > > > > It's a normal C syntax and it's emitted by the kernel verifier,
> > > > > > > parsed by clang/gcc assemblers and emitted by compilers.
> > > > > >
> > > > > >
> > > > > > Okay. I apologize. I am sincerely confused. For instance,
> > > > > >
> > > > > > if (u32)dst >= (u32)src goto +offset
> > > > > >
> > > > > > Looks like nothing that I have ever seen in "normal C syntax".
> > > > >
> > > > > I thought we're talking about table 4 and ALU ops.
> > > > > Above is not a pure C, but it's obvious enough without explanation, no?
> > > >
> > > > To "us", yes. Although I am not an expert, it seems like being
> > > > explicit is important when it comes to writing a spec. I suppose we
> > > > should leave that to Dave and the chairs.
> > > >
> > > > > Also I don't see above anywhere in the doc.
> > > >
> > > > That is from the Appendix. It is currently in Dave's tree and gets
> > > > amalgamated with other files to build the final draft.
> > > >
> > > > https://datatracker.ietf.org/doc/draft-thaler-bpf-isa/
> > >
> > > This is a mirror and it's already outdated.
> > > You should look at the source. Which is git kernel tree.
> >
> > As he discussed at the meeting, he has the github workflow that
> > produces a version of the draft RFC that he will submit to the WG:
> >
> > https://github.com/ietf-wg-bpf/ebpf-docs/blob/update/.github/workflows/build.yml
> >
> > That uses
> >
> > https://github.com/ietf-wg-bpf/ebpf-docs/blob/main/rst/instruction-set-skeleton.rst
>
> correct.
>
> > to build in the acknowledgements and subsequently brings in that
> > Appendix.
>
> correct.
>
> > If he plans to take that out, then that's great. I was just
> > trying to help. Sorry.
>
> No. That workflow will stay.
> The future changes to RFC will be in the form of patches to
> instruction-set-skeleton.rst. Once they land the RFC will be
> regenerated.
> We can regenerate RFC as often as we like.
>
> All I'm saying is that RFC has bugs that were already fixed in
> instruction-set-skeleton.rst. Hence it's outdated.

The Appendix (the opcode table) is not in the kernel repo now and
still has the issues that I outlined above. Will that make it in to
the kernel?

https://github.com/ietf-wg-bpf/ebpf-docs/blob/main/rst/instruction-set-opcodes.rst

Will

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Fri, Jul 28, 2023 at 8:14 PM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> The Appendix (the opcode table) is not in the kernel repo now and
> still has the issues that I outlined above. Will that make it in to
> the kernel?
>
> https://github.com/ietf-wg-bpf/ebpf-docs/blob/main/rst/instruction-set-opcodes.rst

I thought it's auto generated, so it should be easy to update.
If not, let's certainly bring it in.

I suspect it will be the seed for IANA.

Dave, thoughts?

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Fri, Jul 28, 2023 at 8:14 PM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> The Appendix (the opcode table) is not in the kernel repo now and
> still has the issues that I outlined above. Will that make it in to
> the kernel?
>
> https://github.com/ietf-wg-bpf/ebpf-docs/blob/main/rst/instruction-set-opcodes.rst

I thought it's auto generated, so it should be easy to update.
If not, let's certainly bring it in.

I suspect it will be the seed for IANA.

Dave, thoughts?

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

RE: [Bpf] Review of draft-thaler-bpf-isa-01

[Dave Thaler]

> -----Original Message-----
> From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Sent: Friday, July 28, 2023 9:52 PM
> To: Will Hawkins <hawkinsw@obs.cr>
> Cc: Watson Ladd <watsonbladd@gmail.com>; Dave Thaler
> <dthaler@microsoft.com>; bpf@ietf.org; bpf <bpf@vger.kernel.org>
> Subject: Re: [Bpf] Review of draft-thaler-bpf-isa-01
> 
> On Fri, Jul 28, 2023 at 8:14 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > The Appendix (the opcode table) is not in the kernel repo now and
> > still has the issues that I outlined above.

Suggestions (especially concrete changes) welcome :)

> Will that make it in to
> > the kernel?
[...]
> I thought it's auto generated, so it should be easy to update.

It's not yet auto generated, and some parts are hard to auto-generated
because the combinations are just in English text.

> If not, let's certainly bring it in.

At the IETF BPF WG meeting, folks seemed agnostic as to whether it
was brought into the Linux repo or not.  See recording at
https://www.youtube.com/watch?v=jTtPbJqfYwI at 1:15:30 - 1:17:30,
and Christoph was the only one who spoke up, preferring to just keep
a static copy of the Internet Draft in the kernel repository.  I interpreted
this as saying no one cared about having the IANA considerations section
in a separate file there.  But we confirm consensus on the list, so it's fine
to revisit now if there are good reasons to do so.

> I suspect it will be the seed for IANA.
> Dave, thoughts?

That's the intent, yes.  Per RFC 8126, the Internet Draft has the "initial"
contents of the registry at time of publication, after which IANA becomes
the authoritative place going forward since one cannot change the RFC itself.

Dave

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Dave Thaler]

> -----Original Message-----
> From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Sent: Friday, July 28, 2023 9:52 PM
> To: Will Hawkins <hawkinsw@obs.cr>
> Cc: Watson Ladd <watsonbladd@gmail.com>; Dave Thaler
> <dthaler@microsoft.com>; bpf@ietf.org; bpf <bpf@vger.kernel.org>
> Subject: Re: [Bpf] Review of draft-thaler-bpf-isa-01
> 
> On Fri, Jul 28, 2023 at 8:14 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > The Appendix (the opcode table) is not in the kernel repo now and
> > still has the issues that I outlined above.

Suggestions (especially concrete changes) welcome :)

> Will that make it in to
> > the kernel?
[...]
> I thought it's auto generated, so it should be easy to update.

It's not yet auto generated, and some parts are hard to auto-generated
because the combinations are just in English text.

> If not, let's certainly bring it in.

At the IETF BPF WG meeting, folks seemed agnostic as to whether it
was brought into the Linux repo or not.  See recording at
https://www.youtube.com/watch?v=jTtPbJqfYwI at 1:15:30 - 1:17:30,
and Christoph was the only one who spoke up, preferring to just keep
a static copy of the Internet Draft in the kernel repository.  I interpreted
this as saying no one cared about having the IANA considerations section
in a separate file there.  But we confirm consensus on the list, so it's fine
to revisit now if there are good reasons to do so.

> I suspect it will be the seed for IANA.
> Dave, thoughts?

That's the intent, yes.  Per RFC 8126, the Internet Draft has the "initial"
contents of the registry at time of publication, after which IANA becomes
the authoritative place going forward since one cannot change the RFC itself.

Dave
-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Tue, Aug 1, 2023 at 6:55 PM Dave Thaler <dthaler@microsoft.com> wrote:
>
> > -----Original Message-----
> > From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> > Sent: Friday, July 28, 2023 9:52 PM
> > To: Will Hawkins <hawkinsw@obs.cr>
> > Cc: Watson Ladd <watsonbladd@gmail.com>; Dave Thaler
> > <dthaler@microsoft.com>; bpf@ietf.org; bpf <bpf@vger.kernel.org>
> > Subject: Re: [Bpf] Review of draft-thaler-bpf-isa-01
> >
> > On Fri, Jul 28, 2023 at 8:14 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > >
> > > The Appendix (the opcode table) is not in the kernel repo now and
> > > still has the issues that I outlined above.
>
> Suggestions (especially concrete changes) welcome :)
>
> > Will that make it in to
> > > the kernel?
> [...]
> > I thought it's auto generated, so it should be easy to update.
>
> It's not yet auto generated, and some parts are hard to auto-generated
> because the combinations are just in English text.
>
> > If not, let's certainly bring it in.
>
> At the IETF BPF WG meeting, folks seemed agnostic as to whether it
> was brought into the Linux repo or not.  See recording at
> https://www.youtube.com/watch?v=jTtPbJqfYwI at 1:15:30 - 1:17:30,
> and Christoph was the only one who spoke up, preferring to just keep
> a static copy of the Internet Draft in the kernel repository.  I interpreted
> this as saying no one cared about having the IANA considerations section
> in a separate file there.  But we confirm consensus on the list, so it's fine
> to revisit now if there are good reasons to do so.

I think IANA consideration section is orthogonal to giant opcode table.
They're related, but don't have to be together in one .rst file.
I think it's cleaner to have separate instruction-set-opcode.rst

We also went back and forth during the meeting whether hierarchy
of tables is prefered or one table. Currently you have one table and
it actually looks very readable. My preference would be to keep it this
way and carry over to IANA eventually as one table.

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Tue, Aug 1, 2023 at 6:55 PM Dave Thaler <dthaler@microsoft.com> wrote:
>
> > -----Original Message-----
> > From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> > Sent: Friday, July 28, 2023 9:52 PM
> > To: Will Hawkins <hawkinsw@obs.cr>
> > Cc: Watson Ladd <watsonbladd@gmail.com>; Dave Thaler
> > <dthaler@microsoft.com>; bpf@ietf.org; bpf <bpf@vger.kernel.org>
> > Subject: Re: [Bpf] Review of draft-thaler-bpf-isa-01
> >
> > On Fri, Jul 28, 2023 at 8:14 PM Will Hawkins <hawkinsw@obs.cr> wrote:
> > >
> > > The Appendix (the opcode table) is not in the kernel repo now and
> > > still has the issues that I outlined above.
>
> Suggestions (especially concrete changes) welcome :)
>
> > Will that make it in to
> > > the kernel?
> [...]
> > I thought it's auto generated, so it should be easy to update.
>
> It's not yet auto generated, and some parts are hard to auto-generated
> because the combinations are just in English text.
>
> > If not, let's certainly bring it in.
>
> At the IETF BPF WG meeting, folks seemed agnostic as to whether it
> was brought into the Linux repo or not.  See recording at
> https://www.youtube.com/watch?v=jTtPbJqfYwI at 1:15:30 - 1:17:30,
> and Christoph was the only one who spoke up, preferring to just keep
> a static copy of the Internet Draft in the kernel repository.  I interpreted
> this as saying no one cared about having the IANA considerations section
> in a separate file there.  But we confirm consensus on the list, so it's fine
> to revisit now if there are good reasons to do so.

I think IANA consideration section is orthogonal to giant opcode table.
They're related, but don't have to be together in one .rst file.
I think it's cleaner to have separate instruction-set-opcode.rst

We also went back and forth during the meeting whether hierarchy
of tables is prefered or one table. Currently you have one table and
it actually looks very readable. My preference would be to keep it this
way and carry over to IANA eventually as one table.

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

RE: [Bpf] Review of draft-thaler-bpf-isa-01

[Dave Thaler]

Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: 
> On Tue, Aug 1, 2023 at 6:55 PM Dave Thaler <dthaler@microsoft.com>
> wrote:
[...]
>  I interpreted this as saying no one cared about having the IANA
> considerations section in a separate file there.  But we confirm consensus on
> the list, so it's fine to revisit now if there are good reasons to do so.
> 
> I think IANA consideration section is orthogonal to giant opcode table.

It's not orthogonal, such a table is a required part of the IANA Considerations
section.  See https://www.rfc-editor.org/rfc/rfc8126#section-2.2
(specifically the "Initial assignments and reservations").

> They're related, but don't have to be together in one .rst file.

True.

> I think it's cleaner to have separate instruction-set-opcode.rst

Sure.

> We also went back and forth during the meeting whether hierarchy of tables
> is prefered or one table.

During the meeting, no one argued for one table (other than me saying 
if no one has a preference, I'll leave it the way it is), and several people
argued for multiple tables so I took that as potential consensus for multiple
tables.  But again, we confirm consensus on the list so if anyone here has a
reason why one table or multiple tables are technically better, please speak up.

> Currently you have one table and it actually looks
> very readable. My preference would be to keep it this way and carry over to
> IANA eventually as one table.

Personally, I agree that I find one table more readable.  But it sounded like
others in the meeting found multiple tables more readable.

Comparing slides 8 and 9 from the meeting, at https://datatracker.ietf.org/meeting/117/materials/slides-117-bpf-isa-extension-policy-03
folks will notice that on slide 9 in the multiple tables example, I omitted columns from the opcode table other than the single key field (opcode, src, or whatever depending on the
table) and the description.  Thus for say opcode 0x17 (dst -= imm) the one table
version shows src 0x0 and imm any, whereas the multiple tables one doesn't.
Perhaps it could, but one thing I like about the single table version is that as currently
depicted, opcode 0x17 with src != 0x0 is currently undefined (i.e., available for later use),
whereas the multiple table version with a single key field depicts it as defined as "dst -= imm" with English text in the body of the doc that says src must be zero.

Thus the single table version that presents src as a key field unless it contains "any"
means there's plenty of instruction space left unallocated.  Anyone who actually wants
to disallow any future instructions from ever using opcode 0x17 with src != 0x0 might
prefer the multiple tables (i.e., without src as a key field).  But to me, this could
potentially be a technical argument for keeping one table.

Dave

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Dave Thaler]

Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: 
> On Tue, Aug 1, 2023 at 6:55 PM Dave Thaler <dthaler@microsoft.com>
> wrote:
[...]
>  I interpreted this as saying no one cared about having the IANA
> considerations section in a separate file there.  But we confirm consensus on
> the list, so it's fine to revisit now if there are good reasons to do so.
> 
> I think IANA consideration section is orthogonal to giant opcode table.

It's not orthogonal, such a table is a required part of the IANA Considerations
section.  See https://www.rfc-editor.org/rfc/rfc8126#section-2.2
(specifically the "Initial assignments and reservations").

> They're related, but don't have to be together in one .rst file.

True.

> I think it's cleaner to have separate instruction-set-opcode.rst

Sure.

> We also went back and forth during the meeting whether hierarchy of tables
> is prefered or one table.

During the meeting, no one argued for one table (other than me saying 
if no one has a preference, I'll leave it the way it is), and several people
argued for multiple tables so I took that as potential consensus for multiple
tables.  But again, we confirm consensus on the list so if anyone here has a
reason why one table or multiple tables are technically better, please speak up.

> Currently you have one table and it actually looks
> very readable. My preference would be to keep it this way and carry over to
> IANA eventually as one table.

Personally, I agree that I find one table more readable.  But it sounded like
others in the meeting found multiple tables more readable.

Comparing slides 8 and 9 from the meeting, at https://datatracker.ietf.org/meeting/117/materials/slides-117-bpf-isa-extension-policy-03
folks will notice that on slide 9 in the multiple tables example, I omitted columns from the opcode table other than the single key field (opcode, src, or whatever depending on the
table) and the description.  Thus for say opcode 0x17 (dst -= imm) the one table
version shows src 0x0 and imm any, whereas the multiple tables one doesn't.
Perhaps it could, but one thing I like about the single table version is that as currently
depicted, opcode 0x17 with src != 0x0 is currently undefined (i.e., available for later use),
whereas the multiple table version with a single key field depicts it as defined as "dst -= imm" with English text in the body of the doc that says src must be zero.

Thus the single table version that presents src as a key field unless it contains "any"
means there's plenty of instruction space left unallocated.  Anyone who actually wants
to disallow any future instructions from ever using opcode 0x17 with src != 0x0 might
prefer the multiple tables (i.e., without src as a key field).  But to me, this could
potentially be a technical argument for keeping one table.

Dave
-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

RE: [Bpf] Review of draft-thaler-bpf-isa-01

[Dave Thaler]

> -----Original Message-----
> From: Dave Thaler
> Sent: Wednesday, August 2, 2023 5:05 AM
> To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Will Hawkins <hawkinsw@obs.cr>; Watson Ladd
> <watsonbladd@gmail.com>; bpf@ietf.org; bpf <bpf@vger.kernel.org>
> Subject: RE: [Bpf] Review of draft-thaler-bpf-isa-01
> 
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > On Tue, Aug 1, 2023 at 6:55 PM Dave Thaler <dthaler@microsoft.com>
> > wrote:
> [...]
> >  I interpreted this as saying no one cared about having the IANA
> > considerations section in a separate file there.  But we confirm
> > consensus on the list, so it's fine to revisit now if there are good reasons to
> do so.
> >
> > I think IANA consideration section is orthogonal to giant opcode table.
> 
> It's not orthogonal, such a table is a required part of the IANA Considerations
> section.  See https://www.rfc-editor.org/rfc/rfc8126#section-2.2
> (specifically the "Initial assignments and reservations").
> 
> > They're related, but don't have to be together in one .rst file.
> 
> True.
> 
> > I think it's cleaner to have separate instruction-set-opcode.rst
> 
> Sure.

Forgot to add: yes, they're already in separate rst files in my pending
changes, which get combined into the same section of the Internet Draft
when it's generated.

Dave

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Dave Thaler]

> -----Original Message-----
> From: Dave Thaler
> Sent: Wednesday, August 2, 2023 5:05 AM
> To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Will Hawkins <hawkinsw@obs.cr>; Watson Ladd
> <watsonbladd@gmail.com>; bpf@ietf.org; bpf <bpf@vger.kernel.org>
> Subject: RE: [Bpf] Review of draft-thaler-bpf-isa-01
> 
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > On Tue, Aug 1, 2023 at 6:55 PM Dave Thaler <dthaler@microsoft.com>
> > wrote:
> [...]
> >  I interpreted this as saying no one cared about having the IANA
> > considerations section in a separate file there.  But we confirm
> > consensus on the list, so it's fine to revisit now if there are good reasons to
> do so.
> >
> > I think IANA consideration section is orthogonal to giant opcode table.
> 
> It's not orthogonal, such a table is a required part of the IANA Considerations
> section.  See https://www.rfc-editor.org/rfc/rfc8126#section-2.2
> (specifically the "Initial assignments and reservations").
> 
> > They're related, but don't have to be together in one .rst file.
> 
> True.
> 
> > I think it's cleaner to have separate instruction-set-opcode.rst
> 
> Sure.

Forgot to add: yes, they're already in separate rst files in my pending
changes, which get combined into the same section of the Internet Draft
when it's generated.

Dave

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Tue, Jul 25, 2023 at 11:37 AM Watson Ladd <watsonbladd@gmail.com> wrote:
>
> On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > >
> > > I am forwarding the email below (after converting HTML to plain text)
> > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > >
> > > Please use this one for any replies.
> > >
> > > Thanks,
> > > Dave
> > >
> > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > To: bpf@ietf.org
> > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > >
> > > > Dear BPF wg,
> > > >
> > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > defining them mathematically.
> >
> > Hi Watson,
> >
> > This is not "underspecified C" pseudo code.
> > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
>
> I don't see a reference to any description of that in section 4.1.
> It's possible I've overlooked this, and if people think this style of
> definition is good enough that works for me. But I found table 4
> pretty scanty on what exactly happens.

Could you please be specific which instruction in table 4 is not obvious?

> >
> > > > The good news is I think this is very fixable although tedious.
> > > >
> > > > The other thornier issues are memory model etc. But the overall structure seems good
> > > > and the document overall makes sense.
> >
> > What do you mean by "memory model" ?
> > Do you see a reference to it ? Please be specific.
>
> No, and that's the problem. Section 5.2 talks about atomic operations.
> I'd expect that to be paired with a description of barriers so that
> these work, or a big warning about when you need to use them.

That's a good suggestion.
A warning paragraph that BPF ISA does not have barrier instructions
is necessary.

> For
> clarity I'm pretty unfamiliar with bpf as a technology, and it's
> possible that with more knowledge this would make sense. On looking
> back on that I don't even know if the memory space is flat, or
> segmented: can I access maps through a value set to dst+offset, or
> must I always used index? I'm just very confused.

flat vs segmented is an orthogonal topic.
We definitely need to cover it in the architecture doc.
BPF WG charter requires us to produce it as Informational doc eventually.

As far as memory model BPF adopts LKMM (Linux Kernel Memory Model).
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0124r7.html

We can add a reference to it from BPF ISA doc, but since
there are no barrier instructions at the moment the memory model
statement would be premature.
The work on "BPF Memory Model" have been ongoing for quite some time.
For example see:
https://lpc.events/event/11/contributions/941/attachments/859/1667/bpf-memory-model.2020.09.22a.pdf

BPF Memory Model is certainly an important topic, but out of scope for ISA.

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Tue, Jul 25, 2023 at 11:37 AM Watson Ladd <watsonbladd@gmail.com> wrote:
>
> On Tue, Jul 25, 2023 at 9:15 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Tue, Jul 25, 2023 at 7:03 AM Dave Thaler <dthaler@microsoft.com> wrote:
> > >
> > > I am forwarding the email below (after converting HTML to plain text)
> > > to the mailto:bpf@vger.kernel.org list so replies can go to both lists.
> > >
> > > Please use this one for any replies.
> > >
> > > Thanks,
> > > Dave
> > >
> > > > From: Bpf <bpf-bounces@ietf.org> On Behalf Of Watson Ladd
> > > > Sent: Monday, July 24, 2023 10:05 PM
> > > > To: bpf@ietf.org
> > > > Subject: [Bpf] Review of draft-thaler-bpf-isa-01
> > > >
> > > > Dear BPF wg,
> > > >
> > > > I took a look at the draft and think it has some issues, unsurprisingly at this stage. One is
> > > > the specification seems to use an underspecified C pseudo code for operations vs
> > > > defining them mathematically.
> >
> > Hi Watson,
> >
> > This is not "underspecified C" pseudo code.
> > This is assembly syntax parsed and emitted by GCC, LLVM, gas, Linux Kernel, etc.
>
> I don't see a reference to any description of that in section 4.1.
> It's possible I've overlooked this, and if people think this style of
> definition is good enough that works for me. But I found table 4
> pretty scanty on what exactly happens.

Could you please be specific which instruction in table 4 is not obvious?

> >
> > > > The good news is I think this is very fixable although tedious.
> > > >
> > > > The other thornier issues are memory model etc. But the overall structure seems good
> > > > and the document overall makes sense.
> >
> > What do you mean by "memory model" ?
> > Do you see a reference to it ? Please be specific.
>
> No, and that's the problem. Section 5.2 talks about atomic operations.
> I'd expect that to be paired with a description of barriers so that
> these work, or a big warning about when you need to use them.

That's a good suggestion.
A warning paragraph that BPF ISA does not have barrier instructions
is necessary.

> For
> clarity I'm pretty unfamiliar with bpf as a technology, and it's
> possible that with more knowledge this would make sense. On looking
> back on that I don't even know if the memory space is flat, or
> segmented: can I access maps through a value set to dst+offset, or
> must I always used index? I'm just very confused.

flat vs segmented is an orthogonal topic.
We definitely need to cover it in the architecture doc.
BPF WG charter requires us to produce it as Informational doc eventually.

As far as memory model BPF adopts LKMM (Linux Kernel Memory Model).
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0124r7.html

We can add a reference to it from BPF ISA doc, but since
there are no barrier instructions at the moment the memory model
statement would be premature.
The work on "BPF Memory Model" have been ongoing for quite some time.
For example see:
https://lpc.events/event/11/contributions/941/attachments/859/1667/bpf-memory-model.2020.09.22a.pdf

BPF Memory Model is certainly an important topic, but out of scope for ISA.

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Watson Ladd]

In reply to a long conversation:
<snip>
>
> Could you please be specific which instruction in table 4 is not obvious?

The question isn't obvious, the question is unambigious, and C is not
great for this. Maybe with a reference and some text it would get
better.
>
> > >
> > > > > The good news is I think this is very fixable although tedious.
> > > > >
> > > > > The other thornier issues are memory model etc. But the overall structure seems good
> > > > > and the document overall makes sense.
> > >
> > > What do you mean by "memory model" ?
> > > Do you see a reference to it ? Please be specific.
> >
> > No, and that's the problem. Section 5.2 talks about atomic operations.
> > I'd expect that to be paired with a description of barriers so that
> > these work, or a big warning about when you need to use them.
>
> That's a good suggestion.
> A warning paragraph that BPF ISA does not have barrier instructions
> is necessary.
>
> > For
> > clarity I'm pretty unfamiliar with bpf as a technology, and it's
> > possible that with more knowledge this would make sense. On looking
> > back on that I don't even know if the memory space is flat, or
> > segmented: can I access maps through a value set to dst+offset, or
> > must I always used index? I'm just very confused.
>
> flat vs segmented is an orthogonal topic.
> We definitely need to cover it in the architecture doc.
> BPF WG charter requires us to produce it as Informational doc eventually.

Huh? If you access memory through specialized descriptors+offsets
that's very different from arbitrary computations with addresses, even
if they do trap. A little explanation might orient the reader to
understand what is going on. As is I thought "ok, it's flat" and then
saw the maps and really got thrown for a loop.

>
> As far as memory model BPF adopts LKMM (Linux Kernel Memory Model).
> https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0124r7.html
>
> We can add a reference to it from BPF ISA doc, but since
> there are no barrier instructions at the moment the memory model
> statement would be premature.
> The work on "BPF Memory Model" have been ongoing for quite some time.
> For example see:
> https://lpc.events/event/11/contributions/941/attachments/859/1667/bpf-memory-model.2020.09.22a.pdf
>
> BPF Memory Model is certainly an important topic, but out of scope for ISA.

I expect that an ISA defines the semantics of the instructions. Which
absolutely includes the memory model.  Now maybe we're envisioning a
different splitting of this information, but I don't see how it can't
be at a different level if you want to give the instructions
semantics.

Sincerely,
Watson Ladd


--
Astra mortemque praestare gradatim

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Watson Ladd]

In reply to a long conversation:
<snip>
>
> Could you please be specific which instruction in table 4 is not obvious?

The question isn't obvious, the question is unambigious, and C is not
great for this. Maybe with a reference and some text it would get
better.
>
> > >
> > > > > The good news is I think this is very fixable although tedious.
> > > > >
> > > > > The other thornier issues are memory model etc. But the overall structure seems good
> > > > > and the document overall makes sense.
> > >
> > > What do you mean by "memory model" ?
> > > Do you see a reference to it ? Please be specific.
> >
> > No, and that's the problem. Section 5.2 talks about atomic operations.
> > I'd expect that to be paired with a description of barriers so that
> > these work, or a big warning about when you need to use them.
>
> That's a good suggestion.
> A warning paragraph that BPF ISA does not have barrier instructions
> is necessary.
>
> > For
> > clarity I'm pretty unfamiliar with bpf as a technology, and it's
> > possible that with more knowledge this would make sense. On looking
> > back on that I don't even know if the memory space is flat, or
> > segmented: can I access maps through a value set to dst+offset, or
> > must I always used index? I'm just very confused.
>
> flat vs segmented is an orthogonal topic.
> We definitely need to cover it in the architecture doc.
> BPF WG charter requires us to produce it as Informational doc eventually.

Huh? If you access memory through specialized descriptors+offsets
that's very different from arbitrary computations with addresses, even
if they do trap. A little explanation might orient the reader to
understand what is going on. As is I thought "ok, it's flat" and then
saw the maps and really got thrown for a loop.

>
> As far as memory model BPF adopts LKMM (Linux Kernel Memory Model).
> https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0124r7.html
>
> We can add a reference to it from BPF ISA doc, but since
> there are no barrier instructions at the moment the memory model
> statement would be premature.
> The work on "BPF Memory Model" have been ongoing for quite some time.
> For example see:
> https://lpc.events/event/11/contributions/941/attachments/859/1667/bpf-memory-model.2020.09.22a.pdf
>
> BPF Memory Model is certainly an important topic, but out of scope for ISA.

I expect that an ISA defines the semantics of the instructions. Which
absolutely includes the memory model.  Now maybe we're envisioning a
different splitting of this information, but I don't see how it can't
be at a different level if you want to give the instructions
semantics.

Sincerely,
Watson Ladd


--
Astra mortemque praestare gradatim

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Tue, Aug 1, 2023 at 7:34 PM Watson Ladd <watsonbladd@gmail.com> wrote:
>
> In reply to a long conversation:
> <snip>
> >
> > Could you please be specific which instruction in table 4 is not obvious?
>
> The question isn't obvious, the question is unambigious, and C is not
> great for this. Maybe with a reference and some text it would get
> better.
> >
> > > >
> > > > > > The good news is I think this is very fixable although tedious.
> > > > > >
> > > > > > The other thornier issues are memory model etc. But the overall structure seems good
> > > > > > and the document overall makes sense.
> > > >
> > > > What do you mean by "memory model" ?
> > > > Do you see a reference to it ? Please be specific.
> > >
> > > No, and that's the problem. Section 5.2 talks about atomic operations.
> > > I'd expect that to be paired with a description of barriers so that
> > > these work, or a big warning about when you need to use them.
> >
> > That's a good suggestion.
> > A warning paragraph that BPF ISA does not have barrier instructions
> > is necessary.
> >
> > > For
> > > clarity I'm pretty unfamiliar with bpf as a technology, and it's
> > > possible that with more knowledge this would make sense. On looking
> > > back on that I don't even know if the memory space is flat, or
> > > segmented: can I access maps through a value set to dst+offset, or
> > > must I always used index? I'm just very confused.
> >
> > flat vs segmented is an orthogonal topic.
> > We definitely need to cover it in the architecture doc.
> > BPF WG charter requires us to produce it as Informational doc eventually.
>
> Huh? If you access memory through specialized descriptors+offsets
> that's very different from arbitrary computations with addresses, even
> if they do trap. A little explanation might orient the reader to
> understand what is going on. As is I thought "ok, it's flat" and then
> saw the maps and really got thrown for a loop.

It's flat.

> >
> > As far as memory model BPF adopts LKMM (Linux Kernel Memory Model).
> > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0124r7.html
> >
> > We can add a reference to it from BPF ISA doc, but since
> > there are no barrier instructions at the moment the memory model
> > statement would be premature.
> > The work on "BPF Memory Model" have been ongoing for quite some time.
> > For example see:
> > https://lpc.events/event/11/contributions/941/attachments/859/1667/bpf-memory-model.2020.09.22a.pdf
> >
> > BPF Memory Model is certainly an important topic, but out of scope for ISA.
>
> I expect that an ISA defines the semantics of the instructions. Which
> absolutely includes the memory model.  Now maybe we're envisioning a
> different splitting of this information, but I don't see how it can't
> be at a different level if you want to give the instructions
> semantics.

Please read the links above.
BPF ISA is not going to define TSO, dictate weak ordering or anything
like that. It follows Linux Kernel Memory Model which is closer to
C memory model than to what HW CPUs see as memory model.
It's unlikely that we will ever introduce explicit memory barrier
instructions. More likely they will be kfuncs that will map to smp_mb(),
dma_wmb(), and friends in kernel Documentation/memory-barriers.txt.
Analogies with HW ISAs are not applicable here.

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Alexei Starovoitov]

On Tue, Aug 1, 2023 at 7:34 PM Watson Ladd <watsonbladd@gmail.com> wrote:
>
> In reply to a long conversation:
> <snip>
> >
> > Could you please be specific which instruction in table 4 is not obvious?
>
> The question isn't obvious, the question is unambigious, and C is not
> great for this. Maybe with a reference and some text it would get
> better.
> >
> > > >
> > > > > > The good news is I think this is very fixable although tedious.
> > > > > >
> > > > > > The other thornier issues are memory model etc. But the overall structure seems good
> > > > > > and the document overall makes sense.
> > > >
> > > > What do you mean by "memory model" ?
> > > > Do you see a reference to it ? Please be specific.
> > >
> > > No, and that's the problem. Section 5.2 talks about atomic operations.
> > > I'd expect that to be paired with a description of barriers so that
> > > these work, or a big warning about when you need to use them.
> >
> > That's a good suggestion.
> > A warning paragraph that BPF ISA does not have barrier instructions
> > is necessary.
> >
> > > For
> > > clarity I'm pretty unfamiliar with bpf as a technology, and it's
> > > possible that with more knowledge this would make sense. On looking
> > > back on that I don't even know if the memory space is flat, or
> > > segmented: can I access maps through a value set to dst+offset, or
> > > must I always used index? I'm just very confused.
> >
> > flat vs segmented is an orthogonal topic.
> > We definitely need to cover it in the architecture doc.
> > BPF WG charter requires us to produce it as Informational doc eventually.
>
> Huh? If you access memory through specialized descriptors+offsets
> that's very different from arbitrary computations with addresses, even
> if they do trap. A little explanation might orient the reader to
> understand what is going on. As is I thought "ok, it's flat" and then
> saw the maps and really got thrown for a loop.

It's flat.

> >
> > As far as memory model BPF adopts LKMM (Linux Kernel Memory Model).
> > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0124r7.html
> >
> > We can add a reference to it from BPF ISA doc, but since
> > there are no barrier instructions at the moment the memory model
> > statement would be premature.
> > The work on "BPF Memory Model" have been ongoing for quite some time.
> > For example see:
> > https://lpc.events/event/11/contributions/941/attachments/859/1667/bpf-memory-model.2020.09.22a.pdf
> >
> > BPF Memory Model is certainly an important topic, but out of scope for ISA.
>
> I expect that an ISA defines the semantics of the instructions. Which
> absolutely includes the memory model.  Now maybe we're envisioning a
> different splitting of this information, but I don't see how it can't
> be at a different level if you want to give the instructions
> semantics.

Please read the links above.
BPF ISA is not going to define TSO, dictate weak ordering or anything
like that. It follows Linux Kernel Memory Model which is closer to
C memory model than to what HW CPUs see as memory model.
It's unlikely that we will ever introduce explicit memory barrier
instructions. More likely they will be kfuncs that will map to smp_mb(),
dma_wmb(), and friends in kernel Documentation/memory-barriers.txt.
Analogies with HW ISAs are not applicable here.

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[David Vernet]

On Tue, Aug 01, 2023 at 07:33:59PM -0700, Watson Ladd wrote:
> In reply to a long conversation:
> <snip>
> >
> > Could you please be specific which instruction in table 4 is not obvious?
> 
> The question isn't obvious, the question is unambigious, and C is not
> great for this. Maybe with a reference and some text it would get
> better.
> >
> > > >
> > > > > > The good news is I think this is very fixable although tedious.
> > > > > >
> > > > > > The other thornier issues are memory model etc. But the overall structure seems good
> > > > > > and the document overall makes sense.
> > > >
> > > > What do you mean by "memory model" ?
> > > > Do you see a reference to it ? Please be specific.
> > >
> > > No, and that's the problem. Section 5.2 talks about atomic operations.
> > > I'd expect that to be paired with a description of barriers so that
> > > these work, or a big warning about when you need to use them.
> >
> > That's a good suggestion.
> > A warning paragraph that BPF ISA does not have barrier instructions
> > is necessary.
> >
> > > For
> > > clarity I'm pretty unfamiliar with bpf as a technology, and it's
> > > possible that with more knowledge this would make sense. On looking
> > > back on that I don't even know if the memory space is flat, or
> > > segmented: can I access maps through a value set to dst+offset, or
> > > must I always used index? I'm just very confused.
> >
> > flat vs segmented is an orthogonal topic.
> > We definitely need to cover it in the architecture doc.
> > BPF WG charter requires us to produce it as Informational doc eventually.
> 
> Huh? If you access memory through specialized descriptors+offsets
> that's very different from arbitrary computations with addresses, even
> if they do trap. A little explanation might orient the reader to
> understand what is going on. As is I thought "ok, it's flat" and then
> saw the maps and really got thrown for a loop.
> 
> >
> > As far as memory model BPF adopts LKMM (Linux Kernel Memory Model).
> > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0124r7.html
> >
> > We can add a reference to it from BPF ISA doc, but since
> > there are no barrier instructions at the moment the memory model
> > statement would be premature.
> > The work on "BPF Memory Model" have been ongoing for quite some time.
> > For example see:
> > https://lpc.events/event/11/contributions/941/attachments/859/1667/bpf-memory-model.2020.09.22a.pdf
> >
> > BPF Memory Model is certainly an important topic, but out of scope for ISA.
> 
> I expect that an ISA defines the semantics of the instructions. Which
> absolutely includes the memory model.  Now maybe we're envisioning a
> different splitting of this information, but I don't see how it can't
> be at a different level if you want to give the instructions
> semantics.

Hello Watson,

Thank you for sharing your thoughts on the ISA standard document thus
far. I tend to agree with you that defining a memory model (including a
memory consistency model) is inevitable, and I think it will be critical
for ensuring source code compatibility between different BPF
implementations. That said, I'm not sure that it needs to be a blocker
to us ratifying this initial proposed standard (not that you said that
explicitly) for the ISA, for a couple of reasons.

Firstly and perhaps most importantly, there is a precedence for ISA
standards evolving, sometimes significantly, as the platforms and
ecosystems surrounding a standard mature. RISC-V is an excellent example
of this. The 2.0 version of the Unprivileged ISA standard [0] published
in 2019 included_many changes that were not present in the initial
publication [1] of the standard in 2011.

[0]: https://github.com/riscv/riscv-isa-manual/releases/download/Ratified-IMAFDQC/riscv-spec-20191213.pdf
[1]: https://people.eecs.berkeley.edu/~krste/papers/EECS-2011-62.pdf

Note, for example, that the RVWMO  (RISC-V Weak Memory Ordering) Memory
Consistency Model was not ratified until v2.0 of the standard, which was
8 years following the publication of v1. Subsection 2.8 in v1.0 does
describe a Memory Model, but it's very high level, and doesn't even
mention e.g. device I/O. This is despite the v1.0 publication describing
"Atomic Memory Operation Instructions" as follows (excluding the diagram
showing the encoding which I won't take the time to type out in
plaintext form):

> The atomic memory operation (AMO) instructions perform
> read-modify-write operations for multiprocessor synchronization and
> are encoded with an R-type instruction format. These AMO instructions
> atomically load a data value from the address in rs1, place the value
> into register rd, apply a binary operator to the loaded value and the
> value in rs2, then store the result back to the address in rs1. AMOs
> can either operate on 32-bit or 64-bit words in memory. For RV64,
> 32-bit AMOs always sign-extend the value placed in rd. The address
> held in rs1 must be naturally aligned to the size of the operand
> (i.e., eight-byte aligned for 64-bit words and four-byte aligned for
> 32-bit words). If the address is not naturally aligned, a misaligned
> address trap will be generated.  The operations supported are integer
> add, logical AND, logical OR, swap, and signed and unsigned integer
> maximum and minimum.

This is in contrast to v2.0, which dedicates the entirety of Chapter 8:
"A" Standard Extension for Atomic Instructions, Version 2.1 to
discussing and formalizing atomic instructions.

That's all to say, I don't think we need to have a full and "feature
complete" ISA standard for our initial ratification. Speaking for
myself, I think it would be prudent for us to actually hold off on
defining some of these finer points like the memory model until we've
had more time to think about what the proper memory model would look
like for BPF rather than just standardizing what industry has built thus
far.

What we _definitely_ need to think through completely in this early
stage is how we're going to enable extensions to the standard, because
no matter how much work we put into this version, we're inevitably going
to have to write extensions (not that you're claiming otherwise).

In my personal opinion, I think standardizing instruction encoding and
semantics is a reasonable first step. Which leads me to my other reason
why I don't think it's necessary to formally define a memory model for
v1:

One of the unique challenges we're going to have to work through as a
Working Group is how to collaborate between the IETF and Linux Kernel
communities. I think we're doing a great job so far, especially given
how things went at IETF 117, and that folks from both communities are
already engaging on the standard. That said, I am personally of the
opinion that we're most likely to succeed in figuring out an ideal
working relationship / culture for the WG if we can do so while
iterating on a standard that has a minimal amount of complexity, and a
lower likelihood of being contentious.

That of course doesn't mean that we should sacrifice the quality of
whatever standard(s) we write and ratify, but I think that if we can
write a standard that we agree is correct, self-contained, well-written,
and can be easily extended, but that is also "non-controversial" (e.g.
for the case of the ISA, defines instruction encodings that are
implemented by clang and gcc, and whose semantics are implemented by
Linux, Microsoft, Netronome, etc), that it may be prudent to take
advantage of that in these early days to flesh out how our WG will
operate when we have to write more open-ended and potentially
contentious parts of the standard such as the BPF memory model.

What do you think? Thanks again for sharing your thoughts, and for
participating in the WG.

- David

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[David Vernet]

On Tue, Aug 01, 2023 at 07:33:59PM -0700, Watson Ladd wrote:
> In reply to a long conversation:
> <snip>
> >
> > Could you please be specific which instruction in table 4 is not obvious?
> 
> The question isn't obvious, the question is unambigious, and C is not
> great for this. Maybe with a reference and some text it would get
> better.
> >
> > > >
> > > > > > The good news is I think this is very fixable although tedious.
> > > > > >
> > > > > > The other thornier issues are memory model etc. But the overall structure seems good
> > > > > > and the document overall makes sense.
> > > >
> > > > What do you mean by "memory model" ?
> > > > Do you see a reference to it ? Please be specific.
> > >
> > > No, and that's the problem. Section 5.2 talks about atomic operations.
> > > I'd expect that to be paired with a description of barriers so that
> > > these work, or a big warning about when you need to use them.
> >
> > That's a good suggestion.
> > A warning paragraph that BPF ISA does not have barrier instructions
> > is necessary.
> >
> > > For
> > > clarity I'm pretty unfamiliar with bpf as a technology, and it's
> > > possible that with more knowledge this would make sense. On looking
> > > back on that I don't even know if the memory space is flat, or
> > > segmented: can I access maps through a value set to dst+offset, or
> > > must I always used index? I'm just very confused.
> >
> > flat vs segmented is an orthogonal topic.
> > We definitely need to cover it in the architecture doc.
> > BPF WG charter requires us to produce it as Informational doc eventually.
> 
> Huh? If you access memory through specialized descriptors+offsets
> that's very different from arbitrary computations with addresses, even
> if they do trap. A little explanation might orient the reader to
> understand what is going on. As is I thought "ok, it's flat" and then
> saw the maps and really got thrown for a loop.
> 
> >
> > As far as memory model BPF adopts LKMM (Linux Kernel Memory Model).
> > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0124r7.html
> >
> > We can add a reference to it from BPF ISA doc, but since
> > there are no barrier instructions at the moment the memory model
> > statement would be premature.
> > The work on "BPF Memory Model" have been ongoing for quite some time.
> > For example see:
> > https://lpc.events/event/11/contributions/941/attachments/859/1667/bpf-memory-model.2020.09.22a.pdf
> >
> > BPF Memory Model is certainly an important topic, but out of scope for ISA.
> 
> I expect that an ISA defines the semantics of the instructions. Which
> absolutely includes the memory model.  Now maybe we're envisioning a
> different splitting of this information, but I don't see how it can't
> be at a different level if you want to give the instructions
> semantics.

Hello Watson,

Thank you for sharing your thoughts on the ISA standard document thus
far. I tend to agree with you that defining a memory model (including a
memory consistency model) is inevitable, and I think it will be critical
for ensuring source code compatibility between different BPF
implementations. That said, I'm not sure that it needs to be a blocker
to us ratifying this initial proposed standard (not that you said that
explicitly) for the ISA, for a couple of reasons.

Firstly and perhaps most importantly, there is a precedence for ISA
standards evolving, sometimes significantly, as the platforms and
ecosystems surrounding a standard mature. RISC-V is an excellent example
of this. The 2.0 version of the Unprivileged ISA standard [0] published
in 2019 included_many changes that were not present in the initial
publication [1] of the standard in 2011.

[0]: https://github.com/riscv/riscv-isa-manual/releases/download/Ratified-IMAFDQC/riscv-spec-20191213.pdf
[1]: https://people.eecs.berkeley.edu/~krste/papers/EECS-2011-62.pdf

Note, for example, that the RVWMO  (RISC-V Weak Memory Ordering) Memory
Consistency Model was not ratified until v2.0 of the standard, which was
8 years following the publication of v1. Subsection 2.8 in v1.0 does
describe a Memory Model, but it's very high level, and doesn't even
mention e.g. device I/O. This is despite the v1.0 publication describing
"Atomic Memory Operation Instructions" as follows (excluding the diagram
showing the encoding which I won't take the time to type out in
plaintext form):

> The atomic memory operation (AMO) instructions perform
> read-modify-write operations for multiprocessor synchronization and
> are encoded with an R-type instruction format. These AMO instructions
> atomically load a data value from the address in rs1, place the value
> into register rd, apply a binary operator to the loaded value and the
> value in rs2, then store the result back to the address in rs1. AMOs
> can either operate on 32-bit or 64-bit words in memory. For RV64,
> 32-bit AMOs always sign-extend the value placed in rd. The address
> held in rs1 must be naturally aligned to the size of the operand
> (i.e., eight-byte aligned for 64-bit words and four-byte aligned for
> 32-bit words). If the address is not naturally aligned, a misaligned
> address trap will be generated.  The operations supported are integer
> add, logical AND, logical OR, swap, and signed and unsigned integer
> maximum and minimum.

This is in contrast to v2.0, which dedicates the entirety of Chapter 8:
"A" Standard Extension for Atomic Instructions, Version 2.1 to
discussing and formalizing atomic instructions.

That's all to say, I don't think we need to have a full and "feature
complete" ISA standard for our initial ratification. Speaking for
myself, I think it would be prudent for us to actually hold off on
defining some of these finer points like the memory model until we've
had more time to think about what the proper memory model would look
like for BPF rather than just standardizing what industry has built thus
far.

What we _definitely_ need to think through completely in this early
stage is how we're going to enable extensions to the standard, because
no matter how much work we put into this version, we're inevitably going
to have to write extensions (not that you're claiming otherwise).

In my personal opinion, I think standardizing instruction encoding and
semantics is a reasonable first step. Which leads me to my other reason
why I don't think it's necessary to formally define a memory model for
v1:

One of the unique challenges we're going to have to work through as a
Working Group is how to collaborate between the IETF and Linux Kernel
communities. I think we're doing a great job so far, especially given
how things went at IETF 117, and that folks from both communities are
already engaging on the standard. That said, I am personally of the
opinion that we're most likely to succeed in figuring out an ideal
working relationship / culture for the WG if we can do so while
iterating on a standard that has a minimal amount of complexity, and a
lower likelihood of being contentious.

That of course doesn't mean that we should sacrifice the quality of
whatever standard(s) we write and ratify, but I think that if we can
write a standard that we agree is correct, self-contained, well-written,
and can be easily extended, but that is also "non-controversial" (e.g.
for the case of the ISA, defines instruction encodings that are
implemented by clang and gcc, and whose semantics are implemented by
Linux, Microsoft, Netronome, etc), that it may be prudent to take
advantage of that in these early days to flesh out how our WG will
operate when we have to write more open-ended and potentially
contentious parts of the standard such as the BPF memory model.

What do you think? Thanks again for sharing your thoughts, and for
participating in the WG.

- David

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Watson Ladd]

Dear David,

Thank you very much for your lengthy and kind email. I agree that we
should punt on contentious points and aim to standardize what has
already been implemented across a wide range of implementations. Most
of my issues are with the format and presentation of the text, and I
think the content changes it would take are pretty noncontenous. I
don't really have any insight into what the content should be, and I'm
sure that for those who live and breath BPF every day, much of what I
am confused about is indeed obvious.

Concretely I think the following would help improve the
understandability of the document:
* After the register paragraph, describe the memory. As I understand
it it is a 64 bit, byte addressed, flat space, and maps are just
special regions in it. Maybe I'm wrong. There's some stuff about types
in the big space of instructions that maybe makes me think I am wrong.
* Say this is a 2's complement architecture
* I finally understand why the code fields have their low nybble zero.
We should maybe say this.
* Explicitly call out after 5.2 that there is no memory model yet
* Pull up section 5.3.1 to the top, or figure out some way to punt it
to an extension. Maybe introduce maps up top then explain how they are
indexed here.

For extensions if I think I understand the conversation at IETF 117,
it's easy to add more calls to the host system as functions. It's a
lot more of a difficulty to add more instructions, but in the wide
encoding space there is room. We could definitely say that. The memory
model should only modify the behavior of environments with races, so
if things aren't racy, nothing changes. That should work, but maybe I
don't understand what other extensions that people would want to add.
Verification might be an extension, but probably not in the sense we
need to worry about it here?

I hope the above is helpful: as always my ignorance can completely
negate the value of the concrete suggestion, but I do hope it
highlights areas that could use some TLC.

Sincerely,
Watson Ladd

--
Astra mortemque praestare gradatim

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Watson Ladd]

Dear David,

Thank you very much for your lengthy and kind email. I agree that we
should punt on contentious points and aim to standardize what has
already been implemented across a wide range of implementations. Most
of my issues are with the format and presentation of the text, and I
think the content changes it would take are pretty noncontenous. I
don't really have any insight into what the content should be, and I'm
sure that for those who live and breath BPF every day, much of what I
am confused about is indeed obvious.

Concretely I think the following would help improve the
understandability of the document:
* After the register paragraph, describe the memory. As I understand
it it is a 64 bit, byte addressed, flat space, and maps are just
special regions in it. Maybe I'm wrong. There's some stuff about types
in the big space of instructions that maybe makes me think I am wrong.
* Say this is a 2's complement architecture
* I finally understand why the code fields have their low nybble zero.
We should maybe say this.
* Explicitly call out after 5.2 that there is no memory model yet
* Pull up section 5.3.1 to the top, or figure out some way to punt it
to an extension. Maybe introduce maps up top then explain how they are
indexed here.

For extensions if I think I understand the conversation at IETF 117,
it's easy to add more calls to the host system as functions. It's a
lot more of a difficulty to add more instructions, but in the wide
encoding space there is room. We could definitely say that. The memory
model should only modify the behavior of environments with races, so
if things aren't racy, nothing changes. That should work, but maybe I
don't understand what other extensions that people would want to add.
Verification might be an extension, but probably not in the sense we
need to worry about it here?

I hope the above is helpful: as always my ignorance can completely
negate the value of the concrete suggestion, but I do hope it
highlights areas that could use some TLC.

Sincerely,
Watson Ladd

--
Astra mortemque praestare gradatim

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

Watson, thank you!

On Fri, Aug 11, 2023 at 5:36 PM Watson Ladd <watsonbladd@gmail.com> wrote:
>
> Dear David,
>
> Thank you very much for your lengthy and kind email. I agree that we
> should punt on contentious points and aim to standardize what has
> already been implemented across a wide range of implementations. Most
> of my issues are with the format and presentation of the text, and I
> think the content changes it would take are pretty noncontenous. I
> don't really have any insight into what the content should be, and I'm
> sure that for those who live and breath BPF every day, much of what I
> am confused about is indeed obvious.
>
> Concretely I think the following would help improve the
> understandability of the document:
> * After the register paragraph, describe the memory. As I understand
> it it is a 64 bit, byte addressed, flat space, and maps are just
> special regions in it. Maybe I'm wrong. There's some stuff about types
> in the big space of instructions that maybe makes me think I am wrong.
> * Say this is a 2's complement architecture

I just wanted to quickly follow up on this ... I, too, believe that we
should say this in the ISA and put forward a patch with some language.
You can see the discussion about that here:
https://lore.kernel.org/bpf/20230710215818.gCPVzgaK-YEXRFNixwcVrMLKlkiM0FqkQbUytFlDTQQ@z/

Thank you, again, for the conversation!
Will


> * I finally understand why the code fields have their low nybble zero.
> We should maybe say this.
> * Explicitly call out after 5.2 that there is no memory model yet
> * Pull up section 5.3.1 to the top, or figure out some way to punt it
> to an extension. Maybe introduce maps up top then explain how they are
> indexed here.
>
> For extensions if I think I understand the conversation at IETF 117,
> it's easy to add more calls to the host system as functions. It's a
> lot more of a difficulty to add more instructions, but in the wide
> encoding space there is room. We could definitely say that. The memory
> model should only modify the behavior of environments with races, so
> if things aren't racy, nothing changes. That should work, but maybe I
> don't understand what other extensions that people would want to add.
> Verification might be an extension, but probably not in the sense we
> need to worry about it here?
>
> I hope the above is helpful: as always my ignorance can completely
> negate the value of the concrete suggestion, but I do hope it
> highlights areas that could use some TLC.
>
> Sincerely,
> Watson Ladd
>
> --
> Astra mortemque praestare gradatim
>
> --
> Bpf mailing list
> Bpf@ietf.org
> https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[Will Hawkins]

Watson, thank you!

On Fri, Aug 11, 2023 at 5:36 PM Watson Ladd <watsonbladd@gmail.com> wrote:
>
> Dear David,
>
> Thank you very much for your lengthy and kind email. I agree that we
> should punt on contentious points and aim to standardize what has
> already been implemented across a wide range of implementations. Most
> of my issues are with the format and presentation of the text, and I
> think the content changes it would take are pretty noncontenous. I
> don't really have any insight into what the content should be, and I'm
> sure that for those who live and breath BPF every day, much of what I
> am confused about is indeed obvious.
>
> Concretely I think the following would help improve the
> understandability of the document:
> * After the register paragraph, describe the memory. As I understand
> it it is a 64 bit, byte addressed, flat space, and maps are just
> special regions in it. Maybe I'm wrong. There's some stuff about types
> in the big space of instructions that maybe makes me think I am wrong.
> * Say this is a 2's complement architecture

I just wanted to quickly follow up on this ... I, too, believe that we
should say this in the ISA and put forward a patch with some language.
You can see the discussion about that here:
https://lore.kernel.org/bpf/20230710215818.gCPVzgaK-YEXRFNixwcVrMLKlkiM0FqkQbUytFlDTQQ@z/

Thank you, again, for the conversation!
Will


> * I finally understand why the code fields have their low nybble zero.
> We should maybe say this.
> * Explicitly call out after 5.2 that there is no memory model yet
> * Pull up section 5.3.1 to the top, or figure out some way to punt it
> to an extension. Maybe introduce maps up top then explain how they are
> indexed here.
>
> For extensions if I think I understand the conversation at IETF 117,
> it's easy to add more calls to the host system as functions. It's a
> lot more of a difficulty to add more instructions, but in the wide
> encoding space there is room. We could definitely say that. The memory
> model should only modify the behavior of environments with races, so
> if things aren't racy, nothing changes. That should work, but maybe I
> don't understand what other extensions that people would want to add.
> Verification might be an extension, but probably not in the sense we
> need to worry about it here?
>
> I hope the above is helpful: as always my ignorance can completely
> negate the value of the concrete suggestion, but I do hope it
> highlights areas that could use some TLC.
>
> Sincerely,
> Watson Ladd
>
> --
> Astra mortemque praestare gradatim
>
> --
> Bpf mailing list
> Bpf@ietf.org
> https://www.ietf.org/mailman/listinfo/bpf

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[David Vernet]

On Fri, Aug 11, 2023 at 02:36:04PM -0700, Watson Ladd wrote:
> Dear David,

Hi Watson,

> Thank you very much for your lengthy and kind email. I agree that we

And thank you for offering your time and expertise as a reviewer of the
document.

> should punt on contentious points and aim to standardize what has
> already been implemented across a wide range of implementations. Most
> of my issues are with the format and presentation of the text, and I
> think the content changes it would take are pretty noncontenous. I

Ack, sounds good. Just FYI, I realize (and appreciate) that you are
fulfilling your role as a reviewer, as you offered to do at IETF117.
That said, even just as a reviewer, it can often be easier to discuss
proposed changes if there are accompanying patches which illustrate your
intent. For example, moving the Maps section further up the document,
etc. This is not strictly necessary, especially if you're just reviewing
the document, but I do think it would help to ground some of the
discussions. For what it's worth, this is more the "Linux kernel way",
but I'm not sure if this is the norm for IETF.

> don't really have any insight into what the content should be, and I'm
> sure that for those who live and breath BPF every day, much of what I
> am confused about is indeed obvious.

There is a difference between something being obvious and being well
specified, so your review is appreciated either way.

> Concretely I think the following would help improve the
> understandability of the document:
> * After the register paragraph, describe the memory. As I understand

Just FYI, we are going to be moving the register paragraph to a separate
ABI document, as it really belongs in an ABI (Informational) document
than a Proposed Standard for the ISA.

> it it is a 64 bit, byte addressed, flat space, and maps are just
> special regions in it. Maybe I'm wrong. There's some stuff about types
> in the big space of instructions that maybe makes me think I am wrong.

I think it's OK to specify that it's a 64 bit, byte addressed flat
space, so folks can feel free to submit a patch to that effect. My only
worry is that it may set a precedent that we'll have to explain a lot of
other architectural details in the ISA standard, where they don't really
belong. Take a look at how ARM [0] does this, for example. There is an
entirely separate document [1] on the AArch64 memory model, and for good
reason. It describes the hierarchical model of page tables on ARM, what
the memory model is when there's no MMU, how device memory works, etc.

[0]: https://www.arm.com/architecture/learn-the-architecture/a-profile
[1]: https://developer.arm.com/documentation/102376/latest/

So while I don't disagree that it should be non-controversial and would
help clarify the programming environment for the instructions, I want to
make sure that folks are OK with us adding this, but not necessarily
expounding on all of the details and implications. Hopefully this should
be fine, as it's been the norm for other ISAs (e.g. RISC-V) as well.

FWIW, I think this applies to maps as well. It's definitely something we
need to expand on at some point, but the possibility for scope creep is
high.

This is what's there now:

> =========================  ======  ===  =========================================  ===========  ==============
> opcode construction        opcode  src  pseudocode                                 imm type     dst type
> =========================  ======  ===  =========================================  ===========  ==============
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = imm64                                integer      integer
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)                       map fd       map
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst = map_val(map_by_fd(imm)) + next_imm   map fd       data pointer
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_addr(imm)                        variable id  data pointer
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x4  dst = code_addr(imm)                       integer      code pointer
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x5  dst = map_by_idx(imm)                      map index    map
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst = map_val(map_by_idx(imm)) + next_imm  map index    data pointer
> =========================  ======  ===  =========================================  ===========  ==============
>
> where
>
> * map_by_fd(imm) means to convert a 32-bit file descriptor into an address of a map (see `Maps`_)
> * map_by_idx(imm) means to convert a 32-bit index into an address of a map
> * map_val(map) gets the address of the first value in a given map
> * var_addr(imm) gets the address of a platform variable (see `Platform Variables`_) with a given id
> * code_addr(imm) gets the address of the instruction at a specified relative offset in number of (64-bit) instructions
> * the 'imm type' can be used by disassemblers for display
> * the 'dst type' can be used for verification and JIT compilation purposes
>
> Maps
> ~~~~
>
> Maps are shared memory regions accessible by eBPF programs on some
> platforms. A map can have various semantics as defined in a separate
> document, and may or may not have a single contiguous memory region,
> but the 'map_val(map)' is currently only defined for maps that do have
> a single contiguous memory region.

There are a lot of caveats here about the semantics of maps, whether
they do or don't have a contiguous memory region, etc. There likely
isn't a single definition of a "map memory region" that will apply to
all maps. For example, there is a BPF_MAP_TYPE_PERCPU_ARRAY map, which
in contrast with BPF_MAP_TYPE_ARRAY, need not be a contiguous memory
region and could instead leverage however percpu memory is implemented
on the platform. In my personal opinion, what Dave wrote here is rather
ideal in that it's giving the reader some context on what maps are,
without over prescribing them. With that in mind, are you ok with
leaving the maps section as is (modulo moving it as you suggested
below), with the intention being that we'll fill in the details on the
various types of cross-platform maps when we write the proposed standard
on cross-platform map types?

> * Say this is a 2's complement architecture

Hmmm, so, I realize that others have suggested this as well (e.g. Will
pointed out in [2] that he agrees that we should include this), but to
be honest I'm still not quite following why this is something that folks
feel should be included.

[2]: https://lore.kernel.org/all/CADx9qWiJCQyLdz5rG33K2iWtsgXQ65K3aiwQiEsjSwY2ofNy1Q@mail.gmail.com/

The RISC-V standard does indeed specify that signed types are
represented with two's complement, but as far as I know that is the
exception rather than the rule. As Jose explained in [3], it's more
common for ISA specifications to specify how numerical immediates are
encoded in stored instructions rather than dictating how signedness is
represented across the entire architecture.

[3]: https://lore.kernel.org/bpf/871qhe7des.fsf@gnu.org

The ARM64 ISA [4] aligns with what Jose said as well. The Intel and AMD
x86_64 ISAs specify that certain instructions perform two's-complement
negation, but as far as I know they don't dictate the type semantics for
the entire architecture.

[4]: https://developer.arm.com/documentation/ddi0602/latest

The reason that I think it may be prudent to leave this off is that I
don't think we really gain anything by specifying it. I expect that
we'll include this in the psABI document, but it seems unnecessarily
constraining to dictate two's complement in the PS _ISA document_. At
the end of the day I expect our goal will be source-code compatibility
rather than binary compatibility, and while it's entirely possible that
we'll never see it, I think it would be prudent to give platforms the
flexibility to implement signedness in other ways if they have a reason
to do so.

Case in point, while it's not an ISA standard, section 6.2.6
("Representations of types") of the C standard specifies the following:

> 1 The representations of all types are unspecified except as stated in this subclause.
> 2 Except for bit-fields, objects are composed of contiguous sequences of one or more bytes, the number,
> order, and encoding of which are either explicitly specified or implementation-defined.
> ...
> For signed integer types, the bits of the object representation shall be divided into three groups:
> value bits, padding bits, and the sign bit. There need not be any padding bits; signed char shall
> not have any padding bits. There shall be exactly one sign bit. Each bit that is a value bit shall have
> the same value as the same bit in the object representation of the corresponding unsigned type (if
> there are M value bits in the signed type and N in the unsigned type, then M <= N). If the sign bit is
> zero, it shall not affect the resulting value. If the sign bit is one, the value shall be modified in
> one of the following ways:
>
> — the corresponding value with sign bit 0 is negated (sign and magnitude);
> — the sign bit has the value −(2M) (two's complement);
> — the sign bit has the value −(2M − 1) (ones' complement).

In my opinion, the intention of the standard to not over-specify should
apply here as well.

> * I finally understand why the code fields have their low nybble zero.
> We should maybe say this.

Hmmm, sorry, I'm not quite following this part. Could you please clarify
which encoding / section you're referring to specifically?

> * Explicitly call out after 5.2 that there is no memory model yet

I'm not fundamentally opposed to this, though I'll share my subjective
opinion that specifying the absence of something seems unnecessary for a
standard. It seems like it would fit better in an Informational
document? Not sure.

> * Pull up section 5.3.1 to the top, or figure out some way to punt it
> to an extension. Maybe introduce maps up top then explain how they are
> indexed here.

I'm fine with this be reworked if you think it would clarify things.
Please feel free to submit patches that reformats in a way that you
think is clearer. Or Will, or someone else, etc :-)

> For extensions if I think I understand the conversation at IETF 117,
> it's easy to add more calls to the host system as functions. It's a

I'll give a bit of background here on the Linux side. It's easy to add
what we call kfuncs [5], which are host-system / main-kernel functions
that can be called from BPF. These have no UAPI guarantees, so we can
safely add them without having to worry about supporting them forever /
indefinitely. In contrast, we no longer add [6] helper functions because
they do come with UAPI requirements, and thus with much more significant
potential long-term overhead.

[5]: https://www.kernel.org/doc/html/latest/bpf/kfuncs.html
[6]: https://man7.org/linux/man-pages/man7/bpf-helpers.7.html

More on this below.

> lot more of a difficulty to add more instructions, but in the wide
> encoding space there is room. We could definitely say that. The memory

To the point above about helpers vs. kfuncs, I think there's a
nontrivial amount of subtlety we'll have to capture. And at the risk of
being a broken record, I don't think we necessarily want to try and
capture all of the nuances of the difficulty in adding instructions,
certain types of host-system functions, etc, in the ISA document. What I
feel does belong in the ISA document would be a very clear prescription
for how the ISA can be _extended_. The difficulty in extending the ISA
should hopefully be self-evident from that.

In contrast, the [I] architecture and framework document seems like a
more appropriate place to go into details on platform-specific
host-system functions vs. helpers, etc. We also have plans to implement
another proposed standard on the cross-platform helper functions:

* [PS] cross-platform helper functions, e.g., for manipulation of maps,

and I'm sure we'll also describe how that PS can be extended in that
document.

Please let me know if I've misunderstood your suggestion.

> model should only modify the behavior of environments with races, so
> if things aren't racy, nothing changes. That should work, but maybe I

Just to be clear, I think you mean the memory _consistency_ model,
correct? But yes, if you're running in e.g. a uniprocessor environment,
we shouldn't have to worry about any of that. As I said in the prior
email, I definitely agree with you that we'll need to formally define
all of this in a subsequent extension of the document, but think that
we'll be OK with sidestepping it for now. It sounds like we're on the
same page for this one?

> don't understand what other extensions that people would want to add.
> Verification might be an extension, but probably not in the sense we
> need to worry about it here?

Hmmm, I don't expect that verification would ever be relevant to
extending the ISA. There may be some platforms that don't implement
verification at all. Verification will likely be relegated to:

* [I] verifier expectations and building blocks for allowing safe
  execution of untrusted BPF programs,

from our charter, and will likely also be mentioned in the architecture
and framework informational doc.

I expect that future extensions would be something like adding a fence
instruction, etc were we to decide to go that route.

> I hope the above is helpful: as always my ignorance can completely
> negate the value of the concrete suggestion, but I do hope it
> highlights areas that could use some TLC.

Your input is very much appreciated, and I don't disagree with you that
the doc could (as with any document) always be further improved.

Just as a friendly suggestion, as I said above, if you feel strongly
about these proposals I expect that it would be easier to incorporate
them by submitting patches with the proposed changes rather than simply
describing the perceived gaps. That's not a hard requirement by any
means, and I appreciate you volunteering your time to review the
document regardless. Just a suggestion for greasing the wheels.

Kind regards,
David

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[David Vernet]

On Fri, Aug 11, 2023 at 02:36:04PM -0700, Watson Ladd wrote:
> Dear David,

Hi Watson,

> Thank you very much for your lengthy and kind email. I agree that we

And thank you for offering your time and expertise as a reviewer of the
document.

> should punt on contentious points and aim to standardize what has
> already been implemented across a wide range of implementations. Most
> of my issues are with the format and presentation of the text, and I
> think the content changes it would take are pretty noncontenous. I

Ack, sounds good. Just FYI, I realize (and appreciate) that you are
fulfilling your role as a reviewer, as you offered to do at IETF117.
That said, even just as a reviewer, it can often be easier to discuss
proposed changes if there are accompanying patches which illustrate your
intent. For example, moving the Maps section further up the document,
etc. This is not strictly necessary, especially if you're just reviewing
the document, but I do think it would help to ground some of the
discussions. For what it's worth, this is more the "Linux kernel way",
but I'm not sure if this is the norm for IETF.

> don't really have any insight into what the content should be, and I'm
> sure that for those who live and breath BPF every day, much of what I
> am confused about is indeed obvious.

There is a difference between something being obvious and being well
specified, so your review is appreciated either way.

> Concretely I think the following would help improve the
> understandability of the document:
> * After the register paragraph, describe the memory. As I understand

Just FYI, we are going to be moving the register paragraph to a separate
ABI document, as it really belongs in an ABI (Informational) document
than a Proposed Standard for the ISA.

> it it is a 64 bit, byte addressed, flat space, and maps are just
> special regions in it. Maybe I'm wrong. There's some stuff about types
> in the big space of instructions that maybe makes me think I am wrong.

I think it's OK to specify that it's a 64 bit, byte addressed flat
space, so folks can feel free to submit a patch to that effect. My only
worry is that it may set a precedent that we'll have to explain a lot of
other architectural details in the ISA standard, where they don't really
belong. Take a look at how ARM [0] does this, for example. There is an
entirely separate document [1] on the AArch64 memory model, and for good
reason. It describes the hierarchical model of page tables on ARM, what
the memory model is when there's no MMU, how device memory works, etc.

[0]: https://www.arm.com/architecture/learn-the-architecture/a-profile
[1]: https://developer.arm.com/documentation/102376/latest/

So while I don't disagree that it should be non-controversial and would
help clarify the programming environment for the instructions, I want to
make sure that folks are OK with us adding this, but not necessarily
expounding on all of the details and implications. Hopefully this should
be fine, as it's been the norm for other ISAs (e.g. RISC-V) as well.

FWIW, I think this applies to maps as well. It's definitely something we
need to expand on at some point, but the possibility for scope creep is
high.

This is what's there now:

> =========================  ======  ===  =========================================  ===========  ==============
> opcode construction        opcode  src  pseudocode                                 imm type     dst type
> =========================  ======  ===  =========================================  ===========  ==============
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = imm64                                integer      integer
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)                       map fd       map
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst = map_val(map_by_fd(imm)) + next_imm   map fd       data pointer
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_addr(imm)                        variable id  data pointer
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x4  dst = code_addr(imm)                       integer      code pointer
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x5  dst = map_by_idx(imm)                      map index    map
> BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst = map_val(map_by_idx(imm)) + next_imm  map index    data pointer
> =========================  ======  ===  =========================================  ===========  ==============
>
> where
>
> * map_by_fd(imm) means to convert a 32-bit file descriptor into an address of a map (see `Maps`_)
> * map_by_idx(imm) means to convert a 32-bit index into an address of a map
> * map_val(map) gets the address of the first value in a given map
> * var_addr(imm) gets the address of a platform variable (see `Platform Variables`_) with a given id
> * code_addr(imm) gets the address of the instruction at a specified relative offset in number of (64-bit) instructions
> * the 'imm type' can be used by disassemblers for display
> * the 'dst type' can be used for verification and JIT compilation purposes
>
> Maps
> ~~~~
>
> Maps are shared memory regions accessible by eBPF programs on some
> platforms. A map can have various semantics as defined in a separate
> document, and may or may not have a single contiguous memory region,
> but the 'map_val(map)' is currently only defined for maps that do have
> a single contiguous memory region.

There are a lot of caveats here about the semantics of maps, whether
they do or don't have a contiguous memory region, etc. There likely
isn't a single definition of a "map memory region" that will apply to
all maps. For example, there is a BPF_MAP_TYPE_PERCPU_ARRAY map, which
in contrast with BPF_MAP_TYPE_ARRAY, need not be a contiguous memory
region and could instead leverage however percpu memory is implemented
on the platform. In my personal opinion, what Dave wrote here is rather
ideal in that it's giving the reader some context on what maps are,
without over prescribing them. With that in mind, are you ok with
leaving the maps section as is (modulo moving it as you suggested
below), with the intention being that we'll fill in the details on the
various types of cross-platform maps when we write the proposed standard
on cross-platform map types?

> * Say this is a 2's complement architecture

Hmmm, so, I realize that others have suggested this as well (e.g. Will
pointed out in [2] that he agrees that we should include this), but to
be honest I'm still not quite following why this is something that folks
feel should be included.

[2]: https://lore.kernel.org/all/CADx9qWiJCQyLdz5rG33K2iWtsgXQ65K3aiwQiEsjSwY2ofNy1Q@mail.gmail.com/

The RISC-V standard does indeed specify that signed types are
represented with two's complement, but as far as I know that is the
exception rather than the rule. As Jose explained in [3], it's more
common for ISA specifications to specify how numerical immediates are
encoded in stored instructions rather than dictating how signedness is
represented across the entire architecture.

[3]: https://lore.kernel.org/bpf/871qhe7des.fsf@gnu.org

The ARM64 ISA [4] aligns with what Jose said as well. The Intel and AMD
x86_64 ISAs specify that certain instructions perform two's-complement
negation, but as far as I know they don't dictate the type semantics for
the entire architecture.

[4]: https://developer.arm.com/documentation/ddi0602/latest

The reason that I think it may be prudent to leave this off is that I
don't think we really gain anything by specifying it. I expect that
we'll include this in the psABI document, but it seems unnecessarily
constraining to dictate two's complement in the PS _ISA document_. At
the end of the day I expect our goal will be source-code compatibility
rather than binary compatibility, and while it's entirely possible that
we'll never see it, I think it would be prudent to give platforms the
flexibility to implement signedness in other ways if they have a reason
to do so.

Case in point, while it's not an ISA standard, section 6.2.6
("Representations of types") of the C standard specifies the following:

> 1 The representations of all types are unspecified except as stated in this subclause.
> 2 Except for bit-fields, objects are composed of contiguous sequences of one or more bytes, the number,
> order, and encoding of which are either explicitly specified or implementation-defined.
> ...
> For signed integer types, the bits of the object representation shall be divided into three groups:
> value bits, padding bits, and the sign bit. There need not be any padding bits; signed char shall
> not have any padding bits. There shall be exactly one sign bit. Each bit that is a value bit shall have
> the same value as the same bit in the object representation of the corresponding unsigned type (if
> there are M value bits in the signed type and N in the unsigned type, then M <= N). If the sign bit is
> zero, it shall not affect the resulting value. If the sign bit is one, the value shall be modified in
> one of the following ways:
>
> — the corresponding value with sign bit 0 is negated (sign and magnitude);
> — the sign bit has the value −(2M) (two's complement);
> — the sign bit has the value −(2M − 1) (ones' complement).

In my opinion, the intention of the standard to not over-specify should
apply here as well.

> * I finally understand why the code fields have their low nybble zero.
> We should maybe say this.

Hmmm, sorry, I'm not quite following this part. Could you please clarify
which encoding / section you're referring to specifically?

> * Explicitly call out after 5.2 that there is no memory model yet

I'm not fundamentally opposed to this, though I'll share my subjective
opinion that specifying the absence of something seems unnecessary for a
standard. It seems like it would fit better in an Informational
document? Not sure.

> * Pull up section 5.3.1 to the top, or figure out some way to punt it
> to an extension. Maybe introduce maps up top then explain how they are
> indexed here.

I'm fine with this be reworked if you think it would clarify things.
Please feel free to submit patches that reformats in a way that you
think is clearer. Or Will, or someone else, etc :-)

> For extensions if I think I understand the conversation at IETF 117,
> it's easy to add more calls to the host system as functions. It's a

I'll give a bit of background here on the Linux side. It's easy to add
what we call kfuncs [5], which are host-system / main-kernel functions
that can be called from BPF. These have no UAPI guarantees, so we can
safely add them without having to worry about supporting them forever /
indefinitely. In contrast, we no longer add [6] helper functions because
they do come with UAPI requirements, and thus with much more significant
potential long-term overhead.

[5]: https://www.kernel.org/doc/html/latest/bpf/kfuncs.html
[6]: https://man7.org/linux/man-pages/man7/bpf-helpers.7.html

More on this below.

> lot more of a difficulty to add more instructions, but in the wide
> encoding space there is room. We could definitely say that. The memory

To the point above about helpers vs. kfuncs, I think there's a
nontrivial amount of subtlety we'll have to capture. And at the risk of
being a broken record, I don't think we necessarily want to try and
capture all of the nuances of the difficulty in adding instructions,
certain types of host-system functions, etc, in the ISA document. What I
feel does belong in the ISA document would be a very clear prescription
for how the ISA can be _extended_. The difficulty in extending the ISA
should hopefully be self-evident from that.

In contrast, the [I] architecture and framework document seems like a
more appropriate place to go into details on platform-specific
host-system functions vs. helpers, etc. We also have plans to implement
another proposed standard on the cross-platform helper functions:

* [PS] cross-platform helper functions, e.g., for manipulation of maps,

and I'm sure we'll also describe how that PS can be extended in that
document.

Please let me know if I've misunderstood your suggestion.

> model should only modify the behavior of environments with races, so
> if things aren't racy, nothing changes. That should work, but maybe I

Just to be clear, I think you mean the memory _consistency_ model,
correct? But yes, if you're running in e.g. a uniprocessor environment,
we shouldn't have to worry about any of that. As I said in the prior
email, I definitely agree with you that we'll need to formally define
all of this in a subsequent extension of the document, but think that
we'll be OK with sidestepping it for now. It sounds like we're on the
same page for this one?

> don't understand what other extensions that people would want to add.
> Verification might be an extension, but probably not in the sense we
> need to worry about it here?

Hmmm, I don't expect that verification would ever be relevant to
extending the ISA. There may be some platforms that don't implement
verification at all. Verification will likely be relegated to:

* [I] verifier expectations and building blocks for allowing safe
  execution of untrusted BPF programs,

from our charter, and will likely also be mentioned in the architecture
and framework informational doc.

I expect that future extensions would be something like adding a fence
instruction, etc were we to decide to go that route.

> I hope the above is helpful: as always my ignorance can completely
> negate the value of the concrete suggestion, but I do hope it
> highlights areas that could use some TLC.

Your input is very much appreciated, and I don't disagree with you that
the doc could (as with any document) always be further improved.

Just as a friendly suggestion, as I said above, if you feel strongly
about these proposals I expect that it would be easier to incorporate
them by submitting patches with the proposed changes rather than simply
describing the perceived gaps. That's not a hard requirement by any
means, and I appreciate you volunteering your time to review the
document regardless. Just a suggestion for greasing the wheels.

Kind regards,
David

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[David Vernet]

On Mon, Aug 14, 2023 at 11:17:59AM -0500, David Vernet wrote:
> On Fri, Aug 11, 2023 at 02:36:04PM -0700, Watson Ladd wrote:
> > Dear David,
> 
> Hi Watson,

Hi everyone,

Watson and I discussed this today in more detail over a call. I think
we're now on the same page, and I want to update the lists with where we
landed so others can weigh in. The TL;DR is that the ISA document as it
exists today is trying to toe the line between being two different types
of ISA documents:

1. An ARM/x86-esque ISA document that exists somewhat in isolation, and
   just defines the encodings and high-level semantics of instructions.
   For example, if you look at the ARM A64 Instruction Set Architecture
   for Armv8 and Armv8-A document, it literally just jumps straight into
   encodings and high-level semantics of the instructions. There's
   literally zero information about the ARM memory model, execution
   environment, etc. All of that is captured in separate architecture
   documents.

2. The RISC-V ISA model, which goes into significantly more detail on
   the architecture of RISC-V, and formalizes not just the instructions,
   but the execution and memory models, memory consistency model, etc.

I am (and I believe Watson is as well following our discussion) of the
opinion that our ISA document belongs squarely in the first category,
and shouldn't try to also fit into the second. We're defining the
instruction encodings, and describing their semantics at a high level
without much formality. This is intentional -- our WG charter specifies
many more documents that cover all of these details (some of which will
likely be contentious and require a lot of thought and discussion) in
the proper scopes. For example, our planned documents include but are
not limited to:

- [I] an architecture and framework document

- one or more documents that recommend conventions and guidelines
  for producing portable BPF program binaries

- [PS] cross-platform map types allowing native data structure access
  from BPF programs

- [PS] cross-platform helper functions, e.g., for manipulation of maps

- [PS] cross-platform BPF program types that define the higher level
  execution environment for BPF programs

With all that said, I think our ISA document would improve a lot with
the following changes:

1. Removing all the ABI-specific stuff from the ISA document.  For
   example, calling conventions need to go. This was discussed at IETF
   117, so should hopefully be non-controversial. I'll send a patch
   later that moves this to a separate abi.rst document that we can then
   fold into Will's work.

2. This wasn't discussed at IETF 117, but also removing extraneous
   verbiage such as the "Helper functions" and "Maps" paragraphs, and
   maybe more such as "Platform Variables". These blurbs are useful for
   giving some context on the actual instructions in the ISA, but
   they're insufficient on their own to be of practical use.

So the TL;DR is: let's make the ISA document just an ISA document.
We're in it for the long haul, and the time to properly introduce,
define, and explain all of these concepts is when we write the documents
that are meant to capture all of that information.

Watson -- please let me know if I've misrepresented anything. It would
be great to get others' thoughts as well.

Thanks,
David

Re: [Bpf] Review of draft-thaler-bpf-isa-01

[David Vernet]

On Mon, Aug 14, 2023 at 11:17:59AM -0500, David Vernet wrote:
> On Fri, Aug 11, 2023 at 02:36:04PM -0700, Watson Ladd wrote:
> > Dear David,
> 
> Hi Watson,

Hi everyone,

Watson and I discussed this today in more detail over a call. I think
we're now on the same page, and I want to update the lists with where we
landed so others can weigh in. The TL;DR is that the ISA document as it
exists today is trying to toe the line between being two different types
of ISA documents:

1. An ARM/x86-esque ISA document that exists somewhat in isolation, and
   just defines the encodings and high-level semantics of instructions.
   For example, if you look at the ARM A64 Instruction Set Architecture
   for Armv8 and Armv8-A document, it literally just jumps straight into
   encodings and high-level semantics of the instructions. There's
   literally zero information about the ARM memory model, execution
   environment, etc. All of that is captured in separate architecture
   documents.

2. The RISC-V ISA model, which goes into significantly more detail on
   the architecture of RISC-V, and formalizes not just the instructions,
   but the execution and memory models, memory consistency model, etc.

I am (and I believe Watson is as well following our discussion) of the
opinion that our ISA document belongs squarely in the first category,
and shouldn't try to also fit into the second. We're defining the
instruction encodings, and describing their semantics at a high level
without much formality. This is intentional -- our WG charter specifies
many more documents that cover all of these details (some of which will
likely be contentious and require a lot of thought and discussion) in
the proper scopes. For example, our planned documents include but are
not limited to:

- [I] an architecture and framework document

- one or more documents that recommend conventions and guidelines
  for producing portable BPF program binaries

- [PS] cross-platform map types allowing native data structure access
  from BPF programs

- [PS] cross-platform helper functions, e.g., for manipulation of maps

- [PS] cross-platform BPF program types that define the higher level
  execution environment for BPF programs

With all that said, I think our ISA document would improve a lot with
the following changes:

1. Removing all the ABI-specific stuff from the ISA document.  For
   example, calling conventions need to go. This was discussed at IETF
   117, so should hopefully be non-controversial. I'll send a patch
   later that moves this to a separate abi.rst document that we can then
   fold into Will's work.

2. This wasn't discussed at IETF 117, but also removing extraneous
   verbiage such as the "Helper functions" and "Maps" paragraphs, and
   maybe more such as "Platform Variables". These blurbs are useful for
   giving some context on the actual instructions in the ISA, but
   they're insufficient on their own to be of practical use.

So the TL;DR is: let's make the ISA document just an ISA document.
We're in it for the long haul, and the time to properly introduce,
define, and explain all of these concepts is when we write the documents
that are meant to capture all of that information.

Watson -- please let me know if I've misrepresented anything. It would
be great to get others' thoughts as well.

Thanks,
David

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf