[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v5 1/1] Add mmio_hole_size



On 09/29/14 21:27, Boris Ostrovsky wrote:
>
> On 09/11/2014 12:20 PM, Don Slutz wrote:
>> If you add enough PCI devices then all mmio may not fit below 4G
>> which may not be the layout the user wanted. This allows you to
>> increase the below 4G address space that PCI devices can use and
>> therefore in more cases not have any mmio that is above 4G.
>>
>> There are real PCI cards that do not support mmio over 4G, so if you
>> want to emulate them precisely, you may also need to increase the
>> space below 4G for them. There are drivers for these cards that also
>> do not work if they have their mmio space mapped above 4G.
>>
>> This allows growing the MMIO hole to the size needed.
>>
>> This may help with using pci passthru and HVM.
>>
>> Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
>> ---
>> v5:
>>    Add LIBXL_HAVE_BUILDINFO_HVM_MMIO_HOLE_SIZE.
>> v4:
>>      s/pci_hole_min_size/mmio_hole_size/
>>      s/HVM_BELOW_4G_RAM_END/HVM_BELOW_4G_MMIO_START/
>>      Add ignore to hvmloader message
>>      Adjust commit message
>>      Dropped min; stopped mmio_hole_size changing in hvmloader
>>
>>   docs/man/xl.cfg.pod.5             | 12 +++++++
>>   tools/firmware/hvmloader/config.h |  1 -
>>
>>   tools/firmware/hvmloader/pci.c    | 71 
>> +++++++++++++++++++++++++++------------
>>   tools/libxc/xc_hvm_build_x86.c    | 30 +++++++++++++++--
>>   tools/libxc/xenguest.h            | 11 ++++++
>>   tools/libxl/libxl.h               |  5 +++
>>   tools/libxl/libxl_create.c        | 29 ++++++++++++----
>>   tools/libxl/libxl_dm.c            | 24 +++++++++++--
>>   tools/libxl/libxl_dom.c           |  3 +-
>>   tools/libxl/libxl_types.idl       |  1 +
>>   tools/libxl/xl_cmdimpl.c          | 13 +++++++
>>   11 files changed, 166 insertions(+), 34 deletions(-)
>>
>> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
>> index 517ae2f..45f7857 100644
>> --- a/docs/man/xl.cfg.pod.5
>> +++ b/docs/man/xl.cfg.pod.5
>> @@ -1111,6 +1111,18 @@ wallclock (i.e., real) time.
>>     =back
>>   +=head3 Memory layout
>> +
>> +=over 4
>> +
>> +=item B<mmio_hole_size=BYTES>
>> +
>> +Specifies the size the MMIO hole below 4GiB will be.  Only valid for
>> +device_model_version = "qemu-xen".  Note: this requires QEMU 2.1.0
>> +or later.
>
> Can you add text describing restrictions on hole size? Not smaller 
> than 256MB, I think.
>

Yes, I can add some things like "not smaller the 256MiB", "Not larger then
4GiB".  There is no good statement on a max value (See other thread
for more details).

>> +
>> +=back
>> +

...

>> -    while ( (mmio_total > (pci_mem_end - pci_mem_start))
>> -            && ((pci_mem_start << 1) != 0)
>> -            && (allow_memory_relocate
>> -                || (((pci_mem_start << 1) >> PAGE_SHIFT)
>> -                    >= hvm_info->low_mem_pgend)) )
>> -        pci_mem_start <<= 1;
>> +    if ( mmio_hole_size )
>> +    {
>> +        uint64_t max_ram_below_4g = (1ULL << 32) - mmio_hole_size;
>> +
>> +        if ( max_ram_below_4g > HVM_BELOW_4G_MMIO_START )
>> +        {
>> +            printf("max_ram_below_4g=0x"PRIllx
>> +                   " too big for mmio_hole_size=0x"PRIllx
>> +                   " has been ignored.\n",
>> +                   PRIllx_arg(max_ram_below_4g),
>> +                   PRIllx_arg(mmio_hole_size));
>> +        }
>
> Do you need to check whether the hole is too large?
>
> Here and in the toolstack.
>

Not really.  Being unsigned handles most of these cases.  And I have
not found a good value to use.  Just like memory does not say anything
about the low end.

>
>> +        else
>> +        {
>> +            pci_mem_start = max_ram_below_4g;
>> +            printf("pci_mem_start=0x%lx (was 0x%x) for 
>> mmio_hole_size=%lu\n",
>> +                   pci_mem_start, HVM_BELOW_4G_MMIO_START,
>> +                   (long)mmio_hole_size);
>> +        }
>> +    }
>> +    else
>> +    {
>> +        /*
>> +         * At the moment qemu-xen can't deal with relocated memory 
>> regions.
>> +         * It's too close to the release to make a proper fix; for now,
>> +         * only allow the MMIO hole to grow large enough to move 
>> guest memory
>> +         * if we're running qemu-traditional.  Items that don't fit 
>> will be
>> +         * relocated into the 64-bit address space.
>> +         *
>> +         * This loop now does the following:
>> +         * - If allow_memory_relocate, increase the MMIO hole until 
>> it's
>> +         *   big enough, or until it's 2GiB
>> +         * - If !allow_memory_relocate, increase the MMIO hole until 
>> it's
>> +         *   big enough, or until it's 2GiB, or until it overlaps guest
>> +         *   memory
>> +         */
>> +        while ( (mmio_total > (pci_mem_end - pci_mem_start))
>> +                && ((pci_mem_start << 1) != 0)
>> +                && (allow_memory_relocate
>> +                    || (((pci_mem_start << 1) >> PAGE_SHIFT)
>> +                        >= hvm_info->low_mem_pgend)) )
>> +            pci_mem_start <<= 1;
>> +    }
>>         if ( mmio_total > (pci_mem_end - pci_mem_start) )
>>       {
>> diff --git a/tools/libxc/xc_hvm_build_x86.c 
>> b/tools/libxc/xc_hvm_build_x86.c
>> index c81a25b..94da7fa 100644
>> --- a/tools/libxc/xc_hvm_build_x86.c
>> +++ b/tools/libxc/xc_hvm_build_x86.c
>> @@ -628,14 +628,40 @@ int xc_hvm_build_target_mem(xc_interface *xch,
>>                              int target,
>>                              const char *image_name)
>>   {
>> +    return xc_hvm_build_target_mem_with_hole(xch, domid, memsize, 
>> target,
>> +                                             image_name, 0);
>> +}
>> +
>> +int xc_hvm_build_with_hole(xc_interface *xch, uint32_t domid,
>> +                           struct xc_hvm_build_args *args,
>> +                           uint64_t mmio_hole_size)
>> +{
>> +    if (mmio_hole_size)
>> +    {
>> +        uint64_t max_ram_below_4g = (1ULL << 32) - mmio_hole_size;
>> +
>> +        if (max_ram_below_4g < HVM_BELOW_4G_MMIO_START)
>
> "<=", to be consistent with the check in 
> libxl__domain_create_info_setdefault ().

Ok.


>
>> +        {
>> +            args->mmio_size = mmio_hole_size;
>> +        }
>> +    }
>> +    return xc_hvm_build(xch, domid, args);
>> +}

...

>> +        if (info->u.hvm.mmio_hole_size) {
>> +            uint64_t max_ram_below_4g =
>> +                (1ULL << 32) - info->u.hvm.mmio_hole_size;
>> +
>> +            if (max_ram_below_4g <= HVM_BELOW_4G_MMIO_START) {
>
> You seem to be making various checks on hole size in multiple places 
> --- here, in libxl__build_device_model_args_new() and in 
> parse_config_data(). (And, to some degree, in 
> xc_hvm_build_with_hole()). Are they all necessary?
>

No, but helpful.

    -Don Slutz

> -boris
>

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.