From patchwork Mon Oct 14 16:51:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Jackson X-Patchwork-Id: 11189153 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 A4E8B14DB for ; Mon, 14 Oct 2019 16:52:35 +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 74F962067B for ; Mon, 14 Oct 2019 16:52:35 +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="IvaGFhac" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 74F962067B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=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 1iK3Yu-0006o2-Us; Mon, 14 Oct 2019 16:51:12 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iK3Yt-0006nv-3a for xen-devel@lists.xenproject.org; Mon, 14 Oct 2019 16:51:11 +0000 X-Inumbo-ID: d11b9c9b-eea2-11e9-9383-12813bfff9fa Received: from esa1.hc3370-68.iphmx.com (unknown [216.71.145.142]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id d11b9c9b-eea2-11e9-9383-12813bfff9fa; Mon, 14 Oct 2019 16:51:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1571071869; h=from:mime-version:content-transfer-encoding:message-id: date:to:subject:in-reply-to:references; bh=5OFhLxvNyYkBkvvTMxbuO0FyZGRT3BW5gbpDbimC5Wk=; b=IvaGFhac29alGN7/oyAaDnuqnbyUi31I+NnwWezcfpcpaW7c+pi3j/rU 0tSMCfC1CTcW+SJ3fAgEGPYxqZIaOho5qPlfbOdLOdY3iExH3eVLSnHjj HJ7VOgEaS7vkPwBg3VlF+sItxwSUo+K5WqIqVrRsc0AZhaxasYPOwFFZU I=; Authentication-Results: esa1.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=ian.jackson@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@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@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: cviQdfQ9dNBlCUNhtN7yXe5jdbp/2vZYsmuWKBD/aNxplqmCVJK9VwK9MvW0qQTuNGoI4PkhHe MZzFRa3MObigypoFpKm6ByjTXgy3I1kOv7OTpXQCzZW5NQ3FMjPxtUK5hSnsXRXmDr58JOxaoS EFL5hpctSNYPflE1JBf3ZPRJ9Cudzu9fofLP5heuJb7yNccSemRo8rFmXCuZAE/DIMhBKuHZYd rSm8vh2LsAj9jatAEYSmSvPGYwQfqK2Va5ZD5rZJK6OGirRRo9VRRSkL8P3xNcRNUsnLYnNn5A +aA= X-SBRS: 2.7 X-MesageID: 6941076 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.67,296,1566878400"; d="scan'208";a="6941076" From: Ian Jackson MIME-Version: 1.0 Message-ID: <23972.42872.768049.55036@mariner.uk.xensource.com> Date: Mon, 14 Oct 2019 17:51:04 +0100 To: Wei Liu , Paul Durrant , =?iso-8859-1?q?J=FCrgen_Gro=DF?= , George Dunlap , Andrew Cooper , Jan Beulich , Anthony Perard , xen-devel In-Reply-To: <23972.42710.205681.915082@mariner.uk.xensource.com> References: <20191010151111.22125-1-ian.jackson@eu.citrix.com> <20191010151111.22125-10-ian.jackson@eu.citrix.com> <23968.33869.906302.573866@mariner.uk.xensource.com> <33aee478-52b9-df46-67d7-f81702e5e0f9@suse.com> <23968.44828.940652.851219@mariner.uk.xensource.com> <23972.40372.377662.700932@mariner.uk.xensource.com> <20191014164429.ltk64qeoo2ckzwet@debian> <23972.42710.205681.915082@mariner.uk.xensource.com> X-Mailer: VM 8.2.0b under 24.5.1 (i686-pc-linux-gnu) Subject: [Xen-devel] [XEN PATCH v4 for-4.13 10/10] libxl/xl: Overhaul passthrough setting logic passthrough setting logic 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: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" Here's what I hope is the final version... From b60195e857d3699eaa55d9174a69bf91902cddb5 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 7 Oct 2019 17:59:15 +0100 Subject: [XEN PATCH v4 for-4.13 10/10] libxl/xl: Overhaul passthrough setting logic LIBXL_PASSTHROUGH_UNKNOWN (aka "ENABLED" in an earlier uncommitted version of this code) is doing double duty. We actually need all of the following to be specificable: * default ("unknown"): enable PT iff we have devices to pass through specified in the initial config file. * "enabled" (and fail if the platform doesn't support it). * "disabled" (and reject future PT hotplug). * "share_pt"/"sync_pt": enable PT and set a specific PT mode. Defaulting and error checking should be done in libxl. So, we make several changes here. We introduce "enabled". (And we document "unknown".) We move all of the error checking and defaulting code from xl into libxl. Now, libxl__domain_config_setdefault has all of the necessary information to get this right. So we can do it all there. Choosing the specific mode is arch-specific. We can also arrange to have only one place each which calculates (i) whether passthrough needs to be enabled because pt devices were specified (ii) whether pt_share can be used (for each arch). xl now only has to parse the enum in the same way as it parses all other enums. This change fixes a regression from earlier 4.13-pre: until recent changes, passthrough was only enabled by default if passthrough devices were specified. We restore this behaviour. Signed-off-by: Ian Jackson CC: Stefano Stabellini CC: Julien Grall CC: Volodymyr Babchuk CC: Andrew Cooper CC: Paul Durrant CC: Jan Beulich --- v4: Fix trailing whitespace No longer change "unknown" to "unspecified". v3: Drop paragraph about masking another osstest regression, as that's now fixed. Drop redundant "ERROR:" in two log messages. Add a comment about the way "enabled" gets changed to a specific value. Split passthrough mode defaulting into arch specific functions. On ARM, always choose (and insist on) share_pt. Reject share_pt for non-HAP guests. Reject passthrough for PVH guests. Actually document "unspecified" option in xl.cfg(5) Rename "unknown" to "unspecified" v2: New patch in this version of the series. --- docs/man/xl.cfg.5.pod.in | 6 ++++ tools/libxl/libxl_arch.h | 6 ++++ tools/libxl/libxl_arm.c | 24 ++++++++++++++++ tools/libxl/libxl_create.c | 41 +++++++++++++++++++-------- tools/libxl/libxl_types.idl | 5 ++-- tools/libxl/libxl_x86.c | 41 +++++++++++++++++++++++++++ tools/xl/xl_parse.c | 67 ++++----------------------------------------- 7 files changed, 114 insertions(+), 76 deletions(-) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 64bed30bce..7e220d0c20 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -660,6 +660,12 @@ in preference. However, the availability of this option is hardware specific. If B reports B containing B then this option may be used. +=item B + +The default, which chooses between B and B +according to whether passthrough devices are enabled in the config +file. + =back =back diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h index d624159e53..ee6641b3e6 100644 --- a/tools/libxl/libxl_arch.h +++ b/tools/libxl/libxl_arch.h @@ -73,6 +73,12 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, libxl_domain_build_info *b_info); _hidden +int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc, + uint32_t domid, + libxl_domain_config *d_config, + const libxl_physinfo *physinfo); + +_hidden int libxl__arch_extra_memory(libxl__gc *gc, const libxl_domain_build_info *info, uint64_t *out); diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c index bf31b9b3ca..2f1ca69431 100644 --- a/tools/libxl/libxl_arm.c +++ b/tools/libxl/libxl_arm.c @@ -1191,6 +1191,30 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH); } +int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc, + uint32_t domid, + libxl_domain_config *d_config, + const libxl_physinfo *physinfo) +{ + int rc; + libxl_domain_create_info *const c_info = &d_config->c_info; + + if (c_info->passthrough == LIBXL_PASSTHROUGH_ENABLED) { + c_info->passthrough = LIBXL_PASSTHROUGH_SHARE_PT; + } + + if (c_info->passthrough == LIBXL_PASSTHROUGH_SYNC_PT) { + LOGD(ERROR, domid, + "passthrough=\"sync_pt\" not supported on ARM\n"); + rc = ERROR_INVAL; + goto out; + } + + rc = 0; + out: + return rc; +} + /* * Local variables: * mode: C diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 3d2cb52bf7..7423bedf7d 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -57,18 +57,6 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc, if (!c_info->ssidref) c_info->ssidref = SECINITSID_DOMU; - if (info->cap_hvm_directio && - (c_info->passthrough == LIBXL_PASSTHROUGH_UNKNOWN)) { - c_info->passthrough = ((c_info->type == LIBXL_DOMAIN_TYPE_PV) || - !info->cap_iommu_hap_pt_share) ? - LIBXL_PASSTHROUGH_SYNC_PT : LIBXL_PASSTHROUGH_SHARE_PT; - } else if (!info->cap_hvm_directio) { - c_info->passthrough = LIBXL_PASSTHROUGH_DISABLED; - } - - /* An explicit setting should now have been chosen */ - assert(c_info->passthrough != LIBXL_PASSTHROUGH_UNKNOWN); - return 0; } @@ -904,6 +892,7 @@ int libxl__domain_config_setdefault(libxl__gc *gc, libxl_ctx *ctx = libxl__gc_owner(gc); int ret; bool pod_enabled = false; + libxl_domain_create_info *c_info = &d_config->c_info; libxl_physinfo physinfo; ret = libxl_get_physinfo(CTX, &physinfo); @@ -975,6 +964,34 @@ int libxl__domain_config_setdefault(libxl__gc *gc, goto error_out; } + bool need_pt = d_config->num_pcidevs || d_config->num_dtdevs; + if (c_info->passthrough == LIBXL_PASSTHROUGH_UNKNOWN) { + c_info->passthrough = need_pt + ? LIBXL_PASSTHROUGH_ENABLED : LIBXL_PASSTHROUGH_DISABLED; + } + + bool iommu_enabled = physinfo.cap_hvm_directio; + if (c_info->passthrough != LIBXL_PASSTHROUGH_DISABLED && !iommu_enabled) { + LOGD(ERROR, domid, + "passthrough not supported on this platform\n"); + ret = ERROR_INVAL; + goto error_out; + } + + if (c_info->passthrough == LIBXL_PASSTHROUGH_DISABLED && need_pt) { + LOGD(ERROR, domid, + "passthrough disabled but devices are specified"); + ret = ERROR_INVAL; + goto error_out; + } + + ret = libxl__arch_passthrough_mode_setdefault(gc,domid,d_config,&physinfo); + if (ret) goto error_out; + + /* An explicit setting should now have been chosen */ + assert(c_info->passthrough != LIBXL_PASSTHROUGH_UNKNOWN); + assert(c_info->passthrough != LIBXL_PASSTHROUGH_ENABLED); + /* If target_memkb is smaller than max_memkb, the subsequent call * to libxc when building HVM domain will enable PoD mode. */ diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 3ac9494b80..ca47db2961 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -266,8 +266,9 @@ libxl_vkb_backend = Enumeration("vkb_backend", [ libxl_passthrough = Enumeration("passthrough", [ (0, "unknown"), (1, "disabled"), - (2, "sync_pt"), - (3, "share_pt"), + (2, "enabled"), # becomes {sync,share}_pt once defaults are evaluated + (3, "sync_pt"), + (4, "share_pt"), ]) # diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c index c0f88a7eaa..8b804537ba 100644 --- a/tools/libxl/libxl_x86.c +++ b/tools/libxl/libxl_x86.c @@ -631,6 +631,47 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc *gc, libxl_defbool_setdefault(&b_info->acpi, true); } +int libxl__arch_passthrough_mode_setdefault(libxl__gc *gc, + uint32_t domid, + libxl_domain_config *d_config, + const libxl_physinfo *physinfo) +{ + int rc; + libxl_domain_create_info *const c_info = &d_config->c_info; + + if (c_info->passthrough != LIBXL_PASSTHROUGH_DISABLED && + c_info->type == LIBXL_DOMAIN_TYPE_PVH) { + LOGD(ERROR, domid, + "passthrough not yet supported for x86 PVH guests\n"); + rc = ERROR_INVAL; + goto out; + } + + const char *whynot_pt_share = + c_info->type == LIBXL_DOMAIN_TYPE_PV ? "not valid for PV domain" : + !physinfo->cap_iommu_hap_pt_share ? "not supported on this platform" : + !libxl_defbool_val(d_config->c_info.hap) ?"only valid for HAP guests": + NULL; + + if (c_info->passthrough == LIBXL_PASSTHROUGH_ENABLED) { + c_info->passthrough = whynot_pt_share + ? LIBXL_PASSTHROUGH_SYNC_PT : LIBXL_PASSTHROUGH_SHARE_PT; + } + + if (c_info->passthrough == LIBXL_PASSTHROUGH_SHARE_PT && whynot_pt_share) { + LOGD(ERROR, domid, + "passthrough=\"share_pt\" %s\n", + whynot_pt_share); + rc = ERROR_INVAL; + goto out; + } + + rc = 0; + out: + return rc; +} + + /* * Local variables: * mode: C diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 79871c22d0..112f8ee026 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -1222,7 +1222,6 @@ void parse_config_data(const char *config_source, int pci_seize = 0; int i, e; char *kernel_basename; - bool iommu_enabled, iommu_hap_pt_share; libxl_domain_create_info *c_info = &d_config->c_info; libxl_domain_build_info *b_info = &d_config->b_info; @@ -1234,8 +1233,6 @@ void parse_config_data(const char *config_source, exit(EXIT_FAILURE); } - iommu_enabled = physinfo.cap_hvm_directio; - iommu_hap_pt_share = physinfo.cap_iommu_hap_pt_share; libxl_physinfo_dispose(&physinfo); config= xlu_cfg_init(stderr, config_source); @@ -1509,67 +1506,13 @@ void parse_config_data(const char *config_source, } } - if (xlu_cfg_get_string(config, "passthrough", &buf, 0)) { - c_info->passthrough = - (d_config->num_pcidevs || d_config->num_dtdevs) - ? LIBXL_PASSTHROUGH_UNKNOWN : LIBXL_PASSTHROUGH_DISABLED; - } else { - if (!strcasecmp("enabled", buf)) - c_info->passthrough = LIBXL_PASSTHROUGH_UNKNOWN; - else { - libxl_passthrough o; - - e = libxl_passthrough_from_string(buf, &o); - if (e || !strcasecmp("unknown", buf)) { - fprintf(stderr, - "ERROR: unknown passthrough option '%s'\n", - buf); - exit(-ERROR_FAIL); - } - - c_info->passthrough = o; - } - } - - switch (c_info->passthrough) { - case LIBXL_PASSTHROUGH_UNKNOWN: - /* - * Choose a suitable default. libxl would also do this but - * choosing here allows the code calculating 'iommu_memkb' - * below make an informed decision. - */ - c_info->passthrough = - (c_info->type == LIBXL_DOMAIN_TYPE_PV) || !iommu_hap_pt_share - ? LIBXL_PASSTHROUGH_SYNC_PT : LIBXL_PASSTHROUGH_SHARE_PT; - break; - - case LIBXL_PASSTHROUGH_DISABLED: - if (d_config->num_pcidevs || d_config->num_dtdevs) { + if (!xlu_cfg_get_string(config, "passthrough", &buf, 0)) { + if (libxl_passthrough_from_string(buf, &c_info->passthrough)) { fprintf(stderr, - "ERROR: passthrough disabled but devices are specified\n"); - exit(-ERROR_FAIL); - } - break; - case LIBXL_PASSTHROUGH_SHARE_PT: - if (c_info->type == LIBXL_DOMAIN_TYPE_PV) { - fprintf(stderr, - "ERROR: passthrough=\"share_pt\" not valid for PV domain\n"); - exit(-ERROR_FAIL); - } else if (!iommu_hap_pt_share) { - fprintf(stderr, - "ERROR: passthrough=\"share_pt\" not supported on this platform\n"); - exit(-ERROR_FAIL); + "ERROR: unknown passthrough option '%s'\n", + buf); + exit(1); } - break; - case LIBXL_PASSTHROUGH_SYNC_PT: - break; - } - - if ((c_info->passthrough != LIBXL_PASSTHROUGH_DISABLED) && - !iommu_enabled) { - fprintf(stderr, - "ERROR: passthrough not supported on this platform\n"); - exit(-ERROR_FAIL); } if (!xlu_cfg_get_long(config, "shadow_memory", &l, 0))