Re: [Xen-devel] [PATCH RFC 1/7] xen/x86: Add mbd.h header file

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Tue, Sep 09, 2014 at 03:28:27PM +0100, Jan Beulich wrote:
> >>> On 09.09.14 at 15:39, <daniel.kiper@xxxxxxxxxx> wrote:
> > On Mon, Aug 11, 2014 at 10:49:15AM +0100, Jan Beulich wrote:
> >> >>> On 09.08.14 at 01:04, <daniel.kiper@xxxxxxxxxx> wrote:
> >> > --- /dev/null
> >> > +++ b/xen/include/asm-x86/mbd.h
> >> > @@ -0,0 +1,70 @@
> >> > +/*
> >> > + * Copyright (c) 2013, 2014 Oracle Co., Daniel Kiper
> >> > + *
> >> > + * This program is free software; you can redistribute it and/or modify
> >> > + * it under the terms of the GNU General Public License as published by
> >> > + * the Free Software Foundation; either version 2 of the License, or
> >> > + * (at your option) any later version.
> >> > + *
> >> > + * This program is distributed in the hope that it will be useful,
> >> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> > + * GNU General Public License for more details.
> >> > + *
> >> > + * You should have received a copy of the GNU General Public License 
> >> > along
> >> > + * with this program.  If not, see <http://www.gnu.org/licenses/>.
> >> > + */
> >> > +
> >> > +#ifndef __MBD_H__
> >> > +#define __MBD_H__
> >> > +
> >> > +/* Module type. */
> >> > +typedef struct {
> >> > +    u32 start;
> >> > +    u32 end;
> >> > +
> >> > +    /* Pointer to a module command line. */
> >> > +    u32 cmdline;
> >> > +
> >> > +    /* If relocated != 0 then a given module was relocated. */
> >> > +    u32 relocated;
> >> > +} boot_module_t;
> >> > +
> >> > +/*
> >> > + * MultiBoot Data (MBD) type. It is used to define variable which
> >> > + * carry over data from multiboot protocol (any version) through
> >> > + * Xen preloader stage. Later all or parts of this data is used
> >> > + * to initialize Xen Boot Info (XBI) structure.
> >> > + */
> >> > +typedef struct {
> >> > +    /* Pointer to boot loader name. */
> >> > +    u32 boot_loader_name;
> >> > +
> >> > +    /* Pointer to Xen command line. */
> >> > +    u32 cmdline;
> >> > +
> >> > +    /*
> >> > +     * Amount of lower memory (in KiB) accordingly to The Multiboot
> >> > +     * Specification version 0.6.96.
> >> > +     */
> >> > +    u32 mem_lower;
> >> > +
> >> > +    /*
> >> > +     * Amount of upper memory (in KiB) accordingly to The Multiboot
> >> > +     * Specification version 0.6.96.
> >> > +     */
> >> > +    u32 mem_upper;
> >> > +
> >> > +    /* Size (in bytes) of memory map provided by bootloader. */
> >> > +    u32 mmap_size;
> >> > +
> >> > +    /* Pointer to memory map provided by bootloader. */
> >> > +    u32 mmap;
> >> > +
> >> > +    /* Number of modules. */
> >> > +    u32 mods_nr;
> >> > +
> >> > +    /* Pointer to modules description (boot_module_t *). */
> >> > +    u32 mods;
> >> > +} mbd_t;
> >> > +#endif /* __MBD_H__ */
> >>
> >> For several of the fields I wonder whether u32 is really the right
> >> data type (i.e. whether u64 wouldn't be more correct going
> >> forward).
> >
> > All data which we get from multiboot[12] is u32. So, I think it does not
> > make sense to use u64 here.
>
> Wasn't your proposal to make this an abstraction? If so, what
> does the (current) data source have to do with the types here?

No, this structure is used to transport multiboot[12] protocol
data through preloader stage. It is x86 internal stuff. Later,
all boot data from various sources (including MBD) is put into
XBI. So, probably you think about XBI. This struct will be common
stuff for archs. Hence, in case of XBI I agree that we should very
carefully choose relevant types.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel