On 14.12.18 18:24, Janosch Frank wrote:
> On 14.12.18 11:06, 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 0x80 (no idea what that means)
>
> It's great that they set that and never actually check for its effects...
>
> It's (drum roll):
> SCLP_VARIABLE_LENGTH_RESPONSE
Right, it's used for that purpose in QEMU. I'll do some changes in this
patch
- Rename SCLP_VARIABLE_LENGTH_RESPONSE to
SCLP_CM2_VARIABLE_LENGTH_RESPONSE and use it
- Move the SCLP_FC_* to a dedicated section in the header
- Set the control_mask and function_code for every sclp_service_call()
user to deterinistic values (0)
>
>>
>> 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.
>>
>> I guess we can just set the funcion code to 0 ("normal write). Let's
>> document the other bits in case anybody ever wonders what they mean.
>> Also, let's just set the other bits in the mask to 0, not sure if they
>> will actually be used, but this is definetly not wrong.
>>
>> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
>> ---
>
> I rebased my series on top of this patch and it works under LPAR:
> Tested-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>
> Please update the description, but apart from that:
> Reviewed-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
Thanks, I'll resend with the mentioned changes. We can than decide if
you want to carry this along or if Thomas wants to queue this right away.
>
> [...]
>> +static void sclp_read_scp_info(ReadInfo *ri, int length)
>> +{
>> + unsigned int commands[] = { SCLP_CMDW_READ_SCP_INFO_FORCED,
>> + SCLP_CMDW_READ_SCP_INFO };
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(commands); i++) {
>> + ri->h.length = length;
>> + ri->h.function_code = SCLP_FC_NORMAL_WRITE;
>> + ri->h.control_mask[0] = 0;
>> + ri->h.control_mask[1] = 0;
>> + ri->h.control_mask[2] = 0x80;
>
> We could test for the SCLP_RC_INSUFFICIENT_SCCB_LENGTH and redrive with
> a longer response space because we set SCLP_VARIABLE_LENGTH_RESPONSE but
> 4k should be enough for tests. I do not expect this to be executed with
> a few hundred VCPUs anyway.
Indeed, I think we can ignore that for now.
Thanks!
--
Thanks,
David / dhildenb
