[PATCH v2] LoongArch: KVM: Add KVM_LOONGARCH_GET/SET_CSR for bulk CSR migration

[PATCH v2] LoongArch: KVM: Add KVM_LOONGARCH_GET/SET_CSR for bulk CSR migration

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Migrating a vCPU's CSR state takes one KVM_GET/SET_ONE_REG ioctl per
register, thousands of syscalls for a large VM.

Add a bulk pair, KVM_LOONGARCH_GET_CSR / KVM_SET_CSR.  Userspace passes
an array of {index, data} entries; the kernel fills entries[].data on
GET and applies them on SET, both in a single call:

- KVM_LOONGARCH_GET_CSR does a single vcpu_load/put with
  kvm_deliver_intr(), which pulls pending interrupts into ESTAT,
  avoiding the per-register load/put side-effect that made ONE_REG
  snapshots of ESTAT order-sensitive.
- KVM_LOONGARCH_SET_CSR 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.

Entries are {index, data} rather than a positional array, so future CSRs
can be added without resizing the struct or changing the ioctl number.
CSRs above the core range (debug, breakpoint, PMU) stay on
KVM_GET/SET_ONE_REG; struct kvm_sregs is left empty, so the new ioctls
break no userspace.

On a Loongson 3A6000, snapshotting one vCPU's core CSR range (0x0-0x183)
drops from 388 KVM_GET_ONE_REG calls (~2 ms) to a single
KVM_LOONGARCH_GET_CSR ioctl (~6 us).

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 arch/loongarch/include/uapi/asm/kvm.h | 13 +++++
 arch/loongarch/kvm/vcpu.c             | 73 +++++++++++++++++++++++++++
 include/uapi/linux/kvm.h              |  4 ++
 3 files changed, 90 insertions(+)

diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
index cd0b5c11ca9c..59b3b7efa039 100644
--- a/arch/loongarch/include/uapi/asm/kvm.h
+++ b/arch/loongarch/include/uapi/asm/kvm.h
@@ -127,6 +127,19 @@ struct kvm_sync_regs {
 struct kvm_sregs {
 };
 
+/* bulk CSR entries for VM migration */
+struct kvm_loongarch_csr_entry {
+	__u32 index;
+	__u32 reserved;
+	__u64 data;
+};
+
+struct kvm_loongarch_csrs {
+	__u32 ncsrs;	/* number of csr entries */
+	__u32 pad;
+	__DECLARE_FLEX_ARRAY(struct kvm_loongarch_csr_entry, entries);
+};
+
 struct kvm_iocsr_entry {
 	__u32 addr;
 	__u32 pad;
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index 20c207d80e31..9b411c694718 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -1009,6 +1009,73 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs
 	return -ENOIOCTLCMD;
 }
 
+/* max CSR entries handled per ioctl */
+#define KVM_LOONGARCH_MAX_CSR_REGS	0x400
+
+/* Bulk get/set of CSR state for VM migration. */
+static int kvm_loongarch_csr_io(struct kvm_vcpu *vcpu,
+				struct kvm_loongarch_csrs __user *ucsrs,
+				bool write)
+{
+	int i, ret = 0;
+	unsigned long size;
+	struct kvm_loongarch_csrs csrs;
+	struct kvm_loongarch_csr_entry *entries;
+	struct loongarch_csrs *csr = vcpu->arch.csr;
+
+	if (copy_from_user(&csrs, ucsrs, sizeof(csrs)))
+		return -EFAULT;
+
+	if (csrs.ncsrs > KVM_LOONGARCH_MAX_CSR_REGS)
+		return -E2BIG;
+
+	size = array_size(csrs.ncsrs, sizeof(*entries));
+	entries = memdup_user(ucsrs->entries, size);
+	if (IS_ERR(entries))
+		return PTR_ERR(entries);
+
+	if (write) {
+		vcpu->arch.aux_inuse &= ~KVM_LARCH_HWCSR_USABLE;
+	} else {
+		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 < csrs.ncsrs; i++) {
+		unsigned int id = entries[i].index;
+
+		if (get_gcsr_flag(id) & INVALID_GCSR) {
+			ret = -EINVAL;
+			break;
+		}
+
+		if (write) {
+			ret = _kvm_setcsr(vcpu, id, entries[i].data);
+			if (ret)
+				break;
+		} else if (id == LOONGARCH_CSR_ESTAT) {
+			unsigned long estat, gintc;
+
+			/* ESTAT IP0~IP7 come from GINTC */
+			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;
+			entries[i].data = estat | (gintc << VIP_DELTA);
+		} else {
+			entries[i].data = kvm_read_sw_gcsr(csr, id);
+		}
+	}
+
+	if (!write && copy_to_user(ucsrs->entries, entries, size))
+		ret = -EFAULT;
+
+	kfree(entries);
+	return ret;
+}
+
 int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
 {
 	int i;
@@ -1266,6 +1333,12 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 			r = kvm_get_reg(vcpu, &reg);
 		break;
 	}
+	case KVM_LOONGARCH_GET_CSR:
+		r = kvm_loongarch_csr_io(vcpu, argp, false);
+		break;
+	case KVM_LOONGARCH_SET_CSR:
+		r = kvm_loongarch_csr_io(vcpu, argp, true);
+		break;
 	case KVM_ENABLE_CAP: {
 		struct kvm_enable_cap cap;
 
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 419011097fa8..aade65751721 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1670,4 +1670,8 @@ struct kvm_pre_fault_memory {
 	__u64 padding[5];
 };
 
+/* LoongArch: bulk CSR get/set for migration */
+#define KVM_LOONGARCH_GET_CSR	_IOWR(KVMIO, 0xd6, struct kvm_loongarch_csrs)
+#define KVM_LOONGARCH_SET_CSR	_IOW(KVMIO,  0xd7, struct kvm_loongarch_csrs)
+
 #endif /* __LINUX_KVM_H */
-- 
2.43.0

Re: [PATCH v2] LoongArch: KVM: Add KVM_LOONGARCH_GET/SET_CSR for bulk CSR migration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Missing bounds checking on the user-provided CSR index allows arbitrary out-of-bounds kernel memory read and write vulnerabilities via integer signedness conversion.
- [High] The new `KVM_LOONGARCH_GET_CSR` and `KVM_LOONGARCH_SET_CSR` ioctls violate KVM enumeration invariants because they are not discoverable via a KVM capability.
- [Medium] Missing validation of `pad` and `reserved` fields in UAPI structures prevents future extension.
--

--- Patch [1]: [PATCH v2] LoongArch: KVM: Add KVM_LOONGARCH_GET/SET_CSR for bulk CSR migration ---
commit d9e42903e3c1617b11882f82d3b16f57b379c8ac
Author: Tao Cui <cuitao@kylinos.cn>

LoongArch: KVM: Add KVM_LOONGARCH_GET/SET_CSR for bulk CSR migration

This commit introduces bulk get and set ioctls for migrating CSR state in a
single call, replacing the need for many KVM_GET/SET_ONE_REG calls. It adds
the KVM_LOONGARCH_GET_CSR and KVM_LOONGARCH_SET_CSR ioctls and their
associated data structures.

> diff --git a/arch/loongarch/include/uapi/asm/kvm.h b/arch/loongarch/include/uapi/asm/kvm.h
> index cd0b5c11ca9c2..59b3b7efa0397 100644
> --- a/arch/loongarch/include/uapi/asm/kvm.h
> +++ b/arch/loongarch/include/uapi/asm/kvm.h

[ ... ]

> @@ -127,6 +127,19 @@ struct kvm_sync_regs {
>  struct kvm_sregs {
>  };
>  
> +/* bulk CSR entries for VM migration */
> +struct kvm_loongarch_csr_entry {
> +	__u32 index;
> +	__u32 reserved;
> +	__u64 data;
> +};
> +
> +struct kvm_loongarch_csrs {
> +	__u32 ncsrs;	/* number of csr entries */
> +	__u32 pad;
> +	__DECLARE_FLEX_ARRAY(struct kvm_loongarch_csr_entry, entries);
> +};
> +

[Severity: Medium]
Are the pad and reserved fields validated to be zero?

Looking at the kvm_loongarch_csr_io() implementation, the structure is copied
from user space but the padding fields are not checked. If userspace leaves
garbage data in these fields, could it prevent the kernel from safely
repurposing them for future feature flags without breaking ABI compatibility?

>  struct kvm_iocsr_entry {
>  	__u32 addr;
>  	__u32 pad;
> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 20c207d80e31a..9b411c694718c 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c

[ ... ]

> @@ -1009,6 +1009,73 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs

[ ... ]

> +	for (i = 0; i < csrs.ncsrs; i++) {
> +		unsigned int id = entries[i].index;
> +
> +		if (get_gcsr_flag(id) & INVALID_GCSR) {

[Severity: Critical]
Could an attacker pass a value greater than or equal to 0x80000000 for
entries[i].index?

Since get_gcsr_flag() takes a signed int:

arch/loongarch/kvm/main.c:get_gcsr_flag() {
    if (csr < CSR_MAX_NUMS)
        return gcsr_flag[csr];
    ...
}

Would this unmasked 32-bit unsigned index be implicitly converted to a
negative integer, bypassing the csr < CSR_MAX_NUMS signed comparison?

If the out-of-bounds read of gcsr_flag happens to lack the INVALID_GCSR bit,
could this negative ID be passed to _kvm_setcsr() and eventually to
kvm_write_sw_gcsr(), leading to an arbitrary out-of-bounds 64-bit kernel
memory write?

> +			ret = -EINVAL;
> +			break;
> +		}
> +

[ ... ]

> @@ -1266,6 +1333,12 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>  			r = kvm_get_reg(vcpu, &reg);
>  		break;
>  	}
> +	case KVM_LOONGARCH_GET_CSR:
> +		r = kvm_loongarch_csr_io(vcpu, argp, false);
> +		break;
> +	case KVM_LOONGARCH_SET_CSR:
> +		r = kvm_loongarch_csr_io(vcpu, argp, true);
> +		break;

[Severity: High]
Does adding these ioctls without a corresponding KVM capability break KVM
enumeration invariants?

Since KVM_LOONGARCH_GET_CSR and KVM_LOONGARCH_SET_CSR are unconditionally
enabled here without a corresponding KVM_CAP_* constant, userspace tools like
QEMU might not have a programmatic way to discover if the running kernel
supports the new ioctls via KVM_CHECK_EXTENSION. Could this silently break
live migration or capability negotiation?

>  	case KVM_ENABLE_CAP: {
>  		struct kvm_enable_cap cap;
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 419011097fa8e..aade65751721d 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h

[ ... ]

> @@ -1670,4 +1670,8 @@ struct kvm_pre_fault_memory {
>  	__u64 padding[5];
>  };
>  
> +/* LoongArch: bulk CSR get/set for migration */
> +#define KVM_LOONGARCH_GET_CSR	_IOWR(KVMIO, 0xd6, struct kvm_loongarch_csrs)
> +#define KVM_LOONGARCH_SET_CSR	_IOW(KVMIO,  0xd7, struct kvm_loongarch_csrs)
> +
>  #endif /* __LINUX_KVM_H */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722074346.239507-1-cui.tao@linux.dev?part=1