i386: miscellaneous cleanup

[PATCH v5 11/11] target/i386/kvm: Replace ARRAY_SIZE(msr_handlers) with KVM_MSR_FILTER_MAX_RANGES

Posted by Zhao Liu 1 year, 3 months ago

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

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) {
-- 
2.34.1

Re: [PATCH v5 11/11] target/i386/kvm: Replace ARRAY_SIZE(msr_handlers) with KVM_MSR_FILTER_MAX_RANGES

Posted by Paolo Bonzini 1 year, 1 month ago

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

Re: [PATCH v5 11/11] target/i386/kvm: Replace ARRAY_SIZE(msr_handlers) with KVM_MSR_FILTER_MAX_RANGES

Posted by Zhao Liu 1 year, 1 month ago

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