diff mbox

[RFC,v2,2/6] xen/arm: Introduce platform_hvc

Message ID 1486496525-14637-3-git-send-email-edgar.iglesias@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Edgar E. Iglesias Feb. 7, 2017, 7:42 p.m. UTC
From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Introduce platform_hvc as a way to handle hypercalls 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>
---
 xen/arch/arm/platform.c        | 8 ++++++++
 xen/arch/arm/traps.c           | 3 +++
 xen/include/asm-arm/platform.h | 5 +++++
 3 files changed, 16 insertions(+)

Comments

Stefano Stabellini Feb. 13, 2017, 10:08 p.m. UTC | #1
On Tue, 7 Feb 2017, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Introduce platform_hvc as a way to handle hypercalls 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>
> ---
>  xen/arch/arm/platform.c        | 8 ++++++++
>  xen/arch/arm/traps.c           | 3 +++
>  xen/include/asm-arm/platform.h | 5 +++++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
> index 0af6d57..90ea6b8 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_hvc(struct cpu_user_regs *regs)

This is fine, but we need a different name for it, as it can be used to
handle both HVC and SMC calls. Maybe "firmware_call"?


> +{
> +    if ( platform && platform->hvc )
> +        return platform->hvc(regs);
> +
> +    return false;
> +}
> +
>  bool_t platform_has_quirk(uint32_t quirk)
>  {
>      uint32_t quirks = 0;
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index c5a4d41..33950d9 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -44,6 +44,7 @@
>  #include <asm/cpufeature.h>
>  #include <asm/flushtlb.h>
>  #include <asm/monitor.h>
> +#include <asm/platform.h>
>  
>  #include "decode.h"
>  #include "vtimer.h"
> @@ -1430,6 +1431,8 @@ static void do_trap_psci(struct cpu_user_regs *regs)
>          }
>          break;
>      default:
> +        if ( platform_hvc(regs) )
> +                return;
>          domain_crash_synchronous();
>          return;
>      }
> diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
> index 08010ba..4d51f0a 100644
> --- a/xen/include/asm-arm/platform.h
> +++ b/xen/include/asm-arm/platform.h
> @@ -26,6 +26,10 @@ struct platform_desc {
>      void (*reset)(void);
>      /* Platform power-off */
>      void (*poweroff)(void);
> +    /* Platform specific HVC handler.
> +     * Returns true if the call was handled and false if not.
> +     */
> +    bool (*hvc)(struct cpu_user_regs *regs);
>      /*
>       * Platform quirks
>       * Defined has a function because a platform can support multiple
> @@ -55,6 +59,7 @@ int platform_cpu_up(int cpu);
>  #endif
>  void platform_reset(void);
>  void platform_poweroff(void);
> +bool platform_hvc(struct cpu_user_regs *regs);
>  bool_t platform_has_quirk(uint32_t quirk);
>  bool_t platform_device_is_blacklisted(const struct dt_device_node *node);
Edgar E. Iglesias July 31, 2017, 9:30 p.m. UTC | #2
On Mon, Feb 13, 2017 at 02:08:43PM -0800, Stefano Stabellini wrote:
> On Tue, 7 Feb 2017, Edgar E. Iglesias wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> > 
> > Introduce platform_hvc as a way to handle hypercalls 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>
> > ---
> >  xen/arch/arm/platform.c        | 8 ++++++++
> >  xen/arch/arm/traps.c           | 3 +++
> >  xen/include/asm-arm/platform.h | 5 +++++
> >  3 files changed, 16 insertions(+)
> > 
> > diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
> > index 0af6d57..90ea6b8 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_hvc(struct cpu_user_regs *regs)
> 
> This is fine, but we need a different name for it, as it can be used to
> handle both HVC and SMC calls. Maybe "firmware_call"?

Hi,

Sorry for the super long delay.. I'm looking at this again now.

Yes, you're right. I went with
platform_firmware_call() and platform->firmware_call().

I kept the platform_ prefix in the wrapper function to be
consistent with the other hooks.

Thanks,
Edgar


> 
> 
> > +{
> > +    if ( platform && platform->hvc )
> > +        return platform->hvc(regs);
> > +
> > +    return false;
> > +}
> > +
> >  bool_t platform_has_quirk(uint32_t quirk)
> >  {
> >      uint32_t quirks = 0;
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index c5a4d41..33950d9 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -44,6 +44,7 @@
> >  #include <asm/cpufeature.h>
> >  #include <asm/flushtlb.h>
> >  #include <asm/monitor.h>
> > +#include <asm/platform.h>
> >  
> >  #include "decode.h"
> >  #include "vtimer.h"
> > @@ -1430,6 +1431,8 @@ static void do_trap_psci(struct cpu_user_regs *regs)
> >          }
> >          break;
> >      default:
> > +        if ( platform_hvc(regs) )
> > +                return;
> >          domain_crash_synchronous();
> >          return;
> >      }
> > diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
> > index 08010ba..4d51f0a 100644
> > --- a/xen/include/asm-arm/platform.h
> > +++ b/xen/include/asm-arm/platform.h
> > @@ -26,6 +26,10 @@ struct platform_desc {
> >      void (*reset)(void);
> >      /* Platform power-off */
> >      void (*poweroff)(void);
> > +    /* Platform specific HVC handler.
> > +     * Returns true if the call was handled and false if not.
> > +     */
> > +    bool (*hvc)(struct cpu_user_regs *regs);
> >      /*
> >       * Platform quirks
> >       * Defined has a function because a platform can support multiple
> > @@ -55,6 +59,7 @@ int platform_cpu_up(int cpu);
> >  #endif
> >  void platform_reset(void);
> >  void platform_poweroff(void);
> > +bool platform_hvc(struct cpu_user_regs *regs);
> >  bool_t platform_has_quirk(uint32_t quirk);
> >  bool_t platform_device_is_blacklisted(const struct dt_device_node *node);
diff mbox

Patch

diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
index 0af6d57..90ea6b8 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_hvc(struct cpu_user_regs *regs)
+{
+    if ( platform && platform->hvc )
+        return platform->hvc(regs);
+
+    return false;
+}
+
 bool_t platform_has_quirk(uint32_t quirk)
 {
     uint32_t quirks = 0;
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c5a4d41..33950d9 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -44,6 +44,7 @@ 
 #include <asm/cpufeature.h>
 #include <asm/flushtlb.h>
 #include <asm/monitor.h>
+#include <asm/platform.h>
 
 #include "decode.h"
 #include "vtimer.h"
@@ -1430,6 +1431,8 @@  static void do_trap_psci(struct cpu_user_regs *regs)
         }
         break;
     default:
+        if ( platform_hvc(regs) )
+                return;
         domain_crash_synchronous();
         return;
     }
diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
index 08010ba..4d51f0a 100644
--- a/xen/include/asm-arm/platform.h
+++ b/xen/include/asm-arm/platform.h
@@ -26,6 +26,10 @@  struct platform_desc {
     void (*reset)(void);
     /* Platform power-off */
     void (*poweroff)(void);
+    /* Platform specific HVC handler.
+     * Returns true if the call was handled and false if not.
+     */
+    bool (*hvc)(struct cpu_user_regs *regs);
     /*
      * Platform quirks
      * Defined has a function because a platform can support multiple
@@ -55,6 +59,7 @@  int platform_cpu_up(int cpu);
 #endif
 void platform_reset(void);
 void platform_poweroff(void);
+bool platform_hvc(struct cpu_user_regs *regs);
 bool_t platform_has_quirk(uint32_t quirk);
 bool_t platform_device_is_blacklisted(const struct dt_device_node *node);