Re: [PATCH] LoongArch: KVM: Implement KVM_GET/SET_SREGS for bulk CSR migration

[Bibo Mao]

On 2026/7/21 下午8:18, Tao Cui wrote:
> From: Tao Cui <cuitao@xxxxxxxxxx>
>
> KVM_GET/SET_SREGS has so far returned -ENOIOCTLCMD with an empty
> struct kvm_sregs, so migrating a vCPU's CSR state takes one
> KVM_GET/SET_ONE_REG ioctl per register -- thousands of syscalls for a
> large VM.
>
> Define struct kvm_sregs as a flat array of 0x184 entries (the core CSR
> range 0x0-0x183, up to DMWIN3) and implement the two ioctls:
>
> - get_sregs does a single vcpu_load/put, which also pulls pending
>    interrupts into ESTAT, avoiding the per-register load/put side-effect
>    that made ONE_REG snapshots of ESTAT order-sensitive.
> - set_sregs writes all CSRs via _kvm_setcsr in one pass, propagates
>    _kvm_setcsr errors, and clears KVM_LARCH_HWCSR_USABLE up front so a
>    mid-loop failure still forces the next vcpu_load() to reload from SW,
>    matching KVM_SET_ONE_REG.
>
> CSRs above 0x183 (debug, breakpoint, PMU) stay on KVM_GET/SET_ONE_REG.
> struct kvm_sregs was empty, so enlarging it -- and the _IOWR-derived
> ioctl number -- breaks no userspace; userspace built against the updated
> UAPI header can adopt the bulk ioctl.
>
> On a Loongson 3A6000, snapshotting one vCPU's core CSR range drops from
> 388 KVM_GET_ONE_REG calls (~2 ms) to a single KVM_GET_SREGS ioctl
> (~5 us).
Great, I think getting register with bulk method is good.
> Signed-off-by: Tao Cui <cuitao@xxxxxxxxxx>
> ---
>   arch/loongarch/include/uapi/asm/kvm.h |  3 ++
>   arch/loongarch/kvm/vcpu.c             | 43 +++++++++++++++++++++++++--
>   2 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> index cd0b5c11ca9c..dc091f29963d 100644
> --- a/arch/loongarch/include/uapi/asm/kvm.h
> +++ b/arch/loongarch/include/uapi/asm/kvm.h
> @@ -124,7 +124,10 @@ struct kvm_sync_regs {
>   };
>   
>   /* dummy definition */
> +#define KVM_LOONGARCH_NR_SREGS	0x184
> +
>   struct kvm_sregs {
> +	__u64 csr[KVM_LOONGARCH_NR_SREGS];

I think that get/set registers one by one is not so better. There are 
two possible potential problems with this method:
  1. With hard-code size 0x184, there may be compatible issue, in 
future there may be CSR register with index larger than 0x184
  2. There may be order dependency with CSR get or set, such 
timer,interrupt, or feature ctrl registers. Only that there is SW CSR 
shadow CSR register in KVM, so dependency relation disappears with this 
method.

In generic, I think KVM_GET_MSRS/KVM_SET_MSRS is a better method, VMM 
needs know the index and dependency at first, this API can be changed as 
generic and not relative with detailed architecture.

When we submit KVM to community in the beginning, this method was used, 
however the community suggests that new API 
KVM_GET_ONE_REG/KVM_SET_ONE_REG is added, it should be used. Maybe mixed 
of KVM_GET_BULK_REGS/KVM_GET_ONE_REG can be used :)

There is piece of UAPI with KVM_GET_MSRS/KVM_SET_MSRS.
struct kvm_msr_entry {
        __u32 index;
        __u32 reserved;
        __u64 data;
};

/* for KVM_GET_MSRS and KVM_SET_MSRS */
struct kvm_msrs {
        __u32 nmsrs; /* number of msrs in entries */
        __u32 pad;

        __DECLARE_FLEX_ARRAY(struct kvm_msr_entry, entries);
};

Regards
Bibo Mao
>   };
>   
>   struct kvm_iocsr_entry {
> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 20c207d80e31..0890b68efa2a 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c
> @@ -1001,12 +1001,51 @@ static int kvm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>   
>   int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
>   {
> -	return -ENOIOCTLCMD;
> +	int i;
> +	unsigned long estat, gintc;
> +	struct loongarch_csrs *csr = vcpu->arch.csr;
> +
> +	/*
> +	 * Pull pending interrupts into ESTAT with a single vcpu_load/put so
> +	 * the ESTAT value read below matches the current interrupt state.
> +	 * This also avoids the per-register load/put side-effect that makes
> +	 * the ONE_REG path's ESTAT snapshot order-sensitive.  The SW CSR
> +	 * reads run under vcpu->mutex, which serialises this ioctl.
> +	 */
> +	preempt_disable();
> +	vcpu_load(vcpu);
> +	kvm_deliver_intr(vcpu);
> +	vcpu->arch.aux_inuse &= ~KVM_LARCH_SWCSR_LATEST;
> +	vcpu_put(vcpu);
> +	preempt_enable();
> +
> +	for (i = 0; i < KVM_LOONGARCH_NR_SREGS; i++) {
> +		if (i == LOONGARCH_CSR_ESTAT) {
> +			gintc = kvm_read_sw_gcsr(csr, LOONGARCH_CSR_GINTC) & KVM_GINTC_IRQ_MASK;
> +			estat = kvm_read_sw_gcsr(csr, LOONGARCH_CSR_ESTAT) & ~KVM_ESTAT_EXTI_MASK;
> +			sregs->csr[i] = estat | (gintc << VIP_DELTA);
> +		} else {
> +			sregs->csr[i] = kvm_read_sw_gcsr(csr, i); > +		}
> +	}
> +
> +	return 0;
>   }
>   
>   int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
>   {
> -	return -ENOIOCTLCMD;
> +	int i, ret;
> +
> +	/* Clear first so a failing _kvm_setcsr still forces a HW reload. */
> +	vcpu->arch.aux_inuse &= ~KVM_LARCH_HWCSR_USABLE;
> +
> +	for (i = 0; i < KVM_LOONGARCH_NR_SREGS; i++) {
> +		ret = _kvm_setcsr(vcpu, i, sregs->csr[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
>   }
>   
>   int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)