diff mbox series

[v5,04/12] xen/arm: add SVE exception class handling

Message ID 20230412094938.2693890-5-luca.fancellu@arm.com (mailing list archive)
State Superseded
Headers show
Series SVE feature for arm guests | expand

Commit Message

Luca Fancellu April 12, 2023, 9:49 a.m. UTC
SVE has a new exception class with code 0x19, introduce the new code
and handle the exception.

Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
---
Changes from v4:
 - No changes
Changes from v3:
 - No changes
Changes from v2:
 - No changes
Changes from v1:
 - No changes
Changes from RFC:
 - No changes
---
 xen/arch/arm/include/asm/processor.h |  1 +
 xen/arch/arm/traps.c                 | 12 ++++++++++++
 2 files changed, 13 insertions(+)

Comments

Julien Grall April 13, 2023, 1:02 p.m. UTC | #1
Hi,

On 12/04/2023 10:49, Luca Fancellu wrote:
> SVE has a new exception class with code 0x19, introduce the new code
> and handle the exception.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
> ---
> Changes from v4:
>   - No changes
> Changes from v3:
>   - No changes
> Changes from v2:
>   - No changes
> Changes from v1:
>   - No changes
> Changes from RFC:
>   - No changes
> ---
>   xen/arch/arm/include/asm/processor.h |  1 +
>   xen/arch/arm/traps.c                 | 12 ++++++++++++
>   2 files changed, 13 insertions(+)
> 
> diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h
> index bc683334125c..7e42ff8811fc 100644
> --- a/xen/arch/arm/include/asm/processor.h
> +++ b/xen/arch/arm/include/asm/processor.h
> @@ -426,6 +426,7 @@
>   #define HSR_EC_HVC64                0x16
>   #define HSR_EC_SMC64                0x17
>   #define HSR_EC_SYSREG               0x18
> +#define HSR_EC_SVE                  0x19
>   #endif
>   #define HSR_EC_INSTR_ABORT_LOWER_EL 0x20
>   #define HSR_EC_INSTR_ABORT_CURR_EL  0x21
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index a78a99ddadd0..c2e30feafd5a 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2160,6 +2160,13 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
>           perfc_incr(trap_sysreg);
>           do_sysreg(regs, hsr);
>           break;
> +    case HSR_EC_SVE:
> +        GUEST_BUG_ON(regs_mode_is_32bit(regs));
> +        gprintk(XENLOG_WARNING,
> +                "Domain id %d tried to use SVE while not allowed\n",
> +                current->domain->domain_id);

gprintk() will already print the domain/vCPU for you. Also, if you want 
to print a domain ID, then you should use ("%pd", d) rather than ("%d", 
d->domain_id).

> +        inject_undef_exception(regs, hsr);
> +        break;
>   #endif
>   
>       case HSR_EC_INSTR_ABORT_LOWER_EL:
> @@ -2189,6 +2196,11 @@ void do_trap_hyp_sync(struct cpu_user_regs *regs)
>       case HSR_EC_BRK:
>           do_trap_brk(regs, hsr);
>           break;
> +    case HSR_EC_SVE:
> +        /* An SVE exception is a bug somewhere in hypervisor code */
> +        printk("SVE trap at EL2.\n");
> +        do_unexpected_trap("Hypervisor", regs);

I think it would be better if you pass "SVE trap at EL2" as a string 
rather than adding your own printk above.

> +        break;
>   #endif
>       case HSR_EC_DATA_ABORT_CURR_EL:
>       case HSR_EC_INSTR_ABORT_CURR_EL:

Cheers,
Luca Fancellu April 13, 2023, 1:27 p.m. UTC | #2
>> --- a/xen/arch/arm/traps.c
>> +++ b/xen/arch/arm/traps.c
>> @@ -2160,6 +2160,13 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
>>          perfc_incr(trap_sysreg);
>>          do_sysreg(regs, hsr);
>>          break;
>> +    case HSR_EC_SVE:
>> +        GUEST_BUG_ON(regs_mode_is_32bit(regs));
>> +        gprintk(XENLOG_WARNING,
>> +                "Domain id %d tried to use SVE while not allowed\n",
>> +                current->domain->domain_id);
> 
> gprintk() will already print the domain/vCPU for you. Also, if you want to print a domain ID, then you should use ("%pd", d) rather than ("%d", d->domain_id).

Ok I’ll change it to:

gprintk(XENLOG_WARNING, "Domain tried to use SVE while not allowed\n");

> 
>> +        inject_undef_exception(regs, hsr);
>> +        break;
>>  #endif
>>        case HSR_EC_INSTR_ABORT_LOWER_EL:
>> @@ -2189,6 +2196,11 @@ void do_trap_hyp_sync(struct cpu_user_regs *regs)
>>      case HSR_EC_BRK:
>>          do_trap_brk(regs, hsr);
>>          break;
>> +    case HSR_EC_SVE:
>> +        /* An SVE exception is a bug somewhere in hypervisor code */
>> +        printk("SVE trap at EL2.\n");
>> +        do_unexpected_trap("Hypervisor", regs);
> 
> I think it would be better if you pass "SVE trap at EL2" as a string rather than adding your own printk above.

Ok I’ll remove the printk and do just do_unexpected_trap("SVE trap at EL2", regs);

> 
>> +        break;
>>  #endif
>>      case HSR_EC_DATA_ABORT_CURR_EL:
>>      case HSR_EC_INSTR_ABORT_CURR_EL:
> 
> Cheers,
> 
> -- 
> Julien Grall
Bertrand Marquis April 14, 2023, 8:40 a.m. UTC | #3
Hi Luca,

> On 12 Apr 2023, at 11:49, Luca Fancellu <Luca.Fancellu@arm.com> wrote:
> 
> SVE has a new exception class with code 0x19, introduce the new code
> and handle the exception.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>

With the comments from Julien handled you can add my:
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand


> ---
> Changes from v4:
> - No changes
> Changes from v3:
> - No changes
> Changes from v2:
> - No changes
> Changes from v1:
> - No changes
> Changes from RFC:
> - No changes
> ---
> xen/arch/arm/include/asm/processor.h |  1 +
> xen/arch/arm/traps.c                 | 12 ++++++++++++
> 2 files changed, 13 insertions(+)
> 
> diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h
> index bc683334125c..7e42ff8811fc 100644
> --- a/xen/arch/arm/include/asm/processor.h
> +++ b/xen/arch/arm/include/asm/processor.h
> @@ -426,6 +426,7 @@
> #define HSR_EC_HVC64                0x16
> #define HSR_EC_SMC64                0x17
> #define HSR_EC_SYSREG               0x18
> +#define HSR_EC_SVE                  0x19
> #endif
> #define HSR_EC_INSTR_ABORT_LOWER_EL 0x20
> #define HSR_EC_INSTR_ABORT_CURR_EL  0x21
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index a78a99ddadd0..c2e30feafd5a 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2160,6 +2160,13 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
>         perfc_incr(trap_sysreg);
>         do_sysreg(regs, hsr);
>         break;
> +    case HSR_EC_SVE:
> +        GUEST_BUG_ON(regs_mode_is_32bit(regs));
> +        gprintk(XENLOG_WARNING,
> +                "Domain id %d tried to use SVE while not allowed\n",
> +                current->domain->domain_id);
> +        inject_undef_exception(regs, hsr);
> +        break;
> #endif
> 
>     case HSR_EC_INSTR_ABORT_LOWER_EL:
> @@ -2189,6 +2196,11 @@ void do_trap_hyp_sync(struct cpu_user_regs *regs)
>     case HSR_EC_BRK:
>         do_trap_brk(regs, hsr);
>         break;
> +    case HSR_EC_SVE:
> +        /* An SVE exception is a bug somewhere in hypervisor code */
> +        printk("SVE trap at EL2.\n");
> +        do_unexpected_trap("Hypervisor", regs);
> +        break;
> #endif
>     case HSR_EC_DATA_ABORT_CURR_EL:
>     case HSR_EC_INSTR_ABORT_CURR_EL:
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h
index bc683334125c..7e42ff8811fc 100644
--- a/xen/arch/arm/include/asm/processor.h
+++ b/xen/arch/arm/include/asm/processor.h
@@ -426,6 +426,7 @@ 
 #define HSR_EC_HVC64                0x16
 #define HSR_EC_SMC64                0x17
 #define HSR_EC_SYSREG               0x18
+#define HSR_EC_SVE                  0x19
 #endif
 #define HSR_EC_INSTR_ABORT_LOWER_EL 0x20
 #define HSR_EC_INSTR_ABORT_CURR_EL  0x21
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index a78a99ddadd0..c2e30feafd5a 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2160,6 +2160,13 @@  void do_trap_guest_sync(struct cpu_user_regs *regs)
         perfc_incr(trap_sysreg);
         do_sysreg(regs, hsr);
         break;
+    case HSR_EC_SVE:
+        GUEST_BUG_ON(regs_mode_is_32bit(regs));
+        gprintk(XENLOG_WARNING,
+                "Domain id %d tried to use SVE while not allowed\n",
+                current->domain->domain_id);
+        inject_undef_exception(regs, hsr);
+        break;
 #endif
 
     case HSR_EC_INSTR_ABORT_LOWER_EL:
@@ -2189,6 +2196,11 @@  void do_trap_hyp_sync(struct cpu_user_regs *regs)
     case HSR_EC_BRK:
         do_trap_brk(regs, hsr);
         break;
+    case HSR_EC_SVE:
+        /* An SVE exception is a bug somewhere in hypervisor code */
+        printk("SVE trap at EL2.\n");
+        do_unexpected_trap("Hypervisor", regs);
+        break;
 #endif
     case HSR_EC_DATA_ABORT_CURR_EL:
     case HSR_EC_INSTR_ABORT_CURR_EL: