From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 00/04] Kexec / Kdump: Release 20061122 (xen-unstable-12502) Date: Wed, 22 Nov 2006 18:24:08 +0000 Message-ID: <1164219848.12608.78.camel@localhost.localdomain> References: <20061122071050.24010.92547.sendpatchset@localhost> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20061122071050.24010.92547.sendpatchset@localhost> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Magnus Damm Cc: Ian Pratt , Kazuo Moriwaka , xen-devel@lists.xensource.com, magnus.damm@gmail.com, Akio Takebe , Isaku Yamahata , Horms List-Id: xen-devel@lists.xenproject.org Hi Magnus, On Wed, 2006-11-22 at 16:10 +0900, Magnus Damm wrote: > Here comes a new version of the Kexec / Kdump patches for x86 Xen. Not much > has changed since last release, just a minor fix for kdump on x86_64. > > Patches to make kexec-tools Xen aware have recently been sent to the fastboot > list. These patches will be merged in the kexec-tools-testing tree in the > near future. I've taken these patches out for a spin. They look pretty good. I've got a couple of comments. Firstly the patches break native kernel compile. You add usages of kexec_page_to_pfn() and friends to kernel/kexec.c but only include kexec-xen.h ifdef CONFIG_XEN. I fixed it with by removing the ifdef but the preferred way would be to move the native definitions of kexec_* into include/asm-i386/kexec.h and make a xen specific copy in include/asm-i386/mach-xen/asm/kexec.h with the xen versions patched in. Alternatively you could just merge kexec-xen.h into kexec.h. My second comment is WRT to the ELF notes which you add to the kdump. You include a standard PRSTATUS core ELF note per physical CPU but there is some useful physical processor state which is not included in this structure -- most importantly CR3. Since the amount of physical CPU state which is not already included in PRSTATUS is pretty small I think you could just include the whole lot in a Xen specific note per PCPU. I'd basically include anything which is in a Xen panic/oops message but not in PRSTATUS, that's C[0,2,3,4]. Including the debug registers might be handy too. If there was some standard extended PRSTATUS note format for these extra things we could use that would be even better but I don't know of one (but then I don't really know about these things ;-)). You also store dom0's pfn_to_mfn_frame_list_list in a Xen specific note. What is that used for? Given a Xen symbol table it should be possible to locate the shared info for any domain via the xen mappings and hence find the p2m table that way. m2p is at a known virtual address already. The contents of the h/v taint bitmap would be another interesting thing to include in the Xen note. Cheers, Ian.