在 2026/7/16 09:38, Bibo Mao 写道:
> Now pv_feature can be set only once, there is problem with VM migration.
> Where it is set when vCPU is created and after migration, here it is
> allow to set for many times, until vCPU starts to run.
>
Hi Bibo,
The Sashiko AI review raised two concerns on this patch that I think
are valid:
1. Since ran_atleast_once is per-vCPU but pv_features is VM-wide,
userspace could run vCPU 0 and then use vCPU 1 (whose
ran_atleast_once is still false) to change pv_features while
vCPU 0 is executing.
2. Without the old LOONGARCH_PV_FEAT_UPDATED latch, setting different
PV features on different un-run vCPUs silently overwrites
pv_features instead of returning -EINVAL.
Both stem from using a per-vCPU flag to protect VM-wide state.
Would a VM-level bool in kvm_arch (e.g. pv_features_configured),
set on first SET_ATTR or first RUN of any vCPU, work? It would
not be migrated since it is not exposed via any ioctl.
Thanks,
Tao
> Signed-off-by: Bibo Mao <[email protected]>
> ---
> arch/loongarch/include/asm/kvm_host.h | 4 +++-
> arch/loongarch/kvm/vcpu.c | 15 +++++++++++----
> 2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/kvm_host.h
> b/arch/loongarch/include/asm/kvm_host.h
> index 23cfbecebbd7..af376fc44c44 100644
> --- a/arch/loongarch/include/asm/kvm_host.h
> +++ b/arch/loongarch/include/asm/kvm_host.h
> @@ -163,7 +163,6 @@ enum emulation_result {
> #define KVM_LARCH_SWCSR_LATEST (0x1 << 3)
> #define KVM_LARCH_HWCSR_USABLE (0x1 << 4)
>
> -#define LOONGARCH_PV_FEAT_UPDATED BIT_ULL(63)
> #define LOONGARCH_PV_FEAT_MASK (BIT(KVM_FEATURE_IPI) |
> \
> BIT(KVM_FEATURE_PREEMPT) | \
> BIT(KVM_FEATURE_STEAL_TIME) | \
> @@ -250,6 +249,9 @@ struct kvm_vcpu_arch {
> /* cpucfg */
> u32 cpucfg[KVM_MAX_CPUCFG_REGS];
>
> + /* VCPU ran at least once */
> + bool ran_atleast_once;
> +
> /* paravirt steal time */
> struct {
> u64 guest_addr;
> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 20c207d80e31..ce6a1b06d50d 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c
> @@ -1164,11 +1164,14 @@ static int kvm_loongarch_cpucfg_set_attr(struct
> kvm_vcpu *vcpu,
> if (val & ~valid)
> return -EINVAL;
>
> - /* All vCPUs need set the same PV features */
> - if ((kvm->arch.pv_features & LOONGARCH_PV_FEAT_UPDATED)
> - && ((kvm->arch.pv_features & valid) != val))
> + if ((kvm->arch.pv_features & valid) == val)
> + return 0;
> +
> + if (vcpu->arch.ran_atleast_once)
> return -EINVAL;
> - kvm->arch.pv_features = val | LOONGARCH_PV_FEAT_UPDATED;
> +
> + /* All vCPUs need set the same PV features */
> + kvm->arch.pv_features = val;
> return 0;
> default:
> return -ENXIO;
> @@ -1851,6 +1854,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> int r = -EINTR;
> struct kvm_run *run = vcpu->run;
>
> + /* Mark this VCPU ran at least once */
> + if (!vcpu->arch.ran_atleast_once)
> + vcpu->arch.ran_atleast_once = true;
> +
> if (vcpu->mmio_needed) {
> if (!vcpu->mmio_is_write)
> kvm_complete_mmio_read(vcpu, run);
>
> base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
