[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 2/6] xen/arm: zynqmp: Forward plaform specific firmware calls



Hi,

On 24/08/18 00:56, Stefano Stabellini wrote:
On Thu, 23 Aug 2018, Julien Grall wrote:
Hi Stefano,

2018 01:01 AM, Stefano Stabellini wrote:
From: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>

From: Edgar E. Iglesias <edgar.iglesias@xxxxxxxxxx>

Introduce zynqmp_eemi: a function resposible for implementing access

s/resposible/responsible/

I'll fix


controls over the firmware calls. Only calls that are allowed are
forwarded to the firmware.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xxxxxxxxxx>
Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
---
   xen/arch/arm/platforms/Makefile                    |  1 +
   xen/arch/arm/platforms/xilinx-zynqmp-eemi.c        | 38
++++++++++++++++++++++
   xen/arch/arm/platforms/xilinx-zynqmp.c             | 14 ++++++++
   xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h |  3 ++
   4 files changed, 56 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 80e555c..703f915 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -9,3 +9,4 @@ obj-y += sunxi.o
   obj-$(CONFIG_ARM_64) += thunderx.o
   obj-$(CONFIG_ARM_64) += xgene-storm.o
   obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
+obj-$(CONFIG_ARM_64) += 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..c3a19e9
--- /dev/null
+++ b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
@@ -0,0 +1,38 @@
+/*
+ * xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
+ *
+ * Xilinx ZynqMP EEMI API
+ *
+ * Copyright (c) 2018 Xilinx Inc.
+ * Written by Edgar E. Iglesias <edgar.iglesias@xxxxxxxxxx>
+ *
+ * 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 <xen/iocap.h>
+#include <xen/sched.h>
+#include <xen/types.h>
+#include <asm/smccc.h>
+#include <asm/regs.h>
+#include <asm/platforms/xilinx-zynqmp-eemi.h>

Can you please introduce headers when they are actually used?

I'll do


+
+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..c318cf5 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,20 @@ static const struct dt_device_match
zynqmp_blacklist_dev[] __initconst =
       { /* sentinel */ },
   };
   +static bool zynqmp_smc(struct cpu_user_regs *regs)
+{
+    if ( !is_64bit_domain(current->domain) )
+        return false;
+    /* Only support platforms with SMCCC >= 1.1 */
+    if ( smccc_ver < SMCCC_VERSION(1, 1) )
+        return false;

Do you really need to call this at every SMC call?

Do you mean that we could skip the checks? We could do the checks at
init time, but then we would have to memorize the result for each
domain. Not sure it is worth it.

I was mostly speaking about smccc_ver. It feels strange to always check the SMCCC version when calling SMC as the value will never change.

I can see two solutions here:
        1) Provide a different .smc callback depending on the SMCCC version
        2) Provide wrappers to either call SMCCv1.1 or SMCCCv1.0

+
+    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..6630dc8
--- /dev/null
+++ b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
@@ -0,0 +1,3 @@

Missing copyright headers and guard.

The guard is missing, I'll add it. Regarding the copyright, this is an
header file, none of the others in the same directory have the copyright
header -- are you sure we need it?

Technically any file require the copyright. This was not followed in the the header side until recently.

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.