linux-kernel - Re: [PATCH] kvm/debugfs: add file to get vcpu steal time statistics

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <bf9914b8-9b3b-413c-b8a2-db8c4486752e@yandex-team.ru>
Date: Tue, 26 Nov 2024 12:54:12 +0300
From: Denis Plotnikov <den-plotnikov@...dex-team.ru>
To: Sean Christopherson <seanjc@...gle.com>
Cc: kvm@...r.kernel.org, pbonzini@...hat.com, yc-core@...dex-team.ru,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kvm/debugfs: add file to get vcpu steal time statistics

Ping!
In the last message I suggested another approach to get steal time 
statistics. It would be nice to get some opinion according to that!
Thanks!

On 11/5/24 15:43, Denis Plotnikov wrote:
> 
>  > On 9/30/24 18:44, Sean Christopherson wrote:
>>>> No, I mean by using the host userspace VMA to read the memory.
>>>
>>> Oh, I think I got your idea. You mean
>>> using KVM_CAP_X86_MSR_FILTER which...
>>>
>>> "In combination with KVM_CAP_X86_USER_SPACE_MSR, this allows user 
>>> space to
>>> trap and emulate MSRs ..."
>>>
>>> And then having guest's steal time struct valid address read the 
>>> value from
>>> userspace VMM like qemu directly.
>>
>> Yep, exactly!
> 
> By the way, what if we add "steal time" as a kvm statistics item?
> 
> Why I think it's a good idea?
> * it is available via standard KVM_GET_STATS_FD
> * it doesn't introduce any overhead
> * it is quite easy to add with just three lines of code
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1596,6 +1596,7 @@ struct kvm_vcpu_stat {
>          u64 preemption_other;
>          u64 guest_mode;
>          u64 notify_window_exits;
> +       u64 steal_time;
>   };
> 
>   struct x86_instruction_info;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 83fe0a78146fc..cd771aef1558a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -291,6 +291,7 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = {
>          STATS_DESC_COUNTER(VCPU, preemption_other),
>          STATS_DESC_IBOOLEAN(VCPU, guest_mode),
>          STATS_DESC_COUNTER(VCPU, notify_window_exits),
> +       STATS_DESC_TIME_NSEC(VCPU, steal_time),
>   };
> 
>   const struct kvm_stats_header kvm_vcpu_stats_header = {
> @@ -3763,6 +3764,7 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
>          version += 1;
>          unsafe_put_user(version, &st->version, out);
> 
> +       vcpu->stat.steal_time = steal;
> 
> The disadvantage of this approach is that it adds some kind of data 
> duplication but it doesn't seem to be a problem - using shadowing and 
> caching are common practices.
> 
> My concern about intercepting steal time MSR in user space is 
> overcomplication - we need to add significant amount of userspace code 
> to achieve what we can get in much easier and, in my opinion, cleaner 
> way. I think it's a cleaner way because every userspace app (like QEMU) 
> will get steal time without any modification via means provided by kvm. 
> For example, QEMU will be able to get steal time via qmp with 
> "query-stats" command which returns every statistics item provided by 
> KVM_GET_STATS_FD.