2020-03-02 19:58:10

by Sean Christopherson

Subject: [PATCH 5/6] KVM: x86: Rename "found" variable in kvm_cpuid() to "exact_entry_exists"

Rename "found" in kvm_cpuid() to "exact_entry_exists" to better convey
that the intent of the tracepoint's "found/not found" output is to trace
whether the output values are for the actual requested leaf or for some
other (likely unrelated) leaf that was found while processing entries to
emulate funky CPU behavior, e.g. the max basic leaf on Intel CPUs when
the requested CPUID leaf is out of range.

Suggested-by: Jan Kiszka <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/cpuid.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 869526930cf7..b0a4f3c17932 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1002,10 +1002,10 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
{
const u32 function = *eax, index = *ecx;
struct kvm_cpuid_entry2 *entry;
- bool found;
+ bool exact_entry_exists;

entry = kvm_find_cpuid_entry(vcpu, function, index);
- found = entry;
+ exact_entry_exists = !!entry;
/*
* Intel CPUID semantics treats any query for an out-of-range
* leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were
@@ -1047,7 +1047,7 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
}
}
}
- trace_kvm_cpuid(function, *eax, *ebx, *ecx, *edx, found);
+ trace_kvm_cpuid(function, *eax, *ebx, *ecx, *edx, exact_entry_exists);
}
EXPORT_SYMBOL_GPL(kvm_cpuid);

--
2.24.1

2020-03-02 20:21:58

by Jan Kiszka

Subject: Re: [PATCH 5/6] KVM: x86: Rename "found" variable in kvm_cpuid() to "exact_entry_exists"

On 02.03.20 20:57, Sean Christopherson wrote:
> Rename "found" in kvm_cpuid() to "exact_entry_exists" to better convey
> that the intent of the tracepoint's "found/not found" output is to trace
> whether the output values are for the actual requested leaf or for some
> other (likely unrelated) leaf that was found while processing entries to
> emulate funky CPU behavior, e.g. the max basic leaf on Intel CPUs when
> the requested CPUID leaf is out of range.
>
> Suggested-by: Jan Kiszka <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> arch/x86/kvm/cpuid.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 869526930cf7..b0a4f3c17932 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -1002,10 +1002,10 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
> {
> const u32 function = *eax, index = *ecx;
> struct kvm_cpuid_entry2 *entry;
> - bool found;
> + bool exact_entry_exists;
>
> entry = kvm_find_cpuid_entry(vcpu, function, index);
> - found = entry;
> + exact_entry_exists = !!entry;
> /*
> * Intel CPUID semantics treats any query for an out-of-range
> * leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were
> @@ -1047,7 +1047,7 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
> }
> }
> }
> - trace_kvm_cpuid(function, *eax, *ebx, *ecx, *edx, found);
> + trace_kvm_cpuid(function, *eax, *ebx, *ecx, *edx, exact_entry_exists);

Actually, I think we also what to change output in the tracepoint.

Jan

> }
> EXPORT_SYMBOL_GPL(kvm_cpuid);
>
>

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

2020-03-02 20:35:29

by Sean Christopherson

Subject: Re: [PATCH 5/6] KVM: x86: Rename "found" variable in kvm_cpuid() to "exact_entry_exists"

On Mon, Mar 02, 2020 at 09:20:52PM +0100, Jan Kiszka wrote:
> On 02.03.20 20:57, Sean Christopherson wrote:
> >Rename "found" in kvm_cpuid() to "exact_entry_exists" to better convey
> >that the intent of the tracepoint's "found/not found" output is to trace
> >whether the output values are for the actual requested leaf or for some
> >other (likely unrelated) leaf that was found while processing entries to
> >emulate funky CPU behavior, e.g. the max basic leaf on Intel CPUs when
> >the requested CPUID leaf is out of range.
> >
> >Suggested-by: Jan Kiszka <[email protected]>
> >Signed-off-by: Sean Christopherson <[email protected]>
> >---
> > arch/x86/kvm/cpuid.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> >diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> >index 869526930cf7..b0a4f3c17932 100644
> >--- a/arch/x86/kvm/cpuid.c
> >+++ b/arch/x86/kvm/cpuid.c
> >@@ -1002,10 +1002,10 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
> > {
> > const u32 function = *eax, index = *ecx;
> > struct kvm_cpuid_entry2 *entry;
> >- bool found;
> >+ bool exact_entry_exists;
> > entry = kvm_find_cpuid_entry(vcpu, function, index);
> >- found = entry;
> >+ exact_entry_exists = !!entry;
> > /*
> > * Intel CPUID semantics treats any query for an out-of-range
> > * leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were
> >@@ -1047,7 +1047,7 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
> > }
> > }
> > }
> >- trace_kvm_cpuid(function, *eax, *ebx, *ecx, *edx, found);
> >+ trace_kvm_cpuid(function, *eax, *ebx, *ecx, *edx, exact_entry_exists);
>
> Actually, I think we also what to change output in the tracepoint.

Oh, I definitely want to change it, but AIUI it's ABI and shouldn't be
changed. Paolo?

2020-03-02 20:50:09

by Jan Kiszka

Subject: Re: [PATCH 5/6] KVM: x86: Rename "found" variable in kvm_cpuid() to "exact_entry_exists"

On 02.03.20 21:35, Sean Christopherson wrote:
> On Mon, Mar 02, 2020 at 09:20:52PM +0100, Jan Kiszka wrote:
>> On 02.03.20 20:57, Sean Christopherson wrote:
>>> Rename "found" in kvm_cpuid() to "exact_entry_exists" to better convey
>>> that the intent of the tracepoint's "found/not found" output is to trace
>>> whether the output values are for the actual requested leaf or for some
>>> other (likely unrelated) leaf that was found while processing entries to
>>> emulate funky CPU behavior, e.g. the max basic leaf on Intel CPUs when
>>> the requested CPUID leaf is out of range.
>>>
>>> Suggested-by: Jan Kiszka <[email protected]>
>>> Signed-off-by: Sean Christopherson <[email protected]>
>>> ---
>>> arch/x86/kvm/cpuid.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>>> index 869526930cf7..b0a4f3c17932 100644
>>> --- a/arch/x86/kvm/cpuid.c
>>> +++ b/arch/x86/kvm/cpuid.c
>>> @@ -1002,10 +1002,10 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
>>> {
>>> const u32 function = *eax, index = *ecx;
>>> struct kvm_cpuid_entry2 *entry;
>>> - bool found;
>>> + bool exact_entry_exists;
>>> entry = kvm_find_cpuid_entry(vcpu, function, index);
>>> - found = entry;
>>> + exact_entry_exists = !!entry;
>>> /*
>>> * Intel CPUID semantics treats any query for an out-of-range
>>> * leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were
>>> @@ -1047,7 +1047,7 @@ void kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
>>> }
>>> }
>>> }
>>> - trace_kvm_cpuid(function, *eax, *ebx, *ecx, *edx, found);
>>> + trace_kvm_cpuid(function, *eax, *ebx, *ecx, *edx, exact_entry_exists);
>>
>> Actually, I think we also what to change output in the tracepoint.
>
> Oh, I definitely want to change it, but AIUI it's ABI and shouldn't be
> changed. Paolo?
>

This can be discovered via the format string in sysfs and handled by the
consumer. But I'm not an expert in the tracing ABI.

Jan

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux