Re: [Qemu-devel] [PATCH] target/s390x: Implement CSST
On 20.06.2017 01:44, Richard Henderson wrote:
> On 06/19/2017 01:08 AM, Thomas Huth wrote:
>>> +/* Sanity check the function code and storage characteristic. */
>>> +if (fc > 1 || sc > 3) {
>>> +if (!s390_has_feat(S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2)) {
>>> +goto spec_exception;
>>> +}
>>> +if (fc > 2 || sc > 4 || (fc == 2 && (r3 & 1))) {
>>
>> I think you could omit the "fc == 2" here. fc has to be bigger than 1
>> due to the outer if-statement, and if it is not 2, the first "fc > 1"
>> has already triggered. So "fc" has to be 2 here and the "fc == 2" is a
>> redundant check.
>
> Not so. We can also get here with fc == 0 && sc == 4.
Uh, right, sorry for the confusion!
Thomas
Re: [Qemu-devel] [PATCH] target/s390x: Implement CSST
On 06/19/2017 01:08 AM, Thomas Huth wrote:
+/* Sanity check the function code and storage characteristic. */
+if (fc > 1 || sc > 3) {
+if (!s390_has_feat(S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2)) {
+goto spec_exception;
+}
+if (fc > 2 || sc > 4 || (fc == 2 && (r3 & 1))) {
I think you could omit the "fc == 2" here. fc has to be bigger than 1
due to the outer if-statement, and if it is not 2, the first "fc > 1"
has already triggered. So "fc" has to be 2 here and the "fc == 2" is a
redundant check.
Not so. We can also get here with fc == 0 && sc == 4.
r~
Re: [Qemu-devel] [PATCH] target/s390x: Implement CSST
On 06/19/2017 01:08 AM, Thomas Huth wrote:
+/* Sanity check the function code and storage characteristic. */
+if (fc > 1 || sc > 3) {
+if (!s390_has_feat(S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2)) {
+goto spec_exception;
+}
+if (fc > 2 || sc > 4 || (fc == 2 && (r3 & 1))) {
I think you could omit the "fc == 2" here. fc has to be bigger than 1
due to the outer if-statement, and if it is not 2, the first "fc > 1"
has already triggered. So "fc" has to be 2 here and the "fc == 2" is a
redundant check.
Quite right.
r~
Re: [Qemu-devel] [PATCH] target/s390x: Implement CSST
On 06/19/2017 02:52 PM, David Hildenbrand wrote: > On 19.06.2017 14:47, Christian Borntraeger wrote: >> On 06/19/2017 02:41 PM, David Hildenbrand wrote: >>> On 19.06.2017 14:33, Christian Borntraeger wrote: On 06/19/2017 02:05 PM, David Hildenbrand wrote: > On 19.06.2017 12:03, David Hildenbrand wrote: >> On 15.06.2017 22:37, Richard Henderson wrote: >>> There are no uses in a Linux system with which to test, >>> but it Looks Right by my reading of the PoO. >> >> I am using next.git/master with this patch applied: >> https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/?h=features&id=8aa8680aa383bf6e2ac >> >> I am using QEMU with the mvcos patch and your patch applied (and a patch >> that allows enabling csst/csst2). >> >> I am using the following qemu command line: >> >> #!/bin/bash >> /home/dhildenb/git/qemu/s390x-softmmu/qemu-system-s390x \ >> -nographic -nodefaults -machine s390-ccw-virtio,accel=tcg \ >> -cpu qemu,mvcos=on,stfle=on,ldisp=on,ldisphp=on,\ >> eimm=on,stckf=on,csst=on,csst2=on,ginste=on,exrl=on\ >> -m 256M -smp 1 -chardev stdio,id=con0 \ >> -device sclpconsole,chardev=con0 \ >> -kernel vmlinux -initrd /home/dhildenb/initrd.debian >> >> Right now, I can start a z9 compiled kernel. >> >> When trying to start a z10 compiled kernel (which generates many csst), I would be very surprised if the kernel would contain any csst. gcc does not emit csst and the kernel source also does not contain it. >>> >>> I only did a grep on the objdump output: >>> >>> t460s: ~/git/linux-s390 next $ /usr/bin/s390x-linux-gnu-objdump -D >>> vmlinux | grep csst >>> 912826: c8 e2 f1 7c 56 02 csst380(%r15),1538(%r5),%r14 >>> 954684: c8 62 dc 35 2b 65 csst3125(%r13),2917(%r2),%r6 >>> 95e6e4: c8 a2 1c 76 0a 60 csst3190(%r1),2656,%r10 >>> 95f68a: c8 b2 c9 b3 3d 47 csst2483(%r12),3399(%r3),%r11 >>> 96067a: c8 42 d3 59 da 50 csst857(%r13),2640(%r13),%r4 >>> 963642: c8 72 73 c9 1a a0 csst969(%r7),2720(%r1),%r7 >>> 9656de: c8 12 d3 09 7a a0 csst777(%r13),2720(%r7),%r1 >>> 9676a6: c8 32 6d 97 84 7e csst3479(%r6),1150(%r8),%r3 >>> 9d470a: c8 a2 70 11 74 02 csst17(%r7),1026(%r7),%r10 >>> 9d6c4a: c8 a2 de 0c 54 4a csst3596(%r13),1098(%r5),%r10 >>> 9e3af8: c8 a2 de 09 54 73 csst3593(%r13),1139(%r5),%r10 >>> 9e3b02: c8 a2 de 0f 54 73 csst3599(%r13),1139(%r5),%r10 >>> 9e7992: c8 a2 de 0c 54 d4 csst3596(%r13),1236(%r5),%r10 >>> 9e79ea: c8 a2 40 f6 c3 0e csst246(%r4),782(%r12),%r10 >>> 9e7e3c: c8 a2 40 6c d1 74 csst108(%r4),372(%r13),%r10 >>> 9e8036: c8 a2 de 0d 54 cd csst3597(%r13),1229(%r5),%r10 >>> 9e81ea: c8 a2 40 63 2f b8 csst99(%r4),4024(%r2),%r10 >>> 9e81fe: c8 a2 de 0f 54 68 csst3599(%r13),1128(%r5),%r10 >>> 9e8e10: c8 72 93 83 69 bd csst899(%r9),2493(%r6),%r7 >>> 9e8ea4: c8 72 c6 04 54 63 csst1540(%r12),1123(%r5),%r7 >>> 9e8eae: c8 72 c6 f1 98 77 csst1777(%r12),2167(%r9),%r7 >>> 9e8ebc: c8 72 93 ba f5 07 csst954(%r9),1287(%r15),%r7 >>> a0702e: c8 a2 14 74 6e 5f csst1140(%r1),3679(%r6),%r10 >>> a083ea: c8 a2 73 08 74 da csst776(%r7),1242(%r7),%r10 >>> a0ec06: c8 a2 f8 f6 c3 08 csst2294(%r15),776(%r12),%r10 >>> a11890: c8 a2 f8 fa 5f ac csst2298(%r15),4012(%r5),%r10 >>> a11b6e: c8 a2 73 0a 74 74 csst778(%r7),1140(%r7),%r10 >>> a11be0: c8 a2 f8 fa 5f ac csst2298(%r15),4012(%r5),%r10 >>> a11ef4: c8 a2 73 0b b3 7c csst779(%r7),892(%r11),%r10 >>> [...] >>> 14c9e5a: c8 02 d0 2d 00 0d csst45(%r13),13,%r0 >>> >>> >>> That made me assume we have csst in the kernel :) >> >> you used -D (which also disassembles data). Do you still see any csst with >> -d ? >> > > ... probably I should have lunch now :) > > No csst, therefore the panic I am seeing is unrelated to this ... > (I wonder why CSST is a required kernel facility if nobody uses it? hm) I guess because in theory gcc could use that instruction for -march=z9-ec. It does not seem to be hypervisor-managed and should be available on all z9 guests.
Re: [Qemu-devel] [PATCH] target/s390x: Implement CSST
On 19.06.2017 14:47, Christian Borntraeger wrote: > On 06/19/2017 02:41 PM, David Hildenbrand wrote: >> On 19.06.2017 14:33, Christian Borntraeger wrote: >>> On 06/19/2017 02:05 PM, David Hildenbrand wrote: On 19.06.2017 12:03, David Hildenbrand wrote: > On 15.06.2017 22:37, Richard Henderson wrote: >> There are no uses in a Linux system with which to test, >> but it Looks Right by my reading of the PoO. > > I am using next.git/master with this patch applied: > https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/?h=features&id=8aa8680aa383bf6e2ac > > I am using QEMU with the mvcos patch and your patch applied (and a patch > that allows enabling csst/csst2). > > I am using the following qemu command line: > > #!/bin/bash > /home/dhildenb/git/qemu/s390x-softmmu/qemu-system-s390x \ > -nographic -nodefaults -machine s390-ccw-virtio,accel=tcg \ > -cpu qemu,mvcos=on,stfle=on,ldisp=on,ldisphp=on,\ > eimm=on,stckf=on,csst=on,csst2=on,ginste=on,exrl=on\ > -m 256M -smp 1 -chardev stdio,id=con0 \ > -device sclpconsole,chardev=con0 \ > -kernel vmlinux -initrd /home/dhildenb/initrd.debian > > Right now, I can start a z9 compiled kernel. > > When trying to start a z10 compiled kernel (which generates many csst), >>> >>> >>> I would be very surprised if the kernel would contain any csst. gcc does not >>> emit csst and the kernel source also does not contain it. >>> >> >> I only did a grep on the objdump output: >> >> t460s: ~/git/linux-s390 next $ /usr/bin/s390x-linux-gnu-objdump -D >> vmlinux | grep csst >> 912826: c8 e2 f1 7c 56 02 csst380(%r15),1538(%r5),%r14 >> 954684: c8 62 dc 35 2b 65 csst3125(%r13),2917(%r2),%r6 >> 95e6e4: c8 a2 1c 76 0a 60 csst3190(%r1),2656,%r10 >> 95f68a: c8 b2 c9 b3 3d 47 csst2483(%r12),3399(%r3),%r11 >> 96067a: c8 42 d3 59 da 50 csst857(%r13),2640(%r13),%r4 >> 963642: c8 72 73 c9 1a a0 csst969(%r7),2720(%r1),%r7 >> 9656de: c8 12 d3 09 7a a0 csst777(%r13),2720(%r7),%r1 >> 9676a6: c8 32 6d 97 84 7e csst3479(%r6),1150(%r8),%r3 >> 9d470a: c8 a2 70 11 74 02 csst17(%r7),1026(%r7),%r10 >> 9d6c4a: c8 a2 de 0c 54 4a csst3596(%r13),1098(%r5),%r10 >> 9e3af8: c8 a2 de 09 54 73 csst3593(%r13),1139(%r5),%r10 >> 9e3b02: c8 a2 de 0f 54 73 csst3599(%r13),1139(%r5),%r10 >> 9e7992: c8 a2 de 0c 54 d4 csst3596(%r13),1236(%r5),%r10 >> 9e79ea: c8 a2 40 f6 c3 0e csst246(%r4),782(%r12),%r10 >> 9e7e3c: c8 a2 40 6c d1 74 csst108(%r4),372(%r13),%r10 >> 9e8036: c8 a2 de 0d 54 cd csst3597(%r13),1229(%r5),%r10 >> 9e81ea: c8 a2 40 63 2f b8 csst99(%r4),4024(%r2),%r10 >> 9e81fe: c8 a2 de 0f 54 68 csst3599(%r13),1128(%r5),%r10 >> 9e8e10: c8 72 93 83 69 bd csst899(%r9),2493(%r6),%r7 >> 9e8ea4: c8 72 c6 04 54 63 csst1540(%r12),1123(%r5),%r7 >> 9e8eae: c8 72 c6 f1 98 77 csst1777(%r12),2167(%r9),%r7 >> 9e8ebc: c8 72 93 ba f5 07 csst954(%r9),1287(%r15),%r7 >> a0702e: c8 a2 14 74 6e 5f csst1140(%r1),3679(%r6),%r10 >> a083ea: c8 a2 73 08 74 da csst776(%r7),1242(%r7),%r10 >> a0ec06: c8 a2 f8 f6 c3 08 csst2294(%r15),776(%r12),%r10 >> a11890: c8 a2 f8 fa 5f ac csst2298(%r15),4012(%r5),%r10 >> a11b6e: c8 a2 73 0a 74 74 csst778(%r7),1140(%r7),%r10 >> a11be0: c8 a2 f8 fa 5f ac csst2298(%r15),4012(%r5),%r10 >> a11ef4: c8 a2 73 0b b3 7c csst779(%r7),892(%r11),%r10 >> [...] >> 14c9e5a: c8 02 d0 2d 00 0d csst45(%r13),13,%r0 >> >> >> That made me assume we have csst in the kernel :) > > you used -D (which also disassembles data). Do you still see any csst with > -d ? > ... probably I should have lunch now :) No csst, therefore the panic I am seeing is unrelated to this ... (I wonder why CSST is a required kernel facility if nobody uses it? hm) Thanks Christian! -- Thanks, David
Re: [Qemu-devel] [PATCH] target/s390x: Implement CSST
On 06/19/2017 02:41 PM, David Hildenbrand wrote: > On 19.06.2017 14:33, Christian Borntraeger wrote: >> On 06/19/2017 02:05 PM, David Hildenbrand wrote: >>> On 19.06.2017 12:03, David Hildenbrand wrote: On 15.06.2017 22:37, Richard Henderson wrote: > There are no uses in a Linux system with which to test, > but it Looks Right by my reading of the PoO. I am using next.git/master with this patch applied: https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/?h=features&id=8aa8680aa383bf6e2ac I am using QEMU with the mvcos patch and your patch applied (and a patch that allows enabling csst/csst2). I am using the following qemu command line: #!/bin/bash /home/dhildenb/git/qemu/s390x-softmmu/qemu-system-s390x \ -nographic -nodefaults -machine s390-ccw-virtio,accel=tcg \ -cpu qemu,mvcos=on,stfle=on,ldisp=on,ldisphp=on,\ eimm=on,stckf=on,csst=on,csst2=on,ginste=on,exrl=on\ -m 256M -smp 1 -chardev stdio,id=con0 \ -device sclpconsole,chardev=con0 \ -kernel vmlinux -initrd /home/dhildenb/initrd.debian Right now, I can start a z9 compiled kernel. When trying to start a z10 compiled kernel (which generates many csst), >> >> >> I would be very surprised if the kernel would contain any csst. gcc does not >> emit csst and the kernel source also does not contain it. >> > > I only did a grep on the objdump output: > > t460s: ~/git/linux-s390 next $ /usr/bin/s390x-linux-gnu-objdump -D > vmlinux | grep csst > 912826: c8 e2 f1 7c 56 02 csst380(%r15),1538(%r5),%r14 > 954684: c8 62 dc 35 2b 65 csst3125(%r13),2917(%r2),%r6 > 95e6e4: c8 a2 1c 76 0a 60 csst3190(%r1),2656,%r10 > 95f68a: c8 b2 c9 b3 3d 47 csst2483(%r12),3399(%r3),%r11 > 96067a: c8 42 d3 59 da 50 csst857(%r13),2640(%r13),%r4 > 963642: c8 72 73 c9 1a a0 csst969(%r7),2720(%r1),%r7 > 9656de: c8 12 d3 09 7a a0 csst777(%r13),2720(%r7),%r1 > 9676a6: c8 32 6d 97 84 7e csst3479(%r6),1150(%r8),%r3 > 9d470a: c8 a2 70 11 74 02 csst17(%r7),1026(%r7),%r10 > 9d6c4a: c8 a2 de 0c 54 4a csst3596(%r13),1098(%r5),%r10 > 9e3af8: c8 a2 de 09 54 73 csst3593(%r13),1139(%r5),%r10 > 9e3b02: c8 a2 de 0f 54 73 csst3599(%r13),1139(%r5),%r10 > 9e7992: c8 a2 de 0c 54 d4 csst3596(%r13),1236(%r5),%r10 > 9e79ea: c8 a2 40 f6 c3 0e csst246(%r4),782(%r12),%r10 > 9e7e3c: c8 a2 40 6c d1 74 csst108(%r4),372(%r13),%r10 > 9e8036: c8 a2 de 0d 54 cd csst3597(%r13),1229(%r5),%r10 > 9e81ea: c8 a2 40 63 2f b8 csst99(%r4),4024(%r2),%r10 > 9e81fe: c8 a2 de 0f 54 68 csst3599(%r13),1128(%r5),%r10 > 9e8e10: c8 72 93 83 69 bd csst899(%r9),2493(%r6),%r7 > 9e8ea4: c8 72 c6 04 54 63 csst1540(%r12),1123(%r5),%r7 > 9e8eae: c8 72 c6 f1 98 77 csst1777(%r12),2167(%r9),%r7 > 9e8ebc: c8 72 93 ba f5 07 csst954(%r9),1287(%r15),%r7 > a0702e: c8 a2 14 74 6e 5f csst1140(%r1),3679(%r6),%r10 > a083ea: c8 a2 73 08 74 da csst776(%r7),1242(%r7),%r10 > a0ec06: c8 a2 f8 f6 c3 08 csst2294(%r15),776(%r12),%r10 > a11890: c8 a2 f8 fa 5f ac csst2298(%r15),4012(%r5),%r10 > a11b6e: c8 a2 73 0a 74 74 csst778(%r7),1140(%r7),%r10 > a11be0: c8 a2 f8 fa 5f ac csst2298(%r15),4012(%r5),%r10 > a11ef4: c8 a2 73 0b b3 7c csst779(%r7),892(%r11),%r10 > [...] > 14c9e5a: c8 02 d0 2d 00 0d csst45(%r13),13,%r0 > > > That made me assume we have csst in the kernel :) you used -D (which also disassembles data). Do you still see any csst with -d ?
Re: [Qemu-devel] [PATCH] target/s390x: Implement CSST
On 19.06.2017 14:33, Christian Borntraeger wrote: > On 06/19/2017 02:05 PM, David Hildenbrand wrote: >> On 19.06.2017 12:03, David Hildenbrand wrote: >>> On 15.06.2017 22:37, Richard Henderson wrote: There are no uses in a Linux system with which to test, but it Looks Right by my reading of the PoO. >>> >>> I am using next.git/master with this patch applied: >>> https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/?h=features&id=8aa8680aa383bf6e2ac >>> >>> I am using QEMU with the mvcos patch and your patch applied (and a patch >>> that allows enabling csst/csst2). >>> >>> I am using the following qemu command line: >>> >>> #!/bin/bash >>> /home/dhildenb/git/qemu/s390x-softmmu/qemu-system-s390x \ >>> -nographic -nodefaults -machine s390-ccw-virtio,accel=tcg \ >>> -cpu qemu,mvcos=on,stfle=on,ldisp=on,ldisphp=on,\ >>> eimm=on,stckf=on,csst=on,csst2=on,ginste=on,exrl=on\ >>> -m 256M -smp 1 -chardev stdio,id=con0 \ >>> -device sclpconsole,chardev=con0 \ >>> -kernel vmlinux -initrd /home/dhildenb/initrd.debian >>> >>> Right now, I can start a z9 compiled kernel. >>> >>> When trying to start a z10 compiled kernel (which generates many csst), > > > I would be very surprised if the kernel would contain any csst. gcc does not > emit csst and the kernel source also does not contain it. > I only did a grep on the objdump output: t460s: ~/git/linux-s390 next $ /usr/bin/s390x-linux-gnu-objdump -D vmlinux | grep csst 912826: c8 e2 f1 7c 56 02 csst380(%r15),1538(%r5),%r14 954684: c8 62 dc 35 2b 65 csst3125(%r13),2917(%r2),%r6 95e6e4: c8 a2 1c 76 0a 60 csst3190(%r1),2656,%r10 95f68a: c8 b2 c9 b3 3d 47 csst2483(%r12),3399(%r3),%r11 96067a: c8 42 d3 59 da 50 csst857(%r13),2640(%r13),%r4 963642: c8 72 73 c9 1a a0 csst969(%r7),2720(%r1),%r7 9656de: c8 12 d3 09 7a a0 csst777(%r13),2720(%r7),%r1 9676a6: c8 32 6d 97 84 7e csst3479(%r6),1150(%r8),%r3 9d470a: c8 a2 70 11 74 02 csst17(%r7),1026(%r7),%r10 9d6c4a: c8 a2 de 0c 54 4a csst3596(%r13),1098(%r5),%r10 9e3af8: c8 a2 de 09 54 73 csst3593(%r13),1139(%r5),%r10 9e3b02: c8 a2 de 0f 54 73 csst3599(%r13),1139(%r5),%r10 9e7992: c8 a2 de 0c 54 d4 csst3596(%r13),1236(%r5),%r10 9e79ea: c8 a2 40 f6 c3 0e csst246(%r4),782(%r12),%r10 9e7e3c: c8 a2 40 6c d1 74 csst108(%r4),372(%r13),%r10 9e8036: c8 a2 de 0d 54 cd csst3597(%r13),1229(%r5),%r10 9e81ea: c8 a2 40 63 2f b8 csst99(%r4),4024(%r2),%r10 9e81fe: c8 a2 de 0f 54 68 csst3599(%r13),1128(%r5),%r10 9e8e10: c8 72 93 83 69 bd csst899(%r9),2493(%r6),%r7 9e8ea4: c8 72 c6 04 54 63 csst1540(%r12),1123(%r5),%r7 9e8eae: c8 72 c6 f1 98 77 csst1777(%r12),2167(%r9),%r7 9e8ebc: c8 72 93 ba f5 07 csst954(%r9),1287(%r15),%r7 a0702e: c8 a2 14 74 6e 5f csst1140(%r1),3679(%r6),%r10 a083ea: c8 a2 73 08 74 da csst776(%r7),1242(%r7),%r10 a0ec06: c8 a2 f8 f6 c3 08 csst2294(%r15),776(%r12),%r10 a11890: c8 a2 f8 fa 5f ac csst2298(%r15),4012(%r5),%r10 a11b6e: c8 a2 73 0a 74 74 csst778(%r7),1140(%r7),%r10 a11be0: c8 a2 f8 fa 5f ac csst2298(%r15),4012(%r5),%r10 a11ef4: c8 a2 73 0b b3 7c csst779(%r7),892(%r11),%r10 [...] 14c9e5a: c8 02 d0 2d 00 0d csst45(%r13),13,%r0 That made me assume we have csst in the kernel :) Thanks, David
Re: [Qemu-devel] [PATCH] target/s390x: Implement CSST
On 06/19/2017 02:05 PM, David Hildenbrand wrote: > On 19.06.2017 12:03, David Hildenbrand wrote: >> On 15.06.2017 22:37, Richard Henderson wrote: >>> There are no uses in a Linux system with which to test, >>> but it Looks Right by my reading of the PoO. >> >> I am using next.git/master with this patch applied: >> https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/?h=features&id=8aa8680aa383bf6e2ac >> >> I am using QEMU with the mvcos patch and your patch applied (and a patch >> that allows enabling csst/csst2). >> >> I am using the following qemu command line: >> >> #!/bin/bash >> /home/dhildenb/git/qemu/s390x-softmmu/qemu-system-s390x \ >> -nographic -nodefaults -machine s390-ccw-virtio,accel=tcg \ >> -cpu qemu,mvcos=on,stfle=on,ldisp=on,ldisphp=on,\ >> eimm=on,stckf=on,csst=on,csst2=on,ginste=on,exrl=on\ >> -m 256M -smp 1 -chardev stdio,id=con0 \ >> -device sclpconsole,chardev=con0 \ >> -kernel vmlinux -initrd /home/dhildenb/initrd.debian >> >> Right now, I can start a z9 compiled kernel. >> >> When trying to start a z10 compiled kernel (which generates many csst), I would be very surprised if the kernel would contain any csst. gcc does not emit csst and the kernel source also does not contain it.
Re: [Qemu-devel] [PATCH] target/s390x: Implement CSST
On 19.06.2017 12:03, David Hildenbrand wrote:
> On 15.06.2017 22:37, Richard Henderson wrote:
>> There are no uses in a Linux system with which to test,
>> but it Looks Right by my reading of the PoO.
>
> I am using next.git/master with this patch applied:
> https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/?h=features&id=8aa8680aa383bf6e2ac
>
> I am using QEMU with the mvcos patch and your patch applied (and a patch
> that allows enabling csst/csst2).
>
> I am using the following qemu command line:
>
> #!/bin/bash
> /home/dhildenb/git/qemu/s390x-softmmu/qemu-system-s390x \
> -nographic -nodefaults -machine s390-ccw-virtio,accel=tcg \
> -cpu qemu,mvcos=on,stfle=on,ldisp=on,ldisphp=on,\
> eimm=on,stckf=on,csst=on,csst2=on,ginste=on,exrl=on\
> -m 256M -smp 1 -chardev stdio,id=con0 \
> -device sclpconsole,chardev=con0 \
> -kernel vmlinux -initrd /home/dhildenb/initrd.debian
>
> Right now, I can start a z9 compiled kernel.
>
> When trying to start a z10 compiled kernel (which generates many csst),
> qemu simply crashes / the guests exits (have to debug) without any
> command line output.
>
> So either something in csst is broken or in the other instructions for z10.
>
With CONFIG_DEBUG_LOCKDEP I get a pgm exception in check_no_collision()
and the kernel can't find an exception table for it, resulting in a panic().
#0 0x001cf584 in check_no_collision (chain=,
hlock=, curr=)
at kernel/locking/lockdep.c:2111
#1 lookup_chain_cache (chain_key=, hlock=, curr=)
at kernel/locking/lockdep.c:2158
#2 validate_chain (curr=0xdad300 , hlock=0x18398c8
, chain_head=,
chain_key=10957502631322533163, lock=) at
kernel/locking/lockdep.c:2252
#3 0x001d089a in __lock_acquire (lock=,
subclass=, trylock=0, read=0,
check=1, hardirqs_off=, nest_lock=0x0, ip=15545474,
references=,
pin_count=) at kernel/locking/lockdep.c:3367
#4 0x001d15a6 in lock_acquire (lock=,
subclass=, trylock=,
read=, check=1, nest_lock=0x0, ip=15545474) at
kernel/locking/lockdep.c:3855
#5 0x009b76cc in __mutex_lock_common (use_ww_ctx=, ww_ctx=,
ip=, nest_lock=, subclass=, state=,
lock=) at kernel/locking/mutex.c:756
#6 __mutex_lock (lock=0xded840 , state=2, subclass=0,
nest_lock=0x0, ip=)
at kernel/locking/mutex.c:893
#7 0x009b801a in mutex_lock_nested (lock=,
subclass=)
at kernel/locking/mutex.c:908
#8 0x00ed3482 in cgroup_init_subsys (ss=0xdd2340
, early=true)
at kernel/cgroup/cgroup.c:4403
#9 0x00ed3752 in cgroup_init_early () at
kernel/cgroup/cgroup.c:4481
#10 0x00ebd79a in start_kernel () at init/main.c:502
#11 0x00100020 in _stext () at arch/s390/kernel/head64.S:100
1cf552: e3 10 c0 30 00 04 lg %r1,48(%r12)
1cf558: eb 11 00 33 00 0c srlg%r1,%r1,51
1cf55e: eb c1 00 05 00 0d sllg%r12,%r1,5
1cf564: b9 09 00 c1 sgr %r12,%r1
1cf568: eb cc 00 04 00 0d sllg%r12,%r12,4
1cf56e: e3 c0 d0 28 00 08 ag %r12,40(%r13)
1cf574: a7 f4 ff 6f j 1cf452
if (DEBUG_LOCKS_WARN_ON(chain->depth != curr->lockdep_depth - (i
- 1))) {
1cf578: c0 30 00 4c cc e3 larl%r3,b68f3e
1cf57e: c0 20 00 4c 95 37 larl%r2,b61fec
1cf584: c0 e5 00 07 f1 2e brasl %r14,2cd7e0
1cf58a: a7 f4 00 01 j 1cf58c
1cf58e: a7 f4 fc d9 j 1cef40
Without CONFIG_DEBUG_LOCKDEP:
I get a similar crash in static void __init mm_init(void).
Not sure yet what the real root cause is. Maybe something not related to
CSST.
--
Thanks,
David
Re: [Qemu-devel] [PATCH] target/s390x: Implement CSST
On 15.06.2017 22:37, Richard Henderson wrote: > There are no uses in a Linux system with which to test, > but it Looks Right by my reading of the PoO. I am using next.git/master with this patch applied: https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/?h=features&id=8aa8680aa383bf6e2ac I am using QEMU with the mvcos patch and your patch applied (and a patch that allows enabling csst/csst2). I am using the following qemu command line: #!/bin/bash /home/dhildenb/git/qemu/s390x-softmmu/qemu-system-s390x \ -nographic -nodefaults -machine s390-ccw-virtio,accel=tcg \ -cpu qemu,mvcos=on,stfle=on,ldisp=on,ldisphp=on,\ eimm=on,stckf=on,csst=on,csst2=on,ginste=on,exrl=on\ -m 256M -smp 1 -chardev stdio,id=con0 \ -device sclpconsole,chardev=con0 \ -kernel vmlinux -initrd /home/dhildenb/initrd.debian Right now, I can start a z9 compiled kernel. When trying to start a z10 compiled kernel (which generates many csst), qemu simply crashes / the guests exits (have to debug) without any command line output. So either something in csst is broken or in the other instructions for z10. -- Thanks, David
Re: [Qemu-devel] [PATCH] target/s390x: Implement CSST
On 15.06.2017 22:37, Richard Henderson wrote:
> There are no uses in a Linux system with which to test,
> but it Looks Right by my reading of the PoO.
>
> Signed-off-by: Richard Henderson
> ---
> target/s390x/helper.h | 1 +
> target/s390x/insn-data.def | 2 +
> target/s390x/mem_helper.c | 189
> +
> target/s390x/translate.c | 13 +++-
> 4 files changed, 204 insertions(+), 1 deletion(-)
>
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index b268367..456aaa9 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -33,6 +33,7 @@ DEF_HELPER_3(celgb, i64, env, i64, i32)
> DEF_HELPER_3(cdlgb, i64, env, i64, i32)
> DEF_HELPER_3(cxlgb, i64, env, i64, i32)
> DEF_HELPER_4(cdsg, void, env, i64, i32, i32)
> +DEF_HELPER_4(csst, i32, env, i32, i64, i64)
> DEF_HELPER_FLAGS_3(aeb, TCG_CALL_NO_WG, i64, env, i64, i64)
> DEF_HELPER_FLAGS_3(adb, TCG_CALL_NO_WG, i64, env, i64, i64)
> DEF_HELPER_FLAGS_5(axb, TCG_CALL_NO_WG, i64, env, i64, i64, i64, i64)
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index aa4c5b2..ef02a8e 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -256,6 +256,8 @@
> D(0xbb00, CDS, RS_a, Z, r3_D32, r1_D32, new, r1_D32, cs, 0,
> MO_TEQ)
> D(0xeb31, CDSY,RSY_a, LD, r3_D32, r1_D32, new, r1_D32, cs, 0,
> MO_TEQ)
> C(0xeb3e, CDSG,RSY_a, Z, 0, 0, 0, 0, cdsg, 0)
> +/* COMPARE AND SWAP AND STORE */
> +C(0xc802, CSST,SSF, CASS, la1, a2, 0, 0, csst, 0)
>
> /* COMPARE AND TRAP */
> D(0xb972, CRT, RRF_c, GIE, r1_32s, r2_32s, 0, 0, ct, 0, 0)
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 6125725..4a7d770 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -1344,6 +1344,195 @@ void HELPER(cdsg)(CPUS390XState *env, uint64_t addr,
> env->regs[r1 + 1] = int128_getlo(oldv);
> }
>
> +uint32_t HELPER(csst)(CPUS390XState *env, uint32_t r3, uint64_t a1, uint64_t
> a2)
> +{
> +#if !defined(CONFIG_USER_ONLY) || defined(CONFIG_ATOMIC128)
> +uint32_t mem_idx = cpu_mmu_index(env, false);
> +#endif
> +uintptr_t ra = GETPC();
> +uint32_t fc = extract32(env->regs[0], 0, 8);
> +uint32_t sc = extract32(env->regs[0], 8, 8);
> +uint64_t pl = get_address(env, 1) & -16;
> +uint64_t svh, svl;
> +uint32_t cc;
> +
> +/* Sanity check the function code and storage characteristic. */
> +if (fc > 1 || sc > 3) {
> +if (!s390_has_feat(S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2)) {
> +goto spec_exception;
> +}
> +if (fc > 2 || sc > 4 || (fc == 2 && (r3 & 1))) {
I think you could omit the "fc == 2" here. fc has to be bigger than 1
due to the outer if-statement, and if it is not 2, the first "fc > 1"
has already triggered. So "fc" has to be 2 here and the "fc == 2" is a
redundant check.
Thomas
[Qemu-devel] [PATCH] target/s390x: Implement CSST
There are no uses in a Linux system with which to test,
but it Looks Right by my reading of the PoO.
Signed-off-by: Richard Henderson
---
target/s390x/helper.h | 1 +
target/s390x/insn-data.def | 2 +
target/s390x/mem_helper.c | 189 +
target/s390x/translate.c | 13 +++-
4 files changed, 204 insertions(+), 1 deletion(-)
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index b268367..456aaa9 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -33,6 +33,7 @@ DEF_HELPER_3(celgb, i64, env, i64, i32)
DEF_HELPER_3(cdlgb, i64, env, i64, i32)
DEF_HELPER_3(cxlgb, i64, env, i64, i32)
DEF_HELPER_4(cdsg, void, env, i64, i32, i32)
+DEF_HELPER_4(csst, i32, env, i32, i64, i64)
DEF_HELPER_FLAGS_3(aeb, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_3(adb, TCG_CALL_NO_WG, i64, env, i64, i64)
DEF_HELPER_FLAGS_5(axb, TCG_CALL_NO_WG, i64, env, i64, i64, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index aa4c5b2..ef02a8e 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -256,6 +256,8 @@
D(0xbb00, CDS, RS_a, Z, r3_D32, r1_D32, new, r1_D32, cs, 0, MO_TEQ)
D(0xeb31, CDSY,RSY_a, LD, r3_D32, r1_D32, new, r1_D32, cs, 0, MO_TEQ)
C(0xeb3e, CDSG,RSY_a, Z, 0, 0, 0, 0, cdsg, 0)
+/* COMPARE AND SWAP AND STORE */
+C(0xc802, CSST,SSF, CASS, la1, a2, 0, 0, csst, 0)
/* COMPARE AND TRAP */
D(0xb972, CRT, RRF_c, GIE, r1_32s, r2_32s, 0, 0, ct, 0, 0)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 6125725..4a7d770 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1344,6 +1344,195 @@ void HELPER(cdsg)(CPUS390XState *env, uint64_t addr,
env->regs[r1 + 1] = int128_getlo(oldv);
}
+uint32_t HELPER(csst)(CPUS390XState *env, uint32_t r3, uint64_t a1, uint64_t
a2)
+{
+#if !defined(CONFIG_USER_ONLY) || defined(CONFIG_ATOMIC128)
+uint32_t mem_idx = cpu_mmu_index(env, false);
+#endif
+uintptr_t ra = GETPC();
+uint32_t fc = extract32(env->regs[0], 0, 8);
+uint32_t sc = extract32(env->regs[0], 8, 8);
+uint64_t pl = get_address(env, 1) & -16;
+uint64_t svh, svl;
+uint32_t cc;
+
+/* Sanity check the function code and storage characteristic. */
+if (fc > 1 || sc > 3) {
+if (!s390_has_feat(S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2)) {
+goto spec_exception;
+}
+if (fc > 2 || sc > 4 || (fc == 2 && (r3 & 1))) {
+goto spec_exception;
+}
+}
+
+/* Sanity check the alignments. */
+if (extract32(a1, 0, 4 << fc) || extract32(a2, 0, 1 << sc)) {
+goto spec_exception;
+}
+
+/* Sanity check writability of the store address. */
+#ifndef CONFIG_USER_ONLY
+probe_write(env, a2, mem_idx, ra);
+#endif
+
+/* Note that the compare-and-swap is atomic, and the store is atomic, but
+ the complete operation is not. Therefore we do not need to assert
serial
+ context in order to implement this. That said, restart early if we
can't
+ support either operation that is supposed to be atomic. */
+if (parallel_cpus) {
+int mask = 0;
+#if !defined(CONFIG_ATOMIC64)
+mask = -8;
+#elif !defined(CONFIG_ATOMIC128)
+mask = -16;
+#endif
+if (((4 << fc) | (1 << sc)) & mask) {
+cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
+}
+}
+
+/* All loads happen before all stores. For simplicity, load the entire
+ store value area from the parameter list. */
+svh = cpu_ldq_data_ra(env, pl + 16, ra);
+svl = cpu_ldq_data_ra(env, pl + 24, ra);
+
+switch (fc) {
+case 0:
+{
+uint32_t nv = cpu_ldl_data_ra(env, pl, ra);
+uint32_t cv = env->regs[r3];
+uint32_t ov;
+
+if (parallel_cpus) {
+#ifdef CONFIG_USER_ONLY
+uint32_t *haddr = g2h(a1);
+ov = atomic_cmpxchg__nocheck(haddr, cv, nv);
+#else
+TCGMemOpIdx oi = make_memop_idx(MO_TEUL | MO_ALIGN, mem_idx);
+ov = helper_atomic_cmpxchgl_be_mmu(env, a1, cv, nv, oi, ra);
+#endif
+} else {
+ov = cpu_ldl_data_ra(env, a1, ra);
+cpu_stl_data_ra(env, a1, (ov == cv ? nv : ov), ra);
+}
+cc = (ov != cv);
+env->regs[r3] = deposit64(env->regs[r3], 32, 32, ov);
+}
+break;
+
+case 1:
+{
+uint64_t nv = cpu_ldq_data_ra(env, pl, ra);
+uint64_t cv = env->regs[r3];
+uint64_t ov;
+
+if (parallel_cpus) {
+#ifdef CONFIG_USER_ONLY
+# ifdef CONFIG_ATOMIC64
+uint64_t *haddr = g2h(a1);
+ov = atomic_cmpxchg__nocheck(haddr, cv, nv);
+# else
+/* Note that we asserted !parallel_cpus above. */
+g_assert_not_reached();
+# endif
+#else
+TCGMemOpIdx oi = make_memo
