[Qemu-devel] [PATCH] PPC: KVM: Don't tell the user about missing SPR syncs

[Qemu-devel] [PATCH] PPC: KVM: Don't tell the user about missing SPR syncs

[Alexander Graf]

We sync a lot of SPRs automatically between KVM and QEMU now. Some
of these only matter on newer hardware, some only matter on HV KVM.

With the current code runnign on my reasonably recent PR KVM kernel
I get a lot of SPR synchronization warnings though:

  $ ./ppc64-softmmu/qemu-system-ppc64 -nographic -enable-kvm
  Warning: Unable to set SPR 17 to KVM: Invalid argument
  Warning: Unable to set SPR 29 to KVM: Invalid argument
  Warning: Unable to set SPR 157 to KVM: Invalid argument
  Warning: Unable to set SPR 308 to KVM: Invalid argument
  Warning: Unable to set SPR 309 to KVM: Invalid argument
  Warning: Unable to set SPR 318 to KVM: Invalid argument
  Warning: Unable to set SPR 770 to KVM: Invalid argument
  Warning: Unable to set SPR 945 to KVM: Invalid argument
  Warning: Unable to set SPR 946 to KVM: Invalid argument
  Warning: Unable to set SPR 1013 to KVM: Invalid argument
  Warning: Unable to set SPR 17 to KVM: Invalid argument
  Warning: Unable to set SPR 29 to KVM: Invalid argument
  Warning: Unable to set SPR 157 to KVM: Invalid argument
  Warning: Unable to set SPR 308 to KVM: Invalid argument
  Warning: Unable to set SPR 309 to KVM: Invalid argument
  Warning: Unable to set SPR 318 to KVM: Invalid argument
  Warning: Unable to set SPR 770 to KVM: Invalid argument
  Warning: Unable to set SPR 945 to KVM: Invalid argument
  Warning: Unable to set SPR 946 to KVM: Invalid argument
  Warning: Unable to set SPR 1013 to KVM: Invalid argument

Eventually we want to have something like a "verbose" flag that allows
us to get these warnings when we see something goes wrong. But until
then they do more harm than good exposed to casual users, so let's move
them to debug prints.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-ppc/kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 8f3f0bf..dce2156 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -480,7 +480,7 @@ static void kvm_get_one_spr(CPUState *cs, uint64_t id, int spr)
 
     ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
     if (ret != 0) {
-        fprintf(stderr, "Warning: Unable to retrieve SPR %d from KVM: %s\n",
+        DPRINTF("Warning: Unable to retrieve SPR %d from KVM: %s\n",
                 spr, strerror(errno));
     } else {
         switch (id & KVM_REG_SIZE_MASK) {
@@ -529,7 +529,7 @@ static void kvm_put_one_spr(CPUState *cs, uint64_t id, int spr)
 
     ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
     if (ret != 0) {
-        fprintf(stderr, "Warning: Unable to set SPR %d to KVM: %s\n",
+        DPRINTF("Warning: Unable to set SPR %d to KVM: %s\n",
                 spr, strerror(errno));
     }
 }
-- 
1.8.1.4

Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: KVM: Don't tell the user about missing SPR syncs

[Alexey Kardashevskiy]

On 02/04/2014 08:53 AM, Alexander Graf wrote:
> We sync a lot of SPRs automatically between KVM and QEMU now. Some
> of these only matter on newer hardware, some only matter on HV KVM.
> 
> With the current code runnign on my reasonably recent PR KVM kernel
> I get a lot of SPR synchronization warnings though:
> 
>   $ ./ppc64-softmmu/qemu-system-ppc64 -nographic -enable-kvm
>   Warning: Unable to set SPR 17 to KVM: Invalid argument
>   Warning: Unable to set SPR 29 to KVM: Invalid argument
>   Warning: Unable to set SPR 157 to KVM: Invalid argument
>   Warning: Unable to set SPR 308 to KVM: Invalid argument
>   Warning: Unable to set SPR 309 to KVM: Invalid argument
>   Warning: Unable to set SPR 318 to KVM: Invalid argument
>   Warning: Unable to set SPR 770 to KVM: Invalid argument
>   Warning: Unable to set SPR 945 to KVM: Invalid argument
>   Warning: Unable to set SPR 946 to KVM: Invalid argument
>   Warning: Unable to set SPR 1013 to KVM: Invalid argument
>   Warning: Unable to set SPR 17 to KVM: Invalid argument
>   Warning: Unable to set SPR 29 to KVM: Invalid argument
>   Warning: Unable to set SPR 157 to KVM: Invalid argument
>   Warning: Unable to set SPR 308 to KVM: Invalid argument
>   Warning: Unable to set SPR 309 to KVM: Invalid argument
>   Warning: Unable to set SPR 318 to KVM: Invalid argument
>   Warning: Unable to set SPR 770 to KVM: Invalid argument
>   Warning: Unable to set SPR 945 to KVM: Invalid argument
>   Warning: Unable to set SPR 946 to KVM: Invalid argument
>   Warning: Unable to set SPR 1013 to KVM: Invalid argument
> 
> Eventually we want to have something like a "verbose" flag that allows
> us to get these warnings when we see something goes wrong. But until
> then they do more harm than good exposed to casual users, so let's move
> them to debug prints.

Why are not these tracepoints? Then we would not need any "verbose" flag.


> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  target-ppc/kvm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 8f3f0bf..dce2156 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -480,7 +480,7 @@ static void kvm_get_one_spr(CPUState *cs, uint64_t id, int spr)
>  
>      ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
>      if (ret != 0) {
> -        fprintf(stderr, "Warning: Unable to retrieve SPR %d from KVM: %s\n",
> +        DPRINTF("Warning: Unable to retrieve SPR %d from KVM: %s\n",
>                  spr, strerror(errno));
>      } else {
>          switch (id & KVM_REG_SIZE_MASK) {
> @@ -529,7 +529,7 @@ static void kvm_put_one_spr(CPUState *cs, uint64_t id, int spr)
>  
>      ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
>      if (ret != 0) {
> -        fprintf(stderr, "Warning: Unable to set SPR %d to KVM: %s\n",
> +        DPRINTF("Warning: Unable to set SPR %d to KVM: %s\n",
>                  spr, strerror(errno));
>      }
>  }
> 


-- 
Alexey

[Qemu-devel] [PATCH] PPC: KVM: suppress warnings about not supported SPRs

[Alexey Kardashevskiy]

PR KVM lacks support of many SPRs in set/get one register API but it does
really break PR KVM. So convert them to switchable traces for now.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

We use this patch internally for the same purpose.


---
 target-ppc/kvm.c | 7 +++----
 trace-events     | 2 ++
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 3f85b8d..690bd51 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -36,6 +36,7 @@
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/spapr_vio.h"
 #include "sysemu/watchdog.h"
+#include "trace.h"
 
 //#define DEBUG_KVM
 
@@ -480,8 +481,7 @@ static void kvm_get_one_spr(CPUState *cs, uint64_t id, int spr)
 
     ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
     if (ret != 0) {
-        fprintf(stderr, "Warning: Unable to retrieve SPR %d from KVM: %s\n",
-                spr, strerror(errno));
+        trace_kvm_failed_spr_get(spr, strerror(errno));
     } else {
         switch (id & KVM_REG_SIZE_MASK) {
         case KVM_REG_SIZE_U32:
@@ -529,8 +529,7 @@ static void kvm_put_one_spr(CPUState *cs, uint64_t id, int spr)
 
     ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
     if (ret != 0) {
-        fprintf(stderr, "Warning: Unable to set SPR %d to KVM: %s\n",
-                spr, strerror(errno));
+        trace_kvm_failed_spr_set(spr, strerror(errno));
     }
 }
 
diff --git a/trace-events b/trace-events
index 9f4456a..3bbcd51 100644
--- a/trace-events
+++ b/trace-events
@@ -1169,6 +1169,8 @@ kvm_ioctl(int type, void *arg) "type 0x%x, arg %p"
 kvm_vm_ioctl(int type, void *arg) "type 0x%x, arg %p"
 kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type 0x%x, arg %p"
 kvm_run_exit(int cpu_index, uint32_t reason) "cpu_index %d, reason %d"
+kvm_failed_spr_set(int str, const char *msg) "Warning: Unable to set SPR %d to KVM: %s"
+kvm_failed_spr_get(int str, const char *msg) "Warning: Unable to retrieve SPR %d from KVM: %s"
 
 # memory.c
 memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %u"
-- 
1.8.4.rc4

Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: KVM: Don't tell the user about missing SPR syncs

[Alexander Graf]

On 04.02.2014, at 03:05, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 02/04/2014 08:53 AM, Alexander Graf wrote:
>> We sync a lot of SPRs automatically between KVM and QEMU now. Some
>> of these only matter on newer hardware, some only matter on HV KVM.
>> 
>> With the current code runnign on my reasonably recent PR KVM kernel
>> I get a lot of SPR synchronization warnings though:
>> 
>>  $ ./ppc64-softmmu/qemu-system-ppc64 -nographic -enable-kvm
>>  Warning: Unable to set SPR 17 to KVM: Invalid argument
>>  Warning: Unable to set SPR 29 to KVM: Invalid argument
>>  Warning: Unable to set SPR 157 to KVM: Invalid argument
>>  Warning: Unable to set SPR 308 to KVM: Invalid argument
>>  Warning: Unable to set SPR 309 to KVM: Invalid argument
>>  Warning: Unable to set SPR 318 to KVM: Invalid argument
>>  Warning: Unable to set SPR 770 to KVM: Invalid argument
>>  Warning: Unable to set SPR 945 to KVM: Invalid argument
>>  Warning: Unable to set SPR 946 to KVM: Invalid argument
>>  Warning: Unable to set SPR 1013 to KVM: Invalid argument
>>  Warning: Unable to set SPR 17 to KVM: Invalid argument
>>  Warning: Unable to set SPR 29 to KVM: Invalid argument
>>  Warning: Unable to set SPR 157 to KVM: Invalid argument
>>  Warning: Unable to set SPR 308 to KVM: Invalid argument
>>  Warning: Unable to set SPR 309 to KVM: Invalid argument
>>  Warning: Unable to set SPR 318 to KVM: Invalid argument
>>  Warning: Unable to set SPR 770 to KVM: Invalid argument
>>  Warning: Unable to set SPR 945 to KVM: Invalid argument
>>  Warning: Unable to set SPR 946 to KVM: Invalid argument
>>  Warning: Unable to set SPR 1013 to KVM: Invalid argument
>> 
>> Eventually we want to have something like a "verbose" flag that allows
>> us to get these warnings when we see something goes wrong. But until
>> then they do more harm than good exposed to casual users, so let's move
>> them to debug prints.
> 
> Why are not these tracepoints? Then we would not need any "verbose" flag.

True, we should convert all of the debug prints to trace points. But that's for later :).


Alex

Re: [Qemu-devel] [PATCH] PPC: KVM: suppress warnings about not supported SPRs

[Alexander Graf]

On 04.02.2014, at 05:12, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> PR KVM lacks support of many SPRs in set/get one register API but it does
> really break PR KVM. So convert them to switchable traces for now.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Thanks, applied this patch instead of mine.


Alex

Re: [Qemu-devel] [PATCH] PPC: KVM: suppress warnings about not supported SPRs

[Alexey Kardashevskiy]

On 02/04/2014 06:38 PM, Alexander Graf wrote:
> 
> On 04.02.2014, at 05:12, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> PR KVM lacks support of many SPRs in set/get one register API but it does

s/does/does not/g

sorry 8-)


>> really break PR KVM. So convert them to switchable traces for now.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> Thanks, applied this patch instead of mine.




-- 
Alexey