mbox series

[0/5] LoongArch, MIPS: Unify Loongson IOCSR handling

Message ID 20240907-iocsr-v1-0-0c99b3334444@flygoat.com
Headers show
Series LoongArch, MIPS: Unify Loongson IOCSR handling | expand

Message

Jiaxun Yang Sept. 7, 2024, 10:17 a.m. UTC 
Hi folks,

This series unfied LoongArch and MIPS's IOCSR functions and
macros so they will expose same interface to arch-indenpendent
drivers.

This can reduce code deuplication, and also help my unifed IPI driver
and MIPS extio driver effort.

This is touching many sub-systems in once so might be hard to merge.

Huacai, can you apply first three patch via loongarch-next tree.
For last two patch maybe better merge them via a second PR after
all subsystem PRs merged.

Please review.
Thanks

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
Jiaxun Yang (5):
      LoongArch: Rename cpu_has_csr as cpu_has_iocsr
      LoongArch: Probe more CPU features from CPUCFG
      LoongArch: cpu-probe: Move IOCSR probing out of cpu_probe_common
      LoongArch: Extract IOCSR definitions to standalone header
      MIPS: Loongson64: Use shared IOCSR header

 MAINTAINERS                                        |   1 +
 arch/loongarch/include/asm/cpu-features.h          |   2 +-
 arch/loongarch/include/asm/cpu.h                   |   4 +-
 arch/loongarch/include/asm/loongarch.h             |  90 ----------------
 arch/loongarch/kernel/cpu-probe.c                  | 111 ++++++++++++--------
 arch/loongarch/kernel/relocate_kernel.S            |   5 +-
 arch/loongarch/kernel/smp.c                        |  23 +++--
 .../include/asm/mach-loongson64/loongson_regs.h    |  58 +++--------
 arch/mips/kvm/vz.c                                 |   2 +-
 arch/mips/loongson64/smp.c                         |  44 ++++----
 drivers/cpufreq/loongson3_cpufreq.c                |  10 +-
 drivers/irqchip/irq-loongarch-avec.c               |   5 +-
 drivers/irqchip/irq-loongson-eiointc.c             |   5 +-
 drivers/platform/mips/cpu_hwmon.c                  |   7 +-
 include/linux/loongson/iocsr.h                     | 113 +++++++++++++++++++++
 15 files changed, 256 insertions(+), 224 deletions(-)
---
base-commit: 9aaeb87ce1e966169a57f53a02ba05b30880ffb8
change-id: 20240906-iocsr-829075458511

Best regards,

Comments

Jiaxun Yang Sept. 7, 2024, 10:30 a.m. UTC | #1 
在2024年9月7日九月 上午11:25，Huacai Chen写道：
> Hi, Jiaxun,
>
> On Sat, Sep 7, 2024 at 6:17 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>>
>> Hi folks,
>>
>> This series unfied LoongArch and MIPS's IOCSR functions and
>> macros so they will expose same interface to arch-indenpendent
>> drivers.
>>
>> This can reduce code deuplication, and also help my unifed IPI driver
>> and MIPS extio driver effort.
>>
>> This is touching many sub-systems in once so might be hard to merge.
>>
>> Huacai, can you apply first three patch via loongarch-next tree.
>> For last two patch maybe better merge them via a second PR after
>> all subsystem PRs merged.
> The problem is I'm not sure whether IOCSR registers are compatible in
> all Loongson processors.

Maybe we can introduce something like IOCSR flavour later, however my take
would be force all IOCSR devices to be probed by OF/ACPI.

In this case, it's still better to share IOCSR macro and functions between
MIPS/LoongArch Loongson-3 processors, as they are guaranteed to be compatible.

Future incompatible definitions will benefit from better resilience provided
by this series as well.

Thanks
- Jiaxun

>
> Huacai
>
>>
>> Please review.
>> Thanks
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>> Jiaxun Yang (5):
>>       LoongArch: Rename cpu_has_csr as cpu_has_iocsr
>>       LoongArch: Probe more CPU features from CPUCFG
>>       LoongArch: cpu-probe: Move IOCSR probing out of cpu_probe_common
>>       LoongArch: Extract IOCSR definitions to standalone header
>>       MIPS: Loongson64: Use shared IOCSR header
>>
>>  MAINTAINERS                                        |   1 +
>>  arch/loongarch/include/asm/cpu-features.h          |   2 +-
>>  arch/loongarch/include/asm/cpu.h                   |   4 +-
>>  arch/loongarch/include/asm/loongarch.h             |  90 ----------------
>>  arch/loongarch/kernel/cpu-probe.c                  | 111 ++++++++++++--------
>>  arch/loongarch/kernel/relocate_kernel.S            |   5 +-
>>  arch/loongarch/kernel/smp.c                        |  23 +++--
>>  .../include/asm/mach-loongson64/loongson_regs.h    |  58 +++--------
>>  arch/mips/kvm/vz.c                                 |   2 +-
>>  arch/mips/loongson64/smp.c                         |  44 ++++----
>>  drivers/cpufreq/loongson3_cpufreq.c                |  10 +-
>>  drivers/irqchip/irq-loongarch-avec.c               |   5 +-
>>  drivers/irqchip/irq-loongson-eiointc.c             |   5 +-
>>  drivers/platform/mips/cpu_hwmon.c                  |   7 +-
>>  include/linux/loongson/iocsr.h                     | 113 +++++++++++++++++++++
>>  15 files changed, 256 insertions(+), 224 deletions(-)
>> ---
>> base-commit: 9aaeb87ce1e966169a57f53a02ba05b30880ffb8
>> change-id: 20240906-iocsr-829075458511
>>
>> Best regards,
>> --
>> Jiaxun Yang <jiaxun.yang@flygoat.com>
>>
Huacai Chen Sept. 8, 2024, 2:38 a.m. UTC | #2 
Hi, Jiaxun,

On Sat, Sep 7, 2024 at 6:17 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
> It meant to be IOCSR as CSR is not optional for LoongArch.
Keep the CSR definition and add IOCSR definition after it, OK? This
also means the 1st patch can be dropped.

And it is just OK to change the values after CPU_FEATURE_CSR, because
they are only for internal use.

Huacai
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  arch/loongarch/include/asm/cpu-features.h | 2 +-
>  arch/loongarch/include/asm/cpu.h          | 2 +-
>  arch/loongarch/kernel/cpu-probe.c         | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/cpu-features.h b/arch/loongarch/include/asm/cpu-features.h
> index 16a716f88a5c..0190ed28a647 100644
> --- a/arch/loongarch/include/asm/cpu-features.h
> +++ b/arch/loongarch/include/asm/cpu-features.h
> @@ -50,7 +50,7 @@
>  #define cpu_has_lbt_arm                cpu_opt(LOONGARCH_CPU_LBT_ARM)
>  #define cpu_has_lbt_mips       cpu_opt(LOONGARCH_CPU_LBT_MIPS)
>  #define cpu_has_lbt            (cpu_has_lbt_x86|cpu_has_lbt_arm|cpu_has_lbt_mips)
> -#define cpu_has_csr            cpu_opt(LOONGARCH_CPU_CSR)
> +#define cpu_has_iocsr          cpu_opt(LOONGARCH_CPU_IOCSR)
>  #define cpu_has_tlb            cpu_opt(LOONGARCH_CPU_TLB)
>  #define cpu_has_watch          cpu_opt(LOONGARCH_CPU_WATCH)
>  #define cpu_has_vint           cpu_opt(LOONGARCH_CPU_VINT)
> diff --git a/arch/loongarch/include/asm/cpu.h b/arch/loongarch/include/asm/cpu.h
> index 843f9c4ec980..7c44f4ede3a2 100644
> --- a/arch/loongarch/include/asm/cpu.h
> +++ b/arch/loongarch/include/asm/cpu.h
> @@ -115,7 +115,7 @@ enum cpu_type_enum {
>  #define LOONGARCH_CPU_LBT_ARM          BIT_ULL(CPU_FEATURE_LBT_ARM)
>  #define LOONGARCH_CPU_LBT_MIPS         BIT_ULL(CPU_FEATURE_LBT_MIPS)
>  #define LOONGARCH_CPU_TLB              BIT_ULL(CPU_FEATURE_TLB)
> -#define LOONGARCH_CPU_CSR              BIT_ULL(CPU_FEATURE_CSR)
> +#define LOONGARCH_CPU_IOCSR            BIT_ULL(CPU_FEATURE_IOCSR)
>  #define LOONGARCH_CPU_WATCH            BIT_ULL(CPU_FEATURE_WATCH)
>  #define LOONGARCH_CPU_VINT             BIT_ULL(CPU_FEATURE_VINT)
>  #define LOONGARCH_CPU_CSRIPI           BIT_ULL(CPU_FEATURE_CSRIPI)
> diff --git a/arch/loongarch/kernel/cpu-probe.c b/arch/loongarch/kernel/cpu-probe.c
> index 14f0449f5452..4446616d497c 100644
> --- a/arch/loongarch/kernel/cpu-probe.c
> +++ b/arch/loongarch/kernel/cpu-probe.c
> @@ -91,7 +91,7 @@ static void cpu_probe_common(struct cpuinfo_loongarch *c)
>         unsigned int config;
>         unsigned long asid_mask;
>
> -       c->options = LOONGARCH_CPU_CPUCFG | LOONGARCH_CPU_CSR |
> +       c->options = LOONGARCH_CPU_CPUCFG | LOONGARCH_CPU_IOCSR |
>                      LOONGARCH_CPU_TLB | LOONGARCH_CPU_VINT | LOONGARCH_CPU_WATCH;
>
>         elf_hwcap = HWCAP_LOONGARCH_CPUCFG;
>
> --
> 2.46.0
>
Huacai Chen Sept. 8, 2024, 2:47 a.m. UTC | #3 
Hi, Jiaxun,

On Sat, Sep 7, 2024 at 6:17 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
> IOCSR register definition appears to be a platform specific
> spec instead of architecture spec, even for Loongson CPUs
> there is no guarantee that IOCSR will always present.
>
> Thus it's dangerous to perform IOCSR probing without checking
> CPU type and instruction availability.
I don't think this is necessary. Loongson's Chip engineers confirm
that IOCSR is always present in Loongson processors. If other
LoongArch (not Loongson) processors have no IOCSR, they should
implement their own cpu_probe_abc() instead of cpu_probe_loongson().

Huacai

>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  arch/loongarch/kernel/cpu-probe.c | 63 ++++++++++++++++++++++++---------------
>  1 file changed, 39 insertions(+), 24 deletions(-)
>
> diff --git a/arch/loongarch/kernel/cpu-probe.c b/arch/loongarch/kernel/cpu-probe.c
> index 6a82ceb6e321..968b5a35a0d2 100644
> --- a/arch/loongarch/kernel/cpu-probe.c
> +++ b/arch/loongarch/kernel/cpu-probe.c
> @@ -173,22 +173,6 @@ static void cpu_probe_common(struct cpuinfo_loongarch *c)
>         if (config & CPUCFG6_PMP)
>                 c->options |= LOONGARCH_CPU_PMP;
>
> -       config = iocsr_read32(LOONGARCH_IOCSR_FEATURES);
> -       if (config & IOCSRF_CSRIPI)
> -               c->options |= LOONGARCH_CPU_CSRIPI;
> -       if (config & IOCSRF_EXTIOI)
> -               c->options |= LOONGARCH_CPU_EXTIOI;
> -       if (config & IOCSRF_FREQSCALE)
> -               c->options |= LOONGARCH_CPU_SCALEFREQ;
> -       if (config & IOCSRF_FLATMODE)
> -               c->options |= LOONGARCH_CPU_FLATMODE;
> -       if (config & IOCSRF_EIODECODE)
> -               c->options |= LOONGARCH_CPU_EIODECODE;
> -       if (config & IOCSRF_AVEC)
> -               c->options |= LOONGARCH_CPU_AVECINT;
> -       if (config & IOCSRF_VM)
> -               c->options |= LOONGARCH_CPU_HYPERVISOR;
> -
>         config = csr_read32(LOONGARCH_CSR_ASID);
>         config = (config & CSR_ASID_BIT) >> CSR_ASID_BIT_SHIFT;
>         asid_mask = GENMASK(config - 1, 0);
> @@ -231,16 +215,8 @@ static char cpu_full_name[MAX_NAME_LEN] = "        -        ";
>
>  static inline void cpu_probe_loongson(struct cpuinfo_loongarch *c, unsigned int cpu)
>  {
> -       uint64_t *vendor = (void *)(&cpu_full_name[VENDOR_OFFSET]);
> -       uint64_t *cpuname = (void *)(&cpu_full_name[CPUNAME_OFFSET]);
>         const char *core_name = "Unknown";
>
> -       if (!__cpu_full_name[cpu])
> -               __cpu_full_name[cpu] = cpu_full_name;
> -
> -       *vendor = iocsr_read64(LOONGARCH_IOCSR_VENDOR);
> -       *cpuname = iocsr_read64(LOONGARCH_IOCSR_CPUNAME);
> -
>         switch (c->processor_id & PRID_SERIES_MASK) {
>         case PRID_SERIES_LA132:
>                 c->cputype = CPU_LOONGSON32;
> @@ -283,6 +259,36 @@ static inline void cpu_probe_loongson(struct cpuinfo_loongarch *c, unsigned int
>         pr_info("%s Processor probed (%s Core)\n", __cpu_family[cpu], core_name);
>  }
>
> +static inline void iocsr_probe_loongson(struct cpuinfo_loongarch *c, unsigned int cpu)
> +{
> +       uint64_t *vendor = (void *)(&cpu_full_name[VENDOR_OFFSET]);
> +       uint64_t *cpuname = (void *)(&cpu_full_name[CPUNAME_OFFSET]);
> +       unsigned int config;
> +
> +       *vendor = iocsr_read64(LOONGARCH_IOCSR_VENDOR);
> +       *cpuname = iocsr_read64(LOONGARCH_IOCSR_CPUNAME);
> +
> +       if (!__cpu_full_name[cpu])
> +               __cpu_full_name[cpu] = cpu_full_name;
> +
> +       config = iocsr_read32(LOONGARCH_IOCSR_FEATURES);
> +       if (config & IOCSRF_CSRIPI)
> +               c->options |= LOONGARCH_CPU_CSRIPI;
> +       if (config & IOCSRF_EXTIOI)
> +               c->options |= LOONGARCH_CPU_EXTIOI;
> +       if (config & IOCSRF_FREQSCALE)
> +               c->options |= LOONGARCH_CPU_SCALEFREQ;
> +       if (config & IOCSRF_FLATMODE)
> +               c->options |= LOONGARCH_CPU_FLATMODE;
> +       if (config & IOCSRF_EIODECODE)
> +               c->options |= LOONGARCH_CPU_EIODECODE;
> +       if (config & IOCSRF_AVEC)
> +               c->options |= LOONGARCH_CPU_AVECINT;
> +       if (config & IOCSRF_VM)
> +               c->options |= LOONGARCH_CPU_HYPERVISOR;
> +
> +}
> +
>  #ifdef CONFIG_64BIT
>  /* For use by uaccess.h */
>  u64 __ua_limit;
> @@ -331,6 +337,15 @@ void cpu_probe(void)
>                 break;
>         }
>
> +       if (c->options & LOONGARCH_CPU_IOCSR) {
> +               switch (c->cputype) {
> +               case CPU_LOONGSON32:
> +               case CPU_LOONGSON64:
> +                       iocsr_probe_loongson(c, cpu);
> +                       break;
> +               }
> +       }
> +
>         BUG_ON(!__cpu_family[cpu]);
>         BUG_ON(c->cputype == CPU_UNKNOWN);
>
>
> --
> 2.46.0
>
Jiaxun Yang Sept. 8, 2024, 10 a.m. UTC | #4 
在2024年9月8日九月 上午3:47，Huacai Chen写道：
> Hi, Jiaxun,
>
> On Sat, Sep 7, 2024 at 6:17 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>>
>> IOCSR register definition appears to be a platform specific
>> spec instead of architecture spec, even for Loongson CPUs
>> there is no guarantee that IOCSR will always present.
>>
>> Thus it's dangerous to perform IOCSR probing without checking
>> CPU type and instruction availability.
> I don't think this is necessary. Loongson's Chip engineers confirm
> that IOCSR is always present in Loongson processors. If other
> LoongArch (not Loongson) processors have no IOCSR, they should
> implement their own cpu_probe_abc() instead of cpu_probe_loongson().

Hi Huacai,

IOCSR_FEATURE probing process is now in cpu_probe_common, which is shared
among all PRIDs, that's why it needs to be moved out.

It also prepares for different IOCSR definitions, as you said before IOCSR
definitions are not guaranteed to be compatible, so if an incompatible
implementation arise, you can just introduce a new CPU_TYPE for it and
create a new iocsr_probe function.

Thanks
- Jiaxun

>
> Huacai
>
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>