| Message ID | 20241106030728.553238-12-zhao1.liu@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | i386: miscellaneous cleanup | expand |
On 11/6/24 04:07, Zhao Liu wrote: > kvm_install_msr_filters() uses KVM_MSR_FILTER_MAX_RANGES as the bound > when traversing msr_handlers[], while other places still compute the > size by ARRAY_SIZE(msr_handlers). > > In fact, msr_handlers[] is an array with the fixed size > KVM_MSR_FILTER_MAX_RANGES, so there is no difference between the two > ways. > > For the code consistency and to avoid additional computational overhead, > use KVM_MSR_FILTER_MAX_RANGES instead of ARRAY_SIZE(msr_handlers). I agree with the consistency but I'd go the other direction. Paolo > Suggested-by: Zide Chen <zide.chen@intel.com> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > Reviewed-by: Zide Chen <zide.chen@intel.com> > --- > v4: new commit. > --- > target/i386/kvm/kvm.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > index 013c0359acbe..501873475255 100644 > --- a/target/i386/kvm/kvm.c > +++ b/target/i386/kvm/kvm.c > @@ -5885,7 +5885,7 @@ static int kvm_filter_msr(KVMState *s, uint32_t msr, QEMURDMSRHandler *rdmsr, > { > int i, ret; > > - for (i = 0; i < ARRAY_SIZE(msr_handlers); i++) { > + for (i = 0; i < KVM_MSR_FILTER_MAX_RANGES; i++) { > if (!msr_handlers[i].msr) { > msr_handlers[i] = (KVMMSRHandlers) { > .msr = msr, > @@ -5911,7 +5911,7 @@ static int kvm_handle_rdmsr(X86CPU *cpu, struct kvm_run *run) > int i; > bool r; > > - for (i = 0; i < ARRAY_SIZE(msr_handlers); i++) { > + for (i = 0; i < KVM_MSR_FILTER_MAX_RANGES; i++) { > KVMMSRHandlers *handler = &msr_handlers[i]; > if (run->msr.index == handler->msr) { > if (handler->rdmsr) { > @@ -5931,7 +5931,7 @@ static int kvm_handle_wrmsr(X86CPU *cpu, struct kvm_run *run) > int i; > bool r; > > - for (i = 0; i < ARRAY_SIZE(msr_handlers); i++) { > + for (i = 0; i < KVM_MSR_FILTER_MAX_RANGES; i++) { > KVMMSRHandlers *handler = &msr_handlers[i]; > if (run->msr.index == handler->msr) { > if (handler->wrmsr) {
On Tue, Dec 24, 2024 at 04:54:41PM +0100, Paolo Bonzini wrote: > Date: Tue, 24 Dec 2024 16:54:41 +0100 > From: Paolo Bonzini <pbonzini@redhat.com> > Subject: Re: [PATCH v5 11/11] target/i386/kvm: Replace > ARRAY_SIZE(msr_handlers) with KVM_MSR_FILTER_MAX_RANGES > > On 11/6/24 04:07, Zhao Liu wrote: > > kvm_install_msr_filters() uses KVM_MSR_FILTER_MAX_RANGES as the bound > > when traversing msr_handlers[], while other places still compute the > > size by ARRAY_SIZE(msr_handlers). > > > > In fact, msr_handlers[] is an array with the fixed size > > KVM_MSR_FILTER_MAX_RANGES, so there is no difference between the two > > ways. > > > > For the code consistency and to avoid additional computational overhead, > > use KVM_MSR_FILTER_MAX_RANGES instead of ARRAY_SIZE(msr_handlers). > > I agree with the consistency but I'd go the other direction. > OK, I'll switch to the other way. Thanks, Zhao
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 013c0359acbe..501873475255 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -5885,7 +5885,7 @@ static int kvm_filter_msr(KVMState *s, uint32_t msr, QEMURDMSRHandler *rdmsr, { int i, ret; - for (i = 0; i < ARRAY_SIZE(msr_handlers); i++) { + for (i = 0; i < KVM_MSR_FILTER_MAX_RANGES; i++) { if (!msr_handlers[i].msr) { msr_handlers[i] = (KVMMSRHandlers) { .msr = msr, @@ -5911,7 +5911,7 @@ static int kvm_handle_rdmsr(X86CPU *cpu, struct kvm_run *run) int i; bool r; - for (i = 0; i < ARRAY_SIZE(msr_handlers); i++) { + for (i = 0; i < KVM_MSR_FILTER_MAX_RANGES; i++) { KVMMSRHandlers *handler = &msr_handlers[i]; if (run->msr.index == handler->msr) { if (handler->rdmsr) { @@ -5931,7 +5931,7 @@ static int kvm_handle_wrmsr(X86CPU *cpu, struct kvm_run *run) int i; bool r; - for (i = 0; i < ARRAY_SIZE(msr_handlers); i++) { + for (i = 0; i < KVM_MSR_FILTER_MAX_RANGES; i++) { KVMMSRHandlers *handler = &msr_handlers[i]; if (run->msr.index == handler->msr) { if (handler->wrmsr) {