From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [RFC PATCH v2 1/1] Add pci_hole_min_size Date: Wed, 12 Mar 2014 14:50:18 -0400 Message-ID: <5320AC6A.1000209@oracle.com> References: <1394566069-26799-1-git-send-email-dslutz@verizon.com> <1394566069-26799-2-git-send-email-dslutz@verizon.com>, <531F69EB.7080803@oracle.com> <53207D71.8040500@oracle.com> <53209636.3050301@terremark.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53209636.3050301@terremark.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Don Slutz Cc: Stefano Stabellini , Ian Jackson , Ian Campbell , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org 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; >>>> >> >