[PATCH v3 0/2] Fix coverity issues for AST2700

[PATCH v3 0/2] Fix coverity issues for AST2700

[Jamin Lin via]

change from v1:
aspeed/soc: coverity defect: DIVIDE_BY_ZERO
aspeed/sdmc: coverity defect: Control flow issues (DEADCODE)

change from v2:
add more commit log from reviewer suggestion, Cédric.

change from v3:
replace qemu_log_mask with assert dram size 0.

Jamin Lin (2):
  aspeed/soc: Fix possible divide by zero
  aspeed/sdmc: Remove extra R_MAIN_STATUS case

 hw/arm/aspeed_ast27x0.c | 2 ++
 hw/misc/aspeed_sdmc.c   | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v3 1/2] aspeed/soc: Fix possible divide by zero

[Jamin Lin via]

Coverity reports a possible DIVIDE_BY_ZERO issue regarding the
"ram_size" object property. This can not happen because RAM has
predefined valid sizes per SoC. Nevertheless, add a test to
close the issue.

Fixes: Coverity CID 1547113
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
[ clg: Rewrote commit log ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/arm/aspeed_ast27x0.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index b6876b4862..18e6a8b10c 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -211,6 +211,8 @@ static void aspeed_ram_capacity_write(void *opaque, hwaddr addr, uint64_t data,
     ram_size = object_property_get_uint(OBJECT(&s->sdmc), "ram-size",
                                         &error_abort);
 
+    assert(ram_size > 0);
+
     /*
      * Emulate ddr capacity hardware behavior.
      * If writes the data to the address which is beyond the ram size,
-- 
2.25.1

[PATCH v3 1/2] aspeed/soc: Fix possible divide by zero

[Jamin Lin via]

Coverity reports a possible DIVIDE_BY_ZERO issue regarding the
"ram_size" object property. This can not happen because RAM has
predefined valid sizes per SoC. Nevertheless, add a test to
close the issue.

Fixes: Coverity CID 1547113
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
[ clg: Rewrote commit log ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/arm/aspeed_ast27x0.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index b6876b4862..18e6a8b10c 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -211,6 +211,8 @@ static void aspeed_ram_capacity_write(void *opaque, hwaddr addr, uint64_t data,
     ram_size = object_property_get_uint(OBJECT(&s->sdmc), "ram-size",
                                         &error_abort);
 
+    assert(ram_size > 0);
+
     /*
      * Emulate ddr capacity hardware behavior.
      * If writes the data to the address which is beyond the ram size,
-- 
2.25.1

[PATCH v3 2/2] aspeed/sdmc: Remove extra R_MAIN_STATUS case

Coverity reports that the newly added 'case R_MAIN_STATUS' is DEADCODE
because it can not be reached. This is because R_MAIN_STATUS is handled
before in the "Unprotected registers" switch statement. Remove it.

Fixes: Coverity CID 1547112
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
[ clg: Rewrote commit log ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/misc/aspeed_sdmc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index 93e2e29ead..94eed9264d 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -589,7 +589,6 @@ static void aspeed_2700_sdmc_write(AspeedSDMCState *s, uint32_t reg,
     case R_INT_STATUS:
     case R_INT_CLEAR:
     case R_INT_MASK:
-    case R_MAIN_STATUS:
     case R_ERR_STATUS:
     case R_ECC_FAIL_STATUS:
     case R_ECC_FAIL_ADDR:
-- 
2.25.1

[PATCH v3 2/2] aspeed/sdmc: Remove extra R_MAIN_STATUS case

[Jamin Lin via]

Coverity reports that the newly added 'case R_MAIN_STATUS' is DEADCODE
because it can not be reached. This is because R_MAIN_STATUS is handled
before in the "Unprotected registers" switch statement. Remove it.

Fixes: Coverity CID 1547112
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
[ clg: Rewrote commit log ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/misc/aspeed_sdmc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index 93e2e29ead..94eed9264d 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -589,7 +589,6 @@ static void aspeed_2700_sdmc_write(AspeedSDMCState *s, uint32_t reg,
     case R_INT_STATUS:
     case R_INT_CLEAR:
     case R_INT_MASK:
-    case R_MAIN_STATUS:
     case R_ERR_STATUS:
     case R_ECC_FAIL_STATUS:
     case R_ECC_FAIL_ADDR:
-- 
2.25.1

Re: [PATCH v3 0/2] Fix coverity issues for AST2700

[Cédric Le Goater]

Hello Jamin

On 6/25/24 9:07 AM, Jamin Lin wrote:
> change from v1:
> aspeed/soc: coverity defect: DIVIDE_BY_ZERO
> aspeed/sdmc: coverity defect: Control flow issues (DEADCODE)
> 
> change from v2:
> add more commit log from reviewer suggestion, Cédric.
> 
> change from v3:
> replace qemu_log_mask with assert dram size 0.
> 
> Jamin Lin (2):
>    aspeed/soc: Fix possible divide by zero
>    aspeed/sdmc: Remove extra R_MAIN_STATUS case
> 
>   hw/arm/aspeed_ast27x0.c | 2 ++
>   hw/misc/aspeed_sdmc.c   | 1 -
>   2 files changed, 2 insertions(+), 1 deletion(-)
>

LGTM.

Could you please review :

https://lore.kernel.org/qemu-devel/20240625065839.485034-1-clg@redhat.com/T/#u

Thanks,

C.

RE: [PATCH v3 0/2] Fix coverity issues for AST2700

[Jamin Lin]

Hi Cedric, 

> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Tuesday, June 25, 2024 3:22 PM
> To: Jamin Lin <jamin_lin@aspeedtech.com>; Peter Maydell
> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>; Troy
> Lee <leetroy@gmail.com>; Andrew Jeffery <andrew@codeconstruct.com.au>;
> Joel Stanley <joel@jms.id.au>; open list:ASPEED BMCs
> <qemu-arm@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Subject: Re: [PATCH v3 0/2] Fix coverity issues for AST2700
> 
> Hello Jamin
> 
> On 6/25/24 9:07 AM, Jamin Lin wrote:
> > change from v1:
> > aspeed/soc: coverity defect: DIVIDE_BY_ZERO
> > aspeed/sdmc: coverity defect: Control flow issues (DEADCODE)
> >
> > change from v2:
> > add more commit log from reviewer suggestion, Cédric.
> >
> > change from v3:
> > replace qemu_log_mask with assert dram size 0.
> >
> > Jamin Lin (2):
> >    aspeed/soc: Fix possible divide by zero
> >    aspeed/sdmc: Remove extra R_MAIN_STATUS case
> >
> >   hw/arm/aspeed_ast27x0.c | 2 ++
> >   hw/misc/aspeed_sdmc.c   | 1 -
> >   2 files changed, 2 insertions(+), 1 deletion(-)
> >
> 
> LGTM.
> 
> Could you please review :
> 
Done.
Thanks-Jamin
> https://lore.kernel.org/qemu-devel/20240625065839.485034-1-clg@redhat.co
> m/T/#u
> 
> Thanks,
> 
> C.
> 

Re: [PATCH v3 0/2] Fix coverity issues for AST2700

[Cédric Le Goater]

On 6/25/24 9:07 AM, Jamin Lin wrote:
> change from v1:
> aspeed/soc: coverity defect: DIVIDE_BY_ZERO
> aspeed/sdmc: coverity defect: Control flow issues (DEADCODE)
> 
> change from v2:
> add more commit log from reviewer suggestion, Cédric.
> 
> change from v3:
> replace qemu_log_mask with assert dram size 0.
> 
> Jamin Lin (2):
>    aspeed/soc: Fix possible divide by zero
>    aspeed/sdmc: Remove extra R_MAIN_STATUS case
> 
>   hw/arm/aspeed_ast27x0.c | 2 ++
>   hw/misc/aspeed_sdmc.c   | 1 -
>   2 files changed, 2 insertions(+), 1 deletion(-)
> 



Applied to aspeed-next.

Thanks,

C.