Hi Cedric,
> -----Original Message-----
> From: Cédric Le Goater <c...@kaod.org>
> Sent: Tuesday, June 25, 2024 2:38 PM
> To: Jamin Lin <jamin_...@aspeedtech.com>; cmd
> <clement.mathieudrif....@gmail.com>; Peter Maydell
> <peter.mayd...@linaro.org>; 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>
> Cc: Cédric Le Goater <c...@redhat.com>
> Subject: Re: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero
>
> On 6/25/24 8:15 AM, Jamin Lin wrote:
> > Hi cmd, Cedric and Peter,
> >
> >> -----Original Message-----
> >> From: cmd <clement.mathieudrif....@gmail.com>
> >> Sent: Tuesday, June 25, 2024 2:07 PM
> >> To: Cédric Le Goater <c...@kaod.org>; Jamin Lin
> >> <jamin_...@aspeedtech.com>; Peter Maydell <peter.mayd...@linaro.org>;
> >> 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>
> >> Cc: Cédric Le Goater <c...@redhat.com>
> >> Subject: Re: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero
> >>
> >>
> >> On 25/06/2024 08:03, Cédric Le Goater wrote:
> >>> On 6/25/24 8:00 AM, cmd wrote:
> >>>> Hi
> >>>>
> >>>> On 25/06/2024 03:50, Jamin Lin via wrote:
> >>>>> 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_...@aspeedtech.com>
> >>>>> Reviewed-by: Cédric Le Goater <c...@redhat.com> [ clg: Rewrote
> >>>>> commit log ]
> >>>>> Signed-off-by: Cédric Le Goater <c...@redhat.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",
> >>>>> &erro
> r_a
> >> bort);
> >>>>> + if (!ram_size) {
> >>>>> + qemu_log_mask(LOG_GUEST_ERROR,
> >>>>> + "%s: ram_size is zero", __func__);
> >>>>> + return;
> >>>>> + }
> >>>>> +
> >>>> If we are sure that the error cannot happen, shouldn't we assert
> >>>> instead?
> >>>
> >>> Yes. That is what Peter suggested. This needs to be changed.
> >>>
> > Thanks for review and suggestion.
> > How about this change?
> >
> > assert(ram_size > 0);
>
> yes.
>
> I will send another patch fixing a long standing issue in the SDMC model not
> checking the ram_size value in the realize handler. It relies on the
> "ram-size"
> property being set.
>
> Thanks,
>
Will send v3 patch and thanks for your review and help.
Jamin
> C.
>
>
> > If you agree, I will send v3 patch.
> > Thanks-Jamin
> >
> >>>
> >>> Thanks,
> >>>
> >>> C.
> >>>
> >> Ok fine, I didn't see the message, sorry!
> >>
> >> Thanks
> >>
> >> >cmd
> >>
> >>>
> >>>
> >>>>> /*
> >>>>> * Emulate ddr capacity hardware behavior.
> >>>>> * If writes the data to the address which is beyond the
> >>>>> ram size,
> >>>
