Re: [PATCH] target/ppc: Fix sc instruction handling of LEV field

[Harsh Prateek Bora <harshpb@linux.ibm.com>]

On 6/21/23 16:39, Nicholas Piggin wrote:
> The top bits of the LEV field of the sc instruction are to be treated as
> as a reserved field rather than a reserved value, meaning LEV is
> effectively the bottom bit. LEV=0xF should be treated as LEV=1 and be
> a hypercall, for example.
> 
> This changes the instruction execution to just set lev from the low bit
> of the field. Processors which don't support the LEV field will continue
> to ignore it.
> 
> ISA v3.1 defines LEV to be 2 bits, in order to add the 'sc 2' ultracall
> instruction. TCG does not support Ultravisor, so don't worry about
> that bit.
> 
> Suggested-by: "Harsh Prateek Bora" <harshpb@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> This should probably go ahead of the ISA 3.1 LEV in SRR1 patch. I
> don't think they need to be backported to stable though, have not
> caused any real problems.
> 
> Thanks to Harsh for spotting it.
> 
> Thanks,
> Nick
> 
>   target/ppc/translate.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 15a00bd4fa..3c62f9188a 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4424,7 +4424,12 @@ static void gen_sc(DisasContext *ctx)
>   {
>       uint32_t lev;
>   
> -    lev = (ctx->opcode >> 5) & 0x7F;
> +    /*
> +     * LEV is a 7-bit field, but the top 6 bits are treated as a reserved
> +     * field (i.e., ignored). ISA v3.1 changes that to 5 bits, but that is
> +     * for Ultravisor which TCG does not support, so just ignore the top 6.
> +     */
> +    lev = (ctx->opcode >> 5) & 0x1;

should this change be applied to gen_scv() defined next to it as well ?

Otherwise,
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

>       gen_exception_err(ctx, POWERPC_SYSCALL, lev);
>   }
>