diff mbox series

[2/3] gdbstub: implement NOIRQ support for single step on KVM

Message ID 20210914155214.105415-3-mlevitsk@redhat.com
State New
Headers show
Series KVM: qemu patches for few KVM features I developed | expand

Commit Message

Maxim Levitsky Sept. 14, 2021, 3:52 p.m. UTC
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 accel/kvm/kvm-all.c  | 25 ++++++++++++++++++
 gdbstub.c            | 60 ++++++++++++++++++++++++++++++++++++--------
 include/sysemu/kvm.h | 13 ++++++++++
 3 files changed, 88 insertions(+), 10 deletions(-)

Comments

Alex Bennée Oct. 13, 2021, 3:50 p.m. UTC | #1
Maxim Levitsky <mlevitsk@redhat.com> writes:

> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  accel/kvm/kvm-all.c  | 25 ++++++++++++++++++
>  gdbstub.c            | 60 ++++++++++++++++++++++++++++++++++++--------
>  include/sysemu/kvm.h | 13 ++++++++++
>  3 files changed, 88 insertions(+), 10 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 6b187e9c96..e141260796 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -169,6 +169,8 @@ bool kvm_vm_attributes_allowed;
>  bool kvm_direct_msi_allowed;
>  bool kvm_ioeventfd_any_length_allowed;
>  bool kvm_msi_use_devid;
> +bool kvm_has_guest_debug;
> +int kvm_sstep_flags;
>  static bool kvm_immediate_exit;
>  static hwaddr kvm_max_slot_size = ~0;
>  
> @@ -2559,6 +2561,25 @@ static int kvm_init(MachineState *ms)
>      kvm_sregs2 =
>          (kvm_check_extension(s, KVM_CAP_SREGS2) > 0);
>  
> +    kvm_has_guest_debug =
> +        (kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG) > 0);
> +
> +    kvm_sstep_flags = 0;
> +
> +    if (kvm_has_guest_debug) {
> +        /* Assume that single stepping is supported */
> +        kvm_sstep_flags = SSTEP_ENABLE;
> +
> +        int guest_debug_flags =
> +            kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG2);
> +
> +        if (guest_debug_flags > 0) {
> +            if (guest_debug_flags & KVM_GUESTDBG_BLOCKIRQ) {
> +                kvm_sstep_flags |= SSTEP_NOIRQ;
> +            }
> +        }
> +    }
> +
>      kvm_state = s;
>  
>      ret = kvm_arch_init(ms, s);
> @@ -3188,6 +3209,10 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
>  
>      if (cpu->singlestep_enabled) {
>          data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
> +
> +        if (cpu->singlestep_enabled & SSTEP_NOIRQ) {
> +            data.dbg.control |= KVM_GUESTDBG_BLOCKIRQ;
> +        }
>      }
>      kvm_arch_update_guest_debug(cpu, &data.dbg);
>  
> diff --git a/gdbstub.c b/gdbstub.c
> index 5d8e6ae3cd..48bb803bae 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -368,12 +368,11 @@ typedef struct GDBState {
>      gdb_syscall_complete_cb current_syscall_cb;
>      GString *str_buf;
>      GByteArray *mem_buf;
> +    int sstep_flags;
> +    int supported_sstep_flags;
>  } GDBState;
>  
> -/* By default use no IRQs and no timers while single stepping so as to
> - * make single stepping like an ICE HW step.
> - */
> -static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
> +static GDBState gdbserver_state;
>  
>  /* Retrieves flags for single step mode. */
>  static int get_sstep_flags(void)
> @@ -385,11 +384,10 @@ static int get_sstep_flags(void)
>      if (replay_mode != REPLAY_MODE_NONE) {
>          return SSTEP_ENABLE;
>      } else {
> -        return sstep_flags;
> +        return gdbserver_state.sstep_flags;
>      }
>  }
>  
> -static GDBState gdbserver_state;
>  
>  static void init_gdbserver_state(void)
>  {
> @@ -399,6 +397,23 @@ static void init_gdbserver_state(void)
>      gdbserver_state.str_buf = g_string_new(NULL);
>      gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH);
>      gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH + 4);
> +
> +
> +    if (kvm_enabled()) {
> +        gdbserver_state.supported_sstep_flags = kvm_get_supported_sstep_flags();
> +    } else {
> +        gdbserver_state.supported_sstep_flags =
> +            SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER;
> +    }

This fails to build:

o -c ../../gdbstub.c
../../gdbstub.c: In function ‘init_gdbserver_state’:
../../gdbstub.c:403:49: error: implicit declaration of function ‘kvm_get_supported_sstep_flags’ [-Werror=implicit-function-declaration]
  403 |         gdbserver_state.supported_sstep_flags = kvm_get_supported_sstep_flags();
      |                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../gdbstub.c:403:49: error: nested extern declaration of ‘kvm_get_supported_sstep_flags’ [-Werror=nested-externs]
../../gdbstub.c: In function ‘gdbserver_start’:
../../gdbstub.c:3531:27: error: implicit declaration of function ‘kvm_supports_guest_debug’; did you mean ‘kvm_update_guest_debug’? [-Werror=implicit-function-declaration]
 3531 |     if (kvm_enabled() && !kvm_supports_guest_debug()) {
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~
      |                           kvm_update_guest_debug
../../gdbstub.c:3531:27: error: nested extern declaration of ‘kvm_supports_guest_debug’ [-Werror=nested-externs]
cc1: all warnings being treated as errors

In fact looking back I can see I mentioned this last time:

  Subject: Re: [PATCH 2/2] gdbstub: implement NOIRQ support for single step on
   KVM, when kvm's KVM_GUESTDBG_BLOCKIRQ debug flag is supported.
  Date: Mon, 19 Apr 2021 17:29:25 +0100
  In-reply-to: <20210401144152.1031282-3-mlevitsk@redhat.com>
  Message-ID: <871rb69qqk.fsf@linaro.org>

Please in future could you include a revision number for your spin and
mention bellow the --- what changes have been made since the last
posting.
Maxim Levitsky Oct. 13, 2021, 7:52 p.m. UTC | #2
On Wed, 2021-10-13 at 16:50 +0100, Alex Bennée wrote:
> Maxim Levitsky <mlevitsk@redhat.com> writes:
> 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  accel/kvm/kvm-all.c  | 25 ++++++++++++++++++
> >  gdbstub.c            | 60 ++++++++++++++++++++++++++++++++++++--------
> >  include/sysemu/kvm.h | 13 ++++++++++
> >  3 files changed, 88 insertions(+), 10 deletions(-)
> > 
> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > index 6b187e9c96..e141260796 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -169,6 +169,8 @@ bool kvm_vm_attributes_allowed;
> >  bool kvm_direct_msi_allowed;
> >  bool kvm_ioeventfd_any_length_allowed;
> >  bool kvm_msi_use_devid;
> > +bool kvm_has_guest_debug;
> > +int kvm_sstep_flags;
> >  static bool kvm_immediate_exit;
> >  static hwaddr kvm_max_slot_size = ~0;
> >  
> > @@ -2559,6 +2561,25 @@ static int kvm_init(MachineState *ms)
> >      kvm_sregs2 =
> >          (kvm_check_extension(s, KVM_CAP_SREGS2) > 0);
> >  
> > +    kvm_has_guest_debug =
> > +        (kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG) > 0);
> > +
> > +    kvm_sstep_flags = 0;
> > +
> > +    if (kvm_has_guest_debug) {
> > +        /* Assume that single stepping is supported */
> > +        kvm_sstep_flags = SSTEP_ENABLE;
> > +
> > +        int guest_debug_flags =
> > +            kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG2);
> > +
> > +        if (guest_debug_flags > 0) {
> > +            if (guest_debug_flags & KVM_GUESTDBG_BLOCKIRQ) {
> > +                kvm_sstep_flags |= SSTEP_NOIRQ;
> > +            }
> > +        }
> > +    }
> > +
> >      kvm_state = s;
> >  
> >      ret = kvm_arch_init(ms, s);
> > @@ -3188,6 +3209,10 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
> >  
> >      if (cpu->singlestep_enabled) {
> >          data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
> > +
> > +        if (cpu->singlestep_enabled & SSTEP_NOIRQ) {
> > +            data.dbg.control |= KVM_GUESTDBG_BLOCKIRQ;
> > +        }
> >      }
> >      kvm_arch_update_guest_debug(cpu, &data.dbg);
> >  
> > diff --git a/gdbstub.c b/gdbstub.c
> > index 5d8e6ae3cd..48bb803bae 100644
> > --- a/gdbstub.c
> > +++ b/gdbstub.c
> > @@ -368,12 +368,11 @@ typedef struct GDBState {
> >      gdb_syscall_complete_cb current_syscall_cb;
> >      GString *str_buf;
> >      GByteArray *mem_buf;
> > +    int sstep_flags;
> > +    int supported_sstep_flags;
> >  } GDBState;
> >  
> > -/* By default use no IRQs and no timers while single stepping so as to
> > - * make single stepping like an ICE HW step.
> > - */
> > -static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
> > +static GDBState gdbserver_state;
> >  
> >  /* Retrieves flags for single step mode. */
> >  static int get_sstep_flags(void)
> > @@ -385,11 +384,10 @@ static int get_sstep_flags(void)
> >      if (replay_mode != REPLAY_MODE_NONE) {
> >          return SSTEP_ENABLE;
> >      } else {
> > -        return sstep_flags;
> > +        return gdbserver_state.sstep_flags;
> >      }
> >  }
> >  
> > -static GDBState gdbserver_state;
> >  
> >  static void init_gdbserver_state(void)
> >  {
> > @@ -399,6 +397,23 @@ static void init_gdbserver_state(void)
> >      gdbserver_state.str_buf = g_string_new(NULL);
> >      gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH);
> >      gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH + 4);
> > +
> > +
> > +    if (kvm_enabled()) {
> > +        gdbserver_state.supported_sstep_flags = kvm_get_supported_sstep_flags();
> > +    } else {
> > +        gdbserver_state.supported_sstep_flags =
> > +            SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER;
> > +    }
> 
> This fails to build:
> 
> o -c ../../gdbstub.c
> ../../gdbstub.c: In function ‘init_gdbserver_state’:
> ../../gdbstub.c:403:49: error: implicit declaration of function ‘kvm_get_supported_sstep_flags’ [-Werror=implicit-function-declaration]
>   403 |         gdbserver_state.supported_sstep_flags = kvm_get_supported_sstep_flags();
>       |                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../../gdbstub.c:403:49: error: nested extern declaration of ‘kvm_get_supported_sstep_flags’ [-Werror=nested-externs]
> ../../gdbstub.c: In function ‘gdbserver_start’:
> ../../gdbstub.c:3531:27: error: implicit declaration of function ‘kvm_supports_guest_debug’; did you mean ‘kvm_update_guest_debug’? [-Werror=implicit-function-declaration]
>  3531 |     if (kvm_enabled() && !kvm_supports_guest_debug()) {
>       |                           ^~~~~~~~~~~~~~~~~~~~~~~~
>       |                           kvm_update_guest_debug
> ../../gdbstub.c:3531:27: error: nested extern declaration of ‘kvm_supports_guest_debug’ [-Werror=nested-externs]
> cc1: all warnings being treated as errors
> 
> In fact looking back I can see I mentioned this last time:
> 
>   Subject: Re: [PATCH 2/2] gdbstub: implement NOIRQ support for single step on
>    KVM, when kvm's KVM_GUESTDBG_BLOCKIRQ debug flag is supported.
>   Date: Mon, 19 Apr 2021 17:29:25 +0100
>   In-reply-to: <20210401144152.1031282-3-mlevitsk@redhat.com>
>   Message-ID: <871rb69qqk.fsf@linaro.org>
> 
> Please in future could you include a revision number for your spin and
> mention bellow the --- what changes have been made since the last
> posting.

You mean it fails to build without KVM? I swear I tested build with TTG only after you mentioned this 
(or as it seems I only tried to).

Could you give me the ./configure parameters you used?

Sorry for this!
Best regards,
	Maxim Levitsky

> 
>
Alex Bennée Oct. 14, 2021, 1:08 p.m. UTC | #3
Maxim Levitsky <mlevitsk@redhat.com> writes:

> On Wed, 2021-10-13 at 16:50 +0100, Alex Bennée wrote:
>> Maxim Levitsky <mlevitsk@redhat.com> writes:
>> 
>> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
>> > ---
>> >  accel/kvm/kvm-all.c  | 25 ++++++++++++++++++
>> >  gdbstub.c            | 60 ++++++++++++++++++++++++++++++++++++--------
>> >  include/sysemu/kvm.h | 13 ++++++++++
>> >  3 files changed, 88 insertions(+), 10 deletions(-)
>> > 
>> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>> > index 6b187e9c96..e141260796 100644
>> > --- a/accel/kvm/kvm-all.c
>> > +++ b/accel/kvm/kvm-all.c
>> > @@ -169,6 +169,8 @@ bool kvm_vm_attributes_allowed;
>> >  bool kvm_direct_msi_allowed;
>> >  bool kvm_ioeventfd_any_length_allowed;
>> >  bool kvm_msi_use_devid;
>> > +bool kvm_has_guest_debug;
>> > +int kvm_sstep_flags;
>> >  static bool kvm_immediate_exit;
>> >  static hwaddr kvm_max_slot_size = ~0;
>> >  
>> > @@ -2559,6 +2561,25 @@ static int kvm_init(MachineState *ms)
>> >      kvm_sregs2 =
>> >          (kvm_check_extension(s, KVM_CAP_SREGS2) > 0);
>> >  
>> > +    kvm_has_guest_debug =
>> > +        (kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG) > 0);
>> > +
>> > +    kvm_sstep_flags = 0;
>> > +
>> > +    if (kvm_has_guest_debug) {
>> > +        /* Assume that single stepping is supported */
>> > +        kvm_sstep_flags = SSTEP_ENABLE;
>> > +
>> > +        int guest_debug_flags =
>> > +            kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG2);
>> > +
>> > +        if (guest_debug_flags > 0) {
>> > +            if (guest_debug_flags & KVM_GUESTDBG_BLOCKIRQ) {
>> > +                kvm_sstep_flags |= SSTEP_NOIRQ;
>> > +            }
>> > +        }
>> > +    }
>> > +
>> >      kvm_state = s;
>> >  
>> >      ret = kvm_arch_init(ms, s);
>> > @@ -3188,6 +3209,10 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
>> >  
>> >      if (cpu->singlestep_enabled) {
>> >          data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
>> > +
>> > +        if (cpu->singlestep_enabled & SSTEP_NOIRQ) {
>> > +            data.dbg.control |= KVM_GUESTDBG_BLOCKIRQ;
>> > +        }
>> >      }
>> >      kvm_arch_update_guest_debug(cpu, &data.dbg);
>> >  
>> > diff --git a/gdbstub.c b/gdbstub.c
>> > index 5d8e6ae3cd..48bb803bae 100644
>> > --- a/gdbstub.c
>> > +++ b/gdbstub.c
>> > @@ -368,12 +368,11 @@ typedef struct GDBState {
>> >      gdb_syscall_complete_cb current_syscall_cb;
>> >      GString *str_buf;
>> >      GByteArray *mem_buf;
>> > +    int sstep_flags;
>> > +    int supported_sstep_flags;
>> >  } GDBState;
>> >  
>> > -/* By default use no IRQs and no timers while single stepping so as to
>> > - * make single stepping like an ICE HW step.
>> > - */
>> > -static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
>> > +static GDBState gdbserver_state;
>> >  
>> >  /* Retrieves flags for single step mode. */
>> >  static int get_sstep_flags(void)
>> > @@ -385,11 +384,10 @@ static int get_sstep_flags(void)
>> >      if (replay_mode != REPLAY_MODE_NONE) {
>> >          return SSTEP_ENABLE;
>> >      } else {
>> > -        return sstep_flags;
>> > +        return gdbserver_state.sstep_flags;
>> >      }
>> >  }
>> >  
>> > -static GDBState gdbserver_state;
>> >  
>> >  static void init_gdbserver_state(void)
>> >  {
>> > @@ -399,6 +397,23 @@ static void init_gdbserver_state(void)
>> >      gdbserver_state.str_buf = g_string_new(NULL);
>> >      gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH);
>> >      gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH + 4);
>> > +
>> > +
>> > +    if (kvm_enabled()) {
>> > +        gdbserver_state.supported_sstep_flags = kvm_get_supported_sstep_flags();
>> > +    } else {
>> > +        gdbserver_state.supported_sstep_flags =
>> > +            SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER;
>> > +    }
>> 
>> This fails to build:
>> 
>> o -c ../../gdbstub.c
>> ../../gdbstub.c: In function ‘init_gdbserver_state’:
>> ../../gdbstub.c:403:49: error: implicit declaration of function ‘kvm_get_supported_sstep_flags’ [-Werror=implicit-function-declaration]
>>   403 |         gdbserver_state.supported_sstep_flags = kvm_get_supported_sstep_flags();
>>       |                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ../../gdbstub.c:403:49: error: nested extern declaration of ‘kvm_get_supported_sstep_flags’ [-Werror=nested-externs]
>> ../../gdbstub.c: In function ‘gdbserver_start’:
>> ../../gdbstub.c:3531:27: error: implicit declaration of function
>> ‘kvm_supports_guest_debug’; did you mean ‘kvm_update_guest_debug’?
>> [-Werror=implicit-function-declaration]
>>  3531 |     if (kvm_enabled() && !kvm_supports_guest_debug()) {
>>       |                           ^~~~~~~~~~~~~~~~~~~~~~~~
>>       |                           kvm_update_guest_debug
>> ../../gdbstub.c:3531:27: error: nested extern declaration of ‘kvm_supports_guest_debug’ [-Werror=nested-externs]
>> cc1: all warnings being treated as errors
>> 
>> In fact looking back I can see I mentioned this last time:
>> 
>>   Subject: Re: [PATCH 2/2] gdbstub: implement NOIRQ support for single step on
>>    KVM, when kvm's KVM_GUESTDBG_BLOCKIRQ debug flag is supported.
>>   Date: Mon, 19 Apr 2021 17:29:25 +0100
>>   In-reply-to: <20210401144152.1031282-3-mlevitsk@redhat.com>
>>   Message-ID: <871rb69qqk.fsf@linaro.org>
>> 
>> Please in future could you include a revision number for your spin and
>> mention bellow the --- what changes have been made since the last
>> posting.
>
> You mean it fails to build without KVM? I swear I tested build with TTG only after you mentioned this 
> (or as it seems I only tried to).
>
> Could you give me the ./configure parameters you used?

That's with the standard parameters on an x86 host (which will disable
KVM and be TCG only for ARM). I suspect you could get the same results
with --disable-kvm on a native ARM system but I haven't tested that.

>
> Sorry for this!
> Best regards,
> 	Maxim Levitsky
>
>> 
>>
diff mbox series

Patch

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 6b187e9c96..e141260796 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -169,6 +169,8 @@  bool kvm_vm_attributes_allowed;
 bool kvm_direct_msi_allowed;
 bool kvm_ioeventfd_any_length_allowed;
 bool kvm_msi_use_devid;
+bool kvm_has_guest_debug;
+int kvm_sstep_flags;
 static bool kvm_immediate_exit;
 static hwaddr kvm_max_slot_size = ~0;
 
@@ -2559,6 +2561,25 @@  static int kvm_init(MachineState *ms)
     kvm_sregs2 =
         (kvm_check_extension(s, KVM_CAP_SREGS2) > 0);
 
+    kvm_has_guest_debug =
+        (kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG) > 0);
+
+    kvm_sstep_flags = 0;
+
+    if (kvm_has_guest_debug) {
+        /* Assume that single stepping is supported */
+        kvm_sstep_flags = SSTEP_ENABLE;
+
+        int guest_debug_flags =
+            kvm_check_extension(s, KVM_CAP_SET_GUEST_DEBUG2);
+
+        if (guest_debug_flags > 0) {
+            if (guest_debug_flags & KVM_GUESTDBG_BLOCKIRQ) {
+                kvm_sstep_flags |= SSTEP_NOIRQ;
+            }
+        }
+    }
+
     kvm_state = s;
 
     ret = kvm_arch_init(ms, s);
@@ -3188,6 +3209,10 @@  int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
 
     if (cpu->singlestep_enabled) {
         data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
+
+        if (cpu->singlestep_enabled & SSTEP_NOIRQ) {
+            data.dbg.control |= KVM_GUESTDBG_BLOCKIRQ;
+        }
     }
     kvm_arch_update_guest_debug(cpu, &data.dbg);
 
diff --git a/gdbstub.c b/gdbstub.c
index 5d8e6ae3cd..48bb803bae 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -368,12 +368,11 @@  typedef struct GDBState {
     gdb_syscall_complete_cb current_syscall_cb;
     GString *str_buf;
     GByteArray *mem_buf;
+    int sstep_flags;
+    int supported_sstep_flags;
 } GDBState;
 
-/* By default use no IRQs and no timers while single stepping so as to
- * make single stepping like an ICE HW step.
- */
-static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
+static GDBState gdbserver_state;
 
 /* Retrieves flags for single step mode. */
 static int get_sstep_flags(void)
@@ -385,11 +384,10 @@  static int get_sstep_flags(void)
     if (replay_mode != REPLAY_MODE_NONE) {
         return SSTEP_ENABLE;
     } else {
-        return sstep_flags;
+        return gdbserver_state.sstep_flags;
     }
 }
 
-static GDBState gdbserver_state;
 
 static void init_gdbserver_state(void)
 {
@@ -399,6 +397,23 @@  static void init_gdbserver_state(void)
     gdbserver_state.str_buf = g_string_new(NULL);
     gdbserver_state.mem_buf = g_byte_array_sized_new(MAX_PACKET_LENGTH);
     gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH + 4);
+
+
+    if (kvm_enabled()) {
+        gdbserver_state.supported_sstep_flags = kvm_get_supported_sstep_flags();
+    } else {
+        gdbserver_state.supported_sstep_flags =
+            SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER;
+    }
+
+    /*
+     * By default use no IRQs and no timers while single stepping so as to
+     * make single stepping like an ICE HW step.
+     */
+
+    gdbserver_state.sstep_flags = SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER;
+    gdbserver_state.sstep_flags &= gdbserver_state.supported_sstep_flags;
+
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -2017,24 +2032,44 @@  static void handle_v_commands(GArray *params, void *user_ctx)
 
 static void handle_query_qemu_sstepbits(GArray *params, void *user_ctx)
 {
-    g_string_printf(gdbserver_state.str_buf, "ENABLE=%x,NOIRQ=%x,NOTIMER=%x",
-                    SSTEP_ENABLE, SSTEP_NOIRQ, SSTEP_NOTIMER);
+    g_string_printf(gdbserver_state.str_buf, "ENABLE=%x", SSTEP_ENABLE);
+
+    if (gdbserver_state.supported_sstep_flags & SSTEP_NOIRQ) {
+        g_string_append_printf(gdbserver_state.str_buf, ",NOIRQ=%x",
+                               SSTEP_NOIRQ);
+    }
+
+    if (gdbserver_state.supported_sstep_flags & SSTEP_NOTIMER) {
+        g_string_append_printf(gdbserver_state.str_buf, ",NOTIMER=%x",
+                               SSTEP_NOTIMER);
+    }
+
     put_strbuf();
 }
 
 static void handle_set_qemu_sstep(GArray *params, void *user_ctx)
 {
+    int new_sstep_flags;
+
     if (!params->len) {
         return;
     }
 
-    sstep_flags = get_param(params, 0)->val_ul;
+    new_sstep_flags = get_param(params, 0)->val_ul;
+
+    if (new_sstep_flags  & ~gdbserver_state.supported_sstep_flags) {
+        put_packet("E22");
+        return;
+    }
+
+    gdbserver_state.sstep_flags = new_sstep_flags;
     put_packet("OK");
 }
 
 static void handle_query_qemu_sstep(GArray *params, void *user_ctx)
 {
-    g_string_printf(gdbserver_state.str_buf, "0x%x", sstep_flags);
+    g_string_printf(gdbserver_state.str_buf, "0x%x",
+                    gdbserver_state.sstep_flags);
     put_strbuf();
 }
 
@@ -3493,6 +3528,11 @@  int gdbserver_start(const char *device)
         return -1;
     }
 
+    if (kvm_enabled() && !kvm_supports_guest_debug()) {
+        error_report("gdbstub: KVM doesn't support guest debugging");
+        return -1;
+    }
+
     if (!device)
         return -1;
     if (strcmp(device, "none") != 0) {
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index b3d4538c55..9c0665363d 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -47,6 +47,8 @@  extern bool kvm_readonly_mem_allowed;
 extern bool kvm_direct_msi_allowed;
 extern bool kvm_ioeventfd_any_length_allowed;
 extern bool kvm_msi_use_devid;
+extern bool kvm_has_guest_debug;
+extern int kvm_sstep_flags;
 
 #define kvm_enabled()           (kvm_allowed)
 /**
@@ -171,6 +173,17 @@  extern bool kvm_msi_use_devid;
  */
 #define kvm_msi_devid_required() (kvm_msi_use_devid)
 
+/*
+ * Does KVM support guest debugging
+ */
+#define kvm_supports_guest_debug() (kvm_has_guest_debug)
+
+/*
+ * kvm_supported_sstep_flags
+ * Returns: SSTEP_* flags that KVM supports for guest debug
+ */
+#define kvm_get_supported_sstep_flags() (kvm_sstep_flags)
+
 #else
 
 #define kvm_enabled()           (0)