diff mbox series

[v3,04/10] x86/fpu/xstate: Correct guest fpstate size calculation

Message ID 20250307164123.1613414-5-chao.gao@intel.com (mailing list archive)
State New
Headers show
Series Introduce CET supervisor state support | expand

Commit Message

Chao Gao March 7, 2025, 4:41 p.m. UTC 
From: Yang Weijiang <weijiang.yang@intel.com>

The guest fpstate size is calculated based on fpu_user_cfg, while
fpstate->xfeatures is set to fpu_kernel_cfg.default_features in
fpu_alloc_guest_fpstate(). In other words, the guest fpstate doesn't
allocate memory for all supervisor states, even though they are enabled.

Correct the calculation of the guest fpstate size.

Note that this issue does not cause any functional problems because the
guest fpstate is allocated using vmalloc(), which aligns the size to a
full page, providing enough space for all existing supervisor components.
On Emerald Rapids CPUs, the guest fpstate after this correction is ~2880
bytes.

Link: https://lore.kernel.org/kvm/20230914063325.85503-3-weijiang.yang@intel.com/
Fixes: 69f6ed1d14c6 ("x86/fpu: Provide infrastructure for KVM FPU cleanup")
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
 arch/x86/kernel/fpu/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Dave Hansen March 7, 2025, 5:53 p.m. UTC | #1 
On 3/7/25 08:41, Chao Gao wrote:
> Note that this issue does not cause any functional problems because the
> guest fpstate is allocated using vmalloc(), which aligns the size to a
> full page, providing enough space for all existing supervisor components.
> On Emerald Rapids CPUs, the guest fpstate after this correction is ~2880
> bytes.

How about we move up the fpstate pointers at allocation time so they
just scrape the end of the vmalloc buffer? Basically, move the
page-alignment padding to the beginning of the first page instead of the
end of the last page.
Chang S. Bae March 7, 2025, 9:37 p.m. UTC | #2 
On 3/7/2025 8:41 AM, Chao Gao wrote:
> 
> diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> index 6166a928d3f5..adc34914634e 100644
> --- a/arch/x86/kernel/fpu/core.c
> +++ b/arch/x86/kernel/fpu/core.c
> @@ -218,7 +218,7 @@ bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu)
>   	struct fpstate *fpstate;
>   	unsigned int size;
>   
> -	size = fpu_user_cfg.default_size + ALIGN(offsetof(struct fpstate, regs), 64);
> +	size = fpu_kernel_cfg.default_size + ALIGN(offsetof(struct fpstate, regs), 64);
>   	fpstate = vzalloc(size);
>   	if (!fpstate)
>   		return false;

BTW, did you ever base this series on the tip/master branch? The fix has 
already been merged there:

   1937e18cc3cf ("x86/fpu: Fix guest FPU state buffer allocation size")

Thanks,
Chang
Chao Gao March 8, 2025, 2:49 a.m. UTC | #3 
On Fri, Mar 07, 2025 at 01:37:15PM -0800, Chang S. Bae wrote:
>On 3/7/2025 8:41 AM, Chao Gao wrote:
>> 
>> diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
>> index 6166a928d3f5..adc34914634e 100644
>> --- a/arch/x86/kernel/fpu/core.c
>> +++ b/arch/x86/kernel/fpu/core.c
>> @@ -218,7 +218,7 @@ bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu)
>>   	struct fpstate *fpstate;
>>   	unsigned int size;
>> -	size = fpu_user_cfg.default_size + ALIGN(offsetof(struct fpstate, regs), 64);
>> +	size = fpu_kernel_cfg.default_size + ALIGN(offsetof(struct fpstate, regs), 64);
>>   	fpstate = vzalloc(size);
>>   	if (!fpstate)
>>   		return false;
>
>BTW, did you ever base this series on the tip/master branch? The fix has
>already been merged there:
>
>  1937e18cc3cf ("x86/fpu: Fix guest FPU state buffer allocation size")

Thanks for the information. I will remove this patch.

This series is currently based on 6.14-rc5. I should have used the tip/master
branch as the base and will do so next time.
Chao Gao March 8, 2025, 2:56 a.m. UTC | #4 
On Fri, Mar 07, 2025 at 09:53:40AM -0800, Dave Hansen wrote:
>On 3/7/25 08:41, Chao Gao wrote:
>> Note that this issue does not cause any functional problems because the
>> guest fpstate is allocated using vmalloc(), which aligns the size to a
>> full page, providing enough space for all existing supervisor components.
>> On Emerald Rapids CPUs, the guest fpstate after this correction is ~2880
>> bytes.
>
>How about we move up the fpstate pointers at allocation time so they
>just scrape the end of the vmalloc buffer? Basically, move the
>page-alignment padding to the beginning of the first page instead of the
>end of the last page.

That sounds like a good way to detect similar errors and might be helpful for
all other vmalloc'ed buffers.

I can try to implement this for the fpstate pointers. The patch will be put
at the end of the series or even in a separate series.
diff mbox series

Patch

diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 6166a928d3f5..adc34914634e 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -218,7 +218,7 @@  bool fpu_alloc_guest_fpstate(struct fpu_guest *gfpu)
 	struct fpstate *fpstate;
 	unsigned int size;
 
-	size = fpu_user_cfg.default_size + ALIGN(offsetof(struct fpstate, regs), 64);
+	size = fpu_kernel_cfg.default_size + ALIGN(offsetof(struct fpstate, regs), 64);
 	fpstate = vzalloc(size);
 	if (!fpstate)
 		return false;