From patchwork Thu Mar 19 11:47:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Durrant X-Patchwork-Id: 11447067 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 ECBA2913 for ; Thu, 19 Mar 2020 11:49:18 +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 C93A5206D7 for ; Thu, 19 Mar 2020 11:49:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=xen.org header.i=@xen.org header.b="g+0/xHDh" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C93A5206D7 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=xen.org Authentication-Results: mail.kernel.org; spf=pass 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 1jEteX-00015F-SM; Thu, 19 Mar 2020 11:47:57 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1jEteW-000154-9K for xen-devel@lists.xenproject.org; Thu, 19 Mar 2020 11:47:56 +0000 X-Inumbo-ID: 78e5cf5e-69d7-11ea-92cf-bc764e2007e4 Received: from mail.xenproject.org (unknown [104.130.215.37]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 78e5cf5e-69d7-11ea-92cf-bc764e2007e4; Thu, 19 Mar 2020 11:47:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=xen.org; s=20200302mail; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=KYhwuqedCy2ozcekcBLc63wNJ6xnOM+UTgit0NSbGJc=; b=g+0/xHDhrkz4eyu2fUPQz+81A9 9PQdrVF+/gXXyUcX8TObfBKxVzO2vm5QkLrwOp9z7rJiUBK6Y6g/0VqMR4gqkxbpSabSWm4YK96Um j3ppSVPSyJHGRSWkusSmdUU+651vD5n09/Znzo3YkTr/V4oieacVXdfTZ5ke1J/TLv5g=; Received: from xenbits.xenproject.org ([104.239.192.120]) by mail.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1jEteU-0007SK-U0; Thu, 19 Mar 2020 11:47:54 +0000 Received: from 54-240-197-232.amazon.com ([54.240.197.232] helo=u2f063a87eabd5f.cbg10.amazon.com) by xenbits.xenproject.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.89) (envelope-from ) id 1jEteU-0008Up-KL; Thu, 19 Mar 2020 11:47:54 +0000 From: Paul Durrant To: xen-devel@lists.xenproject.org Date: Thu, 19 Mar 2020 11:47:47 +0000 Message-Id: <20200319114748.5168-2-paul@xen.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200319114748.5168-1-paul@xen.org> References: <20200319114748.5168-1-paul@xen.org> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH v4 1/2] libxl: create domain 'error' node in xenstore 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: Stefano Stabellini , Julien Grall , Wei Liu , Paul Durrant , Andrew Cooper , Paul Durrant , Konrad Rzeszutek Wilk , Ian Jackson , George Dunlap , Jan Beulich , Anthony PERARD Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" From: Paul Durrant Several PV drivers (both historically and currently [1]) report errors by writing text into /local/domain/$DOMID/error. This patch creates the node in libxl and makes it writable by the domain, and also adds some text into xenstore-paths.pandoc to state what the node is for. [1] https://xenbits.xen.org/gitweb/?p=pvdrivers/win/xenvif.git;a=blob;f=src/xenvif/frontend.c;hb=HEAD#l459 Signed-off-by: Paul Durrant Acked-by: Ian Jackson --- Cc: Andrew Cooper Cc: George Dunlap Cc: Jan Beulich Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Wei Liu Cc: Anthony PERARD --- docs/misc/xenstore-paths.pandoc | 5 +++++ tools/libxl/libxl_create.c | 3 +++ 2 files changed, 8 insertions(+) diff --git a/docs/misc/xenstore-paths.pandoc b/docs/misc/xenstore-paths.pandoc index 0a6b36146e..e2ab5da54e 100644 --- a/docs/misc/xenstore-paths.pandoc +++ b/docs/misc/xenstore-paths.pandoc @@ -539,6 +539,11 @@ address written in one of these paths to, for example, establish a VNC session to the guest (although clearly some level of trust is placed in the value supplied by the guest in this case). +#### ~/error [w] + +A domain writable path used by some PV drivers to pass error messages +to the toolstack. + ### Paths private to the toolstack #### ~/device-model/$DOMID/state [w] diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 772344c648..e18aad43b5 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -797,6 +797,9 @@ retry_transaction: libxl__xs_mknod(gc, t, GCSPRINTF("%s/attr", dom_path), rwperm, ARRAY_SIZE(rwperm)); + libxl__xs_mknod(gc, t, + GCSPRINTF("%s/error", dom_path), + rwperm, ARRAY_SIZE(rwperm)); if (libxl_defbool_val(info->driver_domain)) { /* From patchwork Thu Mar 19 11:47:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Durrant X-Patchwork-Id: 11447065 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 45C271893 for ; Thu, 19 Mar 2020 11:49:14 +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 17209206D7 for ; Thu, 19 Mar 2020 11:49:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=xen.org header.i=@xen.org header.b="0Snvlexj" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 17209206D7 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=xen.org Authentication-Results: mail.kernel.org; spf=pass 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 1jEtec-00015c-3n; Thu, 19 Mar 2020 11:48:02 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1jEteb-00015S-2m for xen-devel@lists.xenproject.org; Thu, 19 Mar 2020 11:48:01 +0000 X-Inumbo-ID: 79cc21de-69d7-11ea-b34e-bc764e2007e4 Received: from mail.xenproject.org (unknown [104.130.215.37]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 79cc21de-69d7-11ea-b34e-bc764e2007e4; Thu, 19 Mar 2020 11:47:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=xen.org; s=20200302mail; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=tIB03LBdlAJuuETkGMVjtEbnlZkuW0DapwaVXYQucII=; b=0SnvlexjErJHYy31M7GnVAxLE1 prhUXrIaPdp3J+sgxU9jDdBM361PuXdEBrwocOb18XXH0p71Qt+h1L/R0nYoWJYHFYleNA0GbPdkq Qtipu4k7FSHSWx2oJFwZRL02oC2bst9b2npJbZoJ/I66GlfIt84oAojJFEPhGBQemCk4=; Received: from xenbits.xenproject.org ([104.239.192.120]) by mail.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1jEteW-0007SS-Li; Thu, 19 Mar 2020 11:47:56 +0000 Received: from 54-240-197-232.amazon.com ([54.240.197.232] helo=u2f063a87eabd5f.cbg10.amazon.com) by xenbits.xenproject.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.89) (envelope-from ) id 1jEteW-0008Up-CQ; Thu, 19 Mar 2020 11:47:56 +0000 From: Paul Durrant To: xen-devel@lists.xenproject.org Date: Thu, 19 Mar 2020 11:47:48 +0000 Message-Id: <20200319114748.5168-3-paul@xen.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200319114748.5168-1-paul@xen.org> References: <20200319114748.5168-1-paul@xen.org> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH v4 2/2] libxl: make creation of xenstore 'suspend event channel' node optional... 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: Stefano Stabellini , Julien Grall , Wei Liu , Paul Durrant , Andrew Cooper , Paul Durrant , Ian Jackson , George Dunlap , Jan Beulich , Anthony PERARD Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" From: Paul Durrant ... and, if it is not created, make the top level 'device' node in xenstore writable by the guest instead. The purpose and semantics of the suspend event channel node are explained in xenstore-paths.pandoc [1]. It was originally introduced in xend by commit 17636f47a474 "Teach xc_save to use event-channel-based domain suspend if available.". Note that, because, the top-level frontend 'device' node was created writable by the guest in xend, there was no need to explicitly create the 'suspend-event-channel' node as a writable node. However, libxl creates the 'device' node as read-only by the guest and so explicit creation of the 'suspend-event-channel' node is necessary to make it usable. This unfortunately has the side-effect of making some old Windows PV drivers [2] cease to function. This is because they scan the top level 'device' node, find the 'suspend' node and expect it to contain the usual sub-nodes describing a PV frontend. When this is found not to be the case, enumeration ceases and (because the 'suspend' node is observed before the 'vbd' node) no system disk is enumerated. Windows will then crash with bugcheck code 0x7B (missing system disk). This patch adds a boolean 'xend_suspend_evtchn_compat' field into libxl_create_info and a similarly named option in xl.cfg to set it. If the value is true then the xenstore node is not created. Instead the old xend behaviour of making top level device node writable by the guest is re-instated. If the value is false (the default) then the current libxl behaviour persists. xenstore-paths.pandoc is also modified to say that the suspend event channel node may not exist and, if it does not exist, then the guest may create it. A note is also added concerning the writability of the top level device node. [1] https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/xenstore-paths.pandoc;hb=HEAD#l177 [2] https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/5/html/para-virtualized_windows_drivers_guide/sect-para-virtualized_windows_drivers_guide-installing_and_configuring_the_para_virtualized_drivers-installing_the_para_virtualized_drivers NOTE: While adding the new LIBXL_HAVE_CREATEINFO_... definition into libxl.h, this patch corrects the previous stanza which erroneously implies libxl_domain_create_info is a function. Signed-off-by: Paul Durrant Reviewed-by: Ian Jackson --- Cc: Ian Jackson Cc: Wei Liu Cc: Andrew Cooper Cc: George Dunlap Cc: Jan Beulich Cc: Julien Grall Cc: Stefano Stabellini Cc: Anthony PERARD v4: - Change the option name to 'xend_suspend_evtchn_compat' and use it to also gate guest write permission to the top level device node v3: - Actually define LIBXL_HAVE_CREATEINFO_SUSPEND_EVENT_CHANNEL as well as commenting on it v2: - Update xenstore-paths.pandoc and squash patch #3 --- docs/man/xl.cfg.5.pod.in | 13 +++++++++++++ docs/misc/xenstore-paths.pandoc | 12 ++++++++---- tools/libxl/libxl.h | 11 ++++++++++- tools/libxl/libxl_create.c | 24 ++++++++++++++++++------ tools/libxl/libxl_types.idl | 1 + tools/xl/xl_parse.c | 3 +++ 6 files changed, 53 insertions(+), 11 deletions(-) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 0cad561375..0e9e58a41a 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -668,6 +668,19 @@ file. =back +=item B + +If this option is B the xenstore path for the domain's suspend +event channel will not be created. Instead the old xend behaviour of +making the whole xenstore B sub-tree writable by the domain will +be re-instated. + +The existence of the suspend event channel path can cause problems with +certain PV drivers running in the guest (e.g. old Red Hat PV drivers for +Windows). + +If this option is not specified then it will default to B. + =back =head2 Devices diff --git a/docs/misc/xenstore-paths.pandoc b/docs/misc/xenstore-paths.pandoc index e2ab5da54e..ff3ca04069 100644 --- a/docs/misc/xenstore-paths.pandoc +++ b/docs/misc/xenstore-paths.pandoc @@ -176,10 +176,12 @@ The size of the video RAM this domain is configured with. #### ~/device/suspend/event-channel = ""|EVTCHN [w] -The domain's suspend event channel. The toolstack will create this -path with an empty value which the guest may choose to overwrite. +The domain's suspend event channel. The toolstack may create this +path with an empty value which the guest may choose to overwrite. If +the path does not exist then the ~/device path will be writable by the +guest and hence it may create the suspend event channel path. -If the guest overwrites this, it will be with the number of an unbound +If the guest writes this, it will be with the number of an unbound event channel port it has acquired. The toolstack is expected to use an interdomain bind, and then, when it wishes to ask the guest to suspend, to signal the event channel. @@ -267,7 +269,9 @@ circumstances where the generation ID needs to be changed. Paravirtual device frontends are generally specified by their own directory within the XenStore hierarchy. Usually this is under ~/device/$TYPE/$DEVID although there are exceptions, e.g. ~/console -for the first PV console. +for the first PV console. The top level ~/device path itself is normally +read-only to the guest. However it may writable if the +'xend_suspend_evtchn_compat' guest configuration option is enabled. #### ~/device/vbd/$DEVID/* [] diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 35e13428b2..71709dc585 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -1272,10 +1272,19 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src); * LIBXL_HAVE_CREATEINFO_DOMID * * libxl_domain_create_new() and libxl_domain_create_restore() will use - * a domid specified in libxl_domain_create_info(). + * a domid specified in libxl_domain_create_info. */ #define LIBXL_HAVE_CREATEINFO_DOMID +/* + * LIBXL_HAVE_CREATEINFO_XEND_SUSPEND_EVTCHN_COMPAT + * + * libxl_domain_create_info contains a boolean 'xend_suspend_evtchn_compat' + * value to control creation of the xenstore path for a domain's suspend + * event channel. + */ +#define LIBXL_HAVE_CREATEINFO_XEND_SUSPEND_EVTCHN_COMPAT + typedef char **libxl_string_list; void libxl_string_list_dispose(libxl_string_list *sl); int libxl_string_list_length(const libxl_string_list *sl); diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index e18aad43b5..e7cb2dbc2b 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -57,6 +57,8 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc, if (!c_info->ssidref) c_info->ssidref = SECINITSID_DOMU; + libxl_defbool_setdefault(&c_info->xend_suspend_evtchn_compat, false); + return 0; } @@ -748,9 +750,21 @@ retry_transaction: libxl__xs_mknod(gc, t, GCSPRINTF("%s/memory", dom_path), roperm, ARRAY_SIZE(roperm)); - libxl__xs_mknod(gc, t, - GCSPRINTF("%s/device", dom_path), - roperm, ARRAY_SIZE(roperm)); + + if (!libxl_defbool_val(info->xend_suspend_evtchn_compat)) { + libxl__xs_mknod(gc, t, + GCSPRINTF("%s/device", dom_path), + roperm, ARRAY_SIZE(roperm)); + libxl__xs_mknod(gc, t, + GCSPRINTF("%s/device/suspend/event-channel", + dom_path), + rwperm, ARRAY_SIZE(rwperm)); + } else { + libxl__xs_mknod(gc, t, + GCSPRINTF("%s/device", dom_path), + rwperm, ARRAY_SIZE(rwperm)); + } + libxl__xs_mknod(gc, t, GCSPRINTF("%s/control", dom_path), roperm, ARRAY_SIZE(roperm)); @@ -782,9 +796,7 @@ retry_transaction: libxl__xs_mknod(gc, t, GCSPRINTF("%s/control/sysrq", dom_path), rwperm, ARRAY_SIZE(rwperm)); - libxl__xs_mknod(gc, t, - GCSPRINTF("%s/device/suspend/event-channel", dom_path), - rwperm, ARRAY_SIZE(rwperm)); + libxl__xs_mknod(gc, t, GCSPRINTF("%s/data", dom_path), rwperm, ARRAY_SIZE(rwperm)); diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index d0d431614f..f7c473be74 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -418,6 +418,7 @@ libxl_domain_create_info = Struct("domain_create_info",[ ("run_hotplug_scripts",libxl_defbool), ("driver_domain",libxl_defbool), ("passthrough", libxl_passthrough), + ("xend_suspend_evtchn_compat",libxl_defbool), ], dir=DIR_IN) libxl_domain_restore_params = Struct("domain_restore_params", [ diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index b881184804..4450d59f16 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -2725,6 +2725,9 @@ skip_usbdev: parse_vkb_list(config, d_config); + xlu_cfg_get_defbool(config, "xend_suspend_evtchn_compat", + &c_info->xend_suspend_evtchn_compat, 0); + xlu_cfg_destroy(config); }