Re: [PATCH 4/4] s390/virtio: use noop dma ops

[Cornelia Huck]

On Fri, 30 Oct 2015 13:26:09 +0100
Christian Borntraeger  wrote:

> I am currently reworking this to 
> 
>  static inline struct dma_map_ops *get_dma_ops(struct device *dev)
>  {
>   if (dev && dev->archdata.dma_ops)
>   return dev->archdata.dma_ops;
>   return _noop_ops;
>  }
> 
> 
> Which uses the dma_noop_ops for everything unless the device overrides (PCI 
> does)

Yes, opt-in seems less error-prone here.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] s390/virtio: use noop dma ops

[Christian Borntraeger]

Am 30.10.2015 um 13:17 schrieb Cornelia Huck:
> On Tue, 27 Oct 2015 23:48:51 +0100
> Christian Borntraeger  wrote:
> 
>> With all infrastructure in place, lets provide dma_ops for virtio
>> devices on s390.
>>
>> Signed-off-by: Christian Borntraeger 
>> ---
>>  drivers/s390/virtio/kvm_virtio.c | 2 ++
>>  drivers/s390/virtio/virtio_ccw.c | 2 ++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/s390/virtio/kvm_virtio.c 
>> b/drivers/s390/virtio/kvm_virtio.c
>> index 53fb975..05adaa9 100644
>> --- a/drivers/s390/virtio/kvm_virtio.c
>> +++ b/drivers/s390/virtio/kvm_virtio.c
>> @@ -13,6 +13,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -318,6 +319,7 @@ static void add_kvm_device(struct kvm_device_desc *d, 
>> unsigned int offset)
>>  return;
>>  }
>>
>> +kdev->vdev.dev.archdata.dma_ops = _noop_ops;
> 
> This provides dma_ops for the vdev, while Andy's virtio code looks for
> dma_ops in the vdev's parent (in the ccw and pci cases, the proxy
> device; in this case, it would be our root device).
> 
> With
> 
> diff --git a/drivers/s390/virtio/kvm_virtio.c 
> b/drivers/s390/virtio/kvm_virtio.c
> index 05adaa9..5f79c52 100644
> --- a/drivers/s390/virtio/kvm_virtio.c
> +++ b/drivers/s390/virtio/kvm_virtio.c
> @@ -319,7 +319,6 @@ static void add_kvm_device(struct kvm_device_desc *d, 
> unsigned int offset)
>   return;
>   }
> 
> - kdev->vdev.dev.archdata.dma_ops = _noop_ops;
>   kdev->vdev.dev.parent = kvm_root;
>   kdev->vdev.id.device = d->type;
>   kdev->vdev.config = _vq_configspace_ops;
> @@ -473,6 +472,7 @@ static int __init kvm_devices_init(void)
>   vmem_remove_mapping(total_memory_size, PAGE_SIZE);
>   return rc;
>   }
> + kvm_root->archdata.dma_ops = _noop_ops;
> 
>   INIT_WORK(_work, hotplug_devices);
> 
> applied (and the endianness fix in the virtio code), I can boot a
> s390-virtio guest as well.

I am currently reworking this to 

 static inline struct dma_map_ops *get_dma_ops(struct device *dev)
 {
if (dev && dev->archdata.dma_ops)
return dev->archdata.dma_ops;
return _noop_ops;
 }


Which uses the dma_noop_ops for everything unless the device overrides (PCI 
does)

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] s390/virtio: use noop dma ops

[Christian Borntraeger]

Am 28.10.2015 um 01:43 schrieb Joerg Roedel:
> On Tue, Oct 27, 2015 at 11:48:51PM +0100, Christian Borntraeger wrote:
>> @@ -1093,6 +1094,7 @@ static void virtio_ccw_auto_online(void *data, 
>> async_cookie_t cookie)
>>  struct ccw_device *cdev = data;
>>  int ret;
>>  
>> +cdev->dev.archdata.dma_ops = _noop_ops;
>>  ret = ccw_device_set_online(cdev);
>>  if (ret)
>>  dev_warn(>dev, "Failed to set online: %d\n", ret);
> 
> Hmm, drivers usually don't deal with setting the dma_ops for their
> devices, as they depend on the platform and not so much on the device
> itself.
> 
> Can you do this special-case handling from device independent platform
> code, where you also setup dma_ops for other devices?
> 

Yes, fixed in v2.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] s390/virtio: use noop dma ops

[Cornelia Huck]

On Tue, 27 Oct 2015 23:48:51 +0100
Christian Borntraeger  wrote:

> With all infrastructure in place, lets provide dma_ops for virtio
> devices on s390.
> 
> Signed-off-by: Christian Borntraeger 
> ---
>  drivers/s390/virtio/kvm_virtio.c | 2 ++
>  drivers/s390/virtio/virtio_ccw.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/s390/virtio/kvm_virtio.c 
> b/drivers/s390/virtio/kvm_virtio.c
> index 53fb975..05adaa9 100644
> --- a/drivers/s390/virtio/kvm_virtio.c
> +++ b/drivers/s390/virtio/kvm_virtio.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -318,6 +319,7 @@ static void add_kvm_device(struct kvm_device_desc *d, 
> unsigned int offset)
>   return;
>   }
> 
> + kdev->vdev.dev.archdata.dma_ops = _noop_ops;

This provides dma_ops for the vdev, while Andy's virtio code looks for
dma_ops in the vdev's parent (in the ccw and pci cases, the proxy
device; in this case, it would be our root device).

With

diff --git a/drivers/s390/virtio/kvm_virtio.c b/drivers/s390/virtio/kvm_virtio.c
index 05adaa9..5f79c52 100644
--- a/drivers/s390/virtio/kvm_virtio.c
+++ b/drivers/s390/virtio/kvm_virtio.c
@@ -319,7 +319,6 @@ static void add_kvm_device(struct kvm_device_desc *d, 
unsigned int offset)
return;
}
 
-   kdev->vdev.dev.archdata.dma_ops = _noop_ops;
kdev->vdev.dev.parent = kvm_root;
kdev->vdev.id.device = d->type;
kdev->vdev.config = _vq_configspace_ops;
@@ -473,6 +472,7 @@ static int __init kvm_devices_init(void)
vmem_remove_mapping(total_memory_size, PAGE_SIZE);
return rc;
}
+   kvm_root->archdata.dma_ops = _noop_ops;
 
INIT_WORK(_work, hotplug_devices);
 
applied (and the endianness fix in the virtio code), I can boot a
s390-virtio guest as well.

>   kdev->vdev.dev.parent = kvm_root;
>   kdev->vdev.id.device = d->type;
>   kdev->vdev.config = _vq_configspace_ops;

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] s390/virtio: use noop dma ops

[Cornelia Huck]

On Wed, 28 Oct 2015 09:43:34 +0900
Joerg Roedel  wrote:

> On Tue, Oct 27, 2015 at 11:48:51PM +0100, Christian Borntraeger wrote:
> > @@ -1093,6 +1094,7 @@ static void virtio_ccw_auto_online(void *data, 
> > async_cookie_t cookie)
> > struct ccw_device *cdev = data;
> > int ret;
> >  
> > +   cdev->dev.archdata.dma_ops = _noop_ops;
> > ret = ccw_device_set_online(cdev);
> > if (ret)
> > dev_warn(>dev, "Failed to set online: %d\n", ret);
> 
> Hmm, drivers usually don't deal with setting the dma_ops for their
> devices, as they depend on the platform and not so much on the device
> itself.
> 
> Can you do this special-case handling from device independent platform
> code, where you also setup dma_ops for other devices?

Hm, maybe at the bus level?

pci devices get s390_dma_ops, ccw devices get the noop dma ops (just a
bit of dead weight for non-virtio ccw devices, I guess).

The old style s390-virtio devices are the odd ones around, but I'd like
to invest the least time possible there to keep them going.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] s390/virtio: use noop dma ops

[Joerg Roedel]

On Tue, Oct 27, 2015 at 11:48:51PM +0100, Christian Borntraeger wrote:
> @@ -1093,6 +1094,7 @@ static void virtio_ccw_auto_online(void *data, 
> async_cookie_t cookie)
>   struct ccw_device *cdev = data;
>   int ret;
>  
> + cdev->dev.archdata.dma_ops = _noop_ops;
>   ret = ccw_device_set_online(cdev);
>   if (ret)
>   dev_warn(>dev, "Failed to set online: %d\n", ret);

Hmm, drivers usually don't deal with setting the dma_ops for their
devices, as they depend on the platform and not so much on the device
itself.

Can you do this special-case handling from device independent platform
code, where you also setup dma_ops for other devices?


Joerg

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html