* [PATCH 4/4] drm: add convert_lines_toio() variant, fix cirrus builds on powerpc.
From: Gerd Hoffmann @ 2019-04-09 11:59 UTC (permalink / raw)
To: dri-devel
Cc: Stephen Rothwell, David Airlie, Sean Paul, Maarten Lankhorst,
open list, open list:DRM DRIVER FOR QEMU'S CIRRUS DEVICE,
Maxime Ripard, noralf, Daniel Vetter, Dave Airlie, sam
In-Reply-To: <20190409115913.3468-1-kraxel@redhat.com>
The __io_virt() macro is not available on all architectures, so cirrus
can't simply pass a pointer to io memory down to the format conversion
helpers. The format conversion helpers must use memcpy_toio() instead.
Add a convert_lines_toio() variant which does just that. Switch the
drm_fb_*_dstclip() functions used by cirrus to accept a __iomem pointer
as destination and pass that down to convert_lines_toio(). Fix the
calls in the cirrus driver to not use __io_virt() any more.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
include/drm/drm_format_helper.h | 9 ++--
drivers/gpu/drm/cirrus/cirrus.c | 6 +--
drivers/gpu/drm/drm_format_helper.c | 74 ++++++++++++++++++++++-------
3 files changed, 67 insertions(+), 22 deletions(-)
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 6f84380757ee..3532b76c2340 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -15,17 +15,20 @@ struct drm_rect;
void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip);
-void drm_fb_memcpy_dstclip(void *dst, void *vaddr, struct drm_framebuffer *fb,
+void drm_fb_memcpy_dstclip(void __iomem *dst, void *vaddr,
+ struct drm_framebuffer *fb,
struct drm_rect *clip);
void drm_fb_swab16(u16 *dst, void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip);
void drm_fb_xrgb8888_to_rgb565(void *dst, void *vaddr,
struct drm_framebuffer *fb,
struct drm_rect *clip, bool swap);
-void drm_fb_xrgb8888_to_rgb565_dstclip(void *dst, unsigned int dst_pitch,
+void drm_fb_xrgb8888_to_rgb565_dstclip(void __iomem *dst,
+ unsigned int dst_pitch,
void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip, bool swap);
-void drm_fb_xrgb8888_to_rgb888_dstclip(void *dst, unsigned int dst_pitch,
+void drm_fb_xrgb8888_to_rgb888_dstclip(void __iomem *dst,
+ unsigned int dst_pitch,
void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip);
void drm_fb_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index 5095b8ce52c2..be4ea370ba31 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -307,16 +307,16 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
return -ENOMEM;
if (cirrus->cpp == fb->format->cpp[0])
- drm_fb_memcpy_dstclip(__io_virt(cirrus->vram),
+ drm_fb_memcpy_dstclip(cirrus->vram,
vmap, fb, rect);
else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2)
- drm_fb_xrgb8888_to_rgb565_dstclip(__io_virt(cirrus->vram),
+ drm_fb_xrgb8888_to_rgb565_dstclip(cirrus->vram,
cirrus->pitch,
vmap, fb, rect, false);
else if (fb->format->cpp[0] == 4 && cirrus->cpp == 3)
- drm_fb_xrgb8888_to_rgb888_dstclip(__io_virt(cirrus->vram),
+ drm_fb_xrgb8888_to_rgb888_dstclip(cirrus->vram,
cirrus->pitch,
vmap, fb, rect);
diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 01d9ea134618..6a759cbaa263 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -11,6 +11,8 @@
#include <linux/module.h>
#include <linux/slab.h>
+#include <asm/io.h>
+
#include <drm/drm_format_helper.h>
#include <drm/drm_framebuffer.h>
#include <drm/drm_fourcc.h>
@@ -125,6 +127,30 @@ static void convert_lines(void *dst, unsigned int dst_pitch,
kfree(sbuf);
}
+static void convert_lines_toio(void __iomem *dst, unsigned int dst_pitch,
+ void *src, unsigned int src_pitch,
+ unsigned int pixels,
+ unsigned int lines,
+ struct drm_format_convert *conv)
+{
+ u32 dst_linelength = pixels * conv->dst_cpp;
+ u32 y;
+ void *dbuf;
+
+ dbuf = kmalloc(dst_linelength, GFP_KERNEL);
+ if (!dbuf)
+ return;
+
+ for (y = 0; y < lines; y++) {
+ conv->func(dbuf, src, pixels);
+ memcpy_toio(dst, dbuf, dst_linelength);
+ src += src_pitch;
+ dst += dst_pitch;
+ }
+
+ kfree(dbuf);
+}
+
static u32 clip_offset(struct drm_rect *clip, u32 pitch, u32 cpp)
{
return (clip->y1 * pitch) + (clip->x1 * cpp);
@@ -143,6 +169,19 @@ static void drm_fb_memcpy_lines(void *dst, unsigned int dst_pitch,
}
}
+static void drm_fb_memcpy_lines_toio(void __iomem *dst, unsigned int dst_pitch,
+ void *src, unsigned int src_pitch,
+ unsigned int linelength, unsigned int lines)
+{
+ int line;
+
+ for (line = 0; line < lines; line++) {
+ memcpy_toio(dst, src, linelength);
+ src += src_pitch;
+ dst += dst_pitch;
+ }
+}
+
/**
* drm_fb_memcpy - Copy clip buffer
* @dst: Destination buffer
@@ -176,16 +215,17 @@ EXPORT_SYMBOL(drm_fb_memcpy);
* This function applies clipping on dst, i.e. the destination is a
* full framebuffer but only the clip rect content is copied over.
*/
-void drm_fb_memcpy_dstclip(void *dst, void *vaddr, struct drm_framebuffer *fb,
+void drm_fb_memcpy_dstclip(void __iomem *dst, void *vaddr,
+ struct drm_framebuffer *fb,
struct drm_rect *clip)
{
unsigned int cpp = drm_format_plane_cpp(fb->format->format, 0);
unsigned int offset = (clip->y1 * fb->pitches[0]) + (clip->x1 * cpp);
size_t len = (clip->x2 - clip->x1) * cpp;
- drm_fb_memcpy_lines(dst + offset, fb->pitches[0],
- vaddr + offset, fb->pitches[0],
- len, clip->y2 - clip->y1);
+ drm_fb_memcpy_lines_toio(dst + offset, fb->pitches[0],
+ vaddr + offset, fb->pitches[0],
+ len, clip->y2 - clip->y1);
}
EXPORT_SYMBOL(drm_fb_memcpy_dstclip);
@@ -245,7 +285,7 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
/**
* drm_fb_xrgb8888_to_rgb565_dstclip - Convert XRGB8888 to RGB565 clip buffer
- * @dst: RGB565 destination buffer
+ * @dst: RGB565 destination buffer (__iomem)
* @dst_pitch: destination buffer pitch
* @vaddr: XRGB8888 source buffer
* @fb: DRM framebuffer
@@ -256,9 +296,10 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565);
* support XRGB8888.
*
* This function applies clipping on dst, i.e. the destination is a
- * full framebuffer but only the clip rect content is copied over.
+ * full framebuffer, in iomem, but only the clip rect content is copied over.
*/
-void drm_fb_xrgb8888_to_rgb565_dstclip(void *dst, unsigned int dst_pitch,
+void drm_fb_xrgb8888_to_rgb565_dstclip(void __iomem *dst,
+ unsigned int dst_pitch,
void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip, bool swap)
{
@@ -272,15 +313,15 @@ void drm_fb_xrgb8888_to_rgb565_dstclip(void *dst, unsigned int dst_pitch,
size_t pixels = (clip->x2 - clip->x1);
size_t lines = (clip->y2 - clip->y1);
- convert_lines(dst + dst_offset, dst_pitch,
- vaddr + src_offset, fb->pitches[0],
- pixels, lines, conv);
+ convert_lines_toio(dst + dst_offset, dst_pitch,
+ vaddr + src_offset, fb->pitches[0],
+ pixels, lines, conv);
}
EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565_dstclip);
/**
* drm_fb_xrgb8888_to_rgb888_dstclip - Convert XRGB8888 to RGB888 clip buffer
- * @dst: RGB565 destination buffer
+ * @dst: RGB888 destination buffer (__iomem)
* @dst_pitch: destination buffer pitch
* @vaddr: XRGB8888 source buffer
* @fb: DRM framebuffer
@@ -291,9 +332,10 @@ EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb565_dstclip);
* support XRGB8888.
*
* This function applies clipping on dst, i.e. the destination is a
- * full framebuffer but only the clip rect content is copied over.
+ * full framebuffer, in iomem, but only the clip rect content is copied over.
*/
-void drm_fb_xrgb8888_to_rgb888_dstclip(void *dst, unsigned int dst_pitch,
+void drm_fb_xrgb8888_to_rgb888_dstclip(void __iomem *dst,
+ unsigned int dst_pitch,
void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip)
{
@@ -305,9 +347,9 @@ void drm_fb_xrgb8888_to_rgb888_dstclip(void *dst, unsigned int dst_pitch,
size_t pixels = (clip->x2 - clip->x1);
size_t lines = (clip->y2 - clip->y1);
- convert_lines(dst + dst_offset, dst_pitch,
- vaddr + src_offset, fb->pitches[0],
- pixels, lines, conv);
+ convert_lines_toio(dst + dst_offset, dst_pitch,
+ vaddr + src_offset, fb->pitches[0],
+ pixels, lines, conv);
}
EXPORT_SYMBOL(drm_fb_xrgb8888_to_rgb888_dstclip);
--
2.18.1
^ permalink raw reply related
* Re: [RFC PATCH 04/12] s390/cio: introduce cio DMA pool
From: Cornelia Huck @ 2019-04-09 10:44 UTC (permalink / raw)
To: Halil Pasic
Cc: Vasily Gorbik, linux-s390, Eric Farman, Claudio Imbrenda, kvm,
Sebastian Ott, Farhan Ali, virtualization, Martin Schwidefsky,
Viktor Mihajlovski, Janosch Frank
In-Reply-To: <20190404231622.52531-5-pasic@linux.ibm.com>
On Fri, 5 Apr 2019 01:16:14 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> To support protected virtualization cio will need to make sure the
> memory used for communication with the hypervisor is DMA memory.
>
> Let us introduce a DMA pool to cio that will help us in allocating
missing 'and'
> deallocating those chunks of memory.
>
> We use a gen_pool backed with DMA pages to avoid each allocation
> effectively wasting a page, as we typically allocate much less
> than PAGE_SIZE.
>
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> ---
> arch/s390/Kconfig | 1 +
> arch/s390/include/asm/cio.h | 3 +++
> drivers/s390/cio/css.c | 57 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 61 insertions(+)
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 46c69283a67b..e8099ab47368 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -194,6 +194,7 @@ config S390
> select VIRT_TO_BUS
> select HAVE_NMI
> select SWIOTLB
> + select GENERIC_ALLOCATOR
>
>
> config SCHED_OMIT_FRAME_POINTER
> diff --git a/arch/s390/include/asm/cio.h b/arch/s390/include/asm/cio.h
> index 1727180e8ca1..4510e418614a 100644
> --- a/arch/s390/include/asm/cio.h
> +++ b/arch/s390/include/asm/cio.h
> @@ -328,6 +328,9 @@ static inline u8 pathmask_to_pos(u8 mask)
> void channel_subsystem_reinit(void);
> extern void css_schedule_reprobe(void);
>
> +extern void *cio_dma_zalloc(size_t size);
> +extern void cio_dma_free(void *cpu_addr, size_t size);
> +
> /* Function from drivers/s390/cio/chsc.c */
> int chsc_sstpc(void *page, unsigned int op, u16 ctrl, u64 *clock_delta);
> int chsc_sstpi(void *page, void *result, size_t size);
> diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c
> index aea502922646..72629d99d8e4 100644
> --- a/drivers/s390/cio/css.c
> +++ b/drivers/s390/cio/css.c
> @@ -20,6 +20,8 @@
> #include <linux/reboot.h>
> #include <linux/suspend.h>
> #include <linux/proc_fs.h>
> +#include <linux/genalloc.h>
> +#include <linux/dma-mapping.h>
> #include <asm/isc.h>
> #include <asm/crw.h>
>
> @@ -886,6 +888,8 @@ static const struct attribute_group *cssdev_attr_groups[] = {
> NULL,
> };
>
> +static u64 css_dev_dma_mask = DMA_BIT_MASK(31);
> +
> static int __init setup_css(int nr)
> {
> struct channel_subsystem *css;
> @@ -899,6 +903,9 @@ static int __init setup_css(int nr)
> dev_set_name(&css->device, "css%x", nr);
> css->device.groups = cssdev_attr_groups;
> css->device.release = channel_subsystem_release;
> + /* some cio DMA memory needs to be 31 bit addressable */
> + css->device.coherent_dma_mask = DMA_BIT_MASK(31),
> + css->device.dma_mask = &css_dev_dma_mask;
Question: Does this percolate down to the child devices eventually?
E.g., you have a ccw_device getting the mask from its parent
subchannel, which gets it from its parent css? If so, does that clash
with the the mask you used for the virtio_ccw_device in a previous
patch? Or are they two really different things?
>
> mutex_init(&css->mutex);
> css->cssid = chsc_get_cssid(nr);
> @@ -1018,6 +1025,55 @@ static struct notifier_block css_power_notifier = {
> .notifier_call = css_power_event,
> };
>
> +#define POOL_INIT_PAGES 1
> +static struct gen_pool *cio_dma_pool;
> +/* Currently cio supports only a single css */
> +static struct device *cio_dma_css;
That global variable feels wrong, especially if you plan to support
MCSS-E in the future. (Do you? :) If yes, should the dma pool be global
or per-css? As css0 currently is the root device for the channel
subsystem stuff, you'd either need a new parent to hang this off from
or size this with the number of css images.)
For now, just grab channel_subsystems[0]->device directly?
> +static gfp_t cio_dma_flags;
> +
> +static void __init cio_dma_pool_init(void)
> +{
> + void *cpu_addr;
> + dma_addr_t dma_addr;
> + int i;
> +
> + cio_dma_css = &channel_subsystems[0]->device;
> + cio_dma_flags = GFP_DMA | GFP_KERNEL | __GFP_ZERO;
> + cio_dma_pool = gen_pool_create(3, -1);
> + /* No need to free up the resources: compiled in */
> + for (i = 0; i < POOL_INIT_PAGES; ++i) {
> + cpu_addr = dma_alloc_coherent(cio_dma_css, PAGE_SIZE, &dma_addr,
> + cio_dma_flags);
> + if (!cpu_addr)
> + return;
> + gen_pool_add_virt(cio_dma_pool, (unsigned long) cpu_addr,
> + dma_addr, PAGE_SIZE, -1);
> + }
> +
> +}
> +
> +void *cio_dma_zalloc(size_t size)
> +{
> + dma_addr_t dma_addr;
> + unsigned long addr = gen_pool_alloc(cio_dma_pool, size);
> +
> + if (!addr) {
> + addr = (unsigned long) dma_alloc_coherent(cio_dma_css,
> + PAGE_SIZE, &dma_addr, cio_dma_flags);
> + if (!addr)
> + return NULL;
> + gen_pool_add_virt(cio_dma_pool, addr, dma_addr, PAGE_SIZE, -1);
> + addr = gen_pool_alloc(cio_dma_pool, size);
> + }
> + return (void *) addr;
At this point, you're always going via the css0 device. I'm wondering
whether you should pass in the cssid here and use css_by_id(cssid) to
make this future proof. But then, I'm not quite clear from which
context this will be called.
> +}
> +
> +void cio_dma_free(void *cpu_addr, size_t size)
> +{
> + memset(cpu_addr, 0, size);
> + gen_pool_free(cio_dma_pool, (unsigned long) cpu_addr, size);
> +}
> +
> /*
> * Now that the driver core is running, we can setup our channel subsystem.
> * The struct subchannel's are created during probing.
> @@ -1063,6 +1119,7 @@ static int __init css_bus_init(void)
> unregister_reboot_notifier(&css_reboot_notifier);
> goto out_unregister;
> }
> + cio_dma_pool_init();
> css_init_done = 1;
>
> /* Enable default isc for I/O subchannels. */
^ permalink raw reply
* Re: [RFC PATCH 03/12] s390/mm: force swiotlb for protected virtualization
From: Cornelia Huck @ 2019-04-09 10:16 UTC (permalink / raw)
To: Halil Pasic
Cc: Vasily Gorbik, linux-s390, Eric Farman, Claudio Imbrenda, kvm,
Sebastian Ott, Farhan Ali, virtualization, Martin Schwidefsky,
Viktor Mihajlovski, Janosch Frank
In-Reply-To: <20190404231622.52531-4-pasic@linux.ibm.com>
On Fri, 5 Apr 2019 01:16:13 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> On s390 protected virtualization guests also have to use bounce I/O
> buffers. That requires some plumbing.
>
> Let us make sure any device using DMA API accordingly is spared from the
> problems that hypervisor attempting I/O to a non-shared secure page would
> bring.
I have problems parsing this sentence :(
Do you mean that we want to exclude pages for I/O from encryption?
>
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> ---
> arch/s390/Kconfig | 4 ++++
> arch/s390/include/asm/Kbuild | 1 -
> arch/s390/include/asm/dma-mapping.h | 13 +++++++++++
> arch/s390/include/asm/mem_encrypt.h | 18 +++++++++++++++
> arch/s390/mm/init.c | 44 +++++++++++++++++++++++++++++++++++++
> 5 files changed, 79 insertions(+), 1 deletion(-)
> create mode 100644 arch/s390/include/asm/dma-mapping.h
> create mode 100644 arch/s390/include/asm/mem_encrypt.h
(...)
> @@ -126,6 +129,45 @@ void mark_rodata_ro(void)
> pr_info("Write protected read-only-after-init data: %luk\n", size >> 10);
> }
>
> +int set_memory_encrypted(unsigned long addr, int numpages)
> +{
> + /* also called for the swiotlb bounce buffers, make all pages shared */
> + /* TODO: do ultravisor calls */
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(set_memory_encrypted);
> +
> +int set_memory_decrypted(unsigned long addr, int numpages)
> +{
> + /* also called for the swiotlb bounce buffers, make all pages shared */
> + /* TODO: do ultravisor calls */
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(set_memory_decrypted);
> +
> +/* are we a protected virtualization guest? */
> +bool sev_active(void)
> +{
> + /*
> + * TODO: Do proper detection using ultravisor, for now let us fake we
> + * have it so the code gets exercised.
That's the swiotlb stuff, right?
(The patches will obviously need some reordering before it is actually
getting merged.)
> + */
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(sev_active);
> +
> +/* protected virtualization */
> +static void pv_init(void)
> +{
> + if (!sev_active())
> + return;
> +
> + /* make sure bounce buffers are shared */
> + swiotlb_init(1);
> + swiotlb_update_mem_attributes();
> + swiotlb_force = SWIOTLB_FORCE;
> +}
> +
> void __init mem_init(void)
> {
> cpumask_set_cpu(0, &init_mm.context.cpu_attach_mask);
> @@ -134,6 +176,8 @@ void __init mem_init(void)
> set_max_mapnr(max_low_pfn);
> high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
>
> + pv_init();
> +
> /* Setup guest page hinting */
> cmma_init();
>
^ permalink raw reply
* Re: [RFC PATCH 02/12] virtio/s390: DMA support for virtio-ccw
From: Cornelia Huck @ 2019-04-09 9:57 UTC (permalink / raw)
To: Halil Pasic
Cc: Vasily Gorbik, linux-s390, Eric Farman, Claudio Imbrenda, kvm,
Sebastian Ott, Farhan Ali, virtualization, Martin Schwidefsky,
Viktor Mihajlovski, Janosch Frank
In-Reply-To: <20190404231622.52531-3-pasic@linux.ibm.com>
On Fri, 5 Apr 2019 01:16:12 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:
> Currently we have a problem if a virtio-ccw device has
> VIRTIO_F_IOMMU_PLATFORM.
Can you please describe what the actual problem is?
> In future we do want to support DMA API with
> virtio-ccw.
>
> Let us do the plumbing, so the feature VIRTIO_F_IOMMU_PLATFORM works
> with virtio-ccw.
>
> Let us also switch from legacy avail/used accessors to the DMA aware
> ones (even if it isn't strictly necessary).
I think with this change we can remove the legacy accessors, if I
didn't mis-grep.
>
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> ---
> drivers/s390/virtio/virtio_ccw.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
> index edf4afe2d688..5956c9e820bb 100644
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -66,6 +66,7 @@ struct virtio_ccw_device {
> bool device_lost;
> unsigned int config_ready;
> void *airq_info;
> + __u64 dma_mask;
u64?
> };
>
> struct vq_info_block_legacy {
> @@ -536,8 +537,8 @@ static struct virtqueue *virtio_ccw_setup_vq(struct virtio_device *vdev,
> info->info_block->s.desc = queue;
> info->info_block->s.index = i;
> info->info_block->s.num = info->num;
> - info->info_block->s.avail = (__u64)virtqueue_get_avail(vq);
> - info->info_block->s.used = (__u64)virtqueue_get_used(vq);
> + info->info_block->s.avail = (__u64)virtqueue_get_avail_addr(vq);
> + info->info_block->s.used = (__u64)virtqueue_get_used_addr(vq);
> ccw->count = sizeof(info->info_block->s);
> }
> ccw->cmd_code = CCW_CMD_SET_VQ;
> @@ -769,10 +770,8 @@ static u64 virtio_ccw_get_features(struct virtio_device *vdev)
> static void ccw_transport_features(struct virtio_device *vdev)
> {
> /*
> - * Packed ring isn't enabled on virtio_ccw for now,
> - * because virtio_ccw uses some legacy accessors,
> - * e.g. virtqueue_get_avail() and virtqueue_get_used()
> - * which aren't available in packed ring currently.
> + * There shouldn't be anything that precludes supporting paced.
s/paced/packed/
> + * TODO: Remove the limitation after having another look into this.
> */
> __virtio_clear_bit(vdev, VIRTIO_F_RING_PACKED);
> }
> @@ -1255,6 +1254,18 @@ static int virtio_ccw_online(struct ccw_device *cdev)
> ret = -ENOMEM;
> goto out_free;
> }
> + vcdev->vdev.dev.parent = &cdev->dev;
That one makes sense, pci and mmio are doing that as well.
> + cdev->dev.dma_mask = &vcdev->dma_mask;
That one feels a bit weird. Will this change in one of the follow-on
patches? (Have not yet looked at the whole series.)
> +
> + ret = dma_set_mask_and_coherent(&cdev->dev, DMA_BIT_MASK(64));
> + if (ret)
> + ret = dma_set_mask_and_coherent(&cdev->dev,
> + DMA_BIT_MASK(32));
> + if (ret) {
> + dev_warn(&cdev->dev, "Failed to enable 64-bit or 32-bit DMA. Trying to continue, but this might not work.\n");
This does not look like you'd try to continue?
> + goto out_free;
> + }
> +
> vcdev->config_block = kzalloc(sizeof(*vcdev->config_block),
> GFP_DMA | GFP_KERNEL);
> if (!vcdev->config_block) {
^ permalink raw reply
* Re: [PATCH RFC 0/4] vsock/virtio: optimizations to increase the throughput
From: Stefano Garzarella @ 2019-04-09 9:13 UTC (permalink / raw)
To: Jason Wang
Cc: kvm, Michael S. Tsirkin, netdev, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
In-Reply-To: <a98d194d-e257-41ff-82e7-de2d16d2f4d4@redhat.com>
On Mon, Apr 08, 2019 at 02:43:28PM +0800, Jason Wang wrote:
>
> On 2019/4/4 下午6:58, Stefano Garzarella wrote:
> > This series tries to increase the throughput of virtio-vsock with slight
> > changes:
> > - patch 1/4: reduces the number of credit update messages sent to the
> > transmitter
> > - patch 2/4: allows the host to split packets on multiple buffers,
> > in this way, we can remove the packet size limit to
> > VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE
> > - patch 3/4: uses VIRTIO_VSOCK_MAX_PKT_BUF_SIZE as the max packet size
> > allowed
> > - patch 4/4: increases RX buffer size to 64 KiB (affects only host->guest)
> >
> > RFC:
> > - maybe patch 4 can be replaced with multiple queues with different
> > buffer sizes or using EWMA to adapt the buffer size to the traffic
>
>
> Or EWMA + mergeable rx buffer, but if we decide to unify the datapath with
> virtio-net, we can reuse their codes.
>
>
> >
> > - as Jason suggested in a previous thread [1] I'll evaluate to use
> > virtio-net as transport, but I need to understand better how to
> > interface with it, maybe introducing sk_buff in virtio-vsock.
> >
> > Any suggestions?
>
>
> My understanding is this is not a must, but if it makes things easier, we
> can do this.
Hopefully it should simplify the maintainability and avoid duplicated code.
>
> Another thing that may help is to implement sendpage(), which will greatly
> improve the performance.
Thanks for your suggestions!
I'll try to implement sendpage() in VSOCK to measure the improvement.
Cheers,
Stefano
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] virtio: Honour 'may_reduce_num' in vring_create_virtqueue
From: Jason Wang @ 2019-04-09 9:00 UTC (permalink / raw)
To: Cornelia Huck, Michael S . Tsirkin
Cc: Halil Pasic, stable, linux-kernel, kvm, virtualization
In-Reply-To: <20190408123322.24086-1-cohuck@redhat.com>
On 2019/4/8 下午8:33, Cornelia Huck wrote:
> vring_create_virtqueue() allows the caller to specify via the
> may_reduce_num parameter whether the vring code is allowed to
> allocate a smaller ring than specified.
>
> However, the split ring allocation code tries to allocate a
> smaller ring on allocation failure regardless of what the
> caller specified. This may cause trouble for e.g. virtio-pci
> in legacy mode, which does not support ring resizing. (The
> packed ring code does not resize in any case.)
>
> Let's fix this by bailing out immediately in the split ring code
> if the requested size cannot be allocated and may_reduce_num has
> not been specified.
>
> While at it, fix a typo in the usage instructions.
>
> Fixes: 2a2d1382fe9d ("virtio: Add improved queue allocation API")
> Cc: stable@vger.kernel.org # v4.6+
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> drivers/virtio/virtio_ring.c | 2 ++
> include/linux/virtio_ring.h | 2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 18846afb39da..5df92c308286 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -882,6 +882,8 @@ static struct virtqueue *vring_create_virtqueue_split(
> GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO);
> if (queue)
> break;
> + if (!may_reduce_num)
> + return NULL;
> }
>
> if (!num)
> diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
> index fab02133a919..3dc70adfe5f5 100644
> --- a/include/linux/virtio_ring.h
> +++ b/include/linux/virtio_ring.h
> @@ -63,7 +63,7 @@ struct virtqueue;
> /*
> * Creates a virtqueue and allocates the descriptor ring. If
> * may_reduce_num is set, then this may allocate a smaller ring than
> - * expected. The caller should query virtqueue_get_ring_size to learn
> + * expected. The caller should query virtqueue_get_vring_size to learn
> * the actual size of the ring.
> */
> struct virtqueue *vring_create_virtqueue(unsigned int index,
Acked-by: Jason Wang <jasowang@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] drm/qxl: drop prime import/export callbacks
From: Gerd Hoffmann @ 2019-04-09 8:47 UTC (permalink / raw)
To: David Airlie
Cc: David Airlie, open list:DRM DRIVER FOR QXL VIRTUAL GPU, open list,
dri-devel, open list:DRM DRIVER FOR QXL VIRTUAL GPU
In-Reply-To: <CAMwc25q-7rfR=31R=FEVC47z2HvDK5xN+aOKqq00yoXSpudusQ@mail.gmail.com>
Hi,
> > Should we add something like DRM_PRIME_CAP_SAME_DEVICE?
>
> Yeah I expect we need some sort of same device only capability, so
> that dri3 userspace can work.
>
> If we just fail importing in these cases what happens? userspace just
> gets confused, I know we used to print a backtrace if we hit the mmap
> path, but if we didn't do that what happens?
Well, we printed a backtrace and returned -EINVAL. So it looked a bit
scary due to the backtrace which is usually printed for more serious
problems. But we also returned a proper error code.
Userspace was not happy. It was wayland (gnome-shell) which ran into it
it, and it didn't came up with a working display.
cheers,
Gerd
^ permalink raw reply
* Re: [PATCH RFC 0/4] vsock/virtio: optimizations to increase the throughput
From: Jason Wang @ 2019-04-09 8:36 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: kvm, Michael S. Tsirkin, netdev, linux-kernel, virtualization,
David S. Miller, Stefano Garzarella
In-Reply-To: <20190408094416.GQ15001@stefanha-x1.localdomain>
On 2019/4/8 下午5:44, Stefan Hajnoczi wrote:
> On Mon, Apr 08, 2019 at 02:43:28PM +0800, Jason Wang wrote:
>> Another thing that may help is to implement sendpage(), which will greatly
>> improve the performance.
> I can't find documentation for ->sendpage(). Is the idea that you get a
> struct page for the payload and can do zero-copy tx?
Yes.
> (And can userspace
> still write to the page, invalidating checksums in the header?)
>
> Stefan
Userspace can still write to the page, but for correctness (e.g in the
case of SPLICE_F_GIFT describe by vmsplice(2)), it should not do this.
For vmsplice, it may hard to detect the time to reuse the page. Maybe we
MSG_ZEROCOPY[1] is better.
Anyway, sendpage() could be still useful for sendfile() or splice().
Thanks
[1] https://netdevconf.org/2.1/papers/netdev.pdf
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 00/15] Share TTM code among framebuffer drivers
From: kraxel @ 2019-04-09 8:29 UTC (permalink / raw)
To: Dave Airlie
Cc: airlied@linux.ie, puck.chen@hisilicon.com, Zhang, Jerry,
dri-devel@lists.freedesktop.org,
virtualization@lists.linux-foundation.org,
z.liuxinliang@hisilicon.com, hdegoede@redhat.com,
kong.kongxinwei@hisilicon.com, Huang, Ray, Thomas Zimmermann,
zourongrong@gmail.com, Koenig, Christian
In-Reply-To: <CAPM=9tzg08MU0n7EhV=FVSF0FWN3qway76P5=JX1DegYHrcwvg@mail.gmail.com>
Hi,
> > The qemu stdvga (bochs driver) has 16 MB vram by default and can be
> > configured to have up to 256 MB. Plenty of room even for multiple 4k
> > framebuffers if needed. So for the bochs driver all the ttm bo
> > migration logic is not needed, it could just store everything in vram.
>
> To clarify I assume you mean it doesn't need the migrate each bo
> logic, but it still needs the when VRAM fills up migrate stuff logic.
I think even the "when vram fills up" logic isn't that important. The
driver has no acceleration so there is nothing to store beside dumb
framebuffers, and the vram size can easily be increased if needed.
cheers,
Gerd
^ permalink raw reply
* Re: [PATCH 00/15] Share TTM code among framebuffer drivers
From: Dave Airlie @ 2019-04-09 7:42 UTC (permalink / raw)
To: kraxel@redhat.com
Cc: airlied@linux.ie, puck.chen@hisilicon.com, Zhang, Jerry,
dri-devel@lists.freedesktop.org,
virtualization@lists.linux-foundation.org,
z.liuxinliang@hisilicon.com, hdegoede@redhat.com,
kong.kongxinwei@hisilicon.com, Huang, Ray, Thomas Zimmermann,
zourongrong@gmail.com, Koenig, Christian
In-Reply-To: <20190409071249.ngzarvjlztx4jwbh@sirius.home.kraxel.org>
On Tue, 9 Apr 2019 at 17:12, kraxel@redhat.com <kraxel@redhat.com> wrote:
>
> Hi,
>
> > If not for TTM, what would be the alternative? One VMA manager per
> > memory region per device?
>
> Depends pretty much on the device.
>
> The cirrus is a display device with only 4 MB of vram. You can't fit
> much in there. A single 1024x768 @ 24bpp framebuffer needs more 50%
> of the video memory already. Which is why the cirrus driver (before the
> rewrite) had to migrate buffers from/to vram on every page flip[1]. Which
> is one[2] of the reasons why cirrus (after rewrite) doesn't ttm-manage the
> vram any more. gem objects are managed with the shmem helpers instead
> and the active framebuffer is blitted to vram.
>
> The qemu stdvga (bochs driver) has 16 MB vram by default and can be
> configured to have up to 256 MB. Plenty of room even for multiple 4k
> framebuffers if needed. So for the bochs driver all the ttm bo
> migration logic is not needed, it could just store everything in vram.
To clarify I assume you mean it doesn't need the migrate each bo
logic, but it still needs the when VRAM fills up migrate stuff logic.
Dave.
^ permalink raw reply
* Re: [PATCH 00/15] Share TTM code among framebuffer drivers
From: kraxel @ 2019-04-09 7:12 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: airlied@linux.ie, puck.chen@hisilicon.com,
dri-devel@lists.freedesktop.org,
virtualization@lists.linux-foundation.org,
z.liuxinliang@hisilicon.com, hdegoede@redhat.com,
kong.kongxinwei@hisilicon.com, Huang, Ray, zourongrong@gmail.com,
daniel@ffwll.ch, Zhang, Jerry, Koenig, Christian
In-Reply-To: <096a70a7-ed24-2161-29e9-1907221b8a64@suse.de>
Hi,
> If not for TTM, what would be the alternative? One VMA manager per
> memory region per device?
Depends pretty much on the device.
The cirrus is a display device with only 4 MB of vram. You can't fit
much in there. A single 1024x768 @ 24bpp framebuffer needs more 50%
of the video memory already. Which is why the cirrus driver (before the
rewrite) had to migrate buffers from/to vram on every page flip[1]. Which
is one[2] of the reasons why cirrus (after rewrite) doesn't ttm-manage the
vram any more. gem objects are managed with the shmem helpers instead
and the active framebuffer is blitted to vram.
The qemu stdvga (bochs driver) has 16 MB vram by default and can be
configured to have up to 256 MB. Plenty of room even for multiple 4k
framebuffers if needed. So for the bochs driver all the ttm bo
migration logic is not needed, it could just store everything in vram.
A set of drm_gem_vram_* helpers would do the job for bochs.
I'd expect the same applies to the vbox driver.
Dunno about the other drm drivers and the fbdev drivers you plan to
migrate over.
cheers,
Gerd
[1] Note: The page-flip migration logic is present in some of the other
drivers too, not sure whenever they actually need that due to being low
on vram too or whenever they just copied the old cirrus code ...
[2] The other reason is that this allow to convert formats at blit time,
which helps to deal with some cirrus hardware limitations.
^ permalink raw reply
* Re: [PATCH 13/15] drm/vboxvideo: Convert vboxvideo driver to Simple TTM
From: Hans de Goede @ 2019-04-09 7:09 UTC (permalink / raw)
To: Thomas Zimmermann, daniel, airlied, kraxel, christian.koenig,
ray.huang, Jerry.Zhang, z.liuxinliang, zourongrong,
kong.kongxinwei, puck.chen
Cc: dri-devel, virtualization
In-Reply-To: <20190408092144.4548-14-tzimmermann@suse.de>
Hi,
On 08-04-19 11:21, Thomas Zimmermann wrote:
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Patch looks good to me (although perhaps it needs a commit msg):
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> drivers/gpu/drm/vboxvideo/Kconfig | 1 +
> drivers/gpu/drm/vboxvideo/vbox_drv.h | 6 +-
> drivers/gpu/drm/vboxvideo/vbox_ttm.c | 123 ++-------------------------
> 3 files changed, 12 insertions(+), 118 deletions(-)
>
> diff --git a/drivers/gpu/drm/vboxvideo/Kconfig b/drivers/gpu/drm/vboxvideo/Kconfig
> index c1ca87df81df..e29df360978d 100644
> --- a/drivers/gpu/drm/vboxvideo/Kconfig
> +++ b/drivers/gpu/drm/vboxvideo/Kconfig
> @@ -4,6 +4,7 @@ config DRM_VBOXVIDEO
> select DRM_KMS_HELPER
> select DRM_TTM
> select DRM_GEM_TTM_HELPER
> + select DRM_SIMPLE_TTM_HELPER
> select GENERIC_ALLOCATOR
> help
> This is a KMS driver for the virtual Graphics Card used in
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> index 7db4e961805d..d4cfcc6339ef 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.h
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> @@ -20,6 +20,8 @@
> #include <drm/drm_gem.h>
> #include <drm/drm_gem_ttm_helper.h>
>
> +#include <drm/drm_simple_ttm_helper.h>
> +
> #include <drm/ttm/ttm_bo_api.h>
> #include <drm/ttm/ttm_bo_driver.h>
> #include <drm/ttm/ttm_placement.h>
> @@ -78,9 +80,7 @@ struct vbox_private {
>
> int fb_mtrr;
>
> - struct {
> - struct ttm_bo_device bdev;
> - } ttm;
> + struct drm_simple_ttm ttm;
>
> struct mutex hw_mutex; /* protects modeset and accel/vbva accesses */
> struct work_struct hotplug_work;
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_ttm.c b/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> index a1d64e1ea90c..115ec44636ab 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> @@ -11,128 +11,25 @@
> #include <drm/ttm/ttm_page_alloc.h>
> #include "vbox_drv.h"
>
> -static inline struct vbox_private *vbox_bdev(struct ttm_bo_device *bd)
> -{
> - return container_of(bd, struct vbox_private, ttm.bdev);
> -}
> -
> -static int
> -vbox_bo_init_mem_type(struct ttm_bo_device *bdev, u32 type,
> - struct ttm_mem_type_manager *man)
> -{
> - switch (type) {
> - case TTM_PL_SYSTEM:
> - man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> - man->available_caching = TTM_PL_MASK_CACHING;
> - man->default_caching = TTM_PL_FLAG_CACHED;
> - break;
> - case TTM_PL_VRAM:
> - man->func = &ttm_bo_manager_func;
> - man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_MAPPABLE;
> - man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> - man->default_caching = TTM_PL_FLAG_WC;
> - break;
> - default:
> - DRM_ERROR("Unsupported memory type %u\n", (unsigned int)type);
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> -
> -static int vbox_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
> - struct ttm_mem_reg *mem)
> -{
> - struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
> - struct vbox_private *vbox = vbox_bdev(bdev);
> -
> - mem->bus.addr = NULL;
> - mem->bus.offset = 0;
> - mem->bus.size = mem->num_pages << PAGE_SHIFT;
> - mem->bus.base = 0;
> - mem->bus.is_iomem = false;
> - if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> - return -EINVAL;
> - switch (mem->mem_type) {
> - case TTM_PL_SYSTEM:
> - /* system memory */
> - return 0;
> - case TTM_PL_VRAM:
> - mem->bus.offset = mem->start << PAGE_SHIFT;
> - mem->bus.base = pci_resource_start(vbox->ddev.pdev, 0);
> - mem->bus.is_iomem = true;
> - break;
> - default:
> - return -EINVAL;
> - }
> - return 0;
> -}
> -
> -static void vbox_ttm_io_mem_free(struct ttm_bo_device *bdev,
> - struct ttm_mem_reg *mem)
> -{
> -}
> -
> -static void vbox_ttm_backend_destroy(struct ttm_tt *tt)
> -{
> - ttm_tt_fini(tt);
> - kfree(tt);
> -}
> -
> -static struct ttm_backend_func vbox_tt_backend_func = {
> - .destroy = &vbox_ttm_backend_destroy,
> -};
> -
> -static struct ttm_tt *vbox_ttm_tt_create(struct ttm_buffer_object *bo,
> - u32 page_flags)
> -{
> - struct ttm_tt *tt;
> -
> - tt = kzalloc(sizeof(*tt), GFP_KERNEL);
> - if (!tt)
> - return NULL;
> -
> - tt->func = &vbox_tt_backend_func;
> - if (ttm_tt_init(tt, bo, page_flags)) {
> - kfree(tt);
> - return NULL;
> - }
> -
> - return tt;
> -}
> -
> -static struct ttm_bo_driver vbox_bo_driver = {
> - .ttm_tt_create = vbox_ttm_tt_create,
> - .init_mem_type = vbox_bo_init_mem_type,
> - .eviction_valuable = ttm_bo_eviction_valuable,
> +static const struct drm_simple_ttm_funcs vbox_simple_ttm_funcs = {
> .evict_flags = drm_gem_ttm_bo_driver_evict_flags,
> - .verify_access = drm_gem_ttm_bo_driver_verify_access,
> - .io_mem_reserve = &vbox_ttm_io_mem_reserve,
> - .io_mem_free = &vbox_ttm_io_mem_free,
> + .verify_access = drm_gem_ttm_bo_driver_verify_access
> };
>
> int vbox_mm_init(struct vbox_private *vbox)
> {
> int ret;
> struct drm_device *dev = &vbox->ddev;
> - struct ttm_bo_device *bdev = &vbox->ttm.bdev;
>
> - ret = ttm_bo_device_init(&vbox->ttm.bdev,
> - &vbox_bo_driver,
> - dev->anon_inode->i_mapping,
> - true);
> + ret = drm_simple_ttm_init(&vbox->ttm, dev,
> + pci_resource_start(dev->pdev, 0),
> + vbox->available_vram_size,
> + &vbox_simple_ttm_funcs);
> if (ret) {
> - DRM_ERROR("Error initialising bo driver; %d\n", ret);
> + DRM_ERROR("Error initializing Simple TTM; %d\n", ret);
> return ret;
> }
>
> - ret = ttm_bo_init_mm(bdev, TTM_PL_VRAM,
> - vbox->available_vram_size >> PAGE_SHIFT);
> - if (ret) {
> - DRM_ERROR("Failed ttm VRAM init: %d\n", ret);
> - goto err_device_release;
> - }
> -
> #ifdef DRM_MTRR_WC
> vbox->fb_mtrr = drm_mtrr_add(pci_resource_start(dev->pdev, 0),
> pci_resource_len(dev->pdev, 0),
> @@ -142,10 +39,6 @@ int vbox_mm_init(struct vbox_private *vbox)
> pci_resource_len(dev->pdev, 0));
> #endif
> return 0;
> -
> -err_device_release:
> - ttm_bo_device_release(&vbox->ttm.bdev);
> - return ret;
> }
>
> void vbox_mm_fini(struct vbox_private *vbox)
> @@ -157,7 +50,7 @@ void vbox_mm_fini(struct vbox_private *vbox)
> #else
> arch_phys_wc_del(vbox->fb_mtrr);
> #endif
> - ttm_bo_device_release(&vbox->ttm.bdev);
> + drm_simple_ttm_cleanup(&vbox->ttm);
> }
>
> int vbox_mmap(struct file *filp, struct vm_area_struct *vma)
>
^ permalink raw reply
* Re: [PATCH 12/15] drm/vboxvideo: Convert vboxvideo driver to |struct drm_gem_ttm_object|
From: Hans de Goede @ 2019-04-09 7:09 UTC (permalink / raw)
To: Thomas Zimmermann, daniel, airlied, kraxel, christian.koenig,
ray.huang, Jerry.Zhang, z.liuxinliang, zourongrong,
kong.kongxinwei, puck.chen
Cc: dri-devel, virtualization
In-Reply-To: <20190408092144.4548-13-tzimmermann@suse.de>
Hi,
On 08-04-19 11:21, Thomas Zimmermann wrote:
> This patch replaces |struct vbox_bo| and its helpers with the generic
> implementation of |struct drm_gem_ttm_object|. The only change in
> semantics is that &ttm_bo_driver.verify_access() now does the actual
> verification.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Nice cleanup, thank you, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> drivers/gpu/drm/vboxvideo/Kconfig | 1 +
> drivers/gpu/drm/vboxvideo/vbox_drv.c | 5 +-
> drivers/gpu/drm/vboxvideo/vbox_drv.h | 58 +------
> drivers/gpu/drm/vboxvideo/vbox_fb.c | 22 +--
> drivers/gpu/drm/vboxvideo/vbox_main.c | 70 +-------
> drivers/gpu/drm/vboxvideo/vbox_mode.c | 36 +++--
> drivers/gpu/drm/vboxvideo/vbox_ttm.c | 223 +-------------------------
> 7 files changed, 45 insertions(+), 370 deletions(-)
>
> diff --git a/drivers/gpu/drm/vboxvideo/Kconfig b/drivers/gpu/drm/vboxvideo/Kconfig
> index 1f4182e2e980..c1ca87df81df 100644
> --- a/drivers/gpu/drm/vboxvideo/Kconfig
> +++ b/drivers/gpu/drm/vboxvideo/Kconfig
> @@ -3,6 +3,7 @@ config DRM_VBOXVIDEO
> depends on DRM && X86 && PCI
> select DRM_KMS_HELPER
> select DRM_TTM
> + select DRM_GEM_TTM_HELPER
> select GENERIC_ALLOCATOR
> help
> This is a KMS driver for the virtual Graphics Card used in
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> index fb6a0f0b8167..75b165386935 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> @@ -215,9 +215,10 @@ static struct drm_driver driver = {
> .minor = DRIVER_MINOR,
> .patchlevel = DRIVER_PATCHLEVEL,
>
> - .gem_free_object_unlocked = vbox_gem_free_object,
> + .gem_free_object_unlocked =
> + drm_gem_ttm_driver_gem_free_object_unlocked,
> .dumb_create = vbox_dumb_create,
> - .dumb_map_offset = vbox_dumb_mmap_offset,
> + .dumb_map_offset = drm_gem_ttm_driver_dumb_mmap_offset,
> .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> .gem_prime_export = drm_gem_prime_export,
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> index ece31f395540..7db4e961805d 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.h
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h
> @@ -18,6 +18,7 @@
> #include <drm/drm_encoder.h>
> #include <drm/drm_fb_helper.h>
> #include <drm/drm_gem.h>
> +#include <drm/drm_gem_ttm_helper.h>
>
> #include <drm/ttm/ttm_bo_api.h>
> #include <drm/ttm/ttm_bo_driver.h>
> @@ -170,73 +171,16 @@ int vboxfb_create(struct drm_fb_helper *helper,
> struct drm_fb_helper_surface_size *sizes);
> void vbox_fbdev_fini(struct vbox_private *vbox);
>
> -struct vbox_bo {
> - struct ttm_buffer_object bo;
> - struct ttm_placement placement;
> - struct ttm_bo_kmap_obj kmap;
> - struct drm_gem_object gem;
> - struct ttm_place placements[3];
> - int pin_count;
> -};
> -
> -#define gem_to_vbox_bo(gobj) container_of((gobj), struct vbox_bo, gem)
> -
> -static inline struct vbox_bo *vbox_bo(struct ttm_buffer_object *bo)
> -{
> - return container_of(bo, struct vbox_bo, bo);
> -}
> -
> -#define to_vbox_obj(x) container_of(x, struct vbox_gem_object, base)
> -
> -static inline u64 vbox_bo_gpu_offset(struct vbox_bo *bo)
> -{
> - return bo->bo.offset;
> -}
> -
> int vbox_dumb_create(struct drm_file *file,
> struct drm_device *dev,
> struct drm_mode_create_dumb *args);
>
> -void vbox_gem_free_object(struct drm_gem_object *obj);
> -int vbox_dumb_mmap_offset(struct drm_file *file,
> - struct drm_device *dev,
> - u32 handle, u64 *offset);
> -
> int vbox_mm_init(struct vbox_private *vbox);
> void vbox_mm_fini(struct vbox_private *vbox);
>
> -int vbox_bo_create(struct vbox_private *vbox, int size, int align,
> - u32 flags, struct vbox_bo **pvboxbo);
> -
> int vbox_gem_create(struct vbox_private *vbox,
> u32 size, bool iskernel, struct drm_gem_object **obj);
> -
> -int vbox_bo_pin(struct vbox_bo *bo, u32 pl_flag);
> -int vbox_bo_unpin(struct vbox_bo *bo);
> -
> -static inline int vbox_bo_reserve(struct vbox_bo *bo, bool no_wait)
> -{
> - int ret;
> -
> - ret = ttm_bo_reserve(&bo->bo, true, no_wait, NULL);
> - if (ret) {
> - if (ret != -ERESTARTSYS && ret != -EBUSY)
> - DRM_ERROR("reserve failed %p\n", bo);
> - return ret;
> - }
> - return 0;
> -}
> -
> -static inline void vbox_bo_unreserve(struct vbox_bo *bo)
> -{
> - ttm_bo_unreserve(&bo->bo);
> -}
> -
> -void vbox_ttm_placement(struct vbox_bo *bo, int domain);
> -int vbox_bo_push_sysram(struct vbox_bo *bo);
> int vbox_mmap(struct file *filp, struct vm_area_struct *vma);
> -void *vbox_bo_kmap(struct vbox_bo *bo);
> -void vbox_bo_kunmap(struct vbox_bo *bo);
>
> /* vbox_prime.c */
> int vbox_gem_prime_pin(struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_fb.c b/drivers/gpu/drm/vboxvideo/vbox_fb.c
> index b724fe7c0c30..1cf0c6bd58b9 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_fb.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_fb.c
> @@ -51,9 +51,9 @@ int vboxfb_create(struct drm_fb_helper *helper,
> struct drm_framebuffer *fb;
> struct fb_info *info;
> struct drm_gem_object *gobj;
> - struct vbox_bo *bo;
> + struct drm_gem_ttm_object *gbo;
> int size, ret;
> - u64 gpu_addr;
> + s64 gpu_addr;
> u32 pitch;
>
> mode_cmd.width = sizes->surface_width;
> @@ -75,9 +75,9 @@ int vboxfb_create(struct drm_fb_helper *helper,
> if (ret)
> return ret;
>
> - bo = gem_to_vbox_bo(gobj);
> + gbo = drm_gem_ttm_of_gem(gobj);
>
> - ret = vbox_bo_pin(bo, TTM_PL_FLAG_VRAM);
> + ret = drm_gem_ttm_pin(gbo, TTM_PL_FLAG_VRAM);
> if (ret)
> return ret;
>
> @@ -86,7 +86,7 @@ int vboxfb_create(struct drm_fb_helper *helper,
> return PTR_ERR(info);
>
> info->screen_size = size;
> - info->screen_base = (char __iomem *)vbox_bo_kmap(bo);
> + info->screen_base = (char __iomem *)drm_gem_ttm_kmap(gbo, true);
> if (IS_ERR(info->screen_base))
> return PTR_ERR(info->screen_base);
>
> @@ -104,7 +104,9 @@ int vboxfb_create(struct drm_fb_helper *helper,
>
> drm_fb_helper_fill_info(info, helper, sizes);
>
> - gpu_addr = vbox_bo_gpu_offset(bo);
> + gpu_addr = drm_gem_ttm_vram_offset(gbo);
> + if (gpu_addr < 0)
> + return (int)gpu_addr;
> info->fix.smem_start = info->apertures->ranges[0].base + gpu_addr;
> info->fix.smem_len = vbox->available_vram_size - gpu_addr;
>
> @@ -132,12 +134,10 @@ void vbox_fbdev_fini(struct vbox_private *vbox)
> drm_fb_helper_unregister_fbi(&vbox->fb_helper);
>
> if (afb->obj) {
> - struct vbox_bo *bo = gem_to_vbox_bo(afb->obj);
> + struct drm_gem_ttm_object *gbo = drm_gem_ttm_of_gem(afb->obj);
>
> - vbox_bo_kunmap(bo);
> -
> - if (bo->pin_count)
> - vbox_bo_unpin(bo);
> + drm_gem_ttm_kunmap(gbo);
> + drm_gem_ttm_unpin(gbo);
>
> drm_gem_object_put_unlocked(afb->obj);
> afb->obj = NULL;
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_main.c b/drivers/gpu/drm/vboxvideo/vbox_main.c
> index f4d02de5518a..0c3ede058f2b 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_main.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_main.c
> @@ -274,7 +274,7 @@ void vbox_hw_fini(struct vbox_private *vbox)
> int vbox_gem_create(struct vbox_private *vbox,
> u32 size, bool iskernel, struct drm_gem_object **obj)
> {
> - struct vbox_bo *vboxbo;
> + struct drm_gem_ttm_object *gbo;
> int ret;
>
> *obj = NULL;
> @@ -283,14 +283,15 @@ int vbox_gem_create(struct vbox_private *vbox,
> if (size == 0)
> return -EINVAL;
>
> - ret = vbox_bo_create(vbox, size, 0, 0, &vboxbo);
> - if (ret) {
> + gbo = drm_gem_ttm_create(&vbox->ddev, &vbox->ttm.bdev, size, 0, false);
> + if (IS_ERR(gbo)) {
> + ret = PTR_ERR(gbo);
> if (ret != -ERESTARTSYS)
> DRM_ERROR("failed to allocate GEM object\n");
> return ret;
> }
>
> - *obj = &vboxbo->gem;
> + *obj = &gbo->gem;
>
> return 0;
> }
> @@ -298,64 +299,9 @@ int vbox_gem_create(struct vbox_private *vbox,
> int vbox_dumb_create(struct drm_file *file,
> struct drm_device *dev, struct drm_mode_create_dumb *args)
> {
> - struct vbox_private *vbox =
> - container_of(dev, struct vbox_private, ddev);
> - struct drm_gem_object *gobj;
> - u32 handle;
> - int ret;
> -
> - args->pitch = args->width * ((args->bpp + 7) / 8);
> - args->size = args->pitch * args->height;
> -
> - ret = vbox_gem_create(vbox, args->size, false, &gobj);
> - if (ret)
> - return ret;
> -
> - ret = drm_gem_handle_create(file, gobj, &handle);
> - drm_gem_object_put_unlocked(gobj);
> - if (ret)
> - return ret;
> + struct vbox_private *vbox = dev->dev_private;
>
> - args->handle = handle;
> -
> - return 0;
> -}
> -
> -void vbox_gem_free_object(struct drm_gem_object *obj)
> -{
> - struct vbox_bo *vbox_bo = gem_to_vbox_bo(obj);
> + return drm_gem_ttm_fill_create_dumb(file, dev, &vbox->ttm.bdev, 0,
> + false, args);
>
> - ttm_bo_put(&vbox_bo->bo);
> -}
> -
> -static inline u64 vbox_bo_mmap_offset(struct vbox_bo *bo)
> -{
> - return drm_vma_node_offset_addr(&bo->bo.vma_node);
> -}
> -
> -int
> -vbox_dumb_mmap_offset(struct drm_file *file,
> - struct drm_device *dev,
> - u32 handle, u64 *offset)
> -{
> - struct drm_gem_object *obj;
> - int ret;
> - struct vbox_bo *bo;
> -
> - mutex_lock(&dev->struct_mutex);
> - obj = drm_gem_object_lookup(file, handle);
> - if (!obj) {
> - ret = -ENOENT;
> - goto out_unlock;
> - }
> -
> - bo = gem_to_vbox_bo(obj);
> - *offset = vbox_bo_mmap_offset(bo);
> -
> - drm_gem_object_put(obj);
> - ret = 0;
> -
> -out_unlock:
> - mutex_unlock(&dev->struct_mutex);
> - return ret;
> }
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> index 620a6e38f71f..faabf2801739 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> @@ -173,7 +173,8 @@ static void vbox_crtc_set_base_and_mode(struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> int x, int y)
> {
> - struct vbox_bo *bo = gem_to_vbox_bo(to_vbox_framebuffer(fb)->obj);
> + struct drm_gem_ttm_object *gbo =
> + drm_gem_ttm_of_gem(to_vbox_framebuffer(fb)->obj);
> struct vbox_private *vbox = crtc->dev->dev_private;
> struct vbox_crtc *vbox_crtc = to_vbox_crtc(crtc);
> bool needs_modeset = drm_atomic_crtc_needs_modeset(crtc->state);
> @@ -187,7 +188,7 @@ static void vbox_crtc_set_base_and_mode(struct drm_crtc *crtc,
>
> vbox_crtc->x = x;
> vbox_crtc->y = y;
> - vbox_crtc->fb_offset = vbox_bo_gpu_offset(bo);
> + vbox_crtc->fb_offset = drm_gem_ttm_vram_offset(gbo);
>
> /* vbox_do_modeset() checks vbox->single_framebuffer so update it now */
> if (needs_modeset && vbox_set_up_input_mapping(vbox)) {
> @@ -303,14 +304,14 @@ static void vbox_primary_atomic_disable(struct drm_plane *plane,
> static int vbox_primary_prepare_fb(struct drm_plane *plane,
> struct drm_plane_state *new_state)
> {
> - struct vbox_bo *bo;
> + struct drm_gem_ttm_object *gbo;
> int ret;
>
> if (!new_state->fb)
> return 0;
>
> - bo = gem_to_vbox_bo(to_vbox_framebuffer(new_state->fb)->obj);
> - ret = vbox_bo_pin(bo, TTM_PL_FLAG_VRAM);
> + gbo = drm_gem_ttm_of_gem(to_vbox_framebuffer(new_state->fb)->obj);
> + ret = drm_gem_ttm_pin(gbo, TTM_PL_FLAG_VRAM);
> if (ret)
> DRM_WARN("Error %d pinning new fb, out of video mem?\n", ret);
>
> @@ -320,13 +321,13 @@ static int vbox_primary_prepare_fb(struct drm_plane *plane,
> static void vbox_primary_cleanup_fb(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> - struct vbox_bo *bo;
> + struct drm_gem_ttm_object *gbo;
>
> if (!old_state->fb)
> return;
>
> - bo = gem_to_vbox_bo(to_vbox_framebuffer(old_state->fb)->obj);
> - vbox_bo_unpin(bo);
> + gbo = drm_gem_ttm_of_gem(to_vbox_framebuffer(old_state->fb)->obj);
> + drm_gem_ttm_unpin(gbo);
> }
>
> static int vbox_cursor_atomic_check(struct drm_plane *plane,
> @@ -386,7 +387,8 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
> container_of(plane->dev, struct vbox_private, ddev);
> struct vbox_crtc *vbox_crtc = to_vbox_crtc(plane->state->crtc);
> struct drm_framebuffer *fb = plane->state->fb;
> - struct vbox_bo *bo = gem_to_vbox_bo(to_vbox_framebuffer(fb)->obj);
> + struct drm_gem_ttm_object *gbo =
> + drm_gem_ttm_of_gem(to_vbox_framebuffer(fb)->obj);
> u32 width = plane->state->crtc_w;
> u32 height = plane->state->crtc_h;
> size_t data_size, mask_size;
> @@ -405,7 +407,7 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
> vbox_crtc->cursor_enabled = true;
>
> /* pinning is done in prepare/cleanup framebuffer */
> - src = vbox_bo_kmap(bo);
> + src = drm_gem_ttm_kmap(gbo, true);
> if (IS_ERR(src)) {
> mutex_unlock(&vbox->hw_mutex);
> DRM_WARN("Could not kmap cursor bo, skipping update\n");
> @@ -421,7 +423,7 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane,
> data_size = width * height * 4 + mask_size;
>
> copy_cursor_image(src, vbox->cursor_data, width, height, mask_size);
> - vbox_bo_kunmap(bo);
> + drm_gem_ttm_kunmap(gbo);
>
> flags = VBOX_MOUSE_POINTER_VISIBLE | VBOX_MOUSE_POINTER_SHAPE |
> VBOX_MOUSE_POINTER_ALPHA;
> @@ -461,25 +463,25 @@ static void vbox_cursor_atomic_disable(struct drm_plane *plane,
> static int vbox_cursor_prepare_fb(struct drm_plane *plane,
> struct drm_plane_state *new_state)
> {
> - struct vbox_bo *bo;
> + struct drm_gem_ttm_object *gbo;
>
> if (!new_state->fb)
> return 0;
>
> - bo = gem_to_vbox_bo(to_vbox_framebuffer(new_state->fb)->obj);
> - return vbox_bo_pin(bo, TTM_PL_FLAG_SYSTEM);
> + gbo = drm_gem_ttm_of_gem(to_vbox_framebuffer(new_state->fb)->obj);
> + return drm_gem_ttm_pin(gbo, TTM_PL_FLAG_SYSTEM);
> }
>
> static void vbox_cursor_cleanup_fb(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> - struct vbox_bo *bo;
> + struct drm_gem_ttm_object *gbo;
>
> if (!plane->state->fb)
> return;
>
> - bo = gem_to_vbox_bo(to_vbox_framebuffer(plane->state->fb)->obj);
> - vbox_bo_unpin(bo);
> + gbo = drm_gem_ttm_of_gem(to_vbox_framebuffer(plane->state->fb)->obj);
> + drm_gem_ttm_unpin(gbo);
> }
>
> static const u32 vbox_cursor_plane_formats[] = {
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_ttm.c b/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> index 9d78438c2877..a1d64e1ea90c 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_ttm.c
> @@ -16,24 +16,6 @@ static inline struct vbox_private *vbox_bdev(struct ttm_bo_device *bd)
> return container_of(bd, struct vbox_private, ttm.bdev);
> }
>
> -static void vbox_bo_ttm_destroy(struct ttm_buffer_object *tbo)
> -{
> - struct vbox_bo *bo;
> -
> - bo = container_of(tbo, struct vbox_bo, bo);
> -
> - drm_gem_object_release(&bo->gem);
> - kfree(bo);
> -}
> -
> -static bool vbox_ttm_bo_is_vbox_bo(struct ttm_buffer_object *bo)
> -{
> - if (bo->destroy == &vbox_bo_ttm_destroy)
> - return true;
> -
> - return false;
> -}
> -
> static int
> vbox_bo_init_mem_type(struct ttm_bo_device *bdev, u32 type,
> struct ttm_mem_type_manager *man)
> @@ -58,24 +40,6 @@ vbox_bo_init_mem_type(struct ttm_bo_device *bdev, u32 type,
> return 0;
> }
>
> -static void
> -vbox_bo_evict_flags(struct ttm_buffer_object *bo, struct ttm_placement *pl)
> -{
> - struct vbox_bo *vboxbo = vbox_bo(bo);
> -
> - if (!vbox_ttm_bo_is_vbox_bo(bo))
> - return;
> -
> - vbox_ttm_placement(vboxbo, TTM_PL_FLAG_SYSTEM);
> - *pl = vboxbo->placement;
> -}
> -
> -static int vbox_bo_verify_access(struct ttm_buffer_object *bo,
> - struct file *filp)
> -{
> - return 0;
> -}
> -
> static int vbox_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
> struct ttm_mem_reg *mem)
> {
> @@ -141,8 +105,8 @@ static struct ttm_bo_driver vbox_bo_driver = {
> .ttm_tt_create = vbox_ttm_tt_create,
> .init_mem_type = vbox_bo_init_mem_type,
> .eviction_valuable = ttm_bo_eviction_valuable,
> - .evict_flags = vbox_bo_evict_flags,
> - .verify_access = vbox_bo_verify_access,
> + .evict_flags = drm_gem_ttm_bo_driver_evict_flags,
> + .verify_access = drm_gem_ttm_bo_driver_verify_access,
> .io_mem_reserve = &vbox_ttm_io_mem_reserve,
> .io_mem_free = &vbox_ttm_io_mem_free,
> };
> @@ -196,165 +160,6 @@ void vbox_mm_fini(struct vbox_private *vbox)
> ttm_bo_device_release(&vbox->ttm.bdev);
> }
>
> -void vbox_ttm_placement(struct vbox_bo *bo, int domain)
> -{
> - unsigned int i;
> - u32 c = 0;
> -
> - bo->placement.placement = bo->placements;
> - bo->placement.busy_placement = bo->placements;
> -
> - if (domain & TTM_PL_FLAG_VRAM)
> - bo->placements[c++].flags =
> - TTM_PL_FLAG_WC | TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_VRAM;
> - if (domain & TTM_PL_FLAG_SYSTEM)
> - bo->placements[c++].flags =
> - TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
> - if (!c)
> - bo->placements[c++].flags =
> - TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM;
> -
> - bo->placement.num_placement = c;
> - bo->placement.num_busy_placement = c;
> -
> - for (i = 0; i < c; ++i) {
> - bo->placements[i].fpfn = 0;
> - bo->placements[i].lpfn = 0;
> - }
> -}
> -
> -int vbox_bo_create(struct vbox_private *vbox, int size, int align,
> - u32 flags, struct vbox_bo **pvboxbo)
> -{
> - struct vbox_bo *vboxbo;
> - size_t acc_size;
> - int ret;
> -
> - vboxbo = kzalloc(sizeof(*vboxbo), GFP_KERNEL);
> - if (!vboxbo)
> - return -ENOMEM;
> -
> - ret = drm_gem_object_init(&vbox->ddev, &vboxbo->gem, size);
> - if (ret)
> - goto err_free_vboxbo;
> -
> - vboxbo->bo.bdev = &vbox->ttm.bdev;
> -
> - vbox_ttm_placement(vboxbo, TTM_PL_FLAG_VRAM | TTM_PL_FLAG_SYSTEM);
> -
> - acc_size = ttm_bo_dma_acc_size(&vbox->ttm.bdev, size,
> - sizeof(struct vbox_bo));
> -
> - ret = ttm_bo_init(&vbox->ttm.bdev, &vboxbo->bo, size,
> - ttm_bo_type_device, &vboxbo->placement,
> - align >> PAGE_SHIFT, false, acc_size,
> - NULL, NULL, vbox_bo_ttm_destroy);
> - if (ret)
> - goto err_free_vboxbo;
> -
> - *pvboxbo = vboxbo;
> -
> - return 0;
> -
> -err_free_vboxbo:
> - kfree(vboxbo);
> - return ret;
> -}
> -
> -int vbox_bo_pin(struct vbox_bo *bo, u32 pl_flag)
> -{
> - struct ttm_operation_ctx ctx = { false, false };
> - int i, ret;
> -
> - if (bo->pin_count) {
> - bo->pin_count++;
> - return 0;
> - }
> -
> - ret = vbox_bo_reserve(bo, false);
> - if (ret)
> - return ret;
> -
> - vbox_ttm_placement(bo, pl_flag);
> -
> - for (i = 0; i < bo->placement.num_placement; i++)
> - bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
> -
> - ret = ttm_bo_validate(&bo->bo, &bo->placement, &ctx);
> - if (ret == 0)
> - bo->pin_count = 1;
> -
> - vbox_bo_unreserve(bo);
> -
> - return ret;
> -}
> -
> -int vbox_bo_unpin(struct vbox_bo *bo)
> -{
> - struct ttm_operation_ctx ctx = { false, false };
> - int i, ret;
> -
> - if (!bo->pin_count) {
> - DRM_ERROR("unpin bad %p\n", bo);
> - return 0;
> - }
> - bo->pin_count--;
> - if (bo->pin_count)
> - return 0;
> -
> - ret = vbox_bo_reserve(bo, false);
> - if (ret) {
> - DRM_ERROR("Error %d reserving bo, leaving it pinned\n", ret);
> - return ret;
> - }
> -
> - for (i = 0; i < bo->placement.num_placement; i++)
> - bo->placements[i].flags &= ~TTM_PL_FLAG_NO_EVICT;
> -
> - ret = ttm_bo_validate(&bo->bo, &bo->placement, &ctx);
> -
> - vbox_bo_unreserve(bo);
> -
> - return ret;
> -}
> -
> -/*
> - * Move a vbox-owned buffer object to system memory if no one else has it
> - * pinned. The caller must have pinned it previously, and this call will
> - * release the caller's pin.
> - */
> -int vbox_bo_push_sysram(struct vbox_bo *bo)
> -{
> - struct ttm_operation_ctx ctx = { false, false };
> - int i, ret;
> -
> - if (!bo->pin_count) {
> - DRM_ERROR("unpin bad %p\n", bo);
> - return 0;
> - }
> - bo->pin_count--;
> - if (bo->pin_count)
> - return 0;
> -
> - if (bo->kmap.virtual) {
> - ttm_bo_kunmap(&bo->kmap);
> - bo->kmap.virtual = NULL;
> - }
> -
> - vbox_ttm_placement(bo, TTM_PL_FLAG_SYSTEM);
> -
> - for (i = 0; i < bo->placement.num_placement; i++)
> - bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
> -
> - ret = ttm_bo_validate(&bo->bo, &bo->placement, &ctx);
> - if (ret) {
> - DRM_ERROR("pushing to VRAM failed\n");
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> int vbox_mmap(struct file *filp, struct vm_area_struct *vma)
> {
> struct drm_file *file_priv = filp->private_data;
> @@ -362,27 +167,3 @@ int vbox_mmap(struct file *filp, struct vm_area_struct *vma)
>
> return ttm_bo_mmap(filp, vma, &vbox->ttm.bdev);
> }
> -
> -void *vbox_bo_kmap(struct vbox_bo *bo)
> -{
> - int ret;
> -
> - if (bo->kmap.virtual)
> - return bo->kmap.virtual;
> -
> - ret = ttm_bo_kmap(&bo->bo, 0, bo->bo.num_pages, &bo->kmap);
> - if (ret) {
> - DRM_ERROR("Error kmapping bo: %d\n", ret);
> - return NULL;
> - }
> -
> - return bo->kmap.virtual;
> -}
> -
> -void vbox_bo_kunmap(struct vbox_bo *bo)
> -{
> - if (bo->kmap.virtual) {
> - ttm_bo_kunmap(&bo->kmap);
> - bo->kmap.virtual = NULL;
> - }
> -}
>
^ permalink raw reply
* Re: [PATCH] drm/qxl: drop prime import/export callbacks
From: David Airlie @ 2019-04-09 6:06 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: David Airlie, open list, dri-devel,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Dave Airlie,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Dave Airlie
In-Reply-To: <20190409060323.xrw3fwj3yvcxsx3t@sirius.home.kraxel.org>
On Tue, Apr 9, 2019 at 4:03 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Tue, Apr 09, 2019 at 02:01:33PM +1000, Dave Airlie wrote:
> > On Sat, 12 Jan 2019 at 07:13, Dave Airlie <airlied@gmail.com> wrote:
> > >
> > > On Thu, 10 Jan 2019 at 18:17, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > > >
> > > > Also set prime_handle_to_fd and prime_fd_to_handle to NULL,
> > > > so drm will not advertive DRM_PRIME_CAP_{IMPORT,EXPORT} to
> > > > userspace.
> >
> > It's been pointed out to me that disables DRI3 for these devices, I'm
> > not sure that is the solution we actually wanted.
> >
> > any ideas?
>
> Well. Lets have a look at where we stand:
>
> * drm_gem_prime_export() works with qxl, you'll get a dma-buf handle.
>
> * Other drivers trying to map that dma-buf (drm_gem_map_dma_buf()
> callback) will not work, due to the ->gem_prime_get_sg_table()
> callback not being there.
>
> * drm_gem_prime_import() will work with buffers from the same qxl
> device, there is a shortcut for this special case. Otherwise it
> will not work, due to the ->gem_prime_import_sg_table() callback
> not being there.
>
> Bottom line: you can use prime to pass qxl object handles from one
> application to another. But you can't actually export/import qxl
> buffer objects from/to other devices.
>
> Problem is that we have no way to signal to userspace that prime can
> be used that way.
>
> Setting DRM_PRIME_CAP_{IMPORT,EXPORT} even though the driver can't
> do that leads to other problems. Userspace thinks it can have other
> devices (intel vgpu for example) handle the rendering, then import
> the rendered buffer into qxl for scanout.
>
> Should we add something like DRM_PRIME_CAP_SAME_DEVICE?
Yeah I expect we need some sort of same device only capability, so
that dri3 userspace can work.
If we just fail importing in these cases what happens? userspace just
gets confused, I know we used to print a backtrace if we hit the mmap
path, but if we didn't do that what happens?
Dave.
^ permalink raw reply
* Re: [PATCH] drm/qxl: drop prime import/export callbacks
From: Gerd Hoffmann @ 2019-04-09 6:03 UTC (permalink / raw)
To: Dave Airlie
Cc: David Airlie, open list, dri-devel,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Dave Airlie,
open list:DRM DRIVER FOR QXL VIRTUAL GPU
In-Reply-To: <CAPM=9tyEmn2LXhNmPSOi7JAgBc3QZwNtM+8FT8CWebz4GztaJA@mail.gmail.com>
On Tue, Apr 09, 2019 at 02:01:33PM +1000, Dave Airlie wrote:
> On Sat, 12 Jan 2019 at 07:13, Dave Airlie <airlied@gmail.com> wrote:
> >
> > On Thu, 10 Jan 2019 at 18:17, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > > Also set prime_handle_to_fd and prime_fd_to_handle to NULL,
> > > so drm will not advertive DRM_PRIME_CAP_{IMPORT,EXPORT} to
> > > userspace.
>
> It's been pointed out to me that disables DRI3 for these devices, I'm
> not sure that is the solution we actually wanted.
>
> any ideas?
Well. Lets have a look at where we stand:
* drm_gem_prime_export() works with qxl, you'll get a dma-buf handle.
* Other drivers trying to map that dma-buf (drm_gem_map_dma_buf()
callback) will not work, due to the ->gem_prime_get_sg_table()
callback not being there.
* drm_gem_prime_import() will work with buffers from the same qxl
device, there is a shortcut for this special case. Otherwise it
will not work, due to the ->gem_prime_import_sg_table() callback
not being there.
Bottom line: you can use prime to pass qxl object handles from one
application to another. But you can't actually export/import qxl
buffer objects from/to other devices.
Problem is that we have no way to signal to userspace that prime can
be used that way.
Setting DRM_PRIME_CAP_{IMPORT,EXPORT} even though the driver can't
do that leads to other problems. Userspace thinks it can have other
devices (intel vgpu for example) handle the rendering, then import
the rendered buffer into qxl for scanout.
Should we add something like DRM_PRIME_CAP_SAME_DEVICE?
cheers,
Gerd
^ permalink raw reply
* [PATCH net] vhost: flush dcache page when logging dirty pages
From: Jason Wang @ 2019-04-09 4:16 UTC (permalink / raw)
To: mst, jasowang, kvm, virtualization, netdev, linux-kernel
Cc: Christoph Hellwig, James Bottomley, Andrea Arcangeli
We set dirty bit through setting up kmaps and access them through
kernel virtual address, this may result alias in virtually tagged
caches that require a dcache flush afterwards.
Cc: Christoph Hellwig <hch@infradead.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Fixes: 3a4d5c94e9593 ("vhost_net: a kernel-level virtio server")
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 351af88231ad..34a1cedbc5ba 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1711,6 +1711,7 @@ static int set_bit_to_user(int nr, void __user *addr)
base = kmap_atomic(page);
set_bit(bit, base);
kunmap_atomic(base);
+ flush_dcache_page(page);
set_page_dirty_lock(page);
put_page(page);
return 0;
--
2.19.1
^ permalink raw reply related
* [PATCH net] vhost: reject zero size iova range
From: Jason Wang @ 2019-04-09 4:10 UTC (permalink / raw)
To: mst, jasowang, kvm, virtualization, netdev, linux-kernel
We used to accept zero size iova range which will lead a infinite loop
in translate_desc(). Fixing this by failing the request in this case.
Reported-by: syzbot+d21e6e297322a900c128@syzkaller.appspotmail.com
Fixes: 6b1e6cc7 ("vhost: new device IOTLB API")
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 5ace833de746..351af88231ad 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -911,8 +911,12 @@ static int vhost_new_umem_range(struct vhost_umem *umem,
u64 start, u64 size, u64 end,
u64 userspace_addr, int perm)
{
- struct vhost_umem_node *tmp, *node = kmalloc(sizeof(*node), GFP_ATOMIC);
+ struct vhost_umem_node *tmp, *node;
+ if (!size)
+ return -EFAULT;
+
+ node = kmalloc(sizeof(*node), GFP_ATOMIC);
if (!node)
return -ENOMEM;
--
2.19.1
^ permalink raw reply related
* Re: [PATCH] drm/qxl: drop prime import/export callbacks
From: Dave Airlie @ 2019-04-09 4:01 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: David Airlie, open list, dri-devel,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Dave Airlie,
open list:DRM DRIVER FOR QXL VIRTUAL GPU
In-Reply-To: <CAPM=9tx8N6jxVzsT-RqtQFrFb9qLK4qUUje+d=fpTYZ6+PnYpg@mail.gmail.com>
On Sat, 12 Jan 2019 at 07:13, Dave Airlie <airlied@gmail.com> wrote:
>
> On Thu, 10 Jan 2019 at 18:17, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > Also set prime_handle_to_fd and prime_fd_to_handle to NULL,
> > so drm will not advertive DRM_PRIME_CAP_{IMPORT,EXPORT} to
> > userspace.
It's been pointed out to me that disables DRI3 for these devices, I'm
not sure that is the solution we actually wanted.
any ideas?
Dave.
> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>
> Reviewed-by: Dave Airlie <airlied@redhat.com>
> > ---
> > drivers/gpu/drm/qxl/qxl_drv.c | 4 ----
> > drivers/gpu/drm/qxl/qxl_prime.c | 14 --------------
> > 2 files changed, 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> > index 13c8a662f9..ccb090f3ab 100644
> > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > @@ -250,14 +250,10 @@ static struct drm_driver qxl_driver = {
> > #if defined(CONFIG_DEBUG_FS)
> > .debugfs_init = qxl_debugfs_init,
> > #endif
> > - .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> > - .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> > .gem_prime_export = drm_gem_prime_export,
> > .gem_prime_import = drm_gem_prime_import,
> > .gem_prime_pin = qxl_gem_prime_pin,
> > .gem_prime_unpin = qxl_gem_prime_unpin,
> > - .gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
> > - .gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
> > .gem_prime_vmap = qxl_gem_prime_vmap,
> > .gem_prime_vunmap = qxl_gem_prime_vunmap,
> > .gem_prime_mmap = qxl_gem_prime_mmap,
> > diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
> > index a55dece118..df65d3c1a7 100644
> > --- a/drivers/gpu/drm/qxl/qxl_prime.c
> > +++ b/drivers/gpu/drm/qxl/qxl_prime.c
> > @@ -38,20 +38,6 @@ void qxl_gem_prime_unpin(struct drm_gem_object *obj)
> > WARN_ONCE(1, "not implemented");
> > }
> >
> > -struct sg_table *qxl_gem_prime_get_sg_table(struct drm_gem_object *obj)
> > -{
> > - WARN_ONCE(1, "not implemented");
> > - return ERR_PTR(-ENOSYS);
> > -}
> > -
> > -struct drm_gem_object *qxl_gem_prime_import_sg_table(
> > - struct drm_device *dev, struct dma_buf_attachment *attach,
> > - struct sg_table *table)
> > -{
> > - WARN_ONCE(1, "not implemented");
> > - return ERR_PTR(-ENOSYS);
> > -}
> > -
> > void *qxl_gem_prime_vmap(struct drm_gem_object *obj)
> > {
> > WARN_ONCE(1, "not implemented");
> > --
> > 2.9.3
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: INFO: task hung in vhost_net_stop_vq
From: Jason Wang @ 2019-04-09 3:31 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: syzbot, weiyj.lk, KVM list, Michael S. Tsirkin, netdev,
syzkaller-bugs, LKML, virtualization
In-Reply-To: <CACT4Y+Z1s-Lx1UXHKj88kQoOcbiD8gwyuRU3F_+cceP3pzbbrw@mail.gmail.com>
On 2019/3/26 下午6:28, Dmitry Vyukov wrote:
> On Tue, Mar 26, 2019 at 11:17 AM Jason Wang<jasowang@redhat.com> wrote:
>> On 2019/3/25 下午10:02, Michael S. Tsirkin wrote:
>>> Looks like more iotlb locking mess?
>> Looking at the calltrace:
>>
>> [ 221.743675] =============================================
>> [ 221.744297] [ INFO: possible recursive locking detected ]
>> [ 221.744944] 4.7.0+ #1 Not tainted
>> [ 221.745326] ---------------------------------------------
>> [ 221.746128] syz-executor1/6823 is trying to acquire lock:
>> [ 221.746737] (&vq->mutex){+.+...}, at: [<ffffffff84484b70>] vhost_process_iotlb_msg+0xe0/0x9e0
>> [ 221.747789]
>> [ 221.747789] but task is already holding lock:
>> [ 221.748470] (&vq->mutex){+.+...}, at: [<ffffffff84484b70>] vhost_process_iotlb_msg+0xe0/0x9e0
>> [ 221.749535]
>> [ 221.749535] other info that might help us debug this:
>> [ 221.750280] Possible unsafe locking scenario:
>> [ 221.750280]
>> [ 221.750946] CPU0
>> [ 221.751232] ----
>> [ 221.751523] lock(&vq->mutex);
>> [ 221.751922] lock(&vq->mutex);
>> [ 221.752339]
>> [ 221.752339] *** DEADLOCK ***
>> [ 221.752339]
>>
>> I could not think of a path that can hit this. And I could not reproduce with the reproducer in the link in net-next.
> Looking at the bisection log, syzbot is able to reproduce this
> super-reliably on multiple kernel revisions. Are you sure you are
> using the right config/revision? What else can be in play? syzbot uses
> VMs. The image is available.
>
>
Yes, looks like the reason is vhost accept zero size iova range which
lead a infinite loop when trying to translate iova. Will post a patch to
fix this.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net v7] failover: allow name change on IFF_UP slave interfaces
From: Michael S. Tsirkin @ 2019-04-08 21:00 UTC (permalink / raw)
To: Si-Wei Liu
Cc: jiri, kubakici, sridhar.samudrala, alexander.duyck,
virtualization, liran.alon, netdev, boris.ostrovsky, davem
In-Reply-To: <1554755085-29098-1-git-send-email-si-wei.liu@oracle.com>
On Mon, Apr 08, 2019 at 04:24:45PM -0400, Si-Wei Liu wrote:
> When a netdev appears through hot plug then gets enslaved by a failover
> master that is already up and running, the slave will be opened
> right away after getting enslaved. Today there's a race that userspace
> (udev) may fail to rename the slave if the kernel (net_failover)
> opens the slave earlier than when the userspace rename happens.
> Unlike bond or team, the primary slave of failover can't be renamed by
> userspace ahead of time, since the kernel initiated auto-enslavement is
> unable to, or rather, is never meant to be synchronized with the rename
> request from userspace.
>
> As the failover slave interfaces are not designed to be operated
> directly by userspace apps: IP configuration, filter rules with
> regard to network traffic passing and etc., should all be done on master
> interface. In general, userspace apps only care about the
> name of master interface, while slave names are less important as long
> as admin users can see reliable names that may carry
> other information describing the netdev. For e.g., they can infer that
> "ens3nsby" is a standby slave of "ens3", while for a
> name like "eth0" they can't tell which master it belongs to.
>
> Historically the name of IFF_UP interface can't be changed because
> there might be admin script or management software that is already
> relying on such behavior and assumes that the slave name can't be
> changed once UP. But failover is special: with the in-kernel
> auto-enslavement mechanism, the userspace expectation for device
> enumeration and bring-up order is already broken. Previously initramfs
> and various userspace config tools were modified to bypass failover
> slaves because of auto-enslavement and duplicate MAC address. Similarly,
> in case that users care about seeing reliable slave name, the new type
> of failover slaves needs to be taken care of specifically in userspace
> anyway.
>
> It's less risky to lift up the rename restriction on failover slave
> which is already UP. Although it's possible this change may potentially
> break userspace component (most likely configuration scripts or
> management software) that assumes slave name can't be changed while
> UP, it's relatively a limited and controllable set among all userspace
> components, which can be fixed specifically to listen for the rename
> and/or link down/up events on failover slaves.
This was correct with v6. With v7 link down/up events are no longer
emitted.
> Userspace component
> interacting with slaves is expected to be changed to operate on failover
> master interface instead, as the failover slave is dynamic in nature
> which may come and go at any point. The goal is to make the role of
> failover slaves less relevant, and userspace components should only
> deal with failover master in the long run.
>
> Fixes: 30c8bd5aa8b2 ("net: Introduce generic failover module")
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> Reviewed-by: Liran Alon <liran.alon@oracle.com>
>
> --
> v1 -> v2:
> - Drop configurable module parameter (Sridhar)
>
> v2 -> v3:
> - Drop additional IFF_SLAVE_RENAME_OK flag (Sridhar)
> - Send down and up events around rename (Michael S. Tsirkin)
>
> v3 -> v4:
> - Simplify notification to be sent (Stephen Hemminger)
>
> v4 -> v5:
> - Sync up code with latest net-next (Sridhar)
> - Use proper structure initialization (Stephen, Jiri)
>
> v5 -> v6:
> - Make the property of live name change a generic flag (Stephen)
>
> v6 -> v7:
> - Remove NETDEV_CHANGE notification that is deemed unnecessary
> (Stephen)
> ---
I preferred v6 better. If we emit NETDEV_CHANGE then
userspace that expects that name never changes for up
interfaces will work since it will think state changed.
> include/linux/netdevice.h | 3 +++
> net/core/dev.c | 16 +++++++++++++++-
> net/core/failover.c | 6 +++---
> 3 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 78f5ec4e..ea9a63f 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1498,6 +1498,7 @@ struct net_device_ops {
> * @IFF_FAILOVER: device is a failover master device
> * @IFF_FAILOVER_SLAVE: device is lower dev of a failover master device
> * @IFF_L3MDEV_RX_HANDLER: only invoke the rx handler of L3 master device
> + * @IFF_LIVE_RENAME_OK: rename is allowed while device is up and running
> */
> enum netdev_priv_flags {
> IFF_802_1Q_VLAN = 1<<0,
> @@ -1530,6 +1531,7 @@ enum netdev_priv_flags {
> IFF_FAILOVER = 1<<27,
> IFF_FAILOVER_SLAVE = 1<<28,
> IFF_L3MDEV_RX_HANDLER = 1<<29,
> + IFF_LIVE_RENAME_OK = 1<<30,
> };
>
> #define IFF_802_1Q_VLAN IFF_802_1Q_VLAN
> @@ -1561,6 +1563,7 @@ enum netdev_priv_flags {
> #define IFF_FAILOVER IFF_FAILOVER
> #define IFF_FAILOVER_SLAVE IFF_FAILOVER_SLAVE
> #define IFF_L3MDEV_RX_HANDLER IFF_L3MDEV_RX_HANDLER
> +#define IFF_LIVE_RENAME_OK IFF_LIVE_RENAME_OK
>
> /**
> * struct net_device - The DEVICE structure.
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 9823b77..1622d88 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1185,7 +1185,21 @@ int dev_change_name(struct net_device *dev, const char *newname)
> BUG_ON(!dev_net(dev));
>
> net = dev_net(dev);
> - if (dev->flags & IFF_UP)
> +
> + /* Some auto-enslaved devices e.g. failover slaves are
> + * special, as userspace might rename the device after
> + * the interface had been brought up and running since
> + * the point kernel initiated auto-enslavement. Allow
> + * live name change even when these slave devices are
> + * up and running.
> + *
> + * Typically, users of these auto-enslaving devices
> + * don't actually care about slave name change, as
> + * they are supposed to operate on master interface
> + * directly.
> + */
> + if (dev->flags & IFF_UP &&
> + likely(!(dev->priv_flags & IFF_LIVE_RENAME_OK)))
> return -EBUSY;
>
> write_seqcount_begin(&devnet_rename_seq);
> diff --git a/net/core/failover.c b/net/core/failover.c
> index 4a92a98..b5cd3c7 100644
> --- a/net/core/failover.c
> +++ b/net/core/failover.c
> @@ -80,14 +80,14 @@ static int failover_slave_register(struct net_device *slave_dev)
> goto err_upper_link;
> }
>
> - slave_dev->priv_flags |= IFF_FAILOVER_SLAVE;
> + slave_dev->priv_flags |= (IFF_FAILOVER_SLAVE | IFF_LIVE_RENAME_OK);
>
> if (fops && fops->slave_register &&
> !fops->slave_register(slave_dev, failover_dev))
> return NOTIFY_OK;
>
> netdev_upper_dev_unlink(slave_dev, failover_dev);
> - slave_dev->priv_flags &= ~IFF_FAILOVER_SLAVE;
> + slave_dev->priv_flags &= ~(IFF_FAILOVER_SLAVE | IFF_LIVE_RENAME_OK);
> err_upper_link:
> netdev_rx_handler_unregister(slave_dev);
> done:
> @@ -121,7 +121,7 @@ int failover_slave_unregister(struct net_device *slave_dev)
>
> netdev_rx_handler_unregister(slave_dev);
> netdev_upper_dev_unlink(slave_dev, failover_dev);
> - slave_dev->priv_flags &= ~IFF_FAILOVER_SLAVE;
> + slave_dev->priv_flags &= ~(IFF_FAILOVER_SLAVE | IFF_LIVE_RENAME_OK);
>
> if (fops && fops->slave_unregister &&
> !fops->slave_unregister(slave_dev, failover_dev))
> --
> 1.8.3.1
^ permalink raw reply
* Re: [PATCH RFC 3/4] vsock/virtio: change the maximum packet size allowed
From: Stefan Hajnoczi @ 2019-04-08 15:45 UTC (permalink / raw)
To: Stefano Garzarella
Cc: kvm, Michael S. Tsirkin, linux-kernel, virtualization, netdev,
David S. Miller
In-Reply-To: <20190408151735.itsfswajk5ww3ejv@steredhat>
[-- Attachment #1.1: Type: text/plain, Size: 1461 bytes --]
On Mon, Apr 08, 2019 at 05:17:35PM +0200, Stefano Garzarella wrote:
> On Mon, Apr 08, 2019 at 10:57:44AM -0400, Michael S. Tsirkin wrote:
> > On Mon, Apr 08, 2019 at 04:55:31PM +0200, Stefano Garzarella wrote:
> > > > Anyway, any change to this behavior requires compatibility so new guest
> > > > drivers work with old vhost_vsock.ko. Therefore we should probably just
> > > > leave the limit for now.
> > >
> > > I understood your point of view and I completely agree with you.
> > > But, until we don't have a way to expose features/versions between guest
> > > and host,
> >
> > Why not use the standard virtio feature negotiation mechanism for this?
> >
>
> Yes, I have this in my mind :), but I want to understand better if we can
> use virtio-net also for this mechanism.
> For now, I don't think limiting the packets to 64 KiB is a big issue.
>
> What do you think if I postpone this when I have more clear if we can
> use virtio-net or not? (in order to avoid duplicated work)
Yes, I agree. VIRTIO has feature negotiation and we can use it to
change this behavior cleanly.
However, this will require a spec change and this patch series delivers
significant performance improvements that can be merged sooner than
VIRTIO spec changes.
Let's defer the max packet size change via VIRTIO feature bits. It can
be done separately if we decide to stick to the virtio-vsock device
design and not virtio-net.
Stefan
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH RFC 3/4] vsock/virtio: change the maximum packet size allowed
From: Stefano Garzarella @ 2019-04-08 15:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm, linux-kernel, virtualization, Stefan Hajnoczi, netdev,
David S. Miller
In-Reply-To: <20190408105619-mutt-send-email-mst@kernel.org>
On Mon, Apr 08, 2019 at 10:57:44AM -0400, Michael S. Tsirkin wrote:
> On Mon, Apr 08, 2019 at 04:55:31PM +0200, Stefano Garzarella wrote:
> > > Anyway, any change to this behavior requires compatibility so new guest
> > > drivers work with old vhost_vsock.ko. Therefore we should probably just
> > > leave the limit for now.
> >
> > I understood your point of view and I completely agree with you.
> > But, until we don't have a way to expose features/versions between guest
> > and host,
>
> Why not use the standard virtio feature negotiation mechanism for this?
>
Yes, I have this in my mind :), but I want to understand better if we can
use virtio-net also for this mechanism.
For now, I don't think limiting the packets to 64 KiB is a big issue.
What do you think if I postpone this when I have more clear if we can
use virtio-net or not? (in order to avoid duplicated work)
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH RFC 3/4] vsock/virtio: change the maximum packet size allowed
From: Michael S. Tsirkin @ 2019-04-08 14:57 UTC (permalink / raw)
To: Stefano Garzarella
Cc: kvm, linux-kernel, virtualization, Stefan Hajnoczi, netdev,
David S. Miller
In-Reply-To: <20190408145531.yreyawkn5vjqj7sl@steredhat>
On Mon, Apr 08, 2019 at 04:55:31PM +0200, Stefano Garzarella wrote:
> > Anyway, any change to this behavior requires compatibility so new guest
> > drivers work with old vhost_vsock.ko. Therefore we should probably just
> > leave the limit for now.
>
> I understood your point of view and I completely agree with you.
> But, until we don't have a way to expose features/versions between guest
> and host,
Why not use the standard virtio feature negotiation mechanism for this?
> maybe is better to leave the limit in order to be compatible
> with old vhost_vsock.
--
MST
^ permalink raw reply
* Re: [PATCH RFC 3/4] vsock/virtio: change the maximum packet size allowed
From: Stefano Garzarella @ 2019-04-08 14:55 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: kvm, Michael S. Tsirkin, linux-kernel, virtualization, netdev,
David S. Miller
In-Reply-To: <20190408093723.GP15001@stefanha-x1.localdomain>
On Mon, Apr 08, 2019 at 10:37:23AM +0100, Stefan Hajnoczi wrote:
> On Fri, Apr 05, 2019 at 12:07:47PM +0200, Stefano Garzarella wrote:
> > On Fri, Apr 05, 2019 at 09:24:47AM +0100, Stefan Hajnoczi wrote:
> > > On Thu, Apr 04, 2019 at 12:58:37PM +0200, Stefano Garzarella wrote:
> > > > Since now we are able to split packets, we can avoid limiting
> > > > their sizes to VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE.
> > > > Instead, we can use VIRTIO_VSOCK_MAX_PKT_BUF_SIZE as the max
> > > > packet size.
> > > >
> > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > > ---
> > > > net/vmw_vsock/virtio_transport_common.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > > > index f32301d823f5..822e5d07a4ec 100644
> > > > --- a/net/vmw_vsock/virtio_transport_common.c
> > > > +++ b/net/vmw_vsock/virtio_transport_common.c
> > > > @@ -167,8 +167,8 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> > > > vvs = vsk->trans;
> > > >
> > > > /* we can send less than pkt_len bytes */
> > > > - if (pkt_len > VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE)
> > > > - pkt_len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
> > > > + if (pkt_len > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE)
> > > > + pkt_len = VIRTIO_VSOCK_MAX_PKT_BUF_SIZE;
> > >
> > > The next line limits pkt_len based on available credits:
> > >
> > > /* virtio_transport_get_credit might return less than pkt_len credit */
> > > pkt_len = virtio_transport_get_credit(vvs, pkt_len);
> > >
> > > I think drivers/vhost/vsock.c:vhost_transport_do_send_pkt() now works
> > > correctly even with pkt_len > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE.
> >
> > Correct.
> >
> > >
> > > The other ->send_pkt() callback is
> > > net/vmw_vsock/virtio_transport.c:virtio_transport_send_pkt_work() and it
> > > can already send any size packet.
> > >
> > > Do you remember why VIRTIO_VSOCK_MAX_PKT_BUF_SIZE still needs to be the
> > > limit? I'm wondering if we can get rid of it now and just limit packets
> > > to the available credits.
> >
> > There are 2 reasons why I left this limit:
> > 1. When the host receives a packets, it must be <=
> > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE [drivers/vhost/vsock.c:vhost_vsock_alloc_pkt()]
> > So in this way we can limit the packets sent from the guest.
>
> The general intent is to prevent the guest from sending huge buffers.
> This is good.
>
> However, the guest must already obey the credit limit advertized by the
> host. Therefore I think we should be checking against that instead of
> an arbitrary constant limit.
>
> So I think the limit should be the receive buffer size, not
> VIRTIO_VSOCK_MAX_PKT_BUF_SIZE. But at this point the code doesn't know
> which connection the packet is associated with and cannot check the
> receive buffer size. :(
>
> Anyway, any change to this behavior requires compatibility so new guest
> drivers work with old vhost_vsock.ko. Therefore we should probably just
> leave the limit for now.
I understood your point of view and I completely agree with you.
But, until we don't have a way to expose features/versions between guest
and host, maybe is better to leave the limit in order to be compatible
with old vhost_vsock.
>
> > 2. When the host send packets, it help us to increase the parallelism
> > (especially if the guest has 64 KB RX buffers) because the user thread
> > will split packets, calling multiple times transport->stream_enqueue()
> > in net/vmw_vsock/af_vsock.c:vsock_stream_sendmsg() while the
> > vhost_transport_send_pkt_work() send them to the guest.
>
> Sorry, I don't understand the reasoning. Overall this creates more
> work. Are you saying the benefit is that
> vhost_transport_send_pkt_work() can run "early" and notify the guest of
> partial rx data before all of it has been enqueued?
Something like that. Your reasoning is more accurate.
Anyway, I'll do some tests in order to understand better the behaviour!
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH 1/1] virtio_blk: replace 0 by HCTX_TYPE_DEFAULT to index blk_mq_tag_set->map
From: Jens Axboe @ 2019-04-08 14:08 UTC (permalink / raw)
To: Dongli Zhang, virtualization, linux-block, linux-kernel; +Cc: mst
In-Reply-To: <1552354316-20204-1-git-send-email-dongli.zhang@oracle.com>
On 3/11/19 7:31 PM, Dongli Zhang wrote:
> Use HCTX_TYPE_DEFAULT instead of 0 to avoid hardcoding.
Applied, thanks.
--
Jens Axboe
^ 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