All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2
@ 2018-03-12 12:42 mjaggi
  2018-03-12 12:42 ` [PATCH 01/12] arm:Kconfig Rename menu text mjaggi
                   ` (13 more replies)
  0 siblings, 14 replies; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

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

-- 
2.14.1


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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 01/12] arm:Kconfig Rename menu text
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
@ 2018-03-12 12:42 ` mjaggi
  2018-03-13 12:15   ` Julien Grall
  2018-03-12 12:42 ` [PATCH 02/12] arm64: cputype: Add MIDR values for Cavium ThunderX1 CPUs mjaggi
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

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

Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
---
 xen/arch/arm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index f58019d6ed..10a6d1a956 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -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
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 02/12] arm64: cputype: Add MIDR values for Cavium ThunderX1 CPUs
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
  2018-03-12 12:42 ` [PATCH 01/12] arm:Kconfig Rename menu text mjaggi
@ 2018-03-12 12:42 ` mjaggi
  2018-03-13 12:17   ` Julien Grall
  2018-03-12 12:42 ` [PATCH 03/12] arm64: Add config for Cavium Thunder erratum 30115 mjaggi
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

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

Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
---
 xen/include/asm-arm/processor.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 65eb1071e1..649a3ae3ca 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -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)
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 03/12] arm64: Add config for Cavium Thunder erratum 30115
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
  2018-03-12 12:42 ` [PATCH 01/12] arm:Kconfig Rename menu text mjaggi
  2018-03-12 12:42 ` [PATCH 02/12] arm64: cputype: Add MIDR values for Cavium ThunderX1 CPUs mjaggi
@ 2018-03-12 12:42 ` mjaggi
  2018-03-13 12:23   ` Julien Grall
  2018-03-12 12:42 ` [PATCH 04/12] Enable Group1 Traps by default for Cavium ThunderX1 mjaggi
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

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 <manish.jaggi@cavium.com>
---
 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(-)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 10a6d1a956..71976ed07b 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -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"
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index fe9e9facbe..d49698f785 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -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
     {},
 };
diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
index 8b158429c7..521f03521b 100644
--- a/xen/include/asm-arm/cpuerrata.h
+++ b/xen/include/asm-arm/cpuerrata.h
@@ -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
 
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index f00b6dbd39..d409636bf0 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -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__
 
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 04/12] Enable Group1 Traps by default for Cavium ThunderX1
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
                   ` (2 preceding siblings ...)
  2018-03-12 12:42 ` [PATCH 03/12] arm64: Add config for Cavium Thunder erratum 30115 mjaggi
@ 2018-03-12 12:42 ` mjaggi
  2018-03-13 12:27   ` Julien Grall
  2018-03-12 12:42 ` [PATCH 05/12] Placeholder for handling Group1 register traps mjaggi
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

Enable trapping for Group1 register access when
CONFIG_CAVIUM_ERRATUM_30115 is enabled.

Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
---
 xen/arch/arm/gic-v3.c     | 16 ++++++++++++++--
 xen/include/asm-arm/gic.h |  1 +
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 473e26111f..53a772a313 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -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 */
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index d3d7bda50d..e4c77fefd6 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -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)
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 05/12] Placeholder for handling Group1 register traps
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
                   ` (3 preceding siblings ...)
  2018-03-12 12:42 ` [PATCH 04/12] Enable Group1 Traps by default for Cavium ThunderX1 mjaggi
@ 2018-03-12 12:42 ` mjaggi
  2018-03-13 14:30   ` Julien Grall
  2018-03-12 12:42 ` [PATCH 06/12] arm64: vgic-v3: Add ICV_BPR1_EL1 handler mjaggi
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

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 <manish.jaggi@cavium.com>
---
 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(-)

diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 718fe44455..19440c3d8c 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -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
diff --git a/xen/arch/arm/arm64/vsysreg_errata.c b/xen/arch/arm/arm64/vsysreg_errata.c
new file mode 100644
index 0000000000..6af162bdf7
--- /dev/null
+++ b/xen/arch/arm/arm64/vsysreg_errata.c
@@ -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;
+}
+
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index f6f6de3691..9d08cd6ad3 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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();
diff --git a/xen/include/asm-arm/arm64/traps.h b/xen/include/asm-arm/arm64/traps.h
index 2379b578cb..a5ae93ec11 100644
--- a/xen/include/asm-arm/arm64/traps.h
+++ b/xen/include/asm-arm/arm64/traps.h
@@ -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);
 
diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
index 7a0971fdea..dacf3adc85 100644
--- a/xen/include/asm-arm/current.h
+++ b/xen/include/asm-arm/current.h
@@ -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)
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 06/12] arm64: vgic-v3: Add ICV_BPR1_EL1 handler
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
                   ` (4 preceding siblings ...)
  2018-03-12 12:42 ` [PATCH 05/12] Placeholder for handling Group1 register traps mjaggi
@ 2018-03-12 12:42 ` mjaggi
  2018-03-12 12:42 ` [PATCH 07/12] arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler mjaggi
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

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 <manish.jaggi@cavium.com>
---
 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(+)

diff --git a/xen/arch/arm/arm64/vsysreg_errata.c b/xen/arch/arm/arm64/vsysreg_errata.c
index 6af162bdf7..eb2b7ad74a 100644
--- a/xen/arch/arm/arm64/vsysreg_errata.c
+++ b/xen/arch/arm/arm64/vsysreg_errata.c
@@ -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;
diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
index 084d2a1e5d..025a27b0b4 100644
--- a/xen/include/asm-arm/arm64/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -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)
diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
index 65c9dc47cf..68a34cc353 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -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
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 07/12] arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
                   ` (5 preceding siblings ...)
  2018-03-12 12:42 ` [PATCH 06/12] arm64: vgic-v3: Add ICV_BPR1_EL1 handler mjaggi
@ 2018-03-12 12:42 ` mjaggi
  2018-03-12 12:42 ` [PATCH 08/12] arm64: Add accessors for the ICH_APxRn_EL2 registers mjaggi
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

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 <manish.jaggi@cavium.com>
---
 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(+)

diff --git a/xen/arch/arm/arm64/vsysreg_errata.c b/xen/arch/arm/arm64/vsysreg_errata.c
index eb2b7ad74a..93e9143a0d 100644
--- a/xen/arch/arm/arm64/vsysreg_errata.c
+++ b/xen/arch/arm/arm64/vsysreg_errata.c
@@ -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;
diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
index 025a27b0b4..731cabc74a 100644
--- a/xen/include/asm-arm/arm64/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -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)
diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
index 68a34cc353..ff8bda37d1 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -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
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 08/12] arm64: Add accessors for the ICH_APxRn_EL2 registers
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
                   ` (6 preceding siblings ...)
  2018-03-12 12:42 ` [PATCH 07/12] arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler mjaggi
@ 2018-03-12 12:42 ` mjaggi
  2018-03-12 12:42 ` [PATCH 09/12] Expose ich_read/write_lr in vsysreg_errata.c mjaggi
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

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 <manish.jaggi@cavium.com>
---
 xen/arch/arm/arm64/vsysreg_errata.c | 92 +++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/xen/arch/arm/arm64/vsysreg_errata.c b/xen/arch/arm/arm64/vsysreg_errata.c
index 93e9143a0d..b2a95a69dc 100644
--- a/xen/arch/arm/arm64/vsysreg_errata.c
+++ b/xen/arch/arm/arm64/vsysreg_errata.c
@@ -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;
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 09/12] Expose ich_read/write_lr in vsysreg_errata.c
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
                   ` (7 preceding siblings ...)
  2018-03-12 12:42 ` [PATCH 08/12] arm64: Add accessors for the ICH_APxRn_EL2 registers mjaggi
@ 2018-03-12 12:42 ` mjaggi
  2018-03-13 14:31   ` Julien Grall
  2018-03-12 12:42 ` [PATCH 10/12] arm64: Add ICV_IAR1_EL1 handler mjaggi
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

gicv3_ich_read/write_lr functions are duplicated in vsysreg_errata.c

Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
---
 xen/arch/arm/arm64/vsysreg_errata.c | 83 +++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/xen/arch/arm/arm64/vsysreg_errata.c b/xen/arch/arm/arm64/vsysreg_errata.c
index b2a95a69dc..d7bf9d6ce3 100644
--- a/xen/arch/arm/arm64/vsysreg_errata.c
+++ b/xen/arch/arm/arm64/vsysreg_errata.c
@@ -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;
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 10/12] arm64: Add ICV_IAR1_EL1 handler
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
                   ` (8 preceding siblings ...)
  2018-03-12 12:42 ` [PATCH 09/12] Expose ich_read/write_lr in vsysreg_errata.c mjaggi
@ 2018-03-12 12:42 ` mjaggi
  2018-03-12 12:42 ` [PATCH 11/12] arm64: vgic-v3: Add ICV_EOIR1_EL1 handler mjaggi
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

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 <manish.jaggi@cavium.com>
---
 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(+)

diff --git a/xen/arch/arm/arm64/vsysreg_errata.c b/xen/arch/arm/arm64/vsysreg_errata.c
index d7bf9d6ce3..9bc1d7b58a 100644
--- a/xen/arch/arm/arm64/vsysreg_errata.c
+++ b/xen/arch/arm/arm64/vsysreg_errata.c
@@ -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;
diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
index 731cabc74a..53d2251840 100644
--- a/xen/include/asm-arm/arm64/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -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)
diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
index ff8bda37d1..884fce0fd0 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -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
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 11/12] arm64: vgic-v3: Add ICV_EOIR1_EL1 handler
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
                   ` (9 preceding siblings ...)
  2018-03-12 12:42 ` [PATCH 10/12] arm64: Add ICV_IAR1_EL1 handler mjaggi
@ 2018-03-12 12:42 ` mjaggi
  2018-03-12 12:42 ` [PATCH 12/12] arm64: vgic-v3: Add ICV_HPPIR1_EL1 handler mjaggi
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

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 <manish.jaggi@cavium.com>
---
 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(+)

diff --git a/xen/arch/arm/arm64/vsysreg_errata.c b/xen/arch/arm/arm64/vsysreg_errata.c
index 9bc1d7b58a..869d67640f 100644
--- a/xen/arch/arm/arm64/vsysreg_errata.c
+++ b/xen/arch/arm/arm64/vsysreg_errata.c
@@ -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;
diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
index 53d2251840..f9110ebf9c 100644
--- a/xen/include/asm-arm/arm64/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -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)
diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
index 884fce0fd0..b169e2cb78 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -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
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 12/12] arm64: vgic-v3: Add ICV_HPPIR1_EL1 handler
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
                   ` (10 preceding siblings ...)
  2018-03-12 12:42 ` [PATCH 11/12] arm64: vgic-v3: Add ICV_EOIR1_EL1 handler mjaggi
@ 2018-03-12 12:42 ` mjaggi
  2018-03-12 12:58 ` [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 Marc Zyngier
  2018-03-13 12:14 ` Julien Grall
  13 siblings, 0 replies; 23+ messages in thread
From: mjaggi @ 2018-03-12 12:42 UTC (permalink / raw)
  To: julien.grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

From: Manish Jaggi <manish.jaggi@cavium.com>

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 <manish.jaggi@cavium.com>
---
 xen/arch/arm/arm64/vsysreg_errata.c | 24 ++++++++++++++++++++++++
 xen/include/asm-arm/arm64/sysregs.h |  1 +
 2 files changed, 25 insertions(+)

diff --git a/xen/arch/arm/arm64/vsysreg_errata.c b/xen/arch/arm/arm64/vsysreg_errata.c
index 869d67640f..088d39613d 100644
--- a/xen/arch/arm/arm64/vsysreg_errata.c
+++ b/xen/arch/arm/arm64/vsysreg_errata.c
@@ -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;
diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
index f9110ebf9c..c23c4a33b2 100644
--- a/xen/include/asm-arm/arm64/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -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)
-- 
2.14.1


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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
                   ` (11 preceding siblings ...)
  2018-03-12 12:42 ` [PATCH 12/12] arm64: vgic-v3: Add ICV_HPPIR1_EL1 handler mjaggi
@ 2018-03-12 12:58 ` Marc Zyngier
  2018-03-13 12:14 ` Julien Grall
  13 siblings, 0 replies; 23+ messages in thread
From: Marc Zyngier @ 2018-03-12 12:58 UTC (permalink / raw)
  To: mjaggi, julien.grall, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

Manish,

On 12/03/18 12:42, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <manish.jaggi@cavium.com>
> 
> 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.
-- 
Jazz is not dead. It just smells funny...

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2
  2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
                   ` (12 preceding siblings ...)
  2018-03-12 12:58 ` [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 Marc Zyngier
@ 2018-03-13 12:14 ` Julien Grall
  13 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2018-03-13 12:14 UTC (permalink / raw)
  To: mjaggi, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

Hi,

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

Cheers,


On 12/03/18 12:42, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <manish.jaggi@cavium.com>
> 
> 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
> 

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/12] arm:Kconfig Rename menu text
  2018-03-12 12:42 ` [PATCH 01/12] arm:Kconfig Rename menu text mjaggi
@ 2018-03-13 12:15   ` Julien Grall
  2018-03-15  6:10     ` Manish Jaggi
  0 siblings, 1 reply; 23+ messages in thread
From: Julien Grall @ 2018-03-13 12:15 UTC (permalink / raw)
  To: mjaggi, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

Hi,

On 12/03/18 12:42, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <manish.jaggi@cavium.com>
> 
> 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,

> 
> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
> ---
>   xen/arch/arm/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index f58019d6ed..10a6d1a956 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -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
> 

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 02/12] arm64: cputype: Add MIDR values for Cavium ThunderX1 CPUs
  2018-03-12 12:42 ` [PATCH 02/12] arm64: cputype: Add MIDR values for Cavium ThunderX1 CPUs mjaggi
@ 2018-03-13 12:17   ` Julien Grall
  0 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2018-03-13 12:17 UTC (permalink / raw)
  To: mjaggi, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

Hi,

On 12/03/18 12:42, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <manish.jaggi@cavium.com>
> 
> Add MIDR values for Cavium ThunderX1 SoC's (88xx, 81xx, 83xx).
> 
> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
> ---
>   xen/include/asm-arm/processor.h | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 65eb1071e1..649a3ae3ca 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -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.

> +#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?

> +
>   #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,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 03/12] arm64: Add config for Cavium Thunder erratum 30115
  2018-03-12 12:42 ` [PATCH 03/12] arm64: Add config for Cavium Thunder erratum 30115 mjaggi
@ 2018-03-13 12:23   ` Julien Grall
  0 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2018-03-13 12:23 UTC (permalink / raw)
  To: mjaggi, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

Hi,

On 12/03/18 12:42, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <manish.jaggi@cavium.com>
> 
> 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 <manish.jaggi@cavium.com>
> ---
>   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(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 10a6d1a956..71976ed07b 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -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,

> +
>   endmenu
>   
>   source "common/Kconfig"
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index fe9e9facbe..d49698f785 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -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
>       {},
>   };
> diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
> index 8b158429c7..521f03521b 100644
> --- a/xen/include/asm-arm/cpuerrata.h
> +++ b/xen/include/asm-arm/cpuerrata.h
> @@ -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.

>   
>   #undef CHECK_WORKAROUND_HELPER
>   
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index f00b6dbd39..d409636bf0 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -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,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 04/12] Enable Group1 Traps by default for Cavium ThunderX1
  2018-03-12 12:42 ` [PATCH 04/12] Enable Group1 Traps by default for Cavium ThunderX1 mjaggi
@ 2018-03-13 12:27   ` Julien Grall
  0 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2018-03-13 12:27 UTC (permalink / raw)
  To: mjaggi, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

Hi,

title: "arm64: ...".

On 12/03/18 12:42, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <manish.jaggi@cavium.com>
> 
> 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.

> 
> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
> ---
>   xen/arch/arm/gic-v3.c     | 16 ++++++++++++++--
>   xen/include/asm-arm/gic.h |  1 +
>   2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 473e26111f..53a772a313 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -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.

>   #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.

>   
>       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.

> +    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.

> +    }
> +    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 */
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index d3d7bda50d..e4c77fefd6 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -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,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 05/12] Placeholder for handling Group1 register traps
  2018-03-12 12:42 ` [PATCH 05/12] Placeholder for handling Group1 register traps mjaggi
@ 2018-03-13 14:30   ` Julien Grall
  0 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2018-03-13 14:30 UTC (permalink / raw)
  To: mjaggi, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi

Hi Manish,

On 12/03/18 12:42, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <manish.jaggi@cavium.com>
> 
> 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.

> 
> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
> ---
>   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/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(-)
> 
> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
> index 718fe44455..19440c3d8c 100644
> --- a/xen/arch/arm/arm64/Makefile
> +++ b/xen/arch/arm/arm64/Makefile
> @@ -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
> diff --git a/xen/arch/arm/arm64/vsysreg_errata.c b/xen/arch/arm/arm64/vsysreg_errata.c
> new file mode 100644
> index 0000000000..6af162bdf7
> --- /dev/null
> +++ b/xen/arch/arm/arm64/vsysreg_errata.c
> @@ -0,0 +1,28 @@

Missing copyright header.

> +#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.

> +
> +    local_irq_disable();

Please add a comment explain why IRQs are disabled.

> +    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;
> +}
> +

Missing emacs magic.

> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index f6f6de3691..9d08cd6ad3 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -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

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.

> +    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.

> +    {
> +        int ret;

vgic_v3_handle_cpuif_access is returning a bool.

> +        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

> +        ret  = vgic_v3_handle_cpuif_access(regs, hsr);
> +        if ( !ret )
> +        {
> +            advance_pc(regs, hsr);
> +            get_cpu_info()->skip_hyp_tail = 1;

true.

> +            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.

> +    if ( get_cpu_info()->skip_hyp_tail )

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

> +        return;
> +#endif
>       while (1)
>       {
>           local_irq_disable();
> diff --git a/xen/include/asm-arm/arm64/traps.h b/xen/include/asm-arm/arm64/traps.h
> index 2379b578cb..a5ae93ec11 100644
> --- a/xen/include/asm-arm/arm64/traps.h
> +++ b/xen/include/asm-arm/arm64/traps.h
> @@ -2,7 +2,8 @@
>   #define __ASM_ARM64_TRAPS__
>   
>   void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len);
> -

Why removing the newline?

> +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);
>   
> diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
> index 7a0971fdea..dacf3adc85 100644
> --- a/xen/include/asm-arm/current.h
> +++ b/xen/include/asm-arm/current.h
> @@ -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.

>   };
>   
>   static inline struct cpu_info *get_cpu_info(void)
> 

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] 23+ messages in thread

* Re: [PATCH 09/12] Expose ich_read/write_lr in vsysreg_errata.c
  2018-03-12 12:42 ` [PATCH 09/12] Expose ich_read/write_lr in vsysreg_errata.c mjaggi
@ 2018-03-13 14:31   ` Julien Grall
  0 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2018-03-13 14:31 UTC (permalink / raw)
  To: mjaggi, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi



On 12/03/18 12:42, mjaggi@caviumnetworks.com wrote:
> From: Manish Jaggi <manish.jaggi@cavium.com>
> 
> 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,

> 
> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
> ---
>   xen/arch/arm/arm64/vsysreg_errata.c | 83 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 83 insertions(+)
> 
> diff --git a/xen/arch/arm/arm64/vsysreg_errata.c b/xen/arch/arm/arm64/vsysreg_errata.c
> index b2a95a69dc..d7bf9d6ce3 100644
> --- a/xen/arch/arm/arm64/vsysreg_errata.c
> +++ b/xen/arch/arm/arm64/vsysreg_errata.c
> @@ -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;
> 

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/12] arm:Kconfig Rename menu text
  2018-03-13 12:15   ` Julien Grall
@ 2018-03-15  6:10     ` Manish Jaggi
  2018-03-15 11:25       ` Julien Grall
  0 siblings, 1 reply; 23+ messages in thread
From: Manish Jaggi @ 2018-03-15  6:10 UTC (permalink / raw)
  To: Julien Grall, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi


Hi Julien,

On 03/13/2018 05:45 PM, Julien Grall wrote:
> Hi,
>
> On 12/03/18 12:42, mjaggi@caviumnetworks.com wrote:
>> From: Manish Jaggi <manish.jaggi@cavium.com>
>>
>> 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."


> 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.
> Cheers,
>
>>
>> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
>> ---
>>   xen/arch/arm/Kconfig | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>> index f58019d6ed..10a6d1a956 100644
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -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
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/12] arm:Kconfig Rename menu text
  2018-03-15  6:10     ` Manish Jaggi
@ 2018-03-15 11:25       ` Julien Grall
  0 siblings, 0 replies; 23+ messages in thread
From: Julien Grall @ 2018-03-15 11:25 UTC (permalink / raw)
  To: Manish Jaggi, marc.zyngier, sstabellini, jgross, xen-devel; +Cc: manish.jaggi



On 15/03/18 06:10, Manish Jaggi wrote:
> 
> Hi Julien,
> 
> On 03/13/2018 05:45 PM, Julien Grall wrote:
>> Hi,
>>
>> On 12/03/18 12:42, mjaggi@caviumnetworks.com wrote:
>>> From: Manish Jaggi <manish.jaggi@cavium.com>
>>>
>>> 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...

> 
> 
>> 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.
> 
> 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,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2018-03-15 11:25 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-12 12:42 [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 mjaggi
2018-03-12 12:42 ` [PATCH 01/12] arm:Kconfig Rename menu text mjaggi
2018-03-13 12:15   ` Julien Grall
2018-03-15  6:10     ` Manish Jaggi
2018-03-15 11:25       ` Julien Grall
2018-03-12 12:42 ` [PATCH 02/12] arm64: cputype: Add MIDR values for Cavium ThunderX1 CPUs mjaggi
2018-03-13 12:17   ` Julien Grall
2018-03-12 12:42 ` [PATCH 03/12] arm64: Add config for Cavium Thunder erratum 30115 mjaggi
2018-03-13 12:23   ` Julien Grall
2018-03-12 12:42 ` [PATCH 04/12] Enable Group1 Traps by default for Cavium ThunderX1 mjaggi
2018-03-13 12:27   ` Julien Grall
2018-03-12 12:42 ` [PATCH 05/12] Placeholder for handling Group1 register traps mjaggi
2018-03-13 14:30   ` Julien Grall
2018-03-12 12:42 ` [PATCH 06/12] arm64: vgic-v3: Add ICV_BPR1_EL1 handler mjaggi
2018-03-12 12:42 ` [PATCH 07/12] arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler mjaggi
2018-03-12 12:42 ` [PATCH 08/12] arm64: Add accessors for the ICH_APxRn_EL2 registers mjaggi
2018-03-12 12:42 ` [PATCH 09/12] Expose ich_read/write_lr in vsysreg_errata.c mjaggi
2018-03-13 14:31   ` Julien Grall
2018-03-12 12:42 ` [PATCH 10/12] arm64: Add ICV_IAR1_EL1 handler mjaggi
2018-03-12 12:42 ` [PATCH 11/12] arm64: vgic-v3: Add ICV_EOIR1_EL1 handler mjaggi
2018-03-12 12:42 ` [PATCH 12/12] arm64: vgic-v3: Add ICV_HPPIR1_EL1 handler mjaggi
2018-03-12 12:58 ` [PATCH 00/12] arm64: Mediate access to GICv3 sysregs at EL2 Marc Zyngier
2018-03-13 12:14 ` Julien Grall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.