Re: [PATCH] riscv: Add support to no-FPU systems

From: Christoph Hellwig
Date: Thu Jun 21 2018 - 02:39:48 EST


> +ifeq ($(CONFIG_FPU),y)
> KBUILD_AFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)fd$(KBUILD_ARCH_C)
> +else
> +KBUILD_AFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)$(KBUILD_ARCH_C)
> +endif

Can we refactor that KBUILD_ARCH code into something like

riscv-march-y :=
riscv-march-$(CONFIG_ARCH_RV32I) += rv32im
riscv-march-$(CONFIG_ARCH_RV64I) += rv64im
riscv-march-$(CONFIG_RISCV_ISA_A) += a
riscv-march-$(CONFIG_FPU) += fd
riscv-march-$(CONFIG_RISCV_ISA_C) += c

KBUILD_CFLAGS += -march=$(riscv-march-y)

> +#ifdef CONFIG_FPU
> regs->sstatus = SR_SPIE /* User mode, irqs on */ | SR_FS_INITIAL;
> +#else
> + regs->sstatus = SR_SPIE | SR_FS_OFF;
> +#endif

Having the comment in one branch, but not the other is odd. I'd be
tempted to just remove t entirely, but if not it should be move up
or duplicated.

> int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
> {
> +#ifdef CONFIG_FPU
> fstate_save(src, task_pt_regs(src));
> +#endif

Please provide a !CONFIG_FPU stub for fstate_save, please.

> }
> +#endif
>
> static long restore_sigcontext(struct pt_regs *regs,
> struct sigcontext __user *sc)
> @@ -63,6 +65,7 @@ static long restore_sigcontext(struct pt_regs *regs,
> err = __copy_from_user(regs, &sc->sc_regs, sizeof(sc->sc_regs));
> if (unlikely(err))
> return err;
> +#ifdef CONFIG_FPU
> /* Restore the floating-point state. */
> err = restore_d_state(regs, &sc->sc_fpregs.d);
> if (unlikely(err))
> @@ -76,6 +79,7 @@ static long restore_sigcontext(struct pt_regs *regs,
> if (value != 0)
> return -EINVAL;
> }
> +#endif

Same here.

> @@ -127,11 +131,13 @@ static long setup_sigcontext(struct rt_sigframe __user *frame,
> size_t i;
> /* sc_regs is structured the same as the start of pt_regs */
> err = __copy_to_user(&sc->sc_regs, regs, sizeof(sc->sc_regs));
> +#ifdef CONFIG_FPU
> /* Save the floating-point state. */
> err |= save_d_state(regs, &sc->sc_fpregs.d);
> /* We support no other extension state at this time. */
> for (i = 0; i < ARRAY_SIZE(sc->sc_fpregs.q.reserved); i++)
> err |= __put_user(0, &sc->sc_fpregs.q.reserved[i]);
> +#endif

Same here.