Skip to contentSkip to conversation

arm64: Mediate access to GICv3 sysregs at EL2

PATCH Superseded [email protected] 4 participants 23 messages Mar 12, 2018 – Mar 15, 2018

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

[email protected]

author
#0 lore raw

cover arm64: Mediate access to GICv3 sysregs at EL2

From: Manish Jaggi <[email protected]>

This patchset is a Xen port of Marc's patchset.
arm64: KVM: Mediate access to GICv3 sysregs at EL2 [1]

The current RFC patchset is a subset of [1], as it handleing only Group1 traps as a PoC. Most of the trap code is added in vsysreg.c. Trap handler function is kept independent of the usual guest trap handling code.
Looking for feedback on this approach.

The errata has been validated on Cavium ThunderX platform.

Steps to reporduce the errata
- Boot Xen with 2 cores.
- Disable group1 interrupts in domU kernel
- start domU, the kill and start again.
One of the Xen core would hang.

Code in this patchset fixes this issue.

[1] https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/026029.html

Changes from RFC
- Added commit information on ported patches from linux
- Added skip_hyp_tail to control calling leave_hypervisor_tail
- Added CAVIUM_CONFIG_ERRATUM_30115 which will auto enable workaround

Manish Jaggi (12):
arm:Kconfig Rename menu text
arm64: cputype: Add MIDR values for Cavium ThunderX1 CPUs
arm64: Add config for Cavium Thunder erratum 30115 Enable Group1 Traps by default for Cavium ThunderX Placeholder for handling Group1 register traps
arm64: vgic-v3: Add ICV_BPR1_EL1 handler
arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler
arm64: Add accessors for the ICH_APxRn_EL2 registers Expose ich_read/write_lr in vsysreg_errata.c
arm64: Add ICV_IAR1_EL1 handler
arm64: vgic-v3: Add ICV_EOIR1_EL1 handler
arm64: vgic-v3: Add ICV_HPPIR1_EL1 handler

 xen/arch/arm/Kconfig                |   6 +-
 xen/arch/arm/arm64/Makefile         |   1 +
 xen/arch/arm/arm64/vsysreg_errata.c | 660 ++++++++++++++++++++++++++++++++++++
 xen/arch/arm/cpuerrata.c            |  21 ++
 xen/arch/arm/gic-v3.c               |  16 +-
 xen/arch/arm/traps.c                |  20 ++
 xen/include/asm-arm/arm64/sysregs.h |   5 +
 xen/include/asm-arm/arm64/traps.h   |   3 +-
 xen/include/asm-arm/cpuerrata.h     |   1 +
 xen/include/asm-arm/cpufeature.h    |   3 +-
 xen/include/asm-arm/current.h       |   1 +
 xen/include/asm-arm/gic.h           |   1 +
 xen/include/asm-arm/gic_v3_defs.h   |  29 ++
 xen/include/asm-arm/processor.h     |   9 +
 14 files changed, 771 insertions(+), 5 deletions(-)
 create mode 100644 xen/arch/arm/arm64/vsysreg_errata.c
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#1 lore raw

PATCH 1/12 arm:Kconfig Rename menu text

From: Manish Jaggi <[email protected]>

Rename the menu text to Errata Workarounds. Subsequent patches will add config options for SoC specific erratas.

Signed-off-by Manish Jaggi <[email protected]>
---
 xen/arch/arm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
xen/arch/arm/Kconfig +1 -1
index f58019d6ed..10a6d1a956 100644
@@ -59,7 +59,7 @@ config SBSA_VUART_CONSOLE
 
 endmenu
 
-menu "ARM errata workaround via the alternative framework"
+menu "Errata workarounds"
 	depends on HAS_ALTERNATIVE
 
 config ARM64_ERRATUM_827319
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
#2 lore raw

Re: [PATCH 01/12] arm:Kconfig Rename menu text

Hi,

Quoted 4 lines · On 12/03/18 12:42, [email protected] wrote:

From: Manish Jaggi <[email protected]>

Rename the menu text to Errata Workarounds. Subsequent patches will add config options for SoC specific erratas.

Well, your SoC is an Arm SoC, right? So what is the benefits of this new name? More that it still depends on HAS_ALTERNATIVE.

Cheers,

Quoted 19 lines
Signed-off-by Manish Jaggi <[email protected]>
---
  xen/arch/arm/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
xen/arch/arm/Kconfig +1 -1
index f58019d6ed..10a6d1a956 100644
@@ -59,7 +59,7 @@ config SBSA_VUART_CONSOLE
 
  endmenu
 
-menu "ARM errata workaround via the alternative framework"
+menu "Errata workarounds"
  	depends on HAS_ALTERNATIVE >
  config ARM64_ERRATUM_827319
Signature · 6 lines
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#3 lore raw

Re: [PATCH 01/12] arm:Kconfig Rename menu text

Hi Julien,

Quoted 9 lines · Julien Grall

Hi,

Quoted 4 lines · On 12/03/18 12:42, [email protected] wrote:

From: Manish Jaggi <[email protected]>

Rename the menu text to Errata Workarounds. Subsequent patches will add config options for SoC specific erratas.

Well, your SoC is an Arm SoC, right? So what is the benefits of this new name? M

It was based on your last comment

"I would much prefer to see the memu "ARM errata workaround via..." renamed to "Errata Workarounds". So we have only one menu with all workarounds."

Quoted 1 line

ore that it still depends on HAS_ALTERNATIVE.

check_workaroundXXX depends on it. So I kept it as is.

If you think this patch is not required, I can drop it.

Quoted 8 lines

Cheers,

Quoted 6 lines
Signed-off-by Manish Jaggi <[email protected]>

---
  xen/arch/arm/Kconfig | 2 +-

  1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 12 lines
Quoted 11 lines
xen/arch/arm/Kconfig +0 -0
index f58019d6ed..10a6d1a956 100644
@@ -59,7 +59,7 @@ config SBSA_VUART_CONSOLE

    endmenu
  -menu "ARM errata workaround via the alternative framework" +menu "Errata workarounds"
      depends on HAS_ALTERNATIVE >
  config ARM64_ERRATUM_827319

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

#4 lore raw

Re: [PATCH 01/12] arm:Kconfig Rename menu text

Quoted 9 lines · Manish Jaggi

Hi Julien,

Quoted 5 lines · Julien Grall

Hi,

Quoted 2 lines · On 12/03/18 12:42, [email protected] wrote:

From: Manish Jaggi <[email protected]>

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Rename the menu text to Errata Workarounds. Subsequent patches will add config options for SoC specific erratas.

Well, your SoC is an Arm SoC, right? So what is the benefits of this new name? M

It was based on your last comment

"I would much prefer to see the memu "ARM errata workaround via..." renamed to "Errata Workarounds". So we have only one menu with all workarounds."

I am fully aware that I suggested that patch. However, you still have to add a minimum of rationale in the commit message to understand why the rename. You have other people on that mailing list to look at patches...

Quoted 4 lines
Quoted 3 lines

ore that it still depends on HAS_ALTERNATIVE.

check_workaroundXXX depends on it. So I kept it as is.

HAS_ALTERNATIVE is used by both arm32 and arm64.

Quoted 2 lines

If you think this patch is not required, I can drop it.

The way it is does not make sense. I would prefer to drop it for now and just focus on the vGIC errata.

Cheers,

Signature · 6 lines
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#5 lore raw

PATCH 2/12 arm64: cputype: Add MIDR values for Cavium ThunderX1 CPUs

In reply to [email protected]

From: Manish Jaggi <[email protected]>

Add MIDR values for Cavium ThunderX1 SoC's (88xx, 81xx, 83xx).

Signed-off-by Manish Jaggi <[email protected]>
---
 xen/include/asm-arm/processor.h | 9 +++++++++
 1 file changed, 9 insertions(+)
xen/include/asm-arm/processor.h +9 -0
index 65eb1071e1..649a3ae3ca 100644
@@ -43,15 +43,24 @@
 })
 
 #define ARM_CPU_IMP_ARM             0x41
+#define ARM_CPU_IMP_CAVIUM          0x43
 
 #define ARM_CPU_PART_CORTEX_A15     0xC0F
 #define ARM_CPU_PART_CORTEX_A53     0xD03
 #define ARM_CPU_PART_CORTEX_A57     0xD07
 
+#define CAVIUM_CPU_PART_THUNDERX    0x0A1
+#define CAVIUM_CPU_PART_THUNDERX_81XX 0x0A2
+#define CAVIUM_CPU_PART_THUNDERX_83XX 0x0A3
+
 #define MIDR_CORTEX_A15 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A15)
 #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
 #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
 
+#define MIDR_THUNDERX   MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
+#define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
+#define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX)
+
 /* MPIDR Multiprocessor Affinity Register */
 #define _MPIDR_UP           (30)
 #define MPIDR_UP            (_AC(1,U) << _MPIDR_UP)
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
#6 lore raw

Re: [PATCH 02/12] arm64: cputype: Add MIDR values for Cavium ThunderX1 CPUs

Hi,

Quoted 14 lines · On 12/03/18 12:42, [email protected] wrote:

From: Manish Jaggi <[email protected]>

Add MIDR values for Cavium ThunderX1 SoC's (88xx, 81xx, 83xx).

Signed-off-by Manish Jaggi <[email protected]>
---
  xen/include/asm-arm/processor.h | 9 +++++++++
  1 file changed, 9 insertions(+)
xen/include/asm-arm/processor.h +0 -0
index 65eb1071e1..649a3ae3ca 100644

xen/include/asm-arm/processor.h line 52

Quoted 10 lines
xen/include/asm-arm/processor.h +2 -0
@@ -43,15 +43,24 @@
  })
 
  #define ARM_CPU_IMP_ARM             0x41
+#define ARM_CPU_IMP_CAVIUM          0x43
 
  #define ARM_CPU_PART_CORTEX_A15     0xC0F
  #define ARM_CPU_PART_CORTEX_A53     0xD03
  #define ARM_CPU_PART_CORTEX_A57     0xD07
 
+#define CAVIUM_CPU_PART_THUNDERX    0x0A1

In the commit message you say you add MIDR for 88xx but I don't see it. I suppose this is this one. If so, please rename it accordingly.

xen/include/asm-arm/processor.h line 54

Quoted 2 lines
xen/include/asm-arm/processor.h +2 -0
+#define CAVIUM_CPU_PART_THUNDERX_81XX 0x0A2
+#define CAVIUM_CPU_PART_THUNDERX_83XX 0x0A3

Can you please try to at least align between themselves the things to you add?

xen/include/asm-arm/processor.h line 66

Quoted 13 lines
xen/include/asm-arm/processor.h +5 -0
+
  #define MIDR_CORTEX_A15 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A15)
  #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
  #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
 
+#define MIDR_THUNDERX   MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
+#define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
+#define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX)
+
  /* MPIDR Multiprocessor Affinity Register */
  #define _MPIDR_UP           (30)
  #define MPIDR_UP            (_AC(1,U) << _MPIDR_UP)

Cheers,

Signature · 6 lines
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#7 lore raw

PATCH 3/12 arm64: Add config for Cavium Thunder erratum 30115

In reply to [email protected]

From: Manish Jaggi <[email protected]>

Some Cavium Thunder CPUs suffer a problem where a KVM guest may inadvertently cause the host kernel to quit receiving interrupts. This patch adds CONFIG_CAVIUM_ERRATUM_30115. Subsequent patches will provide workaround.

Signed-off-by Manish Jaggi <[email protected]>
---
 xen/arch/arm/Kconfig             |  4 ++++
 xen/arch/arm/cpuerrata.c         | 21 +++++++++++++++++++++
 xen/include/asm-arm/cpuerrata.h  |  1 +
 xen/include/asm-arm/cpufeature.h |  3 ++-
 4 files changed, 28 insertions(+), 1 deletion(-)
xen/arch/arm/Kconfig +4 -0
index 10a6d1a956..71976ed07b 100644
@@ -169,6 +169,10 @@ config ARM64_ERRATUM_834220
 
 	  If unsure, say Y.
 
+config CAVIUM_ERRATUM_30115
+         bool "Cavium vgic errata"
+         depends on HAS_GICV3
+
 endmenu
 
 source "common/Kconfig"
xen/arch/arm/cpuerrata.c +21 -0
index fe9e9facbe..d49698f785 100644
@@ -56,6 +56,27 @@ static const struct arm_cpu_capabilities arm_errata[] = {
         MIDR_RANGE(MIDR_CORTEX_A57, 0x00,
                    (1 << MIDR_VARIANT_SHIFT) | 2),
     },
+#endif
+#ifdef CONFIG_CAVIUM_ERRATUM_30115
+    {
+        /* Cavium ThunderX, T88 pass 1.x - 2.2 */
+        .desc = "Cavium erratum 30115",
+        .capability = ARM64_WORKAROUND_CAVIUM_30115,
+        MIDR_RANGE(MIDR_THUNDERX, 0x00,
+                   (1 << MIDR_VARIANT_SHIFT) | 2),
+    },
+    {
+        /* Cavium ThunderX, T81 pass 1.0 - 1.2 */
+        .desc = "Cavium erratum 30115",
+        .capability = ARM64_WORKAROUND_CAVIUM_30115,
+        MIDR_RANGE(MIDR_THUNDERX_81XX, 0x00, 0x02),
+    },
+    {
+        /* Cavium ThunderX, T83 pass 1.0 */
+        .desc = "Cavium erratum 30115",
+        .capability = ARM64_WORKAROUND_CAVIUM_30115,
+        MIDR_RANGE(MIDR_THUNDERX_83XX, 0x00, 0x00),
+    },
 #endif
     {},
 };
xen/include/asm-arm/cpuerrata.h +1 -0
index 8b158429c7..521f03521b 100644
@@ -41,6 +41,7 @@ static inline bool check_workaround_##erratum(void)             \
 
 CHECK_WORKAROUND_HELPER(766422, ARM32_WORKAROUND_766422, CONFIG_ARM_32)
 CHECK_WORKAROUND_HELPER(834220, ARM64_WORKAROUND_834220, CONFIG_ARM_64)
+CHECK_WORKAROUND_HELPER(30115, ARM64_WORKAROUND_CAVIUM_30115, CONFIG_ARM_64)
 
 #undef CHECK_WORKAROUND_HELPER
xen/include/asm-arm/cpufeature.h +2 -1
index f00b6dbd39..d409636bf0 100644
@@ -42,8 +42,9 @@
 #define LIVEPATCH_FEATURE   4
 #define SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT 5
 #define SKIP_CTXT_SWITCH_SERROR_SYNC 6
+#define ARM64_WORKAROUND_CAVIUM_30115 7
 
-#define ARM_NCAPS           7
+#define ARM_NCAPS           8
 
 #ifndef __ASSEMBLY__
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
#8 lore raw

Re: [PATCH 03/12] arm64: Add config for Cavium Thunder erratum 30115

Hi,

Quoted 20 lines · On 12/03/18 12:42, [email protected] wrote:

From: Manish Jaggi <[email protected]>

Some Cavium Thunder CPUs suffer a problem where a KVM guest may inadvertently cause the host kernel to quit receiving interrupts. This patch adds CONFIG_CAVIUM_ERRATUM_30115. Subsequent patches will provide workaround.

Signed-off-by Manish Jaggi <[email protected]>
---
  xen/arch/arm/Kconfig             |  4 ++++
  xen/arch/arm/cpuerrata.c         | 21 +++++++++++++++++++++
  xen/include/asm-arm/cpuerrata.h  |  1 +
  xen/include/asm-arm/cpufeature.h |  3 ++-
  4 files changed, 28 insertions(+), 1 deletion(-)
xen/arch/arm/Kconfig +0 -0
index 10a6d1a956..71976ed07b 100644

xen/arch/arm/Kconfig config ARM64_ERRATUM_834220 · line 174

Quoted 6 lines
xen/arch/arm/Kconfig +3 -0
@@ -169,6 +169,10 @@ config ARM64_ERRATUM_834220
 
  	  If unsure, say Y.
 
+config CAVIUM_ERRATUM_30115
+         bool "Cavium vgic errata"
+         depends on HAS_GICV3

As said in v1, please add a description of the problem. The Linux Kconfig is quite nice:

bool "Cavium erratum 30115: Guest may disable interrupts in host" default y
help
On ThunderX T88 pass 1.x through 2.2, T81 pass 1.0 through 1.2, and T83 Pass 1.0, KVM guest execution may disable interrupts in host. Trapping both GICv3 group-0 and group-1 accesses sidesteps the issue.

If unsure, say Y.

You also need to modify docs/misc/silicon-errata.txt

Cheers,

Quoted 41 lines
xen/arch/arm/Kconfig +1 -0
+
  endmenu
 
  source "common/Kconfig"
xen/arch/arm/cpuerrata.c +21 -0
index fe9e9facbe..d49698f785 100644
@@ -56,6 +56,27 @@ static const struct arm_cpu_capabilities arm_errata[] = {
          MIDR_RANGE(MIDR_CORTEX_A57, 0x00,
                     (1 << MIDR_VARIANT_SHIFT) | 2),
      },
+#endif
+#ifdef CONFIG_CAVIUM_ERRATUM_30115
+    {
+        /* Cavium ThunderX, T88 pass 1.x - 2.2 */
+        .desc = "Cavium erratum 30115",
+        .capability = ARM64_WORKAROUND_CAVIUM_30115,
+        MIDR_RANGE(MIDR_THUNDERX, 0x00,
+                   (1 << MIDR_VARIANT_SHIFT) | 2),
+    },
+    {
+        /* Cavium ThunderX, T81 pass 1.0 - 1.2 */
+        .desc = "Cavium erratum 30115",
+        .capability = ARM64_WORKAROUND_CAVIUM_30115,
+        MIDR_RANGE(MIDR_THUNDERX_81XX, 0x00, 0x02),
+    },
+    {
+        /* Cavium ThunderX, T83 pass 1.0 */
+        .desc = "Cavium erratum 30115",
+        .capability = ARM64_WORKAROUND_CAVIUM_30115,
+        MIDR_RANGE(MIDR_THUNDERX_83XX, 0x00, 0x00),
+    },
  #endif
      {},
  };
xen/include/asm-arm/cpuerrata.h +0 -0
index 8b158429c7..521f03521b 100644

xen/include/asm-arm/cpuerrata.h static inline bool check_workaround_##erratum(void) \ · line 44

Quoted 4 lines
xen/include/asm-arm/cpuerrata.h +1 -0
@@ -41,6 +41,7 @@ static inline bool check_workaround_##erratum(void)             \
 
  CHECK_WORKAROUND_HELPER(766422, ARM32_WORKAROUND_766422, CONFIG_ARM_32)
  CHECK_WORKAROUND_HELPER(834220, ARM64_WORKAROUND_834220, CONFIG_ARM_64)
+CHECK_WORKAROUND_HELPER(30115, ARM64_WORKAROUND_CAVIUM_30115, CONFIG_ARM_64)

You add this but never used it in this series.

Quoted 9 lines
xen/include/asm-arm/cpuerrata.h +0 -0
  #undef CHECK_WORKAROUND_HELPER
xen/include/asm-arm/cpufeature.h +0 -0
index f00b6dbd39..d409636bf0 100644

xen/include/asm-arm/cpufeature.h line 50

Quoted 10 lines
xen/include/asm-arm/cpufeature.h +2 -1
@@ -42,8 +42,9 @@
  #define LIVEPATCH_FEATURE   4
  #define SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT 5
  #define SKIP_CTXT_SWITCH_SERROR_SYNC 6
+#define ARM64_WORKAROUND_CAVIUM_30115 7
 
-#define ARM_NCAPS           7
+#define ARM_NCAPS           8
 
  #ifndef __ASSEMBLY__

Cheers,

Signature · 6 lines
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#9 lore raw

PATCH 4/12 Enable Group1 Traps by default for Cavium ThunderX1

In reply to [email protected]

From: Manish Jaggi <[email protected]>

Enable trapping for Group1 register access when
CONFIG_CAVIUM_ERRATUM_30115 is enabled.

Signed-off-by Manish Jaggi <[email protected]>
---
 xen/arch/arm/gic-v3.c     | 16 ++++++++++++++--
 xen/include/asm-arm/gic.h |  1 +
 2 files changed, 15 insertions(+), 2 deletions(-)
xen/arch/arm/gic-v3.c +14 -2
index 473e26111f..53a772a313 100644
@@ -44,6 +44,7 @@
 #include <asm/gic_v3_defs.h>
 #include <asm/gic_v3_its.h>
 #include <asm/cpufeature.h>
+#include <asm/cpuerrata.h>
 #include <asm/acpi.h>
 
 /* Global state */

@@ -825,7 +826,7 @@ static void gicv3_cpu_disable(void)
 
 static void gicv3_hyp_init(void)
 {
-    uint32_t vtr;
+    uint32_t vtr, reg32 = GICH_HCR_EN;
 
     vtr = READ_SYSREG32(ICH_VTR_EL2);
     gicv3_info.nr_lrs  = (vtr & GICH_VTR_NRLRGS) + 1;

@@ -836,7 +837,18 @@ static void gicv3_hyp_init(void)
         panic("GICv3: Invalid number of priority bits\n");
 
     WRITE_SYSREG32(GICH_VMCR_EOI | GICH_VMCR_VENG1, ICH_VMCR_EL2);
-    WRITE_SYSREG32(GICH_HCR_EN, ICH_HCR_EL2);
+
+#ifdef CONFIG_CAVIUM_ERRATUM_30115
+    if ( cpus_have_cap(ARM64_WORKAROUND_CAVIUM_30115) )
+    {
+        reg32 |= GICH_HCR_TALL1;
+        printk("%s: 30115 Workaround enabled \r\n", __func__);
+    }
+    else
+        printk("%s: 30115 Workaround not enabled \r\n", __func__);
+#endif
+
+     WRITE_SYSREG32(reg32, ICH_HCR_EL2);
 }
 
 /* Set up the per-CPU parts of the GIC for a secondary CPU */
xen/include/asm-arm/gic.h +1 -0
index d3d7bda50d..e4c77fefd6 100644
@@ -117,6 +117,7 @@
 #define GICH_HCR_VGRP0DIE (1 << 5)
 #define GICH_HCR_VGRP1EIE (1 << 6)
 #define GICH_HCR_VGRP1DIE (1 << 7)
+#define GICH_HCR_TALL1    (1 << 12)
 
 #define GICH_MISR_EOI     (1 << 0)
 #define GICH_MISR_U       (1 << 1)
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
#10 lore raw

Re: [PATCH 04/12] Enable Group1 Traps by default for Cavium ThunderX1

Hi,

title: "arm64: ...".

Quoted 4 lines · On 12/03/18 12:42, [email protected] wrote:

From: Manish Jaggi <[email protected]>

Enable trapping for Group1 register access when
CONFIG_CAVIUM_ERRATUM_30115 is enabled.

This is really odd to enable group1 trapping before the sysreg are actually emulated. This will be impossible to bisect that series on your platform. Please re-order the series to first add sysreg emulation and then hook it.

Quoted 12 lines
Signed-off-by Manish Jaggi <[email protected]>
---
  xen/arch/arm/gic-v3.c     | 16 ++++++++++++++--
  xen/include/asm-arm/gic.h |  1 +
  2 files changed, 15 insertions(+), 2 deletions(-)
xen/arch/arm/gic-v3.c +0 -0
index 473e26111f..53a772a313 100644

xen/arch/arm/gic-v3.c line 47

Quoted 4 lines
xen/arch/arm/gic-v3.c +1 -0
@@ -44,6 +44,7 @@
  #include <asm/gic_v3_defs.h>
  #include <asm/gic_v3_its.h>
  #include <asm/cpufeature.h>
+#include <asm/cpuerrata.h>

Please order them alphabetically.

xen/arch/arm/gic-v3.c static void gicv3_cpu_disable(void) · line 829

Quoted 9 lines
xen/arch/arm/gic-v3.c +1 -1
  #include <asm/acpi.h>
 
  /* Global state */

@@ -825,7 +826,7 @@ static void gicv3_cpu_disable(void)
 
  static void gicv3_hyp_init(void)
  {
-    uint32_t vtr;
+    uint32_t vtr, reg32 = GICH_HCR_EN;

The name reg32 is not obvious. Please rename it to hcr.

xen/arch/arm/gic-v3.c static void gicv3_hyp_init(void) · line 841

Quoted 10 lines
xen/arch/arm/gic-v3.c +2 -1
      vtr = READ_SYSREG32(ICH_VTR_EL2);
      gicv3_info.nr_lrs  = (vtr & GICH_VTR_NRLRGS) + 1;

@@ -836,7 +837,18 @@ static void gicv3_hyp_init(void)
          panic("GICv3: Invalid number of priority bits\n");
 
      WRITE_SYSREG32(GICH_VMCR_EOI | GICH_VMCR_VENG1, ICH_VMCR_EL2);
-    WRITE_SYSREG32(GICH_HCR_EN, ICH_HCR_EL2);
+
+#ifdef CONFIG_CAVIUM_ERRATUM_30115

I would rather avoid to spread the #ifdef everywhere. In that particular case, it is not necessary.

xen/arch/arm/gic-v3.c static void gicv3_hyp_init(void) · line 845

Quoted 4 lines
xen/arch/arm/gic-v3.c +4 -0
+    if ( cpus_have_cap(ARM64_WORKAROUND_CAVIUM_30115) )
+    {
+        reg32 |= GICH_HCR_TALL1;
+        printk("%s: 30115 Workaround enabled \r\n", __func__);

The cpuerrata framework will already print a message when the errata is enabled. So no need for this message.

Quoted 15 lines
xen/arch/arm/gic-v3.c +6 -0
+    }
+    else
+        printk("%s: 30115 Workaround not enabled \r\n", __func__);
+#endif
+
+     WRITE_SYSREG32(reg32, ICH_HCR_EL2);
  }
 
  /* Set up the per-CPU parts of the GIC for a secondary CPU */
xen/include/asm-arm/gic.h +0 -0
index d3d7bda50d..e4c77fefd6 100644

xen/include/asm-arm/gic.h line 123

Quoted 7 lines
xen/include/asm-arm/gic.h +1 -0
@@ -117,6 +117,7 @@
  #define GICH_HCR_VGRP0DIE (1 << 5)
  #define GICH_HCR_VGRP1EIE (1 << 6)
  #define GICH_HCR_VGRP1DIE (1 << 7)
+#define GICH_HCR_TALL1    (1 << 12)
 
  #define GICH_MISR_EOI     (1 << 0)
  #define GICH_MISR_U       (1 << 1)

Cheers,

Signature · 6 lines
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#11 lore raw

PATCH 5/12 Placeholder for handling Group1 register traps

In reply to [email protected]

From: Manish Jaggi <[email protected]>

Since this is a SoC errata and trapping of certain group1 registers should not affect the normal flow. A new file vsysreg_errata.c is added.

Function vgic_v3_handle_cpuif_access is called from do_trap_guest_sync if ARM64_WORKAROUND_CAVIUM_30115 capability is found.

A flag skip_hyp_tail is introduced in struct cpu_info. This flag specifies that leave_hypervisor_tail not to be called when handling group1 traps under this errata.

Signed-off-by Manish Jaggi <[email protected]>
---
 xen/arch/arm/arm64/Makefile         |  1 +
 xen/arch/arm/arm64/vsysreg_errata.c | 28 ++++++++++++++++++++++++++++
 xen/arch/arm/traps.c                | 20 ++++++++++++++++++++
 xen/include/asm-arm/arm64/traps.h   |  3 ++-
 xen/include/asm-arm/current.h       |  1 +
 5 files changed, 52 insertions(+), 1 deletion(-)
xen/arch/arm/arm64/Makefile +1 -0
index 718fe44455..19440c3d8c 100644
@@ -11,3 +11,4 @@ obj-y += smpboot.o
 obj-y += traps.o
 obj-y += vfp.o
 obj-y += vsysreg.o
+obj-$(CONFIG_CAVIUM_ERRATUM_30115) += vsysreg_errata.o
xen/arch/arm/arm64/vsysreg_errata.c new file +28 -0
new file mode 100644
index 0000000000..6af162bdf7
@@ -0,0 +1,28 @@
+#include <asm/current.h>
+#include <asm/regs.h>
+#include <asm/traps.h>
+#include <asm/system.h>
+
+bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr)
+{
+    bool ret = 0;
+
+    local_irq_disable();
+    if ( hsr.ec != HSR_EC_SYSREG )
+    {
+        ret = 1;
+        goto end;
+    }
+
+    switch ( hsr.bits & HSR_SYSREG_REGS_MASK )
+    {
+    default:
+        ret = 1;
+        break;
+    }
+end:
+    local_irq_enable();
+
+    return ret;
+}
+
xen/arch/arm/traps.c +20 -0
index f6f6de3691..9d08cd6ad3 100644
@@ -40,6 +40,7 @@
 #include <asm/acpi.h>
 #include <asm/cpuerrata.h>
 #include <asm/cpufeature.h>
+#include <asm/cpuerrata.h>
 #include <asm/debugger.h>
 #include <asm/event.h>
 #include <asm/flushtlb.h>

@@ -2103,6 +2104,21 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
 {
     const union hsr hsr = { .bits = regs->hsr };
 
+#ifdef CONFIG_CAVIUM_ERRATUM_30115
+    if ( cpus_have_cap(ARM64_WORKAROUND_CAVIUM_30115) )
+    {
+        int ret;
+        get_cpu_info()->skip_hyp_tail = 0;
+        ret  = vgic_v3_handle_cpuif_access(regs, hsr);
+        if ( !ret )
+        {
+            advance_pc(regs, hsr);
+            get_cpu_info()->skip_hyp_tail = 1;
+            return;
+        }
+    }
+#endif
+
     enter_hypervisor_head(regs);
 
     switch (hsr.ec) {

@@ -2295,6 +2311,10 @@ void do_trap_fiq(struct cpu_user_regs *regs)
 
 void leave_hypervisor_tail(void)
 {
+#ifdef CONFIG_CAVIUM_ERRATUM_30115
+    if ( get_cpu_info()->skip_hyp_tail )
+        return;
+#endif
     while (1)
     {
         local_irq_disable();
xen/include/asm-arm/arm64/traps.h +2 -1
index 2379b578cb..a5ae93ec11 100644
@@ -2,7 +2,8 @@
 #define __ASM_ARM64_TRAPS__
 
 void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len);
-
+bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs,
+                                const union hsr hsr);
 void do_sysreg(struct cpu_user_regs *regs,
                const union hsr hsr);
xen/include/asm-arm/current.h +1 -0
index 7a0971fdea..dacf3adc85 100644
@@ -22,6 +22,7 @@ struct cpu_info {
     struct cpu_user_regs guest_cpu_user_regs;
     unsigned long elr;
     unsigned int pad;
+    bool skip_hyp_tail;
 };
 
 static inline struct cpu_info *get_cpu_info(void)
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
#12 lore raw

Re: [PATCH 05/12] Placeholder for handling Group1 register traps

Hi Manish,

Quoted 11 lines · On 12/03/18 12:42, [email protected] wrote:

From: Manish Jaggi <[email protected]>

Since this is a SoC errata and trapping of certain group1 registers should not affect the normal flow. A new file vsysreg_errata.c is added.

Function vgic_v3_handle_cpuif_access is called from do_trap_guest_sync if ARM64_WORKAROUND_CAVIUM_30115 capability is found.

A flag skip_hyp_tail is introduced in struct cpu_info. This flag specifies that leave_hypervisor_tail not to be called when handling group1 traps under this errata.

Please give some rationale in the commit message why leave_hypervisor_tail is skipped on the errata.

Quoted 5 lines
Signed-off-by Manish Jaggi <[email protected]>
---
  xen/arch/arm/arm64/Makefile         |  1 +
  xen/arch/arm/arm64/vsysreg_errata.c | 28 ++++++++++++++++++++++++++++

The name of the file does not make sense, the errata is not about sysreg. It is about vGIC. Please rename it to vgic-v3-sr.c.

xen/arch/arm/arm64/vsysreg_errata.c

Quoted 20 lines
  xen/arch/arm/traps.c                | 20 ++++++++++++++++++++
  xen/include/asm-arm/arm64/traps.h   |  3 ++-
  xen/include/asm-arm/current.h       |  1 +
  5 files changed, 52 insertions(+), 1 deletion(-)
xen/arch/arm/arm64/Makefile +1 -0
index 718fe44455..19440c3d8c 100644
@@ -11,3 +11,4 @@ obj-y += smpboot.o
  obj-y += traps.o
  obj-y += vfp.o
  obj-y += vsysreg.o
+obj-$(CONFIG_CAVIUM_ERRATUM_30115) += vsysreg_errata.o
xen/arch/arm/arm64/vsysreg_errata.c new file +0 -0
new file mode 100644
index 0000000000..6af162bdf7
@@ -0,0 +1,28 @@

Missing copyright header.

xen/arch/arm/arm64/vsysreg_errata.c line 8

Quoted 8 lines
xen/arch/arm/arm64/vsysreg_errata.c new file +8 -0
+#include <asm/current.h>
+#include <asm/regs.h>
+#include <asm/traps.h>
+#include <asm/system.h>
+
+bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr)
+{
+    bool ret = 0;

You should use false/true when using bool. No plain integer.

xen/arch/arm/arm64/vsysreg_errata.c line 10

Quoted 2 lines
xen/arch/arm/arm64/vsysreg_errata.c new file +2 -0
+
+    local_irq_disable();

Please add a comment explain why IRQs are disabled.

Quoted 8 lines
xen/arch/arm/arm64/vsysreg_errata.c new file +8 -0
+    if ( hsr.ec != HSR_EC_SYSREG )
+    {
+        ret = 1;
+        goto end;
+    }
+
+    switch ( hsr.bits & HSR_SYSREG_REGS_MASK )
+    {

xen/arch/arm/arm64/vsysreg_errata.c line 28

Quoted 10 lines
xen/arch/arm/arm64/vsysreg_errata.c new file +10 -0
+    default:
+        ret = 1;
+        break;
+    }
+end:
+    local_irq_enable();
+
+    return ret;
+}
+

Missing emacs magic.

Quoted 13 lines
xen/arch/arm/traps.c +1 -0
index f6f6de3691..9d08cd6ad3 100644
@@ -40,6 +40,7 @@
  #include <asm/acpi.h>
  #include <asm/cpuerrata.h>
  #include <asm/cpufeature.h>
+#include <asm/cpuerrata.h>
  #include <asm/debugger.h>
  #include <asm/event.h>
  #include <asm/flushtlb.h>

xen/arch/arm/traps.c void do_trap_guest_sync(struct cpu_user_regs *regs) · line 2107

Quoted 4 lines
xen/arch/arm/traps.c +1 -0
@@ -2103,6 +2104,21 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
  {
      const union hsr hsr = { .bits = regs->hsr };
 
+#ifdef CONFIG_CAVIUM_ERRATUM_30115

I am not a big fan of #ifdef in the code. I think it would be better to stub the vgic_v3_handle_cpuif_access.

xen/arch/arm/traps.c void do_trap_guest_sync(struct cpu_user_regs *regs) · line 2108

Quoted 1 line
xen/arch/arm/traps.c +1 -0
+    if ( cpus_have_cap(ARM64_WORKAROUND_CAVIUM_30115) )

cpus_have_cap is expensive to use in hot path. This is because the function is going to check is the bits is set on every exit. You want to use check_workaround_* for that purpose as this will be replaced by an alternative.

xen/arch/arm/traps.c void do_trap_guest_sync(struct cpu_user_regs *regs) · line 2110

Quoted 2 lines
xen/arch/arm/traps.c +2 -0
+    {
+        int ret;

vgic_v3_handle_cpuif_access is returning a bool.

xen/arch/arm/traps.c void do_trap_guest_sync(struct cpu_user_regs *regs) · line 2111

Quoted 1 line
xen/arch/arm/traps.c +1 -0
+        get_cpu_info()->skip_hyp_tail = 0;

Same remark as above about bool and integer. But I think this one is not necessary. skip_hyp_tail is going to be false by default. So if you reset to false in the return path when it is true, you avoid

xen/arch/arm/traps.c void do_trap_guest_sync(struct cpu_user_regs *regs) · line 2116

Quoted 5 lines
xen/arch/arm/traps.c +5 -0
+        ret  = vgic_v3_handle_cpuif_access(regs, hsr);
+        if ( !ret )
+        {
+            advance_pc(regs, hsr);
+            get_cpu_info()->skip_hyp_tail = 1;

true.

xen/arch/arm/traps.c void do_trap_fiq(struct cpu_user_regs *regs) · line 2314

Quoted 13 lines
xen/arch/arm/traps.c +6 -0
+            return;
+        }
+    }
+#endif
+
      enter_hypervisor_head(regs);
 
      switch (hsr.ec) {

@@ -2295,6 +2311,10 @@ void do_trap_fiq(struct cpu_user_regs *regs)
 
  void leave_hypervisor_tail(void)
  {
+#ifdef CONFIG_CAVIUM_ERRATUM_30115

I don't think the #ifdef is necessary here. Supporting to skip the hypervisor tail is a nice feature to have.

xen/arch/arm/traps.c void do_trap_fiq(struct cpu_user_regs *regs) · line 2315

Quoted 1 line
xen/arch/arm/traps.c +1 -0
+    if ( get_cpu_info()->skip_hyp_tail )

You want this to be an unlikely(...).

Quoted 10 lines
xen/arch/arm/traps.c +2 -0
+        return;
+#endif
      while (1)
      {
          local_irq_disable();
xen/include/asm-arm/arm64/traps.h +0 -0
index 2379b578cb..a5ae93ec11 100644

xen/include/asm-arm/arm64/traps.h line 5

Quoted 4 lines
xen/include/asm-arm/arm64/traps.h +0 -1
@@ -2,7 +2,8 @@
  #define __ASM_ARM64_TRAPS__
 
  void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len);
-

Why removing the newline?

Quoted 10 lines
xen/include/asm-arm/arm64/traps.h +2 -0
+bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs,
+                                const union hsr hsr);
  void do_sysreg(struct cpu_user_regs *regs,
                 const union hsr hsr);
xen/include/asm-arm/current.h +0 -0
index 7a0971fdea..dacf3adc85 100644

xen/include/asm-arm/current.h struct cpu_info { · line 25

Quoted 4 lines
xen/include/asm-arm/current.h +1 -0
@@ -22,6 +22,7 @@ struct cpu_info {
      struct cpu_user_regs guest_cpu_user_regs;
      unsigned long elr;
      unsigned int pad;
+    bool skip_hyp_tail;

You should just reuse some bits of the padding (i.e 'pad' field) here.

bool skip_hyp_tail:1;
unsigned int pad:31;

Also, some documentation of the code would be highly appreciated.

xen/include/asm-arm/current.h struct cpu_info { · line 28

Quoted 4 lines
xen/include/asm-arm/current.h +0 -0
  };
 
  static inline struct cpu_info *get_cpu_info(void)

Cheers,

Signature · 6 lines
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#13 lore raw

PATCH 6/12 arm64: vgic-v3: Add ICV_BPR1_EL1 handler

In reply to [email protected]

From: Manish Jaggi <[email protected]>

This patch is ported to xen from linux commit
d70c7b31a60f2458f35c226131f2a01a7a98b6cf

Add a handler for reading/writing the guest's view of the ICC_BPR1_EL1 register, which is located in the ICH_VMCR_EL2.BPR1 field.

Signed-off-by Manish Jaggi <[email protected]>
---
 xen/arch/arm/arm64/vsysreg_errata.c | 71 +++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/arm64/sysregs.h |  1 +
 xen/include/asm-arm/gic_v3_defs.h   |  6 ++++
 3 files changed, 78 insertions(+)
xen/arch/arm/arm64/vsysreg_errata.c +71 -0
index 6af162bdf7..eb2b7ad74a 100644
@@ -2,10 +2,77 @@
 #include <asm/regs.h>
 #include <asm/traps.h>
 #include <asm/system.h>
+#include <asm/gic_v3_defs.h>
+
+#define vtr_to_nr_pre_bits(v)     ((((u32)(v) >> 26) & 7) + 1)
+
+static int  __vgic_v3_bpr_min(void)
+{
+    /* See Pseudocode for VPriorityGroup */
+    return 8 - vtr_to_nr_pre_bits(READ_SYSREG32(ICH_VTR_EL2));
+}
+
+static unsigned int __vgic_v3_get_bpr0(u32 vmcr)
+{
+    return (vmcr & ICH_VMCR_BPR0_MASK) >> ICH_VMCR_BPR0_SHIFT;
+}
+
+static unsigned int __vgic_v3_get_bpr1(u32 vmcr)
+{
+    unsigned int bpr;
+
+    if ( vmcr & ICH_VMCR_CBPR_MASK )
+    {
+        bpr = __vgic_v3_get_bpr0(vmcr);
+        if ( bpr < 7 )
+            bpr++;
+    }
+    else
+        bpr = (vmcr & ICH_VMCR_BPR1_MASK) >> ICH_VMCR_BPR1_SHIFT;
+
+    return bpr;
+}
+
+static void  __vgic_v3_read_bpr1(struct cpu_user_regs *regs, int regidx)
+{
+    u32 vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+    set_user_reg(regs, regidx, __vgic_v3_get_bpr1(vmcr));
+}
+
+static void  __vgic_v3_write_bpr1(struct cpu_user_regs *regs, int regidx)
+{
+    u64 val = get_user_reg(regs, regidx);
+    u8 bpr_min = __vgic_v3_bpr_min();
+    u32 vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+
+    if ( vmcr & ICH_VMCR_CBPR_MASK )
+        return;
+
+    /* Enforce BPR limiting */
+    if ( val < bpr_min )
+        val = bpr_min;
+
+    val <<= ICH_VMCR_BPR1_SHIFT;
+    val &= ICH_VMCR_BPR1_MASK;
+    vmcr &= ~ICH_VMCR_BPR1_MASK;
+    vmcr |= val;
+
+    WRITE_SYSREG32(vmcr, ICH_VMCR_EL2);
+}
+
+void handle_bpr1(struct cpu_user_regs *regs, int regidx, bool read,
+                 const union hsr hsr)
+{
+    if ( read )
+        __vgic_v3_read_bpr1(regs, regidx);
+    else
+        __vgic_v3_write_bpr1(regs, regidx);
+}
 
 bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr)
 {
     bool ret = 0;
+    int regidx = hsr.sysreg.reg;
 
     local_irq_disable();
     if ( hsr.ec != HSR_EC_SYSREG )

@@ -16,6 +83,10 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr
 
     switch ( hsr.bits & HSR_SYSREG_REGS_MASK )
     {
+    case HSR_SYSREG_ICC_BPR1_EL1:
+         handle_bpr1(regs, regidx, hsr.sysreg.read, hsr);
+         break;
+
     default:
         ret = 1;
         break;
xen/include/asm-arm/arm64/sysregs.h +1 -0
index 084d2a1e5d..025a27b0b4 100644
@@ -89,6 +89,7 @@
 #define HSR_SYSREG_ICC_ASGI1R_EL1 HSR_SYSREG(3,1,c12,c11,6)
 #define HSR_SYSREG_ICC_SGI0R_EL1  HSR_SYSREG(3,2,c12,c11,7)
 #define HSR_SYSREG_ICC_SRE_EL1    HSR_SYSREG(3,0,c12,c12,5)
+#define HSR_SYSREG_ICC_BPR1_EL1   HSR_SYSREG(3,0,c12,c12,3)
 #define HSR_SYSREG_CONTEXTIDR_EL1 HSR_SYSREG(3,0,c13,c0,1)
 
 #define HSR_SYSREG_PMCR_EL0       HSR_SYSREG(3,3,c9,c12,0)
xen/include/asm-arm/gic_v3_defs.h +6 -0
index 65c9dc47cf..68a34cc353 100644
@@ -157,6 +157,12 @@
 
 #define GICH_VMCR_EOI                (1 << 9)
 #define GICH_VMCR_VENG1              (1 << 1)
+#define ICH_VMCR_CBPR_SHIFT          4
+#define ICH_VMCR_CBPR_MASK           (1 << ICH_VMCR_CBPR_SHIFT)
+#define ICH_VMCR_BPR0_SHIFT          21
+#define ICH_VMCR_BPR0_MASK           (7 << ICH_VMCR_BPR0_SHIFT)
+#define ICH_VMCR_BPR1_SHIFT          18
+#define ICH_VMCR_BPR1_MASK           (7 << ICH_VMCR_BPR1_SHIFT)
 
 #define GICH_LR_VIRTUAL_MASK         0xffff
 #define GICH_LR_VIRTUAL_SHIFT        0
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#14 lore raw

PATCH 7/12 arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler

In reply to [email protected]

From: Manish Jaggi <[email protected]>

This patch is ported to xen from linux commit:
f8b630bc542e0368886ae193d3519c832b270359

Add a handler for reading/writing the guest's view of the ICC_IGRPEN1_EL1 register, which is located in the ICH_VMCR_EL2.VENG1 field.

Signed-off-by Manish Jaggi <[email protected]>
---
 xen/arch/arm/arm64/vsysreg_errata.c | 32 ++++++++++++++++++++++++++++++++
 xen/include/asm-arm/arm64/sysregs.h |  1 +
 xen/include/asm-arm/gic_v3_defs.h   |  2 ++
 3 files changed, 35 insertions(+)
xen/arch/arm/arm64/vsysreg_errata.c +32 -0
index eb2b7ad74a..93e9143a0d 100644
@@ -69,6 +69,34 @@ void handle_bpr1(struct cpu_user_regs *regs, int regidx, bool read,
         __vgic_v3_write_bpr1(regs, regidx);
 }
 
+static void  __vgic_v3_read_igrpen1(struct cpu_user_regs *regs, int regidx)
+{
+    u32 vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+    set_user_reg(regs, regidx, !!(vmcr & ICH_VMCR_ENG1_MASK));
+}
+
+static void  __vgic_v3_write_igrpen1(struct cpu_user_regs *regs, int regidx)
+{
+    u64 val = get_user_reg(regs, regidx);
+    u32 vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+
+    if ( val & 1 )
+        vmcr |= ICH_VMCR_ENG1_MASK;
+    else
+        vmcr &= ~ICH_VMCR_ENG1_MASK;
+
+    WRITE_SYSREG32(vmcr, ICH_VMCR_EL2);
+}
+
+void handle_igrpen1(struct cpu_user_regs *regs, int regidx, bool read,
+                 const union hsr hsr)
+{
+    if ( read )
+        __vgic_v3_read_igrpen1(regs, regidx);
+    else
+        __vgic_v3_write_igrpen1(regs, regidx);
+}
+
 bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr)
 {
     bool ret = 0;

@@ -87,6 +115,10 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr
          handle_bpr1(regs, regidx, hsr.sysreg.read, hsr);
          break;
 
+    case HSR_SYSREG_ICC_IGRPEN1_EL1:
+        handle_igrpen1(regs, regidx, hsr.sysreg.read, hsr);
+        break;
+
     default:
         ret = 1;
         break;
xen/include/asm-arm/arm64/sysregs.h +1 -0
index 025a27b0b4..731cabc74a 100644
@@ -90,6 +90,7 @@
 #define HSR_SYSREG_ICC_SGI0R_EL1  HSR_SYSREG(3,2,c12,c11,7)
 #define HSR_SYSREG_ICC_SRE_EL1    HSR_SYSREG(3,0,c12,c12,5)
 #define HSR_SYSREG_ICC_BPR1_EL1   HSR_SYSREG(3,0,c12,c12,3)
+#define HSR_SYSREG_ICC_IGRPEN1_EL1 HSR_SYSREG(3,0,c12,c12,7)
 #define HSR_SYSREG_CONTEXTIDR_EL1 HSR_SYSREG(3,0,c13,c0,1)
 
 #define HSR_SYSREG_PMCR_EL0       HSR_SYSREG(3,3,c9,c12,0)
xen/include/asm-arm/gic_v3_defs.h +2 -0
index 68a34cc353..ff8bda37d1 100644
@@ -163,6 +163,8 @@
 #define ICH_VMCR_BPR0_MASK           (7 << ICH_VMCR_BPR0_SHIFT)
 #define ICH_VMCR_BPR1_SHIFT          18
 #define ICH_VMCR_BPR1_MASK           (7 << ICH_VMCR_BPR1_SHIFT)
+#define ICH_VMCR_ENG1_SHIFT          1
+#define ICH_VMCR_ENG1_MASK           (1 << ICH_VMCR_ENG1_SHIFT)
 
 #define GICH_LR_VIRTUAL_MASK         0xffff
 #define GICH_LR_VIRTUAL_SHIFT        0
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#15 lore raw

PATCH 8/12 arm64: Add accessors for the ICH_APxRn_EL2 registers

In reply to [email protected]

From: Manish Jaggi <[email protected]>

This patch is ported to xen from linux commit
63000dd8006dc987db31ba670edc23142ea91e01

As we're about to access the Active Priority registers a lot more, let's define accessors that take the register number as a parameter.

Signed-off-by Manish Jaggi <[email protected]>
---
 xen/arch/arm/arm64/vsysreg_errata.c | 92 +++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)
xen/arch/arm/arm64/vsysreg_errata.c +92 -0
index 93e9143a0d..b2a95a69dc 100644
@@ -97,6 +97,98 @@ void handle_igrpen1(struct cpu_user_regs *regs, int regidx, bool read,
         __vgic_v3_write_igrpen1(regs, regidx);
 }
 
+void  __vgic_v3_write_ap0rn(u32 val, int n)
+{
+    switch (n)
+    {
+    case 0:
+        WRITE_SYSREG32(val, ICH_AP0R0_EL2);
+        break;
+    case 1:
+        WRITE_SYSREG32(val, ICH_AP0R1_EL2);
+        break;
+    case 2:
+        WRITE_SYSREG32(val, ICH_AP0R2_EL2);
+        break;
+    case 3:
+        WRITE_SYSREG32(val, ICH_AP0R3_EL2);
+        break;
+    default:
+        unreachable();
+    }
+}
+
+void __vgic_v3_write_ap1rn(u32 val, int n)
+{
+    switch (n)
+    {
+    case 0:
+        WRITE_SYSREG32(val, ICH_AP1R0_EL2);
+        break;
+    case 1:
+        WRITE_SYSREG32(val, ICH_AP1R1_EL2);
+        break;
+    case 2:
+        WRITE_SYSREG32(val, ICH_AP1R2_EL2);
+        break;
+    case 3:
+        WRITE_SYSREG32(val, ICH_AP1R3_EL2);
+        break;
+    default:
+        unreachable();
+    }
+}
+
+u32  __vgic_v3_read_ap0rn(int n)
+{
+    u32 val;
+
+    switch (n)
+    {
+    case 0:
+        val = READ_SYSREG32(ICH_AP0R0_EL2);
+        break;
+    case 1:
+        val = READ_SYSREG32(ICH_AP0R1_EL2);
+        break;
+    case 2:
+        val = READ_SYSREG32(ICH_AP0R2_EL2);
+        break;
+    case 3:
+        val = READ_SYSREG32(ICH_AP0R3_EL2);
+        break;
+    default:
+        unreachable();
+    }
+
+    return val;
+}
+
+u32  __vgic_v3_read_ap1rn(int n)
+{
+    u32 val;
+
+    switch (n)
+    {
+    case 0:
+        val = READ_SYSREG32(ICH_AP1R0_EL2);
+        break;
+    case 1:
+        val = READ_SYSREG32(ICH_AP1R1_EL2);
+        break;
+    case 2:
+        val = READ_SYSREG32(ICH_AP1R2_EL2);
+        break;
+    case 3:
+        val = READ_SYSREG32(ICH_AP1R3_EL2);
+        break;
+    default:
+        unreachable();
+    }
+
+    return val;
+}
+
 bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr)
 {
     bool ret = 0;
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#16 lore raw

PATCH 9/12 Expose ich_read/write_lr in vsysreg_errata.c

In reply to [email protected]

From: Manish Jaggi <[email protected]>

gicv3_ich_read/write_lr functions are duplicated in vsysreg_errata.c

Signed-off-by Manish Jaggi <[email protected]>
---
 xen/arch/arm/arm64/vsysreg_errata.c | 83 +++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)
xen/arch/arm/arm64/vsysreg_errata.c +83 -0
index b2a95a69dc..d7bf9d6ce3 100644
@@ -189,6 +189,89 @@ u32  __vgic_v3_read_ap1rn(int n)
     return val;
 }
 
+static uint64_t gicv3_ich_read_lr(int lr)
+{
+    switch ( lr )
+    {
+    case 0: return READ_SYSREG(ICH_LR0_EL2);
+    case 1: return READ_SYSREG(ICH_LR1_EL2);
+    case 2: return READ_SYSREG(ICH_LR2_EL2);
+    case 3: return READ_SYSREG(ICH_LR3_EL2);
+    case 4: return READ_SYSREG(ICH_LR4_EL2);
+    case 5: return READ_SYSREG(ICH_LR5_EL2);
+    case 6: return READ_SYSREG(ICH_LR6_EL2);
+    case 7: return READ_SYSREG(ICH_LR7_EL2);
+    case 8: return READ_SYSREG(ICH_LR8_EL2);
+    case 9: return READ_SYSREG(ICH_LR9_EL2);
+    case 10: return READ_SYSREG(ICH_LR10_EL2);
+    case 11: return READ_SYSREG(ICH_LR11_EL2);
+    case 12: return READ_SYSREG(ICH_LR12_EL2);
+    case 13: return READ_SYSREG(ICH_LR13_EL2);
+    case 14: return READ_SYSREG(ICH_LR14_EL2);
+    case 15: return READ_SYSREG(ICH_LR15_EL2);
+    default:
+        BUG();
+    }
+}
+
+static void gicv3_ich_write_lr(int lr, uint64_t val)
+{
+    switch ( lr )
+    {
+    case 0:
+        WRITE_SYSREG(val, ICH_LR0_EL2);
+        break;
+    case 1:
+        WRITE_SYSREG(val, ICH_LR1_EL2);
+        break;
+    case 2:
+        WRITE_SYSREG(val, ICH_LR2_EL2);
+        break;
+    case 3:
+        WRITE_SYSREG(val, ICH_LR3_EL2);
+        break;
+    case 4:
+        WRITE_SYSREG(val, ICH_LR4_EL2);
+        break;
+    case 5:
+        WRITE_SYSREG(val, ICH_LR5_EL2);
+        break;
+    case 6:
+        WRITE_SYSREG(val, ICH_LR6_EL2);
+        break;
+    case 7:
+        WRITE_SYSREG(val, ICH_LR7_EL2);
+        break;
+    case 8:
+        WRITE_SYSREG(val, ICH_LR8_EL2);
+        break;
+    case 9:
+        WRITE_SYSREG(val, ICH_LR9_EL2);
+        break;
+    case 10:
+        WRITE_SYSREG(val, ICH_LR10_EL2);
+        break;
+    case 11:
+        WRITE_SYSREG(val, ICH_LR11_EL2);
+        break;
+    case 12:
+        WRITE_SYSREG(val, ICH_LR12_EL2);
+        break;
+    case 13:
+        WRITE_SYSREG(val, ICH_LR13_EL2);
+        break;
+    case 14:
+        WRITE_SYSREG(val, ICH_LR14_EL2);
+        break;
+    case 15:
+        WRITE_SYSREG(val, ICH_LR15_EL2);
+        break;
+    default:
+        return;
+    }
+    isb();
+}
+
 bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr)
 {
     bool ret = 0;
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
#17 lore raw

Re: [PATCH 09/12] Expose ich_read/write_lr in vsysreg_errata.c

Quoted 3 lines · On 12/03/18 12:42, [email protected] wrote:

From: Manish Jaggi <[email protected]>

gicv3_ich_read/write_lr functions are duplicated in vsysreg_errata.c

Please explain the rationale. I.e we want to have the workaround standalone.

Cheers,

Quoted 101 lines
Signed-off-by Manish Jaggi <[email protected]>
---
  xen/arch/arm/arm64/vsysreg_errata.c | 83 +++++++++++++++++++++++++++++++++++++
  1 file changed, 83 insertions(+)
xen/arch/arm/arm64/vsysreg_errata.c +83 -0
index b2a95a69dc..d7bf9d6ce3 100644
@@ -189,6 +189,89 @@ u32  __vgic_v3_read_ap1rn(int n)
      return val;
  }
 
+static uint64_t gicv3_ich_read_lr(int lr)
+{
+    switch ( lr )
+    {
+    case 0: return READ_SYSREG(ICH_LR0_EL2);
+    case 1: return READ_SYSREG(ICH_LR1_EL2);
+    case 2: return READ_SYSREG(ICH_LR2_EL2);
+    case 3: return READ_SYSREG(ICH_LR3_EL2);
+    case 4: return READ_SYSREG(ICH_LR4_EL2);
+    case 5: return READ_SYSREG(ICH_LR5_EL2);
+    case 6: return READ_SYSREG(ICH_LR6_EL2);
+    case 7: return READ_SYSREG(ICH_LR7_EL2);
+    case 8: return READ_SYSREG(ICH_LR8_EL2);
+    case 9: return READ_SYSREG(ICH_LR9_EL2);
+    case 10: return READ_SYSREG(ICH_LR10_EL2);
+    case 11: return READ_SYSREG(ICH_LR11_EL2);
+    case 12: return READ_SYSREG(ICH_LR12_EL2);
+    case 13: return READ_SYSREG(ICH_LR13_EL2);
+    case 14: return READ_SYSREG(ICH_LR14_EL2);
+    case 15: return READ_SYSREG(ICH_LR15_EL2);
+    default:
+        BUG();
+    }
+}
+
+static void gicv3_ich_write_lr(int lr, uint64_t val)
+{
+    switch ( lr )
+    {
+    case 0:
+        WRITE_SYSREG(val, ICH_LR0_EL2);
+        break;
+    case 1:
+        WRITE_SYSREG(val, ICH_LR1_EL2);
+        break;
+    case 2:
+        WRITE_SYSREG(val, ICH_LR2_EL2);
+        break;
+    case 3:
+        WRITE_SYSREG(val, ICH_LR3_EL2);
+        break;
+    case 4:
+        WRITE_SYSREG(val, ICH_LR4_EL2);
+        break;
+    case 5:
+        WRITE_SYSREG(val, ICH_LR5_EL2);
+        break;
+    case 6:
+        WRITE_SYSREG(val, ICH_LR6_EL2);
+        break;
+    case 7:
+        WRITE_SYSREG(val, ICH_LR7_EL2);
+        break;
+    case 8:
+        WRITE_SYSREG(val, ICH_LR8_EL2);
+        break;
+    case 9:
+        WRITE_SYSREG(val, ICH_LR9_EL2);
+        break;
+    case 10:
+        WRITE_SYSREG(val, ICH_LR10_EL2);
+        break;
+    case 11:
+        WRITE_SYSREG(val, ICH_LR11_EL2);
+        break;
+    case 12:
+        WRITE_SYSREG(val, ICH_LR12_EL2);
+        break;
+    case 13:
+        WRITE_SYSREG(val, ICH_LR13_EL2);
+        break;
+    case 14:
+        WRITE_SYSREG(val, ICH_LR14_EL2);
+        break;
+    case 15:
+        WRITE_SYSREG(val, ICH_LR15_EL2);
+        break;
+    default:
+        return;
+    }
+    isb();
+}
+
  bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr)
  {
      bool ret = 0;
Signature · 6 lines
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#18 lore raw

PATCH 10/12 arm64: Add ICV_IAR1_EL1 handler

In reply to [email protected]

From: Manish Jaggi <[email protected]>

This patch is ported to xen from linux commit
132a324ab62fe4fb8d6dcc2ab4eddb0e93b69afe.

Add a handler for reading the guest's view of the ICC_IAR1_EL1 register. This involves finding the highest priority Group-1 interrupt, checking against both PMR and the active group priority, activating the interrupt and setting the group priority as active.

Signed-off-by Manish Jaggi <[email protected]>
---
 xen/arch/arm/arm64/vsysreg_errata.c | 196 ++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/arm64/sysregs.h |   1 +
 xen/include/asm-arm/gic_v3_defs.h   |  17 ++++
 3 files changed, 214 insertions(+)
xen/arch/arm/arm64/vsysreg_errata.c +196 -0
index d7bf9d6ce3..9bc1d7b58a 100644
@@ -3,8 +3,18 @@
 #include <asm/traps.h>
 #include <asm/system.h>
 #include <asm/gic_v3_defs.h>
+#include <xen/sched.h>
+#include <asm/vtimer.h>
+
 
 #define vtr_to_nr_pre_bits(v)     ((((u32)(v) >> 26) & 7) + 1)
+#define vtr_to_nr_apr_regs(v)     (1 << (vtr_to_nr_pre_bits(v) - 5))
+
+#define ESR_ELx_SYS64_ISS_CRM_SHIFT 1
+#define ESR_ELx_SYS64_ISS_CRM_MASK (0xf << ESR_ELx_SYS64_ISS_CRM_SHIFT)
+
+#define ICC_IAR1_EL1_SPURIOUS    0x3ff
+#define VGIC_MAX_SPI             1019
 
 static int  __vgic_v3_bpr_min(void)
 {

@@ -272,6 +282,188 @@ static void gicv3_ich_write_lr(int lr, uint64_t val)
     isb();
 }
 
+static int  __vgic_v3_get_group(const union hsr hsr)
+{
+    u8 crm = (hsr.bits & ESR_ELx_SYS64_ISS_CRM_MASK) >>
+              ESR_ELx_SYS64_ISS_CRM_SHIFT;
+
+    return crm != 8;
+}
+
+unsigned int gic_get_num_lrs(void)
+{
+    uint32_t vtr;
+
+    vtr = READ_SYSREG32(ICH_VTR_EL2);
+    return (vtr & GICH_VTR_NRLRGS) + 1;
+}
+
+static int __vgic_v3_highest_priority_lr(struct cpu_user_regs *regs,
+                                         u32 vmcr, u64 *lr_val)
+{
+    int i, lr = -1;
+    unsigned int used_lrs =  gic_get_num_lrs();
+    u8 priority = GICV3_IDLE_PRIORITY;
+
+    for ( i = 0; i < used_lrs; i++ )
+    {
+        u64 val =  gicv3_ich_read_lr(i);
+        u8 lr_prio = (val & ICH_LR_PRIORITY_MASK) >> ICH_LR_PRIORITY_SHIFT;
+
+        /* Not pending in the state? */
+        if ( (val & ICH_LR_STATE) != ICH_LR_PENDING_BIT )
+            continue;
+
+        /* Group-0 interrupt, but Group-0 disabled? */
+        if ( !(val & ICH_LR_GROUP) && !(vmcr & ICH_VMCR_ENG0_MASK) )
+            continue;
+
+        /* Group-1 interrupt, but Group-1 disabled? */
+        if ( (val & ICH_LR_GROUP) && !(vmcr & ICH_VMCR_ENG1_MASK) )
+            continue;
+
+        /* Not the highest priority? */
+        if ( lr_prio >= priority )
+            continue;
+
+        /* This is a candidate */
+        priority = lr_prio;
+        *lr_val = val;
+        lr = i;
+    }
+
+    if ( lr == -1 )
+        *lr_val = ICC_IAR1_EL1_SPURIOUS;
+
+    return lr;
+}
+
+static int  __vgic_v3_get_highest_active_priority(void)
+{
+    int i;
+    u32 hap = 0;
+    u8 nr_apr_regs = vtr_to_nr_apr_regs(READ_SYSREG32(ICH_VTR_EL2));
+
+    for ( i = 0; i < nr_apr_regs; i++ )
+    {
+        u32 val;
+
+        /*
+         * The ICH_AP0Rn_EL2 and ICH_AP1Rn_EL2 registers
+         * contain the active priority levels for this VCPU
+         * for the maximum number of supported priority
+         * levels, and we return the full priority level only
+         * if the BPR is programmed to its minimum, otherwise
+         * we return a combination of the priority level and
+         * subpriority, as determined by the setting of the
+         * BPR, but without the full subpriority.
+         */
+        val  = __vgic_v3_read_ap0rn(i);
+        val |= __vgic_v3_read_ap1rn(i);
+        if ( !val )
+        {
+            hap += 32;
+            continue;
+        }
+
+        return (hap + __ffs(val)) << __vgic_v3_bpr_min();
+    }
+
+    return GICV3_IDLE_PRIORITY;
+}
+
+/*
+ * Convert a priority to a preemption level, taking the relevant BPR
+ * into account by zeroing the sub-priority bits.
+ */
+static u8  __vgic_v3_pri_to_pre(u8 pri, u32 vmcr, int grp)
+{
+    unsigned int bpr;
+
+    if ( !grp )
+        bpr = __vgic_v3_get_bpr0(vmcr) + 1;
+    else
+        bpr = __vgic_v3_get_bpr1(vmcr);
+
+    return pri & (GENMASK(7, 0) << bpr);
+}
+
+/*
+ * The priority value is independent of any of the BPR values, so we
+ * normalize it using the minumal BPR value. This guarantees that no
+ * matter what the guest does with its BPR, we can always set/get the
+ * same value of a priority.
+ */
+static void  __vgic_v3_set_active_priority(u8 pri, u32 vmcr, int grp)
+{
+    u8 pre, ap;
+    u32 val;
+    int apr;
+
+    pre = __vgic_v3_pri_to_pre(pri, vmcr, grp);
+    ap = pre >> __vgic_v3_bpr_min();
+    apr = ap / 32;
+
+    if ( !grp )
+    {
+        val = __vgic_v3_read_ap0rn(apr);
+        __vgic_v3_write_ap0rn(val | BIT(ap % 32), apr);
+    }
+    else
+    {
+        val = __vgic_v3_read_ap1rn(apr);
+        __vgic_v3_write_ap1rn(val | BIT(ap % 32), apr);
+    }
+}
+
+static void  __vgic_v3_read_iar(struct cpu_user_regs *regs, int regidx,
+                                const union hsr hsr)
+{
+    u64 lr_val;
+    u8 lr_prio, pmr;
+    int lr, grp;
+
+    u32 vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+    grp = __vgic_v3_get_group(hsr);
+
+    lr = __vgic_v3_highest_priority_lr(regs, vmcr, &lr_val);
+    if ( lr < 0 )
+        goto spurious;
+
+    if ( grp != !!(lr_val & ICH_LR_GROUP) )
+        goto spurious;
+
+    pmr = (vmcr & ICH_VMCR_PMR_MASK) >> ICH_VMCR_PMR_SHIFT;
+    lr_prio = (lr_val & ICH_LR_PRIORITY_MASK) >> ICH_LR_PRIORITY_SHIFT;
+    if ( pmr <= lr_prio )
+        goto spurious;
+
+    if ( __vgic_v3_get_highest_active_priority() <=
+         __vgic_v3_pri_to_pre(lr_prio, vmcr, grp) )
+        goto spurious;
+
+    lr_val &= ~ICH_LR_STATE;
+    /* No active state for LPIs */
+    if ( (lr_val & ICH_LR_VIRTUAL_ID_MASK) <= VGIC_MAX_SPI )
+        lr_val |= ICH_LR_ACTIVE_BIT;
+
+    gicv3_ich_write_lr(lr, lr_val);
+    __vgic_v3_set_active_priority(lr_prio, vmcr, grp);
+    set_user_reg(regs, regidx,  lr_val & ICH_LR_VIRTUAL_ID_MASK);
+
+    return;
+
+spurious:
+     set_user_reg(regs, regidx, ICC_IAR1_EL1_SPURIOUS);
+}
+
+void handle_iar(struct cpu_user_regs *regs, int regidx, const union hsr hsr)
+{
+    __vgic_v3_read_iar(regs, regidx, hsr);
+}
+
+
+
 bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr)
 {
     bool ret = 0;

@@ -294,6 +486,10 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr
         handle_igrpen1(regs, regidx, hsr.sysreg.read, hsr);
         break;
 
+    case HSR_SYSREG_ICC_IAR1_EL1:
+        handle_iar(regs, regidx, hsr);
+        break;
+
     default:
         ret = 1;
         break;
xen/include/asm-arm/arm64/sysregs.h +1 -0
index 731cabc74a..53d2251840 100644
@@ -91,6 +91,7 @@
 #define HSR_SYSREG_ICC_SRE_EL1    HSR_SYSREG(3,0,c12,c12,5)
 #define HSR_SYSREG_ICC_BPR1_EL1   HSR_SYSREG(3,0,c12,c12,3)
 #define HSR_SYSREG_ICC_IGRPEN1_EL1 HSR_SYSREG(3,0,c12,c12,7)
+#define HSR_SYSREG_ICC_IAR1_EL1   HSR_SYSREG(3,0,c12,c12,0)
 #define HSR_SYSREG_CONTEXTIDR_EL1 HSR_SYSREG(3,0,c13,c0,1)
 
 #define HSR_SYSREG_PMCR_EL0       HSR_SYSREG(3,3,c9,c12,0)
xen/include/asm-arm/gic_v3_defs.h +17 -0
index ff8bda37d1..884fce0fd0 100644
@@ -67,6 +67,7 @@
  */
 #define GICV3_GICD_IIDR_VAL          0x34c
 #define GICV3_GICR_IIDR_VAL          GICV3_GICD_IIDR_VAL
+#define GICV3_IDLE_PRIORITY          0xff
 
 #define GICR_CTLR                    (0x0000)
 #define GICR_IIDR                    (0x0004)

@@ -165,6 +166,10 @@
 #define ICH_VMCR_BPR1_MASK           (7 << ICH_VMCR_BPR1_SHIFT)
 #define ICH_VMCR_ENG1_SHIFT          1
 #define ICH_VMCR_ENG1_MASK           (1 << ICH_VMCR_ENG1_SHIFT)
+#define ICH_VMCR_ENG0_SHIFT          0
+#define ICH_VMCR_ENG0_MASK           (1 << ICH_VMCR_ENG0_SHIFT)
+#define ICH_VMCR_PMR_SHIFT           24
+#define ICH_VMCR_PMR_MASK            (0xffUL << ICH_VMCR_PMR_SHIFT)
 
 #define GICH_LR_VIRTUAL_MASK         0xffff
 #define GICH_LR_VIRTUAL_SHIFT        0

@@ -182,6 +187,18 @@
 #define GICH_LR_GRP1                 (1UL<<60)
 #define GICH_LR_HW                   (1UL<<61)
 
+#define ICH_LR_PRIORITY_SHIFT        48
+#define ICH_LR_PRIORITY_MASK         (0xffULL << ICH_LR_PRIORITY_SHIFT)
+#define ICH_LR_EOI                   (1ULL << 41)
+#define ICH_LR_GROUP                 (1ULL << 60)
+#define ICH_LR_HW                    (1ULL << 61)
+#define ICH_LR_STATE                 (3ULL << 62)
+#define ICH_LR_PENDING_BIT           (1ULL << 62)
+#define ICH_LR_ACTIVE_BIT            (1ULL << 63)
+#define ICH_LR_PHYS_ID_SHIFT         32
+#define ICH_LR_PHYS_ID_MASK          (0x3ffULL << ICH_LR_PHYS_ID_SHIFT)
+#define ICH_LR_VIRTUAL_ID_MASK       ((1ULL << 32) - 1)
+
 #define GICH_VTR_NRLRGS              0x3f
 #define GICH_VTR_PRIBITS_MASK        0x7
 #define GICH_VTR_PRIBITS_SHIFT       29
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#19 lore raw

PATCH 11/12 arm64: vgic-v3: Add ICV_EOIR1_EL1 handler

In reply to [email protected]

From: Manish Jaggi <[email protected]>

This patch is ported to xen from linux commit
b6f49035b4bf6e2709f2a5fed3107f5438c1fd02

Add a handler for writing the guest's view of the ICC_EOIR1_EL1 register. This involves dropping the priority of the interrupt, and deactivating it if required (EOImode == 0).

Signed-off-by Manish Jaggi <[email protected]>
---
 xen/arch/arm/arm64/vsysreg_errata.c | 134 ++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/arm64/sysregs.h |   1 +
 xen/include/asm-arm/gic_v3_defs.h   |   4 ++
 3 files changed, 139 insertions(+)
xen/arch/arm/arm64/vsysreg_errata.c +134 -0
index 9bc1d7b58a..869d67640f 100644
@@ -15,6 +15,7 @@
 
 #define ICC_IAR1_EL1_SPURIOUS    0x3ff
 #define VGIC_MAX_SPI             1019
+#define VGIC_MIN_LPI             8192
 
 static int  __vgic_v3_bpr_min(void)
 {

@@ -462,7 +463,136 @@ void handle_iar(struct cpu_user_regs *regs, int regidx, const union hsr hsr)
     __vgic_v3_read_iar(regs, regidx, hsr);
 }
 
+static int  __vgic_v3_find_active_lr(int intid, u64 *lr_val)
+{
+    int i;
+    unsigned int used_lrs =  gic_get_num_lrs();
+
+    for ( i = 0; i < used_lrs; i++ )
+    {
+        u64 val = gicv3_ich_read_lr(i);
+
+        if ( (val & ICH_LR_VIRTUAL_ID_MASK) == intid &&
+            (val & ICH_LR_ACTIVE_BIT) )
+        {
+            *lr_val = val;
+            return i;
+        }
+    }
+
+    *lr_val = ICC_IAR1_EL1_SPURIOUS;
+    return -1;
+}
+
+static int  __vgic_v3_clear_highest_active_priority(void)
+{
+    u32 hap = 0;
+    int i;
+    u8 nr_apr_regs = vtr_to_nr_apr_regs(READ_SYSREG32(ICH_VTR_EL2));
+
+    for ( i = 0; i < nr_apr_regs; i++ )
+    {
+        u32 ap0, ap1;
+        int c0, c1;
+
+        ap0 = __vgic_v3_read_ap0rn(i);
+        ap1 = __vgic_v3_read_ap1rn(i);
+        if ( !ap0 && !ap1 )
+        {
+            hap += 32;
+            continue;
+        }
+
+        c0 = ap0 ? __ffs(ap0) : 32;
+        c1 = ap1 ? __ffs(ap1) : 32;
+
+        /* Always clear the LSB, which is the highest priority */
+        if ( c0 < c1 )
+        {
+            ap0 &= ~BIT(c0);
+            __vgic_v3_write_ap0rn(ap0, i);
+            hap += c0;
+        }
+        else
+        {
+            ap1 &= ~BIT(c1);
+            __vgic_v3_write_ap1rn(ap1, i);
+            hap += c1;
+        }
+
+        /* Rescale to 8 bits of priority */
+        return hap << __vgic_v3_bpr_min();
+    }
+
+    return GICV3_IDLE_PRIORITY;
+}
+
+static void  __vgic_v3_clear_active_lr(int lr, u64 lr_val)
+{
+    lr_val &= ~ICH_LR_ACTIVE_BIT;
+    if ( lr_val & ICH_LR_HW )
+    {
+        u32 pid;
+
+        pid = (lr_val & ICH_LR_PHYS_ID_MASK) >> ICH_LR_PHYS_ID_SHIFT;
+        WRITE_SYSREG32(pid, ICC_DIR_EL1);
+    }
+    gicv3_ich_write_lr(lr, lr_val);
+}
+
+static void  __vgic_v3_bump_eoicount(void)
+{
+    u32 hcr;
+
+    hcr = READ_SYSREG32(ICH_HCR_EL2);
+    hcr += 1 << ICH_HCR_EOIcount_SHIFT;
+    WRITE_SYSREG32(hcr, ICH_HCR_EL2);
+}
 
+static void  __vgic_v3_write_eoir(struct cpu_user_regs *regs, int regidx,
+                                  const union hsr hsr)
+{
+    u32 vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+    u32 vid = get_user_reg(regs, regidx);
+    u64 lr_val;
+    u8 lr_prio, act_prio;
+    int lr, grp;
+
+    grp = __vgic_v3_get_group(hsr);
+
+    /* Drop priority in any case */
+    act_prio = __vgic_v3_clear_highest_active_priority();
+
+    /* If EOIing an LPI, no deactivate to be performed */
+    if ( vid >= VGIC_MIN_LPI )
+        return;
+
+    /* EOImode == 1, nothing to be done here */
+    if ( vmcr & ICH_VMCR_EOIM_MASK )
+        return;
+
+    lr = __vgic_v3_find_active_lr(vid, &lr_val);
+    if ( lr == -1 )
+    {
+        __vgic_v3_bump_eoicount();
+        return;
+    }
+
+    lr_prio = (lr_val & ICH_LR_PRIORITY_MASK) >> ICH_LR_PRIORITY_SHIFT;
+
+    /* If priorities or group do not match, the guest has fscked-up. */
+    if ( grp != !!(lr_val & ICH_LR_GROUP) ||
+         __vgic_v3_pri_to_pre(lr_prio, vmcr, grp) != act_prio )
+        return;
+
+    /* Let's now perform the deactivation */
+    __vgic_v3_clear_active_lr(lr, lr_val);
+}
+
+void handle_eoi(struct cpu_user_regs *regs, int regidx, const union hsr hsr)
+{
+    __vgic_v3_write_eoir(regs, regidx, hsr);
+}
 
 bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr)
 {

@@ -490,6 +620,10 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr
         handle_iar(regs, regidx, hsr);
         break;
 
+    case HSR_SYSREG_ICC_EOIR1_EL1:
+        handle_eoi(regs, regidx, hsr);
+        break;
+
     default:
         ret = 1;
         break;
xen/include/asm-arm/arm64/sysregs.h +1 -0
index 53d2251840..f9110ebf9c 100644
@@ -92,6 +92,7 @@
 #define HSR_SYSREG_ICC_BPR1_EL1   HSR_SYSREG(3,0,c12,c12,3)
 #define HSR_SYSREG_ICC_IGRPEN1_EL1 HSR_SYSREG(3,0,c12,c12,7)
 #define HSR_SYSREG_ICC_IAR1_EL1   HSR_SYSREG(3,0,c12,c12,0)
+#define HSR_SYSREG_ICC_EOIR1_EL1  HSR_SYSREG(3,0,c12,c12,1)
 #define HSR_SYSREG_CONTEXTIDR_EL1 HSR_SYSREG(3,0,c13,c0,1)
 
 #define HSR_SYSREG_PMCR_EL0       HSR_SYSREG(3,3,c9,c12,0)
xen/include/asm-arm/gic_v3_defs.h +4 -0
index 884fce0fd0..b169e2cb78 100644
@@ -170,6 +170,10 @@
 #define ICH_VMCR_ENG0_MASK           (1 << ICH_VMCR_ENG0_SHIFT)
 #define ICH_VMCR_PMR_SHIFT           24
 #define ICH_VMCR_PMR_MASK            (0xffUL << ICH_VMCR_PMR_SHIFT)
+#define ICH_VMCR_EOIM_SHIFT          9
+#define ICH_VMCR_EOIM_MASK           (1 << ICH_VMCR_EOIM_SHIFT)
+#define ICH_HCR_EOIcount_SHIFT       27
+#define ICH_HCR_EOIcount_MASK        (0x1f << ICH_HCR_EOIcount_SHIFT)
 
 #define GICH_LR_VIRTUAL_MASK         0xffff
 #define GICH_LR_VIRTUAL_SHIFT        0
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
author
#20 lore raw

PATCH 12/12 arm64: vgic-v3: Add ICV_HPPIR1_EL1 handler

In reply to [email protected]

From: Manish Jaggi <[email protected]>

This patch is ported from linux to xen
commit: 2724c11a1df4b22ee966c04809ea0e808f66b04e
(KVM: arm64: vgic-v3: Add ICV_HPPIR1_EL1 handler)

Add a handler for reading the guest's view of the ICV_HPPIR1_EL1 register. This is a simple parsing of the available LRs, extracting the highest available interrupt.

Signed-off-by Manish Jaggi <[email protected]>
---
 xen/arch/arm/arm64/vsysreg_errata.c | 24 ++++++++++++++++++++++++
 xen/include/asm-arm/arm64/sysregs.h |  1 +
 2 files changed, 25 insertions(+)
xen/arch/arm/arm64/vsysreg_errata.c +24 -0
index 869d67640f..088d39613d 100644
@@ -594,6 +594,26 @@ void handle_eoi(struct cpu_user_regs *regs, int regidx, const union hsr hsr)
     __vgic_v3_write_eoir(regs, regidx, hsr);
 }
 
+void handle_hppir1(struct cpu_user_regs *regs, int regidx, const union hsr hsr)
+{
+    u64 lr_val;
+    int lr, lr_grp, grp;
+    u32 vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+
+    grp = __vgic_v3_get_group(hsr);
+    lr = __vgic_v3_highest_priority_lr(regs, vmcr, &lr_val);
+
+    if ( lr == -1 )
+        goto spurious;
+
+    lr_grp = !!(lr_val & ICH_LR_GROUP);
+    if ( lr_grp != grp )
+        lr_val = ICC_IAR1_EL1_SPURIOUS;
+
+spurious:
+    set_user_reg(regs, regidx, lr_val & ICH_LR_VIRTUAL_ID_MASK);
+}
+
 bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr)
 {
     bool ret = 0;

@@ -624,6 +644,10 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr
         handle_eoi(regs, regidx, hsr);
         break;
 
+    case HSR_SYSREG_ICC_HPPIR1_EL1:
+        handle_hppir1(regs, regidx, hsr);
+        break;
+
     default:
         ret = 1;
         break;
xen/include/asm-arm/arm64/sysregs.h +1 -0
index f9110ebf9c..c23c4a33b2 100644
@@ -93,6 +93,7 @@
 #define HSR_SYSREG_ICC_IGRPEN1_EL1 HSR_SYSREG(3,0,c12,c12,7)
 #define HSR_SYSREG_ICC_IAR1_EL1   HSR_SYSREG(3,0,c12,c12,0)
 #define HSR_SYSREG_ICC_EOIR1_EL1  HSR_SYSREG(3,0,c12,c12,1)
+#define HSR_SYSREG_ICC_HPPIR1_EL1 HSR_SYSREG(3,0,c12,c12,2)
 #define HSR_SYSREG_CONTEXTIDR_EL1 HSR_SYSREG(3,0,c13,c0,1)
 
 #define HSR_SYSREG_PMCR_EL0       HSR_SYSREG(3,3,c9,c12,0)
Signature · 7 lines
2.14.1


_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
#21 lore raw
In reply to [email protected]

Manish,

Quoted 9 lines · On 12/03/18 12:42, [email protected] wrote:

From: Manish Jaggi <[email protected]>

This patchset is a Xen port of Marc's patchset.
arm64: KVM: Mediate access to GICv3 sysregs at EL2 [1]

The current RFC patchset is a subset of [1], as it handleing only Group1 traps as a PoC. Most of the trap code is added in vsysreg.c. Trap handler function is kept independent of the usual guest trap handling code.
Looking for feedback on this approach.

Why Group-1 only? AFAIK, Group-0 is affected as well, and results in a DoS on the secure side. Only handling Group-1 makes it hard to compare to the reference implementation (which handles booth groups), and will introduce pointless churn once you start adding Group-0 handling.

As it stands, this series is a bit pointless. Please consider posting a complete fix.

Thanks,

	M.
Signature · 6 lines
Jazz is not dead. It just smells funny...

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
#22 lore raw
In reply to [email protected]

Hi,

Manish, I don't monitor my linaro email and would appreciate if you use the e-mail address provided by MAINTAINERS.

Cheers,

Quoted 58 lines · On 12/03/18 12:42, [email protected] wrote:

From: Manish Jaggi <[email protected]>

This patchset is a Xen port of Marc's patchset.
arm64: KVM: Mediate access to GICv3 sysregs at EL2 [1]

The current RFC patchset is a subset of [1], as it handleing only Group1 traps as a PoC. Most of the trap code is added in vsysreg.c. Trap handler function is kept independent of the usual guest trap handling code.
Looking for feedback on this approach.

The errata has been validated on Cavium ThunderX platform.

Steps to reporduce the errata
- Boot Xen with 2 cores.
- Disable group1 interrupts in domU kernel
- start domU, the kill and start again.
One of the Xen core would hang.

Code in this patchset fixes this issue.

[1] https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/026029.html

Changes from RFC
- Added commit information on ported patches from linux
- Added skip_hyp_tail to control calling leave_hypervisor_tail
- Added CAVIUM_CONFIG_ERRATUM_30115 which will auto enable workaround

Manish Jaggi (12):
arm:Kconfig Rename menu text
arm64: cputype: Add MIDR values for Cavium ThunderX1 CPUs
arm64: Add config for Cavium Thunder erratum 30115 Enable Group1 Traps by default for Cavium ThunderX Placeholder for handling Group1 register traps
arm64: vgic-v3: Add ICV_BPR1_EL1 handler
arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler
arm64: Add accessors for the ICH_APxRn_EL2 registers Expose ich_read/write_lr in vsysreg_errata.c
arm64: Add ICV_IAR1_EL1 handler
arm64: vgic-v3: Add ICV_EOIR1_EL1 handler
arm64: vgic-v3: Add ICV_HPPIR1_EL1 handler

  xen/arch/arm/Kconfig                |   6 +-
  xen/arch/arm/arm64/Makefile         |   1 +
  xen/arch/arm/arm64/vsysreg_errata.c | 660 ++++++++++++++++++++++++++++++++++++
  xen/arch/arm/cpuerrata.c            |  21 ++
  xen/arch/arm/gic-v3.c               |  16 +-
  xen/arch/arm/traps.c                |  20 ++
  xen/include/asm-arm/arm64/sysregs.h |   5 +
  xen/include/asm-arm/arm64/traps.h   |   3 +-
  xen/include/asm-arm/cpuerrata.h     |   1 +
  xen/include/asm-arm/cpufeature.h    |   3 +-
  xen/include/asm-arm/current.h       |   1 +
  xen/include/asm-arm/gic.h           |   1 +
  xen/include/asm-arm/gic_v3_defs.h   |  29 ++
  xen/include/asm-arm/processor.h     |   9 +
  14 files changed, 771 insertions(+), 5 deletions(-)

create mode 100644 xen/arch/arm/arm64/vsysreg_errata.c

Signature · 6 lines
Julien Grall

_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel