Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 20 Jun 2018 23:39:38 -0700
From:   Christoph Hellwig <hch@infradead.org>
To:     Alan Kao <alankao@andestech.com>
Cc:     Palmer Dabbelt <palmer@sifive.com>, Albert Ou <albert@sifive.com>,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
        Zong Li <zong@andestech.com>,
        Greentime Hu <greentime@andestech.com>
Subject: Re: [PATCH] riscv: Add support to no-FPU systems
Message-ID: <20180621063938.GB19319@infradead.org>
References: <1529544276-28103-1-git-send-email-alankao@andestech.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1529544276-28103-1-git-send-email-alankao@andestech.com>
User-Agent: Mutt/1.9.2 (2017-12-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> +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.