lkml.org 
[lkml]   [2024]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 2/2] riscv: T-Head: Test availability bit before enabling MAEE errata
On Wed, Mar 27, 2024 at 11:03:06AM +0000, Conor Dooley wrote:
> On Wed, Mar 27, 2024 at 11:31:30AM +0100, Christoph Müllner wrote:
> > T-Head's MAEE mechanism (non-compatible equivalent of RVI's Svpbmt)
> > is currently assumed for all T-Head harts. However, QEMU recently
> > decided to drop acceptance of guests that write reserved bits in PTEs.
> > As MAEE uses reserved bits in PTEs and Linux applies the MAEE errata
> > for all T-Head harts, this broke the Linux startup on QEMU emulations
> > of the C906 emulation.
> >
> > This patch attempts to address this issue by testing the MAEE bit
> > in TH_MXSTATUS CSR. As the TH_MXSTATUS CSR is only accessible in M-mode
> > this patch depends on M-mode firmware that handles this for us
> > transparently.
> >
>
> > As this patch breaks Linux bootup on all C9xx machines with MAEE,
> > which don't have M-mode firmware that handles the access to the
> > TH_MXSTATUS CSR, this patch is marked as RFC.

Can we wrap the csr access in a _ASM_EXTABLE()? If firmware handles it,
then we return true/false based on the value. If firmware doesn't handle
it, and we get an illegal instruction exception, then we assume the bit
is set, which is the current behavior.

>
> I think this is gonna be unacceptable in its current state given that it
> causes problems for every other version of the firmware. Breaking real
> systems for the sake of emulation isn't something we can reasonably do.
>
> To make this sort of change acceptable, you're gonna have to add some way
> to differentiate between systems that do and do not support reading this
> CSR. I think we either a) need to check the version of the SBI
> implementation to see if it hits the threshold for supporting this
> feature, or b) add a specific SBI call for this so that we can
> differentiate between firmware not supporting the function and the

The FWFT SBI extension is being developed as a mechanism for S-mode to ask
M-mode things like this, but I think that extension should be used for
features that have potential to be changed by S-mode (even if not
everything will be changeable on all platforms), whereas anything that's
read-only would be better with...

> quote-unquote "hardware" not supporting it. I don't really like option a)
> as it could grow to several different options (each for a different SBI
> implementation) and support for reading the CSR would need to be
> unconditional. I have a feeling that I am missing something though,
> that'd make it doable without introducing a new call.
>
> Thanks,
> Conor.
>
> If only we'd made enabling this be controlled by a specific DT property,
> then disabling it in QEMU would be as simple as not setting that
> property :(

..this, where "DT property" is "ISA extension name". I wonder if we
shouldn't start considering the invention of xlinux_vendor_xyz type
extension names which firmware could add to the ISA string / array,
in order to communicate read-only information like this?

Thanks,
drew

>
> >
> > Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
> > ---
> > arch/riscv/errata/thead/errata.c | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> > index 8c8a8a4b0421..dd7bf6c62a35 100644
> > --- a/arch/riscv/errata/thead/errata.c
> > +++ b/arch/riscv/errata/thead/errata.c
> > @@ -19,6 +19,9 @@
> > #include <asm/patch.h>
> > #include <asm/vendorid_list.h>
> >
> > +#define CSR_TH_MXSTATUS 0x7c0
> > +#define MXSTATUS_MAEE _AC(0x200000, UL)
> > +
> > static bool errata_probe_maee(unsigned int stage,
> > unsigned long arch_id, unsigned long impid)
> > {
> > @@ -28,11 +31,14 @@ static bool errata_probe_maee(unsigned int stage,
> > if (arch_id != 0 || impid != 0)
> > return false;
> >
> > - if (stage == RISCV_ALTERNATIVES_EARLY_BOOT ||
> > - stage == RISCV_ALTERNATIVES_MODULE)
> > - return true;
> > + if (stage != RISCV_ALTERNATIVES_EARLY_BOOT &&
> > + stage != RISCV_ALTERNATIVES_MODULE)
> > + return false;
> >
> > - return false;
> > + if (!(csr_read(CSR_TH_MXSTATUS) & MXSTATUS_MAEE))
> > + return false;
> > +
> > + return true;
> > }
> >
> > /*
> > --
> > 2.44.0
> >



\
 
 \ /
  Last update: 2024-03-27 15:19    [W:0.078 / U:3.288 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site