Re: [PATCH net] vhost: Fix Spectre V1 vulnerability
From: Jason Wang Date: Tue, 30 Oct 2018 14:10:49 +0800 > The idx in vhost_vring_ioctl() was controlled by userspace, hence a > potential exploitation of the Spectre variant 1 vulnerability. > > Fixing this by sanitizing idx before using it to index d->vqs. > > Cc: Michael S. Tsirkin > Cc: Josh Poimboeuf > Cc: Andrea Arcangeli > Signed-off-by: Jason Wang Applied and queued up for -stable. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: PROPOSAL: Extend inline asm syntax with size spec
On Wed, Oct 31, 2018 at 01:55:26PM +0100, Peter Zijlstra wrote:
> On Sat, Oct 13, 2018 at 09:33:35PM +0200, Borislav Petkov wrote:
> > Ok,
> >
> > with Segher's help I've been playing with his patch ontop of bleeding
> > edge gcc 9 and here are my observations. Please double-check me for
> > booboos so that they can be addressed while there's time.
> >
> > So here's what I see ontop of 4.19-rc7:
> >
> > First marked the alternative asm() as inline and undeffed the "inline"
> > keyword. I need to do that because of the funky games we do with
> > "inline" redefinitions in include/linux/compiler_types.h.
> >
> > And Segher hinted at either doing:
> >
> > asm volatile inline(...
> >
> > or
> >
> > asm volatile __inline__(...
> >
> > but both "inline" variants are defined as macros in that file.
> >
> > Which means we either need to #undef inline before using it in asm() or
> > come up with something cleverer.
>
> # git grep -e "\<__inline__\>" | wc -l
> 488
> # git grep -e "\<__inline\>" | wc -l
> 56
> # git grep -e "\" | wc -l
> 69598
>
> And we already have scripts/checkpatch.pl:
>
> # Check for __inline__ and __inline, prefer inline
>
> Which suggests we do:
>
> git grep -l "\<__inline\(\|__\)\>" | while read file
> do
> sed -i -e 's/\<__inline\(\|__\)\>/inline/g' $file
> done
>
> and get it over with.
>
>
> Anyway, with the below patch, I get:
>
>textdata bss dec hex filename
> 173851835064780 1953892 244038551745f8f
> defconfig-build/vmlinux
> 173856785064780 1953892 24404350174617e
> defconfig-build/vmlinux
173876035065468 1953892 244069631746bb3
defconfig-build/vmlinux
If I do an additional:
git grep -l "asm volatile" | while read file
do
sed -i -e 's/asm volatile/asm_volatile/g' $file
done
on the tree...
No changes for:
-#define asm_volatile_goto(x...)do { asm goto(x); asm (""); } while (0)
+#define asm_volatile_goto(x...)do { asm __inline__ goto (x); asm ("");
} while (0)
I suppose all our goto's are small now (my tree includes Nadav's patch
to static_cpu_has).
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 0/5] virgl: fence fd support
Hi Rob, On Thu, 25 Oct 2018 at 19:37, Robert Foss wrote: > > This series implements fence support for drm/virtio and > has been tested using qemu, kmscube and the below branches. > > Rob Herring solved a reference counting issue and > suggested a context check for the execbuf ioctl, his > changes have been included in the below commits to > keep the tree working at all commits. > I've put forward some mostly minor suggestions. The patches look pretty good but there's one piece missing: > > The linux series can be found here: > https://gitlab.collabora.com/robertfoss/linux/commits/virtio_fences_v3 > > As for mesa, the branch can be found here: > https://gitlab.collabora.com/robertfoss/mesa/commits/virtio_fences_v3 > Namely: This should be out for review. The kernel and userspace side of IOCTLs should happen roughly at the same time. Otherwise, there's a huge chance of merging one side of the component, while the other needs architectural changes. HTH Emil ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 5/5] drm/virtio: bump driver version after explicit synchronization addition
On Thu, 25 Oct 2018 at 19:38, Robert Foss wrote: > > From: Gustavo Padovan > > To reflect the (backward compatible) changes in the uabi we are bumping > the driver's version. > > Signed-off-by: Gustavo Padovan > Signed-off-by: Robert Foss Not strictly required, but doesn't hurt either. Reviewed-by: Emil Velikov -Emil ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/5] drm/virtio: add out-fences support for explicit synchronization
Hi Rob,
On Thu, 25 Oct 2018 at 19:38, Robert Foss wrote:
>
> From: Gustavo Padovan
>
> On the out-fence side we get fence returned by the submitted draw call
> and attach it to a sync_file and send the sync_file fd to userspace. On
> error -1 is returned to userspace.
>
Can we have both an IN and OUT fence at the same time? Either way, please
mention that in the commit message.
> Signed-off-by: Gustavo Padovan
> Signed-off-by: Robert Foss
> Suggested-by: Rob Herring
> ---
> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 50 +++---
> 1 file changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 0368195966aa..32e714a1c753 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -106,7 +106,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device
> *dev, void *data,
> struct virtio_gpu_device *vgdev = dev->dev_private;
> struct virtio_gpu_fpriv *vfpriv = drm_file->driver_priv;
> struct drm_gem_object *gobj;
> - struct virtio_gpu_fence *fence;
> + struct virtio_gpu_fence *out_fence;
> struct virtio_gpu_object *qobj;
> int ret;
> uint32_t *bo_handles = NULL;
> @@ -116,7 +116,9 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device
> *dev, void *data,
> int i;
> struct ww_acquire_ctx ticket;
> struct dma_fence *in_fence = NULL;
> + struct sync_file *sync_file;
> int in_fence_fd = exbuf->fence_fd;
> + int out_fence_fd = -1;
> void *buf;
>
> exbuf->fence_fd = -1;
> @@ -143,6 +145,14 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device
> *dev, void *data,
> }
> }
>
> + if (exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_OUT) {
> + out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
> + if (out_fence_fd < 0) {
> + ret = out_fence_fd;
> + goto out_in_fence;
> + }
> + }
> +
If the answer to the above is "no" we want a check around here.
With that the patch is
Reviewed-by: Emil Velikov
-Emil
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 3/5] drm/virtio: add in-fences support for explicit synchronization
Hi Rob,
On Thu, 25 Oct 2018 at 19:38, Robert Foss wrote:
> @@ -124,6 +127,22 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device
> *dev, void *data,
> if ((exbuf->flags & ~VIRTGPU_EXECBUF_FLAGS))
> return -EINVAL;
>
> + if (exbuf->flags & VIRTGPU_EXECBUF_FENCE_FD_IN) {
> + in_fence = sync_file_get_fence(in_fence_fd);
> + if (!in_fence)
> + return -EINVAL;
> +
> + /*
> +* Wait if the fence is from a foreign context, or if the
> fence
> +* array contains any fence from a foreign context.
> +*/
> + if (!dma_fence_match_context(in_fence,
> vgdev->fence_drv.context)) {
> + ret = dma_fence_wait(in_fence, true);
> + if (ret)
> + return ret;
Aren't we missing dma_fence_put() before the return here?
With that
Reviewed-by: Emil Velikov
-Emil
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 2/5] drm/virtio: add uapi for in and out explicit fences
Hi Rob, On Thu, 25 Oct 2018 at 19:38, Robert Foss wrote: > > Add a new field called fence_fd that will be used by userspace to send > in-fences to the kernel and receive out-fences created by the kernel. > > This uapi enables virtio to take advantage of explicit synchronization of > dma-bufs. > > There are two new flags: > > * VIRTGPU_EXECBUF_FENCE_FD_IN to be used when passing an in-fence fd. > * VIRTGPU_EXECBUF_FENCE_FD_OUT to be used when requesting an out-fence fd > > The execbuffer IOCTL is now read-write to allow the userspace to read the > out-fence. > > On error -1 should be returned in the fence_fd field. > > Signed-off-by: Gustavo Padovan > Signed-off-by: Robert Foss > --- > Changes since v2: > - Since exbuf-flags is a new flag, check that unsupported >flags aren't set. > > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 + > include/uapi/drm/virtgpu_drm.h | 13 ++--- > 2 files changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c > b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > index d01a9ed100d1..1af289b28fc4 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c > +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c > @@ -116,9 +116,14 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device > *dev, void *data, > struct ww_acquire_ctx ticket; > void *buf; > > + exbuf->fence_fd = -1; > + Move this after the sanity checking. > if (vgdev->has_virgl_3d == false) > return -ENOSYS; > > + if ((exbuf->flags & ~VIRTGPU_EXECBUF_FLAGS)) > + return -EINVAL; > + I assume this did this trigger when using old userspace? With those the patch is Reviewed-by: Emil Velikov Thanks Emil ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/5] drm/virtio: add virtio_gpu_alloc_fence()
Hi Rob,
On Thu, 25 Oct 2018 at 19:38, Robert Foss wrote:
>
> From: Gustavo Padovan
>
> Refactor fence creation to remove the potential allocation failure from
> the cmd_submit and atomic_commit paths. Now the fence should be allocated
> first and just after we should proceed with the rest of the execution.
>
Commit does a bit more that what the above says:
- dummy, factor out fence creation/destruction
- use per virtio_gpu_framebuffer fence
Personally I'd keep the two separate patches and elaborate on the latter.
Obviously in that case, one will need to add 3 lines worth of
virtio_gpu_fence_alloc() in virtio_gpu_cursor_plane_update which will be nuked
with the next patch.
Not a big deal, but it's up-to the maintainer to make the final call if it's
worth splitting or not.
Couple of minor nitpicks below.
> struct virtio_gpu_device *vgdev = dev->dev_private;
> struct virtio_gpu_output *output = NULL;
> struct virtio_gpu_framebuffer *vgfb;
> - struct virtio_gpu_fence *fence = NULL;
> struct virtio_gpu_object *bo = NULL;
> uint32_t handle;
> int ret = 0;
Add the virtio_gpu_fence_alloc()? And yes it will be nuked with patch 2/...
> +struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device
> *vgdev)
> +{
> + struct virtio_gpu_fence_driver *drv = &vgdev->fence_drv;
> + struct virtio_gpu_fence *fence = kzalloc(sizeof(struct
> virtio_gpu_fence), GFP_ATOMIC);
> + if (!fence)
> + return fence;
> +
> + fence->drv = drv;
> + dma_fence_init(&fence->f, &virtio_fence_ops, &drv->lock,
> drv->context, 0);
Oh no, lines over 80 col... while the original code is pretty and neat.
> +
> + return fence;
> +}
> +
> +void virtio_gpu_fence_cleanup(struct virtio_gpu_fence *fence)
> +{
> + if (!fence)
> + return;
> +
> + if (fence->drv)
> + dma_fence_put(&fence->f);
> + else
> + kfree(fence);
I'm not sure if/how we reach the else case here?
> +}
> +
> int virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_ctrl_hdr *cmd_hdr,
> - struct virtio_gpu_fence **fence)
> + struct virtio_gpu_fence *fence)
> {
With a follow-up commit, we can drop the no longer needed return type.
Which it turns out was never checked ...
> @@ -319,6 +332,8 @@ static int virtio_gpu_resource_create_ioctl(struct
> drm_device *dev, void *data,
> dma_fence_put(&fence->f);
> }
> return 0;
> +fail_fence:
The error labels seems to be called after what they do, not what
fails. fail_backoff seems better IMHO.
> +ttm_eu_backoff_reservation(&ticket, &validate_list);
Indentation seems off (or my client ate it)?
HTH
Emil
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 00/20] vmw_balloon: compaction, shrinker, 64-bit, etc.
On Tue, Oct 30, 2018 at 04:52:55PM +, Nadav Amit wrote: > From: gre...@linuxfoundation.org > Sent: October 30, 2018 at 4:51:19 PM GMT > > To: Nadav Amit > > Cc: Arnd Bergmann , Xavier Deguillard > > , LKML , Michael S. > > Tsirkin , Jason Wang , > > linux...@kvack.org , > > virtualization@lists.linux-foundation.org > > > > Subject: Re: [PATCH v3 00/20] vmw_balloon: compaction, shrinker, 64-bit, > > etc. > > > > > > On Tue, Oct 30, 2018 at 04:32:22PM +, Nadav Amit wrote: > >> From: Nadav Amit > >> Sent: September 26, 2018 at 7:13:16 PM GMT > >>> To: Arnd Bergmann , gre...@linuxfoundation.org > >>> Cc: Xavier Deguillard , > >>> linux-ker...@vger.kernel.org>, Nadav Amit , Michael S. > >>> Tsirkin , Jason Wang , > >>> linux...@kvack.org>, virtualization@lists.linux-foundation.org > >>> Subject: [PATCH v3 00/20] vmw_balloon: compaction, shrinker, 64-bit, etc. > >>> > >>> > >>> This patch-set adds the following enhancements to the VMware balloon > >>> driver: > >>> > >>> 1. Balloon compaction support. > >>> 2. Report the number of inflated/deflated ballooned pages through vmstat. > >>> 3. Memory shrinker to avoid balloon over-inflation (and OOM). > >>> 4. Support VMs with memory limit that is greater than 16TB. > >>> 5. Faster and more aggressive inflation. > >>> > >>> To support compaction we wish to use the existing infrastructure. > >>> However, we need to make slight adaptions for it. We add a new list > >>> interface to balloon-compaction, which is more generic and efficient, > >>> since it does not require as many IRQ save/restore operations. We leave > >>> the old interface that is used by the virtio balloon. > >>> > >>> Big parts of this patch-set are cleanup and documentation. Patches 1-13 > >>> simplify the balloon code, document its behavior and allow the balloon > >>> code to run concurrently. The support for concurrency is required for > >>> compaction and the shrinker interface. > >>> > >>> For documentation we use the kernel-doc format. We are aware that the > >>> balloon interface is not public, but following the kernel-doc format may > >>> be useful one day. > >>> > >>> v2->v3: * Moving the balloon magic-number out of uapi (Greg) > >>> > >>> v1->v2: * Fix build error when THP is off (kbuild) > >>> * Fix build error on i386 (kbuild) > >> > >> Greg, > >> > >> I realize you didn’t apply patches 17-20. Any reason for that? > > > > I have no idea, that was a few thousand patches reviewed ago... > > > > Did I not say anything about this when I applied them? > > > > greg k-h > > You regarded the magic-number in v2, which I fixed for v3. > > Should I resend? Please do, but note that I will not be reviewing anything until after 4.20-rc1 is out. thanks, greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
