From patchwork Wed Oct 23 13:00:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Jackson X-Patchwork-Id: 11206749 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 89E7D1515 for ; Wed, 23 Oct 2019 13:02:00 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5A64E21872 for ; Wed, 23 Oct 2019 13:02:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=citrix.com header.i=@citrix.com header.b="d+tC7sIv" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5A64E21872 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=eu.citrix.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iNGFe-0005wI-N6; Wed, 23 Oct 2019 13:00:34 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iNGFd-0005vX-0F for xen-devel@lists.xenproject.org; Wed, 23 Oct 2019 13:00:33 +0000 X-Inumbo-ID: 1385abea-f595-11e9-a531-bc764e2007e4 Received: from esa1.hc3370-68.iphmx.com (unknown [216.71.145.142]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 1385abea-f595-11e9-a531-bc764e2007e4; Wed, 23 Oct 2019 13:00:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1571835624; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version; bh=UK8D8SXQlm+rrYSLRdhzAFRqrVgJXLVpfsEELklIuFA=; b=d+tC7sIvWZKXtvOPScAO9MuJ9yrFi24BQqyhe6kBuaMEnVGaEeiJBHR6 6o9KE5QYezluPabUq74X5ig3mCrPFHdndx5Gz1ZG2bOj2CSbQpkS5GFBt 58D+eyI1Pakwz1XI3rUmoeDqF26l24wTXozMkAO7nJ2LDrJeelXZNpEPz c=; Authentication-Results: esa1.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=ian.jackson@eu.citrix.com; spf=Pass smtp.mailfrom=Ian.Jackson@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: None (esa1.hc3370-68.iphmx.com: no sender authenticity information available from domain of ian.jackson@eu.citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="Ian.Jackson@citrix.com"; x-sender="ian.jackson@eu.citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa1.hc3370-68.iphmx.com: domain of Ian.Jackson@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="Ian.Jackson@citrix.com"; x-sender="Ian.Jackson@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ip4:168.245.78.127 ~all" Received-SPF: None (esa1.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="Ian.Jackson@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: gJafkAAsC8F8DsrrFFaB1NXm2RjVAasWfshpPrMYM70Bd+1HwUenIo9Ac5B16LyATnJUr5/Nmc C7hwA/J/XWZ+qBF8sgyEfFXNWYbvifhTo7OvBvSNuH/+ZZ/nWmdlJ5hcaHsZz5LWrAvsIHP3lg 0ustwvYQMVSxWPSQmvf+lu7JMk19nd9EzLxBXti93ArA0Vz4+c9ACq5YrTYp1M5mPortUoPalM REPr1WSmjXC+ftEYzRG/eSSN6UswPRZi3UPEkL37r4tG39xY4gPK/cid/MfaaqaUlwhQEoLAOL qYc= X-SBRS: 2.7 X-MesageID: 7411222 X-Ironport-Server: esa1.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.68,220,1569297600"; d="scan'208";a="7411222" From: Ian Jackson To: Date: Wed, 23 Oct 2019 14:00:07 +0100 Message-ID: <20191023130013.32382-6-ian.jackson@eu.citrix.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20191023130013.32382-1-ian.jackson@eu.citrix.com> References: <20191023130013.32382-1-ian.jackson@eu.citrix.com> MIME-Version: 1.0 Subject: [Xen-devel] [XEN PATCH for-4.13 v7 05/11] libxl: Move shadow_memkb and iommu_memkb defaulting into libxl X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Anthony PERARD , =?utf-8?b?SsO8cmdlbiBHcm8=?= =?utf-8?b?w58=?= , Ian Jackson , Wei Liu Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" Defaulting is supposed to be done by libxl. So these calculations should be here in libxl. libxl__domain_config_setdefault has all the necessary information including the values of max_memkb and max_vcpus. The overall functional effect depends on the caller: For xl, no change. The code moves from xl to libxl. For callers who set one or both shadow_memkb and iommu_memkb (whether from libxl_get_required_shadow_memory or otherwise) before calling libxl_domain_need_memory (any version): the new code will leave their setting(s) unchanged. For callers who do not call libxl_domain_need_memory at all, and who fail to set one of these memory values: now they are both are properly set. The shadow and iommu memory to be properly accounted for as intended. For callers which call libxl_domain_need_memory and request the current API (4.13) or which track libxl, the default values are also now right and everything works as intended. For callers which call libxl_domain_need_memory, and request an old pre-4.13 libxl API, and which leave one of these memkb settings unset, we take special measures to preserve the old behaviour. This means that they don't get the additional iommu memory and are at risk of the domain running out of memory as a result of f89f555827a6 "remove late (on-demand) construction of IOMMU page tables". But this is no worse than the state just after f89f555827a6, which already broke such callers in that way. This is perhaps justifiable because of the API stability warning next to libxl_domain_need_memory. An alternative would be to drop the special-casing of these callers. That would cause a discrepancy between libxl_domain_need_memory and libxl_domain_create: the former would not include the iommu memory and the latter would. That seems worse, but it's debateable. Signed-off-by: Ian Jackson Reviewed-by: Anthony PERARD --- v4: Provide a fallback default for iommu_memkb too, for old callers. v2: Replace _Bool with bool Fix logic sense in ok_to_default_memkb_in_create --- tools/libxl/libxl_create.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ tools/libxl/libxl_internal.h | 3 +++ tools/libxl/libxl_mem.c | 4 ++++ tools/xl/xl_parse.c | 15 ++------------- 4 files changed, 53 insertions(+), 13 deletions(-) diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index fd8bb22be9..3b3b6ec381 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -247,9 +247,14 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, libxl__arch_domain_build_info_setdefault(gc, b_info); libxl_defbool_setdefault(&b_info->dm_restrict, false); + if (b_info->iommu_memkb == LIBXL_MEMKB_DEFAULT) + /* Normally defaulted in libxl__domain_create_info_setdefault */ + b_info->iommu_memkb = 0; + switch (b_info->type) { case LIBXL_DOMAIN_TYPE_HVM: if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT) + /* Normally defaulted in libxl__domain_create_info_setdefault */ b_info->shadow_memkb = 0; if (b_info->u.hvm.mmio_hole_memkb == LIBXL_MEMKB_DEFAULT) b_info->u.hvm.mmio_hole_memkb = 0; @@ -395,6 +400,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) b_info->video_memkb = 0; if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT) + /* Normally defaulted in libxl__domain_create_info_setdefault */ b_info->shadow_memkb = 0; if (b_info->u.pv.slack_memkb == LIBXL_MEMKB_DEFAULT) b_info->u.pv.slack_memkb = 0; @@ -862,6 +868,30 @@ static void domcreate_destruction_cb(libxl__egc *egc, libxl__domain_destroy_state *dds, int rc); +static bool ok_to_default_memkb_in_create(libxl__gc *gc) +{ + /* + * This is a fudge. We are trying to find whether the caller + * calls the old version of libxl_domain_need_memory. If they do + * then, because it only gets the b_info, and because it can't + * update the b_info (because it's const), it will base its + * calculations on defaulting shadow_memkb and iommu_memkb to 0 + * In that case we probably shouldn't default them differently + * during libxl_domain_create. + * + * The result is that the behaviour with old callers is the same + * as in 4.13: no additional memory is allocated for shadow and + * iommu (unless the caller set shadow_memkb, eg from a call to + * libxl_get_required_shadow_memory). + */ + return !CTX->libxl_domain_need_memory_0x041200_called || + CTX->libxl_domain_need_memory_called; + /* + * Treat mixed callers as new callers. Presumably they know what + * they are doing. + */ +} + int libxl__domain_config_setdefault(libxl__gc *gc, libxl_domain_config *d_config, uint32_t domid) @@ -974,6 +1004,20 @@ int libxl__domain_config_setdefault(libxl__gc *gc, goto error_out; } + if (d_config->b_info.shadow_memkb == LIBXL_MEMKB_DEFAULT + && ok_to_default_memkb_in_create(gc)) + d_config->b_info.shadow_memkb = + libxl_get_required_shadow_memory(d_config->b_info.max_memkb, + d_config->b_info.max_vcpus); + + /* No IOMMU reservation is needed if passthrough mode is not 'sync_pt' */ + if (d_config->b_info.iommu_memkb == LIBXL_MEMKB_DEFAULT + && ok_to_default_memkb_in_create(gc)) + d_config->b_info.iommu_memkb = + (d_config->c_info.passthrough == LIBXL_PASSTHROUGH_SYNC_PT) + ? libxl_get_required_iommu_memory(d_config->b_info.max_memkb) + : 0; + ret = libxl__domain_build_info_setdefault(gc, &d_config->b_info); if (ret) { LOGD(ERROR, domid, "Unable to set domain build info defaults"); diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 01de5576d9..0185b8ff01 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -637,6 +637,9 @@ struct libxl__ctx { LIBXL_LIST_ENTRY(libxl_ctx) sigchld_users_entry; libxl_version_info version_info; + + bool libxl_domain_need_memory_0x041200_called, + libxl_domain_need_memory_called; }; /* diff --git a/tools/libxl/libxl_mem.c b/tools/libxl/libxl_mem.c index 6042299393..7c01fac7e5 100644 --- a/tools/libxl/libxl_mem.c +++ b/tools/libxl/libxl_mem.c @@ -484,6 +484,8 @@ int libxl_domain_need_memory(libxl_ctx *ctx, GC_INIT(ctx); int rc; + ctx->libxl_domain_need_memory_called = 1; + rc = libxl__domain_config_setdefault(gc, d_config, domid_for_logging); @@ -507,6 +509,8 @@ int libxl_domain_need_memory_0x041200(libxl_ctx *ctx, GC_INIT(ctx); int rc; + ctx->libxl_domain_need_memory_0x041200_called = 1; + libxl_domain_build_info b_info[1]; libxl_domain_build_info_init(b_info); libxl_domain_build_info_copy(ctx, b_info, b_info_in); diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 03a2c54dd2..79871c22d0 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -1572,19 +1572,8 @@ void parse_config_data(const char *config_source, exit(-ERROR_FAIL); } - /* libxl_get_required_shadow_memory() and - * libxl_get_required_iommu_memory() must be called after final values - * (default or specified) for vcpus and memory are set, because the - * calculations depend on those values. */ - b_info->shadow_memkb = !xlu_cfg_get_long(config, "shadow_memory", &l, 0) - ? l * 1024 - : libxl_get_required_shadow_memory(b_info->max_memkb, - b_info->max_vcpus); - - /* No IOMMU reservation is needed if passthrough mode is not 'sync_pt' */ - b_info->iommu_memkb = (c_info->passthrough == LIBXL_PASSTHROUGH_SYNC_PT) - ? libxl_get_required_iommu_memory(b_info->max_memkb) - : 0; + if (!xlu_cfg_get_long(config, "shadow_memory", &l, 0)) + b_info->shadow_memkb = l * 1024; xlu_cfg_get_defbool(config, "nomigrate", &b_info->disable_migrate, 0);