From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [RFC PATCH 1/1] Add pci_hole_min_size Date: Mon, 03 Mar 2014 11:07:40 -0500 Message-ID: <5314A8CC.1060003@oracle.com> References: <1393618535-9587-1-git-send-email-dslutz@verizon.com> <1393618535-9587-2-git-send-email-dslutz@verizon.com> <531108AE.9020809@oracle.com> <5314A017.6030004@terremark.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5314A017.6030004@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/03/2014 10:30 AM, Don Slutz wrote: > On 02/28/14 17:07, Boris Ostrovsky wrote: >> >>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c >>> index a604cd8..24ceac6 100644 >>> --- a/tools/libxl/libxl_create.c >>> +++ b/tools/libxl/libxl_create.c >>> @@ -388,13 +388,15 @@ int libxl__domain_build(libxl__gc *gc, >>> vments[4] = "start_time"; >>> vments[5] = libxl__sprintf(gc, "%lu.%02d", >>> start_time.tv_sec,(int)start_time.tv_usec/10000); >>> - localents = libxl__calloc(gc, 7, sizeof(char *)); >>> + localents = libxl__calloc(gc, 9, sizeof(char *)); >>> localents[0] = "platform/acpi"; >>> localents[1] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : >>> "0"; >>> localents[2] = "platform/acpi_s3"; >>> localents[3] = libxl_defbool_val(info->u.hvm.acpi_s3) ? >>> "1" : "0"; >>> localents[4] = "platform/acpi_s4"; >>> localents[5] = libxl_defbool_val(info->u.hvm.acpi_s4) ? >>> "1" : "0"; >>> + localents[6] = "platform/pci_hole_min_size"; >>> + localents[7] = libxl__sprintf(gc, "%llu", (unsigned long >>> long)info->u.hvm.pci_hole_min_size); >> >> Do you want to always store this parameter? There is a default >> already (HVM_BELOW_4G_MMIO_LENGTH) so if it's not set in the config >> file it may be safe to omit it. >> > > I do not always need to store it. Since none of the rest of these are > conditional stores, I just followed them. Since this is the minimum > size, I can add a check on pci_hole_min_size > > HVM_BELOW_4G_MMIO_LENGTH here and skip the xenstore write if you want. If you decide to do this I think the better place may be in libxl__build_post(). > >> ... >> >> >>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c >>> index 4fc46eb..fe247ee 100644 >>> --- a/tools/libxl/xl_cmdimpl.c >>> +++ b/tools/libxl/xl_cmdimpl.c >>> @@ -1025,6 +1025,12 @@ static void parse_config_data(const char >>> *config_source, >>> xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0); >>> xlu_cfg_get_defbool(config, "vpt_align", >>> &b_info->u.hvm.vpt_align, 0); >>> + if (!xlu_cfg_get_long(config, "pci_hole_min_size", &l, 0)) { >>> + b_info->u.hvm.pci_hole_min_size = (uint64_t) l; >>> + if (dom_info->debug) >>> + fprintf(stderr, "pci_hole_min_size: %llu\n", >>> (unsigned long long) b_info->u.hvm.pci_hole_min_size); >>> + } >> >> You probably want to set b_info->u.hvm.pci_hole_min_size to 0 (or >> HVM_BELOW_4G_MMIO_LENGTH?) in case it's not specified in >> libxl__domain_build_info_setdefault(). > > Since 0 is a valid value, I do not think that > libxl__domain_build_info_setdefault() needs to do any thing. Should I be > specifying a default in the idl of 0? What I meant (but apparently not what I wrote) was that if config file doesn't have "pci_hole_min_size" then b_info->u.hvm.pci_hole_min_size will be left uninitialized and I don't know whether we can assume that it will be zero. -boris > > -Don Slutz >> >> -boris >> >> >>> + >>> if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) { >>> const char *s = libxl_timer_mode_to_string(l); >>> fprintf(stderr, "WARNING: specifying \"timer_mode\" as >>> an integer is deprecated. " >> >