From patchwork Thu Oct 31 15:09:22 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julien Grall X-Patchwork-Id: 178204 Delivered-To: patch@linaro.org Received: by 2002:a92:409a:0:0:0:0:0 with SMTP id d26csp2980656ill; Thu, 31 Oct 2019 08:11:39 -0700 (PDT) X-Google-Smtp-Source: APXvYqwaB2oabx9DvAI7zGs9eNC665K0G3rdJjoGgkH0WxDlzreU16v3QojVJZ7YBMqJBvRgg4uF X-Received: by 2002:a92:d38b:: with SMTP id o11mr6611598ilo.20.1572534698969; Thu, 31 Oct 2019 08:11:38 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1572534698; cv=none; d=google.com; s=arc-20160816; b=cTKLJLOeAne/3Zscb12M9IJ5dYKvRXIc7L2SjBBbvZPIeNMv4V8HZ0pDugRFYa5QH9 usrHZ3Z2Uw+/9U9q1VNWTioYkkzn5pEI7GQZkv2H1a0R+jqs5jxpLLtiCsa+5oX0KQgL zzH9uyQp2mg5Yj8lRq/ZDpjk+klkFX68HDsrJz5QifusLqTaAps5XyLmRwWucM4L6FDy QRTFdblKWQVWHP9WC6s3+IXQusXCe0b1LnX5VEKl43hAJjie9Rfe40tnvqZJkEYoX7DB VdFLgJM+X6nmrrPTqq0iORCOufExGz5v76o4Oij887q8YxLH3kHsYqYUU0/GT1Jsjeav PWCg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version:cc :list-subscribe:list-help:list-post:list-unsubscribe:list-id :precedence:subject:references:in-reply-to:message-id:date:to:from; bh=yYK7OKkW6arL8A9DYSLsohNIVLp9bBoUQ5CBKZUnGUc=; b=UTeKKATC07IYnSSfkRy34lXKvO6/hRj4FV6UFlZLCQYYuQiVXCs/6PuWR9ISfMOIIF 3e6V1AIxDeFNxInxA7WeWQmzZW7yHbPDSl1h99AlByG5KBiXEph14/IcuBRLfkG2IPn2 EZPF8CBPiJZz3kqhRIoGNpspRXRo243B7pP0ksd4jLElMVFBs4K91qtV5EzivyHbmnoy 11ohSeUYgRp+5Hk1lS0rBmrfufHkie1WPNkq26GaJzC6CnR8hMNQUBXc/e+J0HJKBQVi 8c6zGscf8VaW9IM9X+nb+8mepaH/hFeHsNzoKzvm8xK8BG1Qefu0GzhtoKTR2pU/QUHM n85w== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of xen-devel-bounces@lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Return-Path: Received: from lists.xenproject.org (lists.xenproject.org. [192.237.175.120]) by mx.google.com with ESMTPS id f6si7183043ilq.25.2019.10.31.08.11.38 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 31 Oct 2019 08:11:38 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of xen-devel-bounces@lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of xen-devel-bounces@lists.xenproject.org designates 192.237.175.120 as permitted sender) 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 1iQC5x-0007wm-Nj; Thu, 31 Oct 2019 15:10:41 +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 1iQC5w-0007vT-Bl for xen-devel@lists.xenproject.org; Thu, 31 Oct 2019 15:10:40 +0000 X-Inumbo-ID: 7eeec6b6-fbf0-11e9-954c-12813bfff9fa Received: from foss.arm.com (unknown [217.140.110.172]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTP id 7eeec6b6-fbf0-11e9-954c-12813bfff9fa; Thu, 31 Oct 2019 15:09:56 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B34D895D; Thu, 31 Oct 2019 08:09:56 -0700 (PDT) Received: from e108454-lin.cambridge.arm.com (unknown [10.1.196.50]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B08BB3F71E; Thu, 31 Oct 2019 08:09:55 -0700 (PDT) From: Julien Grall To: xen-devel@lists.xenproject.org Date: Thu, 31 Oct 2019 15:09:22 +0000 Message-Id: <20191031150922.22938-20-julien.grall@arm.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20191031150922.22938-1-julien.grall@arm.com> References: <20191031150922.22938-1-julien.grall@arm.com> Subject: [Xen-devel] [PATCH for-4.13 v4 19/19] xen/arm: entry: Ensure the guest state is synced when receiving a vSError 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: jgross@suse.com, Volodymyr Babchuk , Julien Grall , Stefano Stabellini , Julien Grall MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" When a SError/Asynchronous Abort generated by the guest has been consumed, we will skip the handling of the initial exception. This includes the calls to enter_hypervisor_from_guest{, _noirq} that is used to synchronize part of the guest state with the internal representation and re-enable workarounds (e.g. SSBD). However, we still call leave_hypervisor_to_guest() which is used for preempting the guest and synchronizing back part of the guest state. enter_hypervisor_from_guest{, _noirq} works in pair with leave_hypervisor_to_guest(), so skipping the first two may result in a loss of some part of guest state. An example is the new vGIC which will save the state of the LRs on exit from the guest and rewrite all of them on entry to the guest. A more worrying example is SSBD workaround may not be re-enabled. If leave_hypervisor_to_guest() is rescheduling the vCPU, then we may end to run a lot of code with SSBD workaroud disabled. For now, calling leave_hypervisor_to_guest() is not necessary when injecting a vSError to the guest. But it would still be good to give an opportunity to reschedule. So both enter_hypervisor_from_guest() and leave_hypervisor_to_guest() are called. Note that on arm64, the return value for check_pending_vserror is now stored in x19 instead of x0. This is because we want to keep the value across call to C-functions (x0, unlike x19, will not be saved by the callee). Take the opportunity to rename check_pending_vserror() to check_pending_guest_serror() as the function is dealing with host SError and *not* virtual SError. The documentation is also updated accross Arm32 and Arm64 to clarify how Xen is dealing with SError generated by the guest. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- Changes in v4: - Rewording + typo Changes in v3: - Update comments in the code. - Update commit message - Add arm32 support There are two known issues without this patch applied: * The state of the vGIC when using the new version may be lost. * SSBD workaround may be kept disabled while rescheduling the guest. --- xen/arch/arm/arm32/entry.S | 57 ++++++++++++++++++++++++++++++++++++++-------- xen/arch/arm/arm64/entry.S | 54 ++++++++++++++++++++++++++++++++----------- 2 files changed, 88 insertions(+), 23 deletions(-) diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S index 34156c4404..b31056a616 100644 --- a/xen/arch/arm/arm32/entry.S +++ b/xen/arch/arm/arm32/entry.S @@ -27,6 +27,10 @@ /* * Actions that needs to be done after entering the hypervisor from the * guest and before the interrupts are unmasked. + * + * @return: + * r4: Set to a non-zero value if a pending Abort exception took place. + * Otherwise, it will be set to zero. */ arch_enter_hypervisor_from_guest_preirq: #ifdef CONFIG_ARM32_HARDEN_BRANCH_PREDICTOR @@ -56,18 +60,35 @@ arch_enter_hypervisor_from_guest_preirq: SAVE_ONE_BANKED(R11_fiq); SAVE_ONE_BANKED(R12_fiq); /* - * If the SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT has been set in the cpu - * feature, the checking of pending SErrors will be skipped. + * We may have entered the hypervisor with pending asynchronous Abort + * generated by the guest. If we need to categorize them, then + * we need to consume any outstanding asynchronous Abort. + * Otherwise, they can be consumed later on. */ alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT + mov r4, #0 /* r4 := No Abort was consumed */ b skip_check alternative_else_nop_endif /* - * Start to check pending virtual abort in the gap of Guest -> HYP - * world switch. + * Consume pending asynchronous Abort generated by the guest if any. + * + * The only way to consume an Abort interrupt is to unmask it. So + * Abort exception will be unmaked for a small window and then masked + * it again. + * + * It is fine to unmask asynchronous Abort exception as we fully + * control the state of the processor and only limited code will + * be executed if the exception returns (see do_trap_data_abort()). * - * Save ELR_hyp to check whether the pending virtual abort exception + * TODO: The asynchronous abort path should be reworked to + * inject the virtual asynchronous Abort in enter_hypervisor_* + * rather than do_trap_data_abort(). This should make easier to + * understand the path. + */ + + /* + * save elr_hyp to check whether the pending virtual abort exception * takes place while we are doing this trap exception. */ mrs r1, ELR_hyp @@ -112,11 +133,11 @@ abort_guest_exit_end: cmp r1, r2 /* - * Not equal, the pending virtual abort exception took place, the - * initial exception does not have any significance to be handled. - * Exit ASAP. + * Set r4 depending on whether an asynchronous abort were + * consumed. */ - bne return_from_trap + movne r4, #1 + moveq r4, #0 skip_check: b enter_hypervisor_from_guest_preirq @@ -179,12 +200,28 @@ ENDPROC(arch_enter_hypervisor_from_guest_preirq) 1: /* Trap from the guest */ + /* + * arch_enter_hypervisor_from_guest_preirq will return with r4 set to + * a non-zero value if an asynchronous Abort was consumed. + * + * When an asynchronous Abort has been consumed (r4 != 0), we may have + * injected a virtual asynchronous Abort to the guest. + * + * In this case, the initial exception will be discarded (PC has + * been adjusted by inject_vabt_exception()). However, we still + * want to give an opportunity to reschedule the vCPU. So we + * only want to skip the handling of the initial exception (i.e. + * do_trap_*()). + */ bl arch_enter_hypervisor_from_guest_preirq .if \guest_iflags != n cpsie \guest_iflags .endif - bl enter_hypervisor_from_guest + adr lr, 2f + cmp r4, #0 + adrne lr, return_from_trap + b enter_hypervisor_from_guest 2: /* We are ready to handle the trap, setup the registers and jump. */ diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S index a8ba7ab961..d35855af96 100644 --- a/xen/arch/arm/arm64/entry.S +++ b/xen/arch/arm/arm64/entry.S @@ -184,18 +184,41 @@ .macro guest_vector compat, iflags, trap, save_x0_x1=1 entry hyp=0, compat=\compat, save_x0_x1=\save_x0_x1 /* - * The vSError will be checked while SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT - * is not set. If a vSError took place, the initial exception will be - * skipped. Exit ASAP + * We may have entered the hypervisor with pending SErrors + * generated by the guest. If we need to categorize them, then + * we need to check any outstanding SErrors will be consumed. + * + * The function check_pending_guest_serror() will unmask SError + * exception temporarily. This is fine to do before enter_* + * helpers are called because we fully control the state of the + * processor and only limited code willl be executed (see + * do_trap_hyp_serror()). + * + * When a SError has been consumed (x19 != 0), we may have injected a + * virtual SError to the guest. + * + * In this case, the initial exception will be discarded (PC has + * been adjusted by inject_vabt_exception()). However, we still + * want to give an opportunity to reschedule the vCPU. So we + * only want to skip the handling of the initial exception (i.e. + * do_trap_*()). + * + * TODO: The SErrors path should be reworked to inject the vSError in + * enter_hypervisor_* rather than do_trap_hyp_serror. This should make + * easier to understand the path. */ alternative_if_not SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT - bl check_pending_vserror - cbnz x0, 1f + bl check_pending_guest_serror alternative_else_nop_endif bl enter_hypervisor_from_guest_preirq msr daifclr, \iflags bl enter_hypervisor_from_guest + + alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT + cbnz x19, 1f + alternative_else_nop_endif + mov x0, sp bl do_trap_\trap 1: @@ -436,13 +459,17 @@ return_from_trap: eret /* - * This function is used to check pending virtual SError in the gap of - * EL1 -> EL2 world switch. - * The x0 register will be used to indicate the results of detection. - * x0 -- Non-zero indicates a pending virtual SError took place. - * x0 -- Zero indicates no pending virtual SError took place. + * Consume pending SError generated by the guest if any. + * + * @return: + * x19: Set to a non-zero value if a pending Abort exception took place. + * Otherwise, it will be set to zero. + * + * Without RAS extension, the only way to consume a SError is to unmask + * it. So the function will unmask SError exception for a small window and + * then mask it again. */ -check_pending_vserror: +check_pending_guest_serror: /* * Save elr_el2 to check whether the pending SError exception takes * place while we are doing this sync exception. @@ -487,11 +514,12 @@ abort_guest_exit_end: /* * Not equal, the pending SError exception took place, set - * x0 to non-zero. + * x19 to non-zero. */ - cset x0, ne + cset x19, ne ret +ENDPROC(check_pending_guest_serror) /* * Exception vectors.