Access to the Message-Security-Assist-3 (MSA-3) Wrapping Keys does not need to be guarded by read-write locks.

[srorso]

The use of read-write locks ensures read integrity for the wrapping keys. But the timing of wrapping key updates eliminates any read integrity issues.

A Clear Reset is the only event described in the PoOp that recreates the wrapping keys (SA22-78732-11, p. 4-79 right column item 7), and when a Clear Reset is performed, either alone or as part of IPL Clear, all CPUs in the configuration are included in the Clear Reset (Figure 4-22 Manual Initiation of Resets on p. 4.75). Which means they are stopped and cleared.

Although not mentioned, I presume wrapping keys are generated when a configuration is first created on real hardware, perhaps as part of a CPU Power-On Reset when that reset creates a configuration.

Hercules-390 Hyperion recreates wrapping keys anytime any CPU in the configuration is subjected to an Initial CPU Reset. The sysclear and sysreset clear commands include an Initial CPU Reset for each processor in the configuration, among other actions.

The extra regeneration of wrapping keys is not a big issue. But doing the regeneration as part of Initial CPU Reset creates the possibility that the wrapping keys will change while other CPUs in the configuration are running, which creates the requirement for read integrity and the locks that enable same. Emulation of every Cipher Message instruction (any flavor) that uses an encrypted key in the parameter block incurs the overhead of getting and releasing the read lock.

The situation is worse on Windows because fthreads.c does not support read-write locks. Such locks are converted to simple mutex's, and Cipher Message instructions with encrypted keys are effectively single-threaded.

The following changes will eliminate the need for read-write locks or mutex's for Windows to ensure wrapping key read integrity.

1. Remove wrapping key regeneration from routine initial_cpu_reset() in ipl.c.
2. Add wrapping key regeneration to system_reset() in ipl.c in the section that performs functions specific to Clear Reset after all processors in the configuration have stopped and completed Initial CPU Reset. Look for the comment Perform system-reset-clear additional functions at or about line 240.
3. Add wrapping key regeneration to routine impl.c in file impl.c in place of the initialization of the read-write lock for the wrapping keys (at or about line 785; look for #ifdef FEATURE_MESSAGE_SECURITY_ASSIST_EXTENSION_3.

MSA-3 is the only use of read-write locks; removal of this use of read-write locks would also enable removal of all read-write lock code from Hercules-390 Hyperion.

Fish-git Hyperion is already coded this way but still uses read-write locks or mutex's on Windows when accessing/updating the wrapping keys.

This is likely not a big deal, but it does mean that effort expended adding read-write lock support to fthreads.c might be better spent on eliminating the requirement for read-write lock support.

[Fish-Git]

Fish-git Hyperion is already coded this way but still uses read-write locks or mutex's on Windows when accessing/updating the wrapping keys.

Which, as you just explained, are not actually needed. (It was one of the things I happened to notice myself but forgot to change.) I have now added removing it to my TODO list.

This is likely not a big deal, but it does mean that effort expended adding read-write lock support to fthreads.c might be better spent on eliminating the requirement for read-write lock support.

For this particular, specific issue perhaps, but I don't think I would agree to this as a general statement regarding Hercules's need for read-write lock support however.

While I haven't yet taken the time to review/audit all of Hercules's lock usage, I suspect there is probably more than one place where code is simply wanting read-only access to some particular lock-protected field or structure, and use of a read-only lock in such situations would be the technically correct thing to do (and depending on what field we're talking about might actually help improve Hercules's performance somewhat). The device lock and mainlock are two particular locks which immediately spring to mind as likely candidates for r/w lock support.

But again, I have not had time yet to review Hercules's lock mangement to determine whether what I've written above is true or not. That is something that is also on my long TODO list and one which I will eventually get around to doing SOME day.

But thank you for creating this issue, Steve. It has reminded me to do something I intended to do but forgot completely about.

[srorso]

Hi Fish,

Thanks for your comments. I had not thought about the possibility that there might be other areas in Hercules where rwlocks might make more sense but mutex's are used for historical reasons. And your approach (so far) in your project--first mess with wrapping key creation timing, then mess with the locking--makes perfect sense.

Best Regards, Steve Orso