* [PATCH v2 0/2] Fix coverity issues for AST2700
@ 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 ` [PATCH v2 2/2] aspeed/sdmc: Remove extra R_MAIN_STATUS case Jamin Lin via
0 siblings, 2 replies; 9+ 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] 9+ messages in thread
* [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero
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 6:00 ` cmd
2024-06-25 1:50 ` [PATCH v2 2/2] aspeed/sdmc: Remove extra R_MAIN_STATUS case Jamin Lin via
1 sibling, 1 reply; 9+ 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] 9+ messages in thread
* [PATCH v2 2/2] aspeed/sdmc: Remove extra R_MAIN_STATUS case
2024-06-25 1:50 [PATCH v2 0/2] Fix coverity issues for AST2700 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
1 sibling, 0 replies; 9+ 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] 9+ messages in thread
* Re: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero
2024-06-25 1:50 ` [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero Jamin Lin via
@ 2024-06-25 6:00 ` cmd
2024-06-25 6:03 ` Cédric Le Goater
0 siblings, 1 reply; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2024-06-25 6:42 UTC | newest]
Thread overview: 9+ 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 ` [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).