Patchwork [for-1.4] hw/arm_boot: Align device tree to 4KB boundary, not page

login
register
mail settings
Submitter Peter Maydell
Date Jan. 24, 2013, 7:02 p.m.
Message ID <1359054148-11890-1-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/215480/
State New
Headers show

Comments

Peter Maydell - Jan. 24, 2013, 7:02 p.m.
Align the device tree blob to a 4KB boundary, not to QEMU's
idea of a page boundary -- the latter is the smallest possible
page size for the architecture, which on ARM is 1KB.
The documentation for Linux does not impose separation
or alignment requirements on the device tree blob, but
in practice some kernels will happily trash the entire
page the initrd ends in after they have finished uncompressing
the initrd. So 4KB-align the DTB to ensure it does not get
trampled by these kernels.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
For 1.4 because this causes us problems booting 3.8 kernels
with dtb, and the symptoms are highly confusing. I think this
is a kernel bug, but it is one that is worth working around.

In general I think code in hw/ that looks at TARGET_PAGE_*
is probably a bit suspect...

 hw/arm_boot.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
Blue Swirl - Jan. 26, 2013, 3:08 p.m.
Thanks, applied.

On Thu, Jan 24, 2013 at 7:02 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Align the device tree blob to a 4KB boundary, not to QEMU's
> idea of a page boundary -- the latter is the smallest possible
> page size for the architecture, which on ARM is 1KB.
> The documentation for Linux does not impose separation
> or alignment requirements on the device tree blob, but
> in practice some kernels will happily trash the entire
> page the initrd ends in after they have finished uncompressing
> the initrd. So 4KB-align the DTB to ensure it does not get
> trampled by these kernels.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> For 1.4 because this causes us problems booting 3.8 kernels
> with dtb, and the symptoms are highly confusing. I think this
> is a kernel bug, but it is one that is worth working around.
>
> In general I think code in hw/ that looks at TARGET_PAGE_*
> is probably a bit suspect...
>
>  hw/arm_boot.c |    9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
> index 115f583..4065424 100644
> --- a/hw/arm_boot.c
> +++ b/hw/arm_boot.c
> @@ -441,9 +441,12 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>           * we point to the kernel args.
>           */
>          if (info->dtb_filename) {
> -            /* Place the DTB after the initrd in memory */
> -            hwaddr dtb_start = TARGET_PAGE_ALIGN(info->initrd_start +
> -                                                 initrd_size);
> +            /* Place the DTB after the initrd in memory. Note that some
> +             * kernels will trash anything in the 4K page the initrd
> +             * ends in, so make sure the DTB isn't caught up in that.
> +             */
> +            hwaddr dtb_start = QEMU_ALIGN_UP(info->initrd_start + initrd_size,
> +                                             4096);
>              if (load_dtb(dtb_start, info)) {
>                  exit(1);
>              }
> --
> 1.7.9.5
>
>

Patch

diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 115f583..4065424 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -441,9 +441,12 @@  void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
          * we point to the kernel args.
          */
         if (info->dtb_filename) {
-            /* Place the DTB after the initrd in memory */
-            hwaddr dtb_start = TARGET_PAGE_ALIGN(info->initrd_start +
-                                                 initrd_size);
+            /* Place the DTB after the initrd in memory. Note that some
+             * kernels will trash anything in the 4K page the initrd
+             * ends in, so make sure the DTB isn't caught up in that.
+             */
+            hwaddr dtb_start = QEMU_ALIGN_UP(info->initrd_start + initrd_size,
+                                             4096);
             if (load_dtb(dtb_start, info)) {
                 exit(1);
             }