* [PATCH v2 0/2] Fix coverity issues for AST2700 @ 2024-06-25 1:50 ` Jamin Lin via 0 siblings, 0 replies; 11+ messages in thread From: Jamin Lin via @ 2024-06-25 1:50 UTC (permalink / raw) To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: jamin_lin 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, Cédric. Jamin Lin (2): aspeed/soc: Fix possible divide by zero aspeed/sdmc: Remove extra R_MAIN_STATUS case hw/arm/aspeed_ast27x0.c | 6 ++++++ hw/misc/aspeed_sdmc.c | 1 - 2 files changed, 6 insertions(+), 1 deletion(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 0/2] Fix coverity issues for AST2700 @ 2024-06-25 1:50 ` Jamin Lin via 0 siblings, 0 replies; 11+ messages in thread From: Jamin Lin via @ 2024-06-25 1:50 UTC (permalink / raw) To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: jamin_lin 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, Cédric. Jamin Lin (2): aspeed/soc: Fix possible divide by zero aspeed/sdmc: Remove extra R_MAIN_STATUS case hw/arm/aspeed_ast27x0.c | 6 ++++++ hw/misc/aspeed_sdmc.c | 1 - 2 files changed, 6 insertions(+), 1 deletion(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero 2024-06-25 1:50 ` Jamin Lin via @ 2024-06-25 1:50 ` Jamin Lin via -1 siblings, 0 replies; 11+ messages in thread From: Jamin Lin via @ 2024-06-25 1:50 UTC (permalink / raw) To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: jamin_lin, Cédric Le Goater 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 | 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; + } + /* * Emulate ddr capacity hardware behavior. * If writes the data to the address which is beyond the ram size, -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero @ 2024-06-25 1:50 ` Jamin Lin via 0 siblings, 0 replies; 11+ messages in thread From: Jamin Lin via @ 2024-06-25 1:50 UTC (permalink / raw) To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: jamin_lin, Cédric Le Goater 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 | 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; + } + /* * Emulate ddr capacity hardware behavior. * If writes the data to the address which is beyond the ram size, -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero 2024-06-25 1:50 ` Jamin Lin via (?) @ 2024-06-25 6:00 ` cmd 2024-06-25 6:03 ` Cédric Le Goater -1 siblings, 1 reply; 11+ messages in thread From: cmd @ 2024-06-25 6:00 UTC (permalink / raw) To: Jamin Lin, Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: Cédric Le Goater 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_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 | 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; > + } > + If we are sure that the error cannot happen, shouldn't we assert instead? > /* > * Emulate ddr capacity hardware behavior. > * If writes the data to the address which is beyond the ram size, ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero 2024-06-25 6:00 ` cmd @ 2024-06-25 6:03 ` Cédric Le Goater 2024-06-25 6:07 ` cmd 0 siblings, 1 reply; 11+ messages in thread From: Cédric Le Goater @ 2024-06-25 6:03 UTC (permalink / raw) To: cmd, Jamin Lin, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: Cédric Le Goater 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_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 | 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; >> + } >> + > 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, C. >> /* >> * Emulate ddr capacity hardware behavior. >> * If writes the data to the address which is beyond the ram size, ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero 2024-06-25 6:03 ` Cédric Le Goater @ 2024-06-25 6:07 ` cmd 2024-06-25 6:15 ` Jamin Lin 0 siblings, 1 reply; 11+ messages in thread From: cmd @ 2024-06-25 6:07 UTC (permalink / raw) To: Cédric Le Goater, Jamin Lin, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: Cédric Le Goater 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_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 | 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; >>> + } >>> + >> 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, > > 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, > ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero 2024-06-25 6:07 ` cmd @ 2024-06-25 6:15 ` Jamin Lin 2024-06-25 6:37 ` Cédric Le Goater 0 siblings, 1 reply; 11+ messages in thread From: Jamin Lin @ 2024-06-25 6:15 UTC (permalink / raw) To: cmd, Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: Cédric Le Goater Hi cmd, Cedric and Peter, > -----Original Message----- > From: cmd <clement.mathieudrif.etu@gmail.com> > Sent: Tuesday, June 25, 2024 2:07 PM > To: Cédric Le Goater <clg@kaod.org>; 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> > Cc: Cédric Le Goater <clg@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_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 | 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_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); 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, > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero 2024-06-25 6:15 ` Jamin Lin @ 2024-06-25 6:37 ` Cédric Le Goater 2024-06-25 6:41 ` Jamin Lin 0 siblings, 1 reply; 11+ messages in thread From: Cédric Le Goater @ 2024-06-25 6:37 UTC (permalink / raw) To: Jamin Lin, cmd, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: Cédric Le Goater On 6/25/24 8:15 AM, Jamin Lin wrote: > Hi cmd, Cedric and Peter, > >> -----Original Message----- >> From: cmd <clement.mathieudrif.etu@gmail.com> >> Sent: Tuesday, June 25, 2024 2:07 PM >> To: Cédric Le Goater <clg@kaod.org>; 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> >> Cc: Cédric Le Goater <clg@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_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 | 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_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, 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, >>> ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero 2024-06-25 6:37 ` Cédric Le Goater @ 2024-06-25 6:41 ` Jamin Lin 0 siblings, 0 replies; 11+ messages in thread From: Jamin Lin @ 2024-06-25 6:41 UTC (permalink / raw) To: Cédric Le Goater, cmd, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: Cédric Le Goater Hi Cedric, > -----Original Message----- > From: Cédric Le Goater <clg@kaod.org> > Sent: Tuesday, June 25, 2024 2:38 PM > To: Jamin Lin <jamin_lin@aspeedtech.com>; cmd > <clement.mathieudrif.etu@gmail.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> > Cc: Cédric Le Goater <clg@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.etu@gmail.com> > >> Sent: Tuesday, June 25, 2024 2:07 PM > >> To: Cédric Le Goater <clg@kaod.org>; 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> > >> Cc: Cédric Le Goater <clg@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_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 | 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, > >>> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] aspeed/sdmc: Remove extra R_MAIN_STATUS case 2024-06-25 1:50 ` Jamin Lin via (?) (?) @ 2024-06-25 1:50 ` Jamin Lin via -1 siblings, 0 replies; 11+ messages in thread From: Jamin Lin via @ 2024-06-25 1:50 UTC (permalink / raw) To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs, open list:All patches CC here Cc: jamin_lin, Cédric Le Goater 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-06-25 6:42 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-06-25 1:50 [PATCH v2 0/2] Fix coverity issues for AST2700 Jamin Lin via 2024-06-25 1:50 ` Jamin Lin via 2024-06-25 1:50 ` [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero Jamin Lin via 2024-06-25 1:50 ` Jamin Lin via 2024-06-25 6:00 ` cmd 2024-06-25 6:03 ` Cédric Le Goater 2024-06-25 6:07 ` cmd 2024-06-25 6:15 ` Jamin Lin 2024-06-25 6:37 ` Cédric Le Goater 2024-06-25 6:41 ` Jamin Lin 2024-06-25 1:50 ` [PATCH v2 2/2] aspeed/sdmc: Remove extra R_MAIN_STATUS case Jamin Lin via
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.