Re: Patch "riscv: Add a custom ISA extension for the [ms]envcfg CSR" has been added to the 6.7-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 28, 2024 at 08:23:40AM +0000, Conor Dooley wrote:
> On Wed, Mar 27, 2024 at 09:06:39PM -0700, Ron Economos wrote:
> > On 3/27/24 4:48 PM, Conor Dooley wrote:
> > > On Wed, Mar 27, 2024 at 12:00:27PM -0700, Ron Economos wrote:
> > > > On 3/27/24 7:40 AM, Greg KH wrote:
> > > > > On Tue, Mar 05, 2024 at 08:05:30AM +0000, Conor.Dooley@xxxxxxxxxxxxx wrote:
> > > > > > On 05/03/2024 07:44, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
> > > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > > > > > > 
> > > > > > > This is a note to let you know that I've just added the patch titled
> > > > > > > 
> > > > > > >        riscv: Add a custom ISA extension for the [ms]envcfg CSR
> > > > > > > 
> > > > > > > to the 6.7-stable tree which can be found at:
> > > > > > >        http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > > > > > > 
> > > > > > > The filename of the patch is:
> > > > > > >         riscv-add-a-custom-isa-extension-for-the-envcfg-csr.patch
> > > > > > > and it can be found in the queue-6.7 subdirectory.
> > > > > > > 
> > > > > > > If you, or anyone else, feels it should not be added to the stable tree,
> > > > > > > please let <stable@xxxxxxxxxxxxxxx> know about it.
> > > > > > > 
> > > > > > > 
> > > > > > >    From 4774848fef6041716a4883217eb75f6b10eb183b Mon Sep 17 00:00:00 2001
> > > > > > > From: Samuel Holland <samuel.holland@xxxxxxxxxx>
> > > > > > > Date: Tue, 27 Feb 2024 22:55:34 -0800
> > > > > > > Subject: riscv: Add a custom ISA extension for the [ms]envcfg CSR
> > > > > > > 
> > > > > > > From: Samuel Holland <samuel.holland@xxxxxxxxxx>
> > > > > > > 
> > > > > > > commit 4774848fef6041716a4883217eb75f6b10eb183b upstream.
> > > > > > > 
> > > > > > > The [ms]envcfg CSR was added in version 1.12 of the RISC-V privileged
> > > > > > > ISA (aka S[ms]1p12). However, bits in this CSR are defined by several
> > > > > > > other extensions which may be implemented separately from any particular
> > > > > > > version of the privileged ISA (for example, some unrelated errata may
> > > > > > > prevent an implementation from claiming conformance with Ss1p12). As a
> > > > > > > result, Linux cannot simply use the privileged ISA version to determine
> > > > > > > if the CSR is present. It must also check if any of these other
> > > > > > > extensions are implemented. It also cannot probe the existence of the
> > > > > > > CSR at runtime, because Linux does not require Sstrict, so (in the
> > > > > > > absence of additional information) it cannot know if a CSR at that
> > > > > > > address is [ms]envcfg or part of some non-conforming vendor extension.
> > > > > > > 
> > > > > > > Since there are several standard extensions that imply the existence of
> > > > > > > the [ms]envcfg CSR, it becomes unwieldy to check for all of them
> > > > > > > wherever the CSR is accessed. Instead, define a custom Xlinuxenvcfg ISA
> > > > > > > extension bit that is implied by the other extensions and denotes that
> > > > > > > the CSR exists as defined in the privileged ISA, containing at least one
> > > > > > > of the fields common between menvcfg and senvcfg.
> > > > > > > 
> > > > > > > This extension does not need to be parsed from the devicetree or ISA
> > > > > > > string because it can only be implemented as a subset of some other
> > > > > > > standard extension.
> > > > > > > 
> > > > > > > Cc: <stable@xxxxxxxxxxxxxxx> # v6.7+
> > > > > > > Signed-off-by: Samuel Holland <samuel.holland@xxxxxxxxxx>
> > > > > > > Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > > > > > > Reviewed-by: Andrew Jones <ajones@xxxxxxxxxxxxxxxx>
> > > > > > > Link: https://lore.kernel.org/r/20240228065559.3434837-3-samuel.holland@xxxxxxxxxx
> > > > > > > Signed-off-by: Palmer Dabbelt <palmer@xxxxxxxxxxxx>
> > > > > > > Cc: Ron Economos <re@xxxxxxxx>
> > > > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > > > > > ---
> > > > > > >     arch/riscv/include/asm/hwcap.h |    2 ++
> > > > > > >     arch/riscv/kernel/cpufeature.c |   14 ++++++++++++--
> > > > > > >     2 files changed, 14 insertions(+), 2 deletions(-)
> > > > > > > 
> > > > > > > --- a/arch/riscv/include/asm/hwcap.h
> > > > > > > +++ b/arch/riscv/include/asm/hwcap.h
> > > > > > > @@ -58,6 +58,8 @@
> > > > > > >     #define RISCV_ISA_EXT_SMSTATEEN                43
> > > > > > >     #define RISCV_ISA_EXT_ZICOND           44
> > > > > > > 
> > > > > > > +#define RISCV_ISA_EXT_XLINUXENVCFG     127
> > > > > > > +
> > > > > > >     #define RISCV_ISA_EXT_MAX              64
> > > > > > These defines here need to be lower than RISCV_ISA_EXT_MAX.
> > > > > > I think adjusting the value of XLINUXENVCFG to 63 will
> > > > > > suffice here, the max got bumped to 128 in 6.8.
> > > > > Can you send a patch for this?
> > > > > 
> > > > > thanks,
> > > > > 
> > > > > greg k-h
> > > > This patch was subsequently reverted, so there's nothing to do here.
> > > The former might be true, but I don't think the latter is, the patch
> > > was CCed to stable for a reason. I'll add it to my todo list Greg, but
> > > no promises for when I will actually get around to it. It's not my patch
> > > so I don't have a usecase that needs this or w/e.
> > > 
> > > Cheers,
> > > Conor.
> > 
> > I was the one that requested this patch. Here's the history.
> > 
> > The upstream commit 05ab803d1ad8ac505ade77c6bd3f86b1b4ea0dc4 "riscv:
> > Save/restore envcfg CSR during CPU suspend" was added to 6.7.9-rc1
> > because it has a Fixes tag. This caused a build failure because
> > RISCV_ISA_EXT_XLINUXENVCFG wasn't defined.
> > 
> > I asked Greg to apply this upstream commit
> > 4774848fef6041716a4883217eb75f6b10eb183b "riscv: Add a custom ISA
> > extension for the [ms]envcfg CSR" to resolve the issue (since it defines
> > RISCV_ISA_EXT_XLINUXENVCFG). However, I neglected to test the patch and
> > it failed with zicbom and zicboz being undefined.
> 
> Something that seems missing from your history is that the author of
> the patch put "Cc: <stable@xxxxxxxxxxxxxxx> # v6.7+" into the commit
> message, so this should have gone to stable regardless. However, it
> failed to get applied:
> https://lore.kernel.org/all/2024030445-rants-grading-f698@gregkh/
> but Greg picked it up when you asked for it, which I think is where
> this ties into your recollection:
> https://lore.kernel.org/all/2024030538-affair-bristle-25d8@gregkh/
> 
> > At this point I realized that there was a whole series of patches for
> > this code going pretty far back in the commit history. Instead of trying
> > to sort things out with possibly many patches, I requested that Greg
> > drop both "riscv: Save/restore envcfg CSR during CPU suspend" and this
> > patch "riscv: Add a custom ISA extension for the [ms]envcfg CSR". This
> > solved the build issues and 6.7.9 was shipped.
> > 
> > So the real culprit was "riscv: Save/restore envcfg CSR during CPU
> > suspend". This seems like a pretty complex backport for 6.7.x which is
> > going away soon, so that's why I said there's nothing to be done.
> 
> If 6.7 is going away soon, then sure, probably a waste of our time doing
> anything here, but I think all that's required would be to do something
> like:

This will be the last 6.7 release in a few days, so I wouldn't worry
about it.

thanks,

greg k-h




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux