On Thu, Apr 09, 2020 at 08:15:57PM +0200, Christian König wrote: > Am 09.04.20 um 19:09 schrieb Peter Zijlstra: > > On Thu, Apr 09, 2020 at 05:59:56PM +0200, Peter Zijlstra wrote: > > [SNIP] > > > I'll need another approach, let me consider. > > Christian; it says these files are generated, does that generator know > > which functions are wholly in FPU context and which are not? > > Well that "generator" is still a human being :) > > It's just that the formulae for the calculation come from the hardware team > and we are not able to easily transcript them to fixed point calculations.
Well, if it's a human, can this human respect the kernel coding style a bit more :-) Some of that stuff is atrocious. > > My current thinking is that if I annotate all functions that are wholly > > inside kernel_fpu_start() with an __fpu function attribute, then I can > > verify that any call from regular text to fpu text only happens inside > > kernel_fpu_begin()/end(). And I can ensure that all !__fpu annotation > > fuctions only contain !fpu instructions. > > Yeah, that sounds like a good idea to me and should be easily doable. > > > Can that generator add the __fpu function attribute or is that something > > that would need to be done manually (which seems like it would be > > painful, since it is quite a bit of code) ? > > We are currently in the process of moving all the stuff which requires > floating point into a single C file(s) and then make sure that we only call > those within kernel_fpu_begin()/end() blocks. Can you make the build system stick all those .o files in a single archive? That's the only way I can do call validation; external relocatoin records do not contain the section. > Annotating those function with __fpu or even saying to gcc that all code of > those files should go into a special text.fpu segment shouldn't be much of a > problem. Guess what the __fpu attribute does ;-) With the below patch (which is on to of newer versions of the objtool patches send earlier, let me know if you want a full set) that only converts a few files, but fully converts: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c But building it (and this is an absolute pain; when you're reworking this, can you pretty please also fix the Makefiles?), we get: drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o: warning: objtool: dcn_validate_bandwidth()+0x34fa: FPU instruction outside of kernel_fpu_{begin,end}() $ ./scripts/faddr2line defconfig-build/drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o dcn_validate_bandwidth+0x34fa dcn_validate_bandwidth+0x34fa/0x57ce: dcn_validate_bandwidth at /usr/src/linux-2.6/defconfig-build/../drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:1293 (discriminator 5) # ./objdump-func.sh defconfig-build/drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o dcn_validate_bandwidth | grep 34fa 34fa 50fa: f2 0f 10 b5 60 ff ff movsd -0xa0(%rbp),%xmm6 Which seems to indicate there's still problms with the current code. --- arch/x86/include/asm/fpu/api.h | 12 +++++++++++ arch/x86/kernel/vmlinux.lds.S | 1 + .../gpu/drm/amd/display/dc/calcs/dcn_calc_math.c | 25 +++++++++++----------- drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c | 4 ++-- .../display/dc/dml/dcn20/display_rq_dlg_calc_20.c | 10 ++++----- .../amd/display/dc/dml/display_rq_dlg_helpers.c | 2 +- .../gpu/drm/amd/display/dc/dml/dml_common_defs.c | 2 +- drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c | 2 +- drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c | 10 ++++----- drivers/gpu/drm/amd/display/dc/dsc/rc_calc_dpi.c | 4 ++-- drivers/gpu/drm/amd/display/dc/inc/dcn_calc_math.h | 2 ++ tools/objtool/check.c | 7 +++++- tools/objtool/elf.h | 2 +- 13 files changed, 52 insertions(+), 31 deletions(-) diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h index 64be4426fda9..19eaf98bbb0a 100644 --- a/arch/x86/include/asm/fpu/api.h +++ b/arch/x86/include/asm/fpu/api.h @@ -12,11 +12,23 @@ #define _ASM_X86_FPU_API_H #include <linux/bottom_half.h> +#ifdef CONFIG_STACK_VALIDATION + +#define __fpu __section(".text.fpu") + #define _ASM_ANNOTATE_FPU(at) \ ".pushsection .discard.fpu_safe\n" \ ".long " #at " - .\n" \ ".popsection\n" \ +#else + +#define __fpu + +#define _ASM_ANNOTATE_FPU(at) + +#endif /* CONFIG_STACK_VALIDATION */ + #define annotate_fpu() ({ \ asm volatile("%c0:\n\t" \ _ASM_ANNOTATE_FPU(%c0b) \ diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S index 1bf7e312361f..8442f8633d07 100644 --- a/arch/x86/kernel/vmlinux.lds.S +++ b/arch/x86/kernel/vmlinux.lds.S @@ -139,6 +139,7 @@ SECTIONS SOFTIRQENTRY_TEXT *(.fixup) *(.gnu.warning) + *(.text.fpu) #ifdef CONFIG_RETPOLINE __indirect_thunk_start = .; diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c index 07d18e78de49..57ab3aafef5a 100644 --- a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c +++ b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c @@ -36,7 +36,7 @@ * remain as-is as it provides us with a guarantee from HW that it is correct. */ -float dcn_bw_mod(const float arg1, const float arg2) +__fpu float dcn_bw_mod(const float arg1, const float arg2) { if (isNaN(arg1)) return arg2; @@ -45,7 +45,7 @@ float dcn_bw_mod(const float arg1, const float arg2) return arg1 - arg1 * ((int) (arg1 / arg2)); } -float dcn_bw_min2(const float arg1, const float arg2) +__fpu float dcn_bw_min2(const float arg1, const float arg2) { if (isNaN(arg1)) return arg2; @@ -58,7 +58,7 @@ unsigned int dcn_bw_max(const unsigned int arg1, const unsigned int arg2) { return arg1 > arg2 ? arg1 : arg2; } -float dcn_bw_max2(const float arg1, const float arg2) +__fpu float dcn_bw_max2(const float arg1, const float arg2) { if (isNaN(arg1)) return arg2; @@ -67,25 +67,26 @@ float dcn_bw_max2(const float arg1, const float arg2) return arg1 > arg2 ? arg1 : arg2; } -float dcn_bw_floor2(const float arg, const float significance) +__fpu float dcn_bw_floor2(const float arg, const float significance) { if (significance == 0) return 0; return ((int) (arg / significance)) * significance; } -float dcn_bw_floor(const float arg) + +__fpu float dcn_bw_floor(const float arg) { return ((int) (arg)); } -float dcn_bw_ceil(const float arg) +__fpu float dcn_bw_ceil(const float arg) { float flr = dcn_bw_floor2(arg, 1); return flr + 0.00001 >= arg ? arg : flr + 1; } -float dcn_bw_ceil2(const float arg, const float significance) +__fpu float dcn_bw_ceil2(const float arg, const float significance) { float flr = dcn_bw_floor2(arg, significance); if (significance == 0) @@ -93,17 +94,17 @@ float dcn_bw_ceil2(const float arg, const float significance) return flr + 0.00001 >= arg ? arg : flr + significance; } -float dcn_bw_max3(float v1, float v2, float v3) +__fpu float dcn_bw_max3(float v1, float v2, float v3) { return v3 > dcn_bw_max2(v1, v2) ? v3 : dcn_bw_max2(v1, v2); } -float dcn_bw_max5(float v1, float v2, float v3, float v4, float v5) +__fpu float dcn_bw_max5(float v1, float v2, float v3, float v4, float v5) { return dcn_bw_max3(v1, v2, v3) > dcn_bw_max2(v4, v5) ? dcn_bw_max3(v1, v2, v3) : dcn_bw_max2(v4, v5); } -float dcn_bw_pow(float a, float exp) +__fpu float dcn_bw_pow(float a, float exp) { float temp; /*ASSERT(exp == (int)exp);*/ @@ -120,7 +121,7 @@ float dcn_bw_pow(float a, float exp) } } -double dcn_bw_fabs(double a) +__fpu double dcn_bw_fabs(double a) { if (a > 0) return (a); @@ -129,7 +130,7 @@ double dcn_bw_fabs(double a) } -float dcn_bw_log(float a, float b) +__fpu float dcn_bw_log(float a, float b) { int * const exp_ptr = (int *)(&a); int x = *exp_ptr; diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c index 3960a8db94cb..b3e305d9d1c9 100644 --- a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c +++ b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c @@ -435,7 +435,7 @@ static void pipe_ctx_to_e2e_pipe_params ( } -static void dcn_bw_calc_rq_dlg_ttu( +static __fpu void dcn_bw_calc_rq_dlg_ttu( const struct dc *dc, const struct dcn_bw_internal_vars *v, struct pipe_ctx *pipe, @@ -1388,7 +1388,7 @@ static unsigned int dcn_find_normalized_clock_vdd_Level( return vdd_level; } -unsigned int dcn_find_dcfclk_suits_all( +__fpu unsigned int dcn_find_dcfclk_suits_all( const struct dc *dc, struct dc_clocks *clocks) { diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.c b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.c index ca807846032f..0cbb58fc7fb8 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.c @@ -115,7 +115,7 @@ static bool is_dual_plane(enum source_format_class source_format) return ret_val; } -static double get_refcyc_per_delivery(struct display_mode_lib *mode_lib, +static __fpu double get_refcyc_per_delivery(struct display_mode_lib *mode_lib, double refclk_freq_in_mhz, double pclk_freq_in_mhz, bool odm_combine, @@ -162,7 +162,7 @@ static unsigned int get_blk_size_bytes(const enum source_macro_tile_size tile_si return (4 * 1024); } -static void extract_rq_sizing_regs(struct display_mode_lib *mode_lib, +static __fpu void extract_rq_sizing_regs(struct display_mode_lib *mode_lib, display_data_rq_regs_st *rq_regs, const display_data_rq_sizing_params_st rq_sizing) { @@ -313,7 +313,7 @@ static void handle_det_buf_split(struct display_mode_lib *mode_lib, full_swath_bytes_packed_c); } -static void get_meta_and_pte_attr(struct display_mode_lib *mode_lib, +static __fpu void get_meta_and_pte_attr(struct display_mode_lib *mode_lib, display_data_rq_dlg_params_st *rq_dlg_param, display_data_rq_misc_params_st *rq_misc_param, display_data_rq_sizing_params_st *rq_sizing_param, @@ -763,7 +763,7 @@ void dml20_rq_dlg_get_rq_reg(struct display_mode_lib *mode_lib, // Note: currently taken in as is. // Nice to decouple code from hw register implement and extract code that are repeated for luma and chroma. -static void dml20_rq_dlg_get_dlg_params(struct display_mode_lib *mode_lib, +static __fpu void dml20_rq_dlg_get_dlg_params(struct display_mode_lib *mode_lib, const display_e2e_pipe_params_st *e2e_pipe_param, const unsigned int num_pipes, const unsigned int pipe_idx, @@ -1611,7 +1611,7 @@ void dml20_rq_dlg_get_dlg_reg(struct display_mode_lib *mode_lib, dml_print("DML_DLG: Calculation for pipe[%d] end\n", pipe_idx); } -static void calculate_ttu_cursor(struct display_mode_lib *mode_lib, +static __fpu void calculate_ttu_cursor(struct display_mode_lib *mode_lib, double *refcyc_per_req_delivery_pre_cur, double *refcyc_per_req_delivery_cur, double refclk_freq_in_mhz, diff --git a/drivers/gpu/drm/amd/display/dc/dml/display_rq_dlg_helpers.c b/drivers/gpu/drm/amd/display/dc/dml/display_rq_dlg_helpers.c index e2d82aacd3bc..36541cba3894 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/display_rq_dlg_helpers.c +++ b/drivers/gpu/drm/amd/display/dc/dml/display_rq_dlg_helpers.c @@ -133,7 +133,7 @@ void print__rq_dlg_params_st(struct display_mode_lib *mode_lib, display_rq_dlg_p dml_print("DML_RQ_DLG_CALC: =====================================\n"); } -void print__dlg_sys_params_st(struct display_mode_lib *mode_lib, display_dlg_sys_params_st dlg_sys_param) +__fpu void print__dlg_sys_params_st(struct display_mode_lib *mode_lib, display_dlg_sys_params_st dlg_sys_param) { dml_print("DML_RQ_DLG_CALC: =====================================\n"); dml_print("DML_RQ_DLG_CALC: DISPLAY_RQ_DLG_PARAM_ST\n"); diff --git a/drivers/gpu/drm/amd/display/dc/dml/dml_common_defs.c b/drivers/gpu/drm/amd/display/dc/dml/dml_common_defs.c index 723af0b2dda0..e86b1d0128cf 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dml_common_defs.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dml_common_defs.c @@ -28,7 +28,7 @@ #include "dml_inline_defs.h" -double dml_round(double a) +__fpu double dml_round(double a) { double round_pt = 0.5; double ceil = dml_ceil(a, 1); diff --git a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c index 87d682d25278..a0b0eb2f0fe3 100644 --- a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c +++ b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c @@ -323,7 +323,7 @@ static inline uint32_t calc_dsc_bpp_x16(uint32_t stream_bandwidth_kbps, uint32_t /* Get DSC bandwidth range based on [min_bpp, max_bpp] target bitrate range, and timing's pixel clock * and uncompressed bandwidth. */ -static void get_dsc_bandwidth_range( +static __fpu void get_dsc_bandwidth_range( const uint32_t min_bpp, const uint32_t max_bpp, const struct dsc_enc_caps *dsc_caps, diff --git a/drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c b/drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c index 03ae15946c6d..535770930343 100644 --- a/drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c +++ b/drivers/gpu/drm/amd/display/dc/dsc/rc_calc.c @@ -40,7 +40,7 @@ break -void get_qp_set(qp_set qps, enum colour_mode cm, enum bits_per_comp bpc, enum max_min max_min, float bpp) +__fpu void get_qp_set(qp_set qps, enum colour_mode cm, enum bits_per_comp bpc, enum max_min max_min, float bpp) { int mode = MODE_SELECT(444, 422, 420); int sel = table_hash(mode, bpc, max_min); @@ -85,7 +85,7 @@ void get_qp_set(qp_set qps, enum colour_mode cm, enum bits_per_comp bpc, enum ma memcpy(qps, table[index].qps, sizeof(qp_set)); } -double dsc_roundf(double num) +__fpu double dsc_roundf(double num) { if (num < 0.0) num = num - 0.5; @@ -95,7 +95,7 @@ double dsc_roundf(double num) return (int)(num); } -double dsc_ceil(double num) +__fpu double dsc_ceil(double num) { double retval = (int)num; @@ -105,7 +105,7 @@ double dsc_ceil(double num) return (int)retval; } -void get_ofs_set(qp_set ofs, enum colour_mode mode, float bpp) +__fpu void get_ofs_set(qp_set ofs, enum colour_mode mode, float bpp) { int *p = ofs; @@ -172,7 +172,7 @@ int median3(int a, int b, int c) return b; } -void calc_rc_params(struct rc_params *rc, enum colour_mode cm, enum bits_per_comp bpc, float bpp, int slice_width, int slice_height, int minor_version) +__fpu void calc_rc_params(struct rc_params *rc, enum colour_mode cm, enum bits_per_comp bpc, float bpp, int slice_width, int slice_height, int minor_version) { float bpp_group; float initial_xmit_delay_factor; diff --git a/drivers/gpu/drm/amd/display/dc/dsc/rc_calc_dpi.c b/drivers/gpu/drm/amd/display/dc/dsc/rc_calc_dpi.c index 1f6e63b71456..38b3c4ac96dd 100644 --- a/drivers/gpu/drm/amd/display/dc/dsc/rc_calc_dpi.c +++ b/drivers/gpu/drm/amd/display/dc/dsc/rc_calc_dpi.c @@ -98,7 +98,7 @@ static void copy_rc_to_cfg(struct drm_dsc_config *dsc_cfg, const struct rc_param dsc_cfg->rc_buf_thresh[i] = rc->rc_buf_thresh[i]; } -int dscc_compute_dsc_parameters(const struct drm_dsc_config *pps, struct dsc_parameters *dsc_params) +__fpu int dscc_compute_dsc_parameters(const struct drm_dsc_config *pps, struct dsc_parameters *dsc_params) { enum colour_mode mode = pps->convert_rgb ? CM_RGB : (pps->simple_422 ? CM_444 : @@ -115,7 +115,7 @@ int dscc_compute_dsc_parameters(const struct drm_dsc_config *pps, struct dsc_par double d_bytes_per_pixel = dsc_ceil(bpp * slice_width / 8.0) / slice_width; - // TODO: Make sure the formula for calculating this is precise (ceiling vs. floor, and at what point they should be applied) + // TODO: Make sure the formula for calculating this is precise (ceiling vs. floor, and at what point they should be applied if (pps->native_422 || pps->native_420) d_bytes_per_pixel /= 2; diff --git a/drivers/gpu/drm/amd/display/dc/inc/dcn_calc_math.h b/drivers/gpu/drm/amd/display/dc/inc/dcn_calc_math.h index 45a07eeffbb6..d2ea6cf65f7e 100644 --- a/drivers/gpu/drm/amd/display/dc/inc/dcn_calc_math.h +++ b/drivers/gpu/drm/amd/display/dc/inc/dcn_calc_math.h @@ -26,6 +26,8 @@ #ifndef _DCN_CALC_MATH_H_ #define _DCN_CALC_MATH_H_ +#include <asm/fpu/api.h> + float dcn_bw_mod(const float arg1, const float arg2); float dcn_bw_min2(const float arg1, const float arg2); unsigned int dcn_bw_max(const unsigned int arg1, const unsigned int arg2); diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 1607a698eccd..02a51fedd031 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -252,6 +252,9 @@ static int decode_instructions(struct objtool_file *file) strncmp(sec->name, ".discard.", 9)) sec->text = true; + if (!strcmp(sec->name, ".text.fpu")) + sec->fpu = true; + for (offset = 0; offset < sec->len; offset += insn->len) { insn = malloc(sizeof(*insn)); if (!insn) { @@ -288,8 +291,10 @@ static int decode_instructions(struct objtool_file *file) return -1; } - sym_for_each_insn(file, func, insn) + sym_for_each_insn(file, func, insn) { insn->func = func; + insn->fpu_safe = sec->fpu; + } } } diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h index ebbb10c61e24..a743f2f28feb 100644 --- a/tools/objtool/elf.h +++ b/tools/objtool/elf.h @@ -39,7 +39,7 @@ struct section { char *name; int idx; unsigned int len; - bool changed, text, rodata; + bool changed, text, rodata, fpu; }; struct symbol { _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx