* [PATCH v2 1/2] target/ppc: add LPCR[HR] to DisasContext and hflags
2021-09-17 11:47 [PATCH v2 0/2] Require hypervisor privilege for tlbie[l] when PSR=0 and HR=1 matheus.ferst
@ 2021-09-17 11:47 ` matheus.ferst
2021-09-17 11:47 ` [PATCH v2 2/2] target/ppc: Check privilege level based on PSR and LPCR[HR] in tlbie[l] matheus.ferst
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: matheus.ferst @ 2021-09-17 11:47 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: leandro.lupori, danielhb413, richard.henderson, groug,
Matheus Ferst, david
From: Matheus Ferst <matheus.ferst@eldorado.org.br>
Add a Host Radix field (hr) in DisasContext with LPCR[HR] value to allow
us to decide between Radix and HPT while validating instructions
arguments. Note that PowerISA v3.1 does not require LPCR[HR] and PATE.HR
to match if the thread is in ultravisor/hypervisor real addressing mode,
so ctx->hr may be invalid if ctx->hv and ctx->dr are set.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
target/ppc/cpu.h | 1 +
target/ppc/helper_regs.c | 3 +++
target/ppc/translate.c | 2 ++
3 files changed, 6 insertions(+)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 500205229c..e1b8d343cd 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -600,6 +600,7 @@ enum {
HFLAGS_64 = 2, /* computed from MSR_CE and MSR_SF */
HFLAGS_GTSE = 3, /* computed from SPR_LPCR[GTSE] */
HFLAGS_DR = 4, /* MSR_DR */
+ HFLAGS_HR = 5, /* computed from SPR_LPCR[HR] */
HFLAGS_SPE = 6, /* from MSR_SPE if cpu has SPE; avoid overlap w/ MSR_VR */
HFLAGS_TM = 8, /* computed from MSR_TM */
HFLAGS_BE = 9, /* MSR_BE -- from elsewhere on embedded ppc */
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 405450d863..1bfb480ecf 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -106,6 +106,9 @@ static uint32_t hreg_compute_hflags_value(CPUPPCState *env)
if (env->spr[SPR_LPCR] & LPCR_GTSE) {
hflags |= 1 << HFLAGS_GTSE;
}
+ if (env->spr[SPR_LPCR] & LPCR_HR) {
+ hflags |= 1 << HFLAGS_HR;
+ }
#ifndef CONFIG_USER_ONLY
if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) {
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 171b216e17..909a092fde 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -175,6 +175,7 @@ struct DisasContext {
bool spe_enabled;
bool tm_enabled;
bool gtse;
+ bool hr;
ppc_spr_t *spr_cb; /* Needed to check rights for mfspr/mtspr */
int singlestep_enabled;
uint32_t flags;
@@ -8539,6 +8540,7 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
ctx->vsx_enabled = (hflags >> HFLAGS_VSX) & 1;
ctx->tm_enabled = (hflags >> HFLAGS_TM) & 1;
ctx->gtse = (hflags >> HFLAGS_GTSE) & 1;
+ ctx->hr = (hflags >> HFLAGS_HR) & 1;
ctx->singlestep_enabled = 0;
if ((hflags >> HFLAGS_SE) & 1) {
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] target/ppc: Check privilege level based on PSR and LPCR[HR] in tlbie[l]
2021-09-17 11:47 [PATCH v2 0/2] Require hypervisor privilege for tlbie[l] when PSR=0 and HR=1 matheus.ferst
2021-09-17 11:47 ` [PATCH v2 1/2] target/ppc: add LPCR[HR] to DisasContext and hflags matheus.ferst
@ 2021-09-17 11:47 ` matheus.ferst
2021-09-17 14:17 ` [PATCH v2 0/2] Require hypervisor privilege for tlbie[l] when PSR=0 and HR=1 Daniel Henrique Barboza
2021-09-20 7:40 ` David Gibson
3 siblings, 0 replies; 5+ messages in thread
From: matheus.ferst @ 2021-09-17 11:47 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: leandro.lupori, danielhb413, richard.henderson, groug,
Matheus Ferst, david
From: Matheus Ferst <matheus.ferst@eldorado.org.br>
PowerISA v3.0B made tlbie[l] hypervisor privileged when PSR=0 and HR=1.
To allow the check at translation time, we'll use the HR bit of LPCR to
check the MMU mode instead of the PATE.HR.
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
target/ppc/translate.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 909a092fde..25ee9f75b2 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -5517,7 +5517,15 @@ static void gen_tlbiel(DisasContext *ctx)
#if defined(CONFIG_USER_ONLY)
GEN_PRIV;
#else
- CHK_SV;
+ bool psr = (ctx->opcode >> 17) & 0x1;
+
+ if (ctx->pr || (!ctx->hv && !psr && ctx->hr)) {
+ /*
+ * tlbiel is privileged except when PSR=0 and HR=1, making it
+ * hypervisor privileged.
+ */
+ GEN_PRIV;
+ }
gen_helper_tlbie(cpu_env, cpu_gpr[rB(ctx->opcode)]);
#endif /* defined(CONFIG_USER_ONLY) */
@@ -5529,12 +5537,20 @@ static void gen_tlbie(DisasContext *ctx)
#if defined(CONFIG_USER_ONLY)
GEN_PRIV;
#else
+ bool psr = (ctx->opcode >> 17) & 0x1;
TCGv_i32 t1;
- if (ctx->gtse) {
- CHK_SV; /* If gtse is set then tlbie is supervisor privileged */
- } else {
- CHK_HV; /* Else hypervisor privileged */
+ if (ctx->pr) {
+ /* tlbie is privileged... */
+ GEN_PRIV;
+ } else if (!ctx->hv) {
+ if (!ctx->gtse || (!psr && ctx->hr)) {
+ /*
+ * ... except when GTSE=0 or when PSR=0 and HR=1, making it
+ * hypervisor privileged.
+ */
+ GEN_PRIV;
+ }
}
if (NARROW_MODE(ctx)) {
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 0/2] Require hypervisor privilege for tlbie[l] when PSR=0 and HR=1.
2021-09-17 11:47 [PATCH v2 0/2] Require hypervisor privilege for tlbie[l] when PSR=0 and HR=1 matheus.ferst
2021-09-17 11:47 ` [PATCH v2 1/2] target/ppc: add LPCR[HR] to DisasContext and hflags matheus.ferst
2021-09-17 11:47 ` [PATCH v2 2/2] target/ppc: Check privilege level based on PSR and LPCR[HR] in tlbie[l] matheus.ferst
@ 2021-09-17 14:17 ` Daniel Henrique Barboza
2021-09-20 7:40 ` David Gibson
3 siblings, 0 replies; 5+ messages in thread
From: Daniel Henrique Barboza @ 2021-09-17 14:17 UTC (permalink / raw)
To: matheus.ferst, qemu-devel, qemu-ppc
Cc: leandro.lupori, richard.henderson, groug, david
On 9/17/21 08:47, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>
> While working on FreeBSD radix support, Leandro Lupori (CC'ed) noticed
> that the latest build still fails in KVM but works in TCG[1]. This
> difference occurs because the current implementation of "tlbiel" does
> not validate the instruction parameters and always check for supervisor
> privilege.
>
> This patch series partially address this problem by requiring hypervisor
> privilege for radix mode when PSR=0. The validation of other parameters
> can be done when we move storage control instructions to decodetree.
>
> [1] To reproduce the issue, grab an ISO from [2] run qemu as
>
> qemu-system-ppc64 -cpu power9 -m 2G \
> -machine pseries,cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,cap-ccf-assist=off \
> -boot d -vga none -nographic -cdrom FreeBSD-14.0-CURRENT-powerpc-*.iso
>
> or
>
> qemu-system-ppc64 -cpu power9 -m 2G -enable-kvm \
> -machine pseries,cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,cap-ccf-assist=off \
> -boot d -vga none -nographic -cdrom FreeBSD-14.0-CURRENT-powerpc-*.iso
>
> Stop the boot at the prompt and use
>
> OK set radix_mmu=1
> OK boot
>
> [2] https://download.freebsd.org/ftp/snapshots/powerpc/powerpc64/ISO-IMAGES/14.0/
>
> Matheus Ferst (2):
> target/ppc: add LPCR[HR] to DisasContext and hflags
> target/ppc: Check privilege level based on PSR and LPCR[HR] in
> tlbie[l]
For some reason I didn't receive these 2 patches in my mailbox, just this cover
letter. I reviewed both using the qemu-devel archives.
Both patches:
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Thanks,
Daniel
>
> target/ppc/cpu.h | 1 +
> target/ppc/helper_regs.c | 3 +++
> target/ppc/translate.c | 28 +++++++++++++++++++++++-----
> 3 files changed, 27 insertions(+), 5 deletions(-)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 0/2] Require hypervisor privilege for tlbie[l] when PSR=0 and HR=1.
2021-09-17 11:47 [PATCH v2 0/2] Require hypervisor privilege for tlbie[l] when PSR=0 and HR=1 matheus.ferst
` (2 preceding siblings ...)
2021-09-17 14:17 ` [PATCH v2 0/2] Require hypervisor privilege for tlbie[l] when PSR=0 and HR=1 Daniel Henrique Barboza
@ 2021-09-20 7:40 ` David Gibson
3 siblings, 0 replies; 5+ messages in thread
From: David Gibson @ 2021-09-20 7:40 UTC (permalink / raw)
To: matheus.ferst
Cc: leandro.lupori, danielhb413, richard.henderson, qemu-devel,
groug, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 1963 bytes --]
On Fri, Sep 17, 2021 at 08:47:49AM -0300, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>
> While working on FreeBSD radix support, Leandro Lupori (CC'ed) noticed
> that the latest build still fails in KVM but works in TCG[1]. This
> difference occurs because the current implementation of "tlbiel" does
> not validate the instruction parameters and always check for supervisor
> privilege.
>
> This patch series partially address this problem by requiring hypervisor
> privilege for radix mode when PSR=0. The validation of other parameters
> can be done when we move storage control instructions to decodetree.
>
> [1] To reproduce the issue, grab an ISO from [2] run qemu as
>
> qemu-system-ppc64 -cpu power9 -m 2G \
> -machine pseries,cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,cap-ccf-assist=off \
> -boot d -vga none -nographic -cdrom FreeBSD-14.0-CURRENT-powerpc-*.iso
>
> or
>
> qemu-system-ppc64 -cpu power9 -m 2G -enable-kvm \
> -machine pseries,cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,cap-ccf-assist=off \
> -boot d -vga none -nographic -cdrom FreeBSD-14.0-CURRENT-powerpc-*.iso
>
> Stop the boot at the prompt and use
>
> OK set radix_mmu=1
> OK boot
>
> [2]
> https://download.freebsd.org/ftp/snapshots/powerpc/powerpc64/ISO-IMAGES/14.0/
Applied to ppc-for-6.2, thanks.
>
> Matheus Ferst (2):
> target/ppc: add LPCR[HR] to DisasContext and hflags
> target/ppc: Check privilege level based on PSR and LPCR[HR] in
> tlbie[l]
>
> target/ppc/cpu.h | 1 +
> target/ppc/helper_regs.c | 3 +++
> target/ppc/translate.c | 28 +++++++++++++++++++++++-----
> 3 files changed, 27 insertions(+), 5 deletions(-)
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread