[{"id":1771879,"web_url":"http://patchwork.ozlabs.org/comment/1771879/","msgid":"<7ed88631-9983-db74-245c-8d3fd12a037e@siemens.com>","list_archive_url":null,"date":"2017-09-20T08:59:59","subject":"Re: [Qemu-devel] [PATCH] arm: Fix SMC reporting to EL2 when QEMU\n\tprovides PSCI","submitter":{"id":710,"url":"http://patchwork.ozlabs.org/api/people/710/","name":"Jan Kiszka","email":"jan.kiszka@siemens.com"},"content":"On 2017-09-18 09:51, Jan Kiszka wrote:\n> From: Jan Kiszka <jan.kiszka@siemens.com>\n> \n> This properly forwards SMC events to EL2 when PSCI is provided by QEMU\n> itself and, thus, ARM_FEATURE_EL3 is off.\n> \n> Found and tested with the Jailhouse hypervisor.\n> \n> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>\n> ---\n>  target/arm/helper.c    | 2 +-\n>  target/arm/op_helper.c | 8 ++++----\n>  target/arm/psci.c      | 6 ++++++\n>  3 files changed, 11 insertions(+), 5 deletions(-)\n> \n> diff --git a/target/arm/helper.c b/target/arm/helper.c\n> index 4f41841ef6..8c3929762c 100644\n> --- a/target/arm/helper.c\n> +++ b/target/arm/helper.c\n> @@ -3717,7 +3717,7 @@ static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)\n>  \n>      if (arm_feature(env, ARM_FEATURE_EL3)) {\n>          valid_mask &= ~HCR_HCD;\n> -    } else {\n> +    } else if (cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {\n>          valid_mask &= ~HCR_TSC;\n>      }\n>  \n> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c\n> index 6a60464ab9..4b0ef6a234 100644\n> --- a/target/arm/op_helper.c\n> +++ b/target/arm/op_helper.c\n> @@ -960,12 +960,12 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)\n>          return;\n>      }\n>  \n> -    if (!arm_feature(env, ARM_FEATURE_EL3)) {\n> -        /* If we have no EL3 then SMC always UNDEFs */\n> -        undef = true;\n> -    } else if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {\n> +    if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {\n>          /* In NS EL1, HCR controlled routing to EL2 has priority over SMD. */\n>          raise_exception(env, EXCP_HYP_TRAP, syndrome, 2);\n> +    } else if (!arm_feature(env, ARM_FEATURE_EL3)) {\n> +        /* If we have no EL3 then SMC always UNDEFs */\n> +        undef = true;\n>      }\n>  \n>      if (undef) {\n> diff --git a/target/arm/psci.c b/target/arm/psci.c\n> index fc34b263d3..637987ff46 100644\n> --- a/target/arm/psci.c\n> +++ b/target/arm/psci.c\n> @@ -35,6 +35,8 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)\n>       */\n>      CPUARMState *env = &cpu->env;\n>      uint64_t param = is_a64(env) ? env->xregs[0] : env->regs[0];\n> +    int cur_el = arm_current_el(env);\n> +    bool secure = arm_is_secure(env);\n>  \n>      switch (excp_type) {\n>      case EXCP_HVC:\n> @@ -46,6 +48,10 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)\n>          if (cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {\n>              return false;\n>          }\n> +        if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {\n> +            /* The EL2 will handle this. */\n> +            return false;\n> +        }\n>          break;\n>      default:\n>          return false;\n> \n\nFWIW, we've now a stable (and fast!) QEMU setup running Jailhouse for\naarch64. We just got bitten by a deficit that this setup revealed in our\ndevice tree overlay. See also\n\nhttps://groups.google.com/forum/#!topic/jailhouse-dev/ZqWpFyMXtZE\n\nLooking forward to eventually expand this to ARMv7, GICv2, or ITS.\nAnyone working on those edges already or plan to do so?\n\nJan","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xy1Td5mCbz9s7f\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 20 Sep 2017 23:55:17 +1000 (AEST)","from localhost ([::1]:48235 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dufT9-00016H-RV\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 09:55:15 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:55824)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <jan.kiszka@siemens.com>) id 1duex2-0007Bt-Dv\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:23:42 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <jan.kiszka@siemens.com>) id 1duevm-0005qk-NG\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:22:04 -0400","from david.siemens.de ([192.35.17.14]:34127)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <jan.kiszka@siemens.com>)\n\tid 1dueva-0004ab-PH\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:20:45 -0400","from mail3.siemens.de (mail3.siemens.de [139.25.208.14])\n\tby david.siemens.de (8.15.2/8.15.2) with ESMTPS id v8K901Pj028533\n\t(version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK);\n\tWed, 20 Sep 2017 11:00:01 +0200","from md1f2u6c.ww002.siemens.net ([139.22.47.10])\n\tby mail3.siemens.de (8.15.2/8.15.2) with ESMTP id v8K8xxtD028256;\n\tWed, 20 Sep 2017 11:00:01 +0200"],"From":"Jan Kiszka <jan.kiszka@siemens.com>","To":"Peter Maydell <peter.maydell@linaro.org>,\n\tqemu-devel <qemu-devel@nongnu.org>","References":"<1d61ec4d-da94-e96a-e1f6-509a4e80daec@siemens.com>","Message-ID":"<7ed88631-9983-db74-245c-8d3fd12a037e@siemens.com>","Date":"Wed, 20 Sep 2017 10:59:59 +0200","User-Agent":"Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12)\n\tGecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12\n\tMnenhy/0.7.5.666","MIME-Version":"1.0","In-Reply-To":"<1d61ec4d-da94-e96a-e1f6-509a4e80daec@siemens.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [fuzzy]","X-Received-From":"192.35.17.14","Subject":"Re: [Qemu-devel] [PATCH] arm: Fix SMC reporting to EL2 when QEMU\n\tprovides PSCI","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1772960,"web_url":"http://patchwork.ozlabs.org/comment/1772960/","msgid":"<CAFEAcA8H=wz2y+GEXUbxLpGRc8PoLKDUZf+emF=NeAs-zAPj1Q@mail.gmail.com>","list_archive_url":null,"date":"2017-09-21T16:12:44","subject":"Re: [Qemu-devel] [PATCH] arm: Fix SMC reporting to EL2 when QEMU\n\tprovides PSCI","submitter":{"id":5111,"url":"http://patchwork.ozlabs.org/api/people/5111/","name":"Peter Maydell","email":"peter.maydell@linaro.org"},"content":"On 18 September 2017 at 08:51, Jan Kiszka <jan.kiszka@siemens.com> wrote:\n> From: Jan Kiszka <jan.kiszka@siemens.com>\n>\n> This properly forwards SMC events to EL2 when PSCI is provided by QEMU\n> itself and, thus, ARM_FEATURE_EL3 is off.\n>\n> Found and tested with the Jailhouse hypervisor.\n>\n> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>\n> ---\n>  target/arm/helper.c    | 2 +-\n>  target/arm/op_helper.c | 8 ++++----\n>  target/arm/psci.c      | 6 ++++++\n>  3 files changed, 11 insertions(+), 5 deletions(-)\n>\n> diff --git a/target/arm/helper.c b/target/arm/helper.c\n> index 4f41841ef6..8c3929762c 100644\n> --- a/target/arm/helper.c\n> +++ b/target/arm/helper.c\n> @@ -3717,7 +3717,7 @@ static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)\n>\n>      if (arm_feature(env, ARM_FEATURE_EL3)) {\n>          valid_mask &= ~HCR_HCD;\n> -    } else {\n> +    } else if (cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {\n>          valid_mask &= ~HCR_TSC;\n\nThis looks odd, so it needs a comment I think.\n  /* Architecturally HCR.TSC is RES0 if EL3 is not implemented.\n   * However, if we're using the SMC PSCI conduit then QEMU is\n   * effectively acting like EL3 firmware and so the guest at\n   * EL2 should retain the ability to prevent EL1 from being\n   * able to make SMC calls into the ersatz firmware, so in\n   * that case HCR.TSC should be read/write.\n   */\n\n>      }\n>\n> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c\n> index 6a60464ab9..4b0ef6a234 100644\n> --- a/target/arm/op_helper.c\n> +++ b/target/arm/op_helper.c\n> @@ -960,12 +960,12 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)\n>          return;\n>      }\n>\n> -    if (!arm_feature(env, ARM_FEATURE_EL3)) {\n> -        /* If we have no EL3 then SMC always UNDEFs */\n> -        undef = true;\n> -    } else if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {\n> +    if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {\n>          /* In NS EL1, HCR controlled routing to EL2 has priority over SMD. */\n>          raise_exception(env, EXCP_HYP_TRAP, syndrome, 2);\n> +    } else if (!arm_feature(env, ARM_FEATURE_EL3)) {\n> +        /* If we have no EL3 then SMC always UNDEFs */\n> +        undef = true;\n>      }\n>\n>      if (undef) {\n\nI think the logic in this function should be something like:\n\n    if (!arm_feature(env, ARM_FEATURE_EL3) &&\n        cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC)) {\n        /* If we have no EL3 then SMC always UNDEFs and can't be\n         * trapped to EL2. PSCI-via-SMC is a sort of ersatz EL3\n         * firmware within QEMU, and we want an EL2 guest to be able\n         * to forbid its EL1 from making PSCI calls into QEMU's\n         * \"firmware\" via HCR.TSC, so for these purposes treat\n         *  PSCI-via-SMC as implying an EL3.\n         */\n        undef = true;\n    else if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {\n        /* In NS EL1, HCR controlled routing to EL2 has priority over SMD.\n         * We also want an EL2 guest to be able to forbid its EL1 from\n         * making PSCI calls into QEMU's \"firmware\" via HCR.TSC.\n         */\n        raise_exception(env, EXCP_HYP_TRAP, syndrome, 2);\n    }\n\n    if (undef && !arm_is_psci_call(cpu, EXCP_SMC)) {\n        /* If PSCI is enabled and this looks like a valid PSCI call then\n         * suppress the UNDEF -- we'll catch the SMC exception and\n         * implement the PSCI call behaviour there.\n         */\n        raise_exception(env, EXCP_UDEF, syn_uncategorized(),\n                        exception_target_el(env));\n    }\n\n(Totally untested!)\n\n> diff --git a/target/arm/psci.c b/target/arm/psci.c\n> index fc34b263d3..637987ff46 100644\n> --- a/target/arm/psci.c\n> +++ b/target/arm/psci.c\n> @@ -35,6 +35,8 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)\n>       */\n>      CPUARMState *env = &cpu->env;\n>      uint64_t param = is_a64(env) ? env->xregs[0] : env->regs[0];\n> +    int cur_el = arm_current_el(env);\n> +    bool secure = arm_is_secure(env);\n>\n>      switch (excp_type) {\n>      case EXCP_HVC:\n> @@ -46,6 +48,10 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)\n>          if (cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {\n>              return false;\n>          }\n> +        if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {\n> +            /* The EL2 will handle this. */\n> +            return false;\n> +        }\n\nI don't think we should need to change this function -- its\npurpose is \"does this look like a PSCI call\", and if it's\nan SMC exception with the right magic parameters, then it\ndoes look like a PSCI call. Instead we should make the\npre_smc helper choose \"raise a hyp trap\" if that's the right\nthing to be doing (see comment above for what I think is the\nright logic there).\n\nthanks\n-- PMM","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"XJjp7r5B\"; dkim-atps=neutral"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xyhVr2D2Dz9t42\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri, 22 Sep 2017 02:13:40 +1000 (AEST)","from localhost ([::1]:54527 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dv46c-0007zP-5x\n\tfor incoming@patchwork.ozlabs.org; Thu, 21 Sep 2017 12:13:38 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:52728)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <peter.maydell@linaro.org>) id 1dv46A-0007xt-RZ\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 12:13:12 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <peter.maydell@linaro.org>) id 1dv466-0005us-Hr\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 12:13:10 -0400","from mail-wr0-x22d.google.com ([2a00:1450:400c:c0c::22d]:44182)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from <peter.maydell@linaro.org>)\n\tid 1dv466-0005uJ-6I\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 12:13:06 -0400","by mail-wr0-x22d.google.com with SMTP id v109so4990773wrc.1\n\tfor <qemu-devel@nongnu.org>; Thu, 21 Sep 2017 09:13:06 -0700 (PDT)","by 10.223.139.215 with HTTP; Thu, 21 Sep 2017 09:12:44 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=+Zg5WqdVSg2p9O8vMYYOHUI5SMzmhReA4IHsWtiQONQ=;\n\tb=XJjp7r5BwathctoUgCDbZaD6Kj5tAW06RVPIs2clrxFUYNbD0U4euNwPVnql4jK+VA\n\tyXIMauhcE2x5cbuORBMPEfPCFo/RHsd4P/N/U++Ebp8cSfiq7Z31tvwdsEt5BvGfzQnM\n\tFZvZB68baV+PLalnqVhN5eca4i/tFt3La1zdE=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=+Zg5WqdVSg2p9O8vMYYOHUI5SMzmhReA4IHsWtiQONQ=;\n\tb=VcIh1jOYLRPx3vOT5V7LdWdlo1pDBKKxmjVmyoNMFzFdldSRH8p0EODbJexoOiEuqc\n\tz4DYwxppktZihatmLFcftVl4ItPwnw8JMJOZq+erfnFGGUmDHq+FbA+W0gk7Tdq2RUNh\n\t+qz+zC8IpefB54TPHt7Y7DgLrREYfMHZL2V644y4z6rXcAXfmEkHT/YWfSlX0TsgcVxw\n\tmsuDxL/2b32p5fHPMxoyf0W9NcddXx+s2vGQeW2Ydfn9Hh16WDqNsFTLyl9puvYllZcK\n\t1KVndviVqhJvrCwIAtZlLPgdLi0ezRcu2i8ct8M1dm9NnfaPj/MU7DtS5X2De9UvS419\n\tMNSw==","X-Gm-Message-State":"AHPjjUhwBl3fyI4UJLNn/qnUU9/q0ecTTHWq8soOAfX5c5vVLGskFkVW\n\tksgcfjyTUYAEw1kNGVqOl95h7o/1vzE9eiN1bd9sSWg7","X-Google-Smtp-Source":"AOwi7QAqV6AAU8+GTYAB8ycTz4BHXmFQylg5braj9MyRXe7Q3HzOMdlxIyjX2Y1hvobRdHl6f9iTRZ4DRJO+9eqp58A=","X-Received":"by 10.223.198.202 with SMTP id c10mr2351609wrh.230.1506010385041;\n\tThu, 21 Sep 2017 09:13:05 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<1d61ec4d-da94-e96a-e1f6-509a4e80daec@siemens.com>","References":"<1d61ec4d-da94-e96a-e1f6-509a4e80daec@siemens.com>","From":"Peter Maydell <peter.maydell@linaro.org>","Date":"Thu, 21 Sep 2017 17:12:44 +0100","Message-ID":"<CAFEAcA8H=wz2y+GEXUbxLpGRc8PoLKDUZf+emF=NeAs-zAPj1Q@mail.gmail.com>","To":"Jan Kiszka <jan.kiszka@siemens.com>","Content-Type":"text/plain; charset=\"UTF-8\"","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2a00:1450:400c:c0c::22d","Subject":"Re: [Qemu-devel] [PATCH] arm: Fix SMC reporting to EL2 when QEMU\n\tprovides PSCI","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel <qemu-devel@nongnu.org>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1773011,"web_url":"http://patchwork.ozlabs.org/comment/1773011/","msgid":"<acd13dea-1b0c-c21a-08b1-1ddba1f8099b@siemens.com>","list_archive_url":null,"date":"2017-09-21T17:24:03","subject":"Re: [Qemu-devel] [PATCH] arm: Fix SMC reporting to EL2 when QEMU\n\tprovides PSCI","submitter":{"id":710,"url":"http://patchwork.ozlabs.org/api/people/710/","name":"Jan Kiszka","email":"jan.kiszka@siemens.com"},"content":"On 2017-09-21 18:12, Peter Maydell wrote:\n> On 18 September 2017 at 08:51, Jan Kiszka <jan.kiszka@siemens.com> wrote:\n>> From: Jan Kiszka <jan.kiszka@siemens.com>\n>>\n>> This properly forwards SMC events to EL2 when PSCI is provided by QEMU\n>> itself and, thus, ARM_FEATURE_EL3 is off.\n>>\n>> Found and tested with the Jailhouse hypervisor.\n>>\n>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>\n>> ---\n>>  target/arm/helper.c    | 2 +-\n>>  target/arm/op_helper.c | 8 ++++----\n>>  target/arm/psci.c      | 6 ++++++\n>>  3 files changed, 11 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/target/arm/helper.c b/target/arm/helper.c\n>> index 4f41841ef6..8c3929762c 100644\n>> --- a/target/arm/helper.c\n>> +++ b/target/arm/helper.c\n>> @@ -3717,7 +3717,7 @@ static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)\n>>\n>>      if (arm_feature(env, ARM_FEATURE_EL3)) {\n>>          valid_mask &= ~HCR_HCD;\n>> -    } else {\n>> +    } else if (cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {\n>>          valid_mask &= ~HCR_TSC;\n> \n> This looks odd, so it needs a comment I think.\n>   /* Architecturally HCR.TSC is RES0 if EL3 is not implemented.\n>    * However, if we're using the SMC PSCI conduit then QEMU is\n>    * effectively acting like EL3 firmware and so the guest at\n>    * EL2 should retain the ability to prevent EL1 from being\n>    * able to make SMC calls into the ersatz firmware, so in\n>    * that case HCR.TSC should be read/write.\n>    */\n> \n\nAck.\n\n>>      }\n>>\n>> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c\n>> index 6a60464ab9..4b0ef6a234 100644\n>> --- a/target/arm/op_helper.c\n>> +++ b/target/arm/op_helper.c\n>> @@ -960,12 +960,12 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)\n>>          return;\n>>      }\n>>\n>> -    if (!arm_feature(env, ARM_FEATURE_EL3)) {\n>> -        /* If we have no EL3 then SMC always UNDEFs */\n>> -        undef = true;\n>> -    } else if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {\n>> +    if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {\n>>          /* In NS EL1, HCR controlled routing to EL2 has priority over SMD. */\n>>          raise_exception(env, EXCP_HYP_TRAP, syndrome, 2);\n>> +    } else if (!arm_feature(env, ARM_FEATURE_EL3)) {\n>> +        /* If we have no EL3 then SMC always UNDEFs */\n>> +        undef = true;\n>>      }\n>>\n>>      if (undef) {\n> \n> I think the logic in this function should be something like:\n> \n>     if (!arm_feature(env, ARM_FEATURE_EL3) &&\n>         cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC)) {\n>         /* If we have no EL3 then SMC always UNDEFs and can't be\n>          * trapped to EL2. PSCI-via-SMC is a sort of ersatz EL3\n>          * firmware within QEMU, and we want an EL2 guest to be able\n>          * to forbid its EL1 from making PSCI calls into QEMU's\n>          * \"firmware\" via HCR.TSC, so for these purposes treat\n>          *  PSCI-via-SMC as implying an EL3.\n>          */\n>         undef = true;\n>     else if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {\n>         /* In NS EL1, HCR controlled routing to EL2 has priority over SMD.\n>          * We also want an EL2 guest to be able to forbid its EL1 from\n>          * making PSCI calls into QEMU's \"firmware\" via HCR.TSC.\n>          */\n>         raise_exception(env, EXCP_HYP_TRAP, syndrome, 2);\n>     }\n> \n>     if (undef && !arm_is_psci_call(cpu, EXCP_SMC)) {\n>         /* If PSCI is enabled and this looks like a valid PSCI call then\n>          * suppress the UNDEF -- we'll catch the SMC exception and\n>          * implement the PSCI call behaviour there.\n>          */\n>         raise_exception(env, EXCP_UDEF, syn_uncategorized(),\n>                         exception_target_el(env));\n>     }\n> \n> (Totally untested!)\n\nI'll try this.\n\n> \n>> diff --git a/target/arm/psci.c b/target/arm/psci.c\n>> index fc34b263d3..637987ff46 100644\n>> --- a/target/arm/psci.c\n>> +++ b/target/arm/psci.c\n>> @@ -35,6 +35,8 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)\n>>       */\n>>      CPUARMState *env = &cpu->env;\n>>      uint64_t param = is_a64(env) ? env->xregs[0] : env->regs[0];\n>> +    int cur_el = arm_current_el(env);\n>> +    bool secure = arm_is_secure(env);\n>>\n>>      switch (excp_type) {\n>>      case EXCP_HVC:\n>> @@ -46,6 +48,10 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)\n>>          if (cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {\n>>              return false;\n>>          }\n>> +        if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {\n>> +            /* The EL2 will handle this. */\n>> +            return false;\n>> +        }\n> \n> I don't think we should need to change this function -- its\n> purpose is \"does this look like a PSCI call\", and if it's\n> an SMC exception with the right magic parameters, then it\n> does look like a PSCI call. Instead we should make the\n> pre_smc helper choose \"raise a hyp trap\" if that's the right\n> thing to be doing (see comment above for what I think is the\n> right logic there).\n\nIf we remove this filter, we will have to patch arm_cpu_do_interrupt in\naddition IIRC. I once had a version which had a similar ordering in\npre_smc as above, but it still didn't deliver the call to EL2.\n\nBTW, my feeling is that things are not completely correct for the HVC\ncase as well, at least the ordering of checks. But I didn't try that yet.\n\nJan","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xyk4p0dGpz9t3Z\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri, 22 Sep 2017 03:24:40 +1000 (AEST)","from localhost ([::1]:54846 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dv5DJ-0000Xi-2d\n\tfor incoming@patchwork.ozlabs.org; Thu, 21 Sep 2017 13:24:37 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:45776)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <jan.kiszka@siemens.com>) id 1dv5Cv-0000XX-US\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 13:24:15 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <jan.kiszka@siemens.com>) id 1dv5Cs-0005BL-Dr\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 13:24:13 -0400","from david.siemens.de ([192.35.17.14]:35221)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <jan.kiszka@siemens.com>)\n\tid 1dv5Cs-00059R-3L\n\tfor qemu-devel@nongnu.org; Thu, 21 Sep 2017 13:24:10 -0400","from mail1.siemens.de (mail1.siemens.de [139.23.33.14])\n\tby david.siemens.de (8.15.2/8.15.2) with ESMTPS id v8LHO61s023924\n\t(version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK);\n\tThu, 21 Sep 2017 19:24:06 +0200","from md1f2u6c.ww002.siemens.net ([139.25.34.127])\n\tby mail1.siemens.de (8.15.2/8.15.2) with ESMTP id v8LHO4kT024125;\n\tThu, 21 Sep 2017 19:24:05 +0200"],"To":"Peter Maydell <peter.maydell@linaro.org>","References":"<1d61ec4d-da94-e96a-e1f6-509a4e80daec@siemens.com>\n\t<CAFEAcA8H=wz2y+GEXUbxLpGRc8PoLKDUZf+emF=NeAs-zAPj1Q@mail.gmail.com>","From":"Jan Kiszka <jan.kiszka@siemens.com>","Message-ID":"<acd13dea-1b0c-c21a-08b1-1ddba1f8099b@siemens.com>","Date":"Thu, 21 Sep 2017 19:24:03 +0200","User-Agent":"Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12)\n\tGecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12\n\tMnenhy/0.7.5.666","MIME-Version":"1.0","In-Reply-To":"<CAFEAcA8H=wz2y+GEXUbxLpGRc8PoLKDUZf+emF=NeAs-zAPj1Q@mail.gmail.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [fuzzy]","X-Received-From":"192.35.17.14","Subject":"Re: [Qemu-devel] [PATCH] arm: Fix SMC reporting to EL2 when QEMU\n\tprovides PSCI","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"qemu-devel <qemu-devel@nongnu.org>","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}}]