Re: [PATCH RFC v1 02/20] KVM: x86: Refactor GPR accessors to differentiate register access types

[Paolo Bonzini <pbonzini@redhat.com>]

On 11/10/25 19:01, Chang S. Bae wrote:
> Refactor the GPR accessors to introduce internal helpers to distinguish
> between legacy and extended registers.
> 
> x86 CPUs introduce additional GPRs, but those registers will initially
> remain unused in the kernel and will not be saved in KVM register cache
> on every VM exit. Guest states are expected to remain live in hardware
> registers.
> 
> This abstraction layer centralizes the selection of access methods,
> providing a unified interface. For now, the EGPR accessors are
> placeholders to be implemented later.
> 
> Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
> ---
>   arch/x86/include/asm/kvm_host.h      | 18 ++++++++++++
>   arch/x86/include/asm/kvm_vcpu_regs.h | 16 ++++++++++
>   arch/x86/kvm/fpu.h                   |  6 ++++
>   arch/x86/kvm/x86.h                   | 44 ++++++++++++++++++++++++++--
>   4 files changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 48598d017d6f..940f83c121cf 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -212,6 +212,24 @@ enum {
>   	VCPU_SREG_GS,
>   	VCPU_SREG_TR,
>   	VCPU_SREG_LDTR,
> +#ifdef CONFIG_X86_64
> +	VCPU_XREG_R16 = __VCPU_XREG_R16,
> +	VCPU_XREG_R17 = __VCPU_XREG_R17,
> +	VCPU_XREG_R18 = __VCPU_XREG_R18,
> +	VCPU_XREG_R19 = __VCPU_XREG_R19,
> +	VCPU_XREG_R20 = __VCPU_XREG_R20,
> +	VCPU_XREG_R21 = __VCPU_XREG_R21,
> +	VCPU_XREG_R22 = __VCPU_XREG_R22,
> +	VCPU_XREG_R23 = __VCPU_XREG_R23,
> +	VCPU_XREG_R24 = __VCPU_XREG_R24,
> +	VCPU_XREG_R25 = __VCPU_XREG_R25,
> +	VCPU_XREG_R26 = __VCPU_XREG_R26,
> +	VCPU_XREG_R27 = __VCPU_XREG_R27,
> +	VCPU_XREG_R28 = __VCPU_XREG_R28,
> +	VCPU_XREG_R29 = __VCPU_XREG_R29,
> +	VCPU_XREG_R30 = __VCPU_XREG_R30,
> +	VCPU_XREG_R31 = __VCPU_XREG_R31,
> +#endif
>   };
>   
>   enum exit_fastpath_completion {
> diff --git a/arch/x86/include/asm/kvm_vcpu_regs.h b/arch/x86/include/asm/kvm_vcpu_regs.h
> index 1af2cb59233b..dd0cc171f405 100644
> --- a/arch/x86/include/asm/kvm_vcpu_regs.h
> +++ b/arch/x86/include/asm/kvm_vcpu_regs.h
> @@ -20,6 +20,22 @@
>   #define __VCPU_REGS_R13 13
>   #define __VCPU_REGS_R14 14
>   #define __VCPU_REGS_R15 15
> +#define __VCPU_XREG_R16 16
> +#define __VCPU_XREG_R17 17
> +#define __VCPU_XREG_R18 18
> +#define __VCPU_XREG_R19 19
> +#define __VCPU_XREG_R20 20
> +#define __VCPU_XREG_R21 21
> +#define __VCPU_XREG_R22 22
> +#define __VCPU_XREG_R23 23
> +#define __VCPU_XREG_R24 24
> +#define __VCPU_XREG_R25 25
> +#define __VCPU_XREG_R26 26
> +#define __VCPU_XREG_R27 27
> +#define __VCPU_XREG_R28 28
> +#define __VCPU_XREG_R29 29
> +#define __VCPU_XREG_R30 30
> +#define __VCPU_XREG_R31 31
>   #endif
>   
>   #endif /* _ASM_X86_KVM_VCPU_REGS_H */
> diff --git a/arch/x86/kvm/fpu.h b/arch/x86/kvm/fpu.h
> index 3ba12888bf66..159239b3a651 100644
> --- a/arch/x86/kvm/fpu.h
> +++ b/arch/x86/kvm/fpu.h
> @@ -4,6 +4,7 @@
>   #define __KVM_FPU_H_
>   
>   #include <asm/fpu/api.h>
> +#include <asm/kvm_vcpu_regs.h>
>   
>   typedef u32		__attribute__((vector_size(16))) sse128_t;
>   #define __sse128_u	union { sse128_t vec; u64 as_u64[2]; u32 as_u32[4]; }
> @@ -137,4 +138,9 @@ static inline void kvm_write_mmx_reg(int reg, const u64 *data)
>   	kvm_fpu_put();
>   }
>   
> +#ifdef CONFIG_X86_64
> +static inline unsigned long kvm_read_egpr(int reg) { return 0; }
> +static inline void kvm_write_egpr(int reg, unsigned long data) { }
> +#endif
> +
>   #endif
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 4edadd64d3d5..74ae8f12b5a1 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -400,9 +400,49 @@ static inline bool vcpu_match_mmio_gpa(struct kvm_vcpu *vcpu, gpa_t gpa)
>   	return false;
>   }
>   
> +#ifdef CONFIG_X86_64
> +static inline unsigned long _kvm_gpr_read(struct kvm_vcpu *vcpu, int reg)
> +{
> +	switch (reg) {
> +	case VCPU_REGS_RAX ... VCPU_REGS_R15:
> +		return kvm_register_read_raw(vcpu, reg);
> +	case VCPU_XREG_R16 ... VCPU_XREG_R31:
> +		return kvm_read_egpr(reg);
> +	default:
> +		WARN_ON_ONCE(1);
> +	}
> +
> +	return 0;
> +}
> +
> +static inline void _kvm_gpr_write(struct kvm_vcpu *vcpu, int reg, unsigned long val)
> +{
> +	switch (reg) {
> +	case VCPU_REGS_RAX ... VCPU_REGS_R15:
> +		kvm_register_write_raw(vcpu, reg, val);
> +		break;
> +	case VCPU_XREG_R16 ... VCPU_XREG_R31:
> +		kvm_write_egpr(reg, val);
> +		break;
> +	default:
> +		WARN_ON_ONCE(1);
> +	}
> +}
> +#else
> +static inline unsigned long _kvm_gpr_read(struct kvm_vcpu *vcpu, int reg)
> +{
> +	return kvm_register_read_raw(vcpu, reg);
> +}

Please leave these as kvm_gpr_{read,write}_raw.  It's easier to review 
than the leading underscore.  (It's not hard to undo if you use "git 
format-patch --stdout", do a mass replace on the resulting patch file, 
and then reapply the patch with "git am").

Paolo

> +static inline void _kvm_gpr_write(struct kvm_vcpu *vcpu, int reg, unsigned long val)
> +{
> +	kvm_register_write_raw(vcpu, reg, val);
> +}
> +#endif
> +
>   static inline unsigned long kvm_gpr_read(struct kvm_vcpu *vcpu, int reg)
>   {
> -	unsigned long val = kvm_register_read_raw(vcpu, reg);
> +	unsigned long val = _kvm_gpr_read(vcpu, reg);
>   
>   	return is_64_bit_mode(vcpu) ? val : (u32)val;
>   }
> @@ -411,7 +451,7 @@ static inline void kvm_gpr_write(struct kvm_vcpu *vcpu, int reg, unsigned long v
>   {
>   	if (!is_64_bit_mode(vcpu))
>   		val = (u32)val;
> -	return kvm_register_write_raw(vcpu, reg, val);
> +	_kvm_gpr_write(vcpu, reg, val);
>   }
>   
>   static inline bool kvm_check_has_quirk(struct kvm *kvm, u64 quirk)