On 2/23/24 12:21, Sean Christopherson wrote:
> Combine possible_passthrough_msr_slot() and is_valid_passthrough_msr()
> into a single function, vmx_get_passthrough_msr_slot(), and have the
> combined helper return the slot on success, using a negative value to
> indicate "failure".
>
> Combining the operations avoids iterating over the array of passthrough
> MSRs twice for relevant MSRs.
>
> Suggested-by: Dongli Zhang <dongli.zhang@xxxxxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> arch/x86/kvm/vmx/vmx.c | 63 +++++++++++++++++-------------------------
> 1 file changed, 25 insertions(+), 38 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 014cf47dc66b..969fd3aa0da3 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -658,25 +658,14 @@ static inline bool cpu_need_virtualize_apic_accesses(struct kvm_vcpu *vcpu)
> return flexpriority_enabled && lapic_in_kernel(vcpu);
> }
>
> -static int possible_passthrough_msr_slot(u32 msr)
> +static int vmx_get_passthrough_msr_slot(u32 msr)
> {
> - u32 i;
> -
> - for (i = 0; i < ARRAY_SIZE(vmx_possible_passthrough_msrs); i++)
> - if (vmx_possible_passthrough_msrs[i] == msr)
> - return i;
> -
> - return -ENOENT;
> -}
> -
> -static bool is_valid_passthrough_msr(u32 msr)
> -{
> - bool r;
> + int i;
>
> switch (msr) {
> case 0x800 ... 0x8ff:
> /* x2APIC MSRs. These are handled in vmx_update_msr_bitmap_x2apic() */
> - return true;
> + return -ENOENT;
> case MSR_IA32_RTIT_STATUS:
> case MSR_IA32_RTIT_OUTPUT_BASE:
> case MSR_IA32_RTIT_OUTPUT_MASK:
> @@ -691,14 +680,16 @@ static bool is_valid_passthrough_msr(u32 msr)
> case MSR_LBR_CORE_FROM ... MSR_LBR_CORE_FROM + 8:
> case MSR_LBR_CORE_TO ... MSR_LBR_CORE_TO + 8:
> /* LBR MSRs. These are handled in vmx_update_intercept_for_lbr_msrs() */
> - return true;
> + return -ENOENT;
> }
>
> - r = possible_passthrough_msr_slot(msr) != -ENOENT;
> -
> - WARN(!r, "Invalid MSR %x, please adapt vmx_possible_passthrough_msrs[]", msr);
> + for (i = 0; i < ARRAY_SIZE(vmx_possible_passthrough_msrs); i++) {
> + if (vmx_possible_passthrough_msrs[i] == msr)
> + return i;
> + }
>
> - return r;
> + WARN(1, "Invalid MSR %x, please adapt vmx_possible_passthrough_msrs[]", msr);
Reviewed-by: Dongli Zhang <dongli.zhang@xxxxxxxxxx>
Not sure which is better:
WARN(1 ... , or WARN(true ...
Thank you very much!
Dongli Zhang
