Re: [PATCH v2 6/8] virtio/s390: add indirection to indicators access

Michael Mueller
2019-05-29T11:05:41Z

On 27.05.19 14:10, Cornelia Huck wrote:
> On Mon, 27 May 2019 13:57:06 +0200
> Halil Pasic <pasic@linux.ibm.com> wrote:
>
>> On Mon, 27 May 2019 13:00:28 +0200
>> Cornelia Huck <cohuck@redhat.com> wrote:
>>
>>> On Thu, 23 May 2019 18:22:07 +0200
>>> Michael Mueller <mimu@linux.ibm.com> wrote:
>>>
>>>> From: Halil Pasic <pasic@linux.ibm.com>
>>>>
>>>> This will come in handy soon when we pull out the indicators from
>>>> virtio_ccw_device to a memory area that is shared with the hypervisor
>>>> (in particular for protected virtualization guests).
>>>>
>>>> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
>>>> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>> drivers/s390/virtio/virtio_ccw.c | 40 +++++++++++++++++++++++++---------------
>>>> 1 file changed, 25 insertions(+), 15 deletions(-)
>>>>
>>>
>>>> @@ -338,17 +348,17 @@ static void virtio_ccw_drop_indicator(struct virtio_ccw_device *vcdev,
>>>> ccw->cda = (__u32)(unsigned long) thinint_area;
>>>> } else {
>>>> /* payload is the address of the indicators */
>>>> - indicatorp = kmalloc(sizeof(&vcdev->indicators),
>>>> + indicatorp = kmalloc(sizeof(indicators(vcdev)),
>>>> GFP_DMA | GFP_KERNEL);
>>>> if (!indicatorp)
>>>> return;
>>>> *indicatorp = 0;
>>>> ccw->cmd_code = CCW_CMD_SET_IND;
>>>> - ccw->count = sizeof(&vcdev->indicators);
>>>> + ccw->count = sizeof(indicators(vcdev));
>>>> ccw->cda = (__u32)(unsigned long) indicatorp;
>>>> }
>>>> /* Deregister indicators from host. */
>>>> - vcdev->indicators = 0;
>>>> + *indicators(vcdev) = 0;
>>>
>>> I'm not too hot about this notation, but it's not wrong and a minor
>>> thing :)
>>
>> I don't have any better ideas :/
>>
>>>
>>>> ccw->flags = 0;
>>>> ret = ccw_io_helper(vcdev, ccw,
>>>> vcdev->is_thinint ?
>>>
>>> Patch looks reasonable and not dependent on the other patches here.
>>>
>>
>> looks reasonable == r-b?
>>
>> Not dependent in a sense that this patch could be made a first patch in
>> the series. A subsequent patch depends on it.
>
> What is the plan with these patches? I can either pick patch 5+6 and
> let them go through the virtio tree, or give my r-b and let them go
> through the s390 tree. The former is probably the quicker route, but
> the latter has less potential for dependency issues.

please give your r-b then for these.

>

Michael