* [PATCH v1 01/15] arm64: cputype: Add MIDR values for Cavium ThunderX1 CPU family
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-20 7:38 ` Julien Grall
2018-03-16 11:58 ` [PATCH v1 02/15] arm64: Add config for Cavium Thunder erratum 30115 Manish Jaggi
` (14 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
Add MIDR values for Cavium ThunderX1 SoC family.
ThunderX1, 81XX, 83XX.
Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 65eb1071e1..62ad244785 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] 37+ messages in thread* Re: [PATCH v1 01/15] arm64: cputype: Add MIDR values for Cavium ThunderX1 CPU family
2018-03-16 11:58 ` [PATCH v1 01/15] arm64: cputype: Add MIDR values for Cavium ThunderX1 CPU family Manish Jaggi
@ 2018-03-20 7:38 ` Julien Grall
0 siblings, 0 replies; 37+ messages in thread
From: Julien Grall @ 2018-03-20 7:38 UTC (permalink / raw)
To: Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
Hi Manish,
On 03/16/2018 11:58 AM, Manish Jaggi wrote:
> Add MIDR values for Cavium ThunderX1 SoC family.
Did you intend to use a : instead of .?
> ThunderX1, 81XX, 83XX.
>
> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
>
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 65eb1071e1..62ad244785 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)
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v1 02/15] arm64: Add config for Cavium Thunder erratum 30115
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
2018-03-16 11:58 ` [PATCH v1 01/15] arm64: cputype: Add MIDR values for Cavium ThunderX1 CPU family Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-20 7:43 ` Julien Grall
2018-03-16 11:58 ` [PATCH v1 03/15] arm: Placeholder for handling Group0/1 traps for Cavium Erratum 30115 Manish Jaggi
` (13 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
Some Cavium Thunder CPUs suffer a problem where a Xen 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>
diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
index c9854c39f4..a2546d4bb5 100644
--- a/docs/misc/arm/silicon-errata.txt
+++ b/docs/misc/arm/silicon-errata.txt
@@ -48,3 +48,4 @@ stable hypervisors.
| ARM | Cortex-A57 | #852523 | N/A |
| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 |
| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 |
+| CAVIUM | ThunderX1 | #30115 | CAVIUM_ERRATUM_30115 |
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index f58019d6ed..762b761f7d 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -169,6 +169,17 @@ config ARM64_ERRATUM_834220
If unsure, say Y.
+config CAVIUM_ERRATUM_30115
+ bool "Cavium Erratum 30115"
+ depends on HAS_GICV3
+ help
+ On ThunderX T88 pass 1.x through 2.2, T81 pass 1.0 through
+ 1.2, and T83 Pass 1.0, guest execution may disable
+ interrupts in host. Trapping both GICv3 group-0 and group-1
+ accesses sidesteps the issue.
+
+ If unsure, say Y.
+
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] 37+ messages in thread* Re: [PATCH v1 02/15] arm64: Add config for Cavium Thunder erratum 30115
2018-03-16 11:58 ` [PATCH v1 02/15] arm64: Add config for Cavium Thunder erratum 30115 Manish Jaggi
@ 2018-03-20 7:43 ` Julien Grall
2018-03-21 5:06 ` Manish Jaggi
0 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2018-03-20 7:43 UTC (permalink / raw)
To: Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
Hi,
On 03/16/2018 11:58 AM, Manish Jaggi wrote:
> Some Cavium Thunder CPUs suffer a problem where a Xen 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>
>
> diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
> index c9854c39f4..a2546d4bb5 100644
> --- a/docs/misc/arm/silicon-errata.txt
> +++ b/docs/misc/arm/silicon-errata.txt
> @@ -48,3 +48,4 @@ stable hypervisors.
> | ARM | Cortex-A57 | #852523 | N/A |
> | ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 |
> | ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 |
> +| CAVIUM | ThunderX1 | #30115 | CAVIUM_ERRATUM_30115 |
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index f58019d6ed..762b761f7d 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -169,6 +169,17 @@ config ARM64_ERRATUM_834220
>
> If unsure, say Y.
>
> +config CAVIUM_ERRATUM_30115
> + bool "Cavium Erratum 30115"
> + depends on HAS_GICV3
> + help
> + On ThunderX T88 pass 1.x through 2.2, T81 pass 1.0 through
> + 1.2, and T83 Pass 1.0, guest execution may disable
> + interrupts in host. Trapping both GICv3 group-0 and group-1
> + accesses sidesteps the issue.
> +
> + If unsure, say Y.
> +
> 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 */
This is quite odd. You specify a number in the commit message here, but
in the previous one you just say "thunderx1". Can you please try to
agree on the name? Like the right naming is MIDR_THUNDERX_88.
> + .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)
Please add cavium_ in the erratum name. So it is easy to know where the
erratum is from.
>
> #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] 37+ messages in thread* Re: [PATCH v1 02/15] arm64: Add config for Cavium Thunder erratum 30115
2018-03-20 7:43 ` Julien Grall
@ 2018-03-21 5:06 ` Manish Jaggi
2018-03-21 7:49 ` Julien Grall
0 siblings, 1 reply; 37+ messages in thread
From: Manish Jaggi @ 2018-03-21 5:06 UTC (permalink / raw)
To: Julien Grall, Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
On 03/20/2018 01:13 PM, Julien Grall wrote:
> Hi,
>
> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>> Some Cavium Thunder CPUs suffer a problem where a Xen 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>
>>
>> diff --git a/docs/misc/arm/silicon-errata.txt
>> b/docs/misc/arm/silicon-errata.txt
>> index c9854c39f4..a2546d4bb5 100644
>> --- a/docs/misc/arm/silicon-errata.txt
>> +++ b/docs/misc/arm/silicon-errata.txt
>> @@ -48,3 +48,4 @@ stable hypervisors.
>> | ARM | Cortex-A57 | #852523 |
>> N/A |
>> | ARM | Cortex-A57 | #832075 |
>> ARM64_ERRATUM_832075 |
>> | ARM | Cortex-A57 | #834220 |
>> ARM64_ERRATUM_834220 |
>> +| CAVIUM | ThunderX1 | #30115 |
>> CAVIUM_ERRATUM_30115 |
>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>> index f58019d6ed..762b761f7d 100644
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -169,6 +169,17 @@ config ARM64_ERRATUM_834220
>> If unsure, say Y.
>> +config CAVIUM_ERRATUM_30115
>> + bool "Cavium Erratum 30115"
>> + depends on HAS_GICV3
>> + help
>> + On ThunderX T88 pass 1.x through 2.2, T81 pass 1.0 through
>> + 1.2, and T83 Pass 1.0, guest execution may disable
>> + interrupts in host. Trapping both GICv3 group-0 and group-1
>> + accesses sidesteps the issue.
>> +
>> + If unsure, say Y.
>> +
>> 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 */
>
> This is quite odd. You specify a number in the commit message here,
> but in the previous one you just say "thunderx1". Can you please try
> to agree on the name? Like the right naming is MIDR_THUNDERX_88.
ThunderX1 is 88xx. Also this patch is using the same patch from linux.
Please see
https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/025815.html
>
>> + .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)
>
> Please add cavium_ in the erratum name. So it is easy to know where
> the erratum is from.
ok
>
>> #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,
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v1 02/15] arm64: Add config for Cavium Thunder erratum 30115
2018-03-21 5:06 ` Manish Jaggi
@ 2018-03-21 7:49 ` Julien Grall
0 siblings, 0 replies; 37+ messages in thread
From: Julien Grall @ 2018-03-21 7:49 UTC (permalink / raw)
To: Manish Jaggi, Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
On 03/21/2018 05:06 AM, Manish Jaggi wrote:
> On 03/20/2018 01:13 PM, Julien Grall wrote:
>> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>>> 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 */
>>
>> This is quite odd. You specify a number in the commit message here,
>> but in the previous one you just say "thunderx1". Can you please try
>> to agree on the name? Like the right naming is MIDR_THUNDERX_88.
> ThunderX1 is 88xx. Also this patch is using the same patch from linux.
> Please see
> https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/025815.html
If you re-use a patch from Linux then write it in the commit message...
And in patch #1, you should clarify you are re-using Linux name.
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] 37+ messages in thread
* [PATCH v1 03/15] arm: Placeholder for handling Group0/1 traps for Cavium Erratum 30115
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
2018-03-16 11:58 ` [PATCH v1 01/15] arm64: cputype: Add MIDR values for Cavium ThunderX1 CPU family Manish Jaggi
2018-03-16 11:58 ` [PATCH v1 02/15] arm64: Add config for Cavium Thunder erratum 30115 Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-20 8:08 ` Julien Grall
2018-03-16 11:58 ` [PATCH v1 04/15] arm64: vgic-v3: Add ICV_BPR1_EL1 handler Manish Jaggi
` (12 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
Since this is a SoC errata and trapping of certain group1 registers
should not affect the normal flow. A new file vgic-v3-sr.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. enter_hypervisor_head is not invoked when workaround
30115 is in place. enter_hypervisor_head and leave_hypervisor_tail are
invoked in sync, if one is not called other one should be skipped,
otherwise guest vGIC state be out-of-date.
Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 718fe44455..02cc115239 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) += vgic-v3-sr.o
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
new file mode 100644
index 0000000000..56b02fd45b
--- /dev/null
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -0,0 +1,56 @@
+/*
+ * xen/arch/arm/arm64/vgic-v3-sr.c
+ *
+ * Code to handle Cavium Erratum 30115
+ *
+ * Manish Jaggi <manish.jaggi@cavium.com>
+ * Copyright (c) 2018 Cavium.
+ *
+ * Ths program is free software; you can redistribute it and/or
+ * modify it under the terms and conditions of the GNU General Public
+ * License, version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <asm/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 = true;
+
+ /* Disabling interrupts to prevent change in guest state */
+ local_irq_disable();
+ if ( hsr.ec != HSR_EC_SYSREG )
+ {
+ ret = false;
+ goto end;
+ }
+
+ switch ( hsr.bits & HSR_SYSREG_REGS_MASK )
+ {
+ default:
+ ret = false;
+ break;
+ }
+end:
+ local_irq_enable();
+
+ return ret;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
+
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index f6f6de3691..25778018fb 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2103,6 +2103,27 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
{
const union hsr hsr = { .bits = regs->hsr };
+ if ( check_workaround_30115() )
+ {
+ bool ret;
+ ret = vgic_v3_handle_cpuif_access(regs, hsr);
+ if ( ret )
+ {
+ /* if true, g0/g1 vgic register trap is emulated for errata
+ * so rest of handling of do_trap_guest_sync is not required.
+ */
+ advance_pc(regs, hsr);
+ /*
+ * enter_hypervisor_head is not invoked when workaround 30115
+ * is in place. enter_hypervisor_head and leave_hypervisor_tail
+ * are invoked in sync, if one is not called other one should be
+ * skipped, otherwise guest vGIC state be out-of-date.
+ */
+ get_cpu_info()->skip_hyp_tail = true;
+ return;
+ }
+ }
+
enter_hypervisor_head(regs);
switch (hsr.ec) {
@@ -2295,6 +2316,16 @@ void do_trap_fiq(struct cpu_user_regs *regs)
void leave_hypervisor_tail(void)
{
+ /*
+ * if skip_hyp_tail is set simply retrun;
+ */
+ if ( unlikely(get_cpu_info()->skip_hyp_tail) )
+ {
+ /* clear it */
+ get_cpu_info()->skip_hyp_tail = false;
+ return;
+ }
+
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..45fe582abd 100644
--- a/xen/include/asm-arm/arm64/traps.h
+++ b/xen/include/asm-arm/arm64/traps.h
@@ -3,6 +3,9 @@
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..d7b3f4ddb4 100644
--- a/xen/include/asm-arm/current.h
+++ b/xen/include/asm-arm/current.h
@@ -21,7 +21,8 @@ DECLARE_PER_CPU(struct vcpu *, curr_vcpu);
struct cpu_info {
struct cpu_user_regs guest_cpu_user_regs;
unsigned long elr;
- unsigned int pad;
+ unsigned int pad:31;
+ bool skip_hyp_tail:1;
};
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] 37+ messages in thread* Re: [PATCH v1 03/15] arm: Placeholder for handling Group0/1 traps for Cavium Erratum 30115
2018-03-16 11:58 ` [PATCH v1 03/15] arm: Placeholder for handling Group0/1 traps for Cavium Erratum 30115 Manish Jaggi
@ 2018-03-20 8:08 ` Julien Grall
0 siblings, 0 replies; 37+ messages in thread
From: Julien Grall @ 2018-03-20 8:08 UTC (permalink / raw)
To: Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
Hi Manish,
On 03/16/2018 11:58 AM, Manish Jaggi wrote:
> Since this is a SoC errata and trapping of certain group1 registers
> should not affect the normal flow. A new file vgic-v3-sr.c is added.
You trap both group1 and group0. But your first sentence is a bit
difficult to understand. How about:
"The errata will require to emulate the GIC virtual CPU interface in
Xen. Because the hypervisor will update its internal state of the vGIC,
we want to avoid messing up with it. So the errata is handled separately
from the rest of the hypervisor."
>
> 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
The wrapping of the commit message looks wrong.
> that leave_hypervisor_tail not to be called when handling group1 traps
> under this errata.
No the flag is much more generic than what you claim here. The flag is
used to skip the hypervisor_tail when enter_hypervisor_head. Your trap
handling is one of the user.
Technically this is 2 distinct features, one is using the other and
should be in separate patches. I am ok to keep in a single patch, but
you should get the commit message right.
> enter_hypervisor_head is not invoked when workaround
> 30115 is in place. enter_hypervisor_head and leave_hypervisor_tail are
> invoked in sync, if one is not called other one should be skipped,
> otherwise guest vGIC state be out-of-date.
>
> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
>
> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
> index 718fe44455..02cc115239 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) += vgic-v3-sr.o
> diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
> new file mode 100644
> index 0000000000..56b02fd45b
> --- /dev/null
> +++ b/xen/arch/arm/arm64/vgic-v3-sr.c
> @@ -0,0 +1,56 @@
> +/*
> + * xen/arch/arm/arm64/vgic-v3-sr.c
> + *
> + * Code to handle Cavium Erratum 30115
> + *
> + * Manish Jaggi <manish.jaggi@cavium.com>
> + * Copyright (c) 2018 Cavium.
> + *
> + * Ths program is free software; you can redistribute it and/or
> + * modify it under the terms and conditions of the GNU General Public
> + * License, version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <asm/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 = true;
> +
> + /* Disabling interrupts to prevent change in guest state */
> + local_irq_disable();
> + if ( hsr.ec != HSR_EC_SYSREG )
> + {
> + ret = false;
> + goto end;
> + }
> +
> + switch ( hsr.bits & HSR_SYSREG_REGS_MASK )
> + {
> + default:
> + ret = false;
> + break;
> + }
> +end:
> + local_irq_enable();
> +
> + return ret;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> +
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index f6f6de3691..25778018fb 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2103,6 +2103,27 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
> {
> const union hsr hsr = { .bits = regs->hsr };
>
> + if ( check_workaround_30115() )
> + {
> + bool ret;
newline here. But ...
> + ret = vgic_v3_handle_cpuif_access(regs, hsr);
I don't think the separate variable is required.
You could just do if (vgic_v3_handle_cpuif_access(regs, hsr))...
> + if ( ret )
> + {
> + /* if true, g0/g1 vgic register trap is emulated for errata
> + * so rest of handling of do_trap_guest_sync is not required.
> + */
Please follow Xen coding style comments:
/*
* If ...
The if should also have the first letter uppercase and it would be
better to say "group" rather than "g". It is much clearer.
> + advance_pc(regs, hsr);
> + /*
> + * enter_hypervisor_head is not invoked when workaround 30115
> + * is in place. enter_hypervisor_head and leave_hypervisor_tail
> + * are invoked in sync, if one is not called other one should be
> + * skipped, otherwise guest vGIC state be out-of-date.
> + */
> + get_cpu_info()->skip_hyp_tail = true;
> + return;
> + }
> + }
> +
> enter_hypervisor_head(regs);
>
> switch (hsr.ec) {
> @@ -2295,6 +2316,16 @@ void do_trap_fiq(struct cpu_user_regs *regs)
>
> void leave_hypervisor_tail(void)
> {
> + /*
> + * if skip_hyp_tail is set simply retrun;
> + */
No can use the single line comment style here. And s/retrun/return/.
> + if ( unlikely(get_cpu_info()->skip_hyp_tail) )
> + {
> + /* clear it */
Well, that pretty obvious from the code. It would be much nicer to state
the less obvious, i.e why you clear skip_hyp_tail here.
> + get_cpu_info()->skip_hyp_tail = false;
> + return;
> + }
> +
> 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..45fe582abd 100644
> --- a/xen/include/asm-arm/arm64/traps.h
> +++ b/xen/include/asm-arm/arm64/traps.h
> @@ -3,6 +3,9 @@
>
> 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..d7b3f4ddb4 100644
> --- a/xen/include/asm-arm/current.h
> +++ b/xen/include/asm-arm/current.h
> @@ -21,7 +21,8 @@ DECLARE_PER_CPU(struct vcpu *, curr_vcpu);
> struct cpu_info {
> struct cpu_user_regs guest_cpu_user_regs;
> unsigned long elr;
> - unsigned int pad;
> + unsigned int pad:31;
> + bool skip_hyp_tail:1;
Firstly, please switch pad and skip_hyp_tail. We want to keep the
padding towards the end of the structure.
Furthermore, when I asked comments, it was not why you set skip_hyp_tail
= true on the erratum. It was on the field, so developer can understand
how this field can been used.
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] 37+ messages in thread
* [PATCH v1 04/15] arm64: vgic-v3: Add ICV_BPR1_EL1 handler
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (2 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 03/15] arm: Placeholder for handling Group0/1 traps for Cavium Erratum 30115 Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-21 8:11 ` Julien Grall
2018-03-16 11:58 ` [PATCH v1 05/15] arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler Manish Jaggi
` (11 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
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>
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 56b02fd45b..364785d3ac 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -20,10 +20,76 @@
#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) ((((uint32_t)(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(uint32_t vmcr)
+{
+ return (vmcr & ICH_VMCR_BPR0_MASK) >> ICH_VMCR_BPR0_SHIFT;
+}
+
+static unsigned int __vgic_v3_get_bpr1(uint32_t 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)
+{
+ uint32_t 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)
+{
+ register_t val = get_user_reg(regs, regidx);
+ uint8_t bpr_min = __vgic_v3_bpr_min();
+ uint32_t 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, const union hsr hsr)
+{
+ if ( hsr.sysreg.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 = true;
+ int regidx = hsr.sysreg.reg;
/* Disabling interrupts to prevent change in guest state */
local_irq_disable();
@@ -35,6 +101,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);
+ break;
+
default:
ret = false;
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] 37+ messages in thread* Re: [PATCH v1 04/15] arm64: vgic-v3: Add ICV_BPR1_EL1 handler
2018-03-16 11:58 ` [PATCH v1 04/15] arm64: vgic-v3: Add ICV_BPR1_EL1 handler Manish Jaggi
@ 2018-03-21 8:11 ` Julien Grall
2018-03-26 13:11 ` Manish Jaggi
0 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2018-03-21 8:11 UTC (permalink / raw)
To: Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
Hi Manish,
On 03/16/2018 11:58 AM, Manish Jaggi wrote:
> 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>
As you port commit by commit, please at least mention Marc as he is the
original author of that patch.
>
> diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
> index 56b02fd45b..364785d3ac 100644
> --- a/xen/arch/arm/arm64/vgic-v3-sr.c
> +++ b/xen/arch/arm/arm64/vgic-v3-sr.c
> @@ -20,10 +20,76 @@
> #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) ((((uint32_t)(v) >> 26) & 7) + 1)
> +
> +static int __vgic_v3_bpr_min(void)
Please remove one of the space.
As you convert to Xen coding style, please remove __ from all function
names.
> +{
> + /* See Pseudocode for VPriorityGroup */
> + return 8 - vtr_to_nr_pre_bits(READ_SYSREG32(ICH_VTR_EL2));
> +}
> +
> +static unsigned int __vgic_v3_get_bpr0(uint32_t vmcr)
> +{
> + return (vmcr & ICH_VMCR_BPR0_MASK) >> ICH_VMCR_BPR0_SHIFT;
> +}
> +
> +static unsigned int __vgic_v3_get_bpr1(uint32_t 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)
> +{
> + uint32_t 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)
> +{
> + register_t val = get_user_reg(regs, regidx);
> + uint8_t bpr_min = __vgic_v3_bpr_min();
> + uint32_t 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, const union hsr hsr)
I am pretty sure I said it in the past. If the function is not exported
it, it *must* be static. I am not going to repeat it and expect all the
sites to be fixed in the next version.
The naming is also a bit odd. All of the file is using "vgic_v3_". So
please try to be consistent.
> +{
> + if ( hsr.sysreg.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 = true;
> + int regidx = hsr.sysreg.reg;
>
> /* Disabling interrupts to prevent change in guest state */
> local_irq_disable();
> @@ -35,6 +101,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);
Please use vreg_emulate_* helpers.
> + break;
> +
> default:
> ret = false;
> 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
>
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] 37+ messages in thread* Re: [PATCH v1 04/15] arm64: vgic-v3: Add ICV_BPR1_EL1 handler
2018-03-21 8:11 ` Julien Grall
@ 2018-03-26 13:11 ` Manish Jaggi
0 siblings, 0 replies; 37+ messages in thread
From: Manish Jaggi @ 2018-03-26 13:11 UTC (permalink / raw)
To: Julien Grall, Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
On 03/21/2018 01:41 PM, Julien Grall wrote:
> Hi Manish,
>
> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>> 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>
>
> As you port commit by commit, please at least mention Marc as he is
> the original author of that patch.
I have a little confusion, should a signoff be added once it is reviewed
or even if it is ported?
I thought once it is acked, we add signoff.
Please suggest.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v1 05/15] arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (3 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 04/15] arm64: vgic-v3: Add ICV_BPR1_EL1 handler Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-21 8:38 ` Julien Grall
2018-03-16 11:58 ` [PATCH v1 06/15] arm64: Add accessors for the ICH_APxRn_EL2 registers Manish Jaggi
` (10 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
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>
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 364785d3ac..114d5107a9 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -86,6 +86,34 @@ void handle_bpr1(struct cpu_user_regs *regs, int regidx, const union hsr hsr)
__vgic_v3_write_bpr1(regs, regidx);
}
+static void __vgic_v3_read_igrpen1(struct cpu_user_regs *regs, int regidx)
+{
+ uint32_t 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)
+{
+ register_t val = get_user_reg(regs, regidx);
+ uint32_t 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,
+ const union hsr hsr)
+{
+ if ( hsr.sysreg.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 = true;
@@ -105,6 +133,10 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr
handle_bpr1(regs, regidx, hsr);
break;
+ case HSR_SYSREG_ICC_IGRPEN1_EL1:
+ handle_igrpen1(regs, regidx, hsr);
+ break;
+
default:
ret = false;
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] 37+ messages in thread* Re: [PATCH v1 05/15] arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler
2018-03-16 11:58 ` [PATCH v1 05/15] arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler Manish Jaggi
@ 2018-03-21 8:38 ` Julien Grall
2018-03-26 13:09 ` Manish Jaggi
0 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2018-03-21 8:38 UTC (permalink / raw)
To: Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
On 03/16/2018 11:58 AM, Manish Jaggi wrote:
> 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
The wrapping looks wrong.
> register, which is located in the ICH_VMCR_EL2.VENG1 field.
>
> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
>
> diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
> index 364785d3ac..114d5107a9 100644
> --- a/xen/arch/arm/arm64/vgic-v3-sr.c
> +++ b/xen/arch/arm/arm64/vgic-v3-sr.c
> @@ -86,6 +86,34 @@ void handle_bpr1(struct cpu_user_regs *regs, int regidx, const union hsr hsr)
> __vgic_v3_write_bpr1(regs, regidx);
> }
>
> +static void __vgic_v3_read_igrpen1(struct cpu_user_regs *regs, int regidx)
Please remove the __ for all the functions.
> +{
> + uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
Newline.
> + set_user_reg(regs, regidx, !!(vmcr & ICH_VMCR_ENG1_MASK));
> +}
> +
> +static void __vgic_v3_write_igrpen1(struct cpu_user_regs *regs, int regidx)
> +{
> + register_t val = get_user_reg(regs, regidx);
> + uint32_t 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);
So basically, you ported commit without even looking if there were
change on top. For instance the latest code has a specific has an helper
to update vmcr. Can you please make sure that all the code is ported?
> +}
> +
> +void handle_igrpen1(struct cpu_user_regs *regs, int regidx,
> + const union hsr hsr)
> +{
> + if ( hsr.sysreg.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 = true;
> @@ -105,6 +133,10 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr
> handle_bpr1(regs, regidx, hsr);
> break;
>
> + case HSR_SYSREG_ICC_IGRPEN1_EL1:
> + handle_igrpen1(regs, regidx, hsr);
> + break;
> +
> default:
> ret = false;
> 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
>
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] 37+ messages in thread* Re: [PATCH v1 05/15] arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler
2018-03-21 8:38 ` Julien Grall
@ 2018-03-26 13:09 ` Manish Jaggi
0 siblings, 0 replies; 37+ messages in thread
From: Manish Jaggi @ 2018-03-26 13:09 UTC (permalink / raw)
To: Julien Grall, Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
Hi Julien,
On 03/21/2018 02:08 PM, Julien Grall wrote:
>
>
> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>> 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
>
> The wrapping looks wrong.
>
>> register, which is located in the ICH_VMCR_EL2.VENG1 field.
>>
>> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
>>
>> diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c
>> b/xen/arch/arm/arm64/vgic-v3-sr.c
>> index 364785d3ac..114d5107a9 100644
>> --- a/xen/arch/arm/arm64/vgic-v3-sr.c
>> +++ b/xen/arch/arm/arm64/vgic-v3-sr.c
>> @@ -86,6 +86,34 @@ void handle_bpr1(struct cpu_user_regs *regs, int
>> regidx, const union hsr hsr)
>> __vgic_v3_write_bpr1(regs, regidx);
>> }
>> +static void __vgic_v3_read_igrpen1(struct cpu_user_regs *regs,
>> int regidx)
>
> Please remove the __ for all the functions.
ok
>
>> +{
>> + uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
>
> Newline.
For 2 line function should there be a newline?
Will add it.
>
>> + set_user_reg(regs, regidx, !!(vmcr & ICH_VMCR_ENG1_MASK));
>> +}
>> +
>> +static void __vgic_v3_write_igrpen1(struct cpu_user_regs *regs, int
>> regidx)
>> +{
>> + register_t val = get_user_reg(regs, regidx);
>> + uint32_t 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);
>
> So basically, you ported commit without even looking if there were
> change on top. For instance the latest code has a specific has an
> helper to update vmcr. Can you please make sure that all the code is
> ported?
>
Well julien, I did that and it was in original code as well.
__vgic_v3_write_vmcr was called which was calling write_gicreg(vmcr,
ICH_VMCR_EL2).
Xen aleady has a macro so it is better to use xen macro.
I can write that in commit log if that helps in easier review
>> +}
>> +
>> +void handle_igrpen1(struct cpu_user_regs *regs, int regidx,
>> + const union hsr hsr)
>> +{
>> + if ( hsr.sysreg.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 = true;
>> @@ -105,6 +133,10 @@ bool vgic_v3_handle_cpuif_access(struct
>> cpu_user_regs *regs, const union hsr hsr
>> handle_bpr1(regs, regidx, hsr);
>> break;
>> + case HSR_SYSREG_ICC_IGRPEN1_EL1:
>> + handle_igrpen1(regs, regidx, hsr);
>> + break;
>> +
>> default:
>> ret = false;
>> 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
>>
>
> Cheers,
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v1 06/15] arm64: Add accessors for the ICH_APxRn_EL2 registers
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (4 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 05/15] arm64: vgic-v3: Add ICV_IGRPEN1_EL1 handler Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-26 13:19 ` Manish Jaggi
2018-03-16 11:58 ` [PATCH v1 07/15] Expose ich_read/write_lr in vgic-v3-sr.c Manish Jaggi
` (9 subsequent siblings)
15 siblings, 1 reply; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
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.
This patch only has accessors, another patch will have register trap handlers
Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 114d5107a9..1aaade40dc 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -114,6 +114,98 @@ void handle_igrpen1(struct cpu_user_regs *regs, int regidx,
__vgic_v3_write_igrpen1(regs, regidx);
}
+void __vgic_v3_write_ap0rn(uint32_t 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(uint32_t 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();
+ }
+}
+
+uint32_t __vgic_v3_read_ap0rn(int n)
+{
+ uint32_t 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;
+}
+
+uint32_t __vgic_v3_read_ap1rn(int n)
+{
+ uint32_t 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 = true;
--
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] 37+ messages in thread* Re: [PATCH v1 06/15] arm64: Add accessors for the ICH_APxRn_EL2 registers
2018-03-16 11:58 ` [PATCH v1 06/15] arm64: Add accessors for the ICH_APxRn_EL2 registers Manish Jaggi
@ 2018-03-26 13:19 ` Manish Jaggi
2018-03-26 14:36 ` Marc Zyngier
0 siblings, 1 reply; 37+ messages in thread
From: Manish Jaggi @ 2018-03-26 13:19 UTC (permalink / raw)
To: Manish Jaggi, xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Hi Marc,
I have a query on this patch. The original patch was using these
functions so it was ok to make them static.
But this patch is not touching the xen vgic code similar to what your
patch did.
Will it be ok to merge this patch with
https://www.spinics.net/lists/arm-kernel/msg587089.html
-manish
On 03/16/2018 05:28 PM, Manish Jaggi wrote:
> 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.
>
> This patch only has accessors, another patch will have register trap handlers
>
> Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
>
> diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
> index 114d5107a9..1aaade40dc 100644
> --- a/xen/arch/arm/arm64/vgic-v3-sr.c
> +++ b/xen/arch/arm/arm64/vgic-v3-sr.c
> @@ -114,6 +114,98 @@ void handle_igrpen1(struct cpu_user_regs *regs, int regidx,
> __vgic_v3_write_igrpen1(regs, regidx);
> }
>
> +void __vgic_v3_write_ap0rn(uint32_t 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(uint32_t 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();
> + }
> +}
> +
> +uint32_t __vgic_v3_read_ap0rn(int n)
> +{
> + uint32_t 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;
> +}
> +
> +uint32_t __vgic_v3_read_ap1rn(int n)
> +{
> + uint32_t 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 = true;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v1 06/15] arm64: Add accessors for the ICH_APxRn_EL2 registers
2018-03-26 13:19 ` Manish Jaggi
@ 2018-03-26 14:36 ` Marc Zyngier
0 siblings, 0 replies; 37+ messages in thread
From: Marc Zyngier @ 2018-03-26 14:36 UTC (permalink / raw)
To: Manish Jaggi, Manish Jaggi, xen-devel, julien.grall, sstabellini,
andre.przywara
On 26/03/18 14:19, Manish Jaggi wrote:
> Hi Marc,
>
> I have a query on this patch. The original patch was using these
> functions so it was ok to make them static.
> But this patch is not touching the xen vgic code similar to what your
> patch did.
>
> Will it be ok to merge this patch with
> https://www.spinics.net/lists/arm-kernel/msg587089.html
I can only repeat the argument I try to convey earlier. By changing the
structure of the series, you're making it harder to review it, as it is
not possible to look at two patches side by side and work out what changed.
In the end, that's your call. If you want to change the shape of the
series, go for it. But also appreciate the consequences of doing so.
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] 37+ messages in thread
* [PATCH v1 07/15] Expose ich_read/write_lr in vgic-v3-sr.c
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (5 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 06/15] arm64: Add accessors for the ICH_APxRn_EL2 registers Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-16 11:58 ` [PATCH v1 08/15] arm64: Add ICV_IAR1_EL1 handler Manish Jaggi
` (8 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
gicv3_ich_read/write_lr functions are duplicated in vgic-v3-sr.c
This is done to make the file independent of the xen vgic code for
handling the errata.
Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 1aaade40dc..93ac6f03a9 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -206,6 +206,89 @@ uint32_t __vgic_v3_read_ap1rn(int n)
return val;
}
+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();
+ }
+}
+
+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 = true;
--
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] 37+ messages in thread* [PATCH v1 08/15] arm64: Add ICV_IAR1_EL1 handler
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (6 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 07/15] Expose ich_read/write_lr in vgic-v3-sr.c Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-16 11:58 ` [PATCH v1 09/15] arm64: vgic-v3: Add ICV_EOIR1_EL1 handler Manish Jaggi
` (7 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
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>
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 93ac6f03a9..8af943b37a 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -21,8 +21,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) ((((uint32_t)(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)
{
@@ -289,6 +299,186 @@ void gicv3_ich_write_lr(int lr, uint64_t val)
isb();
}
+static int __vgic_v3_get_group(const union hsr hsr)
+{
+ uint8_t 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,
+ uint32_t vmcr, uint64_t *lr_val)
+{
+ int i, lr = -1;
+ unsigned int used_lrs = gic_get_num_lrs();
+ uint8_t priority = GICV3_IDLE_PRIORITY;
+
+ for ( i = 0; i < used_lrs; i++ )
+ {
+ uint64_t val = gicv3_ich_read_lr(i);
+ uint8_t 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;
+ uint32_t hap = 0;
+ uint8_t nr_apr_regs = vtr_to_nr_apr_regs(READ_SYSREG32(ICH_VTR_EL2));
+
+ for ( i = 0; i < nr_apr_regs; i++ )
+ {
+ uint32_t 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 uint8_t __vgic_v3_pri_to_pre(uint8_t pri, uint32_t 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(uint8_t pri, uint32_t vmcr, int grp)
+{
+ uint8_t pre, ap;
+ uint32_t 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)
+{
+ uint64_t lr_val;
+ uint8_t lr_prio, pmr;
+ int lr, grp;
+
+ uint32_t 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 = true;
@@ -312,6 +502,10 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr
handle_igrpen1(regs, regidx, hsr);
break;
+ case HSR_SYSREG_ICC_IAR1_EL1:
+ handle_iar(regs, regidx, hsr);
+ break;
+
default:
ret = false;
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] 37+ messages in thread* [PATCH v1 09/15] arm64: vgic-v3: Add ICV_EOIR1_EL1 handler
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (7 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 08/15] arm64: Add ICV_IAR1_EL1 handler Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-16 11:58 ` [PATCH v1 10/15] arm64: vgic-v3: Add ICV_HPPIR1_EL1 handler Manish Jaggi
` (6 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
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>
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 8af943b37a..35bad3953f 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -33,6 +33,7 @@
#define ICC_IAR1_EL1_SPURIOUS 0x3ff
#define VGIC_MAX_SPI 1019
+#define VGIC_MIN_LPI 8192
static int __vgic_v3_bpr_min(void)
{
@@ -479,6 +480,137 @@ 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, uint64_t *lr_val)
+{
+ int i;
+ unsigned int used_lrs = gic_get_num_lrs();
+
+ for ( i = 0; i < used_lrs; i++ )
+ {
+ uint64_t 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)
+{
+ int i;
+ uint32_t hap = 0;
+ uint8_t nr_apr_regs = vtr_to_nr_apr_regs(READ_SYSREG32(ICH_VTR_EL2));
+
+ for ( i = 0; i < nr_apr_regs; i++ )
+ {
+ uint32_t 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, uint64_t lr_val)
+{
+ lr_val &= ~ICH_LR_ACTIVE_BIT;
+ if ( lr_val & ICH_LR_HW )
+ {
+ uint32_t 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)
+{
+ uint32_t 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)
+{
+ uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+ register_t vid = get_user_reg(regs, regidx);
+ uint64_t lr_val;
+ uint8_t 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)
{
bool ret = true;
@@ -506,6 +638,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 = false;
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..fd2f89602f 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -140,9 +140,9 @@
#define GICR_PENDBASER_INNER_CACHEABILITY_SHIFT 7
#define GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT 56
#define GICR_PENDBASER_SHAREABILITY_MASK \
- (3UL << GICR_PENDBASER_SHAREABILITY_SHIFT)
+(3UL << GICR_PENDBASER_SHAREABILITY_SHIFT)
#define GICR_PENDBASER_INNER_CACHEABILITY_MASK \
- (7UL << GICR_PENDBASER_INNER_CACHEABILITY_SHIFT)
+(7UL << GICR_PENDBASER_INNER_CACHEABILITY_SHIFT)
#define GICR_PENDBASER_OUTER_CACHEABILITY_MASK \
(7UL << GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT)
#define GICR_PENDBASER_PTZ BIT(62)
@@ -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] 37+ messages in thread* [PATCH v1 10/15] arm64: vgic-v3: Add ICV_HPPIR1_EL1 handler
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (8 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 09/15] arm64: vgic-v3: Add ICV_EOIR1_EL1 handler Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-16 11:58 ` [PATCH v1 11/15] arm64: vgic-v3: Add ICV_BPR0_EL1 handler Manish Jaggi
` (5 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
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>
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 35bad3953f..537e164062 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -611,6 +611,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)
+{
+ uint64_t lr_val;
+ int lr, lr_grp, grp;
+ uint32_t 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 = true;
@@ -642,6 +662,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 = false;
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] 37+ messages in thread* [PATCH v1 11/15] arm64: vgic-v3: Add ICV_BPR0_EL1 handler
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (9 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 10/15] arm64: vgic-v3: Add ICV_HPPIR1_EL1 handler Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-16 11:58 ` [PATCH v1 12/15] arm64: vgic-v3: Add ICV_IGNREN0_EL1 handler Manish Jaggi
` (4 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
This patch is ported to xen from linux commit:
423de85a98c2b50715a0784a74f6124fbc0b1548
Add a handler for reading/writing the guest's view of the ICC_BPR0_EL1
register, which is located in the ICH_VMCR_EL2.BPR0 field.
Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 537e164062..e79a56619d 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -97,6 +97,41 @@ void handle_bpr1(struct cpu_user_regs *regs, int regidx, const union hsr hsr)
__vgic_v3_write_bpr1(regs, regidx);
}
+static void __vgic_v3_read_bpr0(struct cpu_user_regs *regs, int regidx)
+{
+ uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+ set_user_reg(regs, regidx, __vgic_v3_get_bpr0(vmcr));
+}
+
+static void __vgic_v3_write_bpr0(struct cpu_user_regs *regs, int regidx)
+{
+ register_t val = get_user_reg(regs, regidx);
+ uint8_t bpr_min = __vgic_v3_bpr_min();
+ uint32_t 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_BPR0_SHIFT;
+ val &= ICH_VMCR_BPR0_MASK;
+ vmcr &= ~ICH_VMCR_BPR0_MASK;
+ vmcr |= val;
+
+ WRITE_SYSREG32(vmcr, ICH_VMCR_EL2);
+}
+
+void handle_bpr0(struct cpu_user_regs *regs, int regidx, const union hsr hsr)
+{
+ if ( hsr.sysreg.read )
+ __vgic_v3_read_bpr0(regs, regidx);
+ else
+ __vgic_v3_write_bpr0(regs, regidx);
+}
+
static void __vgic_v3_read_igrpen1(struct cpu_user_regs *regs, int regidx)
{
uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
@@ -646,6 +681,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_BPR0_EL1:
+ handle_bpr0(regs, regidx, hsr);
+ break;
+
case HSR_SYSREG_ICC_BPR1_EL1:
handle_bpr1(regs, regidx, hsr);
break;
diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
index c23c4a33b2..ef01576b01 100644
--- a/xen/include/asm-arm/arm64/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -94,6 +94,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_ICC_BPR0_EL1 HSR_SYSREG(3,0,c12,c8,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)
--
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] 37+ messages in thread* [PATCH v1 12/15] arm64: vgic-v3: Add ICV_IGNREN0_EL1 handler
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (10 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 11/15] arm64: vgic-v3: Add ICV_BPR0_EL1 handler Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-16 11:58 ` [PATCH v1 13/15] arm64: vgic-v3: Add misc Group-0 handlers Manish Jaggi
` (3 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
This patch is ported to xen from linux commit:
fbc48a0011deb3d51cb657ca9c0f9083f41c0665
Add a handler for reading/writing the guest's view of the
ICC_IGRPEN0_EL1 register, which is located in the ICH_VMCR_EL2.VENG0
field.
Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index e79a56619d..8951a75481 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -160,6 +160,34 @@ void handle_igrpen1(struct cpu_user_regs *regs, int regidx,
__vgic_v3_write_igrpen1(regs, regidx);
}
+static void __vgic_v3_read_igrpen0(struct cpu_user_regs *regs, int regidx)
+{
+ uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+ set_user_reg(regs, regidx, !!(vmcr & ICH_VMCR_ENG0_MASK));
+}
+
+static void __vgic_v3_write_igrpen0(struct cpu_user_regs *regs, int regidx)
+{
+ register_t val = get_user_reg(regs, regidx);
+ uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+
+ if ( val & 1 )
+ vmcr |= ICH_VMCR_ENG0_MASK;
+ else
+ vmcr &= ~ICH_VMCR_ENG0_MASK;
+
+ WRITE_SYSREG32(vmcr, ICH_VMCR_EL2);
+}
+
+void handle_igrpen0(struct cpu_user_regs *regs, int regidx,
+ const union hsr hsr)
+{
+ if ( hsr.sysreg.read )
+ __vgic_v3_read_igrpen0(regs, regidx);
+ else
+ __vgic_v3_write_igrpen0(regs, regidx);
+}
+
void __vgic_v3_write_ap0rn(uint32_t val, int n)
{
switch (n)
@@ -689,6 +717,10 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr
handle_bpr1(regs, regidx, hsr);
break;
+ case HSR_SYSREG_ICC_IGRPEN0_EL1:
+ handle_igrpen0(regs, regidx, hsr);
+ break;
+
case HSR_SYSREG_ICC_IGRPEN1_EL1:
handle_igrpen1(regs, regidx, hsr);
break;
diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
index ef01576b01..f9049a6b04 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_IGRPEN0_EL1 HSR_SYSREG(3,0,c12,c12,6)
#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)
--
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] 37+ messages in thread* [PATCH v1 13/15] arm64: vgic-v3: Add misc Group-0 handlers
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (11 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 12/15] arm64: vgic-v3: Add ICV_IGNREN0_EL1 handler Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-16 11:58 ` [PATCH v1 14/15] arm64: vgic-v3: Add ICV_AP(0/1)Rn_EL1 handler Manish Jaggi
` (2 subsequent siblings)
15 siblings, 0 replies; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
This patch is ported to xen from linux commit:
eab0b2dc4f6f34147e3d10da49ab8032e15dbea0
A number of Group-0 registers can be handled by the same accessors
as that of Group-1, so let's add the required system register encodings
and catch them in the dispatching function.
Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 8951a75481..5af5142b21 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -725,14 +725,17 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr
handle_igrpen1(regs, regidx, hsr);
break;
+ case HSR_SYSREG_ICC_IAR0_EL1:
case HSR_SYSREG_ICC_IAR1_EL1:
handle_iar(regs, regidx, hsr);
break;
+ case HSR_SYSREG_ICC_EOIR0_EL1:
case HSR_SYSREG_ICC_EOIR1_EL1:
handle_eoi(regs, regidx, hsr);
break;
+ case HSR_SYSREG_ICC_HPPIR0_EL1:
case HSR_SYSREG_ICC_HPPIR1_EL1:
handle_hppir1(regs, regidx, hsr);
break;
diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
index f9049a6b04..8d1bd12348 100644
--- a/xen/include/asm-arm/arm64/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -95,6 +95,9 @@
#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_ICC_IAR0_EL1 HSR_SYSREG(3,0,c12,c8,0)
+#define HSR_SYSREG_ICC_EOIR0_EL1 HSR_SYSREG(3,0,c12,c8,1)
+#define HSR_SYSREG_ICC_HPPIR0_EL1 HSR_SYSREG(3,0,c12,c8,2)
#define HSR_SYSREG_ICC_BPR0_EL1 HSR_SYSREG(3,0,c12,c8,3)
#define HSR_SYSREG_CONTEXTIDR_EL1 HSR_SYSREG(3,0,c13,c0,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] 37+ messages in thread* [PATCH v1 14/15] arm64: vgic-v3: Add ICV_AP(0/1)Rn_EL1 handler
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (12 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 13/15] arm64: vgic-v3: Add misc Group-0 handlers Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-16 11:58 ` [PATCH v1 15/15] Enable Group0/1 Traps by default for Cavium ThunderX1 Manish Jaggi
2018-03-20 7:46 ` [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Julien Grall
15 siblings, 0 replies; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
This patch is a xen port of linux commit f9e7449c780f688bf61a13dfa8c344afeb4ad6e0.
Add a handler for reading/writing the guest's view of the ICV_AP1Rn_EL1
registers. We just map them to the corresponding ICH_AP(0/1)Rn_EL2 registers.
Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 5af5142b21..2ca1145336 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -694,6 +694,66 @@ spurious:
set_user_reg(regs, regidx, lr_val & ICH_LR_VIRTUAL_ID_MASK);
}
+static void __vgic_v3_read_apxrn(struct cpu_user_regs *regs, int regidx,
+ const union hsr hsr, int n)
+{
+ uint32_t val;
+
+ if ( !__vgic_v3_get_group(hsr) )
+ val = __vgic_v3_read_ap0rn(n);
+ else
+ val = __vgic_v3_read_ap1rn(n);
+
+ set_user_reg(regs, regidx, val);
+}
+
+static void __vgic_v3_write_apxrn(struct cpu_user_regs *regs, int regidx,
+ const union hsr hsr, int n)
+{
+ uint32_t val = get_user_reg(regs, regidx);
+
+ if (!__vgic_v3_get_group(hsr))
+ __vgic_v3_write_ap0rn(val, n);
+ else
+ __vgic_v3_write_ap1rn(val, n);
+}
+
+void handle_apxr0(struct cpu_user_regs *regs, int regidx,
+ const union hsr hsr)
+{
+ if(hsr.sysreg.read)
+ __vgic_v3_read_apxrn(regs, regidx, hsr, 0);
+ else
+ __vgic_v3_write_apxrn(regs, regidx, hsr, 0);
+}
+
+void handle_apxr1(struct cpu_user_regs *regs, int regidx,
+ const union hsr hsr)
+{
+ if(hsr.sysreg.read)
+ __vgic_v3_read_apxrn(regs, regidx, hsr, 1);
+ else
+ __vgic_v3_write_apxrn(regs, regidx, hsr, 1);
+}
+
+void handle_apxr2(struct cpu_user_regs *regs, int regidx,
+ const union hsr hsr)
+{
+ if(hsr.sysreg.read)
+ __vgic_v3_read_apxrn(regs, regidx, hsr, 2);
+ else
+ __vgic_v3_write_apxrn(regs, regidx, hsr, 2);
+}
+
+void handle_apxr3(struct cpu_user_regs *regs, int regidx,
+ const union hsr hsr)
+{
+ if(hsr.sysreg.read)
+ __vgic_v3_read_apxrn(regs, regidx, hsr, 3);
+ else
+ __vgic_v3_write_apxrn(regs, regidx, hsr, 3);
+}
+
bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr)
{
bool ret = true;
@@ -740,6 +800,26 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs, const union hsr hsr
handle_hppir1(regs, regidx, hsr);
break;
+ case HSR_SYSREG_ICC_AP0Rn_EL1(0):
+ case HSR_SYSREG_ICC_AP1Rn_EL1(0):
+ handle_apxr0(regs, regidx, hsr);
+ break;
+
+ case HSR_SYSREG_ICC_AP0Rn_EL1(1):
+ case HSR_SYSREG_ICC_AP1Rn_EL1(1):
+ handle_apxr1(regs, regidx, hsr);
+ break;
+
+ case HSR_SYSREG_ICC_AP0Rn_EL1(2):
+ case HSR_SYSREG_ICC_AP1Rn_EL1(2):
+ handle_apxr2(regs, regidx, hsr);
+ break;
+
+ case HSR_SYSREG_ICC_AP0Rn_EL1(3):
+ case HSR_SYSREG_ICC_AP1Rn_EL1(3):
+ handle_apxr3(regs, regidx, hsr);
+ break;
+
default:
ret = false;
break;
diff --git a/xen/include/asm-arm/arm64/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
index 8d1bd12348..e446b5de1c 100644
--- a/xen/include/asm-arm/arm64/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -85,6 +85,17 @@
#define HSR_SYSREG_PMINTENCLR_EL1 HSR_SYSREG(3,0,c9,c14,2)
#define HSR_SYSREG_MAIR_EL1 HSR_SYSREG(3,0,c10,c2,0)
#define HSR_SYSREG_AMAIR_EL1 HSR_SYSREG(3,0,c10,c3,0)
+#define HSR_SYSREG_ICC_AP0Rn_EL1(n) HSR_SYSREG(3,0,c12,c8,4|n)
+#define HSR_SYSREG_ICC_AP0R0_EL1 HSR_SYSREG_ICC_AP0Rn_EL1(0)
+#define HSR_SYSREG_ICC_AP0R1_EL1 HSR_SYSREG_ICC_AP0Rn_EL1(1)
+#define HSR_SYSREG_ICC_AP0R2_EL1 HSR_SYSREG_ICC_AP0Rn_EL1(2)
+#define HSR_SYSREG_ICC_AP0R3_EL1 HSR_SYSREG_ICC_AP0Rn_EL1(3)
+#define HSR_SYSREG_ICC_AP1Rn_EL1(n) HSR_SYSREG(3,0,c12,c9, n)
+#define HSR_SYSREG_ICC_AP1R0_EL1 HSR_SYSREG_ICC_AP1Rn_EL1(0)
+#define HSR_SYSREG_ICC_AP1R1_EL1 HSR_SYSREG_ICC_AP1Rn_EL1(1)
+#define HSR_SYSREG_ICC_AP1R2_EL1 HSR_SYSREG_ICC_AP1Rn_EL1(2)
+#define HSR_SYSREG_ICC_AP1R3_EL1 HSR_SYSREG_ICC_AP1Rn_EL1(3)
+
#define HSR_SYSREG_ICC_SGI1R_EL1 HSR_SYSREG(3,0,c12,c11,5)
#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)
--
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] 37+ messages in thread* [PATCH v1 15/15] Enable Group0/1 Traps by default for Cavium ThunderX1
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (13 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 14/15] arm64: vgic-v3: Add ICV_AP(0/1)Rn_EL1 handler Manish Jaggi
@ 2018-03-16 11:58 ` Manish Jaggi
2018-03-20 7:46 ` [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Julien Grall
15 siblings, 0 replies; 37+ messages in thread
From: Manish Jaggi @ 2018-03-16 11:58 UTC (permalink / raw)
To: xen-devel, julien.grall, sstabellini, marc.zyngier,
andre.przywara
Cc: manish.jaggi
Enable trapping for Group0/1 register access when
CONFIG_CAVIUM_ERRATUM_30115 is enabled.
Signed-off-by: Manish Jaggi <manish.jaggi@cavium.com>
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 473e26111f..6ffed6a634 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -43,6 +43,7 @@
#include <asm/gic.h>
#include <asm/gic_v3_defs.h>
#include <asm/gic_v3_its.h>
+#include <asm/cpuerrata.h>
#include <asm/cpufeature.h>
#include <asm/acpi.h>
@@ -825,7 +826,7 @@ static void gicv3_cpu_disable(void)
static void gicv3_hyp_init(void)
{
- uint32_t vtr;
+ uint32_t vtr, hcr = GICH_HCR_EN;
vtr = READ_SYSREG32(ICH_VTR_EL2);
gicv3_info.nr_lrs = (vtr & GICH_VTR_NRLRGS) + 1;
@@ -836,7 +837,14 @@ 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);
+
+ if ( cpus_have_cap(ARM64_WORKAROUND_CAVIUM_30115) )
+ {
+ hcr |= GICH_HCR_TALL1;
+ hcr |= GICH_HCR_TALL0;
+ }
+
+ WRITE_SYSREG32(hcr, 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..c76a330b6b 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -117,6 +117,8 @@
#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_HCR_TALL0 (1 << 11)
#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] 37+ messages in thread* Re: [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2
2018-03-16 11:58 [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Manish Jaggi
` (14 preceding siblings ...)
2018-03-16 11:58 ` [PATCH v1 15/15] Enable Group0/1 Traps by default for Cavium ThunderX1 Manish Jaggi
@ 2018-03-20 7:46 ` Julien Grall
2018-03-21 4:58 ` Manish Jaggi
15 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2018-03-20 7:46 UTC (permalink / raw)
To: Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
On 03/16/2018 11:58 AM, Manish Jaggi wrote:
> 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.
This cover letter does not seem to match the series. Please update it on
every time you send a series.
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] 37+ messages in thread* Re: [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2
2018-03-20 7:46 ` [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2 Julien Grall
@ 2018-03-21 4:58 ` Manish Jaggi
2018-03-21 5:02 ` Julien Grall
2018-03-21 8:45 ` Julien Grall
0 siblings, 2 replies; 37+ messages in thread
From: Manish Jaggi @ 2018-03-21 4:58 UTC (permalink / raw)
To: Julien Grall, Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
Hi Julien,
On 03/20/2018 01:16 PM, Julien Grall wrote:
>
>
> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>> 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.
>
> This cover letter does not seem to match the series. Please update it
> on every time you send a series.
%s/vsysreg.c/vgic-v3-sr..
Could you please review the other patches in the series, so that I can
send v2.
>
> Cheers,
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2
2018-03-21 4:58 ` Manish Jaggi
@ 2018-03-21 5:02 ` Julien Grall
2018-03-21 8:45 ` Julien Grall
1 sibling, 0 replies; 37+ messages in thread
From: Julien Grall @ 2018-03-21 5:02 UTC (permalink / raw)
To: Manish Jaggi, Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
On 03/21/2018 04:58 AM, Manish Jaggi wrote:
>
> Hi Julien,
>
> On 03/20/2018 01:16 PM, Julien Grall wrote:
>>
>>
>> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>>> 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.
>>
>> This cover letter does not seem to match the series. Please update it
>> on every time you send a series.
> %s/vsysreg.c/vgic-v3-sr..
"The current RFC patchset is a subset of [1], as it handling only Group
1 traps as a PoC". This is clearly not a PoC anymore nor only handling
Group 1.
>
> Could you please review the other patches in the series, so that I can
> send v2.
It is in my queue of patch to review.
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] 37+ messages in thread
* Re: [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2
2018-03-21 4:58 ` Manish Jaggi
2018-03-21 5:02 ` Julien Grall
@ 2018-03-21 8:45 ` Julien Grall
2018-03-21 9:38 ` Manish Jaggi
1 sibling, 1 reply; 37+ messages in thread
From: Julien Grall @ 2018-03-21 8:45 UTC (permalink / raw)
To: Manish Jaggi, Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
On 03/21/2018 04:58 AM, Manish Jaggi wrote:
>
> Hi Julien,
>
> On 03/20/2018 01:16 PM, Julien Grall wrote:
>>
>>
>> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>>> 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.
>>
>> This cover letter does not seem to match the series. Please update it
>> on every time you send a series.
> %s/vsysreg.c/vgic-v3-sr..
>
> Could you please review the other patches in the series, so that I can
> send v2.
Here the major comments for the series (included patch not reviewed):
1) You seem to miss some patches from Linux. I would like to understand
why they are not there.
2) Strangely some commits does not match the Linux one either in order
and content (I am not speaking about the changes required by Xen). For
instance this is the case of patch #14 "arm64: vgic-v3: Add
ICV_AP(0/1)Rn_EL1 handler". If you port commit from Linux, then you
should follow the same. This help a lot for review.
3)
Code organization:
1) Please drop __ from all functions
2) All functions not exported *should* be static. At the same time you
need to make sure that the series are bisectable. So you probably hook
the file in the build system at the end rather than in #3.
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] 37+ messages in thread
* Re: [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2
2018-03-21 8:45 ` Julien Grall
@ 2018-03-21 9:38 ` Manish Jaggi
2018-03-21 9:56 ` Julien Grall
0 siblings, 1 reply; 37+ messages in thread
From: Manish Jaggi @ 2018-03-21 9:38 UTC (permalink / raw)
To: Julien Grall, Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
On 03/21/2018 02:15 PM, Julien Grall wrote:
>
>
> On 03/21/2018 04:58 AM, Manish Jaggi wrote:
>>
>> Hi Julien,
>>
>> On 03/20/2018 01:16 PM, Julien Grall wrote:
>>>
>>>
>>> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>>>> 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.
>>>
>>> This cover letter does not seem to match the series. Please update
>>> it on every time you send a series.
>> %s/vsysreg.c/vgic-v3-sr..
>>
>> Could you please review the other patches in the series, so that I
>> can send v2.
>
> Here the major comments for the series (included patch not reviewed):
> 1) You seem to miss some patches from Linux. I would like to
> understand why they are not there.
if code is ported to xen, it is perfectly fine to take only relevant
patches.
For instance we are not providing any command line option to
individually enable group1 grou0 traps.
> 2) Strangely some commits does not match the Linux one either in
> order and content (I am not speaking about the changes required by
> Xen). For instance this is the case of patch #14 "arm64: vgic-v3: Add
> ICV_AP(0/1)Rn_EL1 handler". If you port commit from Linux, then you
> should follow the same. This help a lot for review.
Since we are not doing individually enable of group0/1, it doesnt make
sense to have two set of patches for ICV_AP0 / ICV_AP1. So I merged it.
> 3)
>
> Code organization:
> 1) Please drop __ from all functions
ok I will change.
> 2) All functions not exported *should* be static. At the same time
> you need to make sure that the series are bisectable. So you probably
> hook the file in the build system at the end rather than in #3.
>
ok.
> Cheers,
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2
2018-03-21 9:38 ` Manish Jaggi
@ 2018-03-21 9:56 ` Julien Grall
2018-03-23 6:42 ` Manish Jaggi
0 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2018-03-21 9:56 UTC (permalink / raw)
To: Manish Jaggi, Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
Hi Manish,
On 03/21/2018 09:38 AM, Manish Jaggi wrote:
>
>
> On 03/21/2018 02:15 PM, Julien Grall wrote:
>>
>>
>> On 03/21/2018 04:58 AM, Manish Jaggi wrote:
>>>
>>> Hi Julien,
>>>
>>> On 03/20/2018 01:16 PM, Julien Grall wrote:
>>>>
>>>>
>>>> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>>>>> 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.
>>>>
>>>> This cover letter does not seem to match the series. Please update
>>>> it on every time you send a series.
>>> %s/vsysreg.c/vgic-v3-sr..
>>>
>>> Could you please review the other patches in the series, so that I
>>> can send v2.
>>
>> Here the major comments for the series (included patch not reviewed):
>> 1) You seem to miss some patches from Linux. I would like to
>> understand why they are not there.
> if code is ported to xen, it is perfectly fine to take only relevant
> patches.
It is usually expected from the contributor to have some sort of
explanation in the cover letter. In particular when you are based on a
series from Linux.
Where I am more worried is there are patch on top in Linux, that you
didn't backport. So it would be really nice to understand why those
patches are not in Xen.
A non-exhaustive list:
- KVM: arm64: Log an error if trapping a write-to-read-only GICv3 access
- KVM: arm64: Log an error if trapping a read-from-write-only
GICv3 access
> For instance we are not providing any command line option to
> individually enable group1 grou0 traps.
I think the command line option could be useful for testing. Developer
don't necessarily have a Thunder-X in hand.
>> 2) Strangely some commits does not match the Linux one either in
>> order and content (I am not speaking about the changes required by
>> Xen). For instance this is the case of patch #14 "arm64: vgic-v3: Add
>> ICV_AP(0/1)Rn_EL1 handler". If you port commit from Linux, then you
>> should follow the same. This help a lot for review.
> Since we are not doing individually enable of group0/1, it doesnt make
> sense to have two set of patches for ICV_AP0 / ICV_AP1. So I merged it.
Sorry, but it does not make sense. Looking at the series you pointed. I
don't see a patch just for ICV_AP0. Instead it is part of " KVM: arm64:
vgic-v3: Enable trapping of Group-0 system registers". You ported that
patch in Xen.
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] 37+ messages in thread* Re: [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2
2018-03-21 9:56 ` Julien Grall
@ 2018-03-23 6:42 ` Manish Jaggi
2018-03-23 6:58 ` Julien Grall
0 siblings, 1 reply; 37+ messages in thread
From: Manish Jaggi @ 2018-03-23 6:42 UTC (permalink / raw)
To: Julien Grall, Manish Jaggi, xen-devel, sstabellini, marc.zyngier,
andre.przywara
On 03/21/2018 03:26 PM, Julien Grall wrote:
> Hi Manish,
>
> On 03/21/2018 09:38 AM, Manish Jaggi wrote:
>>
>>
>> On 03/21/2018 02:15 PM, Julien Grall wrote:
>>>
>>>
>>> On 03/21/2018 04:58 AM, Manish Jaggi wrote:
>>>>
>>>> Hi Julien,
>>>>
>>>> On 03/20/2018 01:16 PM, Julien Grall wrote:
>>>>>
>>>>>
>>>>> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>>>>>> 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.
>>>>>
>>>>> This cover letter does not seem to match the series. Please update
>>>>> it on every time you send a series.
>>>> %s/vsysreg.c/vgic-v3-sr..
>>>>
>>>> Could you please review the other patches in the series, so that I
>>>> can send v2.
>>>
>>> Here the major comments for the series (included patch not reviewed):
>>> 1) You seem to miss some patches from Linux. I would like to
>>> understand why they are not there.
>> if code is ported to xen, it is perfectly fine to take only relevant
>> patches.
>
> It is usually expected from the contributor to have some sort of
> explanation in the cover letter. In particular when you are based on a
> series from Linux.
>
> Where I am more worried is there are patch on top in Linux, that you
> didn't backport. So it would be really nice to understand why those
> patches are not in Xen.
>
> A non-exhaustive list:
> - KVM: arm64: Log an error if trapping a write-to-read-only GICv3
> access
> - KVM: arm64: Log an error if trapping a read-from-write-only
> GICv3 access
>
>
>> For instance we are not providing any command line option to
>> individually enable group1 grou0 traps.
>
> I think the command line option could be useful for testing. Developer
> don't necessarily have a Thunder-X in hand.
>
>>> 2) Strangely some commits does not match the Linux one either in
>>> order and content (I am not speaking about the changes required by
>>> Xen). For instance this is the case of patch #14 "arm64: vgic-v3:
>>> Add ICV_AP(0/1)Rn_EL1 handler". If you port commit from Linux, then
>>> you should follow the same. This help a lot for review.
>> Since we are not doing individually enable of group0/1, it doesnt
>> make sense to have two set of patches for ICV_AP0 / ICV_AP1. So I
>> merged it.
>
> Sorry, but it does not make sense. Looking at the series you pointed.
> I don't see a patch just for ICV_AP0. Instead it is part of " KVM:
> arm64: vgic-v3: Enable trapping of Group-0 system registers". You
> ported that patch in Xen.
>
If you see this patch, you will find this one specifically for ICV_AP1
https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/026040.html
> Cheers,
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2
2018-03-23 6:42 ` Manish Jaggi
@ 2018-03-23 6:58 ` Julien Grall
2018-03-26 4:43 ` Manish Jaggi
0 siblings, 1 reply; 37+ messages in thread
From: Julien Grall @ 2018-03-23 6:58 UTC (permalink / raw)
To: Manish Jaggi
Cc: sstabellini, marc.zyngier, andre.przywara, Jaggi, Manish,
Julien Grall, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 3389 bytes --]
(Sorry for the formatting)
On 23 Mar 2018 14:46, "Manish Jaggi" <mjaggi@caviumnetworks.com> wrote:
On 03/21/2018 03:26 PM, Julien Grall wrote:
> Hi Manish,
>
> On 03/21/2018 09:38 AM, Manish Jaggi wrote:
>
>>
>>
>> On 03/21/2018 02:15 PM, Julien Grall wrote:
>>
>>>
>>>
>>> On 03/21/2018 04:58 AM, Manish Jaggi wrote:
>>>
>>>>
>>>> Hi Julien,
>>>>
>>>> On 03/20/2018 01:16 PM, Julien Grall wrote:
>>>>
>>>>>
>>>>>
>>>>> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>>>>>
>>>>>> 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.
>>>>>>
>>>>>
>>>>> This cover letter does not seem to match the series. Please update it
>>>>> on every time you send a series.
>>>>>
>>>> %s/vsysreg.c/vgic-v3-sr..
>>>>
>>>> Could you please review the other patches in the series, so that I can
>>>> send v2.
>>>>
>>>
>>> Here the major comments for the series (included patch not reviewed):
>>> 1) You seem to miss some patches from Linux. I would like to
>>> understand why they are not there.
>>>
>> if code is ported to xen, it is perfectly fine to take only relevant
>> patches.
>>
>
> It is usually expected from the contributor to have some sort of
> explanation in the cover letter. In particular when you are based on a
> series from Linux.
>
> Where I am more worried is there are patch on top in Linux, that you
> didn't backport. So it would be really nice to understand why those patches
> are not in Xen.
>
> A non-exhaustive list:
> - KVM: arm64: Log an error if trapping a write-to-read-only GICv3
> access
> - KVM: arm64: Log an error if trapping a read-from-write-only
> GICv3 access
>
>
> For instance we are not providing any command line option to individually
>> enable group1 grou0 traps.
>>
>
> I think the command line option could be useful for testing. Developer
> don't necessarily have a Thunder-X in hand.
>
> 2) Strangely some commits does not match the Linux one either in order
>>> and content (I am not speaking about the changes required by Xen). For
>>> instance this is the case of patch #14 "arm64: vgic-v3: Add
>>> ICV_AP(0/1)Rn_EL1 handler". If you port commit from Linux, then you should
>>> follow the same. This help a lot for review.
>>>
>> Since we are not doing individually enable of group0/1, it doesnt make
>> sense to have two set of patches for ICV_AP0 / ICV_AP1. So I merged it.
>>
>
> Sorry, but it does not make sense. Looking at the series you pointed. I
> don't see a patch just for ICV_AP0. Instead it is part of " KVM: arm64:
> vgic-v3: Enable trapping of Group-0 system registers". You ported that
> patch in Xen.
>
> If you see this patch, you will find this one specifically for ICV_AP1
https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/026040.html
You didn't get my point... You still don't explain why you move the
ICV_AP0 from "Enable trapping of Group-0 system registers" to that patch.
If you take commit from Linux then don't move code between commit around
unless there is a good reason.
Please try to make the review a bit easier...
Cheers,
[-- Attachment #1.2: Type: text/html, Size: 5182 bytes --]
[-- Attachment #2: Type: text/plain, Size: 157 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2
2018-03-23 6:58 ` Julien Grall
@ 2018-03-26 4:43 ` Manish Jaggi
2018-03-26 8:24 ` Marc Zyngier
0 siblings, 1 reply; 37+ messages in thread
From: Manish Jaggi @ 2018-03-26 4:43 UTC (permalink / raw)
To: Julien Grall
Cc: sstabellini, marc.zyngier, andre.przywara, Jaggi, Manish,
Julien Grall, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 4712 bytes --]
On 03/23/2018 12:28 PM, Julien Grall wrote:
> (Sorry for the formatting)
>
> On 23 Mar 2018 14:46, "Manish Jaggi" <mjaggi@caviumnetworks.com
> <mailto:mjaggi@caviumnetworks.com>> wrote:
>
>
>
> On 03/21/2018 03:26 PM, Julien Grall wrote:
>
> Hi Manish,
>
> On 03/21/2018 09:38 AM, Manish Jaggi wrote:
>
>
>
> On 03/21/2018 02:15 PM, Julien Grall wrote:
>
>
>
> On 03/21/2018 04:58 AM, Manish Jaggi wrote:
>
>
> Hi Julien,
>
> On 03/20/2018 01:16 PM, Julien Grall wrote:
>
>
>
> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>
> 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.
>
>
> This cover letter does not seem to match the
> series. Please update it on every time you
> send a series.
>
> %s/vsysreg.c/vgic-v3-sr..
>
> Could you please review the other patches in the
> series, so that I can send v2.
>
>
> Here the major comments for the series (included patch
> not reviewed):
> 1) You seem to miss some patches from Linux. I
> would like to understand why they are not there.
>
> if code is ported to xen, it is perfectly fine to take
> only relevant patches.
>
>
> It is usually expected from the contributor to have some sort
> of explanation in the cover letter. In particular when you are
> based on a series from Linux.
>
> Where I am more worried is there are patch on top in Linux,
> that you didn't backport. So it would be really nice to
> understand why those patches are not in Xen.
>
> A non-exhaustive list:
> - KVM: arm64: Log an error if trapping a
> write-to-read-only GICv3 access
> - KVM: arm64: Log an error if trapping a
> read-from-write-only GICv3 access
>
>
> For instance we are not providing any command line option
> to individually enable group1 grou0 traps.
>
>
> I think the command line option could be useful for testing.
> Developer don't necessarily have a Thunder-X in hand.
>
> 2) Strangely some commits does not match the Linux
> one either in order and content (I am not speaking
> about the changes required by Xen). For instance this
> is the case of patch #14 "arm64: vgic-v3: Add
> ICV_AP(0/1)Rn_EL1 handler". If you port commit from
> Linux, then you should follow the same. This help a
> lot for review.
>
> Since we are not doing individually enable of group0/1, it
> doesnt make sense to have two set of patches for ICV_AP0 /
> ICV_AP1. So I merged it.
>
>
> Sorry, but it does not make sense. Looking at the series you
> pointed. I don't see a patch just for ICV_AP0. Instead it is
> part of " KVM: arm64: vgic-v3: Enable trapping of Group-0
> system registers". You ported that patch in Xen.
>
> If you see this patch, you will find this one specifically for ICV_AP1
> https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/026040.html
> <https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/026040.html>
>
>
> You didn't get my point... You still don't explain why you move the
> ICV_AP0 from "Enable trapping of Group-0 system registers" to that
> patch. If you take commit from Linux then don't move code between
> commit around unless there is a good reason.
I gave a good reason, that it make sense to club two patches as we are
not individually enabling g0/g1 for 30115 errata.
There is no documented rule that code taken from linux has to be 1:1
mapping with linux patch.
When complete smmu.c is taken as one file the rule is not applied?
>
> Please try to make the review a bit easier...
>
> Cheers,
>
[-- Attachment #1.2: Type: text/html, Size: 9338 bytes --]
[-- Attachment #2: Type: text/plain, Size: 157 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v1 00/15] arm64: Mediate access to GICv3 sysregs at EL2
2018-03-26 4:43 ` Manish Jaggi
@ 2018-03-26 8:24 ` Marc Zyngier
0 siblings, 0 replies; 37+ messages in thread
From: Marc Zyngier @ 2018-03-26 8:24 UTC (permalink / raw)
To: Manish Jaggi, Julien Grall
Cc: xen-devel, Julien Grall, sstabellini, andre.przywara,
Jaggi, Manish
On 26/03/18 05:43, Manish Jaggi wrote:
>
>
> On 03/23/2018 12:28 PM, Julien Grall wrote:
>> (Sorry for the formatting)
>>
>> On 23 Mar 2018 14:46, "Manish Jaggi" <mjaggi@caviumnetworks.com
>> <mailto:mjaggi@caviumnetworks.com>> wrote:
>>
>>
>>
>> On 03/21/2018 03:26 PM, Julien Grall wrote:
>>
>> Hi Manish,
>>
>> On 03/21/2018 09:38 AM, Manish Jaggi wrote:
>>
>>
>>
>> On 03/21/2018 02:15 PM, Julien Grall wrote:
>>
>>
>>
>> On 03/21/2018 04:58 AM, Manish Jaggi wrote:
>>
>>
>> Hi Julien,
>>
>> On 03/20/2018 01:16 PM, Julien Grall wrote:
>>
>>
>>
>> On 03/16/2018 11:58 AM, Manish Jaggi wrote:
>>
>> 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.
>>
>>
>> This cover letter does not seem to match the
>> series. Please update it on every time you
>> send a series.
>>
>> %s/vsysreg.c/vgic-v3-sr..
>>
>> Could you please review the other patches in the
>> series, so that I can send v2.
>>
>>
>> Here the major comments for the series (included patch
>> not reviewed):
>> 1) You seem to miss some patches from Linux. I
>> would like to understand why they are not there.
>>
>> if code is ported to xen, it is perfectly fine to take
>> only relevant patches.
>>
>>
>> It is usually expected from the contributor to have some sort
>> of explanation in the cover letter. In particular when you are
>> based on a series from Linux.
>>
>> Where I am more worried is there are patch on top in Linux,
>> that you didn't backport. So it would be really nice to
>> understand why those patches are not in Xen.
>>
>> A non-exhaustive list:
>> - KVM: arm64: Log an error if trapping a
>> write-to-read-only GICv3 access
>> - KVM: arm64: Log an error if trapping a
>> read-from-write-only GICv3 access
>>
>>
>> For instance we are not providing any command line option
>> to individually enable group1 grou0 traps.
>>
>>
>> I think the command line option could be useful for testing.
>> Developer don't necessarily have a Thunder-X in hand.
>>
>> 2) Strangely some commits does not match the Linux
>> one either in order and content (I am not speaking
>> about the changes required by Xen). For instance this
>> is the case of patch #14 "arm64: vgic-v3: Add
>> ICV_AP(0/1)Rn_EL1 handler". If you port commit from
>> Linux, then you should follow the same. This help a
>> lot for review.
>>
>> Since we are not doing individually enable of group0/1, it
>> doesnt make sense to have two set of patches for ICV_AP0 /
>> ICV_AP1. So I merged it.
>>
>>
>> Sorry, but it does not make sense. Looking at the series you
>> pointed. I don't see a patch just for ICV_AP0. Instead it is
>> part of " KVM: arm64: vgic-v3: Enable trapping of Group-0
>> system registers". You ported that patch in Xen.
>>
>> If you see this patch, you will find this one specifically for ICV_AP1
>> https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/026040.html
>> <https://lists.cs.columbia.edu/pipermail/kvmarm/2017-June/026040.html>
>>
>>
>> You didn't get my point... You still don't explain why you move the
>> ICV_AP0 from "Enable trapping of Group-0 system registers" to that
>> patch. If you take commit from Linux then don't move code between
>> commit around unless there is a good reason.
> I gave a good reason, that it make sense to club two patches as we are
> not individually enabling g0/g1 for 30115 errata.
> There is no documented rule that code taken from linux has to be 1:1
> mapping with linux patch.
As the original author of these patches, I want to be able to easily
refer to the original design to see what has changed, and understand
why. I suspect I'm not the only one in this case. Given that there are
at most 4 people in this community who understand enough about the GIC
architecture to follow exactly what these patches are doing, you might
as well try to make it easier for them by offering a 1:1 mapping between
the original code and your port.
Now, if you're happy about these patches not being reviewed, and thus
not merged, that's absolutely your call. In which case, how about not
posting them at all?
> When complete smmu.c is taken as one file the rule is not applied?
A bad decision one day doesn't mean you should always make the same
mistake. People usually learn.
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] 37+ messages in thread