Patchwork KVM: PPC: Fix unknown SPR error message in emulation

login
register
mail settings
Submitter Mihai Caraman
Date April 24, 2012, 9:23 a.m.
Message ID <1335259406-3424-1-git-send-email-mihai.caraman@freescale.com>
Download mbox | patch
Permalink /patch/154628/
State New
Headers show

Comments

Mihai Caraman - April 24, 2012, 9:23 a.m.
mtspr/mfspr emulation prints an error message for unknown SPRs. The message
was badly formatted displaying the hex value without 0x prefix. Use decimal
representation in accordance with the manuals, though the Linux headers
annoyingly use hex.

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
 arch/powerpc/kvm/emulate.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Alexander Graf - April 26, 2012, 1:25 p.m.
On 24.04.2012, at 11:23, Mihai Caraman wrote:

> mtspr/mfspr emulation prints an error message for unknown SPRs. The message
> was badly formatted displaying the hex value without 0x prefix. Use decimal
> representation in accordance with the manuals, though the Linux headers
> annoyingly use hex.
> 
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> arch/powerpc/kvm/emulate.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
> index afc9154..06d12c4 100644
> --- a/arch/powerpc/kvm/emulate.c
> +++ b/arch/powerpc/kvm/emulate.c
> @@ -296,7 +296,7 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
> 			default:
> 				emulated = kvmppc_core_emulate_mfspr(vcpu, sprn, rt);
> 				if (emulated == EMULATE_FAIL) {
> -					printk("mfspr: unknown spr %x\n", sprn);
> +					printk("mfspr: unknown spr %u\n", sprn);

This means that if an older kernel threw an error on let's say SPR_VRSAVE, we got an error saying that it couldn't find "100", while with new kernels we'd get 256. However, we don't have any indication if we're on an old or new kernel, making user failures pretty hard to debug.

So either we change the message to hex always, with 0x prefixed, or we rephrase it to give us some indication if the user is running a patched kernel.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood - April 26, 2012, 5:47 p.m.
On 04/26/2012 08:25 AM, Alexander Graf wrote:
> 
> On 24.04.2012, at 11:23, Mihai Caraman wrote:
> 
>> mtspr/mfspr emulation prints an error message for unknown SPRs. The message
>> was badly formatted displaying the hex value without 0x prefix. Use decimal
>> representation in accordance with the manuals, though the Linux headers
>> annoyingly use hex.
>>
>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>> ---
>> arch/powerpc/kvm/emulate.c |    4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
>> index afc9154..06d12c4 100644
>> --- a/arch/powerpc/kvm/emulate.c
>> +++ b/arch/powerpc/kvm/emulate.c
>> @@ -296,7 +296,7 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
>> 			default:
>> 				emulated = kvmppc_core_emulate_mfspr(vcpu, sprn, rt);
>> 				if (emulated == EMULATE_FAIL) {
>> -					printk("mfspr: unknown spr %x\n", sprn);
>> +					printk("mfspr: unknown spr %u\n", sprn);
> 
> This means that if an older kernel threw an error on let's say
> SPR_VRSAVE, we got an error saying that it couldn't find "100", while

Eww.  Perhaps we should grep for bare %x and fix them all at once?  And
not let any more in...

> with new kernels we'd get 256. However, we don't have any indication
> if we're on an old or new kernel, making user failures pretty hard to
> debug.
> 
> So either we change the message to hex always, with 0x prefixed, or
> we rephrase it to give us some indication if the user is running a
> patched kernel.

How about:
printk("mfspr: unknown spr %u (dec)\n", sprn);

Or if we just care about having some sort of marker that those familiar
with the history will understand, and don't want something nicer-looking
that doesn't make it look like appending (dec) should be normal practice:

printk("mfspr: unknown spr #%u\n", sprn);

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Caraman Mihai Claudiu-B02008 - April 28, 2012, 7:34 p.m.
On 04/26/2012 08:25 AM, Alexander Graf wrote:
>
> On 24.04.2012, at 11:23, Mihai Caraman wrote:
>
>> mtspr/mfspr emulation prints an error message for unknown SPRs. The message
>> was badly formatted displaying the hex value without 0x prefix. Use decimal
>> representation in accordance with the manuals, though the Linux headers
>> annoyingly use hex.
> 
> However, we don't have any indication if we're on an old or new kernel, making user \
> failures pretty hard to debug.

One argument for switching to decimal was that booke206 mfpmr/mtpmr emulation
already used it but I see now that this support is not upstream yet.

-Mike
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index afc9154..06d12c4 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -296,7 +296,7 @@  int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 			default:
 				emulated = kvmppc_core_emulate_mfspr(vcpu, sprn, rt);
 				if (emulated == EMULATE_FAIL) {
-					printk("mfspr: unknown spr %x\n", sprn);
+					printk("mfspr: unknown spr %u\n", sprn);
 					kvmppc_set_gpr(vcpu, rt, 0);
 				}
 				break;
@@ -364,7 +364,7 @@  int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
 			default:
 				emulated = kvmppc_core_emulate_mtspr(vcpu, sprn, rs);
 				if (emulated == EMULATE_FAIL)
-					printk("mtspr: unknown spr %x\n", sprn);
+					printk("mfspr: unknown spr %u\n", sprn);
 				break;
 			}
 			kvmppc_set_exit_type(vcpu, EMULATED_MTSPR_EXITS);