pseries: phyp dump: hypervisor-assisted dump
Subsystems (2)
cover pseries: phyp dump: hypervisor-assisted dump
The following series of patches implement a basic framework for hypervisor-assisted dump. The very first patch provides documentation explaining what this is :-) . Yes, its supposed to be an improvement over kdump.
The patches mostly work; a list of open issues / todo list is included in the documentation. It also appears that the not-yet-released firmware versions this was tested on are still, ahem, incomplete; this work is also pending.
-- Linas & Manish
PATCH 1/8 pseries: phyp dump: Docmentation
Basic documentation for hypervisor-assisted dump.
----
Documentation/powerpc/phyp-assisted-dump.txt | 129 +++++++++++++++++++++++++++ 1 file changed, 129 insertions(+)
Documentation/powerpc/phyp-assisted-dump.txt new file +129 -0
@@ -0,0 +1,129 @@ + + Hypervisor-Assisted Dump + ------------------------ + November 2007 + +The goal of hypervisor-assisted dump is to enable the dump of +a crashed system, and to do so from a fully-reset system, and +to minimize the total elapsed time until the system is back +in production use. + +As compared to kdump or other strategies, hypervisor-assisted +dump offers several strong, practical advantages: + +-- Unlike kdump, the system has been reset, and loaded + with a fresh copy of the kernel. In particular, + PCI and I/O devices have been reinitialized and are + in a clean, consistent state. +-- As the dump is performed, the dumped memory becomes + immediately available to the system for normal use. +-- After the dump is completed, no further reboots are + required; the system will be fully usable, and running + in it's normal, production mode on it normal kernel. + +The above can only be accomplished by coordination with, +and assistance from the hypervisor. The procedure is +as follows: + +-- When a system crashes, the hypervisor will save + the low 256MB of RAM to a previously registered + save region. It will also save system state, system + registers, and hardware PTE's. + +-- After the low 256MB area has been saved, the + hypervisor will reset PCI and other hardware state. + It will *not* clear RAM. It will then launch the + bootloader, as normal. + +-- The freshly booted kernel will notice that there + is a new node (ibm,dump-kernel) in the device tree, + indicating that there is crash data available from + a previous boot. It will boot into only 256MB of RAM, + reserving the rest of system memory. + +-- Userspace tools will parse /sys/kernel/release_region + and read /proc/vmcore to obtain the contents of memory, + which holds the previous crashed kernel. The userspace + tools may copy this info to disk, or network, nas, san, + iscsi, etc. as desired. + + For Example: the values in /sys/kernel/release-region + would look something like this (address-range pairs). + CPU:0x177fee000-0x10000: HPTE:0x177ffe020-0x1000: / + DUMP:0x177fff020-0x10000000, 0x10000000-0x16F1D370A + +-- As the userspace tools complete saving a portion of + dump, they echo an offset and size to + /sys/kernel/release_region to release the reserved + memory back to general use. + + An example of this is: + "echo 0x40000000 0x10000000 > /sys/kernel/release_region" + which will release 256MB at the 1GB boundary. + +Please note that the hypervisor-assisted dump feature +is only available on Power6-based systems with recent +firmware versions. + +Implementation details: +---------------------- +In order for this scheme to work, memory needs to be reserved +quite early in the boot cycle. However, access to the device +tree this early in the boot cycle is difficult, and device-tree +access is needed to determine if there is a crash data waiting. +To work around this problem, all but 256MB of RAM is reserved +during early boot. A short while later in boot, a check is made +to determine if there is dump data waiting. If there isn't, +then the reserved memory is released to general kernel use. +If there is dump data, then the /sys/kernel/release_region +file is created, and the reserved memory is held. + +If there is no waiting dump data, then all but 256MB of the +reserved ram will be released for general kernel use. The +highest 256 MB of RAM will *not* be released: this region +will be kept permanently reserved, so that it can act as +a receptacle for a copy of the low 256MB in the case a crash +does occur. See, however, "open issues" below, as to whether +such a reserved region is really needed. + +Currently the dump will be copied from /proc/vmcore to a +a new file upon user intervention. The starting address +to be read and the range for each data point in provided +in /sys/kernel/release_region. + +The tools to examine the dump will be same as the ones +used for kdump. + + +General notes: +-------------- +Security: please note that there are potential security issues +with any sort of dump mechanism. In particular, plaintext +(unencrypted) data, and possibly passwords, may be present in +the dump data. Userspace tools must take adequate precautions to +preserve security. + +Open issues/ToDo: +------------ + o The various code paths that tell the hypervisor that a crash + occurred, vs. it simply being a normal reboot, should be + reviewed, and possibly clarified/fixed. + + o Instead of using /sys/kernel, should there be a /sys/dump + instead? There is a dump_subsys being created by the s390 code, + perhaps the pseries code should use a similar layout as well. + + o Is reserving a 256MB region really required? The goal of + reserving a 256MB scratch area is to make sure that no + important crash data is clobbered when the hypervisor + save low mem to the scratch area. But, if one could assure + that nothing important is located in some 256MB area, then + it would not need to be reserved. Something that can be + improved in subsequent versions. + + o Still working the kdump team to integrate this with kdump, + some work remains but this would not affect the current + patches. + + o Still need to write a shell script, to copy the dump away. + Currently I am parsing it manually.
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Documentation/powerpc/phyp-assisted-dump.txt line 4
Quoted 4 lines · Manish Ahuja
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+ + Hypervisor-Assisted Dump + ------------------------ + November 2007
Date is unneeded (and, uhm, dated :)
Documentation/powerpc/phyp-assisted-dump.txt line 9
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+The goal of hypervisor-assisted dump is to enable the dump of +a crashed system, and to do so from a fully-reset system, and +to minimize the total elapsed time until the system is back +in production use.
Is it actually faster than kdump?
Quoted 42 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +42
-0+As compared to kdump or other strategies, hypervisor-assisted +dump offers several strong, practical advantages: + +-- Unlike kdump, the system has been reset, and loaded + with a fresh copy of the kernel. In particular, + PCI and I/O devices have been reinitialized and are + in a clean, consistent state. +-- As the dump is performed, the dumped memory becomes + immediately available to the system for normal use. +-- After the dump is completed, no further reboots are + required; the system will be fully usable, and running + in it's normal, production mode on it normal kernel. + +The above can only be accomplished by coordination with, +and assistance from the hypervisor. The procedure is +as follows: + +-- When a system crashes, the hypervisor will save + the low 256MB of RAM to a previously registered + save region. It will also save system state, system + registers, and hardware PTE's. + +-- After the low 256MB area has been saved, the + hypervisor will reset PCI and other hardware state. + It will *not* clear RAM. It will then launch the + bootloader, as normal. + +-- The freshly booted kernel will notice that there + is a new node (ibm,dump-kernel) in the device tree, + indicating that there is crash data available from + a previous boot. It will boot into only 256MB of RAM, + reserving the rest of system memory. + +-- Userspace tools will parse /sys/kernel/release_region + and read /proc/vmcore to obtain the contents of memory, + which holds the previous crashed kernel. The userspace + tools may copy this info to disk, or network, nas, san, + iscsi, etc. as desired. + + For Example: the values in /sys/kernel/release-region + would look something like this (address-range pairs). + CPU:0x177fee000-0x10000: HPTE:0x177ffe020-0x1000: /
Documentation/powerpc/phyp-assisted-dump.txt line 62
Quoted 10 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +10
-0+ DUMP:0x177fff020-0x10000000, 0x10000000-0x16F1D370A + +-- As the userspace tools complete saving a portion of + dump, they echo an offset and size to + /sys/kernel/release_region to release the reserved + memory back to general use. + + An example of this is: + "echo 0x40000000 0x10000000 > /sys/kernel/release_region" + which will release 256MB at the 1GB boundary.
This violates the "one file, one value" rule of sysfs, but nobody really takes that seriously, I guess. In any case, consider documenting this in Documentation/ABI.
Documentation/powerpc/phyp-assisted-dump.txt line 66
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+ +Please note that the hypervisor-assisted dump feature +is only available on Power6-based systems with recent +firmware versions.
This statement will of course become dated/incorrect so I recommend removing it.
Documentation/powerpc/phyp-assisted-dump.txt line 73
Quoted 7 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +7
-0+ +Implementation details: +---------------------- +In order for this scheme to work, memory needs to be reserved +quite early in the boot cycle. However, access to the device +tree this early in the boot cycle is difficult, and device-tree +access is needed to determine if there is a crash data waiting.
I don't think this bit about early device tree access is correct. By the time your code is reserving memory (from early_init_devtree(), I think), RTAS has been instantiated and you are able to test for the existence of /rtas/ibm,dump-kernel.
Documentation/powerpc/phyp-assisted-dump.txt line 77
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+To work around this problem, all but 256MB of RAM is reserved +during early boot. A short while later in boot, a check is made +to determine if there is dump data waiting. If there isn't, +then the reserved memory is released to general kernel use.
So I think these gymnastics are unneeded -- unless I'm misunderstanding something, you should be able to determine very early whether to reserve that memory.
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+If there is dump data, then the /sys/kernel/release_region +file is created, and the reserved memory is held. + +If there is no waiting dump data, then all but 256MB of the
Documentation/powerpc/phyp-assisted-dump.txt line 91
Quoted 10 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +10
-0+reserved ram will be released for general kernel use. The +highest 256 MB of RAM will *not* be released: this region +will be kept permanently reserved, so that it can act as +a receptacle for a copy of the low 256MB in the case a crash +does occur. See, however, "open issues" below, as to whether +such a reserved region is really needed. + +Currently the dump will be copied from /proc/vmcore to a +a new file upon user intervention. The starting address +to be read and the range for each data point in provided
^is
Quoted 13 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +13
-0+in /sys/kernel/release_region. + +The tools to examine the dump will be same as the ones +used for kdump. + + +General notes: +-------------- +Security: please note that there are potential security issues +with any sort of dump mechanism. In particular, plaintext +(unencrypted) data, and possibly passwords, may be present in +the dump data. Userspace tools must take adequate precautions to +preserve security.
Documentation/powerpc/phyp-assisted-dump.txt line 114
Quoted 10 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +10
-0+ +Open issues/ToDo: +------------ + o The various code paths that tell the hypervisor that a crash + occurred, vs. it simply being a normal reboot, should be + reviewed, and possibly clarified/fixed. + + o Instead of using /sys/kernel, should there be a /sys/dump + instead? There is a dump_subsys being created by the s390 code, + perhaps the pseries code should use a similar layout as well.
Well, it seems to me that there's little reason to duplicate the s390 layout unless we can actually share code.
FWIW, I've been thinking about making a /sys/firmware/phyp hierarchy which could contain much of the System P-specific functions (DLPAR, lparcfg, other crud in /proc/ppc64)... seems suited to this platform-specific dump mechanism.
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 84 lines · Nathan Lynch
Documentation/powerpc/phyp-assisted-dump.txt line 4
Quoted 4 lines · Manish Ahuja
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+ + Hypervisor-Assisted Dump + ------------------------ + November 2007Date is unneeded (and, uhm, dated :)
Documentation/powerpc/phyp-assisted-dump.txt line 9
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+The goal of hypervisor-assisted dump is to enable the dump of +a crashed system, and to do so from a fully-reset system, and +to minimize the total elapsed time until the system is back +in production use.Is it actually faster than kdump?
Quoted 42 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +42
-0+As compared to kdump or other strategies, hypervisor-assisted +dump offers several strong, practical advantages: + +-- Unlike kdump, the system has been reset, and loaded + with a fresh copy of the kernel. In particular, + PCI and I/O devices have been reinitialized and are + in a clean, consistent state. +-- As the dump is performed, the dumped memory becomes + immediately available to the system for normal use. +-- After the dump is completed, no further reboots are + required; the system will be fully usable, and running + in it's normal, production mode on it normal kernel. + +The above can only be accomplished by coordination with, +and assistance from the hypervisor. The procedure is +as follows: + +-- When a system crashes, the hypervisor will save + the low 256MB of RAM to a previously registered + save region. It will also save system state, system + registers, and hardware PTE's. + +-- After the low 256MB area has been saved, the + hypervisor will reset PCI and other hardware state. + It will *not* clear RAM. It will then launch the + bootloader, as normal. + +-- The freshly booted kernel will notice that there + is a new node (ibm,dump-kernel) in the device tree, + indicating that there is crash data available from + a previous boot. It will boot into only 256MB of RAM, + reserving the rest of system memory. + +-- Userspace tools will parse /sys/kernel/release_region + and read /proc/vmcore to obtain the contents of memory, + which holds the previous crashed kernel. The userspace + tools may copy this info to disk, or network, nas, san, + iscsi, etc. as desired. + + For Example: the values in /sys/kernel/release-region + would look something like this (address-range pairs). + CPU:0x177fee000-0x10000: HPTE:0x177ffe020-0x1000: /Documentation/powerpc/phyp-assisted-dump.txt line 62
Quoted 10 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +10
-0+ DUMP:0x177fff020-0x10000000, 0x10000000-0x16F1D370A + +-- As the userspace tools complete saving a portion of + dump, they echo an offset and size to + /sys/kernel/release_region to release the reserved + memory back to general use. + + An example of this is: + "echo 0x40000000 0x10000000 > /sys/kernel/release_region" + which will release 256MB at the 1GB boundary.This violates the "one file, one value" rule of sysfs, but nobody really takes that seriously, I guess. In any case, consider documenting this in
Documentation/ABI.Documentation/powerpc/phyp-assisted-dump.txt line 66
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+ +Please note that the hypervisor-assisted dump feature +is only available on Power6-based systems with recent +firmware versions.This statement will of course become dated/incorrect so I recommend removing it.
Quoted 12 lines
Documentation/powerpc/phyp-assisted-dump.txt line 73
Quoted 7 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +7
-0+ +Implementation details: +---------------------- +In order for this scheme to work, memory needs to be reserved +quite early in the boot cycle. However, access to the device +tree this early in the boot cycle is difficult, and device-tree +access is needed to determine if there is a crash data waiting.I don't think this bit about early device tree access is correct. By the time your code is reserving memory (from
early_init_devtree(), I think), RTAS has been instantiated and you are able to test for the existence of /rtas/ibm,dump-kernel.
Yep it's early_init_devtree(), and yes it's fairly easy to access the (flattened) device tree at that point.
Quoted 8 lines
Documentation/powerpc/phyp-assisted-dump.txt line 77
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+To work around this problem, all but 256MB of RAM is reserved +during early boot. A short while later in boot, a check is made +to determine if there is dump data waiting. If there isn't, +then the reserved memory is released to general kernel use.So I think these gymnastics are unneeded -- unless I'm misunderstanding something, you should be able to determine very early whether to reserve that memory.
I agree.
cheers
Signature · 8 lines
Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
In reply to Nathan LynchHi Nathan,
Thank you for looking at this.
Quoted 7 lines · Nathan Lynch
Documentation/powerpc/phyp-assisted-dump.txt line 4
Quoted 4 lines · Manish Ahuja
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+ + Hypervisor-Assisted Dump + ------------------------ + November 2007Date is unneeded (and, uhm, dated :)
I'll argue very very strongly against this. One of the worst things I deal with is undated documents. Google pulls up multiple versions of a document, and you start wondering: "which one of these is the *right one*?" Its only after trying some suggested sequence of commands that aren't working, and more time googling and effort wasted, that you finallly realize that you are reading the version 0.1 of documentation from 1999 that some idiot did not have the decency to put a date on. (A version number is acceptable). Documentation is often in mediocre state, don't compound the problem be leaving off critical info.
Quoted 6 lines
Documentation/powerpc/phyp-assisted-dump.txt line 9
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+The goal of hypervisor-assisted dump is to enable the dump of +a crashed system, and to do so from a fully-reset system, and +to minimize the total elapsed time until the system is back +in production use.Is it actually faster than kdump?
This is a basic presumption; it should blow it away, as, after all, it requires one less reboot! As a side effect, the system is in production *while* the dump is being taken; with kdump, you can't go into production until after the dump is finished, and the system has been rebooted a second time. On systems with terabytes of RAM, the time difference can be hours.
Quoted 9 lines
Documentation/powerpc/phyp-assisted-dump.txt line 62
Quoted 8 lines
+-- As the userspace tools complete saving a portion of
Documentation/powerpc/phyp-assisted-dump.txt new file +7
-0+ dump, they echo an offset and size to + /sys/kernel/release_region to release the reserved + memory back to general use. + + An example of this is: + "echo 0x40000000 0x10000000 > /sys/kernel/release_region" + which will release 256MB at the 1GB boundary.
Quoted 3 lines
This violates the "one file, one value" rule of sysfs, but nobody really takes that seriously, I guess. In any case, consider documenting this in
Documentation/ABI.
This interface will almost certainly be changed, in order to allow phyp dump to work with the kdump user-space tools. Its provisional right now, until the details of user-space is hammered out.
Quoted 6 lines
Documentation/powerpc/phyp-assisted-dump.txt line 66
Quoted 3 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +3
-0+Please note that the hypervisor-assisted dump feature +is only available on Power6-based systems with recent +firmware versions.This statement will of course become dated/incorrect so I recommend removing it.
A provisional statement. I figure it should get taken out after a few years.
Quoted 11 lines
Documentation/powerpc/phyp-assisted-dump.txt line 73
Quoted 6 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +6
-0+Implementation details: +---------------------- +In order for this scheme to work, memory needs to be reserved +quite early in the boot cycle. However, access to the device +tree this early in the boot cycle is difficult, and device-tree +access is needed to determine if there is a crash data waiting.I don't think this bit about early device tree access is correct. By the time your code is reserving memory (from
early_init_devtree(), I think), RTAS has been instantiated and you are able to test for the existence of /rtas/ibm,dump-kernel.
If I remember right, it was still too early to look up this token directly, so we wrote some code to crawl the flat device tree to find it. But not only was that a lot of work, but I somehow decided that doing this to the flat tree was wrong, as otherwise someone would surely have written the access code. If this can be made to work, that would be great, but we couldn't make it work at the time.
Quoted 8 lines
Documentation/powerpc/phyp-assisted-dump.txt line 77
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+To work around this problem, all but 256MB of RAM is reserved +during early boot. A short while later in boot, a check is made +to determine if there is dump data waiting. If there isn't, +then the reserved memory is released to general kernel use.So I think these gymnastics are unneeded -- unless I'm misunderstanding something, you should be able to determine very early whether to reserve that memory.
Only if you can get at rtas, but you can't get at rtas at that point. I think we would have been the earliest users of rtas at that point. Possibly the order of how things are initialized could be changed.
Quoted 4 lines
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+Open issues/ToDo: +------------ + o The various code paths that tell the hypervisor that a crash + occurred, vs. it simply being a normal reboot, should be
Quoted 8 lines
Documentation/powerpc/phyp-assisted-dump.txt line 114
Quoted 5 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +5
-0+ reviewed, and possibly clarified/fixed. + + o Instead of using /sys/kernel, should there be a /sys/dump + instead? There is a dump_subsys being created by the s390 code, + perhaps the pseries code should use a similar layout as well.Well, it seems to me that there's little reason to duplicate the s390 layout unless we can actually share code.
I'm thinking "user-space tools". This could probably be decided very quickly after a short chat with the s390 architects about tools and standardization and dump formats and the support process. In the end, its some third-level support people who will be dissecting the dump using god-knows-what tool (a hacked kdgb ???), so things should get standardized as much as possible so that we don't duplicate work.
Quoted 4 lines
FWIW, I've been thinking about making a
/sys/firmware/phyphierarchy which could contain much of the System P-specific functions (DLPAR, lparcfg, other crud in/proc/ppc64)... seems suited to this platform-specific dump mechanism.
Works for me. I was always unclear what /sys/firmware was supposed to contain.
--linas
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Hi Linas,
Quoted 11 lines · Linas Vepstas
Quoted 8 lines · Nathan Lynch
Documentation/powerpc/phyp-assisted-dump.txt line 9
Quoted 5 lines · Manish Ahuja
Documentation/powerpc/phyp-assisted-dump.txt new file +5
-0+ +The goal of hypervisor-assisted dump is to enable the dump of +a crashed system, and to do so from a fully-reset system, and +to minimize the total elapsed time until the system is back +in production use.Is it actually faster than kdump?
Quoted 1 line
This is a basic presumption;
I used the word "actually". I already know that it is intended to be faster. :)
Quoted 2 lines
it should blow it away, as, after all,
it requires one less reboot!
There's more than rebooting going on during system dump processing. Depending on the system type, booting may not be where most time is spent.
Quoted 2 lines
As a side effect, the system is in
production *while* the dump is being taken;
A dubious feature IMO. Seems that the design potentially trades reliability of first failure data capture for availability. E.g. system crashes, reboots, resumes processing while copying dump, crashes again before dump procedure is complete. How is that handled, if at all?
Quoted 5 lines
with kdump,
you can't go into production until after the dump is finished, and the system has been rebooted a second time. On systems with terabytes of RAM, the time difference can be hours.
The difference in time it takes to resume the normal workload may be significant, yes. But the time it takes to get a usable dump image would seem to be the basically the same.
Since you bring up large systems... a system with terabytes of RAM is practically guaranteed to be a NUMA configuration with dozens of cpus. When processing a dump on such a system, I wonder how well we fare:
can we successfully boot with (say) 128 cpus and 256MB of usable memory? Do we have to hot-online nodes as system memory is freed up (and does that even work)? We need to be able to restore the system to its optimal topology when the dump is finished; if the best we can do is a degraded configuration, the workload will suffer and the system admin is likely to just reboot the machine again so the kernel will have the right NUMA topology.
Quoted 28 lines
Quoted 11 lines
Documentation/powerpc/phyp-assisted-dump.txt line 73
Quoted 6 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +6
-0+Implementation details: +---------------------- +In order for this scheme to work, memory needs to be reserved +quite early in the boot cycle. However, access to the device +tree this early in the boot cycle is difficult, and device-tree +access is needed to determine if there is a crash data waiting.I don't think this bit about early device tree access is correct. By the time your code is reserving memory (from
early_init_devtree(), I think), RTAS has been instantiated and you are able to test for the existence of /rtas/ibm,dump-kernel.If I remember right, it was still too early to look up this token directly, so we wrote some code to crawl the flat device tree to find it. But not only was that a lot of work, but I somehow decided that doing this to the flat tree was wrong, as otherwise someone would surely have written the access code. If this can be made to work, that would be great, but we couldn't make it work at the time.
Quoted 8 lines
Documentation/powerpc/phyp-assisted-dump.txt line 77
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+To work around this problem, all but 256MB of RAM is reserved +during early boot. A short while later in boot, a check is made +to determine if there is dump data waiting. If there isn't, +then the reserved memory is released to general kernel use.So I think these gymnastics are unneeded -- unless I'm misunderstanding something, you should be able to determine very early whether to reserve that memory.
Quoted 1 line
Only if you can get at rtas, but you can't get at rtas at that point.
Sorry, but I think you are mistaken (see Michael's earlier reply).
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 12 lines
I used the word "actually". I already know that it is intended to be faster. :)
Quoted 2 lines
it should blow it away, as, after all,
it requires one less reboot!There's more than rebooting going on during system dump processing. Depending on the system type, booting may not be where most time is spent.
Quoted 8 lines
Quoted 2 lines
As a side effect, the system is in
production *while* the dump is being taken;A dubious feature IMO. Seems that the design potentially trades reliability of first failure data capture for availability. E.g. system crashes, reboots, resumes processing while copying dump, crashes again before dump procedure is complete. How is that handled, if at all?
This is a simple version. The intent was not to have a complex dump taking mechanism in version 1. Subsequent versions will see planned improvement on the way the pages are tracked and freed.
Also it is very easily possible now, to register for another dump as soon as the scratch area is copied to a user designated region. But for now this simple implementation exists.
It is also possible to extend this further to only preserve pages that are kernel pages and free the non required pages like user/data pages etc. This would reduce the space preserved and would prevent any issues that are caused by reserving everything in memory except for the first 256 MB.
Improvements and future versions are planned to make this efficient. But for now the intent is to get this off the ground and handle simple cases.
Quoted 56 lines
Quoted 7 lines
with kdump,
you can't go into production until after the dump is finished, and the system has been rebooted a second time. On systems with terabytes of RAM, the time difference can be hours.The difference in time it takes to resume the normal workload may be significant, yes. But the time it takes to get a usable dump image would seem to be the basically the same.
Since you bring up large systems... a system with terabytes of RAM is practically guaranteed to be a NUMA configuration with dozens of cpus. When processing a dump on such a system, I wonder how well we fare:
can we successfully boot with (say) 128 cpus and 256MB of usable memory? Do we have to hot-online nodes as system memory is freed up (and does that even work)? We need to be able to restore the system to its optimal topology when the dump is finished; if the best we can do is a degraded configuration, the workload will suffer and the system admin is likely to just reboot the machine again so the kernel will have the right NUMA topology.Quoted 17 lines
Quoted 10 lines
Documentation/powerpc/phyp-assisted-dump.txt line 73
Quoted 6 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +6
-0+Implementation details: +---------------------- +In order for this scheme to work, memory needs to be reserved +quite early in the boot cycle. However, access to the device +tree this early in the boot cycle is difficult, and device-tree +access is needed to determine if there is a crash data waiting.I don't think this bit about early device tree access is correct. By the time your code is reserving memory (from
early_init_devtree(), I think), RTAS has been instantiated and you are able to test for the existence of /rtas/ibm,dump-kernel.If I remember right, it was still too early to look up this token directly, so we wrote some code to crawl the flat device tree to find it. But not only was that a lot of work, but I somehow decided that doing this to the flat tree was wrong, as otherwise someone would surely have written the access code. If this can be made to work, that would be great, but we couldn't make it work at the time.
Quoted 8 lines
Quoted 7 lines
Documentation/powerpc/phyp-assisted-dump.txt line 77
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+To work around this problem, all but 256MB of RAM is reserved +during early boot. A short while later in boot, a check is made +to determine if there is dump data waiting. If there isn't, +then the reserved memory is released to general kernel use.So I think these gymnastics are unneeded -- unless I'm misunderstanding something, you should be able to determine very early whether to reserve that memory.
Only if you can get at rtas, but you can't get at rtas at that point.
Sorry, but I think you are mistaken (see Michael's earlier reply).
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
In reply to Nathan LynchQuoted 18 lines · Nathan Lynch
Hi Linas,
Quoted 15 lines · Linas Vepstas
Quoted 8 lines · Nathan Lynch
Documentation/powerpc/phyp-assisted-dump.txt line 9
Quoted 5 lines · Manish Ahuja
Documentation/powerpc/phyp-assisted-dump.txt new file +5
-0+ +The goal of hypervisor-assisted dump is to enable the dump of +a crashed system, and to do so from a fully-reset system, and +to minimize the total elapsed time until the system is back +in production use.Is it actually faster than kdump?
This is a basic presumption;
As a side effect, the system is in
production *while* the dump is being taken;
It's in "production" with 256MB of RAM? Err. Sure as the dump progresses more RAM will be freed, but that's hardly production. I think Nathan's right, any sysadmin who wants predictability will probably double reboot anyway.
Quoted 6 lines
Quoted 5 lines
with kdump,
you can't go into production until after the dump is finished, and the system has been rebooted a second time. On systems with terabytes of RAM, the time difference can be hours.
Quoted 10 lines
Since you bring up large systems... a system with terabytes of RAM is practically guaranteed to be a NUMA configuration with dozens of cpus. When processing a dump on such a system, I wonder how well we fare:
can we successfully boot with (say) 128 cpus and 256MB of usable memory? Do we have to hot-online nodes as system memory is freed up (and does that even work)? We need to be able to restore the system to its optimal topology when the dump is finished; if the best we can do is a degraded configuration, the workload will suffer and the system admin is likely to just reboot the machine again so the kernel will have the right NUMA topology.
Yeah that's a good question. Even if the hot-onlining works, there's still kernel data structures allocated at boot which want to be node-local. So the end result will be != a "production" boot.
Quoted 28 lines
Quoted 28 lines
Quoted 11 lines
Documentation/powerpc/phyp-assisted-dump.txt line 73
Quoted 6 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +6
-0+Implementation details: +---------------------- +In order for this scheme to work, memory needs to be reserved +quite early in the boot cycle. However, access to the device +tree this early in the boot cycle is difficult, and device-tree +access is needed to determine if there is a crash data waiting.I don't think this bit about early device tree access is correct. By the time your code is reserving memory (from
early_init_devtree(), I think), RTAS has been instantiated and you are able to test for the existence of /rtas/ibm,dump-kernel.If I remember right, it was still too early to look up this token directly, so we wrote some code to crawl the flat device tree to find it. But not only was that a lot of work, but I somehow decided that doing this to the flat tree was wrong, as otherwise someone would surely have written the access code. If this can be made to work, that would be great, but we couldn't make it work at the time.
Quoted 8 lines
Documentation/powerpc/phyp-assisted-dump.txt line 77
Quoted 4 lines
Documentation/powerpc/phyp-assisted-dump.txt new file +4
-0+To work around this problem, all but 256MB of RAM is reserved +during early boot. A short while later in boot, a check is made +to determine if there is dump data waiting. If there isn't, +then the reserved memory is released to general kernel use.So I think these gymnastics are unneeded -- unless I'm misunderstanding something, you should be able to determine very early whether to reserve that memory.
Quoted 1 line
Quoted 1 line
Only if you can get at rtas, but you can't get at rtas at that point.
AFAICT you don't need to get at RTAS, you just need to look at the device tree to see if the property is present, and that is trivial.
You probably just need to add a check in early_init_dt_scan_rtas() which sets a flag for the PHYP dump stuff, or add your own scan routine if you need.
cheers
Signature · 8 lines
Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 4 lines
It's in "production" with 256MB of RAM? Err. Sure as the dump progresses more RAM will be freed, but that's hardly production. I think Nathan's right, any sysadmin who wants predictability will probably double reboot anyway.
Thats a changeable parameter. Its something we chose for now. It by no means is set in stone. Its not a design parameter. If you like to allocate 1GB we can. But that is something we did for now. we expect this to be a variable value dependent upon the size of the system. So if you have 128 GB system and you can spare 10 gb, you should be able to have 10 GB to boot with.
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
In reply to Michael EllermanQuoted 9 lines · Michael Ellerman
Quoted 2 lines
Quoted 2 lines
Only if you can get at rtas, but you can't get at rtas at that point.
AFAICT you don't need to get at RTAS, you just need to look at the device tree to see if the property is present, and that is trivial.
You probably just need to add a check in
early_init_dt_scan_rtas()which sets a flag for the PHYP dump stuff, or add your own scan routine if you need.
I no longer remember the details. I do remember spending a lot of time trying to figure out how to do this. I know I didn't want to write my own scan routine; maybe that's what stopped me. As it happens, we also did most of the development on a broken phyp which simply did not even have this property, no matter what, and so that may have brain-damaged me.
I went for the "most elegant" solution, where "most elegant" is defined as "fewest lines of code", "least effort", etc.
Manish may need some hands-on help to extract this token during early boot. Hopefully, he'll let us know.
--linas
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 11 lines · Linas Vepstas
Quoted 9 lines · Michael Ellerman
Quoted 2 lines
Quoted 2 lines
Only if you can get at rtas, but you can't get at rtas at that point.
AFAICT you don't need to get at RTAS, you just need to look at the device tree to see if the property is present, and that is trivial.
You probably just need to add a check in
early_init_dt_scan_rtas()which sets a flag for the PHYP dump stuff, or add your own scan routine if you need.
Quoted 5 lines
I no longer remember the details. I do remember spending a lot of time trying to figure out how to do this. I know I didn't want to write my own scan routine; maybe that's what stopped me. As it happens, we also did most of the development on a broken phyp which simply did not even have this property, no matter what, and so that may have brain-damaged me.
Sure, the API docs for the kernel are a little lacking ;)
Quoted 5 lines
I went for the "most elegant" solution, where "most elegant" is defined as "fewest lines of code", "least effort", etc.
Manish may need some hands-on help to extract this token during early boot. Hopefully, he'll let us know.
It would just be something like:
arch/powerpc/kernel/rtas.c +5 -0
@@ -901,6 +901,11 @@ int __init early_init_dt_scan_rtas(unsigned long node, rtas.size = *sizep; } +#ifdef CONFIG_PHYP_DUMP +if (of_get_flat_dt_prop (node,"ibm,dump-kernel" ,NULL )) + phyp_dump_is_active++; +#endif +#ifdef CONFIG_UDBG_RTAS_CONSOLE basep =of_get_flat_dt_prop (node,"put-term-char" ,NULL );if (basep)
Or to do your own scan routine:
arch/powerpc/kernel/prom.c +1 -0
@@ -1022,6 +1022,7 @@ void __init early_init_devtree(void *params)/* Some machines might need RTAS info for debugging, grab it now. */ of_scan_flat_dt (early_init_dt_scan_rtas,NULL );#endif +of_scan_flat_dt (early_init_dt_scan_phyp_dump,NULL );/* Retrieve various informations from the /chosen node of the * device-tree, including the platform type, initrd location and
arch/powerpc/kernel/rtas.c +13 -0
@@ -883,6 +883,19 @@ void __init rtas_initialize(void)#endif } +int __init early_init_dt_scan_phyp_dump (unsigned long node, +const char *uname,int depth,void *data) +{ +#ifdef CONFIG_PHYP_DUMP +if (depth !=1 ||strcmp (uname,"rtas" ) !=0 ) +return 0 ; + +if (of_get_flat_dt_prop (node,"ibm,dump-kernel" ,NULL )) + phyp_dump_is_active++; +#endif +return 1 ; +} +int __init early_init_dt_scan_rtas (unsigned long node,const char *uname,int depth,void *data) {
cheers
Signature · 8 lines
Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
In reply to Nathan LynchQuoted 8 lines · Nathan Lynch
Hi Linas,
Quoted 3 lines · Linas Vepstas
As a side effect, the system is in
production *while* the dump is being taken;A dubious feature IMO.
Hmm. Take it up with Ken Rozendal, this is supposed to be one of the two main selling points of this thing.
Quoted 5 lines
Seems that the design potentially trades
reliability of first failure data capture for availability. E.g. system crashes, reboots, resumes processing while copying dump, crashes again before dump procedure is complete. How is that handled, if at all?
Its handled by the hypervisor. phyp maintains the copy of the RMO of first crash, until such time that the OS declares the dump of the RMO to be complete. So you'll always have the RMO of the first crash.
For the rest of RAM, it will come in two parts: some portion will have been dumped already. The rest has not yet been dumped, and it will still be there, preserved across the second crash.
So you get both RMO and all of RAM from the first crash.
Quoted 9 lines
Quoted 5 lines
with kdump,
you can't go into production until after the dump is finished, and the system has been rebooted a second time. On systems with terabytes of RAM, the time difference can be hours.The difference in time it takes to resume the normal workload may be significant, yes. But the time it takes to get a usable dump image would seem to be the basically the same.
Yes.
Quoted 10 lines
Since you bring up large systems... a system with terabytes of RAM is practically guaranteed to be a NUMA configuration with dozens of cpus. When processing a dump on such a system, I wonder how well we fare:
can we successfully boot with (say) 128 cpus and 256MB of usable memory? Do we have to hot-online nodes as system memory is freed up (and does that even work)? We need to be able to restore the system to its optimal topology when the dump is finished; if the best we can do is a degraded configuration, the workload will suffer and the system admin is likely to just reboot the machine again so the kernel will have the right NUMA topology.
Heh. That's the elbow-grease of this thing. The easy part is to get the core function working. The hard part is to test these various configs, and when they don't work, figure out what went wrong. That will take perseverence and brains.
--linas
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 4 lines · Linas Vepstas
Heh. That's the elbow-grease of this thing. The easy part is to get the core function working. The hard part is to test these various configs, and when they don't work, figure out what went wrong. That will take perseverence and brains.
This just sounds like a whole lot of extra work to get a feature that already exists. Also, features like these seem to just get tested when the next enterprise distro is released, so they're broken for long stretches of time in mainline.
There's a bunch of problems like the NUMA ones, which would by far be easiest to solve by just doing another reboot or kexec, wouldn't they?
-Olof
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 9 lines · Olof Johansson
Quoted 4 lines · Linas Vepstas
Heh. That's the elbow-grease of this thing. The easy part is to get the core function working. The hard part is to test these various configs, and when they don't work, figure out what went wrong. That will take perseverence and brains.
This just sounds like a whole lot of extra work to get a feature that already exists.
Well, no. kexec is horribly ill-behaved with respect to PCI. The kexec kernel starts running with PCI devices in some random state; maybe they're DMA'ing or who knows what. kexec tries real hard to whack a few needed pci devices into submission but it has been hit-n-miss, and the source of 90% of the kexec headaches and debugging effort. Its not pretty.
If all pci-host bridges could shut-down or settle the bus, and raise the #RST line high, and then if all BIOS'es supported this, you'd be right. But they can't ....
--linas
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 11 lines · Linas Vepstas
Quoted 9 lines · Olof Johansson
Quoted 4 lines · Linas Vepstas
Heh. That's the elbow-grease of this thing. The easy part is to get the core function working. The hard part is to test these various configs, and when they don't work, figure out what went wrong. That will take perseverence and brains.
This just sounds like a whole lot of extra work to get a feature that already exists.
Quoted 6 lines
Well, no. kexec is horribly ill-behaved with respect to PCI. The kexec kernel starts running with PCI devices in some random state; maybe they're DMA'ing or who knows what. kexec tries real hard to whack a few needed pci devices into submission but it has been hit-n-miss, and the source of 90% of the kexec headaches and debugging effort. Its not pretty.
Isn't that what EEH and the IOMMU are for? :)
cheers
Signature · 8 lines
Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
In reply to Linas VepstasQuoted 11 lines · Linas Vepstas
Quoted 9 lines · Olof Johansson
Quoted 4 lines · Linas Vepstas
Heh. That's the elbow-grease of this thing. The easy part is to get the core function working. The hard part is to test these various configs, and when they don't work, figure out what went wrong. That will take perseverence and brains.
This just sounds like a whole lot of extra work to get a feature that already exists.
Quoted 6 lines
Well, no. kexec is horribly ill-behaved with respect to PCI. The kexec kernel starts running with PCI devices in some random state; maybe they're DMA'ing or who knows what. kexec tries real hard to whack a few needed pci devices into submission but it has been hit-n-miss, and the source of 90% of the kexec headaches and debugging effort. Its not pretty.
It surprises me that this hasn't been possible to resolve with less than architecting a completely new interface, given that the platform has all this fancy support for isolating and resetting adapters. After all, the exact same thing has to be done by the hypervisor before rebooting the partition.
Quoted 3 lines
If all pci-host bridges could shut-down or settle the bus, and raise the #RST line high, and then if all BIOS'es supported this, you'd be right. But they can't ....
This argument doesn't hold. We're not talking about some generic PC with a crappy BIOS here, we're specifically talking about POWER6 PHYP. It certainly already has ways to reset adapters in it, or EEH wouldn't work. Actually, the whole phyp dump feature wouldn't work either, since it's exactly what the firmware has to do under the covers as well.
-Olof
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 19 lines · Olof Johansson
Quoted 11 lines · Linas Vepstas
Quoted 9 lines · Olof Johansson
Quoted 4 lines · Linas Vepstas
Heh. That's the elbow-grease of this thing. The easy part is to get the core function working. The hard part is to test these various configs, and when they don't work, figure out what went wrong. That will take perseverence and brains.
This just sounds like a whole lot of extra work to get a feature that already exists.
Quoted 6 lines
Well, no. kexec is horribly ill-behaved with respect to PCI. The kexec kernel starts running with PCI devices in some random state; maybe they're DMA'ing or who knows what. kexec tries real hard to whack a few needed pci devices into submission but it has been hit-n-miss, and the source of 90% of the kexec headaches and debugging effort. Its not pretty.
Quoted 5 lines
It surprises me that this hasn't been possible to resolve with less than architecting a completely new interface, given that the platform has all this fancy support for isolating and resetting adapters. After all, the exact same thing has to be done by the hypervisor before rebooting the partition.
OK, point taken.
-- The phyp interfaces are there for AIX, which I guess must
not have kexec-like ability. So this is a case of Linux leveraging
a feature architected for AIX.
-- There's also this idea, somewhat weak, that the crash may
have corrupted the ram where the kexec kernel sits. For someone who is used to seeing crashes due to null pointer deref's, this seems fairly unlikely. But perhaps crashes in production systems are more mind-bending. (we did have a case where a USB stick used for boot continued to scribble on memory long after it was supposed to be quiet and unused. This resulted in
a very hard to debug crash.)
A solution to a corrupted
kexec kernel would be to disable memory access to where kexec sits, e.g un-mapping or making r/o the pages where it lies. This begs the questions of "who unhides the kexec kernel", and "what if this 'who' gets corrupted"?
In short, the kexec kernel does not boot exactly the same as a cold boot, and so this opens a can of worms about "well, what's different, how do we minimize these differences, etc." and I think that lead AIX to punt, and say "lets just use one single, well-known boot loader/ boot sequence instead of inventing a new one", thus leading to the phyp design.
But that's just my guess.. :-)
--linas
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 31 lines · Linas Vepstas
Quoted 22 lines · Olof Johansson
Quoted 13 lines · Linas Vepstas
Quoted 10 lines · Olof Johansson
Quoted 4 lines · Linas Vepstas
Heh. That's the elbow-grease of this thing. The easy part is to get the core function working. The hard part is to test these various configs, and when they don't work, figure out what went wrong. That will take perseverence and brains.
This just sounds like a whole lot of extra work to get a feature that already exists.
Quoted 6 lines
Well, no. kexec is horribly ill-behaved with respect to PCI. The kexec kernel starts running with PCI devices in some random state; maybe they're DMA'ing or who knows what. kexec tries real hard to whack a few needed pci devices into submission but it has been hit-n-miss, and the source of 90% of the kexec headaches and debugging effort. Its not pretty.
Quoted 5 lines
It surprises me that this hasn't been possible to resolve with less than architecting a completely new interface, given that the platform has all this fancy support for isolating and resetting adapters. After all, the exact same thing has to be done by the hypervisor before rebooting the partition.
Quoted 5 lines
OK, point taken.
-- The phyp interfaces are there for AIX, which I guess must
not have kexec-like ability. So this is a case of Linux leveraging
a feature architected for AIX.
Certainly AIX was in a more difficult position at the time, because they don't have a kexec equivalent, and thus were collecting dump data with a potentially faulty kernel. It makes sense to have something outside the partition collect or maintain the data; ideally, some kind of service partition would extract dump data from a failed partition, but giving one partition total access to the memory of another is clearly risky. Both the PHYP-assistance method and the kexec method are ways to simulate that without the risk.
At the risk of repeating what others have already said, the PHYP-assistance method provides some advantages that the kexec method cannot:
- Availability of the system for production use before the dump data is
collected. As was mentioned before, some production systems may choose not to operate with the limited memory initially available after the reboot, but it sure is nice to provide the option.
- Ensuring that the devices are in a good state. PHYP doesn't expose a method
to force adapters into a frozen state, (which I agree would be useful), and I don't know of any plans to do so. What we are starting to see is that some drivers need modifications in order to work correctly with kdump [1]. Supporting PHYP-assisted dump would eliminate those issues.
- The small possibility that the kexec area could have been munged by the
failing kernel, preventing it from being able to collect a dump.
The NUMA issues are daunting, but not insurmountable. Release early, release often, n'est-ce pas?
Mike
[1] http://ozlabs.org/pipermail/linuxppc-dev/2007-November/045663.html
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 7 lines · Mike Strosaker
At the risk of repeating what others have already said, the PHYP-assistance method provides some advantages that the kexec method cannot:
- Availability of the system for production use before the dump data is
collected. As was mentioned before, some production systems may choose not to operate with the limited memory initially available after the reboot, but it sure is nice to provide the option.
I'm more concerned that this design encourages the user to resume a workload *which is almost certainly known to result in a system crash* before collection of crash data is complete. Maybe the gamble will pay off most of the time, but I wouldn't want to be working support when it doesn't.
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 9 lines · Nathan Lynch
Quoted 7 lines · Mike Strosaker
At the risk of repeating what others have already said, the PHYP-assistance method provides some advantages that the kexec method cannot:
- Availability of the system for production use before the dump data is
collected. As was mentioned before, some production systems may choose not to operate with the limited memory initially available after the reboot, but it sure is nice to provide the option.
Quoted 5 lines
I'm more concerned that this design encourages the user to resume a workload *which is almost certainly known to result in a system crash* before collection of crash data is complete. Maybe the gamble will pay off most of the time, but I wouldn't want to be working support when it doesn't.
Workloads that cause crashes within hours of startup tend to be weeded-out/discovered during pre-production test of the system to be deployed. Since its pre-production test, dumps can be taken in a leisurely manner. Heck, even a session at the xmon prompt can be contemplated.
The problem is when the crash only reproduces after days or weeks of uptime, on a production machine. Since the machine is in production, its got to be brought back up ASAP. Since its crashing only after days/weeks, the dump should have plenty of time to complete. (And if it crashes quickly after that reboot ... well, support people always welcome ways in which a bug can be reproduced more quickly/easily).
--linas
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 22 lines · Linas Vepstas
Quoted 9 lines · Nathan Lynch
Quoted 7 lines · Mike Strosaker
At the risk of repeating what others have already said, the PHYP-assistance method provides some advantages that the kexec method cannot:
- Availability of the system for production use before the dump data is
collected. As was mentioned before, some production systems may choose not to operate with the limited memory initially available after the reboot, but it sure is nice to provide the option.Quoted 5 lines
I'm more concerned that this design encourages the user to resume a workload *which is almost certainly known to result in a system crash* before collection of crash data is complete. Maybe the gamble will pay off most of the time, but I wouldn't want to be working support when it doesn't.
Workloads that cause crashes within hours of startup tend to be weeded-out/discovered during pre-production test of the system to be deployed. Since its pre-production test, dumps can be taken in a leisurely manner. Heck, even a session at the xmon prompt can be contemplated.
Quoted 7 lines
The problem is when the crash only reproduces after days or weeks of uptime, on a production machine. Since the machine is in production, its got to be brought back up ASAP. Since its crashing only after days/weeks, the dump should have plenty of time to complete. (And if it crashes quickly after that reboot ... well, support people always welcome ways in which a bug can be reproduced more quickly/easily).
How do you expect to have it in full production if you don't have all resources available for it? It's not until the dump has finished that you can return all memory to the production environment and use it.
This can very easily be argued in both direction, with no clear winner:
If the crash is stress-induced (say a slashdotted website), for those cases it seems more rational to take the time, collect _good data_ even if it takes a little longer, and then go back into production. Especially if the alternative is to go back into production immediately, collect about half of the data, and then crash again. Rinse and repeat.
Anyway -- I can agree that some of the arguments w.r.t robustness and reliability of collecting dumps can be higher using this approach. It really surprises me that there's no way to reset a device through PHYP though. Seems like such a fundamental feature.
I think people are overly optimistic if they think it'll be possible to do all of this reliably (as in with consistent performance) without a second reboot though. At least without similar amounts of work being done as it would have taken to fix kdump's reliability in the first place.
Speaking of reboots. PHYP isn't known for being quick at rebooting a partition, it used to take in the order of minutes even on a small machine. Has that been fixed? If not, the avoiding an extra reboot argument hardly seems like a benefit versus kdump+kexec, which reboots nearly instantly and without involvement from PHYP.
-Olof
Re: [PATCH 1/8] pseries: phyp dump: Docmentation
Quoted 4 lines · Olof Johansson
How do you expect to have it in full production if you don't have all resources available for it? It's not until the dump has finished that you can return all memory to the production environment and use it.
With the PHYP dump, each chunk of RAM is returned for general use immediately after being dumped; so its not an all-or-nothing proposition. Production systems don't often hit 100% RAM use right out of the gate, they often take hours or days to get there, so again, there should be time to dump.
Quoted 6 lines
This can very easily be argued in both direction, with no clear winner:
If the crash is stress-induced (say a slashdotted website), for those cases it seems more rational to take the time, collect_gooddata_even if it takes a little longer, and then go back into production. Especially if the alternative is to go back into production immediately, collect about half of the data, and then crash again. Rinse and repeat.
Again, the mode of operation for the phyp dump is that you'll always have all of the data from the *first* crash, even if there are multiple crashes. That's because the the as-yet undumped RAM is not put back into production.
Quoted 2 lines
really surprises me that there's no way to reset a device through PHYP though. Seems like such a fundamental feature.
I don't know who said that; that's not right. The EEH function certainly does allow you to halt/restart PCI traffic to a particular device and also to reset the device. So, yes, the pSeries kexec code should call into the eeh subsystem to rationalize the device state.
Quoted 3 lines
I think people are overly optimistic if they think it'll be possible to do all of this reliably (as in with consistent performance) without a second reboot though.
The NUMA issues do concern me. But then, the whole virtualized, fractional-cpu, tickless operation stuff sounds like a performance tuning nightmare to begin with.
Quoted 2 lines
At least without similar amounts of work being
done as it would have taken to fix kdump's reliability in the first place.
:-)
Quoted 3 lines
Speaking of reboots. PHYP isn't known for being quick at rebooting a partition, it used to take in the order of minutes even on a small machine. Has that been fixed?
Dunno. Probably not.
Quoted 3 lines
If not, the avoiding an extra reboot
argument hardly seems like a benefit versus kdump+kexec, which reboots nearly instantly and without involvement from PHYP.
OK, let me tell you what I'm up against right now. I'm dealing with sporadic corruption on my home box.
About a month ago, I bought a whizzy ASUS M2NE motherboard & an AMD64 2-core cpu, and two sticks of RAM, 1GB per stick. I have one new hard drive, SATA, and one old hard drive, from my old machine, the PATA. The two disks are mirrored in a RAID-1 config. Running Ubuntu.
During install/upgrade a month ago, I noticed some of the install files seemed to have gotten corrupted, but that downloading them again got me a working version. This put a serious frown on my face: maybe a bad ethernet card or connection !?
Two weeks ago, gcc stopped working one morning, although it worked fine the night before. I'd done nothing in the interim but sleep. Reinstalling it made it work again. Yesterday, something else stopped working. I found the offending library, I compared file checksums against a known-good version, and they were off. (!!!) Disk corruption?
Then apt-get stopped working. The /var/lib/dpkg/status file had randomly corrupted single bytes. Its ascii, I hand repaired it; it had maybe 10 bad bytes out of 2MB total size.
I installed tripwire. Between the first run of tripwire, and the second, less than an hour later, it reported several dozen files have changed checksums. Manual inspection of some of these files against known-good versions show that, at least this morning, that's no longer the case.
System hasn't crashed in a month, since first boot. So what's going on? Is it possible that one of the two disks is serving up bad data, which explains the funny checksum behaviour? Or maybe its bad RAM, so that a fresh disk read shows good data? If its bad ram, why doesn't the system crash? I forced fsck last night, fsck came back spotless.
So ... moral of the story: If phyp is doing some sort of hardware checks and validation, that's great. I wish I could afford a pSeries system for my home computer, because my impression is that they are very stable, and don't do things like data corruption. I'm such a friggin cheapskate that I can't bear to spend many thousands instead of many hundreds of dollars. However, I will trade a longer boot for the dream of higher reliability.
--linas
PATCH 2/8 pseries: phyp dump: config file
In reply to Manish AhujaAdd hypervisor-assisted dump to kernel config
-----
arch/powerpc/Kconfig | 11 +++++++++++ 1 file changed, 11 insertions(+)
arch/powerpc/Kconfig +11 -0
@@ -261,6 +261,17 @@ config CRASH_DUMP Don't change this unless you know what you are doing. +config PHYP_DUMP + bool "Hypervisor-assisted dump (EXPERIMENTAL)" + depends on PPC_PSERIES && EXPERIMENTAL + default y + help + Hypervisor-assisted dump is meant to be a kdump replacement + offering robustness and speed not possible without system + hypervisor assistence. + + If unsure, say "Y" + config PPCBUG_NVRAM bool "Enable reading PPCBUG NVRAM during boot" if PPLUS || LOPEC default y if PPC_PREP
Re: [PATCH 2/8] pseries: phyp dump: config file
Quoted 5 lines · On Mon, 07 Jan 2008 18:16:08 -0600 Manish Ahuja <[email protected]> wrote:
Add hypervisor-assisted dump to kernel config
Signed-off-by Linas Vepstas <[email protected]>
This patch should probably come last in the series so that all the code is there before some bisecting autobuilder tries to configure it.
Signature · 3 lines
Cheers, Stephen Rothwell [email protected] http://www.canb.auug.org.au/~sfr/
PATCH 3/8 pseries: phyp dump: reserve-release proof-of-concept
In reply to Manish AhujaInitial patch for reserving memory in early boot, and freeing it later. If the previous boot had ended with a crash, the reserved memory would contain a copy of the crashed kernel data.
----
arch/powerpc/kernel/prom.c | 33 +++++++++++++ arch/powerpc/platforms/pseries/Makefile | 1 arch/powerpc/platforms/pseries/phyp_dump.c | 71 +++++++++++++++++++++++++++++ include/asm-powerpc/phyp_dump.h | 32 +++++++++++++ 4 files changed, 137 insertions(+)
include/asm-powerpc/phyp_dump.h new file +32 -0
@@ -0,0 +1,32 @@ +/* +* Hypervisor-assisted dump +* +* Linas Vepstas, Manish Ahuja 2007 +* Copyright (c) 2007 IBM Corp. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License +* as published by the Free Software Foundation; either version +* 2 of the License, or (at your option) any later version. +*/ + +#ifndef _PPC64_PHYP_DUMP_H +#define _PPC64_PHYP_DUMP_H + +#ifdef CONFIG_PHYP_DUMP + +/* The RMR region will be saved for later dumping +* whenever the kernel crashes. Set this to 256MB. */ +#define PHYP_DUMP_RMR_START 0x0 +#define PHYP_DUMP_RMR_END (1UL<<28) + +struct phyp_dump { +/* Memory that is reserved during very early boot. */ +unsigned long init_reserve_start; +unsigned long init_reserve_size; +}; + +extern struct phyp_dump *phyp_dump_info; + +#endif /* CONFIG_PHYP_DUMP */ +#endif /* _PPC64_PHYP_DUMP_H */
arch/powerpc/platforms/pseries/phyp_dump.c new file +71 -0
71 lines not on this page · raw message
arch/powerpc/platforms/pseries/Makefile +1 -0
@@ -18,3 +18,4 @@ obj-$(CONFIG_HOTPLUG_CPU) += hotplug-cpu obj-$(CONFIG_HVC_CONSOLE) += hvconsole.o obj-$(CONFIG_HVCS) += hvcserver.o obj-$(CONFIG_HCALL_STATS) += hvCall_inst.o +obj-$(CONFIG_PHYP_DUMP) += phyp_dump.o
arch/powerpc/kernel/prom.c +33 -0
@@ -51,6 +51,7 @@#include <asm/machdep.h> #include <asm/pSeries_reconfig.h> #include <asm/pci-bridge.h> +#include <asm/phyp_dump.h> #include <asm/kexec.h> #ifdef DEBUG
@@ -1011,6 +1012,37 @@ static void __init early_reserve_mem(voi#endif } +#ifdef CONFIG_PHYP_DUMP + +/** +* reserve_crashed_mem() - reserve all not-yet-dumped mmemory +* +* This routine will reserve almost all of the memory in the +* system, except for a few hundred megabytes used to boot the +* new kernel. As the reserved memory is dumped to the dump +* device (by userland tools), it will be freed and made available. +*/ +static void __init reserve_crashed_mem (void ) +{ +unsigned long crashed_base, crashed_size; + +/* Reserve *everything* above the RMR. We'll free this real soon. */ + crashed_base =PHYP_DUMP_RMR_END ; + crashed_size =lmb_end_of_DRAM () - crashed_base; + +/* XXX crashed_ram_end is wrong, since it may be beyond +* the memory_limit, it will need to be adjusted. */ +lmb_reserve (crashed_base, crashed_size); + + phyp_dump_info->init_reserve_start = crashed_base; + phyp_dump_info->init_reserve_size = crashed_size; +} + +#else +static inline void __init reserve_crashed_mem (void ) {} +#endif /* CONFIG_PHYP_DUMP */ + +void __init early_init_devtree (void *params) {DBG (" -> early_init_devtree(%p)\n" , params);
@@ -1043,6 +1075,7 @@ void __init early_init_devtree(void *parreserve_kdump_trampoline ();reserve_crashkernel ();early_reserve_mem (); +reserve_crashed_mem ();lmb_enforce_memory_limit (memory_limit);lmb_analyze ();
Re: [PATCH 3/8] pseries: phyp dump: reserve-release proof-of-concept
Hi Manish,
Just some trivial things ...
Quoted 4 lines · On Mon, 07 Jan 2008 18:25:31 -0600 Manish Ahuja <[email protected]> wrote:
+++
linux-2.6.24-rc2-git4/include/asm-powerpc/phyp_dump.h2007-11-19 17:44:21.000000000 -0600
+#ifndef_PPC64_PHYP_DUMP_H
We more usually use _ASM_POWERPC_PHYP_DUMP_H
include/asm-powerpc/phyp_dump.h line 16
Quoted 3 lines
include/asm-powerpc/phyp_dump.h new file +3
-0+#define _PPC64_PHYP_DUMP_H + +#ifdef CONFIG_PHYP_DUMP
Do these things really need protecting by this CONFIG variable? i.e. does anything change depending on the visibility of these various symbols?
arch/powerpc/platforms/pseries/phyp_dump.c line 5
Quoted 7 lines
+++
linux-2.6.24-rc2-git4/arch/powerpc/platforms/pseries/phyp_dump.c2007-11-19 19:07:49.000000000 -0600arch/powerpc/platforms/pseries/phyp_dump.c new file +5
-0@@ -0,0 +1,71 @@ +/* +* Hypervisor-assisted dump +* +* Linas Vepstas, Manish Ahuja 2007 +* Copyrhgit (c) 2007 IBM Corp.
^^^^^^^^^
typo. and you should really be using '©' in new copyright notices (or nothing).
arch/powerpc/platforms/pseries/phyp_dump.c line 33
Quoted 8 lines
arch/powerpc/platforms/pseries/phyp_dump.c new file +8
-0+/** +* release_memory_range -- release memory previously lmb_reserved +* @start_pfn: starting physical frame number +* @nr_pages: number of pages to free. +* +* This routine will release memory that had been previously +* lmb_reserved in early boot. The released memory becomes +* available for genreal use.
^^^^^^^
typo.
arch/powerpc/platforms/pseries/phyp_dump.c line 44
Quoted 9 lines
arch/powerpc/platforms/pseries/phyp_dump.c new file +9
-0+release_memory_range(unsigned long start_pfn, unsigned long nr_pages) +{ +struct page *rpage; +unsigned long end_pfn; +long i; + + end_pfn = start_pfn + nr_pages; + +for (i=start_pfn; i <= end_pfn; i++) {
spaces around '='
arch/powerpc/platforms/pseries/phyp_dump.c line 71
Quoted 6 lines
arch/powerpc/platforms/pseries/phyp_dump.c new file +5
-0+static int __init phyp_dump_setup (void ) +{ +} + +subsys_initcall (phyp_dump_setup);
Normally we don't leave a blank line here.
Signature · 3 lines
Cheers, Stephen Rothwell [email protected] http://www.canb.auug.org.au/~sfr/
Re: [PATCH 3/8] pseries: phyp dump: reserve-release proof-of-concept
In reply to Manish AhujaQuoted 3 lines · Manish Ahuja
Initial patch for reserving memory in early boot, and freeing it later. If the previous boot had ended with a crash, the reserved memory would contain a copy of the crashed kernel data.
The main problem I see here is that if this option is turned on, the kernel now has only 256MB of memory from early boot until subsys_initcalls are done -- on any machine, and whether or not there is actually a dump. That means, for instance, that a machine running bare-metal (such as a G5) might not be able to allocate the hash table for the MMU. Also, any allocations made during that time won't be able to be node-local.
So it will be necessary to read the flattened device tree early on to see whether or not there is a dump, so that we don't reserve most of memory in the cases where there isn't a dump.
Paul.
PATCH 4/8 pseries: phyp dump: use sysfs to release reserved mem
In reply to Manish AhujaCheck to see if there actually is data from a previously crashed kernel waiting. If so, Allow user-sapce tools to grab the data (by reading /proc/kcore). When user-space finishes dumping a section, it must release that memory by writing to sysfs. For example,
echo "0x40000000 0x10000000" > /sys/kernel/release_region
will release 256MB starting at the 1GB. The released memory becomes free for general use.
------
arch/powerpc/platforms/pseries/phyp_dump.c | 101 +++++++++++++++++++++++++++-- 1 file changed, 96 insertions(+), 5 deletions(-)
arch/powerpc/platforms/pseries/phyp_dump.c +96 -5
@@ -12,17 +12,24 @@*/ #include <linux/init.h> +#include <linux/kobject.h> #include <linux/mm.h> +#include <linux/of.h> #include <linux/pfn.h> #include <linux/swap.h> +#include <linux/sysfs.h> #include <asm/page.h> #include <asm/phyp_dump.h> +#include <asm/rtas.h> /* Global, used to communicate data between early boot and late boot */ static struct phyp_dump phyp_dump_global;struct phyp_dump *phyp_dump_info = &phyp_dump_global; +static int ibm_configure_kernel_dump; + +/* ------------------------------------------------- */ /** * release_memory_range -- release memory previously lmb_reserved * @start_pfn: starting physical frame number
@@ -52,18 +59,102 @@ release_memory_range(unsigned long start } } -static int __init phyp_dump_setup (void ) +/* ------------------------------------------------- */ +/** +* sysfs_release_region -- sysfs interface to release memory range. +* +* Usage: +* "echo <start addr> <length> > /sys/kernel/release_region" +* +* Example: +* "echo 0x40000000 0x10000000 > /sys/kernel/release_region" +* +* will release 256MB starting at 1GB. +*/ +static ssize_t +store_release_region (struct kset *kset,const char *buf,size_t count) { +unsigned long start_addr, length, end_addr;unsigned long start_pfn, nr_pages; +ssize_t ret; -/* If no memory was reserved in early boot, there is nothing to do */ -if (phyp_dump_info->init_reserve_size ==0 ) -return 0 ; + ret =sscanf (buf,"%lx %lx" , &start_addr, &length); +if (ret !=2 ) +return -EINVAL ; + +/* Range-check - don't free any reserved memory that +* wasn't reserved for phyp-dump */ +if (start_addr < phyp_dump_info->init_reserve_start) + start_addr = phyp_dump_info->init_reserve_start; + + end_addr = phyp_dump_info->init_reserve_start + + phyp_dump_info->init_reserve_size; +if (start_addr+length > end_addr) + length = end_addr - start_addr; + +/* Release the region of memory assed in by user */ + start_pfn =PFN_DOWN (start_addr); + nr_pages =PFN_DOWN (length); +release_memory_range (start_pfn, nr_pages); + +return count; +} + +static ssize_t +show_release_region (struct kset * kset,char *buf) +{ +return sprintf (buf,"ola\n" ); +} + +static struct subsys_attribute rr =__ATTR (release_region,0600 , + show_release_region, + store_release_region); + +/* ------------------------------------------------- */ + +static void release_all (void ) +{ +unsigned long start_pfn, nr_pages; -/* Release memory that was reserved in early boot */ +/* Release all memory that was reserved in early boot */ start_pfn =PFN_DOWN (phyp_dump_info->init_reserve_start); nr_pages =PFN_DOWN (phyp_dump_info->init_reserve_size);release_memory_range (start_pfn, nr_pages); +} + +static int __init phyp_dump_setup (void ) +{ +struct device_node *rtas; +const int *dump_header; +int header_len =0 ; +int rc; + +/* If no memory was reserved in early boot, there is nothing to do */ +if (phyp_dump_info->init_reserve_size ==0 ) +return 0 ; + +/* Return if phyp dump not supported */ + ibm_configure_kernel_dump =rtas_token ("ibm,configure-kernel-dump" ); +if (ibm_configure_kernel_dump ==RTAS_UNKNOWN_SERVICE ) { +release_all (); +return -ENOSYS ; + } + +/* Is there dump data waiting for us? */ + rtas =of_find_node_by_path ("/rtas" ); + dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len); +if (dump_header ==NULL ) { +release_all (); +return 0 ; + } + +/* Should we create a dump_subsys, analogous to s390/ipl.c ? */ + rc =subsys_create_file (&kernel_subsys, &rr); +if (rc) { +printk (KERN_ERR "phyp-dump: unable to create sysfs file (%d)\n" , rc); +release_all (); +return 0 ; + }return 0 ; }
Re: [PATCH 4/8] pseries: phyp dump: use sysfs to release reserved mem
arch/powerpc/platforms/pseries/phyp_dump.c release_memory_range(unsigned long start · line 145
Quoted 7 lines · On Mon, 07 Jan 2008 18:21:57 -0600 Manish Ahuja <[email protected]> wrote:
arch/powerpc/platforms/pseries/phyp_dump.c +5
-0+static int __init phyp_dump_setup (void ) +{ +/* Is there dump data waiting for us? */ + rtas =of_find_node_by_path ("/rtas" ); + dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len);
^^^^^^^^^^^
You could pass NULL here as header_len appears to be unused. Also you need "of_node_put(rtas)" somewhere (probably just here would do).
Quoted 4 lines
arch/powerpc/platforms/pseries/phyp_dump.c +4
-0+if (dump_header ==NULL ) { +release_all (); +return 0 ; + }
Signature · 3 lines
Cheers, Stephen Rothwell [email protected] http://www.canb.auug.org.au/~sfr/
Re: [PATCH 4/8] pseries: phyp dump: use sysfs to release reserved mem
Quoted 11 lines · Stephen Rothwell
arch/powerpc/platforms/pseries/phyp_dump.c release_memory_range(unsigned long start · line 145
Quoted 7 lines · On Mon, 07 Jan 2008 18:21:57 -0600 Manish Ahuja <[email protected]> wrote:
arch/powerpc/platforms/pseries/phyp_dump.c +5
-0+static int __init phyp_dump_setup (void ) +{ +/* Is there dump data waiting for us? */ + rtas =of_find_node_by_path ("/rtas" ); + dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len);^^^^^^^^^^^You could pass NULL here as
header_lenappears to be unused. Also you need "of_node_put(rtas)" somewhere (probably just here would do).
Perhaps the routine should have been "of_get_node_by_path()" ?
In ye olden days, "finds" didn't require put, but "gets" did. I'm guessing that this has now all been fixed up for the of_xxx routines, but I think that pci_find_xxx still does not require a pci_put.
Why did I bother to write this email? I dunno...
--linas
PATCH 5/8 pseries: phyp dump: register dump area.
In reply to Manish AhujaSet up the actual dump header, register it with the hypervisor.
------
arch/powerpc/platforms/pseries/phyp_dump.c | 169 +++++++++++++++++++++++++++-- 1 file changed, 163 insertions(+), 6 deletions(-)
arch/powerpc/platforms/pseries/phyp_dump.c +163 -6
@@ -30,6 +30,134 @@ struct phyp_dump *phyp_dump_info = &phypstatic int ibm_configure_kernel_dump;/* ------------------------------------------------- */ +/* RTAS interfaces to declare the dump regions */ + +struct dump_section { +u32 dump_flags; +u16 source_type; +u16 error_flags; +u64 source_address; +u64 source_length; +u64 length_copied; +u64 destination_address; +}; + +struct phyp_dump_header { +u32 version; +u16 num_of_sections; +u16 status; + +u32 first_offset_section; +u32 dump_disk_section; +u64 block_num_dd; +u64 num_of_blocks_dd; +u32 offset_dd; +u32 maxtime_to_auto; +/* No dump disk path string used */ + +struct dump_section cpu_data; +struct dump_section hpte_data; +struct dump_section kernel_data; +}; + +/* The dump header *must be* in low memory, so .bss it */ +static struct phyp_dump_header phdr; + +#define NUM_DUMP_SECTIONS 3 +#define DUMP_HEADER_VERSION 0x1 +#define DUMP_REQUEST_FLAG 0x1 +#define DUMP_SOURCE_CPU 0x0001 +#define DUMP_SOURCE_HPTE 0x0002 +#define DUMP_SOURCE_RMO 0x0011 + +/** +* init_dump_header() - initialize the header declaring a dump +* Returns: length of dump save area. +* +* When the hypervisor saves crashed state, it needs to put +* it somewhere. The dump header tells the hypervisor where +* the data can be saved. +*/ +static unsigned long init_dump_header (struct phyp_dump_header *ph) +{ +struct device_node *rtas; +const unsigned int *sizes; +int len; +unsigned long cpu_state_size =0 ; +unsigned long hpte_region_size =0 ; +unsigned long addr_offset =0 ; + +/* Get the required dump region sizes */ + rtas =of_find_node_by_path ("/rtas" ); + sizes =of_get_property (rtas,"ibm,configure-kernel-dump-sizes" , &len); +if (!sizes || len <20 ) +return 0 ; + +if (sizes[0 ] ==1 ) + cpu_state_size = *((unsigned long *) &sizes[1 ]); + +if (sizes[3 ] ==2 ) + hpte_region_size = *((unsigned long *) &sizes[4 ]); + +/* Set up the dump header */ + ph->version =DUMP_HEADER_VERSION ; + ph->num_of_sections =NUM_DUMP_SECTIONS ; + ph->status =0 ; + + ph->first_offset_section = + (u32) &(((struct phyp_dump_header *)0 )->cpu_data); + ph->dump_disk_section =0 ; + ph->block_num_dd =0 ; + ph->num_of_blocks_dd =0 ; + ph->offset_dd =0 ; + + ph->maxtime_to_auto =0 ;/* disabled */ + +/* The first two sections are mandatory */ + ph->cpu_data.dump_flags =DUMP_REQUEST_FLAG ; + ph->cpu_data.source_type =DUMP_SOURCE_CPU ; + ph->cpu_data.source_address =0 ; + ph->cpu_data.source_length = cpu_state_size; + ph->cpu_data.destination_address = addr_offset; + addr_offset += cpu_state_size; + + ph->hpte_data.dump_flags =DUMP_REQUEST_FLAG ; + ph->hpte_data.source_type =DUMP_SOURCE_HPTE ; + ph->hpte_data.source_address =0 ; + ph->hpte_data.source_length = hpte_region_size; + ph->hpte_data.destination_address = addr_offset; + addr_offset += hpte_region_size; + +/* This section describes the low kernel region */ + ph->kernel_data.dump_flags =DUMP_REQUEST_FLAG ; + ph->kernel_data.source_type =DUMP_SOURCE_RMO ; + ph->kernel_data.source_address =PHYP_DUMP_RMR_START ; + ph->kernel_data.source_length =PHYP_DUMP_RMR_END ; + ph->kernel_data.destination_address = addr_offset; + addr_offset += ph->kernel_data.source_length; + +return addr_offset; +} + +static void register_dump_area (struct phyp_dump_header *ph,unsigned long addr) +{ +int rc; + ph->cpu_data.destination_address += addr; + ph->hpte_data.destination_address += addr; + ph->kernel_data.destination_address += addr; + +do { + rc =rtas_call (ibm_configure_kernel_dump,3 ,1 ,NULL , +1 , ph,sizeof (struct phyp_dump_header )); + }while (rtas_busy_delay (rc)); + +if (rc) + { +printk (KERN_ERR "phyp-dump: unexpected error (%d) on register\n" , rc); + } +} + +/* ------------------------------------------------- */ /** * release_memory_range -- release memory previously lmb_reserved * @start_pfn: starting physical frame number
@@ -125,7 +253,11 @@ static void release_all (void)static int __init phyp_dump_setup (void ) {struct device_node *rtas; -const int *dump_header; +const struct phyp_dump_header *dump_header; +unsigned long dump_area_start; +unsigned long dump_area_length; +unsigned long free_area_length; +unsigned long start_pfn, nr_pages;int header_len =0 ;int rc;
@@ -140,22 +272,47 @@ static int __init phyp_dump_setup(void)return -ENOSYS ; } -/* Is there dump data waiting for us? */ +/* Is there dump data waiting for us? If there isn't, +* then register a new dump area, and release all of +* the rest of the reserved ram. +* +* The /rtas/ibm,kernel-dump rtas node is present only +* if there is dump data waiting for us. +*/ rtas =of_find_node_by_path ("/rtas" ); dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len); + + dump_area_length =init_dump_header (&phdr); + free_area_length = phyp_dump_info->init_reserve_size - dump_area_length; + dump_area_start = phyp_dump_info->init_reserve_start + free_area_length; + dump_area_start = dump_area_start &PAGE_MASK ;/* align down */ + free_area_length = dump_area_start - phyp_dump_info->init_reserve_start; +if (dump_header ==NULL ) { -release_all (); -return 0 ; +register_dump_area (&phdr, dump_area_start); + goto release_mem; } +/* Don't allow user to release the 256MB scratch area */ + phyp_dump_info->init_reserve_size = free_area_length; +/* Should we create a dump_subsys, analogous to s390/ipl.c ? */ rc =subsys_create_file (&kernel_subsys, &rr);if (rc) {printk (KERN_ERR "phyp-dump: unable to create sysfs file (%d)\n" , rc); -release_all (); -return 0 ; + goto release_mem; } +/* ToDo: re-register the dump area, for next time. */ + +return 0 ; + +release_mem: +/* release everything except the top 256 MB scratch area */ + start_pfn =PFN_DOWN (phyp_dump_info->init_reserve_start); + nr_pages =PFN_DOWN (free_area_length); +release_memory_range (start_pfn, nr_pages); +return 0 ; }
Quoted 13 lines
documentation explaining what this is :-) . Yes, its supposed to be an improvement over kdump.
The patches mostly work; a list of open issues / todo list is included in the documentation. It also appears that the not-yet-released firmware versions this was tested on are still, ahem, incomplete; this work is also pending.
-- Linas & Manish
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 5/8] pseries: phyp dump: register dump area.
Hi Manish,
arch/powerpc/platforms/pseries/phyp_dump.c struct phyp_dump *phyp_dump_info = &phyp · line 91
Quoted 7 lines · On Mon, 07 Jan 2008 18:28:30 -0600 Manish Ahuja <[email protected]> wrote:
+++
linux-2.6.24-rc3-git1/arch/powerpc/platforms/pseries/phyp_dump.c2007-11-21 16:06:52.000000000 -0600arch/powerpc/platforms/pseries/phyp_dump.c +4
-0+static unsigned long init_dump_header (struct phyp_dump_header *ph) +{ +/* Get the required dump region sizes */ + rtas =of_find_node_by_path ("/rtas" );
You need to of_node_put(rtas) somewhere.
arch/powerpc/platforms/pseries/phyp_dump.c struct phyp_dump *phyp_dump_info = &phyp · line 97
Quoted 2 lines
arch/powerpc/platforms/pseries/phyp_dump.c +2
-0+if (sizes[0 ] ==1 ) + cpu_state_size = *((unsigned long *) &sizes[1 ]);
We normally don't put spaces after casts.
arch/powerpc/platforms/pseries/phyp_dump.c struct phyp_dump *phyp_dump_info = &phyp · line 108
Quoted 2 lines
arch/powerpc/platforms/pseries/phyp_dump.c +2
-0+ ph->first_offset_section = + (u32) &(((struct phyp_dump_header *)0 )->cpu_data);
(u32)offsetof(struct phyp_dump_header, cpu_data);
arch/powerpc/platforms/pseries/phyp_dump.c struct phyp_dump *phyp_dump_info = &phyp · line 157
Quoted 7 lines
arch/powerpc/platforms/pseries/phyp_dump.c +6
-0+static void register_dump_area (struct phyp_dump_header *ph,unsigned long addr) +{ +if (rc) + { +printk (KERN_ERR "phyp-dump: unexpected error (%d) on register\n" , rc); + }
The braces are not needed.
arch/powerpc/platforms/pseries/phyp_dump.c static int __init phyp_dump_setup(void) · line 285
Quoted 2 lines
arch/powerpc/platforms/pseries/phyp_dump.c +2
-0+ + dump_area_length =init_dump_header (&phdr);
We don't put spaces after function names.
arch/powerpc/platforms/pseries/phyp_dump.c static int __init phyp_dump_setup(void) · line 292
Quoted 9 lines
arch/powerpc/platforms/pseries/phyp_dump.c +6
-2+ free_area_length = phyp_dump_info->init_reserve_size - dump_area_length; + dump_area_start = phyp_dump_info->init_reserve_start + free_area_length; + dump_area_start = dump_area_start &PAGE_MASK ;/* align down */ + free_area_length = dump_area_start - phyp_dump_info->init_reserve_start; +if (dump_header ==NULL ) { -release_all (); -return 0 ; +register_dump_area (&phdr, dump_area_start);
Ditto.
Signature · 3 lines
Cheers, Stephen Rothwell [email protected] http://www.canb.auug.org.au/~sfr/
PATCH 6/8 pseries: phyp dump: debugging print routines.
In reply to Manish AhujaProvide some basic debugging support.
-----
arch/powerpc/platforms/pseries/phyp_dump.c | 53 ++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-)
arch/powerpc/platforms/pseries/phyp_dump.c +52 -1
@@ -2,7 +2,7 @@* Hypervisor-assisted dump * * Linas Vepstas, Manish Ahuja 2007 -* Copyrhgit (c) 2007 IBM Corp. +* Copyright (c) 2007 IBM Corp. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License
@@ -139,6 +139,51 @@ static unsigned long init_dump_header(streturn addr_offset; } +#ifdef DEBUG +static void print_dump_header (const struct phyp_dump_header *ph) +{ +printk (KERN_INFO "dump header:\n" ); +/* setup some ph->sections required */ +printk (KERN_INFO "version = %d\n" , ph->version); +printk (KERN_INFO "Sections = %d\n" , ph->num_of_sections); +printk (KERN_INFO "Status = 0x%x\n" , ph->status); + +/* No ph->disk, so all should be set to 0 */ +printk (KERN_INFO "Offset to first section 0x%x\n" , ph->first_offset_section); +printk (KERN_INFO "dump disk sections should be zero\n" ); +printk (KERN_INFO "dump disk section = %d\n" ,ph->dump_disk_section); +printk (KERN_INFO "block num = %ld\n" ,ph->block_num_dd); +printk (KERN_INFO "number of blocks = %ld\n" ,ph->num_of_blocks_dd); +printk (KERN_INFO "dump disk offset = %d\n" ,ph->offset_dd); +printk (KERN_INFO "Max auto time= %d\n" ,ph->maxtime_to_auto); + +/*set cpu state and hpte states as well scratch pad area */ +printk (KERN_INFO " CPU AREA \n" ); +printk (KERN_INFO "cpu dump_flags =%d\n" ,ph->cpu_data.dump_flags); +printk (KERN_INFO "cpu source_type =%d\n" ,ph->cpu_data.source_type); +printk (KERN_INFO "cpu error_flags =%d\n" ,ph->cpu_data.error_flags); +printk (KERN_INFO "cpu source_address =%lx\n" ,ph->cpu_data.source_address); +printk (KERN_INFO "cpu source_length =%lx\n" ,ph->cpu_data.source_length); +printk (KERN_INFO "cpu length_copied =%lx\n" ,ph->cpu_data.length_copied); + +printk (KERN_INFO " HPTE AREA \n" ); +printk (KERN_INFO "HPTE dump_flags =%d\n" ,ph->hpte_data.dump_flags); +printk (KERN_INFO "HPTE source_type =%d\n" ,ph->hpte_data.source_type); +printk (KERN_INFO "HPTE error_flags =%d\n" ,ph->hpte_data.error_flags); +printk (KERN_INFO "HPTE source_address =%lx\n" ,ph->hpte_data.source_address); +printk (KERN_INFO "HPTE source_length =%lx\n" ,ph->hpte_data.source_length); +printk (KERN_INFO "HPTE length_copied =%lx\n" ,ph->hpte_data.length_copied); + +printk (KERN_INFO " SRSD AREA \n" ); +printk (KERN_INFO "SRSD dump_flags =%d\n" ,ph->kernel_data.dump_flags); +printk (KERN_INFO "SRSD source_type =%d\n" ,ph->kernel_data.source_type); +printk (KERN_INFO "SRSD error_flags =%d\n" ,ph->kernel_data.error_flags); +printk (KERN_INFO "SRSD source_address =%lx\n" ,ph->kernel_data.source_address); +printk (KERN_INFO "SRSD source_length =%lx\n" ,ph->kernel_data.source_length); +printk (KERN_INFO "SRSD length_copied =%lx\n" ,ph->kernel_data.length_copied); +} +#endif +static void register_dump_area (struct phyp_dump_header *ph,unsigned long addr) {int rc;
@@ -154,6 +199,9 @@ static void register_dump_area(struct phif (rc) {printk (KERN_ERR "phyp-dump: unexpected error (%d) on register\n" , rc); +#ifdef DEBUG +print_dump_header (ph); +#endif } }
@@ -271,6 +319,9 @@ static int __init phyp_dump_setup(void)release_all ();return -ENOSYS ; } +#ifdef DEBUG +print_dump_header (dump_header); +#endif /* Is there dump data waiting for us? If there isn't, * then register a new dump area, and release all of
Re: [PATCH 6/8] pseries: phyp dump: debugging print routines.
arch/powerpc/platforms/pseries/phyp_dump.c static unsigned long init_dump_header(st
Quoted 6 lines · Manish Ahuja
arch/powerpc/platforms/pseries/phyp_dump.c +5
-0+#ifdef DEBUG +static void print_dump_header(const struct phyp_dump_header *ph) +{ +=A0=A0=A0=A0=A0=A0=A0printk(KERN_INFO "dump header:\n"); +=A0=A0=A0=A0=A0=A0=A0printk(KERN_INFO "SRSD length_copied =3D%lx\n",ph->=
kernel_data.length_copied);
arch/powerpc/platforms/pseries/phyp_dump.c static unsigned long init_dump_header(st · line 186
Quoted 3 lines
arch/powerpc/platforms/pseries/phyp_dump.c +3
-0+} +#endif +
If you move the #if to inside of the function,
Quoted 1 line
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0printk (
KERN_ERR"phyp-du=
mp: unexpected error (%d) on register\n", rc);
arch/powerpc/platforms/pseries/phyp_dump.c static unsigned long init_dump_header(st · line 185
Quoted 9 lines
arch/powerpc/platforms/pseries/phyp_dump.c +3
-0+#ifdef DEBUG +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0print_dump_header (ph); +#endif =A0=A0=A0=A0=A0=A0=A0=A0}
=A0=A0=A0=A0=A0=A0=A0=A0}
arch/powerpc/platforms/pseries/phyp_dump.c +3
-0+#ifdef DEBUG +=A0=A0=A0=A0=A0=A0=A0print_dump_header (dump_header); +#endif
you don't need the #ifdefs here.
Arnd <><
Re: [PATCH 6/8] pseries: phyp dump: debugging print routines.
In reply to Manish AhujaHi Manish,
Just a trivial comment.
arch/powerpc/platforms/pseries/phyp_dump.c static unsigned long init_dump_header(st · line 154
Quoted 2 lines · On Mon, 07 Jan 2008 18:35:09 -0600 Manish Ahuja <[email protected]> wrote:
arch/powerpc/platforms/pseries/phyp_dump.c +1
-0+printk (KERN_INFO "dump disk section = %d\n" ,ph->dump_disk_section);
Please put a space after the ','. Here and later.
Signature · 3 lines
Cheers, Stephen Rothwell [email protected] http://www.canb.auug.org.au/~sfr/
PATCH 7/8 pseries: phyp dump: Unregister and print dump areas.
In reply to Manish AhujaRoutines to invalidate and unregister dump routines. Unregister has not been used yet, I will release another patch for that at a later stage with the kdump integration patches.
There is also a routine which calculates the regions to be freed and exports that through sysfs.
--- arch/powerpc/platforms/pseries/phyp_dump.c | 79 ++++++++++++++++++++++++++--- 1 file changed, 73 insertions(+), 6 deletions(-)
arch/powerpc/platforms/pseries/phyp_dump.c +73 -6
@@ -69,6 +69,10 @@ static struct phyp_dump_header phdr;#define DUMP_SOURCE_CPU 0x0001#define DUMP_SOURCE_HPTE 0x0002#define DUMP_SOURCE_RMO 0x0011 +#define DUMP_ERROR_FLAG 0x2000 +#define DUMP_TRIGGERED 0x4000 +#define DUMP_PERFORMED 0x8000 +/** * init_dump_header() - initialize the header declaring a dump
@@ -187,10 +191,15 @@ static void print_dump_header(const strustatic void register_dump_area (struct phyp_dump_header *ph,unsigned long addr) {int rc; - ph->cpu_data.destination_address += addr; - ph->hpte_data.destination_address += addr; - ph->kernel_data.destination_address += addr; +/* Add addr value if not initialized before */ +if (ph->cpu_data.destination_address ==0 ) { + ph->cpu_data.destination_address += addr; + ph->hpte_data.destination_address += addr; + ph->kernel_data.destination_address += addr; + } + +/* ToDo Invalidate kdump and free memory range. */ do { rc = rtas_call(ibm_configure_kernel_dump, 3, 1, NULL, 1, ph, sizeof(struct phyp_dump_header));
@@ -205,6 +214,49 @@ static void register_dump_area(struct ph } } +static void invalidate_last_dump (struct phyp_dump_header *ph,unsigned long addr) +{ +int rc; + +/* Add addr value if not initialized before */ +if (ph->cpu_data.destination_address ==0 ) { + ph->cpu_data.destination_address = addr; + ph->hpte_data.destination_address += addr; + ph->kernel_data.destination_address += addr; + } + +do { + rc =rtas_call (ibm_configure_kernel_dump,3 ,1 ,NULL , +2 , ph,sizeof (struct phyp_dump_header )); + }while (rtas_busy_delay (rc)); + +if (rc) + { +printk (KERN_ERR "phyp-dump: unexpected error (%d) on invalidate\n" , rc); +#ifdef DEBUG +print_dump_header (ph); +#endif + } +} + +static void unregister_dump_area (struct phyp_dump_header *ph) +{ +int rc; + +do { + rc =rtas_call (ibm_configure_kernel_dump,3 ,1 ,NULL , +3 , ph,sizeof (struct phyp_dump_header )); + }while (rtas_busy_delay (rc)); + +if (rc) + { +printk (KERN_ERR "phyp-dump: unexpected error (%d) on unregister\n" , rc); +#ifdef DEBUG +print_dump_header (ph); +#endif + } +} +/* ------------------------------------------------- */ /** * release_memory_range -- release memory previously lmb_reserved
@@ -279,7 +331,17 @@ store_release_region(struct kset *kset,static ssize_t show_release_region (struct kset * kset,char *buf) { -return sprintf (buf,"ola\n" ); +u64 second_addr_range; + +/* total reserved size - start of scratch area */ + second_addr_range = phdr.cpu_data.destination_address - + phyp_dump_info->init_reserve_size; +return sprintf (buf,"CPU:0x%lx-0x%lx: HPTE:0x%lx-0x%lx:" +" DUMP:0x%lx-0x%lx, 0x%lx-0x%lx:\n" , + phdr.cpu_data.destination_address, phdr.cpu_data.length_copied, + phdr.hpte_data.destination_address, phdr.hpte_data.length_copied, + phdr.kernel_data.destination_address, phdr.kernel_data.length_copied, + phyp_dump_info->init_reserve_start, second_addr_range); } static struct subsys_attribute rr = __ATTR(release_region, 0600,
@@ -344,6 +406,13 @@ static int __init phyp_dump_setup(void) goto release_mem; } +/* re-register the dump area, if old dump was invalid */ +if ((dump_header) && (dump_header->status &DUMP_ERROR_FLAG )) { +invalidate_last_dump (&phdr, dump_area_start); +register_dump_area (&phdr, dump_area_start); + goto release_mem; + } +/* Don't allow user to release the 256MB scratch area */ phyp_dump_info->init_reserve_size = free_area_length;
@@ -354,8 +423,6 @@ static int __init phyp_dump_setup(void) goto release_mem; } -/* ToDo: re-register the dump area, for next time. */ -return 0 ; release_mem:
Re: [PATCH 7/8] pseries: phyp dump: Unregister and print dump areas.
Hi Manish,
Just more trivial stuff.
arch/powerpc/platforms/pseries/phyp_dump.c static void print_dump_header(const stru · line 197
Quoted 11 lines · On Mon, 07 Jan 2008 18:37:31 -0600 Manish Ahuja <[email protected]> wrote:
static void
register_dump_area(struct phyp_dump_header *ph, unsigned long addr){arch/powerpc/platforms/pseries/phyp_dump.c +3
-3int rc; - ph->cpu_data.destination_address += addr; - ph->hpte_data.destination_address += addr; - ph->kernel_data.destination_address += addr; +/* Add addr value if not initialized before */ +if (ph->cpu_data.destination_address ==0 ) { + ph->cpu_data.destination_address += addr;
Could be just '=' like further down, right?
arch/powerpc/platforms/pseries/phyp_dump.c static void register_dump_area(struct ph · line 234
Quoted 5 lines
arch/powerpc/platforms/pseries/phyp_dump.c +4
-0+static void invalidate_last_dump (struct phyp_dump_header *ph,unsigned long addr) +{ +if (rc) + {
We prefer
if (rc) {arch/powerpc/platforms/pseries/phyp_dump.c static void register_dump_area(struct ph · line 252
Quoted 5 lines
arch/powerpc/platforms/pseries/phyp_dump.c +4
-0+static void unregister_dump_area (struct phyp_dump_header *ph) +{ +if (rc) + {
And again.
Quoted 2 lines
static
ssize_t
show_release_region(struct kset * kset, char *buf)
We normally put the function name on the same line as the type.
arch/powerpc/platforms/pseries/phyp_dump.c store_release_region(struct kset *kset, · line 344
Quoted 13 lines
{
arch/powerpc/platforms/pseries/phyp_dump.c +11
-1-return sprintf (buf,"ola\n" ); +u64 second_addr_range; + +/* total reserved size - start of scratch area */ + second_addr_range = phdr.cpu_data.destination_address - + phyp_dump_info->init_reserve_size; +return sprintf (buf,"CPU:0x%lx-0x%lx: HPTE:0x%lx-0x%lx:" +" DUMP:0x%lx-0x%lx, 0x%lx-0x%lx:\n" , + phdr.cpu_data.destination_address, phdr.cpu_data.length_copied, + phdr.hpte_data.destination_address, phdr.hpte_data.length_copied, + phdr.kernel_data.destination_address, phdr.kernel_data.length_copied, + phyp_dump_info->init_reserve_start, second_addr_range);
This indentation should be (probably) two tabs.
arch/powerpc/platforms/pseries/phyp_dump.c static int __init phyp_dump_setup(void) · line 410
Quoted 2 lines
arch/powerpc/platforms/pseries/phyp_dump.c +2
-0+/* re-register the dump area, if old dump was invalid */ +if ((dump_header) && (dump_header->status &DUMP_ERROR_FLAG )) {
^ ^
Extra parentheses.
arch/powerpc/platforms/pseries/phyp_dump.c static int __init phyp_dump_setup(void) · line 412
Quoted 2 lines
arch/powerpc/platforms/pseries/phyp_dump.c +2
-0+invalidate_last_dump (&phdr, dump_area_start); +register_dump_area (&phdr, dump_area_start);
No spaces after function names.
Signature · 3 lines
Cheers, Stephen Rothwell [email protected] http://www.canb.auug.org.au/~sfr/
Re: [PATCH 7/8] pseries: phyp dump: Unregister and print dump areas.
Stephen,
Quoted 5 lines
arch/powerpc/platforms/pseries/phyp_dump.c static void print_dump_header(const stru · line 197
Quoted 3 lines
arch/powerpc/platforms/pseries/phyp_dump.c +3
-0+/* Add addr value if not initialized before */ +if (ph->cpu_data.destination_address ==0 ) { + ph->cpu_data.destination_address += addr;Could be just '=' like further down, right?
Actually the one below should be += as well. Thanks for catching it.
Quoted 11 lines
arch/powerpc/platforms/pseries/phyp_dump.c store_release_region(struct kset *kset, · line 344
Quoted 9 lines
arch/powerpc/platforms/pseries/phyp_dump.c +9
-0+/* total reserved size - start of scratch area */ + second_addr_range = phdr.cpu_data.destination_address - + phyp_dump_info->init_reserve_size; +return sprintf (buf,"CPU:0x%lx-0x%lx: HPTE:0x%lx-0x%lx:" +" DUMP:0x%lx-0x%lx, 0x%lx-0x%lx:\n" , + phdr.cpu_data.destination_address, phdr.cpu_data.length_copied, + phdr.hpte_data.destination_address, phdr.hpte_data.length_copied, + phdr.kernel_data.destination_address, phdr.kernel_data.length_copied, + phyp_dump_info->init_reserve_start, second_addr_range);This indentation should be (probably) two tabs.
I kept it one with a few spaces as otherwise it was exceeding 80, I guess, I can just have one per line and that should take care of that.
Quoted 5 lines
arch/powerpc/platforms/pseries/phyp_dump.c static int __init phyp_dump_setup(void) · line 410
Quoted 3 lines
arch/powerpc/platforms/pseries/phyp_dump.c +2
-0+/* re-register the dump area, if old dump was invalid */ +if ((dump_header) && (dump_header->status &DUMP_ERROR_FLAG )) {^ ^
Extra parentheses.
Just for clarity.. I would prefer that, if thats okay.
Quoted 6 lines
arch/powerpc/platforms/pseries/phyp_dump.c static int __init phyp_dump_setup(void) · line 412
Quoted 3 lines
arch/powerpc/platforms/pseries/phyp_dump.c +2
-0+invalidate_last_dump (&phdr, dump_area_start); +register_dump_area (&phdr, dump_area_start);No spaces after function names.
Yeah, will take that out from here and other files as well.
Thanks,
Manish
PATCH 8/8 pseries: phyp dump: Tracking memory range freed.
In reply to Manish AhujaThis patch tracks the size freed. For now it does a simple rudimentary calculation of the ranges freed. The idea is to keep it simple at the external shell script level and send in large chunks for now.
-----
--- arch/powerpc/platforms/pseries/phyp_dump.c | 36 +++++++++++++++++++++++++++++ include/asm-powerpc/phyp_dump.h | 3 ++ 2 files changed, 39 insertions(+)
include/asm-powerpc/phyp_dump.h +3 -0
@@ -24,6 +24,9 @@ struct phyp_dump {/* Memory that is reserved during very early boot. */ unsigned long init_reserve_start;unsigned long init_reserve_size; +/* Scratch area memory details */ +unsigned long scratch_reserve_start; +unsigned long scratch_reserve_size; };extern struct phyp_dump *phyp_dump_info;
arch/powerpc/platforms/pseries/phyp_dump.c +36 -0
@@ -287,6 +287,39 @@ release_memory_range(unsigned long start } } +/** +* track_freed_range -- Counts the range being freed. +* Once the counter goes to zero, it re-registers dump for +* future use. +*/ +static void +track_freed_range (unsigned long addr,unsigned long length) +{ +static unsigned long scratch_area_size, reserved_area_size; + +if (addr < phyp_dump_info->init_reserve_start) +return ; + +if ((addr >= phyp_dump_info->init_reserve_start) && + (addr <= phyp_dump_info->init_reserve_start + + phyp_dump_info->init_reserve_size)) + reserved_area_size += length; + +if ((addr >= phyp_dump_info->scratch_reserve_start) && + (addr <= phyp_dump_info->scratch_reserve_start + + phyp_dump_info->scratch_reserve_size)) + scratch_area_size += length; + +if ((reserved_area_size == phyp_dump_info->init_reserve_start) && + (scratch_area_size == phyp_dump_info->scratch_reserve_size)) { + +invalidate_last_dump (&phdr, + phyp_dump_info->scratch_reserve_start); +register_dump_area (&phdr, + phyp_dump_info->scratch_reserve_start); + } +} +/* ------------------------------------------------- */ /** * sysfs_release_region -- sysfs interface to release memory range.
@@ -310,6 +343,8 @@ store_release_region(struct kset *kset,if (ret !=2 )return -EINVAL ; +track_freed_range (start_addr, length); +/* Range-check - don't free any reserved memory that * wasn't reserved for phyp-dump */ if (start_addr < phyp_dump_info->init_reserve_start)
@@ -414,6 +449,7 @@ static int __init phyp_dump_setup(void) }/* Don't allow user to release the 256MB scratch area */ +/* this might be wrong */ phyp_dump_info->init_reserve_size = free_area_length;/* Should we create a dump_subsys, analogous to s390/ipl.c ? */
cover pseries: phyp dump: hypervisor-assisted dump
In reply to Manish AhujaThe following series of patches implement a basic framework for hypervisor-assisted dump. The very first patch provides documentation explaining what this is :-). Yes, its supposed to be an improvement over kdump.
A list of open issues / todo list is included in the documentation. It also appears that the not-yet-released firmware versions this was tested on are still, ahem, incomplete; this work is also pending.
I have included most of the changes requested. Although, I did find one or two, fixed in a later patch file rather than the first location they appeared at.
Also it now does not block any memory on machines other than power6 boxes which have the requisite firmware. This is from a power5 box.
from jal-lp6 a power5 machine.
.........
Phyp-dump not supported on this hardware
Using pSeries machine description
console [udbg-1] enabled
.......
Since I changed a few more things, I am reposting all the patches.
-- Manish & Linas.
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-dev
PATCH 1/8 pseries: phyp dump: Docmentation
Basic documentation for hypervisor-assisted dump.
----
Documentation/powerpc/phyp-assisted-dump.txt | 127 +++++++++++++++++++++++++++ 1 file changed, 127 insertions(+)
Documentation/powerpc/phyp-assisted-dump.txt new file +127 -0
@@ -0,0 +1,127 @@ + + Hypervisor-Assisted Dump + ------------------------ + November 2007 + +The goal of hypervisor-assisted dump is to enable the dump of +a crashed system, and to do so from a fully-reset system, and +to minimize the total elapsed time until the system is back +in production use. + +As compared to kdump or other strategies, hypervisor-assisted +dump offers several strong, practical advantages: + +-- Unlike kdump, the system has been reset, and loaded + with a fresh copy of the kernel. In particular, + PCI and I/O devices have been reinitialized and are + in a clean, consistent state. +-- As the dump is performed, the dumped memory becomes + immediately available to the system for normal use. +-- After the dump is completed, no further reboots are + required; the system will be fully usable, and running + in it's normal, production mode on it normal kernel. + +The above can only be accomplished by coordination with, +and assistance from the hypervisor. The procedure is +as follows: + +-- When a system crashes, the hypervisor will save + the low 256MB of RAM to a previously registered + save region. It will also save system state, system + registers, and hardware PTE's. + +-- After the low 256MB area has been saved, the + hypervisor will reset PCI and other hardware state. + It will *not* clear RAM. It will then launch the + bootloader, as normal. + +-- The freshly booted kernel will notice that there + is a new node (ibm,dump-kernel) in the device tree, + indicating that there is crash data available from + a previous boot. It will boot into only 256MB of RAM, + reserving the rest of system memory. + +-- Userspace tools will parse /sys/kernel/release_region + and read /proc/vmcore to obtain the contents of memory, + which holds the previous crashed kernel. The userspace + tools may copy this info to disk, or network, nas, san, + iscsi, etc. as desired. + + For Example: the values in /sys/kernel/release-region + would look something like this (address-range pairs). + CPU:0x177fee000-0x10000: HPTE:0x177ffe020-0x1000: / + DUMP:0x177fff020-0x10000000, 0x10000000-0x16F1D370A + +-- As the userspace tools complete saving a portion of + dump, they echo an offset and size to + /sys/kernel/release_region to release the reserved + memory back to general use. + + An example of this is: + "echo 0x40000000 0x10000000 > /sys/kernel/release_region" + which will release 256MB at the 1GB boundary. + +Please note that the hypervisor-assisted dump feature +is only available on Power6-based systems with recent +firmware versions. + +Implementation details: +---------------------- + +During boot, a check is made to see if firmware supports +this feature on this particular machine. If it does, then +we check to see if a active dump is waiting for us. If yes +then everything but 256 MB of RAM is reserved during early +boot. This area is released once we collect a dump from user +land scripts that are run. If there is dump data, then +the /sys/kernel/release_region file is created, and +the reserved memory is held. + +If there is no waiting dump data, then only the highest +256MB of the ram is reserved as a scratch area. This area +is *not* be released: this region will be kept permanently +reserved, so that it can act as a receptacle for a copy +of the low 256MB in the case a crash does occur. See, +however, "open issues" below, as to whether +such a reserved region is really needed. + +Currently the dump will be copied from /proc/vmcore to a +a new file upon user intervention. The starting address +to be read and the range for each data point in provided +in /sys/kernel/release_region. + +The tools to examine the dump will be same as the ones +used for kdump. + +General notes: +-------------- +Security: please note that there are potential security issues +with any sort of dump mechanism. In particular, plaintext +(unencrypted) data, and possibly passwords, may be present in +the dump data. Userspace tools must take adequate precautions to +preserve security. + +Open issues/ToDo: +------------ + o The various code paths that tell the hypervisor that a crash + occurred, vs. it simply being a normal reboot, should be + reviewed, and possibly clarified/fixed. + + o Instead of using /sys/kernel, should there be a /sys/dump + instead? There is a dump_subsys being created by the s390 code, + perhaps the pseries code should use a similar layout as well. + + o Is reserving a 256MB region really required? The goal of + reserving a 256MB scratch area is to make sure that no + important crash data is clobbered when the hypervisor + save low mem to the scratch area. But, if one could assure + that nothing important is located in some 256MB area, then + it would not need to be reserved. Something that can be + improved in subsequent versions. + + o Still working the kdump team to integrate this with kdump, + some work remains but this would not affect the current + patches. + + o Still need to write a shell script, to copy the dump away. + Currently I am parsing it manually.
PATCH 2/8 pseries: phyp dump: reserve-release proof-of-concept
In reply to Manish AhujaInitial patch for reserving memory in early boot, and freeing it later. If the previous boot had ended with a crash, the reserved memory would contain a copy of the crashed kernel data.
----
arch/powerpc/kernel/prom.c | 50 ++++++++++++++++++++ arch/powerpc/kernel/rtas.c | 32 +++++++++++++ arch/powerpc/platforms/pseries/Makefile | 1 arch/powerpc/platforms/pseries/phyp_dump.c | 71 +++++++++++++++++++++++++++++ include/asm-powerpc/phyp_dump.h | 38 +++++++++++++++ include/asm/rtas.h | 3 + 6 files changed, 195 insertions(+)
include/asm-powerpc/phyp_dump.h new file +38 -0
@@ -0,0 +1,38 @@ +/* +* Hypervisor-assisted dump +* +* Linas Vepstas, Manish Ahuja 2007 +* Copyright (c) 2007 IBM Corp. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License +* as published by the Free Software Foundation; either version +* 2 of the License, or (at your option) any later version. +*/ + +#ifndef _PPC64_PHYP_DUMP_H +#define _PPC64_PHYP_DUMP_H + +#ifdef CONFIG_PHYP_DUMP + +/* The RMR region will be saved for later dumping +* whenever the kernel crashes. Set this to 256MB. */ +#define PHYP_DUMP_RMR_START 0x0 +#define PHYP_DUMP_RMR_END (1UL<<28) + +struct phyp_dump { +/* Memory that is reserved during very early boot. */ +unsigned long init_reserve_start; +unsigned long init_reserve_size; +/* Check status during boot if dump supported, active & present*/ +unsigned long phyp_dump_configured; +unsigned long phyp_dump_is_active; +/* store cpu & hpte size */ +unsigned long cpu_state_size; +unsigned long hpte_region_size; +}; + +extern struct phyp_dump *phyp_dump_info; + +#endif /* CONFIG_PHYP_DUMP */ +#endif /* _PPC64_PHYP_DUMP_H */
arch/powerpc/platforms/pseries/phyp_dump.c new file +71 -0
71 lines not on this page · raw message
arch/powerpc/platforms/pseries/Makefile +1 -0
@@ -18,3 +18,4 @@ obj-$(CONFIG_HOTPLUG_CPU) += hotplug-cpu obj-$(CONFIG_HVC_CONSOLE) += hvconsole.o obj-$(CONFIG_HVCS) += hvcserver.o obj-$(CONFIG_HCALL_STATS) += hvCall_inst.o +obj-$(CONFIG_PHYP_DUMP) += phyp_dump.o
arch/powerpc/kernel/prom.c +50 -0
74 lines not on this page · raw message
arch/powerpc/kernel/rtas.c +32 -0
@@ -39,6 +39,7 @@#include <asm/syscalls.h> #include <asm/smp.h> #include <asm/atomic.h> +#include <asm/phyp_dump.h> struct rtas_t rtas = { .lock =SPIN_LOCK_UNLOCKED
@@ -883,6 +884,37 @@ void __init rtas_initialize(void)#endif } +int __init early_init_dt_scan_phyp_dump (unsigned long node, +const char *uname,int depth,void *data) +{ +#ifdef CONFIG_PHYP_DUMP +const unsigned int *sizes; + + phyp_dump_info->phyp_dump_configured =0 ; + phyp_dump_info->phyp_dump_is_active =0 ; + +if (depth !=1 ||strcmp (uname,"rtas" ) !=0 ) +return 0 ; + +if (of_get_flat_dt_prop (node,"ibm,configure-kernel-dump" ,NULL )) + phyp_dump_info->phyp_dump_configured++; + +if (of_get_flat_dt_prop (node,"ibm,dump-kernel" ,NULL )) + phyp_dump_info->phyp_dump_is_active++; + + sizes =of_get_flat_dt_prop (node,"ibm,configure-kernel-dump-sizes" ,NULL ); +if (!sizes) +return 0 ; + +if (sizes[0 ] ==1 ) + phyp_dump_info->cpu_state_size = *((unsigned long *)&sizes[1 ]); + +if (sizes[3 ] ==2 ) + phyp_dump_info->hpte_region_size = *((unsigned long *)&sizes[4 ]); +#endif +return 1 ; +} +int __init early_init_dt_scan_rtas (unsigned long node,const char *uname,int depth,void *data) {
include/asm/rtas.h +3 -0
@@ -183,6 +183,9 @@ extern unsigned int rtas_busy_delay(intextern int early_init_dt_scan_rtas (unsigned long node,const char *uname,int depth,void *data); +int early_init_dt_scan_phyp_dump (unsigned long node, +const char *uname,int depth,void *data); +extern void pSeries_log_error (char *buf,unsigned int err_type,int fatal);
Re: [PATCH 2/8] pseries: phyp dump: reserve-release proof-of-concept
Quoted 7 lines · Manish Ahuja
Initial patch for reserving memory in early boot, and freeing it later. If the previous boot had ended with a crash, the reserved memory would contain a copy of the crashed kernel data.
Signed-off-by Manish Ahuja <[email protected]>Signed-off-by Linas Vepstas <[email protected]>
Quoted 7 lines
----
arch/powerpc/kernel/prom.c | 50 ++++++++++++++++++++ arch/powerpc/kernel/rtas.c | 32 +++++++++++++ arch/powerpc/platforms/pseries/Makefile | 1 arch/powerpc/platforms/pseries/phyp_dump.c | 71 +++++++++++++++++++++++++++++ include/asm-powerpc/phyp_dump.h | 38 +++++++++++++++ include/asm/rtas.h | 3 +
^^^^^^^^^^^^^^^^^^
asm/rtas.h doesn't exist. You need to clean your tree, and patch asm-powerpc/rtas.h instead.
cheers
Signature · 8 lines
Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person
PATCH 2/8 pseries: phyp dump: reserve-release proof-of-concept
Michael,
Fixed.
-Manish
----------
Initial patch for reserving memory in early boot, and freeing it later. If the previous boot had ended with a crash, the reserved memory would contain a copy of the crashed kernel data.
----
arch/powerpc/kernel/prom.c | 50 ++++++++++++++++++++ arch/powerpc/kernel/rtas.c | 32 +++++++++++++ arch/powerpc/platforms/pseries/Makefile | 1 arch/powerpc/platforms/pseries/phyp_dump.c | 71 +++++++++++++++++++++++++++++ include/asm-powerpc/phyp_dump.h | 38 +++++++++++++++ include/asm-powerpc/rtas.h | 3 + 6 files changed, 195 insertions(+)
include/asm-powerpc/phyp_dump.h new file +38 -0
@@ -0,0 +1,38 @@ +/* +* Hypervisor-assisted dump +* +* Linas Vepstas, Manish Ahuja 2007 +* Copyright (c) 2007 IBM Corp. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License +* as published by the Free Software Foundation; either version +* 2 of the License, or (at your option) any later version. +*/ + +#ifndef _PPC64_PHYP_DUMP_H +#define _PPC64_PHYP_DUMP_H + +#ifdef CONFIG_PHYP_DUMP + +/* The RMR region will be saved for later dumping +* whenever the kernel crashes. Set this to 256MB. */ +#define PHYP_DUMP_RMR_START 0x0 +#define PHYP_DUMP_RMR_END (1UL<<28) + +struct phyp_dump { +/* Memory that is reserved during very early boot. */ +unsigned long init_reserve_start; +unsigned long init_reserve_size; +/* Check status during boot if dump supported, active & present*/ +unsigned long phyp_dump_configured; +unsigned long phyp_dump_is_active; +/* store cpu & hpte size */ +unsigned long cpu_state_size; +unsigned long hpte_region_size; +}; + +extern struct phyp_dump *phyp_dump_info; + +#endif /* CONFIG_PHYP_DUMP */ +#endif /* _PPC64_PHYP_DUMP_H */
arch/powerpc/platforms/pseries/phyp_dump.c new file +71 -0
71 lines not on this page · raw message
arch/powerpc/platforms/pseries/Makefile +1 -0
@@ -18,3 +18,4 @@ obj-$(CONFIG_HOTPLUG_CPU) += hotplug-cpu obj-$(CONFIG_HVC_CONSOLE) += hvconsole.o obj-$(CONFIG_HVCS) += hvcserver.o obj-$(CONFIG_HCALL_STATS) += hvCall_inst.o +obj-$(CONFIG_PHYP_DUMP) += phyp_dump.o
arch/powerpc/kernel/prom.c +50 -0
74 lines not on this page · raw message
arch/powerpc/kernel/rtas.c +32 -0
@@ -39,6 +39,7 @@#include <asm/syscalls.h> #include <asm/smp.h> #include <asm/atomic.h> +#include <asm/phyp_dump.h> struct rtas_t rtas = { .lock =SPIN_LOCK_UNLOCKED
@@ -883,6 +884,37 @@ void __init rtas_initialize(void)#endif } +int __init early_init_dt_scan_phyp_dump (unsigned long node, +const char *uname,int depth,void *data) +{ +#ifdef CONFIG_PHYP_DUMP +const unsigned int *sizes; + + phyp_dump_info->phyp_dump_configured =0 ; + phyp_dump_info->phyp_dump_is_active =0 ; + +if (depth !=1 ||strcmp (uname,"rtas" ) !=0 ) +return 0 ; + +if (of_get_flat_dt_prop (node,"ibm,configure-kernel-dump" ,NULL )) + phyp_dump_info->phyp_dump_configured++; + +if (of_get_flat_dt_prop (node,"ibm,dump-kernel" ,NULL )) + phyp_dump_info->phyp_dump_is_active++; + + sizes =of_get_flat_dt_prop (node,"ibm,configure-kernel-dump-sizes" ,NULL ); +if (!sizes) +return 0 ; + +if (sizes[0 ] ==1 ) + phyp_dump_info->cpu_state_size = *((unsigned long *)&sizes[1 ]); + +if (sizes[3 ] ==2 ) + phyp_dump_info->hpte_region_size = *((unsigned long *)&sizes[4 ]); +#endif +return 1 ; +} +int __init early_init_dt_scan_rtas (unsigned long node,const char *uname,int depth,void *data) {
include/asm-powerpc/rtas.h +3 -0
@@ -183,6 +183,9 @@ extern unsigned int rtas_busy_delay(intextern int early_init_dt_scan_rtas (unsigned long node,const char *uname,int depth,void *data); +int early_init_dt_scan_phyp_dump (unsigned long node, +const char *uname,int depth,void *data); +extern void pSeries_log_error (char *buf,unsigned int err_type,int fatal);
Re: [PATCH 2/8] pseries: phyp dump: reserve-release proof-of-concept
In reply to Manish AhujaQuoted 22 lines · Manish Ahuja
Initial patch for reserving memory in early boot, and freeing it later. If the previous boot had ended with a crash, the reserved memory would contain a copy of the crashed kernel data.
Signed-off-by Manish Ahuja <[email protected]>Signed-off-by Linas Vepstas <[email protected]>----
arch/powerpc/kernel/prom.c | 50 ++++++++++++++++++++ arch/powerpc/kernel/rtas.c | 32 +++++++++++++ arch/powerpc/platforms/pseries/Makefile | 1 arch/powerpc/platforms/pseries/phyp_dump.c | 71 +++++++++++++++++++++++++++++ include/asm-powerpc/phyp_dump.h | 38 +++++++++++++++ include/asm/rtas.h | 3 + 6 files changed, 195 insertions(+)Index:
2.6.24-rc5/include/asm-powerpc/phyp_dump.h
===================================================================
include/asm-powerpc/phyp_dump.h line 5
Quoted 5 lines
include/asm-powerpc/phyp_dump.h new file +5
-0@@ -0,0 +1,38 @@ +/* +* Hypervisor-assisted dump +* +* Linas Vepstas, Manish Ahuja 2007 +* Copyright (c) 2007 IBM Corp.
Hi Manish,
Sorry for the minor nits but this should be:
---
* Linas Vepstas, Manish Ahuja 2008
* Copyright 2008 IBM Corp.
---
You can optionally use the '©' symbol after word 'Copyright' but you shouldn't use '(c)' anymore.
Also in at least one place you've misspelt "Copyright"
Yours Tony
linux.conf.au http://linux.conf.au/ || http://lca2008.linux.org.au/
Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!
Re: [PATCH 2/8] pseries: phyp dump: reserve-release proof-of-concept
Quoted 8 lines · Tony Breeds
Hi Manish,
Sorry for the minor nits but this should be:---
* Linas Vepstas, Manish Ahuja 2008
* Copyright 2008 IBM Corp.
---
Quoted 4 lines
You can optionally use the '??' symbol after word 'Copyright' but you shouldn't use '(c)' anymore.
Also in at least one place you've misspelt "Copyright"
If we're going to nitpick, then I'd like to point out that the whole series needs to be run through checkpatch and at least the whitespace issues should be taken care of.
I'm still not convinced that this is a useful feature compared to hardening kdump, especially now that ehea can handle kexec/kdump (patch posted the other day). But in the end it's up to Paul if he wants to take it or not, not me.
-Olof
Re: [PATCH 2/8] pseries: phyp dump: reserve-release proof-of-concept
Olof,
I will run it through checkpatch before resubmitting.
Thanks,
Manish
Quoted 26 lines · Olof Johansson
Quoted 8 lines · Tony Breeds
Hi Manish,
Sorry for the minor nits but this should be:---
* Linas Vepstas, Manish Ahuja 2008
* Copyright 2008 IBM Corp.
---Quoted 4 lines
You can optionally use the '??' symbol after word 'Copyright' but you shouldn't use '(c)' anymore.
Also in at least one place you've misspelt "Copyright"
If we're going to nitpick, then I'd like to point out that the whole series needs to be run through checkpatch and at least the whitespace issues should be taken care of.
I'm still not convinced that this is a useful feature compared to hardening kdump, especially now that ehea can handle kexec/kdump (patch posted the other day). But in the end it's up to Paul if he wants to take it or not, not me.
-Olof
PATCH 3/8 pseries: phyp dump: use sysfs to release reserved mem
In reply to Manish AhujaCheck to see if there actually is data from a previously crashed kernel waiting. If so, Allow user-space tools to grab the data (by reading /proc/kcore). When user-space finishes dumping a section, it must release that memory by writing to sysfs. For example,
echo "0x40000000 0x10000000" > /sys/kernel/release_region
will release 256MB starting at the 1GB. The released memory becomes free for general use.
------
arch/powerpc/platforms/pseries/phyp_dump.c | 88 +++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 6 deletions(-)
arch/powerpc/platforms/pseries/phyp_dump.c +82 -6
@@ -12,17 +12,24 @@*/ #include <linux/init.h> +#include <linux/kobject.h> #include <linux/mm.h> +#include <linux/of.h> #include <linux/pfn.h> #include <linux/swap.h> +#include <linux/sysfs.h> #include <asm/page.h> #include <asm/phyp_dump.h> +#include <asm/rtas.h> /* Global, used to communicate data between early boot and late boot */ static struct phyp_dump phyp_dump_global;struct phyp_dump *phyp_dump_info = &phyp_dump_global; +static int ibm_configure_kernel_dump; + +/* ------------------------------------------------- */ /** * release_memory_range -- release memory previously lmb_reserved * @start_pfn: starting physical frame number
@@ -52,20 +59,89 @@ release_memory_range(unsigned long start } } -static int __init phyp_dump_setup (void ) +/* ------------------------------------------------- */ +/** +* sysfs_release_region -- sysfs interface to release memory range. +* +* Usage: +* "echo <start addr> <length> > /sys/kernel/release_region" +* +* Example: +* "echo 0x40000000 0x10000000 > /sys/kernel/release_region" +* +* will release 256MB starting at 1GB. +*/ +static ssize_t +store_release_region (struct kset *kset,const char *buf,size_t count) { +unsigned long start_addr, length, end_addr;unsigned long start_pfn, nr_pages; +ssize_t ret; + + ret =sscanf (buf,"%lx %lx" , &start_addr, &length); +if (ret !=2 ) +return -EINVAL ; + +/* Range-check - don't free any reserved memory that +* wasn't reserved for phyp-dump */ +if (start_addr < phyp_dump_info->init_reserve_start) + start_addr = phyp_dump_info->init_reserve_start; + + end_addr = phyp_dump_info->init_reserve_start + + phyp_dump_info->init_reserve_size; +if (start_addr+length > end_addr) + length = end_addr - start_addr; + +/* Release the region of memory assed in by user */ + start_pfn =PFN_DOWN (start_addr); + nr_pages =PFN_DOWN (length); +release_memory_range (start_pfn, nr_pages); + +return count; +} + +static ssize_t +show_release_region (struct kset * kset,char *buf) +{ +return sprintf (buf,"ola\n" ); +} + +static struct subsys_attribute rr =__ATTR (release_region,0600 , + show_release_region, + store_release_region); + +static int __init phyp_dump_setup (void ) +{ +struct device_node *rtas; +const int *dump_header; +int header_len =0 ; +int rc;/* If no memory was reserved in early boot, there is nothing to do */ if (phyp_dump_info->init_reserve_size ==0 )return 0 ; -/* Release memory that was reserved in early boot */ - start_pfn =PFN_DOWN (phyp_dump_info->init_reserve_start); - nr_pages =PFN_DOWN (phyp_dump_info->init_reserve_size); -release_memory_range (start_pfn, nr_pages); +/* Return if phyp dump not supported */ +if (!phyp_dump_info->phyp_dump_configured) { +return -ENOSYS ; + } + +/* Is there dump data waiting for us? */ + rtas =of_find_node_by_path ("/rtas" ); + dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len); +if (dump_header ==NULL ) { +release_all (); +return 0 ; + } + +/* Should we create a dump_subsys, analogous to s390/ipl.c ? */ + rc =subsys_create_file (&kernel_subsys, &rr); +if (rc) { +printk (KERN_ERR "phyp-dump: unable to create sysfs file (%d)\n" , rc); +release_all (); +return 0 ; + }return 0 ; } -subsys_initcall (phyp_dump_setup);
Re: [PATCH 3/8] pseries: phyp dump: use sysfs to release reserved mem
Hi Manish,
Just a small comment.
arch/powerpc/platforms/pseries/phyp_dump.c release_memory_range(unsigned long start · line 131
Quoted 4 lines · On Tue, 12 Feb 2008 01:11:58 -0600 Manish Ahuja <[email protected]> wrote:
arch/powerpc/platforms/pseries/phyp_dump.c +3
-0+/* Is there dump data waiting for us? */ + rtas =of_find_node_by_path ("/rtas" ); + dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len);
You need an of_node_put(rtas) here.
Quoted 4 lines
arch/powerpc/platforms/pseries/phyp_dump.c +4
-0+if (dump_header ==NULL ) { +release_all (); +return 0 ; + }
Signature · 3 lines
Cheers, Stephen Rothwell [email protected] http://www.canb.auug.org.au/~sfr/
Re: [PATCH 3/8] pseries: phyp dump: use sysfs to release reserved mem
As noted, its fixed in patch 4.
If its okay for this time, I will prefer to leave it there.
-Manish
Quoted 16 lines · Stephen Rothwell
Hi Manish,
Just a small comment.
arch/powerpc/platforms/pseries/phyp_dump.c release_memory_range(unsigned long start · line 131
Quoted 3 lines · On Tue, 12 Feb 2008 01:11:58 -0600 Manish Ahuja <[email protected]> wrote:
arch/powerpc/platforms/pseries/phyp_dump.c +3
-0+/* Is there dump data waiting for us? */ + rtas =of_find_node_by_path ("/rtas" ); + dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len);You need an
of_node_put(rtas)here.Quoted 4 lines
arch/powerpc/platforms/pseries/phyp_dump.c +4
-0+if (dump_header ==NULL ) { +release_all (); +return 0 ; + }
Re: [PATCH 3/8] pseries: phyp dump: use sysfs to release reserved mem
In reply to Manish AhujaOn Tue, Feb 12, 2008 at 01:11:58AM -0600, Manish Ahuja wrote:
<snip>
arch/powerpc/platforms/pseries/phyp_dump.c release_memory_range(unsigned long start · line 111
Quoted 9 lines
arch/powerpc/platforms/pseries/phyp_dump.c +9
-0+static ssize_t +show_release_region (struct kset * kset,char *buf) +{ +return sprintf (buf,"ola\n" ); +} + +static struct subsys_attribute rr =__ATTR (release_region,0600 , + show_release_region, + store_release_region);
Any reason this sysfs attribute can't be write only? The show method doesn't seem needed.
Quoted 2 lines
arch/powerpc/platforms/pseries/phyp_dump.c +2
-0+static int __init phyp_dump_setup (void ) +{
<snip>
arch/powerpc/platforms/pseries/phyp_dump.c release_memory_range(unsigned long start · line 131
Quoted 3 lines
arch/powerpc/platforms/pseries/phyp_dump.c +3
-0+/* Is there dump data waiting for us? */ + rtas =of_find_node_by_path ("/rtas" ); + dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len);
Hmm this isn't good. You need to check rtas != NULL.
Quoted 7 lines
arch/powerpc/platforms/pseries/phyp_dump.c +7
-0+if (dump_header ==NULL ) { +release_all (); +return 0 ; + } + +/* Should we create a dump_subsys, analogous to s390/ipl.c ? */ + rc =subsys_create_file (&kernel_subsys, &rr);
arch/powerpc/platforms/pseries/phyp_dump.c release_memory_range(unsigned long start · line 147
Quoted 10 lines
arch/powerpc/platforms/pseries/phyp_dump.c +5
-1+if (rc) { +printk (KERN_ERR "phyp-dump: unable to create sysfs file (%d)\n" , rc); +release_all (); +return 0 ; + }return 0 ; } -subsys_initcall (phyp_dump_setup);
Hmm I think this really should be a:
machine_subsys_initcall(pseries, phyp_dump_setup)
Yours Tony
linux.conf.au http://linux.conf.au/ || http://lca2008.linux.org.au/
Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!
Re: [PATCH 3/8] pseries: phyp dump: use sysfs to release reserved mem
Quoted 4 lines · Tony Breeds
On Tue, Feb 12, 2008 at 01:11:58AM -0600, Manish Ahuja wrote:
<snip>
Quoted 12 lines
arch/powerpc/platforms/pseries/phyp_dump.c release_memory_range(unsigned long start · line 111
Quoted 9 lines
arch/powerpc/platforms/pseries/phyp_dump.c +9
-0+static ssize_t +show_release_region (struct kset * kset,char *buf) +{ +return sprintf (buf,"ola\n" ); +} + +static struct subsys_attribute rr =__ATTR (release_region,0600 , + show_release_region, + store_release_region);Any reason this sysfs attribute can't be write only? The show method doesn't seem needed.
yes, its used later in the code.
Quoted 11 lines
Quoted 3 lines
arch/powerpc/platforms/pseries/phyp_dump.c +2
-0+static int __init phyp_dump_setup (void ) +{<snip>
arch/powerpc/platforms/pseries/phyp_dump.c release_memory_range(unsigned long start · line 131
Quoted 3 lines
arch/powerpc/platforms/pseries/phyp_dump.c +3
-0+/* Is there dump data waiting for us? */ + rtas =of_find_node_by_path ("/rtas" ); + dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len);Hmm this isn't good. You need to check rtas != NULL.
yes, will fix this as well.
Quoted 27 lines
Quoted 8 lines
arch/powerpc/platforms/pseries/phyp_dump.c +7
-0+if (dump_header ==NULL ) { +release_all (); +return 0 ; + } + +/* Should we create a dump_subsys, analogous to s390/ipl.c ? */ + rc =subsys_create_file (&kernel_subsys, &rr);arch/powerpc/platforms/pseries/phyp_dump.c release_memory_range(unsigned long start · line 147
Quoted 10 lines
arch/powerpc/platforms/pseries/phyp_dump.c +5
-1+if (rc) { +printk (KERN_ERR "phyp-dump: unable to create sysfs file (%d)\n" , rc); +release_all (); +return 0 ; + }return 0 ; } -subsys_initcall (phyp_dump_setup);Hmm I think this really should be a:
machine_subsys_initcall(pseries, phyp_dump_setup)Yours Tony
linux.conf.au http://linux.conf.au/ || http://lca2008.linux.org.au/
Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!
Re: [PATCH 3/8] pseries: phyp dump: use sysfs to release reserved mem
Quoted 5 lines · Manish Ahuja
Quoted 2 lines · Tony Breeds
Any reason this sysfs attribute can't be write only? The show method doesn't seem needed.
yes, its used later in the code.
I see that now, thanks. From my point of view it would make reviewing these patches easier if each patch was a correct and simple as possible. In this case it would have made the review easier if the sysfs attribute was write only now and then modified to add the read side when it's actually implemented. The same goes for fixing typosi, cosmetic changes and reference counting.
Looking forward to a respin of this patch series.
Yours Tony
linux.conf.au http://linux.conf.au/ || http://lca2008.linux.org.au/
Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!
Re: [PATCH 3/8] pseries: phyp dump: use sysfs to release reserved mem
In reply to Tony BreedsQuoted 15 lines · Tony Breeds
arch/powerpc/platforms/pseries/phyp_dump.c release_memory_range(unsigned long start · line 111
Quoted 9 lines · Manish Ahuja
arch/powerpc/platforms/pseries/phyp_dump.c +9
-0+static ssize_t +show_release_region (struct kset * kset,char *buf) +{ +return sprintf (buf,"ola\n" ); +} + +static struct subsys_attribute rr =__ATTR (release_region,0600 , + show_release_region, + store_release_region);Any reason this sysfs attribute can't be write only? The show method
doesn't seem needed.
This was supposed to be a place-holder; a later patch would add detailed info. The goal was to have user-land tools that would operate these files to progressively dump and release memory regions; however, until these userland tools get written, the proper interface remains murky (e.g. real addresses? virtual addresses? just delta's or a whole memory map? some sort of numa flags or whatever?)
--linas
PATCH 4/8 pseries: phyp dump: register dump area.
In reply to Manish AhujaSet up the actual dump header, register it with the hypervisor.
------
arch/powerpc/platforms/pseries/phyp_dump.c | 136 +++++++++++++++++++++++++++-- 1 file changed, 129 insertions(+), 7 deletions(-)
arch/powerpc/platforms/pseries/phyp_dump.c +129 -7
@@ -30,6 +30,117 @@ struct phyp_dump *phyp_dump_info = &phypstatic int ibm_configure_kernel_dump;/* ------------------------------------------------- */ +/* RTAS interfaces to declare the dump regions */ + +struct dump_section { +u32 dump_flags; +u16 source_type; +u16 error_flags; +u64 source_address; +u64 source_length; +u64 length_copied; +u64 destination_address; +}; + +struct phyp_dump_header { +u32 version; +u16 num_of_sections; +u16 status; + +u32 first_offset_section; +u32 dump_disk_section; +u64 block_num_dd; +u64 num_of_blocks_dd; +u32 offset_dd; +u32 maxtime_to_auto; +/* No dump disk path string used */ + +struct dump_section cpu_data; +struct dump_section hpte_data; +struct dump_section kernel_data; +}; + +/* The dump header *must be* in low memory, so .bss it */ +static struct phyp_dump_header phdr; + +#define NUM_DUMP_SECTIONS 3 +#define DUMP_HEADER_VERSION 0x1 +#define DUMP_REQUEST_FLAG 0x1 +#define DUMP_SOURCE_CPU 0x0001 +#define DUMP_SOURCE_HPTE 0x0002 +#define DUMP_SOURCE_RMO 0x0011 + +/** +* init_dump_header() - initialize the header declaring a dump +* Returns: length of dump save area. +* +* When the hypervisor saves crashed state, it needs to put +* it somewhere. The dump header tells the hypervisor where +* the data can be saved. +*/ +static unsigned long init_dump_header (struct phyp_dump_header *ph) +{ +unsigned long addr_offset =0 ; + +/* Set up the dump header */ + ph->version =DUMP_HEADER_VERSION ; + ph->num_of_sections =NUM_DUMP_SECTIONS ; + ph->status =0 ; + + ph->first_offset_section = + (u32 )offsetof(struct phyp_dump_header , cpu_data); + ph->dump_disk_section =0 ; + ph->block_num_dd =0 ; + ph->num_of_blocks_dd =0 ; + ph->offset_dd =0 ; + + ph->maxtime_to_auto =0 ;/* disabled */ + +/* The first two sections are mandatory */ + ph->cpu_data.dump_flags =DUMP_REQUEST_FLAG ; + ph->cpu_data.source_type =DUMP_SOURCE_CPU ; + ph->cpu_data.source_address =0 ; + ph->cpu_data.source_length = phyp_dump_info->cpu_state_size; + ph->cpu_data.destination_address = addr_offset; + addr_offset += phyp_dump_info->cpu_state_size; + + ph->hpte_data.dump_flags =DUMP_REQUEST_FLAG ; + ph->hpte_data.source_type =DUMP_SOURCE_HPTE ; + ph->hpte_data.source_address =0 ; + ph->hpte_data.source_length = phyp_dump_info->hpte_region_size; + ph->hpte_data.destination_address = addr_offset; + addr_offset += phyp_dump_info->hpte_region_size; + +/* This section describes the low kernel region */ + ph->kernel_data.dump_flags =DUMP_REQUEST_FLAG ; + ph->kernel_data.source_type =DUMP_SOURCE_RMO ; + ph->kernel_data.source_address =PHYP_DUMP_RMR_START ; + ph->kernel_data.source_length =PHYP_DUMP_RMR_END ; + ph->kernel_data.destination_address = addr_offset; + addr_offset += ph->kernel_data.source_length; + +return addr_offset; +} + +static void register_dump_area (struct phyp_dump_header *ph,unsigned long addr) +{ +int rc; + ph->cpu_data.destination_address += addr; + ph->hpte_data.destination_address += addr; + ph->kernel_data.destination_address += addr; + +do { + rc =rtas_call (ibm_configure_kernel_dump,3 ,1 ,NULL , +1 , ph,sizeof (struct phyp_dump_header )); + }while (rtas_busy_delay (rc)); + +if (rc) + { +printk (KERN_ERR "phyp-dump: unexpected error (%d) on register\n" , rc); + } +} + +/* ------------------------------------------------- */ /** * release_memory_range -- release memory previously lmb_reserved * @start_pfn: starting physical frame number
@@ -113,7 +224,9 @@ static struct subsys_attribute rr = __ATstatic int __init phyp_dump_setup (void ) {struct device_node *rtas; -const int *dump_header; +const struct phyp_dump_header *dump_header; +unsigned long dump_area_start; +unsigned long dump_area_length;int header_len =0 ;int rc;
@@ -126,22 +239,31 @@ static int __init phyp_dump_setup(void)return -ENOSYS ; } -/* Is there dump data waiting for us? */ +/* Is there dump data waiting for us? If there isn't, +* then register a new dump area, and release all of +* the rest of the reserved ram. +* +* The /rtas/ibm,kernel-dump rtas node is present only +* if there is dump data waiting for us. +*/ rtas =of_find_node_by_path ("/rtas" ); dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len); +of_node_put (rtas); + + dump_area_length =init_dump_header (&phdr); + dump_area_start = phyp_dump_info->init_reserve_start &PAGE_MASK ;/* align down */ +if (dump_header ==NULL ) { -release_all (); +register_dump_area (&phdr, dump_area_start);return 0 ; }/* Should we create a dump_subsys, analogous to s390/ipl.c ? */ rc =subsys_create_file (&kernel_subsys, &rr); -if (rc) { +if (rc)printk (KERN_ERR "phyp-dump: unable to create sysfs file (%d)\n" , rc); -release_all (); -return 0 ; - } +/* ToDo: re-register the dump area, for next time. */ return 0 ; }subsys_initcall (phyp_dump_setup);
Re: [PATCH 4/8] pseries: phyp dump: register dump area.
Hi Manish,
arch/powerpc/platforms/pseries/phyp_dump.c static int __init phyp_dump_setup(void) · line 251
Quoted 11 lines
arch/powerpc/platforms/pseries/phyp_dump.c +8
-1-/* Is there dump data waiting for us? */ +/* Is there dump data waiting for us? If there isn't, +* then register a new dump area, and release all of +* the rest of the reserved ram. +* +* The /rtas/ibm,kernel-dump rtas node is present only +* if there is dump data waiting for us. +*/ rtas =of_find_node_by_path ("/rtas" ); dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len); +of_node_put (rtas);
Oh, here is the of_node_put() - you should move that to patch 3.
Signature · 3 lines
Cheers, Stephen Rothwell [email protected] http://www.canb.auug.org.au/~sfr/
Re: [PATCH 4/8] pseries: phyp dump: register dump area.
For now, if we can leave this patch as is, that will be great. That move requires me to work all remaining patches as they apply uncleanly after that.
I will bunch those two together functionally next time onwards.
Thanks,
Manish
Quoted 16 lines · Stephen Rothwell
Hi Manish,
arch/powerpc/platforms/pseries/phyp_dump.c static int __init phyp_dump_setup(void) · line 251
Quoted 11 lines
arch/powerpc/platforms/pseries/phyp_dump.c +8
-1-/* Is there dump data waiting for us? */ +/* Is there dump data waiting for us? If there isn't, +* then register a new dump area, and release all of +* the rest of the reserved ram. +* +* The /rtas/ibm,kernel-dump rtas node is present only +* if there is dump data waiting for us. +*/ rtas =of_find_node_by_path ("/rtas" ); dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len); +of_node_put (rtas);Oh, here is the
of_node_put()- you should move that to patch 3.
PATCH 5/8 pseries: phyp dump: debugging print routines.
In reply to Manish AhujaProvide some basic debugging support.
-----
arch/powerpc/platforms/pseries/phyp_dump.c | 64 +++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 4 deletions(-)
arch/powerpc/platforms/pseries/phyp_dump.c +60 -4
@@ -2,7 +2,7 @@* Hypervisor-assisted dump * * Linas Vepstas, Manish Ahuja 2007 -* Copyrhgit (c) 2007 IBM Corp. +* Copyright (c) 2007 IBM Corp. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License
@@ -122,6 +122,61 @@ static unsigned long init_dump_header(streturn addr_offset; } +static void print_dump_header (const struct phyp_dump_header *ph) +{ +#ifdef DEBUG +printk (KERN_INFO "dump header:\n" ); +/* setup some ph->sections required */ +printk (KERN_INFO "version = %d\n" , ph->version); +printk (KERN_INFO "Sections = %d\n" , ph->num_of_sections); +printk (KERN_INFO "Status = 0x%x\n" , ph->status); + +/* No ph->disk, so all should be set to 0 */ +printk (KERN_INFO "Offset to first section 0x%x\n" , + ph->first_offset_section); +printk (KERN_INFO "dump disk sections should be zero\n" ); +printk (KERN_INFO "dump disk section = %d\n" , ph->dump_disk_section); +printk (KERN_INFO "block num = %ld\n" , ph->block_num_dd); +printk (KERN_INFO "number of blocks = %ld\n" , ph->num_of_blocks_dd); +printk (KERN_INFO "dump disk offset = %d\n" , ph->offset_dd); +printk (KERN_INFO "Max auto time= %d\n" , ph->maxtime_to_auto); + +/*set cpu state and hpte states as well scratch pad area */ +printk (KERN_INFO " CPU AREA \n" ); +printk (KERN_INFO "cpu dump_flags =%d\n" , ph->cpu_data.dump_flags); +printk (KERN_INFO "cpu source_type =%d\n" , ph->cpu_data.source_type); +printk (KERN_INFO "cpu error_flags =%d\n" , ph->cpu_data.error_flags); +printk (KERN_INFO "cpu source_address =%lx\n" , + ph->cpu_data.source_address); +printk (KERN_INFO "cpu source_length =%lx\n" , + ph->cpu_data.source_length); +printk (KERN_INFO "cpu length_copied =%lx\n" , + ph->cpu_data.length_copied); + +printk (KERN_INFO " HPTE AREA \n" ); +printk (KERN_INFO "HPTE dump_flags =%d\n" , ph->hpte_data.dump_flags); +printk (KERN_INFO "HPTE source_type =%d\n" , ph->hpte_data.source_type); +printk (KERN_INFO "HPTE error_flags =%d\n" , ph->hpte_data.error_flags); +printk (KERN_INFO "HPTE source_address =%lx\n" , + ph->hpte_data.source_address); +printk (KERN_INFO "HPTE source_length =%lx\n" , + ph->hpte_data.source_length); +printk (KERN_INFO "HPTE length_copied =%lx\n" , + ph->hpte_data.length_copied); + +printk (KERN_INFO " SRSD AREA \n" ); +printk (KERN_INFO "SRSD dump_flags =%d\n" , ph->kernel_data.dump_flags); +printk (KERN_INFO "SRSD source_type =%d\n" , ph->kernel_data.source_type); +printk (KERN_INFO "SRSD error_flags =%d\n" , ph->kernel_data.error_flags); +printk (KERN_INFO "SRSD source_address =%lx\n" , + ph->kernel_data.source_address); +printk (KERN_INFO "SRSD source_length =%lx\n" , + ph->kernel_data.source_length); +printk (KERN_INFO "SRSD length_copied =%lx\n" , + ph->kernel_data.length_copied); +#endif +} +static void register_dump_area (struct phyp_dump_header *ph,unsigned long addr) {int rc;
@@ -134,9 +189,9 @@ static void register_dump_area(struct ph 1, ph, sizeof(struct phyp_dump_header)); }while (rtas_busy_delay (rc)); -if (rc) - { -printk (KERN_ERR "phyp-dump: unexpected error (%d) on register\n" , rc); +if (rc) { +printk (KERN_ERR "phyp-dump: unexpected error (%d) on register\n" , rc); +print_dump_header (ph); } }
@@ -238,6 +293,7 @@ static int __init phyp_dump_setup(void)if (!phyp_dump_info->phyp_dump_configured) {return -ENOSYS ; } +print_dump_header (dump_header);/* Is there dump data waiting for us? If there isn't, * then register a new dump area, and release all of
PATCH 6/8 pseries: phyp dump: Invalidate and print dump areas.
In reply to Manish AhujaRoutines to
a. invalidate dump
b. Calculate region that is reserved and needs to be freed. This is
exported through sysfs interface.
Unregister has been removed for now as it wasn't being used.
-----
--- arch/powerpc/platforms/pseries/phyp_dump.c | 85 +++++++++++++++++++++++++---- include/asm/phyp_dump.h | 6 +- 2 files changed, 79 insertions(+), 12 deletions(-)
arch/powerpc/platforms/pseries/phyp_dump.c +74 -11
@@ -69,6 +69,10 @@ static struct phyp_dump_header phdr;#define DUMP_SOURCE_CPU 0x0001#define DUMP_SOURCE_HPTE 0x0002#define DUMP_SOURCE_RMO 0x0011 +#define DUMP_ERROR_FLAG 0x2000 +#define DUMP_TRIGGERED 0x4000 +#define DUMP_PERFORMED 0x8000 +/** * init_dump_header() - initialize the header declaring a dump
@@ -180,9 +184,15 @@ static void print_dump_header(const stru static void register_dump_area(struct phyp_dump_header *ph, unsigned long addr) { int rc; - ph->cpu_data.destination_address += addr; - ph->hpte_data.destination_address += addr; - ph->kernel_data.destination_address += addr; + + /* Add addr value if not initialized before */ + if (ph->cpu_data.destination_address == 0) { + ph->cpu_data.destination_address += addr; + ph->hpte_data.destination_address += addr; + ph->kernel_data.destination_address += addr; + } + + /* ToDo Invalidate kdump and free memory range. */ do { rc = rtas_call(ibm_configure_kernel_dump, 3, 1, NULL,
@@ -195,6 +205,30 @@ static void register_dump_area(struct ph } } +static +void invalidate_last_dump (struct phyp_dump_header *ph,unsigned long addr) +{ +int rc; + +/* Add addr value if not initialized before */ +if (ph->cpu_data.destination_address ==0 ) { + ph->cpu_data.destination_address += addr; + ph->hpte_data.destination_address += addr; + ph->kernel_data.destination_address += addr; + } + +do { + rc =rtas_call (ibm_configure_kernel_dump,3 ,1 ,NULL , +2 , ph,sizeof (struct phyp_dump_header )); + }while (rtas_busy_delay (rc)); + +if (rc) { +printk (KERN_ERR "phyp-dump: unexpected error (%d) " +"on invalidate\n" , rc); +print_dump_header (ph); + } +} +/* ------------------------------------------------- */ /** * release_memory_range -- release memory previously lmb_reserved
@@ -205,8 +239,8 @@ static void register_dump_area(struct ph* lmb_reserved in early boot. The released memory becomes * available for genreal use. */ -static void -release_memory_range (unsigned long start_pfn,unsigned long nr_pages) +static +void release_memory_range (unsigned long start_pfn,unsigned long nr_pages) {struct page *rpage;unsigned long end_pfn;
@@ -237,8 +271,8 @@ release_memory_range(unsigned long start* * will release 256MB starting at 1GB. */ -static ssize_t -store_release_region (struct kset *kset,const char *buf,size_t count) +static +ssize_t store_release_region (struct kset *kset,const char *buf,size_t count) {unsigned long start_addr, length, end_addr;unsigned long start_pfn, nr_pages;
@@ -266,10 +300,23 @@ store_release_region(struct kset *kset,return count; } -static ssize_t -show_release_region (struct kset * kset,char *buf) +static ssize_t show_release_region (struct kset * kset,char *buf) { -return sprintf (buf,"ola\n" ); +u64 second_addr_range; + +/* total reserved size - start of scratch area */ + second_addr_range = phyp_dump_info->init_reserve_size - + phyp_dump_info->reserved_scratch_size; +return sprintf (buf,"CPU:0x%lx-0x%lx: HPTE:0x%lx-0x%lx:" +" DUMP:0x%lx-0x%lx, 0x%lx-0x%lx:\n" , + phdr.cpu_data.destination_address, + phdr.cpu_data.length_copied, + phdr.hpte_data.destination_address, + phdr.hpte_data.length_copied, + phdr.kernel_data.destination_address, + phdr.kernel_data.length_copied, + phyp_dump_info->init_reserve_start, + second_addr_range); } static struct subsys_attribute rr = __ATTR(release_region, 0600,
@@ -293,7 +340,6 @@ static int __init phyp_dump_setup(void)if (!phyp_dump_info->phyp_dump_configured) {return -ENOSYS ; } -print_dump_header (dump_header);/* Is there dump data waiting for us? If there isn't, * then register a new dump area, and release all of
@@ -305,6 +351,7 @@ static int __init phyp_dump_setup(void) rtas =of_find_node_by_path ("/rtas" ); dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len);of_node_put (rtas); +print_dump_header (dump_header); dump_area_length =init_dump_header (&phdr); dump_area_start = phyp_dump_info->init_reserve_start &PAGE_MASK ;/* align down */
@@ -314,6 +361,22 @@ static int __init phyp_dump_setup(void)return 0 ; } +/* re-register the dump area, if old dump was invalid */ +if ((dump_header) && (dump_header->status &DUMP_ERROR_FLAG )) { +invalidate_last_dump (&phdr, dump_area_start); +register_dump_area (&phdr, dump_area_start); +return 0 ; + } + +if (dump_header) { + phyp_dump_info->reserved_scratch_addr = + dump_header->cpu_data.destination_address; + phyp_dump_info->reserved_scratch_size = + dump_header->cpu_data.source_length + + dump_header->hpte_data.source_length + + dump_header->kernel_data.source_length; + } +/* Should we create a dump_subsys, analogous to s390/ipl.c ? */ rc =subsys_create_file (&kernel_subsys, &rr);if (rc)
include/asm/phyp_dump.h +5 -1
@@ -18,7 +18,8 @@/* The RMR region will be saved for later dumping * whenever the kernel crashes. Set this to 256MB. */ #define PHYP_DUMP_RMR_START 0x0 -#define PHYP_DUMP_RMR_END (1UL<<28) +/*#define PHYP_DUMP_RMR_END (1UL<<28)*/ +#define PHYP_DUMP_RMR_END (1UL<<27)struct phyp_dump {/* Memory that is reserved during very early boot. */
@@ -30,6 +31,9 @@ struct phyp_dump {/* store cpu & hpte size */ unsigned long cpu_state_size;unsigned long hpte_region_size; +/* previous scratch area values */ +unsigned long reserved_scratch_addr; +unsigned long reserved_scratch_size; };extern struct phyp_dump *phyp_dump_info;
Re: [PATCH 6/8] pseries: phyp dump: Invalidate and print dump areas.
Hi Manish,
arch/powerpc/platforms/pseries/phyp_dump.c static void register_dump_area(struct ph · line 243
Quoted 5 lines · On Tue, 12 Feb 2008 01:18:22 -0600 Manish Ahuja <[email protected]> wrote:
arch/powerpc/platforms/pseries/phyp_dump.c +2
-2-static void -release_memory_range (unsigned long start_pfn,unsigned long nr_pages) +static +void release_memory_range (unsigned long start_pfn,unsigned long nr_pages)
Cosmetic changes like this should normally be put in separate patch as they just distract from a real review (which this isn't :-))
Signature · 3 lines
Cheers, Stephen Rothwell [email protected] http://www.canb.auug.org.au/~sfr/
Re: [PATCH 6/8] pseries: phyp dump: Invalidate and print dump areas.
Quoted 11 lines · Stephen Rothwell
Hi Manish,
arch/powerpc/platforms/pseries/phyp_dump.c static void register_dump_area(struct ph · line 243
Quoted 4 lines · On Tue, 12 Feb 2008 01:18:22 -0600 Manish Ahuja <[email protected]> wrote:
arch/powerpc/platforms/pseries/phyp_dump.c +2
-2-static void -release_memory_range (unsigned long start_pfn,unsigned long nr_pages) +static +void release_memory_range (unsigned long start_pfn,unsigned long nr_pages)Cosmetic changes like this should normally be put in separate patch as they just distract from a real review (which this isn't :-))
Okay, will do from future.
PATCH 6/8 pseries: phyp dump: Invalidate and print dump areas.
In reply to Manish AhujaChanged asm to asm-powerpc.
Hopefully this was the last of them.
-Manish
----------------------------------------
Routines to
a. invalidate dump
b. Calculate region that is reserved and needs to be freed. This is
exported through sysfs interface.
Unregister has been removed for now as it wasn't being used.
-----
--- arch/powerpc/platforms/pseries/phyp_dump.c | 85 +++++++++++++++++++++++++---- include/asm-powerpc/phyp_dump.h | 3 + 2 files changed, 77 insertions(+), 11 deletions(-)
arch/powerpc/platforms/pseries/phyp_dump.c +74 -11
@@ -69,6 +69,10 @@ static struct phyp_dump_header phdr;#define DUMP_SOURCE_CPU 0x0001#define DUMP_SOURCE_HPTE 0x0002#define DUMP_SOURCE_RMO 0x0011 +#define DUMP_ERROR_FLAG 0x2000 +#define DUMP_TRIGGERED 0x4000 +#define DUMP_PERFORMED 0x8000 +/** * init_dump_header() - initialize the header declaring a dump
@@ -180,9 +184,15 @@ static void print_dump_header(const stru static void register_dump_area(struct phyp_dump_header *ph, unsigned long addr) { int rc; - ph->cpu_data.destination_address += addr; - ph->hpte_data.destination_address += addr; - ph->kernel_data.destination_address += addr; + + /* Add addr value if not initialized before */ + if (ph->cpu_data.destination_address == 0) { + ph->cpu_data.destination_address += addr; + ph->hpte_data.destination_address += addr; + ph->kernel_data.destination_address += addr; + } + + /* ToDo Invalidate kdump and free memory range. */ do { rc = rtas_call(ibm_configure_kernel_dump, 3, 1, NULL,
@@ -195,6 +205,30 @@ static void register_dump_area(struct ph } } +static +void invalidate_last_dump (struct phyp_dump_header *ph,unsigned long addr) +{ +int rc; + +/* Add addr value if not initialized before */ +if (ph->cpu_data.destination_address ==0 ) { + ph->cpu_data.destination_address += addr; + ph->hpte_data.destination_address += addr; + ph->kernel_data.destination_address += addr; + } + +do { + rc =rtas_call (ibm_configure_kernel_dump,3 ,1 ,NULL , +2 , ph,sizeof (struct phyp_dump_header )); + }while (rtas_busy_delay (rc)); + +if (rc) { +printk (KERN_ERR "phyp-dump: unexpected error (%d) " +"on invalidate\n" , rc); +print_dump_header (ph); + } +} +/* ------------------------------------------------- */ /** * release_memory_range -- release memory previously lmb_reserved
@@ -205,8 +239,8 @@ static void register_dump_area(struct ph* lmb_reserved in early boot. The released memory becomes * available for genreal use. */ -static void -release_memory_range (unsigned long start_pfn,unsigned long nr_pages) +static +void release_memory_range (unsigned long start_pfn,unsigned long nr_pages) {struct page *rpage;unsigned long end_pfn;
@@ -237,8 +271,8 @@ release_memory_range(unsigned long start* * will release 256MB starting at 1GB. */ -static ssize_t -store_release_region (struct kset *kset,const char *buf,size_t count) +static +ssize_t store_release_region (struct kset *kset,const char *buf,size_t count) {unsigned long start_addr, length, end_addr;unsigned long start_pfn, nr_pages;
@@ -266,10 +300,23 @@ store_release_region(struct kset *kset,return count; } -static ssize_t -show_release_region (struct kset * kset,char *buf) +static ssize_t show_release_region (struct kset * kset,char *buf) { -return sprintf (buf,"ola\n" ); +u64 second_addr_range; + +/* total reserved size - start of scratch area */ + second_addr_range = phyp_dump_info->init_reserve_size - + phyp_dump_info->reserved_scratch_size; +return sprintf (buf,"CPU:0x%lx-0x%lx: HPTE:0x%lx-0x%lx:" +" DUMP:0x%lx-0x%lx, 0x%lx-0x%lx:\n" , + phdr.cpu_data.destination_address, + phdr.cpu_data.length_copied, + phdr.hpte_data.destination_address, + phdr.hpte_data.length_copied, + phdr.kernel_data.destination_address, + phdr.kernel_data.length_copied, + phyp_dump_info->init_reserve_start, + second_addr_range); } static struct subsys_attribute rr = __ATTR(release_region, 0600,
@@ -293,7 +340,6 @@ static int __init phyp_dump_setup(void)if (!phyp_dump_info->phyp_dump_configured) {return -ENOSYS ; } -print_dump_header (dump_header);/* Is there dump data waiting for us? If there isn't, * then register a new dump area, and release all of
@@ -305,6 +351,7 @@ static int __init phyp_dump_setup(void) rtas =of_find_node_by_path ("/rtas" ); dump_header =of_get_property (rtas,"ibm,kernel-dump" , &header_len);of_node_put (rtas); +print_dump_header (dump_header); dump_area_length =init_dump_header (&phdr); dump_area_start = phyp_dump_info->init_reserve_start &PAGE_MASK ;/* align down */
@@ -314,6 +361,22 @@ static int __init phyp_dump_setup(void)return 0 ; } +/* re-register the dump area, if old dump was invalid */ +if ((dump_header) && (dump_header->status &DUMP_ERROR_FLAG )) { +invalidate_last_dump (&phdr, dump_area_start); +register_dump_area (&phdr, dump_area_start); +return 0 ; + } + +if (dump_header) { + phyp_dump_info->reserved_scratch_addr = + dump_header->cpu_data.destination_address; + phyp_dump_info->reserved_scratch_size = + dump_header->cpu_data.source_length + + dump_header->hpte_data.source_length + + dump_header->kernel_data.source_length; + } +/* Should we create a dump_subsys, analogous to s390/ipl.c ? */ rc =subsys_create_file (&kernel_subsys, &rr);if (rc)
include/asm-powerpc/phyp_dump.h +3 -0
@@ -30,6 +30,9 @@ struct phyp_dump {/* store cpu & hpte size */ unsigned long cpu_state_size;unsigned long hpte_region_size; +/* previous scratch area values */ +unsigned long reserved_scratch_addr; +unsigned long reserved_scratch_size; };extern struct phyp_dump *phyp_dump_info;
PATCH 7/8 pseries: phyp dump: Tracking memory range freed.
In reply to Manish AhujaThis patch tracks the size freed. For now it does a simple rudimentary calculation of the ranges freed. The idea is to keep it simple at the external shell script level and send in large chunks for now.
-----
--- arch/powerpc/platforms/pseries/phyp_dump.c | 35 +++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
arch/powerpc/platforms/pseries/phyp_dump.c +35 -0
@@ -259,6 +259,39 @@ void release_memory_range(unsigned long } } +/** +* track_freed_range -- Counts the range being freed. +* Once the counter goes to zero, it re-registers dump for +* future use. +*/ +static void +track_freed_range (unsigned long addr,unsigned long length) +{ +static unsigned long scratch_area_size, reserved_area_size; + +if (addr < phyp_dump_info->init_reserve_start) +return ; + +if ((addr >= phyp_dump_info->init_reserve_start) && + (addr <= phyp_dump_info->init_reserve_start + + phyp_dump_info->init_reserve_size)) + reserved_area_size += length; + +if ((addr >= phyp_dump_info->reserved_scratch_addr) && + (addr <= phyp_dump_info->reserved_scratch_addr + + phyp_dump_info->reserved_scratch_size)) + scratch_area_size += length; + +if ((reserved_area_size == phyp_dump_info->init_reserve_size) && + (scratch_area_size == phyp_dump_info->reserved_scratch_size)) { + +invalidate_last_dump (&phdr, + phyp_dump_info->reserved_scratch_addr); +register_dump_area (&phdr, + phyp_dump_info->reserved_scratch_addr); + } +} +/* ------------------------------------------------- */ /** * sysfs_release_region -- sysfs interface to release memory range.
@@ -282,6 +315,8 @@ ssize_t store_release_region(struct ksetif (ret !=2 )return -EINVAL ; +track_freed_range (start_addr, length); +/* Range-check - don't free any reserved memory that * wasn't reserved for phyp-dump */ if (start_addr < phyp_dump_info->init_reserve_start)
PATCH 8/8 pseries: phyp dump: config file
In reply to Manish AhujaAdd hypervisor-assisted dump to kernel config
-----
arch/powerpc/Kconfig | 11 +++++++++++ 1 file changed, 11 insertions(+)
arch/powerpc/Kconfig +11 -0
@@ -266,6 +266,17 @@ config CRASH_DUMP Don't change this unless you know what you are doing. +config PHYP_DUMP + bool "Hypervisor-assisted dump (EXPERIMENTAL)" + depends on PPC_PSERIES && EXPERIMENTAL + default y + help + Hypervisor-assisted dump is meant to be a kdump replacement + offering robustness and speed not possible without system + hypervisor assistence. + + If unsure, say "Y" + config PPCBUG_NVRAM bool "Enable reading PPCBUG NVRAM during boot" if PPLUS || LOPEC default y if PPC_PREP