Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200914083622.116554-1-ilias.apalodimas@linaro.org>
 <20200914122042.GA24441@willie-the-truck> <20200914123504.GA124316@apalos.home>
 <20200914132350.GA126552@apalos.home> <20200914140114.GG24441@willie-the-truck>
 <20200914181234.0f1df8ba@carbon> <20200914170205.GA20549@apalos.home>
 <CAKU6vyaxnzWVA=MPAuDwtu4UOTWS6s0cZOYQKVhQg5Mue7Wbww@mail.gmail.com>
 <20200914175516.GA21832@apalos.home> <CAKU6vybuEGYtqh9gL9bwFaJ6xD=diN-0w_Mgc2Xyu4tHMdWgAA@mail.gmail.com>
 <20200914182756.GA22294@apalos.home>
In-Reply-To: <20200914182756.GA22294@apalos.home>
From:   Xi Wang <xi.wang@gmail.com>
Date:   Mon, 14 Sep 2020 11:52:16 -0700
Message-ID: <CAKU6vyYhG20qaA1iKwD=-pZHWjZYEZvX6Qmjs=aA-uJ-uwCw7w@mail.gmail.com>
Subject: Re: [PATCH] arm64: bpf: Fix branch offset in JIT
To:     Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc:     Jesper Dangaard Brouer <brouer@redhat.com>,
        Will Deacon <will@kernel.org>, bpf@vger.kernel.org,
        ardb@kernel.org, naresh.kamboju@linaro.org,
        Jean-Philippe Brucker <jean-philippe@linaro.org>,
        Yauheni Kaliuta <yauheni.kaliuta@redhat.com>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Alexei Starovoitov <ast@kernel.org>,
        Zi Shen Lim <zlim.lnx@gmail.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        Andrii Nakryiko <andriin@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@chromium.org>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Anders Roxell <anders.roxell@linaro.org>,
        =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn.topel@gmail.com>,
        Luke Nelson <luke.r.nels@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Sep 14, 2020 at 11:28 AM Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
> Even if that's true, is any reason at all why we should skip the first element
> of the array, that's now needed since 7c2e988f400 to jump back to the first
> instruction?
> Introducing 2 extra if conditions and hotfix the array on the fly (and for
> every future invocation of that), seems better to you?

My point was that there's no inherently correct/wrong way to construct
offsets.  As Luke explained in his email, 1) there are two different
strategies used by the JITs and 2) there are likely similar bugs
beyond arm64.

Each strategy has pros and cons, and I'm fine with either.  I like the
strategy used in your patch because it's more intuitive (offset[i] is
the start of the emitted instructions for BPF instruction i, rather
than the end), though the changes to the construction process are
trickier.

If we decide to patch the arm64 JIT the way you proposed, we should
consider whether to change other JITs consistently.