Re: [RFC PATCH v2 1/1] Add pci_hole_min_size

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 03/12/2014 01:15 PM, Don Slutz wrote:
> On 03/12/14 11:29, Boris Ostrovsky wrote:
>> On 03/12/2014 11:07 AM, Slutz, Donald Christopher wrote:
>>>
>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>> index 5c06dfa..72842aa 100644
>>>> --- a/tools/libxl/libxl_dm.c
>>>> +++ b/tools/libxl/libxl_dm.c
>>>> @@ -656,6 +656,21 @@ static char ** 
>>>> libxl__build_device_model_args_new(libxl__gc *gc,
>>>>            } else {
>>>>                flexarray_append(dm_args, "xenfv");
>>>>            }
>>>> +        if (b_info->u.hvm.pci_hole_min_size) {
>>>> +            unsigned long long max_ram_below_4g = (1ULL << 32) -
>>>> +                b_info->u.hvm.pci_hole_min_size;
>>>> +
>>>> +            if (max_ram_below_4g > 0xF0000000ULL)
>>> Is this '>' or '<'?
>>>
>>> '>' is right.  This is the current value (I had issues getting the 
>>> inculde file that defined this and so hard coded it.)
>>
>>
>> Then pci_hole_min_size is too small, not too big, isn't it?
>>
>> -boris
>>
>
> No, but in unsigned, 4G - pci_hole_min_size will never be negative. 
> This currently only handles pci_hole_min_size that are > 4G. Note: 4G 
> is the same as 0.  What to do about 3.75G to 4.0G is not clear.  At 
> one time I had code that limited max_ram_below_4g to be either 0 or >= 
> 16M (real mode max) in addition, but was not 100% sure it is needed. 
> (Or in pci_hole_min_size terms either 0 or < 16M.)  16M has some 
> goodness in that seabios (and grub) can run, but linux does not like 
> the memory layout.  Below 16M seabios (and hvmloader, etc) most likely 
> will not be able to run.

I didn't realize this was guarding for pci_hole_min_size > 4G, I was 
thinking that if, say, pci_hole_min_size is 128M then you will get the 
warning that it is too big, which is presumably not what you want to see.

-boris


>
>    -Don Slutz
>
>>
>>>
>>>> +            {
>>>> +                LIBXL__LOG(ctx, LIBXL__LOG_WARNING,
>>>> +                           "pci_hole_min_size too big => 
>>>> max_ram_below_4g=%llu > %llu (new adjusted value)\n",
>>>> +                           max_ram_below_4g, 0xF0000000ULL);
>>>> +                max_ram_below_4g = 0xF0000000ULL;
>>> Do you need to adjust pci_hole_min_size as well?
>>>
>>> The limiting in hvmloader/pci.c looks to be missing.  I think that 
>>> the auto correction of bad values needs to be done where they are used.
>>>
>>> Will add more in next version.
>>>
>>>      -Don Slutz
>>>
>>>
>>> -boris
>>>
>>>> +            }
>>>> +            flexarray_append_pair(dm_args, "-global",
>>>> + GCSPRINTF("pc-memory-layout.max-ram-below-4g=%llu",
>>>> +                                     max_ram_below_4g));
>>>> +        }
>>>>            for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != 
>>>> NULL; i++)
>>>>                flexarray_append(dm_args, b_info->extra_hvm[i]);
>>>>            break;
>>>>
>>
>