* [PATCH] hw/arm/boot: Use the IEC binary prefix definitions
@ 2019-09-21 10:34 Philippe Mathieu-Daudé
2019-09-21 15:26 ` Richard Henderson
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-21 10:34 UTC (permalink / raw)
To: qemu-devel, qemu-trivial
Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
IEC binary prefixes ease code review: the unit is explicit.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/arm/boot.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index bf97ef3e33..59bb2fa0d3 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -1095,7 +1095,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
* we might still make a bad choice here.
*/
info->initrd_start = info->loader_start +
- MIN(info->ram_size / 2, 128 * 1024 * 1024);
+ MIN(info->ram_size / 2, 128 * MiB);
if (image_high_addr) {
info->initrd_start = MAX(info->initrd_start, image_high_addr);
}
@@ -1155,13 +1155,13 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
*
* Let's play safe and prealign it to 2MB to give us some space.
*/
- align = 2 * 1024 * 1024;
+ align = 2 * MiB;
} else {
/*
* Some 32bit kernels will trash anything in the 4K page the
* initrd ends in, so make sure the DTB isn't caught up in that.
*/
- align = 4096;
+ align = 4 * KiB;
}
/* Place the DTB after the initrd in memory with alignment. */
@@ -1178,7 +1178,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
info->loader_start + KERNEL_ARGS_ADDR;
fixupcontext[FIXUP_ARGPTR_HI] =
(info->loader_start + KERNEL_ARGS_ADDR) >> 32;
- if (info->ram_size >= (1ULL << 32)) {
+ if (info->ram_size >= 4 * GiB) {
error_report("RAM size must be less than 4GB to boot"
" Linux kernel using ATAGS (try passing a device tree"
" using -dtb)");
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] hw/arm/boot: Use the IEC binary prefix definitions
2019-09-21 10:34 [PATCH] hw/arm/boot: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
@ 2019-09-21 15:26 ` Richard Henderson
2019-09-23 8:29 ` Stefano Garzarella
2019-09-23 10:17 ` Thomas Huth
2 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2019-09-21 15:26 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, qemu-trivial
Cc: Peter Maydell, qemu-arm
On 9/21/19 3:34 AM, Philippe Mathieu-Daudé wrote:
> IEC binary prefixes ease code review: the unit is explicit.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/arm/boot.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/arm/boot: Use the IEC binary prefix definitions
2019-09-21 10:34 [PATCH] hw/arm/boot: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
2019-09-21 15:26 ` Richard Henderson
@ 2019-09-23 8:29 ` Stefano Garzarella
2019-09-23 8:43 ` Philippe Mathieu-Daudé
2019-09-23 10:17 ` Thomas Huth
2 siblings, 1 reply; 6+ messages in thread
From: Stefano Garzarella @ 2019-09-23 8:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-trivial, Peter Maydell, qemu-arm, qemu-devel
On Sat, Sep 21, 2019 at 12:34:04PM +0200, Philippe Mathieu-Daudé wrote:
> IEC binary prefixes ease code review: the unit is explicit.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/arm/boot.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index bf97ef3e33..59bb2fa0d3 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
Very appreciated!
What about fixing also this other line?
@@ -575,7 +575,7 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
goto fail;
}
- if (scells < 2 && binfo->ram_size >= (1ULL << 32)) {
+ if (scells < 2 && binfo->ram_size >= 4 * GiB) {
/* This is user error so deserves a friendlier error message
* than the failure of setprop_sized_cells would provide
> @@ -1095,7 +1095,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> * we might still make a bad choice here.
> */
> info->initrd_start = info->loader_start +
> - MIN(info->ram_size / 2, 128 * 1024 * 1024);
> + MIN(info->ram_size / 2, 128 * MiB);
> if (image_high_addr) {
> info->initrd_start = MAX(info->initrd_start, image_high_addr);
> }
> @@ -1155,13 +1155,13 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> *
> * Let's play safe and prealign it to 2MB to give us some space.
> */
> - align = 2 * 1024 * 1024;
> + align = 2 * MiB;
> } else {
> /*
> * Some 32bit kernels will trash anything in the 4K page the
> * initrd ends in, so make sure the DTB isn't caught up in that.
> */
> - align = 4096;
> + align = 4 * KiB;
> }
>
> /* Place the DTB after the initrd in memory with alignment. */
> @@ -1178,7 +1178,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> info->loader_start + KERNEL_ARGS_ADDR;
> fixupcontext[FIXUP_ARGPTR_HI] =
> (info->loader_start + KERNEL_ARGS_ADDR) >> 32;
> - if (info->ram_size >= (1ULL << 32)) {
> + if (info->ram_size >= 4 * GiB) {
> error_report("RAM size must be less than 4GB to boot"
> " Linux kernel using ATAGS (try passing a device tree"
> " using -dtb)");
With or without the new line fix:
Acked-by: Stefano Garzarella <sgarzare@redhat.com>
Thanks,
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] hw/arm/boot: Use the IEC binary prefix definitions
2019-09-23 8:29 ` Stefano Garzarella
@ 2019-09-23 8:43 ` Philippe Mathieu-Daudé
2019-09-23 8:45 ` Stefano Garzarella
0 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-23 8:43 UTC (permalink / raw)
To: Stefano Garzarella; +Cc: qemu-trivial, Peter Maydell, qemu-arm, qemu-devel
On 9/23/19 10:29 AM, Stefano Garzarella wrote:
> On Sat, Sep 21, 2019 at 12:34:04PM +0200, Philippe Mathieu-Daudé wrote:
>> IEC binary prefixes ease code review: the unit is explicit.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> hw/arm/boot.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index bf97ef3e33..59bb2fa0d3 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>
> Very appreciated!
>
> What about fixing also this other line?
>
> @@ -575,7 +575,7 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
> goto fail;
> }
>
> - if (scells < 2 && binfo->ram_size >= (1ULL << 32)) {
> + if (scells < 2 && binfo->ram_size >= 4 * GiB) {
Good idea!
> /* This is user error so deserves a friendlier error message
> * than the failure of setprop_sized_cells would provide
>
>
>> @@ -1095,7 +1095,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
>> * we might still make a bad choice here.
>> */
>> info->initrd_start = info->loader_start +
>> - MIN(info->ram_size / 2, 128 * 1024 * 1024);
>> + MIN(info->ram_size / 2, 128 * MiB);
>> if (image_high_addr) {
>> info->initrd_start = MAX(info->initrd_start, image_high_addr);
>> }
>> @@ -1155,13 +1155,13 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
>> *
>> * Let's play safe and prealign it to 2MB to give us some space.
>> */
>> - align = 2 * 1024 * 1024;
>> + align = 2 * MiB;
>> } else {
>> /*
>> * Some 32bit kernels will trash anything in the 4K page the
>> * initrd ends in, so make sure the DTB isn't caught up in that.
>> */
>> - align = 4096;
>> + align = 4 * KiB;
>> }
>>
>> /* Place the DTB after the initrd in memory with alignment. */
>> @@ -1178,7 +1178,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
>> info->loader_start + KERNEL_ARGS_ADDR;
>> fixupcontext[FIXUP_ARGPTR_HI] =
>> (info->loader_start + KERNEL_ARGS_ADDR) >> 32;
>> - if (info->ram_size >= (1ULL << 32)) {
>> + if (info->ram_size >= 4 * GiB) {
>> error_report("RAM size must be less than 4GB to boot"
>> " Linux kernel using ATAGS (try passing a device tree"
>> " using -dtb)");
>
> With or without the new line fix:
>
> Acked-by: Stefano Garzarella <sgarzare@redhat.com>
Did you mean Reviewed-by?
Regards,
Phil.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] hw/arm/boot: Use the IEC binary prefix definitions
2019-09-23 8:43 ` Philippe Mathieu-Daudé
@ 2019-09-23 8:45 ` Stefano Garzarella
0 siblings, 0 replies; 6+ messages in thread
From: Stefano Garzarella @ 2019-09-23 8:45 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-trivial, Peter Maydell, qemu-arm, qemu-devel
On Mon, Sep 23, 2019 at 10:43:12AM +0200, Philippe Mathieu-Daudé wrote:
> On 9/23/19 10:29 AM, Stefano Garzarella wrote:
> > On Sat, Sep 21, 2019 at 12:34:04PM +0200, Philippe Mathieu-Daudé wrote:
> >> IEC binary prefixes ease code review: the unit is explicit.
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> ---
> >> hw/arm/boot.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> >> index bf97ef3e33..59bb2fa0d3 100644
> >> --- a/hw/arm/boot.c
> >> +++ b/hw/arm/boot.c
> >
> > Very appreciated!
> >
> > What about fixing also this other line?
> >
> > @@ -575,7 +575,7 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
> > goto fail;
> > }
> >
> > - if (scells < 2 && binfo->ram_size >= (1ULL << 32)) {
> > + if (scells < 2 && binfo->ram_size >= 4 * GiB) {
>
> Good idea!
>
> > /* This is user error so deserves a friendlier error message
> > * than the failure of setprop_sized_cells would provide
> >
> >
> >> @@ -1095,7 +1095,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> >> * we might still make a bad choice here.
> >> */
> >> info->initrd_start = info->loader_start +
> >> - MIN(info->ram_size / 2, 128 * 1024 * 1024);
> >> + MIN(info->ram_size / 2, 128 * MiB);
> >> if (image_high_addr) {
> >> info->initrd_start = MAX(info->initrd_start, image_high_addr);
> >> }
> >> @@ -1155,13 +1155,13 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> >> *
> >> * Let's play safe and prealign it to 2MB to give us some space.
> >> */
> >> - align = 2 * 1024 * 1024;
> >> + align = 2 * MiB;
> >> } else {
> >> /*
> >> * Some 32bit kernels will trash anything in the 4K page the
> >> * initrd ends in, so make sure the DTB isn't caught up in that.
> >> */
> >> - align = 4096;
> >> + align = 4 * KiB;
> >> }
> >>
> >> /* Place the DTB after the initrd in memory with alignment. */
> >> @@ -1178,7 +1178,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> >> info->loader_start + KERNEL_ARGS_ADDR;
> >> fixupcontext[FIXUP_ARGPTR_HI] =
> >> (info->loader_start + KERNEL_ARGS_ADDR) >> 32;
> >> - if (info->ram_size >= (1ULL << 32)) {
> >> + if (info->ram_size >= 4 * GiB) {
> >> error_report("RAM size must be less than 4GB to boot"
> >> " Linux kernel using ATAGS (try passing a device tree"
> >> " using -dtb)");
> >
> > With or without the new line fix:
> >
> > Acked-by: Stefano Garzarella <sgarzare@redhat.com>
>
> Did you mean Reviewed-by?
Yes, sorry! :-)
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/arm/boot: Use the IEC binary prefix definitions
2019-09-21 10:34 [PATCH] hw/arm/boot: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
2019-09-21 15:26 ` Richard Henderson
2019-09-23 8:29 ` Stefano Garzarella
@ 2019-09-23 10:17 ` Thomas Huth
2 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2019-09-23 10:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, qemu-trivial
Cc: Peter Maydell, qemu-arm
[-- Attachment #1.1: Type: text/plain, Size: 2145 bytes --]
On 21/09/2019 12.34, Philippe Mathieu-Daudé wrote:
> IEC binary prefixes ease code review: the unit is explicit.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/arm/boot.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index bf97ef3e33..59bb2fa0d3 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -1095,7 +1095,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> * we might still make a bad choice here.
> */
> info->initrd_start = info->loader_start +
> - MIN(info->ram_size / 2, 128 * 1024 * 1024);
> + MIN(info->ram_size / 2, 128 * MiB);
> if (image_high_addr) {
> info->initrd_start = MAX(info->initrd_start, image_high_addr);
> }
> @@ -1155,13 +1155,13 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> *
> * Let's play safe and prealign it to 2MB to give us some space.
> */
> - align = 2 * 1024 * 1024;
> + align = 2 * MiB;
> } else {
> /*
> * Some 32bit kernels will trash anything in the 4K page the
> * initrd ends in, so make sure the DTB isn't caught up in that.
> */
> - align = 4096;
> + align = 4 * KiB;
> }
>
> /* Place the DTB after the initrd in memory with alignment. */
> @@ -1178,7 +1178,7 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> info->loader_start + KERNEL_ARGS_ADDR;
> fixupcontext[FIXUP_ARGPTR_HI] =
> (info->loader_start + KERNEL_ARGS_ADDR) >> 32;
> - if (info->ram_size >= (1ULL << 32)) {
> + if (info->ram_size >= 4 * GiB) {
> error_report("RAM size must be less than 4GB to boot"
> " Linux kernel using ATAGS (try passing a device tree"
> " using -dtb)");
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-23 10:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-21 10:34 [PATCH] hw/arm/boot: Use the IEC binary prefix definitions Philippe Mathieu-Daudé
2019-09-21 15:26 ` Richard Henderson
2019-09-23 8:29 ` Stefano Garzarella
2019-09-23 8:43 ` Philippe Mathieu-Daudé
2019-09-23 8:45 ` Stefano Garzarella
2019-09-23 10:17 ` Thomas Huth
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).