Linux-RISC-V Archive on lore.kernel.org
 help / color / Atom feed
* [v1 PATCH  0/2] Cleanup isa string access and print 
@ 2019-10-04  1:19 Atish Patra
  2019-10-04  1:19 ` [v1 PATCH 1/2] RISC-V: Remove unsupported isa string info print Atish Patra
  2019-10-04  1:20 ` [v1 PATCH 2/2] RISC-V: Consolidate isa correctness check Atish Patra
  0 siblings, 2 replies; 6+ messages in thread
From: Atish Patra @ 2019-10-04  1:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: Albert Ou, Richard Fontana, Greg Kroah-Hartman, Palmer Dabbelt,
	Johan Hovold, Alexandre Ghiti, Atish Patra, Thomas Gleixner,
	Paul Walmsley, Anup Patel, Andrew Morton, linux-riscv

This is a cleanup series addressing issues around isa string accesses
and prints. Patch 1 is actually a revised patch as a result of discussion
in the following thread.

http://lists.infradead.org/pipermail/linux-riscv/2019-September/006702.html

Patch 2 is an additional cleanup that tries to consolidate all isa
string related checks.

Atish Patra (2):
RISC-V: Remove unsupported isa string info print
RISC-V: Consolidate isa correctness check

arch/riscv/include/asm/processor.h |  1 +
arch/riscv/kernel/cpu.c            | 85 ++++++++++++------------------
arch/riscv/kernel/cpufeature.c     |  4 +-
arch/riscv/kernel/smpboot.c        |  4 ++
4 files changed, 39 insertions(+), 55 deletions(-)

--
2.21.0


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [v1 PATCH  1/2] RISC-V: Remove unsupported isa string info print
  2019-10-04  1:19 [v1 PATCH 0/2] Cleanup isa string access and print Atish Patra
@ 2019-10-04  1:19 ` Atish Patra
  2019-10-08 15:39   ` Christoph Hellwig
  2019-10-04  1:20 ` [v1 PATCH 2/2] RISC-V: Consolidate isa correctness check Atish Patra
  1 sibling, 1 reply; 6+ messages in thread
From: Atish Patra @ 2019-10-04  1:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: Albert Ou, Richard Fontana, Greg Kroah-Hartman, Palmer Dabbelt,
	Johan Hovold, Alexandre Ghiti, Atish Patra, Thomas Gleixner,
	Paul Walmsley, Anup Patel, Andrew Morton, linux-riscv

/proc/cpuinfo should just print all the isa string as an information
instead of determining what is supported or not. ELF hwcap can be
used by the userspace to figure out that.

Simplify the isa string printing by removing the unsupported isa string
print and all related code.

The relevant discussion can be found at
http://lists.infradead.org/pipermail/linux-riscv/2019-September/006702.html

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/kernel/cpu.c | 45 +++--------------------------------------
 1 file changed, 3 insertions(+), 42 deletions(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 7da3c6a93abd..40a3c442ac5f 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -46,51 +46,12 @@ int riscv_of_processor_hartid(struct device_node *node)
 
 #ifdef CONFIG_PROC_FS
 
-static void print_isa(struct seq_file *f, const char *orig_isa)
+static void print_isa(struct seq_file *f, const char *isa)
 {
-	static const char *ext = "mafdcsu";
-	const char *isa = orig_isa;
-	const char *e;
-
-	/*
-	 * Linux doesn't support rv32e or rv128i, and we only support booting
-	 * kernels on harts with the same ISA that the kernel is compiled for.
-	 */
-#if defined(CONFIG_32BIT)
-	if (strncmp(isa, "rv32i", 5) != 0)
-		return;
-#elif defined(CONFIG_64BIT)
-	if (strncmp(isa, "rv64i", 5) != 0)
-		return;
-#endif
-
-	/* Print the base ISA, as we already know it's legal. */
+	/* Print the entire ISA as it is */
 	seq_puts(f, "isa\t\t: ");
-	seq_write(f, isa, 5);
-	isa += 5;
-
-	/*
-	 * Check the rest of the ISA string for valid extensions, printing those
-	 * we find.  RISC-V ISA strings define an order, so we only print the
-	 * extension bits when they're in order. Hide the supervisor (S)
-	 * extension from userspace as it's not accessible from there.
-	 */
-	for (e = ext; *e != '\0'; ++e) {
-		if (isa[0] == e[0]) {
-			if (isa[0] != 's')
-				seq_write(f, isa, 1);
-
-			isa++;
-		}
-	}
+	seq_write(f, isa, strlen(isa));
 	seq_puts(f, "\n");
-
-	/*
-	 * If we were given an unsupported ISA in the device tree then print
-	 * a bit of info describing what went wrong.
-	 */
-	if (isa[0] != '\0')
-		pr_info("unsupported ISA \"%s\" in device tree\n", orig_isa);
 }
 
 static void print_mmu(struct seq_file *f, const char *mmu_type)
-- 
2.21.0


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [v1 PATCH  2/2] RISC-V: Consolidate isa correctness check
  2019-10-04  1:19 [v1 PATCH 0/2] Cleanup isa string access and print Atish Patra
  2019-10-04  1:19 ` [v1 PATCH 1/2] RISC-V: Remove unsupported isa string info print Atish Patra
@ 2019-10-04  1:20 ` Atish Patra
  2019-10-08 15:44   ` Christoph Hellwig
  1 sibling, 1 reply; 6+ messages in thread
From: Atish Patra @ 2019-10-04  1:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: Albert Ou, Richard Fontana, Greg Kroah-Hartman, Palmer Dabbelt,
	Johan Hovold, Alexandre Ghiti, Atish Patra, Thomas Gleixner,
	Paul Walmsley, Anup Patel, Andrew Morton, linux-riscv

Currently, isa string is read and checked for correctness at multiple
places.

Consolidate them into one function and use it only during early bootup.
In case of a incorrect isa string, the cpu shouldn't boot at all.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
 arch/riscv/include/asm/processor.h |  1 +
 arch/riscv/kernel/cpu.c            | 40 ++++++++++++++++++++++--------
 arch/riscv/kernel/cpufeature.c     |  4 +--
 arch/riscv/kernel/smpboot.c        |  4 +++
 4 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index f539149d04c2..189bf98f9a3f 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -74,6 +74,7 @@ static inline void wait_for_interrupt(void)
 }
 
 struct device_node;
+int riscv_read_check_isa(struct device_node *node, const char **isa);
 int riscv_of_processor_hartid(struct device_node *node);
 
 extern void riscv_fill_hwcap(void);
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 40a3c442ac5f..95ef5c91823d 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -8,13 +8,42 @@
 #include <linux/of.h>
 #include <asm/smp.h>
 
+int riscv_read_check_isa(struct device_node *node, const char **isa)
+{
+	u32 hart;
+
+	if (of_property_read_u32(node, "reg", &hart)) {
+		pr_warn("Found CPU without hart ID\n");
+		return -ENODEV;
+	}
+
+	if (of_property_read_string(node, "riscv,isa", isa)) {
+		pr_warn("CPU with hartid=%d has no \"riscv,isa\" property\n",
+			hart);
+		return -ENODEV;
+	}
+
+	/*
+	 * Linux doesn't support rv32e or rv128i, and we only support booting
+	 * kernels on harts with the same ISA that the kernel is compiled for.
+	 */
+#if defined(CONFIG_32BIT)
+	if (strncmp(*isa, "rv32i", 5) != 0)
+		return -ENODEV;
+#elif defined(CONFIG_64BIT)
+	if (strncmp(*isa, "rv64i", 5) != 0)
+		return -ENODEV;
+#endif
+
+	return 0;
+}
+
 /*
  * Returns the hart ID of the given device tree node, or -ENODEV if the node
  * isn't an enabled and valid RISC-V hart node.
  */
 int riscv_of_processor_hartid(struct device_node *node)
 {
-	const char *isa;
 	u32 hart;
 
 	if (!of_device_is_compatible(node, "riscv")) {
@@ -32,15 +61,6 @@ int riscv_of_processor_hartid(struct device_node *node)
 		return -ENODEV;
 	}
 
-	if (of_property_read_string(node, "riscv,isa", &isa)) {
-		pr_warn("CPU with hartid=%d has no \"riscv,isa\" property\n", hart);
-		return -ENODEV;
-	}
-	if (isa[0] != 'r' || isa[1] != 'v') {
-		pr_warn("CPU with hartid=%d has an invalid ISA of \"%s\"\n", hart, isa);
-		return -ENODEV;
-	}
-
 	return hart;
 }
 
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index b1ade9a49347..eaad5aa07403 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -38,10 +38,8 @@ void riscv_fill_hwcap(void)
 		if (riscv_of_processor_hartid(node) < 0)
 			continue;
 
-		if (of_property_read_string(node, "riscv,isa", &isa)) {
-			pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
+		if (riscv_read_check_isa(node, &isa) < 0)
 			continue;
-		}
 
 		for (i = 0; i < strlen(isa); ++i)
 			this_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 18ae6da5115e..15ee71297abf 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -60,12 +60,16 @@ void __init setup_smp(void)
 	int hart;
 	bool found_boot_cpu = false;
 	int cpuid = 1;
+	const char *isa;
 
 	for_each_of_cpu_node(dn) {
 		hart = riscv_of_processor_hartid(dn);
 		if (hart < 0)
 			continue;
 
+		if (riscv_read_check_isa(dn, &isa) < 0)
+			continue;
+
 		if (hart == cpuid_to_hartid_map(0)) {
 			BUG_ON(found_boot_cpu);
 			found_boot_cpu = 1;
-- 
2.21.0


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [v1 PATCH  1/2] RISC-V: Remove unsupported isa string info print
  2019-10-04  1:19 ` [v1 PATCH 1/2] RISC-V: Remove unsupported isa string info print Atish Patra
@ 2019-10-08 15:39   ` Christoph Hellwig
  0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2019-10-08 15:39 UTC (permalink / raw)
  To: Atish Patra
  Cc: Albert Ou, Greg Kroah-Hartman, Anup Patel, Palmer Dabbelt,
	Johan Hovold, linux-riscv, linux-kernel, Richard Fontana,
	Paul Walmsley, Thomas Gleixner, Alexandre Ghiti, Andrew Morton

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [v1 PATCH  2/2] RISC-V: Consolidate isa correctness check
  2019-10-04  1:20 ` [v1 PATCH 2/2] RISC-V: Consolidate isa correctness check Atish Patra
@ 2019-10-08 15:44   ` Christoph Hellwig
  2019-10-09 22:02     ` Atish Patra
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2019-10-08 15:44 UTC (permalink / raw)
  To: Atish Patra
  Cc: Albert Ou, Greg Kroah-Hartman, Anup Patel, Palmer Dabbelt,
	Johan Hovold, linux-riscv, linux-kernel, Richard Fontana,
	Paul Walmsley, Thomas Gleixner, Alexandre Ghiti, Andrew Morton

> +int riscv_read_check_isa(struct device_node *node, const char **isa)
> +{
> +	u32 hart;
> +
> +	if (of_property_read_u32(node, "reg", &hart)) {
> +		pr_warn("Found CPU without hart ID\n");
> +		return -ENODEV;
> +	}
> +
> +	if (of_property_read_string(node, "riscv,isa", isa)) {
> +		pr_warn("CPU with hartid=%d has no \"riscv,isa\" property\n",
> +			hart);
> +		return -ENODEV;
> +	}
> +
> +	/*
> +	 * Linux doesn't support rv32e or rv128i, and we only support booting
> +	 * kernels on harts with the same ISA that the kernel is compiled for.
> +	 */
> +#if defined(CONFIG_32BIT)
> +	if (strncmp(*isa, "rv32i", 5) != 0)
> +		return -ENODEV;
> +#elif defined(CONFIG_64BIT)
> +	if (strncmp(*isa, "rv64i", 5) != 0)
> +		return -ENODEV;
> +#endif

Using IS_ENABLED here would clean the checks up a bit.

> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index b1ade9a49347..eaad5aa07403 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -38,10 +38,8 @@ void riscv_fill_hwcap(void)
>  		if (riscv_of_processor_hartid(node) < 0)
>  			continue;
>  
> -		if (of_property_read_string(node, "riscv,isa", &isa)) {
> -			pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
> +		if (riscv_read_check_isa(node, &isa) < 0)
>  			continue;

Do we really get rid of warnings if we didn't find anything proper?

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [v1 PATCH  2/2] RISC-V: Consolidate isa correctness check
  2019-10-08 15:44   ` Christoph Hellwig
@ 2019-10-09 22:02     ` Atish Patra
  0 siblings, 0 replies; 6+ messages in thread
From: Atish Patra @ 2019-10-09 22:02 UTC (permalink / raw)
  To: hch
  Cc: aou, gregkh, anup, palmer, johan, aghiti, rfontana, tglx,
	paul.walmsley, linux-riscv, akpm, linux-kernel

On Tue, 2019-10-08 at 08:44 -0700, Christoph Hellwig wrote:
> > +int riscv_read_check_isa(struct device_node *node, const char
> > **isa)
> > +{
> > +	u32 hart;
> > +
> > +	if (of_property_read_u32(node, "reg", &hart)) {
> > +		pr_warn("Found CPU without hart ID\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (of_property_read_string(node, "riscv,isa", isa)) {
> > +		pr_warn("CPU with hartid=%d has no \"riscv,isa\"
> > property\n",
> > +			hart);
> > +		return -ENODEV;
> > +	}
> > +
> > +	/*
> > +	 * Linux doesn't support rv32e or rv128i, and we only support
> > booting
> > +	 * kernels on harts with the same ISA that the kernel is
> > compiled for.
> > +	 */
> > +#if defined(CONFIG_32BIT)
> > +	if (strncmp(*isa, "rv32i", 5) != 0)
> > +		return -ENODEV;
> > +#elif defined(CONFIG_64BIT)
> > +	if (strncmp(*isa, "rv64i", 5) != 0)
> > +		return -ENODEV;
> > +#endif
> 
> Using IS_ENABLED here would clean the checks up a bit.
> 
> > diff --git a/arch/riscv/kernel/cpufeature.c
> > b/arch/riscv/kernel/cpufeature.c
> > index b1ade9a49347..eaad5aa07403 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -38,10 +38,8 @@ void riscv_fill_hwcap(void)
> >  		if (riscv_of_processor_hartid(node) < 0)
> >  			continue;
> >  
> > -		if (of_property_read_string(node, "riscv,isa", &isa)) {
> > -			pr_warn("Unable to find \"riscv,isa\"
> > devicetree entry\n");
> > +		if (riscv_read_check_isa(node, &isa) < 0)
> >  			continue;
> 
> Do we really get rid of warnings if we didn't find anything proper?

Ok. Added back the warnings and IS_ENABLED in v2.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-04  1:19 [v1 PATCH 0/2] Cleanup isa string access and print Atish Patra
2019-10-04  1:19 ` [v1 PATCH 1/2] RISC-V: Remove unsupported isa string info print Atish Patra
2019-10-08 15:39   ` Christoph Hellwig
2019-10-04  1:20 ` [v1 PATCH 2/2] RISC-V: Consolidate isa correctness check Atish Patra
2019-10-08 15:44   ` Christoph Hellwig
2019-10-09 22:02     ` Atish Patra

Linux-RISC-V Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-riscv/0 linux-riscv/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-riscv linux-riscv/ https://lore.kernel.org/linux-riscv \
		linux-riscv@lists.infradead.org infradead-linux-riscv@archiver.kernel.org
	public-inbox-index linux-riscv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-riscv


AGPL code for this site: git clone https://public-inbox.org/ public-inbox