All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] KVM: qemu patches for few KVM features I developed
@ 2021-09-14 15:52 ` Maxim Levitsky
  0 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-09-14 15:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcelo Tosatti, Paolo Bonzini, Philippe Mathieu-Daudé,
	kvm, Alex Bennée, Maxim Levitsky

These patches implement the qemu side logic to support
the KVM features I developed recently.

First two patches are for features that are already accepted
upstream, and I already posted them on the qemu mailing list once.

And the 3rd patch is for nested TSC scaling on SVM
which isn't yet accepted in KVM but can already be added to qemu since
it is conditional on KVM supporting it, and ABI wise it only relies
on SVM spec.

Best regards,
    Maxim Levitsky

Maxim Levitsky (3):
  KVM: use KVM_{GET|SET}_SREGS2 when supported.
  gdbstub: implement NOIRQ support for single step on KVM
  KVM: SVM: add migration support for nested TSC scaling

 accel/kvm/kvm-all.c   |  30 +++++++++++
 gdbstub.c             |  60 +++++++++++++++++----
 include/sysemu/kvm.h  |  17 ++++++
 target/i386/cpu.c     |   5 ++
 target/i386/cpu.h     |   7 +++
 target/i386/kvm/kvm.c | 122 +++++++++++++++++++++++++++++++++++++++++-
 target/i386/machine.c |  53 ++++++++++++++++++
 7 files changed, 282 insertions(+), 12 deletions(-)

-- 
2.26.3



^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 0/3] KVM: qemu patches for few KVM features I developed
@ 2021-09-14 15:52 ` Maxim Levitsky
  0 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-09-14 15:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Philippe Mathieu-Daudé,
	Marcelo Tosatti, Maxim Levitsky, Paolo Bonzini, Alex Bennée

These patches implement the qemu side logic to support
the KVM features I developed recently.

First two patches are for features that are already accepted
upstream, and I already posted them on the qemu mailing list once.

And the 3rd patch is for nested TSC scaling on SVM
which isn't yet accepted in KVM but can already be added to qemu since
it is conditional on KVM supporting it, and ABI wise it only relies
on SVM spec.

Best regards,
    Maxim Levitsky

Maxim Levitsky (3):
  KVM: use KVM_{GET|SET}_SREGS2 when supported.
  gdbstub: implement NOIRQ support for single step on KVM
  KVM: SVM: add migration support for nested TSC scaling

 accel/kvm/kvm-all.c   |  30 +++++++++++
 gdbstub.c             |  60 +++++++++++++++++----
 include/sysemu/kvm.h  |  17 ++++++
 target/i386/cpu.c     |   5 ++
 target/i386/cpu.h     |   7 +++
 target/i386/kvm/kvm.c | 122 +++++++++++++++++++++++++++++++++++++++++-
 target/i386/machine.c |  53 ++++++++++++++++++
 7 files changed, 282 insertions(+), 12 deletions(-)

-- 
2.26.3




^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/3] KVM: use KVM_{GET|SET}_SREGS2 when supported.
  2021-09-14 15:52 ` Maxim Levitsky
@ 2021-09-14 15:52   ` Maxim Levitsky
  -1 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-09-14 15:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcelo Tosatti, Paolo Bonzini, Philippe Mathieu-Daudé,
	kvm, Alex Bennée, Maxim Levitsky

This allows to make PDPTRs part of the migration
stream and thus not reload them after migration which
is against X86 spec.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 accel/kvm/kvm-all.c   |   5 ++
 include/sysemu/kvm.h  |   4 ++
 target/i386/cpu.h     |   3 ++
 target/i386/kvm/kvm.c | 107 +++++++++++++++++++++++++++++++++++++++++-
 target/i386/machine.c |  30 ++++++++++++
 5 files changed, 147 insertions(+), 2 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 0125c17edb..6b187e9c96 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -163,6 +163,7 @@ bool kvm_msi_via_irqfd_allowed;
 bool kvm_gsi_routing_allowed;
 bool kvm_gsi_direct_mapping;
 bool kvm_allowed;
+bool kvm_sregs2;
 bool kvm_readonly_mem_allowed;
 bool kvm_vm_attributes_allowed;
 bool kvm_direct_msi_allowed;
@@ -2554,6 +2555,10 @@ static int kvm_init(MachineState *ms)
     kvm_ioeventfd_any_length_allowed =
         (kvm_check_extension(s, KVM_CAP_IOEVENTFD_ANY_LENGTH) > 0);
 
+
+    kvm_sregs2 =
+        (kvm_check_extension(s, KVM_CAP_SREGS2) > 0);
+
     kvm_state = s;
 
     ret = kvm_arch_init(ms, s);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index a1ab1ee12d..b3d4538c55 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -32,6 +32,7 @@
 #ifdef CONFIG_KVM_IS_POSSIBLE
 
 extern bool kvm_allowed;
+extern bool kvm_sregs2;
 extern bool kvm_kernel_irqchip;
 extern bool kvm_split_irqchip;
 extern bool kvm_async_interrupts_allowed;
@@ -139,6 +140,9 @@ extern bool kvm_msi_use_devid;
  */
 #define kvm_gsi_direct_mapping() (kvm_gsi_direct_mapping)
 
+
+#define kvm_supports_sregs2() (kvm_sregs2)
+
 /**
  * kvm_readonly_mem_enabled:
  *
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 71ae3141c3..9adae12426 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1436,6 +1436,9 @@ typedef struct CPUX86State {
     SegmentCache idt; /* only base and limit are used */
 
     target_ulong cr[5]; /* NOTE: cr1 is unused */
+
+    bool pdptrs_valid;
+    uint64_t pdptrs[4];
     int32_t a20_mask;
 
     BNDReg bnd_regs[4];
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 500d2e0e68..841b3b98f7 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2587,6 +2587,61 @@ static int kvm_put_sregs(X86CPU *cpu)
     return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS, &sregs);
 }
 
+static int kvm_put_sregs2(X86CPU *cpu)
+{
+    CPUX86State *env = &cpu->env;
+    struct kvm_sregs2 sregs;
+    int i;
+
+    sregs.flags = 0;
+
+    if ((env->eflags & VM_MASK)) {
+        set_v8086_seg(&sregs.cs, &env->segs[R_CS]);
+        set_v8086_seg(&sregs.ds, &env->segs[R_DS]);
+        set_v8086_seg(&sregs.es, &env->segs[R_ES]);
+        set_v8086_seg(&sregs.fs, &env->segs[R_FS]);
+        set_v8086_seg(&sregs.gs, &env->segs[R_GS]);
+        set_v8086_seg(&sregs.ss, &env->segs[R_SS]);
+    } else {
+        set_seg(&sregs.cs, &env->segs[R_CS]);
+        set_seg(&sregs.ds, &env->segs[R_DS]);
+        set_seg(&sregs.es, &env->segs[R_ES]);
+        set_seg(&sregs.fs, &env->segs[R_FS]);
+        set_seg(&sregs.gs, &env->segs[R_GS]);
+        set_seg(&sregs.ss, &env->segs[R_SS]);
+    }
+
+    set_seg(&sregs.tr, &env->tr);
+    set_seg(&sregs.ldt, &env->ldt);
+
+    sregs.idt.limit = env->idt.limit;
+    sregs.idt.base = env->idt.base;
+    memset(sregs.idt.padding, 0, sizeof sregs.idt.padding);
+    sregs.gdt.limit = env->gdt.limit;
+    sregs.gdt.base = env->gdt.base;
+    memset(sregs.gdt.padding, 0, sizeof sregs.gdt.padding);
+
+    sregs.cr0 = env->cr[0];
+    sregs.cr2 = env->cr[2];
+    sregs.cr3 = env->cr[3];
+    sregs.cr4 = env->cr[4];
+
+    sregs.cr8 = cpu_get_apic_tpr(cpu->apic_state);
+    sregs.apic_base = cpu_get_apic_base(cpu->apic_state);
+
+    sregs.efer = env->efer;
+
+    if (env->pdptrs_valid) {
+        for (i = 0; i < 4; i++) {
+            sregs.pdptrs[i] = env->pdptrs[i];
+        }
+        sregs.flags |= KVM_SREGS2_FLAGS_PDPTRS_VALID;
+    }
+
+    return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS2, &sregs);
+}
+
+
 static void kvm_msr_buf_reset(X86CPU *cpu)
 {
     memset(cpu->kvm_msr_buf, 0, MSR_BUF_SIZE);
@@ -3252,6 +3307,53 @@ static int kvm_get_sregs(X86CPU *cpu)
     return 0;
 }
 
+static int kvm_get_sregs2(X86CPU *cpu)
+{
+    CPUX86State *env = &cpu->env;
+    struct kvm_sregs2 sregs;
+    int i, ret;
+
+    ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_SREGS2, &sregs);
+    if (ret < 0) {
+        return ret;
+    }
+
+    get_seg(&env->segs[R_CS], &sregs.cs);
+    get_seg(&env->segs[R_DS], &sregs.ds);
+    get_seg(&env->segs[R_ES], &sregs.es);
+    get_seg(&env->segs[R_FS], &sregs.fs);
+    get_seg(&env->segs[R_GS], &sregs.gs);
+    get_seg(&env->segs[R_SS], &sregs.ss);
+
+    get_seg(&env->tr, &sregs.tr);
+    get_seg(&env->ldt, &sregs.ldt);
+
+    env->idt.limit = sregs.idt.limit;
+    env->idt.base = sregs.idt.base;
+    env->gdt.limit = sregs.gdt.limit;
+    env->gdt.base = sregs.gdt.base;
+
+    env->cr[0] = sregs.cr0;
+    env->cr[2] = sregs.cr2;
+    env->cr[3] = sregs.cr3;
+    env->cr[4] = sregs.cr4;
+
+    env->efer = sregs.efer;
+
+    env->pdptrs_valid = sregs.flags & KVM_SREGS2_FLAGS_PDPTRS_VALID;
+
+    if (env->pdptrs_valid) {
+        for (i = 0; i < 4; i++) {
+            env->pdptrs[i] = sregs.pdptrs[i];
+        }
+    }
+
+    /* changes to apic base and cr8/tpr are read back via kvm_arch_post_run */
+    x86_update_hflags(env);
+
+    return 0;
+}
+
 static int kvm_get_msrs(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
@@ -4077,7 +4179,8 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
     assert(cpu_is_stopped(cpu) || qemu_cpu_is_self(cpu));
 
     /* must be before kvm_put_nested_state so that EFER.SVME is set */
-    ret = kvm_put_sregs(x86_cpu);
+    ret = kvm_supports_sregs2() ? kvm_put_sregs2(x86_cpu) :
+                                  kvm_put_sregs(x86_cpu);
     if (ret < 0) {
         return ret;
     }
@@ -4182,7 +4285,7 @@ int kvm_arch_get_registers(CPUState *cs)
     if (ret < 0) {
         goto out;
     }
-    ret = kvm_get_sregs(cpu);
+    ret = kvm_supports_sregs2() ? kvm_get_sregs2(cpu) : kvm_get_sregs(cpu);
     if (ret < 0) {
         goto out;
     }
diff --git a/target/i386/machine.c b/target/i386/machine.c
index b0943118d1..154666e7c0 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -1415,6 +1415,35 @@ static const VMStateDescription vmstate_msr_tsx_ctrl = {
     }
 };
 
+static bool pdptrs_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+    return env->pdptrs_valid;
+}
+
+static int pdptrs_post_load(void *opaque, int version_id)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+    env->pdptrs_valid = true;
+    return 0;
+}
+
+
+static const VMStateDescription vmstate_pdptrs = {
+    .name = "cpu/pdptrs",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = pdptrs_needed,
+    .post_load = pdptrs_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64_ARRAY(env.pdptrs, X86CPU, 4),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+
 const VMStateDescription vmstate_x86_cpu = {
     .name = "cpu",
     .version_id = 12,
@@ -1551,6 +1580,7 @@ const VMStateDescription vmstate_x86_cpu = {
         &vmstate_nested_state,
 #endif
         &vmstate_msr_tsx_ctrl,
+        &vmstate_pdptrs,
         NULL
     }
 };
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 1/3] KVM: use KVM_{GET|SET}_SREGS2 when supported.
@ 2021-09-14 15:52   ` Maxim Levitsky
  0 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-09-14 15:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Philippe Mathieu-Daudé,
	Marcelo Tosatti, Maxim Levitsky, Paolo Bonzini, Alex Bennée

This allows to make PDPTRs part of the migration
stream and thus not reload them after migration which
is against X86 spec.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 accel/kvm/kvm-all.c   |   5 ++
 include/sysemu/kvm.h  |   4 ++
 target/i386/cpu.h     |   3 ++
 target/i386/kvm/kvm.c | 107 +++++++++++++++++++++++++++++++++++++++++-
 target/i386/machine.c |  30 ++++++++++++
 5 files changed, 147 insertions(+), 2 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 0125c17edb..6b187e9c96 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -163,6 +163,7 @@ bool kvm_msi_via_irqfd_allowed;
 bool kvm_gsi_routing_allowed;
 bool kvm_gsi_direct_mapping;
 bool kvm_allowed;
+bool kvm_sregs2;
 bool kvm_readonly_mem_allowed;
 bool kvm_vm_attributes_allowed;
 bool kvm_direct_msi_allowed;
@@ -2554,6 +2555,10 @@ static int kvm_init(MachineState *ms)
     kvm_ioeventfd_any_length_allowed =
         (kvm_check_extension(s, KVM_CAP_IOEVENTFD_ANY_LENGTH) > 0);
 
+
+    kvm_sregs2 =
+        (kvm_check_extension(s, KVM_CAP_SREGS2) > 0);
+
     kvm_state = s;
 
     ret = kvm_arch_init(ms, s);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index a1ab1ee12d..b3d4538c55 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -32,6 +32,7 @@
 #ifdef CONFIG_KVM_IS_POSSIBLE
 
 extern bool kvm_allowed;
+extern bool kvm_sregs2;
 extern bool kvm_kernel_irqchip;
 extern bool kvm_split_irqchip;
 extern bool kvm_async_interrupts_allowed;
@@ -139,6 +140,9 @@ extern bool kvm_msi_use_devid;
  */
 #define kvm_gsi_direct_mapping() (kvm_gsi_direct_mapping)
 
+
+#define kvm_supports_sregs2() (kvm_sregs2)
+
 /**
  * kvm_readonly_mem_enabled:
  *
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 71ae3141c3..9adae12426 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1436,6 +1436,9 @@ typedef struct CPUX86State {
     SegmentCache idt; /* only base and limit are used */
 
     target_ulong cr[5]; /* NOTE: cr1 is unused */
+
+    bool pdptrs_valid;
+    uint64_t pdptrs[4];
     int32_t a20_mask;
 
     BNDReg bnd_regs[4];
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 500d2e0e68..841b3b98f7 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2587,6 +2587,61 @@ static int kvm_put_sregs(X86CPU *cpu)
     return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS, &sregs);
 }
 
+static int kvm_put_sregs2(X86CPU *cpu)
+{
+    CPUX86State *env = &cpu->env;
+    struct kvm_sregs2 sregs;
+    int i;
+
+    sregs.flags = 0;
+
+    if ((env->eflags & VM_MASK)) {
+        set_v8086_seg(&sregs.cs, &env->segs[R_CS]);
+        set_v8086_seg(&sregs.ds, &env->segs[R_DS]);
+        set_v8086_seg(&sregs.es, &env->segs[R_ES]);
+        set_v8086_seg(&sregs.fs, &env->segs[R_FS]);
+        set_v8086_seg(&sregs.gs, &env->segs[R_GS]);
+        set_v8086_seg(&sregs.ss, &env->segs[R_SS]);
+    } else {
+        set_seg(&sregs.cs, &env->segs[R_CS]);
+        set_seg(&sregs.ds, &env->segs[R_DS]);
+        set_seg(&sregs.es, &env->segs[R_ES]);
+        set_seg(&sregs.fs, &env->segs[R_FS]);
+        set_seg(&sregs.gs, &env->segs[R_GS]);
+        set_seg(&sregs.ss, &env->segs[R_SS]);
+    }
+
+    set_seg(&sregs.tr, &env->tr);
+    set_seg(&sregs.ldt, &env->ldt);
+
+    sregs.idt.limit = env->idt.limit;
+    sregs.idt.base = env->idt.base;
+    memset(sregs.idt.padding, 0, sizeof sregs.idt.padding);
+    sregs.gdt.limit = env->gdt.limit;
+    sregs.gdt.base = env->gdt.base;
+    memset(sregs.gdt.padding, 0, sizeof sregs.gdt.padding);
+
+    sregs.cr0 = env->cr[0];
+    sregs.cr2 = env->cr[2];
+    sregs.cr3 = env->cr[3];
+    sregs.cr4 = env->cr[4];
+
+    sregs.cr8 = cpu_get_apic_tpr(cpu->apic_state);
+    sregs.apic_base = cpu_get_apic_base(cpu->apic_state);
+
+    sregs.efer = env->efer;
+
+    if (env->pdptrs_valid) {
+        for (i = 0; i < 4; i++) {
+            sregs.pdptrs[i] = env->pdptrs[i];
+        }
+        sregs.flags |= KVM_SREGS2_FLAGS_PDPTRS_VALID;
+    }
+
+    return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS2, &sregs);
+}
+
+
 static void kvm_msr_buf_reset(X86CPU *cpu)
 {
     memset(cpu->kvm_msr_buf, 0, MSR_BUF_SIZE);
@@ -3252,6 +3307,53 @@ static int kvm_get_sregs(X86CPU *cpu)
     return 0;
 }
 
+static int kvm_get_sregs2(X86CPU *cpu)
+{
+    CPUX86State *env = &cpu->env;
+    struct kvm_sregs2 sregs;
+    int i, ret;
+
+    ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_SREGS2, &sregs);
+    if (ret < 0) {
+        return ret;
+    }
+
+    get_seg(&env->segs[R_CS], &sregs.cs);
+    get_seg(&env->segs[R_DS], &sregs.ds);
+    get_seg(&env->segs[R_ES], &sregs.es);
+    get_seg(&env->segs[R_FS], &sregs.fs);
+    get_seg(&env->segs[R_GS], &sregs.gs);
+    get_seg(&env->segs[R_SS], &sregs.ss);
+
+    get_seg(&env->tr, &sregs.tr);
+    get_seg(&env->ldt, &sregs.ldt);
+
+    env->idt.limit = sregs.idt.limit;
+    env->idt.base = sregs.idt.base;
+    env->gdt.limit = sregs.gdt.limit;
+    env->gdt.base = sregs.gdt.base;
+
+    env->cr[0] = sregs.cr0;
+    env->cr[2] = sregs.cr2;
+    env->cr[3] = sregs.cr3;
+    env->cr[4] = sregs.cr4;
+
+    env->efer = sregs.efer;
+
+    env->pdptrs_valid = sregs.flags & KVM_SREGS2_FLAGS_PDPTRS_VALID;
+
+    if (env->pdptrs_valid) {
+        for (i = 0; i < 4; i++) {
+            env->pdptrs[i] = sregs.pdptrs[i];
+        }
+    }
+
+    /* changes to apic base and cr8/tpr are read back via kvm_arch_post_run */
+    x86_update_hflags(env);
+
+    return 0;
+}
+
 static int kvm_get_msrs(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
@@ -4077,7 +4179,8 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
     assert(cpu_is_stopped(cpu) || qemu_cpu_is_self(cpu));
 
     /* must be before kvm_put_nested_state so that EFER.SVME is set */
-    ret = kvm_put_sregs(x86_cpu);
+    ret = kvm_supports_sregs2() ? kvm_put_sregs2(x86_cpu) :
+                                  kvm_put_sregs(x86_cpu);
     if (ret < 0) {
         return ret;
     }
@@ -4182,7 +4285,7 @@ int kvm_arch_get_registers(CPUState *cs)
     if (ret < 0) {
         goto out;
     }
-    ret = kvm_get_sregs(cpu);
+    ret = kvm_supports_sregs2() ? kvm_get_sregs2(cpu) : kvm_get_sregs(cpu);
     if (ret < 0) {
         goto out;
     }
diff --git a/target/i386/machine.c b/target/i386/machine.c
index b0943118d1..154666e7c0 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -1415,6 +1415,35 @@ static const VMStateDescription vmstate_msr_tsx_ctrl = {
     }
 };
 
+static bool pdptrs_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+    return env->pdptrs_valid;
+}
+
+static int pdptrs_post_load(void *opaque, int version_id)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+    env->pdptrs_valid = true;
+    return 0;
+}
+
+
+static const VMStateDescription vmstate_pdptrs = {
+    .name = "cpu/pdptrs",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = pdptrs_needed,
+    .post_load = pdptrs_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64_ARRAY(env.pdptrs, X86CPU, 4),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+
 const VMStateDescription vmstate_x86_cpu = {
     .name = "cpu",
     .version_id = 12,
@@ -1551,6 +1580,7 @@ const VMStateDescription vmstate_x86_cpu = {
         &vmstate_nested_state,
 #endif
         &vmstate_msr_tsx_ctrl,
+        &vmstate_pdptrs,
         NULL
     }
 };
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/3] gdbstub: implement NOIRQ support for single step on KVM
  2021-09-14 15:52 ` Maxim Levitsky
@ 2021-09-14 15:52   ` Maxim Levitsky
  -1 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-09-14 15:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcelo Tosatti, Paolo Bonzini, Philippe Mathieu-Daudé,
	kvm, Alex Bennée, Maxim Levitsky

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;
+    }
+
+    /*
+     * 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)
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/3] gdbstub: implement NOIRQ support for single step on KVM
@ 2021-09-14 15:52   ` Maxim Levitsky
  0 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-09-14 15:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Philippe Mathieu-Daudé,
	Marcelo Tosatti, Maxim Levitsky, Paolo Bonzini, Alex Bennée

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;
+    }
+
+    /*
+     * 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)
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/3] KVM: SVM: add migration support for nested TSC scaling
  2021-09-14 15:52 ` Maxim Levitsky
@ 2021-09-14 15:52   ` Maxim Levitsky
  -1 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-09-14 15:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcelo Tosatti, Paolo Bonzini, Philippe Mathieu-Daudé,
	kvm, Alex Bennée, Maxim Levitsky

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 target/i386/cpu.c     |  5 +++++
 target/i386/cpu.h     |  4 ++++
 target/i386/kvm/kvm.c | 15 +++++++++++++++
 target/i386/machine.c | 23 +++++++++++++++++++++++
 4 files changed, 47 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6b029f1bdf..0870b53509 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5770,6 +5770,11 @@ static void x86_cpu_reset(DeviceState *dev)
     if (kvm_enabled()) {
         kvm_arch_reset_vcpu(cpu);
     }
+
+    if (env->features[FEAT_SVM] & CPUID_SVM_TSCSCALE) {
+        env->amd_tsc_scale_msr =  MSR_AMD64_TSC_RATIO_DEFAULT;
+    }
+
 #endif
 }
 
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 9adae12426..b9e1a3b7db 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -491,6 +491,9 @@ typedef enum X86Seg {
 #define MSR_GSBASE                      0xc0000101
 #define MSR_KERNELGSBASE                0xc0000102
 #define MSR_TSC_AUX                     0xc0000103
+#define MSR_AMD64_TSC_RATIO             0xc0000104
+
+#define MSR_AMD64_TSC_RATIO_DEFAULT     0x100000000ULL
 
 #define MSR_VM_HSAVE_PA                 0xc0010117
 
@@ -1522,6 +1525,7 @@ typedef struct CPUX86State {
     uint32_t tsx_ctrl;
 
     uint64_t spec_ctrl;
+    uint64_t amd_tsc_scale_msr;
     uint64_t virt_ssbd;
 
     /* End of state preserved by INIT (dummy marker).  */
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 841b3b98f7..bd53a55148 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -106,6 +106,7 @@ static bool has_msr_hv_reenlightenment;
 static bool has_msr_xss;
 static bool has_msr_umwait;
 static bool has_msr_spec_ctrl;
+static bool has_tsc_scale_msr;
 static bool has_msr_tsx_ctrl;
 static bool has_msr_virt_ssbd;
 static bool has_msr_smi_count;
@@ -2157,6 +2158,9 @@ static int kvm_get_supported_msrs(KVMState *s)
             case MSR_IA32_SPEC_CTRL:
                 has_msr_spec_ctrl = true;
                 break;
+            case MSR_AMD64_TSC_RATIO:
+                has_tsc_scale_msr = true;
+                break;
             case MSR_IA32_TSX_CTRL:
                 has_msr_tsx_ctrl = true;
                 break;
@@ -2968,6 +2972,10 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
     if (has_msr_spec_ctrl) {
         kvm_msr_entry_add(cpu, MSR_IA32_SPEC_CTRL, env->spec_ctrl);
     }
+    if (has_tsc_scale_msr) {
+        kvm_msr_entry_add(cpu, MSR_AMD64_TSC_RATIO, env->amd_tsc_scale_msr);
+    }
+
     if (has_msr_tsx_ctrl) {
         kvm_msr_entry_add(cpu, MSR_IA32_TSX_CTRL, env->tsx_ctrl);
     }
@@ -3409,6 +3417,10 @@ static int kvm_get_msrs(X86CPU *cpu)
     if (has_msr_spec_ctrl) {
         kvm_msr_entry_add(cpu, MSR_IA32_SPEC_CTRL, 0);
     }
+    if (has_tsc_scale_msr) {
+        kvm_msr_entry_add(cpu, MSR_AMD64_TSC_RATIO, 0);
+    }
+
     if (has_msr_tsx_ctrl) {
         kvm_msr_entry_add(cpu, MSR_IA32_TSX_CTRL, 0);
     }
@@ -3813,6 +3825,9 @@ static int kvm_get_msrs(X86CPU *cpu)
         case MSR_IA32_SPEC_CTRL:
             env->spec_ctrl = msrs[i].data;
             break;
+        case MSR_AMD64_TSC_RATIO:
+            env->amd_tsc_scale_msr = msrs[i].data;
+            break;
         case MSR_IA32_TSX_CTRL:
             env->tsx_ctrl = msrs[i].data;
             break;
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 154666e7c0..39c8faf0ce 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -1280,6 +1280,28 @@ static const VMStateDescription vmstate_spec_ctrl = {
     }
 };
 
+
+static bool amd_tsc_scale_msr_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+
+    return env->amd_tsc_scale_msr &&
+           env->amd_tsc_scale_msr != MSR_AMD64_TSC_RATIO_DEFAULT;
+}
+
+static const VMStateDescription amd_tsc_scale_msr_ctrl = {
+    .name = "cpu/amd_tsc_scale_msr",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = amd_tsc_scale_msr_needed,
+    .fields = (VMStateField[]){
+        VMSTATE_UINT64(env.amd_tsc_scale_msr, X86CPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+
 static bool intel_pt_enable_needed(void *opaque)
 {
     X86CPU *cpu = opaque;
@@ -1568,6 +1590,7 @@ const VMStateDescription vmstate_x86_cpu = {
         &vmstate_pkru,
         &vmstate_pkrs,
         &vmstate_spec_ctrl,
+        &amd_tsc_scale_msr_ctrl,
         &vmstate_mcg_ext_ctl,
         &vmstate_msr_intel_pt,
         &vmstate_msr_virt_ssbd,
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/3] KVM: SVM: add migration support for nested TSC scaling
@ 2021-09-14 15:52   ` Maxim Levitsky
  0 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-09-14 15:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Philippe Mathieu-Daudé,
	Marcelo Tosatti, Maxim Levitsky, Paolo Bonzini, Alex Bennée

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 target/i386/cpu.c     |  5 +++++
 target/i386/cpu.h     |  4 ++++
 target/i386/kvm/kvm.c | 15 +++++++++++++++
 target/i386/machine.c | 23 +++++++++++++++++++++++
 4 files changed, 47 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6b029f1bdf..0870b53509 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5770,6 +5770,11 @@ static void x86_cpu_reset(DeviceState *dev)
     if (kvm_enabled()) {
         kvm_arch_reset_vcpu(cpu);
     }
+
+    if (env->features[FEAT_SVM] & CPUID_SVM_TSCSCALE) {
+        env->amd_tsc_scale_msr =  MSR_AMD64_TSC_RATIO_DEFAULT;
+    }
+
 #endif
 }
 
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 9adae12426..b9e1a3b7db 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -491,6 +491,9 @@ typedef enum X86Seg {
 #define MSR_GSBASE                      0xc0000101
 #define MSR_KERNELGSBASE                0xc0000102
 #define MSR_TSC_AUX                     0xc0000103
+#define MSR_AMD64_TSC_RATIO             0xc0000104
+
+#define MSR_AMD64_TSC_RATIO_DEFAULT     0x100000000ULL
 
 #define MSR_VM_HSAVE_PA                 0xc0010117
 
@@ -1522,6 +1525,7 @@ typedef struct CPUX86State {
     uint32_t tsx_ctrl;
 
     uint64_t spec_ctrl;
+    uint64_t amd_tsc_scale_msr;
     uint64_t virt_ssbd;
 
     /* End of state preserved by INIT (dummy marker).  */
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 841b3b98f7..bd53a55148 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -106,6 +106,7 @@ static bool has_msr_hv_reenlightenment;
 static bool has_msr_xss;
 static bool has_msr_umwait;
 static bool has_msr_spec_ctrl;
+static bool has_tsc_scale_msr;
 static bool has_msr_tsx_ctrl;
 static bool has_msr_virt_ssbd;
 static bool has_msr_smi_count;
@@ -2157,6 +2158,9 @@ static int kvm_get_supported_msrs(KVMState *s)
             case MSR_IA32_SPEC_CTRL:
                 has_msr_spec_ctrl = true;
                 break;
+            case MSR_AMD64_TSC_RATIO:
+                has_tsc_scale_msr = true;
+                break;
             case MSR_IA32_TSX_CTRL:
                 has_msr_tsx_ctrl = true;
                 break;
@@ -2968,6 +2972,10 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
     if (has_msr_spec_ctrl) {
         kvm_msr_entry_add(cpu, MSR_IA32_SPEC_CTRL, env->spec_ctrl);
     }
+    if (has_tsc_scale_msr) {
+        kvm_msr_entry_add(cpu, MSR_AMD64_TSC_RATIO, env->amd_tsc_scale_msr);
+    }
+
     if (has_msr_tsx_ctrl) {
         kvm_msr_entry_add(cpu, MSR_IA32_TSX_CTRL, env->tsx_ctrl);
     }
@@ -3409,6 +3417,10 @@ static int kvm_get_msrs(X86CPU *cpu)
     if (has_msr_spec_ctrl) {
         kvm_msr_entry_add(cpu, MSR_IA32_SPEC_CTRL, 0);
     }
+    if (has_tsc_scale_msr) {
+        kvm_msr_entry_add(cpu, MSR_AMD64_TSC_RATIO, 0);
+    }
+
     if (has_msr_tsx_ctrl) {
         kvm_msr_entry_add(cpu, MSR_IA32_TSX_CTRL, 0);
     }
@@ -3813,6 +3825,9 @@ static int kvm_get_msrs(X86CPU *cpu)
         case MSR_IA32_SPEC_CTRL:
             env->spec_ctrl = msrs[i].data;
             break;
+        case MSR_AMD64_TSC_RATIO:
+            env->amd_tsc_scale_msr = msrs[i].data;
+            break;
         case MSR_IA32_TSX_CTRL:
             env->tsx_ctrl = msrs[i].data;
             break;
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 154666e7c0..39c8faf0ce 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -1280,6 +1280,28 @@ static const VMStateDescription vmstate_spec_ctrl = {
     }
 };
 
+
+static bool amd_tsc_scale_msr_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+
+    return env->amd_tsc_scale_msr &&
+           env->amd_tsc_scale_msr != MSR_AMD64_TSC_RATIO_DEFAULT;
+}
+
+static const VMStateDescription amd_tsc_scale_msr_ctrl = {
+    .name = "cpu/amd_tsc_scale_msr",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = amd_tsc_scale_msr_needed,
+    .fields = (VMStateField[]){
+        VMSTATE_UINT64(env.amd_tsc_scale_msr, X86CPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+
 static bool intel_pt_enable_needed(void *opaque)
 {
     X86CPU *cpu = opaque;
@@ -1568,6 +1590,7 @@ const VMStateDescription vmstate_x86_cpu = {
         &vmstate_pkru,
         &vmstate_pkrs,
         &vmstate_spec_ctrl,
+        &amd_tsc_scale_msr_ctrl,
         &vmstate_mcg_ext_ctl,
         &vmstate_msr_intel_pt,
         &vmstate_msr_virt_ssbd,
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] KVM: qemu patches for few KVM features I developed
  2021-09-14 15:52 ` Maxim Levitsky
@ 2021-09-30 11:52   ` Maxim Levitsky
  -1 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-09-30 11:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcelo Tosatti, Paolo Bonzini, Philippe Mathieu-Daudé,
	kvm, Alex Bennée

On Tue, 2021-09-14 at 18:52 +0300, Maxim Levitsky wrote:
> These patches implement the qemu side logic to support
> the KVM features I developed recently.
> 
> First two patches are for features that are already accepted
> upstream, and I already posted them on the qemu mailing list once.
> 
> And the 3rd patch is for nested TSC scaling on SVM
> which isn't yet accepted in KVM but can already be added to qemu since
> it is conditional on KVM supporting it, and ABI wise it only relies
> on SVM spec.
> 
> Best regards,
>     Maxim Levitsky
> 
> Maxim Levitsky (3):
>   KVM: use KVM_{GET|SET}_SREGS2 when supported.
>   gdbstub: implement NOIRQ support for single step on KVM
>   KVM: SVM: add migration support for nested TSC scaling
> 
>  accel/kvm/kvm-all.c   |  30 +++++++++++
>  gdbstub.c             |  60 +++++++++++++++++----
>  include/sysemu/kvm.h  |  17 ++++++
>  target/i386/cpu.c     |   5 ++
>  target/i386/cpu.h     |   7 +++
>  target/i386/kvm/kvm.c | 122 +++++++++++++++++++++++++++++++++++++++++-
>  target/i386/machine.c |  53 ++++++++++++++++++
>  7 files changed, 282 insertions(+), 12 deletions(-)
> 
> -- 
> 2.26.3
> 
> 
Very polite ping on these patches.

Best regards,
   Maxim Levitsky


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] KVM: qemu patches for few KVM features I developed
@ 2021-09-30 11:52   ` Maxim Levitsky
  0 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-09-30 11:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, Marcelo Tosatti,
	Philippe Mathieu-Daudé,
	kvm

On Tue, 2021-09-14 at 18:52 +0300, Maxim Levitsky wrote:
> These patches implement the qemu side logic to support
> the KVM features I developed recently.
> 
> First two patches are for features that are already accepted
> upstream, and I already posted them on the qemu mailing list once.
> 
> And the 3rd patch is for nested TSC scaling on SVM
> which isn't yet accepted in KVM but can already be added to qemu since
> it is conditional on KVM supporting it, and ABI wise it only relies
> on SVM spec.
> 
> Best regards,
>     Maxim Levitsky
> 
> Maxim Levitsky (3):
>   KVM: use KVM_{GET|SET}_SREGS2 when supported.
>   gdbstub: implement NOIRQ support for single step on KVM
>   KVM: SVM: add migration support for nested TSC scaling
> 
>  accel/kvm/kvm-all.c   |  30 +++++++++++
>  gdbstub.c             |  60 +++++++++++++++++----
>  include/sysemu/kvm.h  |  17 ++++++
>  target/i386/cpu.c     |   5 ++
>  target/i386/cpu.h     |   7 +++
>  target/i386/kvm/kvm.c | 122 +++++++++++++++++++++++++++++++++++++++++-
>  target/i386/machine.c |  53 ++++++++++++++++++
>  7 files changed, 282 insertions(+), 12 deletions(-)
> 
> -- 
> 2.26.3
> 
> 
Very polite ping on these patches.

Best regards,
   Maxim Levitsky



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] KVM: qemu patches for few KVM features I developed
  2021-09-14 15:52 ` Maxim Levitsky
@ 2021-10-13  9:32   ` Maxim Levitsky
  -1 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcelo Tosatti, Paolo Bonzini, Philippe Mathieu-Daudé,
	kvm, Alex Bennée

On Tue, 2021-09-14 at 18:52 +0300, Maxim Levitsky wrote:
> These patches implement the qemu side logic to support
> the KVM features I developed recently.
> 
> First two patches are for features that are already accepted
> upstream, and I already posted them on the qemu mailing list once.
> 
> And the 3rd patch is for nested TSC scaling on SVM
> which isn't yet accepted in KVM but can already be added to qemu since
> it is conditional on KVM supporting it, and ABI wise it only relies
> on SVM spec.
> 
> Best regards,
>     Maxim Levitsky
> 
> Maxim Levitsky (3):
>   KVM: use KVM_{GET|SET}_SREGS2 when supported.
>   gdbstub: implement NOIRQ support for single step on KVM
>   KVM: SVM: add migration support for nested TSC scaling
> 
>  accel/kvm/kvm-all.c   |  30 +++++++++++
>  gdbstub.c             |  60 +++++++++++++++++----
>  include/sysemu/kvm.h  |  17 ++++++
>  target/i386/cpu.c     |   5 ++
>  target/i386/cpu.h     |   7 +++
>  target/i386/kvm/kvm.c | 122 +++++++++++++++++++++++++++++++++++++++++-
>  target/i386/machine.c |  53 ++++++++++++++++++
>  7 files changed, 282 insertions(+), 12 deletions(-)
> 
> -- 
> 2.26.3
> 
> 
Kind ping on these patches.

Best regards,
	Maxim Levitsky


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/3] KVM: qemu patches for few KVM features I developed
@ 2021-10-13  9:32   ` Maxim Levitsky
  0 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, Marcelo Tosatti,
	Philippe Mathieu-Daudé,
	kvm

On Tue, 2021-09-14 at 18:52 +0300, Maxim Levitsky wrote:
> These patches implement the qemu side logic to support
> the KVM features I developed recently.
> 
> First two patches are for features that are already accepted
> upstream, and I already posted them on the qemu mailing list once.
> 
> And the 3rd patch is for nested TSC scaling on SVM
> which isn't yet accepted in KVM but can already be added to qemu since
> it is conditional on KVM supporting it, and ABI wise it only relies
> on SVM spec.
> 
> Best regards,
>     Maxim Levitsky
> 
> Maxim Levitsky (3):
>   KVM: use KVM_{GET|SET}_SREGS2 when supported.
>   gdbstub: implement NOIRQ support for single step on KVM
>   KVM: SVM: add migration support for nested TSC scaling
> 
>  accel/kvm/kvm-all.c   |  30 +++++++++++
>  gdbstub.c             |  60 +++++++++++++++++----
>  include/sysemu/kvm.h  |  17 ++++++
>  target/i386/cpu.c     |   5 ++
>  target/i386/cpu.h     |   7 +++
>  target/i386/kvm/kvm.c | 122 +++++++++++++++++++++++++++++++++++++++++-
>  target/i386/machine.c |  53 ++++++++++++++++++
>  7 files changed, 282 insertions(+), 12 deletions(-)
> 
> -- 
> 2.26.3
> 
> 
Kind ping on these patches.

Best regards,
	Maxim Levitsky



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] gdbstub: implement NOIRQ support for single step on KVM
  2021-09-14 15:52   ` Maxim Levitsky
@ 2021-10-13 15:50     ` Alex Bennée
  -1 siblings, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2021-10-13 15:50 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: qemu-devel, Marcelo Tosatti, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	kvm


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.


-- 
Alex Bennée

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] gdbstub: implement NOIRQ support for single step on KVM
@ 2021-10-13 15:50     ` Alex Bennée
  0 siblings, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2021-10-13 15:50 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: Paolo Bonzini, Marcelo Tosatti, Philippe Mathieu-Daudé,
	qemu-devel, kvm


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.


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] gdbstub: implement NOIRQ support for single step on KVM
  2021-10-13 15:50     ` Alex Bennée
@ 2021-10-13 19:52       ` Maxim Levitsky
  -1 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-10-13 19:52 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, Marcelo Tosatti, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	kvm

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

> 
> 



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] gdbstub: implement NOIRQ support for single step on KVM
@ 2021-10-13 19:52       ` Maxim Levitsky
  0 siblings, 0 replies; 18+ messages in thread
From: Maxim Levitsky @ 2021-10-13 19:52 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Paolo Bonzini, Marcelo Tosatti, Philippe Mathieu-Daudé,
	qemu-devel, kvm

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

> 
> 




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] gdbstub: implement NOIRQ support for single step on KVM
  2021-10-13 19:52       ` Maxim Levitsky
@ 2021-10-14 13:08         ` Alex Bennée
  -1 siblings, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2021-10-14 13:08 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: qemu-devel, Marcelo Tosatti, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	kvm


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


-- 
Alex Bennée

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/3] gdbstub: implement NOIRQ support for single step on KVM
@ 2021-10-14 13:08         ` Alex Bennée
  0 siblings, 0 replies; 18+ messages in thread
From: Alex Bennée @ 2021-10-14 13:08 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: Paolo Bonzini, Marcelo Tosatti, Philippe Mathieu-Daudé,
	qemu-devel, kvm


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


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-10-14 13:13 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-14 15:52 [PATCH 0/3] KVM: qemu patches for few KVM features I developed Maxim Levitsky
2021-09-14 15:52 ` Maxim Levitsky
2021-09-14 15:52 ` [PATCH 1/3] KVM: use KVM_{GET|SET}_SREGS2 when supported Maxim Levitsky
2021-09-14 15:52   ` Maxim Levitsky
2021-09-14 15:52 ` [PATCH 2/3] gdbstub: implement NOIRQ support for single step on KVM Maxim Levitsky
2021-09-14 15:52   ` Maxim Levitsky
2021-10-13 15:50   ` Alex Bennée
2021-10-13 15:50     ` Alex Bennée
2021-10-13 19:52     ` Maxim Levitsky
2021-10-13 19:52       ` Maxim Levitsky
2021-10-14 13:08       ` Alex Bennée
2021-10-14 13:08         ` Alex Bennée
2021-09-14 15:52 ` [PATCH 3/3] KVM: SVM: add migration support for nested TSC scaling Maxim Levitsky
2021-09-14 15:52   ` Maxim Levitsky
2021-09-30 11:52 ` [PATCH 0/3] KVM: qemu patches for few KVM features I developed Maxim Levitsky
2021-09-30 11:52   ` Maxim Levitsky
2021-10-13  9:32 ` Maxim Levitsky
2021-10-13  9:32   ` Maxim Levitsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.