On 17.12.18 14:10, Janosch Frank wrote: > 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. > Okay, I#ll leave this up to your patches and only memset the struct to zero in the new function I introduce. -- Thanks, David / dhildenb
