Re: [PATCH 11/13] kexec-bzImage: Support for loading bzImage using 64bit entry

[Borislav Petkov]

On Mon, Jun 16, 2014 at 04:06:08PM -0400, Vivek Goyal wrote:
> There can be more than one loader and the one which claims first
> to recognize the image will get to load the image. So once 32 bit
> loader support comes in, it might happen that we ask 64bit loader
> first and it rejects the image and then we ask 32bit loader.

What does that have to do with anything??

> So these message are really debug message which tells why loader
> is not accepting an image. It might not be image destined for that
> loader at all.
> 
> pr_debug() allows being verbose if user wants to for debugging purposes.
> You just have to make sure that CONFIG_DYNAMIC_DEBUG=y and enable verbosity
> in individual file.
> 
> echo 'file kexec-bzimage.c +p' > /sys/kernel/debug/dynamic_debug/control

So people are supposed to enable dynamic_debug just so that they see
*why* their image doesn't load.

Doesn't sound optimal to me.

> Same here. We will potentially be trying multiple loaders and if every
> loader prints messages for rejection by default, it is too much of
> info, IMO.

For max two loaders on one architecture? I don't think so. Now you're
just arguing for the sake of it.

> I like doing memory allocations early in the functions (as far as
> possible) and error out if need be. If memory is available to begin
> with for all the data structures needed by this function, it is kind
> of pointless to do rest of the processing.

We're talking about memory for a single void * which is ridiculous. And
I think simplifying the error paths is a much higher win than doing some
minor allocation.

> Hmm..., If you feel strongly about it, I can make this change. I
> thought I just made it easier to share the code between 32bit and
> 64bit by this.

Someone later can do that - right now this code is 64-bit only as far as
we're concerned and if it can be made to work on 32-bit, then people are
free to do so.

> I think it just makes it safer that we don't try to copy more than
> size of destination, in case ->eddbuf_entries is not right or corrupted.
> 
> I see copy_edd() does similar thing.
> 
> memcpy(edd.edd_info, boot_params.eddbuf, sizeof(edd.edd_info));
> edd.edd_info_nr = boot_params.eddbuf_entries;
> 
> So may be it is not a bad idea to copy based on max size of data
> structures.

Ok, makes sense.

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/