On 12/17/20 3:31 PM, Janosch Frank wrote:
> On 12/17/20 11:34 AM, Thomas Huth wrote:
>> On 17/12/2020 10.59, Christian Borntraeger wrote:
>>>
>>>
>>> On 17.12.20 10:53, Thomas Huth wrote:
>>>> On 11/12/2020 11.00, Janosch Frank wrote:
>>>>> Not much to test except for the privilege and specification
>>>>> exceptions.
>>>>>
>>>>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>>>>> Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>
>>>>> ---
>>>>> lib/s390x/sclp.c | 2 ++
>>>>> lib/s390x/sclp.h | 6 +++++-
>>>>> s390x/intercept.c | 19 +++++++++++++++++++
>>>>> 3 files changed, 26 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
>>>>> index cf6ea7c..0001993 100644
>>>>> --- a/lib/s390x/sclp.c
>>>>> +++ b/lib/s390x/sclp.c
>>>>> @@ -138,6 +138,8 @@ void sclp_facilities_setup(void)
>>>>>
>>>>> assert(read_info);
>>>>>
>>>>> + sclp_facilities.has_diag318 = read_info->byte_134_diag318;
>>>>> +
>>>>> cpu = (void *)read_info + read_info->offset_cpu;
>>>>> for (i = 0; i < read_info->entries_cpu; i++, cpu++) {
>>>>> if (cpu->address == cpu0_addr) {
>>>>> diff --git a/lib/s390x/sclp.h b/lib/s390x/sclp.h
>>>>> index 6c86037..58f8e54 100644
>>>>> --- a/lib/s390x/sclp.h
>>>>> +++ b/lib/s390x/sclp.h
>>>>> @@ -105,7 +105,8 @@ extern struct sclp_facilities sclp_facilities;
>>>>>
>>>>> struct sclp_facilities {
>>>>> uint64_t has_sief2 : 1;
>>>>> - uint64_t : 63;
>>>>> + uint64_t has_diag318 : 1;
>>>>> + uint64_t : 62;
>>>>> };
>>>>>
>>>>> typedef struct ReadInfo {
>>>>> @@ -130,6 +131,9 @@ typedef struct ReadInfo {
>>>>> uint16_t highest_cpu;
>>>>> uint8_t _reserved5[124 - 122]; /* 122-123 */
>>>>> uint32_t hmfai;
>>>>> + uint8_t reserved7[134 - 128];
>>>>> + uint8_t byte_134_diag318 : 1;
>>>>> + uint8_t : 7;
>>>>> struct CPUEntry entries[0];
>>>>
>>>> ... the entries[] array can be moved around here without any further ado?
>>>> Looks confusing to me. Should there be a CPUEntry array here at all, or only
>>>> in ReadCpuInfo?
>>>
>>> there is offset_cpu for the cpu entries at the beginning of the structure.
>>
>> Ah, thanks, right, this was used earlier in the patch series, now I
>> remember. But I think the "struct CPUEntry entries[0]" here is rather
>> confusing, since there is no guarantee that the entries are really at this
>> location ... I think this line should rather be replaced by a comment saying
>> that offset_cpu should be used instead.
>
> Sure, as long as it's clear that there's something at the end, I'm fine
> with it.
I would add that to the "fix style issues" patch or into an own patch.
Any preferences?
- struct CPUEntry entries[0];
+ /*
+ * The cpu entries follow, they start at the offset specified
+ * in offset_cpu.
+ */
>
>>
>> Thomas
>>
>
