Re: [PATCH] hw/intc/loongson_liointc: Fix per core ISR handling

[Philippe Mathieu-Daudé]

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

Re: [PATCH] hw/intc/loongson_liointc: Fix per core ISR handling

[Peter Maydell]

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

Re: [PATCH] hw/intc/loongson_liointc: Fix per core ISR handling

[Huacai Chen]

Reviewed-by: Huacai Chen 

On Tue, Jan 12, 2021 at 9:25 AM Jiaxun Yang  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 
> ---
>  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
>

[PATCH] hw/intc/loongson_liointc: Fix per core ISR handling

[Jiaxun Yang]

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