* Re: [Qemu-devel] security implications of caching with virtio pmem (was Re: [PATCH v3 0/5] kvm "virtio pmem" device)
From: Michael S. Tsirkin @ 2019-02-11 23:07 UTC (permalink / raw)
To: David Hildenbrand
Cc: Pankaj Gupta, jack, kvm, linux-nvdimm, Dave Chinner, qemu-devel,
virtualization, pbonzini, adilger kernel, zwisler,
Andrea Arcangeli, dave jiang, darrick wong, vishal l verma, willy,
hch, linux-acpi, jmoyer, nilal, riel, stefanha, dchinner,
dan j williams, lcapitulino, linux-ext4, tytso,
xiaoguangrong eric, rjw, linux-kernel, linux-xfs, linux-fsdevel,
imammedo@
In-Reply-To: <b2be7796-9dee-b9fd-87e5-1ae570aa7db6@redhat.com>
On Mon, Feb 11, 2019 at 11:58:15PM +0100, David Hildenbrand wrote:
> On 11.02.19 23:29, Dave Chinner wrote:
> > On Mon, Feb 11, 2019 at 02:29:46AM -0500, Pankaj Gupta wrote:
> >> Hello Dave,
> >> Are we okay with this?
> >
> > Sure.
> >
> > I'm not sure I agree with all the analysis presented, but, well, I
> > haven't looked any deeper because I'm tired of being shouted at and
> > being called argumentative for daring to ask hard questions about
> > this topic....
>
> I think if you have concerns, they should definitely be discussed.
> Making people frustrated that review code is not what we want. Not at all.
>
> I suggest that Pankaj properly documents what we found out so far about
> security concerns and properly describes intended use cases and answers
> other questions you had in the cover letter / documentation of the
> follow up series.
>
> Thanks Dave!
Right. Also, there's an open question that you posed:
Also other storage devices have caches like that (well, the caches size
depends on the device) - thinking especially about storage systems -
which would in my opinion, also allow similar leaks. How are such
security concerns handled there? Are they different (besides eventually
access speed)?
and that needs some looking into, and reporting on.
> >
> > Cheers,
> >
> > Dave.
> >
>
>
> --
>
> Thanks,
>
> David / dhildenb
^ permalink raw reply
* Re: [Qemu-devel] security implications of caching with virtio pmem (was Re: [PATCH v3 0/5] kvm "virtio pmem" device)
From: David Hildenbrand @ 2019-02-11 22:58 UTC (permalink / raw)
To: Dave Chinner, Pankaj Gupta
Cc: jack, kvm, Michael S. Tsirkin, linux-nvdimm, qemu-devel,
virtualization, pbonzini, adilger kernel, zwisler,
Andrea Arcangeli, dave jiang, darrick wong, vishal l verma, willy,
hch, linux-acpi, jmoyer, nilal, riel, stefanha, dchinner,
dan j williams, lcapitulino, linux-ext4, tytso,
xiaoguangrong eric, rjw, linux-kernel, linux-xfs, linux-fsdevel,
imammedo
In-Reply-To: <20190211222907.GR14116@dastard>
On 11.02.19 23:29, Dave Chinner wrote:
> On Mon, Feb 11, 2019 at 02:29:46AM -0500, Pankaj Gupta wrote:
>> Hello Dave,
>> Are we okay with this?
>
> Sure.
>
> I'm not sure I agree with all the analysis presented, but, well, I
> haven't looked any deeper because I'm tired of being shouted at and
> being called argumentative for daring to ask hard questions about
> this topic....
I think if you have concerns, they should definitely be discussed.
Making people frustrated that review code is not what we want. Not at all.
I suggest that Pankaj properly documents what we found out so far about
security concerns and properly describes intended use cases and answers
other questions you had in the cover letter / documentation of the
follow up series.
Thanks Dave!
>
> Cheers,
>
> Dave.
>
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [Qemu-devel] security implications of caching with virtio pmem (was Re: [PATCH v3 0/5] kvm "virtio pmem" device)
From: Dave Chinner @ 2019-02-11 22:29 UTC (permalink / raw)
To: Pankaj Gupta
Cc: jack, kvm, linux-nvdimm, qemu-devel, virtualization, pbonzini,
adilger kernel, zwisler, Andrea Arcangeli, dave jiang,
darrick wong, vishal l verma, Michael S. Tsirkin, willy, hch,
linux-acpi, jmoyer, nilal, riel, stefanha, dchinner,
dan j williams, lcapitulino, linux-ext4, tytso,
xiaoguangrong eric, rjw, linux-kernel, linux-xfs, linux-fsdevel,
imammedo
In-Reply-To: <888328358.132676.1549870186945.JavaMail.zimbra@redhat.com>
On Mon, Feb 11, 2019 at 02:29:46AM -0500, Pankaj Gupta wrote:
> Hello Dave,
> Are we okay with this?
Sure.
I'm not sure I agree with all the analysis presented, but, well, I
haven't looked any deeper because I'm tired of being shouted at and
being called argumentative for daring to ask hard questions about
this topic....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [Qemu-devel] security implications of caching with virtio pmem (was Re: [PATCH v3 0/5] kvm "virtio pmem" device)
From: Pankaj Gupta @ 2019-02-11 7:29 UTC (permalink / raw)
To: Michael S. Tsirkin, dchinner
Cc: jack, kvm, linux-nvdimm, qemu-devel, virtualization,
adilger kernel, zwisler, Andrea Arcangeli, dave jiang,
darrick wong, vishal l verma, willy, hch, linux-acpi, jmoyer,
nilal, riel, stefanha, pbonzini, dan j williams, lcapitulino,
linux-ext4, tytso, xiaoguangrong eric, rjw, linux-kernel,
linux-xfs, linux-fsdevel, imammedo
In-Reply-To: <20190204170515-mutt-send-email-mst@kernel.org>
Hi Michael,
Thanks for looking into this and summarizing in detail.
> > This patch series has implementation for "virtio pmem".
> > "virtio pmem" is fake persistent memory(nvdimm) in guest
> > which allows to bypass the guest page cache. This also
> > implements a VIRTIO based asynchronous flush mechanism.
>
>
> At Pankaj's request I looked at information leak implications of virtio
> pmem in light of the recent page cache side channels paper
> (https://arxiv.org/pdf/1901.01161.pdf) - to see what
> kind of side channels it might create if any. TLDR - I think that
> depending on the host side implementation there could be some, but this
> might be addressable by better documentation in both code and spec.
> The fake dax approach backing the guest memory by a host page cache
> does seem to have potential issues.
>
> For clarity: we are talking about leaking information either to a VM, or
> within a VM (I did not look into leaks to hypervisor in configurations
> such as SEV) through host page cache.
>
> Leaks into a VM: It seems clear that while pmem allows memory accesses
> versus read/write with e.g. a block device, from host page cache point
> of view this doesn't matter much: reads populate cache in the same way
> as memory faults. Thus ignoring presence of information leaks (which is
> an interesting question e.g. in light of recent discard support) pmem
> doesn't seem to be any better or worse for leaking information into a
> VM.
>
> Leaks within VM: Right now pmem seems to bypass the guest page cache
> completely. Whether pmem memory is then resident in a page cache would
> be up to the device/host. Assuming that it is, the "Preventing
> Efficient Eviction while Increasing the System Performance"
> countermeasure for the page cache side channel attack would appear to
> become ineffective with pmem. What is suggested is a per-process
> management of the page cache, and host does not have visibility of
> processes within a VM. Another possible countermeasure - not discussed
> in the paper - could be modify the applications to lock the security
> relevant pages in memory. Again this becomes impractical with pmem as
> host does not have visibility into that. However note that as long
> as the only countermeasure linux uses is "Privileged Access"
> (i.e. blocking mincore) nothing can be done as guest page cache
> remains as vulnerable as host page cache.
>
>
> Countermeasures: which host-side countermeasures can be designed would
> depend on which countermeasures are used guest-side - we would need to
> make sure they are not broken by pmem. For "Preventing Efficient
> Eviction while Increasing the System Performance" modifying the host
> implementation to ensure that pmem device bypasses the host page cache
> would seem to address the security problem.Similarly, ensuring that a
> real memory device (e.g. DAX, RAM such as hugetlbfs, pmem for nested
> virt) is used for pmem would make the memory locking countermeasure
> work. Whether with such limitations the device is still useful
> performance wise is an open question. These questions probably should
> be addressed in the documentation, spec and possible qemu code.
>
>
>
> Severity of the security implications: some people argue that the
> security implications of the page cache leaks are minor. I do not have
> an opinion on this: the severity would seem to depend on the specific
> configuration.
>
>
> Other security implications: recent discussion seems to suggest there
> are other concerns around e.g. resource management and thus DOS
> potential. If that's so, it's a matter for a separate discussion
> as I didn't look into that in depth.
>
> Some or all of the above might be based on a misunderstanding of the
> current pmem code, the whitepaper and linux page cache in general.
> If so I apologise, do not hesitate to call out any mistakes.
I agree similar to any guest VM(without virtio-pmem) or any host
userspace process, virtio-pmem may also have some security implications.
We need to document these in virtio-pmem host device specification
and in qemu code. This is to make sure while creating virtio-pmem
device, we are aware of these implications and create/use
host side device backing file accordingly as per the use-case
described by David here [1].
I will document these in next version of my patch series.
Hello Dave,
Are we okay with this?
Thank you everyone for the discussion.
[1] https://marc.info/?l=linux-kernel&m=154946989419403&w=2
Best regards,
Pankaj
^ permalink raw reply
* Re: Resource management for ndo_xdp_xmit (Was: [PATCH net] virtio_net: Account for tx bytes and packets on sending xdp_frames)
From: Jakub Kicinski @ 2019-02-09 2:05 UTC (permalink / raw)
To: Saeed Mahameed
Cc: toke@toke.dk, hawk@kernel.org, mst@redhat.com,
netdev@vger.kernel.org, dsahern@gmail.com,
john.fastabend@gmail.com,
virtualization@lists.linux-foundation.org, thoiland@redhat.com,
brouer@redhat.com, borkmann@iogearbox.net, davem@davemloft.net,
Tariq Toukan
In-Reply-To: <b4eb9dd1c7aaa3215ade8d3d17f397588a519ca3.camel@mellanox.com>
On Sat, 9 Feb 2019 00:18:31 +0000, Saeed Mahameed wrote:
> On Fri, 2019-02-08 at 15:17 -0800, Saeed Mahameed wrote:
> > On Thu, 2019-02-07 at 19:08 +0000, Saeed Mahameed wrote:
> > >
> > > So
> > > 1) on dev_map_update_elem() we will call
> > > dev->dev->ndo_bpf() to notify the device on the intention to
> > > start/stop
> > > redirect, and wait for it to create/destroy the HW resources
> > > before/after actually updating the map
> > >
> >
> > silly me, dev_map_update_elem must be atomic, we can't hook driver
> > resource allocation to it, it must come as a separate request
> > (syscall)
> > from user space to request to create XDP redirect resources.
> >
>
> Well, it is possible to render dev_map_update_elem non-atomic and fail
> BPF programs who try to update it in the verifier
> check_map_func_compatibility.
>
> if you know of any case where devmap needs to be updated from the BPF
> program please let me know.
Did we find a solution to non-map redirect?
Sorry if I missed the discussion, I couldn't make the iovisor call this
week due to travel.
^ permalink raw reply
* Re: [PATCH net] virtio_net: Account for tx bytes and packets on sending xdp_frames
From: Jesper Dangaard Brouer @ 2019-02-08 16:02 UTC (permalink / raw)
To: Saeed Mahameed
Cc: jakub.kicinski@netronome.com, hawk@kernel.org, mst@redhat.com,
netdev@vger.kernel.org, john.fastabend@gmail.com, brouer,
virtualization@lists.linux-foundation.org, thoiland@redhat.com,
dsahern@gmail.com, borkmann@iogearbox.net, davem@davemloft.net,
Tariq Toukan
In-Reply-To: <140ecbe1e25f54f90d859cc696c4119aa96bc6eb.camel@mellanox.com>
On Wed, 6 Feb 2019 00:06:33 +0000 Saeed Mahameed <saeedm@mellanox.com> wrote:
> 2) Driver should keep track of XDP decisions statistics, report them in
> ethtool and in the new API suggested by David. track even (XDP_PASS) ?
>
> Maybe instead of having all drivers track the statistics on their own,
> we should move the responsibility to upper layer.
>
> Idea: since we already have rxq_info structure per XDP ring (no false
> sharing) and available per xdp_buff we can do:
>
> +++ b/include/linux/filter.h
> @@ -651,7 +651,9 @@ static __always_inline u32 bpf_prog_run_xdp(const
> struct bpf_prog *prog,
> * already takes rcu_read_lock() when fetching the program, so
> * it's not necessary here anymore.
> */
> - return BPF_PROG_RUN(prog, xdp);
> + u32 ret = BPF_PROG_RUN(prog, xdp);
> + xdp->xdp_rxq_info.stats[ret]++
> + return ret;
> }
>
> still we need a way (API) to report the rxq_info to whoever needs to
> read current XDP stats
I'm capturing this as tasks under the XDP-project github page:
https://github.com/xdp-project/xdp-project/pull/13/files
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH] drm/virtio: do NOT reuse resource ids
From: Gerd Hoffmann @ 2019-02-08 14:04 UTC (permalink / raw)
To: dri-devel, David Airlie
Cc: David Airlie, open list, Daniel Vetter,
open list:VIRTIO GPU DRIVER
Bisected guest kernel changes crashing qemu. Landed at
"6c1cd97bda drm/virtio: fix resource id handling". Looked again, and
noticed we where not only leaking *some* ids, but *all* ids. The old
code never ever called virtio_gpu_resource_id_put().
So, commit 6c1cd97bda effectively makes the linux kernel starting
re-using IDs after releasing them, and apparently virglrenderer can't
deal with that. Oops.
This patch puts a temporary stopgap into place for the 5.0 release.
Cc: David Airlie <airlied@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
Notes:
Hi Dave, I'll be offline next week, so please commit to
drm-next-fixes if you think this patch is acceptable.
drivers/gpu/drm/virtio/virtgpu_object.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index f39a183d59..e7e9460350 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -28,10 +28,21 @@
static int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
uint32_t *resid)
{
+#if 0
int handle = ida_alloc(&vgdev->resource_ida, GFP_KERNEL);
if (handle < 0)
return handle;
+#else
+ static int handle;
+
+ /*
+ * FIXME: dirty hack to avoid re-using IDs, virglrenderer
+ * can't deal with that. Needs fixing in virglrenderer, also
+ * should figure a better way to handle that in the guest.
+ */
+ handle++;
+#endif
*resid = handle + 1;
return 0;
@@ -39,7 +50,9 @@ static int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
static void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t id)
{
+#if 0
ida_free(&vgdev->resource_ida, id - 1);
+#endif
}
static void virtio_gpu_ttm_bo_destroy(struct ttm_buffer_object *tbo)
--
2.9.3
^ permalink raw reply related
* Re: [PATCH 2/5] drm/ttm: Define a single DRM_FILE_PAGE_OFFSET constant
From: Thomas Hellstrom via Virtualization @ 2019-02-07 19:07 UTC (permalink / raw)
To: kraxel@redhat.com, zourongrong@gmail.com,
christian.koenig@amd.com, hdegoede@redhat.com,
tzimmermann@suse.de, alexander.deucher@amd.com,
David1.Zhou@amd.com, ray.huang@amd.com, bskeggs@redhat.com,
kong.kongxinwei@hisilicon.com, airlied@redhat.com,
daniel@ffwll.ch, Jerry.Zhang@amd.com, Linux-graphics-maintainer,
gregkh@linuxfoundation.org, z.liuxinliang@hisilicon.com,
puck.chen@hisilicon.com, arnd@arndb.de
Cc: spice-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
dri-devel@lists.freedesktop.org,
virtualization@lists.linux-foundation.org
In-Reply-To: <20190207085931.29569-3-tzimmermann@suse.de>
On Thu, 2019-02-07 at 09:59 +0100, Thomas Zimmermann wrote:
> Most TTM drivers define the constant DRM_FILE_PAGE_OFFSET of the same
> value. The only exception is vboxvideo, which is being converted to
> the
> new offset by this patch. Unifying the constants in a single place
> simplifies the driver code.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 --
> drivers/gpu/drm/ast/ast_drv.h | 2 --
> drivers/gpu/drm/bochs/bochs.h | 2 --
> drivers/gpu/drm/cirrus/cirrus_drv.h | 1 -
> drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 2 --
> drivers/gpu/drm/mgag200/mgag200_drv.h | 1 -
> drivers/gpu/drm/nouveau/nouveau_drv.h | 2 --
> drivers/gpu/drm/qxl/qxl_drv.h | 3 ---
> drivers/gpu/drm/radeon/radeon_ttm.c | 2 --
> drivers/gpu/drm/virtio/virtgpu_ttm.c | 2 --
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 -
> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 2 +-
> drivers/staging/vboxvideo/vbox_drv.h | 2 --
> include/drm/ttm/ttm_bo_driver.h | 2 ++
> 15 files changed, 4 insertions(+), 24 deletions(-)
>
>
...
> diff --git a/include/drm/ttm/ttm_bo_driver.h
> b/include/drm/ttm/ttm_bo_driver.h
> index cbf3180cb612..c0bed72492f3 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -49,6 +49,8 @@
> #define TTM_MEMTYPE_FLAG_MAPPABLE (1 << 1) /* Memory
> mappable */
> #define TTM_MEMTYPE_FLAG_CMA (1 << 3) /* Can't map
> aperture */
>
> +#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
> +
IMO strictly, the place in the drm file address space where a driver
chooses to insert the TTM bo address space is to be determined by DRM,
(as also the define indicates) rather than TTM. So I think the right
place for this define is a drm header, not a ttm header.
Thanks,
Thomas
> struct ttm_mem_type_manager;
>
> struct ttm_mem_type_manager_func {
^ permalink raw reply
* [PATCH v7 5/5] virtio-blk: Consider virtio_max_dma_size() for maximum segment size
From: Joerg Roedel @ 2019-02-07 11:59 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, Konrad Rzeszutek Wilk,
Christoph Hellwig
Cc: Jens Axboe, Thomas.Lendacky, jroedel, brijesh.singh, joro,
jon.grimm, jfehlig, linux-kernel, linux-block, iommu,
virtualization
In-Reply-To: <20190207115917.20535-1-joro@8bytes.org>
From: Joerg Roedel <jroedel@suse.de>
Segments can't be larger than the maximum DMA mapping size
supported on the platform. Take that into account when
setting the maximum segment size for a block device.
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
drivers/block/virtio_blk.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index b16a887bbd02..4bc083b7c9b5 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -723,7 +723,7 @@ static int virtblk_probe(struct virtio_device *vdev)
struct request_queue *q;
int err, index;
- u32 v, blk_size, sg_elems, opt_io_size;
+ u32 v, blk_size, max_size, sg_elems, opt_io_size;
u16 min_io_size;
u8 physical_block_exp, alignment_offset;
@@ -826,14 +826,16 @@ static int virtblk_probe(struct virtio_device *vdev)
/* No real sector limit. */
blk_queue_max_hw_sectors(q, -1U);
+ max_size = virtio_max_dma_size(vdev);
+
/* Host can optionally specify maximum segment size and number of
* segments. */
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX,
struct virtio_blk_config, size_max, &v);
if (!err)
- blk_queue_max_segment_size(q, v);
- else
- blk_queue_max_segment_size(q, -1U);
+ max_size = min(max_size, v);
+
+ blk_queue_max_segment_size(q, max_size);
/* Host can optionally specify the block size of the device */
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE,
--
2.17.1
^ permalink raw reply related
* [PATCH v7 4/5] virtio: Introduce virtio_max_dma_size()
From: Joerg Roedel @ 2019-02-07 11:59 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, Konrad Rzeszutek Wilk,
Christoph Hellwig
Cc: Jens Axboe, Thomas.Lendacky, jroedel, brijesh.singh, joro,
jon.grimm, jfehlig, linux-kernel, linux-block, iommu,
virtualization
In-Reply-To: <20190207115917.20535-1-joro@8bytes.org>
From: Joerg Roedel <jroedel@suse.de>
This function returns the maximum segment size for a single
dma transaction of a virtio device. The possible limit comes
from the SWIOTLB implementation in the Linux kernel, that
has an upper limit of (currently) 256kb of contiguous
memory it can map. Other DMA-API implementations might also
have limits.
Use the new dma_max_mapping_size() function to determine the
maximum mapping size when DMA-API is in use for virtio.
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
drivers/virtio/virtio_ring.c | 11 +++++++++++
include/linux/virtio.h | 2 ++
2 files changed, 13 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cd7e755484e3..8a31c6862b2b 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -266,6 +266,17 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
return false;
}
+size_t virtio_max_dma_size(struct virtio_device *vdev)
+{
+ size_t max_segment_size = SIZE_MAX;
+
+ if (vring_use_dma_api(vdev))
+ max_segment_size = dma_max_mapping_size(&vdev->dev);
+
+ return max_segment_size;
+}
+EXPORT_SYMBOL_GPL(virtio_max_dma_size);
+
static void *vring_alloc_queue(struct virtio_device *vdev, size_t size,
dma_addr_t *dma_handle, gfp_t flag)
{
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index fa1b5da2804e..673fe3ef3607 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -157,6 +157,8 @@ int virtio_device_freeze(struct virtio_device *dev);
int virtio_device_restore(struct virtio_device *dev);
#endif
+size_t virtio_max_dma_size(struct virtio_device *vdev);
+
#define virtio_device_for_each_vq(vdev, vq) \
list_for_each_entry(vq, &vdev->vqs, list)
--
2.17.1
^ permalink raw reply related
* [PATCH v7 3/5] dma: Introduce dma_max_mapping_size()
From: Joerg Roedel @ 2019-02-07 11:59 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, Konrad Rzeszutek Wilk,
Christoph Hellwig
Cc: Jens Axboe, Thomas.Lendacky, jroedel, brijesh.singh, joro,
jon.grimm, jfehlig, linux-kernel, linux-block, iommu,
virtualization
In-Reply-To: <20190207115917.20535-1-joro@8bytes.org>
From: Joerg Roedel <jroedel@suse.de>
The function returns the maximum size that can be mapped
using DMA-API functions. The patch also adds the
implementation for direct DMA and a new dma_map_ops pointer
so that other implementations can expose their limit.
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
Documentation/DMA-API.txt | 8 ++++++++
include/linux/dma-mapping.h | 8 ++++++++
kernel/dma/direct.c | 11 +++++++++++
kernel/dma/mapping.c | 14 ++++++++++++++
4 files changed, 41 insertions(+)
diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index e133ccd60228..acfe3d0f78d1 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -195,6 +195,14 @@ Requesting the required mask does not alter the current mask. If you
wish to take advantage of it, you should issue a dma_set_mask()
call to set the mask to the value returned.
+::
+
+ size_t
+ dma_direct_max_mapping_size(struct device *dev);
+
+Returns the maximum size of a mapping for the device. The size parameter
+of the mapping functions like dma_map_single(), dma_map_page() and
+others should not be larger than the returned value.
Part Id - Streaming DMA mappings
--------------------------------
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f6ded992c183..5b21f14802e1 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -130,6 +130,7 @@ struct dma_map_ops {
enum dma_data_direction direction);
int (*dma_supported)(struct device *dev, u64 mask);
u64 (*get_required_mask)(struct device *dev);
+ size_t (*max_mapping_size)(struct device *dev);
};
#define DMA_MAPPING_ERROR (~(dma_addr_t)0)
@@ -257,6 +258,8 @@ static inline void dma_direct_sync_sg_for_cpu(struct device *dev,
}
#endif
+size_t dma_direct_max_mapping_size(struct device *dev);
+
#ifdef CONFIG_HAS_DMA
#include <asm/dma-mapping.h>
@@ -460,6 +463,7 @@ int dma_supported(struct device *dev, u64 mask);
int dma_set_mask(struct device *dev, u64 mask);
int dma_set_coherent_mask(struct device *dev, u64 mask);
u64 dma_get_required_mask(struct device *dev);
+size_t dma_max_mapping_size(struct device *dev);
#else /* CONFIG_HAS_DMA */
static inline dma_addr_t dma_map_page_attrs(struct device *dev,
struct page *page, size_t offset, size_t size,
@@ -561,6 +565,10 @@ static inline u64 dma_get_required_mask(struct device *dev)
{
return 0;
}
+static inline size_t dma_max_mapping_size(struct device *dev)
+{
+ return 0;
+}
#endif /* CONFIG_HAS_DMA */
static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 355d16acee6d..6310ad01f915 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -380,3 +380,14 @@ int dma_direct_supported(struct device *dev, u64 mask)
*/
return mask >= __phys_to_dma(dev, min_mask);
}
+
+size_t dma_direct_max_mapping_size(struct device *dev)
+{
+ size_t size = SIZE_MAX;
+
+ /* If SWIOTLB is active, use its maximum mapping size */
+ if (is_swiotlb_active())
+ size = swiotlb_max_mapping_size(dev);
+
+ return size;
+}
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index a11006b6d8e8..5753008ab286 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -357,3 +357,17 @@ void dma_cache_sync(struct device *dev, void *vaddr, size_t size,
ops->cache_sync(dev, vaddr, size, dir);
}
EXPORT_SYMBOL(dma_cache_sync);
+
+size_t dma_max_mapping_size(struct device *dev)
+{
+ const struct dma_map_ops *ops = get_dma_ops(dev);
+ size_t size = SIZE_MAX;
+
+ if (dma_is_direct(ops))
+ size = dma_direct_max_mapping_size(dev);
+ else if (ops && ops->max_mapping_size)
+ size = ops->max_mapping_size(dev);
+
+ return size;
+}
+EXPORT_SYMBOL_GPL(dma_max_mapping_size);
--
2.17.1
^ permalink raw reply related
* [PATCH v7 2/5] swiotlb: Add is_swiotlb_active() function
From: Joerg Roedel @ 2019-02-07 11:59 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, Konrad Rzeszutek Wilk,
Christoph Hellwig
Cc: Jens Axboe, Thomas.Lendacky, jroedel, brijesh.singh, joro,
jon.grimm, jfehlig, linux-kernel, linux-block, iommu,
virtualization
In-Reply-To: <20190207115917.20535-1-joro@8bytes.org>
From: Joerg Roedel <jroedel@suse.de>
This function will be used from dma_direct code to determine
the maximum segment size of a dma mapping.
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
include/linux/swiotlb.h | 6 ++++++
kernel/dma/swiotlb.c | 9 +++++++++
2 files changed, 15 insertions(+)
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index d3980aeed4a0..29bc3a203283 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -77,6 +77,7 @@ bool swiotlb_map(struct device *dev, phys_addr_t *phys, dma_addr_t *dma_addr,
void __init swiotlb_exit(void);
unsigned int swiotlb_max_segment(void);
size_t swiotlb_max_mapping_size(struct device *dev);
+bool is_swiotlb_active(void);
#else
#define swiotlb_force SWIOTLB_NO_FORCE
static inline bool is_swiotlb_buffer(phys_addr_t paddr)
@@ -100,6 +101,11 @@ static inline size_t swiotlb_max_mapping_size(struct device *dev)
{
return SIZE_MAX;
}
+
+static inline bool is_swiotlb_active(void)
+{
+ return false;
+}
#endif /* CONFIG_SWIOTLB */
extern void swiotlb_print_info(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 9cb21259cb0b..c873f9cc2146 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -667,3 +667,12 @@ size_t swiotlb_max_mapping_size(struct device *dev)
{
return ((size_t)1 << IO_TLB_SHIFT) * IO_TLB_SEGSIZE;
}
+
+bool is_swiotlb_active(void)
+{
+ /*
+ * When SWIOTLB is initialized, even if io_tlb_start points to physical
+ * address zero, io_tlb_end surely doesn't.
+ */
+ return io_tlb_end != 0;
+}
--
2.17.1
^ permalink raw reply related
* [PATCH v7 1/5] swiotlb: Introduce swiotlb_max_mapping_size()
From: Joerg Roedel @ 2019-02-07 11:59 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, Konrad Rzeszutek Wilk,
Christoph Hellwig
Cc: Jens Axboe, Thomas.Lendacky, jroedel, brijesh.singh, joro,
jon.grimm, jfehlig, linux-kernel, linux-block, iommu,
virtualization
In-Reply-To: <20190207115917.20535-1-joro@8bytes.org>
From: Joerg Roedel <jroedel@suse.de>
The function returns the maximum size that can be remapped
by the SWIOTLB implementation. This function will be later
exposed to users through the DMA-API.
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
include/linux/swiotlb.h | 5 +++++
kernel/dma/swiotlb.c | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 7c007ed7505f..d3980aeed4a0 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -76,6 +76,7 @@ bool swiotlb_map(struct device *dev, phys_addr_t *phys, dma_addr_t *dma_addr,
size_t size, enum dma_data_direction dir, unsigned long attrs);
void __init swiotlb_exit(void);
unsigned int swiotlb_max_segment(void);
+size_t swiotlb_max_mapping_size(struct device *dev);
#else
#define swiotlb_force SWIOTLB_NO_FORCE
static inline bool is_swiotlb_buffer(phys_addr_t paddr)
@@ -95,6 +96,10 @@ static inline unsigned int swiotlb_max_segment(void)
{
return 0;
}
+static inline size_t swiotlb_max_mapping_size(struct device *dev)
+{
+ return SIZE_MAX;
+}
#endif /* CONFIG_SWIOTLB */
extern void swiotlb_print_info(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 1fb6fd68b9c7..9cb21259cb0b 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -662,3 +662,8 @@ swiotlb_dma_supported(struct device *hwdev, u64 mask)
{
return __phys_to_dma(hwdev, io_tlb_end - 1) <= mask;
}
+
+size_t swiotlb_max_mapping_size(struct device *dev)
+{
+ return ((size_t)1 << IO_TLB_SHIFT) * IO_TLB_SEGSIZE;
+}
--
2.17.1
^ permalink raw reply related
* [PATCH v7 0/5] Fix virtio-blk issue with SWIOTLB
From: Joerg Roedel @ 2019-02-07 11:59 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, Konrad Rzeszutek Wilk,
Christoph Hellwig
Cc: Jens Axboe, Thomas.Lendacky, jroedel, brijesh.singh, joro,
jon.grimm, jfehlig, linux-kernel, linux-block, iommu,
virtualization
Hi,
here is the next version of this patch-set. Previous
versions can be found here:
V1: https://lore.kernel.org/lkml/20190110134433.15672-1-joro@8bytes.org/
V2: https://lore.kernel.org/lkml/20190115132257.6426-1-joro@8bytes.org/
V3: https://lore.kernel.org/lkml/20190123163049.24863-1-joro@8bytes.org/
V4: https://lore.kernel.org/lkml/20190129084342.26030-1-joro@8bytes.org/
V5: https://lore.kernel.org/lkml/20190130164007.26497-1-joro@8bytes.org/
V6: https://lore.kernel.org/lkml/20190131163403.11363-1-joro@8bytes.org/
The problem solved here is a limitation of the SWIOTLB implementation,
which does not support allocations larger than 256kb. When the
virtio-blk driver tries to read/write a block larger than that, the
allocation of the dma-handle fails and an IO error is reported.
Changes to v6 are:
- Fix build errors with CONFIG_SWIOTLB=n
Please review.
Thanks,
Joerg
Joerg Roedel (5):
swiotlb: Introduce swiotlb_max_mapping_size()
swiotlb: Add is_swiotlb_active() function
dma: Introduce dma_max_mapping_size()
virtio: Introduce virtio_max_dma_size()
virtio-blk: Consider virtio_max_dma_size() for maximum segment size
Documentation/DMA-API.txt | 8 ++++++++
drivers/block/virtio_blk.c | 10 ++++++----
drivers/virtio/virtio_ring.c | 11 +++++++++++
include/linux/dma-mapping.h | 8 ++++++++
include/linux/swiotlb.h | 11 +++++++++++
include/linux/virtio.h | 2 ++
kernel/dma/direct.c | 11 +++++++++++
kernel/dma/mapping.c | 14 ++++++++++++++
kernel/dma/swiotlb.c | 14 ++++++++++++++
9 files changed, 85 insertions(+), 4 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH 0/5] Clean up TTM mmap offsets
From: Koenig, Christian @ 2019-02-07 9:36 UTC (permalink / raw)
To: Thomas Zimmermann, Deucher, Alexander, Zhou, David(ChunMing),
airlied@redhat.com, kraxel@redhat.com,
z.liuxinliang@hisilicon.com, zourongrong@gmail.com,
kong.kongxinwei@hisilicon.com, puck.chen@hisilicon.com,
bskeggs@redhat.com, thellstrom@vmware.com,
linux-graphics-maintainer@vmware.com, daniel@ffwll.ch,
hdegoede@redhat.com, arnd@arndb.de, gregkh@linuxfoundation.org,
Huang, Ray, Zhang, Jerry
Cc: spice-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
dri-devel@lists.freedesktop.org,
virtualization@lists.linux-foundation.org
In-Reply-To: <20190207085931.29569-1-tzimmermann@suse.de>
Am 07.02.19 um 09:59 schrieb Thomas Zimmermann:
> Almost all TTM-based drivers use the same values for the mmap-able
> range of BO addresses. Each driver therefore duplicates the
> DRM_FILE_PAGE_OFFSET constant. OTOH, the mmap range's size is not
> configurable by drivers.
>
> This patch set replaces driver-specific configuration with a single
> setup. All code is located within TTM. TTM and GEM share the same
> range for mmap-able BOs.
>
> Thomas Zimmermann (5):
> staging/vboxvideo: Use same BO mmap offset as other drivers
> drm/ttm: Define a single DRM_FILE_PAGE_OFFSET constant
> drm/ttm: Remove file_page_offset parameter from ttm_bo_device_init()
> drm/ttm: Quick-test mmap offset in ttm_bo_mmap()
> drm: Use the same mmap-range offset and size for GEM and TTM
Reviewed-by: Christian König <christian.koenig@amd.com> for the whole
series.
Nice cleanup! Thanks,
Christian.
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 ++----------
> drivers/gpu/drm/ast/ast_drv.h | 2 --
> drivers/gpu/drm/ast/ast_ttm.c | 10 ++--------
> drivers/gpu/drm/bochs/bochs.h | 2 --
> drivers/gpu/drm/bochs/bochs_mm.c | 10 ++--------
> drivers/gpu/drm/cirrus/cirrus_drv.h | 1 -
> drivers/gpu/drm/cirrus/cirrus_ttm.c | 10 ++--------
> drivers/gpu/drm/drm_gem.c | 17 -----------------
> drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 12 ++----------
> drivers/gpu/drm/mgag200/mgag200_drv.h | 1 -
> drivers/gpu/drm/mgag200/mgag200_ttm.c | 10 ++--------
> drivers/gpu/drm/nouveau/nouveau_drv.h | 2 --
> drivers/gpu/drm/nouveau/nouveau_ttm.c | 4 ----
> drivers/gpu/drm/qxl/qxl_drv.h | 3 ---
> drivers/gpu/drm/qxl/qxl_ttm.c | 11 +++--------
> drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++-----------
> drivers/gpu/drm/ttm/ttm_bo.c | 6 +++---
> drivers/gpu/drm/ttm/ttm_bo_vm.c | 3 +++
> drivers/gpu/drm/virtio/virtgpu_ttm.c | 4 +---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 -
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 -
> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 11 ++---------
> drivers/staging/vboxvideo/vbox_drv.h | 2 --
> drivers/staging/vboxvideo/vbox_ttm.c | 12 +++---------
> include/drm/drm_vma_manager.h | 12 ++++++++++++
> include/drm/ttm/ttm_bo_driver.h | 2 +-
> 26 files changed, 42 insertions(+), 132 deletions(-)
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 0/5 v6] Fix virtio-blk issue with SWIOTLB
From: Joerg Roedel @ 2019-02-07 8:49 UTC (permalink / raw)
To: Joerg Roedel
Cc: Jens Axboe, Thomas.Lendacky, jon.grimm, brijesh.singh,
Konrad Rzeszutek Wilk, Michael S. Tsirkin, jfehlig, linux-kernel,
virtualization, linux-block, iommu, Christoph Hellwig
In-Reply-To: <20190207084613.GW32526@8bytes.org>
On Thu, Feb 07, 2019 at 09:46:13AM +0100, Joerg Roedel wrote:
> Hmm, I didn't get any kbuild emails for this series. Can you please
> forward it me so that I can look into it?
Nevermind, just found them in another inbox.
Joerg
^ permalink raw reply
* Re: [PATCH 0/5 v6] Fix virtio-blk issue with SWIOTLB
From: Joerg Roedel @ 2019-02-07 8:46 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jens Axboe, Thomas.Lendacky, jroedel, brijesh.singh,
Konrad Rzeszutek Wilk, jon.grimm, jfehlig, linux-kernel,
virtualization, linux-block, iommu, Christoph Hellwig
In-Reply-To: <20190205155133-mutt-send-email-mst@kernel.org>
Hi Michael,
On Tue, Feb 05, 2019 at 03:52:38PM -0500, Michael S. Tsirkin wrote:
> On Thu, Jan 31, 2019 at 05:33:58PM +0100, Joerg Roedel wrote:
> > Changes to v5 are:
> >
> > - Changed patch 3 to uninline dma_max_mapping_size()
>
> And this lead to problems reported by kbuild :(
Hmm, I didn't get any kbuild emails for this series. Can you please
forward it me so that I can look into it?
> BTW when you repost, can I ask you to pls include
> the version in all patches? Both --subject-prefix
> and -v flags to git format-patch will do this for you.
Will do, thanks.
Regards,
Joerg
^ permalink raw reply
* Resource management for ndo_xdp_xmit (Was: [PATCH net] virtio_net: Account for tx bytes and packets on sending xdp_frames)
From: Jesper Dangaard Brouer @ 2019-02-07 7:48 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Toke Høiland-Jørgensen, jakub.kicinski@netronome.com,
hawk@kernel.org, mst@redhat.com, netdev@vger.kernel.org,
john.fastabend@gmail.com, brouer,
virtualization@lists.linux-foundation.org,
Toke Høiland-Jørgensen, dsahern@gmail.com,
borkmann@iogearbox.net, davem@davemloft.net, Tariq Toukan
In-Reply-To: <140ecbe1e25f54f90d859cc696c4119aa96bc6eb.camel@mellanox.com>
On Wed, 6 Feb 2019 00:06:33 +0000 Saeed Mahameed <saeedm@mellanox.com> wrote:
> On Mon, 2019-02-04 at 19:13 -0800, David Ahern wrote:
[...]
> >
> > mlx5 needs some work. As I recall it still has the bug/panic
> > removing xdp programs - at least I don't recall seeing a patch for
> > it.
>
> Only when xdp_redirect to mlx5, and removing the program while
> redirect is happening, this is actually due to a lack of
> synchronization means between different drivers, we have some ideas
> to overcome this using a standard XDP API, or just use a hack in mlx5
> driver which i don't like:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git/commit/?h=topic/xdp-redirect-fix&id=a3652d03cc35fd3ad62744986c8ccaca74c9f20c
>
> I will be working on this towards the end of this week.
Toke and I have been discussing how to solve this.
The main idea for fixing this is to tie resource allocation to interface
insertion into interface maps (kernel/bpf/devmap.c). As the =devmap=
already have the needed synchronisation mechanisms and steps for safely
adding and removing =net_devices= (e.g. stopping RX side, flushing
remaining frames, waiting RCU period before freeing objects, etc.)
As described here:
https://github.com/xdp-project/xdp-project/blob/master/xdp-project.org#better-ndo_xdp_xmit-resource-management
--Jesper
^ permalink raw reply
* Re: [PATCH 3/6] mm/balloon_compaction: list interfaces
From: Michael S. Tsirkin @ 2019-02-07 1:26 UTC (permalink / raw)
To: Nadav Amit
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
Julien Freche
In-Reply-To: <0DFA5F3F-8358-4268-83C7-9937C5F0CFFF@vmware.com>
On Thu, Feb 07, 2019 at 12:43:51AM +0000, Nadav Amit wrote:
> > On Feb 6, 2019, at 4:32 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Feb 06, 2019 at 03:57:03PM -0800, Nadav Amit wrote:
> >> Introduce interfaces for ballooning enqueueing and dequeueing of a list
> >> of pages. These interfaces reduce the overhead of storing and restoring
> >> IRQs by batching the operations. In addition they do not panic if the
> >> list of pages is empty.
> >>
>
> [Snip]
>
> First, thanks for the quick feedback.
>
> >> +
> >> +/**
> >> + * balloon_page_list_enqueue() - inserts a list of pages into the balloon page
> >> + * list.
> >> + * @b_dev_info: balloon device descriptor where we will insert a new page to
> >> + * @pages: pages to enqueue - allocated using balloon_page_alloc.
> >> + *
> >> + * Driver must call it to properly enqueue a balloon pages before definitively
> >> + * removing it from the guest system.
> >> + */
> >> +void balloon_page_list_enqueue(struct balloon_dev_info *b_dev_info,
> >> + struct list_head *pages)
> >> +{
> >> + struct page *page, *tmp;
> >> + unsigned long flags;
> >> +
> >> + spin_lock_irqsave(&b_dev_info->pages_lock, flags);
> >> + list_for_each_entry_safe(page, tmp, pages, lru)
> >> + balloon_page_enqueue_one(b_dev_info, page);
> >> + spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
> >
> > As this is scanning pages one by one anyway, it will be useful
> > to have this return the # of pages enqueued.
>
> Sure.
>
> >
> >> +}
> >> +EXPORT_SYMBOL_GPL(balloon_page_list_enqueue);
> >> +
> >> +/**
> >> + * balloon_page_list_dequeue() - removes pages from balloon's page list and
> >> + * returns a list of the pages.
> >> + * @b_dev_info: balloon device decriptor where we will grab a page from.
> >> + * @pages: pointer to the list of pages that would be returned to the caller.
> >> + * @n_req_pages: number of requested pages.
> >> + *
> >> + * Driver must call it to properly de-allocate a previous enlisted balloon pages
> >> + * before definetively releasing it back to the guest system. This function
> >> + * tries to remove @n_req_pages from the ballooned pages and return it to the
> >> + * caller in the @pages list.
> >> + *
> >> + * Note that this function may fail to dequeue some pages temporarily empty due
> >> + * to compaction isolated pages.
> >> + *
> >> + * Return: number of pages that were added to the @pages list.
> >> + */
> >> +int balloon_page_list_dequeue(struct balloon_dev_info *b_dev_info,
> >> + struct list_head *pages, int n_req_pages)
> >
> > Are we sure this int never overflows? Why not just use u64
> > or size_t straight away?
>
> size_t it is.
>
> >
> >> +{
> >> + struct page *page, *tmp;
> >> + unsigned long flags;
> >> + int n_pages = 0;
> >> +
> >> + spin_lock_irqsave(&b_dev_info->pages_lock, flags);
> >> + list_for_each_entry_safe(page, tmp, &b_dev_info->pages, lru) {
> >> + /*
> >> + * Block others from accessing the 'page' while we get around
> >> + * establishing additional references and preparing the 'page'
> >> + * to be released by the balloon driver.
> >> + */
> >> + if (!trylock_page(page))
> >> + continue;
> >> +
> >> + if (IS_ENABLED(CONFIG_BALLOON_COMPACTION) &&
> >> + PageIsolated(page)) {
> >> + /* raced with isolation */
> >> + unlock_page(page);
> >> + continue;
> >> + }
> >> + balloon_page_delete(page);
> >> + __count_vm_event(BALLOON_DEFLATE);
> >> + unlock_page(page);
> >> + list_add(&page->lru, pages);
> >> + if (++n_pages >= n_req_pages)
> >> + break;
> >> + }
> >> + spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
> >> +
> >> + return n_pages;
> >> +}
> >> +EXPORT_SYMBOL_GPL(balloon_page_list_dequeue);
> >> +
> >
> > This looks quite reasonable. In fact virtio can be reworked to use
> > this too and then the original one can be dropped.
> >
> > Have the time?
>
> Obviously not, but I am willing to make the time. What I cannot “make" is an
> approval to send patches for other hypervisors. Let me run a quick check
> with our FOSS people here.
>
> Anyhow, I hope it would not prevent the patches from getting to the next
> release.
>
No, that's not a blocker.
--
MST
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 3/6] mm/balloon_compaction: list interfaces
From: Michael S. Tsirkin @ 2019-02-07 0:32 UTC (permalink / raw)
To: Nadav Amit
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, virtualization,
linux-mm, Julien Freche
In-Reply-To: <20190206235706.4851-4-namit@vmware.com>
On Wed, Feb 06, 2019 at 03:57:03PM -0800, Nadav Amit wrote:
> Introduce interfaces for ballooning enqueueing and dequeueing of a list
> of pages. These interfaces reduce the overhead of storing and restoring
> IRQs by batching the operations. In addition they do not panic if the
> list of pages is empty.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: linux-mm@kvack.org
> Cc: virtualization@lists.linux-foundation.org
> Reviewed-by: Xavier Deguillard <xdeguillard@vmware.com>
> Signed-off-by: Nadav Amit <namit@vmware.com>
> ---
> include/linux/balloon_compaction.h | 4 +
> mm/balloon_compaction.c | 139 +++++++++++++++++++++--------
> 2 files changed, 105 insertions(+), 38 deletions(-)
>
> diff --git a/include/linux/balloon_compaction.h b/include/linux/balloon_compaction.h
> index 53051f3d8f25..2c5a8e09e413 100644
> --- a/include/linux/balloon_compaction.h
> +++ b/include/linux/balloon_compaction.h
> @@ -72,6 +72,10 @@ extern struct page *balloon_page_alloc(void);
> extern void balloon_page_enqueue(struct balloon_dev_info *b_dev_info,
> struct page *page);
> extern struct page *balloon_page_dequeue(struct balloon_dev_info *b_dev_info);
> +extern void balloon_page_list_enqueue(struct balloon_dev_info *b_dev_info,
> + struct list_head *pages);
> +extern int balloon_page_list_dequeue(struct balloon_dev_info *b_dev_info,
> + struct list_head *pages, int n_req_pages);
>
> static inline void balloon_devinfo_init(struct balloon_dev_info *balloon)
> {
> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
> index ef858d547e2d..b8e82864f82c 100644
> --- a/mm/balloon_compaction.c
> +++ b/mm/balloon_compaction.c
> @@ -10,6 +10,100 @@
> #include <linux/export.h>
> #include <linux/balloon_compaction.h>
>
> +static int balloon_page_enqueue_one(struct balloon_dev_info *b_dev_info,
> + struct page *page)
> +{
> + /*
> + * Block others from accessing the 'page' when we get around to
> + * establishing additional references. We should be the only one
> + * holding a reference to the 'page' at this point.
> + */
> + if (!trylock_page(page)) {
> + WARN_ONCE(1, "balloon inflation failed to enqueue page\n");
> + return -EFAULT;
> + }
> + list_del(&page->lru);
> + balloon_page_insert(b_dev_info, page);
> + unlock_page(page);
> + __count_vm_event(BALLOON_INFLATE);
> + return 0;
> +}
> +
> +/**
> + * balloon_page_list_enqueue() - inserts a list of pages into the balloon page
> + * list.
> + * @b_dev_info: balloon device descriptor where we will insert a new page to
> + * @pages: pages to enqueue - allocated using balloon_page_alloc.
> + *
> + * Driver must call it to properly enqueue a balloon pages before definitively
> + * removing it from the guest system.
> + */
> +void balloon_page_list_enqueue(struct balloon_dev_info *b_dev_info,
> + struct list_head *pages)
> +{
> + struct page *page, *tmp;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&b_dev_info->pages_lock, flags);
> + list_for_each_entry_safe(page, tmp, pages, lru)
> + balloon_page_enqueue_one(b_dev_info, page);
> + spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
As this is scanning pages one by one anyway, it will be useful
to have this return the # of pages enqueued.
> +}
> +EXPORT_SYMBOL_GPL(balloon_page_list_enqueue);
> +
> +/**
> + * balloon_page_list_dequeue() - removes pages from balloon's page list and
> + * returns a list of the pages.
> + * @b_dev_info: balloon device decriptor where we will grab a page from.
> + * @pages: pointer to the list of pages that would be returned to the caller.
> + * @n_req_pages: number of requested pages.
> + *
> + * Driver must call it to properly de-allocate a previous enlisted balloon pages
> + * before definetively releasing it back to the guest system. This function
> + * tries to remove @n_req_pages from the ballooned pages and return it to the
> + * caller in the @pages list.
> + *
> + * Note that this function may fail to dequeue some pages temporarily empty due
> + * to compaction isolated pages.
> + *
> + * Return: number of pages that were added to the @pages list.
> + */
> +int balloon_page_list_dequeue(struct balloon_dev_info *b_dev_info,
> + struct list_head *pages, int n_req_pages)
Are we sure this int never overflows? Why not just use u64
or size_t straight away?
> +{
> + struct page *page, *tmp;
> + unsigned long flags;
> + int n_pages = 0;
> +
> + spin_lock_irqsave(&b_dev_info->pages_lock, flags);
> + list_for_each_entry_safe(page, tmp, &b_dev_info->pages, lru) {
> + /*
> + * Block others from accessing the 'page' while we get around
> + * establishing additional references and preparing the 'page'
> + * to be released by the balloon driver.
> + */
> + if (!trylock_page(page))
> + continue;
> +
> + if (IS_ENABLED(CONFIG_BALLOON_COMPACTION) &&
> + PageIsolated(page)) {
> + /* raced with isolation */
> + unlock_page(page);
> + continue;
> + }
> + balloon_page_delete(page);
> + __count_vm_event(BALLOON_DEFLATE);
> + unlock_page(page);
> + list_add(&page->lru, pages);
> + if (++n_pages >= n_req_pages)
> + break;
> + }
> + spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
> +
> + return n_pages;
> +}
> +EXPORT_SYMBOL_GPL(balloon_page_list_dequeue);
> +
This looks quite reasonable. In fact virtio can be reworked to use
this too and then the original one can be dropped.
Have the time?
> /*
> * balloon_page_alloc - allocates a new page for insertion into the balloon
> * page list.
> @@ -43,17 +137,9 @@ void balloon_page_enqueue(struct balloon_dev_info *b_dev_info,
> {
> unsigned long flags;
>
> - /*
> - * Block others from accessing the 'page' when we get around to
> - * establishing additional references. We should be the only one
> - * holding a reference to the 'page' at this point.
> - */
> - BUG_ON(!trylock_page(page));
> spin_lock_irqsave(&b_dev_info->pages_lock, flags);
> - balloon_page_insert(b_dev_info, page);
> - __count_vm_event(BALLOON_INFLATE);
> + balloon_page_enqueue_one(b_dev_info, page);
> spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
> - unlock_page(page);
> }
> EXPORT_SYMBOL_GPL(balloon_page_enqueue);
>
> @@ -70,36 +156,13 @@ EXPORT_SYMBOL_GPL(balloon_page_enqueue);
> */
> struct page *balloon_page_dequeue(struct balloon_dev_info *b_dev_info)
> {
> - struct page *page, *tmp;
> unsigned long flags;
> - bool dequeued_page;
> + LIST_HEAD(pages);
> + int n_pages;
>
> - dequeued_page = false;
> - spin_lock_irqsave(&b_dev_info->pages_lock, flags);
> - list_for_each_entry_safe(page, tmp, &b_dev_info->pages, lru) {
> - /*
> - * Block others from accessing the 'page' while we get around
> - * establishing additional references and preparing the 'page'
> - * to be released by the balloon driver.
> - */
> - if (trylock_page(page)) {
> -#ifdef CONFIG_BALLOON_COMPACTION
> - if (PageIsolated(page)) {
> - /* raced with isolation */
> - unlock_page(page);
> - continue;
> - }
> -#endif
> - balloon_page_delete(page);
> - __count_vm_event(BALLOON_DEFLATE);
> - unlock_page(page);
> - dequeued_page = true;
> - break;
> - }
> - }
> - spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
> + n_pages = balloon_page_list_dequeue(b_dev_info, &pages, 1);
>
> - if (!dequeued_page) {
> + if (n_pages != 1) {
> /*
> * If we are unable to dequeue a balloon page because the page
> * list is empty and there is no isolated pages, then something
> @@ -112,9 +175,9 @@ struct page *balloon_page_dequeue(struct balloon_dev_info *b_dev_info)
> !b_dev_info->isolated_pages))
> BUG();
> spin_unlock_irqrestore(&b_dev_info->pages_lock, flags);
> - page = NULL;
> + return NULL;
> }
> - return page;
> + return list_first_entry(&pages, struct page, lru);
> }
> EXPORT_SYMBOL_GPL(balloon_page_dequeue);
>
> --
> 2.17.1
^ permalink raw reply
* [PULL] virtio: fixes
From: Michael S. Tsirkin @ 2019-02-06 19:32 UTC (permalink / raw)
To: Linus Torvalds; +Cc: kvm, mst, netdev, linux-kernel, virtualization
The following changes since commit 49a57857aeea06ca831043acbb0fa5e0f50602fd:
Linux 5.0-rc3 (2019-01-21 13:14:44 +1300)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
for you to fetch changes up to 9c0644ee4aa8792f1e60a2b014b4710faaddafeb:
virtio: drop internal struct from UAPI (2019-02-05 15:29:48 -0500)
----------------------------------------------------------------
virtio: fixes
A small fix for a uapi header, and a fix for VDPA for non-x86 guests.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
----------------------------------------------------------------
Michael S. Tsirkin (1):
virtio: drop internal struct from UAPI
Tiwei Bie (1):
virtio: support VIRTIO_F_ORDER_PLATFORM
drivers/virtio/virtio_ring.c | 15 ++++++++++++++-
include/uapi/linux/virtio_config.h | 6 ++++++
include/uapi/linux/virtio_ring.h | 10 ----------
3 files changed, 20 insertions(+), 11 deletions(-)
^ permalink raw reply
* Re: security implications of caching with virtio pmem (was Re: [PATCH v3 0/5] kvm "virtio pmem" device)
From: Michael S. Tsirkin @ 2019-02-06 18:01 UTC (permalink / raw)
To: David Hildenbrand
Cc: Pankaj Gupta, jack, kvm, linux-nvdimm, qemu-devel, virtualization,
adilger.kernel, zwisler, eblake, Andrea Arcangeli, dave.jiang,
darrick.wong, vishal.l.verma, willy, hch, linux-acpi, jmoyer,
nilal, riel, stefanha, pbonzini, dan.j.williams, lcapitulino,
linux-ext4, tytso, xiaoguangrong.eric, rjw, linux-kernel,
linux-xfs, linux-fsdevel, imammedo
In-Reply-To: <5b8a06a7-be44-698f-f319-6b2cbcf1eb8a@redhat.com>
On Wed, Feb 06, 2019 at 03:00:26PM +0100, David Hildenbrand wrote:
> On 04.02.19 23:56, Michael S. Tsirkin wrote:
> >
> > On Wed, Jan 09, 2019 at 08:17:31PM +0530, Pankaj Gupta wrote:
> >> This patch series has implementation for "virtio pmem".
> >> "virtio pmem" is fake persistent memory(nvdimm) in guest
> >> which allows to bypass the guest page cache. This also
> >> implements a VIRTIO based asynchronous flush mechanism.
> >
> >
> > At Pankaj's request I looked at information leak implications of virtio
> > pmem in light of the recent page cache side channels paper
> > (https://arxiv.org/pdf/1901.01161.pdf) - to see what
> > kind of side channels it might create if any. TLDR - I think that
> > depending on the host side implementation there could be some, but this
> > might be addressable by better documentation in both code and spec.
> > The fake dax approach backing the guest memory by a host page cache
> > does seem to have potential issues.
> >
> > For clarity: we are talking about leaking information either to a VM, or
> > within a VM (I did not look into leaks to hypervisor in configurations
> > such as SEV) through host page cache.
> >
> > Leaks into a VM: It seems clear that while pmem allows memory accesses
> > versus read/write with e.g. a block device, from host page cache point
> > of view this doesn't matter much: reads populate cache in the same way
> > as memory faults. Thus ignoring presence of information leaks (which is
> > an interesting question e.g. in light of recent discard support) pmem
> > doesn't seem to be any better or worse for leaking information into a
> > VM.
>
> +1, just a different way to access that cache.
>
> Conceptually a virtio-pmem devices is from the guest view a "device with
> a managed buffer". Some accesses might be faster than others. There are
> no guarantees on how fast a certain access is. And yes, actions on other
> guests can result in accesses being slower but not faster.
>
> Also other storage devices have caches like that (well, the caches size
> depends on the device) - thinking especially about storage systems -
> which would in my opinion, also allow similar leaks. How are such
> security concerns handled there? Are they different (besides eventually
> access speed)?
>
> >
> > Leaks within VM: Right now pmem seems to bypass the guest page cache
> > completely. Whether pmem memory is then resident in a page cache would
> > be up to the device/host. Assuming that it is, the "Preventing
> > Efficient Eviction while Increasing the System Performance"
> > countermeasure for the page cache side channel attack would appear to
> > become ineffective with pmem. What is suggested is a per-process
> > management of the page cache, and host does not have visibility of
> > processes within a VM. Another possible countermeasure - not discussed
> > in the paper - could be modify the applications to lock the security
> > relevant pages in memory. Again this becomes impractical with pmem as
> > host does not have visibility into that. However note that as long
> > as the only countermeasure linux uses is "Privileged Access"
> > (i.e. blocking mincore) nothing can be done as guest page cache
> > remains as vulnerable as host page cache.
>
> This sounds very use-case specific. If I run a VM only with a very
> specific workload (say, a container running one application), I usually
> don't care about leaks within the VM. At least not leaks between
> applications ;)
>
> In contrast, to running different applications (e.g. containers from
> different customers) on one system, I really care about leaks within a VM.
Clearly, not everyone cares about closing off information leaks.
> >
> >
> > Countermeasures: which host-side countermeasures can be designed would
> > depend on which countermeasures are used guest-side - we would need to
> > make sure they are not broken by pmem. For "Preventing Efficient
> > Eviction while Increasing the System Performance" modifying the host
> > implementation to ensure that pmem device bypasses the host page cache
> > would seem to address the security problem.Similarly, ensuring that a
> > real memory device (e.g. DAX, RAM such as hugetlbfs, pmem for nested
> > virt) is used for pmem would make the memory locking countermeasure
> > work. Whether with such limitations the device is still useful
> > performance wise is an open question. These questions probably should
> > be addressed in the documentation, spec and possible qemu code.
> >
> I also want to note that using a disk/file as memory backend with
> NVDIMMs in QEMU essentially results in the exact same questions we have
> with virtio-pmem.
>
> E.g. kata-containers use nvdimms for the rootfile system (read-only) as
> far as I am aware.
>
> Conceptually, a virtio-pmem device is just an emulated nvdimm device
> with a flush interface. And the nice thing is, that it is designed to
> also work on architectures that don't speak "nvdimm".
>
> >
> > Severity of the security implications: some people argue that the
> > security implications of the page cache leaks are minor. I do not have
> > an opinion on this: the severity would seem to depend on the specific
> > configuration.
>
> I guess configuration and use case.
Good point.
> Nice summary, thanks for looking into this Michael!
>
>
> --
>
> Thanks,
>
> David / dhildenb
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 0/6] vmw_balloon: 64-bit limit support, compaction, shrinker
From: Michael S. Tsirkin @ 2019-02-06 17:52 UTC (permalink / raw)
To: Nadav Amit
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, virtualization,
linux-mm, Julien Freche
In-Reply-To: <20190206051336.2425-1-namit@vmware.com>
On Tue, Feb 05, 2019 at 09:13:30PM -0800, Nadav Amit wrote:
> Various enhancements for VMware balloon, some of which are remainder
> from a previous patch-set.
>
> Patch 1: Aumps the version number, following recent changes
> Patch 2: Adds support for 64-bit memory limit
> Patches 3-4: Support for compaction
> Patch 5: Support for memory shrinker - disabled by default
> Patch 6: Split refused pages to improve performance
>
> Since the 3rd patch requires Michael Tsirkin ack, which has not arrived
> in the last couple of times the patch was sent, please consider applying
> patches 1-2 for 5.1.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: linux-mm@kvack.org
> Cc: virtualization@lists.linux-foundation.org
I don't seem to have got anything except patch 0 either directly
or through virtualization@lists.linux-foundation.org
Could you bounce the relevant patches there?
Thanks!
> Nadav Amit (5):
> vmw_balloon: bump version number
> mm/balloon_compaction: list interfaces
> vmw_balloon: compaction support
> vmw_balloon: add memory shrinker
> vmw_balloon: split refused pages
>
> Xavier Deguillard (1):
> vmw_balloon: support 64-bit memory limit
>
> drivers/misc/Kconfig | 1 +
> drivers/misc/vmw_balloon.c | 511 ++++++++++++++++++++++++++---
> include/linux/balloon_compaction.h | 4 +
> mm/balloon_compaction.c | 139 +++++---
> 4 files changed, 566 insertions(+), 89 deletions(-)
>
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH net] virtio_net: Account for tx bytes and packets on sending xdp_frames
From: Jakub Kicinski @ 2019-02-06 15:52 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: mst@redhat.com, netdev@vger.kernel.org, john.fastabend@gmail.com,
virtualization@lists.linux-foundation.org, Saeed Mahameed,
thoiland@redhat.com, dsahern@gmail.com, borkmann@iogearbox.net,
davem@davemloft.net, Tariq Toukan
In-Reply-To: <20190206144814.46996933@carbon>
On Wed, 6 Feb 2019 14:48:14 +0100, Jesper Dangaard Brouer wrote:
> On Wed, 6 Feb 2019 00:06:33 +0000
> Saeed Mahameed <saeedm@mellanox.com> wrote:
>
> > 3) Unrelated, In non XDP case, if skb allocation fails or driver fails
> > to pass the skb up to the stack for somereason, should the driver
> > increase rx packets ? IMHO the answer should be yes if we want to have
> > similar behavior between XDP and non XDP cases.
>
> I don't think "skb allocation fails" should increase rx packets
> counter. The difference is that these events are outside sysadm/users
> control, and is an error detected inside the driver. The XDP program
> takes a policy choice to XDP_DROP a packet, which can be accounted
> inside the XDP prog (as the samples show) or as we also discuss via a
> more generic XDP-action counters.
FWIW that's my understanding as well. My understanding of Linux stats
is that they are incrementing one counter per packet. I.e. in RX
direction success packets are those given to the stack, and for TX
those given to the hardware. Standards (IETF/IEEE) usually count stats
on the same layer boundary, but I think software generally counts when
it's done with the packet.
I haven't seen it documented anywhere, yet. I have tried to document
it in the docs of the recent RFC:
https://patchwork.ozlabs.org/patch/1032332/
Incidentally XDP_DROP may have been better named XDP_DISCARD from stats
perspective ;)
^ permalink raw reply
* Re: security implications of caching with virtio pmem (was Re: [PATCH v3 0/5] kvm "virtio pmem" device)
From: David Hildenbrand @ 2019-02-06 14:00 UTC (permalink / raw)
To: Michael S. Tsirkin, Pankaj Gupta
Cc: jack, kvm, linux-nvdimm, qemu-devel, virtualization,
adilger.kernel, zwisler, eblake, Andrea Arcangeli, dave.jiang,
darrick.wong, vishal.l.verma, willy, hch, linux-acpi, jmoyer,
nilal, riel, stefanha, imammedo, dan.j.williams, lcapitulino,
linux-ext4, tytso, xiaoguangrong.eric, rjw, linux-kernel,
linux-xfs, linux-fsdevel, pbonzini
In-Reply-To: <20190204170515-mutt-send-email-mst@kernel.org>
On 04.02.19 23:56, Michael S. Tsirkin wrote:
>
> On Wed, Jan 09, 2019 at 08:17:31PM +0530, Pankaj Gupta wrote:
>> This patch series has implementation for "virtio pmem".
>> "virtio pmem" is fake persistent memory(nvdimm) in guest
>> which allows to bypass the guest page cache. This also
>> implements a VIRTIO based asynchronous flush mechanism.
>
>
> At Pankaj's request I looked at information leak implications of virtio
> pmem in light of the recent page cache side channels paper
> (https://arxiv.org/pdf/1901.01161.pdf) - to see what
> kind of side channels it might create if any. TLDR - I think that
> depending on the host side implementation there could be some, but this
> might be addressable by better documentation in both code and spec.
> The fake dax approach backing the guest memory by a host page cache
> does seem to have potential issues.
>
> For clarity: we are talking about leaking information either to a VM, or
> within a VM (I did not look into leaks to hypervisor in configurations
> such as SEV) through host page cache.
>
> Leaks into a VM: It seems clear that while pmem allows memory accesses
> versus read/write with e.g. a block device, from host page cache point
> of view this doesn't matter much: reads populate cache in the same way
> as memory faults. Thus ignoring presence of information leaks (which is
> an interesting question e.g. in light of recent discard support) pmem
> doesn't seem to be any better or worse for leaking information into a
> VM.
+1, just a different way to access that cache.
Conceptually a virtio-pmem devices is from the guest view a "device with
a managed buffer". Some accesses might be faster than others. There are
no guarantees on how fast a certain access is. And yes, actions on other
guests can result in accesses being slower but not faster.
Also other storage devices have caches like that (well, the caches size
depends on the device) - thinking especially about storage systems -
which would in my opinion, also allow similar leaks. How are such
security concerns handled there? Are they different (besides eventually
access speed)?
>
> Leaks within VM: Right now pmem seems to bypass the guest page cache
> completely. Whether pmem memory is then resident in a page cache would
> be up to the device/host. Assuming that it is, the "Preventing
> Efficient Eviction while Increasing the System Performance"
> countermeasure for the page cache side channel attack would appear to
> become ineffective with pmem. What is suggested is a per-process
> management of the page cache, and host does not have visibility of
> processes within a VM. Another possible countermeasure - not discussed
> in the paper - could be modify the applications to lock the security
> relevant pages in memory. Again this becomes impractical with pmem as
> host does not have visibility into that. However note that as long
> as the only countermeasure linux uses is "Privileged Access"
> (i.e. blocking mincore) nothing can be done as guest page cache
> remains as vulnerable as host page cache.
This sounds very use-case specific. If I run a VM only with a very
specific workload (say, a container running one application), I usually
don't care about leaks within the VM. At least not leaks between
applications ;)
In contrast, to running different applications (e.g. containers from
different customers) on one system, I really care about leaks within a VM.
>
>
> Countermeasures: which host-side countermeasures can be designed would
> depend on which countermeasures are used guest-side - we would need to
> make sure they are not broken by pmem. For "Preventing Efficient
> Eviction while Increasing the System Performance" modifying the host
> implementation to ensure that pmem device bypasses the host page cache
> would seem to address the security problem.Similarly, ensuring that a
> real memory device (e.g. DAX, RAM such as hugetlbfs, pmem for nested
> virt) is used for pmem would make the memory locking countermeasure
> work. Whether with such limitations the device is still useful
> performance wise is an open question. These questions probably should
> be addressed in the documentation, spec and possible qemu code.
>
I also want to note that using a disk/file as memory backend with
NVDIMMs in QEMU essentially results in the exact same questions we have
with virtio-pmem.
E.g. kata-containers use nvdimms for the rootfile system (read-only) as
far as I am aware.
Conceptually, a virtio-pmem device is just an emulated nvdimm device
with a flush interface. And the nice thing is, that it is designed to
also work on architectures that don't speak "nvdimm".
>
> Severity of the security implications: some people argue that the
> security implications of the page cache leaks are minor. I do not have
> an opinion on this: the severity would seem to depend on the specific
> configuration.
I guess configuration and use case.
Nice summary, thanks for looking into this Michael!
--
Thanks,
David / dhildenb
^ 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