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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 08.08.14 at 10:39, <tiejun.chen@intel.com> 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).

> --- 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.

Jan