On 15/10/2020 14.40, Janosch Frank wrote:
> On 10/14/20 9:27 PM, Collin Walling wrote:
>> The DIAGNOSE 0x0318 instruction, unique to s390x, is a privileged call
>> that must be intercepted via SIE, handled in userspace, and the
>> information set by the instruction is communicated back to KVM.
>
> It might be nice to have a few words in here about what information can
> be set via the diag.
>
>>
>> To test the instruction interception, an ad-hoc handler is defined which
>> simply has a VM execute the instruction and then userspace will extract
>> the necessary info. The handler is defined such that the instruction
>> invocation occurs only once. It is up the the caller to determine how the
>> info returned by this handler should be used.
>>
>> The diag318 info is communicated from userspace to KVM via a sync_regs
>> call. This is tested during a sync_regs test, where the diag318 info is
>> requested via the handler, then the info is stored in the appropriate
>> register in KVM via a sync registers call.
>>
>> The diag318 info is checked to be 0 after a normal and clear reset.
>>
>> If KVM does not support diag318, then the tests will print a message
>> stating that diag318 was skipped, and the asserts will simply test
>> against a value of 0.
>>
>> Signed-off-by: Collin Walling <walling@xxxxxxxxxxxxx>
>
> Checkpatch throws lots of errors on this patch.
> Could you check if my workflow misteriously introduced windows line
> endings or if they were introduced on your side?
How did you feed the patch into checkpatch? IIRC mails are often sent with
CR-LF line endings by default - it's "git am" that is converting the line
endings back to the Unix default. So for running a patch through checkpatch,
you might need to do "git am" first and then export it again.
>> +uint64_t get_diag318_info(void)
>> +{
>> + static uint64_t diag318_info;
>> + static bool printed_skip;
>> +
>> + /*
>> + * If KVM does not support diag318, then return 0 to
>> + * ensure tests do not break.
>> + */
>> + if (!kvm_check_cap(KVM_CAP_S390_DIAG318)) {
>> + if (!printed_skip) {
>> + fprintf(stdout, "KVM_CAP_S390_DIAG318 not supported. "
>
> Whitespace after .
>
>> + "Skipping diag318 test.\n");
It's a multi-line text, so the whitespace is needed, isn't it?
>> + printed_skip = true;
>> + }
>> + return 0;
>> + }
Thomas
