Re: [Xen-devel] [RFC][PATCH 4/5] tools:firmware:hvmloader: reserve RMRR mappings in e820

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

On 2014/8/8 17:01, Jan Beulich wrote:
> > > > On 08.08.14 at 10:39, <tiejun.chen@xxxxxxxxx> wrote:
> > On 2014/8/8 15:43, Jan Beulich wrote:
> > > More reasonable, albeit now you lost the fetch-just-once behavior.
> > > Furthermore - do you really need this to be a dynamic allocation?
> > So,
> >
> > diff --git a/tools/firmware/hvmloader/util.c
> > b/tools/firmware/hvmloader/util.c
> > index 80d822f..d55773e 100644
> > --- a/tools/firmware/hvmloader/util.c
> > +++ b/tools/firmware/hvmloader/util.c
> > @@ -766,6 +766,17 @@ struct shared_info *get_shared_info(void)
> >        return shared_info;
> >    }
> >
> > +struct e820map *get_rmrr_map_info(void)
> > +{
> > +    if ( rmrr_e820map.nr_map )
> > +        return &rmrr_e820map;
> > +
> > +    if ( hypercall_memory_op(XENMEM_RMRR_memory_map, &rmrr_e820map) != 0 )
> > +        BUG();
> > +
> > +    return &rmrr_e820map;
> > +}
> Still not quite, since now you still re-invoke the hypercall if the first
> one didn't return any entries (e.g. on a VT-d-less system).
I think we just should focus on if we already call that hypercall.

struct e820map *get_rmrr_map_info(void)
{
    static int no_rmrr = 1;

    if ( no_rmrr == 0 )
        return &rmrr_e820map;

    if ( hypercall_memory_op(XENMEM_RMRR_memory_map, &rmrr_e820map) != 0 )
        BUG();

    no_rmrr = 0;

    return &rmrr_e820map;
}

If rmrr_e820map is void, the subsequent codes always do nothing since 
rmrr_e820map.nr_map is zero.
> > --- a/tools/firmware/hvmloader/util.h
> > +++ b/tools/firmware/hvmloader/util.h
> > @@ -236,6 +236,8 @@ unsigned long create_pir_tables(void);
> >    void smp_initialise(void);
> >
> >    #include "e820.h"
> > +struct e820map rmrr_e820map;
> > +struct e820map *get_rmrr_map_info(void);
> >    int build_e820_table(struct e820entry *e820,
> >                         unsigned int lowmem_reserved_base,
> >                         unsigned int bios_image_base);
> >
> >
> > > The structure you use right now is fixed size. Which gets me to
> > > another point though: Is it said in any part of the VT-d spec that
> > > there is an upper limit to the number of RMRRs? The re-use of the
> > After look up some relevant info in VT-D specs, I think we have no any
> > direct field to limit the number of RMRRs. Just in 8.1 DMA Remapping
> > Reporting Structure, there are some fields to be referred here:
> >
> > #1 Length:
> >
> > in bytes, of the description table including the length of the
> > associated remapping structures.
> >
> > #2 Remapping Structures[]:
> >
> > A list of structures. The list will contain one or more DMA Remapping
> > Hardware Unit Definition (DRHD) structures, and zero or more Reserved
> > Memory Region Reporting (RMRR) and Root Port ATS Capability Reporting
> > (ATSR) structures.
> >
> > Seems no any explicit restriction.
> IOW a fixed-length interface structure isn't a proper fit here.
Yes.

Thanks
Tiejun

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