Re: [RFC] arm64: kexec_file_load support

Re: [RFC] arm64: kexec_file_load support

[Thiago Jung Bauermann]

Am Freitag, 01 Juli 2016, 14:11:12 schrieb AKASHI Takahiro:
> I'm not sure whether there is any demand for kexec_file_load
> support on arm64, but anyhow I'm working on this and now
> my early prototype code does work fine.

It is necessary if you want to support loading only signed kernels, and also 
if you want IMA to measure the kernel in its event log.

> There is, however, one essential issue:
> While arm64 kernel requires a device tree blob to be set up
> correctly at boot time, the current system call API doesn't
> have this parameter.
>     int kexec_file_load(int kernel_fd, int initrd_fd,
>                         unsigned long cmdline_len, const char
> *cmdline_ptr, unsigned long flags);
> 
> Should we invent a new system call, like kexec_file_load2,
> and, if so, what kind of interface would be desired?

I'm facing the same issue on powerpc. What I'm doing is taking the device 
tree that was used to boot the current kernel and modifying it as necessary 
to pass it to the next kernel.

I agree that it would be better if we could have a system call where a 
custom device tree could be passed. One suggestion is:


kexec_file_load2(int fds[], int fd_types[], int nr_fds,
		 unsigned long cmdline_len, const char *cmdline_ptr,
		unsigned long flags);

Where fds is an array with nr_fds file descriptors and fd_types is an array 
specifying what each fd in fds is. So for example, if fds[i] is the kernel, 
then fd_types[i] would have the value KEXEC_FILE_KERNEL_FD. If fds[i] is the 
device tree blob, fd_types[i], would have the value KEXEC_FILE_DTB and so 
on. That way, the syscall can be extended for an arbitrary number and types 
of segments that have to be loaded, just like kexec_load.

Another option is to have a struct:

kexec_file_load2(struct kexec_file_params *params, unsigned long params_sz);

Where:

struct kexec_file_params {
	int version;	/* allows struct to be extended in the future */
	int fds[];
	int fd_types[];
	int nr_fds;
	unsigned long cmdline_len;
	const char *cmdline_ptr;
	unsigned long flags;
};

This is even more flexible.

[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC] arm64: kexec_file_load support

[AKASHI Takahiro]

Hi,

On Fri, Jul 01, 2016 at 12:46:31PM -0300, Thiago Jung Bauermann wrote:
> Am Freitag, 01 Juli 2016, 14:11:12 schrieb AKASHI Takahiro:
> > I'm not sure whether there is any demand for kexec_file_load
> > support on arm64, but anyhow I'm working on this and now
> > my early prototype code does work fine.
> 
> It is necessary if you want to support loading only signed kernels, and also 
> if you want IMA to measure the kernel in its event log.
> 
> > There is, however, one essential issue:
> > While arm64 kernel requires a device tree blob to be set up
> > correctly at boot time, the current system call API doesn't
> > have this parameter.
> >     int kexec_file_load(int kernel_fd, int initrd_fd,
> >                         unsigned long cmdline_len, const char
> > *cmdline_ptr, unsigned long flags);
> > 
> > Should we invent a new system call, like kexec_file_load2,
> > and, if so, what kind of interface would be desired?
> 
> I'm facing the same issue on powerpc. What I'm doing is taking the device 
> tree that was used to boot the current kernel and modifying it as necessary 
> to pass it to the next kernel.

That is exactly what I do.

> I agree that it would be better if we could have a system call where a 
> custom device tree could be passed. One suggestion is:

For powerpc, you might be able to use dtbImage instead of Image
without changing the kernel interfaces.
> 
> kexec_file_load2(int fds[], int fd_types[], int nr_fds,
> 		 unsigned long cmdline_len, const char *cmdline_ptr,
> 		unsigned long flags);

You don't want to simply add one more argument, i.e. dtb_fd, don't you.

I prefer a slightly-simpler interface:
        struct kexec_file_fd {
                enum kexec_file_type;
                int fd;
        }

        int kexec_file_load2(struct kexec_file_fd[], int nr_fds, int flags);

Or if you want to keep the compatibility with the existing system call,

        int kexec_file_load(int kernel_fd, int initrd_fd,
                        unsigned long cmdline_len, const char *cmdline_ptr,
                        unsigned long flags,
                        int struct kexec_file_fd[], int nr_fds);

Here SYSCALL_DEFINE7() have to be defined, and I'm not sure that we will not
have a problem in adding a system call with more than 6 arguments.

> Where fds is an array with nr_fds file descriptors and fd_types is an array 
> specifying what each fd in fds is. So for example, if fds[i] is the kernel, 
> then fd_types[i] would have the value KEXEC_FILE_KERNEL_FD. If fds[i] is the 
> device tree blob, fd_types[i], would have the value KEXEC_FILE_DTB and so 
> on. That way, the syscall can be extended for an arbitrary number and types 
> of segments that have to be loaded, just like kexec_load.
> 
> Another option is to have a struct:
> 
> kexec_file_load2(struct kexec_file_params *params, unsigned long params_sz);

Wow, we can add any number of new parameters with this interface.

Thanks,
-Takahiro AKASHI

> Where:
> 
> struct kexec_file_params {
> 	int version;	/* allows struct to be extended in the future */
> 	int fds[];
> 	int fd_types[];
> 	int nr_fds;
> 	unsigned long cmdline_len;
> 	const char *cmdline_ptr;
> 	unsigned long flags;
> };
> 
> This is even more flexible.
> 
> []'s
> Thiago Jung Bauermann
> IBM Linux Technology Center
> 

Re: [RFC] arm64: kexec_file_load support

[Thiago Jung Bauermann]

Hello,

Am Montag, 04 Juli 2016, 15:58:15 schrieb AKASHI Takahiro:
> On Fri, Jul 01, 2016 at 12:46:31PM -0300, Thiago Jung Bauermann wrote:
> > I agree that it would be better if we could have a system call where a
> 
> > custom device tree could be passed. One suggestion is:
> For powerpc, you might be able to use dtbImage instead of Image
> without changing the kernel interfaces.

That works for custom kernels, but for signed kernels from a distro, I 
believe that's not an option.

> > kexec_file_load2(int fds[], int fd_types[], int nr_fds,
> > 
> > 		 unsigned long cmdline_len, const char *cmdline_ptr,
> > 		
> > 		unsigned long flags);
> 
> You don't want to simply add one more argument, i.e. dtb_fd, don't you.

I'm just trying to avoid having to add another argument later if we find out 
someone is loading another segment that we didn't know about. :-)

The older kexec_load system call allows passing an arbitrary number of 
segments (sort of, currently capped at 16) to the kernel, so my suggestions 
preserve that feature.

If people think that adding another argument for dtb_fd is enough, I won't 
mind.

> I prefer a slightly-simpler interface:
>         struct kexec_file_fd {
>                 enum kexec_file_type;
>                 int fd;
>         }
> 
>         int kexec_file_load2(struct kexec_file_fd[], int nr_fds, int
> flags);

I like this one.

> Or if you want to keep the compatibility with the existing system call,
> 
>         int kexec_file_load(int kernel_fd, int initrd_fd,
>                         unsigned long cmdline_len, const char
> *cmdline_ptr, unsigned long flags,
>                         int struct kexec_file_fd[], int nr_fds);
> 
> Here SYSCALL_DEFINE7() have to be defined, and I'm not sure that we will
> not have a problem in adding a system call with more than 6 arguments.

That's very clever. We can do what you suggest above or even just add dtb_fd 
with SYSCALL_DEFINE6. Either option would be good.

> > Where fds is an array with nr_fds file descriptors and fd_types is an
> > array specifying what each fd in fds is. So for example, if fds[i] is
> > the kernel, then fd_types[i] would have the value KEXEC_FILE_KERNEL_FD.
> > If fds[i] is the device tree blob, fd_types[i], would have the value
> > KEXEC_FILE_DTB and so on. That way, the syscall can be extended for an
> > arbitrary number and types of segments that have to be loaded, just
> > like kexec_load.
> > 
> > Another option is to have a struct:
> > 
> > kexec_file_load2(struct kexec_file_params *params, unsigned long
> > params_sz);
> Wow, we can add any number of new parameters with this interface.

Yeah, maybe it's a bit too much.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC] arm64: kexec_file_load support

[AKASHI Takahiro]

On Mon, Jul 04, 2016 at 07:50:19PM -0300, Thiago Jung Bauermann wrote:
> Hello,
> 
> Am Montag, 04 Juli 2016, 15:58:15 schrieb AKASHI Takahiro:
> > On Fri, Jul 01, 2016 at 12:46:31PM -0300, Thiago Jung Bauermann wrote:
> > > I agree that it would be better if we could have a system call where a
> > 
> > > custom device tree could be passed. One suggestion is:
> > For powerpc, you might be able to use dtbImage instead of Image
> > without changing the kernel interfaces.
> 
> That works for custom kernels, but for signed kernels from a distro, I 
> believe that's not an option.
> 
> > > kexec_file_load2(int fds[], int fd_types[], int nr_fds,
> > > 
> > > 		 unsigned long cmdline_len, const char *cmdline_ptr,
> > > 		
> > > 		unsigned long flags);
> > 
> > You don't want to simply add one more argument, i.e. dtb_fd, don't you.
> 
> I'm just trying to avoid having to add another argument later if we find out 
> someone is loading another segment that we didn't know about. :-)
> 
> The older kexec_load system call allows passing an arbitrary number of 
> segments (sort of, currently capped at 16) to the kernel, so my suggestions 
> preserve that feature.
> 
> If people think that adding another argument for dtb_fd is enough, I won't 
> mind.

That is the question :)
As far as I look though existing arch code, there will be no extra
parameters (segments) required other than dtb.

Thanks,
-Takahiro AKASHI

> > I prefer a slightly-simpler interface:
> >         struct kexec_file_fd {
> >                 enum kexec_file_type;
> >                 int fd;
> >         }
> > 
> >         int kexec_file_load2(struct kexec_file_fd[], int nr_fds, int
> > flags);
> 
> I like this one.
> 
> > Or if you want to keep the compatibility with the existing system call,
> > 
> >         int kexec_file_load(int kernel_fd, int initrd_fd,
> >                         unsigned long cmdline_len, const char
> > *cmdline_ptr, unsigned long flags,
> >                         int struct kexec_file_fd[], int nr_fds);
> > 
> > Here SYSCALL_DEFINE7() have to be defined, and I'm not sure that we will
> > not have a problem in adding a system call with more than 6 arguments.
> 
> That's very clever. We can do what you suggest above or even just add dtb_fd 
> with SYSCALL_DEFINE6. Either option would be good.
> 
> > > Where fds is an array with nr_fds file descriptors and fd_types is an
> > > array specifying what each fd in fds is. So for example, if fds[i] is
> > > the kernel, then fd_types[i] would have the value KEXEC_FILE_KERNEL_FD.
> > > If fds[i] is the device tree blob, fd_types[i], would have the value
> > > KEXEC_FILE_DTB and so on. That way, the syscall can be extended for an
> > > arbitrary number and types of segments that have to be loaded, just
> > > like kexec_load.
> > > 
> > > Another option is to have a struct:
> > > 
> > > kexec_file_load2(struct kexec_file_params *params, unsigned long
> > > params_sz);
> > Wow, we can add any number of new parameters with this interface.
> 
> Yeah, maybe it's a bit too much.
> 
> -- 
> []'s
> Thiago Jung Bauermann
> IBM Linux Technology Center
> 

Re: [RFC] arm64: kexec_file_load support

[Dave Young]

On 07/04/16 at 03:58pm, AKASHI Takahiro wrote:
> Hi,
> 
> On Fri, Jul 01, 2016 at 12:46:31PM -0300, Thiago Jung Bauermann wrote:
> > Am Freitag, 01 Juli 2016, 14:11:12 schrieb AKASHI Takahiro:
> > > I'm not sure whether there is any demand for kexec_file_load
> > > support on arm64, but anyhow I'm working on this and now
> > > my early prototype code does work fine.
> > 
> > It is necessary if you want to support loading only signed kernels, and also 
> > if you want IMA to measure the kernel in its event log.
> > 
> > > There is, however, one essential issue:
> > > While arm64 kernel requires a device tree blob to be set up
> > > correctly at boot time, the current system call API doesn't
> > > have this parameter.
> > >     int kexec_file_load(int kernel_fd, int initrd_fd,
> > >                         unsigned long cmdline_len, const char
> > > *cmdline_ptr, unsigned long flags);
> > > 
> > > Should we invent a new system call, like kexec_file_load2,
> > > and, if so, what kind of interface would be desired?
> > 
> > I'm facing the same issue on powerpc. What I'm doing is taking the device 
> > tree that was used to boot the current kernel and modifying it as necessary 
> > to pass it to the next kernel.
> 
> That is exactly what I do.
> 
> > I agree that it would be better if we could have a system call where a 
> > custom device tree could be passed. One suggestion is:
> 
> For powerpc, you might be able to use dtbImage instead of Image
> without changing the kernel interfaces.
> > 
> > kexec_file_load2(int fds[], int fd_types[], int nr_fds,
> > 		 unsigned long cmdline_len, const char *cmdline_ptr,
> > 		unsigned long flags);
> 
> You don't want to simply add one more argument, i.e. dtb_fd, don't you.
> 
> I prefer a slightly-simpler interface:
>         struct kexec_file_fd {
>                 enum kexec_file_type;
>                 int fd;
>         }
> 
>         int kexec_file_load2(struct kexec_file_fd[], int nr_fds, int flags);
> 
> Or if you want to keep the compatibility with the existing system call,
> 
>         int kexec_file_load(int kernel_fd, int initrd_fd,
>                         unsigned long cmdline_len, const char *cmdline_ptr,
>                         unsigned long flags,
>                         int struct kexec_file_fd[], int nr_fds);
> 
> Here SYSCALL_DEFINE7() have to be defined, and I'm not sure that we will not
> have a problem in adding a system call with more than 6 arguments.
> 
> > Where fds is an array with nr_fds file descriptors and fd_types is an array 
> > specifying what each fd in fds is. So for example, if fds[i] is the kernel, 
> > then fd_types[i] would have the value KEXEC_FILE_KERNEL_FD. If fds[i] is the 
> > device tree blob, fd_types[i], would have the value KEXEC_FILE_DTB and so 
> > on. That way, the syscall can be extended for an arbitrary number and types 
> > of segments that have to be loaded, just like kexec_load.
> > 
> > Another option is to have a struct:
> > 
> > kexec_file_load2(struct kexec_file_params *params, unsigned long params_sz);
> 
> Wow, we can add any number of new parameters with this interface.
> 
> Thanks,
> -Takahiro AKASHI
> 
> > Where:
> > 
> > struct kexec_file_params {
> > 	int version;	/* allows struct to be extended in the future */
> > 	int fds[];
> > 	int fd_types[];
> > 	int nr_fds;
> > 	unsigned long cmdline_len;
> > 	const char *cmdline_ptr;
> > 	unsigned long flags;
> > };
> > 
> > This is even more flexible.

I would like to vote for this one, and use kexec_file_fd fds[] in the struct 
 
Thanks
Dave

Re: [RFC] arm64: kexec_file_load support

[AKASHI Takahiro]

Hi Dave,

On Tue, Jul 05, 2016 at 09:25:56AM +0800, Dave Young wrote:
> On 07/04/16 at 03:58pm, AKASHI Takahiro wrote:
> > Hi,
> > 
> > On Fri, Jul 01, 2016 at 12:46:31PM -0300, Thiago Jung Bauermann wrote:
> > > Am Freitag, 01 Juli 2016, 14:11:12 schrieb AKASHI Takahiro:
> > > > I'm not sure whether there is any demand for kexec_file_load
> > > > support on arm64, but anyhow I'm working on this and now
> > > > my early prototype code does work fine.
> > > 
> > > It is necessary if you want to support loading only signed kernels, and also 
> > > if you want IMA to measure the kernel in its event log.
> > > 
> > > > There is, however, one essential issue:
> > > > While arm64 kernel requires a device tree blob to be set up
> > > > correctly at boot time, the current system call API doesn't
> > > > have this parameter.
> > > >     int kexec_file_load(int kernel_fd, int initrd_fd,
> > > >                         unsigned long cmdline_len, const char
> > > > *cmdline_ptr, unsigned long flags);
> > > > 
> > > > Should we invent a new system call, like kexec_file_load2,
> > > > and, if so, what kind of interface would be desired?
> > > 
> > > I'm facing the same issue on powerpc. What I'm doing is taking the device 
> > > tree that was used to boot the current kernel and modifying it as necessary 
> > > to pass it to the next kernel.
> > 
> > That is exactly what I do.
> > 
> > > I agree that it would be better if we could have a system call where a 
> > > custom device tree could be passed. One suggestion is:
> > 
> > For powerpc, you might be able to use dtbImage instead of Image
> > without changing the kernel interfaces.
> > > 
> > > kexec_file_load2(int fds[], int fd_types[], int nr_fds,
> > > 		 unsigned long cmdline_len, const char *cmdline_ptr,
> > > 		unsigned long flags);
> > 
> > You don't want to simply add one more argument, i.e. dtb_fd, don't you.
> > 
> > I prefer a slightly-simpler interface:
> >         struct kexec_file_fd {
> >                 enum kexec_file_type;
> >                 int fd;
> >         }
> > 
> >         int kexec_file_load2(struct kexec_file_fd[], int nr_fds, int flags);
> > 
> > Or if you want to keep the compatibility with the existing system call,
> > 
> >         int kexec_file_load(int kernel_fd, int initrd_fd,
> >                         unsigned long cmdline_len, const char *cmdline_ptr,
> >                         unsigned long flags,
> >                         int struct kexec_file_fd[], int nr_fds);
> > 
> > Here SYSCALL_DEFINE7() have to be defined, and I'm not sure that we will not
> > have a problem in adding a system call with more than 6 arguments.
> > 
> > > Where fds is an array with nr_fds file descriptors and fd_types is an array 
> > > specifying what each fd in fds is. So for example, if fds[i] is the kernel, 
> > > then fd_types[i] would have the value KEXEC_FILE_KERNEL_FD. If fds[i] is the 
> > > device tree blob, fd_types[i], would have the value KEXEC_FILE_DTB and so 
> > > on. That way, the syscall can be extended for an arbitrary number and types 
> > > of segments that have to be loaded, just like kexec_load.
> > > 
> > > Another option is to have a struct:
> > > 
> > > kexec_file_load2(struct kexec_file_params *params, unsigned long params_sz);
> > 
> > Wow, we can add any number of new parameters with this interface.
> > 
> > Thanks,
> > -Takahiro AKASHI
> > 
> > > Where:
> > > 
> > > struct kexec_file_params {
> > > 	int version;	/* allows struct to be extended in the future */
> > > 	int fds[];
> > > 	int fd_types[];
> > > 	int nr_fds;
> > > 	unsigned long cmdline_len;
> > > 	const char *cmdline_ptr;
> > > 	unsigned long flags;
> > > };
> > > 
> > > This is even more flexible.
> 
> I would like to vote for this one, and use kexec_file_fd fds[] in the struct 

If we take this approach, we'd better take "flags" out of struct,
and my preference would be:

        enum kexec_file_type {
                KEXEC_FILE_TYPE_KERNEL;
                KEXEC_FILE_TYPE_INITRD;
                KEXEC_FILE_TYPE_DTB;
        }

        struct kexec_file_fd {
                enum kexec_file_type;
                int fd;
        }

        sturct kexec_file_params {
                int version;
                unsigned char *cmdline;
                unsigned long cmdline_len;
                int nr_fds;
                struct kexec_file_fd fds[0];
        }

        int kexec_file_load2(int kernel_fd, unsigned long flags,
                                sturct kexec_file_params extra);

So we don't have to retrieve extra if KEXEC_FILE_UNLOAD
(or kernel_fd < 0?),
and only once retrieve extra if extra != NULL && nr_fds == 0.

Thanks,
-Takahiro AKASHI

> Thanks
> Dave

Re: [RFC] arm64: kexec_file_load support

[Dave Young]

On 07/05/16 at 05:03pm, AKASHI Takahiro wrote:
> Hi Dave,
> 
> On Tue, Jul 05, 2016 at 09:25:56AM +0800, Dave Young wrote:
> > On 07/04/16 at 03:58pm, AKASHI Takahiro wrote:
> > > Hi,
> > > 
> > > On Fri, Jul 01, 2016 at 12:46:31PM -0300, Thiago Jung Bauermann wrote:
> > > > Am Freitag, 01 Juli 2016, 14:11:12 schrieb AKASHI Takahiro:
> > > > > I'm not sure whether there is any demand for kexec_file_load
> > > > > support on arm64, but anyhow I'm working on this and now
> > > > > my early prototype code does work fine.
> > > > 
> > > > It is necessary if you want to support loading only signed kernels, and also 
> > > > if you want IMA to measure the kernel in its event log.
> > > > 
> > > > > There is, however, one essential issue:
> > > > > While arm64 kernel requires a device tree blob to be set up
> > > > > correctly at boot time, the current system call API doesn't
> > > > > have this parameter.
> > > > >     int kexec_file_load(int kernel_fd, int initrd_fd,
> > > > >                         unsigned long cmdline_len, const char
> > > > > *cmdline_ptr, unsigned long flags);
> > > > > 
> > > > > Should we invent a new system call, like kexec_file_load2,
> > > > > and, if so, what kind of interface would be desired?
> > > > 
> > > > I'm facing the same issue on powerpc. What I'm doing is taking the device 
> > > > tree that was used to boot the current kernel and modifying it as necessary 
> > > > to pass it to the next kernel.
> > > 
> > > That is exactly what I do.
> > > 
> > > > I agree that it would be better if we could have a system call where a 
> > > > custom device tree could be passed. One suggestion is:
> > > 
> > > For powerpc, you might be able to use dtbImage instead of Image
> > > without changing the kernel interfaces.
> > > > 
> > > > kexec_file_load2(int fds[], int fd_types[], int nr_fds,
> > > > 		 unsigned long cmdline_len, const char *cmdline_ptr,
> > > > 		unsigned long flags);
> > > 
> > > You don't want to simply add one more argument, i.e. dtb_fd, don't you.
> > > 
> > > I prefer a slightly-simpler interface:
> > >         struct kexec_file_fd {
> > >                 enum kexec_file_type;
> > >                 int fd;
> > >         }
> > > 
> > >         int kexec_file_load2(struct kexec_file_fd[], int nr_fds, int flags);
> > > 
> > > Or if you want to keep the compatibility with the existing system call,
> > > 
> > >         int kexec_file_load(int kernel_fd, int initrd_fd,
> > >                         unsigned long cmdline_len, const char *cmdline_ptr,
> > >                         unsigned long flags,
> > >                         int struct kexec_file_fd[], int nr_fds);
> > > 
> > > Here SYSCALL_DEFINE7() have to be defined, and I'm not sure that we will not
> > > have a problem in adding a system call with more than 6 arguments.
> > > 
> > > > Where fds is an array with nr_fds file descriptors and fd_types is an array 
> > > > specifying what each fd in fds is. So for example, if fds[i] is the kernel, 
> > > > then fd_types[i] would have the value KEXEC_FILE_KERNEL_FD. If fds[i] is the 
> > > > device tree blob, fd_types[i], would have the value KEXEC_FILE_DTB and so 
> > > > on. That way, the syscall can be extended for an arbitrary number and types 
> > > > of segments that have to be loaded, just like kexec_load.
> > > > 
> > > > Another option is to have a struct:
> > > > 
> > > > kexec_file_load2(struct kexec_file_params *params, unsigned long params_sz);
> > > 
> > > Wow, we can add any number of new parameters with this interface.
> > > 
> > > Thanks,
> > > -Takahiro AKASHI
> > > 
> > > > Where:
> > > > 
> > > > struct kexec_file_params {
> > > > 	int version;	/* allows struct to be extended in the future */
> > > > 	int fds[];
> > > > 	int fd_types[];
> > > > 	int nr_fds;
> > > > 	unsigned long cmdline_len;
> > > > 	const char *cmdline_ptr;
> > > > 	unsigned long flags;
> > > > };
> > > > 
> > > > This is even more flexible.
> > 
> > I would like to vote for this one, and use kexec_file_fd fds[] in the struct 
> 
> If we take this approach, we'd better take "flags" out of struct,
> and my preference would be:
> 
>         enum kexec_file_type {
>                 KEXEC_FILE_TYPE_KERNEL;
>                 KEXEC_FILE_TYPE_INITRD;
>                 KEXEC_FILE_TYPE_DTB;
>         }
> 
>         struct kexec_file_fd {
>                 enum kexec_file_type;
>                 int fd;
>         }
> 
>         sturct kexec_file_params {
>                 int version;
>                 unsigned char *cmdline;
>                 unsigned long cmdline_len;
>                 int nr_fds;
>                 struct kexec_file_fd fds[0];
>         }
> 
>         int kexec_file_load2(int kernel_fd, unsigned long flags,
>                                 sturct kexec_file_params extra);
> 
> So we don't have to retrieve extra if KEXEC_FILE_UNLOAD
> (or kernel_fd < 0?),
> and only once retrieve extra if extra != NULL && nr_fds == 0.

If so maybe change a bit from your precious mentioned 7 args proposal like
below?

struct kexec_file_fd {
	enum kexec_file_type;
	int fd;
}

struct kexec_fdset {
	int nr_fd;
	struct kexec_file_fd fd[0];
}

int kexec_file_load(int kernel_fd, int initrd_fd,
		    unsigned long cmdline_len, const char *cmdline_ptr,
		    unsigned long flags, struct kexec_fdset *extra_fds);

Thanks
Dave

Re: [RFC] arm64: kexec_file_load support

[Thiago Jung Bauermann]

Am Donnerstag, 07 Juli 2016, 14:12:45 schrieb Dave Young:
> If so maybe change a bit from your precious mentioned 7 args proposal like
> below?
> 
> struct kexec_file_fd {
> 	enum kexec_file_type;
> 	int fd;
> }
> 
> struct kexec_fdset {
> 	int nr_fd;
> 	struct kexec_file_fd fd[0];
> }
> 
> int kexec_file_load(int kernel_fd, int initrd_fd,
> 		    unsigned long cmdline_len, const char *cmdline_ptr,
> 		    unsigned long flags, struct kexec_fdset *extra_fds);


Is there a way for the kernel to distinguish whether the process passed 5 or 
6 arguments? How can it know whether extra_fds is a valid argument or just 
garbage? I think we have to define a new flag KEXEC_FILE_EXTRA_FDS so that 
the process can signal that it is using the new interface.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC] arm64: kexec_file_load support

[Dave Young]

On 07/08/16 at 11:48am, Thiago Jung Bauermann wrote:
> Am Donnerstag, 07 Juli 2016, 14:12:45 schrieb Dave Young:
> > If so maybe change a bit from your precious mentioned 7 args proposal like
> > below?
> > 
> > struct kexec_file_fd {
> > 	enum kexec_file_type;
> > 	int fd;
> > }
> > 
> > struct kexec_fdset {
> > 	int nr_fd;
> > 	struct kexec_file_fd fd[0];
> > }
> > 
> > int kexec_file_load(int kernel_fd, int initrd_fd,
> > 		    unsigned long cmdline_len, const char *cmdline_ptr,
> > 		    unsigned long flags, struct kexec_fdset *extra_fds);
> 
> 
> Is there a way for the kernel to distinguish whether the process passed 5 or 
> 6 arguments? How can it know whether extra_fds is a valid argument or just 
> garbage? I think we have to define a new flag KEXEC_FILE_EXTRA_FDS so that 
> the process can signal that it is using the new interface.

Agreed, a new flag is needed.

Thanks
Dave

Re: [RFC] arm64: kexec_file_load support

[AKASHI Takahiro]

On Fri, Jul 08, 2016 at 11:48:44AM -0300, Thiago Jung Bauermann wrote:
> Am Donnerstag, 07 Juli 2016, 14:12:45 schrieb Dave Young:
> > If so maybe change a bit from your precious mentioned 7 args proposal like
> > below?
> > 
> > struct kexec_file_fd {
> > 	enum kexec_file_type;
> > 	int fd;
> > }
> > 
> > struct kexec_fdset {
> > 	int nr_fd;
> > 	struct kexec_file_fd fd[0];
> > }
> > 
> > int kexec_file_load(int kernel_fd, int initrd_fd,
> > 		    unsigned long cmdline_len, const char *cmdline_ptr,
> > 		    unsigned long flags, struct kexec_fdset *extra_fds);
> 
> 
> Is there a way for the kernel to distinguish whether the process passed 5 or 
> 6 arguments? How can it know whether extra_fds is a valid argument or just 
> garbage? I think we have to define a new flag KEXEC_FILE_EXTRA_FDS so that 
> the process can signal that it is using the new interface.

Good point. What I had in my mind is "open" system call.
Glibc's implementation of open() checks for the second argument, flags,
to determine whether or not the third argument, mode, is required.

So our current consensus is that, for the time being, we will extend
the existing system call interface to allow extra file descriptors,
and *if* we really need a different type of parameters in the future,
we will add another system call.

If we all agree, I can post a kernel patch for this change as a RFC.
(I've already verified it with my prototype of kexec_file_load support
on arm64.)

Thanks,
-Takahiro AKASHI

> -- 
> []'s
> Thiago Jung Bauermann
> IBM Linux Technology Center
> 

Re: [RFC] arm64: kexec_file_load support

[Dave Young]

On 07/11/16 at 04:19pm, AKASHI Takahiro wrote:
> On Fri, Jul 08, 2016 at 11:48:44AM -0300, Thiago Jung Bauermann wrote:
> > Am Donnerstag, 07 Juli 2016, 14:12:45 schrieb Dave Young:
> > > If so maybe change a bit from your precious mentioned 7 args proposal like
> > > below?
> > > 
> > > struct kexec_file_fd {
> > > 	enum kexec_file_type;
> > > 	int fd;
> > > }
> > > 
> > > struct kexec_fdset {
> > > 	int nr_fd;
> > > 	struct kexec_file_fd fd[0];
> > > }
> > > 
> > > int kexec_file_load(int kernel_fd, int initrd_fd,
> > > 		    unsigned long cmdline_len, const char *cmdline_ptr,
> > > 		    unsigned long flags, struct kexec_fdset *extra_fds);
> > 
> > 
> > Is there a way for the kernel to distinguish whether the process passed 5 or 
> > 6 arguments? How can it know whether extra_fds is a valid argument or just 
> > garbage? I think we have to define a new flag KEXEC_FILE_EXTRA_FDS so that 
> > the process can signal that it is using the new interface.
> 
> Good point. What I had in my mind is "open" system call.
> Glibc's implementation of open() checks for the second argument, flags,
> to determine whether or not the third argument, mode, is required.
> 
> So our current consensus is that, for the time being, we will extend
> the existing system call interface to allow extra file descriptors,
> and *if* we really need a different type of parameters in the future,
> we will add another system call.

Sounds good to me.

> 
> If we all agree, I can post a kernel patch for this change as a RFC.
> (I've already verified it with my prototype of kexec_file_load support
> on arm64.)
> 
> Thanks,
> -Takahiro AKASHI
> 
> > -- 
> > []'s
> > Thiago Jung Bauermann
> > IBM Linux Technology Center
> > 

Thanks
Dave