Re: [PATCH v2] target/mips: Only update MVPControl.EVP bit if executed by master VPE

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Ping?

On 27/4/21 15:33, Philippe Mathieu-Daudé wrote:
> According to the 'MIPS MT Application-Specific Extension' manual:
> 
>    If the VPE executing the instruction is not a Master VPE,
>    with the MVP bit of the VPEConf0 register set, the EVP bit
>    is unchanged by the instruction.
> 
> Modify the DVPE/EVPE opcodes to only update the MVPControl.EVP bit
> if executed on a master VPE.
> 
> Reported-by: Hansni Bu <https://launchpad.net/%7Ehansni/+contactuser>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1926277
> Fixes: f249412c749 ("mips: Add MT halting and waking of VPEs")
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Supersedes: <20210427103555.112652-1-f4bug@amsat.org>
> v2: Check VPEConf0.MVP bit (hansni)
> ---
>   target/mips/cp0_helper.c | 32 ++++++++++++++++++--------------
>   1 file changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/target/mips/cp0_helper.c b/target/mips/cp0_helper.c
> index aae2af6eccc..d5f274f5cdf 100644
> --- a/target/mips/cp0_helper.c
> +++ b/target/mips/cp0_helper.c
> @@ -1635,12 +1635,14 @@ target_ulong helper_dvpe(CPUMIPSState *env)
>       CPUState *other_cs = first_cpu;
>       target_ulong prev = env->mvp->CP0_MVPControl;
>   
> -    CPU_FOREACH(other_cs) {
> -        MIPSCPU *other_cpu = MIPS_CPU(other_cs);
> -        /* Turn off all VPEs except the one executing the dvpe.  */
> -        if (&other_cpu->env != env) {
> -            other_cpu->env.mvp->CP0_MVPControl &= ~(1 << CP0MVPCo_EVP);
> -            mips_vpe_sleep(other_cpu);
> +    if (env->CP0_VPEConf0 & (1 << CP0VPEC0_MVP)) {
> +        CPU_FOREACH(other_cs) {
> +            MIPSCPU *other_cpu = MIPS_CPU(other_cs);
> +            /* Turn off all VPEs except the one executing the dvpe.  */
> +            if (&other_cpu->env != env) {
> +                other_cpu->env.mvp->CP0_MVPControl &= ~(1 << CP0MVPCo_EVP);
> +                mips_vpe_sleep(other_cpu);
> +            }
>           }
>       }
>       return prev;
> @@ -1651,15 +1653,17 @@ target_ulong helper_evpe(CPUMIPSState *env)
>       CPUState *other_cs = first_cpu;
>       target_ulong prev = env->mvp->CP0_MVPControl;
>   
> -    CPU_FOREACH(other_cs) {
> -        MIPSCPU *other_cpu = MIPS_CPU(other_cs);
> +    if (env->CP0_VPEConf0 & (1 << CP0VPEC0_MVP)) {
> +        CPU_FOREACH(other_cs) {
> +            MIPSCPU *other_cpu = MIPS_CPU(other_cs);
>   
> -        if (&other_cpu->env != env
> -            /* If the VPE is WFI, don't disturb its sleep.  */
> -            && !mips_vpe_is_wfi(other_cpu)) {
> -            /* Enable the VPE.  */
> -            other_cpu->env.mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
> -            mips_vpe_wake(other_cpu); /* And wake it up.  */
> +            if (&other_cpu->env != env
> +                /* If the VPE is WFI, don't disturb its sleep.  */
> +                && !mips_vpe_is_wfi(other_cpu)) {
> +                /* Enable the VPE.  */
> +                other_cpu->env.mvp->CP0_MVPControl |= (1 << CP0MVPCo_EVP);
> +                mips_vpe_wake(other_cpu); /* And wake it up.  */
> +            }
>           }
>       }
>       return prev;