1. Patchew
  2. QEMU
  3. View series

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

Philippe Mathieu-Daudé posted 1 patch 4 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210427133343.159718-1-f4bug@amsat.org
Test checkpatch passed
Maintainers: "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <arikalo@gmail.com>
target/mips/cp0_helper.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
[PATCH v2] target/mips: Only update MVPControl.EVP bit if executed by master VPE
Posted by Philippe Mathieu-Daudé 4 years, 9 months ago 
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;
-- 
2.26.3
Re: [PATCH v2] target/mips: Only update MVPControl.EVP bit if executed by master VPE
Posted by Philippe Mathieu-Daudé 6 months, 2 weeks ago 
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(-)

Patch queued.
[Bug 1926277] Re: [PATCH v2] target/mips: Only update MVPControl.EVP bit if executed by master VPE
Posted by Jiaxun Yang 6 months, 4 weeks ago 
在2021年4月27日周二 下午2:33，Philippe Mathieu-Daudé写道：
> 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>

Confirmed with uarch behaviour.

Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>

Thanks
Jiaxun


> ---
> 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;
> -- 
> 2.26.3

-- 
- Jiaxun

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1926277

Title:
  MIPS MT dvpe does not regard VPEConf0.MVP

Status in QEMU:
  Invalid

Bug description:
  Hi,

  According to MIPS32® Architecture for Programmers VolumeIV-f: The
  MIPS® MT Application-Specific Extension to the MIPS32® Architecture,
  for instruction: dvpe, evpe:

  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.

  The pseudo code is:

  data ←  MVPControl
  GPR[rt] ←  data
  if(VPEConf0.MVP = 1) then
    MVPControl.EVP ←  sc
  endif

  However the helper functions of dvpe, evpe does not regard the
  VPEConf0.MVP bit, namely, it does not check if the VPE is a master
  VPE. Code is copied below as:

  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);
          }
      }
      return prev;
  }

  Is this a bug?

  QEMU head commit: 0cef06d18762374c94eb4d511717a4735d668a24 is checked.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1926277/+subscriptions
Re: [PATCH v2] target/mips: Only update MVPControl.EVP bit if executed by master VPE
Posted by Philippe Mathieu-Daudé 6 months, 4 weeks ago 
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;

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.