* Re: [Qemu-devel] [PATCH] hw: fix to display correct memory size
2013-08-23 14:57 [Qemu-devel] [PATCH] hw: fix to display correct memory size Yongbok Kim
@ 2013-08-23 16:20 ` Andreas Färber
2013-08-25 16:11 ` Alexander Graf
2013-08-25 16:09 ` Alexander Graf
2013-08-28 16:48 ` Aurelien Jarno
2 siblings, 1 reply; 6+ messages in thread
From: Andreas Färber @ 2013-08-23 16:20 UTC (permalink / raw)
To: Yongbok Kim; +Cc: critian.cuna, leon.alrae, qemu-devel, aurelien, agraf
Am 23.08.2013 16:57, schrieb Yongbok Kim:
> A parenthesis placed inappropriately caused displaying
> wrong memory size bigger than 4GB.
>
> Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
> ---
> hw/mips/mips_malta.c | 2 +-
> hw/mips/mips_r4k.c | 2 +-
> hw/ppc/mac_oldworld.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
Thanks for fixing this even beyond MIPS machines!
In theory I would've preferred a patch using the correct format string
and dropping the unsigned int cast completely, but our RAM_ADDR_FMT uses
hexadecimal format, so:
Reviewed-by: Andreas Färber <afaerber@suse.de>
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index f8d064c..23ac1ca 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -954,7 +954,7 @@ void mips_malta_init(QEMUMachineInitArgs *args)
> if (ram_size > (256 << 20)) {
> fprintf(stderr,
> "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n",
> - ((unsigned int)ram_size / (1 << 20)));
> + (unsigned int)(ram_size / (1 << 20)));
Seeing that in the Malta case your other patch will trivially conflict
with this bugfix, would you have time to turn it into a series of three
patches, this being the first and adding "Cc: qemu-stable@nongnu.org" to
its commit message for inclusion in 1.6.1, then an additional patch
replacing fprintf(stderr, "qemu: ...\n", ...) with error_report("...",
...) (note no trailing \n and it will also automatically prefix the
right qemu-system-mips* executable name) and finally your 2 GiB patch?
Regards,
Andreas
> exit(1);
> }
> memory_region_init_ram(ram, NULL, "mips_malta.ram", ram_size);
> diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
> index 044f232..e8108ac 100644
> --- a/hw/mips/mips_r4k.c
> +++ b/hw/mips/mips_r4k.c
> @@ -201,7 +201,7 @@ void mips_r4k_init(QEMUMachineInitArgs *args)
> if (ram_size > (256 << 20)) {
> fprintf(stderr,
> "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n",
> - ((unsigned int)ram_size / (1 << 20)));
> + (unsigned int)(ram_size / (1 << 20)));
> exit(1);
> }
> memory_region_init_ram(ram, NULL, "mips_r4k.ram", ram_size);
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 42bb9d5..d7d1758 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -124,7 +124,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args)
> if (ram_size > (2047 << 20)) {
> fprintf(stderr,
> "qemu: Too much memory for this machine: %d MB, maximum 2047 MB\n",
> - ((unsigned int)ram_size / (1 << 20)));
> + (unsigned int)(ram_size / (1 << 20)));
> exit(1);
> }
>
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] hw: fix to display correct memory size
2013-08-23 16:20 ` Andreas Färber
@ 2013-08-25 16:11 ` Alexander Graf
2013-08-27 15:47 ` Yongbok Kim
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2013-08-25 16:11 UTC (permalink / raw)
To: Andreas Färber
Cc: Yongbok Kim, leon.alrae, qemu-devel, aurelien, critian.cuna
On 23.08.2013, at 17:20, Andreas Färber wrote:
> Am 23.08.2013 16:57, schrieb Yongbok Kim:
>> A parenthesis placed inappropriately caused displaying
>> wrong memory size bigger than 4GB.
>>
>> Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
>> ---
>> hw/mips/mips_malta.c | 2 +-
>> hw/mips/mips_r4k.c | 2 +-
>> hw/ppc/mac_oldworld.c | 2 +-
>> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> Thanks for fixing this even beyond MIPS machines!
>
> In theory I would've preferred a patch using the correct format string
> and dropping the unsigned int cast completely, but our RAM_ADDR_FMT uses
> hexadecimal format, so:
>
> Reviewed-by: Andreas Färber <afaerber@suse.de>
>
>> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
>> index f8d064c..23ac1ca 100644
>> --- a/hw/mips/mips_malta.c
>> +++ b/hw/mips/mips_malta.c
>> @@ -954,7 +954,7 @@ void mips_malta_init(QEMUMachineInitArgs *args)
>> if (ram_size > (256 << 20)) {
>> fprintf(stderr,
>> "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n",
>> - ((unsigned int)ram_size / (1 << 20)));
>> + (unsigned int)(ram_size / (1 << 20)));
>
> Seeing that in the Malta case your other patch will trivially conflict
> with this bugfix, would you have time to turn it into a series of three
> patches, this being the first and adding "Cc: qemu-stable@nongnu.org" to
> its commit message for inclusion in 1.6.1, then an additional patch
> replacing fprintf(stderr, "qemu: ...\n", ...) with error_report("...",
> ...) (note no trailing \n and it will also automatically prefix the
> right qemu-system-mips* executable name) and finally your 2 GiB patch?
Ah, saw this comment too late. Andreas' suggestions is obviously even better :).
Alex
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] hw: fix to display correct memory size
2013-08-25 16:11 ` Alexander Graf
@ 2013-08-27 15:47 ` Yongbok Kim
0 siblings, 0 replies; 6+ messages in thread
From: Yongbok Kim @ 2013-08-27 15:47 UTC (permalink / raw)
To: Alexander Graf, Andreas Färber
Cc: Cristian Cuna, Leon Alrae, qemu-devel@nongnu.org,
aurelien@aurel32.net
Because this patch is not the critical, probably I can re-send this and the additional patches (Andreas' suggestion) as trivial patches when the 2GiB patch applied.
Regards,
Yongbok
-----Original Message-----
From: Alexander Graf [mailto:agraf@suse.de]
Sent: 25 August 2013 17:12
To: Andreas Färber
Cc: Yongbok Kim; qemu-devel@nongnu.org; Leon Alrae; aurelien@aurel32.net; critian.cuna@imgtec.com
Subject: Re: [Qemu-devel] [PATCH] hw: fix to display correct memory size
On 23.08.2013, at 17:20, Andreas Färber wrote:
> Am 23.08.2013 16:57, schrieb Yongbok Kim:
>> A parenthesis placed inappropriately caused displaying wrong memory
>> size bigger than 4GB.
>>
>> Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
>> ---
>> hw/mips/mips_malta.c | 2 +-
>> hw/mips/mips_r4k.c | 2 +-
>> hw/ppc/mac_oldworld.c | 2 +-
>> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> Thanks for fixing this even beyond MIPS machines!
>
> In theory I would've preferred a patch using the correct format string
> and dropping the unsigned int cast completely, but our RAM_ADDR_FMT
> uses hexadecimal format, so:
>
> Reviewed-by: Andreas Färber <afaerber@suse.de>
>
>> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index
>> f8d064c..23ac1ca 100644
>> --- a/hw/mips/mips_malta.c
>> +++ b/hw/mips/mips_malta.c
>> @@ -954,7 +954,7 @@ void mips_malta_init(QEMUMachineInitArgs *args)
>> if (ram_size > (256 << 20)) {
>> fprintf(stderr,
>> "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n",
>> - ((unsigned int)ram_size / (1 << 20)));
>> + (unsigned int)(ram_size / (1 << 20)));
>
> Seeing that in the Malta case your other patch will trivially conflict
> with this bugfix, would you have time to turn it into a series of
> three patches, this being the first and adding "Cc:
> qemu-stable@nongnu.org" to its commit message for inclusion in 1.6.1,
> then an additional patch replacing fprintf(stderr, "qemu: ...\n", ...)
> with error_report("...",
> ...) (note no trailing \n and it will also automatically prefix the
> right qemu-system-mips* executable name) and finally your 2 GiB patch?
Ah, saw this comment too late. Andreas' suggestions is obviously even better :).
Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] hw: fix to display correct memory size
2013-08-23 14:57 [Qemu-devel] [PATCH] hw: fix to display correct memory size Yongbok Kim
2013-08-23 16:20 ` Andreas Färber
@ 2013-08-25 16:09 ` Alexander Graf
2013-08-28 16:48 ` Aurelien Jarno
2 siblings, 0 replies; 6+ messages in thread
From: Alexander Graf @ 2013-08-25 16:09 UTC (permalink / raw)
To: Yongbok Kim
Cc: qemu-trivial, leon.alrae, qemu-devel@nongnu.org qemu-devel,
Aurelien Jarno, critian.cuna
On 23.08.2013, at 15:57, Yongbok Kim wrote:
> A parenthesis placed inappropriately caused displaying
> wrong memory size bigger than 4GB.
>
> Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
Acked-by: Alexander Graf <agraf@suse.de>
I think this should easily go in through the qemu-trivial queue.
Alex
> ---
> hw/mips/mips_malta.c | 2 +-
> hw/mips/mips_r4k.c | 2 +-
> hw/ppc/mac_oldworld.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index f8d064c..23ac1ca 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -954,7 +954,7 @@ void mips_malta_init(QEMUMachineInitArgs *args)
> if (ram_size > (256 << 20)) {
> fprintf(stderr,
> "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n",
> - ((unsigned int)ram_size / (1 << 20)));
> + (unsigned int)(ram_size / (1 << 20)));
> exit(1);
> }
> memory_region_init_ram(ram, NULL, "mips_malta.ram", ram_size);
> diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
> index 044f232..e8108ac 100644
> --- a/hw/mips/mips_r4k.c
> +++ b/hw/mips/mips_r4k.c
> @@ -201,7 +201,7 @@ void mips_r4k_init(QEMUMachineInitArgs *args)
> if (ram_size > (256 << 20)) {
> fprintf(stderr,
> "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n",
> - ((unsigned int)ram_size / (1 << 20)));
> + (unsigned int)(ram_size / (1 << 20)));
> exit(1);
> }
> memory_region_init_ram(ram, NULL, "mips_r4k.ram", ram_size);
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 42bb9d5..d7d1758 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -124,7 +124,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args)
> if (ram_size > (2047 << 20)) {
> fprintf(stderr,
> "qemu: Too much memory for this machine: %d MB, maximum 2047 MB\n",
> - ((unsigned int)ram_size / (1 << 20)));
> + (unsigned int)(ram_size / (1 << 20)));
> exit(1);
> }
>
> --
> 1.7.4
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] hw: fix to display correct memory size
2013-08-23 14:57 [Qemu-devel] [PATCH] hw: fix to display correct memory size Yongbok Kim
2013-08-23 16:20 ` Andreas Färber
2013-08-25 16:09 ` Alexander Graf
@ 2013-08-28 16:48 ` Aurelien Jarno
2 siblings, 0 replies; 6+ messages in thread
From: Aurelien Jarno @ 2013-08-28 16:48 UTC (permalink / raw)
To: Yongbok Kim; +Cc: leon.alrae, qemu-devel, critian.cuna, agraf
On Fri, Aug 23, 2013 at 03:57:18PM +0100, Yongbok Kim wrote:
> A parenthesis placed inappropriately caused displaying
> wrong memory size bigger than 4GB.
>
> Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
> ---
> hw/mips/mips_malta.c | 2 +-
> hw/mips/mips_r4k.c | 2 +-
> hw/ppc/mac_oldworld.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index f8d064c..23ac1ca 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -954,7 +954,7 @@ void mips_malta_init(QEMUMachineInitArgs *args)
> if (ram_size > (256 << 20)) {
> fprintf(stderr,
> "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n",
> - ((unsigned int)ram_size / (1 << 20)));
> + (unsigned int)(ram_size / (1 << 20)));
> exit(1);
> }
> memory_region_init_ram(ram, NULL, "mips_malta.ram", ram_size);
> diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
> index 044f232..e8108ac 100644
> --- a/hw/mips/mips_r4k.c
> +++ b/hw/mips/mips_r4k.c
> @@ -201,7 +201,7 @@ void mips_r4k_init(QEMUMachineInitArgs *args)
> if (ram_size > (256 << 20)) {
> fprintf(stderr,
> "qemu: Too much memory for this machine: %d MB, maximum 256 MB\n",
> - ((unsigned int)ram_size / (1 << 20)));
> + (unsigned int)(ram_size / (1 << 20)));
> exit(1);
> }
> memory_region_init_ram(ram, NULL, "mips_r4k.ram", ram_size);
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 42bb9d5..d7d1758 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -124,7 +124,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args)
> if (ram_size > (2047 << 20)) {
> fprintf(stderr,
> "qemu: Too much memory for this machine: %d MB, maximum 2047 MB\n",
> - ((unsigned int)ram_size / (1 << 20)));
> + (unsigned int)(ram_size / (1 << 20)));
> exit(1);
> }
For the MIPS part:
Acked-by: Aurelien Jarno <aurelien@aurel32.net>
But as already said, it's probably better to wait for the Malta 2GB
patch to be merged first and send it to the trivial patches queue.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 6+ messages in thread