mips: Minor simplifications for KVM use

[PATCH 1/2] hw/mips/mips_int: De-duplicate KVM interrupt delivery

Posted by Philippe Mathieu-Daudé 5 years, 9 months ago

Refactor duplicated code in a single place.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/mips_int.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
index 796730b11d..4a1bf846da 100644
--- a/hw/mips/mips_int.c
+++ b/hw/mips/mips_int.c
@@ -47,17 +47,12 @@ static void cpu_mips_irq_request(void *opaque, int irq, int level)
 
     if (level) {
         env->CP0_Cause |= 1 << (irq + CP0Ca_IP);
-
-        if (kvm_enabled() && irq == 2) {
-            kvm_mips_set_interrupt(cpu, irq, level);
-        }
-
     } else {
         env->CP0_Cause &= ~(1 << (irq + CP0Ca_IP));
+    }
 
-        if (kvm_enabled() && irq == 2) {
-            kvm_mips_set_interrupt(cpu, irq, level);
-        }
+    if (kvm_enabled() && irq == 2) {
+        kvm_mips_set_interrupt(cpu, irq, level);
     }
 
     if (env->CP0_Cause & CP0Ca_IP_mask) {
-- 
2.21.1

Re: [PATCH 1/2] hw/mips/mips_int: De-duplicate KVM interrupt delivery

Posted by chen huacai 5 years, 9 months ago

Hi, Philippe,

On Wed, Apr 29, 2020 at 4:30 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Refactor duplicated code in a single place.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/mips/mips_int.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
> index 796730b11d..4a1bf846da 100644
> --- a/hw/mips/mips_int.c
> +++ b/hw/mips/mips_int.c
> @@ -47,17 +47,12 @@ static void cpu_mips_irq_request(void *opaque, int irq, int level)
>
>      if (level) {
>          env->CP0_Cause |= 1 << (irq + CP0Ca_IP);
> -
> -        if (kvm_enabled() && irq == 2) {
> -            kvm_mips_set_interrupt(cpu, irq, level);
> -        }
> -
>      } else {
>          env->CP0_Cause &= ~(1 << (irq + CP0Ca_IP));
> +    }
Since the if-else has become one line, so can we remove { and } here?

>
> -        if (kvm_enabled() && irq == 2) {
> -            kvm_mips_set_interrupt(cpu, irq, level);
> -        }
> +    if (kvm_enabled() && irq == 2) {
> +        kvm_mips_set_interrupt(cpu, irq, level);
>      }
>
>      if (env->CP0_Cause & CP0Ca_IP_mask) {
> --
> 2.21.1
>
>


-- 
Huacai Chen

Re: [PATCH 1/2] hw/mips/mips_int: De-duplicate KVM interrupt delivery

Posted by Philippe Mathieu-Daudé 5 years, 9 months ago

On 4/29/20 10:48 AM, chen huacai wrote:
> Hi, Philippe,
> 
> On Wed, Apr 29, 2020 at 4:30 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> Refactor duplicated code in a single place.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   hw/mips/mips_int.c | 11 +++--------
>>   1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
>> index 796730b11d..4a1bf846da 100644
>> --- a/hw/mips/mips_int.c
>> +++ b/hw/mips/mips_int.c
>> @@ -47,17 +47,12 @@ static void cpu_mips_irq_request(void *opaque, int irq, int level)
>>
>>       if (level) {
>>           env->CP0_Cause |= 1 << (irq + CP0Ca_IP);
>> -
>> -        if (kvm_enabled() && irq == 2) {
>> -            kvm_mips_set_interrupt(cpu, irq, level);
>> -        }
>> -
>>       } else {
>>           env->CP0_Cause &= ~(1 << (irq + CP0Ca_IP));
>> +    }
> Since the if-else has become one line, so can we remove { and } here?

This is the QEMU coding style, see CODING_STYLE.rst:

Block structure
===============

Every indented statement is braced; even if the block contains just one
statement.  The opening brace is on the line that contains the control
flow statement that introduces the new block; the closing brace is on the
same line as the else keyword, or on a line by itself if there is no else
keyword.  Example:

.. code-block:: c

     if (a == 5) {
         printf("a was 5.\n");
     } else if (a == 6) {
         printf("a was 6.\n");
     } else {
         printf("a was something else entirely.\n");
     }

Rationale: a consistent (except for functions...) bracing style reduces
ambiguity and avoids needless churn when lines are added or removed.
Furthermore, it is the QEMU coding style.

> 
>>
>> -        if (kvm_enabled() && irq == 2) {
>> -            kvm_mips_set_interrupt(cpu, irq, level);
>> -        }
>> +    if (kvm_enabled() && irq == 2) {
>> +        kvm_mips_set_interrupt(cpu, irq, level);
>>       }
>>
>>       if (env->CP0_Cause & CP0Ca_IP_mask) {
>> --
>> 2.21.1
>>
>>
> 
>

Re: [PATCH 1/2] hw/mips/mips_int: De-duplicate KVM interrupt delivery

Posted by Thomas Huth 5 years, 8 months ago

On 12/05/2020 09.08, Philippe Mathieu-Daudé wrote:
> On 4/29/20 10:48 AM, chen huacai wrote:
>> Hi, Philippe,
>>
>> On Wed, Apr 29, 2020 at 4:30 PM Philippe Mathieu-Daudé
>> <f4bug@amsat.org> wrote:
>>>
>>> Refactor duplicated code in a single place.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>   hw/mips/mips_int.c | 11 +++--------
>>>   1 file changed, 3 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
>>> index 796730b11d..4a1bf846da 100644
>>> --- a/hw/mips/mips_int.c
>>> +++ b/hw/mips/mips_int.c
>>> @@ -47,17 +47,12 @@ static void cpu_mips_irq_request(void *opaque,
>>> int irq, int level)
>>>
>>>       if (level) {
>>>           env->CP0_Cause |= 1 << (irq + CP0Ca_IP);
>>> -
>>> -        if (kvm_enabled() && irq == 2) {
>>> -            kvm_mips_set_interrupt(cpu, irq, level);
>>> -        }
>>> -
>>>       } else {
>>>           env->CP0_Cause &= ~(1 << (irq + CP0Ca_IP));
>>> +    }
>>>
>>> -        if (kvm_enabled() && irq == 2) {
>>> -            kvm_mips_set_interrupt(cpu, irq, level);
>>> -        }
>>> +    if (kvm_enabled() && irq == 2) {
>>> +        kvm_mips_set_interrupt(cpu, irq, level);
>>>       }
>>>
>>>       if (env->CP0_Cause & CP0Ca_IP_mask) {
>>> -- 
>>> 2.21.1

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 1/2] hw/mips/mips_int: De-duplicate KVM interrupt delivery

Posted by Philippe Mathieu-Daudé 5 years, 8 months ago

On 5/26/20 9:57 AM, Thomas Huth wrote:
> On 12/05/2020 09.08, Philippe Mathieu-Daudé wrote:
>> On 4/29/20 10:48 AM, chen huacai wrote:
>>> Hi, Philippe,
>>>
>>> On Wed, Apr 29, 2020 at 4:30 PM Philippe Mathieu-Daudé
>>> <f4bug@amsat.org> wrote:
>>>>
>>>> Refactor duplicated code in a single place.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>>   hw/mips/mips_int.c | 11 +++--------
>>>>   1 file changed, 3 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
>>>> index 796730b11d..4a1bf846da 100644
>>>> --- a/hw/mips/mips_int.c
>>>> +++ b/hw/mips/mips_int.c
>>>> @@ -47,17 +47,12 @@ static void cpu_mips_irq_request(void *opaque,
>>>> int irq, int level)
>>>>
>>>>       if (level) {
>>>>           env->CP0_Cause |= 1 << (irq + CP0Ca_IP);
>>>> -
>>>> -        if (kvm_enabled() && irq == 2) {
>>>> -            kvm_mips_set_interrupt(cpu, irq, level);
>>>> -        }
>>>> -
>>>>       } else {
>>>>           env->CP0_Cause &= ~(1 << (irq + CP0Ca_IP));
>>>> +    }
>>>>
>>>> -        if (kvm_enabled() && irq == 2) {
>>>> -            kvm_mips_set_interrupt(cpu, irq, level);
>>>> -        }
>>>> +    if (kvm_enabled() && irq == 2) {
>>>> +        kvm_mips_set_interrupt(cpu, irq, level);
>>>>       }
>>>>
>>>>       if (env->CP0_Cause & CP0Ca_IP_mask) {
>>>> -- 
>>>> 2.21.1
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks, queued to mips-next.