From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0639FC10F11 for ; Wed, 10 Apr 2019 23:37:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CC7FD20645 for ; Wed, 10 Apr 2019 23:37:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726830AbfDJXhG (ORCPT ); Wed, 10 Apr 2019 19:37:06 -0400 Received: from tyo162.gate.nec.co.jp ([114.179.232.162]:58629 "EHLO tyo162.gate.nec.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726215AbfDJXhG (ORCPT ); Wed, 10 Apr 2019 19:37:06 -0400 Received: from mailgate02.nec.co.jp ([114.179.233.122]) by tyo162.gate.nec.co.jp (8.15.1/8.15.1) with ESMTPS id x3ANah5O015380 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 11 Apr 2019 08:36:43 +0900 Received: from mailsv01.nec.co.jp (mailgate-v.nec.co.jp [10.204.236.94]) by mailgate02.nec.co.jp (8.15.1/8.15.1) with ESMTP id x3ANagIs027706; Thu, 11 Apr 2019 08:36:43 +0900 Received: from mail01b.kamome.nec.co.jp (mail01b.kamome.nec.co.jp [10.25.43.2]) by mailsv01.nec.co.jp (8.15.1/8.15.1) with ESMTP id x3ANaPuq031742; Thu, 11 Apr 2019 08:36:42 +0900 Received: from bpxc99gp.gisp.nec.co.jp ([10.38.151.135] [10.38.151.135]) by mail01b.kamome.nec.co.jp with ESMTP id BT-MMP-4172004; Thu, 11 Apr 2019 08:34:53 +0900 Received: from BPXM12GP.gisp.nec.co.jp ([10.38.151.204]) by BPXC07GP.gisp.nec.co.jp ([10.38.151.135]) with mapi id 14.03.0319.002; Thu, 11 Apr 2019 08:34:52 +0900 From: Junichi Nomura To: Borislav Petkov CC: Dave Young , Chao Fan , Baoquan He , Kairui Song , "x86@kernel.org" , "kexec@lists.infradead.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v4] x86/boot: Use efi_setup_data for searching RSDP on kexec-ed kernel Thread-Topic: [PATCH v4] x86/boot: Use efi_setup_data for searching RSDP on kexec-ed kernel Thread-Index: AQHU78DnNYRsXw6c0kywKGsyat34OKY1dZqA Date: Wed, 10 Apr 2019 23:34:51 +0000 Message-ID: <7cbc096d-0548-18b1-a335-8ba114f234a7@ce.jp.nec.com> References: <20190408231011.GA5402@jeru.linux.bs1.fc.nec.co.jp> <20190410171431.GE26580@zn.tnic> In-Reply-To: <20190410171431.GE26580@zn.tnic> Accept-Language: en-US, ja-JP Content-Language: ja-JP X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.34.125.85] Content-Type: text/plain; charset="utf-8" Content-ID: <42379DDBC7FCB549917A8ED9EFB6AABE@gisp.nec.co.jp> Content-Transfer-Encoding: base64 MIME-Version: 1.0 X-TM-AS-MML: disable Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org T24gNC8xMS8xOSAyOjE0IEFNLCBCb3Jpc2xhdiBQZXRrb3Ygd3JvdGU6DQo+IE9uIE1vbiwgQXBy IDA4LCAyMDE5IGF0IDExOjEwOjE3UE0gKzAwMDAsIEp1bmljaGkgTm9tdXJhIHdyb3RlOg0KPj4g LSNpZmRlZiBDT05GSUdfRUZJDQo+PiAtCXVuc2lnbmVkIGxvbmcgc3lzdGFiLCBzeXN0YWJfdGFi bGVzLCBjb25maWdfdGFibGVzOw0KPj4gLQl1bnNpZ25lZCBpbnQgbnJfdGFibGVzOw0KPj4gK3N0 YXRpYyB2b2lkIGVmaV9yZWFkX2Jvb3RfcGFyYW1zKHZvaWQpDQo+PiArew0KPj4gKwlzdHJ1Y3Qg ZWZpX3NldHVwX2RhdGEgKmVzZDsNCj4+ICAJc3RydWN0IGVmaV9pbmZvICplaTsNCj4+IC0JYm9v bCBlZmlfNjQ7DQo+PiAtCWludCBzaXplLCBpOw0KPj4gIAljaGFyICpzaWc7DQo+PiAgDQo+PiAr CWtleGVjX2VmaV9zZXR1cF9kYXRhID0gZWZpX2dldF9rZXhlY19zZXR1cF9kYXRhX2FkZHIoKTsN Cj4gDQo+IFdoeSBpcyB0aGF0IHdyaXR0ZW4gaGVyZSBhbmQgdGVzdGVkIGluIGFub3RoZXIgZnVu Y3Rpb24/IT8NCg0KQm90aCBlZmlfZ2V0X3JzZHBfYWRkcigpIGFuZCBrZXhlY19nZXRfcnNkcF9h ZGRyKCkgbmVlZCB0byBjaGVjaw0KdGhlIHJlc3VsdCBvZiBlZmlfZ2V0X2tleGVjX3NldHVwX2Rh dGFfYWRkcigpOyB0aGUgZm9ybWVyIHRvIGNoZWNrDQp3aGV0aGVyIHRvIGV4aXQgZWFybHksIHRo ZSBsYXR0ZXIgdG8gdXNlIHRoZSBhZGRyZXNzIG9mIHRoZSB0YWJsZXMuDQpJIHRob3VnaHQgaXQn cyBiZXR0ZXIgdG8gc3RvcmUgdGhlIHJlc3VsdCBpbnN0ZWFkIG9mIGNhbGxpbmcgdHdpY2UuDQoN Cj4+ICsNCj4+ICAJZWkgPSAmYm9vdF9wYXJhbXMtPmVmaV9pbmZvOw0KPj4gIAlzaWcgPSAoY2hh ciAqKSZlaS0+ZWZpX2xvYWRlcl9zaWduYXR1cmU7DQo+PiAgDQo+PiAgCWlmICghc3RybmNtcChz aWcsIEVGSTY0X0xPQURFUl9TSUdOQVRVUkUsIDQpKSB7DQo+PiAgCQllZmlfNjQgPSB0cnVlOw0K Pj4gKwkJZWZpX2Jvb3RlZCA9IHRydWU7DQo+IA0KPiBXaGF0IGlzIHRoYXQgdWdsaW5lc3MgZm9y PyBIYXZlIHlvdSBoZWFyZCBvZiBmdW5jdGlvbnMgcmV0dXJuaW5nIHZhbHVlcz8NCg0KU2FtZSBh cyBhYm92ZS4gSSBkaWRuJ3Qgd2FudCB0byBkbyBzaWduYXR1cmUgY2hlY2sgdHdpY2UsIGluDQpl ZmlfZ2V0X3JzZHBfYWRkcigpIGFuZCBrZXhlY19nZXRfcnNkcF9hZGRyKCkuDQpBbHNvLCB0aGUg c2lnbmF0dXJlIGNoZWNrIGhhcyAyIHJldHVybiB2YWx1ZXMsIHdoZXRoZXIgaXQgd2FzIDMyYml0 DQpvciA2NGJpdCwgYW5kIHdoZXRoZXIgdGhlIHNpZ25hdHVyZSB3YXMgdmFsaWQgb3Igbm90Lg0K SSBjb3VsZCByZXR1cm4gb25lIG9mIHRoZW0gdmlhIHBvaW50ZXIgcGFzc2VkIHBhcmFtZXRlciBi dXQgSSB0aG91Z2h0DQppdCdzIGEgbGl0dGxlIGJpdCB1Z2x5LiAgT3IgSSBjb3VsZCBlbmNvZGUg dGhlbSBhcyBzb21ldGhpbmcgbGlrZQ0KRUZJX1NJR05BVFVSRV82NCwgRUZJX1NJR05BVFVSRV8z MiwgYW5kIEVGSV9TSUdOQVRVUkVfSU5WQUxJRC4NCkJ1dCBJJ20gbm90IHN1cmUgaXQncyBnb29k IHRvIGludHJvZHVjZSBzdWNoIGEgdGhpbmcganVzdCBmb3IgaGVyZS4NCg0KPj4gK3N0YXRpYyBh Y3BpX3BoeXNpY2FsX2FkZHJlc3MgZWZpX2dldF9yc2RwX2FkZHIodm9pZCkNCj4+ICt7DQo+PiAr I2lmZGVmIENPTkZJR19FRkkNCj4+ICsJdW5zaWduZWQgbG9uZyBjb25maWdfdGFibGVzOw0KPj4g Kwl1bnNpZ25lZCBpbnQgbnJfdGFibGVzOw0KPj4gKw0KPj4gKwllZmlfcmVhZF9ib290X3BhcmFt cygpOw0KPiANCj4gV2h5IGRvIHlvdSByZWFkIGJvb3QgcGFyYW1zIGhlcmU/DQo+IA0KPiBObywg bm8sIG5vLg0KPiANCj4gRmlyc3QgeW91IGRvDQo+IA0KPiAJZWZpX2dldF9yc2RwX2FkZHIoKQ0K PiANCj4gaWYgeW91IGNhbm5vdCBnZXQgYW4gYWRkcmVzcywgeW91DQoNCkJ1dCBlZmlfZ2V0X3Jz ZHBfYWRkcigpIG5lZWRzIHRvIGNoZWNrIHdoZXRoZXIgdGhlIGtlcm5lbCB3YXMNCmtleGVjIGJv b3RlZCB0byBhdm9pZCBhY2Nlc3NpbmcgaW52YWxpZCBFRkkgdGFibGUgYWRkcmVzcy4NCmVmaV9n ZXRfa2V4ZWNfc2V0dXBfZGF0YV9hZGRyKCkgaXMgdGhlIG9ubHkgbWV0aG9kIEkga25vdw0KdG8g Y2hlY2sgaWYgaXQgd2FzIGtleGVjLWJvb3RlZC4NCg0KPiAJLSBwYXJzZSBib290IHBhcmFtcw0K PiAJLSB0aGVuIHBhcnNlIEVGSSB0YWJsZXMgZnJvbSB0aGUgYWRkcmVzcyB0aGUga2V4ZWNlZCBr ZXJuZWwgcmVjZWl2ZWQNCj4gDQo+IE5vIGludGVybWl4aW5nIG9mIGNvZGUgcGF0aHMgYW5kIGFz c2lnbmluZyB2YXJpYWJsZXMgaW4gb25lIGZ1bmN0aW9uIGFuZA0KPiB1c2luZyB0aGVtIGluIGFu b3RoZXIuDQoNClllYWgsIEkgZG9uJ3QgbGlrZSB0aGF0LiBCdXQgaWYgd2UgYXJlIHRvIGhhbmRs ZSAzMmJpdCBFRkkgY2FzZSwNCmVmaV9nZXRfcnNkcF9hZGRyKCkgYW5kIGtleGVjX2dldF9yc2Rw X2FkZHIoKSBiZWNvbWUgZnVsbCBvZg0KZHVwbGljYXRpb24uDQoNCj4gWW91IHdlcmUgb24gdGhl IHJpZ2h0IHRyYWNrIHdpdGggdjMuLi4NCg0KLS0gDQpKdW4naWNoaSBOb211cmEsIE5FQyBDb3Jw b3JhdGlvbiAvIE5FQyBTb2x1dGlvbiBJbm5vdmF0b3JzLCBMdGQu From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from tyo162.gate.nec.co.jp ([114.179.232.162]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hEMm3-00084B-8s for kexec@lists.infradead.org; Wed, 10 Apr 2019 23:37:01 +0000 From: Junichi Nomura Subject: Re: [PATCH v4] x86/boot: Use efi_setup_data for searching RSDP on kexec-ed kernel Date: Wed, 10 Apr 2019 23:34:51 +0000 Message-ID: <7cbc096d-0548-18b1-a335-8ba114f234a7@ce.jp.nec.com> References: <20190408231011.GA5402@jeru.linux.bs1.fc.nec.co.jp> <20190410171431.GE26580@zn.tnic> In-Reply-To: <20190410171431.GE26580@zn.tnic> Content-Language: ja-JP Content-ID: <42379DDBC7FCB549917A8ED9EFB6AABE@gisp.nec.co.jp> MIME-Version: 1.0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Borislav Petkov Cc: Chao Fan , Kairui Song , Baoquan He , "x86@kernel.org" , "kexec@lists.infradead.org" , "linux-kernel@vger.kernel.org" , Dave Young On 4/11/19 2:14 AM, Borislav Petkov wrote: > On Mon, Apr 08, 2019 at 11:10:17PM +0000, Junichi Nomura wrote: >> -#ifdef CONFIG_EFI >> - unsigned long systab, systab_tables, config_tables; >> - unsigned int nr_tables; >> +static void efi_read_boot_params(void) >> +{ >> + struct efi_setup_data *esd; >> struct efi_info *ei; >> - bool efi_64; >> - int size, i; >> char *sig; >> >> + kexec_efi_setup_data = efi_get_kexec_setup_data_addr(); > > Why is that written here and tested in another function?!? Both efi_get_rsdp_addr() and kexec_get_rsdp_addr() need to check the result of efi_get_kexec_setup_data_addr(); the former to check whether to exit early, the latter to use the address of the tables. I thought it's better to store the result instead of calling twice. >> + >> ei = &boot_params->efi_info; >> sig = (char *)&ei->efi_loader_signature; >> >> if (!strncmp(sig, EFI64_LOADER_SIGNATURE, 4)) { >> efi_64 = true; >> + efi_booted = true; > > What is that ugliness for? Have you heard of functions returning values? Same as above. I didn't want to do signature check twice, in efi_get_rsdp_addr() and kexec_get_rsdp_addr(). Also, the signature check has 2 return values, whether it was 32bit or 64bit, and whether the signature was valid or not. I could return one of them via pointer passed parameter but I thought it's a little bit ugly. Or I could encode them as something like EFI_SIGNATURE_64, EFI_SIGNATURE_32, and EFI_SIGNATURE_INVALID. But I'm not sure it's good to introduce such a thing just for here. >> +static acpi_physical_address efi_get_rsdp_addr(void) >> +{ >> +#ifdef CONFIG_EFI >> + unsigned long config_tables; >> + unsigned int nr_tables; >> + >> + efi_read_boot_params(); > > Why do you read boot params here? > > No, no, no. > > First you do > > efi_get_rsdp_addr() > > if you cannot get an address, you But efi_get_rsdp_addr() needs to check whether the kernel was kexec booted to avoid accessing invalid EFI table address. efi_get_kexec_setup_data_addr() is the only method I know to check if it was kexec-booted. > - parse boot params > - then parse EFI tables from the address the kexeced kernel received > > No intermixing of code paths and assigning variables in one function and > using them in another. Yeah, I don't like that. But if we are to handle 32bit EFI case, efi_get_rsdp_addr() and kexec_get_rsdp_addr() become full of duplication. > You were on the right track with v3... -- Jun'ichi Nomura, NEC Corporation / NEC Solution Innovators, Ltd. _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec