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 <lviv...@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