Re: powerpc/nvram: Fix an incorrect partition merge
On Thu, 2015-10-12 at 07:30:02 UTC, xinhui wrote: > From: Pan Xinhui > > When we merge two contiguous partitions whose signatures are marked > NVRAM_SIG_FREE, We need update prev's length and checksum, then write it > to nvram, not cur's. So lets fix this mistake now. > > Also use memset instead of strncpy to set the partition's name. It's > more readable if we want to fill up with duplicate chars . > > Signed-off-by: Pan Xinhui Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/11b7e154b132232535befe51c5 cheers
Re: powerpc/nvram: Fix an incorrect partition merge
On 2016年06月07日 19:39, Michael Ellerman wrote: On Mon, 2016-06-06 at 13:31 +0800, xinhui wrote: On 2016年06月03日 19:47, Michael Ellerman wrote: On Thu, 2015-10-12 at 07:30:02 UTC, xinhui wrote: From: Pan Xinhui When we merge two contiguous partitions whose signatures are marked NVRAM_SIG_FREE, We need update prev's length and checksum, then write it to nvram, not cur's. So lets fix this mistake now. Also use memset instead of strncpy to set the partition's name. It's more readable if we want to fill up with duplicate chars . Does this ever happen in practice? ie. should we backport the fix to stable kernels? I did not see that nvram warning in practice. BUT I suggest to backport it to stable kernel. :) Let me recall the story. :) In past days, I was using pstore to keep some kernel logs. and sometimes I found my own logs and the panic logs did not show. pstore use a fixed-address reserved memory In x86 while nvram instead in ppc. Then I spent some days to review the nvram codes. And worked out three patches to fix all issues that I found in nvram. BUT looks like I only sent out two of them. :) I lost the third patch maybe... OK. Has it always been broken? no. after nvram partition corruption hit, all nvram partitions will be erased and re-alloc after the second machine reboot. I don't know who does it but i guess it is the firmware. :) Actually I meant has the code always contained the bug, or was it added recently. fix fa2b4e54d41f3c9f1bee6a7d63ecd4f0ab161e89 ("powerpc/nvram: Improve partition removal") and it is from Ben. So cc Ben :) cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: powerpc/nvram: Fix an incorrect partition merge
On Mon, 2016-06-06 at 13:31 +0800, xinhui wrote: > On 2016年06月03日 19:47, Michael Ellerman wrote: > > On Thu, 2015-10-12 at 07:30:02 UTC, xinhui wrote: > > > From: Pan Xinhui > > > > > > When we merge two contiguous partitions whose signatures are marked > > > NVRAM_SIG_FREE, We need update prev's length and checksum, then write it > > > to nvram, not cur's. So lets fix this mistake now. > > > > > > Also use memset instead of strncpy to set the partition's name. It's > > > more readable if we want to fill up with duplicate chars . > > > > Does this ever happen in practice? ie. should we backport the fix to stable > > kernels? > > I did not see that nvram warning in practice. BUT I suggest to backport it to > stable kernel. :) > > Let me recall the story. :) > In past days, I was using pstore to keep some kernel logs. and sometimes I > found my own logs and the panic logs did not show. > pstore use a fixed-address reserved memory In x86 while nvram instead in ppc. > > Then I spent some days to review the nvram codes. > And worked out three patches to fix all issues that I found in nvram. BUT > looks like I only sent out two of them. :) > I lost the third patch maybe... OK. > > Has it always been broken? > > no. after nvram partition corruption hit, all nvram partitions will be erased > and re-alloc after the second machine reboot. > I don't know who does it but i guess it is the firmware. :) Actually I meant has the code always contained the bug, or was it added recently. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: powerpc/nvram: Fix an incorrect partition merge
xinhui writes: >> Has it always been broken? >> > > no. after nvram partition corruption hit, all nvram partitions will be > erased and re-alloc after the second machine reboot. > I don't know who does it but i guess it is the firmware. :) It is. PAPR says we format NVRAM when it's corrupted. This is also true for NVRAM for PowerNV (not just pseries guest). -- Stewart Smith OPAL Architect, IBM. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: powerpc/nvram: Fix an incorrect partition merge
On 2016年06月03日 19:47, Michael Ellerman wrote: On Thu, 2015-10-12 at 07:30:02 UTC, xinhui wrote: From: Pan Xinhui When we merge two contiguous partitions whose signatures are marked NVRAM_SIG_FREE, We need update prev's length and checksum, then write it to nvram, not cur's. So lets fix this mistake now. Also use memset instead of strncpy to set the partition's name. It's more readable if we want to fill up with duplicate chars . Does this ever happen in practice? ie. should we backport the fix to stable kernels? I did not see that nvram warning in practice. BUT I suggest to backport it to stable kernel. :) Let me recall the story. :) In past days, I was using pstore to keep some kernel logs. and sometimes I found my own logs and the panic logs did not show. pstore use a fixed-address reserved memory In x86 while nvram instead in ppc. Then I spent some days to review the nvram codes. And worked out three patches to fix all issues that I found in nvram. BUT looks like I only sent out two of them. :) I lost the third patch maybe... Has it always been broken? no. after nvram partition corruption hit, all nvram partitions will be erased and re-alloc after the second machine reboot. I don't know who does it but i guess it is the firmware. :) thanks xinhui cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: powerpc/nvram: Fix an incorrect partition merge
On Thu, 2015-10-12 at 07:30:02 UTC, xinhui wrote: > From: Pan Xinhui > > When we merge two contiguous partitions whose signatures are marked > NVRAM_SIG_FREE, We need update prev's length and checksum, then write it > to nvram, not cur's. So lets fix this mistake now. > > Also use memset instead of strncpy to set the partition's name. It's > more readable if we want to fill up with duplicate chars . Does this ever happen in practice? ie. should we backport the fix to stable kernels? Has it always been broken? cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/nvram: Fix an incorrect partition merge
Hi, all I do some tests *without* my fix patch. after reboot, I saw logs below. [0.271236] WARNING: nvram partition checksum was 58, should be 24! [0.271239] Terminating nvram partition scan If I do tests *with* my fix patch, logs are: [0.291419] NVRAM Partitions- [0.291422] indx sig chkslen name [0.291425]0 51 15 512 ibm,CPU0log [0.291428] 8192 51 94 128 ibm,CPU1log [0.291431] 1024070 fc 256 common [0.291434] 14336a0 b5 131 ibm,rtas-log [0.291437] 16432a0 4f 251 lnx,oops-log [0.291440] 204487f 26 2818 HOW TO REPRODUCE this warning?: root@ubuntu:/home/pp/host# ./a.out 0 xinhui 4096 root@ubuntu:/home/pp/host# ./a.out 0 xinhui2 4096 root@ubuntu:/home/pp/host# ./a.out 0 xinhui3 4096 root@ubuntu:/home/pp/host# ./a.out 1 xinhui2 root@ubuntu:/home/pp/host# ./a.out 1 xinhui3 root@ubuntu:/home/pp/host# ./a.out 1 xinhui then logs from dmesg are: [ 844.205337] XINHUI: dev_nvram_ioctl, [xinhui],[0] [ 844.205449] NVRAM Partitions- [ 844.205482] indx sig chkslen name [ 844.205510]0 51 15 512 ibm,CPU0log [ 844.205541] 8192 51 94 128 ibm,CPU1log [ 844.205573] 1024070 fc 256 common [ 844.205604] 14336a0 b5 131 ibm,rtas-log [ 844.205636] 16432a0 4f 251 lnx,oops-log [ 844.205667] 204487f 5e 2818 free space [ 851.636438] XINHUI: dev_nvram_ioctl, [xinhui],[4096] [ 851.636534] NVRAM Partitions- [ 851.636573] indx sig chkslen name [ 851.636614]0 51 15 512 ibm,CPU0log [ 851.636651] 8192 51 94 128 ibm,CPU1log [ 851.636688] 1024070 fc 256 common [ 851.636725] 14336a0 b5 131 ibm,rtas-log [ 851.636762] 16432a0 4f 251 lnx,oops-log [ 851.636798] 20448ef 89 257 xinhui [ 851.636836] 245607f 5c 2561 free space [ 855.354409] XINHUI: dev_nvram_ioctl, [xinhui2],[4096] [ 855.354600] NVRAM Partitions- [ 855.354639] indx sig chkslen name [ 855.354671]0 51 15 512 ibm,CPU0log [ 855.354708] 8192 51 94 128 ibm,CPU1log [ 855.355499] 1024070 fc 256 common [ 855.356186] 14336a0 b5 131 ibm,rtas-log [ 855.356816] 16432a0 4f 251 lnx,oops-log [ 855.357429] 20448ef 89 257 xinhui [ 855.357966] 24560ef bb 257 xinhui2 [ 855.358466] 286727f 5a 2304 free space [ 857.866574] XINHUI: dev_nvram_ioctl, [xinhui3],[4096] [ 857.867449] NVRAM Partitions- [ 857.868250] indx sig chkslen name [ 857.869082]0 51 15 512 ibm,CPU0log [ 857.869901] 8192 51 94 128 ibm,CPU1log [ 857.870727] 1024070 fc 256 common [ 857.871569] 14336a0 b5 131 ibm,rtas-log [ 857.872392] 16432a0 4f 251 lnx,oops-log [ 857.873202] 20448ef 89 257 xinhui [ 857.874019] 24560ef bb 257 xinhui2 [ 857.874811] 28672ef bc 257 xinhui3 [ 857.875568] 327847f 58 2047 free space [ 1015.661796] XINHUI: dev_nvram_ioctl, [xinhui2],[16383] [ 1015.662670] NVRAM Partitions- [ 1015.663457] indx sig chkslen name [ 1015.664150]0 51 15 512 ibm,CPU0log [ 1015.664788] 8192 51 94 128 ibm,CPU1log [ 1015.665396] 1024070 fc 256 common [ 1015.665948] 14336a0 b5 131 ibm,rtas-log [ 1015.666470] 16432a0 4f 251 lnx,oops-log [ 1015.666977] 20448ef 89 257 xinhui [ 1015.667455] 245607f 1b 257 [ 1015.667914] 28672ef bc 257 xinhui3 [ 1015.668359] 327847f 58 2047 free space [ 1017.452055] XINHUI: dev_nvram_ioctl, [xinhui3],[16383] [ 1017.452902] NVRAM Partitions- [ 1017.453731] indx sig chkslen name [ 1017.454531]0 51 15 512 ibm,CPU0log [ 1017.455341] 8192 51 94 128 ibm,CPU1log [ 1017.456121] 1024070 fc 256 common [ 1017.456916] 14336a0 b5 131 ibm,rtas-log [ 1017.457705] 16432
[PATCH] powerpc/nvram: Fix an incorrect partition merge
From: Pan Xinhui When we merge two contiguous partitions whose signatures are marked NVRAM_SIG_FREE, We need update prev's length and checksum, then write it to nvram, not cur's. So lets fix this mistake now. Also use memset instead of strncpy to set the partition's name. It's more readable if we want to fill up with duplicate chars . Signed-off-by: Pan Xinhui --- arch/powerpc/kernel/nvram_64.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c index 21a278b7..40e80ca 100644 --- a/arch/powerpc/kernel/nvram_64.c +++ b/arch/powerpc/kernel/nvram_64.c @@ -969,7 +969,7 @@ int __init nvram_remove_partition(const char *name, int sig, /* Make partition a free partition */ part->header.signature = NVRAM_SIG_FREE; - strncpy(part->header.name, "", 12); + memset(part->header.name, 'w', 12); part->header.checksum = nvram_checksum(&part->header); rc = nvram_write_header(part); if (rc <= 0) { @@ -987,8 +987,8 @@ int __init nvram_remove_partition(const char *name, int sig, } if (prev) { prev->header.length += part->header.length; - prev->header.checksum = nvram_checksum(&part->header); - rc = nvram_write_header(part); + prev->header.checksum = nvram_checksum(&prev->header); + rc = nvram_write_header(prev); if (rc <= 0) { printk(KERN_ERR "nvram_remove_partition: nvram_write failed (%d)\n", rc); return rc; -- 2.5.0 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev