[Qemu-devel] [PATCH v2 0/3] ppc/pnv: increase kernel size limit to 256MiB

[Qemu-devel] [PATCH v2 0/3] ppc/pnv: increase kernel size limit to 256MiB

[Murilo Opsfelder Araujo]

First patch increases kernel size limit to 256MiB to fit images generated with
CONFIG_DEBUG_INFO.

Second patch just fixes a misleading comment regarding initrd size limit.

Third patch uses IEC prefixes to represent size constants.

v1..v2:
  Use IEC prefixes as per Eric Blake' suggestion.

Murilo Opsfelder Araujo (3):
  ppc/pnv: increase kernel size limit to 256MiB
  ppc/pnv: add INITRD_MAX_SIZE constant
  ppc/pnv: use IEC binary prefixes to represent sizes

 hw/ppc/pnv.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH v2 1/3] ppc/pnv: increase kernel size limit to 256MiB

[Murilo Opsfelder Araujo]

Building kernel with CONFIG_DEBUG_INFO_REDUCED can generate a ~90MB image and
building with CONFIG_DEBUG_INFO can generate a ~225M one, both exceeds the
current limit of 32MiB.

Increasing kernel size limit to 256MiB should fit for now.

Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
---
 hw/ppc/pnv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index da540860a2..1dbb62c4c9 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -54,6 +54,7 @@
 #define FW_MAX_SIZE             0x00400000
 
 #define KERNEL_LOAD_ADDR        0x20000000
+#define KERNEL_MAX_SIZE         (256 * MiB)
 #define INITRD_LOAD_ADDR        0x60000000
 
 static const char *pnv_chip_core_typename(const PnvChip *o)
@@ -588,7 +589,7 @@ static void pnv_init(MachineState *machine)
         long kernel_size;
 
         kernel_size = load_image_targphys(machine->kernel_filename,
-                                          KERNEL_LOAD_ADDR, 0x2000000);
+                                          KERNEL_LOAD_ADDR, KERNEL_MAX_SIZE);
         if (kernel_size < 0) {
             error_report("Could not load kernel '%s'",
                          machine->kernel_filename);
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 1/3] ppc/pnv: increase kernel size limit to 256MiB

[Cédric Le Goater]

On 2/25/19 6:01 PM, Murilo Opsfelder Araujo wrote:
> Building kernel with CONFIG_DEBUG_INFO_REDUCED can generate a ~90MB image and
> building with CONFIG_DEBUG_INFO can generate a ~225M one, both exceeds the
> current limit of 32MiB.
> 
> Increasing kernel size limit to 256MiB should fit for now.



Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> 
> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> ---
>  hw/ppc/pnv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index da540860a2..1dbb62c4c9 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -54,6 +54,7 @@
>  #define FW_MAX_SIZE             0x00400000
>  
>  #define KERNEL_LOAD_ADDR        0x20000000
> +#define KERNEL_MAX_SIZE         (256 * MiB)
>  #define INITRD_LOAD_ADDR        0x60000000
>  
>  static const char *pnv_chip_core_typename(const PnvChip *o)
> @@ -588,7 +589,7 @@ static void pnv_init(MachineState *machine)
>          long kernel_size;
>  
>          kernel_size = load_image_targphys(machine->kernel_filename,
> -                                          KERNEL_LOAD_ADDR, 0x2000000);
> +                                          KERNEL_LOAD_ADDR, KERNEL_MAX_SIZE);
>          if (kernel_size < 0) {
>              error_report("Could not load kernel '%s'",
>                           machine->kernel_filename);
> 

[Qemu-devel] [PATCH v2 2/3] ppc/pnv: add INITRD_MAX_SIZE constant

[Murilo Opsfelder Araujo]

The current 0x10000000 value is actually 256MiB, not 128MB as the comment
suggests. Move it to a constant and fix the comment (no change in the size
value).

Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
---
 hw/ppc/pnv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 1dbb62c4c9..c308ae423a 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -56,6 +56,7 @@
 #define KERNEL_LOAD_ADDR        0x20000000
 #define KERNEL_MAX_SIZE         (256 * MiB)
 #define INITRD_LOAD_ADDR        0x60000000
+#define INITRD_MAX_SIZE         (256 * MiB)
 
 static const char *pnv_chip_core_typename(const PnvChip *o)
 {
@@ -601,7 +602,7 @@ static void pnv_init(MachineState *machine)
     if (machine->initrd_filename) {
         pnv->initrd_base = INITRD_LOAD_ADDR;
         pnv->initrd_size = load_image_targphys(machine->initrd_filename,
-                                  pnv->initrd_base, 0x10000000); /* 128MB max */
+                                  pnv->initrd_base, INITRD_MAX_SIZE);
         if (pnv->initrd_size < 0) {
             error_report("Could not load initial ram disk '%s'",
                          machine->initrd_filename);
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 2/3] ppc/pnv: add INITRD_MAX_SIZE constant

[Cédric Le Goater]

On 2/25/19 6:01 PM, Murilo Opsfelder Araujo wrote:
> The current 0x10000000 value is actually 256MiB, not 128MB as the comment
> suggests. Move it to a constant and fix the comment (no change in the size
> value).
> 
> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>	



Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/ppc/pnv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 1dbb62c4c9..c308ae423a 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -56,6 +56,7 @@
>  #define KERNEL_LOAD_ADDR        0x20000000
>  #define KERNEL_MAX_SIZE         (256 * MiB)
>  #define INITRD_LOAD_ADDR        0x60000000
> +#define INITRD_MAX_SIZE         (256 * MiB)
>  
>  static const char *pnv_chip_core_typename(const PnvChip *o)
>  {
> @@ -601,7 +602,7 @@ static void pnv_init(MachineState *machine)
>      if (machine->initrd_filename) {
>          pnv->initrd_base = INITRD_LOAD_ADDR;
>          pnv->initrd_size = load_image_targphys(machine->initrd_filename,
> -                                  pnv->initrd_base, 0x10000000); /* 128MB max */
> +                                  pnv->initrd_base, INITRD_MAX_SIZE);
>          if (pnv->initrd_size < 0) {
>              error_report("Could not load initial ram disk '%s'",
>                           machine->initrd_filename);
> 

[Qemu-devel] [PATCH v2 3/3] ppc/pnv: use IEC binary prefixes to represent sizes

[Murilo Opsfelder Araujo]

Using IEC binary prefixes from qemu/units.h provides a more human-friendly value
to size constants.

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
---
 hw/ppc/pnv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index c308ae423a..b6c5d1eff9 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -47,11 +47,11 @@
 
 #include <libfdt.h>
 
-#define FDT_MAX_SIZE            0x00100000
+#define FDT_MAX_SIZE            (1 * MiB)
 
 #define FW_FILE_NAME            "skiboot.lid"
 #define FW_LOAD_ADDR            0x0
-#define FW_MAX_SIZE             0x00400000
+#define FW_MAX_SIZE             (4 * MiB)
 
 #define KERNEL_LOAD_ADDR        0x20000000
 #define KERNEL_MAX_SIZE         (256 * MiB)
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 3/3] ppc/pnv: use IEC binary prefixes to represent sizes

[Cédric Le Goater]

On 2/25/19 6:01 PM, Murilo Opsfelder Araujo wrote:
> Using IEC binary prefixes from qemu/units.h provides a more human-friendly value
> to size constants.
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>



Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  hw/ppc/pnv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index c308ae423a..b6c5d1eff9 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -47,11 +47,11 @@
>  
>  #include <libfdt.h>
>  
> -#define FDT_MAX_SIZE            0x00100000
> +#define FDT_MAX_SIZE            (1 * MiB)
>  
>  #define FW_FILE_NAME            "skiboot.lid"
>  #define FW_LOAD_ADDR            0x0
> -#define FW_MAX_SIZE             0x00400000
> +#define FW_MAX_SIZE             (4 * MiB)
>  
>  #define KERNEL_LOAD_ADDR        0x20000000
>  #define KERNEL_MAX_SIZE         (256 * MiB)
> 

Re: [Qemu-devel] [PATCH v2 0/3] ppc/pnv: increase kernel size limit to 256MiB

[Cédric Le Goater]

On 2/25/19 6:01 PM, Murilo Opsfelder Araujo wrote:
> First patch increases kernel size limit to 256MiB to fit images generated with
> CONFIG_DEBUG_INFO.
> 
> Second patch just fixes a misleading comment regarding initrd size limit.
> 
> Third patch uses IEC prefixes to represent size constants.

They look fine.

Have you tried these patches on a P9 machine ? You will need to use
the powernv-4.0 branch on my github for that.

Thanks,

C. 

> 
> v1..v2:
>   Use IEC prefixes as per Eric Blake' suggestion.
> 
> Murilo Opsfelder Araujo (3):
>   ppc/pnv: increase kernel size limit to 256MiB
>   ppc/pnv: add INITRD_MAX_SIZE constant
>   ppc/pnv: use IEC binary prefixes to represent sizes
> 
>  hw/ppc/pnv.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/3] ppc/pnv: increase kernel size limit to 256MiB

[Murilo Opsfelder Araujo]

On Mon, Feb 25, 2019 at 07:58:06PM +0100, Cédric Le Goater wrote:
> On 2/25/19 6:01 PM, Murilo Opsfelder Araujo wrote:
> > First patch increases kernel size limit to 256MiB to fit images generated with
> > CONFIG_DEBUG_INFO.
> >
> > Second patch just fixes a misleading comment regarding initrd size limit.
> >
> > Third patch uses IEC prefixes to represent size constants.
>
> They look fine.
>
> Have you tried these patches on a P9 machine ? You will need to use
> the powernv-4.0 branch on my github for that.

I had actually smoke-tested it on my laptop:

[    0.000000] Linux version 5.0.0-rc8-panda (muriloo@localhost) (gcc version 6.4.0 (Buildroot 2018.02-rc2-00006-g39101b7)) #1 SMP Mon Feb 25 11:00:15 -03 2019
[    0.000000] Found initrd at 0xc000000060000000:0xc0000000601e6cfc
[    0.000000] OPAL: Found non-mapped LPC bus on chip 0
[    0.000000] Using PowerNV machine description
[    0.000000] printk: bootconsole [udbg0] enabled
[    0.000000] CPU maps initialized for 1 thread per core
[    0.000000]  (thread shift is 0)
[    0.000000] Allocated 3336 bytes for 1 pacas

My patches apply smoothly on top of your powernv-4.0 branch, in case you're
willing to give it a try.

Let's chat offline to get it tested on a p9 machine.

Thanks for the review!

--
Murilo