| Message ID | 20231122030621.3759313-2-samuel.holland@sifive.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | riscv: Add kernel-mode FPU support for amdgpu | expand |
On Tue, Nov 21, 2023 at 07:05:13PM -0800, Samuel Holland wrote: > +static inline void kernel_fpu_begin(void) > +{ > + preempt_disable(); > + fstate_save(current, task_pt_regs(current)); > + csr_set(CSR_SSTATUS, SR_FS); > +} > + > +static inline void kernel_fpu_end(void) > +{ > + csr_clear(CSR_SSTATUS, SR_FS); > + fstate_restore(current, task_pt_regs(current)); > + preempt_enable(); > +} Is there any critical reason to inline these two? I'd much rather see them out of line and exported instead of the low-level helpers.
Hi Samuel, kernel test robot noticed the following build errors: [auto build test ERROR on drm-misc/drm-misc-next] [also build test ERROR on linus/master v6.7-rc2 next-20231122] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Samuel-Holland/riscv-Add-support-for-kernel-mode-FPU/20231122-111015 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20231122030621.3759313-2-samuel.holland%40sifive.com patch subject: [PATCH 1/3] riscv: Add support for kernel-mode FPU config: riscv-allnoconfig (https://download.01.org/0day-ci/archive/20231123/202311230215.DBFyWPqb-lkp@intel.com/config) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231123/202311230215.DBFyWPqb-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202311230215.DBFyWPqb-lkp@intel.com/ All errors (new ones prefixed by >>): >> arch/riscv/kernel/process.c:229:19: error: use of undeclared identifier '__fstate_save' 229 | EXPORT_SYMBOL_GPL(__fstate_save); | ^ >> arch/riscv/kernel/process.c:230:19: error: use of undeclared identifier '__fstate_restore' 230 | EXPORT_SYMBOL_GPL(__fstate_restore); | ^ 2 errors generated. vim +/__fstate_save +229 arch/riscv/kernel/process.c 228 > 229 EXPORT_SYMBOL_GPL(__fstate_save); > 230 EXPORT_SYMBOL_GPL(__fstate_restore);
Hi Samuel,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.7-rc2 next-20231122]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Samuel-Holland/riscv-Add-support-for-kernel-mode-FPU/20231122-111015
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20231122030621.3759313-2-samuel.holland%40sifive.com
patch subject: [PATCH 1/3] riscv: Add support for kernel-mode FPU
config: riscv-randconfig-r111-20231123 (https://download.01.org/0day-ci/archive/20231123/202311230628.TkL31MjJ-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231123/202311230628.TkL31MjJ-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311230628.TkL31MjJ-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/linkage.h:7,
from include/linux/printk.h:8,
from include/asm-generic/bug.h:22,
from arch/riscv/include/asm/bug.h:83,
from include/linux/bug.h:5,
from arch/riscv/include/asm/current.h:13,
from include/linux/sched.h:12,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/node.h:18,
from include/linux/cpu.h:17,
from arch/riscv/kernel/process.c:10:
>> arch/riscv/kernel/process.c:229:19: error: '__fstate_save' undeclared here (not in a function); did you mean 'fstate_save'?
229 | EXPORT_SYMBOL_GPL(__fstate_save);
| ^~~~~~~~~~~~~
include/linux/export.h:74:23: note: in definition of macro '__EXPORT_SYMBOL'
74 | extern typeof(sym) sym; \
| ^~~
include/linux/export.h:87:41: note: in expansion of macro '_EXPORT_SYMBOL'
87 | #define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "GPL")
| ^~~~~~~~~~~~~~
arch/riscv/kernel/process.c:229:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL'
229 | EXPORT_SYMBOL_GPL(__fstate_save);
| ^~~~~~~~~~~~~~~~~
>> arch/riscv/kernel/process.c:230:19: error: '__fstate_restore' undeclared here (not in a function); did you mean 'fstate_restore'?
230 | EXPORT_SYMBOL_GPL(__fstate_restore);
| ^~~~~~~~~~~~~~~~
include/linux/export.h:74:23: note: in definition of macro '__EXPORT_SYMBOL'
74 | extern typeof(sym) sym; \
| ^~~
include/linux/export.h:87:41: note: in expansion of macro '_EXPORT_SYMBOL'
87 | #define EXPORT_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "GPL")
| ^~~~~~~~~~~~~~
arch/riscv/kernel/process.c:230:1: note: in expansion of macro 'EXPORT_SYMBOL_GPL'
230 | EXPORT_SYMBOL_GPL(__fstate_restore);
| ^~~~~~~~~~~~~~~~~
vim +229 arch/riscv/kernel/process.c
228
> 229 EXPORT_SYMBOL_GPL(__fstate_save);
> 230 EXPORT_SYMBOL_GPL(__fstate_restore);
Hi Christoph, On 2023-11-22 2:33 AM, Christoph Hellwig wrote: > On Tue, Nov 21, 2023 at 07:05:13PM -0800, Samuel Holland wrote: >> +static inline void kernel_fpu_begin(void) >> +{ >> + preempt_disable(); >> + fstate_save(current, task_pt_regs(current)); >> + csr_set(CSR_SSTATUS, SR_FS); >> +} >> + >> +static inline void kernel_fpu_end(void) >> +{ >> + csr_clear(CSR_SSTATUS, SR_FS); >> + fstate_restore(current, task_pt_regs(current)); >> + preempt_enable(); >> +} > > Is there any critical reason to inline these two? I'd much rather see > them out of line and exported instead of the low-level helpers. No, I will define them out of line in v2. Regards, Samuel
diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h index f90d8e42f3c7..4b15f1292fc4 100644 --- a/arch/riscv/include/asm/switch_to.h +++ b/arch/riscv/include/asm/switch_to.h @@ -63,6 +63,20 @@ static __always_inline bool has_fpu(void) return riscv_has_extension_likely(RISCV_ISA_EXT_f) || riscv_has_extension_likely(RISCV_ISA_EXT_d); } + +static inline void kernel_fpu_begin(void) +{ + preempt_disable(); + fstate_save(current, task_pt_regs(current)); + csr_set(CSR_SSTATUS, SR_FS); +} + +static inline void kernel_fpu_end(void) +{ + csr_clear(CSR_SSTATUS, SR_FS); + fstate_restore(current, task_pt_regs(current)); + preempt_enable(); +} #else static __always_inline bool has_fpu(void) { return false; } #define fstate_save(task, regs) do { } while (0) diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c index 4f21d970a129..6a18bc709d1c 100644 --- a/arch/riscv/kernel/process.c +++ b/arch/riscv/kernel/process.c @@ -225,3 +225,6 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args) p->thread.sp = (unsigned long)childregs; /* kernel sp */ return 0; } + +EXPORT_SYMBOL_GPL(__fstate_save); +EXPORT_SYMBOL_GPL(__fstate_restore);
This is needed to support recent hardware in the amdgpu DRM driver. The FPU code in that driver is not performance-critical, so only provide the minimal support. Signed-off-by: Samuel Holland <samuel.holland@sifive.com> --- arch/riscv/include/asm/switch_to.h | 14 ++++++++++++++ arch/riscv/kernel/process.c | 3 +++ 2 files changed, 17 insertions(+)