[PATCH v7 0/6] zynqmp: Add forwarding of platform specific firmware calls

[PATCH v7 0/6] zynqmp: Add forwarding of platform specific firmware calls

[Stefano Stabellini]

Hi all,

Only minor changes in this patch, mainly:
- add comments
- add a warning
- use full fid to check for action

Cheers,

Stefano


The following changes since commit 82855aba5bf91e50c81526167c11d4aeaf665e66:

  tools/libxc: Fix error handling in get_cpuid_domain_info() (2018-11-30 14:21:12 +0000)

are available in the git repository at:

  http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git zynqmp-v7

for you to fetch changes up to 0a44fe20fb0a19292b3f84cdadd3e4af4d3b9d92:

  xen/zynqmp: add IPI calls virtualization (2018-12-17 14:03:26 -0800)

----------------------------------------------------------------
Edgar E. Iglesias (5):
      xen/arm: introduce platform_smc
      xen/arm: zynqmp: Forward plaform specific firmware calls
      xen/arm: zynqmp: introduce zynqmp specific defines
      xen/arm: zynqmp: implement zynqmp_eemi
      xen/arm: zynqmp: Remove blacklist of ZynqMP's PM node

Stefano Stabellini (1):
      xen/zynqmp: add IPI calls virtualization

 xen/arch/arm/platform.c                            |   8 +
 xen/arch/arm/platforms/Makefile                    |   1 +
 xen/arch/arm/platforms/xilinx-zynqmp-eemi.c        | 221 +++++++++++++++++++++
 xen/arch/arm/platforms/xilinx-zynqmp.c             |  27 ++-
 xen/arch/arm/vsmc.c                                |   4 +
 xen/include/asm-arm/platform.h                     |   3 +
 xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 135 +++++++++++++
 7 files changed, 393 insertions(+), 6 deletions(-)
 create mode 100644 xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
 create mode 100644 xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v7 1/6] xen/arm: introduce platform_smc

[Stefano Stabellini]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

From: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Introduce platform_smc as a way to handle firmware calls that Xen does
not know about in a platform specific way. This is particularly useful
for implementing the SiP (SoC implementation specific) service calls.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
Changes in v4:
- add likely
---
 xen/arch/arm/platform.c        | 8 ++++++++
 xen/arch/arm/vsmc.c            | 4 ++++
 xen/include/asm-arm/platform.h | 3 +++
 3 files changed, 15 insertions(+)

diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
index 6989e58..3426056 100644
--- a/xen/arch/arm/platform.c
+++ b/xen/arch/arm/platform.c
@@ -127,6 +127,14 @@ void platform_poweroff(void)
         platform->poweroff();
 }
 
+bool platform_smc(struct cpu_user_regs *regs)
+{
+    if ( likely(platform && platform->smc) )
+        return platform->smc(regs);
+
+    return false;
+}
+
 bool platform_has_quirk(uint32_t quirk)
 {
     uint32_t quirks = 0;
diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index c4ccae6..c72b9a0 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -25,6 +25,7 @@
 #include <asm/smccc.h>
 #include <asm/traps.h>
 #include <asm/vpsci.h>
+#include <asm/platform.h>
 
 /* Number of functions currently supported by Hypervisor Service. */
 #define XEN_SMCCC_FUNCTION_COUNT 3
@@ -272,6 +273,9 @@ static bool vsmccc_handle_call(struct cpu_user_regs *regs)
         case ARM_SMCCC_OWNER_STANDARD:
             handled = handle_sssc(regs);
             break;
+        case ARM_SMCCC_OWNER_SIP:
+            handled = platform_smc(regs);
+            break;
         }
     }
 
diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
index bf92581..ed4d30a 100644
--- a/xen/include/asm-arm/platform.h
+++ b/xen/include/asm-arm/platform.h
@@ -25,6 +25,8 @@ struct platform_desc {
     void (*reset)(void);
     /* Platform power-off */
     void (*poweroff)(void);
+    /* Platform specific SMC handler */
+    bool (*smc)(struct cpu_user_regs *regs);
     /*
      * Platform quirks
      * Defined has a function because a platform can support multiple
@@ -54,6 +56,7 @@ int platform_cpu_up(int cpu);
 #endif
 void platform_reset(void);
 void platform_poweroff(void);
+bool platform_smc(struct cpu_user_regs *regs);
 bool platform_has_quirk(uint32_t quirk);
 bool platform_device_is_blacklisted(const struct dt_device_node *node);
 
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v7 2/6] xen/arm: zynqmp: Forward plaform specific firmware calls

[Stefano Stabellini]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

From: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Introduce zynqmp_eemi: a function responsible for implementing access
controls over the firmware calls. Only calls that are allowed are
forwarded to the firmware.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

---
Changes in v7:
- print a warning once if not smccc 1.1

Changes in v6:
- remove is_domain_64 check
- add check for smccc 1.1
- code style

Changes in v4:
- fix typo
- add header guard
- add emacs magic
- remove #includes that will only be used later
- add copyright notice to header
- remove SMCCC 1.1 check
---
 xen/arch/arm/platforms/Makefile                    |  1 +
 xen/arch/arm/platforms/xilinx-zynqmp-eemi.c        | 34 ++++++++++++++++++++++
 xen/arch/arm/platforms/xilinx-zynqmp.c             | 23 +++++++++++++++
 xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 30 +++++++++++++++++++
 4 files changed, 88 insertions(+)
 create mode 100644 xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
 create mode 100644 xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h

diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
index bd724a1..01608f8 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_ALL_PLAT)   += sunxi.o
 obj-$(CONFIG_ALL64_PLAT) += thunderx.o
 obj-$(CONFIG_ALL64_PLAT) += xgene-storm.o
 obj-$(CONFIG_MPSOC_PLATFORM)  += xilinx-zynqmp.o
+obj-$(CONFIG_MPSOC_PLATFORM)  += xilinx-zynqmp-eemi.o
diff --git a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
new file mode 100644
index 0000000..369bb3f
--- /dev/null
+++ b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
@@ -0,0 +1,34 @@
+/*
+ * xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
+ *
+ * Xilinx ZynqMP EEMI API
+ *
+ * Copyright (c) 2018 Xilinx Inc.
+ * Written by Edgar E. Iglesias <edgar.iglesias@xilinx.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms and conditions of the GNU General Public
+ * License, version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <asm/regs.h>
+#include <asm/platforms/xilinx-zynqmp-eemi.h>
+
+bool zynqmp_eemi(struct cpu_user_regs *regs)
+{
+    return false;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/platforms/xilinx-zynqmp.c b/xen/arch/arm/platforms/xilinx-zynqmp.c
index d8ceded..8bc7a9f 100644
--- a/xen/arch/arm/platforms/xilinx-zynqmp.c
+++ b/xen/arch/arm/platforms/xilinx-zynqmp.c
@@ -18,6 +18,8 @@
  */
 
 #include <asm/platform.h>
+#include <asm/platforms/xilinx-zynqmp-eemi.h>
+#include <asm/smccc.h>
 
 static const char * const zynqmp_dt_compat[] __initconst =
 {
@@ -32,8 +34,29 @@ static const struct dt_device_match zynqmp_blacklist_dev[] __initconst =
     { /* sentinel */ },
 };
 
+static bool zynqmp_smc(struct cpu_user_regs *regs)
+{
+    /*
+     * ZynqMP firmware is based on SMCCC 1.1. If SMCCC 1.1 is not
+     * available something is wrong, don't try to handle it.
+     */
+    if ( !cpus_have_const_cap(ARM_SMCCC_1_1) )
+    {
+        static bool once = true;
+        if ( once )
+        {
+            printk(XENLOG_WARNING "ZynqMP firmware Error: no SMCCC 1.1 "
+                   "support. Disabling firmware calls.");
+            once = false;
+        }
+        return false;
+    }
+    return zynqmp_eemi(regs);
+}
+
 PLATFORM_START(xilinx_zynqmp, "Xilinx ZynqMP")
     .compatible = zynqmp_dt_compat,
+    .smc = zynqmp_smc,
     .blacklist_dev = zynqmp_blacklist_dev,
 PLATFORM_END
 
diff --git a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
new file mode 100644
index 0000000..43cefb5
--- /dev/null
+++ b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2018 Xilinx Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms and conditions of the GNU General Public
+ * License, version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __ASM_ARM_PLATFORMS_ZYNQMP_H
+#define __ASM_ASM_PLATFORMS_ZYNQMP_H
+
+#include <asm/processor.h>
+
+extern bool zynqmp_eemi(struct cpu_user_regs *regs);
+
+#endif /* __ASM_ARM_PLATFORMS_ZYNQMP_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v7 3/6] xen/arm: zynqmp: introduce zynqmp specific defines

[Stefano Stabellini]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

From: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Introduce zynqmp specific defines for the firmware calls.
See EEMI:
https://www.xilinx.com/support/documentation/user_guides/ug1200-eemi-api.pdf

The error codes are described, under XIlPM Error Codes:
https://www.xilinx.com/support/documentation/user_guides/ug1137-zynq-ultrascale-mpsoc-swdev.pdf

- pm_api_id
These are the EEMI function IDs. Unavoidable.

- pm_ret_status
These are the EEMI return statuses. Unavoidable.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

---

Changes in v7:
- introduce EEMI_FID
- remove tabs

Changes in v6:
- improve commit message
- remove MM_*, node ids and reset ids

Changes in v5:
- remove MMIO access related definitions

Changes in v4:
- define PM_MMIO_SHIFT
---
 xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 90 ++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
index 43cefb5..9779b6a 100644
--- a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
+++ b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
@@ -15,6 +15,96 @@
 #define __ASM_ASM_PLATFORMS_ZYNQMP_H
 
 #include <asm/processor.h>
+#include <asm/smccc.h>
+
+/* Service calls.  */
+#define PM_GET_TRUSTZONE_VERSION	0xa03
+
+/* SMC function IDs for SiP Service queries */
+#define ZYNQMP_SIP_SVC_CALL_COUNT       0xff00
+#define ZYNQMP_SIP_SVC_UID              0xff01
+#define ZYNQMP_SIP_SVC_VERSION          0xff03
+
+#define EEMI_FID(fid) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+                                         ARM_SMCCC_CONV_64,   \
+                                         ARM_SMCCC_OWNER_SIP, \
+                                         fid)
+
+enum pm_api_id {
+    /* Miscellaneous API functions: */
+    PM_GET_API_VERSION = 1, /* Do not change or move */
+    PM_SET_CONFIGURATION,
+    PM_GET_NODE_STATUS,
+    PM_GET_OP_CHARACTERISTIC,
+    PM_REGISTER_NOTIFIER,
+    /* API for suspending of PUs: */
+    PM_REQ_SUSPEND,
+    PM_SELF_SUSPEND,
+    PM_FORCE_POWERDOWN,
+    PM_ABORT_SUSPEND,
+    PM_REQ_WAKEUP,
+    PM_SET_WAKEUP_SOURCE,
+    PM_SYSTEM_SHUTDOWN,
+    /* API for managing PM slaves: */
+    PM_REQ_NODE,
+    PM_RELEASE_NODE,
+    PM_SET_REQUIREMENT,
+    PM_SET_MAX_LATENCY,
+    /* Direct control API functions: */
+    PM_RESET_ASSERT,
+    PM_RESET_GET_STATUS,
+    PM_MMIO_WRITE,
+    PM_MMIO_READ,
+    PM_INIT,
+    PM_FPGA_LOAD,
+    PM_FPGA_GET_STATUS,
+    PM_GET_CHIPID,
+    /* ID 25 is been used by U-boot to process secure boot images */
+    /* Secure library generic API functions */
+    PM_SECURE_SHA = 26,
+    PM_SECURE_RSA,
+    /* Pin control API functions */
+    PM_PINCTRL_REQUEST,
+    PM_PINCTRL_RELEASE,
+    PM_PINCTRL_GET_FUNCTION,
+    PM_PINCTRL_SET_FUNCTION,
+    PM_PINCTRL_CONFIG_PARAM_GET,
+    PM_PINCTRL_CONFIG_PARAM_SET,
+    /* PM IOCTL API */
+    PM_IOCTL,
+    /* API to query information from firmware */
+    PM_QUERY_DATA,
+    /* Clock control API functions */
+    PM_CLOCK_ENABLE,
+    PM_CLOCK_DISABLE,
+    PM_CLOCK_GETSTATE,
+    PM_CLOCK_SETDIVIDER,
+    PM_CLOCK_GETDIVIDER,
+    PM_CLOCK_SETRATE,
+    PM_CLOCK_GETRATE,
+    PM_CLOCK_SETPARENT,
+    PM_CLOCK_GETPARENT,
+    PM_API_MAX
+};
+
+/**
+ * @XST_PM_SUCCESS:		Success
+ * @XST_PM_INTERNAL:	Unexpected error
+ * @XST_PM_CONFLICT:	Conflicting requirements
+ * @XST_PM_NO_ACCESS:	Access rights violation
+ * @XST_PM_INVALID_NODE:	Does not apply to node passed as argument
+ * @XST_PM_DOUBLE_REQ:	Duplicate request
+ * @XST_PM_ABORT_SUSPEND:	Target has aborted suspend
+ */
+enum pm_ret_status {
+    XST_PM_SUCCESS = 0,
+    XST_PM_INTERNAL = 2000,
+    XST_PM_CONFLICT,
+    XST_PM_NO_ACCESS,
+    XST_PM_INVALID_NODE,
+    XST_PM_DOUBLE_REQ,
+    XST_PM_ABORT_SUSPEND,
+};
 
 extern bool zynqmp_eemi(struct cpu_user_regs *regs);
 
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v7 4/6] xen/arm: zynqmp: implement zynqmp_eemi

[Stefano Stabellini]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

From: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

zynqmp_eemi uses the defined functions and structs to decide whether to
make a call to the firmware, or to simply return a predefined value.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
---
Changes in v7:
- add in-code comment
- remove tabs
- use EEMI_FID

Changes in v6:
- mmio_access removal moved to previous patch
- forward to firmware mandatory smc32 calls
- check that the function id belongs to the right range before
  proceeding
- basic is_hardware_domain implementation for domain_has_node_access and
  domain_has_reset_access

Changes in v5:
- remove mmio_access handling

Changes in v4:
- add #include as needed
- improve comment
- code style
---
 xen/arch/arm/platforms/xilinx-zynqmp-eemi.c | 171 +++++++++++++++++++++++++++-
 1 file changed, 170 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
index 369bb3f..f2fc5b5 100644
--- a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
+++ b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
@@ -17,11 +17,180 @@
  */
 
 #include <asm/regs.h>
+#include <xen/sched.h>
+#include <asm/smccc.h>
 #include <asm/platforms/xilinx-zynqmp-eemi.h>
 
+/*
+ * EEMI firmware API:
+ * https://www.xilinx.com/support/documentation/user_guides/ug1200-eemi-api.pdf
+ *
+ * Power domain node_ids identify the area of effect of the power
+ * management operations. They are the first parameter passed to power
+ * management EEMI calls.
+ *
+ * Reset IDs identify the area of effect of a reset operation. They are
+ * the first parameter passed to reset EEMI calls.
+ *
+ * For now, let the hardware domain access to all power domain nodes and
+ * all reset lines. In the future, we'll check for ownership of
+ * resources by specific virtual machines.
+ */
+static inline bool domain_has_node_access(struct domain *d, uint32_t nodeid)
+{
+    return is_hardware_domain(d);
+}
+
+static inline bool domain_has_reset_access(struct domain *d, uint32_t rst)
+{
+    return is_hardware_domain(d);
+}
+
 bool zynqmp_eemi(struct cpu_user_regs *regs)
 {
-    return false;
+    struct arm_smccc_res res;
+    uint32_t fid = get_user_reg(regs, 0);
+    uint32_t nodeid = get_user_reg(regs, 1);
+    unsigned int pm_fn = fid & 0xFFFF;
+    enum pm_ret_status ret;
+
+    switch ( fid )
+    {
+    /* Mandatory SMC32 functions. */
+    case ARM_SMCCC_CALL_COUNT_FID(SIP):
+    case ARM_SMCCC_CALL_UID_FID(SIP):
+    case ARM_SMCCC_REVISION_FID(SIP):
+        goto forward_to_fw;
+    /*
+     * We can't allow CPUs to suspend without Xen knowing about it.
+     * We accept but ignore the request and wait for the guest to issue
+     * a WFI or PSCI call which Xen will trap and act accordingly upon.
+     */
+    case EEMI_FID(PM_SELF_SUSPEND):
+        ret = XST_PM_SUCCESS;
+        goto done;
+
+    case EEMI_FID(PM_GET_NODE_STATUS):
+    /* API for PUs.  */
+    case EEMI_FID(PM_REQ_SUSPEND):
+    case EEMI_FID(PM_FORCE_POWERDOWN):
+    case EEMI_FID(PM_ABORT_SUSPEND):
+    case EEMI_FID(PM_REQ_WAKEUP):
+    case EEMI_FID(PM_SET_WAKEUP_SOURCE):
+    /* API for slaves.  */
+    case EEMI_FID(PM_REQ_NODE):
+    case EEMI_FID(PM_RELEASE_NODE):
+    case EEMI_FID(PM_SET_REQUIREMENT):
+    case EEMI_FID(PM_SET_MAX_LATENCY):
+        if ( !domain_has_node_access(current->domain, nodeid) )
+        {
+            gprintk(XENLOG_WARNING,
+                    "zynqmp-pm: fn=%u No access to node %u\n", pm_fn, nodeid);
+            ret = XST_PM_NO_ACCESS;
+            goto done;
+        }
+        goto forward_to_fw;
+
+    case EEMI_FID(PM_RESET_ASSERT):
+    case EEMI_FID(PM_RESET_GET_STATUS):
+        if ( !domain_has_reset_access(current->domain, nodeid) )
+        {
+            gprintk(XENLOG_WARNING,
+                    "zynqmp-pm: fn=%u No access to reset %u\n", pm_fn, nodeid);
+            ret = XST_PM_NO_ACCESS;
+            goto done;
+        }
+        goto forward_to_fw;
+
+    /* These calls are safe and always allowed.  */
+    case EEMI_FID(ZYNQMP_SIP_SVC_CALL_COUNT):
+    case EEMI_FID(ZYNQMP_SIP_SVC_UID):
+    case EEMI_FID(ZYNQMP_SIP_SVC_VERSION):
+    case EEMI_FID(PM_GET_TRUSTZONE_VERSION):
+    case EEMI_FID(PM_GET_API_VERSION):
+    case EEMI_FID(PM_GET_CHIPID):
+        goto forward_to_fw;
+
+    /* No MMIO access is allowed from non-secure domains */
+    case EEMI_FID(PM_MMIO_WRITE):
+    case EEMI_FID(PM_MMIO_READ):
+        gprintk(XENLOG_WARNING,
+                "zynqmp-pm: fn=%u No MMIO access to %u\n", pm_fn, nodeid);
+        ret = XST_PM_NO_ACCESS;
+        goto done;
+
+    /* Exclusive to the hardware domain.  */
+    case EEMI_FID(PM_INIT):
+    case EEMI_FID(PM_SET_CONFIGURATION):
+    case EEMI_FID(PM_FPGA_LOAD):
+    case EEMI_FID(PM_FPGA_GET_STATUS):
+    case EEMI_FID(PM_SECURE_SHA):
+    case EEMI_FID(PM_SECURE_RSA):
+    case EEMI_FID(PM_PINCTRL_SET_FUNCTION):
+    case EEMI_FID(PM_PINCTRL_REQUEST):
+    case EEMI_FID(PM_PINCTRL_RELEASE):
+    case EEMI_FID(PM_PINCTRL_GET_FUNCTION):
+    case EEMI_FID(PM_PINCTRL_CONFIG_PARAM_GET):
+    case EEMI_FID(PM_PINCTRL_CONFIG_PARAM_SET):
+    case EEMI_FID(PM_IOCTL):
+    case EEMI_FID(PM_QUERY_DATA):
+    case EEMI_FID(PM_CLOCK_ENABLE):
+    case EEMI_FID(PM_CLOCK_DISABLE):
+    case EEMI_FID(PM_CLOCK_GETSTATE):
+    case EEMI_FID(PM_CLOCK_GETDIVIDER):
+    case EEMI_FID(PM_CLOCK_SETDIVIDER):
+    case EEMI_FID(PM_CLOCK_SETRATE):
+    case EEMI_FID(PM_CLOCK_GETRATE):
+    case EEMI_FID(PM_CLOCK_SETPARENT):
+    case EEMI_FID(PM_CLOCK_GETPARENT):
+        if ( !is_hardware_domain(current->domain) )
+        {
+            gprintk(XENLOG_WARNING, "eemi: fn=%u No access", pm_fn);
+            ret = XST_PM_NO_ACCESS;
+            goto done;
+        }
+        goto forward_to_fw;
+
+    /* These calls are never allowed.  */
+    case EEMI_FID(PM_SYSTEM_SHUTDOWN):
+        ret = XST_PM_NO_ACCESS;
+        goto done;
+
+    default:
+        gprintk(XENLOG_WARNING, "zynqmp-pm: Unhandled PM Call: %u\n", fid);
+        return false;
+    }
+
+forward_to_fw:
+    /*
+     * ZynqMP firmware calls (EEMI) take an argument that specifies the
+     * area of effect of the function called. Specifically, node ids for
+     * power management functions and reset ids for reset functions.
+     *
+     * The code above checks if a virtual machine has access rights over
+     * the node id, reset id, etc. Now that the check has been done, we
+     * can forward the whole command to firmware without additional
+     * parameters checks.
+     */
+    arm_smccc_1_1_smc(get_user_reg(regs, 0),
+                      get_user_reg(regs, 1),
+                      get_user_reg(regs, 2),
+                      get_user_reg(regs, 3),
+                      get_user_reg(regs, 4),
+                      get_user_reg(regs, 5),
+                      get_user_reg(regs, 6),
+                      get_user_reg(regs, 7),
+                      &res);
+
+    set_user_reg(regs, 0, res.a0);
+    set_user_reg(regs, 1, res.a1);
+    set_user_reg(regs, 2, res.a2);
+    set_user_reg(regs, 3, res.a3);
+    return true;
+
+done:
+    set_user_reg(regs, 0, ret);
+    return true;
 }
 
 /*
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v7 5/6] xen/arm: zynqmp: Remove blacklist of ZynqMP's PM node

[Stefano Stabellini]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

From: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Stop blacklisting ZynqMP's power management node. It is now possible
since we allow the hardware domain to issue HVC/SMC calls to firmware.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/platforms/xilinx-zynqmp.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/xen/arch/arm/platforms/xilinx-zynqmp.c b/xen/arch/arm/platforms/xilinx-zynqmp.c
index 8bc7a9f..d14a999 100644
--- a/xen/arch/arm/platforms/xilinx-zynqmp.c
+++ b/xen/arch/arm/platforms/xilinx-zynqmp.c
@@ -27,13 +27,6 @@ static const char * const zynqmp_dt_compat[] __initconst =
     NULL
 };
 
-static const struct dt_device_match zynqmp_blacklist_dev[] __initconst =
-{
-    /* Power management is not yet supported.  */
-    DT_MATCH_COMPATIBLE("xlnx,zynqmp-pm"),
-    { /* sentinel */ },
-};
-
 static bool zynqmp_smc(struct cpu_user_regs *regs)
 {
     /*
@@ -57,7 +50,6 @@ static bool zynqmp_smc(struct cpu_user_regs *regs)
 PLATFORM_START(xilinx_zynqmp, "Xilinx ZynqMP")
     .compatible = zynqmp_dt_compat,
     .smc = zynqmp_smc,
-    .blacklist_dev = zynqmp_blacklist_dev,
 PLATFORM_END
 
 /*
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v7 6/6] xen/zynqmp: add IPI calls virtualization

[Stefano Stabellini]

ZynqMP IPI mailbox calls are a small set of EEMI sister calls, often
used in conjunction with EEMI related functionalities.

Unfortunately they are not part of the EEMI spec, or any other public
spec, but the implementation is upstream in ATF:

https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/xilinx/zynqmp/ipi_mailbox_service/ipi_mailbox_svc.h

And patches are close to getting into Linux:

https://patchwork.kernel.org/cover/10689501/

Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

---

Changes in v7:
- add IPI_MAILBOX_FID and use it
- remove tabs

Changes in v6:
- new patch
---
 xen/arch/arm/platforms/xilinx-zynqmp-eemi.c        | 18 ++++++++++++++++++
 xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 15 +++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
index f2fc5b5..7cd3936 100644
--- a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
+++ b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
@@ -25,6 +25,9 @@
  * EEMI firmware API:
  * https://www.xilinx.com/support/documentation/user_guides/ug1200-eemi-api.pdf
  *
+ * IPI firmware API:
+ * https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/xilinx/zynqmp/ipi_mailbox_service/ipi_mailbox_svc.h
+ *
  * Power domain node_ids identify the area of effect of the power
  * management operations. They are the first parameter passed to power
  * management EEMI calls.
@@ -156,6 +159,21 @@ bool zynqmp_eemi(struct cpu_user_regs *regs)
         ret = XST_PM_NO_ACCESS;
         goto done;
 
+    case IPI_MAILBOX_FID(IPI_MAILBOX_OPEN):
+    case IPI_MAILBOX_FID(IPI_MAILBOX_RELEASE):
+    case IPI_MAILBOX_FID(IPI_MAILBOX_STATUS_ENQUIRY):
+    case IPI_MAILBOX_FID(IPI_MAILBOX_NOTIFY):
+    case IPI_MAILBOX_FID(IPI_MAILBOX_ACK):
+    case IPI_MAILBOX_FID(IPI_MAILBOX_ENABLE_IRQ):
+    case IPI_MAILBOX_FID(IPI_MAILBOX_DISABLE_IRQ):
+        if ( !is_hardware_domain(current->domain) )
+        {
+            gprintk(XENLOG_WARNING, "IPI mailbox: fn=%u No access", pm_fn);
+            ret = XST_PM_NO_ACCESS;
+            goto done;
+        }
+        goto forward_to_fw;
+
     default:
         gprintk(XENLOG_WARNING, "zynqmp-pm: Unhandled PM Call: %u\n", fid);
         return false;
diff --git a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
index 9779b6a..c27fec6 100644
--- a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
+++ b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
@@ -106,6 +106,21 @@ enum pm_ret_status {
     XST_PM_ABORT_SUSPEND,
 };
 
+/* IPI SMC function numbers enum definition and fids */
+#define IPI_MAILBOX_FID(fid) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+                                                ARM_SMCCC_CONV_32,   \
+                                                ARM_SMCCC_OWNER_SIP, \
+                                                fid)
+enum ipi_api_id {
+    IPI_MAILBOX_OPEN = 0x1000,
+    IPI_MAILBOX_RELEASE,
+    IPI_MAILBOX_STATUS_ENQUIRY,
+    IPI_MAILBOX_NOTIFY,
+    IPI_MAILBOX_ACK,
+    IPI_MAILBOX_ENABLE_IRQ,
+    IPI_MAILBOX_DISABLE_IRQ,
+};
+
 extern bool zynqmp_eemi(struct cpu_user_regs *regs);
 
 #endif /* __ASM_ARM_PLATFORMS_ZYNQMP_H */
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v7 2/6] xen/arm: zynqmp: Forward plaform specific firmware calls

[Julien Grall]

Hi Stefano,

On 12/17/18 10:10 PM, Stefano Stabellini wrote:
> diff --git a/xen/arch/arm/platforms/xilinx-zynqmp.c b/xen/arch/arm/platforms/xilinx-zynqmp.c
> index d8ceded..8bc7a9f 100644
> --- a/xen/arch/arm/platforms/xilinx-zynqmp.c
> +++ b/xen/arch/arm/platforms/xilinx-zynqmp.c
> @@ -18,6 +18,8 @@
>    */
>   
>   #include <asm/platform.h>
> +#include <asm/platforms/xilinx-zynqmp-eemi.h>
> +#include <asm/smccc.h>
>   
>   static const char * const zynqmp_dt_compat[] __initconst =
>   {
> @@ -32,8 +34,29 @@ static const struct dt_device_match zynqmp_blacklist_dev[] __initconst =
>       { /* sentinel */ },
>   };
>   
> +static bool zynqmp_smc(struct cpu_user_regs *regs)
> +{
> +    /*
> +     * ZynqMP firmware is based on SMCCC 1.1. If SMCCC 1.1 is not
> +     * available something is wrong, don't try to handle it.
> +     */
> +    if ( !cpus_have_const_cap(ARM_SMCCC_1_1) )
> +    {
> +        static bool once = true;

NIT: Newline here please.

With that:

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v7 3/6] xen/arm: zynqmp: introduce zynqmp specific defines

[Julien Grall]

Hi Stefano,

On 12/17/18 10:10 PM, Stefano Stabellini wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> From: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> 
> Introduce zynqmp specific defines for the firmware calls.
> See EEMI:
> https://www.xilinx.com/support/documentation/user_guides/ug1200-eemi-api.pdf
> 
> The error codes are described, under XIlPM Error Codes:
> https://www.xilinx.com/support/documentation/user_guides/ug1137-zynq-ultrascale-mpsoc-swdev.pdf
> 
> - pm_api_id
> These are the EEMI function IDs. Unavoidable.
> 
> - pm_ret_status
> These are the EEMI return statuses. Unavoidable.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> 
> ---
> 
> Changes in v7:
> - introduce EEMI_FID
> - remove tabs
> 
> Changes in v6:
> - improve commit message
> - remove MM_*, node ids and reset ids
> 
> Changes in v5:
> - remove MMIO access related definitions
> 
> Changes in v4:
> - define PM_MMIO_SHIFT
> ---
>   xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 90 ++++++++++++++++++++++
>   1 file changed, 90 insertions(+)
> 
> diff --git a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> index 43cefb5..9779b6a 100644
> --- a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> +++ b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> @@ -15,6 +15,96 @@
>   #define __ASM_ASM_PLATFORMS_ZYNQMP_H
>   
>   #include <asm/processor.h>
> +#include <asm/smccc.h>
> +
> +/* Service calls.  */
> +#define PM_GET_TRUSTZONE_VERSION	0xa03

Why does not this belong to the pm_api_id below?

> +
> +/* SMC function IDs for SiP Service queries */
> +#define ZYNQMP_SIP_SVC_CALL_COUNT       0xff00
> +#define ZYNQMP_SIP_SVC_UID              0xff01
> +#define ZYNQMP_SIP_SVC_VERSION          0xff03 > +
> +#define EEMI_FID(fid) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> +                                         ARM_SMCCC_CONV_64,   \
> +                                         ARM_SMCCC_OWNER_SIP, \
> +                                         fid)
> +
> +enum pm_api_id {
> +    /* Miscellaneous API functions: */
> +    PM_GET_API_VERSION = 1, /* Do not change or move */
> +    PM_SET_CONFIGURATION,
> +    PM_GET_NODE_STATUS,
> +    PM_GET_OP_CHARACTERISTIC,
> +    PM_REGISTER_NOTIFIER,
> +    /* API for suspending of PUs: */
> +    PM_REQ_SUSPEND,
> +    PM_SELF_SUSPEND,
> +    PM_FORCE_POWERDOWN,
> +    PM_ABORT_SUSPEND,
> +    PM_REQ_WAKEUP,
> +    PM_SET_WAKEUP_SOURCE,
> +    PM_SYSTEM_SHUTDOWN,
> +    /* API for managing PM slaves: */
> +    PM_REQ_NODE,
> +    PM_RELEASE_NODE,
> +    PM_SET_REQUIREMENT,
> +    PM_SET_MAX_LATENCY,
> +    /* Direct control API functions: */
> +    PM_RESET_ASSERT,
> +    PM_RESET_GET_STATUS,
> +    PM_MMIO_WRITE,
> +    PM_MMIO_READ,
> +    PM_INIT,
> +    PM_FPGA_LOAD,
> +    PM_FPGA_GET_STATUS,
> +    PM_GET_CHIPID,
> +    /* ID 25 is been used by U-boot to process secure boot images */
> +    /* Secure library generic API functions */
> +    PM_SECURE_SHA = 26,
> +    PM_SECURE_RSA,
> +    /* Pin control API functions */
> +    PM_PINCTRL_REQUEST,
> +    PM_PINCTRL_RELEASE,
> +    PM_PINCTRL_GET_FUNCTION,
> +    PM_PINCTRL_SET_FUNCTION,
> +    PM_PINCTRL_CONFIG_PARAM_GET,
> +    PM_PINCTRL_CONFIG_PARAM_SET,
> +    /* PM IOCTL API */
> +    PM_IOCTL,
> +    /* API to query information from firmware */
> +    PM_QUERY_DATA,
> +    /* Clock control API functions */
> +    PM_CLOCK_ENABLE,
> +    PM_CLOCK_DISABLE,
> +    PM_CLOCK_GETSTATE,
> +    PM_CLOCK_SETDIVIDER,
> +    PM_CLOCK_GETDIVIDER,
> +    PM_CLOCK_SETRATE,
> +    PM_CLOCK_GETRATE,
> +    PM_CLOCK_SETPARENT,
> +    PM_CLOCK_GETPARENT,
> +    PM_API_MAX
> +};
> +
> +/**
> + * @XST_PM_SUCCESS:		Success
> + * @XST_PM_INTERNAL:	Unexpected error
> + * @XST_PM_CONFLICT:	Conflicting requirements
> + * @XST_PM_NO_ACCESS:	Access rights violation
> + * @XST_PM_INVALID_NODE:	Does not apply to node passed as argument
> + * @XST_PM_DOUBLE_REQ:	Duplicate request
> + * @XST_PM_ABORT_SUSPEND:	Target has aborted suspend
> + */
> +enum pm_ret_status {
> +    XST_PM_SUCCESS = 0,
> +    XST_PM_INTERNAL = 2000,
> +    XST_PM_CONFLICT,
> +    XST_PM_NO_ACCESS,
> +    XST_PM_INVALID_NODE,
> +    XST_PM_DOUBLE_REQ,
> +    XST_PM_ABORT_SUSPEND,
> +};
>   
>   extern bool zynqmp_eemi(struct cpu_user_regs *regs);
>   
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v7 4/6] xen/arm: zynqmp: implement zynqmp_eemi

[Julien Grall]

Hi Stefano,

On 12/17/18 10:10 PM, Stefano Stabellini wrote:
> +    /* These calls are safe and always allowed.  */
> +    case EEMI_FID(ZYNQMP_SIP_SVC_CALL_COUNT):
> +    case EEMI_FID(ZYNQMP_SIP_SVC_UID):
> +    case EEMI_FID(ZYNQMP_SIP_SVC_VERSION):

I am a bit surprised that you implement those one using SMC64. Why would 
you duplicate the SMC32 version (ARM_SMCCC_CALL_COUNT_FID(SIP))?

I also don't seem to find them neither in the spec nor in the ATF code.

The rest of the code looks good to me.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v7 5/6] xen/arm: zynqmp: Remove blacklist of ZynqMP's PM node

[Julien Grall]

Hi Stefano,

On 12/17/18 10:10 PM, Stefano Stabellini wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> From: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> 
> Stop blacklisting ZynqMP's power management node. It is now possible
> since we allow the hardware domain to issue HVC/SMC calls to firmware.
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v7 6/6] xen/zynqmp: add IPI calls virtualization

[Julien Grall]

Hi Stefano,

On 12/17/18 10:10 PM, Stefano Stabellini wrote:
> ZynqMP IPI mailbox calls are a small set of EEMI sister calls, often
> used in conjunction with EEMI related functionalities.
> 
> Unfortunately they are not part of the EEMI spec, or any other public
> spec, but the implementation is upstream in ATF:
> 
> https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/xilinx/zynqmp/ipi_mailbox_service/ipi_mailbox_svc.h
> 
> And patches are close to getting into Linux:
> 
> https://patchwork.kernel.org/cover/10689501/
> 
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

> 
> ---
> 
> Changes in v7:
> - add IPI_MAILBOX_FID and use it
> - remove tabs
> 
> Changes in v6:
> - new patch
> ---
>   xen/arch/arm/platforms/xilinx-zynqmp-eemi.c        | 18 ++++++++++++++++++
>   xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 15 +++++++++++++++
>   2 files changed, 33 insertions(+)
> 
> diff --git a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> index f2fc5b5..7cd3936 100644
> --- a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> +++ b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> @@ -25,6 +25,9 @@
>    * EEMI firmware API:
>    * https://www.xilinx.com/support/documentation/user_guides/ug1200-eemi-api.pdf
>    *
> + * IPI firmware API:
> + * https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/xilinx/zynqmp/ipi_mailbox_service/ipi_mailbox_svc.h
> + *
>    * Power domain node_ids identify the area of effect of the power
>    * management operations. They are the first parameter passed to power
>    * management EEMI calls.
> @@ -156,6 +159,21 @@ bool zynqmp_eemi(struct cpu_user_regs *regs)
>           ret = XST_PM_NO_ACCESS;
>           goto done;
>   
> +    case IPI_MAILBOX_FID(IPI_MAILBOX_OPEN):
> +    case IPI_MAILBOX_FID(IPI_MAILBOX_RELEASE):
> +    case IPI_MAILBOX_FID(IPI_MAILBOX_STATUS_ENQUIRY):
> +    case IPI_MAILBOX_FID(IPI_MAILBOX_NOTIFY):
> +    case IPI_MAILBOX_FID(IPI_MAILBOX_ACK):
> +    case IPI_MAILBOX_FID(IPI_MAILBOX_ENABLE_IRQ):
> +    case IPI_MAILBOX_FID(IPI_MAILBOX_DISABLE_IRQ):
> +        if ( !is_hardware_domain(current->domain) )
> +        {
> +            gprintk(XENLOG_WARNING, "IPI mailbox: fn=%u No access", pm_fn);
> +            ret = XST_PM_NO_ACCESS;
> +            goto done;
> +        }
> +        goto forward_to_fw;
> +
>       default:
>           gprintk(XENLOG_WARNING, "zynqmp-pm: Unhandled PM Call: %u\n", fid);
>           return false;
> diff --git a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> index 9779b6a..c27fec6 100644
> --- a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> +++ b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> @@ -106,6 +106,21 @@ enum pm_ret_status {
>       XST_PM_ABORT_SUSPEND,
>   };
>   
> +/* IPI SMC function numbers enum definition and fids */
> +#define IPI_MAILBOX_FID(fid) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> +                                                ARM_SMCCC_CONV_32,   \
> +                                                ARM_SMCCC_OWNER_SIP, \
> +                                                fid)
> +enum ipi_api_id {
> +    IPI_MAILBOX_OPEN = 0x1000,
> +    IPI_MAILBOX_RELEASE,
> +    IPI_MAILBOX_STATUS_ENQUIRY,
> +    IPI_MAILBOX_NOTIFY,
> +    IPI_MAILBOX_ACK,
> +    IPI_MAILBOX_ENABLE_IRQ,
> +    IPI_MAILBOX_DISABLE_IRQ,
> +};
> +
>   extern bool zynqmp_eemi(struct cpu_user_regs *regs);
>   
>   #endif /* __ASM_ARM_PLATFORMS_ZYNQMP_H */
> 

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v7 4/6] xen/arm: zynqmp: implement zynqmp_eemi

[Julien Grall]

Hi,

On 12/17/18 10:10 PM, Stefano Stabellini wrote:
> +    /* These calls are safe and always allowed.  */
> +    case EEMI_FID(ZYNQMP_SIP_SVC_CALL_COUNT):
> +    case EEMI_FID(ZYNQMP_SIP_SVC_UID):
> +    case EEMI_FID(ZYNQMP_SIP_SVC_VERSION):
> +    case EEMI_FID(PM_GET_TRUSTZONE_VERSION):
> +    case EEMI_FID(PM_GET_API_VERSION):

Above you say the call to PM_GET_API_VERSION are safe and always 
allowed. But looking at the ATF implementation the first call to 
PM_GET_API_VERSION will enable IPI IRQ.

AFAICT, Dom0 will be the only domain to access IPI. So what happen if, 
in the Dom0less case, the guest is booting before and calling 
PM_GET_API_VERSION?

I haven't looked in depth the other SIP functions to see whether there 
are other potential issue.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v7 3/6] xen/arm: zynqmp: introduce zynqmp specific defines

[Stefano Stabellini]

On Tue, 18 Dec 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 12/17/18 10:10 PM, Stefano Stabellini wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> > 
> > From: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > 
> > Introduce zynqmp specific defines for the firmware calls.
> > See EEMI:
> > https://www.xilinx.com/support/documentation/user_guides/ug1200-eemi-api.pdf
> > 
> > The error codes are described, under XIlPM Error Codes:
> > https://www.xilinx.com/support/documentation/user_guides/ug1137-zynq-ultrascale-mpsoc-swdev.pdf
> > 
> > - pm_api_id
> > These are the EEMI function IDs. Unavoidable.
> > 
> > - pm_ret_status
> > These are the EEMI return statuses. Unavoidable.
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> > 
> > ---
> > 
> > Changes in v7:
> > - introduce EEMI_FID
> > - remove tabs
> > 
> > Changes in v6:
> > - improve commit message
> > - remove MM_*, node ids and reset ids
> > 
> > Changes in v5:
> > - remove MMIO access related definitions
> > 
> > Changes in v4:
> > - define PM_MMIO_SHIFT
> > ---
> >   xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 90
> > ++++++++++++++++++++++
> >   1 file changed, 90 insertions(+)
> > 
> > diff --git a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> > b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> > index 43cefb5..9779b6a 100644
> > --- a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> > +++ b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
> > @@ -15,6 +15,96 @@
> >   #define __ASM_ASM_PLATFORMS_ZYNQMP_H
> >     #include <asm/processor.h>
> > +#include <asm/smccc.h>
> > +
> > +/* Service calls.  */
> > +#define PM_GET_TRUSTZONE_VERSION	0xa03
> 
> Why does not this belong to the pm_api_id below?

You are right, I'll move it below into the enum.


> > +
> > +/* SMC function IDs for SiP Service queries */
> > +#define ZYNQMP_SIP_SVC_CALL_COUNT       0xff00
> > +#define ZYNQMP_SIP_SVC_UID              0xff01
> > +#define ZYNQMP_SIP_SVC_VERSION          0xff03 > +
> > +#define EEMI_FID(fid) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> > +                                         ARM_SMCCC_CONV_64,   \
> > +                                         ARM_SMCCC_OWNER_SIP, \
> > +                                         fid)
> > +
> > +enum pm_api_id {
> > +    /* Miscellaneous API functions: */
> > +    PM_GET_API_VERSION = 1, /* Do not change or move */
> > +    PM_SET_CONFIGURATION,
> > +    PM_GET_NODE_STATUS,
> > +    PM_GET_OP_CHARACTERISTIC,
> > +    PM_REGISTER_NOTIFIER,
> > +    /* API for suspending of PUs: */
> > +    PM_REQ_SUSPEND,
> > +    PM_SELF_SUSPEND,
> > +    PM_FORCE_POWERDOWN,
> > +    PM_ABORT_SUSPEND,
> > +    PM_REQ_WAKEUP,
> > +    PM_SET_WAKEUP_SOURCE,
> > +    PM_SYSTEM_SHUTDOWN,
> > +    /* API for managing PM slaves: */
> > +    PM_REQ_NODE,
> > +    PM_RELEASE_NODE,
> > +    PM_SET_REQUIREMENT,
> > +    PM_SET_MAX_LATENCY,
> > +    /* Direct control API functions: */
> > +    PM_RESET_ASSERT,
> > +    PM_RESET_GET_STATUS,
> > +    PM_MMIO_WRITE,
> > +    PM_MMIO_READ,
> > +    PM_INIT,
> > +    PM_FPGA_LOAD,
> > +    PM_FPGA_GET_STATUS,
> > +    PM_GET_CHIPID,
> > +    /* ID 25 is been used by U-boot to process secure boot images */
> > +    /* Secure library generic API functions */
> > +    PM_SECURE_SHA = 26,
> > +    PM_SECURE_RSA,
> > +    /* Pin control API functions */
> > +    PM_PINCTRL_REQUEST,
> > +    PM_PINCTRL_RELEASE,
> > +    PM_PINCTRL_GET_FUNCTION,
> > +    PM_PINCTRL_SET_FUNCTION,
> > +    PM_PINCTRL_CONFIG_PARAM_GET,
> > +    PM_PINCTRL_CONFIG_PARAM_SET,
> > +    /* PM IOCTL API */
> > +    PM_IOCTL,
> > +    /* API to query information from firmware */
> > +    PM_QUERY_DATA,
> > +    /* Clock control API functions */
> > +    PM_CLOCK_ENABLE,
> > +    PM_CLOCK_DISABLE,
> > +    PM_CLOCK_GETSTATE,
> > +    PM_CLOCK_SETDIVIDER,
> > +    PM_CLOCK_GETDIVIDER,
> > +    PM_CLOCK_SETRATE,
> > +    PM_CLOCK_GETRATE,
> > +    PM_CLOCK_SETPARENT,
> > +    PM_CLOCK_GETPARENT,
> > +    PM_API_MAX
> > +};
> > +
> > +/**
> > + * @XST_PM_SUCCESS:		Success
> > + * @XST_PM_INTERNAL:	Unexpected error
> > + * @XST_PM_CONFLICT:	Conflicting requirements
> > + * @XST_PM_NO_ACCESS:	Access rights violation
> > + * @XST_PM_INVALID_NODE:	Does not apply to node passed as argument
> > + * @XST_PM_DOUBLE_REQ:	Duplicate request
> > + * @XST_PM_ABORT_SUSPEND:	Target has aborted suspend
> > + */
> > +enum pm_ret_status {
> > +    XST_PM_SUCCESS = 0,
> > +    XST_PM_INTERNAL = 2000,
> > +    XST_PM_CONFLICT,
> > +    XST_PM_NO_ACCESS,
> > +    XST_PM_INVALID_NODE,
> > +    XST_PM_DOUBLE_REQ,
> > +    XST_PM_ABORT_SUSPEND,
> > +};
> >     extern bool zynqmp_eemi(struct cpu_user_regs *regs);

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v7 4/6] xen/arm: zynqmp: implement zynqmp_eemi

[Stefano Stabellini]

On Tue, 18 Dec 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 12/17/18 10:10 PM, Stefano Stabellini wrote:
> > +    /* These calls are safe and always allowed.  */
> > +    case EEMI_FID(ZYNQMP_SIP_SVC_CALL_COUNT):
> > +    case EEMI_FID(ZYNQMP_SIP_SVC_UID):
> > +    case EEMI_FID(ZYNQMP_SIP_SVC_VERSION):
> 
> I am a bit surprised that you implement those one using SMC64. Why would you
> duplicate the SMC32 version (ARM_SMCCC_CALL_COUNT_FID(SIP))?

This is a mistake introduced in the last couple of versions, good catch.
ZYNQMP_SIP_SVC_* were preexisting, they go back to the initial
implementation by Edgar. The way the code was written before, it didn't
matter if the call was SMC32 or SMC64. Now that we match on the full
FID, and that we have the right SMC32 calls handled, I'll remove these 3
cases completely.


> I also don't seem to find them neither in the spec nor in the ATF code.

Yes, I raised the issue internally. In any case, I think it makes sense
to handle the mandatory calls in Xen by forwarding them to firmware.


> The rest of the code looks good to me.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v7 4/6] xen/arm: zynqmp: implement zynqmp_eemi

[Stefano Stabellini]

On Tue, 18 Dec 2018, Julien Grall wrote:
> Hi,
> 
> On 12/17/18 10:10 PM, Stefano Stabellini wrote:
> > +    /* These calls are safe and always allowed.  */
> > +    case EEMI_FID(ZYNQMP_SIP_SVC_CALL_COUNT):
> > +    case EEMI_FID(ZYNQMP_SIP_SVC_UID):
> > +    case EEMI_FID(ZYNQMP_SIP_SVC_VERSION):
> > +    case EEMI_FID(PM_GET_TRUSTZONE_VERSION):
> > +    case EEMI_FID(PM_GET_API_VERSION):
> 
> Above you say the call to PM_GET_API_VERSION are safe and always allowed. But
> looking at the ATF implementation the first call to PM_GET_API_VERSION will
> enable IPI IRQ.
> 
> AFAICT, Dom0 will be the only domain to access IPI. So what happen if, in the
> Dom0less case, the guest is booting before and calling PM_GET_API_VERSION?
> 
> I haven't looked in depth the other SIP functions to see whether there are
> other potential issue.

On Xilinx MPSoC, the power management handler runs on a separate
processor (a Microblaze processor). Xilinx calls it "PMU". The IPI IRQ
enabled by ATF is for it to communicate with the PMU, it should not be
exposed to virtual machines. Nothing to do on our side here.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v7 4/6] xen/arm: zynqmp: implement zynqmp_eemi

[Julien Grall]

Hi Stefano,

On 18/12/2018 22:36, Stefano Stabellini wrote:
> On Tue, 18 Dec 2018, Julien Grall wrote:
>> Hi,
>>
>> On 12/17/18 10:10 PM, Stefano Stabellini wrote:
>>> +    /* These calls are safe and always allowed.  */
>>> +    case EEMI_FID(ZYNQMP_SIP_SVC_CALL_COUNT):
>>> +    case EEMI_FID(ZYNQMP_SIP_SVC_UID):
>>> +    case EEMI_FID(ZYNQMP_SIP_SVC_VERSION):
>>> +    case EEMI_FID(PM_GET_TRUSTZONE_VERSION):
>>> +    case EEMI_FID(PM_GET_API_VERSION):
>>
>> Above you say the call to PM_GET_API_VERSION are safe and always allowed. But
>> looking at the ATF implementation the first call to PM_GET_API_VERSION will
>> enable IPI IRQ.
>>
>> AFAICT, Dom0 will be the only domain to access IPI. So what happen if, in the
>> Dom0less case, the guest is booting before and calling PM_GET_API_VERSION?
>>
>> I haven't looked in depth the other SIP functions to see whether there are
>> other potential issue.
> 
> On Xilinx MPSoC, the power management handler runs on a separate
> processor (a Microblaze processor). Xilinx calls it "PMU". The IPI IRQ
> enabled by ATF is for it to communicate with the PMU, it should not be
> exposed to virtual machines. Nothing to do on our side here.

I am a bit confused, this does not seems to match the comment in the ATF code:
		/*
		 * Enable IPI IRQ
		 * assume the rich OS is OK to handle callback IRQs now.
		 * Even if we were wrong, it would not enable the IRQ in
		 * the GIC.
		 */

What would happen if a guest is calling PM_GET_API_VERSION and we are not ready 
to handle callback?

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v7 4/6] xen/arm: zynqmp: implement zynqmp_eemi

[Stefano Stabellini]

On Wed, 19 Dec 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 18/12/2018 22:36, Stefano Stabellini wrote:
> > On Tue, 18 Dec 2018, Julien Grall wrote:
> > > Hi,
> > > 
> > > On 12/17/18 10:10 PM, Stefano Stabellini wrote:
> > > > +    /* These calls are safe and always allowed.  */
> > > > +    case EEMI_FID(ZYNQMP_SIP_SVC_CALL_COUNT):
> > > > +    case EEMI_FID(ZYNQMP_SIP_SVC_UID):
> > > > +    case EEMI_FID(ZYNQMP_SIP_SVC_VERSION):
> > > > +    case EEMI_FID(PM_GET_TRUSTZONE_VERSION):
> > > > +    case EEMI_FID(PM_GET_API_VERSION):
> > > 
> > > Above you say the call to PM_GET_API_VERSION are safe and always allowed.
> > > But
> > > looking at the ATF implementation the first call to PM_GET_API_VERSION
> > > will
> > > enable IPI IRQ.
> > > 
> > > AFAICT, Dom0 will be the only domain to access IPI. So what happen if, in
> > > the
> > > Dom0less case, the guest is booting before and calling PM_GET_API_VERSION?
> > > 
> > > I haven't looked in depth the other SIP functions to see whether there are
> > > other potential issue.
> > 
> > On Xilinx MPSoC, the power management handler runs on a separate
> > processor (a Microblaze processor). Xilinx calls it "PMU". The IPI IRQ
> > enabled by ATF is for it to communicate with the PMU, it should not be
> > exposed to virtual machines. Nothing to do on our side here.
> 
> I am a bit confused, this does not seems to match the comment in the ATF code:
> 		/*
> 		 * Enable IPI IRQ
> 		 * assume the rich OS is OK to handle callback IRQs now.
> 		 * Even if we were wrong, it would not enable the IRQ in
> 		 * the GIC.
> 		 */
> 
> What would happen if a guest is calling PM_GET_API_VERSION and we are not
> ready to handle callback?

CC'ing Wendy that knows more than me about this.

This comment is truly confusing. As I wrote, the IPI IRQ is used by ATF
to communicate with the PMU. However, ATF can also be configured not to
use the IPI IRQ by disabling the flag ZYNQMP_WDT_RESTART. In that case,
an OS could enable and use the IPI IRQ for itself. Note that upstream
Linux doesn't use this interrupt at all at the moment.

When Xen is present, Linux Dom0 could enable the IPI IRQ as any other
interrupts. In the future, a DomU could enable the IPI IRQ if both the
IRQ and related IPI buffer have been remapped into the virtual machine
(using regular device assignment techniques).

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel