Re: [patch 1/3 v2] Add function get_bootparam
On 11/17/13 at 07:29pm, H. Peter Anvin wrote: On 11/17/2013 06:22 PM, Dave Young wrote: On 11/13/13 at 08:50am, Dave Young wrote: On 11/12/13 at 06:51pm, Greg KH wrote: On Tue, Nov 12, 2013 at 01:37:25AM -0800, H. Peter Anvin wrote: On 11/12/2013 12:30 AM, Greg KH wrote: And these binary data blobs are a standard somewhere, and will not change per kernel version change? If so, that structure is fine with me. Correct. The structure is documented in Documentation/x86/boot.txt, and has been largely invariant (but extended) since the beginning of Linux. It has sometimes taken some serious work to keep it that way. Ok, then use the binary sysfs file interface for these blobs, and everyone will be happy. Since we got an agreement I will move them to sysfs in next version. I have created one patch for the sysfs part this weekend, bug I'm hesitating to remove the debugfs part because currently there's already user, I tend to keep them in debugfs for now. What do you think, Peter? Yes, it makes sense to keep them for compatibility, at least for now. What is the specific user that already exists? kexec-tools is using it to setup hardware_subarch.. -- Thanks Dave -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/3 v2] Add function get_bootparam
In other words, we didn't catch that one in time. Oh well. Dave Young dyo...@redhat.com wrote: On 11/17/13 at 07:29pm, H. Peter Anvin wrote: On 11/17/2013 06:22 PM, Dave Young wrote: On 11/13/13 at 08:50am, Dave Young wrote: On 11/12/13 at 06:51pm, Greg KH wrote: On Tue, Nov 12, 2013 at 01:37:25AM -0800, H. Peter Anvin wrote: On 11/12/2013 12:30 AM, Greg KH wrote: And these binary data blobs are a standard somewhere, and will not change per kernel version change? If so, that structure is fine with me. Correct. The structure is documented in Documentation/x86/boot.txt, and has been largely invariant (but extended) since the beginning of Linux. It has sometimes taken some serious work to keep it that way. Ok, then use the binary sysfs file interface for these blobs, and everyone will be happy. Since we got an agreement I will move them to sysfs in next version. I have created one patch for the sysfs part this weekend, bug I'm hesitating to remove the debugfs part because currently there's already user, I tend to keep them in debugfs for now. What do you think, Peter? Yes, it makes sense to keep them for compatibility, at least for now. What is the specific user that already exists? kexec-tools is using it to setup hardware_subarch.. -- Thanks Dave -- Sent from my mobile phone. Please pardon brevity and lack of formatting. -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/3 v2] Add function get_bootparam
On Tue, Nov 12, 2013 at 04:08:54PM +0800, Dave Young wrote: On 11/11/13 at 11:58pm, Greg KH wrote: kexec-tools can have a fallback to debugfs if we really need it, but making people mount debugfs to have some essential piece of functionality scares the heck out of me. I agree, that would not be good. I'm not sure what exactly the sysfs file is wanting to look like? The efi section variable sysfs file isn't ok (multiple lines with multiple values.) What is this going to look like? The current structure in debugfs is like below, export every field of setup header as one file will be too much IMHO: [root@darkstar debug]# tree boot_params boot_params ├── data /* binary data of setup header */ ├── setup_data /* setup_data link list nodes */ │ ├── 0 │ │ ├── data /* binary data of setup data */ │ │ └── type /* type of setup data */ │ └── 1 │ ├── data │ └── type └── version /* boot protocol version */ And these binary data blobs are a standard somewhere, and will not change per kernel version change? If so, that structure is fine with me. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/3 v2] Add function get_bootparam
On 11/12/13 at 12:30am, Greg KH wrote: On Tue, Nov 12, 2013 at 04:08:54PM +0800, Dave Young wrote: On 11/11/13 at 11:58pm, Greg KH wrote: kexec-tools can have a fallback to debugfs if we really need it, but making people mount debugfs to have some essential piece of functionality scares the heck out of me. I agree, that would not be good. I'm not sure what exactly the sysfs file is wanting to look like? The efi section variable sysfs file isn't ok (multiple lines with multiple values.) What is this going to look like? The current structure in debugfs is like below, export every field of setup header as one file will be too much IMHO: [root@darkstar debug]# tree boot_params boot_params ├── data /* binary data of setup header */ ├── setup_data /* setup_data link list nodes */ │ ├── 0 │ │ ├── data /* binary data of setup data */ │ │ └── type /* type of setup data */ │ └── 1 │ ├── data │ └── type └── version /* boot protocol version */ And these binary data blobs are a standard somewhere, and will not change per kernel version change? IMO the topology should be standard. The content could change, for example later bumping boot protocol version, we could add new field in setup header.. hpa should know more about this.. Peter could you answer this question? If so, that structure is fine with me. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/3 v2] Add function get_bootparam
On 11/12/2013 12:30 AM, Greg KH wrote: And these binary data blobs are a standard somewhere, and will not change per kernel version change? If so, that structure is fine with me. Correct. The structure is documented in Documentation/x86/boot.txt, and has been largely invariant (but extended) since the beginning of Linux. It has sometimes taken some serious work to keep it that way. -hpa -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/3 v2] Add function get_bootparam
On Tue, Nov 12, 2013 at 01:37:25AM -0800, H. Peter Anvin wrote: On 11/12/2013 12:30 AM, Greg KH wrote: And these binary data blobs are a standard somewhere, and will not change per kernel version change? If so, that structure is fine with me. Correct. The structure is documented in Documentation/x86/boot.txt, and has been largely invariant (but extended) since the beginning of Linux. It has sometimes taken some serious work to keep it that way. Ok, then use the binary sysfs file interface for these blobs, and everyone will be happy. -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/3 v2] Add function get_bootparam
On Tue, Nov 12, 2013 at 04:58:50PM +1300, Matthew Garrett wrote: Well, there's one alternative - don't export this at all and only support UEFI kernels that have an in-kernel loader, then just pass the data internally. Hi Matthew, I am still writing that in-kernel support for kexec/kdump. I guess in next 2 weeks I might be able to post a basic RFC version. (no EFI support in this version though). Having said that, I think that it is good to export these values to user space and make existing code work with UEFI. New code (if it gets in at all) will slowly mature over a period of time and transition to it will be slow. I think initially it will be used only if secureboot is enabled. And in the mean time we need a good solution for kexec/kdump with UEFI. Thanks Vivek -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/3 v2] Add function get_bootparam
On 11/12/13 at 06:51pm, Greg KH wrote: On Tue, Nov 12, 2013 at 01:37:25AM -0800, H. Peter Anvin wrote: On 11/12/2013 12:30 AM, Greg KH wrote: And these binary data blobs are a standard somewhere, and will not change per kernel version change? If so, that structure is fine with me. Correct. The structure is documented in Documentation/x86/boot.txt, and has been largely invariant (but extended) since the beginning of Linux. It has sometimes taken some serious work to keep it that way. Ok, then use the binary sysfs file interface for these blobs, and everyone will be happy. Since we got an agreement I will move them to sysfs in next version. Thanks everyone. -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/3 v2] Add function get_bootparam
On 11/11/13 at 06:27pm, Toshi Kani wrote: On Tue, 2013-11-05 at 16:29 +0800, dyo...@redhat.com wrote: Not only setup_subarch will get data from debugfs file boot_params/data, later code for adding efi_info will also need do same thing. Thus add a common function here for later use. get_bootparam() calls find_mnt_by_fsname(debugfs), which assumes that debugfs is mounted with device name debugfs. This function fails when: - debugfs is not mounted, or - debugfs is mounted with a different device name, such as nodev [1]. This issue is not introduced by this patch. But the original code, which sets hardware_subarch, seems to work even if it failed to access debugfs (which is ignored) since hardware_subarch is zero most of the cases anyway. With this change, however, this failure now makes the 2nd kernel unbootable. So, it needs to be addressed to make this code path work reliably (or kexec should fail at least). Toshi, Thanks for your suggestion, it make sense. Will fix it. [1] https://access.redhat.com/site/documentation/en-US/Red_Hat_Enterprise_MRG/2/html/Realtime_Tuning_Guide/sect-Realtime_Tuning_Guide-Realtime_Specific_Tuning-Mount_debugfs.html Thanks, Toshi -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/3 v2] Add function get_bootparam
On 11/11/2013 05:27 PM, Toshi Kani wrote: On Tue, 2013-11-05 at 16:29 +0800, dyo...@redhat.com wrote: Not only setup_subarch will get data from debugfs file boot_params/data, later code for adding efi_info will also need do same thing. Thus add a common function here for later use. get_bootparam() calls find_mnt_by_fsname(debugfs), which assumes that debugfs is mounted with device name debugfs. This function fails when: - debugfs is not mounted, or - debugfs is mounted with a different device name, such as nodev [1]. This issue is not introduced by this patch. But the original code, which sets hardware_subarch, seems to work even if it failed to access debugfs (which is ignored) since hardware_subarch is zero most of the cases anyway. With this change, however, this failure now makes the 2nd kernel unbootable. So, it needs to be addressed to make this code path work reliably (or kexec should fail at least). Greg, Ingo, Since there is now a legitimate user of this stuff, can we actually export this in sysfs (or something else other than sploit^Wdebugfs)? kexec-tools can have a fallback to debugfs if we really need it, but making people mount debugfs to have some essential piece of functionality scares the heck out of me. -hpa -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/3 v2] Add function get_bootparam
* H. Peter Anvin h...@zytor.com wrote: On 11/11/2013 05:27 PM, Toshi Kani wrote: On Tue, 2013-11-05 at 16:29 +0800, dyo...@redhat.com wrote: Not only setup_subarch will get data from debugfs file boot_params/data, later code for adding efi_info will also need do same thing. Thus add a common function here for later use. get_bootparam() calls find_mnt_by_fsname(debugfs), which assumes that debugfs is mounted with device name debugfs. This function fails when: - debugfs is not mounted, or - debugfs is mounted with a different device name, such as nodev [1]. This issue is not introduced by this patch. But the original code, which sets hardware_subarch, seems to work even if it failed to access debugfs (which is ignored) since hardware_subarch is zero most of the cases anyway. With this change, however, this failure now makes the 2nd kernel unbootable. So, it needs to be addressed to make this code path work reliably (or kexec should fail at least). Greg, Ingo, Since there is now a legitimate user of this stuff, can we actually export this in sysfs (or something else other than sploit^Wdebugfs)? kexec-tools can have a fallback to debugfs if we really need it, but making people mount debugfs to have some essential piece of functionality scares the heck out of me. No principial objections from me: anything that actually turns out to prove itself in debugfs and ends up mattering for real ought to move to sysfs. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/3 v2] Add function get_bootparam
Not only setup_subarch will get data from debugfs file
boot_params/data, later code for adding efi_info will
also need do same thing. Thus add a common function here
for later use.
v1-v2: make get_bootparam() static
Signed-off-by: Dave Young dyo...@redhat.com
---
kexec/arch/i386/x86-linux-setup.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
--- kexec-tools.orig/kexec/arch/i386/x86-linux-setup.c
+++ kexec-tools/kexec/arch/i386/x86-linux-setup.c
@@ -436,10 +436,9 @@ char *find_mnt_by_fsname(char *fsname)
return mntdir;
}
-void setup_subarch(struct x86_linux_param_header *real_mode)
+static void get_bootparam(void *buf, off_t offset, size_t size)
{
int data_file;
- const off_t offset = offsetof(typeof(*real_mode), hardware_subarch);
char *debugfs_mnt;
char filename[PATH_MAX];
@@ -447,7 +446,7 @@ void setup_subarch(struct x86_linux_para
if (!debugfs_mnt)
return;
snprintf(filename, PATH_MAX, %s/%s, debugfs_mnt, boot_params/data);
- filename[PATH_MAX-1] = 0;
+ filename[PATH_MAX - 1] = 0;
free(debugfs_mnt);
data_file = open(filename, O_RDONLY);
@@ -455,11 +454,18 @@ void setup_subarch(struct x86_linux_para
return;
if (lseek(data_file, offset, SEEK_SET) 0)
goto close;
- read(data_file, real_mode-hardware_subarch, sizeof(uint32_t));
+ read(data_file, buf, size);
close:
close(data_file);
}
+void setup_subarch(struct x86_linux_param_header *real_mode)
+{
+ off_t offset = offsetof(typeof(*real_mode), hardware_subarch);
+
+ get_bootparam(real_mode-hardware_subarch, offset, sizeof(uint32_t));
+}
+
void setup_linux_system_parameters(struct kexec_info *info,
struct x86_linux_param_header *real_mode)
{
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
