From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0ADD2C2B9F4 for ; Fri, 25 Jun 2021 17:47:36 +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 BBA9861864 for ; Fri, 25 Jun 2021 17:47:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BBA9861864 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.147352.271548 (Exim 4.92) (envelope-from ) id 1lwpvG-0005XB-2W; Fri, 25 Jun 2021 17:47:22 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 147352.271548; Fri, 25 Jun 2021 17:47:22 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1lwpvF-0005X4-Vn; Fri, 25 Jun 2021 17:47:21 +0000 Received: by outflank-mailman (input) for mailman id 147352; Fri, 25 Jun 2021 17:47:21 +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.92) (envelope-from ) id 1lwpvF-0005Wy-9g for xen-devel@lists.xenproject.org; Fri, 25 Jun 2021 17:47:21 +0000 Received: from mail.kernel.org (unknown [198.145.29.99]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 351e135f-7696-44e6-b8c0-d18fb6054ae9; Fri, 25 Jun 2021 17:47:20 +0000 (UTC) Received: by mail.kernel.org (Postfix) with ESMTPSA id 822FF6157E; Fri, 25 Jun 2021 17:47:19 +0000 (UTC) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 351e135f-7696-44e6-b8c0-d18fb6054ae9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1624643239; bh=nOVeSVwc3vAa+JVLD6a/lw25Pvfe+iUBs+PTwaec6tU=; h=Date:From:To:cc:Subject:In-Reply-To:References:From; b=SrXsxrd6S6BN8XcXzR1f3GVK+MewsoyV7ruAbm/7cSQ8e5C5wxjmCbI5YOQGnfag8 BxRDNlGTsz2vDZgyGyKGdFGEpYWh+IH7SyHNbf8HPiqMJY5mQQ70cva3n4zz6YIeFF UKjTEGuU1zB/MQect/A/K7mjiMhy/6h24FguYuQ3RpRGdoptylyCfscaGuNJFmeK11 /9iPE4+pQoAqoUCy72ECKEuIBlo4SnFJmjSTa+3zb1EwTHOL2HlOKffOATABjE7Yfy uWuTqsvg1rBB/8ArIj6UXV/RzvhS1ijPeKxiPEx/H9PICV2lMAipLgwLcCA03w/CkJ /dPvmk6bA1zRA== Date: Fri, 25 Jun 2021 10:47:18 -0700 (PDT) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-T480s To: Julien Grall cc: Stefano Stabellini , xen-devel@lists.xenproject.org, Volodymyr_Babchuk@epam.com Subject: Re: [PATCH] xen/arm: add forward_smc command line option for debugging In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII On Fri, 25 Jun 2021, Julien Grall wrote: > Hi, > > On 25/06/2021 02:51, Stefano Stabellini wrote: > > It has become clear that an option to disable trapping SMC calls to Xen > > is very useful for debugging user issues. > > > > Instead of having to provide a > > patch to users every time, it would be great if we could just tell them > > to add forward_smc=true to the Xen command line. > > I can understand this woud be useful to go a bit further in dom0 boot. But I > am quite sceptical on the idea of providing an option directly in Xen because: > > 1) This breaks other SMC uses in Xen (optee, VM monitor...) > 2) There are no guarantee that the SMC call will not wreck Xen. To be clear, I > don't refer to a malicious OS here, but a normal OS that boot > 3) Very likely the next steps for the user (or better call it the developper > because that option should really not be used by a normal user) will be to > decide whether they should modify the kernel or implement a mediator in Xen. > > > This option is obviously unsafe and unsecure and only meant for > > debugging. Make clear in the description that if you pass > > forward_smc=true then the system is not security supported. > > > > Signed-off-by: Stefano Stabellini > > > > diff --git a/docs/misc/xen-command-line.pandoc > > b/docs/misc/xen-command-line.pandoc > > index 3ece83a427..0833fe80fc 100644 > > --- a/docs/misc/xen-command-line.pandoc > > +++ b/docs/misc/xen-command-line.pandoc > > @@ -2501,6 +2501,16 @@ vwfi to `native` reduces irq latency significantly. > > It can also lead to > > suboptimal scheduling decisions, but only when the system is > > oversubscribed (i.e., in total there are more vCPUs than pCPUs). > > +### forward_smc (arm) > > +> `= ` > > + > > +> Default: `false` > > + > > +If enabled, instead of trapping firmware SMC calls to Xen, allow SMC > > +calls from VMs directly to the firmware. This option is UNSAFE and it is > > +only meant for debugging. Systems with forward_smc=true are not security > > +supported. > > + > > ### watchdog (x86) > > > `= force | ` > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > > index e7384381cc..0580ac5762 100644 > > --- a/xen/arch/arm/traps.c > > +++ b/xen/arch/arm/traps.c > > @@ -95,11 +95,15 @@ static int __init parse_vwfi(const char *s) > > } > > custom_param("vwfi", parse_vwfi); > > +static bool forward_smc = false; > > +boolean_param("forward_smc", forward_smc); > > + > > register_t get_default_hcr_flags(void) > > { > > return (HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM| > > (vwfi != NATIVE ? (HCR_TWI|HCR_TWE) : 0) | > > - HCR_TID3|HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB|HCR_TSW); > > + (forward_smc ? 0 : HCR_TSC) | > > + HCR_TID3|HCR_TAC|HCR_SWIO|HCR_TIDCP|HCR_FB|HCR_TSW); > > A system wide option to turn off SMC trapping is a no-go because this would > only be usable for debugging dom0 and not a guest. > > So at the minimum this should be a per-domain option. Also, I think we still > want to integrate with the rest of the SMC users. So Xen should still trap the > SMC and the forward should happen in vsmccc_handle_call(). > > This would cover my first point. Yes, you are totally right. I thought about it this morning as well. This patch would break even PSCI :-( It would be best implemented in platform_smc as forward_to_fw (see xen/arch/arm/platforms/xilinx-zynqmp-eemi.c:forward_to_fw). > For the second and third point, I still like > to understand how this is going to help the developer to fully port the > board/OS to Xen with this option disabled? This is meant to help with bug triage only. There are a number of bugs that can happen if certain platform SMCs are intercerpted by Xen instead of being forwarded to the hardware. I found myself having to provide a patch to forward_to_fw all platform SMCs as a first test to triage bugs a few times recently. It is never a fix, only a way to understand the next step of debugging. Also Alex stumbled across something similar on a non-Xilinx board (MacchiatoBin) so I thought it was time for a better debugging option. I think for debugging purposes it would be sufficient if all platform SMCs were forward_to_fw from all domains. Of course it is totally unsafe, but it is just for debugging. But I can also see the value in having a command line option to forward_to_fw all platform SMCs from dom0 only and maybe a separate patch later to add a per-domain option to forward_to_fw platform SMCs for specific domains if needed. That would be safer and more flexible but a little more work.