Re: [BUG] can't boot up: unable to handle kernel paging request at ffffffffff340003
m...@console-pimps.org writes: On Mon, 10 Feb, at 03:23:33PM, Madper Xie wrote: Howdy, With old kernel (from 3.10 to 3.14-rc1), my hp box shows following outputs: ~~~ [0.009166] Freeing SMP alternatives memory: 20K (82234000 - 82239000) [0.010302] ioremap: invalid physical address 1376e0180001 [0.010303] [ cut here ] [0.010308] WARNING: CPU: 0 PID: 0 at arch/x86/mm/ioremap.c:83 __ioremap_caller+0x357/0x380() OK, this looks like a bogus BGRT address, that's a 64-bit physical address. I suspect the BGRT address is actually 0x1376e018, but that's just a guess. This looks like a BIOS bug. ~~~ However the box can boot up and works well. After I update kernel to 3.14-rc2, it panic when booting. Here is console output: ~~~ [0.010366] BUG: unable to handle kernel paging request at ff340003 [0.017376] IP: [81d85ba4] efi_bgrt_init+0x9d/0x133 [0.023153] PGD 159eb067 PUD 159ed067 PMD 16240067 PTE 9376e0180163 [0.029842] Oops: 0009 [#1] SMP Well, nuts. I think this is my bad. Does reverting commit 081cd62a010f (x86/efi: Allow mapping BGRT on x86-32) make things work? Or at least, work as well as they did prior to -rc2? Sure. Yes, after revert 081cd62a it can boot up. (Of cause the ioremap failed as before.) [0.010340] ioremap: invalid physical address 1376e0180001 [0.010342] [ cut here ] [0.010346] WARNING: CPU: 0 PID: 0 at arch/x86/mm/ioremap.c:83 __ioremap_caller+0x357/0x380() I wonder if we're calling early_memremap() when the fixmap slots are no longer valid. Could you try building the 'urgent' branch at, git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git it doesn't help, seems the same panic output: [0.001341] BUG: unable to handle kernel paging request at ff340003 [0.008339] IP: [81a73794] efi_bgrt_init+0x9d/0x133 [0.014111] PGD 28fb067 PUD 28fd067 PMD 2bef067 PTE 9376e0180163 [0.020530] Oops: 0009 [#1] SMP [0.023792] Modules linked in: [0.026864] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.14.0-rc2+ #1 [0.033213] Hardware name: Hewlett-Packard HP Compaq Elite 8300 SFF/3397, BIOS K01 v02.05 05/07/2012 [0.042341] task: 818ff460 ti: 818ec000 task.ti: 818ec000 [0.049822] RIP: 0010:[81a73794] [81a73794] efi_bgrt_init+0x9d/0x133 [0.058014] RSP: :818edee0 EFLAGS: 00010202 [0.063325] RAX: ff340001 RBX: ff340001 RCX: 0abf [0.070456] RDX: 8163 RSI: 9376e0180163 RDI: ff34 [0.077588] RBP: 818edef8 R08: 8163 R09: 8163 [0.084720] R10: d9bb5000 R11: 8118907f R12: 0001 [0.091853] R13: 2e50 R14: 0001 R15: 000213d4c000 [0.098984] FS: () GS:88021ea0() knlGS: [0.107072] CS: 0010 DS: ES: CR0: 80050033 [0.112817] CR2: ff340003 CR3: 028f8000 CR4: 000406b0 [0.119952] Stack: [0.121965] 880213d4c000 2e50 818edf48 [0.129427] 81a73311 00f71ed85640 47ddbe4b4424ac57 a9929ff050ed979e [0.136890] 81af0900 88021ed85640 81afa2c0 [0.144353] Call Trace: [0.146801] [81a73311] efi_enter_virtual_mode+0x3d9/0x42c [0.153162] [81a56ea9] start_kernel+0x386/0x403 [0.158652] [81a5692c] ? repair_env_string+0x5c/0x5c [0.164569] [81a56120] ? early_idt_handlers+0x120/0x120 [0.170748] [81a565de] x86_64_start_reservations+0x2a/0x2c [0.177188] [81a5671e] x86_64_start_kernel+0x13e/0x14d [0.183284] Code: 89 c3 75 24 48 8b 05 e4 09 1f 00 be 06 00 00 00 48 8b 78 28 e8 8b d0 ff ff 48 85 c0 48 89 c3 0f 84 95 00 00 00 41 b4 01 45 84 e4 44 8b [0.203257] RIP [81a73794] efi_bgrt_init+0x9d/0x133 [0.209105] RSP 818edee0 [0.212593] CR2: ff340003 [0.215914] ---[ end trace fbce8f13012fb82c ]--- [0.220533] Kernel panic - not syncing: Fatal exception [0.225758] Rebooting in 3 seconds.. [0.229211] ACPI MEMORY or I/O RESET_REG. -- Thanks, Madper -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG] can't boot up: unable to handle kernel paging request at ffffffffff340003
Howdy, With old kernel (from 3.10 to 3.14-rc1), my hp box shows following outputs: ~~~ [0.009166] Freeing SMP alternatives memory: 20K (82234000 - 82239000) [0.010302] ioremap: invalid physical address 1376e0180001 [0.010303] [ cut here ] [0.010308] WARNING: CPU: 0 PID: 0 at arch/x86/mm/ioremap.c:83 __ioremap_caller+0x357/0x380() [0.010309] Modules linked in: [0.010312] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.14.0-rc1 #8 [0.010313] Hardware name: Hewlett-Packard HP Compaq Elite 8300 SFF/3397, BIOS K01 v02.05 05/07/2012 [0.010314] 0009 819dde58 816b5eb9 [0.010317] 819dde90 8107793d 1376e0180006 [0.010319] 81e0e2c0 0006 819ddea0 [0.010322] Call Trace: [0.010327] [816b5eb9] dump_stack+0x4d/0x66 [0.010331] [8107793d] warn_slowpath_common+0x7d/0xa0 [0.010333] [81077a1a] warn_slowpath_null+0x1a/0x20 [0.010335] [81060f37] __ioremap_caller+0x357/0x380 [0.010339] [81d85b5f] ? efi_bgrt_init+0x8b/0x12b [0.010341] [81060f77] ioremap_nocache+0x17/0x20 [0.010343] [81d85b5f] efi_bgrt_init+0x8b/0x12b [0.010345] [81d8522a] efi_late_init+0x9/0xb [0.010349] [81d68f59] start_kernel+0x436/0x450 [0.010351] [81d6892c] ? repair_env_string+0x5c/0x5c [0.010353] [81d68120] ? early_idt_handlers+0x120/0x120 [0.010355] [81d685de] x86_64_start_reservations+0x2a/0x2c [0.010356] [81d6871e] x86_64_start_kernel+0x13e/0x14d [0.010368] ---[ end trace b6511b740249fba3 ]--- ~~~ However the box can boot up and works well. After I update kernel to 3.14-rc2, it panic when booting. Here is console output: ~~~ [0.010366] BUG: unable to handle kernel paging request at ff340003 [0.017376] IP: [81d85ba4] efi_bgrt_init+0x9d/0x133 [0.023153] PGD 159eb067 PUD 159ed067 PMD 16240067 PTE 9376e0180163 [0.029842] Oops: 0009 [#1] SMP [0.033108] Modules linked in: [0.036184] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.14.0-rc2+ #7 [0.042540] Hardware name: Hewlett-Packard HP Compaq Elite 8300 SFF/3397, BIOS K01 v02.05 05/07/2012 [0.051679] task: 819ef4c0 ti: 819dc000 task.ti: 819dc000 [0.059167] RIP: 0010:[81d85ba4] [81d85ba4] efi_bgrt_init+0x9d/0x133 [0.067368] RSP: :819ddf20 EFLAGS: 00010202 [0.072684] RAX: ff340001 RBX: ff340001 RCX: 0abf [0.079825] RDX: 8163 RSI: 9376e0180163 RDI: ff34 [0.086967] RBP: 819ddf38 R08: 8163 R09: 8163 [0.094109] R10: 0001 R11: R12: 0001 [0.101249] R13: 88021ecc5640 R14: 81e0e2c0 R15: 819ddfb0 [0.108391] FS: () GS:880215c0() knlGS: [0.116489] CS: 0010 DS: ES: CR0: 80050033 [0.122239] CR2: ff340003 CR3: 159e8000 CR4: 001406f0 [0.129381] Stack: [0.131396] 81e04940 88021ecc5640 819ddf48 [0.138867] 81d8525d 819ddf88 81d68f59 81d6892c [0.146338] 81e0e2c0 81d6 0020 [0.153809] Call Trace: [0.156261] [81d8525d] efi_late_init+0x9/0xb [0.161499] [81d68f59] start_kernel+0x436/0x450 [0.166987] [81d6892c] ? repair_env_string+0x5c/0x5c [0.172913] [81d68120] ? early_idt_handlers+0x120/0x120 [0.179098] [81d685de] x86_64_start_reservations+0x2a/0x2c [0.185547] [81d6871e] x86_64_start_kernel+0x13e/0x14d [0.191646] Code: 89 c3 75 24 48 8b 05 34 37 53 00 be 06 00 00 00 48 8b 78 28 e8 93 d0 ff ff 48 85 c0 48 89 c [0.211644] RIP [81d85ba4] efi_bgrt_init+0x9d/0x133 [0.217501] RSP 819ddf20 [0.220994] CR2: ff340003 [0.224320] ---[ end trace 4f8e0a3f5b9ff6b1 ]--- [0.228946] Kernel panic - not syncing: Attempted to kill the idle task! [0.235655] Rebooting in 3 seconds.. [0.239146] ACPI MEMORY or I/O RESET_REG. BTW: 3.14-rc2 works well on my DELL desktop. I paste the dmidecode output of my HP desktop here: # dmidecode 2.12 # SMBIOS entry point at 0xd943d918 SMBIOS 2.7 present. 58 structures occupying 2472 bytes. Table at 0x000E92EB. Handle 0x, DMI type 0, 24 bytes BIOS Information Vendor: Hewlett-Packard Version: K01 v02.05 Release Date: 05/07/2012 Address: 0xF Runtime Size: 64 kB ROM Size: 16384 kB Characteristics: PCI is supported PNP is supported BIOS is upgradeable BIOS
[BUG] Kernel OOPS when reboot if I set reboot=efi,{warm, cold} (and some questions :-)
Howdy Folks, Happy new yeah, happy new bug! With a uefi system, I meet following panic when reboot after I adding `reboot=efi,warm` [call trace]: 0[ 698.736637] reboot: Restarting system 5[ 698.737407] reboot: machine restart 1[ 698.738399] BUG: unable to handle kernel paging request at ded53e60 1[ 698.738924] IP: [8800dedb946d] 0x8800dedb946d 4[ 698.739408] PGD 0 4[ 698.739868] Oops: [#1] SMP 4[ 698.740316] Modules linked in: nf_conntrack_netbios_ns nf_conntrack_broadcast ipt_MASQUERADE ip6t_REJECT ipt_REJECT xt_conntrack ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw iptable_filter ip_tables sg vfat fat snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_intel arc4 ath9k snd_hda_codec snd_hwdep ath9k_common ath9k_hw snd_seq snd_seq_device ath3k ath snd_pcm snd_page_alloc mac80211 btusb cfg80211 r8169 snd_timer bluetooth snd rfkill mii soundcore shpchp iTCO_wdt iTCO_vendor_support lpc_ich mfd_core pcspkr microcode dcdbas i2c_i801Panic#2 Part2 4[ 698.743245] serio_raw i2c_core video xfs libcrc32c usb_storage sd_mod crc_t10dif crct10dif_common ahci libahci libata dm_mirror dm_region_hash dm_log dm_mod 4[ 698.744276] CPU: 0 PID: 1939 Comm: reboot Not tainted 3.13.0-rc6+ #10 4[ 698.744793] Hardware name: Dell Inc. XPS 8500 /0NW73C, BIOS A09 09/05/2012 4[ 698.745314] task: 8800db6fbe30 ti: 88003eb86000 task.ti: 88003eb86000 4[ 698.745850] RIP: 0010:[8800dedb946d] [8800dedb946d] 0x8800dedb946d 4[ 698.746394] RSP: 0018:88003eb87cb0 EFLAGS: 00010046 4[ 698.746937] RAX: ded53e18 RBX: RCX: 8800dedb9a80 4[ 698.747494] RDX: 8800dedb9c50 RSI: RDI: 8800dedb9428 4[ 698.748055] RBP: 0001 R08: R09: 88003eb87ce0 4[ 698.748619] R10: 81a082c0 R11: 88003eb87cf8 R12: 28121969 4[ 698.749190] R13: fffe R14: 0cf9 R15: 4[ 698.749771] FS: 7f16f206c880() GS:88021ec0() knlGS: 4[ 698.750364] CS: 0010 DS: ES: CR0: 80050033 4[ 698.750963] CR2: ded53e60 CR3: 3e2d6000 CR4: 001407f0 Panic#2 Part1 4[ 698.751572] Stack: 4[ 698.752182] 81a082c0 88003eb87d50 0082 c9000a27fff8 4[ 698.752816] 88003eb87d00 0411 0001 0001 4[ 698.753455] 8800dedb9428 8105c556 4c564c54 4[ 698.754097] Call Trace: 4[ 698.754740] [8105c556] ? efi_call4+0x46/0x80 4[ 698.755398] [8105bc5c] ? virt_efi_reset_system+0x2c/0x30 4[ 698.756062] [8103f8d5] ? native_machine_emergency_restart+0x1a5/0x270 4[ 698.756731] [810491f0] ? native_apic_msr_write+0x30/0x40 4[ 698.757399] [8104357b] ? disable_local_APIC+0x4b/0x50 4[ 698.758059] [8103f427] ? native_machine_restart+0x37/0x40 4[ 698.758719] [8103f62f] ? machine_restart+0xf/0x20 4[ 698.759379] [8108e865] ? kernel_restart+0x45/0x60 4[ 698.760034] [8108eba9] ? SYSC_reboot+0x229/0x260 4[ 698.760685] [811b5ee9] ? do_readv_writev+0x169/0x220 4[ 698.761331] [811b6c5e] ? __fput+0x17e/0x260 4[ 698.761972] [811b6d8e] ? fput+0xe/0x10 4[ 698.762609] [8108ec7e] ? SyS_reboot+0xe/0x10 4[ 698.763242] [815e87e9] ? system_call_fastpath+0x16/0x1b 4[ 698.763876] Code: 4d 8d 4b e8 49 89 43 d8 48 8b 05 47 08 00 00 48 8d 15 f8 07 00 00 48 8d 0d 21 06 00 00 bd 01 00 00 00 45 33 c0 49 89 6b e8 33 f6 ff 50 48 48 3b c6 40 0f b6 fe 0f 4d fd e8 bd 05 00 00 40 3a c6 1[ 698.765285] RIP [8800dedb946d] 0x8800dedb946d 4[ 698.765972] RSP 88003eb87cb0 4[ 698.766647] CR2: ded53e60 4[ 698.768462] ---[ end trace b97908aef93ab59d ]--- 0[ 702.809043] Kernel panic - not syncing: Fatal exception According to Call Trace, the panic happen when call ResetSystem() which is a uefi run time service. It's seems a asm code in efi_stub_64.S I disassembly it with crash got following outputs: crash dis efi_call4 0x810518f0 efi_call4: mov%rsp,%rax [ ... ] 0x81051918 efi_call4+40: movaps %xmm3,0x30(%rsp) 0x8105191d efi_call4+45: movaps %xmm4,0x20(%rsp) 0x81051922 efi_call4+50: movaps %xmm5,0x10(%rsp) 0x81051927 efi_call4+55: sub$0x20,%rsp [ ... ] So it seems the panic happen when SAVE_XMM? Does SAVE_XMM will trigger paging? (`unable to handle kernel paging request at ded53e60`) in case it's a hardware special issue: My testbed is a DELL xps x8500 And I'll try to test it on another uefi system. -- Best, Madper -- To
[PATCH v3] efi-pstore: Make efi-pstore return a unique id
This version for fixing following errors:
efi-pstore.c:(.text+0x84c6cc): undefined reference to `__udivdi3'
efi-pstore.c:(.text+0x84c6e0): undefined reference to `__umoddi3'
Pstore fs expects that backends provide a uniqued id which could avoid
pstore making entries as duplication or denominating entries the same
name. So I combine the timestamp, part and count into id.
Signed-off-by: Madper Xie c...@redhat.com
---
drivers/firmware/efi/efi-pstore.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/efi/efi-pstore.c
b/drivers/firmware/efi/efi-pstore.c
index 5002d50..154fa5f 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -39,6 +39,12 @@ struct pstore_read_data {
char **buf;
};
+static inline u64 generic_id(unsigned long timestamp,
+unsigned int part, int count)
+{
+ return (timestamp * 100 + part) * 1000 + count;
+}
+
static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
{
efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
@@ -57,7 +63,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
void *data)
if (sscanf(name, dump-type%u-%u-%d-%lu-%c,
cb_data-type, part, cnt, time, data_type) == 5) {
- *cb_data-id = part;
+ *cb_data-id = generic_id(time, part, cnt);
*cb_data-count = cnt;
cb_data-timespec-tv_sec = time;
cb_data-timespec-tv_nsec = 0;
@@ -67,7 +73,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
void *data)
*cb_data-compressed = false;
} else if (sscanf(name, dump-type%u-%u-%d-%lu,
cb_data-type, part, cnt, time) == 4) {
- *cb_data-id = part;
+ *cb_data-id = generic_id(time, part, cnt);
*cb_data-count = cnt;
cb_data-timespec-tv_sec = time;
cb_data-timespec-tv_nsec = 0;
@@ -79,7 +85,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
void *data)
* which doesn't support holding
* multiple logs, remains.
*/
- *cb_data-id = part;
+ *cb_data-id = generic_id(time, part, 0);
*cb_data-count = 0;
cb_data-timespec-tv_sec = time;
cb_data-timespec-tv_nsec = 0;
@@ -199,14 +205,16 @@ static int efi_pstore_erase(enum pstore_type_id type, u64
id, int count,
char name[DUMP_NAME_LEN];
efi_char16_t efi_name[DUMP_NAME_LEN];
int found, i;
+ unsigned int part;
- sprintf(name, dump-type%u-%u-%d-%lu, type, (unsigned int)id, count,
- time.tv_sec);
+ do_div(id, 1000);
+ part = do_div(id, 100);
+ sprintf(name, dump-type%u-%u-%d-%lu, type, part, count, time.tv_sec);
for (i = 0; i DUMP_NAME_LEN; i++)
efi_name[i] = name[i];
- edata.id = id;
+ edata.id = part;
edata.type = type;
edata.count = count;
edata.time = time;
--
1.8.4.2
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] efi-pstore: Make efi-pstore return a unique id
m...@console-pimps.org writes: On Sat, 23 Nov, at 07:55:37PM, Madper Xie wrote: Pstore fs expects that backends provide a uniqued id which could avoid pstore making entries as duplication or denominating entries the same name. So I combine the timestamp, part and count into id. Signed-off-by: Madper Xie c...@redhat.com --- drivers/firmware/efi/efi-pstore.c | 19 +-- 1 file changed, 13 insertions(+), 6 deletions(-) Thanks unless anyone complains, I'm going to apply this. Does this need to be tagged for stable? Since it fix a real issue, my answer is Yes. -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] efi-pstore: Make efi-pstore return a unique id
Pstore fs expects that backends provide a uniqued id which could avoid
pstore making entries as duplication or denominating entries the same
name. So I combine the timestamp, part and count into id.
Signed-off-by: Madper Xie c...@redhat.com
---
drivers/firmware/efi/efi-pstore.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/efi/efi-pstore.c
b/drivers/firmware/efi/efi-pstore.c
index 5002d50..c6075f8 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -39,6 +39,12 @@ struct pstore_read_data {
char **buf;
};
+static inline u64 generic_id(unsigned long timestamp,
+unsigned int part, int count)
+{
+ return (timestamp * 100 + part) * 1000 + count;
+}
+
static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
{
efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
@@ -57,7 +63,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
void *data)
if (sscanf(name, dump-type%u-%u-%d-%lu-%c,
cb_data-type, part, cnt, time, data_type) == 5) {
- *cb_data-id = part;
+ *cb_data-id = generic_id(time, part, cnt);
*cb_data-count = cnt;
cb_data-timespec-tv_sec = time;
cb_data-timespec-tv_nsec = 0;
@@ -67,7 +73,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
void *data)
*cb_data-compressed = false;
} else if (sscanf(name, dump-type%u-%u-%d-%lu,
cb_data-type, part, cnt, time) == 4) {
- *cb_data-id = part;
+ *cb_data-id = generic_id(time, part, cnt);
*cb_data-count = cnt;
cb_data-timespec-tv_sec = time;
cb_data-timespec-tv_nsec = 0;
@@ -79,7 +85,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
void *data)
* which doesn't support holding
* multiple logs, remains.
*/
- *cb_data-id = part;
+ *cb_data-id = generic_id(time, part, 0);
*cb_data-count = 0;
cb_data-timespec-tv_sec = time;
cb_data-timespec-tv_nsec = 0;
@@ -199,14 +205,15 @@ static int efi_pstore_erase(enum pstore_type_id type, u64
id, int count,
char name[DUMP_NAME_LEN];
efi_char16_t efi_name[DUMP_NAME_LEN];
int found, i;
+ unsigned int part;
- sprintf(name, dump-type%u-%u-%d-%lu, type, (unsigned int)id, count,
- time.tv_sec);
+ part = (id / 1000) % 100;
+ sprintf(name, dump-type%u-%u-%d-%lu, type, part, count, time.tv_sec);
for (i = 0; i DUMP_NAME_LEN; i++)
efi_name[i] = name[i];
- edata.id = id;
+ edata.id = part;
edata.type = type;
edata.count = count;
edata.time = time;
--
1.8.4.2
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Make efi-pstore return a unique id
seiji.agu...@hds.com writes: Then will lost the sequence of our log. We will get lots of entries like dmesg-efi-`unique but meaningless number here` in pstore fs. Who will know which file is the latest record? Ah, that's good point. And another side, the combin of timestamp, count and part is unique. Why we generate a unique number from a unique number? if you think making a string from three ints and then a parse it to a int again is odd, i'd like to use ((timestamp * 100 + part) * 100 + count. How about calculating the number of digit of part /count, instead of using fixed 100? We can ensure the uniqueness of each id by doing it. digit_num = log2(part)/log2(10) + 1 timestamp * 10^(digit_num + 1) Then we must implement a log10 and a pow func... And reverting id to timestamp, part and count will hard if we using unfixed length of count and id. So i'd like to using fixed length. My dell xps has 128kb nvram space. and it at most store 126 pstore entries. So I think 1000 will be a safe value for count. For part, 100 should be okay. I don't think there will be a large kmsg and we must split it into more than 100 parts. Seiji -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86, efi: change name of efi_no_storage_paranoia parameter to efi_storage_paranoia
isimatu.yasu...@jp.fujitsu.com writes: Hi Matt, Sorry for late the reply. (2013/11/11 19:54), Matt Fleming wrote: On Mon, 11 Nov, at 05:52:59PM, Yasuaki Ishimatsu wrote: Hi Matt, I uses FUJITSU's x86 box. This does not become bricked even if I use all efi variable storage. Thus I want a way to not need to specify efi_no_storage_paranoia parameter. The efi_no_storage_paranoia parameter was introduced because some machines do not initiate garbage collection of the NVRAM until you allocate all space - basically it's a switch to turn off the save 5KB of stoarge at all times workaround that is needed to avoid bricking some machines. The intention of the switch is not to allow you to fill your NVRAM just because you can. If that is something you want to do then I think it's fair to require you to explicitly turn on efi_no_storage_paranoia. But I'm assuming here that you are doing something like writing lots and lots of pstore entries and just want to write as many as your variable storage will allow? Or are you doing something more fundamental like creating Boot entries? What are you doing to run into the 5KB reserve? How much NVRAM does your machine come with? I just add boot entry to NVRAM by efibootmgr command. But when Linux boots up, the remaining NVRAM is less than 5Kbyte. So I cannnot add new entry. Howdy Yasuaki, If the remaining NVRAM is less than 5Kb, your writing will trigger a NVRAM storage reclamation. However you still failed creating entry. So I'm just curious what itmes occupy lots of nvram storage space. Thanks, Yasuaki Ishimatsu -- Best, Madper -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86, efi: change name of efi_no_storage_paranoia parameter to efi_storage_paranoia
Howdy Yasuaki, I know some boxes with small NVRAM (less than 64kb?) will meet many issues if we always keep ~5kb free space. But if we really do not keep some free space as default, many box will become a brick. (In fact, I just fixed a bricked dell xps 8500 last week. 5kb is not enough for dell xps...) So maybe the better way is: we check the size of nvram and decide how many space we retain. isimatu.yasu...@jp.fujitsu.com writes: Hi Matt, I uses FUJITSU's x86 box. This does not become bricked even if I use all efi variable storage. Thus I want a way to not need to specify efi_no_storage_paranoia parameter. Thanks, Yasuaki Ishimatsu (2013/11/08 23:34), Matt Fleming wrote: On Fri, 08 Nov, at 07:32:51PM, Yasuaki Ishimatsu wrote: Everything started with an issue that killed Samsung laptops: http://mjg59.dreamwidth.org/22855.html Later it was found that if you write too much into UEFI variables many UEFI implementations will do bad things. Thanks for the information. I will read it. Out of curiosity, what hardware are you using? -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best, Madper Xie. -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] EFI: Delete out-of-date comments of efi_query_variable_store
For now we only ensure about 5kb free space for avoiding our board
refusing boot. But the comment lies that we retain 50% space.
Signed-off-by: Madper Xie c...@redhat.com
---
arch/x86/platform/efi/efi.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index c7e22ab..1a39d4d 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -1039,9 +1039,8 @@ u64 efi_mem_attributes(unsigned long phys_addr)
}
/*
- * Some firmware has serious problems when using more than 50% of the EFI
- * variable store, i.e. it triggers bugs that can brick machines. Ensure that
- * we never use more than this safe limit.
+ * Some firmware implementations refuse to boot if there's insufficient space
+ * in the variable store. Ensure that we never use more than a safe limit.
*
* Return EFI_SUCCESS if it is safe to write 'size' bytes to the variable
* store.
@@ -1060,10 +1059,9 @@ efi_status_t efi_query_variable_store(u32 attributes,
unsigned long size)
return status;
/*
-* Some firmware implementations refuse to boot if there's insufficient
-* space in the variable store. We account for that by refusing the
-* write if permitting it would reduce the available space to under
-* 5KB. This figure was provided by Samsung, so should be safe.
+* We account for that by refusing the write if permitting it would
+* reduce the available space to under 5KB. This figure was provided by
+* Samsung, so should be safe.
*/
if ((remaining_size - size EFI_MIN_RESERVE)
!efi_no_storage_paranoia) {
--
1.8.4.2
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG]: DELL XPS 8500 become a brick after fill too many entries to nvram.
Howdy all, For now we ensure at least ~5kb free space. But my dell xps still become a brick after I add too many entries to my nvram. So maybe 5kb is not safe enough. and 5kb is just aginst Samsung's laptop. So should we enlarge EFI_MIN_RESERVE? -- Best, Madper Xie. -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86, efi: change name of efi_no_storage_paranoia parameter to efi_storage_paranoia
rich...@nod.at writes: Am 08.11.2013 08:33, schrieb Yasuaki Ishimatsu: According to above works, efi_no_storage_paranoia parameter was prepared for sane UEFI which can do gc and fulfills spec. But why need a system with a sane UEFI set the parameter? It is wrong. A system with a broken UEFI should set the parameter. And how does one know that his UEFI is broken? Oh my board is briked because I wrote too much into a variable, maybe setting efi_storage_paranoia would have saved me. Let's try with the next board... ;) Agreed. It's hard for people to fix their briked motherboard. At least it's hard for someone who is the first time meet this issue like me. :-( and IMO, at least 51% of uefi firmwares on the world is buggy... ;) However, if we simply make all buggy firmware become a brick, the vendors will more careful in their next generation of products... But it's painful for everyone, both customers and vendors. Thanks, //richard -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best, Madper Xie. -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] efivars,efi-pstore: Hold off deletion of sysfs entry until the scan is completed
seiji.agu...@hds.com writes: Change from v3: - Introduce EFI_VARS_DATA_SIZE_MAX macro. - Add some description why a scanning/deleting logic is needed. Currently, when mounting pstore file system, a read callback of efi_pstore driver runs mutiple times as below. - In the first read callback, scan efivar_sysfs_list from head and pass a kmsg buffer of a entry to an upper pstore layer. - In the second read callback, rescan efivar_sysfs_list from the entry and pass another kmsg buffer to it. - Repeat the scan and pass until the end of efivar_sysfs_list. In this process, an entry is read across the multiple read function calls. To avoid race between the read and erasion, the whole process above is protected by a spinlock, holding in open() and releasing in close(). At the same time, kmemdup() is called to pass the buffer to pstore filesystem during it. And then, it causes a following lockdep warning. To make the dynamic memory allocation runnable without taking spinlock, holding off a deletion of sysfs entry if it happens while scanning it via efi_pstore, and deleting it after the scan is completed. To implement it, this patch introduces two flags, scanning and deleting, to efivar_entry. On the code basis, it seems that all the scanning and deleting logic is not needed because __efivars-lock are not dropped when reading from the EFI variable store. But, the scanning and deleting logic is still needed because an efi-pstore and a pstore filesystem works as follows. In case an entry(A) is found, the pointer is saved to psi-data. And efi_pstore_read() passes the entry(A) to a pstore filesystem by releasing __efivars-lock. And then, the pstore filesystem calls efi_pstore_read() again and the same entry(A), which is saved to psi-data, is used for resuming to scan a sysfs-list. So, to protect the entry(A), the logic is needed. [1.143710] [ cut here ] [1.144058] WARNING: CPU: 1 PID: 1 at kernel/lockdep.c:2740 lockdep_trace_alloc+0x104/0x110() [1.144058] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags)) [1.144058] Modules linked in: [1.144058] CPU: 1 PID: 1 Comm: systemd Not tainted 3.11.0-rc5 #2 [1.144058] 0009 8800797e9ae0 816614a5 8800797e9b28 [1.144058] 8800797e9b18 8105510d 0080 0046 [1.144058] 00d0 03af 81ccd0c0 8800797e9b78 [1.144058] Call Trace: [1.144058] [816614a5] dump_stack+0x54/0x74 [1.144058] [8105510d] warn_slowpath_common+0x7d/0xa0 [1.144058] [8105517c] warn_slowpath_fmt+0x4c/0x50 [1.144058] [8131290f] ? vsscanf+0x57f/0x7b0 [1.144058] [810bbd74] lockdep_trace_alloc+0x104/0x110 [1.144058] [81192da0] __kmalloc_track_caller+0x50/0x280 [1.144058] [815147bb] ? efi_pstore_read_func.part.1+0x12b/0x170 [1.144058] [8115b260] kmemdup+0x20/0x50 [1.144058] [815147bb] efi_pstore_read_func.part.1+0x12b/0x170 [1.144058] [81514800] ? efi_pstore_read_func.part.1+0x170/0x170 [1.144058] [815148b4] efi_pstore_read_func+0xb4/0xe0 [1.144058] [81512b7b] __efivar_entry_iter+0xfb/0x120 [1.144058] [8151428f] efi_pstore_read+0x3f/0x50 [1.144058] [8128d7ba] pstore_get_records+0x9a/0x150 [1.158207] [812af25c] ? selinux_d_instantiate+0x1c/0x20 [1.158207] [8128ce30] ? parse_options+0x80/0x80 [1.158207] [8128ced5] pstore_fill_super+0xa5/0xc0 [1.158207] [811ae7d2] mount_single+0xa2/0xd0 [1.158207] [8128ccf8] pstore_mount+0x18/0x20 [1.158207] [811ae8b9] mount_fs+0x39/0x1b0 [1.158207] [81160550] ? __alloc_percpu+0x10/0x20 [1.158207] [811c9493] vfs_kern_mount+0x63/0xf0 [1.158207] [811cbb0e] do_mount+0x23e/0xa20 [1.158207] [8115b51b] ? strndup_user+0x4b/0xf0 [1.158207] [811cc373] SyS_mount+0x83/0xc0 [1.158207] [81673cc2] system_call_fastpath+0x16/0x1b [1.158207] ---[ end trace 61981bc62de9f6f4 ]--- Signed-off-by: Seiji Aguchi seiji.agu...@hds.com After applied this patch, I can't see the warning again when I mounting pstore fs and deleting pstore entries. Tested-by: Madper Xie c...@redhat.com --- drivers/firmware/efi/efi-pstore.c | 143 +++--- drivers/firmware/efi/efivars.c| 12 ++-- drivers/firmware/efi/vars.c | 12 +++- include/linux/efi.h | 4 ++ 4 files changed, 155 insertions(+), 16 deletions(-) -- Best, Madper Xie. -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] make all stored entries accessible.
1. checking type, id, psi, count and timespec when finding duplicate entries. 2. adding count and timestamp for differentiating. Madper Xie (2): pstore: avoid incorrectly mark entry as duplicate pstore: Differentiating names by adding count and timestamp fs/pstore/inode.c | 35 +++ 1 file changed, 23 insertions(+), 12 deletions(-) -- 1.8.4.2 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] pstore: Differentiating names by adding count and timestamp
From: Madper Xie bbbo...@gmail.com
pstore denominates dumped file as type-psname-id. it makes many file have
the same name if there are many entries in backend have the same id.
So adding count and timestamp to file name for differentiating.
Signed-off-by: Madper Xie c...@redhat.com
---
fs/pstore/inode.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 0ae994c..36b502f 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -285,7 +285,7 @@ int pstore_mkfile(enum pstore_type_id type, char *psname,
u64 id, int count,
int rc = 0;
charname[PSTORE_NAMELEN];
struct pstore_private *private, *pos;
- unsigned long flags;
+ unsigned long flags, timestamp;
spin_lock_irqsave(allpstore_lock, flags);
list_for_each_entry(pos, allpstore, list) {
@@ -316,35 +316,42 @@ int pstore_mkfile(enum pstore_type_id type, char *psname,
u64 id, int count,
private-count = count;
private-psi = psi;
memcpy(private-time, time, sizeof(struct timespec));
+ timestamp = time.tv_sec;
switch (type) {
case PSTORE_TYPE_DMESG:
- sprintf(name, dmesg-%s-%lld%s, psname, id,
- compressed ? .enc.z : );
+ sprintf(name, dmesg-%s-%lld-%d-%ld%s, psname, id, count,
+ timestamp, compressed ? .enc.z : );
break;
case PSTORE_TYPE_CONSOLE:
- sprintf(name, console-%s, psname);
+ sprintf(name, console-%s-%d-%ld, psname, count, timestamp);
break;
case PSTORE_TYPE_FTRACE:
- sprintf(name, ftrace-%s, psname);
+ sprintf(name, ftrace-%s-%d-%ld, psname, count, timestamp);
break;
case PSTORE_TYPE_MCE:
- sprintf(name, mce-%s-%lld, psname, id);
+ sprintf(name, mce-%s-%lld-%d-%ld, psname, id, count,
+ timestamp);
break;
case PSTORE_TYPE_PPC_RTAS:
- sprintf(name, rtas-%s-%lld, psname, id);
+ sprintf(name, rtas-%s-%lld-%d-%ld, psname, id, count,
+ timestamp);
break;
case PSTORE_TYPE_PPC_OF:
- sprintf(name, powerpc-ofw-%s-%lld, psname, id);
+ sprintf(name, powerpc-ofw-%s-%lld-%d-%ld, psname, id, count,
+ timestamp);
break;
case PSTORE_TYPE_PPC_COMMON:
- sprintf(name, powerpc-common-%s-%lld, psname, id);
+ sprintf(name, powerpc-common-%s-%lld-%d-%ld, psname, id,
+ count, timestamp);
break;
case PSTORE_TYPE_UNKNOWN:
- sprintf(name, unknown-%s-%lld, psname, id);
+ sprintf(name, unknown-%s-%lld-%d-%ld, psname, id, count,
+ timestamp);
break;
default:
- sprintf(name, type%d-%s-%lld, type, psname, id);
+ sprintf(name, type%d-%s-%lld-%d-%ld, type, psname, id, count,
+ timestamp);
break;
}
--
1.8.4.2
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] pstore: avoid incorrectly mark entry as duplicate
seiji.agu...@hds.com writes:
-Original Message-
From: Madper Xie [mailto:c...@redhat.com]
Sent: Wednesday, October 30, 2013 5:45 AM
To: tony.l...@intel.com; keesc...@chromium.org; ccr...@android.com;
an...@enomsg.org; Seiji Aguchi
Cc: linux-efi@vger.kernel.org; linux-ker...@vger.kernel.org;
bbbo...@gmail.com; Madper Xie
Subject: [PATCH 1/2] pstore: avoid incorrectly mark entry as duplicate
From: Madper Xie bbbo...@gmail.com
pstore try to find duplicate entries by check both ID, type and psi.
They are not really enough for efi backend. dumped vars always have
the same type, psi and ID. like follows:
dump-type0-9-1-1382511508-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-9-1-1382515661-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
The duplicate entries won't appear in pstorefs. And a complain will be
print -- pstore: failed to load 76 record(s) from 'efi'
So I add two more checks: timespec and count.
Signed-off-by: Madper Xie c...@redhat.com
Looks good to me.
Acked-by: Seiji Aguchi seiji.agu...@hds.com
But, this patch has to be tested by other backend drivers like erst and
ramoops.
In my understanding, erst has supported to log multiple events.
So, I'm interested why the same error hasn't happened in the other drivers.
Seiji
Thanks for your review. I'll try to test other backends. :-)
---
fs/pstore/inode.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 1282384..0ae994c 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -47,6 +47,7 @@ static LIST_HEAD(allpstore);
struct pstore_private {
struct list_head list;
struct pstore_info *psi;
+struct timespec time;
enum pstore_type_id type;
u64 id;
int count;
@@ -290,7 +291,9 @@ int pstore_mkfile(enum pstore_type_id type, char
*psname, u64 id, int count,
list_for_each_entry(pos, allpstore, list) {
if (pos-type == type
pos-id == id
-pos-psi == psi) {
+pos-psi == psi
+pos-count == count
+!timespec_compare(pos-time, time)) {
rc = -EEXIST;
break;
}
@@ -312,6 +315,7 @@ int pstore_mkfile(enum pstore_type_id type, char
*psname, u64 id, int count,
private-id = id;
private-count = count;
private-psi = psi;
+memcpy(private-time, time, sizeof(struct timespec));
switch (type) {
case PSTORE_TYPE_DMESG:
--
1.8.4.2
--
Best,
Madper Xie.
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] make all stored entries accessible.
tony.l...@intel.com writes: So, do you mean efivars should fix to use the id in a proper way? It would avoid the need for all these tests, and additions to the filename to guarantee uniqueness. Not sure what options efivars has to create a unique, persistent id for each record. It's a fundamental part of how ERST works (though the unique ID is just based around a timestamp). Okay, maybe there are three options here: 1. combine timestamp, count and part into id. for now, in efi-pstore.c, *id = part. and we could simply change it to unique one. F.E. *id = (timestamp * 100 + part) * 100 + count. 2. change the id's type. let id become a string. so every backend could write anything to id. then it will become a part of filename in pstore filesystem. (but we need fix all backends since we modified api.) 3. apply the patches I have sent... even if the filename will be ugly and gory... Which one do you prefer? I acked Madper's patch 2/2 earlier today, but when I look at your test result, I'm not sure if it is reasonable for users to make multiple numbers visible to the file name. -r--r--r-- 1 root root 17499 Oct 30 13:41 dmesg-erst-5940651313304961029--2129078373-1383165669 after I added the count = 0 initialization the filename gets a tiny bit less scary: -r--r--r-- 1 root root 17499 Oct 30 13:41 dmesg-erst-5940651313304961029-0-1383165669 -Tony -- Best, Madper Xie. -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Differentiating names by adding a timestamp.
pstore denominate dumped file as type-psname-id. it makes many file have
the same name if there are many entries in backend have the same id.
So adding a timestamp to file name.
Signed-off-by: Madper Xie c...@redhat.com
---
fs/pstore/inode.c | 26 --
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 1282384..bf6ed3a 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -315,32 +315,38 @@ int pstore_mkfile(enum pstore_type_id type, char *psname,
u64 id, int count,
switch (type) {
case PSTORE_TYPE_DMESG:
- sprintf(name, dmesg-%s-%lld%s, psname, id,
- compressed ? .enc.z : );
+ sprintf(name, dmesg-%s-%lld-%lld%s, psname, id,
+ timespec_to_ns(time), compressed ? .enc.z : );
break;
case PSTORE_TYPE_CONSOLE:
- sprintf(name, console-%s, psname);
+ sprintf(name, console-%s-%lld, psname, timespec_to_ns(time));
break;
case PSTORE_TYPE_FTRACE:
- sprintf(name, ftrace-%s, psname);
+ sprintf(name, ftrace-%s-%lld, psname, timespec_to_ns(time));
break;
case PSTORE_TYPE_MCE:
- sprintf(name, mce-%s-%lld, psname, id);
+ sprintf(name, mce-%s-%lld-%lld, psname, id,
+ timespec_to_ns(time));
break;
case PSTORE_TYPE_PPC_RTAS:
- sprintf(name, rtas-%s-%lld, psname, id);
+ sprintf(name, rtas-%s-%lld-%lld, psname, id,
+ timespec_to_ns(time));
break;
case PSTORE_TYPE_PPC_OF:
- sprintf(name, powerpc-ofw-%s-%lld, psname, id);
+ sprintf(name, powerpc-ofw-%s-%lld-%lld, psname, id,
+ timespec_to_ns(time));
break;
case PSTORE_TYPE_PPC_COMMON:
- sprintf(name, powerpc-common-%s-%lld, psname, id);
+ sprintf(name, powerpc-common-%s-%lld-%lld, psname, id,
+ timespec_to_ns(time));
break;
case PSTORE_TYPE_UNKNOWN:
- sprintf(name, unknown-%s-%lld, psname, id);
+ sprintf(name, unknown-%s-%lld-%lld, psname, id,
+ timespec_to_ns(time));
break;
default:
- sprintf(name, type%d-%s-%lld, type, psname, id);
+ sprintf(name, type%d-%s-%lld-%lld, type, psname, id,
+ timespec_to_ns(time));
break;
}
--
1.8.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] pstore: failed to load 76 record(s) from 'efi'
-deec89e117eb PchS3Peim-e6c2f70a-b604-4877-85ba-deec89e117eb PciSerialPortsLocationVar-560bf58a-1e0d-4d7e-953f-2980a261e031 PK-8be4df61-93ca-11d2-aa0d-00e098032b8c PlatformLang-8be4df61-93ca-11d2-aa0d-00e098032b8c PlatformLangCodes-8be4df61-93ca-11d2-aa0d-00e098032b8c PNP0501_0_NV-560bf58a-1e0d-4d7e-953f-2980a261e031 PNP0501_0_VV-560bf58a-1e0d-4d7e-953f-2980a261e031 PNP0510_0_NV-560bf58a-1e0d-4d7e-953f-2980a261e031 PNP0510_0_VV-560bf58a-1e0d-4d7e-953f-2980a261e031 PreviousMemoryTypeInformation-4c19049f-4137-4dd3-9c10-8b97a83ffdfa RestorePostScreen-8be4df61-93ca-11d2-aa0d-00e098032b8c S3SS-4bafc2b4-02dc-4104-b236-d6f1b98d9e84 SaPegData-c4975200-64f1-4fb6-9773-f6a9f89d985e SbAslBufferPtrVar-01f33c25-764d-43ea-aeea-6b5a41f3f3e8 ScramblerBaseSeed-87f22dcb-7304-4105-bb7c-317143ccc23b SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c SerialPortsEnabledVar-560bf58a-1e0d-4d7e-953f-2980a261e031 Setup-80e1202e-2697-4264-9cc9-80762c3e5863 Setup-ec87d643-eba4-4bb5-a1e5-3f3e36b20da9 SetupMode-8be4df61-93ca-11d2-aa0d-00e098032b8c SetupPlatformData-ec87d643-eba4-4bb5-a1e5-3f3e36b20da9 SetupSnbPpmFeatures-ec87d643-eba4-4bb5-a1e5-3f3e36b20da9 SignatureSupport-8be4df61-93ca-11d2-aa0d-00e098032b8c SioSerialPortsLocationVar-560bf58a-1e0d-4d7e-953f-2980a261e031 SMBIOS_START_ADDR-0a602c5b-05a0-40c4-9181-edcd891d0036 StdDefaults-4599d26f-1a11-49b8-b91f-858745cff824 TdtAdvancedSetupDataVar-7b77fb8b-1e0d-4d7e-953f-3980a261e076 test1-12341234-1234-1234-1234-123412341234 test-12341234-1234-1234-1234-123412341234 test2-12341234-1234-2134-1234-123412341234 test3-12341234-1234-1234-1234-123412341234 test4-12341234-1234-1234-1234-123412341234 test5-12341234-1234-1234-1234-123412341234 test6-12341234-1234-1234-1234-123412341234 Timeout-8be4df61-93ca-11d2-aa0d-00e098032b8c UsbHandOnOffEntry-0a602c5b-05a0-40c4-9181-edcd891d0018 UsbMassDevNum-ec87d643-eba4-4bb5-a1e5-3f3e36b20da9 UsbMassDevValid-ec87d643-eba4-4bb5-a1e5-3f3e36b20da9 USB_POINT-8be4df61-93ca-11d2-aa0d-00e098032b8c UsbSupport-ec87d643-eba4-4bb5-a1e5-3f3e36b20da9 WdtPersistentData-78ce2354-cfbc-4643-aeba-07a27fa892bf -- Best, Madper Xie. -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pstore: avoid incorrectly mark entry as duplicate
pstore try to find duplicate entries by check both ID, type and psi.
They are not really enough for efi backend. dumped vars always have
the same type, psi and ID. like follows:
dump-type0-9-1-1382511508-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-9-1-1382515661-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
The duplicate entries won't appear in pstorefs. And a complain will be
print -- pstore: failed to load 76 record(s) from 'efi'
So I add one more check: timespec.
Signed-off-by: Madper Xie c...@redhat.com
---
fs/pstore/inode.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 1282384..f70f1e5 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -47,6 +47,7 @@ static LIST_HEAD(allpstore);
struct pstore_private {
struct list_head list;
struct pstore_info *psi;
+ struct timespec time;
enum pstore_type_id type;
u64 id;
int count;
@@ -290,7 +291,8 @@ int pstore_mkfile(enum pstore_type_id type, char *psname,
u64 id, int count,
list_for_each_entry(pos, allpstore, list) {
if (pos-type == type
pos-id == id
- pos-psi == psi) {
+ pos-psi == psi
+ !timespec_compare(pos-time, time)) {
rc = -EEXIST;
break;
}
@@ -312,6 +314,7 @@ int pstore_mkfile(enum pstore_type_id type, char *psname,
u64 id, int count,
private-id = id;
private-count = count;
private-psi = psi;
+ memcpy(private-time, time, sizeof(struct timespec));
switch (type) {
case PSTORE_TYPE_DMESG:
--
1.8.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate
richard.weinber...@gmail.com writes:
On Wed, Oct 23, 2013 at 4:55 PM, Madper Xie c...@redhat.com wrote:
pstore try to find duplicate entries by check both ID, type and psi.
They are not really enough for efi backend. dumped vars always have
the same type, psi and ID. like follows:
dump-type0-9-1-1382511508-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-9-1-1382515661-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
The duplicate entries won't appear in pstorefs. And a complain will be
print -- pstore: failed to load 76 record(s) from 'efi'
So I add one more check: timespec.
Signed-off-by: Madper Xie c...@redhat.com
---
fs/pstore/inode.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index 1282384..f70f1e5 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -47,6 +47,7 @@ static LIST_HEAD(allpstore);
struct pstore_private {
struct list_head list;
struct pstore_info *psi;
+ struct timespec time;
enum pstore_type_id type;
u64 id;
int count;
@@ -290,7 +291,8 @@ int pstore_mkfile(enum pstore_type_id type, char
*psname, u64 id, int count,
list_for_each_entry(pos, allpstore, list) {
if (pos-type == type
pos-id == id
- pos-psi == psi) {
+ pos-psi == psi
+ !timespec_compare(pos-time, time)) {
rc = -EEXIST;
break;
}
@@ -312,6 +314,7 @@ int pstore_mkfile(enum pstore_type_id type, char
*psname, u64 id, int count,
private-id = id;
private-count = count;
private-psi = psi;
+ memcpy(private-time, time, sizeof(struct timespec));
switch (type) {
case PSTORE_TYPE_DMESG:
As discussed on IRC, why don't you compare the variable names?
Howdy Richard,
Let's analyze the name of duplicate entries:
dump-type0-9-1-1382511508-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-9-1-1382515661-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
The have the same name except timestamp. In this case, there is no
difference between compare names and compare timestamps.
( Yeah, I'm pretty sure the guid will be the same one --
LINUX_EFI_CRASH_GUID )
But efi is only one backend. For other backends, we don't sure if they
will add timestamp to name. So...
--
Best,
Madper Xie.
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate
tony.l...@gmail.com writes:
On Wed, Oct 23, 2013 at 7:55 AM, Madper Xie c...@redhat.com wrote:
The duplicate entries won't appear in pstorefs. And a complain will be
print -- pstore: failed to load 76 record(s) from 'efi'
Maybe I don't quite get this - but it sounds like you have a whole lot
of entries using up space in efivars that have similar names - differing
just in the timestamp - that won't show up in the pstore filesystem - because
we'd try to name them all the same thing.
Maybe I misunderstand you...
Sure pstore try to name them all the same thing, but it's another
issue. and it doesn't prevent entries showing up in pstore fs.
Consider the following case: (after efi-pstore support append mode, it
always like this case):
I choose four dumped efivars from my DELL XPS:
dump-type0-9-1-1380441690-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-9-1-1380448560-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-9-1-1380460890-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
dump-type0-9-1-1382496073-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0
^^ [ ^ ]
!! !
typetimestampGUID
When pstore load them from efivars, pstore incorrectly assuming that
efivars with the same TYPE, ID and GUID are duplicate.
list_for_each_entry(pos, allpstore, list) {
if (pos-type == type /---
pos-id == id -| as I said, it only check type,id,psi
pos-psi == psi) { \---
rc = -EEXIST;- then set -EEXIST, and ignore *dup* entry
break;
}
}
You can see the code above, for those four entries, only one could be
showed in pstorefs, all others will get a -EEXIST. So I add a timestamp
check here, it's the only different part.
How did all those things end up in efivars?
before the patch I can see
dmesg-efi-1 dmesg-efi-11 dmesg-efi-3 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9
dmesg-efi-10 dmesg-efi-2 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8
after apply the patch:
[root@dhcp-13-41 vars]# ls /dev/pstore/
dmesg-efi-1 dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5
dmesg-efi-6 dmesg-efi-8
dmesg-efi-1 dmesg-efi-10 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5
dmesg-efi-7 dmesg-efi-8
dmesg-efi-1 dmesg-efi-10 dmesg-efi-12 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5
dmesg-efi-7 dmesg-efi-8
dmesg-efi-1 dmesg-efi-10 dmesg-efi-13 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5
dmesg-efi-7 dmesg-efi-9
dmesg-efi-1 dmesg-efi-10 dmesg-efi-14 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5
dmesg-efi-7 dmesg-efi-9
dmesg-efi-1 dmesg-efi-10 dmesg-efi-15 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5
dmesg-efi-7 dmesg-efi-9
dmesg-efi-1 dmesg-efi-10 dmesg-efi-16 dmesg-efi-3 dmesg-efi-4 dmesg-efi-5
dmesg-efi-7 dmesg-efi-9
dmesg-efi-1 dmesg-efi-10 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6
dmesg-efi-7 dmesg-efi-9
dmesg-efi-1 dmesg-efi-10 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6
dmesg-efi-7 dmesg-efi-9
dmesg-efi-1 dmesg-efi-10 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6
dmesg-efi-8 dmesg-efi-9
dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6
dmesg-efi-8 dmesg-efi-9
dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6
dmesg-efi-8 dmesg-efi-9
dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6
dmesg-efi-8
dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6
dmesg-efi-8
dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6
dmesg-efi-8
Wouldn't the right fix be to make pstore allow them all to appear (using the
timestamp to differentiate names?) so that we could see them, log them,
and then remove them from pstore (in turn freeing up efivars space - which
people keep telling me is in short supply).
Yeah, many file have the same name, just like my case above. But it not
really block the file shows up and should be solved in another
patch. And I'm trying fix it.
-Tony
--
Best,
Madper Xie.
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] efivars,efi-pstore: Hold off deletion of sysfs entry until, the scan is completed
Hi folks, I tested it on my DELL XPS desktop. And it won't show any warnings when I mounting pstore and deleting pstore items after this patch applied. Tested-by: Madper Xie c...@redhat.com writes: Oops, It seems my mu4e(a email client for emacs)'s auto-indent breaks the patch... I apologize for this... seiji.agu...@hds.com writes: Hi Madper, I tested this patch on 3.12-rc4. Could you please send me the log when you failed to apply? Seiji -Original Message- From: Madper Xie [mailto:c...@redhat.com] Sent: Thursday, October 17, 2013 1:54 AM To: Seiji Aguchi Cc: linux-ker...@vger.kernel.org; linux-efi@vger.kernel.org; matt.flem...@intel.com; tony.l...@intel.com; Tomoki Sekiyama; dle- deve...@lists.sourceforge.net Subject: Re: [PATCH v3] efivars,efi-pstore: Hold off deletion of sysfs entry until, the scan is completed Howdy Seiji, I failed appily this patch to both 3.12-rc2 and 3.12-rc4. Could you please let me know which is the right tree for this patch? Thanks, Madper. seiji.agu...@hds.com writes: Change from v2: - Move dynamic memory allocation to efi_pstore_read() before holding efivars-lock to protect entry-var.Data. - Access to entry-scanning while holding efivars-lock. - Move a comment about a returned value from efi_pstore_read_func() to efi_pstore_read() because size 0 case may happen in efi_pstore_read(). Currently, when mounting pstore file system, a read callback of efi_pstore driver runs mutiple times as below. - In the first read callback, scan efivar_sysfs_list from head and pass a kmsg buffer of a entry to an upper pstore layer. - In the second read callback, rescan efivar_sysfs_list from the entry and pass another kmsg buffer to it. - Repeat the scan and pass until the end of efivar_sysfs_list. In this process, an entry is read across the multiple read function calls. To avoid race between the read and erasion, the whole process above is protected by a spinlock, holding in open() and releasing in close(). At the same time, kmemdup() is called to pass the buffer to pstore filesystem during it. And then, it causes a following lockdep warning. To make the dynamic memory allocation runnable without taking spinlock, holding off a deletion of sysfs entry if it happens while scanning it via efi_pstore, and deleting it after the scan is completed. To implement it, this patch introduces two flags, scanning and deleting, to efivar_entry. [1.143710] [ cut here ] [1.144058] WARNING: CPU: 1 PID: 1 at kernel/lockdep.c:2740 lockdep_trace_alloc+0x104/0x110() [1.144058] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags)) [1.144058] Modules linked in: [1.144058] CPU: 1 PID: 1 Comm: systemd Not tainted 3.11.0-rc5 #2 [1.144058] 0009 8800797e9ae0 816614a5 8800797e9b28 [1.144058] 8800797e9b18 8105510d 0080 0046 [1.144058] 00d0 03af 81ccd0c0 8800797e9b78 [1.144058] Call Trace: [1.144058] [816614a5] dump_stack+0x54/0x74 [1.144058] [8105510d] warn_slowpath_common+0x7d/0xa0 [1.144058] [8105517c] warn_slowpath_fmt+0x4c/0x50 [1.144058] [8131290f] ? vsscanf+0x57f/0x7b0 [1.144058] [810bbd74] lockdep_trace_alloc+0x104/0x110 [1.144058] [81192da0] __kmalloc_track_caller+0x50/0x280 [1.144058] [815147bb] ? efi_pstore_read_func.part.1+0x12b/0x170 [1.144058] [8115b260] kmemdup+0x20/0x50 [1.144058] [815147bb] efi_pstore_read_func.part.1+0x12b/0x170 [1.144058] [81514800] ? efi_pstore_read_func.part.1+0x170/0x170 [1.144058] [815148b4] efi_pstore_read_func+0xb4/0xe0 [1.144058] [81512b7b] __efivar_entry_iter+0xfb/0x120 [1.144058] [8151428f] efi_pstore_read+0x3f/0x50 [1.144058] [8128d7ba] pstore_get_records+0x9a/0x150 [1.158207] [812af25c] ? selinux_d_instantiate+0x1c/0x20 [1.158207] [8128ce30] ? parse_options+0x80/0x80 [1.158207] [8128ced5] pstore_fill_super+0xa5/0xc0 [1.158207] [811ae7d2] mount_single+0xa2/0xd0 [1.158207] [8128ccf8] pstore_mount+0x18/0x20 [1.158207] [811ae8b9] mount_fs+0x39/0x1b0 [1.158207] [81160550] ? __alloc_percpu+0x10/0x20 [1.158207] [811c9493] vfs_kern_mount+0x63/0xf0 [1.158207] [811cbb0e] do_mount+0x23e/0xa20 [1.158207] [8115b51b] ? strndup_user+0x4b/0xf0 [1.158207] [811cc373] SyS_mount+0x83/0xc0 [1.158207] [81673cc2] system_call_fastpath+0x16/0x1b [1.158207] ---[ end trace 61981bc62de9f6f4 ]--- Signed-off-by: Seiji Aguchi seiji.agu...@hds.com --- drivers/firmware/efi/efi
Re: [PATCH v3] efivars,efi-pstore: Hold off deletion of sysfs entry until, the scan is completed
**)psi-data);
+ efivar_entry_iter_end();
+ if (size = 0)
+ kfree(*data.buf);
+ return size;
}
static int efi_pstore_write(enum pstore_type_id type,
@@ -184,9 +297,17 @@ static int efi_pstore_erase_func(struct efivar_entry
*entry, void *data)
return 0;
}
+ if (entry-scanning) {
+ /*
+ * Skip deletion because this entry will be deleted
+ * after scanning is completed.
+ */
+ entry-deleting = true;
+ } else
+ list_del(entry-list);
+
/* found */
__efivar_entry_delete(entry);
- list_del(entry-list);
return 1;
}
@@ -214,10 +335,12 @@ static int efi_pstore_erase(enum pstore_type_id type,
u64 id, int count,
efivar_entry_iter_begin();
found = __efivar_entry_iter(efi_pstore_erase_func, efivar_sysfs_list,
edata, entry);
- efivar_entry_iter_end();
- if (found)
+ if (found !entry-scanning) {
+ efivar_entry_iter_end();
efivar_unregister(entry);
+ } else
+ efivar_entry_iter_end();
return 0;
}
diff --git a/drivers/firmware/efi/efivars.c b/drivers/firmware/efi/efivars.c
index 8a7432a..8c5a61a 100644
--- a/drivers/firmware/efi/efivars.c
+++ b/drivers/firmware/efi/efivars.c
@@ -383,12 +383,16 @@ static ssize_t efivar_delete(struct file *filp, struct
kobject *kobj,
else if (__efivar_entry_delete(entry))
err = -EIO;
- efivar_entry_iter_end();
-
- if (err)
+ if (err) {
+ efivar_entry_iter_end();
return err;
+ }
- efivar_unregister(entry);
+ if (!entry-scanning) {
+ efivar_entry_iter_end();
+ efivar_unregister(entry);
+ } else
+ efivar_entry_iter_end();
/* It's dead Jim */
return count;
diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index 391c67b..b22659c 100644
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -683,8 +683,16 @@ struct efivar_entry *efivar_entry_find(efi_char16_t
*name, efi_guid_t guid,
if (!found)
return NULL;
- if (remove)
- list_del(entry-list);
+ if (remove) {
+ if (entry-scanning) {
+ /*
+ * The entry will be deleted
+ * after scanning is completed.
+ */
+ entry-deleting = true;
+ } else
+ list_del(entry-list);
+ }
return entry;
}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 5f8f176..04088fb 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -782,6 +782,8 @@ struct efivar_entry {
struct efi_variable var;
struct list_head list;
struct kobject kobj;
+ bool scanning;
+ bool deleting;
};
extern struct list_head efivar_sysfs_list;
--
Best,
Madper Xie.
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
