mbox series

[kvm-unit-tests,0/5] s390x: Dirty cc before executing tested instructions

Message ID 20240108132921.255769-1-frankja@linux.ibm.com (mailing list archive)
Headers show
Series s390x: Dirty cc before executing tested instructions | expand

Message

Janosch Frank Jan. 8, 2024, 1:29 p.m. UTC 
A recent s390 KVM fixpatch [1] showed us that checking the cc is not
enough when emulation code forgets to set the cc. There might just be
the correct cc in the PSW which would make the cc check succeed.

This series intentionally dirties the cc for sigp, uvc, some io
instructions and sclp to make cc setting errors more apparent. I had a
cursory look through the tested instructions and those are the most
prominent ones with defined cc values.

Since the issue appeared in PQAP my AP test series is now dependent on
this series.

[1] https://lore.kernel.org/kvm/20231201181657.1614645-1-farman@linux.ibm.com/

Janosch Frank (5):
  lib: s390x: Add spm cc shift constant
  lib: s390x: sigp: Dirty CC before sigp execution
  lib: s390x: uv: Dirty CC before uvc execution
  lib: s390x: css: Dirty CC before css instructions
  s390x: sclp: Dirty CC before sclp execution

 lib/s390x/asm/arch_def.h |  2 ++
 lib/s390x/asm/sigp.h     |  6 +++++-
 lib/s390x/asm/uv.h       |  6 +++++-
 lib/s390x/css.h          | 18 ++++++++++++++----
 s390x/mvpg.c             |  6 ++++--
 s390x/sclp.c             |  5 ++++-
 6 files changed, 34 insertions(+), 9 deletions(-)

Comments

Nina Schoetterl-Glausch Jan. 16, 2024, 1:02 p.m. UTC | #1 
On Mon, 2024-01-08 at 13:29 +0000, Janosch Frank wrote:
> A recent s390 KVM fixpatch [1] showed us that checking the cc is not
> enough when emulation code forgets to set the cc. There might just be
> the correct cc in the PSW which would make the cc check succeed.
> 
> This series intentionally dirties the cc for sigp, uvc, some io
> instructions and sclp to make cc setting errors more apparent. I had a
> cursory look through the tested instructions and those are the most
> prominent ones with defined cc values.
> 
> Since the issue appeared in PQAP my AP test series is now dependent on
> this series.
> 
> [1] https://lore.kernel.org/kvm/20231201181657.1614645-1-farman@linux.ibm.com/

Using SET PROGRAM MASK the way you're doing in this series will also set the
program mask to 0, right?

In case you have some non zero register %[reg] and you want to set CC to 1 you
could do:

or	%[reg],%[reg] /* set CC to 1 */

In general, if I understand TEST UNDER MASK right, you could do:

tmll	%[set_cc],3

to set the CC to the value in %[set_cc] (without any shifting).
Janosch Frank Jan. 17, 2024, 2:21 p.m. UTC | #2 
On 1/16/24 14:02, Nina Schoetterl-Glausch wrote:
> On Mon, 2024-01-08 at 13:29 +0000, Janosch Frank wrote:
>> A recent s390 KVM fixpatch [1] showed us that checking the cc is not
>> enough when emulation code forgets to set the cc. There might just be
>> the correct cc in the PSW which would make the cc check succeed.
>>
>> This series intentionally dirties the cc for sigp, uvc, some io
>> instructions and sclp to make cc setting errors more apparent. I had a
>> cursory look through the tested instructions and those are the most
>> prominent ones with defined cc values.
>>
>> Since the issue appeared in PQAP my AP test series is now dependent on
>> this series.
>>
>> [1] https://lore.kernel.org/kvm/20231201181657.1614645-1-farman@linux.ibm.com/
> 
> Using SET PROGRAM MASK the way you're doing in this series will also set the
> program mask to 0, right?
> 
> In case you have some non zero register %[reg] and you want to set CC to 1 you
> could do:
> 
> or	%[reg],%[reg] /* set CC to 1 */
> 
> In general, if I understand TEST UNDER MASK right, you could do:
> 
> tmll	%[set_cc],3
> 
> to set the CC to the value in %[set_cc] (without any shifting).


That is a wonderful solution to this problem.
I'll send out a new version in the next couple of days.