Re: [PATCH 05/20] target/i386/tcg: add APX support to XSAVE/XRSTOR

["Chang S. Bae" <chang.seok.bae@intel.com>]

On 8/25/2026 5:29 AM, Paolo Bonzini wrote:
> With it, add a new hidden flag that will be used to enable the REX2 prefix
> and EVEX map4.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/cpu.h            |  3 ++
>   target/i386/tcg/tcg-cpu.h    | 16 +++++++---
>   target/i386/helper.c         | 11 +++++++
>   target/i386/tcg/fpu_helper.c | 59 +++++++++++++++++++++++++++++++++---
>   target/i386/tcg/tcg-cpu.c    |  5 +--
>   5 files changed, 83 insertions(+), 11 deletions(-)

...

>   
> -    /* MPX State: */
> -    XSaveBNDREG bndreg_state;
> -    XSaveBNDCSR bndcsr_state;
> +    /* Overlapping MPX and APX States: */
> +    union {
> +        struct {
> +            XSaveBNDREG bndreg;
> +            XSaveBNDCSR bndcsr;
> +        } mpx_state;
> +        XSaveAPX apx_state;
> +    };
> +

Nit:
Along with this, I was curios if something like this would be 
worthwhile, or too much:

#define XSAVE_APX_OFFSET	XSAVE_BNDREG_OFFSET


>   
> +void cpu_sync_apx_hflag(CPUX86State *env)
> +{
> +    if ((env->cr[4] & CR4_OSXSAVE_MASK)
> +        && (env->xcr0 & XSTATE_APX_MASK) == XSTATE_APX_MASK) {
> +        env->hflags |= HF_APX_EN_MASK;
> +    } else{
> +        env->hflags &= ~HF_APX_EN_MASK;
> +    }
> +}

This check could be simply (xcr0 & XSTATE_APX_MASK) since testing one 
bit here unlike the AVX check below.

> +
>   void cpu_sync_avx_hflag(CPUX86State *env)
>   {
>       if ((env->cr[4] & CR4_OSXSAVE_MASK)
> @@ -250,6 +260,7 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)


Overall, the change looks fine from my perspective. If allowed,

   Reviewed-by: Chang S. Bae <chang.seok.bae@intel.com>