[PATCH v4 2/2] drm/virtio: Use vmalloc for command buffer allocations.
Userspace requested command buffer allocations could be too large
to make as a contiguous allocation. Use vmalloc if necessary to
satisfy those allocations.
Signed-off-by: David Riley
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 4 +-
drivers/gpu/drm/virtio/virtgpu_vq.c| 78 +++---
2 files changed, 72 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index f5083c538f9c..9af1ec62434f 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -143,7 +143,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device
*dev, void *data,
goto out_unused_fd;
}
- buf = memdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
+ buf = vmemdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
if (IS_ERR(buf)) {
ret = PTR_ERR(buf);
goto out_unresv;
@@ -172,7 +172,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device
*dev, void *data,
return 0;
out_memdup:
- kfree(buf);
+ kvfree(buf);
out_unresv:
if (buflist)
virtio_gpu_array_unlock_resv(buflist);
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5a64c776138d..9f9b782dd332 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -155,7 +155,7 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
{
if (vbuf->resp_size > MAX_INLINE_RESP_SIZE)
kfree(vbuf->resp_buf);
- kfree(vbuf->data_buf);
+ kvfree(vbuf->data_buf);
kmem_cache_free(vgdev->vbufs, vbuf);
}
@@ -256,13 +256,54 @@ void virtio_gpu_dequeue_cursor_func(struct work_struct
*work)
wake_up(>cursorq.ack_queue);
}
+/* Create sg_table from a vmalloc'd buffer. */
+static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size, int *sg_ents)
+{
+ int ret, s, i;
+ struct sg_table *sgt;
+ struct scatterlist *sg;
+ struct page *pg;
+
+ if (WARN_ON(!PAGE_ALIGNED(data)))
+ return NULL;
+
+ sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
+ if (!sgt)
+ return NULL;
+
+ *sg_ents = DIV_ROUND_UP(size, PAGE_SIZE);
+ ret = sg_alloc_table(sgt, *sg_ents, GFP_KERNEL);
+ if (ret) {
+ kfree(sgt);
+ return NULL;
+ }
+
+ for_each_sg(sgt->sgl, sg, *sg_ents, i) {
+ pg = vmalloc_to_page(data);
+ if (!pg) {
+ sg_free_table(sgt);
+ kfree(sgt);
+ return NULL;
+ }
+
+ s = min_t(int, PAGE_SIZE, size);
+ sg_set_page(sg, pg, s, 0);
+
+ size -= s;
+ data += s;
+ }
+
+ return sgt;
+}
+
static bool virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device
*vgdev,
- struct virtio_gpu_vbuffer *vbuf)
+ struct virtio_gpu_vbuffer *vbuf,
+ struct scatterlist *vout)
__releases(>ctrlq.qlock)
__acquires(>ctrlq.qlock)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
- struct scatterlist *sgs[3], vcmd, vout, vresp;
+ struct scatterlist *sgs[3], vcmd, vresp;
int outcnt = 0, incnt = 0;
bool notify = false;
int ret;
@@ -274,9 +315,8 @@ static bool virtio_gpu_queue_ctrl_buffer_locked(struct
virtio_gpu_device *vgdev,
sgs[outcnt + incnt] =
outcnt++;
- if (vbuf->data_size) {
- sg_init_one(, vbuf->data_buf, vbuf->data_size);
- sgs[outcnt + incnt] =
+ if (vout) {
+ sgs[outcnt + incnt] = vout;
outcnt++;
}
@@ -308,7 +348,24 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
struct virtio_gpu_fence *fence)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
+ struct scatterlist *vout = NULL, sg;
+ struct sg_table *sgt = NULL;
bool notify;
+ int outcnt = 0;
+
+ if (vbuf->data_size) {
+ if (is_vmalloc_addr(vbuf->data_buf)) {
+ sgt = vmalloc_to_sgt(vbuf->data_buf, vbuf->data_size,
+);
+ if (!sgt)
+ return -ENOMEM;
+ vout = sgt->sgl;
+ } else {
+ sg_init_one(, vbuf->data_buf, vbuf->data_size);
+ vout =
+ outcnt = 1;
+ }
+ }
again:
spin_lock(>ctrlq.qlock);
@@ -321,7 +378,7 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
* to wait
[PATCH v4 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version.
Factor function in preparation to generating scatterlist prior to locking.
Signed-off-by: David Riley
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 7fd2851f7b97..5a64c776138d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -302,18 +302,6 @@ static bool virtio_gpu_queue_ctrl_buffer_locked(struct
virtio_gpu_device *vgdev,
return notify;
}
-static void virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
-struct virtio_gpu_vbuffer *vbuf)
-{
- bool notify;
-
- spin_lock(>ctrlq.qlock);
- notify = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
- spin_unlock(>ctrlq.qlock);
- if (notify)
- virtqueue_notify(vgdev->ctrlq.vq);
-}
-
static void virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device
*vgdev,
struct virtio_gpu_vbuffer *vbuf,
struct virtio_gpu_ctrl_hdr *hdr,
@@ -339,7 +327,7 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
goto again;
}
- if (fence) {
+ if (hdr && fence) {
virtio_gpu_fence_emit(vgdev, hdr, fence);
if (vbuf->objs) {
virtio_gpu_array_add_fence(vbuf->objs, >f);
@@ -352,6 +340,12 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
virtqueue_notify(vgdev->ctrlq.vq);
}
+static void virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
+struct virtio_gpu_vbuffer *vbuf)
+{
+ virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, NULL, NULL);
+}
+
static void virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf)
{
--
2.23.0.162.g0b9fbb3734-goog
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v4 0/2] drm/virtio: Use vmalloc for command buffer alllocations.
Userspace requested command buffer allocations could be too large to make as a contiguous allocation. Use vmalloc if necessary to satisfy those allocations. v1: Initial version. v2: Properly account for number of free descriptors required. v3: Remove offset handling for vmalloc'd buffers. v4: Rebase onto drm-misc-next. David Riley (2): drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version. drm/virtio: Use vmalloc for command buffer allocations. drivers/gpu/drm/virtio/virtgpu_ioctl.c | 4 +- drivers/gpu/drm/virtio/virtgpu_vq.c| 98 -- 2 files changed, 79 insertions(+), 23 deletions(-) -- 2.23.0.162.g0b9fbb3734-goog ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version.
They were based off of Linus' https://github.com/torvalds/linux master from yesterday. I can rebase onto drm-misc-next. On Tue, Sep 10, 2019 at 10:12 PM Gerd Hoffmann wrote: > > On Tue, Sep 10, 2019 at 01:06:50PM -0700, David Riley wrote: > > Factor function in preparation to generating scatterlist prior to locking. > > Patches are looking good now, but they don't apply. What tree was used > to create them? > > Latest virtio-gpu driver bits are in drm-misc-next (see > https://cgit.freedesktop.org/drm/drm-misc). > > cheers, > Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] vhost: block speculation of translated descriptors
On Wed, Sep 11, 2019 at 09:52:25AM -0400, Michael S. Tsirkin wrote: > On Wed, Sep 11, 2019 at 08:10:00AM -0400, Michael S. Tsirkin wrote: > > iovec addresses coming from vhost are assumed to be > > pre-validated, but in fact can be speculated to a value > > out of range. > > > > Userspace address are later validated with array_index_nospec so we can > > be sure kernel info does not leak through these addresses, but vhost > > must also not leak userspace info outside the allowed memory table to > > guests. > > > > Following the defence in depth principle, make sure > > the address is not validated out of node range. > > > > Signed-off-by: Michael S. Tsirkin > > Acked-by: Jason Wang > > Tested-by: Jason Wang > > --- > > Cc: secur...@kernel.org > > Pls advise on whether you'd like me to merge this directly, > Cc stable, or handle it in some other way. I think you're fine taking it directly, with a cc stable and a Fixes: tag. Cheers, Will ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5 0/4] virtio-fs: shared file system for virtual machines
On Tue, Sep 10, 2019 at 05:12:02PM +0200, Miklos Szeredi wrote: > I've folded the series from Vivek and fixed a couple of TODO comments > myself. AFAICS two issues remain that need to be resolved in the short > term, one way or the other: freeze/restore and full virtqueue. I have researched freeze/restore and come to the conclusion that it needs to be a future feature. It will probably come together with live migration support for reasons mentioned below. Most virtio devices have fairly simply power management freeze/restore functions that shut down the device and bring it back to the state held in memory, respectively. virtio-fs, as well as virtio-9p and virtio-gpu, are different because they contain session state. It is not easily possible to bring back the state held in memory after the device has been reset. The following areas of the FUSE protocol are stateful and need special attention: * FUSE_INIT - this is pretty easy, we must re-negotiate the same settings as before. * FUSE_LOOKUP -> fuse_inode (inode_map) The session contains a set of inode numbers that have been looked up using FUSE_LOOKUP. They are ephemeral in the current virtiofsd implementation and vary across device reset. Therefore we are unable to restore the same inode numbers upon restore. The solution is persistent inode numbers in virtiofsd. This is also needed to make open_by_handle_at(2) work and probably for live migration. * FUSE_OPEN -> fh (fd_map) The session contains FUSE file handles for open files. There is currently no way of re-opening a file so that a specific fh is returned. A mechanism to do so probably isn't necessary if the driver can update the fh to the new one produced by the device for all open files instead. * FUSE_OPENDIR -> fh (dirp_map) Same story as for FUSE_OPEN but for open directories. * FUSE_GETLK/SETLK/SETLKW -> (inode->posix_locks and fcntl(F_OFD_GET/SETLK)) The session contains file locks. The driver must reacquire them upon restore. It's unclear what to do when locking fails. Live migration has the same problem since the FUSE session will be moved to a new virtio-fs device instance. It makes sense to tackle both features together. This is something that can be implemented in the next year, but it's not a quick fix. Stefan signature.asc Description: PGP signature ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] drm/qxl: get vga ioports
>
> qxl has two modes: "native" (used by the drm driver) and "vga" (vga
> compatibility mode, typically used for boot display and firmware
> framebuffers).
>
> Accessing any vga ioport will switch the qxl device into vga mode.
> The qxl driver never does that, but other drivers accessing vga ports
> can trigger that too and therefore disturb qxl operation. So aquire
> the legacy vga ioports from vgaarb to avoid that.
>
> Reproducer: Boot kvm guest with both qxl and i915 vgpu, with qxl being
> first in pci scan order.
>
> v2: Skip this for secondary qxl cards which don't have vga mode in the
> first place (Frediano).
>
> Cc: Frediano Ziglio
> Signed-off-by: Gerd Hoffmann
Acked-by: Frediano Ziglio
> ---
> drivers/gpu/drm/qxl/qxl_drv.c | 20 +++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index b57a37543613..fcb48ac60598 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -63,6 +63,11 @@ module_param_named(num_heads, qxl_num_crtc, int, 0400);
> static struct drm_driver qxl_driver;
> static struct pci_driver qxl_pci_driver;
>
> +static bool is_vga(struct pci_dev *pdev)
> +{
> + return pdev->class == PCI_CLASS_DISPLAY_VGA << 8;
> +}
> +
> static int
> qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> {
> @@ -87,9 +92,17 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> if (ret)
> goto disable_pci;
>
> + if (is_vga(pdev)) {
> + ret = vga_get_interruptible(pdev, VGA_RSRC_LEGACY_IO);
> + if (ret) {
> + DRM_ERROR("can't get legacy vga ioports\n");
> + goto disable_pci;
> + }
> + }
> +
> ret = qxl_device_init(qdev, _driver, pdev);
> if (ret)
> - goto disable_pci;
> + goto put_vga;
>
> ret = qxl_modeset_init(qdev);
> if (ret)
> @@ -109,6 +122,9 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> qxl_modeset_fini(qdev);
> unload:
> qxl_device_fini(qdev);
> +put_vga:
> + if (is_vga(pdev))
> + vga_put(pdev, VGA_RSRC_LEGACY_IO);
> disable_pci:
> pci_disable_device(pdev);
> free_dev:
> @@ -126,6 +142,8 @@ qxl_pci_remove(struct pci_dev *pdev)
>
> qxl_modeset_fini(qdev);
> qxl_device_fini(qdev);
> + if (is_vga(pdev))
> + vga_put(pdev, VGA_RSRC_LEGACY_IO);
>
> dev->dev_private = NULL;
> kfree(qdev);
Frediano
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5 0/4] virtio-fs: shared file system for virtual machines
On Tue, Sep 10, 2019 at 05:12:02PM +0200, Miklos Szeredi wrote: > Git tree for this version is available here: > > git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v5 > > Only post patches that actually add virtiofs (virtiofs-v5-base..virtiofs-v5). > > I've folded the series from Vivek and fixed a couple of TODO comments > myself. AFAICS two issues remain that need to be resolved in the short > term, one way or the other: freeze/restore and full virtqueue. Hi Miklos, We are already handling full virtqueue by waiting a bit and retrying. I think TODO in virtio_fs_enqueue_req() is stale. Caller already handles virtqueue full situation by retrying. Havind said that, this could be improved by using some sort of wait queue or completion privimitve. Thanks Vivek ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] vhost: block speculation of translated descriptors
On Wed, Sep 11, 2019 at 08:10:00AM -0400, Michael S. Tsirkin wrote: > iovec addresses coming from vhost are assumed to be > pre-validated, but in fact can be speculated to a value > out of range. > > Userspace address are later validated with array_index_nospec so we can > be sure kernel info does not leak through these addresses, but vhost > must also not leak userspace info outside the allowed memory table to > guests. > > Following the defence in depth principle, make sure > the address is not validated out of node range. > > Signed-off-by: Michael S. Tsirkin > Acked-by: Jason Wang > Tested-by: Jason Wang > --- Cc: secur...@kernel.org Pls advise on whether you'd like me to merge this directly, Cc stable, or handle it in some other way. > changes from v1: fix build on 32 bit > > drivers/vhost/vhost.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 5dc174ac8cac..34ea219936e3 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -2071,8 +2071,10 @@ static int translate_desc(struct vhost_virtqueue *vq, > u64 addr, u32 len, > _iov = iov + ret; > size = node->size - addr + node->start; > _iov->iov_len = min((u64)len - s, size); > - _iov->iov_base = (void __user *)(unsigned long) > - (node->userspace_addr + addr - node->start); > + _iov->iov_base = (void __user *) > + ((unsigned long)node->userspace_addr + > + array_index_nospec((unsigned long)(addr - node->start), > + node->size)); > s += size; > addr += size; > ++ret; > -- > MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] vhost: block speculation of translated descriptors
On Wed, Sep 11, 2019 at 03:12:35PM +0200, Michal Hocko wrote: > On Wed 11-09-19 09:03:10, Michael S. Tsirkin wrote: > > On Wed, Sep 11, 2019 at 02:33:16PM +0200, Michal Hocko wrote: > > > On Wed 11-09-19 08:25:03, Michael S. Tsirkin wrote: > > > > On Wed, Sep 11, 2019 at 02:16:28PM +0200, Michal Hocko wrote: > > > > > On Wed 11-09-19 08:10:00, Michael S. Tsirkin wrote: > > > > > > iovec addresses coming from vhost are assumed to be > > > > > > pre-validated, but in fact can be speculated to a value > > > > > > out of range. > > > > > > > > > > > > Userspace address are later validated with array_index_nospec so we > > > > > > can > > > > > > be sure kernel info does not leak through these addresses, but vhost > > > > > > must also not leak userspace info outside the allowed memory table > > > > > > to > > > > > > guests. > > > > > > > > > > > > Following the defence in depth principle, make sure > > > > > > the address is not validated out of node range. > > > > > > > > > > > > Signed-off-by: Michael S. Tsirkin > > > > > > Acked-by: Jason Wang > > > > > > Tested-by: Jason Wang > > > > > > > > > > no need to mark fo stable? Other spectre fixes tend to be backported > > > > > even when the security implications are not really clear. The risk > > > > > should be low and better to be covered in case. > > > > > > > > This is not really a fix - more a defence in depth thing, > > > > quite similar to e.g. commit b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd > > > > x86: Introduce __uaccess_begin_nospec() and uaccess_try_nospec > > > > in scope. > > > > > > > > That one doesn't seem to be tagged for stable. Was it queued > > > > there in practice? > > > > > > not marked for stable but it went in. At least to 4.4. > > > > So I guess the answer is I don't know. If you feel it's > > justified, then sure, feel free to forward. > > Well, that obviously depends on you as a maintainer but the point is > that spectre gatgets are quite hard to find. There is a smack check > AFAIK but that generates quite some false possitives and it is PITA to > crawl through those. If you want an interesting (I am not saying > vulnerable on purpose) gatget then it would be great to mark it for > stable so all stable consumers (disclaimer: I am not one of those) and > add that really great feeling of safety ;) > > So take this as my 2c OK it seems secur...@kernel.org is the way to handle these things. I'll try that. > -- > Michal Hocko > SUSE Labs ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] vhost: block speculation of translated descriptors
On Wed 11-09-19 09:03:10, Michael S. Tsirkin wrote: > On Wed, Sep 11, 2019 at 02:33:16PM +0200, Michal Hocko wrote: > > On Wed 11-09-19 08:25:03, Michael S. Tsirkin wrote: > > > On Wed, Sep 11, 2019 at 02:16:28PM +0200, Michal Hocko wrote: > > > > On Wed 11-09-19 08:10:00, Michael S. Tsirkin wrote: > > > > > iovec addresses coming from vhost are assumed to be > > > > > pre-validated, but in fact can be speculated to a value > > > > > out of range. > > > > > > > > > > Userspace address are later validated with array_index_nospec so we > > > > > can > > > > > be sure kernel info does not leak through these addresses, but vhost > > > > > must also not leak userspace info outside the allowed memory table to > > > > > guests. > > > > > > > > > > Following the defence in depth principle, make sure > > > > > the address is not validated out of node range. > > > > > > > > > > Signed-off-by: Michael S. Tsirkin > > > > > Acked-by: Jason Wang > > > > > Tested-by: Jason Wang > > > > > > > > no need to mark fo stable? Other spectre fixes tend to be backported > > > > even when the security implications are not really clear. The risk > > > > should be low and better to be covered in case. > > > > > > This is not really a fix - more a defence in depth thing, > > > quite similar to e.g. commit b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd > > > x86: Introduce __uaccess_begin_nospec() and uaccess_try_nospec > > > in scope. > > > > > > That one doesn't seem to be tagged for stable. Was it queued > > > there in practice? > > > > not marked for stable but it went in. At least to 4.4. > > So I guess the answer is I don't know. If you feel it's > justified, then sure, feel free to forward. Well, that obviously depends on you as a maintainer but the point is that spectre gatgets are quite hard to find. There is a smack check AFAIK but that generates quite some false possitives and it is PITA to crawl through those. If you want an interesting (I am not saying vulnerable on purpose) gatget then it would be great to mark it for stable so all stable consumers (disclaimer: I am not one of those) and add that really great feeling of safety ;) So take this as my 2c -- Michal Hocko SUSE Labs ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] vhost: block speculation of translated descriptors
On Wed, Sep 11, 2019 at 02:33:16PM +0200, Michal Hocko wrote: > On Wed 11-09-19 08:25:03, Michael S. Tsirkin wrote: > > On Wed, Sep 11, 2019 at 02:16:28PM +0200, Michal Hocko wrote: > > > On Wed 11-09-19 08:10:00, Michael S. Tsirkin wrote: > > > > iovec addresses coming from vhost are assumed to be > > > > pre-validated, but in fact can be speculated to a value > > > > out of range. > > > > > > > > Userspace address are later validated with array_index_nospec so we can > > > > be sure kernel info does not leak through these addresses, but vhost > > > > must also not leak userspace info outside the allowed memory table to > > > > guests. > > > > > > > > Following the defence in depth principle, make sure > > > > the address is not validated out of node range. > > > > > > > > Signed-off-by: Michael S. Tsirkin > > > > Acked-by: Jason Wang > > > > Tested-by: Jason Wang > > > > > > no need to mark fo stable? Other spectre fixes tend to be backported > > > even when the security implications are not really clear. The risk > > > should be low and better to be covered in case. > > > > This is not really a fix - more a defence in depth thing, > > quite similar to e.g. commit b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd > > x86: Introduce __uaccess_begin_nospec() and uaccess_try_nospec > > in scope. > > > > That one doesn't seem to be tagged for stable. Was it queued > > there in practice? > > not marked for stable but it went in. At least to 4.4. So I guess the answer is I don't know. If you feel it's justified, then sure, feel free to forward. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] vhost: block speculation of translated descriptors
On Wed 11-09-19 08:25:03, Michael S. Tsirkin wrote: > On Wed, Sep 11, 2019 at 02:16:28PM +0200, Michal Hocko wrote: > > On Wed 11-09-19 08:10:00, Michael S. Tsirkin wrote: > > > iovec addresses coming from vhost are assumed to be > > > pre-validated, but in fact can be speculated to a value > > > out of range. > > > > > > Userspace address are later validated with array_index_nospec so we can > > > be sure kernel info does not leak through these addresses, but vhost > > > must also not leak userspace info outside the allowed memory table to > > > guests. > > > > > > Following the defence in depth principle, make sure > > > the address is not validated out of node range. > > > > > > Signed-off-by: Michael S. Tsirkin > > > Acked-by: Jason Wang > > > Tested-by: Jason Wang > > > > no need to mark fo stable? Other spectre fixes tend to be backported > > even when the security implications are not really clear. The risk > > should be low and better to be covered in case. > > This is not really a fix - more a defence in depth thing, > quite similar to e.g. commit b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd > x86: Introduce __uaccess_begin_nospec() and uaccess_try_nospec > in scope. > > That one doesn't seem to be tagged for stable. Was it queued > there in practice? not marked for stable but it went in. At least to 4.4. -- Michal Hocko SUSE Labs ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] vhost: block speculation of translated descriptors
On Wed, Sep 11, 2019 at 02:16:28PM +0200, Michal Hocko wrote: > On Wed 11-09-19 08:10:00, Michael S. Tsirkin wrote: > > iovec addresses coming from vhost are assumed to be > > pre-validated, but in fact can be speculated to a value > > out of range. > > > > Userspace address are later validated with array_index_nospec so we can > > be sure kernel info does not leak through these addresses, but vhost > > must also not leak userspace info outside the allowed memory table to > > guests. > > > > Following the defence in depth principle, make sure > > the address is not validated out of node range. > > > > Signed-off-by: Michael S. Tsirkin > > Acked-by: Jason Wang > > Tested-by: Jason Wang > > no need to mark fo stable? Other spectre fixes tend to be backported > even when the security implications are not really clear. The risk > should be low and better to be covered in case. This is not really a fix - more a defence in depth thing, quite similar to e.g. commit b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd x86: Introduce __uaccess_begin_nospec() and uaccess_try_nospec in scope. That one doesn't seem to be tagged for stable. Was it queued there in practice? > > --- > > > > changes from v1: fix build on 32 bit > > > > drivers/vhost/vhost.c | 6 -- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > index 5dc174ac8cac..34ea219936e3 100644 > > --- a/drivers/vhost/vhost.c > > +++ b/drivers/vhost/vhost.c > > @@ -2071,8 +2071,10 @@ static int translate_desc(struct vhost_virtqueue > > *vq, u64 addr, u32 len, > > _iov = iov + ret; > > size = node->size - addr + node->start; > > _iov->iov_len = min((u64)len - s, size); > > - _iov->iov_base = (void __user *)(unsigned long) > > - (node->userspace_addr + addr - node->start); > > + _iov->iov_base = (void __user *) > > + ((unsigned long)node->userspace_addr + > > +array_index_nospec((unsigned long)(addr - node->start), > > + node->size)); > > s += size; > > addr += size; > > ++ret; > > -- > > MST > > -- > Michal Hocko > SUSE Labs ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5 0/4] virtio-fs: shared file system for virtual machines
On Tue, Sep 10, 2019 at 05:12:02PM +0200, Miklos Szeredi wrote: > Git tree for this version is available here: > > git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v5 > > Only post patches that actually add virtiofs (virtiofs-v5-base..virtiofs-v5). > > I've folded the series from Vivek and fixed a couple of TODO comments > myself. AFAICS two issues remain that need to be resolved in the short > term, one way or the other: freeze/restore and full virtqueue. Thank you! I am investigating freeze/restore. Stefan signature.asc Description: PGP signature ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] vhost: block speculation of translated descriptors
On Wed 11-09-19 08:10:00, Michael S. Tsirkin wrote: > iovec addresses coming from vhost are assumed to be > pre-validated, but in fact can be speculated to a value > out of range. > > Userspace address are later validated with array_index_nospec so we can > be sure kernel info does not leak through these addresses, but vhost > must also not leak userspace info outside the allowed memory table to > guests. > > Following the defence in depth principle, make sure > the address is not validated out of node range. > > Signed-off-by: Michael S. Tsirkin > Acked-by: Jason Wang > Tested-by: Jason Wang no need to mark fo stable? Other spectre fixes tend to be backported even when the security implications are not really clear. The risk should be low and better to be covered in case. > --- > > changes from v1: fix build on 32 bit > > drivers/vhost/vhost.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 5dc174ac8cac..34ea219936e3 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -2071,8 +2071,10 @@ static int translate_desc(struct vhost_virtqueue *vq, > u64 addr, u32 len, > _iov = iov + ret; > size = node->size - addr + node->start; > _iov->iov_len = min((u64)len - s, size); > - _iov->iov_base = (void __user *)(unsigned long) > - (node->userspace_addr + addr - node->start); > + _iov->iov_base = (void __user *) > + ((unsigned long)node->userspace_addr + > + array_index_nospec((unsigned long)(addr - node->start), > + node->size)); > s += size; > addr += size; > ++ret; > -- > MST -- Michal Hocko SUSE Labs ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2] vhost: block speculation of translated descriptors
iovec addresses coming from vhost are assumed to be pre-validated, but in fact can be speculated to a value out of range. Userspace address are later validated with array_index_nospec so we can be sure kernel info does not leak through these addresses, but vhost must also not leak userspace info outside the allowed memory table to guests. Following the defence in depth principle, make sure the address is not validated out of node range. Signed-off-by: Michael S. Tsirkin Acked-by: Jason Wang Tested-by: Jason Wang --- changes from v1: fix build on 32 bit drivers/vhost/vhost.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 5dc174ac8cac..34ea219936e3 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -2071,8 +2071,10 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, _iov = iov + ret; size = node->size - addr + node->start; _iov->iov_len = min((u64)len - s, size); - _iov->iov_base = (void __user *)(unsigned long) - (node->userspace_addr + addr - node->start); + _iov->iov_base = (void __user *) + ((unsigned long)node->userspace_addr + +array_index_nospec((unsigned long)(addr - node->start), + node->size)); s += size; addr += size; ++ret; -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[vhost:linux-next 16/17] include/linux/page_reporting.h:9:34: note: in expansion of macro 'pageblock_order'
tree: https://kernel.googlesource.com/pub/scm/linux/kernel/git/mst/vhost.git linux-next head: 39c226b6b576b23d6d558331e6895f02b0892555 commit: 990055c63121520ad29deca72b1167b392563ddd [16/17] virtio-balloon: Add support for providing unused page reports to host config: riscv-allmodconfig (attached as .config) compiler: riscv64-linux-gcc (GCC) 7.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 990055c63121520ad29deca72b1167b392563ddd # save the attached .config to linux build tree GCC_VERSION=7.4.0 make.cross ARCH=riscv If you fix the issue, kindly add following tag Reported-by: kbuild test robot All warnings (new ones prefixed by >>): In file included from arch/riscv/include/asm/thread_info.h:11:0, from include/linux/thread_info.h:38, from include/asm-generic/preempt.h:5, from ./arch/riscv/include/generated/asm/preempt.h:1, from include/linux/preempt.h:78, from include/linux/radix-tree.h:14, from include/linux/idr.h:15, from include/linux/kernfs.h:13, from include/linux/sysfs.h:16, from include/linux/kobject.h:20, from include/linux/device.h:16, from drivers/scsi/snic/snic_attrs.c:19: include/linux/page_reporting.h: In function '__del_page_from_reported_list': arch/riscv/include/asm/page.h:24:22: error: 'PMD_SHIFT' undeclared (first use in this function); did you mean 'NMI_SHIFT'? #define HPAGE_SHIFT PMD_SHIFT ^ arch/riscv/include/asm/page.h:27:34: note: in expansion of macro 'HPAGE_SHIFT' #define HUGETLB_PAGE_ORDER (HPAGE_SHIFT - PAGE_SHIFT) ^~~ include/linux/pageblock-flags.h:41:26: note: in expansion of macro 'HUGETLB_PAGE_ORDER' #define pageblock_order HUGETLB_PAGE_ORDER ^~ >> include/linux/page_reporting.h:9:34: note: in expansion of macro >> 'pageblock_order' #define PAGE_REPORTING_MIN_ORDER pageblock_order ^~~ include/linux/page_reporting.h:61:44: note: in expansion of macro 'PAGE_REPORTING_MIN_ORDER' zone->reported_pages[page_private(page) - PAGE_REPORTING_MIN_ORDER]--; ^~~~ arch/riscv/include/asm/page.h:24:22: note: each undeclared identifier is reported only once for each function it appears in #define HPAGE_SHIFT PMD_SHIFT ^ arch/riscv/include/asm/page.h:27:34: note: in expansion of macro 'HPAGE_SHIFT' #define HUGETLB_PAGE_ORDER (HPAGE_SHIFT - PAGE_SHIFT) ^~~ include/linux/pageblock-flags.h:41:26: note: in expansion of macro 'HUGETLB_PAGE_ORDER' #define pageblock_order HUGETLB_PAGE_ORDER ^~ >> include/linux/page_reporting.h:9:34: note: in expansion of macro >> 'pageblock_order' #define PAGE_REPORTING_MIN_ORDER pageblock_order ^~~ include/linux/page_reporting.h:61:44: note: in expansion of macro 'PAGE_REPORTING_MIN_ORDER' zone->reported_pages[page_private(page) - PAGE_REPORTING_MIN_ORDER]--; ^~~~ include/linux/page_reporting.h: In function 'get_unreported_tail': arch/riscv/include/asm/page.h:24:22: error: 'PMD_SHIFT' undeclared (first use in this function); did you mean 'NMI_SHIFT'? #define HPAGE_SHIFT PMD_SHIFT ^ arch/riscv/include/asm/page.h:27:34: note: in expansion of macro 'HPAGE_SHIFT' #define HUGETLB_PAGE_ORDER (HPAGE_SHIFT - PAGE_SHIFT) ^~~ include/linux/pageblock-flags.h:41:26: note: in expansion of macro 'HUGETLB_PAGE_ORDER' #define pageblock_order HUGETLB_PAGE_ORDER ^~ >> include/linux/page_reporting.h:9:34: note: in expansion of macro >> 'pageblock_order' #define PAGE_REPORTING_MIN_ORDER pageblock_order ^~~ include/linux/page_reporting.h:77:15: note: in expansion of macro 'PAGE_REPORTING_MIN_ORDER' if (order >= PAGE_REPORTING_MIN_ORDER && ^~~~ include/linux/page_reporting.h: In function 'add_page_to_reported_list': arch/riscv/include/asm/page.h:24:22: error: 'PMD_SHIFT' undeclared (first use in this function); did you mean 'NMI_SHIFT'? #define HPAGE_SHIFT PMD_SHIFT ^ arch/riscv/include/asm/page.h:27:34: note: in expansion of macro 'HPAGE_SHIFT' #define HUGETLB_PAGE_ORDER
[vhost:linux-next 8/9] drivers/vhost/vhost.c:2076:5: note: in expansion of macro 'array_index_nospec'
tree: https://kernel.googlesource.com/pub/scm/linux/kernel/git/mst/vhost.git linux-next head: f2c4b499aecc0c5a1ec67f3a2a7310cb7168a8ab commit: 4c145987a955269da79312a79ec26638712644bb [8/9] vhost: block speculation of translated descriptors config: mips-malta_kvm_defconfig (attached as .config) compiler: mipsel-linux-gcc (GCC) 7.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 4c145987a955269da79312a79ec26638712644bb # save the attached .config to linux build tree GCC_VERSION=7.4.0 make.cross ARCH=mips If you fix the issue, kindly add following tag Reported-by: kbuild test robot All error/warnings (new ones prefixed by >>): In file included from include/linux/kernel.h:11:0, from include/linux/list.h:9, from include/linux/wait.h:7, from include/linux/eventfd.h:13, from drivers/vhost/vhost.c:13: drivers/vhost/vhost.c: In function 'translate_desc': >> include/linux/compiler.h:350:38: error: call to '__compiletime_assert_2077' >> declared with attribute error: BUILD_BUG_ON failed: sizeof(_s) > sizeof(long) _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/compiler.h:331:4: note: in definition of macro '__compiletime_assert' prefix ## suffix();\ ^~ include/linux/compiler.h:350:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~ include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~ include/linux/build_bug.h:50:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^~~~ >> include/linux/nospec.h:55:2: note: in expansion of macro 'BUILD_BUG_ON' BUILD_BUG_ON(sizeof(_i) > sizeof(long)); \ ^~~~ >> drivers/vhost/vhost.c:2076:5: note: in expansion of macro >> 'array_index_nospec' array_index_nospec(addr - node->start, ^~ >> include/linux/compiler.h:350:38: error: call to '__compiletime_assert_2077' >> declared with attribute error: BUILD_BUG_ON failed: sizeof(_s) > sizeof(long) _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/compiler.h:331:4: note: in definition of macro '__compiletime_assert' prefix ## suffix();\ ^~ include/linux/compiler.h:350:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~ include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~ include/linux/build_bug.h:50:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^~~~ include/linux/nospec.h:56:2: note: in expansion of macro 'BUILD_BUG_ON' BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ ^~~~ >> drivers/vhost/vhost.c:2076:5: note: in expansion of macro >> 'array_index_nospec' array_index_nospec(addr - node->start, ^~ -- In file included from include/linux/kernel.h:11:0, from include/linux/list.h:9, from include/linux/wait.h:7, from include/linux/eventfd.h:13, from drivers//vhost/vhost.c:13: drivers//vhost/vhost.c: In function 'translate_desc': >> include/linux/compiler.h:350:38: error: call to '__compiletime_assert_2077' >> declared with attribute error: BUILD_BUG_ON failed: sizeof(_s) > sizeof(long) _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^ include/linux/compiler.h:331:4: note: in definition of macro '__compiletime_assert' prefix ## suffix();\ ^~ include/linux/compiler.h:350:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~ include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~ include/linux/build_bug.h:50:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) ^~~~ >>
[PATCH v2 1/4] drm/vram: Move VRAM memory manager to GEM VRAM implementation
The separation between GEM VRAM objects and the memory manager is
artificial, as they are only used with each other. Copying both
implementations into the same file is a first step to simplifying
the code.
This patch only moves code without functional changes.
v2:
* update for debugfs support
* typos in commit message
Signed-off-by: Thomas Zimmermann
Acked-by: Gerd Hoffmann
---
Documentation/gpu/drm-mm.rst | 12 -
drivers/gpu/drm/Makefile | 3 +-
drivers/gpu/drm/ast/ast_drv.c | 1 -
drivers/gpu/drm/ast/ast_main.c| 1 -
drivers/gpu/drm/ast/ast_ttm.c | 1 -
drivers/gpu/drm/bochs/bochs.h | 1 -
drivers/gpu/drm/drm_gem_vram_helper.c | 348 -
drivers/gpu/drm/drm_vram_mm_helper.c | 353 --
.../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 1 -
drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 1 -
drivers/gpu/drm/mgag200/mgag200_drv.h | 1 -
drivers/gpu/drm/vboxvideo/vbox_drv.h | 2 -
include/drm/drm_gem_vram_helper.h | 86 +
include/drm/drm_vram_mm_helper.h | 77
14 files changed, 434 insertions(+), 454 deletions(-)
delete mode 100644 drivers/gpu/drm/drm_vram_mm_helper.c
diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst
index a70a1d9f30ec..99d56015e077 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -400,18 +400,6 @@ GEM VRAM Helper Functions Reference
.. kernel-doc:: drivers/gpu/drm/drm_gem_vram_helper.c
:export:
-VRAM MM Helper Functions Reference
---
-
-.. kernel-doc:: drivers/gpu/drm/drm_vram_mm_helper.c
- :doc: overview
-
-.. kernel-doc:: include/drm/drm_vram_mm_helper.h
- :internal:
-
-.. kernel-doc:: drivers/gpu/drm/drm_vram_mm_helper.c
- :export:
-
GEM TTM Helper Functions Reference
---
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index b906bab29740..9f1c7c486f88 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -33,8 +33,7 @@ drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
drm_vram_helper-y := drm_gem_vram_helper.o \
-drm_vram_helper_common.o \
-drm_vram_mm_helper.o
+drm_vram_helper_common.o
obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o
drm_ttm_helper-y := drm_gem_ttm_helper.o
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 6ed6ff49efc0..e0e8770462bc 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -35,7 +35,6 @@
#include
#include
#include
-#include
#include "ast_drv.h"
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 50de8e47659c..21715d6a9b56 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -33,7 +33,6 @@
#include
#include
#include
-#include
#include "ast_drv.h"
diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
index c52d92294171..08ba0a917593 100644
--- a/drivers/gpu/drm/ast/ast_ttm.c
+++ b/drivers/gpu/drm/ast/ast_ttm.c
@@ -30,7 +30,6 @@
#include
#include
-#include
#include "ast_drv.h"
diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
index 68483a2fc12c..917767173ee6 100644
--- a/drivers/gpu/drm/bochs/bochs.h
+++ b/drivers/gpu/drm/bochs/bochs.h
@@ -10,7 +10,6 @@
#include
#include
#include
-#include
/* -- */
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index becf1013e02b..353f98075579 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -1,8 +1,10 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#include
+#include
+#include
#include
#include
-#include
#include
#include
#include
@@ -15,6 +17,11 @@ static const struct drm_gem_object_funcs
drm_gem_vram_object_funcs;
*
* This library provides a GEM buffer object that is backed by video RAM
* (VRAM). It can be used for framebuffer devices with dedicated memory.
+ *
+ * The data structure drm_vram_mm and its helpers implement a memory
+ * manager for simple framebuffer devices with dedicated video memory. Buffer
+ * objects are either placed in video RAM or evicted to system memory. The rsp.
+ * buffer object is provided by drm_gem_vram_object.
*/
/*
@@ -736,3 +743,342 @@ static const struct drm_gem_object_funcs
drm_gem_vram_object_funcs = {
.vunmap = drm_gem_vram_object_vunmap,
.print_info = drm_gem_ttm_print_info,
};
+
+/*
+ * VRAM memory manager
+ */
+
+/*
+ * TTM TT
+ */
+
+static void backend_func_destroy(struct ttm_tt *tt)
+{
+ ttm_tt_fini(tt);
+
[PATCH v2 4/4] drm/vram: Unconditonally set BO call-back functions
The statement's condition is always true. Signed-off-by: Thomas Zimmermann Acked-by: Gerd Hoffmann --- drivers/gpu/drm/drm_gem_vram_helper.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 1a05e2a97b93..ab9f8523d887 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -93,8 +93,7 @@ static int drm_gem_vram_init(struct drm_device *dev, int ret; size_t acc_size; - if (!gbo->bo.base.funcs) - gbo->bo.base.funcs = _gem_vram_object_funcs; + gbo->bo.base.funcs = _gem_vram_object_funcs; ret = drm_gem_object_init(dev, >bo.base, size); if (ret) -- 2.23.0 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 2/4] drm/vram: Have VRAM MM call GEM VRAM functions directly
VRAM MM and GEM VRAM buffer objects are only used with each other;
connected via 3 function pointers. Simplify this code by making the
memory manager call the rsp. functions of the BOs directly; and
remove the functions from the BO's public interface.
v2:
* typos in commit message
Signed-off-by: Thomas Zimmermann
Acked-by: Gerd Hoffmann
---
drivers/gpu/drm/ast/ast_ttm.c | 2 +-
drivers/gpu/drm/bochs/bochs_mm.c| 3 +-
drivers/gpu/drm/drm_gem_vram_helper.c | 119 ++--
drivers/gpu/drm/drm_vram_helper_common.c| 8 +-
drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 2 +-
drivers/gpu/drm/mgag200/mgag200_ttm.c | 3 +-
drivers/gpu/drm/vboxvideo/vbox_ttm.c| 3 +-
include/drm/drm_gem_vram_helper.h | 24 +---
include/drm/drm_vram_mm_helper.h| 32 --
9 files changed, 44 insertions(+), 152 deletions(-)
delete mode 100644 include/drm/drm_vram_mm_helper.h
diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
index 08ba0a917593..fad34106083a 100644
--- a/drivers/gpu/drm/ast/ast_ttm.c
+++ b/drivers/gpu/drm/ast/ast_ttm.c
@@ -41,7 +41,7 @@ int ast_mm_init(struct ast_private *ast)
vmm = drm_vram_helper_alloc_mm(
dev, pci_resource_start(dev->pdev, 0),
- ast->vram_size, _gem_vram_mm_funcs);
+ ast->vram_size);
if (IS_ERR(vmm)) {
ret = PTR_ERR(vmm);
DRM_ERROR("Error initializing VRAM MM; %d\n", ret);
diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
index 8f9bb886f7ad..1b74f530b07c 100644
--- a/drivers/gpu/drm/bochs/bochs_mm.c
+++ b/drivers/gpu/drm/bochs/bochs_mm.c
@@ -11,8 +11,7 @@ int bochs_mm_init(struct bochs_device *bochs)
struct drm_vram_mm *vmm;
vmm = drm_vram_helper_alloc_mm(bochs->dev, bochs->fb_base,
- bochs->fb_size,
- _gem_vram_mm_funcs);
+ bochs->fb_size);
return PTR_ERR_OR_ZERO(vmm);
}
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 353f98075579..c87fed608ffa 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -7,7 +7,6 @@
#include
#include
#include
-#include
#include
static const struct drm_gem_object_funcs drm_gem_vram_object_funcs;
@@ -464,68 +463,25 @@ static bool drm_is_gem_vram(struct ttm_buffer_object *bo)
return (bo->destroy == ttm_buffer_object_destroy);
}
-/**
- * drm_gem_vram_bo_driver_evict_flags() - \
- Implements ttm_bo_driver.evict_flags
- * @bo:TTM buffer object. Refers to drm_gem_vram_object.bo
- * @pl:TTM placement information.
- */
-void drm_gem_vram_bo_driver_evict_flags(struct ttm_buffer_object *bo,
- struct ttm_placement *pl)
+static void drm_gem_vram_bo_driver_evict_flags(struct drm_gem_vram_object *gbo,
+ struct ttm_placement *pl)
{
- struct drm_gem_vram_object *gbo;
-
- /* TTM may pass BOs that are not GEM VRAM BOs. */
- if (!drm_is_gem_vram(bo))
- return;
-
- gbo = drm_gem_vram_of_bo(bo);
drm_gem_vram_placement(gbo, TTM_PL_FLAG_SYSTEM);
*pl = gbo->placement;
}
-EXPORT_SYMBOL(drm_gem_vram_bo_driver_evict_flags);
-/**
- * drm_gem_vram_bo_driver_verify_access() - \
- Implements ttm_bo_driver.verify_access
- * @bo:TTM buffer object. Refers to
drm_gem_vram_object.bo
- * @filp: File pointer.
- *
- * Returns:
- * 0 on success, or
- * a negative errno code otherwise.
- */
-int drm_gem_vram_bo_driver_verify_access(struct ttm_buffer_object *bo,
-struct file *filp)
+static int drm_gem_vram_bo_driver_verify_access(struct drm_gem_vram_object
*gbo,
+ struct file *filp)
{
- struct drm_gem_vram_object *gbo = drm_gem_vram_of_bo(bo);
-
return drm_vma_node_verify_access(>bo.base.vma_node,
filp->private_data);
}
-EXPORT_SYMBOL(drm_gem_vram_bo_driver_verify_access);
-/**
- * drm_gem_vram_bo_driver_move_notify() -
- * Implements ttm_bo_driver.move_notify
- * @bo:TTM buffer object. Refers to
drm_gem_vram_object.bo
- * @evict: True, if the BO is being evicted from graphics memory;
- * false otherwise.
- * @new_mem: New memory region, or NULL on destruction
- */
-void drm_gem_vram_bo_driver_move_notify(struct ttm_buffer_object *bo,
- bool evict,
- struct ttm_mem_reg *new_mem)
+static void drm_gem_vram_bo_driver_move_notify(struct drm_gem_vram_object *gbo,
+ bool evict,
+
[PATCH v2 3/4] drm/vram: Unexport internal functions of VRAM MM
The init, cleanup and mmap functions of VRAM MM are only used internally.
Remove them from the public interface.
v2:
* update for debugfs support
Signed-off-by: Thomas Zimmermann
Acked-by: Gerd Hoffmann
---
drivers/gpu/drm/drm_gem_vram_helper.c | 38 ---
include/drm/drm_gem_vram_helper.h | 6 -
2 files changed, 5 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index c87fed608ffa..1a05e2a97b93 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -889,19 +889,8 @@ int drm_vram_mm_debugfs_init(struct drm_minor *minor)
}
EXPORT_SYMBOL(drm_vram_mm_debugfs_init);
-/**
- * drm_vram_mm_init() - Initialize an instance of VRAM MM.
- * @vmm: the VRAM MM instance to initialize
- * @dev: the DRM device
- * @vram_base: the base address of the video memory
- * @vram_size: the size of the video memory in bytes
- *
- * Returns:
- * 0 on success, or
- * a negative error code otherwise.
- */
-int drm_vram_mm_init(struct drm_vram_mm *vmm, struct drm_device *dev,
-uint64_t vram_base, size_t vram_size)
+static int drm_vram_mm_init(struct drm_vram_mm *vmm, struct drm_device *dev,
+ uint64_t vram_base, size_t vram_size)
{
int ret;
@@ -920,34 +909,17 @@ int drm_vram_mm_init(struct drm_vram_mm *vmm, struct
drm_device *dev,
return 0;
}
-EXPORT_SYMBOL(drm_vram_mm_init);
-/**
- * drm_vram_mm_cleanup() - Cleans up an initialized instance of VRAM MM.
- * @vmm: the VRAM MM instance to clean up
- */
-void drm_vram_mm_cleanup(struct drm_vram_mm *vmm)
+static void drm_vram_mm_cleanup(struct drm_vram_mm *vmm)
{
ttm_bo_device_release(>bdev);
}
-EXPORT_SYMBOL(drm_vram_mm_cleanup);
-/**
- * drm_vram_mm_mmap() - Helper for implementing file_operations.mmap()
- * @filp: the mapping's file structure
- * @vma: the mapping's memory area
- * @vmm: the VRAM MM instance
- *
- * Returns:
- * 0 on success, or
- * a negative error code otherwise.
- */
-int drm_vram_mm_mmap(struct file *filp, struct vm_area_struct *vma,
-struct drm_vram_mm *vmm)
+static int drm_vram_mm_mmap(struct file *filp, struct vm_area_struct *vma,
+ struct drm_vram_mm *vmm)
{
return ttm_bo_mmap(filp, vma, >bdev);
}
-EXPORT_SYMBOL(drm_vram_mm_mmap);
/*
* Helpers for integration with struct drm_device
diff --git a/include/drm/drm_gem_vram_helper.h
b/include/drm/drm_gem_vram_helper.h
index fd978e0c9542..9aaef4f8c327 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -171,12 +171,6 @@ static inline struct drm_vram_mm *drm_vram_mm_of_bdev(
}
int drm_vram_mm_debugfs_init(struct drm_minor *minor);
-int drm_vram_mm_init(struct drm_vram_mm *vmm, struct drm_device *dev,
-uint64_t vram_base, size_t vram_size);
-void drm_vram_mm_cleanup(struct drm_vram_mm *vmm);
-
-int drm_vram_mm_mmap(struct file *filp, struct vm_area_struct *vma,
-struct drm_vram_mm *vmm);
/*
* Helpers for integration with struct drm_device
--
2.23.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 0/4] Merge VRAM MM and GEM VRAM source files
VRAM MM and GEM VRAM are only used with each other. This patch set moves VRAM MM into GEM VRAM source files and cleans up the helper's public interface. Version 2 of the patch set doesn't contain functional changes. I'm reposting due to the rebasing onto the debugfs patches. v2: * updated for debugfs support * fixed typos in comments Thomas Zimmermann (4): drm/vram: Move VRAM memory manager to GEM VRAM implementation drm/vram: Have VRAM MM call GEM VRAM functions directly drm/vram: Unexport internal functions of VRAM MM drm/vram: Unconditonally set BO call-back functions Documentation/gpu/drm-mm.rst | 12 - drivers/gpu/drm/Makefile | 3 +- drivers/gpu/drm/ast/ast_drv.c | 1 - drivers/gpu/drm/ast/ast_main.c| 1 - drivers/gpu/drm/ast/ast_ttm.c | 3 +- drivers/gpu/drm/bochs/bochs.h | 1 - drivers/gpu/drm/bochs/bochs_mm.c | 3 +- drivers/gpu/drm/drm_gem_vram_helper.c | 406 +++--- drivers/gpu/drm/drm_vram_helper_common.c | 8 +- drivers/gpu/drm/drm_vram_mm_helper.c | 353 --- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 1 - drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 3 +- drivers/gpu/drm/mgag200/mgag200_drv.h | 1 - drivers/gpu/drm/mgag200/mgag200_ttm.c | 3 +- drivers/gpu/drm/vboxvideo/vbox_drv.h | 2 - drivers/gpu/drm/vboxvideo/vbox_ttm.c | 3 +- include/drm/drm_gem_vram_helper.h | 92 +++- include/drm/drm_vram_mm_helper.h | 109 - 18 files changed, 421 insertions(+), 584 deletions(-) delete mode 100644 drivers/gpu/drm/drm_vram_mm_helper.c delete mode 100644 include/drm/drm_vram_mm_helper.h -- 2.23.0 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
On 2019/9/11 下午5:36, Michael S. Tsirkin wrote:
On Wed, Sep 11, 2019 at 10:38:39AM +0800, Jason Wang wrote:
On 2019/9/10 下午9:52, Michael S. Tsirkin wrote:
On Tue, Sep 10, 2019 at 09:13:02PM +0800, Jason Wang wrote:
On 2019/9/10 下午6:01, Michael S. Tsirkin wrote:
+#ifndef _LINUX_VIRTIO_MDEV_H
+#define _LINUX_VIRTIO_MDEV_H
+
+#include
+#include
+#include
+
+/*
+ * Ioctls
+ */
Pls add a bit more content here. It's redundant to state these
are ioctls. Much better to document what does each one do.
Ok.
+
+struct virtio_mdev_callback {
+ irqreturn_t (*callback)(void *);
+ void *private;
+};
+
+#define VIRTIO_MDEV 0xAF
+#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \
+struct virtio_mdev_callback)
+#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \
+ struct virtio_mdev_callback)
Function pointer in an ioctl parameter? How does this ever make sense?
I admit this is hacky (casting).
And can't we use a couple of registers for this, and avoid ioctls?
Yes, how about something like interrupt numbers for each virtqueue and
config?
Should we just reuse VIRTIO_PCI_COMMON_Q_XXX then?
You mean something like VIRTIO_PCI_COMMON_Q_MSIX? Then it becomes a PCI
transport in fact. And using either MSIX or irq number is actually another
layer of indirection. So I think we can just write callback function and
parameter through registers.
I just realized, all these registers are just encoded so you
can pass stuff through read/write. But it can instead be
just a normal C function call with no messy encoding.
So why do we want to do this encoding?
Just because it was easier to start as a POC since mdev_parent_ops is
the only way to communicate between mdev driver and mdev device right
now. We can invent private ops besides mdev_parent_ops, e.g a private
pointer in mdev_parent_ops. I can try this in next version.
Thanks
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport
On Wed, Sep 11, 2019 at 10:38:39AM +0800, Jason Wang wrote:
>
> On 2019/9/10 下午9:52, Michael S. Tsirkin wrote:
> > On Tue, Sep 10, 2019 at 09:13:02PM +0800, Jason Wang wrote:
> > > On 2019/9/10 下午6:01, Michael S. Tsirkin wrote:
> > > > > +#ifndef _LINUX_VIRTIO_MDEV_H
> > > > > +#define _LINUX_VIRTIO_MDEV_H
> > > > > +
> > > > > +#include
> > > > > +#include
> > > > > +#include
> > > > > +
> > > > > +/*
> > > > > + * Ioctls
> > > > > + */
> > > > Pls add a bit more content here. It's redundant to state these
> > > > are ioctls. Much better to document what does each one do.
> > >
> > > Ok.
> > >
> > >
> > > > > +
> > > > > +struct virtio_mdev_callback {
> > > > > + irqreturn_t (*callback)(void *);
> > > > > + void *private;
> > > > > +};
> > > > > +
> > > > > +#define VIRTIO_MDEV 0xAF
> > > > > +#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \
> > > > > + struct virtio_mdev_callback)
> > > > > +#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \
> > > > > + struct virtio_mdev_callback)
> > > > Function pointer in an ioctl parameter? How does this ever make sense?
> > >
> > > I admit this is hacky (casting).
> > >
> > >
> > > > And can't we use a couple of registers for this, and avoid ioctls?
> > >
> > > Yes, how about something like interrupt numbers for each virtqueue and
> > > config?
> > Should we just reuse VIRTIO_PCI_COMMON_Q_XXX then?
>
>
> You mean something like VIRTIO_PCI_COMMON_Q_MSIX? Then it becomes a PCI
> transport in fact. And using either MSIX or irq number is actually another
> layer of indirection. So I think we can just write callback function and
> parameter through registers.
I just realized, all these registers are just encoded so you
can pass stuff through read/write. But it can instead be
just a normal C function call with no messy encoding.
So why do we want to do this encoding?
>
> >
> >
> > > > > +
> > > > > +#define VIRTIO_MDEV_DEVICE_API_STRING"virtio-mdev"
> > > > > +
> > > > > +/*
> > > > > + * Control registers
> > > > > + */
> > > > > +
> > > > > +/* Magic value ("virt" string) - Read Only */
> > > > > +#define VIRTIO_MDEV_MAGIC_VALUE 0x000
> > > > > +
> > > > > +/* Virtio device version - Read Only */
> > > > > +#define VIRTIO_MDEV_VERSION 0x004
> > > > > +
> > > > > +/* Virtio device ID - Read Only */
> > > > > +#define VIRTIO_MDEV_DEVICE_ID0x008
> > > > > +
> > > > > +/* Virtio vendor ID - Read Only */
> > > > > +#define VIRTIO_MDEV_VENDOR_ID0x00c
> > > > > +
> > > > > +/* Bitmask of the features supported by the device (host)
> > > > > + * (32 bits per set) - Read Only */
> > > > > +#define VIRTIO_MDEV_DEVICE_FEATURES 0x010
> > > > > +
> > > > > +/* Device (host) features set selector - Write Only */
> > > > > +#define VIRTIO_MDEV_DEVICE_FEATURES_SEL 0x014
> > > > > +
> > > > > +/* Bitmask of features activated by the driver (guest)
> > > > > + * (32 bits per set) - Write Only */
> > > > > +#define VIRTIO_MDEV_DRIVER_FEATURES 0x020
> > > > > +
> > > > > +/* Activated features set selector - Write Only */
> > > > > +#define VIRTIO_MDEV_DRIVER_FEATURES_SEL 0x024
> > > > > +
> > > > > +/* Queue selector - Write Only */
> > > > > +#define VIRTIO_MDEV_QUEUE_SEL0x030
> > > > > +
> > > > > +/* Maximum size of the currently selected queue - Read Only */
> > > > > +#define VIRTIO_MDEV_QUEUE_NUM_MAX0x034
> > > > > +
> > > > > +/* Queue size for the currently selected queue - Write Only */
> > > > > +#define VIRTIO_MDEV_QUEUE_NUM0x038
> > > > > +
> > > > > +/* Ready bit for the currently selected queue - Read Write */
> > > > > +#define VIRTIO_MDEV_QUEUE_READY 0x044
> > > > Is this same as started?
> > >
> > > Do you mean "status"?
> > I really meant "enabled", didn't remember the correct name.
> > As in: VIRTIO_PCI_COMMON_Q_ENABLE
>
>
> Yes, it's the same.
>
> Thanks
>
>
> >
> > > > > +
> > > > > +/* Alignment of virtqueue - Read Only */
> > > > > +#define VIRTIO_MDEV_QUEUE_ALIGN 0x048
> > > > > +
> > > > > +/* Queue notifier - Write Only */
> > > > > +#define VIRTIO_MDEV_QUEUE_NOTIFY 0x050
> > > > > +
> > > > > +/* Device status register - Read Write */
> > > > > +#define VIRTIO_MDEV_STATUS 0x060
> > > > > +
> > > > > +/* Selected queue's Descriptor Table address, 64 bits in two halves
> > > > > */
> > > > > +#define VIRTIO_MDEV_QUEUE_DESC_LOW 0x080
> > > > > +#define VIRTIO_MDEV_QUEUE_DESC_HIGH 0x084
> > > > > +
> > > > > +/* Selected queue's Available Ring address, 64 bits in two halves */
> > > > > +#define VIRTIO_MDEV_QUEUE_AVAIL_LOW 0x090
> > > > > +#define VIRTIO_MDEV_QUEUE_AVAIL_HIGH 0x094
> > > > > +
> > > > > +/* Selected queue's Used Ring address, 64 bits in two halves */
> > > > > +#define VIRTIO_MDEV_QUEUE_USED_LOW 0x0a0
> > > > > +#define VIRTIO_MDEV_QUEUE_USED_HIGH
[vhost:linux-next 14/17] include/linux/mmzone.h:815:3: error: implicit declaration of function 'move_page_to_reported_list'; did you mean 'move_to_free_list'?
tree: https://kernel.googlesource.com/pub/scm/linux/kernel/git/mst/vhost.git
linux-next
head: 39c226b6b576b23d6d558331e6895f02b0892555
commit: 50ed0c2ecb2e254a50e4ad775840f29d42cf6c00 [14/17] mm: Introduce Reported
pages
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 7.4.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 50ed0c2ecb2e254a50e4ad775840f29d42cf6c00
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=m68k
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
In file included from include/linux/page_reporting.h:5:0,
from :0:
include/linux/mmzone.h: In function 'add_to_free_list_tail':
include/linux/mmzone.h:791:27: error: implicit declaration of function
'get_unreported_tail' [-Werror=implicit-function-declaration]
struct list_head *tail = get_unreported_tail(zone, order, migratetype);
^~~
include/linux/mmzone.h:791:27: warning: initialization makes pointer from
integer without a cast [-Wint-conversion]
include/linux/mmzone.h: In function 'move_to_free_list':
include/linux/mmzone.h:807:27: warning: initialization makes pointer from
integer without a cast [-Wint-conversion]
struct list_head *tail = get_unreported_tail(zone, order, migratetype);
^~~
>> include/linux/mmzone.h:815:3: error: implicit declaration of function
>> 'move_page_to_reported_list'; did you mean 'move_to_free_list'?
>> [-Werror=implicit-function-declaration]
move_page_to_reported_list(page, zone, migratetype);
^~
move_to_free_list
include/linux/mmzone.h: In function 'del_page_from_free_list':
include/linux/mmzone.h:831:3: error: implicit declaration of function
'del_page_from_reported_list'; did you mean 'del_page_from_free_list'?
[-Werror=implicit-function-declaration]
del_page_from_reported_list(page, zone);
^~~
del_page_from_free_list
In file included from :0:0:
include/linux/page_reporting.h: At top level:
include/linux/page_reporting.h:74:1: error: conflicting types for
'get_unreported_tail'
get_unreported_tail(struct zone *zone, unsigned int order, int migratetype)
^~~
In file included from include/linux/page_reporting.h:5:0,
from :0:
include/linux/mmzone.h:791:27: note: previous implicit declaration of
'get_unreported_tail' was here
struct list_head *tail = get_unreported_tail(zone, order, migratetype);
^~~
In file included from :0:0:
include/linux/page_reporting.h:106:20: warning: conflicting types for
'del_page_from_reported_list'
static inline void del_page_from_reported_list(struct page *page,
^~~
include/linux/page_reporting.h:106:20: error: static declaration of
'del_page_from_reported_list' follows non-static declaration
In file included from include/linux/page_reporting.h:5:0,
from :0:
include/linux/mmzone.h:831:3: note: previous implicit declaration of
'del_page_from_reported_list' was here
del_page_from_reported_list(page, zone);
^~~
In file included from :0:0:
include/linux/page_reporting.h:118:20: warning: conflicting types for
'move_page_to_reported_list'
static inline void move_page_to_reported_list(struct page *page,
^~
include/linux/page_reporting.h:118:20: error: static declaration of
'move_page_to_reported_list' follows non-static declaration
In file included from include/linux/page_reporting.h:5:0,
from :0:
include/linux/mmzone.h:815:3: note: previous implicit declaration of
'move_page_to_reported_list' was here
move_page_to_reported_list(page, zone, migratetype);
^~
cc1: some warnings being treated as errors
vim +815 include/linux/mmzone.h
786
787 /* Used for pages not on another list */
788 static inline void add_to_free_list_tail(struct page *page, struct zone
*zone,
789 unsigned int order, int
migratetype)
790 {
> 791 struct list_head *tail = get_unreported_tail(zone, order,
> migratetype);
792
793 /*
794 * To prevent the unreported pages from being interleaved with
the
795 * reported ones while we are actively processing pages we will
use
796 * the head of the reported pages to determine the tail of the
free
797 * list.
798 */
799 list_add_tail(>lru, tail);
Re: [PATCH v2] virtio: add VIRTIO_RING_NO_LEGACY
On Tue, Sep 10, 2019 at 05:53:44PM +, Matej Genci wrote:
> Add macro to disable legacy functions vring_init and vring_size.
>
> Signed-off-by: Matej Genci
And I guess we should be able to define this macro in
drivers/virtio/virtio_pci_modern.c
?
Will be handy to make sure we don't mask too much.
> ---
>
> V2: Put all legacy APIs inside guards.
>
> ---
> include/uapi/linux/virtio_ring.h | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/uapi/linux/virtio_ring.h
> b/include/uapi/linux/virtio_ring.h
> index 4c4e24c291a5..efe5a421b4ea 100644
> --- a/include/uapi/linux/virtio_ring.h
> +++ b/include/uapi/linux/virtio_ring.h
> @@ -118,6 +118,8 @@ struct vring_used {
> struct vring_used_elem ring[];
> };
>
> +#ifndef VIRTIO_RING_NO_LEGACY
> +
> struct vring {
> unsigned int num;
>
> @@ -128,6 +130,8 @@ struct vring {
> struct vring_used *used;
> };
>
> +#endif /* VIRTIO_RING_NO_LEGACY */
> +
> /* Alignment requirements for vring elements.
> * When using pre-virtio 1.0 layout, these fall out naturally.
> */
> @@ -135,6 +139,8 @@ struct vring {
> #define VRING_USED_ALIGN_SIZE 4
> #define VRING_DESC_ALIGN_SIZE 16
>
> +#ifndef VIRTIO_RING_NO_LEGACY
> +
> /* The standard layout for the ring is a continuous chunk of memory which
> looks
> * like this. We assume num is a power of 2.
> *
> @@ -195,6 +201,8 @@ static inline int vring_need_event(__u16 event_idx, __u16
> new_idx, __u16 old)
> return (__u16)(new_idx - event_idx - 1) < (__u16)(new_idx - old);
> }
>
> +#endif /* VIRTIO_RING_NO_LEGACY */
> +
> struct vring_packed_desc_event {
> /* Descriptor Ring Change Event Offset/Wrap Counter. */
> __le16 off_wrap;
> --
> 2.22.0
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] virtio: add VIRTIO_RING_NO_LEGACY
On Tue, Sep 10, 2019 at 05:53:44PM +, Matej Genci wrote:
> Add macro to disable legacy functions vring_init and vring_size.
>
> Signed-off-by: Matej Genci
> ---
>
> V2: Put all legacy APIs inside guards.
>
> ---
> include/uapi/linux/virtio_ring.h | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/uapi/linux/virtio_ring.h
> b/include/uapi/linux/virtio_ring.h
> index 4c4e24c291a5..efe5a421b4ea 100644
> --- a/include/uapi/linux/virtio_ring.h
> +++ b/include/uapi/linux/virtio_ring.h
> @@ -118,6 +118,8 @@ struct vring_used {
> struct vring_used_elem ring[];
> };
>
> +#ifndef VIRTIO_RING_NO_LEGACY
> +
> struct vring {
> unsigned int num;
>
> @@ -128,6 +130,8 @@ struct vring {
> struct vring_used *used;
> };
>
> +#endif /* VIRTIO_RING_NO_LEGACY */
> +
> /* Alignment requirements for vring elements.
> * When using pre-virtio 1.0 layout, these fall out naturally.
> */
> @@ -135,6 +139,8 @@ struct vring {
> #define VRING_USED_ALIGN_SIZE 4
> #define VRING_DESC_ALIGN_SIZE 16
>
> +#ifndef VIRTIO_RING_NO_LEGACY
> +
> /* The standard layout for the ring is a continuous chunk of memory which
> looks
> * like this. We assume num is a power of 2.
> *
> @@ -195,6 +201,8 @@ static inline int vring_need_event(__u16 event_idx, __u16
> new_idx, __u16 old)
> return (__u16)(new_idx - event_idx - 1) < (__u16)(new_idx - old);
> }
>
> +#endif /* VIRTIO_RING_NO_LEGACY */
> +
> struct vring_packed_desc_event {
> /* Descriptor Ring Change Event Offset/Wrap Counter. */
> __le16 off_wrap;
OK almost but vring_need_event is actually useful for all variants
so should be outside the guards :) Sorry about it.
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
