Re: [PATCH 10/10] virtio/s390: make airq summary indicators DMA

Michael Mueller
2019-05-15T13:43:23Z

On 13.05.19 14:20, Cornelia Huck wrote:
> On Fri, 26 Apr 2019 20:32:45 +0200
> Halil Pasic <pasic@linux.ibm.com> wrote:
>
>> Hypervisor needs to interact with the summary indicators, so these
>> need to be DMA memory as well (at least for protected virtualization
>> guests).
>>
>> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
>> ---
>> drivers/s390/virtio/virtio_ccw.c | 24 +++++++++++++++++-------
>> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> (...)
>
>> @@ -237,7 +243,8 @@ static void virtio_airq_handler(struct airq_struct *airq)
>> read_unlock(&info->lock);
>> }
>>
>> -static struct airq_info *new_airq_info(void)
>> +/* call with drivers/s390/virtio/virtio_ccw.cheld */
>
> Hm, where is airq_areas_lock defined? If it was introduced in one of
> the previous patches, I have missed it.

There is no airq_areas_lock defined currently. My assumption is that
this will be used in context with the likely race condition this
part of the patch is talking about.

@@ -273,8 +281,9 @@ static unsigned long get_airq_indicator(struct
virtqueue *vqs[], int nvqs,
unsigned long bit, flags;

for (i = 0; i < MAX_AIRQ_AREAS && !indicator_addr; i++) {
+ /* TODO: this seems to be racy */
if (!airq_areas[i])
- airq_areas[i] = new_airq_info();
+ airq_areas[i] = new_airq_info(i);

As this shall be handled by a separate patch I will drop the comment
in regard to airq_areas_lock from this patch as well for v2.

Michael

>
>> +static struct airq_info *new_airq_info(int index)
>> {
>> struct airq_info *info;
>> int rc;
>