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 <address@hidden>
>>>> ---
>>>>  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