Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
MIME-Version: 1.0
References: <20230926150316.1129648-1-cleger@rivosinc.com> <20230930-patchy-curdle-ef5ee6e1a17c@spud>
 <8ce6cd97-6d63-4174-a290-40690c81e205@rivosinc.com> <20231002-spearman-doze-70cc026ac13e@spud>
 <693e6584-1e66-48c0-aa7c-61d9f88abd4c@rivosinc.com>
In-Reply-To: <693e6584-1e66-48c0-aa7c-61d9f88abd4c@rivosinc.com>
From:   ron minnich <rminnich@gmail.com>
Date:   Mon, 2 Oct 2023 09:32:56 -0600
Message-ID: <CAP6exYLtQuyqMgG6jRAhizp-52ebu3OLNSXe8a8xvLALYFD3kA@mail.gmail.com>
Subject: Re: [PATCH 0/7] Add support to handle misaligned accesses in S-mode
To:     =?UTF-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= <cleger@rivosinc.com>
Cc:     Conor Dooley <conor@kernel.org>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Atish Patra <atishp@rivosinc.com>,
        Andrew Jones <ajones@ventanamicro.com>,
        Evan Green <evan@rivosinc.com>,
        =?UTF-8?B?QmrDtnJuIFRvcGVs?= <bjorn@rivosinc.com>,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
        Daniel Maslowski <cyrevolt@googlemail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

This was a very interesting read. One other thought crossed my mind,
which is that a RISC-V implementation might make the alignment
delegation hard-wired to always delegate to S mode. I.e, the bit might
be WARL and always 1. For what I'm doing, this would actually be
pretty convenient. Just want to make sure this code can accommodate
that -- wdyt?

We have found lots of value in our experiments with delegating
alignment traps to Linux -- not least because they tend to locate
problems in the kernel :-) -- we've found issues in module loading,
early startup (there's a needed .align2 directive for sbi secondary
startup, AFAICT) and the timing code for misaligned load/store
handling.

I don't know how you test this unaligned trap handling, but it might
be worthwhile to work that out. You can test via oreboot and the
visionfive2, save we have not figured out why SMP startup is going
wrong, yet :-), so we're not as feature-complete as needed. But soon.

Thanks!

On Mon, Oct 2, 2023 at 5:19=E2=80=AFAM Cl=C3=A9ment L=C3=A9ger <cleger@rivo=
sinc.com> wrote:
>
>
>
> On 02/10/2023 12:49, Conor Dooley wrote:
> > On Mon, Oct 02, 2023 at 09:40:04AM +0200, Cl=C3=A9ment L=C3=A9ger wrote=
:
> >>
> >>
> >> On 30/09/2023 11:23, Conor Dooley wrote:
> >>> On Tue, Sep 26, 2023 at 05:03:09PM +0200, Cl=C3=A9ment L=C3=A9ger wro=
te:
> >>>> Since commit 61cadb9 ("Provide new description of misaligned load/st=
ore
> >>>> behavior compatible with privileged architecture.") in the RISC-V IS=
A
> >>>> manual, it is stated that misaligned load/store might not be support=
ed.
> >>>> However, the RISC-V kernel uABI describes that misaligned accesses a=
re
> >>>> supported. In order to support that, this series adds support for S-=
mode
> >>>> handling of misaligned accesses as well support for prctl(PR_UNALIGN=
).
> >>>>
> >>>> Handling misaligned access in kernel allows for a finer grain contro=
l
> >>>> of the misaligned accesses behavior, and thanks to the prctl call, c=
an
> >>>> allow disabling misaligned access emulation to generate SIGBUS. User
> >>>> space can then optimize its software by removing such access based o=
n
> >>>> SIGBUS generation.
> >>>>
> >>>> Currently, this series is useful for people that uses a SBI that doe=
s
> >>>> not handled misaligned traps. In a near future, this series will mak=
e
> >>>> use a SBI extension [1] allowing to request delegation of the
> >>>> misaligned load/store traps to the S-mode software. This extension h=
as
> >>>> been submitted for review to the riscv tech-prs group. An OpenSBI
> >>>> implementation for this spec is available at [2].
> >>>>
> >>>> This series can be tested using the spike simulator [3] and an openS=
BI
> >>>> version [4] which allows to always delegate misaligned load/store to
> >>>> S-mode.
> >>>
> >>> Some patches in this series do not build for any configs, some are
> >>> broken for clang builds and others are broken for nommu. Please try t=
o> build test this more thoroughly before you submit the next version.
> >>
> >> Hi Conor,
> >>
> >> Thanks for the feedback, I'll check that.
> >>
> >>>
> >>> Also, AIUI, this series should be marked RFC since the SBI extension
> >>> this relies on has not been frozen.
> >>
> >> This series does not actually uses the SBI extension but provides a wa=
y
> >> to detect if misaligned accesses are not handled by hardware nor by th=
e
> >> SBI. It has been reported by Ron & Daniel they they have a minimal SBI
> >> implementation that does not handle misaligned accesses and that they
> >> would like to make use of the PR_SET_UNALIGN feature. This is what thi=
s
> >> series addresses (and thus does not depend on the mentioned SBI extens=
ion).
> >
> > Ah, I must have misread then. Apologies.
>
> No worries, maybe I should actually remove this from the cover letter to
> avoid any confusion !
>
> Cl=C3=A9ment