mbox series

[0/2] i386/cpu: Two minor fixes for x86_cpu_enable_xsave_components()

Message ID 20240115091325.1904229-1-xiaoyao.li@intel.com
Headers show
Series i386/cpu: Two minor fixes for x86_cpu_enable_xsave_components() | expand

Message

Xiaoyao Li Jan. 15, 2024, 9:13 a.m. UTC
The two bugs were introduced when xsaves feature was added by commit
301e90675c3f ("target/i386: Enable support for XSAVES based features").

Xiaoyao Li (2):
  i386/cpu: Clear FEAT_XSAVE_XSS_LO/HI leafs when CPUID_EXT_XSAVE is not
    available
  i386/cpu: Mask with XCR0/XSS mask for FEAT_XSAVE_XCR0_HI and
    FEAT_XSAVE_XSS_HI leafs

 target/i386/cpu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Zhao Liu Jan. 16, 2024, 2:19 p.m. UTC | #1
Hi Xiaoyao,

On Mon, Jan 15, 2024 at 04:13:23AM -0500, Xiaoyao Li wrote:
> Date: Mon, 15 Jan 2024 04:13:23 -0500
> From: Xiaoyao Li <xiaoyao.li@intel.com>
> Subject: [PATCH 0/2] i386/cpu: Two minor fixes for
>  x86_cpu_enable_xsave_components()
> X-Mailer: git-send-email 2.34.1
> 
> The two bugs were introduced when xsaves feature was added by commit
> 301e90675c3f ("target/i386: Enable support for XSAVES based features").

Could you please provide more details about reproducing these two bugs?
If I'm able, I'd be glad to help you to test and verify them.

Regards,
Zhao

> 
> Xiaoyao Li (2):
>   i386/cpu: Clear FEAT_XSAVE_XSS_LO/HI leafs when CPUID_EXT_XSAVE is not
>     available
>   i386/cpu: Mask with XCR0/XSS mask for FEAT_XSAVE_XCR0_HI and
>     FEAT_XSAVE_XSS_HI leafs
> 
>  target/i386/cpu.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> -- 
> 2.34.1
> 
>
Xiaoyao Li Jan. 16, 2024, 3:42 p.m. UTC | #2
On 1/16/2024 10:19 PM, Zhao Liu wrote:
> Hi Xiaoyao,
> 
> On Mon, Jan 15, 2024 at 04:13:23AM -0500, Xiaoyao Li wrote:
>> Date: Mon, 15 Jan 2024 04:13:23 -0500
>> From: Xiaoyao Li <xiaoyao.li@intel.com>
>> Subject: [PATCH 0/2] i386/cpu: Two minor fixes for
>>   x86_cpu_enable_xsave_components()
>> X-Mailer: git-send-email 2.34.1
>>
>> The two bugs were introduced when xsaves feature was added by commit
>> 301e90675c3f ("target/i386: Enable support for XSAVES based features").
> 
> Could you please provide more details about reproducing these two bugs?
> If I'm able, I'd be glad to help you to test and verify them.

There are potential bugs and currently we don't have test step to 
trigger it. Because for patch 1, KVM doesn't support arch-lbr 
virtualization yet, which is the first user in QEMU of xss. Once KVM 
merges the arch-lbr series, using "-cpu xxx,+arch-lbr,-xsave" can expose 
arch-lbr to guest, which violates the architectural behavior of xfeatures.

For patch2, current code just happens to work correctly because there is 
not xfeature in upper 32-bit get defined yet. But I think make the code 
logically correct is important and we shouldn't depend on the 
happened-to-work code.


> Regards,
> Zhao
> 
>>
>> Xiaoyao Li (2):
>>    i386/cpu: Clear FEAT_XSAVE_XSS_LO/HI leafs when CPUID_EXT_XSAVE is not
>>      available
>>    i386/cpu: Mask with XCR0/XSS mask for FEAT_XSAVE_XCR0_HI and
>>      FEAT_XSAVE_XSS_HI leafs
>>
>>   target/i386/cpu.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> -- 
>> 2.34.1
>>
>>
Paolo Bonzini Jan. 25, 2024, 9:59 a.m. UTC | #3
Queued, thanks.

Paolo