[PATCH v4 4/4] x86: Pass memory range via E820 for kdump

[Vivek Goyal]

On Fri, Mar 28, 2014 at 12:52:54PM +0800, WANG Chao wrote:

[..]
> > > +static void add_setup_data(struct kexec_info *info,
> > > +			   struct x86_linux_param_header *real_mode,
> > > +			   struct setup_data *sd)
> > > +{
> > 
> > What is setup_data? A little comment above function will make it easy
> > to read. Is it that list of elements which contains extra memory map
> > entries?
> 
> Not exactly. All extra memory maps (for SETUP_E820_EXT type) are
> sealed into a single setup_data structure. Different types of setup_data
> are linked in a list.
> 
> setup_data can be used to pass extra data for boot, for example EFI
> data (SETUP_EFI), extended E820 map (SETUP_E820_EXT), SETUP_PCI and
> SETUP_DTB. These types are defined when defining struct setup_data.
> 
> It's offically documented in Documentation/x86/boot.txt.
> 
> Field name:	setup_data
> Type:		write (special)
> Offset/size:	0x250/8
> Protocol:	2.09+
> 
>   The 64-bit physical pointer to NULL terminated single linked list of
>   struct setup_data. This is used to define a more extensible boot
>   parameters passing mechanism. The definition of struct setup_data
>   is as follow:
> 
>   struct setup_data {
> 	  u64 next;
> 	  u32 type;
> 	  u32 len;
> 	  u8  data[0];
>   };
> 
>   Where, the next is a 64-bit physical pointer to the next node of
>   linked list, the next field of the last node is 0; the type is used
>   to identify the contents of data; the len is the length of data
>   field; the data holds the real payload.
> 
>   This list may be modified at a number of points during the bootup
>   process. Therefore, when modifying this list one should always make
>   sure to consider the case where the linked list already contains
>   entries.
> 
> I think I would comment add_setup_data as follows:
> 
> /*
>  * Added another instance to single linked list of struct setup_data.
>  * Please refer to kernel Documentation/x86/boot.txt for more details
>  * about setup_data structure.
>  */

Thanks for the explanation. Above comment will help the reader.

Thanks
Vivek