Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH v2] LoongArch: KVM: Advertise already-supported capabilities
@ 2026-08-05  5:45 Tao Cui
  2026-08-05  6:06 ` sashiko-bot
  2026-08-06 11:50 ` Bibo Mao
  0 siblings, 2 replies; 5+ messages in thread
From: Tao Cui @ 2026-08-05  5:45 UTC (permalink / raw)
  To: zhaotianrui, maobibo
  Cc: chenhuacai, kernel, kvm, loongarch, linux-kernel, cui.tao

Several LoongArch KVM features are fully implemented but were never
reported through their standard KVM_CHECK_EXTENSION probes, so userspace
cannot discover them.  Advertise the ones that already work and have a
real consumer:

  - KVM_CAP_STEAL_TIME: steal time works end-to-end (host-side record
    plus guest paravirt side), and the probe returns kvm_pvtime_supported().

  - KVM_CAP_VCPU_ATTRIBUTES: per-vCPU device-attribute ioctls are
    implemented (CPUCFG, PVTIME control); advertise the discovery cap.

This adds no new functionality; it only makes existing features
discoverable via the standard probe.

Signed-off-by: Tao Cui <cui.tao@linux.dev>
---
Changes in v2:
  - Drop KVM_CAP_IRQFD_RESAMPLE: the only consumer is VFIO INTx, which
    needs IOMMU/VFIO not yet available on LoongArch.
  - Drop KVM_CAP_VM_GPA_BITS: the cap implies a GPA-width setter that
    LoongArch does not implement.

 arch/loongarch/kvm/vm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
index 1317c718f896..0a51931d6f6e 100644
--- a/arch/loongarch/kvm/vm.c
+++ b/arch/loongarch/kvm/vm.c
@@ -122,6 +122,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_IOEVENTFD:
 	case KVM_CAP_MP_STATE:
 	case KVM_CAP_SET_GUEST_DEBUG:
+	case KVM_CAP_VCPU_ATTRIBUTES:
 		r = 1;
 		break;
 	case KVM_CAP_NR_VCPUS:
@@ -136,6 +137,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_NR_MEMSLOTS:
 		r = KVM_USER_MEM_SLOTS;
 		break;
+	case KVM_CAP_STEAL_TIME:
+		r = kvm_pvtime_supported();
+		break;
 	default:
 		r = 0;
 		break;

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] LoongArch: KVM: Advertise already-supported capabilities
  2026-08-05  5:45 [PATCH v2] LoongArch: KVM: Advertise already-supported capabilities Tao Cui
@ 2026-08-05  6:06 ` sashiko-bot
  2026-08-05  8:28   ` Tao Cui
  2026-08-06 11:50 ` Bibo Mao
  1 sibling, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-08-05  6:06 UTC (permalink / raw)
  To: Tao Cui; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The unsafe user-access macros (`unsafe_put_user()` and `unsafe_get_user()`) are used without the required `user_access_begin()` and `user_access_end()` guards, violating the kernel API contract.
- [High] A lockless read-modify-write on the shared VM state `kvm->arch.pv_features` allows userspace to bypass the validation check that requires all vCPUs to share identical PV features.
--

--- Patch [1]: [PATCH v2] LoongArch: KVM: Advertise already-supported capabilities ---
commit 91e3758510b96f271c2589373d1340c246fab45a
Author: Tao Cui <cui.tao@linux.dev>

LoongArch: KVM: Advertise already-supported capabilities
    
This commit advertises KVM_CAP_STEAL_TIME and KVM_CAP_VCPU_ATTRIBUTES on
LoongArch, making existing features discoverable to userspace.

> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> index 1317c718f896a..0a51931d6f6e0 100644
> --- a/arch/loongarch/kvm/vm.c
> +++ b/arch/loongarch/kvm/vm.c
> @@ -122,6 +122,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  	case KVM_CAP_IOEVENTFD:
>  	case KVM_CAP_MP_STATE:
>  	case KVM_CAP_SET_GUEST_DEBUG:
> +	case KVM_CAP_VCPU_ATTRIBUTES:

[Severity: High]
This is a pre-existing issue, but does advertising KVM_CAP_VCPU_ATTRIBUTES
expose a data race in kvm_loongarch_cpucfg_set_attr()?

Looking at arch/loongarch/kvm/vcpu.c, there is a lockless read-modify-write
sequence on the shared VM state:

arch/loongarch/kvm/vcpu.c:kvm_loongarch_cpucfg_set_attr() {
...
	/* All vCPUs need set the same PV features */
	if ((kvm->arch.pv_features & LOONGARCH_PV_FEAT_UPDATED)
			&& ((kvm->arch.pv_features & valid) != val))
		return -EINVAL;
	kvm->arch.pv_features = val | LOONGARCH_PV_FEAT_UPDATED;
...
}

If multiple threads concurrently call KVM_SET_DEVICE_ATTR on different vCPUs,
can they bypass this validation check by both reading before either writes,
resulting in divergent configurations being silently applied globally instead
of returning -EINVAL?

>  		r = 1;
>  		break;
>  	case KVM_CAP_NR_VCPUS:

[ ... ]

> @@ -136,6 +137,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  	case KVM_CAP_NR_MEMSLOTS:
>  		r = KVM_USER_MEM_SLOTS;
>  		break;
> +	case KVM_CAP_STEAL_TIME:

[Severity: High]
This isn't a bug introduced by this patch, but does advertising
KVM_CAP_STEAL_TIME expose an issue with unsafe memory accesses?

In arch/loongarch/kvm/vcpu.c, unsafe_put_user() is used without the required
user_access_begin() and user_access_end() guards:

arch/loongarch/kvm/vcpu.c:kvm_update_stolen_time() {
...
	unsafe_put_user(0, &st->preempted, out);
...
}

And similarly in kvm_vcpu_set_pv_preempted():

arch/loongarch/kvm/vcpu.c:kvm_vcpu_set_pv_preempted() {
...
	unsafe_put_user(KVM_VCPU_PREEMPTED, &st->preempted, out);
...
}

Since omitting user_access_begin() skips the access_ok() check at the access
site, could this cause a kernel fault if hardware-enforced user access
protections are introduced?

> +		r = kvm_pvtime_supported();
> +		break;
>  	default:
>  		r = 0;
>  		break;

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] LoongArch: KVM: Advertise already-supported capabilities
  2026-08-05  6:06 ` sashiko-bot
@ 2026-08-05  8:28   ` Tao Cui
  2026-08-06 11:57     ` Bibo Mao
  0 siblings, 1 reply; 5+ messages in thread
From: Tao Cui @ 2026-08-05  8:28 UTC (permalink / raw)
  To: sashiko-reviews, Bibo Mao; +Cc: cui.tao, kvm


Hi Bibo,

The Sashiko automated review of this patch flagged two pre-existing items
— neither is from this patch. Wanted to get your thoughts,

在 2026/8/5 14:06, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The unsafe user-access macros (`unsafe_put_user()` and `unsafe_get_user()`) are used without the required `user_access_begin()` and `user_access_end()` guards, violating the kernel API contract.
The steal-time code uses unsafe_put_user()/unsafe_get_user() without
   the usual user_access_begin()/end() around them. On LoongArch it
   doesn't actually cause any trouble — there's no hardware user-access
   protection (user_access_begin() just resolves to access_ok()),
   __put_user is a plain store, and the ghc->hva address is already
   validated by kvm_gfn_to_hva_cache_init(), so the access_ok() is
   redundant anyway. So I see it more as a convention inconsistency /
   style nit than a real bug. I'd be happy to put together a small
   cleanup wrapping those in user_access_begin()/end() (or switching to
   kvm_write_guest_cached()). Does that sound like just cleanup to you,
   and would you want it?
> - [High] A lockless read-modify-write on the shared VM state `kvm->arch.pv_features` allows userspace to bypass the validation check that requires all vCPUs to share identical PV features.
The check-then-set on kvm->arch.pv_features in cpucfg_set_attr() is a
   genuine TOCTOU — again pre-existing, unrelated to this patch. It's not
   really reachable in practice: 0/3000 with two vCPUs hammering it
   concurrently, though a debug widening of the window does reproduce it
   every time (3000/3000). It'd basically need a userspace firing
   KVM_SET_DEVICE_ATTR with different values on two vCPUs at once. The
   odds are low, but I do think edge cases like this are still worth
   fixing — happy to send a separate patch using a mutex or cmpxchg.

Neither of these affects v2 itself, so I'd leave it as-is.

Thanks,
Tao
> --
> 
> --- Patch [1]: [PATCH v2] LoongArch: KVM: Advertise already-supported capabilities ---
> commit 91e3758510b96f271c2589373d1340c246fab45a
> Author: Tao Cui <cui.tao@linux.dev>
> 
> LoongArch: KVM: Advertise already-supported capabilities
>     
> This commit advertises KVM_CAP_STEAL_TIME and KVM_CAP_VCPU_ATTRIBUTES on
> LoongArch, making existing features discoverable to userspace.
> 
>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>> index 1317c718f896a..0a51931d6f6e0 100644
>> --- a/arch/loongarch/kvm/vm.c
>> +++ b/arch/loongarch/kvm/vm.c
>> @@ -122,6 +122,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>  	case KVM_CAP_IOEVENTFD:
>>  	case KVM_CAP_MP_STATE:
>>  	case KVM_CAP_SET_GUEST_DEBUG:
>> +	case KVM_CAP_VCPU_ATTRIBUTES:
> 
> [Severity: High]
> This is a pre-existing issue, but does advertising KVM_CAP_VCPU_ATTRIBUTES
> expose a data race in kvm_loongarch_cpucfg_set_attr()?
> 
> Looking at arch/loongarch/kvm/vcpu.c, there is a lockless read-modify-write
> sequence on the shared VM state:
> 
> arch/loongarch/kvm/vcpu.c:kvm_loongarch_cpucfg_set_attr() {
> ...
> 	/* All vCPUs need set the same PV features */
> 	if ((kvm->arch.pv_features & LOONGARCH_PV_FEAT_UPDATED)
> 			&& ((kvm->arch.pv_features & valid) != val))
> 		return -EINVAL;
> 	kvm->arch.pv_features = val | LOONGARCH_PV_FEAT_UPDATED;
> ...
> }
> 
> If multiple threads concurrently call KVM_SET_DEVICE_ATTR on different vCPUs,
> can they bypass this validation check by both reading before either writes,
> resulting in divergent configurations being silently applied globally instead
> of returning -EINVAL?
> 
>>  		r = 1;
>>  		break;
>>  	case KVM_CAP_NR_VCPUS:
> 
> [ ... ]
> 
>> @@ -136,6 +137,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>  	case KVM_CAP_NR_MEMSLOTS:
>>  		r = KVM_USER_MEM_SLOTS;
>>  		break;
>> +	case KVM_CAP_STEAL_TIME:
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does advertising
> KVM_CAP_STEAL_TIME expose an issue with unsafe memory accesses?
> 
> In arch/loongarch/kvm/vcpu.c, unsafe_put_user() is used without the required
> user_access_begin() and user_access_end() guards:
> 
> arch/loongarch/kvm/vcpu.c:kvm_update_stolen_time() {
> ...
> 	unsafe_put_user(0, &st->preempted, out);
> ...
> }
> 
> And similarly in kvm_vcpu_set_pv_preempted():
> 
> arch/loongarch/kvm/vcpu.c:kvm_vcpu_set_pv_preempted() {
> ...
> 	unsafe_put_user(KVM_VCPU_PREEMPTED, &st->preempted, out);
> ...
> }
> 
> Since omitting user_access_begin() skips the access_ok() check at the access
> site, could this cause a kernel fault if hardware-enforced user access
> protections are introduced?
> 
>> +		r = kvm_pvtime_supported();
>> +		break;
>>  	default:
>>  		r = 0;
>>  		break;
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] LoongArch: KVM: Advertise already-supported capabilities
  2026-08-05  5:45 [PATCH v2] LoongArch: KVM: Advertise already-supported capabilities Tao Cui
  2026-08-05  6:06 ` sashiko-bot
@ 2026-08-06 11:50 ` Bibo Mao
  1 sibling, 0 replies; 5+ messages in thread
From: Bibo Mao @ 2026-08-06 11:50 UTC (permalink / raw)
  To: Tao Cui, zhaotianrui; +Cc: chenhuacai, kernel, kvm, loongarch, linux-kernel



On 2026/8/5 下午1:45, Tao Cui wrote:
> Several LoongArch KVM features are fully implemented but were never
> reported through their standard KVM_CHECK_EXTENSION probes, so userspace
> cannot discover them.  Advertise the ones that already work and have a
> real consumer:
> 
>    - KVM_CAP_STEAL_TIME: steal time works end-to-end (host-side record
>      plus guest paravirt side), and the probe returns kvm_pvtime_supported().
> 
>    - KVM_CAP_VCPU_ATTRIBUTES: per-vCPU device-attribute ioctls are
>      implemented (CPUCFG, PVTIME control); advertise the discovery cap.
> 
> This adds no new functionality; it only makes existing features
> discoverable via the standard probe.
> 
> Signed-off-by: Tao Cui <cui.tao@linux.dev>
> ---
> Changes in v2:
>    - Drop KVM_CAP_IRQFD_RESAMPLE: the only consumer is VFIO INTx, which
>      needs IOMMU/VFIO not yet available on LoongArch.
>    - Drop KVM_CAP_VM_GPA_BITS: the cap implies a GPA-width setter that
>      LoongArch does not implement.
> 
>   arch/loongarch/kvm/vm.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
> index 1317c718f896..0a51931d6f6e 100644
> --- a/arch/loongarch/kvm/vm.c
> +++ b/arch/loongarch/kvm/vm.c
> @@ -122,6 +122,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>   	case KVM_CAP_IOEVENTFD:
>   	case KVM_CAP_MP_STATE:
>   	case KVM_CAP_SET_GUEST_DEBUG:
> +	case KVM_CAP_VCPU_ATTRIBUTES:
>   		r = 1;
>   		break;
>   	case KVM_CAP_NR_VCPUS:
> @@ -136,6 +137,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>   	case KVM_CAP_NR_MEMSLOTS:
>   		r = KVM_USER_MEM_SLOTS;
>   		break;
> +	case KVM_CAP_STEAL_TIME:
> +		r = kvm_pvtime_supported();
> +		break;
>   	default:
>   		r = 0;
>   		break;
> 
Reviewed-by: Bibo Mao <maobibo@loongson.cn>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] LoongArch: KVM: Advertise already-supported capabilities
  2026-08-05  8:28   ` Tao Cui
@ 2026-08-06 11:57     ` Bibo Mao
  0 siblings, 0 replies; 5+ messages in thread
From: Bibo Mao @ 2026-08-06 11:57 UTC (permalink / raw)
  To: Tao Cui, sashiko-reviews; +Cc: kvm



On 2026/8/5 下午4:28, Tao Cui wrote:
> 
> Hi Bibo,
> 
> The Sashiko automated review of this patch flagged two pre-existing items
> — neither is from this patch. Wanted to get your thoughts,
> 
> 在 2026/8/5 14:06, sashiko-bot@kernel.org 写道:
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>
>> Pre-existing issues:
>> - [High] The unsafe user-access macros (`unsafe_put_user()` and `unsafe_get_user()`) are used without the required `user_access_begin()` and `user_access_end()` guards, violating the kernel API contract.
> The steal-time code uses unsafe_put_user()/unsafe_get_user() without
>     the usual user_access_begin()/end() around them. On LoongArch it
>     doesn't actually cause any trouble — there's no hardware user-access
>     protection (user_access_begin() just resolves to access_ok()),
>     __put_user is a plain store, and the ghc->hva address is already
>     validated by kvm_gfn_to_hva_cache_init(), so the access_ok() is
>     redundant anyway. So I see it more as a convention inconsistency /
>     style nit than a real bug. I'd be happy to put together a small
>     cleanup wrapping those in user_access_begin()/end() (or switching to
>     kvm_write_guest_cached()). Does that sound like just cleanup to you,
>     and would you want it?
>> - [High] A lockless read-modify-write on the shared VM state `kvm->arch.pv_features` allows userspace to bypass the validation check that requires all vCPUs to share identical PV features.
> The check-then-set on kvm->arch.pv_features in cpucfg_set_attr() is a
>     genuine TOCTOU — again pre-existing, unrelated to this patch. It's not
>     really reachable in practice: 0/3000 with two vCPUs hammering it
>     concurrently, though a debug widening of the window does reproduce it
>     every time (3000/3000). It'd basically need a userspace firing
>     KVM_SET_DEVICE_ATTR with different values on two vCPUs at once. The
>     odds are low, but I do think edge cases like this are still worth
>     fixing — happy to send a separate patch using a mutex or cmpxchg.
Happy to see the patch.

It is design problem to set pv_features per-VM, there is concurrent 
access issue and it can only be set for one time.

Regards
Bibo Mao
> 
> Neither of these affects v2 itself, so I'd leave it as-is.
> 
> Thanks,
> Tao
>> --
>>
>> --- Patch [1]: [PATCH v2] LoongArch: KVM: Advertise already-supported capabilities ---
>> commit 91e3758510b96f271c2589373d1340c246fab45a
>> Author: Tao Cui <cui.tao@linux.dev>
>>
>> LoongArch: KVM: Advertise already-supported capabilities
>>      
>> This commit advertises KVM_CAP_STEAL_TIME and KVM_CAP_VCPU_ATTRIBUTES on
>> LoongArch, making existing features discoverable to userspace.
>>
>>> diff --git a/arch/loongarch/kvm/vm.c b/arch/loongarch/kvm/vm.c
>>> index 1317c718f896a..0a51931d6f6e0 100644
>>> --- a/arch/loongarch/kvm/vm.c
>>> +++ b/arch/loongarch/kvm/vm.c
>>> @@ -122,6 +122,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>>   	case KVM_CAP_IOEVENTFD:
>>>   	case KVM_CAP_MP_STATE:
>>>   	case KVM_CAP_SET_GUEST_DEBUG:
>>> +	case KVM_CAP_VCPU_ATTRIBUTES:
>>
>> [Severity: High]
>> This is a pre-existing issue, but does advertising KVM_CAP_VCPU_ATTRIBUTES
>> expose a data race in kvm_loongarch_cpucfg_set_attr()?
>>
>> Looking at arch/loongarch/kvm/vcpu.c, there is a lockless read-modify-write
>> sequence on the shared VM state:
>>
>> arch/loongarch/kvm/vcpu.c:kvm_loongarch_cpucfg_set_attr() {
>> ...
>> 	/* All vCPUs need set the same PV features */
>> 	if ((kvm->arch.pv_features & LOONGARCH_PV_FEAT_UPDATED)
>> 			&& ((kvm->arch.pv_features & valid) != val))
>> 		return -EINVAL;
>> 	kvm->arch.pv_features = val | LOONGARCH_PV_FEAT_UPDATED;
>> ...
>> }
>>
>> If multiple threads concurrently call KVM_SET_DEVICE_ATTR on different vCPUs,
>> can they bypass this validation check by both reading before either writes,
>> resulting in divergent configurations being silently applied globally instead
>> of returning -EINVAL?
>>
>>>   		r = 1;
>>>   		break;
>>>   	case KVM_CAP_NR_VCPUS:
>>
>> [ ... ]
>>
>>> @@ -136,6 +137,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>>   	case KVM_CAP_NR_MEMSLOTS:
>>>   		r = KVM_USER_MEM_SLOTS;
>>>   		break;
>>> +	case KVM_CAP_STEAL_TIME:
>>
>> [Severity: High]
>> This isn't a bug introduced by this patch, but does advertising
>> KVM_CAP_STEAL_TIME expose an issue with unsafe memory accesses?
>>
>> In arch/loongarch/kvm/vcpu.c, unsafe_put_user() is used without the required
>> user_access_begin() and user_access_end() guards:
>>
>> arch/loongarch/kvm/vcpu.c:kvm_update_stolen_time() {
>> ...
>> 	unsafe_put_user(0, &st->preempted, out);
>> ...
>> }
>>
>> And similarly in kvm_vcpu_set_pv_preempted():
>>
>> arch/loongarch/kvm/vcpu.c:kvm_vcpu_set_pv_preempted() {
>> ...
>> 	unsafe_put_user(KVM_VCPU_PREEMPTED, &st->preempted, out);
>> ...
>> }
>>
>> Since omitting user_access_begin() skips the access_ok() check at the access
>> site, could this cause a kernel fault if hardware-enforced user access
>> protections are introduced?
>>
>>> +		r = kvm_pvtime_supported();
>>> +		break;
>>>   	default:
>>>   		r = 0;
>>>   		break;
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-06 11:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05  5:45 [PATCH v2] LoongArch: KVM: Advertise already-supported capabilities Tao Cui
2026-08-05  6:06 ` sashiko-bot
2026-08-05  8:28   ` Tao Cui
2026-08-06 11:57     ` Bibo Mao
2026-08-06 11:50 ` Bibo Mao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox