[PATCH v3 10/20] i386/cpu: Add missing migratable xsave features

Zhao Liu posted 20 patches 3 months, 2 weeks ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Zhao Liu <zhao1.liu@intel.com>, Marcelo Tosatti <mtosatti@redhat.com>
There is a newer version of this series
[PATCH v3 10/20] i386/cpu: Add missing migratable xsave features
Posted by Zhao Liu 3 months, 2 weeks ago
Xtile-cfg & xtile-data are both user xstates. Their xstates are cached
in X86CPUState, and there's a related vmsd "vmstate_amx_xtile", so that
it's safe to mark them as migratable.

Arch lbr xstate is a supervisor xstate, and it is save & load by saving
& loading related arch lbr MSRs, which are cached in X86CPUState, and
there's a related vmsd "vmstate_arch_lbr". So it's also safe to mark it
as migratable (even though KVM hasn't supported it - its migration
support is completed in QEMU).

PT is still unmigratable since KVM disabled it and there's no vmsd and
no other emulation/simulation support.

Tested-by: Farrah Chen <farrah.chen@intel.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1917376dbea9..b01729ad36d2 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1522,7 +1522,8 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
         .migratable_flags = XSTATE_FP_MASK | XSTATE_SSE_MASK |
             XSTATE_YMM_MASK | XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK |
             XSTATE_OPMASK_MASK | XSTATE_ZMM_Hi256_MASK | XSTATE_Hi16_ZMM_MASK |
-            XSTATE_PKRU_MASK,
+            XSTATE_PKRU_MASK | XSTATE_ARCH_LBR_MASK | XSTATE_XTILE_CFG_MASK |
+            XSTATE_XTILE_DATA_MASK,
     },
     [FEAT_XSAVE_XCR0_HI] = {
         .type = CPUID_FEATURE_WORD,
-- 
2.34.1
Re: [PATCH v3 10/20] i386/cpu: Add missing migratable xsave features
Posted by Zhao Liu 3 months, 2 weeks ago
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 1917376dbea9..b01729ad36d2 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1522,7 +1522,8 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>          .migratable_flags = XSTATE_FP_MASK | XSTATE_SSE_MASK |
>              XSTATE_YMM_MASK | XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK |
>              XSTATE_OPMASK_MASK | XSTATE_ZMM_Hi256_MASK | XSTATE_Hi16_ZMM_MASK |
> -            XSTATE_PKRU_MASK,
> +            XSTATE_PKRU_MASK | XSTATE_ARCH_LBR_MASK | XSTATE_XTILE_CFG_MASK |

ARCH LBR belongs to FEAT_XSAVE_XSS_LO.

> +            XSTATE_XTILE_DATA_MASK,
>      },
>      [FEAT_XSAVE_XCR0_HI] = {
>          .type = CPUID_FEATURE_WORD,
> -- 
> 2.34.1
>
Re: [PATCH v3 10/20] i386/cpu: Add missing migratable xsave features
Posted by Xiaoyao Li 3 months, 2 weeks ago
On 10/24/2025 2:56 PM, Zhao Liu wrote:
> Xtile-cfg & xtile-data are both user xstates. Their xstates are cached
> in X86CPUState, and there's a related vmsd "vmstate_amx_xtile", so that
> it's safe to mark them as migratable.
> 
> Arch lbr xstate is a supervisor xstate, and it is save & load by saving
> & loading related arch lbr MSRs, which are cached in X86CPUState, and
> there's a related vmsd "vmstate_arch_lbr". So it's also safe to mark it
> as migratable (even though KVM hasn't supported it - its migration
> support is completed in QEMU).
> 
> PT is still unmigratable since KVM disabled it and there's no vmsd and
> no other emulation/simulation support.

The patch itself looks reasonable.

I'm wondering why there is no issue reported since I believe folks 
tested the functionality of AMX live migration when AMX support was 
upstreamed. So I explore a bit and find that the 
migrable_flags/ungratable_flags in XCR0/XSS leaf don't take any effect 
because of the
x86_cpu_enable_xsave_components()

Though the feature expansion in x86_cpu_expand_features() under

	if (xcc->max_features) {
		...
	}

only enables migratable features when cpu->migratable is true, 
x86_cpu_enable_xsave_components() overwrite the value later.

> Tested-by: Farrah Chen <farrah.chen@intel.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   target/i386/cpu.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 1917376dbea9..b01729ad36d2 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1522,7 +1522,8 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>           .migratable_flags = XSTATE_FP_MASK | XSTATE_SSE_MASK |
>               XSTATE_YMM_MASK | XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK |
>               XSTATE_OPMASK_MASK | XSTATE_ZMM_Hi256_MASK | XSTATE_Hi16_ZMM_MASK |
> -            XSTATE_PKRU_MASK,
> +            XSTATE_PKRU_MASK | XSTATE_ARCH_LBR_MASK | XSTATE_XTILE_CFG_MASK |
> +            XSTATE_XTILE_DATA_MASK,
>       },
>       [FEAT_XSAVE_XCR0_HI] = {
>           .type = CPUID_FEATURE_WORD,
Re: [PATCH v3 10/20] i386/cpu: Add missing migratable xsave features
Posted by Zhao Liu 3 months, 2 weeks ago
> Though the feature expansion in x86_cpu_expand_features() under
> 
> 	if (xcc->max_features) {
> 		...
> 	}
> 
> only enables migratable features when cpu->migratable is true,
> x86_cpu_enable_xsave_components() overwrite the value later.

I have not changed the related logic, and this was intentional...too,
which is planed to be cleaned up after CET.
Re: [PATCH v3 10/20] i386/cpu: Add missing migratable xsave features
Posted by Zhao Liu 3 months, 2 weeks ago
On Mon, Oct 27, 2025 at 06:19:40PM +0800, Zhao Liu wrote:
> Date: Mon, 27 Oct 2025 18:19:40 +0800
> From: Zhao Liu <zhao1.liu@intel.com>
> Subject: Re: [PATCH v3 10/20] i386/cpu: Add missing migratable xsave
>  features
> 
> > Though the feature expansion in x86_cpu_expand_features() under
> > 
> > 	if (xcc->max_features) {
> > 		...
> > 	}
> > 
> > only enables migratable features when cpu->migratable is true,
> > x86_cpu_enable_xsave_components() overwrite the value later.
> 
> I have not changed the related logic, and this was intentional...too,
> which is planed to be cleaned up after CET.

There's only 1 use case of migratable_flags, so I would try to drop
it directly.

The xsave-managed/enabled feature is not suitable as the configurable
feature. Therefore, it is best to keep it non-configurable as it is
currently.

At least with this fix, the support for the new xsave feature —
including APX next — will not be broken, and the migratable flag
refactoring will become a separate RFC.

Regards,
Zhao


Re: [PATCH v3 10/20] i386/cpu: Add missing migratable xsave features
Posted by Xiaoyao Li 3 months, 2 weeks ago
On 10/27/2025 7:18 PM, Zhao Liu wrote:
> On Mon, Oct 27, 2025 at 06:19:40PM +0800, Zhao Liu wrote:
>> Date: Mon, 27 Oct 2025 18:19:40 +0800
>> From: Zhao Liu <zhao1.liu@intel.com>
>> Subject: Re: [PATCH v3 10/20] i386/cpu: Add missing migratable xsave
>>   features
>>
>>> Though the feature expansion in x86_cpu_expand_features() under
>>>
>>> 	if (xcc->max_features) {
>>> 		...
>>> 	}
>>>
>>> only enables migratable features when cpu->migratable is true,
>>> x86_cpu_enable_xsave_components() overwrite the value later.
>>
>> I have not changed the related logic, and this was intentional...too,
>> which is planed to be cleaned up after CET.
> 
> There's only 1 use case of migratable_flags, so I would try to drop
> it directly.
> 
> The xsave-managed/enabled feature is not suitable as the configurable
> feature. Therefore, it is best to keep it non-configurable as it is
> currently.
> 
> At least with this fix, the support for the new xsave feature —
> including APX next — will not be broken, 

can you elaborate what will be broken without the patch?

As I see, we can drop the .migratable_flags directly.

migrable_flags is only used in x86_cpu_get_migratable_flags(), which is 
only called by x86_cpu_get_supported_feature_word() when passed @cpu is 
not null and cpu->migratable is true. So it only affects the case of

   x86_cpu_expand_features()
     -> x86_cpu_get_supported_feature_word()

And only FEAT_XSAVE_XCR0_LO defines .migratable_flags

As I commented earlier, though the .migrable_flags determines the return 
value of x86_cpu_get_supported_feature_word() for 
features[FEAT_XSAVE_XCR0_LO] in x86_cpu_expand_features(), eventually 
the x86_cpu_enable_xsave_components() overwrites 
features[FEAT_XSAVE_XCR0_LO]. So even we set the migratable_flags to 0 
for FEAT_XSAVE_XCR0_LO, it doesn't have any issue.

So I think we can remove migratable_flags totally.

> and the migratable flag
> refactoring will become a separate RFC.
> 
> Regards,
> Zhao
> 


Re: [PATCH v3 10/20] i386/cpu: Add missing migratable xsave features
Posted by Zhao Liu 3 months, 1 week ago
> can you elaborate what will be broken without the patch?

Obviously, the migratable_flags has been broken.

> As I commented earlier, though the .migrable_flags determines the return
> value of x86_cpu_get_supported_feature_word() for
> features[FEAT_XSAVE_XCR0_LO] in x86_cpu_expand_features(), eventually the
> x86_cpu_enable_xsave_components() overwrites features[FEAT_XSAVE_XCR0_LO].
> So even we set the migratable_flags to 0 for FEAT_XSAVE_XCR0_LO, it doesn't
> have any issue.

No. this seemingly 'correct' result what you see is just due to
different bugs influencing each other: the flags are wrong, the code
path is wrong, and the impact produced by the flags is also wrong.

> So I think we can remove migratable_flags totally.

migratable_flags is used for feature leaves that are non-migratable by
default, while unmigratable_flags is used for feature leaves that are
migratable by default. Simply removing it doesn't need much effort, but
additional clarification is needed - about whether and how it affects
the migratable/unmigratable feature setting. Therefore, it's better to
do such refactor in the separate thread rather than combining it with
CET for now.

Regards,
Zhao
Re: [PATCH v3 10/20] i386/cpu: Add missing migratable xsave features
Posted by Xiaoyao Li 3 months ago
On 10/30/2025 11:56 PM, Zhao Liu wrote:
>> can you elaborate what will be broken without the patch?
> 
> Obviously, the migratable_flags has been broken.
> 
>> As I commented earlier, though the .migrable_flags determines the return
>> value of x86_cpu_get_supported_feature_word() for
>> features[FEAT_XSAVE_XCR0_LO] in x86_cpu_expand_features(), eventually the
>> x86_cpu_enable_xsave_components() overwrites features[FEAT_XSAVE_XCR0_LO].
>> So even we set the migratable_flags to 0 for FEAT_XSAVE_XCR0_LO, it doesn't
>> have any issue.
> 
> No. this seemingly 'correct' result what you see is just due to
> different bugs influencing each other: the flags are wrong, the code
> path is wrong, and the impact produced by the flags is also wrong.
> 
>> So I think we can remove migratable_flags totally.
> 
> migratable_flags is used for feature leaves that are non-migratable by
> default, while unmigratable_flags is used for feature leaves that are
> migratable by default. Simply removing it doesn't need much effort, but
> additional clarification is needed - about whether and how it affects
> the migratable/unmigratable feature setting. Therefore, it's better to
> do such refactor in the separate thread rather than combining it with
> CET for now.

I meant we don't need to bother touching .migratable_flags field as this 
patch because it doesn't make any difference to the functionality. Just 
drop it from the series and feel free to clean it up with a separate series.

> Regards,
> Zhao
> 
>