From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [RFC PATCH 1/1] Add pci_hole_min_size Date: Mon, 03 Mar 2014 10:30:31 -0500 Message-ID: <5314A017.6030004@terremark.com> References: <1393618535-9587-1-git-send-email-dslutz@verizon.com> <1393618535-9587-2-git-send-email-dslutz@verizon.com> <531108AE.9020809@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <531108AE.9020809@oracle.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: Boris Ostrovsky , Don Slutz Cc: Stefano Stabellini , Ian Jackson , Ian Campbell , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 02/28/14 17:07, Boris Ostrovsky wrote: > On 02/28/2014 03:15 PM, Don Slutz wrote: >> This allows growing the pci_hole to the size needed. >> >> Signed-off-by: Don Slutz >> --- >> tools/firmware/hvmloader/pci.c | 10 ++++++++++ >> tools/libxc/xc_hvm_build_x86.c | 22 ++++++++++++++++++++++ >> tools/libxc/xenguest.h | 11 +++++++++++ >> tools/libxl/libxl_create.c | 4 +++- >> tools/libxl/libxl_dm.c | 13 +++++++++++-- >> tools/libxl/libxl_dom.c | 3 ++- >> tools/libxl/libxl_types.idl | 1 + >> tools/libxl/xl_cmdimpl.c | 6 ++++++ >> 8 files changed, 66 insertions(+), 4 deletions(-) >> >> diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c >> index 627e8cb..b36d00b 100644 >> --- a/tools/firmware/hvmloader/pci.c >> +++ b/tools/firmware/hvmloader/pci.c >> @@ -58,6 +58,7 @@ void pci_setup(void) >> uint64_t bar_sz; >> } *bars = (struct bars *)scratch_start; >> unsigned int i, nr_bars = 0; >> + uint64_t pci_hole_min_size; >> const char *s; >> /* >> @@ -85,6 +86,9 @@ void pci_setup(void) >> printf("Relocating guest memory for lowmem MMIO space %s\n", >> allow_memory_relocate?"enabled":"disabled"); >> + s = xenstore_read("platform/pci_hole_min_size", "0"); > > 'if (s)' > Ok, will switch to: s = xenstore_read("platform/pci_hole_min_size", NULL); if ( s ) >> + pci_hole_min_size = strtoll(s, NULL, 0); >> + >> /* Program PCI-ISA bridge with appropriate link routes. */ >> isa_irq = 0; >> for ( link = 0; link < 4; link++ ) >> @@ -236,6 +240,12 @@ void pci_setup(void) >> pci_writew(devfn, PCI_COMMAND, cmd); >> } >> + if (pci_hole_min_size) { >> + pci_mem_start = (1ull << 32) - pci_hole_min_size; >> + printf("pci_mem_start=0x%lx (was 0x%x) for pci_hole_min_size=%llu\n", >> + pci_mem_start, PCI_MEM_START, pci_hole_min_size); >> + } >> + >> /* >> * 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, > > ... > >> 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. > ... > > >> 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? -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. " >