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

[Liran Alon <liran.alon@oracle.com>]

> 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
>>> 
>>