[RFC patch 2/4] riscv: Get CPU manufacturer information
Guo Ren
guoren at kernel.org
Wed Mar 10 03:56:39 GMT 2021
Got it. Thx for more clarification.
On Wed, Mar 10, 2021 at 11:40 AM Palmer Dabbelt <palmer at dabbelt.com> wrote:
>
> On Mon, 08 Mar 2021 21:11:04 PST (-0800), anup at brainfault.org wrote:
> > On Tue, Mar 9, 2021 at 6:58 AM Guo Ren <guoren at kernel.org> wrote:
> >>
> >> Hi Vincent,
> >>
> >> On Mon, Mar 8, 2021 at 11:58 AM Vincent Chen <vincent.chen at sifive.com> 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 at 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
> >> How about let opensbi emulate csr_read(CSR_MXXX) for S mode, then we
> >> needn't to define new sbi call.
> >
> > Accessing M-mode CSRs from the S-mode kernel will only make things
> > complicated for hypervisors because now hypervisors will also end-up
> > emulating M-mode CSRs.
>
> IMO that's not really the problem: hypervisors are going to have to emulate
> CSRs anyway, so adding more isn't a big deal. The problem is having S-mode
> software depend on M-mode. More explicitly:
>
> * The RISC-V specs are nominally layered, which means S-mode doesn't include
> any of the M-mode bits. M-mode CSR accesses are simply illegal S-mode
> instructions. I know the CSRs are a bit special in that they're chunked out
> by privilege mode, but there's nothing preventing the ISA from later
> reallocating these M-mode CSR bits in S-mode (aside from then having a modal
> ISA, which is a design anti-goal).
> * All behavior in M-mode is implementation defined, including these CSR
> accesses. While they're obviously allocated and a behavior is specified by
> the ISA, there's nothing that says M-mode has to actually implement these in
> any sane fashion (ie, it can return 0 or shut down or whatever).
>
> So is essence, adding these M-mode CSR accesses into S-mode kernels introduces
> an entirely new ISA extension that we're just making up on the spot and
> assuming will be implemented by firmware. At a bare minimum we'd need to have
> that defined by a specification, but even then I don't see it as the right way
> to go.
>
> > Best would be to only access S-mode CSRs and SBI calls from
> > S-mode kernel.
>
> Agreed. I'm not opposed to expanding the scope of the M-mode kernels to boot
> on more systems, but users who want S-mode shouldn't end up with M-mode
> dependencies sneaking in. So in this case, I'm very much in favor of the SBI
> call over accessing an M-mode CSR.
>
> >
> >>
> >> > +}
> >> > 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();
> >> > +
> >> > + 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>
> >> >
> >> > /*
> >> > * This is called extremly early, before parse_dtb(), to allow initializing
> >> > --
> >> > 2.7.4
> >> >
> >> How
> >>
> >> --
> >> Best Regards
> >> Guo Ren
> >>
> >> ML: https://lore.kernel.org/linux-csky/
> >>
> >> _______________________________________________
> >> linux-riscv mailing list
> >> linux-riscv at lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
More information about the linux-riscv
mailing list