2019-08-15 16:41:58

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH v2 04/15] kvm: x86: Add per-VM APICv state debugfs

Currently, there is no way to tell whether APICv is active
on a particular VM. This often cause confusion since APICv
can be deactivated at runtime.

Introduce a debugfs entry to report APICv state of a VM.
This creates a read-only file:

/sys/kernel/debug/kvm/70860-14/apicv-state

Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
arch/x86/kvm/debugfs.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/arch/x86/kvm/debugfs.c b/arch/x86/kvm/debugfs.c
index d62852c..bd9fd25 100644
--- a/arch/x86/kvm/debugfs.c
+++ b/arch/x86/kvm/debugfs.c
@@ -48,8 +48,30 @@ static int vcpu_get_tsc_scaling_frac_bits(void *data, u64 *val)

DEFINE_SIMPLE_ATTRIBUTE(vcpu_tsc_scaling_frac_fops, vcpu_get_tsc_scaling_frac_bits, NULL, "%llu\n");

+static int kvm_get_apicv_state(void *data, u64 *val)
+{
+ struct kvm *kvm = (struct kvm *)data;
+
+ mutex_lock(&kvm->arch.apicv_lock);
+ *val = kvm->arch.apicv_state;
+ mutex_unlock(&kvm->arch.apicv_lock);
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(apicv_state_fops, kvm_get_apicv_state, NULL, "%llu\n");
+
int kvm_arch_create_vm_debugfs(struct kvm *kvm)
{
+ struct dentry *ret;
+
+ if (kvm_x86_ops->get_enable_apicv(kvm)) {
+ ret = debugfs_create_file("apicv-state", 0444,
+ kvm->debugfs_dentry,
+ kvm, &apicv_state_fops);
+ if (!ret)
+ return -ENOMEM;
+ }
+
return 0;
}

--
1.8.3.1


2019-08-19 09:58:28

by Alexander Graf

[permalink] [raw]
Subject: Re: [PATCH v2 04/15] kvm: x86: Add per-VM APICv state debugfs



On 15.08.19 18:25, Suthikulpanit, Suravee wrote:
> Currently, there is no way to tell whether APICv is active
> on a particular VM. This often cause confusion since APICv
> can be deactivated at runtime.
>
> Introduce a debugfs entry to report APICv state of a VM.
> This creates a read-only file:
>
> /sys/kernel/debug/kvm/70860-14/apicv-state
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>

Shouldn't this first and foremost be a VM ioctl so that user space can
inquire its own state?


Alex

2019-08-26 21:49:05

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: Re: [PATCH v2 04/15] kvm: x86: Add per-VM APICv state debugfs

Alex,

On 8/19/2019 4:57 AM, Alexander Graf wrote:
>
>
> On 15.08.19 18:25, Suthikulpanit, Suravee wrote:
>> Currently, there is no way to tell whether APICv is active
>> on a particular VM. This often cause confusion since APICv
>> can be deactivated at runtime.
>>
>> Introduce a debugfs entry to report APICv state of a VM.
>> This creates a read-only file:
>>
>>     /sys/kernel/debug/kvm/70860-14/apicv-state
>>
>> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>
> Shouldn't this first and foremost be a VM ioctl so that user space can inquire its own state?
>
>
> Alex

I introduce this mainly for debugging similar to how KVM is currently provides
some per-VCPU information:

/sys/kernel/debug/kvm/15957-14/vcpu0/
lapic_timer_advance_ns
tsc-offset
tsc-scaling-ratio
tsc-scaling-ratio-frac-bits

I'm not sure if this needs to be VM ioctl at this point. If this information is
useful for user-space tool to inquire via ioctl, we can also provide it.

Thanks,
Suravee

2019-08-27 08:22:20

by Alexander Graf

[permalink] [raw]
Subject: Re: [PATCH v2 04/15] kvm: x86: Add per-VM APICv state debugfs



On 26.08.19 21:41, Suthikulpanit, Suravee wrote:
> Alex,
>
> On 8/19/2019 4:57 AM, Alexander Graf wrote:
>>
>>
>> On 15.08.19 18:25, Suthikulpanit, Suravee wrote:
>>> Currently, there is no way to tell whether APICv is active
>>> on a particular VM. This often cause confusion since APICv
>>> can be deactivated at runtime.
>>>
>>> Introduce a debugfs entry to report APICv state of a VM.
>>> This creates a read-only file:
>>>
>>>     /sys/kernel/debug/kvm/70860-14/apicv-state
>>>
>>> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>>
>> Shouldn't this first and foremost be a VM ioctl so that user space can inquire its own state?
>>
>>
>> Alex
>
> I introduce this mainly for debugging similar to how KVM is currently provides
> some per-VCPU information:
>
> /sys/kernel/debug/kvm/15957-14/vcpu0/
> lapic_timer_advance_ns
> tsc-offset
> tsc-scaling-ratio
> tsc-scaling-ratio-frac-bits
>
> I'm not sure if this needs to be VM ioctl at this point. If this information is
> useful for user-space tool to inquire via ioctl, we can also provide it.

I'm mostly thinking of something like "info apic" in QEMU which to me
seems like the natural place for APIC information exposure to a user.
The problem with debugfs is that it's not accessible to the user that
created the VM, but only root, right?

That said, I don't feel very strongly here.


Alex



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


2019-08-28 18:43:10

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: Re: [PATCH v2 04/15] kvm: x86: Add per-VM APICv state debugfs

Alex,

On 8/27/19 3:20 AM, Alexander Graf wrote:
>
>
> On 26.08.19 21:41, Suthikulpanit, Suravee wrote:
>> Alex,
>>
>> On 8/19/2019 4:57 AM, Alexander Graf wrote:
>>>
>>>
>>> On 15.08.19 18:25, Suthikulpanit, Suravee wrote:
>>>> Currently, there is no way to tell whether APICv is active
>>>> on a particular VM. This often cause confusion since APICv
>>>> can be deactivated at runtime.
>>>>
>>>> Introduce a debugfs entry to report APICv state of a VM.
>>>> This creates a read-only file:
>>>>
>>>>      /sys/kernel/debug/kvm/70860-14/apicv-state
>>>>
>>>> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>>>
>>> Shouldn't this first and foremost be a VM ioctl so that user space
>>> can inquire its own state?
>>>
>>>
>>> Alex
>>
>> I introduce this mainly for debugging similar to how KVM is currently
>> provides
>> some per-VCPU information:
>>
>>       /sys/kernel/debug/kvm/15957-14/vcpu0/
>>           lapic_timer_advance_ns
>>           tsc-offset
>>           tsc-scaling-ratio
>>           tsc-scaling-ratio-frac-bits
>>
>> I'm not sure if this needs to be VM ioctl at this point. If this
>> information is
>> useful for user-space tool to inquire via ioctl, we can also provide it.
>
> I'm mostly thinking of something like "info apic" in QEMU which to me
> seems like the natural place for APIC information exposure to a user.

I could not find QEMU "info apic". I assume you meant "info lapic",
which provides information specific to each local APIC (i.e. per-vcpu).

> The problem with debugfs is that it's not accessible to the user that
> created the VM, but only root, right?

Hm, you are right. I'll also look into also adding ioctl interface then.

Thanks,
Suravee

2019-08-28 19:38:17

by Alexander Graf

[permalink] [raw]
Subject: Re: [PATCH v2 04/15] kvm: x86: Add per-VM APICv state debugfs



> Am 28.08.2019 um 20:41 schrieb Suthikulpanit, Suravee <[email protected]>:
>
> Alex,
>
>> On 8/27/19 3:20 AM, Alexander Graf wrote:
>>
>>
>>> On 26.08.19 21:41, Suthikulpanit, Suravee wrote:
>>> Alex,
>>>
>>>> On 8/19/2019 4:57 AM, Alexander Graf wrote:
>>>>
>>>>
>>>>> On 15.08.19 18:25, Suthikulpanit, Suravee wrote:
>>>>> Currently, there is no way to tell whether APICv is active
>>>>> on a particular VM. This often cause confusion since APICv
>>>>> can be deactivated at runtime.
>>>>>
>>>>> Introduce a debugfs entry to report APICv state of a VM.
>>>>> This creates a read-only file:
>>>>>
>>>>> /sys/kernel/debug/kvm/70860-14/apicv-state
>>>>>
>>>>> Signed-off-by: Suravee Suthikulpanit <[email protected]>
>>>>
>>>> Shouldn't this first and foremost be a VM ioctl so that user space
>>>> can inquire its own state?
>>>>
>>>>
>>>> Alex
>>>
>>> I introduce this mainly for debugging similar to how KVM is currently
>>> provides
>>> some per-VCPU information:
>>>
>>> /sys/kernel/debug/kvm/15957-14/vcpu0/
>>> lapic_timer_advance_ns
>>> tsc-offset
>>> tsc-scaling-ratio
>>> tsc-scaling-ratio-frac-bits
>>>
>>> I'm not sure if this needs to be VM ioctl at this point. If this
>>> information is
>>> useful for user-space tool to inquire via ioctl, we can also provide it.
>>
>> I'm mostly thinking of something like "info apic" in QEMU which to me
>> seems like the natural place for APIC information exposure to a user.
>
> I could not find QEMU "info apic". I assume you meant "info lapic",
> which provides information specific to each local APIC (i.e. per-vcpu).

Or maybe even "info kvm" :). I think you get the point.

>
>> The problem with debugfs is that it's not accessible to the user that
>> created the VM, but only root, right?
>
> Hm, you are right. I'll also look into also adding ioctl interface then.

Thanks!

Alex




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879