diff mbox series

[v2,3/4] kvm: vmx: Rename function find_msr() to vmx_find_msr_index()

Message ID 20191105191910.56505-4-aaronlewis@google.com (mailing list archive)
State New, archived
Headers show
Series Add support for capturing the highest observable L2 TSC | expand

Commit Message

Aaron Lewis Nov. 5, 2019, 7:19 p.m. UTC 
Rename function find_msr() to vmx_find_msr_index() to share
implementations between vmx.c and nested.c in an upcoming change.

Reviewed-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 10 +++++-----
 arch/x86/kvm/vmx/vmx.h |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

--
2.24.0.rc1.363.gb1bccd3e3d-goog

Comments

Liran Alon Nov. 5, 2019, 9:31 p.m. UTC | #1 
> On 5 Nov 2019, at 21:19, Aaron Lewis <aaronlewis@google.com> wrote:
> 
> Rename function find_msr() to vmx_find_msr_index() to share
> implementations between vmx.c and nested.c in an upcoming change.
> 
> Reviewed-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> ---
> arch/x86/kvm/vmx/vmx.c | 10 +++++-----
> arch/x86/kvm/vmx/vmx.h |  1 +
> 2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index c0160ca9ddba..39c701730297 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -835,7 +835,7 @@ static void clear_atomic_switch_msr_special(struct vcpu_vmx *vmx,
> 	vm_exit_controls_clearbit(vmx, exit);
> }
> 
> -static int find_msr(struct vmx_msrs *m, unsigned int msr)
> +int vmx_find_msr_index(struct vmx_msrs *m, u32 msr)

The change from static to non-static should happen in the next patch instead of this rename patch.
Otherwise, if the next patch is reverted, compiling vmx.c will result in a warning.

The rest of the patch looks fine.

-Liran

> {
> 	unsigned int i;
> 
> @@ -869,7 +869,7 @@ static void clear_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr)
> 		}
> 		break;
> 	}
> -	i = find_msr(&m->guest, msr);
> +	i = vmx_find_msr_index(&m->guest, msr);
> 	if (i < 0)
> 		goto skip_guest;
> 	--m->guest.nr;
> @@ -877,7 +877,7 @@ static void clear_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr)
> 	vmcs_write32(VM_ENTRY_MSR_LOAD_COUNT, m->guest.nr);
> 
> skip_guest:
> -	i = find_msr(&m->host, msr);
> +	i = vmx_find_msr_index(&m->host, msr);
> 	if (i < 0)
> 		return;
> 
> @@ -936,9 +936,9 @@ static void add_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr,
> 		wrmsrl(MSR_IA32_PEBS_ENABLE, 0);
> 	}
> 
> -	i = find_msr(&m->guest, msr);
> +	i = vmx_find_msr_index(&m->guest, msr);
> 	if (!entry_only)
> -		j = find_msr(&m->host, msr);
> +		j = vmx_find_msr_index(&m->host, msr);
> 
> 	if ((i < 0 && m->guest.nr == NR_MSR_ENTRIES) ||
> 		(j < 0 &&  m->host.nr == NR_MSR_ENTRIES)) {
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index 0c6835bd6945..34b5fef603d8 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -334,6 +334,7 @@ void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu);
> struct shared_msr_entry *find_msr_entry(struct vcpu_vmx *vmx, u32 msr);
> void pt_update_intercept_for_msr(struct vcpu_vmx *vmx);
> void vmx_update_host_rsp(struct vcpu_vmx *vmx, unsigned long host_rsp);
> +int vmx_find_msr_index(struct vmx_msrs *m, u32 msr);
> 
> #define POSTED_INTR_ON  0
> #define POSTED_INTR_SN  1
> --
> 2.24.0.rc1.363.gb1bccd3e3d-goog
>
Jim Mattson Nov. 7, 2019, 12:11 a.m. UTC | #2 
On Tue, Nov 5, 2019 at 1:31 PM Liran Alon <liran.alon@oracle.com> wrote:
>
>
>
> > On 5 Nov 2019, at 21:19, Aaron Lewis <aaronlewis@google.com> wrote:
> >
> > Rename function find_msr() to vmx_find_msr_index() to share
> > implementations between vmx.c and nested.c in an upcoming change.
> >
> > Reviewed-by: Jim Mattson <jmattson@google.com>
> > Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> > ---
> > arch/x86/kvm/vmx/vmx.c | 10 +++++-----
> > arch/x86/kvm/vmx/vmx.h |  1 +
> > 2 files changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index c0160ca9ddba..39c701730297 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -835,7 +835,7 @@ static void clear_atomic_switch_msr_special(struct vcpu_vmx *vmx,
> >       vm_exit_controls_clearbit(vmx, exit);
> > }
> >
> > -static int find_msr(struct vmx_msrs *m, unsigned int msr)
> > +int vmx_find_msr_index(struct vmx_msrs *m, u32 msr)
>
> The change from static to non-static should happen in the next patch instead of this rename patch.
> Otherwise, if the next patch is reverted, compiling vmx.c will result in a warning.

What warning are you anticipating?

> The rest of the patch looks fine.
>
> -Liran
>
> > {
> >       unsigned int i;
> >
> > @@ -869,7 +869,7 @@ static void clear_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr)
> >               }
> >               break;
> >       }
> > -     i = find_msr(&m->guest, msr);
> > +     i = vmx_find_msr_index(&m->guest, msr);
> >       if (i < 0)
> >               goto skip_guest;
> >       --m->guest.nr;
> > @@ -877,7 +877,7 @@ static void clear_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr)
> >       vmcs_write32(VM_ENTRY_MSR_LOAD_COUNT, m->guest.nr);
> >
> > skip_guest:
> > -     i = find_msr(&m->host, msr);
> > +     i = vmx_find_msr_index(&m->host, msr);
> >       if (i < 0)
> >               return;
> >
> > @@ -936,9 +936,9 @@ static void add_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr,
> >               wrmsrl(MSR_IA32_PEBS_ENABLE, 0);
> >       }
> >
> > -     i = find_msr(&m->guest, msr);
> > +     i = vmx_find_msr_index(&m->guest, msr);
> >       if (!entry_only)
> > -             j = find_msr(&m->host, msr);
> > +             j = vmx_find_msr_index(&m->host, msr);
> >
> >       if ((i < 0 && m->guest.nr == NR_MSR_ENTRIES) ||
> >               (j < 0 &&  m->host.nr == NR_MSR_ENTRIES)) {
> > diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> > index 0c6835bd6945..34b5fef603d8 100644
> > --- a/arch/x86/kvm/vmx/vmx.h
> > +++ b/arch/x86/kvm/vmx/vmx.h
> > @@ -334,6 +334,7 @@ void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu);
> > struct shared_msr_entry *find_msr_entry(struct vcpu_vmx *vmx, u32 msr);
> > void pt_update_intercept_for_msr(struct vcpu_vmx *vmx);
> > void vmx_update_host_rsp(struct vcpu_vmx *vmx, unsigned long host_rsp);
> > +int vmx_find_msr_index(struct vmx_msrs *m, u32 msr);
> >
> > #define POSTED_INTR_ON  0
> > #define POSTED_INTR_SN  1
> > --
> > 2.24.0.rc1.363.gb1bccd3e3d-goog
> >
>
Liran Alon Nov. 7, 2019, 12:35 a.m. UTC | #3 
> On 7 Nov 2019, at 2:11, Jim Mattson <jmattson@google.com> wrote:
> 
> On Tue, Nov 5, 2019 at 1:31 PM Liran Alon <liran.alon@oracle.com> wrote:
>> 
>> 
>> 
>>> On 5 Nov 2019, at 21:19, Aaron Lewis <aaronlewis@google.com> wrote:
>>> 
>>> Rename function find_msr() to vmx_find_msr_index() to share
>>> implementations between vmx.c and nested.c in an upcoming change.
>>> 
>>> Reviewed-by: Jim Mattson <jmattson@google.com>
>>> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
>>> ---
>>> arch/x86/kvm/vmx/vmx.c | 10 +++++-----
>>> arch/x86/kvm/vmx/vmx.h |  1 +
>>> 2 files changed, 6 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>>> index c0160ca9ddba..39c701730297 100644
>>> --- a/arch/x86/kvm/vmx/vmx.c
>>> +++ b/arch/x86/kvm/vmx/vmx.c
>>> @@ -835,7 +835,7 @@ static void clear_atomic_switch_msr_special(struct vcpu_vmx *vmx,
>>>      vm_exit_controls_clearbit(vmx, exit);
>>> }
>>> 
>>> -static int find_msr(struct vmx_msrs *m, unsigned int msr)
>>> +int vmx_find_msr_index(struct vmx_msrs *m, u32 msr)
>> 
>> The change from static to non-static should happen in the next patch instead of this rename patch.
>> Otherwise, if the next patch is reverted, compiling vmx.c will result in a warning.
> 
> What warning are you anticipating?

Sorry right this doesn’t produce a warning.
However, comment still stands that function should change from static to non-static only once it’s needed outside of source file.
Which happens on next patch. Just as a good practice.

> 
>> The rest of the patch looks fine.
>> 
>> -Liran
>> 
>>> {
>>>      unsigned int i;
>>> 
>>> @@ -869,7 +869,7 @@ static void clear_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr)
>>>              }
>>>              break;
>>>      }
>>> -     i = find_msr(&m->guest, msr);
>>> +     i = vmx_find_msr_index(&m->guest, msr);
>>>      if (i < 0)
>>>              goto skip_guest;
>>>      --m->guest.nr;
>>> @@ -877,7 +877,7 @@ static void clear_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr)
>>>      vmcs_write32(VM_ENTRY_MSR_LOAD_COUNT, m->guest.nr);
>>> 
>>> skip_guest:
>>> -     i = find_msr(&m->host, msr);
>>> +     i = vmx_find_msr_index(&m->host, msr);
>>>      if (i < 0)
>>>              return;
>>> 
>>> @@ -936,9 +936,9 @@ static void add_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr,
>>>              wrmsrl(MSR_IA32_PEBS_ENABLE, 0);
>>>      }
>>> 
>>> -     i = find_msr(&m->guest, msr);
>>> +     i = vmx_find_msr_index(&m->guest, msr);
>>>      if (!entry_only)
>>> -             j = find_msr(&m->host, msr);
>>> +             j = vmx_find_msr_index(&m->host, msr);
>>> 
>>>      if ((i < 0 && m->guest.nr == NR_MSR_ENTRIES) ||
>>>              (j < 0 &&  m->host.nr == NR_MSR_ENTRIES)) {
>>> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
>>> index 0c6835bd6945..34b5fef603d8 100644
>>> --- a/arch/x86/kvm/vmx/vmx.h
>>> +++ b/arch/x86/kvm/vmx/vmx.h
>>> @@ -334,6 +334,7 @@ void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu);
>>> struct shared_msr_entry *find_msr_entry(struct vcpu_vmx *vmx, u32 msr);
>>> void pt_update_intercept_for_msr(struct vcpu_vmx *vmx);
>>> void vmx_update_host_rsp(struct vcpu_vmx *vmx, unsigned long host_rsp);
>>> +int vmx_find_msr_index(struct vmx_msrs *m, u32 msr);
>>> 
>>> #define POSTED_INTR_ON  0
>>> #define POSTED_INTR_SN  1
>>> --
>>> 2.24.0.rc1.363.gb1bccd3e3d-goog
>>> 
>>
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index c0160ca9ddba..39c701730297 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -835,7 +835,7 @@  static void clear_atomic_switch_msr_special(struct vcpu_vmx *vmx,
 	vm_exit_controls_clearbit(vmx, exit);
 }

-static int find_msr(struct vmx_msrs *m, unsigned int msr)
+int vmx_find_msr_index(struct vmx_msrs *m, u32 msr)
 {
 	unsigned int i;

@@ -869,7 +869,7 @@  static void clear_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr)
 		}
 		break;
 	}
-	i = find_msr(&m->guest, msr);
+	i = vmx_find_msr_index(&m->guest, msr);
 	if (i < 0)
 		goto skip_guest;
 	--m->guest.nr;
@@ -877,7 +877,7 @@  static void clear_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr)
 	vmcs_write32(VM_ENTRY_MSR_LOAD_COUNT, m->guest.nr);

 skip_guest:
-	i = find_msr(&m->host, msr);
+	i = vmx_find_msr_index(&m->host, msr);
 	if (i < 0)
 		return;

@@ -936,9 +936,9 @@  static void add_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr,
 		wrmsrl(MSR_IA32_PEBS_ENABLE, 0);
 	}

-	i = find_msr(&m->guest, msr);
+	i = vmx_find_msr_index(&m->guest, msr);
 	if (!entry_only)
-		j = find_msr(&m->host, msr);
+		j = vmx_find_msr_index(&m->host, msr);

 	if ((i < 0 && m->guest.nr == NR_MSR_ENTRIES) ||
 		(j < 0 &&  m->host.nr == NR_MSR_ENTRIES)) {
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 0c6835bd6945..34b5fef603d8 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -334,6 +334,7 @@  void vmx_set_virtual_apic_mode(struct kvm_vcpu *vcpu);
 struct shared_msr_entry *find_msr_entry(struct vcpu_vmx *vmx, u32 msr);
 void pt_update_intercept_for_msr(struct vcpu_vmx *vmx);
 void vmx_update_host_rsp(struct vcpu_vmx *vmx, unsigned long host_rsp);
+int vmx_find_msr_index(struct vmx_msrs *m, u32 msr);

 #define POSTED_INTR_ON  0
 #define POSTED_INTR_SN  1