[PATCH] KVM: PPC: Book3S HV: Treat unrecognized TM instructions as illegal

[PATCH] KVM: PPC: Book3S HV: Treat unrecognized TM instructions as illegal

[Gustavo Romero]

On P9 DD2.2 due to a CPU defect some TM instructions need to be emulated by
KVM. This is handled at first by the hardware raising a softpatch interrupt
when certain TM instructions that need KVM assistance are executed in the
guest. Some TM instructions, although not defined in the Power ISA, might
raise a softpatch interrupt. For instance, 'tresume.' instruction as
defined in the ISA must have bit 31 set (1), but an instruction that
matches 'tresume.' OP and XO opcodes but has bit 31 not set (0), like
0x7cfe9ddc, also raises a softpatch interrupt, for example, if a code
like the following is executed in the guest it will raise a softpatch
interrupt just like a 'tresume.' when the TM facility is enabled:

int main() { asm("tabort. 0; .long 0x7cfe9ddc;"); }

Currently in such a case KVM throws a complete trace like the following:

[345523.705984] WARNING: CPU: 24 PID: 64413 at arch/powerpc/kvm/book3s_hv_tm.c:211 kvmhv_p9_tm_emulation+0x68/0x620 [kvm_hv]
[345523.705985] Modules linked in: kvm_hv(E) xt_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp ip6table_mangle ip6table_nat
iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ebtable_filter ebtables ip6table_filter
ip6_tables iptable_filter bridge stp llc sch_fq_codel ipmi_powernv at24 vmx_crypto ipmi_devintf ipmi_msghandler
ibmpowernv uio_pdrv_genirq kvm opal_prd uio leds_powernv ib_iser rdma_cm iw_cm ib_cm ib_core iscsi_tcp libiscsi_tcp
libiscsi scsi_transport_iscsi ip_tables x_tables autofs4 btrfs blake2b_generic zstd_compress raid10 raid456
async_raid6_recov async_memcpy async_pq async_xor async_tx libcrc32c xor raid6_pq raid1 raid0 multipath linear tg3
crct10dif_vpmsum crc32c_vpmsum ipr [last unloaded: kvm_hv]
[345523.706030] CPU: 24 PID: 64413 Comm: CPU 0/KVM Tainted: G        W   E     5.5.0+ #1
[345523.706031] NIP:  c0080000072cb9c0 LR: c0080000072b5e80 CTR: c0080000085c7850
[345523.706034] REGS: c000000399467680 TRAP: 0700   Tainted: G        W   E      (5.5.0+)
[345523.706034] MSR:  900000010282b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE,TM[E]>  CR: 24022428  XER: 00000000
[345523.706042] CFAR: c0080000072b5e7c IRQMASK: 0
                GPR00: c0080000072b5e80 c000000399467910 c0080000072db500 c000000375ccc720
                GPR04: c000000375ccc720 00000003fbec0000 0000a10395dda5a6 0000000000000000
                GPR08: 000000007cfe9ddc 7cfe9ddc000005dc 7cfe9ddc7c0005dc c0080000072cd530
                GPR12: c0080000085c7850 c0000003fffeb800 0000000000000001 00007dfb737f0000
                GPR16: c0002001edcca558 0000000000000000 0000000000000000 0000000000000001
                GPR20: c000000001b21258 c0002001edcca558 0000000000000018 0000000000000000
                GPR24: 0000000001000000 ffffffffffffffff 0000000000000001 0000000000001500
                GPR28: c0002001edcc4278 c00000037dd80000 800000050280f033 c000000375ccc720
[345523.706062] NIP [c0080000072cb9c0] kvmhv_p9_tm_emulation+0x68/0x620 [kvm_hv]
[345523.706065] LR [c0080000072b5e80] kvmppc_handle_exit_hv.isra.53+0x3e8/0x798 [kvm_hv]
[345523.706066] Call Trace:
[345523.706069] [c000000399467910] [c000000399467940] 0xc000000399467940 (unreliable)
[345523.706071] [c000000399467950] [c000000399467980] 0xc000000399467980
[345523.706075] [c0000003994679f0] [c0080000072bd1c4] kvmhv_run_single_vcpu+0xa1c/0xb80 [kvm_hv]
[345523.706079] [c000000399467ac0] [c0080000072bd8e0] kvmppc_vcpu_run_hv+0x5b8/0xb00 [kvm_hv]
[345523.706087] [c000000399467b90] [c0080000085c93cc] kvmppc_vcpu_run+0x34/0x48 [kvm]
[345523.706095] [c000000399467bb0] [c0080000085c582c] kvm_arch_vcpu_ioctl_run+0x244/0x420 [kvm]
[345523.706101] [c000000399467c40] [c0080000085b7498] kvm_vcpu_ioctl+0x3d0/0x7b0 [kvm]
[345523.706105] [c000000399467db0] [c0000000004adf9c] ksys_ioctl+0x13c/0x170
[345523.706107] [c000000399467e00] [c0000000004adff8] sys_ioctl+0x28/0x80
[345523.706111] [c000000399467e20] [c00000000000b278] system_call+0x5c/0x68
[345523.706112] Instruction dump:
[345523.706114] 419e0390 7f8a4840 409d0048 6d497c00 2f89075d 419e021c 6d497c00 2f8907dd
[345523.706119] 419e01c0 6d497c00 2f8905dd 419e00a4 <0fe00000> 38210040 38600000 ebc1fff0

and then treats the executed instruction as 'nop' whilst it should actually
be treated as an illegal instruction since it's not defined by the ISA.

This commit changes the handling of the case above by treating the
unrecognized TM instructions that can raise a softpatch but are not
defined in the ISA as illegal ones instead of as 'nop' and by gently
reporting it to the host instead of throwing a trace.

Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
---
 arch/powerpc/kvm/book3s_hv_tm.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv_tm.c b/arch/powerpc/kvm/book3s_hv_tm.c
index 0db937497169..d342a9e11298 100644
--- a/arch/powerpc/kvm/book3s_hv_tm.c
+++ b/arch/powerpc/kvm/book3s_hv_tm.c
@@ -3,6 +3,8 @@
  * Copyright 2017 Paul Mackerras, IBM Corp. <paulus@au1.ibm.com>
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kvm_host.h>
 
 #include <asm/kvm_ppc.h>
@@ -208,6 +210,8 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
 	}
 
 	/* What should we do here? We didn't recognize the instruction */
-	WARN_ON_ONCE(1);
+	kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+	pr_warn_ratelimited("Unrecognized TM-related instruction %#x for emulation", instr);
+
 	return RESUME_GUEST;
 }
-- 
2.17.1

Re: [PATCH] KVM: PPC: Book3S HV: Treat unrecognized TM instructions as illegal

[Segher Boessenkool]

On Thu, Feb 13, 2020 at 10:15:32AM -0500, Gustavo Romero wrote:
> On P9 DD2.2 due to a CPU defect some TM instructions need to be emulated by
> KVM. This is handled at first by the hardware raising a softpatch interrupt
> when certain TM instructions that need KVM assistance are executed in the
> guest. Some TM instructions, although not defined in the Power ISA, might
> raise a softpatch interrupt. For instance, 'tresume.' instruction as
> defined in the ISA must have bit 31 set (1), but an instruction that
> matches 'tresume.' OP and XO opcodes but has bit 31 not set (0), like
> 0x7cfe9ddc, also raises a softpatch interrupt, for example, if a code
> like the following is executed in the guest it will raise a softpatch
> interrupt just like a 'tresume.' when the TM facility is enabled:
> 
> int main() { asm("tabort. 0; .long 0x7cfe9ddc;"); }
> 
> Currently in such a case KVM throws a complete trace like the following:

[snip]

> and then treats the executed instruction as 'nop' whilst it should actually
> be treated as an illegal instruction since it's not defined by the ISA.
> 
> This commit changes the handling of the case above by treating the
> unrecognized TM instructions that can raise a softpatch but are not
> defined in the ISA as illegal ones instead of as 'nop' and by gently
> reporting it to the host instead of throwing a trace.
> 
> Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>

Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>

> ---
>  arch/powerpc/kvm/book3s_hv_tm.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_hv_tm.c b/arch/powerpc/kvm/book3s_hv_tm.c
> index 0db937497169..d342a9e11298 100644
> --- a/arch/powerpc/kvm/book3s_hv_tm.c
> +++ b/arch/powerpc/kvm/book3s_hv_tm.c
> @@ -3,6 +3,8 @@
>   * Copyright 2017 Paul Mackerras, IBM Corp. <paulus@au1.ibm.com>
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/kvm_host.h>
>  
>  #include <asm/kvm_ppc.h>
> @@ -208,6 +210,8 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
>  	}
>  
>  	/* What should we do here? We didn't recognize the instruction */
> -	WARN_ON_ONCE(1);
> +	kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> +	pr_warn_ratelimited("Unrecognized TM-related instruction %#x for emulation", instr);
> +
>  	return RESUME_GUEST;
>  }

Do we actually know it is TM-related here?  Otherwise, looks good to me :-)


Segher

Re: [PATCH] KVM: PPC: Book3S HV: Treat unrecognized TM instructions as illegal

[Gustavo Romero]

Hi Segher,

Thanks a lot for reviewing it.

On 02/13/2020 08:31 PM, Segher Boessenkool wrote:

<snip>

>> ---
>>   arch/powerpc/kvm/book3s_hv_tm.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv_tm.c b/arch/powerpc/kvm/book3s_hv_tm.c
>> index 0db937497169..d342a9e11298 100644
>> --- a/arch/powerpc/kvm/book3s_hv_tm.c
>> +++ b/arch/powerpc/kvm/book3s_hv_tm.c
>> @@ -3,6 +3,8 @@
>>    * Copyright 2017 Paul Mackerras, IBM Corp. <paulus@au1.ibm.com>
>>    */
>>   
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>>   #include <linux/kvm_host.h>
>>   
>>   #include <asm/kvm_ppc.h>
>> @@ -208,6 +210,8 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
>>   	}
>>   
>>   	/* What should we do here? We didn't recognize the instruction */
>> -	WARN_ON_ONCE(1);
>> +	kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>> +	pr_warn_ratelimited("Unrecognized TM-related instruction %#x for emulation", instr);
>> +
>>   	return RESUME_GUEST;
>>   }
> 
> Do we actually know it is TM-related here?  Otherwise, looks good to me :-)

Correct, I understand it's only TM-related here, so I don't expect anything else to hit 0x1500.


Best regards,
Gustavo

Re: [PATCH] KVM: PPC: Book3S HV: Treat unrecognized TM instructions as illegal

[Michael Neuling]

On Thu, 2020-02-13 at 10:15 -0500, Gustavo Romero wrote:
> On P9 DD2.2 due to a CPU defect some TM instructions need to be emulated by
> KVM. This is handled at first by the hardware raising a softpatch interrupt
> when certain TM instructions that need KVM assistance are executed in the
> guest. Some TM instructions, although not defined in the Power ISA, might
> raise a softpatch interrupt. For instance, 'tresume.' instruction as
> defined in the ISA must have bit 31 set (1), but an instruction that
> matches 'tresume.' OP and XO opcodes but has bit 31 not set (0), like
> 0x7cfe9ddc, also raises a softpatch interrupt, for example, if a code
> like the following is executed in the guest it will raise a softpatch
> interrupt just like a 'tresume.' when the TM facility is enabled:
> 
> int main() { asm("tabort. 0; .long 0x7cfe9ddc;"); }
> 
> Currently in such a case KVM throws a complete trace like the following:
> 
> [345523.705984] WARNING: CPU: 24 PID: 64413 at arch/powerpc/kvm/book3s_hv_tm.c:211 kvmhv_p9_tm_emulation+0x68/0x620 [kvm_hv]
> [345523.705985] Modules linked in: kvm_hv(E) xt_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp ip6table_mangle ip6table_nat
> iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ebtable_filter ebtables ip6table_filter
> ip6_tables iptable_filter bridge stp llc sch_fq_codel ipmi_powernv at24 vmx_crypto ipmi_devintf ipmi_msghandler
> ibmpowernv uio_pdrv_genirq kvm opal_prd uio leds_powernv ib_iser rdma_cm iw_cm ib_cm ib_core iscsi_tcp libiscsi_tcp
> libiscsi scsi_transport_iscsi ip_tables x_tables autofs4 btrfs blake2b_generic zstd_compress raid10 raid456
> async_raid6_recov async_memcpy async_pq async_xor async_tx libcrc32c xor raid6_pq raid1 raid0 multipath linear tg3
> crct10dif_vpmsum crc32c_vpmsum ipr [last unloaded: kvm_hv]
> [345523.706030] CPU: 24 PID: 64413 Comm: CPU 0/KVM Tainted: G        W   E     5.5.0+ #1
> [345523.706031] NIP:  c0080000072cb9c0 LR: c0080000072b5e80 CTR: c0080000085c7850
> [345523.706034] REGS: c000000399467680 TRAP: 0700   Tainted: G        W   E      (5.5.0+)
> [345523.706034] MSR:  900000010282b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE,TM[E]>  CR: 24022428  XER: 00000000
> [345523.706042] CFAR: c0080000072b5e7c IRQMASK: 0
>                 GPR00: c0080000072b5e80 c000000399467910 c0080000072db500 c000000375ccc720
>                 GPR04: c000000375ccc720 00000003fbec0000 0000a10395dda5a6 0000000000000000
>                 GPR08: 000000007cfe9ddc 7cfe9ddc000005dc 7cfe9ddc7c0005dc c0080000072cd530
>                 GPR12: c0080000085c7850 c0000003fffeb800 0000000000000001 00007dfb737f0000
>                 GPR16: c0002001edcca558 0000000000000000 0000000000000000 0000000000000001
>                 GPR20: c000000001b21258 c0002001edcca558 0000000000000018 0000000000000000
>                 GPR24: 0000000001000000 ffffffffffffffff 0000000000000001 0000000000001500
>                 GPR28: c0002001edcc4278 c00000037dd80000 800000050280f033 c000000375ccc720
> [345523.706062] NIP [c0080000072cb9c0] kvmhv_p9_tm_emulation+0x68/0x620 [kvm_hv]
> [345523.706065] LR [c0080000072b5e80] kvmppc_handle_exit_hv.isra.53+0x3e8/0x798 [kvm_hv]
> [345523.706066] Call Trace:
> [345523.706069] [c000000399467910] [c000000399467940] 0xc000000399467940 (unreliable)
> [345523.706071] [c000000399467950] [c000000399467980] 0xc000000399467980
> [345523.706075] [c0000003994679f0] [c0080000072bd1c4] kvmhv_run_single_vcpu+0xa1c/0xb80 [kvm_hv]
> [345523.706079] [c000000399467ac0] [c0080000072bd8e0] kvmppc_vcpu_run_hv+0x5b8/0xb00 [kvm_hv]
> [345523.706087] [c000000399467b90] [c0080000085c93cc] kvmppc_vcpu_run+0x34/0x48 [kvm]
> [345523.706095] [c000000399467bb0] [c0080000085c582c] kvm_arch_vcpu_ioctl_run+0x244/0x420 [kvm]
> [345523.706101] [c000000399467c40] [c0080000085b7498] kvm_vcpu_ioctl+0x3d0/0x7b0 [kvm]
> [345523.706105] [c000000399467db0] [c0000000004adf9c] ksys_ioctl+0x13c/0x170
> [345523.706107] [c000000399467e00] [c0000000004adff8] sys_ioctl+0x28/0x80
> [345523.706111] [c000000399467e20] [c00000000000b278] system_call+0x5c/0x68
> [345523.706112] Instruction dump:
> [345523.706114] 419e0390 7f8a4840 409d0048 6d497c00 2f89075d 419e021c 6d497c00 2f8907dd
> [345523.706119] 419e01c0 6d497c00 2f8905dd 419e00a4 <0fe00000> 38210040 38600000 ebc1fff0
> 
> and then treats the executed instruction as 'nop' whilst it should actually
> be treated as an illegal instruction since it's not defined by the ISA.

The ISA has this: 

   1.3.3 Reserved Fields, Reserved Values, and Reserved SPRs

   Reserved fields in instructions are ignored by the pro-
   cessor.

Hence the hardware will ignore reserved bits. For example executing your little
program on P8 just exits normally with 0x7cfe9ddc being executed as a NOP.

Hence, we should NOP this, not generate an illegal.

Mikey

> This commit changes the handling of the case above by treating the
> unrecognized TM instructions that can raise a softpatch but are not
> defined in the ISA as illegal ones instead of as 'nop' and by gently
> reporting it to the host instead of throwing a trace.
> 
> Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
> ---
>  arch/powerpc/kvm/book3s_hv_tm.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_hv_tm.c b/arch/powerpc/kvm/book3s_hv_tm.c
> index 0db937497169..d342a9e11298 100644
> --- a/arch/powerpc/kvm/book3s_hv_tm.c
> +++ b/arch/powerpc/kvm/book3s_hv_tm.c
> @@ -3,6 +3,8 @@
>   * Copyright 2017 Paul Mackerras, IBM Corp. <paulus@au1.ibm.com>
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/kvm_host.h>
>  
>  #include <asm/kvm_ppc.h>
> @@ -208,6 +210,8 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
>  	}
>  
>  	/* What should we do here? We didn't recognize the instruction */
> -	WARN_ON_ONCE(1);
> +	kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> +	pr_warn_ratelimited("Unrecognized TM-related instruction %#x for emulation", instr);
> +
>  	return RESUME_GUEST;
>  }

Re: [PATCH] KVM: PPC: Book3S HV: Treat unrecognized TM instructions as illegal

[Segher Boessenkool]

On Mon, Feb 17, 2020 at 12:07:31PM +1100, Michael Neuling wrote:
> On Thu, 2020-02-13 at 10:15 -0500, Gustavo Romero wrote:
> > On P9 DD2.2 due to a CPU defect some TM instructions need to be emulated by
> > KVM. This is handled at first by the hardware raising a softpatch interrupt
> > when certain TM instructions that need KVM assistance are executed in the
> > guest. Some TM instructions, although not defined in the Power ISA, might
> > raise a softpatch interrupt. For instance, 'tresume.' instruction as
> > defined in the ISA must have bit 31 set (1), but an instruction that
> > matches 'tresume.' OP and XO opcodes but has bit 31 not set (0), like
> > 0x7cfe9ddc, also raises a softpatch interrupt, for example, if a code
> > like the following is executed in the guest it will raise a softpatch
> > interrupt just like a 'tresume.' when the TM facility is enabled:
> > 
> > int main() { asm("tabort. 0; .long 0x7cfe9ddc;"); }

> > and then treats the executed instruction as 'nop' whilst it should actually
> > be treated as an illegal instruction since it's not defined by the ISA.
> 
> The ISA has this: 
> 
>    1.3.3 Reserved Fields, Reserved Values, and Reserved SPRs
> 
>    Reserved fields in instructions are ignored by the pro-
>    cessor.
> 
> Hence the hardware will ignore reserved bits. For example executing your little
> program on P8 just exits normally with 0x7cfe9ddc being executed as a NOP.
> 
> Hence, we should NOP this, not generate an illegal.

It is not a reserved bit.

The IMC entry for it matches op1=011111 op2=1////01110 presumably, which
catches all TM instructions and nothing else (bits 0..5 and bits 21..30).
That does not look at bit 31, the softpatch handler has to deal with this.

Some TM insns have bit 31 as 1 and some have it as /.  All instructions
with a "." in the mnemonic have bit 31 is 1, all other have it reserved.
The tables in appendices D, E, F show tend. and tsr. as having it
reserved, which contradicts the individual instruction description (and
does not make much sense).  (Only tcheck has /, everything else has 1;
everything else has a mnemonic with a dot, and does write CR0 always).


Segher

Re: [PATCH] KVM: PPC: Book3S HV: Treat unrecognized TM instructions as illegal

[Michael Neuling]

On Sun, 2020-02-16 at 23:57 -0600, Segher Boessenkool wrote:
> On Mon, Feb 17, 2020 at 12:07:31PM +1100, Michael Neuling wrote:
> > On Thu, 2020-02-13 at 10:15 -0500, Gustavo Romero wrote:
> > > On P9 DD2.2 due to a CPU defect some TM instructions need to be emulated
> > > by
> > > KVM. This is handled at first by the hardware raising a softpatch
> > > interrupt
> > > when certain TM instructions that need KVM assistance are executed in the
> > > guest. Some TM instructions, although not defined in the Power ISA, might
> > > raise a softpatch interrupt. For instance, 'tresume.' instruction as
> > > defined in the ISA must have bit 31 set (1), but an instruction that
> > > matches 'tresume.' OP and XO opcodes but has bit 31 not set (0), like
> > > 0x7cfe9ddc, also raises a softpatch interrupt, for example, if a code
> > > like the following is executed in the guest it will raise a softpatch
> > > interrupt just like a 'tresume.' when the TM facility is enabled:
> > > 
> > > int main() { asm("tabort. 0; .long 0x7cfe9ddc;"); }
> > > and then treats the executed instruction as 'nop' whilst it should
> > > actually
> > > be treated as an illegal instruction since it's not defined by the ISA.
> > 
> > The ISA has this: 
> > 
> >    1.3.3 Reserved Fields, Reserved Values, and Reserved SPRs
> > 
> >    Reserved fields in instructions are ignored by the pro-
> >    cessor.
> > 
> > Hence the hardware will ignore reserved bits. For example executing your
> > little
> > program on P8 just exits normally with 0x7cfe9ddc being executed as a NOP.
> > 
> > Hence, we should NOP this, not generate an illegal.
> 
> It is not a reserved bit.
> 
> The IMC entry for it matches op1=011111 op2=1////01110 presumably, which
> catches all TM instructions and nothing else (bits 0..5 and bits 21..30).
> That does not look at bit 31, the softpatch handler has to deal with this.
> 
> Some TM insns have bit 31 as 1 and some have it as /.  All instructions
> with a "." in the mnemonic have bit 31 is 1, all other have it reserved.
> The tables in appendices D, E, F show tend. and tsr. as having it
> reserved, which contradicts the individual instruction description (and
> does not make much sense).  (Only tcheck has /, everything else has 1;
> everything else has a mnemonic with a dot, and does write CR0 always).

Wow, interesting. 

P8 seems to be treating 31 as a reserved bit (with the table definition rather
than the individual instruction description). I'm inclined to match P8 even
though it's inconsistent with the dot mnemonic as you say.

Mikey

Re: [PATCH] KVM: PPC: Book3S HV: Treat unrecognized TM instructions as illegal

[Segher Boessenkool]

On Mon, Feb 17, 2020 at 05:23:07PM +1100, Michael Neuling wrote:
> > > Hence, we should NOP this, not generate an illegal.
> > 
> > It is not a reserved bit.
> > 
> > The IMC entry for it matches op1=011111 op2=1////01110 presumably, which
> > catches all TM instructions and nothing else (bits 0..5 and bits 21..30).
> > That does not look at bit 31, the softpatch handler has to deal with this.
> > 
> > Some TM insns have bit 31 as 1 and some have it as /.  All instructions
> > with a "." in the mnemonic have bit 31 is 1, all other have it reserved.
> > The tables in appendices D, E, F show tend. and tsr. as having it
> > reserved, which contradicts the individual instruction description (and
> > does not make much sense).  (Only tcheck has /, everything else has 1;
> > everything else has a mnemonic with a dot, and does write CR0 always).
> 
> Wow, interesting. 
> 
> P8 seems to be treating 31 as a reserved bit (with the table definition rather
> than the individual instruction description). I'm inclined to match P8 even
> though it's inconsistent with the dot mnemonic as you say.

"The POWER8 core ignores the state of reserved bits in the instructions
(denoted by “///” in the instruction definition) and executes the
instruction normally. Software should set these bits to ‘0’ per the
Power ISA." (p8 UM, 3.1.1.3; same in the p9 UM).


Segher

Re: [PATCH] KVM: PPC: Book3S HV: Treat unrecognized TM instructions as illegal

[Gustavo Romero]

Hi,

On 02/17/2020 04:37 AM, Segher Boessenkool wrote:
> On Mon, Feb 17, 2020 at 05:23:07PM +1100, Michael Neuling wrote:
>>>> Hence, we should NOP this, not generate an illegal.
>>>
>>> It is not a reserved bit.
>>>
>>> The IMC entry for it matches op1=011111 op2=1////01110 presumably, which
>>> catches all TM instructions and nothing else (bits 0..5 and bits 21..30).
>>> That does not look at bit 31, the softpatch handler has to deal with this.
>>>
>>> Some TM insns have bit 31 as 1 and some have it as /.  All instructions
>>> with a "." in the mnemonic have bit 31 is 1, all other have it reserved.
>>> The tables in appendices D, E, F show tend. and tsr. as having it
>>> reserved, which contradicts the individual instruction description (and
>>> does not make much sense).  (Only tcheck has /, everything else has 1;
>>> everything else has a mnemonic with a dot, and does write CR0 always).
>>
>> Wow, interesting.
>>
>> P8 seems to be treating 31 as a reserved bit (with the table definition rather
>> than the individual instruction description). I'm inclined to match P8 even
>> though it's inconsistent with the dot mnemonic as you say.
> 
> "The POWER8 core ignores the state of reserved bits in the instructions
> (denoted by “///” in the instruction definition) and executes the
> instruction normally. Software should set these bits to ‘0’ per the
> Power ISA." (p8 UM, 3.1.1.3; same in the p9 UM).

For the records, I've sent a v2 addressing Mikey's comments:

https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-February/204502.html

or

https://marc.info/?l=kvm-ppc&m=158206045520038&w=2

Thanks for the review.


Best regards,
Gustavo