From: Matheus Ferst <matheus.ferst@eldorado.org.br>
The code in linux-user/ppc/cpu_loop.c expects POWERPC_EXCP_PRIV
exception with error POWERPC_EXCP_PRIV_OPC or POWERPC_EXCP_PRIV_REG,
while POWERPC_EXCP_INVAL_SPR is expected in POWERPC_EXCP_INVAL
exceptions. This mismatch caused an EXCP_DUMP with the message "Unknown
privilege violation (03)", as seen in [1].
Fixes: 9b2fadda3e01 ("ppc: Rework generation of priv and inval interrupts")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/588
[1] https://gitlab.com/qemu-project/qemu/-/issues/588
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
Is there any case where throwing a PRIV/INVAL exception with a
INVAL/PRIV error makes sense? It seems wrong, but maybe I'm missing
something... especially with the HV_EMU to program check conversion.
Also, if this patch is correct, it seems that all invalid SPR access
would be nop or privilege exceptions. In this case, is
POWERPC_EXCP_INVAL_SPR still needed?
---
target/ppc/translate.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 40232201bb..abbc3a5bb9 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4827,11 +4827,11 @@ static inline void gen_op_mfspr(DisasContext *ctx)
*/
if (sprn & 0x10) {
if (ctx->pr) {
- gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
+ gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
}
} else {
if (ctx->pr || sprn == 0 || sprn == 4 || sprn == 5 || sprn == 6) {
- gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
+ gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_REG);
}
}
}
@@ -5014,11 +5014,11 @@ static void gen_mtspr(DisasContext *ctx)
*/
if (sprn & 0x10) {
if (ctx->pr) {
- gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
+ gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
}
} else {
if (ctx->pr || sprn == 0) {
- gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
+ gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_REG);
}
}
}
--
2.25.1
matheus.ferst@eldorado.org.br writes:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>
> The code in linux-user/ppc/cpu_loop.c expects POWERPC_EXCP_PRIV
> exception with error POWERPC_EXCP_PRIV_OPC or POWERPC_EXCP_PRIV_REG,
> while POWERPC_EXCP_INVAL_SPR is expected in POWERPC_EXCP_INVAL
> exceptions. This mismatch caused an EXCP_DUMP with the message "Unknown
> privilege violation (03)", as seen in [1].
>
> Fixes: 9b2fadda3e01 ("ppc: Rework generation of priv and inval interrupts")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/588
>
> [1] https://gitlab.com/qemu-project/qemu/-/issues/588
>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
This patch seems to do the right thing. So:
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Now, I'm not sure if the code around it does the right thing. =)
Specifically the else blocks (read_cb == NULL) and (write_cb ==
NULL). From _spr_register I understand that cb == NULL means this is not
a recognized SPR by this processor*. So in my mind 100% of them should be
invalid SPR exceptions.
The reserved SPRs should be registered in cpu_init and handled as
"known, but privileged" or "known, but noop". Maybe using SPR_NOACCESS
and/or a new SPR_NOOP. It might be a bit tricky because they have no names,
but that is an implementation detail.
* - there's some nuance here because of the system vs. linux-user build
time configuration so I'm not entirely sure.
Let's think a bit more about this. Everything seems to work just fine
the way it is so there's no rush. But I think this code could perhaps be
simplified and some of these assumptions handled at build time with
spr_register.
> ---
> Is there any case where throwing a PRIV/INVAL exception with a
> INVAL/PRIV error makes sense? It seems wrong, but maybe I'm missing
> something... especially with the HV_EMU to program check conversion.
>
> Also, if this patch is correct, it seems that all invalid SPR access
> would be nop or privilege exceptions. In this case, is
> POWERPC_EXCP_INVAL_SPR still needed?
I agree that as it stands this is not needed. But we might want to bring
it back given the points I mentioned above. So let's keep it for now.
> ---
> target/ppc/translate.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 40232201bb..abbc3a5bb9 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4827,11 +4827,11 @@ static inline void gen_op_mfspr(DisasContext *ctx)
> */
> if (sprn & 0x10) {
> if (ctx->pr) {
> - gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> + gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
> }
> } else {
> if (ctx->pr || sprn == 0 || sprn == 4 || sprn == 5 || sprn == 6) {
> - gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> + gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_REG);
> }
> }
> }
> @@ -5014,11 +5014,11 @@ static void gen_mtspr(DisasContext *ctx)
> */
> if (sprn & 0x10) {
> if (ctx->pr) {
> - gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> + gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
> }
> } else {
> if (ctx->pr || sprn == 0) {
> - gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> + gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_REG);
> }
> }
> }
Le 13/01/2022 à 18:04, matheus.ferst@eldorado.org.br a écrit :
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>
> The code in linux-user/ppc/cpu_loop.c expects POWERPC_EXCP_PRIV
> exception with error POWERPC_EXCP_PRIV_OPC or POWERPC_EXCP_PRIV_REG,
> while POWERPC_EXCP_INVAL_SPR is expected in POWERPC_EXCP_INVAL
> exceptions. This mismatch caused an EXCP_DUMP with the message "Unknown
> privilege violation (03)", as seen in [1].
>
> Fixes: 9b2fadda3e01 ("ppc: Rework generation of priv and inval interrupts")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/588
>
> [1] https://gitlab.com/qemu-project/qemu/-/issues/588
>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
> Is there any case where throwing a PRIV/INVAL exception with a
> INVAL/PRIV error makes sense? It seems wrong, but maybe I'm missing
> something... especially with the HV_EMU to program check conversion.
>
> Also, if this patch is correct, it seems that all invalid SPR access
> would be nop or privilege exceptions. In this case, is
> POWERPC_EXCP_INVAL_SPR still needed?
> ---
> target/ppc/translate.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 40232201bb..abbc3a5bb9 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4827,11 +4827,11 @@ static inline void gen_op_mfspr(DisasContext *ctx)
> */
> if (sprn & 0x10) {
> if (ctx->pr) {
> - gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> + gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
> }
> } else {
> if (ctx->pr || sprn == 0 || sprn == 4 || sprn == 5 || sprn == 6) {
> - gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> + gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_REG);
> }
> }
> }
> @@ -5014,11 +5014,11 @@ static void gen_mtspr(DisasContext *ctx)
> */
> if (sprn & 0x10) {
> if (ctx->pr) {
> - gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> + gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
> }
> } else {
> if (ctx->pr || sprn == 0) {
> - gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> + gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_REG);
> }
> }
> }
It seems logic to emit a POWERPC_EXCP_PRIV_XXX exception with gen_priv_exception() (POWERPC_EXCP_PRIV).
Moreover in line above we have a gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG) if the register
cannot be read (SPR_NOACCESS).
But in helper_load_dcr() we have POWERPC_EXCP_PRIV_REG with POWERPC_EXCP_INVAL (whereas in the
helper_store_dcr() function we have POWERPC_EXCP_INVAL with POWERPC_EXCP_INVAL_INVAL).
It looks like another bug.
and in gen_slbfee() we have also a POWERPC_EXCP_PRIV_REG with gen_inval_exception()
(POWERPC_EXCP_INVAL).
What is interesting is gen_inval_exception() uses POWERPC_EXCP_HV_EMU that could make thinking we
could try to emulate the operation (for KVM PR, for instance).
Thanks,
Laurent
On 04/03/2022 11:42, Laurent Vivier wrote:
> Le 13/01/2022 à 18:04, matheus.ferst@eldorado.org.br a écrit :
>> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>>
>> The code in linux-user/ppc/cpu_loop.c expects POWERPC_EXCP_PRIV
>> exception with error POWERPC_EXCP_PRIV_OPC or POWERPC_EXCP_PRIV_REG,
>> while POWERPC_EXCP_INVAL_SPR is expected in POWERPC_EXCP_INVAL
>> exceptions. This mismatch caused an EXCP_DUMP with the message "Unknown
>> privilege violation (03)", as seen in [1].
>>
>> Fixes: 9b2fadda3e01 ("ppc: Rework generation of priv and inval
>> interrupts")
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/588
>>
>> [1] https://gitlab.com/qemu-project/qemu/-/issues/588
>>
>> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
>> ---
>> Is there any case where throwing a PRIV/INVAL exception with a
>> INVAL/PRIV error makes sense? It seems wrong, but maybe I'm missing
>> something... especially with the HV_EMU to program check conversion.
>>
>> Also, if this patch is correct, it seems that all invalid SPR access
>> would be nop or privilege exceptions. In this case, is
>> POWERPC_EXCP_INVAL_SPR still needed?
>> ---
>> target/ppc/translate.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index 40232201bb..abbc3a5bb9 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -4827,11 +4827,11 @@ static inline void gen_op_mfspr(DisasContext
>> *ctx)
>> */
>> if (sprn & 0x10) {
>> if (ctx->pr) {
>> - gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
>> + gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
>> }
>> } else {
>> if (ctx->pr || sprn == 0 || sprn == 4 || sprn == 5 ||
>> sprn == 6) {
>> - gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
>> + gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_REG);
>> }
>> }
>> }
>> @@ -5014,11 +5014,11 @@ static void gen_mtspr(DisasContext *ctx)
>> */
>> if (sprn & 0x10) {
>> if (ctx->pr) {
>> - gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
>> + gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
>> }
>> } else {
>> if (ctx->pr || sprn == 0) {
>> - gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
>> + gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_REG);
>> }
>> }
>> }
>
> It seems logic to emit a POWERPC_EXCP_PRIV_XXX exception with
> gen_priv_exception() (POWERPC_EXCP_PRIV).
>
> Moreover in line above we have a gen_priv_exception(ctx,
> POWERPC_EXCP_PRIV_REG) if the register
> cannot be read (SPR_NOACCESS).
>
> But in helper_load_dcr() we have POWERPC_EXCP_PRIV_REG with
> POWERPC_EXCP_INVAL (whereas in the
> helper_store_dcr() function we have POWERPC_EXCP_INVAL with
> POWERPC_EXCP_INVAL_INVAL).
> It looks like another bug.
The instructions that could invoke these helpers in user-mode (mfdcrux
and mtdcrux) are behind PPC_DCRUX, and no CPU has this insns_flag, so I
guess the code is wrong, but we cannot hit the bug in linux-user.
Similarly, we have gen_hvpriv_exception with POWERPC_EXCP_INVAL_SPR in
spr_groupA_write_allowed and gen_inval_exception with
POWERPC_EXCP_PRIV_REG in spr_write_excp_vector, but both are behind
!defined(CONFIG_USER_ONLY).
> and in gen_slbfee() we have also a POWERPC_EXCP_PRIV_REG with
> gen_inval_exception()
> (POWERPC_EXCP_INVAL).
This one is easier to test. Looking at si_code:
void sigill_handle(int sig, siginfo_t *si, void *ucontext)
{
_exit(si->si_code);
}
int main(void)
{
struct sigaction sa = {.sa_sigaction = sigill_handle, .sa_flags =
SA_SIGINFO};
uint64_t t = 0, b = 0;
sigaction(SIGILL, &sa, NULL);
asm("slbfee. %0, %1" : "=r" (t) : "r" (b));
return 0;
}
We have:
$ ./slbee; echo $?
5 # ILL_PRVOPC
$ ./qemu-ppc64 ./slbfee; echo $?
2 # ILL_ILLOPN
So I think we should be using gen_priv_exception or gen_hvpriv_exception
with POWERPC_EXCP_PRIV_OPC.
> What is interesting is gen_inval_exception() uses POWERPC_EXCP_HV_EMU
> that could make thinking we
> could try to emulate the operation (for KVM PR, for instance).
IIUC, we should use gen_hvpriv_exception in those cases, so we have
POWERPC_EXCP_HV_EMU with POWERPC_EXCP_PRIV | something.
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
© 2016 - 2026 Red Hat, Inc.