Re: [PATCH 02/12] KVM: s390: define GISA format-0 data structure

Michael Mueller
2018-01-18T15:49:48Z

On 17.01.18 08:57, Heiko Carstens wrote:
> On Tue, Jan 16, 2018 at 09:25:12PM +0100, David Hildenbrand wrote:
>>
>>> +struct kvm_s390_gisa {
>>> + u32 next_alert;
>>> + u8 ipm;
>>> + u8 reserved01;
>>> + u8:6;
>>> + u8 g:1;
>>> + u8 c:1;
>>> + u8 iam;
>>> + u8 reserved02[4];
>>> + u32 airq_count;
>>> +};
>>> +
>>> /*
>>> - * sie_page2 has to be allocated as DMA because fac_list and crycb need
>>> - * 31bit addresses in the sie control block.
>>> + * sie_page2 has to be allocated as DMA because fac_list, crycb and
>>> + * gisa need 31bit addresses in the sie control block.
>>> */
>>> struct sie_page2 {
>>> __u64 fac_list[S390_ARCH_FAC_LIST_SIZE_U64]; /* 0x0000 */
>>> struct kvm_s390_crypto_cb crycb; /* 0x0800 */
>>> - u8 reserved900[0x1000 - 0x900]; /* 0x0900 */
>>> + struct kvm_s390_gisa gisa __aligned(sizeof(struct kvm_s390_gisa)); /* 0x0900 */
>>
>> can we instead specify sie_page2 as packed if really needed? (I can see
>> that we have a BUILD_BUG_ON below, which is nice)
>>
>> (alignment within well defined data structures looks strange)

I'm doing that because the gisa requires "integral boundary" alignment
that changes with a later patch when I introduce format-1.

>
> The alignment, if needed, should go to the definition of struct
> kvm_s390_gisa above. The structure needs at least an alignment of eight
> bytes since bitops are used to modify ipm (in a later patch), by using a
> (long *) cast to this structure.

adding this to the definition does not allow to use sizeof() for the
currently defined struct. Thus I would have to explicitly specify the
size... :(

> But of course it is all naturally aligned anyway... ;)
>

Thanks
Michael