Hello

On 6/25/24 3:55 AM, Jamin Lin wrote:
Hi Cedric,
-----Original Message-----
From: Cédric Le Goater <c...@kaod.org>
Sent: Monday, June 24, 2024 9:58 PM
To: Peter Maydell <peter.mayd...@linaro.org>; Jamin Lin
<jamin_...@aspeedtech.com>
Cc: Steven Lee <steven_...@aspeedtech.com>; Troy Lee
<leet...@gmail.com>; Andrew Jeffery <and...@codeconstruct.com.au>; Joel
Stanley <j...@jms.id.au>; open list:ASPEED BMCs <qemu-...@nongnu.org>;
open list:All patches CC here <qemu-devel@nongnu.org>; Troy Lee
<troy_...@aspeedtech.com>; Yunlin Tang <yunlin.t...@aspeedtech.com>
Subject: Re: [PATCH v1 1/2] aspeed/soc: fix coverity issue

On 6/24/24 2:18 PM, Peter Maydell wrote:
On Wed, 19 Jun 2024 at 10:35, Jamin Lin <jamin_...@aspeedtech.com>
wrote:

Fix coverity defect: DIVIDE_BY_ZERO.

Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com>
---
   hw/arm/aspeed_ast27x0.c | 6 ++++++
   1 file changed, 6 insertions(+)

diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c index
b6876b4862..d14a46df6f 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -211,6 +211,12 @@ 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);

+    if (!ram_size) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: ram_size is zero",  __func__);
+        return;
+    }
+

Isn't this a QEMU bug rather than a guest error? The RAM size
presumably should never be zero unless the board set the ram-size
property on the SDMC incorrectly. So the SDMC device should check (and
return an error from its realize
method) that the ram-size property is valid,

That's the case in aspeed_sdmc_set_ram_size() which is called from the
aspeed machine init routine when the ram size is set.

Setting the machine ram size to zero on the command line doesn't report an
error though and the size is the default.

and then here we can just assert(ram_size != 0).

Yes.

Jamin, could you please send a v2 with the commit logs update I proposed ?
See the patches on my aspeed-9.1 branch.
I resend v2 patch with your commit log, 
https://www.mail-archive.com/qemu-devel@nongnu.org/msg1050302.html

We'll need a v3 with the assert. No hurries. We still have some time
before the soft freeze.

Thanks,

C.




Do we need to drop this patch, 
https://www.mail-archive.com/qemu-devel@nongnu.org/msg1050301.html?

Thanks-Jamin

Thanks,

C.


Reply via email to