* Re: [PATCH v3 03/23] drm/qxl: simplify slot management
From: Noralf Trønnes @ 2019-01-25 15:52 UTC (permalink / raw)
To: Gerd Hoffmann, dri-devel
Cc: David Airlie, open list:DRM DRIVER FOR QXL VIRTUAL GPU,
Dave Airlie, open list, open list:DRM DRIVER FOR QXL VIRTUAL GPU
In-Reply-To: <20190118122020.27596-4-kraxel@redhat.com>
Den 18.01.2019 13.20, skrev Gerd Hoffmann:
> Drop pointless indirection, remove the mem_slots array and index
> variables, drop dynamic allocation. Store memslots in qxl_device
> instead.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
Looks sane:
Acked-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3 02/23] drm/qxl: drop unused qxl_fb_virtual_address
From: Noralf Trønnes @ 2019-01-25 15:44 UTC (permalink / raw)
To: Gerd Hoffmann, dri-devel
Cc: David Airlie, open list:DRM DRIVER FOR QXL VIRTUAL GPU,
Dave Airlie, open list, open list:DRM DRIVER FOR QXL VIRTUAL GPU
In-Reply-To: <20190118122020.27596-3-kraxel@redhat.com>
Den 18.01.2019 13.19, skrev Gerd Hoffmann:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
Acked-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3 01/23] drm/qxl: drop ttm_mem_reg arg from qxl_hw_surface_alloc()
From: Noralf Trønnes @ 2019-01-25 15:44 UTC (permalink / raw)
To: Gerd Hoffmann, dri-devel
Cc: David Airlie, open list:DRM DRIVER FOR QXL VIRTUAL GPU,
Dave Airlie, open list, open list:DRM DRIVER FOR QXL VIRTUAL GPU
In-Reply-To: <20190118122020.27596-2-kraxel@redhat.com>
Den 18.01.2019 13.19, skrev Gerd Hoffmann:
> Not used, is always NULL.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
Acked-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 01/26] drm/irq: Don't check for DRIVER_HAVE_IRQ in drm_irq_(un)install
From: Emil Velikov @ 2019-01-25 15:09 UTC (permalink / raw)
To: Daniel Vetter
Cc: linux-arm-msm, Intel Graphics Development, amd-gfx mailing list,
open list:VIRTIO GPU DRIVER, linux-renesas-soc, DRI Development,
spice-devel, Daniel Vetter, linux-amlogic, freedreno, LAKML
In-Reply-To: <20190124165831.16427-2-daniel.vetter@ffwll.ch>
On Thu, 24 Jan 2019 at 16:58, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> If a non-legacy driver calls these it's valid to assume there is
> interrupt support. The flag is really only needed for legacy drivers.
... legacy drivers which issue the IRQ via the DRM_IOCTL_CONTROL legacy IOCTL.
At a later stage, we might as well add LEGACY to the name:
DRIVER_LEGACY_HAVE_IRQ?
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
> drivers/gpu/drm/arm/hdlcd_drv.c | 2 +-
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +-
> drivers/gpu/drm/drm_irq.c | 6 ------
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +-
> drivers/gpu/drm/gma500/psb_drv.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> drivers/gpu/drm/meson/meson_drv.c | 2 +-
> drivers/gpu/drm/msm/msm_drv.c | 3 +--
> drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 +--
> drivers/gpu/drm/qxl/qxl_drv.c | 2 +-
> drivers/gpu/drm/radeon/radeon_drv.c | 2 +-
> drivers/gpu/drm/shmobile/shmob_drm_drv.c | 2 +-
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
> drivers/gpu/drm/vc4/vc4_drv.c | 1 -
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
> drivers/staging/vboxvideo/vbox_drv.c | 2 +-
Local grep shows one more non-legacy entry in
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
With that file addressed and trivial comment additions, patches: 1, 2,
3 and 26 are
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
HTH
Emil
^ permalink raw reply
* Re: [PATCH 01/26] drm/irq: Don't check for DRIVER_HAVE_IRQ in drm_irq_(un)install
From: Sam Ravnborg @ 2019-01-25 10:40 UTC (permalink / raw)
To: Daniel Vetter
Cc: linux-arm-msm, Intel Graphics Development, amd-gfx,
virtualization, linux-renesas-soc, DRI Development, spice-devel,
Daniel Vetter, linux-amlogic, freedreno, linux-arm-kernel
In-Reply-To: <20190124165831.16427-2-daniel.vetter@ffwll.ch>
Hi Daniel.
On Thu, Jan 24, 2019 at 05:58:06PM +0100, Daniel Vetter wrote:
> If a non-legacy driver calls these it's valid to assume there is
> interrupt support. The flag is really only needed for legacy drivers.
>
> Also remove all the flag usage from non-legacy drivers.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: intel-gfx@lists.freedesktop.org
> Cc: linux-amlogic@lists.infradead.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Cc: virtualization@lists.linux-foundation.org
> Cc: spice-devel@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> Cc: linux-renesas-soc@vger.kernel.org
The actual code changes looks good.
But if this is the right thing to do I cannot tell.
On this (limited) basis I provide an:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
^ permalink raw reply
* Re: [PATCH net-next V4 5/5] vhost: access vq metadata through kernel virtual address
From: Jason Wang @ 2019-01-25 9:24 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aarcange, netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20190124214402-mutt-send-email-mst@kernel.org>
On 2019/1/25 上午11:03, Michael S. Tsirkin wrote:
>> +/* Suppress the vma that needs writeback since we can not track dirty
>> + * pages now.
>> + */
>> +static bool vma_can_vmap(struct vm_area_struct *vma)
>> +{
>> + return vma_is_anonymous(vma) || is_vm_hugetlb_page(vma) ||
>> + vma_is_shmem(vma);
>> +}
>> +
> IIUC a second but anonymous memory needs writeback too, just to swap.
> I'm not an MM person so I might be off.
Right, my fault, I mean the vma that needs dirty page tracking.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next V4 5/5] vhost: access vq metadata through kernel virtual address
From: Jason Wang @ 2019-01-25 9:21 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: aarcange, netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20190124214402-mutt-send-email-mst@kernel.org>
On 2019/1/25 上午11:03, Michael S. Tsirkin wrote:
> On Wed, Jan 23, 2019 at 05:55:57PM +0800, Jason Wang wrote:
>> It was noticed that the copy_user() friends that was used to access
>> virtqueue metdata tends to be very expensive for dataplane
>> implementation like vhost since it involves lots of software checks,
>> speculation barrier, hardware feature toggling (e.g SMAP). The
>> extra cost will be more obvious when transferring small packets since
>> the time spent on metadata accessing become more significant.
>>
>> This patch tries to eliminate those overheads by accessing them
>> through kernel virtual address by vmap(). To make the pages can be
>> migrated, instead of pinning them through GUP, we use MMU notifiers to
>> invalidate vmaps and re-establish vmaps during each round of metadata
>> prefetching if necessary. For devices that doesn't use metadata
>> prefetching, the memory accessors fallback to normal copy_user()
>> implementation gracefully. The invalidation was synchronized with
>> datapath through vq mutex, and in order to avoid hold vq mutex during
>> range checking, MMU notifier was teared down when trying to modify vq
>> metadata.
>>
>> Another thing is kernel lacks efficient solution for tracking dirty
>> pages by vmap(), this will lead issues if vhost is using file backed
>> memory which needs care of writeback. This patch solves this issue by
>> just skipping the vma that is file backed and fallback to normal
>> copy_user() friends. This might introduce some overheads for file
>> backed users but consider this use case is rare we could do
>> optimizations on top.
>>
>> Note that this was only done when device IOTLB is not enabled. We
>> could use similar method to optimize it in the future.
>>
>> Tests shows at most about 22% improvement on TX PPS when using
>> virtio-user + vhost_net + xdp1 + TAP on 2.6GHz Broadwell:
>>
>> SMAP on | SMAP off
>> Before: 5.0Mpps | 6.6Mpps
>> After: 6.1Mpps | 7.4Mpps
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> drivers/vhost/vhost.c | 288 +++++++++++++++++++++++++++++++++++++++++-
>> drivers/vhost/vhost.h | 13 ++
>> mm/shmem.c | 1 +
>> 3 files changed, 300 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index 37e2cac8e8b0..096ae3298d62 100644
>> --- a/drivers/vhost/vhost.c
>> +++ b/drivers/vhost/vhost.c
>> @@ -440,6 +440,9 @@ void vhost_dev_init(struct vhost_dev *dev,
>> vq->indirect = NULL;
>> vq->heads = NULL;
>> vq->dev = dev;
>> + memset(&vq->avail_ring, 0, sizeof(vq->avail_ring));
>> + memset(&vq->used_ring, 0, sizeof(vq->used_ring));
>> + memset(&vq->desc_ring, 0, sizeof(vq->desc_ring));
>> mutex_init(&vq->mutex);
>> vhost_vq_reset(dev, vq);
>> if (vq->handle_kick)
>> @@ -510,6 +513,73 @@ static size_t vhost_get_desc_size(struct vhost_virtqueue *vq, int num)
>> return sizeof(*vq->desc) * num;
>> }
>>
>> +static void vhost_uninit_vmap(struct vhost_vmap *map)
>> +{
>> + if (map->addr)
>> + vunmap(map->unmap_addr);
>> +
>> + map->addr = NULL;
>> + map->unmap_addr = NULL;
>> +}
>> +
>> +static int vhost_invalidate_vmap(struct vhost_virtqueue *vq,
>> + struct vhost_vmap *map,
>> + unsigned long ustart,
>> + size_t size,
>> + unsigned long start,
>> + unsigned long end,
>> + bool blockable)
>> +{
>> + if (end < ustart || start > ustart - 1 + size)
>> + return 0;
>> +
>> + if (!blockable)
>> + return -EAGAIN;
>> +
>> + mutex_lock(&vq->mutex);
>> + vhost_uninit_vmap(map);
>> + mutex_unlock(&vq->mutex);
>> +
>> + return 0;
>> +}
>> +
>> +static int vhost_invalidate_range_start(struct mmu_notifier *mn,
>> + const struct mmu_notifier_range *range)
>> +{
>> + struct vhost_dev *dev = container_of(mn, struct vhost_dev,
>> + mmu_notifier);
>> + int i;
>> +
>> + for (i = 0; i < dev->nvqs; i++) {
>> + struct vhost_virtqueue *vq = dev->vqs[i];
>> +
>> + if (vhost_invalidate_vmap(vq, &vq->avail_ring,
>> + (unsigned long)vq->avail,
>> + vhost_get_avail_size(vq, vq->num),
>> + range->start, range->end,
>> + range->blockable))
>> + return -EAGAIN;
>> + if (vhost_invalidate_vmap(vq, &vq->desc_ring,
>> + (unsigned long)vq->desc,
>> + vhost_get_desc_size(vq, vq->num),
>> + range->start, range->end,
>> + range->blockable))
>> + return -EAGAIN;
>> + if (vhost_invalidate_vmap(vq, &vq->used_ring,
>> + (unsigned long)vq->used,
>> + vhost_get_used_size(vq, vq->num),
>> + range->start, range->end,
>> + range->blockable))
>> + return -EAGAIN;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct mmu_notifier_ops vhost_mmu_notifier_ops = {
>> + .invalidate_range_start = vhost_invalidate_range_start,
>> +};
>> +
>> /* Caller should have device mutex */
>> long vhost_dev_set_owner(struct vhost_dev *dev)
>> {
> It seems questionable to merely track .invalidate_range_start.
> Don't we care about keeping pages young/accessed?
My understanding is the young stuffs were only needed for secondary MMU
where the hva is not used. This is not the case of vhost since anyway
guest will access those pages through userspace address.
> MMU will think they aren't and will penalize vhost by pushing
> them out.
>
> I note that MMU documentation says
> * invalidate_range_start() and invalidate_range_end() must be
> * paired
> and it seems questionable that they are not paired here.
I can see some users with the unpaired invalidate_range_start(). Maybe I
miss something but I can not find anything that we need to do after the
page is unmaped.
>
>
> I also wonder about things like write-protecting the pages.
> It does not look like a range is invalidated when page
> is write-protected, even though I might have missed that.
> If not we can be corrupting memory in a variety of ways
> e.g. when using KSM, or with COW.
Yes, we probably need to implement change_pte() method which will do
vunmap().
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next V4 5/5] vhost: access vq metadata through kernel virtual address
From: Jason Wang @ 2019-01-25 9:16 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20190124215912-mutt-send-email-mst@kernel.org>
On 2019/1/25 上午11:00, Michael S. Tsirkin wrote:
> On Thu, Jan 24, 2019 at 12:07:54PM +0800, Jason Wang wrote:
>>> Meanwhile, could you pls post data comparing this last patch with the
>>> below? This removes the speculation barrier replacing it with a
>>> (useless but at least more lightweight) data dependency.
>> SMAP off
>>
>> Your patch: 7.2MPPs
>>
>> vmap: 7.4Mpps
> OK so while we keep looking into vmap, why don't we merge something like
> the below? Seems quite straight forward ...
>
The problem is it gives ~8% regression on PPS when SMAP is on. This is
probably because the latency of lfence is hided by previous stac as
mentioned in the commit b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 19/26] drm/qxl: Use drm_fb_helper_fill_info
From: Daniel Vetter @ 2019-01-25 8:52 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
virtualization, spice-devel, Daniel Vetter, Dave Airlie
In-Reply-To: <20190125063926.hdgfq7ouggqgjkoa@sirius.home.kraxel.org>
On Fri, Jan 25, 2019 at 07:39:26AM +0100, Gerd Hoffmann wrote:
> On Thu, Jan 24, 2019 at 05:58:24PM +0100, Daniel Vetter wrote:
> > This should not result in any changes.
>
> I'd love to merge https://patchwork.freedesktop.org/series/53951/
> instead (which will -- among other things -- switch qxl over to the
> generic fbdev emulation and remove the code you are patching here).
Ah yes very happy to drop my patch in favour of yours.
> Anyone willing to review? Dave?
Noralf might be a good choice too, with you cross-reviewing some of his
in-flight work.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH 19/26] drm/qxl: Use drm_fb_helper_fill_info
From: Gerd Hoffmann @ 2019-01-25 6:39 UTC (permalink / raw)
To: Daniel Vetter
Cc: Dave Airlie, Intel Graphics Development, DRI Development,
virtualization, spice-devel, Daniel Vetter
In-Reply-To: <20190124165831.16427-20-daniel.vetter@ffwll.ch>
On Thu, Jan 24, 2019 at 05:58:24PM +0100, Daniel Vetter wrote:
> This should not result in any changes.
I'd love to merge https://patchwork.freedesktop.org/series/53951/
instead (which will -- among other things -- switch qxl over to the
generic fbdev emulation and remove the code you are patching here).
Anyone willing to review? Dave?
thanks,
Gerd
^ permalink raw reply
* Re: [PATCH net-next V4 5/5] vhost: access vq metadata through kernel virtual address
From: Michael S. Tsirkin @ 2019-01-25 3:03 UTC (permalink / raw)
To: Jason Wang; +Cc: aarcange, netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20190123095557.30168-6-jasowang@redhat.com>
On Wed, Jan 23, 2019 at 05:55:57PM +0800, Jason Wang wrote:
> It was noticed that the copy_user() friends that was used to access
> virtqueue metdata tends to be very expensive for dataplane
> implementation like vhost since it involves lots of software checks,
> speculation barrier, hardware feature toggling (e.g SMAP). The
> extra cost will be more obvious when transferring small packets since
> the time spent on metadata accessing become more significant.
>
> This patch tries to eliminate those overheads by accessing them
> through kernel virtual address by vmap(). To make the pages can be
> migrated, instead of pinning them through GUP, we use MMU notifiers to
> invalidate vmaps and re-establish vmaps during each round of metadata
> prefetching if necessary. For devices that doesn't use metadata
> prefetching, the memory accessors fallback to normal copy_user()
> implementation gracefully. The invalidation was synchronized with
> datapath through vq mutex, and in order to avoid hold vq mutex during
> range checking, MMU notifier was teared down when trying to modify vq
> metadata.
>
> Another thing is kernel lacks efficient solution for tracking dirty
> pages by vmap(), this will lead issues if vhost is using file backed
> memory which needs care of writeback. This patch solves this issue by
> just skipping the vma that is file backed and fallback to normal
> copy_user() friends. This might introduce some overheads for file
> backed users but consider this use case is rare we could do
> optimizations on top.
>
> Note that this was only done when device IOTLB is not enabled. We
> could use similar method to optimize it in the future.
>
> Tests shows at most about 22% improvement on TX PPS when using
> virtio-user + vhost_net + xdp1 + TAP on 2.6GHz Broadwell:
>
> SMAP on | SMAP off
> Before: 5.0Mpps | 6.6Mpps
> After: 6.1Mpps | 7.4Mpps
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vhost/vhost.c | 288 +++++++++++++++++++++++++++++++++++++++++-
> drivers/vhost/vhost.h | 13 ++
> mm/shmem.c | 1 +
> 3 files changed, 300 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 37e2cac8e8b0..096ae3298d62 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -440,6 +440,9 @@ void vhost_dev_init(struct vhost_dev *dev,
> vq->indirect = NULL;
> vq->heads = NULL;
> vq->dev = dev;
> + memset(&vq->avail_ring, 0, sizeof(vq->avail_ring));
> + memset(&vq->used_ring, 0, sizeof(vq->used_ring));
> + memset(&vq->desc_ring, 0, sizeof(vq->desc_ring));
> mutex_init(&vq->mutex);
> vhost_vq_reset(dev, vq);
> if (vq->handle_kick)
> @@ -510,6 +513,73 @@ static size_t vhost_get_desc_size(struct vhost_virtqueue *vq, int num)
> return sizeof(*vq->desc) * num;
> }
>
> +static void vhost_uninit_vmap(struct vhost_vmap *map)
> +{
> + if (map->addr)
> + vunmap(map->unmap_addr);
> +
> + map->addr = NULL;
> + map->unmap_addr = NULL;
> +}
> +
> +static int vhost_invalidate_vmap(struct vhost_virtqueue *vq,
> + struct vhost_vmap *map,
> + unsigned long ustart,
> + size_t size,
> + unsigned long start,
> + unsigned long end,
> + bool blockable)
> +{
> + if (end < ustart || start > ustart - 1 + size)
> + return 0;
> +
> + if (!blockable)
> + return -EAGAIN;
> +
> + mutex_lock(&vq->mutex);
> + vhost_uninit_vmap(map);
> + mutex_unlock(&vq->mutex);
> +
> + return 0;
> +}
> +
> +static int vhost_invalidate_range_start(struct mmu_notifier *mn,
> + const struct mmu_notifier_range *range)
> +{
> + struct vhost_dev *dev = container_of(mn, struct vhost_dev,
> + mmu_notifier);
> + int i;
> +
> + for (i = 0; i < dev->nvqs; i++) {
> + struct vhost_virtqueue *vq = dev->vqs[i];
> +
> + if (vhost_invalidate_vmap(vq, &vq->avail_ring,
> + (unsigned long)vq->avail,
> + vhost_get_avail_size(vq, vq->num),
> + range->start, range->end,
> + range->blockable))
> + return -EAGAIN;
> + if (vhost_invalidate_vmap(vq, &vq->desc_ring,
> + (unsigned long)vq->desc,
> + vhost_get_desc_size(vq, vq->num),
> + range->start, range->end,
> + range->blockable))
> + return -EAGAIN;
> + if (vhost_invalidate_vmap(vq, &vq->used_ring,
> + (unsigned long)vq->used,
> + vhost_get_used_size(vq, vq->num),
> + range->start, range->end,
> + range->blockable))
> + return -EAGAIN;
> + }
> +
> + return 0;
> +}
> +
> +static const struct mmu_notifier_ops vhost_mmu_notifier_ops = {
> + .invalidate_range_start = vhost_invalidate_range_start,
> +};
> +
> /* Caller should have device mutex */
> long vhost_dev_set_owner(struct vhost_dev *dev)
> {
It seems questionable to merely track .invalidate_range_start.
Don't we care about keeping pages young/accessed?
MMU will think they aren't and will penalize vhost by pushing
them out.
I note that MMU documentation says
* invalidate_range_start() and invalidate_range_end() must be
* paired
and it seems questionable that they are not paired here.
I also wonder about things like write-protecting the pages.
It does not look like a range is invalidated when page
is write-protected, even though I might have missed that.
If not we can be corrupting memory in a variety of ways
e.g. when using KSM, or with COW.
> @@ -541,7 +611,14 @@ long vhost_dev_set_owner(struct vhost_dev *dev)
> if (err)
> goto err_cgroup;
>
> + dev->mmu_notifier.ops = &vhost_mmu_notifier_ops;
> + err = mmu_notifier_register(&dev->mmu_notifier, dev->mm);
> + if (err)
> + goto err_mmu_notifier;
> +
> return 0;
> +err_mmu_notifier:
> + vhost_dev_free_iovecs(dev);
> err_cgroup:
> kthread_stop(worker);
> dev->worker = NULL;
> @@ -632,6 +709,97 @@ static void vhost_clear_msg(struct vhost_dev *dev)
> spin_unlock(&dev->iotlb_lock);
> }
>
> +/* Suppress the vma that needs writeback since we can not track dirty
> + * pages now.
> + */
> +static bool vma_can_vmap(struct vm_area_struct *vma)
> +{
> + return vma_is_anonymous(vma) || is_vm_hugetlb_page(vma) ||
> + vma_is_shmem(vma);
> +}
> +
IIUC a second but anonymous memory needs writeback too, just to swap.
I'm not an MM person so I might be off.
> +static int vhost_init_vmap(struct vhost_dev *dev,
> + struct vhost_vmap *map, unsigned long uaddr,
> + size_t size, int write)
> +{
> + struct mm_struct *mm = dev->mm;
> + struct vm_area_struct *vma;
> + struct page **pages;
> + int npages = DIV_ROUND_UP(size, PAGE_SIZE);
> + int npinned;
> + void *vaddr;
> + int err = 0;
> +
> + down_read(&mm->mmap_sem);
> + vma = find_vma(mm, uaddr);
> + if (!vma || !vma_can_vmap(vma) ||
> + vma->vm_end < uaddr - 1 + size) {
> + err = -EINVAL;
> + goto err_vma;
> + }
> +
> + pages = kmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
> + if (!pages) {
> + err = -ENOMEM;
> + goto err_alloc;
> + }
> +
> + npinned = get_user_pages_fast(uaddr, npages, write, pages);
> + if (npinned != npages) {
> + err = -EFAULT;
> + goto err_gup;
> + }
> +
> + vaddr = vmap(pages, npages, VM_MAP, PAGE_KERNEL);
> + if (!vaddr) {
> + err = EFAULT;
> + goto err_gup;
> + }
> +
> + map->addr = vaddr + (uaddr & (PAGE_SIZE - 1));
> + map->unmap_addr = vaddr;
> +
> +err_gup:
> + /* Don't pin pages, mmu notifier will notify us about page
> + * migration.
> + */
> + if (npinned > 0)
> + release_pages(pages, npinned);
> +err_alloc:
> + kfree(pages);
> +err_vma:
> + up_read(&mm->mmap_sem);
> + return err;
> +}
> +
> +static void vhost_clean_vmaps(struct vhost_virtqueue *vq)
> +{
> + vhost_uninit_vmap(&vq->avail_ring);
> + vhost_uninit_vmap(&vq->desc_ring);
> + vhost_uninit_vmap(&vq->used_ring);
> +}
> +
> +static int vhost_setup_avail_vmap(struct vhost_virtqueue *vq,
> + unsigned long avail)
> +{
> + return vhost_init_vmap(vq->dev, &vq->avail_ring, avail,
> + vhost_get_avail_size(vq, vq->num), false);
> +}
> +
> +static int vhost_setup_desc_vmap(struct vhost_virtqueue *vq,
> + unsigned long desc)
> +{
> + return vhost_init_vmap(vq->dev, &vq->desc_ring, desc,
> + vhost_get_desc_size(vq, vq->num), false);
> +}
> +
> +static int vhost_setup_used_vmap(struct vhost_virtqueue *vq,
> + unsigned long used)
> +{
> + return vhost_init_vmap(vq->dev, &vq->used_ring, used,
> + vhost_get_used_size(vq, vq->num), true);
> +}
> +
> void vhost_dev_cleanup(struct vhost_dev *dev)
> {
> int i;
> @@ -661,8 +829,12 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
> kthread_stop(dev->worker);
> dev->worker = NULL;
> }
> - if (dev->mm)
> + if (dev->mm) {
> + mmu_notifier_unregister(&dev->mmu_notifier, dev->mm);
> mmput(dev->mm);
> + }
> + for (i = 0; i < dev->nvqs; i++)
> + vhost_clean_vmaps(dev->vqs[i]);
> dev->mm = NULL;
> }
> EXPORT_SYMBOL_GPL(vhost_dev_cleanup);
> @@ -891,6 +1063,16 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
>
> static inline int vhost_put_avail_event(struct vhost_virtqueue *vq)
> {
> + if (!vq->iotlb) {
> + struct vring_used *used = vq->used_ring.addr;
> +
> + if (likely(used)) {
> + *((__virtio16 *)&used->ring[vq->num]) =
> + cpu_to_vhost16(vq, vq->avail_idx);
> + return 0;
> + }
> + }
> +
> return vhost_put_user(vq, cpu_to_vhost16(vq, vq->avail_idx),
> vhost_avail_event(vq));
> }
> @@ -899,6 +1081,16 @@ static inline int vhost_put_used(struct vhost_virtqueue *vq,
> struct vring_used_elem *head, int idx,
> int count)
> {
> + if (!vq->iotlb) {
> + struct vring_used *used = vq->used_ring.addr;
> +
> + if (likely(used)) {
> + memcpy(used->ring + idx, head,
> + count * sizeof(*head));
> + return 0;
> + }
> + }
> +
> return vhost_copy_to_user(vq, vq->used->ring + idx, head,
> count * sizeof(*head));
> }
> @@ -906,6 +1098,15 @@ static inline int vhost_put_used(struct vhost_virtqueue *vq,
> static inline int vhost_put_used_flags(struct vhost_virtqueue *vq)
>
> {
> + if (!vq->iotlb) {
> + struct vring_used *used = vq->used_ring.addr;
> +
> + if (likely(used)) {
> + used->flags = cpu_to_vhost16(vq, vq->used_flags);
> + return 0;
> + }
> + }
> +
> return vhost_put_user(vq, cpu_to_vhost16(vq, vq->used_flags),
> &vq->used->flags);
> }
> @@ -913,6 +1114,15 @@ static inline int vhost_put_used_flags(struct vhost_virtqueue *vq)
> static inline int vhost_put_used_idx(struct vhost_virtqueue *vq)
>
> {
> + if (!vq->iotlb) {
> + struct vring_used *used = vq->used_ring.addr;
> +
> + if (likely(used)) {
> + used->idx = cpu_to_vhost16(vq, vq->last_used_idx);
> + return 0;
> + }
> + }
> +
> return vhost_put_user(vq, cpu_to_vhost16(vq, vq->last_used_idx),
> &vq->used->idx);
> }
> @@ -958,12 +1168,30 @@ static void vhost_dev_unlock_vqs(struct vhost_dev *d)
> static inline int vhost_get_avail_idx(struct vhost_virtqueue *vq,
> __virtio16 *idx)
> {
> + if (!vq->iotlb) {
> + struct vring_avail *avail = vq->avail_ring.addr;
> +
> + if (likely(avail)) {
> + *idx = avail->idx;
> + return 0;
> + }
> + }
> +
> return vhost_get_avail(vq, *idx, &vq->avail->idx);
> }
>
> static inline int vhost_get_avail_head(struct vhost_virtqueue *vq,
> __virtio16 *head, int idx)
> {
> + if (!vq->iotlb) {
> + struct vring_avail *avail = vq->avail_ring.addr;
> +
> + if (likely(avail)) {
> + *head = avail->ring[idx & (vq->num - 1)];
> + return 0;
> + }
> + }
> +
> return vhost_get_avail(vq, *head,
> &vq->avail->ring[idx & (vq->num - 1)]);
> }
> @@ -971,24 +1199,60 @@ static inline int vhost_get_avail_head(struct vhost_virtqueue *vq,
> static inline int vhost_get_avail_flags(struct vhost_virtqueue *vq,
> __virtio16 *flags)
> {
> + if (!vq->iotlb) {
> + struct vring_avail *avail = vq->avail_ring.addr;
> +
> + if (likely(avail)) {
> + *flags = avail->flags;
> + return 0;
> + }
> + }
> +
> return vhost_get_avail(vq, *flags, &vq->avail->flags);
> }
>
> static inline int vhost_get_used_event(struct vhost_virtqueue *vq,
> __virtio16 *event)
> {
> + if (!vq->iotlb) {
> + struct vring_avail *avail = vq->avail_ring.addr;
> +
> + if (likely(avail)) {
> + *event = (__virtio16)avail->ring[vq->num];
> + return 0;
> + }
> + }
> +
> return vhost_get_avail(vq, *event, vhost_used_event(vq));
> }
>
> static inline int vhost_get_used_idx(struct vhost_virtqueue *vq,
> __virtio16 *idx)
> {
> + if (!vq->iotlb) {
> + struct vring_used *used = vq->used_ring.addr;
> +
> + if (likely(used)) {
> + *idx = used->idx;
> + return 0;
> + }
> + }
> +
> return vhost_get_used(vq, *idx, &vq->used->idx);
> }
>
> static inline int vhost_get_desc(struct vhost_virtqueue *vq,
> struct vring_desc *desc, int idx)
> {
> + if (!vq->iotlb) {
> + struct vring_desc *d = vq->desc_ring.addr;
> +
> + if (likely(d)) {
> + *desc = *(d + idx);
> + return 0;
> + }
> + }
> +
> return vhost_copy_from_user(vq, desc, vq->desc + idx, sizeof(*desc));
> }
>
> @@ -1329,8 +1593,16 @@ int vq_meta_prefetch(struct vhost_virtqueue *vq)
> {
> unsigned int num = vq->num;
>
> - if (!vq->iotlb)
> + if (!vq->iotlb) {
> + if (unlikely(!vq->avail_ring.addr))
> + vhost_setup_avail_vmap(vq, (unsigned long)vq->avail);
> + if (unlikely(!vq->desc_ring.addr))
> + vhost_setup_desc_vmap(vq, (unsigned long)vq->desc);
> + if (unlikely(!vq->used_ring.addr))
> + vhost_setup_used_vmap(vq, (unsigned long)vq->used);
> +
> return 1;
> + }
>
> return iotlb_access_ok(vq, VHOST_ACCESS_RO, (u64)(uintptr_t)vq->desc,
> vhost_get_desc_size(vq, num), VHOST_ADDR_DESC) &&
> @@ -1482,6 +1754,13 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
>
> mutex_lock(&vq->mutex);
>
> + /* Unregister MMU notifer to allow invalidation callback
> + * can access vq->avail, vq->desc , vq->used and vq->num
> + * without holding vq->mutex.
> + */
> + if (d->mm)
> + mmu_notifier_unregister(&d->mmu_notifier, d->mm);
> +
> switch (ioctl) {
> case VHOST_SET_VRING_NUM:
> /* Resizing ring with an active backend?
> @@ -1498,6 +1777,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
> r = -EINVAL;
> break;
> }
> + vhost_clean_vmaps(vq);
> vq->num = s.num;
> break;
> case VHOST_SET_VRING_BASE:
> @@ -1575,6 +1855,8 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
> }
> }
>
> + vhost_clean_vmaps(vq);
> +
> vq->log_used = !!(a.flags & (0x1 << VHOST_VRING_F_LOG));
> vq->desc = (void __user *)(unsigned long)a.desc_user_addr;
> vq->avail = (void __user *)(unsigned long)a.avail_user_addr;
> @@ -1655,6 +1937,8 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
> if (pollstart && vq->handle_kick)
> r = vhost_poll_start(&vq->poll, vq->kick);
>
> + if (d->mm)
> + mmu_notifier_register(&d->mmu_notifier, d->mm);
> mutex_unlock(&vq->mutex);
>
> if (pollstop && vq->handle_kick)
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 4e21011b6628..c04bc327db9f 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -12,6 +12,8 @@
> #include <linux/virtio_config.h>
> #include <linux/virtio_ring.h>
> #include <linux/atomic.h>
> +#include <linux/pagemap.h>
> +#include <linux/mmu_notifier.h>
>
> struct vhost_work;
> typedef void (*vhost_work_fn_t)(struct vhost_work *work);
> @@ -80,6 +82,11 @@ enum vhost_uaddr_type {
> VHOST_NUM_ADDRS = 3,
> };
>
> +struct vhost_vmap {
> + void *addr;
> + void *unmap_addr;
> +};
> +
> /* The virtqueue structure describes a queue attached to a device. */
> struct vhost_virtqueue {
> struct vhost_dev *dev;
> @@ -90,6 +97,11 @@ struct vhost_virtqueue {
> struct vring_desc __user *desc;
> struct vring_avail __user *avail;
> struct vring_used __user *used;
> +
> + struct vhost_vmap avail_ring;
> + struct vhost_vmap desc_ring;
> + struct vhost_vmap used_ring;
> +
> const struct vhost_umem_node *meta_iotlb[VHOST_NUM_ADDRS];
> struct file *kick;
> struct eventfd_ctx *call_ctx;
> @@ -158,6 +170,7 @@ struct vhost_msg_node {
>
> struct vhost_dev {
> struct mm_struct *mm;
> + struct mmu_notifier mmu_notifier;
> struct mutex mutex;
> struct vhost_virtqueue **vqs;
> int nvqs;
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 6ece1e2fe76e..745e7c7f7a6c 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -237,6 +237,7 @@ bool vma_is_shmem(struct vm_area_struct *vma)
> {
> return vma->vm_ops == &shmem_vm_ops;
> }
> +EXPORT_SYMBOL_GPL(vma_is_shmem);
>
> static LIST_HEAD(shmem_swaplist);
> static DEFINE_MUTEX(shmem_swaplist_mutex);
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH net-next V4 5/5] vhost: access vq metadata through kernel virtual address
From: Michael S. Tsirkin @ 2019-01-25 3:00 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <335ba55b-087f-4b35-6311-540070b9647f@redhat.com>
On Thu, Jan 24, 2019 at 12:07:54PM +0800, Jason Wang wrote:
> > Meanwhile, could you pls post data comparing this last patch with the
> > below? This removes the speculation barrier replacing it with a
> > (useless but at least more lightweight) data dependency.
>
>
> SMAP off
>
> Your patch: 7.2MPPs
>
> vmap: 7.4Mpps
OK so while we keep looking into vmap, why don't we merge something like
the below? Seems quite straight forward ...
> I don't test SMAP on, since it will be much slow for sure.
>
> Thanks
>
>
> >
> > Thanks!
> >
> >
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index bac939af8dbb..352ee7e14476 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -739,7 +739,7 @@ static int vhost_copy_to_user(struct vhost_virtqueue *vq, void __user *to,
> > int ret;
> > if (!vq->iotlb)
> > - return __copy_to_user(to, from, size);
> > + return copy_to_user(to, from, size);
> > else {
> > /* This function should be called after iotlb
> > * prefetch, which means we're sure that all vq
> > @@ -752,7 +752,7 @@ static int vhost_copy_to_user(struct vhost_virtqueue *vq, void __user *to,
> > VHOST_ADDR_USED);
> > if (uaddr)
> > - return __copy_to_user(uaddr, from, size);
> > + return copy_to_user(uaddr, from, size);
> > ret = translate_desc(vq, (u64)(uintptr_t)to, size, vq->iotlb_iov,
> > ARRAY_SIZE(vq->iotlb_iov),
> > @@ -774,7 +774,7 @@ static int vhost_copy_from_user(struct vhost_virtqueue *vq, void *to,
> > int ret;
> > if (!vq->iotlb)
> > - return __copy_from_user(to, from, size);
> > + return copy_from_user(to, from, size);
> > else {
> > /* This function should be called after iotlb
> > * prefetch, which means we're sure that vq
> > @@ -787,7 +787,7 @@ static int vhost_copy_from_user(struct vhost_virtqueue *vq, void *to,
> > struct iov_iter f;
> > if (uaddr)
> > - return __copy_from_user(to, uaddr, size);
> > + return copy_from_user(to, uaddr, size);
> > ret = translate_desc(vq, (u64)(uintptr_t)from, size, vq->iotlb_iov,
> > ARRAY_SIZE(vq->iotlb_iov),
> > @@ -855,13 +855,13 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
> > ({ \
> > int ret = -EFAULT; \
> > if (!vq->iotlb) { \
> > - ret = __put_user(x, ptr); \
> > + ret = put_user(x, ptr); \
> > } else { \
> > __typeof__(ptr) to = \
> > (__typeof__(ptr)) __vhost_get_user(vq, ptr, \
> > sizeof(*ptr), VHOST_ADDR_USED); \
> > if (to != NULL) \
> > - ret = __put_user(x, to); \
> > + ret = put_user(x, to); \
> > else \
> > ret = -EFAULT; \
> > } \
> > @@ -872,14 +872,14 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
> > ({ \
> > int ret; \
> > if (!vq->iotlb) { \
> > - ret = __get_user(x, ptr); \
> > + ret = get_user(x, ptr); \
> > } else { \
> > __typeof__(ptr) from = \
> > (__typeof__(ptr)) __vhost_get_user(vq, ptr, \
> > sizeof(*ptr), \
> > type); \
> > if (from != NULL) \
> > - ret = __get_user(x, from); \
> > + ret = get_user(x, from); \
> > else \
> > ret = -EFAULT; \
> > } \
^ permalink raw reply
* Re: [PATCH net-next V4 5/5] vhost: access vq metadata through kernel virtual address
From: Jason Wang @ 2019-01-25 2:33 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20190123235219-mutt-send-email-mst@kernel.org>
On 2019/1/24 下午12:53, Michael S. Tsirkin wrote:
>>>> - How hard is it to figure out which mode uses which code.
>> It's as simple as tracing __get_user() usage in vhost process?
>>
>> Thanks
> Well there are now mtu notifiers etc etc. It's hardly as well
> contained as that.
>
>
We can setup filter out exactly what sets of function that we wan to
trace. E.g we can only trace the usage of __get_user() and
invalidate_range_start(). This should be sufficient.
In the long run, we may want to have some tracepoints for vhost_net.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [Xen-devel] [RFC] virtio_ring: check dma_mem for xen_domain
From: Michael S. Tsirkin @ 2019-01-24 20:34 UTC (permalink / raw)
To: Stefano Stabellini
Cc: jgross@suse.com, Peng Fan, linux-remoteproc@vger.kernel.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, hch@infradead.org,
jliang@xilinx.com, luto@kernel.org,
xen-devel@lists.xenproject.org, boris.ostrovsky@oracle.com,
bjorn.andersson@linaro.org
In-Reply-To: <alpine.DEB.2.10.1901241110290.17936@sstabellini-ThinkPad-X260>
On Thu, Jan 24, 2019 at 11:14:53AM -0800, Stefano Stabellini wrote:
> On Thu, 24 Jan 2019, Peng Fan wrote:
> > Hi stefano,
> >
> > > -----Original Message-----
> > > From: Stefano Stabellini [mailto:sstabellini@kernel.org]
> > > Sent: 2019年1月24日 7:44
> > > To: hch@infradead.org
> > > Cc: Stefano Stabellini <sstabellini@kernel.org>; Peng Fan
> > > <peng.fan@nxp.com>; mst@redhat.com; jasowang@redhat.com;
> > > xen-devel@lists.xenproject.org; linux-remoteproc@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; virtualization@lists.linux-foundation.org;
> > > luto@kernel.org; jgross@suse.com; boris.ostrovsky@oracle.com;
> > > bjorn.andersson@linaro.org; jliang@xilinx.com
> > > Subject: Re: [Xen-devel] [RFC] virtio_ring: check dma_mem for xen_domain
> > >
> > > On Wed, 23 Jan 2019, hch@infradead.org wrote:
> > > > On Wed, Jan 23, 2019 at 01:04:33PM -0800, Stefano Stabellini wrote:
> > > > > If vring_use_dma_api is actually supposed to return true when
> > > > > dma_dev->dma_mem is set, then both Peng's patch and the patch I
> > > > > wrote are not fixing the real issue here.
> > > > >
> > > > > I don't know enough about remoteproc to know where the problem
> > > > > actually lies though.
> > > >
> > > > The problem is the following:
> > > >
> > > > Devices can declare a specific memory region that they want to use
> > > > when the driver calls dma_alloc_coherent for the device, this is done
> > > > using the shared-dma-pool DT attribute, which comes in two variants
> > > > that would be a little to much to explain here.
> > > >
> > > > remoteproc makes use of that because apparently the device can only
> > > > communicate using that region. But it then feeds back memory obtained
> > > > with dma_alloc_coherent into the virtio code. For that it calls
> > > > vmalloc_to_page on the dma_alloc_coherent, which is a huge no-go for
> > > > the ĐMA API and only worked accidentally on a few platform, and
> > > > apparently arm64 just changed a few internals that made it stop
> > > > working for remoteproc.
> > > >
> > > > The right answer is to not use the DMA API to allocate memory from a
> > > > device-speficic region, but to tie the driver directly into the DT
> > > > reserved memory API in a way that allows it to easilt obtain a struct
> > > > device for it.
> > >
> > > If I understand correctly, Peng should be able to reproduce the problem on
> > > native Linux without any Xen involvement simply by forcing
> > > vring_use_dma_api to return true. Peng, can you confirm?
> >
> > It is another issue without xen involvement,
> > There is an thread talking this: https://patchwork.kernel.org/patch/10742923/
> >
> > Without xen, vring_use_dma_api will return false.
> > With xen, if vring_use_dma_api returns true, it will dma_map_xx and trigger dump.
>
> It is true that for Xen on ARM DomUs it is not necessary today to return
> true from vring_use_dma_api. However, returning true from
> vring_use_dma_api should not break Linux. When the rpmesg issue is
> fixed, this problem should also go away without any need for additional
> changes on the xen side I think.
Let less systems bypass the standard virtio logic (using feature bit
to figure out bypassing DMA API), the better.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [PATCH 19/26] drm/qxl: Use drm_fb_helper_fill_info
From: Daniel Vetter @ 2019-01-24 16:58 UTC (permalink / raw)
To: DRI Development
Cc: spice-devel, Daniel Vetter, Intel Graphics Development,
virtualization, Daniel Vetter, Dave Airlie
In-Reply-To: <20190124165831.16427-1-daniel.vetter@ffwll.ch>
This should not result in any changes.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
drivers/gpu/drm/qxl/qxl_fb.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
index d028471597ef..22dc9a7a643b 100644
--- a/drivers/gpu/drm/qxl/qxl_fb.c
+++ b/drivers/gpu/drm/qxl/qxl_fb.c
@@ -219,8 +219,6 @@ static int qxlfb_create(struct drm_fb_helper *helper,
goto out_unref;
}
- info->par = helper;
-
fb = drm_gem_fbdev_fb_create(&qdev->ddev, sizes, 64, gobj,
&qxlfb_fb_funcs);
if (IS_ERR(fb)) {
@@ -232,10 +230,6 @@ static int qxlfb_create(struct drm_fb_helper *helper,
/* setup helper with fb data */
qdev->fb_helper.fb = fb;
- strcpy(info->fix.id, "qxldrmfb");
-
- drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
-
info->fbops = &qxlfb_ops;
/*
@@ -247,8 +241,7 @@ static int qxlfb_create(struct drm_fb_helper *helper,
info->screen_base = shadow;
info->screen_size = gobj->size;
- drm_fb_helper_fill_var(info, &qdev->fb_helper, sizes->fb_width,
- sizes->fb_height);
+ drm_fb_helper_fill_info(info, &qdev->fb_helper);
/* setup aperture base/size for vesafb takeover */
info->apertures->ranges[0].base = qdev->ddev.mode_config.fb_base;
--
2.20.1
^ permalink raw reply related
* [PATCH 01/26] drm/irq: Don't check for DRIVER_HAVE_IRQ in drm_irq_(un)install
From: Daniel Vetter @ 2019-01-24 16:58 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, linux-arm-msm, Intel Graphics Development, amd-gfx,
virtualization, linux-renesas-soc, spice-devel, Daniel Vetter,
linux-amlogic, freedreno, linux-arm-kernel
In-Reply-To: <20190124165831.16427-1-daniel.vetter@ffwll.ch>
If a non-legacy driver calls these it's valid to assume there is
interrupt support. The flag is really only needed for legacy drivers.
Also remove all the flag usage from non-legacy drivers.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: intel-gfx@lists.freedesktop.org
Cc: linux-amlogic@lists.infradead.org
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: virtualization@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Cc: linux-renesas-soc@vger.kernel.org
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
drivers/gpu/drm/arm/hdlcd_drv.c | 2 +-
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +-
drivers/gpu/drm/drm_irq.c | 6 ------
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +-
drivers/gpu/drm/gma500/psb_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/meson/meson_drv.c | 2 +-
drivers/gpu/drm/msm/msm_drv.c | 3 +--
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 +--
drivers/gpu/drm/qxl/qxl_drv.c | 2 +-
drivers/gpu/drm/radeon/radeon_drv.c | 2 +-
drivers/gpu/drm/shmobile/shmob_drm_drv.c | 2 +-
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
drivers/gpu/drm/vc4/vc4_drv.c | 1 -
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
drivers/staging/vboxvideo/vbox_drv.c | 2 +-
17 files changed, 15 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0c22bae0c736..22502417c18c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1189,7 +1189,7 @@ amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe,
static struct drm_driver kms_driver = {
.driver_features =
DRIVER_USE_AGP | DRIVER_ATOMIC |
- DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
+ DRIVER_IRQ_SHARED | DRIVER_GEM |
DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
.load = amdgpu_driver_load_kms,
.open = amdgpu_driver_open_kms,
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index e68935b80917..8fc0b884c428 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -229,7 +229,7 @@ static int hdlcd_debugfs_init(struct drm_minor *minor)
DEFINE_DRM_GEM_CMA_FOPS(fops);
static struct drm_driver hdlcd_driver = {
- .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
+ .driver_features = DRIVER_GEM |
DRIVER_MODESET | DRIVER_PRIME |
DRIVER_ATOMIC,
.irq_handler = hdlcd_irq,
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 034a91112098..0be13eceedba 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -720,7 +720,7 @@ static void atmel_hlcdc_dc_irq_uninstall(struct drm_device *dev)
DEFINE_DRM_GEM_CMA_FOPS(fops);
static struct drm_driver atmel_hlcdc_dc_driver = {
- .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
+ .driver_features = DRIVER_GEM |
DRIVER_MODESET | DRIVER_PRIME |
DRIVER_ATOMIC,
.irq_handler = atmel_hlcdc_dc_irq_handler,
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 45a07652fa00..c5babb3e4752 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -103,9 +103,6 @@ int drm_irq_install(struct drm_device *dev, int irq)
int ret;
unsigned long sh_flags = 0;
- if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ))
- return -EOPNOTSUPP;
-
if (irq == 0)
return -EINVAL;
@@ -174,9 +171,6 @@ int drm_irq_uninstall(struct drm_device *dev)
bool irq_enabled;
int i;
- if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ))
- return -EOPNOTSUPP;
-
irq_enabled = dev->irq_enabled;
dev->irq_enabled = false;
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 54ace3436605..dfc73aade325 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -137,7 +137,7 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
DEFINE_DRM_GEM_CMA_FOPS(fsl_dcu_drm_fops);
static struct drm_driver fsl_dcu_drm_driver = {
- .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET
+ .driver_features = DRIVER_GEM | DRIVER_MODESET
| DRIVER_PRIME | DRIVER_ATOMIC,
.load = fsl_dcu_load,
.unload = fsl_dcu_unload,
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index ac32ab5aa002..7cf14aeb1c28 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -468,7 +468,7 @@ static const struct file_operations psb_gem_fops = {
};
static struct drm_driver driver = {
- .driver_features = DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | \
+ .driver_features = DRIVER_IRQ_SHARED | \
DRIVER_MODESET | DRIVER_GEM,
.load = psb_driver_load,
.unload = psb_driver_unload,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 337ee650d2de..550cfb945942 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -3010,7 +3010,7 @@ static struct drm_driver driver = {
* deal with them for Intel hardware.
*/
.driver_features =
- DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME |
+ DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME |
DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_SYNCOBJ,
.release = i915_driver_release,
.open = i915_driver_open,
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 2cbc6c7bb823..2281ed3eb774 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -94,7 +94,7 @@ static irqreturn_t meson_irq(int irq, void *arg)
DEFINE_DRM_GEM_CMA_FOPS(fops);
static struct drm_driver meson_driver = {
- .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
+ .driver_features = DRIVER_GEM |
DRIVER_MODESET | DRIVER_PRIME |
DRIVER_ATOMIC,
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index d2cdc7b553fe..8747fb32a106 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1063,8 +1063,7 @@ static const struct file_operations fops = {
};
static struct drm_driver msm_driver = {
- .driver_features = DRIVER_HAVE_IRQ |
- DRIVER_GEM |
+ .driver_features = DRIVER_GEM |
DRIVER_PRIME |
DRIVER_RENDER |
DRIVER_ATOMIC |
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 54c8cdaeb576..967379f3f571 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -322,8 +322,7 @@ DEFINE_DRM_GEM_CMA_FOPS(fops);
static struct drm_driver mxsfb_driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET |
- DRIVER_PRIME | DRIVER_ATOMIC |
- DRIVER_HAVE_IRQ,
+ DRIVER_PRIME | DRIVER_ATOMIC,
.irq_handler = mxsfb_irq_handler,
.irq_preinstall = mxsfb_irq_preinstall,
.irq_uninstall = mxsfb_irq_preinstall,
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 56f2c65969c6..44daac129205 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -243,7 +243,7 @@ static struct pci_driver qxl_pci_driver = {
static struct drm_driver qxl_driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
- DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED |
+ DRIVER_IRQ_SHARED |
DRIVER_ATOMIC,
.dumb_create = qxl_mode_dumb_create,
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 8897c3d18fbb..450a9d473c30 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -535,7 +535,7 @@ radeon_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe,
static struct drm_driver kms_driver = {
.driver_features =
DRIVER_USE_AGP |
- DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
+ DRIVER_IRQ_SHARED | DRIVER_GEM |
DRIVER_PRIME | DRIVER_RENDER,
.load = radeon_driver_load_kms,
.open = radeon_driver_open_kms,
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
index 2002e7c55e80..cb821adfc321 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c
@@ -127,7 +127,7 @@ static irqreturn_t shmob_drm_irq(int irq, void *arg)
DEFINE_DRM_GEM_CMA_FOPS(shmob_drm_fops);
static struct drm_driver shmob_drm_driver = {
- .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET
+ .driver_features = DRIVER_GEM | DRIVER_MODESET
| DRIVER_PRIME,
.irq_handler = shmob_drm_irq,
.gem_free_object_unlocked = drm_gem_cma_free_object,
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 13b11acd4a5e..3030af9e7b35 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -512,7 +512,7 @@ static int tilcdc_debugfs_init(struct drm_minor *minor)
DEFINE_DRM_GEM_CMA_FOPS(fops);
static struct drm_driver tilcdc_driver = {
- .driver_features = (DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET |
+ .driver_features = (DRIVER_GEM | DRIVER_MODESET |
DRIVER_PRIME | DRIVER_ATOMIC),
.irq_handler = tilcdc_irq,
.gem_free_object_unlocked = drm_gem_cma_free_object,
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index f6f5cd80c04d..5fcd2f0da7f7 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -175,7 +175,6 @@ static struct drm_driver vc4_drm_driver = {
.driver_features = (DRIVER_MODESET |
DRIVER_ATOMIC |
DRIVER_GEM |
- DRIVER_HAVE_IRQ |
DRIVER_RENDER |
DRIVER_PRIME |
DRIVER_SYNCOBJ),
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 25afb1d594e3..d159d0400013 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1582,7 +1582,7 @@ static const struct file_operations vmwgfx_driver_fops = {
};
static struct drm_driver driver = {
- .driver_features = DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED |
+ .driver_features = DRIVER_IRQ_SHARED |
DRIVER_MODESET | DRIVER_PRIME | DRIVER_RENDER | DRIVER_ATOMIC,
.load = vmw_driver_load,
.unload = vmw_driver_unload,
diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c
index cc6532d8c2fa..78cbcd68d4f3 100644
--- a/drivers/staging/vboxvideo/vbox_drv.c
+++ b/drivers/staging/vboxvideo/vbox_drv.c
@@ -221,7 +221,7 @@ static void vbox_master_drop(struct drm_device *dev, struct drm_file *file_priv)
static struct drm_driver driver = {
.driver_features =
- DRIVER_MODESET | DRIVER_GEM | DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED |
+ DRIVER_MODESET | DRIVER_GEM | DRIVER_IRQ_SHARED |
DRIVER_PRIME | DRIVER_ATOMIC,
.dev_priv_size = 0,
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v7 0/7] Add virtio-iommu driver
From: Jean-Philippe Brucker @ 2019-01-24 16:03 UTC (permalink / raw)
To: Joerg Roedel
Cc: Mark Rutland, virtio-dev@lists.oasis-open.org, Lorenzo Pieralisi,
tnowicki@caviumnetworks.com, mst@redhat.com, Marc Zyngier,
linux-pci@vger.kernel.org, Will Deacon, Robin Murphy,
virtualization@lists.linux-foundation.org, eric.auger@redhat.com,
iommu@lists.linux-foundation.org, robh+dt@kernel.org,
bhelgaas@google.com, kvmarm@lists.cs.columbia.edu,
devicetree@vger.kernel.org
In-Reply-To: <20190123083435.x3svwqp472mdgglw@8bytes.org>
Hi Joerg,
On 23/01/2019 08:34, Joerg Roedel wrote:
> Hi Jean-Philippe,
>
> thanks for all your hard work on this!
>
> On Tue, Jan 15, 2019 at 12:19:52PM +0000, Jean-Philippe Brucker wrote:
>> Implement the virtio-iommu driver, following specification v0.9 [1].
>
> To make progress on this I think the spec needs to be close to something
> that can be included into the official virtio-specification. Have you
> proposed the specification for inclusion there?
I haven't yet. I did send a few drafts of the spec to the mailing list,
using arbitrary version numbers (0.1 - 0.9), and received excellent
feedback from Eric, Kevin, Ashok and others [2], but I hadn't formally
asked for inclusion yet. Since I haven't made any major change to the
interface in a while, I'll get on that.
> This is because I can't merge a driver that might be incompatible to
> future implementations because the specification needs to be changed on
> its way to an official standard.
Makes sense, though I think other virtio devices have been developed a
little more organically: device and driver code got upstreamed first,
and then the specification describing their interface got merged into
the standard. For example I believe that code for crypto, input and GPU
devices were upstreamed long before the specification was merged. Once
an implementation is upstream, the interface is expected to be
backward-compatible (all subsequent changes are introduced using feature
bits).
So I've been working with this process in mind, also described by Jens
at KVM forum 2017 [3]:
(1) Reserve a device ID, and get that merged into virtio (ID 23 for
virtio-iommu was reserved last year)
(2) Open-source an implementation (this driver and Eric's device)
(3) Formalize and upstream the device specification
But I get that some overlap between (2) and (3) would have been better.
So far the spec document has been reviewed mainly from the IOMMU point
of view, and might require more changes to be in line with the other
virtio devices -- hopefully just wording changes. I'll kick off step
(3), but I think the virtio folks are a bit busy with finalizing the 1.1
spec so I expect it to take a while.
Thanks,
Jean
[2] RFC https://markmail.org/thread/l6b2rpc46nua4egs
0.4 https://markmail.org/thread/f5k37mab7tnrslin
0.5 https://markmail.org/thread/tz65oolu5do7hi6n
0.6 https://markmail.org/thread/dppbg6owzrx2km2n
0.7 https://markmail.org/thread/dgdy4hicswpakmsq
[3] The future of virtio: riddles, myths and surprises
https://www.linux-kvm.org/images/0/03/Virtio_fall_2017.pdf
https://www.youtube.com/watch?v=z9cWwgYH97A
> I had a short discussion with Michael S. Tsirkin about that and from
> what I understood the spec needs to be proposed for inclusion on the
> virtio-comment[1] mailing list and later the TC needs to vote on it.
> Please work with Michael on this to get the specification official (or
> at least pretty close to something that will be part of the official
> virtio standard).
>
> Regards,
>
> Joerg
>
> [1] https://www.oasis-open.org/committees/comments/index.php?wg_abbrev=virtio
^ permalink raw reply
* Re: [PATCH 2/5] swiotlb: Add is_swiotlb_active() function
From: Joerg Roedel @ 2019-01-24 15:00 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, jon.grimm, brijesh.singh, Konrad Rzeszutek Wilk,
Michael S . Tsirkin, jfehlig, linux-kernel, virtualization,
linux-block, iommu, jroedel
In-Reply-To: <20190124084107.GA19441@lst.de>
On Thu, Jan 24, 2019 at 09:41:07AM +0100, Christoph Hellwig wrote:
> On Thu, Jan 24, 2019 at 09:29:23AM +0100, Joerg Roedel wrote:
> > > As I've just introduced and fixed a bug in this area in the current
> > > cycle - I don't think no_iotlb_memory is what your want (and maybe
> > > not useful at all): if the arch valls swiotlb_exit after previously
> > > initializing a buffer it won't be set. You probably want to check
> > > for non-zero io_tlb_start and/or io_tlb_end.
> >
> > Okay, but that requires that I also set io_tlb_start and friends back to
> > zero in the failure path of swiotlb_init(). Otherwise it could be left
> > non-zero in case swiotlb_init_with_tbl() returns an error.
>
> Indeed, and we'll need to do that anyway as otherwise the dma mapping
> path might cause problems similar to the one when swiotlb_exit is
> called that I fixed.
Turns out the the error path in swiotlb_init() is redundant because it
will never be executed. If the function returns it will always return 0
because in case of failure it will just panic (through memblock_alloc).
I'll clean that up in a separate patch-set. There are more users of that
function and all of them panic when the function fails.
Joerg
^ permalink raw reply
* Re: [PATCH] drm: Split out drm_probe_helper.h
From: Daniel Vetter @ 2019-01-24 12:39 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Neil Armstrong, Daniel Vetter, Liviu Dudau, DRI Development,
virtualization, Laurent Pinchart, Benjamin Gaignard,
Daniel Vetter, linux-stm32, linux-samsung-soc,
Oleksandr Andrushchenko, amd-gfx, linux-rockchip, nouveau, CK Hu,
spice-devel, Jani Nikula, linux-arm-msm, intel-gfx, etnaviv,
Jani Nikula, linux-mediatek
In-Reply-To: <20190124094646.GR3271@phenom.ffwll.local>
On Thu, Jan 24, 2019 at 10:46:47AM +0100, Daniel Vetter wrote:
> On Wed, Jan 23, 2019 at 06:00:15PM +0100, Sam Ravnborg wrote:
> > Hi Daniel.
> >
> > On Thu, Jan 17, 2019 at 10:03:34PM +0100, Daniel Vetter wrote:
> > > Having the probe helper stuff (which pretty much everyone needs) in
> > > the drm_crtc_helper.h file (which atomic drivers should never need) is
> > > confusing. Split them out.
> > >
> > > To make sure I actually achieved the goal here I went through all
> > > drivers. And indeed, all atomic drivers are now free of
> > > drm_crtc_helper.h includes.
> >
> > How are the plans to get this patchset merged?
> > There were dependencies on onging drmP.h removal in i915 IIRC?
> > I guess my "Minimize drmP.h dependencies" patch-set also have a role in this.
>
> Working on it, a pull request got lost. I originally wanted to get this
> all resolved this week, probably going to be next week now :-(
We're lucky, all sorted now & patch merged. I'm also merging your series,
correctly interleaved.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH 5/5] virtio-blk: Consider virtio_max_dma_size() for maximum segment size
From: Joerg Roedel @ 2019-01-24 9:51 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, jon.grimm, brijesh.singh, Konrad Rzeszutek Wilk,
Michael S . Tsirkin, jfehlig, linux-kernel, virtualization,
linux-block, iommu, jroedel
In-Reply-To: <20190124084221.GB19441@lst.de>
On Thu, Jan 24, 2019 at 09:42:21AM +0100, Christoph Hellwig wrote:
> Yes. But more importantly it would fix the limit for all other block
> drivers that set large segment sizes when running over swiotlb.
True, so it would be something like the diff below? I havn't worked on
the block layer, so I don't know if that needs additional checks for
->dev or anything.
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 3e7038e475ee..9a927280c904 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -1,6 +1,7 @@
/*
* Functions related to setting various queue properties from drivers
*/
+#include <linux/dma-mapping.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>
@@ -303,13 +304,17 @@ EXPORT_SYMBOL_GPL(blk_queue_max_discard_segments);
**/
void blk_queue_max_segment_size(struct request_queue *q, unsigned int max_size)
{
+ unsigned int dma_max_size;
+
if (max_size < PAGE_SIZE) {
max_size = PAGE_SIZE;
printk(KERN_INFO "%s: set to minimum %d\n",
__func__, max_size);
}
- q->limits.max_segment_size = max_size;
+ dma_max_size = dma_max_mapping_size(q->backing_dev_info->dev);
+
+ q->limits.max_segment_size = min(max_size, dma_max_size);
}
EXPORT_SYMBOL(blk_queue_max_segment_size);
^ permalink raw reply related
* Re: [PATCH] drm: Split out drm_probe_helper.h
From: Daniel Vetter @ 2019-01-24 9:46 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Neil Armstrong, Daniel Vetter, Liviu Dudau, DRI Development,
virtualization, Laurent Pinchart, Benjamin Gaignard,
Daniel Vetter, linux-stm32, linux-samsung-soc,
Oleksandr Andrushchenko, amd-gfx, linux-rockchip, nouveau, CK Hu,
spice-devel, Jani Nikula, linux-arm-msm, intel-gfx, etnaviv,
Jani Nikula, linux-mediatek
In-Reply-To: <20190123170015.GA23138@ravnborg.org>
On Wed, Jan 23, 2019 at 06:00:15PM +0100, Sam Ravnborg wrote:
> Hi Daniel.
>
> On Thu, Jan 17, 2019 at 10:03:34PM +0100, Daniel Vetter wrote:
> > Having the probe helper stuff (which pretty much everyone needs) in
> > the drm_crtc_helper.h file (which atomic drivers should never need) is
> > confusing. Split them out.
> >
> > To make sure I actually achieved the goal here I went through all
> > drivers. And indeed, all atomic drivers are now free of
> > drm_crtc_helper.h includes.
>
> How are the plans to get this patchset merged?
> There were dependencies on onging drmP.h removal in i915 IIRC?
> I guess my "Minimize drmP.h dependencies" patch-set also have a role in this.
Working on it, a pull request got lost. I originally wanted to get this
all resolved this week, probably going to be next week now :-(
-Daniel
> This does not hold up any work of mine, mainly wanted to make
> sure this was not lost between all the other patches.
>
> Sam
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH 1/5] swiotlb: Introduce swiotlb_max_mapping_size()
From: Christoph Hellwig @ 2019-01-24 8:47 UTC (permalink / raw)
To: Joerg Roedel
Cc: Jens Axboe, jon.grimm, brijesh.singh, Konrad Rzeszutek Wilk,
Michael S . Tsirkin, jfehlig, linux-kernel, virtualization,
linux-block, iommu, jroedel, Christoph Hellwig
In-Reply-To: <20190124082431.GH32526@8bytes.org>
On Thu, Jan 24, 2019 at 09:24:31AM +0100, Joerg Roedel wrote:
> On Wed, Jan 23, 2019 at 10:28:13PM +0100, Christoph Hellwig wrote:
> > On Wed, Jan 23, 2019 at 05:30:45PM +0100, Joerg Roedel wrote:
> > > +extern size_t swiotlb_max_mapping_size(struct device *dev);
> >
> > No need for the extern keyword on function declarations in headers.
>
> Right, but all other function declarations in that header file have
> 'extern' too, so I added it also for that one.
Your patch 3 also doesn't use an extern. And I have to say I
much prefer it that way..
^ permalink raw reply
* Re: [PATCH 5/5] virtio-blk: Consider virtio_max_dma_size() for maximum segment size
From: Christoph Hellwig @ 2019-01-24 8:42 UTC (permalink / raw)
To: Joerg Roedel
Cc: Jens Axboe, jon.grimm, brijesh.singh, Konrad Rzeszutek Wilk,
Michael S . Tsirkin, jfehlig, linux-kernel, virtualization,
linux-block, iommu, jroedel, Christoph Hellwig
In-Reply-To: <20190124084011.GJ32526@8bytes.org>
On Thu, Jan 24, 2019 at 09:40:11AM +0100, Joerg Roedel wrote:
> > I wonder if we should just move the dma max segment size check
> > into blk_queue_max_segment_size so that all block drivers benefit
> > from it. Even if not I think at least the SCSI midlayer should
> > be updated to support it.
>
> In that case the limit would also apply to virtio-blk even if it doesn't
> use the DMA-API. If that is acceptable we can move the check to
> blk_queue_max_segment_size().
Yes. But more importantly it would fix the limit for all other block
drivers that set large segment sizes when running over swiotlb.
^ permalink raw reply
* Re: [PATCH 2/5] swiotlb: Add is_swiotlb_active() function
From: Christoph Hellwig @ 2019-01-24 8:41 UTC (permalink / raw)
To: Joerg Roedel
Cc: Jens Axboe, jon.grimm, brijesh.singh, Konrad Rzeszutek Wilk,
Michael S . Tsirkin, jfehlig, linux-kernel, virtualization,
linux-block, iommu, jroedel, Christoph Hellwig
In-Reply-To: <20190124082923.GI32526@8bytes.org>
On Thu, Jan 24, 2019 at 09:29:23AM +0100, Joerg Roedel wrote:
> > As I've just introduced and fixed a bug in this area in the current
> > cycle - I don't think no_iotlb_memory is what your want (and maybe
> > not useful at all): if the arch valls swiotlb_exit after previously
> > initializing a buffer it won't be set. You probably want to check
> > for non-zero io_tlb_start and/or io_tlb_end.
>
> Okay, but that requires that I also set io_tlb_start and friends back to
> zero in the failure path of swiotlb_init(). Otherwise it could be left
> non-zero in case swiotlb_init_with_tbl() returns an error.
Indeed, and we'll need to do that anyway as otherwise the dma mapping
path might cause problems similar to the one when swiotlb_exit is
called that I fixed.
^ permalink raw reply
* Re: [PATCH 5/5] virtio-blk: Consider virtio_max_dma_size() for maximum segment size
From: Joerg Roedel @ 2019-01-24 8:40 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, jon.grimm, brijesh.singh, Konrad Rzeszutek Wilk,
Michael S . Tsirkin, jfehlig, linux-kernel, virtualization,
linux-block, iommu, jroedel
In-Reply-To: <20190123213139.GD9032@lst.de>
On Wed, Jan 23, 2019 at 10:31:39PM +0100, Christoph Hellwig wrote:
> On Wed, Jan 23, 2019 at 05:30:49PM +0100, Joerg Roedel wrote:
> > + max_size = virtio_max_dma_size(vdev);
> > +
> > /* Host can optionally specify maximum segment size and number of
> > * segments. */
> > err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX,
> > struct virtio_blk_config, size_max, &v);
> > if (!err)
> > - blk_queue_max_segment_size(q, v);
> > - else
> > - blk_queue_max_segment_size(q, -1U);
> > + max_size = min(max_size, v);
> > +
> > + blk_queue_max_segment_size(q, max_size);
>
> I wonder if we should just move the dma max segment size check
> into blk_queue_max_segment_size so that all block drivers benefit
> from it. Even if not I think at least the SCSI midlayer should
> be updated to support it.
In that case the limit would also apply to virtio-blk even if it doesn't
use the DMA-API. If that is acceptable we can move the check to
blk_queue_max_segment_size().
Regards,
Joerg
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox