On 17.12.18 13:43, David Hildenbrand wrote: > On 17.12.18 12:19, Janosch Frank wrote: >> On 17.12.18 11:16, David Hildenbrand wrote: >>> Looking at what the kernel does, looks like we should >>> - Check for more errors >>> - Try to execute !FORCED read if the FORCED one is unknown >>> - Set the function code to a normal write >>> - Set control_mask[2] to SCLP_VARIABLE_LENGTH_RESPONSE >>> >>> The kernel seems to set the function code to 0x80. Looking at where that >>> comes from, turns out this is a "dump indicator" to produce more >>> meaningful dumps. Looking for other function codes used in the kernel, I >>> discovered 0x40, which is used for "single increment assign" to speed up >>> memory hotplug. I was not able to find out what "control_mask[2] = 0x80" >>> means. >> >> We figured out what control_mask[2] = 0x80 is, so you can drop that. >> Maybe rephrase that anyway and put it to the second to last paragraph. >> > > Does it actually make sense to set 0x80 at all or should we simply drop it? I don't think we need it, we were fine before without it. >> >>> >>> Tested-by: Janosch Frank <frankja@xxxxxxxxxxxxx> >>> Reviewed-by: Janosch Frank <frankja@xxxxxxxxxxxxx> >>> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx> >>> --- >>> >>> v1 -> v2: >>> - Use define for control mask value 0x80 >>> - Rename SCLP_VARIABLE_LENGTH_RESPONSE to SCLP_CM2_VARIABLE_LENGTH_RESPONSE >>> - Cleanup sclp.h header file a bit (move to sections) >>> - Make sure control mask and function code are in a deterministic state >>> when issuing sclp_service_call >>> >>> lib/s390x/sclp-ascii.c | 7 +++++++ >>> lib/s390x/sclp.c | 26 ++++++++++++++++++++++++-- >>> lib/s390x/sclp.h | 13 +++++++++---- >>> 3 files changed, 40 insertions(+), 6 deletions(-) >>> >>> diff --git a/lib/s390x/sclp-ascii.c b/lib/s390x/sclp-ascii.c >>> index 893ca17..344a414 100644 >>> --- a/lib/s390x/sclp-ascii.c >>> +++ b/lib/s390x/sclp-ascii.c >>> @@ -38,6 +38,10 @@ static void sclp_set_write_mask(void) >>> WriteEventMask *sccb = (void *)_sccb; >>> >>> sccb->h.length = sizeof(WriteEventMask); >>> + sccb->h.function_code = SCLP_FC_NORMAL_WRITE; >>> + sccb->h.control_mask[0] = 0; >>> + sccb->h.control_mask[1] = 0; >>> + sccb->h.control_mask[2] = 0; >> >> I opted for a memset and I would prefer it here too, as I'll introduce >> it anyway. > > Make sense, but only for the mask and not the whole header I guess? I memset the whole struct.
Attachment:
signature.asc
Description: OpenPGP digital signature
