From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mx1.redhat.com ([209.132.183.28]) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WJUdt-0005CL-Gx for kexec@lists.infradead.org; Fri, 28 Feb 2014 21:06:50 +0000 Date: Fri, 28 Feb 2014 16:06:11 -0500 From: Vivek Goyal Subject: Re: [PATCH 11/11] kexec: Support for Kexec on panic using new system call Message-ID: <20140228210611.GB17195@redhat.com> References: <1390849071-21989-1-git-send-email-vgoyal@redhat.com> <1390849071-21989-12-git-send-email-vgoyal@redhat.com> <20140228172857.GG4326@pd.tnic> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140228172857.GG4326@pd.tnic> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "kexec" Errors-To: kexec-bounces+dwmw2=twosheds.infradead.org@lists.infradead.org To: Borislav Petkov Cc: mjg59@srcf.ucam.org, jkosina@suse.cz, greg@kroah.com, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, ebiederm@xmission.com, hpa@zytor.com T24gRnJpLCBGZWIgMjgsIDIwMTQgYXQgMDY6Mjg6NTdQTSArMDEwMCwgQm9yaXNsYXYgUGV0a292 IHdyb3RlOgoKWy4uXQo+ID4gKy8qIE1lbW9yeSB0byBiYWNrdXAgZHVyaW5nIGNyYXNoIGtkdW1w ICovCj4gPiArI2RlZmluZSBLRVhFQ19CQUNLVVBfU1JDX1NUQVJUCSgwVUwpCj4gPiArI2RlZmlu ZSBLRVhFQ19CQUNLVVBfU1JDX0VORAkoNjU1MzYwVUwpCS8qIDY0MEsgKi8KPiAKPiBJIGd1ZXNz Cj4gCj4gI2RlZmluZSBLRVhFQ19CQUNLVVBfU1JDX0VORAkoNjQwICogMTAyNFVMKQo+IAo+IHNo b3VsZCBiZSBtb3JlIGNsZWFyLgoKV2lsbCBDaGFuZ2UuCgpbLi5dCj4gPiArLyogQWxpZ25tZW50 IHJlcXVpcmVkIGZvciBlbGYgaGVhZGVyIHNlZ21lbnQgKi8KPiA+ICsjZGVmaW5lIEVMRl9DT1JF X0hFQURFUl9BTElHTiAgIDQwOTYKPiA+ICsKPiA+ICsvKiBUaGlzIHByaW1hcmlseSByZXByc2Vu dHMgbnVtYmVyIG9mIHNwbGl0IHJhbmdlcyBkdWUgdG8gZXhjbHVzaW9uICovCj4gPiArI2RlZmlu ZSBDUkFTSF9NQVhfUkFOR0VTCTE2Cj4gPiArCj4gPiArc3RydWN0IGNyYXNoX21lbV9yYW5nZSB7 Cj4gPiArCXVuc2lnbmVkIGxvbmcgbG9uZyBzdGFydCwgZW5kOwo+IAo+IHU2ND8KCk9rLCB0aGF0 J3Mgc2hvcnRlci4gQ2FuIHVzZSB0aGF0LgoKWy4uXQo+ID4gKwo+ID4gKy8qIFVzZWQgd2hpbGUg cHJlcGFyZWluZyBtZW1vcnkgbWFwIGVudHJpZXMgZm9yIHNlY29uZCBrZXJuZWwgKi8KPiAKPiBz L3ByZXBhcmVpbmcvcHJlcGFyaW5nLwoKWWVwIHR5cG8uIFdpbGwgZml4LgoKWy4uXQo+ID4gK3N0 YXRpYyBpbnQgZ2V0X2dhcnRfcmFuZ2VzX2NhbGxiYWNrKHU2NCBzdGFydCwgdTY0IGVuZCwgdm9p ZCAqYXJnKQo+ID4gK3sKPiA+ICsJc3RydWN0IGNyYXNoX2VsZl9kYXRhICpjZWQgPSBhcmc7Cj4g PiArCj4gPiArCWNlZC0+Z2FydF9zdGFydCA9IHN0YXJ0Owo+ID4gKwljZWQtPmdhcnRfZW5kID0g ZW5kOwo+ID4gKwo+ID4gKwkvKiBOb3QgZXhwZWN0aW5nIG1vcmUgdGhhbiAxIGdhcnQgYXBlcnR1 cmUgKi8KPiA+ICsJcmV0dXJuIDE7Cj4gPiArfQo+ID4gKwo+ID4gKwo+ID4gKy8qIEdhdGhlciBh bGwgdGhlIHJlcXVpcmVkIGluZm9ybWF0aW9uIHRvIHByZXBhcmUgZWxmIGhlYWRlcnMgZm9yIHJh bSByZWdpb25zICovCj4gPiArc3RhdGljIGludCBmaWxsX3VwX2NlZChzdHJ1Y3QgY3Jhc2hfZWxm X2RhdGEgKmNlZCwgc3RydWN0IGtpbWFnZSAqaW1hZ2UpCj4gCj4gQWxsIG90aGVyIGZ1bmN0aW9u cyBoYXZlIG5pY2UsIHNwZWxsZWQgb3V0IG5hbWVzIGJ1dCBub3QgdGhpcyBvbmUgOikKPiAKPiBX aHkgbm90IGZpbGxfdXBfY3Jhc2hfZWxmX2RhdGEoKT8KCldpbGwgY2hhbmdlIGl0LgoKPiAKPiA+ ICt7Cj4gPiArCXVuc2lnbmVkIGludCBucl9yYW5nZXMgPSAwOwo+ID4gKwo+ID4gKwljZWQtPmlt YWdlID0gaW1hZ2U7Cj4gPiArCj4gPiArCXdhbGtfc3lzdGVtX3JhbV9yYW5nZSgwLCAtMSwgJm5y X3JhbmdlcywKPiA+ICsJCQkJZ2V0X25yX3JhbV9yYW5nZXNfY2FsbGJhY2spOwo+ID4gKwo+ID4g KwljZWQtPm1heF9ucl9yYW5nZXMgPSBucl9yYW5nZXM7Cj4gPiArCj4gPiArCS8qCj4gPiArCSAq IFdlIGRvbid0IGNyZWF0ZSBFTEYgaGVhZGVycyBmb3IgR0FSVCBhcGVydHVyZSBhcyBhbiBhdHRl bXB0Cj4gPiArCSAqIHRvIGR1bXAgdGhpcyBtZW1vcnkgaW4gc2Vjb25kIGtlcm5lbCBsZWFkcyB0 byBoYW5nL2NyYXNoLgo+ID4gKwkgKiBJZiBnYXJ0IGFwZXJ0dXJlIGlzIHByZXNlbnQsIG9uZSBu ZWVkcyB0byBleGNsdWRlIHRoYXQgcmVnaW9uCj4gPiArCSAqIGFuZCB0aGF0IGNvdWxkIGxlYWQg dG8gbmVlZCBvZiBleHRyYSBwaGRyLgo+ID4gKwkgKi8KPiA+ICsKPiAKPiBzdXBlcmZsdW91cyBu ZXdsaW5lLgoKV2lsbCByZW1vdmUuCgpbLi5dCj4gPiAraW50IGxvYWRfY3Jhc2hkdW1wX3NlZ21l bnRzKHN0cnVjdCBraW1hZ2UgKmltYWdlKQo+ID4gK3sKPiA+ICsJdW5zaWduZWQgbG9uZyBzcmNf c3RhcnQsIHNyY19zejsKPiA+ICsJdW5zaWduZWQgbG9uZyBlbGZfYWRkciwgZWxmX3N6Owo+ID4g KwlpbnQgcmV0Owo+ID4gKwo+ID4gKwkvKgo+ID4gKwkgKiBEZXRlcm1pbmUgYW5kIGxvYWQgYSBz ZWdtZW50IGZvciBiYWNrdXAgYXJlYS4gRmlyc3QgNjQwSyBSQU0KPiA+ICsJICogcmVnaW9uIGlz IGJhY2t1cCBzb3VyY2UKPiA+ICsJICovCj4gPiArCj4gPiArCXJldCA9IHdhbGtfc3lzdGVtX3Jh bV9yZXMoS0VYRUNfQkFDS1VQX1NSQ19TVEFSVCwgS0VYRUNfQkFDS1VQX1NSQ19FTkQsCj4gPiAr CQkJCWltYWdlLCBkZXRlcm1pbmVfYmFja3VwX3JlZ2lvbik7Cj4gPiArCj4gPiArCS8qIFplcm8g b3IgcG9zdGl2ZSByZXR1cm4gdmFsdWVzIGFyZSBvayAqLwo+ID4gKwlpZiAocmV0IDwgMCkKPiA+ ICsJCXJldHVybiByZXQ7Cj4gPiArCj4gPiArCXNyY19zdGFydCA9IGltYWdlLT5hcmNoLmJhY2t1 cF9zcmNfc3RhcnQ7Cj4gPiArCXNyY19zeiA9IGltYWdlLT5hcmNoLmJhY2t1cF9zcmNfc3o7Cj4g PiArCj4gPiArCS8qIEFkZCBiYWNrdXAgc2VnbWVudC4gKi8KPiA+ICsJaWYgKHNyY19zeikgewo+ ID4gKwkJcmV0ID0ga2V4ZWNfYWRkX2J1ZmZlcihpbWFnZSwgX192YShzcmNfc3RhcnQpLCBzcmNf c3osIHNyY19zeiwKPiA+ICsJCQkJCVBBR0VfU0laRSwgMCwgLTEsIDAsCj4gPiArCQkJCQkmaW1h Z2UtPmFyY2guYmFja3VwX2xvYWRfYWRkcik7Cj4gPiArCQlpZiAocmV0KQo+ID4gKwkJCXJldHVy biByZXQ7Cj4gPiArCX0KPiA+ICsKPiA+ICsJLyogUHJlcGFyZSBlbGYgaGVhZGVycyBhbmQgYWRk IGEgc2VnbWVudCAqLwo+ID4gKwlyZXQgPSBwcmVwYXJlX2VsZl9oZWFkZXJzKGltYWdlLCAmZWxm X2FkZHIsICZlbGZfc3opOwo+ID4gKwlpZiAocmV0KQo+ID4gKwkJcmV0dXJuIHJldDsKPiA+ICsK PiA+ICsJaW1hZ2UtPmFyY2guZWxmX2hlYWRlcnMgPSBlbGZfYWRkcjsKPiA+ICsJaW1hZ2UtPmFy Y2guZWxmX2hlYWRlcnNfc3ogPSBlbGZfc3o7Cj4gPiArCj4gPiArCXJldCA9IGtleGVjX2FkZF9i dWZmZXIoaW1hZ2UsIChjaGFyICopZWxmX2FkZHIsIGVsZl9zeiwgZWxmX3N6LAo+IAo+IEZvciBz b21lIHJlYXNvbiwgbXkgY29tcGlsZXIgY29tcGxhaW5zIGhlcmU6Cj4gCj4gYXJjaC94ODYva2Vy bmVsL2NyYXNoLmM6IEluIGZ1bmN0aW9uIOKAmGxvYWRfY3Jhc2hkdW1wX3NlZ21lbnRz4oCZOgo+ IGFyY2gveDg2L2tlcm5lbC9jcmFzaC5jOjcwNDo2OiB3YXJuaW5nOiDigJhlbGZfc3rigJkgbWF5 IGJlIHVzZWQgdW5pbml0aWFsaXplZCBpbiB0aGlzIGZ1bmN0aW9uIFstV3VuaW5pdGlhbGl6ZWRd Cj4gYXJjaC94ODYva2VybmVsL2NyYXNoLmM6NzA0OjI0OiB3YXJuaW5nOiDigJhlbGZfYWRkcuKA mSBtYXkgYmUgdXNlZCB1bmluaXRpYWxpemVkIGluIHRoaXMgZnVuY3Rpb24gWy1XdW5pbml0aWFs aXplZF0KPiAKPiBJdCBpcyBsaWtlbHkgYm9ndXMsIHRob3VnaC4KCkhtbS4uLiwgSSBkaWQgbm90 IHNlZSB0aGVzZSB3YXJuaW5ncyB3aXRoIG15IHNldHVwLiBlbGZfYWRkciBhbmQgZWxmX3N6Cndp bGwgYmUgaW5pdGlhbGl6ZWQgYnkgcHJlcGFyZV9lbGZfaGVhZGVycygpLiBOb3Qgc3VyZSB3aHkg Y29tcGlsZXIgaXMKY29tcGxhaW5pbmcuCgpbLi5dCj4gPiArCXByX2RlYnVnKCJGaW5hbCBjb21t YW5kIGxpbmUgaXM6JXNcbiIsIGNtZGxpbmVfcHRyKTsKPiAKPiBvbmUgc3BhY2UgYWZ0ZXIgIjoi CgpPay4gd2lsbCBkby4KCj4gCj4gVGhlIHJlc3QgbG9va3Mgb2sgdG8gbWUsIGJ1dCB0aGF0IGRv ZXNuJ3QgbWVhbiBhIHdob2xlIGxvdCBjb25zaWRlcmluZwo+IG15IHZlcnkgbGltaXRlZCBrZXhl YyBrbm93bGVkZ2UuCgpUaGFua3MgZm9yIHJldmlldy4gV2UgbmVlZCBtYW55IGV5ZXMgb24gdGhp cyBwYXRjaCBzZXQuIEkgd2lsbCBtYWtlCmNoYW5nZXMgYW5kIHBvc3QgYW5vdGhlciB2ZXJzaW9u IGZvciByZXZpZXcuCgpUaGFua3MKVml2ZWsKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fCmtleGVjIG1haWxpbmcgbGlzdAprZXhlY0BsaXN0cy5pbmZyYWRl YWQub3JnCmh0dHA6Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8va2V4ZWMK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752552AbaB1VGz (ORCPT ); Fri, 28 Feb 2014 16:06:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42445 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752464AbaB1VGx (ORCPT ); Fri, 28 Feb 2014 16:06:53 -0500 Date: Fri, 28 Feb 2014 16:06:11 -0500 From: Vivek Goyal To: Borislav Petkov Cc: linux-kernel@vger.kernel.org, kexec@lists.infradead.org, ebiederm@xmission.com, hpa@zytor.com, mjg59@srcf.ucam.org, greg@kroah.com, jkosina@suse.cz Subject: Re: [PATCH 11/11] kexec: Support for Kexec on panic using new system call Message-ID: <20140228210611.GB17195@redhat.com> References: <1390849071-21989-1-git-send-email-vgoyal@redhat.com> <1390849071-21989-12-git-send-email-vgoyal@redhat.com> <20140228172857.GG4326@pd.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20140228172857.GG4326@pd.tnic> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 28, 2014 at 06:28:57PM +0100, Borislav Petkov wrote: [..] > > +/* Memory to backup during crash kdump */ > > +#define KEXEC_BACKUP_SRC_START (0UL) > > +#define KEXEC_BACKUP_SRC_END (655360UL) /* 640K */ > > I guess > > #define KEXEC_BACKUP_SRC_END (640 * 1024UL) > > should be more clear. Will Change. [..] > > +/* Alignment required for elf header segment */ > > +#define ELF_CORE_HEADER_ALIGN 4096 > > + > > +/* This primarily reprsents number of split ranges due to exclusion */ > > +#define CRASH_MAX_RANGES 16 > > + > > +struct crash_mem_range { > > + unsigned long long start, end; > > u64? Ok, that's shorter. Can use that. [..] > > + > > +/* Used while prepareing memory map entries for second kernel */ > > s/prepareing/preparing/ Yep typo. Will fix. [..] > > +static int get_gart_ranges_callback(u64 start, u64 end, void *arg) > > +{ > > + struct crash_elf_data *ced = arg; > > + > > + ced->gart_start = start; > > + ced->gart_end = end; > > + > > + /* Not expecting more than 1 gart aperture */ > > + return 1; > > +} > > + > > + > > +/* Gather all the required information to prepare elf headers for ram regions */ > > +static int fill_up_ced(struct crash_elf_data *ced, struct kimage *image) > > All other functions have nice, spelled out names but not this one :) > > Why not fill_up_crash_elf_data()? Will change it. > > > +{ > > + unsigned int nr_ranges = 0; > > + > > + ced->image = image; > > + > > + walk_system_ram_range(0, -1, &nr_ranges, > > + get_nr_ram_ranges_callback); > > + > > + ced->max_nr_ranges = nr_ranges; > > + > > + /* > > + * We don't create ELF headers for GART aperture as an attempt > > + * to dump this memory in second kernel leads to hang/crash. > > + * If gart aperture is present, one needs to exclude that region > > + * and that could lead to need of extra phdr. > > + */ > > + > > superfluous newline. Will remove. [..] > > +int load_crashdump_segments(struct kimage *image) > > +{ > > + unsigned long src_start, src_sz; > > + unsigned long elf_addr, elf_sz; > > + int ret; > > + > > + /* > > + * Determine and load a segment for backup area. First 640K RAM > > + * region is backup source > > + */ > > + > > + ret = walk_system_ram_res(KEXEC_BACKUP_SRC_START, KEXEC_BACKUP_SRC_END, > > + image, determine_backup_region); > > + > > + /* Zero or postive return values are ok */ > > + if (ret < 0) > > + return ret; > > + > > + src_start = image->arch.backup_src_start; > > + src_sz = image->arch.backup_src_sz; > > + > > + /* Add backup segment. */ > > + if (src_sz) { > > + ret = kexec_add_buffer(image, __va(src_start), src_sz, src_sz, > > + PAGE_SIZE, 0, -1, 0, > > + &image->arch.backup_load_addr); > > + if (ret) > > + return ret; > > + } > > + > > + /* Prepare elf headers and add a segment */ > > + ret = prepare_elf_headers(image, &elf_addr, &elf_sz); > > + if (ret) > > + return ret; > > + > > + image->arch.elf_headers = elf_addr; > > + image->arch.elf_headers_sz = elf_sz; > > + > > + ret = kexec_add_buffer(image, (char *)elf_addr, elf_sz, elf_sz, > > For some reason, my compiler complains here: > > arch/x86/kernel/crash.c: In function ‘load_crashdump_segments’: > arch/x86/kernel/crash.c:704:6: warning: ‘elf_sz’ may be used uninitialized in this function [-Wuninitialized] > arch/x86/kernel/crash.c:704:24: warning: ‘elf_addr’ may be used uninitialized in this function [-Wuninitialized] > > It is likely bogus, though. Hmm..., I did not see these warnings with my setup. elf_addr and elf_sz will be initialized by prepare_elf_headers(). Not sure why compiler is complaining. [..] > > + pr_debug("Final command line is:%s\n", cmdline_ptr); > > one space after ":" Ok. will do. > > The rest looks ok to me, but that doesn't mean a whole lot considering > my very limited kexec knowledge. Thanks for review. We need many eyes on this patch set. I will make changes and post another version for review. Thanks Vivek