Re: [RFC patch 2/4] riscv: Get CPU manufacturer information

[Sean Anderson <seanga2@gmail.com>]

On 3/8/21 6:30 PM, Damien Le Moal wrote:
> On 2021/03/08 12:59, Vincent Chen wrote:
>> Issue 3 SBI calls to get the vendor ID, architecture ID and implementation
>> ID early in boot so we only need to take the SBI call overhead once.
>>
>> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
>> ---
>>   arch/riscv/include/asm/csr.h       |  3 +++
>>   arch/riscv/include/asm/hwcap.h     |  6 ++++++
>>   arch/riscv/include/asm/processor.h |  2 ++
>>   arch/riscv/include/asm/soc.h       |  1 +
>>   arch/riscv/kernel/cpufeature.c     | 17 +++++++++++++++++
>>   arch/riscv/kernel/setup.c          |  2 ++
>>   arch/riscv/kernel/soc.c            |  1 +
>>   7 files changed, 32 insertions(+)
>>
>> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
>> index caadfc1d7487..87ac65696871 100644
>> --- a/arch/riscv/include/asm/csr.h
>> +++ b/arch/riscv/include/asm/csr.h
>> @@ -115,6 +115,9 @@
>>   #define CSR_MIP			0x344
>>   #define CSR_PMPCFG0		0x3a0
>>   #define CSR_PMPADDR0		0x3b0
>> +#define CSR_MVENDORID		0xf11
>> +#define CSR_MARCHID		0xf12
>> +#define CSR_MIMPID		0xf13
>>   #define CSR_MHARTID		0xf14
>>   
>>   #ifdef CONFIG_RISCV_M_MODE
>> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
>> index 5ce50468aff1..b7409487c9d2 100644
>> --- a/arch/riscv/include/asm/hwcap.h
>> +++ b/arch/riscv/include/asm/hwcap.h
>> @@ -44,6 +44,12 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
>>   #define riscv_isa_extension_available(isa_bitmap, ext)	\
>>   	__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
>>   
>> +struct cpu_manufacturer_info_t {
>> +	unsigned long vendor_id;
>> +	unsigned long arch_id;
>> +	unsigned long imp_id;
>> +};
>> +
>>   #endif
>>   
>>   #endif /* _ASM_RISCV_HWCAP_H */
>> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
>> index 3a240037bde2..4e11a9621d14 100644
>> --- a/arch/riscv/include/asm/processor.h
>> +++ b/arch/riscv/include/asm/processor.h
>> @@ -72,6 +72,8 @@ int riscv_of_parent_hartid(struct device_node *node);
>>   
>>   extern void riscv_fill_hwcap(void);
>>   
>> +void riscv_fill_cpu_manufacturer_info(void);
>> +
>>   #endif /* __ASSEMBLY__ */
>>   
>>   #endif /* _ASM_RISCV_PROCESSOR_H */
>> diff --git a/arch/riscv/include/asm/soc.h b/arch/riscv/include/asm/soc.h
>> index f494066051a2..03dee6db404c 100644
>> --- a/arch/riscv/include/asm/soc.h
>> +++ b/arch/riscv/include/asm/soc.h
>> @@ -10,6 +10,7 @@
>>   #include <linux/of.h>
>>   #include <linux/linkage.h>
>>   #include <linux/types.h>
>> +#include <asm/hwcap.h>
>>   
>>   #define SOC_EARLY_INIT_DECLARE(name, compat, fn)			\
>>   	static const struct of_device_id __soc_early_init__##name	\
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index ac202f44a670..389162ee19ea 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -12,6 +12,8 @@
>>   #include <asm/hwcap.h>
>>   #include <asm/smp.h>
>>   #include <asm/switch_to.h>
>> +#include <asm/sbi.h>
>> +#include <asm/csr.h>
>>   
>>   unsigned long elf_hwcap __read_mostly;
>>   
>> @@ -22,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
>>   bool has_fpu __read_mostly;
>>   #endif
>>   
>> +struct cpu_manufacturer_info_t cpu_mfr_info;
>> +
>>   /**
>>    * riscv_isa_extension_base() - Get base extension word
>>    *
>> @@ -149,3 +153,16 @@ void riscv_fill_hwcap(void)
>>   		has_fpu = true;
>>   #endif
>>   }
>> +
>> +void riscv_fill_cpu_manufacturer_info(void)
>> +{
>> +#ifndef CONFIG_RISCV_M_MODE
>> +	cpu_mfr_info.vendor_id = sbi_get_vendorid();
>> +	cpu_mfr_info.arch_id = sbi_get_archid();
>> +	cpu_mfr_info.imp_id = sbi_get_impid();
>> +#else
>> +	cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
>> +	cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
>> +	cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
>> +#endif
> 
> Why ? reading the registers will work with M-Mode too. It was there before when
> we temporarily had the builtin DTB lookup based on vendor/arch/imp (see defunct
> soc_lookup_builtin_dtb() in 5.11).

I don't understand what the objection is here. S-mode does not have
these CSRs, so it uses SBI. M-mode does, so it reads the CSRs. This is
the same logic as soc_lookup_builtin_dtb, except it works with S-mode as
well.

--Sean

> 
>> +}
>> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
>> index e85bacff1b50..03621ce9092c 100644
>> --- a/arch/riscv/kernel/setup.c
>> +++ b/arch/riscv/kernel/setup.c
>> @@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
>>   #endif
>>   
>>   	riscv_fill_hwcap();
>> +
> 
> Nit: I do not think the white libe is really necessary here.
> 
>> +	riscv_fill_cpu_manufacturer_info();
>>   }
>>   
>>   static int __init topology_init(void)
>> diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
>> index a0516172a33c..58f6fd91743a 100644
>> --- a/arch/riscv/kernel/soc.c
>> +++ b/arch/riscv/kernel/soc.c
>> @@ -6,6 +6,7 @@
>>   #include <linux/libfdt.h>
>>   #include <linux/pgtable.h>
>>   #include <asm/soc.h>
>> +#include <asm/hwcap.h>
> 
> Why is this necessary ?
> 
>>   
>>   /*
>>    * This is called extremly early, before parse_dtb(), to allow initializing
>>
> 
> 


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