diff mbox series

hw/intc/loongson_liointc: Fix per core ISR handling

Message ID 20210112012527.28927-1-jiaxun.yang@flygoat.com (mailing list archive)
State New, archived
Headers show
Series hw/intc/loongson_liointc: Fix per core ISR handling | expand

Commit Message

Jiaxun Yang Jan. 12, 2021, 1:25 a.m. UTC
Per core ISR is a set of 32-bit registers spaced by 8 bytes.
This patch fixed calculation of it's size and also added check
of alignment at reading & writing.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 hw/intc/loongson_liointc.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Huacai Chen Jan. 15, 2021, 2:48 a.m. UTC | #1
Reviewed-by: Huacai Chen <chenhuacai@kernel.org>

On Tue, Jan 12, 2021 at 9:25 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
> Per core ISR is a set of 32-bit registers spaced by 8 bytes.
> This patch fixed calculation of it's size and also added check
> of alignment at reading & writing.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  hw/intc/loongson_liointc.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/loongson_liointc.c b/hw/intc/loongson_liointc.c
> index f823d484e0..cc11b544cb 100644
> --- a/hw/intc/loongson_liointc.c
> +++ b/hw/intc/loongson_liointc.c
> @@ -41,7 +41,7 @@
>  #define R_IEN_CLR               0x2c
>  #define R_ISR_SIZE              0x8
>  #define R_START                 0x40
> -#define R_END                   0x64
> +#define R_END                   (R_START + R_ISR_SIZE * NUM_CORES)
>
>  struct loongson_liointc {
>      SysBusDevice parent_obj;
> @@ -125,7 +125,12 @@ liointc_read(void *opaque, hwaddr addr, unsigned int size)
>      }
>
>      if (addr >= R_START && addr < R_END) {
> -        int core = (addr - R_START) / R_ISR_SIZE;
> +        hwaddr offset = addr - R_START;
> +        int core = offset / R_ISR_SIZE;
> +
> +        if (offset % R_ISR_SIZE) {
> +            goto out;
> +        }
>          r = p->per_core_isr[core];
>          goto out;
>      }
> @@ -169,7 +174,12 @@ liointc_write(void *opaque, hwaddr addr,
>      }
>
>      if (addr >= R_START && addr < R_END) {
> -        int core = (addr - R_START) / R_ISR_SIZE;
> +        hwaddr offset = addr - R_START;
> +        int core = offset / R_ISR_SIZE;
> +
> +        if (offset % R_ISR_SIZE) {
> +            goto out;
> +        }
>          p->per_core_isr[core] = value;
>          goto out;
>      }
> --
> 2.30.0
>
Peter Maydell Feb. 19, 2021, 10:35 p.m. UTC | #2
This patch has been reviewed and fixes a Coverity issue;
Philippe, are you planning to take it through your MIPS tree?

-- PMM

On Tue, 12 Jan 2021 at 01:28, Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
> Per core ISR is a set of 32-bit registers spaced by 8 bytes.
> This patch fixed calculation of it's size and also added check
> of alignment at reading & writing.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  hw/intc/loongson_liointc.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/hw/intc/loongson_liointc.c b/hw/intc/loongson_liointc.c
> index f823d484e0..cc11b544cb 100644
> --- a/hw/intc/loongson_liointc.c
> +++ b/hw/intc/loongson_liointc.c
> @@ -41,7 +41,7 @@
>  #define R_IEN_CLR               0x2c
>  #define R_ISR_SIZE              0x8
>  #define R_START                 0x40
> -#define R_END                   0x64
> +#define R_END                   (R_START + R_ISR_SIZE * NUM_CORES)
>
>  struct loongson_liointc {
>      SysBusDevice parent_obj;
> @@ -125,7 +125,12 @@ liointc_read(void *opaque, hwaddr addr, unsigned int size)
>      }
>
>      if (addr >= R_START && addr < R_END) {
> -        int core = (addr - R_START) / R_ISR_SIZE;
> +        hwaddr offset = addr - R_START;
> +        int core = offset / R_ISR_SIZE;
> +
> +        if (offset % R_ISR_SIZE) {
> +            goto out;
> +        }
>          r = p->per_core_isr[core];
>          goto out;
>      }
> @@ -169,7 +174,12 @@ liointc_write(void *opaque, hwaddr addr,
>      }
>
>      if (addr >= R_START && addr < R_END) {
> -        int core = (addr - R_START) / R_ISR_SIZE;
> +        hwaddr offset = addr - R_START;
> +        int core = offset / R_ISR_SIZE;
> +
> +        if (offset % R_ISR_SIZE) {
> +            goto out;
> +        }
>          p->per_core_isr[core] = value;
>          goto out;
>      }
> --
> 2.30.0
>
>
Philippe Mathieu-Daudé Feb. 19, 2021, 11:08 p.m. UTC | #3
On 2/19/21 11:35 PM, Peter Maydell wrote:
> This patch has been reviewed and fixes a Coverity issue;
> Philippe, are you planning to take it through your MIPS tree?

Sorry felt through the crack, now applied to mips-next (I'll send
a pull request next week).

Thanks!

> 
> -- PMM
> 
> On Tue, 12 Jan 2021 at 01:28, Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>>
>> Per core ISR is a set of 32-bit registers spaced by 8 bytes.
>> This patch fixed calculation of it's size and also added check
>> of alignment at reading & writing.
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>  hw/intc/loongson_liointc.c | 16 +++++++++++++---
>>  1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/intc/loongson_liointc.c b/hw/intc/loongson_liointc.c
>> index f823d484e0..cc11b544cb 100644
>> --- a/hw/intc/loongson_liointc.c
>> +++ b/hw/intc/loongson_liointc.c
>> @@ -41,7 +41,7 @@
>>  #define R_IEN_CLR               0x2c
>>  #define R_ISR_SIZE              0x8
>>  #define R_START                 0x40
>> -#define R_END                   0x64
>> +#define R_END                   (R_START + R_ISR_SIZE * NUM_CORES)
>>
>>  struct loongson_liointc {
>>      SysBusDevice parent_obj;
>> @@ -125,7 +125,12 @@ liointc_read(void *opaque, hwaddr addr, unsigned int size)
>>      }
>>
>>      if (addr >= R_START && addr < R_END) {
>> -        int core = (addr - R_START) / R_ISR_SIZE;
>> +        hwaddr offset = addr - R_START;
>> +        int core = offset / R_ISR_SIZE;
>> +
>> +        if (offset % R_ISR_SIZE) {
>> +            goto out;
>> +        }
>>          r = p->per_core_isr[core];
>>          goto out;
>>      }
>> @@ -169,7 +174,12 @@ liointc_write(void *opaque, hwaddr addr,
>>      }
>>
>>      if (addr >= R_START && addr < R_END) {
>> -        int core = (addr - R_START) / R_ISR_SIZE;
>> +        hwaddr offset = addr - R_START;
>> +        int core = offset / R_ISR_SIZE;
>> +
>> +        if (offset % R_ISR_SIZE) {
>> +            goto out;
>> +        }
>>          p->per_core_isr[core] = value;
>>          goto out;
>>      }
>> --
>> 2.30.0
>>
>>
>
diff mbox series

Patch

diff --git a/hw/intc/loongson_liointc.c b/hw/intc/loongson_liointc.c
index f823d484e0..cc11b544cb 100644
--- a/hw/intc/loongson_liointc.c
+++ b/hw/intc/loongson_liointc.c
@@ -41,7 +41,7 @@ 
 #define R_IEN_CLR               0x2c
 #define R_ISR_SIZE              0x8
 #define R_START                 0x40
-#define R_END                   0x64
+#define R_END                   (R_START + R_ISR_SIZE * NUM_CORES)
 
 struct loongson_liointc {
     SysBusDevice parent_obj;
@@ -125,7 +125,12 @@  liointc_read(void *opaque, hwaddr addr, unsigned int size)
     }
 
     if (addr >= R_START && addr < R_END) {
-        int core = (addr - R_START) / R_ISR_SIZE;
+        hwaddr offset = addr - R_START;
+        int core = offset / R_ISR_SIZE;
+
+        if (offset % R_ISR_SIZE) {
+            goto out;
+        }
         r = p->per_core_isr[core];
         goto out;
     }
@@ -169,7 +174,12 @@  liointc_write(void *opaque, hwaddr addr,
     }
 
     if (addr >= R_START && addr < R_END) {
-        int core = (addr - R_START) / R_ISR_SIZE;
+        hwaddr offset = addr - R_START;
+        int core = offset / R_ISR_SIZE;
+
+        if (offset % R_ISR_SIZE) {
+            goto out;
+        }
         p->per_core_isr[core] = value;
         goto out;
     }