On Tue, May 10, 2016 at 04:29:53PM -0700, Yu-cheng Yu wrote:
> The XSAVE area of kernel can be in standard or compacted format;
"The kernel xstate area... "
and can we call it the xstate area as there are a bunch of XSAVE*
insns touching it. The file which deals with it is even called that:
arch/x86/kernel/fpu/xstate.c
> it is always in standard format for user mode. When XSAVES is
> enabled, the kernel uses the compacted format and it is necessary
> to use a separate fpu_user_xstate_size for signal/ptrace frames.
>
> Based on an earlier patch from Fenghua Yu <fenghua...@intel.com>
>
> Signed-off-by: Fenghua Yu <fenghua...@intel.com>
> [yu-cheng...@intel.com: rebase to current, rename to fpu_user_xstate_size]
> Signed-off-by: Yu-cheng Yu <yu-cheng...@intel.com>
> Reviewed-by: Dave Hansen <dave.han...@intel.com>
Maybe I wasn't as clear as I hoped to be. Let me be more specific:
So you either need to do:
---
From: Fenghua
...
Signed-off-by: Fenghua
Signed-off-by: You
...
---
or
---
Based on an earlier patch from Fenghua Yu <fenghua...@intel.com>.
Signed-off-by: You
---
with the second variant making you the author implicitly because you're
the sender.
Makes more sense this way?
> ---
> arch/x86/include/asm/fpu/xstate.h | 1 -
> arch/x86/include/asm/processor.h | 1 +
> arch/x86/kernel/fpu/init.c | 5 ++-
> arch/x86/kernel/fpu/signal.c | 27 ++++++++++----
> arch/x86/kernel/fpu/xstate.c | 76
> ++++++++++++++++++++++++---------------
> 5 files changed, 73 insertions(+), 37 deletions(-)
...
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index b48ef35..dfac87d 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -44,6 +44,13 @@ static unsigned int xstate_sizes[XFEATURE_MAX] = { [ 0
> ... XFEATURE_MAX - 1] =
> static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask)*8];
>
> /*
> + * The XSAVE area of kernel can be in standard or compacted format;
"The kernel xstate area... "
> + * it is always in standard format for user mode. This is the user
> + * mode standard format size used for signal and ptrace frames.
> + */
But yes, that's very nice commenting.
> +unsigned int fpu_user_xstate_size;
> +
> +/*
> * Clear all of the X86_FEATURE_* bits that are unavailable
> * when the CPU has no XSAVE support.
> */
...
> @@ -591,7 +598,15 @@ static bool is_supported_xstate_size(unsigned int
> test_xstate_size)
> static int init_xstate_size(void)
> {
> /* Recompute the context size for enabled features: */
> - unsigned int possible_xstate_size = calculate_xstate_size();
> + unsigned int possible_xstate_size;
> + unsigned int xsave_size;
> +
> + xsave_size = get_xsave_size();
> +
> + if (cpu_has_xsaves)
So next time you submit patches against -tip, please merge them with
tip/master and see if they still build, at least.
During last review I told you to use boot_cpu_has(X86_FEATURE_XSAVES)
because those cpu_has_XXX things are going away and are gone in tip
already.
Please be more careful when incorporating review comments - otherwise it
is waste of both yours and reviewer's time.
Thanks.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284
(AG Nürnberg)
--
