All of lore.kernel.org
 help / color / mirror / Atom feed
From: Conor Dooley <conor.dooley@microchip.com>
To: Andrew Jones <ajones@ventanamicro.com>
Cc: Conor Dooley <conor@kernel.org>,
	<linux-riscv@lists.infradead.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Heiko Stuebner <heiko@sntech.de>,
	Anup Patel <apatel@ventanamicro.com>,
	Atish Patra <atishp@rivosinc.com>
Subject: Re: [PATCH 1/3] RISC-V: Improve use of isa2hwcap[]
Date: Mon, 24 Oct 2022 08:16:53 +0100	[thread overview]
Message-ID: <Y1Y75dyTeqVAIzNn@wendy> (raw)
In-Reply-To: <20221024064811.rtu4nsia23yuga7w@kamzik>

On Mon, Oct 24, 2022 at 08:48:11AM +0200, Andrew Jones wrote:
> On Sun, Oct 23, 2022 at 08:28:20PM +0100, Conor Dooley wrote:
> > On Fri, Oct 21, 2022 at 12:59:03PM +0200, Andrew Jones wrote:
> > > Improve isa2hwcap[] by removing it from static storage, as
> > > riscv_fill_hwcap() is only called once, and by reducing its size
> > > from 256 bytes to 26. The latter improvement is possible because
> > > isa2hwcap[] will never be indexed with capital letters and we can
> > > precompute the offsets from 'a'.
> > 
> > Hey Drew, couple questions for you - mostly due to naivety I think..
> > 
> > How do we know that isa2hwcap will never interact with capital letters?
> > It pulls the isa string from dt and the no-capitals enforcement comes
> > from there since one with capitals is invalid? I didn't dig particularly
> > deeply into the code, but is there a risk that we regress some user that
> > has a dt with capitals in the isa string? Or is that a "your dt was wrong
> > and you're out-of-tree so that's your problem" situation?
> 
> Our ISA string parser here in riscv_fill_hwcap() ignores non-capital
> letters, see arch/riscv/kernel/cpufeature.c lines 141, 165 and 196.

Ah, I missed the one on L165, that's a bit silly... Apologies!

> (Maybe we should be noisier about that in case there are DTs out there
> with capitals which aren't realizing they're being ignored.)

Possibly, but it's not this patch that has introduced the behaviour.
Checking very briefly though, looks like it was 2a31c54be097 ("RISC-V:
Minimal parser for "riscv, isa" strings") that introduced the checks
and that was not really all that long ago..

> 
> > 
> > Secondly, in the UAPI header, the COMPAT_HWCAP_ISA_FOO defines are
> > computed as I - A rather than i - a. Should those be changed too for the
> > sake of consistently using the lowercase everywhere, or do you think
> > that doesn't really matter?
> 
> I think I'd leave it since it doesn't change the math and I'm reluctant
> to churn UAPI.

Yeah, figured you'd be of that opinion.
Anyways, only "issue" I had was my own confusion so
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks...


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2022-10-24  7:17 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-21 10:59 [PATCH 0/3] RISC-V: Ensure Zicbom has a valid block size Andrew Jones
2022-10-21 10:59 ` [PATCH 1/3] RISC-V: Improve use of isa2hwcap[] Andrew Jones
2022-10-23 19:28   ` Conor Dooley
2022-10-24  6:48     ` Andrew Jones
2022-10-24  7:16       ` Conor Dooley [this message]
2022-10-21 10:59 ` [PATCH 2/3] RISC-V: Introduce riscv_isa_extension_check Andrew Jones
2022-10-23 19:32   ` Conor Dooley
2022-10-21 10:59 ` [PATCH 3/3] RISC-V: Ensure Zicbom has a valid block size Andrew Jones
2022-10-23 19:38   ` Conor Dooley
2022-10-24  7:09     ` Andrew Jones
2022-10-24  8:17       ` Conor Dooley
2022-10-24  8:35         ` Andrew Jones
2022-10-24  9:26           ` Conor Dooley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y1Y75dyTeqVAIzNn@wendy \
    --to=conor.dooley@microchip.com \
    --cc=ajones@ventanamicro.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=apatel@ventanamicro.com \
    --cc=atishp@rivosinc.com \
    --cc=conor@kernel.org \
    --cc=heiko@sntech.de \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.