* [PATCH v6 1/6] xen/arm: introduce platform_smc
2018-12-14 19:11 [PATCH v6 0/6] zynqmp: Add forwarding of platform specific firmware calls Stefano Stabellini
@ 2018-12-14 19:11 ` Stefano Stabellini
2018-12-14 19:11 ` [PATCH v6 2/6] xen/arm: zynqmp: Forward plaform specific firmware calls Stefano Stabellini
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Stefano Stabellini @ 2018-12-14 19:11 UTC (permalink / raw)
To: xen-devel
Cc: edgar.iglesias, Stefano Stabellini, julien.grall, sstabellini,
saeed.nowshadi
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v6 2/6] xen/arm: zynqmp: Forward plaform specific firmware calls
2018-12-14 19:11 [PATCH v6 0/6] zynqmp: Add forwarding of platform specific firmware calls Stefano Stabellini
2018-12-14 19:11 ` [PATCH v6 1/6] xen/arm: introduce platform_smc Stefano Stabellini
@ 2018-12-14 19:11 ` Stefano Stabellini
2018-12-17 11:20 ` Julien Grall
2018-12-14 19:11 ` [PATCH v6 3/6] xen/arm: zynqmp: introduce zynqmp specific defines Stefano Stabellini
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Stefano Stabellini @ 2018-12-14 19:11 UTC (permalink / raw)
To: xen-devel
Cc: edgar.iglesias, Stefano Stabellini, julien.grall, sstabellini,
saeed.nowshadi
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 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 | 14 +++++++++
xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 30 +++++++++++++++++++
4 files changed, 79 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..b1e67fd 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)
+{
+ /*
+ * 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) )
+ 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v6 2/6] xen/arm: zynqmp: Forward plaform specific firmware calls
2018-12-14 19:11 ` [PATCH v6 2/6] xen/arm: zynqmp: Forward plaform specific firmware calls Stefano Stabellini
@ 2018-12-17 11:20 ` Julien Grall
2018-12-17 18:17 ` Stefano Stabellini
0 siblings, 1 reply; 15+ messages in thread
From: Julien Grall @ 2018-12-17 11:20 UTC (permalink / raw)
To: Stefano Stabellini, xen-devel
Cc: edgar.iglesias, Stefano Stabellini, saeed.nowshadi
Hi,
On 14/12/2018 19:11, Stefano Stabellini wrote:
> 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 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 | 14 +++++++++
> xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 30 +++++++++++++++++++
> 4 files changed, 79 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..b1e67fd 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)
> +{
> + /*
> + * ZynqMP firmware is based on SMCCC 1.1. If SMCCC 1.1 is not
> + * available something is wrong, don't try to handle it.
> + */
Why not just denying booting Xen on such platform? I guess we would need to add
a callback (e.g presmp_init) in the platform for that purpose.
> + if ( !cpus_have_const_cap(ARM_SMCCC_1_1) )
> + 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:
> + */
>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v6 2/6] xen/arm: zynqmp: Forward plaform specific firmware calls
2018-12-17 11:20 ` Julien Grall
@ 2018-12-17 18:17 ` Stefano Stabellini
2018-12-17 19:22 ` Julien Grall
0 siblings, 1 reply; 15+ messages in thread
From: Stefano Stabellini @ 2018-12-17 18:17 UTC (permalink / raw)
To: Julien Grall
Cc: edgar.iglesias, Stefano Stabellini, Stefano Stabellini,
saeed.nowshadi, xen-devel
On Mon, 17 Dec 2018, Julien Grall wrote:
> Hi,
>
> On 14/12/2018 19:11, Stefano Stabellini wrote:
> > 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 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 | 14 +++++++++
> > xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 30
> > +++++++++++++++++++
> > 4 files changed, 79 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..b1e67fd 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)
> > +{
> > + /*
> > + * ZynqMP firmware is based on SMCCC 1.1. If SMCCC 1.1 is not
> > + * available something is wrong, don't try to handle it.
> > + */
>
> Why not just denying booting Xen on such platform? I guess we would need to
> add a callback (e.g presmp_init) in the platform for that purpose.
Yes, we would need a new callback. I wasn't too keen on adding one more.
The other reason for doing it this way is that even if the user doesn't
have the right firmware version (I cannot imagine what version could it
be), it makes sense to stop any firmware related functions, including
EEMI, but continue booting nonetheless. I guess I should also have added
a clear warning to say that firmware functionalities have been disabled
because wrong firmware or no-firmware is present.
What do you think?
> > + if ( !cpus_have_const_cap(ARM_SMCCC_1_1) )
> > + 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:
> > + */
> >
>
> Cheers,
>
> --
> Julien Grall
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v6 2/6] xen/arm: zynqmp: Forward plaform specific firmware calls
2018-12-17 18:17 ` Stefano Stabellini
@ 2018-12-17 19:22 ` Julien Grall
2018-12-17 21:58 ` Stefano Stabellini
0 siblings, 1 reply; 15+ messages in thread
From: Julien Grall @ 2018-12-17 19:22 UTC (permalink / raw)
To: Stefano Stabellini
Cc: edgar.iglesias, Stefano Stabellini, nd, saeed.nowshadi, xen-devel
Hi Stefano,
On 17/12/2018 18:17, Stefano Stabellini wrote:
> On Mon, 17 Dec 2018, Julien Grall wrote:
>> Hi,
>>
>> On 14/12/2018 19:11, Stefano Stabellini wrote:
>>> 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 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 | 14 +++++++++
>>> xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 30
>>> +++++++++++++++++++
>>> 4 files changed, 79 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..b1e67fd 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)
>>> +{
>>> + /*
>>> + * ZynqMP firmware is based on SMCCC 1.1. If SMCCC 1.1 is not
>>> + * available something is wrong, don't try to handle it.
>>> + */
>>
>> Why not just denying booting Xen on such platform? I guess we would need to
>> add a callback (e.g presmp_init) in the platform for that purpose.
>
> Yes, we would need a new callback. I wasn't too keen on adding one more.
Checking at every SMC is a bit pointless :). That's platform specific
code, so I don't much mind the solution here.
>
> The other reason for doing it this way is that even if the user doesn't
> have the right firmware version (I cannot imagine what version could it
> be), it makes sense to stop any firmware related functions, including
> EEMI, but continue booting nonetheless. I guess I should also have added
> a clear warning to say that firmware functionalities have been disabled
> because wrong firmware or no-firmware is present.
Most likely the hardware domain would not behave correctly. So I am not
sure that worth trying to continue in Xen.
>
> What do you think?
The warning would definitely be an improvement. Anyway as I said above,
that's platform specific code. So I don't much care whether this is call
everytime.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v6 2/6] xen/arm: zynqmp: Forward plaform specific firmware calls
2018-12-17 19:22 ` Julien Grall
@ 2018-12-17 21:58 ` Stefano Stabellini
0 siblings, 0 replies; 15+ messages in thread
From: Stefano Stabellini @ 2018-12-17 21:58 UTC (permalink / raw)
To: Julien Grall
Cc: edgar.iglesias, Stefano Stabellini, Stefano Stabellini,
xen-devel, saeed.nowshadi, nd
On Mon, 17 Dec 2018, Julien Grall wrote:
> Hi Stefano,
>
> On 17/12/2018 18:17, Stefano Stabellini wrote:
> > On Mon, 17 Dec 2018, Julien Grall wrote:
> >> Hi,
> >>
> >> On 14/12/2018 19:11, Stefano Stabellini wrote:
> >>> 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 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 | 14 +++++++++
> >>> xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 30
> >>> +++++++++++++++++++
> >>> 4 files changed, 79 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..b1e67fd 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)
> >>> +{
> >>> + /*
> >>> + * ZynqMP firmware is based on SMCCC 1.1. If SMCCC 1.1 is not
> >>> + * available something is wrong, don't try to handle it.
> >>> + */
> >>
> >> Why not just denying booting Xen on such platform? I guess we would need to
> >> add a callback (e.g presmp_init) in the platform for that purpose.
> >
> > Yes, we would need a new callback. I wasn't too keen on adding one more.
>
> Checking at every SMC is a bit pointless :). That's platform specific
> code, so I don't much mind the solution here.
>
> >
> > The other reason for doing it this way is that even if the user doesn't
> > have the right firmware version (I cannot imagine what version could it
> > be), it makes sense to stop any firmware related functions, including
> > EEMI, but continue booting nonetheless. I guess I should also have added
> > a clear warning to say that firmware functionalities have been disabled
> > because wrong firmware or no-firmware is present.
>
> Most likely the hardware domain would not behave correctly. So I am not
> sure that worth trying to continue in Xen.
>
> >
> > What do you think?
>
> The warning would definitely be an improvement. Anyway as I said above,
> that's platform specific code. So I don't much care whether this is call
> everytime.
I gave it a look, but I think it is not worth adding a new hook for
this, moreover we would have to change the implementation of .smc
dynamically which is not supported (PLATFORM_START structs are
read-only). I think it is not worth the trouble. I'll add a warning
printed only once to this patch.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v6 3/6] xen/arm: zynqmp: introduce zynqmp specific defines
2018-12-14 19:11 [PATCH v6 0/6] zynqmp: Add forwarding of platform specific firmware calls Stefano Stabellini
2018-12-14 19:11 ` [PATCH v6 1/6] xen/arm: introduce platform_smc Stefano Stabellini
2018-12-14 19:11 ` [PATCH v6 2/6] xen/arm: zynqmp: Forward plaform specific firmware calls Stefano Stabellini
@ 2018-12-14 19:11 ` Stefano Stabellini
2018-12-14 19:11 ` [PATCH v6 4/6] xen/arm: zynqmp: implement zynqmp_eemi Stefano Stabellini
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Stefano Stabellini @ 2018-12-14 19:11 UTC (permalink / raw)
To: xen-devel
Cc: edgar.iglesias, Stefano Stabellini, julien.grall, sstabellini,
saeed.nowshadi
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 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 | 86 ++++++++++++++++++++++
1 file changed, 86 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..84b8cbd 100644
--- a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
+++ b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
@@ -16,6 +16,92 @@
#include <asm/processor.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 PM_MMIO_SHIFT 32
+
+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);
#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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v6 4/6] xen/arm: zynqmp: implement zynqmp_eemi
2018-12-14 19:11 [PATCH v6 0/6] zynqmp: Add forwarding of platform specific firmware calls Stefano Stabellini
` (2 preceding siblings ...)
2018-12-14 19:11 ` [PATCH v6 3/6] xen/arm: zynqmp: introduce zynqmp specific defines Stefano Stabellini
@ 2018-12-14 19:11 ` Stefano Stabellini
2018-12-17 11:39 ` Julien Grall
2018-12-14 19:11 ` [PATCH v6 5/6] xen/arm: zynqmp: Remove blacklist of ZynqMP's PM node Stefano Stabellini
2018-12-14 19:11 ` [PATCH v6 6/6] xen/zynqmp: add IPI calls virtualization Stefano Stabellini
5 siblings, 1 reply; 15+ messages in thread
From: Stefano Stabellini @ 2018-12-14 19:11 UTC (permalink / raw)
To: xen-devel
Cc: edgar.iglesias, Stefano Stabellini, julien.grall, sstabellini,
saeed.nowshadi
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 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 | 180 +++++++++++++++++++++++++++-
1 file changed, 179 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..e0456ae 100644
--- a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
+++ b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
@@ -17,11 +17,189 @@
*/
#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;
+ unsigned int pm_fn;
+ enum pm_ret_status ret;
+
+ /* Check for the mandatory SMC32 functions first */
+ switch ( fid )
+ {
+ case ARM_SMCCC_CALL_COUNT_FID(SIP):
+ case ARM_SMCCC_CALL_UID_FID(SIP):
+ case ARM_SMCCC_REVISION_FID(SIP):
+ goto forward_to_fw;
+ default:
+ break;
+ }
+
+ /* EEMI calls are SMC64 SIP Fast Calls */
+ if ( !(fid & ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,
+ ARM_SMCCC_CONV_64,
+ ARM_SMCCC_OWNER_SIP,
+ 0x0)) )
+ {
+ ret = ARM_SMCCC_NOT_SUPPORTED;
+ goto done;
+ }
+
+ nodeid = get_user_reg(regs, 1);
+ pm_fn = fid & 0xFFFF;
+
+ switch ( pm_fn )
+ {
+ /*
+ * 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 PM_SELF_SUSPEND:
+ ret = XST_PM_SUCCESS;
+ goto done;
+
+ case PM_GET_NODE_STATUS:
+ /* API for PUs. */
+ case PM_REQ_SUSPEND:
+ case PM_FORCE_POWERDOWN:
+ case PM_ABORT_SUSPEND:
+ case PM_REQ_WAKEUP:
+ case PM_SET_WAKEUP_SOURCE:
+ /* API for slaves. */
+ case PM_REQ_NODE:
+ case PM_RELEASE_NODE:
+ case PM_SET_REQUIREMENT:
+ case 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 PM_RESET_ASSERT:
+ case 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 ZYNQMP_SIP_SVC_CALL_COUNT:
+ case ZYNQMP_SIP_SVC_UID:
+ case ZYNQMP_SIP_SVC_VERSION:
+ case PM_GET_TRUSTZONE_VERSION:
+ case PM_GET_API_VERSION:
+ case PM_GET_CHIPID:
+ goto forward_to_fw;
+
+ /* No MMIO access is allowed from non-secure domains */
+ case PM_MMIO_WRITE:
+ case 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 PM_INIT:
+ case PM_SET_CONFIGURATION:
+ case PM_FPGA_LOAD:
+ case PM_FPGA_GET_STATUS:
+ case PM_SECURE_SHA:
+ case PM_SECURE_RSA:
+ case PM_PINCTRL_SET_FUNCTION:
+ case PM_PINCTRL_REQUEST:
+ case PM_PINCTRL_RELEASE:
+ case PM_PINCTRL_GET_FUNCTION:
+ case PM_PINCTRL_CONFIG_PARAM_GET:
+ case PM_PINCTRL_CONFIG_PARAM_SET:
+ case PM_IOCTL:
+ case PM_QUERY_DATA:
+ case PM_CLOCK_ENABLE:
+ case PM_CLOCK_DISABLE:
+ case PM_CLOCK_GETSTATE:
+ case PM_CLOCK_GETDIVIDER:
+ case PM_CLOCK_SETDIVIDER:
+ case PM_CLOCK_SETRATE:
+ case PM_CLOCK_GETRATE:
+ case PM_CLOCK_SETPARENT:
+ case 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 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:
+ 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v6 4/6] xen/arm: zynqmp: implement zynqmp_eemi
2018-12-14 19:11 ` [PATCH v6 4/6] xen/arm: zynqmp: implement zynqmp_eemi Stefano Stabellini
@ 2018-12-17 11:39 ` Julien Grall
2018-12-17 18:50 ` Stefano Stabellini
0 siblings, 1 reply; 15+ messages in thread
From: Julien Grall @ 2018-12-17 11:39 UTC (permalink / raw)
To: Stefano Stabellini, xen-devel
Cc: edgar.iglesias, Stefano Stabellini, saeed.nowshadi
Hi,
On 14/12/2018 19:11, Stefano Stabellini wrote:
> 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 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 | 180 +++++++++++++++++++++++++++-
> 1 file changed, 179 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..e0456ae 100644
> --- a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> +++ b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> @@ -17,11 +17,189 @@
> */
>
> #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;
> + unsigned int pm_fn;
> + enum pm_ret_status ret;
> +
> + /* Check for the mandatory SMC32 functions first */
> + switch ( fid )
> + {
> + case ARM_SMCCC_CALL_COUNT_FID(SIP):
> + case ARM_SMCCC_CALL_UID_FID(SIP):
> + case ARM_SMCCC_REVISION_FID(SIP):
> + goto forward_to_fw;
> + default:
> + break;
> + }
> +
> + /* EEMI calls are SMC64 SIP Fast Calls */
> + if ( !(fid & ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,
> + ARM_SMCCC_CONV_64,
> + ARM_SMCCC_OWNER_SIP,
> + 0x0)) )
I am afraid this does not work as you expect. This only check that at least one
bit is set, it does not check the bits have the correct value.
But this is merely a hack to avoid having everywhere:
ARM_SMCCC_CALL_VAL(..., ..., ..., 0x0)
This is mostly due to how you define the IDs. I can see two solutions
1) Stop using the enum and define use IDs using ARM_SMCCC_CALL_VAL(...)
2) Introduce EEMI_FID to wrap the call and use everywhere
1) is probably the best but I am happy with 2) as well. This also has the
advantage to avoid handling SMCCC32 functions aside and match how the other SMCC
subsystem are implemented in Xen (see vpsci and vsmc).
> + {
> + ret = ARM_SMCCC_NOT_SUPPORTED;
> + goto done;
> + }
> +
> + nodeid = get_user_reg(regs, 1);
> + pm_fn = fid & 0xFFFF;
> +
> + switch ( pm_fn )
> + {
> + /*
> + * 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 PM_SELF_SUSPEND:
> + ret = XST_PM_SUCCESS;
> + goto done;
> +
> + case PM_GET_NODE_STATUS:
> + /* API for PUs. */
> + case PM_REQ_SUSPEND:
> + case PM_FORCE_POWERDOWN:
> + case PM_ABORT_SUSPEND:
> + case PM_REQ_WAKEUP:
> + case PM_SET_WAKEUP_SOURCE:
> + /* API for slaves. */
> + case PM_REQ_NODE:
> + case PM_RELEASE_NODE:
> + case PM_SET_REQUIREMENT:
> + case 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 PM_RESET_ASSERT:
> + case 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 ZYNQMP_SIP_SVC_CALL_COUNT:
> + case ZYNQMP_SIP_SVC_UID:
> + case ZYNQMP_SIP_SVC_VERSION:
> + case PM_GET_TRUSTZONE_VERSION:
> + case PM_GET_API_VERSION:
> + case PM_GET_CHIPID:
> + goto forward_to_fw;
> +
> + /* No MMIO access is allowed from non-secure domains */
> + case PM_MMIO_WRITE:
> + case 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 PM_INIT:
> + case PM_SET_CONFIGURATION:
> + case PM_FPGA_LOAD:
> + case PM_FPGA_GET_STATUS:
> + case PM_SECURE_SHA:
> + case PM_SECURE_RSA:
> + case PM_PINCTRL_SET_FUNCTION:
> + case PM_PINCTRL_REQUEST:
> + case PM_PINCTRL_RELEASE:
> + case PM_PINCTRL_GET_FUNCTION:
> + case PM_PINCTRL_CONFIG_PARAM_GET:
> + case PM_PINCTRL_CONFIG_PARAM_SET:
> + case PM_IOCTL:
> + case PM_QUERY_DATA:
> + case PM_CLOCK_ENABLE:
> + case PM_CLOCK_DISABLE:
> + case PM_CLOCK_GETSTATE:
> + case PM_CLOCK_GETDIVIDER:
> + case PM_CLOCK_SETDIVIDER:
> + case PM_CLOCK_SETRATE:
> + case PM_CLOCK_GETRATE:
> + case PM_CLOCK_SETPARENT:
> + case 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 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:
On the previous version, I have requested a comment in the code explaining why
forward the commands without sanity check.
> + 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;
> }
>
> /*
>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v6 4/6] xen/arm: zynqmp: implement zynqmp_eemi
2018-12-17 11:39 ` Julien Grall
@ 2018-12-17 18:50 ` Stefano Stabellini
2018-12-17 19:18 ` Julien Grall
0 siblings, 1 reply; 15+ messages in thread
From: Stefano Stabellini @ 2018-12-17 18:50 UTC (permalink / raw)
To: Julien Grall
Cc: edgar.iglesias, Stefano Stabellini, Stefano Stabellini,
saeed.nowshadi, xen-devel
On Mon, 17 Dec 2018, Julien Grall wrote:
> Hi,
>
> On 14/12/2018 19:11, Stefano Stabellini wrote:
> > 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 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 | 180
> > +++++++++++++++++++++++++++-
> > 1 file changed, 179 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..e0456ae 100644
> > --- a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> > +++ b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
> > @@ -17,11 +17,189 @@
> > */
> > #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;
> > + unsigned int pm_fn;
> > + enum pm_ret_status ret;
> > +
> > + /* Check for the mandatory SMC32 functions first */
> > + switch ( fid )
> > + {
> > + case ARM_SMCCC_CALL_COUNT_FID(SIP):
> > + case ARM_SMCCC_CALL_UID_FID(SIP):
> > + case ARM_SMCCC_REVISION_FID(SIP):
> > + goto forward_to_fw;
> > + default:
> > + break;
> > + }
> > +
> > + /* EEMI calls are SMC64 SIP Fast Calls */
> > + if ( !(fid & ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,
> > + ARM_SMCCC_CONV_64,
> > + ARM_SMCCC_OWNER_SIP,
> > + 0x0)) )
>
> I am afraid this does not work as you expect. This only check that at least
> one bit is set, it does not check the bits have the correct value.
>
> But this is merely a hack to avoid having everywhere:
>
> ARM_SMCCC_CALL_VAL(..., ..., ..., 0x0)
>
> This is mostly due to how you define the IDs. I can see two solutions
> 1) Stop using the enum and define use IDs using
> ARM_SMCCC_CALL_VAL(...)
> 2) Introduce EEMI_FID to wrap the call and use everywhere
>
> 1) is probably the best but I am happy with 2) as well. This also has the
> advantage to avoid handling SMCCC32 functions aside and match how the other
> SMCC subsystem are implemented in Xen (see vpsci and vsmc).
Yes, I can see how either 1) or 2) would improve the code. I'll choose
2) because because I kind of like the enums by now.
It will also highlight a difference I hadn't even noticed myself: the
IPI Mailbox calls (last patch) are actually ARM_SMCCC_CONV_32 calls.
Good call on using the full FID.
> > + {
> > + ret = ARM_SMCCC_NOT_SUPPORTED;
> > + goto done;
> > + }
> > +
> > + nodeid = get_user_reg(regs, 1);
> > + pm_fn = fid & 0xFFFF;
> > +
> > + switch ( pm_fn )
> > + {
> > + /*
> > + * 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 PM_SELF_SUSPEND:
> > + ret = XST_PM_SUCCESS;
> > + goto done;
> > +
> > + case PM_GET_NODE_STATUS:
> > + /* API for PUs. */
> > + case PM_REQ_SUSPEND:
> > + case PM_FORCE_POWERDOWN:
> > + case PM_ABORT_SUSPEND:
> > + case PM_REQ_WAKEUP:
> > + case PM_SET_WAKEUP_SOURCE:
> > + /* API for slaves. */
> > + case PM_REQ_NODE:
> > + case PM_RELEASE_NODE:
> > + case PM_SET_REQUIREMENT:
> > + case 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 PM_RESET_ASSERT:
> > + case 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 ZYNQMP_SIP_SVC_CALL_COUNT:
> > + case ZYNQMP_SIP_SVC_UID:
> > + case ZYNQMP_SIP_SVC_VERSION:
> > + case PM_GET_TRUSTZONE_VERSION:
> > + case PM_GET_API_VERSION:
> > + case PM_GET_CHIPID:
> > + goto forward_to_fw;
> > +
> > + /* No MMIO access is allowed from non-secure domains */
> > + case PM_MMIO_WRITE:
> > + case 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 PM_INIT:
> > + case PM_SET_CONFIGURATION:
> > + case PM_FPGA_LOAD:
> > + case PM_FPGA_GET_STATUS:
> > + case PM_SECURE_SHA:
> > + case PM_SECURE_RSA:
> > + case PM_PINCTRL_SET_FUNCTION:
> > + case PM_PINCTRL_REQUEST:
> > + case PM_PINCTRL_RELEASE:
> > + case PM_PINCTRL_GET_FUNCTION:
> > + case PM_PINCTRL_CONFIG_PARAM_GET:
> > + case PM_PINCTRL_CONFIG_PARAM_SET:
> > + case PM_IOCTL:
> > + case PM_QUERY_DATA:
> > + case PM_CLOCK_ENABLE:
> > + case PM_CLOCK_DISABLE:
> > + case PM_CLOCK_GETSTATE:
> > + case PM_CLOCK_GETDIVIDER:
> > + case PM_CLOCK_SETDIVIDER:
> > + case PM_CLOCK_SETRATE:
> > + case PM_CLOCK_GETRATE:
> > + case PM_CLOCK_SETPARENT:
> > + case 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 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:
>
> On the previous version, I have requested a comment in the code explaining why
> forward the commands without sanity check.
I added something above but I can add something here too and make it clearer.
> > + 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;
> > }
> > /*
> >
>
> Cheers,
>
> --
> Julien Grall
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v6 4/6] xen/arm: zynqmp: implement zynqmp_eemi
2018-12-17 18:50 ` Stefano Stabellini
@ 2018-12-17 19:18 ` Julien Grall
2018-12-17 19:36 ` Stefano Stabellini
0 siblings, 1 reply; 15+ messages in thread
From: Julien Grall @ 2018-12-17 19:18 UTC (permalink / raw)
To: Stefano Stabellini
Cc: edgar.iglesias, Stefano Stabellini, Stefano Stabellini,
xen-devel, saeed.nowshadi, nd
Hi Stefano,
On 17/12/2018 18:50, Stefano Stabellini wrote:
> On Mon, 17 Dec 2018, Julien Grall wrote:
>> On 14/12/2018 19:11, Stefano Stabellini wrote:
>>> +forward_to_fw:
>>
>> On the previous version, I have requested a comment in the code explaining why
>> forward the commands without sanity check.
>
> I added something above but I can add something here too and make it clearer.
I can't find any explanation in the patch. Could you point out where you
added it?
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 4/6] xen/arm: zynqmp: implement zynqmp_eemi
2018-12-17 19:18 ` Julien Grall
@ 2018-12-17 19:36 ` Stefano Stabellini
0 siblings, 0 replies; 15+ messages in thread
From: Stefano Stabellini @ 2018-12-17 19:36 UTC (permalink / raw)
To: Julien Grall
Cc: edgar.iglesias, Stefano Stabellini, Stefano Stabellini,
xen-devel, saeed.nowshadi, Stefano Stabellini, nd
On Mon, 17 Dec 2018, Julien Grall wrote:
> Hi Stefano,
>
> On 17/12/2018 18:50, Stefano Stabellini wrote:
> > On Mon, 17 Dec 2018, Julien Grall wrote:
> >> On 14/12/2018 19:11, Stefano Stabellini wrote:
> >>> +forward_to_fw:
> >>
> >> On the previous version, I have requested a comment in the code explaining why
> >> forward the commands without sanity check.
> >
> > I added something above but I can add something here too and make it clearer.
>
> I can't find any explanation in the patch. Could you point out where you
> added it?
I wrote above domain_has_node_access:
* 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.
This is the new comment I plan to add just before fowarding the SMC:
/*
* 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.
*/
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v6 5/6] xen/arm: zynqmp: Remove blacklist of ZynqMP's PM node
2018-12-14 19:11 [PATCH v6 0/6] zynqmp: Add forwarding of platform specific firmware calls Stefano Stabellini
` (3 preceding siblings ...)
2018-12-14 19:11 ` [PATCH v6 4/6] xen/arm: zynqmp: implement zynqmp_eemi Stefano Stabellini
@ 2018-12-14 19:11 ` Stefano Stabellini
2018-12-14 19:11 ` [PATCH v6 6/6] xen/zynqmp: add IPI calls virtualization Stefano Stabellini
5 siblings, 0 replies; 15+ messages in thread
From: Stefano Stabellini @ 2018-12-14 19:11 UTC (permalink / raw)
To: xen-devel
Cc: edgar.iglesias, Stefano Stabellini, julien.grall, sstabellini,
saeed.nowshadi
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 b1e67fd..78d48ff 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)
{
/*
@@ -48,7 +41,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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v6 6/6] xen/zynqmp: add IPI calls virtualization
2018-12-14 19:11 [PATCH v6 0/6] zynqmp: Add forwarding of platform specific firmware calls Stefano Stabellini
` (4 preceding siblings ...)
2018-12-14 19:11 ` [PATCH v6 5/6] xen/arm: zynqmp: Remove blacklist of ZynqMP's PM node Stefano Stabellini
@ 2018-12-14 19:11 ` Stefano Stabellini
5 siblings, 0 replies; 15+ messages in thread
From: Stefano Stabellini @ 2018-12-14 19:11 UTC (permalink / raw)
To: xen-devel
Cc: edgar.iglesias, Stefano Stabellini, julien.grall, sstabellini,
saeed.nowshadi
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 v6:
- new patch
---
xen/arch/arm/platforms/xilinx-zynqmp-eemi.c | 18 ++++++++++++++++++
xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h | 11 +++++++++++
2 files changed, 29 insertions(+)
diff --git a/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c b/xen/arch/arm/platforms/xilinx-zynqmp-eemi.c
index e0456ae..8ef4157 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.
@@ -175,6 +178,21 @@ bool zynqmp_eemi(struct cpu_user_regs *regs)
ret = XST_PM_NO_ACCESS;
goto done;
+ case IPI_MAILBOX_OPEN:
+ case IPI_MAILBOX_RELEASE:
+ case IPI_MAILBOX_STATUS_ENQUIRY:
+ case IPI_MAILBOX_NOTIFY:
+ case IPI_MAILBOX_ACK:
+ case IPI_MAILBOX_ENABLE_IRQ:
+ case 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 84b8cbd..2e48c80 100644
--- a/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
+++ b/xen/include/asm-arm/platforms/xilinx-zynqmp-eemi.h
@@ -102,6 +102,17 @@ enum pm_ret_status {
XST_PM_ABORT_SUSPEND,
};
+/* IPI SMC function numbers enum definition */
+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
^ permalink raw reply related [flat|nested] 15+ messages in thread