diff mbox series

[1/2] linux-user/s390x: Fix single-stepping SVC

Message ID 20230510230213.330134-2-iii@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series linux-user/s390x: Fix single-stepping SVC | expand

Commit Message

Ilya Leoshkevich May 10, 2023, 11:02 p.m. UTC
Currently single-stepping SVC executes two instructions. The reason is
that EXCP_DEBUG for the SVC instruction itself is masked by EXCP_SVC.
Fix by re-raising EXCP_DEBUG.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 linux-user/s390x/cpu_loop.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Michael Tokarev May 11, 2023, 10:55 a.m. UTC | #1
11.05.2023 02:02, Ilya Leoshkevich wrote:
> Currently single-stepping SVC executes two instructions. The reason is
> that EXCP_DEBUG for the SVC instruction itself is masked by EXCP_SVC.
> Fix by re-raising EXCP_DEBUG.

Is it a -stable material?

/mjt
Ilya Leoshkevich May 11, 2023, 11:20 a.m. UTC | #2
On Thu, 2023-05-11 at 13:55 +0300, Michael Tokarev wrote:
> 11.05.2023 02:02, Ilya Leoshkevich wrote:
> > Currently single-stepping SVC executes two instructions. The reason
> > is
> > that EXCP_DEBUG for the SVC instruction itself is masked by
> > EXCP_SVC.
> > Fix by re-raising EXCP_DEBUG.
> 
> Is it a -stable material?
> 
> /mjt

While I would personally love to see this in -stable, I don't think it
fits the official criteria - it's not a security fix and it's not a
regression.
Michael Tokarev May 11, 2023, 12:32 p.m. UTC | #3
11.05.2023 14:20, Ilya Leoshkevich wrote:
> On Thu, 2023-05-11 at 13:55 +0300, Michael Tokarev wrote:

>> Is it a -stable material?

> While I would personally love to see this in -stable, I don't think it
> fits the official criteria - it's not a security fix and it's not a
> regression.

I'm not sure I follow. It's definitely okay to include a bug fix into
-stable, this has been done countless times in the past..

/mjt
Ilya Leoshkevich May 11, 2023, 1:45 p.m. UTC | #4
On Thu, 2023-05-11 at 15:32 +0300, Michael Tokarev wrote:
> 11.05.2023 14:20, Ilya Leoshkevich wrote:
> > On Thu, 2023-05-11 at 13:55 +0300, Michael Tokarev wrote:
> 
> > > Is it a -stable material?
> 
> > While I would personally love to see this in -stable, I don't think
> > it
> > fits the official criteria - it's not a security fix and it's not a
> > regression.
> 
> I'm not sure I follow. It's definitely okay to include a bug fix into
> -stable, this has been done countless times in the past..
> 
> /mjt

Okay, then let's include it into -stable.

It's just that I'm not too familiar with the QEMU -stable process, so
I read [1], and it sounded quite strict.

[1] https://www.qemu.org/docs/master/devel/stable-process.html
Michael Tokarev May 11, 2023, 1:50 p.m. UTC | #5
11.05.2023 16:45, Ilya Leoshkevich wrote:
>> 11.05.2023 14:20, Ilya Leoshkevich wrote:

>>> While I would personally love to see this in -stable, I don't think it
>>> fits the official criteria - it's not a security fix and it's not a
>>> regression.

> Okay, then let's include it into -stable.
> 
> It's just that I'm not too familiar with the QEMU -stable process, so
> I read [1], and it sounded quite strict.
> 
> [1] https://www.qemu.org/docs/master/devel/stable-process.html

The text there reads:

    If you think the patch would be important for users of
    the current release (or for a distribution picking fixes),
    it is usually a good candidate for stable.

:)

Please Cc: qemu-stable@nongnu.org the next time you think
something is good to have there.

/mjt
diff mbox series

Patch

diff --git a/linux-user/s390x/cpu_loop.c b/linux-user/s390x/cpu_loop.c
index 285bc60071a..8b7ac2879ef 100644
--- a/linux-user/s390x/cpu_loop.c
+++ b/linux-user/s390x/cpu_loop.c
@@ -86,6 +86,15 @@  void cpu_loop(CPUS390XState *env)
             } else if (ret != -QEMU_ESIGRETURN) {
                 env->regs[2] = ret;
             }
+
+            if (unlikely(cs->singlestep_enabled)) {
+                /*
+                 * cpu_tb_exec() did not raise EXCP_DEBUG, because it has seen
+                 * that EXCP_SVC was already pending.
+                 */
+                cs->exception_index = EXCP_DEBUG;
+            }
+
             break;
 
         case EXCP_DEBUG: