[Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel

[Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel

[Laurent Vivier]

If we don't provide the page size in target-ppc:cpu_get_dump_info(),
the default one (TARGET_PAGE_SIZE, 4KB) is used to create
the compressed dump. It works fine with Macintosh, but not with
pseries as the kernel default page size is 64KB.

Without this patch, if we generate a compressed dump in the QEMU monitor:

    (qemu) dump-guest-memory -z qemu.dump

This dump cannot be read by crash:

    # crash vmlinux qemu.dump
    ...
    WARNING: cannot translate vmemmap kernel virtual addresses:
             commands requiring page structure contents will fail
    ...

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 target-ppc/arch_dump.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target-ppc/arch_dump.c b/target-ppc/arch_dump.c
index df1fd8c..ad37a59 100644
--- a/target-ppc/arch_dump.c
+++ b/target-ppc/arch_dump.c
@@ -220,6 +220,11 @@ int cpu_get_dump_info(ArchDumpInfo *info,
     } else {
         info->d_endian = ELFDATA2LSB;
     }
+    /* 64KB is the page size default for pseries kernel */
+    if (strncmp(object_get_typename(qdev_get_machine()),
+                "pseries-", 8) == 0) {
+        info->page_size = (1U << 16);
+    }
 
     return 0;
 }
-- 
2.5.5

Re: [Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

On Wed, Aug 03, 2016 at 09:55:07PM +0200, Laurent Vivier wrote:
> If we don't provide the page size in target-ppc:cpu_get_dump_info(),
> the default one (TARGET_PAGE_SIZE, 4KB) is used to create
> the compressed dump. It works fine with Macintosh, but not with
> pseries as the kernel default page size is 64KB.
> 
> Without this patch, if we generate a compressed dump in the QEMU monitor:
> 
>     (qemu) dump-guest-memory -z qemu.dump
> 
> This dump cannot be read by crash:
> 
>     # crash vmlinux qemu.dump
>     ...
>     WARNING: cannot translate vmemmap kernel virtual addresses:
>              commands requiring page structure contents will fail
>     ...
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  target-ppc/arch_dump.c | 5 +++++
>  1 file changed, 5 insertions(+)

Urgh.. so, really the page size used by the guest kernel is a
guest-side detail, and it's certainly possible to build a 4kiB page
guest kernel, although 64kiB is the norm.

This might be the best we can do, but it'd be nice if we could probe
or otherwise avoid relying on this assumption about the guest kernel.

> 
> diff --git a/target-ppc/arch_dump.c b/target-ppc/arch_dump.c
> index df1fd8c..ad37a59 100644
> --- a/target-ppc/arch_dump.c
> +++ b/target-ppc/arch_dump.c
> @@ -220,6 +220,11 @@ int cpu_get_dump_info(ArchDumpInfo *info,
>      } else {
>          info->d_endian = ELFDATA2LSB;
>      }
> +    /* 64KB is the page size default for pseries kernel */
> +    if (strncmp(object_get_typename(qdev_get_machine()),
> +                "pseries-", 8) == 0) {
> +        info->page_size = (1U << 16);
> +    }
>  
>      return 0;
>  }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel

[Laurent Vivier]

On 04/08/2016 04:38, David Gibson wrote:
> On Wed, Aug 03, 2016 at 09:55:07PM +0200, Laurent Vivier wrote:
>> If we don't provide the page size in target-ppc:cpu_get_dump_info(),
>> the default one (TARGET_PAGE_SIZE, 4KB) is used to create
>> the compressed dump. It works fine with Macintosh, but not with
>> pseries as the kernel default page size is 64KB.
>>
>> Without this patch, if we generate a compressed dump in the QEMU monitor:
>>
>>     (qemu) dump-guest-memory -z qemu.dump
>>
>> This dump cannot be read by crash:
>>
>>     # crash vmlinux qemu.dump
>>     ...
>>     WARNING: cannot translate vmemmap kernel virtual addresses:
>>              commands requiring page structure contents will fail
>>     ...
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>  target-ppc/arch_dump.c | 5 +++++
>>  1 file changed, 5 insertions(+)
> 
> Urgh.. so, really the page size used by the guest kernel is a
> guest-side detail, and it's certainly possible to build a 4kiB page
> guest kernel, although 64kiB is the norm.

virtio-balloon doesn't work with 4K kernel.

> This might be the best we can do, but it'd be nice if we could probe
> or otherwise avoid relying on this assumption about the guest kernel.

I agree with you but none of the other architectures probes for the page
size.

For instance ARM: |I cc: Drew to know how he has chosen the values]

    if (arm_feature(env, ARM_FEATURE_AARCH64)) {
...
        info->page_size = (1 << 16);
...
    } else {
...
        info->page_size = (1 << 12);
...
    }

In the kernel:

arch/arm64/include/asm/page.h:

#define PAGE_SHIFT		CONFIG_ARM64_PAGE_SHIFT

arch/arm64/Kconfig:

config ARM64_PAGE_SHIFT
        int
        default 16 if ARM64_64K_PAGES
        default 14 if ARM64_16K_PAGES
        default 12

choice
        prompt "Page size"
        default ARM64_4K_PAGES
        help
          Page size (translation granule) configuration.

config ARM64_4K_PAGES
        bool "4KB"
        help
          This feature enables 4KB pages support.

config ARM64_16K_PAGES
        bool "16KB"
        help
          The system will use 16KB pages support. AArch32 emulation
          requires applications compiled with 16K (or a multiple of 16K)
          aligned segments.

config ARM64_64K_PAGES
        bool "64KB"
        help
          This feature enables 64KB pages support (4KB by default)
          allowing only two levels of page tables and faster TLB
          look-up. AArch32 emulation requires applications compiled
          with 64K aligned segments.

endchoice

I think we can't rely on the CPU state or the memory content as they can
be corrupted.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3624 bytes --]

On Thu, Aug 04, 2016 at 10:41:16AM +0200, Laurent Vivier wrote:
1;4402;0c> 
> 
> On 04/08/2016 04:38, David Gibson wrote:
> > On Wed, Aug 03, 2016 at 09:55:07PM +0200, Laurent Vivier wrote:
> >> If we don't provide the page size in target-ppc:cpu_get_dump_info(),
> >> the default one (TARGET_PAGE_SIZE, 4KB) is used to create
> >> the compressed dump. It works fine with Macintosh, but not with
> >> pseries as the kernel default page size is 64KB.
> >>
> >> Without this patch, if we generate a compressed dump in the QEMU monitor:
> >>
> >>     (qemu) dump-guest-memory -z qemu.dump
> >>
> >> This dump cannot be read by crash:
> >>
> >>     # crash vmlinux qemu.dump
> >>     ...
> >>     WARNING: cannot translate vmemmap kernel virtual addresses:
> >>              commands requiring page structure contents will fail
> >>     ...
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >>  target-ppc/arch_dump.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> > 
> > Urgh.. so, really the page size used by the guest kernel is a
> > guest-side detail, and it's certainly possible to build a 4kiB page
> > guest kernel, although 64kiB is the norm.
> 
> virtio-balloon doesn't work with 4K kernel.

It doesn't?  Balloon has rather a lot of flaws, but I didn't think
that was one of them.

> > This might be the best we can do, but it'd be nice if we could probe
> > or otherwise avoid relying on this assumption about the guest kernel.
> 
> I agree with you but none of the other architectures probes for the page
> size.

Yeah :/

> For instance ARM: |I cc: Drew to know how he has chosen the values]
> 
>     if (arm_feature(env, ARM_FEATURE_AARCH64)) {
> ...
>         info->page_size = (1 << 16);
> ...
>     } else {
> ...
>         info->page_size = (1 << 12);
> ...
>     }
> 
> In the kernel:
> 
> arch/arm64/include/asm/page.h:
> 
> #define PAGE_SHIFT		CONFIG_ARM64_PAGE_SHIFT
> 
> arch/arm64/Kconfig:
> 
> config ARM64_PAGE_SHIFT
>         int
>         default 16 if ARM64_64K_PAGES
>         default 14 if ARM64_16K_PAGES
>         default 12
> 
> choice
>         prompt "Page size"
>         default ARM64_4K_PAGES
>         help
>           Page size (translation granule) configuration.
> 
> config ARM64_4K_PAGES
>         bool "4KB"
>         help
>           This feature enables 4KB pages support.
> 
> config ARM64_16K_PAGES
>         bool "16KB"
>         help
>           The system will use 16KB pages support. AArch32 emulation
>           requires applications compiled with 16K (or a multiple of 16K)
>           aligned segments.
> 
> config ARM64_64K_PAGES
>         bool "64KB"
>         help
>           This feature enables 64KB pages support (4KB by default)
>           allowing only two levels of page tables and faster TLB
>           look-up. AArch32 emulation requires applications compiled
>           with 64K aligned segments.
> 
> endchoice
> 
> I think we can't rely on the CPU state or the memory content as they can
> be corrupted.

I guess.  I don't know that we can really get what we want from there
anyway, at least not without even more assumptions about the guest
state than.

Hrm.  I guess I'm ok with the change, but I'd like the commit message
updated to recognize that this is a compromise just designed to work
with the most common guests.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel

[Laurent Vivier]

On 05/08/2016 09:49, David Gibson wrote:
> On Thu, Aug 04, 2016 at 10:41:16AM +0200, Laurent Vivier wrote:
> 1;4402;0c> 
>>
>> On 04/08/2016 04:38, David Gibson wrote:
>>> On Wed, Aug 03, 2016 at 09:55:07PM +0200, Laurent Vivier wrote:
>>>> If we don't provide the page size in target-ppc:cpu_get_dump_info(),
>>>> the default one (TARGET_PAGE_SIZE, 4KB) is used to create
>>>> the compressed dump. It works fine with Macintosh, but not with
>>>> pseries as the kernel default page size is 64KB.
>>>>
>>>> Without this patch, if we generate a compressed dump in the QEMU monitor:
>>>>
>>>>     (qemu) dump-guest-memory -z qemu.dump
>>>>
>>>> This dump cannot be read by crash:
>>>>
>>>>     # crash vmlinux qemu.dump
>>>>     ...
>>>>     WARNING: cannot translate vmemmap kernel virtual addresses:
>>>>              commands requiring page structure contents will fail
>>>>     ...
>>>>
>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>> ---
>>>>  target-ppc/arch_dump.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>
>>> Urgh.. so, really the page size used by the guest kernel is a
>>> guest-side detail, and it's certainly possible to build a 4kiB page
>>> guest kernel, although 64kiB is the norm.
>>
>> virtio-balloon doesn't work with 4K kernel.
> 
> It doesn't?  Balloon has rather a lot of flaws, but I didn't think
> that was one of them.
> 
>>> This might be the best we can do, but it'd be nice if we could probe
>>> or otherwise avoid relying on this assumption about the guest kernel.
>>
>> I agree with you but none of the other architectures probes for the page
>> size.
> 
> Yeah :/
> 
>> For instance ARM: |I cc: Drew to know how he has chosen the values]
>>
>>     if (arm_feature(env, ARM_FEATURE_AARCH64)) {
>> ...
>>         info->page_size = (1 << 16);
>> ...
>>     } else {
>> ...
>>         info->page_size = (1 << 12);
>> ...
>>     }
>>
>> In the kernel:
>>
>> arch/arm64/include/asm/page.h:
>>
>> #define PAGE_SHIFT		CONFIG_ARM64_PAGE_SHIFT
>>
>> arch/arm64/Kconfig:
>>
>> config ARM64_PAGE_SHIFT
>>         int
>>         default 16 if ARM64_64K_PAGES
>>         default 14 if ARM64_16K_PAGES
>>         default 12
>>
>> choice
>>         prompt "Page size"
>>         default ARM64_4K_PAGES
>>         help
>>           Page size (translation granule) configuration.
>>
>> config ARM64_4K_PAGES
>>         bool "4KB"
>>         help
>>           This feature enables 4KB pages support.
>>
>> config ARM64_16K_PAGES
>>         bool "16KB"
>>         help
>>           The system will use 16KB pages support. AArch32 emulation
>>           requires applications compiled with 16K (or a multiple of 16K)
>>           aligned segments.
>>
>> config ARM64_64K_PAGES
>>         bool "64KB"
>>         help
>>           This feature enables 64KB pages support (4KB by default)
>>           allowing only two levels of page tables and faster TLB
>>           look-up. AArch32 emulation requires applications compiled
>>           with 64K aligned segments.
>>
>> endchoice
>>
>> I think we can't rely on the CPU state or the memory content as they can
>> be corrupted.
> 
> I guess.  I don't know that we can really get what we want from there
> anyway, at least not without even more assumptions about the guest
> state than.
> 
> Hrm.  I guess I'm ok with the change, but I'd like the commit message
> updated to recognize that this is a compromise just designed to work
> with the most common guests.
> 

Could you update the message or should I send a new patch?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel

[Thomas Huth]

[-- Attachment #1: Type: text/plain, Size: 1798 bytes --]

On 05.08.2016 09:49, David Gibson wrote:
> On Thu, Aug 04, 2016 at 10:41:16AM +0200, Laurent Vivier wrote:
> 1;4402;0c> 
>>
>> On 04/08/2016 04:38, David Gibson wrote:
>>> On Wed, Aug 03, 2016 at 09:55:07PM +0200, Laurent Vivier wrote:
>>>> If we don't provide the page size in target-ppc:cpu_get_dump_info(),
>>>> the default one (TARGET_PAGE_SIZE, 4KB) is used to create
>>>> the compressed dump. It works fine with Macintosh, but not with
>>>> pseries as the kernel default page size is 64KB.
>>>>
>>>> Without this patch, if we generate a compressed dump in the QEMU monitor:
>>>>
>>>>     (qemu) dump-guest-memory -z qemu.dump
>>>>
>>>> This dump cannot be read by crash:
>>>>
>>>>     # crash vmlinux qemu.dump
>>>>     ...
>>>>     WARNING: cannot translate vmemmap kernel virtual addresses:
>>>>              commands requiring page structure contents will fail
>>>>     ...
>>>>
>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>> ---
>>>>  target-ppc/arch_dump.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>
>>> Urgh.. so, really the page size used by the guest kernel is a
>>> guest-side detail, and it's certainly possible to build a 4kiB page
>>> guest kernel, although 64kiB is the norm.
>>
>> virtio-balloon doesn't work with 4K kernel.
> 
> It doesn't?  Balloon has rather a lot of flaws, but I didn't think
> that was one of them.

It currently doesn't work when the guest uses 4k page size but the host
uses 64k page size. Do you remember this bug ticket:
https://bugzilla.redhat.com/show_bug.cgi?id=1323988 ?
... we just decided not to spent time on this because no distro is using
4k page size for the pseries platform anymore, and the virtio-balloon
code is currently under major reconstruction anyway.

 Thomas



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel

[Andrew Jones]

On Thu, Aug 04, 2016 at 10:41:16AM +0200, Laurent Vivier wrote:
> 
> 
> On 04/08/2016 04:38, David Gibson wrote:
> > On Wed, Aug 03, 2016 at 09:55:07PM +0200, Laurent Vivier wrote:
> >> If we don't provide the page size in target-ppc:cpu_get_dump_info(),
> >> the default one (TARGET_PAGE_SIZE, 4KB) is used to create
> >> the compressed dump. It works fine with Macintosh, but not with
> >> pseries as the kernel default page size is 64KB.
> >>
> >> Without this patch, if we generate a compressed dump in the QEMU monitor:
> >>
> >>     (qemu) dump-guest-memory -z qemu.dump
> >>
> >> This dump cannot be read by crash:
> >>
> >>     # crash vmlinux qemu.dump
> >>     ...
> >>     WARNING: cannot translate vmemmap kernel virtual addresses:
> >>              commands requiring page structure contents will fail
> >>     ...
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >>  target-ppc/arch_dump.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> > 
> > Urgh.. so, really the page size used by the guest kernel is a
> > guest-side detail, and it's certainly possible to build a 4kiB page
> > guest kernel, although 64kiB is the norm.
> 
> virtio-balloon doesn't work with 4K kernel.
> 
> > This might be the best we can do, but it'd be nice if we could probe
> > or otherwise avoid relying on this assumption about the guest kernel.
> 
> I agree with you but none of the other architectures probes for the page
> size.
> 
> For instance ARM: |I cc: Drew to know how he has chosen the values]
> 
>     if (arm_feature(env, ARM_FEATURE_AARCH64)) {
> ...
>         info->page_size = (1 << 16);
> ...
>     } else {
> ...
>         info->page_size = (1 << 12);
> ...
>     }
>

info->page_size is used to determine the dumpfile's block size. The
block size needs to be at least the page size, but a multiple of page
size works fine too. As we can't probe for the currently used guest
page size, and a multiple of page size is fine, then using the guest's
maximum allowed page size is the best we can do.

Thanks,
drew

Re: [Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel

[Andrew Jones]

On Wed, Aug 03, 2016 at 09:55:07PM +0200, Laurent Vivier wrote:
> If we don't provide the page size in target-ppc:cpu_get_dump_info(),
> the default one (TARGET_PAGE_SIZE, 4KB) is used to create
> the compressed dump. It works fine with Macintosh, but not with
> pseries as the kernel default page size is 64KB.
> 
> Without this patch, if we generate a compressed dump in the QEMU monitor:
> 
>     (qemu) dump-guest-memory -z qemu.dump
> 
> This dump cannot be read by crash:
> 
>     # crash vmlinux qemu.dump
>     ...
>     WARNING: cannot translate vmemmap kernel virtual addresses:
>              commands requiring page structure contents will fail
>     ...
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  target-ppc/arch_dump.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/target-ppc/arch_dump.c b/target-ppc/arch_dump.c
> index df1fd8c..ad37a59 100644
> --- a/target-ppc/arch_dump.c
> +++ b/target-ppc/arch_dump.c
> @@ -220,6 +220,11 @@ int cpu_get_dump_info(ArchDumpInfo *info,
>      } else {
>          info->d_endian = ELFDATA2LSB;
>      }
> +    /* 64KB is the page size default for pseries kernel */

This comment should rather say '...is the max page size...' than
'default' to be accurate for the reasoning. I have a comment like
that in the arm version,

 info->page_size = (1 << 16); /* aarch64 max pagesize */

> +    if (strncmp(object_get_typename(qdev_get_machine()),
> +                "pseries-", 8) == 0) {
> +        info->page_size = (1U << 16);
> +    }
>  
>      return 0;
>  }
> -- 
> 2.5.5
> 
>

Otherwise,

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel

[Laurent Vivier]

On 05/08/2016 11:26, Andrew Jones wrote:
> On Thu, Aug 04, 2016 at 10:41:16AM +0200, Laurent Vivier wrote:
>>
>>
>> On 04/08/2016 04:38, David Gibson wrote:
>>> On Wed, Aug 03, 2016 at 09:55:07PM +0200, Laurent Vivier wrote:
>>>> If we don't provide the page size in target-ppc:cpu_get_dump_info(),
>>>> the default one (TARGET_PAGE_SIZE, 4KB) is used to create
>>>> the compressed dump. It works fine with Macintosh, but not with
>>>> pseries as the kernel default page size is 64KB.
>>>>
>>>> Without this patch, if we generate a compressed dump in the QEMU monitor:
>>>>
>>>>     (qemu) dump-guest-memory -z qemu.dump
>>>>
>>>> This dump cannot be read by crash:
>>>>
>>>>     # crash vmlinux qemu.dump
>>>>     ...
>>>>     WARNING: cannot translate vmemmap kernel virtual addresses:
>>>>              commands requiring page structure contents will fail
>>>>     ...
>>>>
>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>> ---
>>>>  target-ppc/arch_dump.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>
>>> Urgh.. so, really the page size used by the guest kernel is a
>>> guest-side detail, and it's certainly possible to build a 4kiB page
>>> guest kernel, although 64kiB is the norm.
>>
>> virtio-balloon doesn't work with 4K kernel.
>>
>>> This might be the best we can do, but it'd be nice if we could probe
>>> or otherwise avoid relying on this assumption about the guest kernel.
>>
>> I agree with you but none of the other architectures probes for the page
>> size.
>>
>> For instance ARM: |I cc: Drew to know how he has chosen the values]
>>
>>     if (arm_feature(env, ARM_FEATURE_AARCH64)) {
>> ...
>>         info->page_size = (1 << 16);
>> ...
>>     } else {
>> ...
>>         info->page_size = (1 << 12);
>> ...
>>     }
>>
> 
> info->page_size is used to determine the dumpfile's block size. The
> block size needs to be at least the page size, but a multiple of page
> size works fine too. As we can't probe for the currently used guest
> page size, and a multiple of page size is fine, then using the guest's
> maximum allowed page size is the best we can do.

Thank you for the explanation.

So we can unconditionally use 64KB, even for mac99 with a 64bit
processor or a 32bit processor (that are always 4K page size)?

The maximum page size in the kernel can be 256kB [1], should we use this
value instead?

Laurent

[1] linux/arch/powerpc/include/asm/page.h
/*
 * On regular PPC32 page size is 4K (but we support 4K/16K/64K/256K pages
 * on PPC44x). For PPC64 we support either 4K or 64K software
 * page size. When using 64K pages however, whether we are really supporting
 * 64K pages in HW or not is irrelevant to those definitions.
 */
#if defined(CONFIG_PPC_256K_PAGES)
#define PAGE_SHIFT              18
#elif defined(CONFIG_PPC_64K_PAGES)
#define PAGE_SHIFT              16
#elif defined(CONFIG_PPC_16K_PAGES)
#define PAGE_SHIFT              14
#else
#define PAGE_SHIFT              12
#endif

Re: [Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2547 bytes --]

On Fri, Aug 05, 2016 at 11:26:47AM +0200, Andrew Jones wrote:
> On Thu, Aug 04, 2016 at 10:41:16AM +0200, Laurent Vivier wrote:
> > 
> > 
> > On 04/08/2016 04:38, David Gibson wrote:
> > > On Wed, Aug 03, 2016 at 09:55:07PM +0200, Laurent Vivier wrote:
> > >> If we don't provide the page size in target-ppc:cpu_get_dump_info(),
> > >> the default one (TARGET_PAGE_SIZE, 4KB) is used to create
> > >> the compressed dump. It works fine with Macintosh, but not with
> > >> pseries as the kernel default page size is 64KB.
> > >>
> > >> Without this patch, if we generate a compressed dump in the QEMU monitor:
> > >>
> > >>     (qemu) dump-guest-memory -z qemu.dump
> > >>
> > >> This dump cannot be read by crash:
> > >>
> > >>     # crash vmlinux qemu.dump
> > >>     ...
> > >>     WARNING: cannot translate vmemmap kernel virtual addresses:
> > >>              commands requiring page structure contents will fail
> > >>     ...
> > >>
> > >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > >> ---
> > >>  target-ppc/arch_dump.c | 5 +++++
> > >>  1 file changed, 5 insertions(+)
> > > 
> > > Urgh.. so, really the page size used by the guest kernel is a
> > > guest-side detail, and it's certainly possible to build a 4kiB page
> > > guest kernel, although 64kiB is the norm.
> > 
> > virtio-balloon doesn't work with 4K kernel.
> > 
> > > This might be the best we can do, but it'd be nice if we could probe
> > > or otherwise avoid relying on this assumption about the guest kernel.
> > 
> > I agree with you but none of the other architectures probes for the page
> > size.
> > 
> > For instance ARM: |I cc: Drew to know how he has chosen the values]
> > 
> >     if (arm_feature(env, ARM_FEATURE_AARCH64)) {
> > ...
> >         info->page_size = (1 << 16);
> > ...
> >     } else {
> > ...
> >         info->page_size = (1 << 12);
> > ...
> >     }
> >
> 
> info->page_size is used to determine the dumpfile's block size. The
> block size needs to be at least the page size, but a multiple of page
> size works fine too. As we can't probe for the currently used guest
> page size, and a multiple of page size is fine, then using the guest's
> maximum allowed page size is the best we can do.

Aha!  Well that makes life much easier.  I'll adjust the commit
message and apply.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH] ppc64: fix compressed dump with pseries kernel

[Andrew Jones]

On Fri, Aug 05, 2016 at 11:46:33AM +0200, Laurent Vivier wrote:
> 
> 
> On 05/08/2016 11:26, Andrew Jones wrote:
> > On Thu, Aug 04, 2016 at 10:41:16AM +0200, Laurent Vivier wrote:
> >>
> >>
> >> On 04/08/2016 04:38, David Gibson wrote:
> >>> On Wed, Aug 03, 2016 at 09:55:07PM +0200, Laurent Vivier wrote:
> >>>> If we don't provide the page size in target-ppc:cpu_get_dump_info(),
> >>>> the default one (TARGET_PAGE_SIZE, 4KB) is used to create
> >>>> the compressed dump. It works fine with Macintosh, but not with
> >>>> pseries as the kernel default page size is 64KB.
> >>>>
> >>>> Without this patch, if we generate a compressed dump in the QEMU monitor:
> >>>>
> >>>>     (qemu) dump-guest-memory -z qemu.dump
> >>>>
> >>>> This dump cannot be read by crash:
> >>>>
> >>>>     # crash vmlinux qemu.dump
> >>>>     ...
> >>>>     WARNING: cannot translate vmemmap kernel virtual addresses:
> >>>>              commands requiring page structure contents will fail
> >>>>     ...
> >>>>
> >>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >>>> ---
> >>>>  target-ppc/arch_dump.c | 5 +++++
> >>>>  1 file changed, 5 insertions(+)
> >>>
> >>> Urgh.. so, really the page size used by the guest kernel is a
> >>> guest-side detail, and it's certainly possible to build a 4kiB page
> >>> guest kernel, although 64kiB is the norm.
> >>
> >> virtio-balloon doesn't work with 4K kernel.
> >>
> >>> This might be the best we can do, but it'd be nice if we could probe
> >>> or otherwise avoid relying on this assumption about the guest kernel.
> >>
> >> I agree with you but none of the other architectures probes for the page
> >> size.
> >>
> >> For instance ARM: |I cc: Drew to know how he has chosen the values]
> >>
> >>     if (arm_feature(env, ARM_FEATURE_AARCH64)) {
> >> ...
> >>         info->page_size = (1 << 16);
> >> ...
> >>     } else {
> >> ...
> >>         info->page_size = (1 << 12);
> >> ...
> >>     }
> >>
> > 
> > info->page_size is used to determine the dumpfile's block size. The
> > block size needs to be at least the page size, but a multiple of page
> > size works fine too. As we can't probe for the currently used guest
> > page size, and a multiple of page size is fine, then using the guest's
> > maximum allowed page size is the best we can do.
> 
> Thank you for the explanation.
> 
> So we can unconditionally use 64KB, even for mac99 with a 64bit
> processor or a 32bit processor (that are always 4K page size)?

If they're always going to be 4K, then I'd leave them 4K. This is because
I don't know enough about how the crash utility works today, or will work
in the future, to know what the trade-offs are. IOW, if we assume that
block-size == page-size is optimal, then we want that, or only to diverge
minimally from it when necessary. That said, from what I do know about
dumpfiles and the crash utility, I can't see how it would matter much,
except for wasting more space with the header block (the header block
isn't compressed)

> 
> The maximum page size in the kernel can be 256kB [1], should we use this
> value instead?

I like how you use machine type to decide. If you have machine types that
may use 256K, then you can add new conditions for them, but, for the
reason I stated above, I wouldn't unconditionally use a block size that's
much larger than necessary.

Thanks,
drew

> 
> Laurent
> 
> [1] linux/arch/powerpc/include/asm/page.h
> /*
>  * On regular PPC32 page size is 4K (but we support 4K/16K/64K/256K pages
>  * on PPC44x). For PPC64 we support either 4K or 64K software
>  * page size. When using 64K pages however, whether we are really supporting
>  * 64K pages in HW or not is irrelevant to those definitions.
>  */
> #if defined(CONFIG_PPC_256K_PAGES)
> #define PAGE_SHIFT              18
> #elif defined(CONFIG_PPC_64K_PAGES)
> #define PAGE_SHIFT              16
> #elif defined(CONFIG_PPC_16K_PAGES)
> #define PAGE_SHIFT              14
> #else
> #define PAGE_SHIFT              12
> #endif
> 
>