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
