Re: [PATCH 4/4] KVM: s390: Add a channel I/O based virtio transport driver.

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 13/12/12 12:19, Alexander Graf wrote:
> 
> On 13.12.2012, at 12:13, Christian Borntraeger wrote:
> 
>> On 13/12/12 12:06, Alexander Graf wrote:
>>>
>>> On 12.12.2012, at 21:12, Christian Borntraeger wrote:
>>>
>>>> On 12/12/12 19:31, Alexander Graf wrote:
>>>> +static inline long do_kvm_notify(struct subchannel_id schid,
>>>>>> +                 unsigned long queue_index)
>>>>>> +{
>>>>>> +    register unsigned long __nr asm("1") = KVM_S390_VIRTIO_CCW_NOTIFY;
>>>>>> +    register struct subchannel_id __schid asm("2") = schid;
>>>>>
>>>>> How does the compiler know that the struct fits within a register?
>>>>>
>>>>
>>>> Well it knows that sizeof struct subchannel_id is 4.
>>>
>>> How so? The asm/ccwdev.h header simply says
>>>
>>>  struct subchannel_id;
>>
>> see original patch:
>>
>>>>> +#include <asm/cio.h>
>>>>> +#include <asm/ccwdev.h>
>>>>> +#include <asm/schid.h>
>>>>> +
>>>>> +/*
>>>>> + * virtio related functions
>>>>> + */
>>>>> +
>>
>> asm/schid.h includes uapi/asm/schid.h and that contains the definition.
> 
> My point is that the asm/schid.h include should be in asm/ccwdev.h then, not in virtio_ccw.c. Because semantically you are creating a dependency due to the new function that returns a struct schid.

Ok, so you actually want to have patch 3 slightly modified as described above.
Yes makes sense.

Christian