* [PATCH v3 0/5] kvm "virtio pmem" device
From: Pankaj Gupta @ 2019-01-09 14:47 UTC (permalink / raw)
To: linux-kernel, kvm, qemu-devel, linux-nvdimm, linux-fsdevel,
virtualization, linux-acpi, linux-ext4, linux-xfs
Cc: pagupta, jack, lcapitulino, adilger.kernel, zwisler, eblake,
dave.jiang, darrick.wong, vishal.l.verma, mst, willy, hch, jmoyer,
nilal, riel, stefanha, imammedo, dan.j.williams, tytso,
xiaoguangrong.eric, rjw, pbonzini
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.
Sharing guest kernel driver in this patchset with the
changes suggested in v2. Tested with Qemu side device
emulation for virtio-pmem [6].
Details of project idea for 'virtio pmem' flushing interface
is shared [3] & [4].
Implementation is divided into two parts:
New virtio pmem guest driver and qemu code changes for new
virtio pmem paravirtualized device.
1. Guest virtio-pmem kernel driver
---------------------------------
- Reads persistent memory range from paravirt device and
registers with 'nvdimm_bus'.
- 'nvdimm/pmem' driver uses this information to allocate
persistent memory region and setup filesystem operations
to the allocated memory.
- virtio pmem driver implements asynchronous flushing
interface to flush from guest to host.
2. Qemu virtio-pmem device
---------------------------------
- Creates virtio pmem device and exposes a memory range to
KVM guest.
- At host side this is file backed memory which acts as
persistent memory.
- Qemu side flush uses aio thread pool API's and virtio
for asynchronous guest multi request handling.
David Hildenbrand CCed also posted a modified version[6] of
qemu virtio-pmem code based on updated Qemu memory device API.
Virtio-pmem errors handling:
----------------------------------------
Checked behaviour of virtio-pmem for below types of errors
Need suggestions on expected behaviour for handling these errors?
- Hardware Errors: Uncorrectable recoverable Errors:
a] virtio-pmem:
- As per current logic if error page belongs to Qemu process,
host MCE handler isolates(hwpoison) that page and send SIGBUS.
Qemu SIGBUS handler injects exception to KVM guest.
- KVM guest then isolates the page and send SIGBUS to guest
userspace process which has mapped the page.
b] Existing implementation for ACPI pmem driver:
- Handles such errors with MCE notifier and creates a list
of bad blocks. Read/direct access DAX operation return EIO
if accessed memory page fall in bad block list.
- It also starts backgound scrubbing.
- Similar functionality can be reused in virtio-pmem with MCE
notifier but without scrubbing(no ACPI/ARS)? Need inputs to
confirm if this behaviour is ok or needs any change?
Changes from PATCH v2: [1]
- Disable MAP_SYNC for ext4 & XFS filesystems - [Dan]
- Use name 'virtio pmem' in place of 'fake dax'
Changes from PATCH v1: [2]
- 0-day build test for build dependency on libnvdimm
Changes suggested by - [Dan Williams]
- Split the driver into two parts virtio & pmem
- Move queuing of async block request to block layer
- Add "sync" parameter in nvdimm_flush function
- Use indirect call for nvdimm_flush
- Don’t move declarations to common global header e.g nd.h
- nvdimm_flush() return 0 or -EIO if it fails
- Teach nsio_rw_bytes() that the flush can fail
- Rename nvdimm_flush() to generic_nvdimm_flush()
- Use 'nd_region->provider_data' for long dereferencing
- Remove virtio_pmem_freeze/restore functions
- Remove BSD license text with SPDX license text
- Add might_sleep() in virtio_pmem_flush - [Luiz]
- Make spin_lock_irqsave() narrow
Changes from RFC v3
- Rebase to latest upstream - Luiz
- Call ndregion->flush in place of nvdimm_flush- Luiz
- kmalloc return check - Luiz
- virtqueue full handling - Stefan
- Don't map entire virtio_pmem_req to device - Stefan
- request leak, correct sizeof req- Stefan
- Move declaration to virtio_pmem.c
Changes from RFC v2:
- Add flush function in the nd_region in place of switching
on a flag - Dan & Stefan
- Add flush completion function with proper locking and wait
for host side flush completion - Stefan & Dan
- Keep userspace API in uapi header file - Stefan, MST
- Use LE fields & New device id - MST
- Indentation & spacing suggestions - MST & Eric
- Remove extra header files & add licensing - Stefan
Changes from RFC v1:
- Reuse existing 'pmem' code for registering persistent
memory and other operations instead of creating an entirely
new block driver.
- Use VIRTIO driver to register memory information with
nvdimm_bus and create region_type accordingly.
- Call VIRTIO flush from existing pmem driver.
Pankaj Gupta (5):
libnvdimm: nd_region flush callback support
virtio-pmem: Add virtio-pmem guest driver
libnvdimm: add nd_region buffered dax_dev flag
ext4: disable map_sync for virtio pmem
xfs: disable map_sync for virtio pmem
[2] https://lkml.org/lkml/2018/8/31/407
[3] https://www.spinics.net/lists/kvm/msg149761.html
[4] https://www.spinics.net/lists/kvm/msg153095.html
[5] https://lkml.org/lkml/2018/8/31/413
[6] https://marc.info/?l=qemu-devel&m=153555721901824&w=2
drivers/acpi/nfit/core.c | 4 -
drivers/dax/super.c | 17 +++++
drivers/nvdimm/claim.c | 6 +
drivers/nvdimm/nd.h | 1
drivers/nvdimm/pmem.c | 15 +++-
drivers/nvdimm/region_devs.c | 45 +++++++++++++-
drivers/nvdimm/virtio_pmem.c | 84 ++++++++++++++++++++++++++
drivers/virtio/Kconfig | 10 +++
drivers/virtio/Makefile | 1
drivers/virtio/pmem.c | 125 +++++++++++++++++++++++++++++++++++++++
fs/ext4/file.c | 11 +++
fs/xfs/xfs_file.c | 8 ++
include/linux/dax.h | 9 ++
include/linux/libnvdimm.h | 11 +++
include/linux/virtio_pmem.h | 60 ++++++++++++++++++
include/uapi/linux/virtio_ids.h | 1
include/uapi/linux/virtio_pmem.h | 10 +++
17 files changed, 406 insertions(+), 12 deletions(-)
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [PATCH v3 1/5] libnvdimm: nd_region flush callback support
From: Pankaj Gupta @ 2019-01-09 14:47 UTC (permalink / raw)
To: linux-kernel, kvm, qemu-devel, linux-nvdimm, linux-fsdevel,
virtualization, linux-acpi, linux-ext4, linux-xfs
Cc: pagupta, jack, lcapitulino, adilger.kernel, zwisler, eblake,
dave.jiang, darrick.wong, vishal.l.verma, mst, willy, hch, jmoyer,
nilal, riel, stefanha, imammedo, dan.j.williams, tytso,
xiaoguangrong.eric, rjw, pbonzini
In-Reply-To: <20190109144736.17452-1-pagupta@redhat.com>
This patch adds functionality to perform flush from guest
to host over VIRTIO. We are registering a callback based
on 'nd_region' type. virtio_pmem driver requires this special
flush function. For rest of the region types we are registering
existing flush function. Report error returned by host fsync
failure to userspace.
This also handles asynchronous flush requests from the block layer
by creating a child bio and chaining it with parent bio.
Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
---
drivers/acpi/nfit/core.c | 4 ++--
drivers/nvdimm/claim.c | 6 ++++--
drivers/nvdimm/nd.h | 1 +
drivers/nvdimm/pmem.c | 12 ++++++++----
drivers/nvdimm/region_devs.c | 38 ++++++++++++++++++++++++++++++++++++--
include/linux/libnvdimm.h | 5 ++++-
6 files changed, 55 insertions(+), 11 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index b072cfc..f154852 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2234,7 +2234,7 @@ static void write_blk_ctl(struct nfit_blk *nfit_blk, unsigned int bw,
offset = to_interleave_offset(offset, mmio);
writeq(cmd, mmio->addr.base + offset);
- nvdimm_flush(nfit_blk->nd_region);
+ nvdimm_flush(nfit_blk->nd_region, NULL, false);
if (nfit_blk->dimm_flags & NFIT_BLK_DCR_LATCH)
readq(mmio->addr.base + offset);
@@ -2283,7 +2283,7 @@ static int acpi_nfit_blk_single_io(struct nfit_blk *nfit_blk,
}
if (rw)
- nvdimm_flush(nfit_blk->nd_region);
+ nvdimm_flush(nfit_blk->nd_region, NULL, false);
rc = read_blk_stat(nfit_blk, lane) ? -EIO : 0;
return rc;
diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c
index fb667bf..a1dfa06 100644
--- a/drivers/nvdimm/claim.c
+++ b/drivers/nvdimm/claim.c
@@ -263,7 +263,7 @@ static int nsio_rw_bytes(struct nd_namespace_common *ndns,
struct nd_namespace_io *nsio = to_nd_namespace_io(&ndns->dev);
unsigned int sz_align = ALIGN(size + (offset & (512 - 1)), 512);
sector_t sector = offset >> 9;
- int rc = 0;
+ int rc = 0, ret = 0;
if (unlikely(!size))
return 0;
@@ -301,7 +301,9 @@ static int nsio_rw_bytes(struct nd_namespace_common *ndns,
}
memcpy_flushcache(nsio->addr + offset, buf, size);
- nvdimm_flush(to_nd_region(ndns->dev.parent));
+ ret = nvdimm_flush(to_nd_region(ndns->dev.parent), NULL, false);
+ if (ret)
+ rc = ret;
return rc;
}
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index 98317e7..d53a2d1 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -160,6 +160,7 @@ struct nd_region {
struct nd_interleave_set *nd_set;
struct nd_percpu_lane __percpu *lane;
struct nd_mapping mapping[0];
+ int (*flush)(struct nd_region *nd_region);
};
struct nd_blk_region {
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 6071e29..5d6a4a1 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -192,6 +192,7 @@ static blk_status_t pmem_do_bvec(struct pmem_device *pmem, struct page *page,
static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
{
+ int ret = 0;
blk_status_t rc = 0;
bool do_acct;
unsigned long start;
@@ -201,7 +202,7 @@ static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
struct nd_region *nd_region = to_region(pmem);
if (bio->bi_opf & REQ_PREFLUSH)
- nvdimm_flush(nd_region);
+ ret = nvdimm_flush(nd_region, bio, true);
do_acct = nd_iostat_start(bio, &start);
bio_for_each_segment(bvec, bio, iter) {
@@ -216,7 +217,10 @@ static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
nd_iostat_end(bio, start);
if (bio->bi_opf & REQ_FUA)
- nvdimm_flush(nd_region);
+ ret = nvdimm_flush(nd_region, bio, true);
+
+ if (ret)
+ bio->bi_status = errno_to_blk_status(ret);
bio_endio(bio);
return BLK_QC_T_NONE;
@@ -528,14 +532,14 @@ static int nd_pmem_remove(struct device *dev)
sysfs_put(pmem->bb_state);
pmem->bb_state = NULL;
}
- nvdimm_flush(to_nd_region(dev->parent));
+ nvdimm_flush(to_nd_region(dev->parent), NULL, false);
return 0;
}
static void nd_pmem_shutdown(struct device *dev)
{
- nvdimm_flush(to_nd_region(dev->parent));
+ nvdimm_flush(to_nd_region(dev->parent), NULL, false);
}
static void nd_pmem_notify(struct device *dev, enum nvdimm_event event)
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index fa37afc..5508727 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -290,7 +290,9 @@ static ssize_t deep_flush_store(struct device *dev, struct device_attribute *att
return rc;
if (!flush)
return -EINVAL;
- nvdimm_flush(nd_region);
+ rc = nvdimm_flush(nd_region, NULL, false);
+ if (rc)
+ return rc;
return len;
}
@@ -1065,6 +1067,11 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
dev->of_node = ndr_desc->of_node;
nd_region->ndr_size = resource_size(ndr_desc->res);
nd_region->ndr_start = ndr_desc->res->start;
+ if (ndr_desc->flush)
+ nd_region->flush = ndr_desc->flush;
+ else
+ nd_region->flush = generic_nvdimm_flush;
+
nd_device_register(dev);
return nd_region;
@@ -1105,11 +1112,36 @@ struct nd_region *nvdimm_volatile_region_create(struct nvdimm_bus *nvdimm_bus,
}
EXPORT_SYMBOL_GPL(nvdimm_volatile_region_create);
+int nvdimm_flush(struct nd_region *nd_region, struct bio *bio, bool async)
+{
+ int rc = 0;
+
+ /* Create child bio for asynchronous flush and chain with
+ * parent bio. Otherwise directly call nd_region flush.
+ */
+ if (async && bio->bi_iter.bi_sector != -1) {
+
+ struct bio *child = bio_alloc(GFP_ATOMIC, 0);
+
+ if (!child)
+ return -ENOMEM;
+ bio_copy_dev(child, bio);
+ child->bi_opf = REQ_PREFLUSH;
+ child->bi_iter.bi_sector = -1;
+ bio_chain(child, bio);
+ submit_bio(child);
+ } else {
+ if (nd_region->flush(nd_region))
+ rc = -EIO;
+ }
+
+ return rc;
+}
/**
* nvdimm_flush - flush any posted write queues between the cpu and pmem media
* @nd_region: blk or interleaved pmem region
*/
-void nvdimm_flush(struct nd_region *nd_region)
+int generic_nvdimm_flush(struct nd_region *nd_region)
{
struct nd_region_data *ndrd = dev_get_drvdata(&nd_region->dev);
int i, idx;
@@ -1133,6 +1165,8 @@ void nvdimm_flush(struct nd_region *nd_region)
if (ndrd_get_flush_wpq(ndrd, i, 0))
writeq(1, ndrd_get_flush_wpq(ndrd, i, idx));
wmb();
+
+ return 0;
}
EXPORT_SYMBOL_GPL(nvdimm_flush);
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 097072c..b49632c 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -115,6 +115,7 @@ struct nd_mapping_desc {
int position;
};
+struct nd_region;
struct nd_region_desc {
struct resource *res;
struct nd_mapping_desc *mapping;
@@ -126,6 +127,7 @@ struct nd_region_desc {
int numa_node;
unsigned long flags;
struct device_node *of_node;
+ int (*flush)(struct nd_region *nd_region);
};
struct device;
@@ -201,7 +203,8 @@ unsigned long nd_blk_memremap_flags(struct nd_blk_region *ndbr);
unsigned int nd_region_acquire_lane(struct nd_region *nd_region);
void nd_region_release_lane(struct nd_region *nd_region, unsigned int lane);
u64 nd_fletcher64(void *addr, size_t len, bool le);
-void nvdimm_flush(struct nd_region *nd_region);
+int nvdimm_flush(struct nd_region *nd_region, struct bio *bio, bool async);
+int generic_nvdimm_flush(struct nd_region *nd_region);
int nvdimm_has_flush(struct nd_region *nd_region);
int nvdimm_has_cache(struct nd_region *nd_region);
--
2.9.3
^ permalink raw reply related
* [PATCH v3 2/5] virtio-pmem: Add virtio pmem driver
From: Pankaj Gupta @ 2019-01-09 14:47 UTC (permalink / raw)
To: linux-kernel, kvm, qemu-devel, linux-nvdimm, linux-fsdevel,
virtualization, linux-acpi, linux-ext4, linux-xfs
Cc: pagupta, jack, lcapitulino, adilger.kernel, zwisler, eblake,
dave.jiang, darrick.wong, vishal.l.verma, mst, willy, hch, jmoyer,
nilal, riel, stefanha, imammedo, dan.j.williams, tytso,
xiaoguangrong.eric, rjw, pbonzini
In-Reply-To: <20190109144736.17452-1-pagupta@redhat.com>
This patch adds virtio-pmem driver for KVM guest.
Guest reads the persistent memory range information from
Qemu over VIRTIO and registers it on nvdimm_bus. It also
creates a nd_region object with the persistent memory
range information so that existing 'nvdimm/pmem' driver
can reserve this into system memory map. This way
'virtio-pmem' driver uses existing functionality of pmem
driver to register persistent memory compatible for DAX
capable filesystems.
This also provides function to perform guest flush over
VIRTIO from 'pmem' driver when userspace performs flush
on DAX memory range.
Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
---
drivers/nvdimm/virtio_pmem.c | 84 ++++++++++++++++++++++++++
drivers/virtio/Kconfig | 10 ++++
drivers/virtio/Makefile | 1 +
drivers/virtio/pmem.c | 124 +++++++++++++++++++++++++++++++++++++++
include/linux/virtio_pmem.h | 60 +++++++++++++++++++
include/uapi/linux/virtio_ids.h | 1 +
include/uapi/linux/virtio_pmem.h | 10 ++++
7 files changed, 290 insertions(+)
create mode 100644 drivers/nvdimm/virtio_pmem.c
create mode 100644 drivers/virtio/pmem.c
create mode 100644 include/linux/virtio_pmem.h
create mode 100644 include/uapi/linux/virtio_pmem.h
diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
new file mode 100644
index 0000000..2a1b1ba
--- /dev/null
+++ b/drivers/nvdimm/virtio_pmem.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * virtio_pmem.c: Virtio pmem Driver
+ *
+ * Discovers persistent memory range information
+ * from host and provides a virtio based flushing
+ * interface.
+ */
+#include <linux/virtio_pmem.h>
+#include "nd.h"
+
+ /* The interrupt handler */
+void host_ack(struct virtqueue *vq)
+{
+ unsigned int len;
+ unsigned long flags;
+ struct virtio_pmem_request *req, *req_buf;
+ struct virtio_pmem *vpmem = vq->vdev->priv;
+
+ spin_lock_irqsave(&vpmem->pmem_lock, flags);
+ while ((req = virtqueue_get_buf(vq, &len)) != NULL) {
+ req->done = true;
+ wake_up(&req->host_acked);
+
+ if (!list_empty(&vpmem->req_list)) {
+ req_buf = list_first_entry(&vpmem->req_list,
+ struct virtio_pmem_request, list);
+ list_del(&vpmem->req_list);
+ req_buf->wq_buf_avail = true;
+ wake_up(&req_buf->wq_buf);
+ }
+ }
+ spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
+}
+EXPORT_SYMBOL_GPL(host_ack);
+
+ /* The request submission function */
+int virtio_pmem_flush(struct nd_region *nd_region)
+{
+ int err;
+ unsigned long flags;
+ struct scatterlist *sgs[2], sg, ret;
+ struct virtio_device *vdev = nd_region->provider_data;
+ struct virtio_pmem *vpmem = vdev->priv;
+ struct virtio_pmem_request *req;
+
+ might_sleep();
+ req = kmalloc(sizeof(*req), GFP_KERNEL);
+ if (!req)
+ return -ENOMEM;
+
+ req->done = req->wq_buf_avail = false;
+ strcpy(req->name, "FLUSH");
+ init_waitqueue_head(&req->host_acked);
+ init_waitqueue_head(&req->wq_buf);
+ sg_init_one(&sg, req->name, strlen(req->name));
+ sgs[0] = &sg;
+ sg_init_one(&ret, &req->ret, sizeof(req->ret));
+ sgs[1] = &ret;
+
+ spin_lock_irqsave(&vpmem->pmem_lock, flags);
+ err = virtqueue_add_sgs(vpmem->req_vq, sgs, 1, 1, req, GFP_ATOMIC);
+ if (err) {
+ dev_err(&vdev->dev, "failed to send command to virtio pmem device\n");
+
+ list_add_tail(&vpmem->req_list, &req->list);
+ spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
+
+ /* When host has read buffer, this completes via host_ack */
+ wait_event(req->wq_buf, req->wq_buf_avail);
+ spin_lock_irqsave(&vpmem->pmem_lock, flags);
+ }
+ virtqueue_kick(vpmem->req_vq);
+ spin_unlock_irqrestore(&vpmem->pmem_lock, flags);
+
+ /* When host has read buffer, this completes via host_ack */
+ wait_event(req->host_acked, req->done);
+ err = req->ret;
+ kfree(req);
+
+ return err;
+};
+EXPORT_SYMBOL_GPL(virtio_pmem_flush);
+MODULE_LICENSE("GPL");
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index 3589764..9f634a2 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -42,6 +42,16 @@ config VIRTIO_PCI_LEGACY
If unsure, say Y.
+config VIRTIO_PMEM
+ tristate "Support for virtio pmem driver"
+ depends on VIRTIO
+ depends on LIBNVDIMM
+ help
+ This driver provides support for virtio based flushing interface
+ for persistent memory range.
+
+ If unsure, say M.
+
config VIRTIO_BALLOON
tristate "Virtio balloon driver"
depends on VIRTIO
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index 3a2b5c5..143ce91 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -6,3 +6,4 @@ virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
+obj-$(CONFIG_VIRTIO_PMEM) += pmem.o ../nvdimm/virtio_pmem.o
diff --git a/drivers/virtio/pmem.c b/drivers/virtio/pmem.c
new file mode 100644
index 0000000..51f5349
--- /dev/null
+++ b/drivers/virtio/pmem.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * virtio_pmem.c: Virtio pmem Driver
+ *
+ * Discovers persistent memory range information
+ * from host and registers the virtual pmem device
+ * with libnvdimm core.
+ */
+#include <linux/virtio_pmem.h>
+#include <../../drivers/nvdimm/nd.h>
+
+static struct virtio_device_id id_table[] = {
+ { VIRTIO_ID_PMEM, VIRTIO_DEV_ANY_ID },
+ { 0 },
+};
+
+ /* Initialize virt queue */
+static int init_vq(struct virtio_pmem *vpmem)
+{
+ struct virtqueue *vq;
+
+ /* single vq */
+ vpmem->req_vq = vq = virtio_find_single_vq(vpmem->vdev,
+ host_ack, "flush_queue");
+ if (IS_ERR(vq))
+ return PTR_ERR(vq);
+
+ spin_lock_init(&vpmem->pmem_lock);
+ INIT_LIST_HEAD(&vpmem->req_list);
+
+ return 0;
+};
+
+static int virtio_pmem_probe(struct virtio_device *vdev)
+{
+ int err = 0;
+ struct resource res;
+ struct virtio_pmem *vpmem;
+ struct nvdimm_bus *nvdimm_bus;
+ struct nd_region_desc ndr_desc;
+ int nid = dev_to_node(&vdev->dev);
+ struct nd_region *nd_region;
+
+ if (!vdev->config->get) {
+ dev_err(&vdev->dev, "%s failure: config disabled\n",
+ __func__);
+ return -EINVAL;
+ }
+
+ vdev->priv = vpmem = devm_kzalloc(&vdev->dev, sizeof(*vpmem),
+ GFP_KERNEL);
+ if (!vpmem) {
+ err = -ENOMEM;
+ goto out_err;
+ }
+
+ vpmem->vdev = vdev;
+ err = init_vq(vpmem);
+ if (err)
+ goto out_err;
+
+ virtio_cread(vpmem->vdev, struct virtio_pmem_config,
+ start, &vpmem->start);
+ virtio_cread(vpmem->vdev, struct virtio_pmem_config,
+ size, &vpmem->size);
+
+ res.start = vpmem->start;
+ res.end = vpmem->start + vpmem->size-1;
+ vpmem->nd_desc.provider_name = "virtio-pmem";
+ vpmem->nd_desc.module = THIS_MODULE;
+
+ vpmem->nvdimm_bus = nvdimm_bus = nvdimm_bus_register(&vdev->dev,
+ &vpmem->nd_desc);
+ if (!nvdimm_bus)
+ goto out_vq;
+
+ dev_set_drvdata(&vdev->dev, nvdimm_bus);
+ memset(&ndr_desc, 0, sizeof(ndr_desc));
+
+ ndr_desc.res = &res;
+ ndr_desc.numa_node = nid;
+ ndr_desc.flush = virtio_pmem_flush;
+ set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
+ nd_region = nvdimm_pmem_region_create(nvdimm_bus, &ndr_desc);
+ nd_region->provider_data = dev_to_virtio
+ (nd_region->dev.parent->parent);
+
+ if (!nd_region)
+ goto out_nd;
+
+ //virtio_device_ready(vdev);
+ return 0;
+out_nd:
+ err = -ENXIO;
+ nvdimm_bus_unregister(nvdimm_bus);
+out_vq:
+ vdev->config->del_vqs(vdev);
+out_err:
+ dev_err(&vdev->dev, "failed to register virtio pmem memory\n");
+ return err;
+}
+
+static void virtio_pmem_remove(struct virtio_device *vdev)
+{
+ struct virtio_pmem *vpmem = vdev->priv;
+ struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(&vdev->dev);
+
+ nvdimm_bus_unregister(nvdimm_bus);
+ vdev->config->del_vqs(vdev);
+ kfree(vpmem);
+}
+
+static struct virtio_driver virtio_pmem_driver = {
+ .driver.name = KBUILD_MODNAME,
+ .driver.owner = THIS_MODULE,
+ .id_table = id_table,
+ .probe = virtio_pmem_probe,
+ .remove = virtio_pmem_remove,
+};
+
+module_virtio_driver(virtio_pmem_driver);
+MODULE_DEVICE_TABLE(virtio, id_table);
+MODULE_DESCRIPTION("Virtio pmem driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/virtio_pmem.h b/include/linux/virtio_pmem.h
new file mode 100644
index 0000000..224f9d9
--- /dev/null
+++ b/include/linux/virtio_pmem.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * virtio_pmem.h: virtio pmem Driver
+ *
+ * Discovers persistent memory range information
+ * from host and provides a virtio based flushing
+ * interface.
+ **/
+
+#ifndef _LINUX_VIRTIO_PMEM_H
+#define _LINUX_VIRTIO_PMEM_H
+
+#include <linux/virtio_ids.h>
+#include <linux/module.h>
+#include <linux/virtio_config.h>
+#include <uapi/linux/virtio_pmem.h>
+#include <linux/libnvdimm.h>
+#include <linux/spinlock.h>
+
+struct virtio_pmem_request {
+ /* Host return status corresponding to flush request */
+ int ret;
+
+ /* command name*/
+ char name[16];
+
+ /* Wait queue to process deferred work after ack from host */
+ wait_queue_head_t host_acked;
+ bool done;
+
+ /* Wait queue to process deferred work after virt queue buffer avail */
+ wait_queue_head_t wq_buf;
+ bool wq_buf_avail;
+ struct list_head list;
+};
+
+struct virtio_pmem {
+ struct virtio_device *vdev;
+
+ /* Virtio pmem request queue */
+ struct virtqueue *req_vq;
+
+ /* nvdimm bus registers virtio pmem device */
+ struct nvdimm_bus *nvdimm_bus;
+ struct nvdimm_bus_descriptor nd_desc;
+
+ /* List to store deferred work if virtqueue is full */
+ struct list_head req_list;
+
+ /* Synchronize virtqueue data */
+ spinlock_t pmem_lock;
+
+ /* Memory region information */
+ uint64_t start;
+ uint64_t size;
+};
+
+void host_ack(struct virtqueue *vq);
+int virtio_pmem_flush(struct nd_region *nd_region);
+#endif
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index 6d5c3b2..3463895 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -43,5 +43,6 @@
#define VIRTIO_ID_INPUT 18 /* virtio input */
#define VIRTIO_ID_VSOCK 19 /* virtio vsock transport */
#define VIRTIO_ID_CRYPTO 20 /* virtio crypto */
+#define VIRTIO_ID_PMEM 25 /* virtio pmem */
#endif /* _LINUX_VIRTIO_IDS_H */
diff --git a/include/uapi/linux/virtio_pmem.h b/include/uapi/linux/virtio_pmem.h
new file mode 100644
index 0000000..fa3f7d5
--- /dev/null
+++ b/include/uapi/linux/virtio_pmem.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _UAPI_LINUX_VIRTIO_PMEM_H
+#define _UAPI_LINUX_VIRTIO_PMEM_H
+
+struct virtio_pmem_config {
+ __le64 start;
+ __le64 size;
+};
+#endif
--
2.9.3
^ permalink raw reply related
* [PATCH v3 3/5] libnvdimm: add nd_region buffered dax_dev flag
From: Pankaj Gupta @ 2019-01-09 14:47 UTC (permalink / raw)
To: linux-kernel, kvm, qemu-devel, linux-nvdimm, linux-fsdevel,
virtualization, linux-acpi, linux-ext4, linux-xfs
Cc: pagupta, jack, lcapitulino, adilger.kernel, zwisler, eblake,
dave.jiang, darrick.wong, vishal.l.verma, mst, willy, hch, jmoyer,
nilal, riel, stefanha, imammedo, dan.j.williams, tytso,
xiaoguangrong.eric, rjw, pbonzini
In-Reply-To: <20190109144736.17452-1-pagupta@redhat.com>
This patch adds 'DAXDEV_BUFFERED' flag which is set
for virtio pmem corresponding nd_region. This later
is used to disable MAP_SYNC functionality for ext4
& xfs filesystem.
Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
---
drivers/dax/super.c | 17 +++++++++++++++++
drivers/nvdimm/pmem.c | 3 +++
drivers/nvdimm/region_devs.c | 7 +++++++
drivers/virtio/pmem.c | 1 +
include/linux/dax.h | 9 +++++++++
include/linux/libnvdimm.h | 6 ++++++
6 files changed, 43 insertions(+)
diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index 6e928f3..9128740 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -167,6 +167,8 @@ enum dax_device_flags {
DAXDEV_ALIVE,
/* gate whether dax_flush() calls the low level flush routine */
DAXDEV_WRITE_CACHE,
+ /* flag to disable MAP_SYNC for virtio based host page cache flush */
+ DAXDEV_BUFFERED,
};
/**
@@ -335,6 +337,21 @@ bool dax_write_cache_enabled(struct dax_device *dax_dev)
}
EXPORT_SYMBOL_GPL(dax_write_cache_enabled);
+void virtio_pmem_host_cache(struct dax_device *dax_dev, bool wc)
+{
+ if (wc)
+ set_bit(DAXDEV_BUFFERED, &dax_dev->flags);
+ else
+ clear_bit(DAXDEV_BUFFERED, &dax_dev->flags);
+}
+EXPORT_SYMBOL_GPL(virtio_pmem_host_cache);
+
+bool virtio_pmem_host_cache_enabled(struct dax_device *dax_dev)
+{
+ return test_bit(DAXDEV_BUFFERED, &dax_dev->flags);
+}
+EXPORT_SYMBOL_GPL(virtio_pmem_host_cache_enabled);
+
bool dax_alive(struct dax_device *dax_dev)
{
lockdep_assert_held(&dax_srcu);
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index fe1217b..8d190a3 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -472,6 +472,9 @@ static int pmem_attach_disk(struct device *dev,
return -ENOMEM;
}
dax_write_cache(dax_dev, nvdimm_has_cache(nd_region));
+
+ /* Set buffered bit in 'dax_dev' for virtio pmem */
+ virtio_pmem_host_cache(dax_dev, nvdimm_is_buffered(nd_region));
pmem->dax_dev = dax_dev;
gendev = disk_to_dev(disk);
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index f8218b4..1f8b2be 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -1264,6 +1264,13 @@ int nd_region_conflict(struct nd_region *nd_region, resource_size_t start,
return device_for_each_child(&nvdimm_bus->dev, &ctx, region_conflict);
}
+int nvdimm_is_buffered(struct nd_region *nd_region)
+{
+ return is_nd_pmem(&nd_region->dev) &&
+ test_bit(ND_REGION_BUFFERED, &nd_region->flags);
+}
+EXPORT_SYMBOL_GPL(nvdimm_is_buffered);
+
void __exit nd_region_devs_exit(void)
{
ida_destroy(®ion_ida);
diff --git a/drivers/virtio/pmem.c b/drivers/virtio/pmem.c
index 51f5349..901767b 100644
--- a/drivers/virtio/pmem.c
+++ b/drivers/virtio/pmem.c
@@ -81,6 +81,7 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
ndr_desc.numa_node = nid;
ndr_desc.flush = virtio_pmem_flush;
set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
+ set_bit(ND_REGION_BUFFERED, &ndr_desc.flags);
nd_region = nvdimm_pmem_region_create(nvdimm_bus, &ndr_desc);
nd_region->provider_data = dev_to_virtio
(nd_region->dev.parent->parent);
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 0dd316a..d16e03e 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -37,6 +37,8 @@ void put_dax(struct dax_device *dax_dev);
void kill_dax(struct dax_device *dax_dev);
void dax_write_cache(struct dax_device *dax_dev, bool wc);
bool dax_write_cache_enabled(struct dax_device *dax_dev);
+void virtio_pmem_host_cache(struct dax_device *dax_dev, bool wc);
+bool virtio_pmem_host_cache_enabled(struct dax_device *dax_dev);
#else
static inline struct dax_device *dax_get_by_host(const char *host)
{
@@ -64,6 +66,13 @@ static inline bool dax_write_cache_enabled(struct dax_device *dax_dev)
{
return false;
}
+static inline void virtio_pmem_host_cache(struct dax_device *dax_dev, bool wc)
+{
+}
+static inline bool virtio_pmem_host_cache_enabled(struct dax_device *dax_dev)
+{
+ return false;
+}
#endif
struct writeback_control;
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index ca8bc07..94616f1 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -64,6 +64,11 @@ enum {
*/
ND_REGION_PERSIST_MEMCTRL = 2,
+ /* provides virtio based asynchronous flush mechanism for buffered
+ * host page cache.
+ */
+ ND_REGION_BUFFERED = 3,
+
/* mark newly adjusted resources as requiring a label update */
DPA_RESOURCE_ADJUSTED = 1 << 0,
};
@@ -265,6 +270,7 @@ int generic_nvdimm_flush(struct nd_region *nd_region);
int nvdimm_has_flush(struct nd_region *nd_region);
int nvdimm_has_cache(struct nd_region *nd_region);
int nvdimm_in_overwrite(struct nvdimm *nvdimm);
+int nvdimm_is_buffered(struct nd_region *nd_region);
static inline int nvdimm_ctl(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
unsigned int buf_len, int *cmd_rc)
--
2.9.3
^ permalink raw reply related
* [PATCH v3 4/5] ext4: disable map_sync for virtio pmem
From: Pankaj Gupta @ 2019-01-09 14:47 UTC (permalink / raw)
To: linux-kernel, kvm, qemu-devel, linux-nvdimm, linux-fsdevel,
virtualization, linux-acpi, linux-ext4, linux-xfs
Cc: pagupta, jack, lcapitulino, adilger.kernel, zwisler, eblake,
dave.jiang, darrick.wong, vishal.l.verma, mst, willy, hch, jmoyer,
nilal, riel, stefanha, imammedo, dan.j.williams, tytso,
xiaoguangrong.eric, rjw, pbonzini
In-Reply-To: <20190109144736.17452-1-pagupta@redhat.com>
Virtio pmem provides asynchronous host page cache flush
mechanism. We don't support 'MAP_SYNC' with virtio pmem
and ext4.
Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
---
fs/ext4/file.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 69d65d4..e54f48b 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -360,8 +360,10 @@ static const struct vm_operations_struct ext4_file_vm_ops = {
static int ext4_file_mmap(struct file *file, struct vm_area_struct *vma)
{
struct inode *inode = file->f_mapping->host;
+ struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
+ struct dax_device *dax_dev = sbi->s_daxdev;
- if (unlikely(ext4_forced_shutdown(EXT4_SB(inode->i_sb))))
+ if (unlikely(ext4_forced_shutdown(sbi)))
return -EIO;
/*
@@ -371,6 +373,13 @@ static int ext4_file_mmap(struct file *file, struct vm_area_struct *vma)
if (!IS_DAX(file_inode(file)) && (vma->vm_flags & VM_SYNC))
return -EOPNOTSUPP;
+ /* We don't support synchronous mappings with guest direct access
+ * and virtio based host page cache flush mechanism.
+ */
+ if (IS_DAX(file_inode(file)) && virtio_pmem_host_cache_enabled(dax_dev)
+ && (vma->vm_flags & VM_SYNC))
+ return -EOPNOTSUPP;
+
file_accessed(file);
if (IS_DAX(file_inode(file))) {
vma->vm_ops = &ext4_dax_vm_ops;
--
2.9.3
^ permalink raw reply related
* [PATCH v3 5/5] xfs: disable map_sync for virtio pmem
From: Pankaj Gupta @ 2019-01-09 14:47 UTC (permalink / raw)
To: linux-kernel, kvm, qemu-devel, linux-nvdimm, linux-fsdevel,
virtualization, linux-acpi, linux-ext4, linux-xfs
Cc: pagupta, jack, lcapitulino, adilger.kernel, zwisler, eblake,
dave.jiang, darrick.wong, vishal.l.verma, mst, willy, hch, jmoyer,
nilal, riel, stefanha, imammedo, dan.j.williams, tytso,
xiaoguangrong.eric, rjw, pbonzini
In-Reply-To: <20190109144736.17452-1-pagupta@redhat.com>
Virtio pmem provides asynchronous host page cache flush
mechanism. we don't support 'MAP_SYNC' with virtio pmem
and xfs.
Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
---
fs/xfs/xfs_file.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index e474250..eae4aa4 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1190,6 +1190,14 @@ xfs_file_mmap(
if (!IS_DAX(file_inode(filp)) && (vma->vm_flags & VM_SYNC))
return -EOPNOTSUPP;
+ /* We don't support synchronous mappings with guest direct access
+ * and virtio based host page cache mechanism.
+ */
+ if (IS_DAX(file_inode(filp)) && virtio_pmem_host_cache_enabled(
+ xfs_find_daxdev_for_inode(file_inode(filp))) &&
+ (vma->vm_flags & VM_SYNC))
+ return -EOPNOTSUPP;
+
file_accessed(filp);
vma->vm_ops = &xfs_file_vm_ops;
if (IS_DAX(file_inode(filp)))
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
From: Michael S. Tsirkin @ 2019-01-09 14:49 UTC (permalink / raw)
To: Wei Wang
Cc: virtio-dev, kvm, cohuck, linux-kernel, virtualization, pasic,
pbonzini, dgilbert
In-Reply-To: <5C35CE55.6030200@intel.com>
On Wed, Jan 09, 2019 at 06:35:01PM +0800, Wei Wang wrote:
> On 01/08/2019 04:46 PM, Christian Borntraeger wrote:
> >
> > On 08.01.2019 06:35, Wei Wang wrote:
> > > On 01/07/2019 09:49 PM, Christian Borntraeger wrote:
> > > > On 07.01.2019 08:01, Wei Wang wrote:
> > > > > virtio-ccw has deadlock issues with reading the config space inside the
> > > > > interrupt context, so we tweak the virtballoon_changed implementation
> > > > > by moving the config read operations into the related workqueue contexts.
> > > > > The config_read_bitmap is used as a flag to the workqueue callbacks
> > > > > about the related config fields that need to be read.
> > > > >
> > > > > The cmd_id_received is also renamed to cmd_id_received_cache, and
> > > > > the value should be obtained via virtio_balloon_cmd_id_received.
> > > > >
> > > > > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > > > > Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> > > > > Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> > > > > Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
> > > > Together with
> > > > virtio_pci: use queue idx instead of array idx to set up the vq
> > > > virtio: don't allocate vqs when names[i] = NULL
> > > >
> > > > Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > > OK. I don't plan to send a new version of the above patches as no changes needed so far.
> > >
> > > Michael, if the above two patches look good to you, please help add the related tested-by
> > > and reviewed-by tags. Thanks.
> > Can we also make sure that
> >
> > virtio_pci: use queue idx instead of array idx to set up the vq
> > virtio: don't allocate vqs when names[i] = NULL
> >
> > also land in stable?
> >
>
> You could also send the request to stable after it gets merged to Linus'
> tree.
> The stable review committee will decide whether to take it.
>
> Please see Option 2:
>
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>
>
> Best,
> Wei
That's mostly for 3rd party reporters.
Why not Cc stable directly in the patches?
--
MST
^ permalink raw reply
* Re: [PATCH RFC 1/4] include/linux/compiler*.h: fix OPTIMIZER_HIDE_VAR
From: Michael S. Tsirkin @ 2019-01-09 14:50 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Andrea Parri, Peter Zijlstra, Akira Yokosawa, Will Deacon,
virtualization, David Howells, linux-arch, linux-sparse,
Alan Stern, Paul E. McKenney, Boqun Feng, Daniel Lustig,
Nicholas Piggin, Luc Maranget, Eli Friedman, Jade Alglave,
Network Development, Nick Desaulniers, LKML, Eric Christopher,
Joe Perches, Linus Torvalds, Luc Van Oostenryck <luc.van>
In-Reply-To: <CANiq72nEiw8+cZNnBOwF1ehv232ZJe5qzhdbaTEg0rOoV2p4sg@mail.gmail.com>
On Wed, Jan 09, 2019 at 11:35:52AM +0100, Miguel Ojeda wrote:
> On Tue, Jan 8, 2019 at 6:44 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > Also for more context, see:
> > commit 7829fb09a2b4 ("lib: make memzero_explicit more robust against
> > dead store elimination")
>
> By the way, shouldn't that barrier_data() be directly in compiler.h
> too, since it is for both gcc & clang?
>
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> >
> > + Miguel
> > Miguel, would you mind taking this into your compiler-attributes tree?
>
> Sure, at least we get quickly some linux-next time.
>
> Note it would be nice to separate the patch into two (one for the
> comments, another for OPTIMIZER_HIDE_VAR), and also possibly another
> for barrier_data().
>
> Cheers,
> Miguel
Okay, I will try.
--
MST
^ permalink raw reply
* Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
From: Michael S. Tsirkin @ 2019-01-09 14:52 UTC (permalink / raw)
To: Christian Borntraeger
Cc: virtio-dev, kvm, cohuck, linux-kernel, virtualization, pasic,
pbonzini, dgilbert
In-Reply-To: <8f0977ee-1cc1-56da-f4a6-641b8887648a@de.ibm.com>
On Wed, Jan 09, 2019 at 01:07:16PM +0100, Christian Borntraeger wrote:
> On 09.01.2019 11:35, Wei Wang wrote:
> > On 01/08/2019 04:46 PM, Christian Borntraeger wrote:
> >>
> >> On 08.01.2019 06:35, Wei Wang wrote:
> >>> On 01/07/2019 09:49 PM, Christian Borntraeger wrote:
> >>>> On 07.01.2019 08:01, Wei Wang wrote:
> >>>>> virtio-ccw has deadlock issues with reading the config space inside the
> >>>>> interrupt context, so we tweak the virtballoon_changed implementation
> >>>>> by moving the config read operations into the related workqueue contexts.
> >>>>> The config_read_bitmap is used as a flag to the workqueue callbacks
> >>>>> about the related config fields that need to be read.
> >>>>>
> >>>>> The cmd_id_received is also renamed to cmd_id_received_cache, and
> >>>>> the value should be obtained via virtio_balloon_cmd_id_received.
> >>>>>
> >>>>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >>>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> >>>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> >>>>> Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
> >>>> Together with
> >>>> virtio_pci: use queue idx instead of array idx to set up the vq
> >>>> virtio: don't allocate vqs when names[i] = NULL
> >>>>
> >>>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >>> OK. I don't plan to send a new version of the above patches as no changes needed so far.
> >>>
> >>> Michael, if the above two patches look good to you, please help add the related tested-by
> >>> and reviewed-by tags. Thanks.
> >> Can we also make sure that
> >>
> >> virtio_pci: use queue idx instead of array idx to set up the vq
> >> virtio: don't allocate vqs when names[i] = NULL
> >>
> >> also land in stable?
> >>
> >
> > You could also send the request to stable after it gets merged to Linus' tree.
> > The stable review committee will decide whether to take it.
> >
> > Please see Option 2:
> >
> > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> >
>
> Those patches are not upstream yet, Correct?
>
> Michael,
>
> can you add the stable tag before submitting? If not, can you give me a heads up when doing the
> pull request so that I can ping the stable folks.
Can you reply to patches that you feel are needed on stable with just
Cc: stable@vger.kernel.org
in the message body?
Then it's all automatically handled by ack attaching scripts.
--
MST
^ permalink raw reply
* Re: [PATCH v3 4/5] ext4: disable map_sync for virtio pmem
From: Pankaj Gupta @ 2019-01-09 14:54 UTC (permalink / raw)
To: Jan Kara
Cc: kvm, linux-nvdimm, qemu-devel, virtualization, adilger kernel,
zwisler, eblake, dave jiang, darrick wong, vishal l verma, mst,
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: <20190109144215.GI15397@quack2.suse.cz>
>
> On Wed 09-01-19 19:26:05, Pankaj Gupta wrote:
> > Virtio pmem provides asynchronous host page cache flush
> > mechanism. We don't support 'MAP_SYNC' with virtio pmem
> > and ext4.
> >
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> ...
> > @@ -371,6 +373,13 @@ static int ext4_file_mmap(struct file *file, struct
> > vm_area_struct *vma)
> > if (!IS_DAX(file_inode(file)) && (vma->vm_flags & VM_SYNC))
> > return -EOPNOTSUPP;
> >
> > + /* We don't support synchronous mappings with guest direct access
> > + * and virtio based host page cache flush mechanism.
> > + */
> > + if (IS_DAX(file_inode(file)) && virtio_pmem_host_cache_enabled(dax_dev)
> > + && (vma->vm_flags & VM_SYNC))
> > + return -EOPNOTSUPP;
> > +
>
> Shouldn't there rather be some generic way of doing this? Having
> virtio_pmem_host_cache_enabled() check in filesystem code just looks like
> filesystem sniffing into details is should not care about... Maybe just
> naming this (or having a wrapper) dax_dev_map_sync_supported()?
Thanks for the feedback.
Just wanted to avoid 'dax' in function name just to differentiate this with real dax.
But yes can add a wrapper: dax_dev_map_sync_supported()
Thanks,
Pankaj
^ permalink raw reply
* Re: [PATCH v2 15/15] drm/bochs: reserve bo for pin/unpin
From: Gerd Hoffmann @ 2019-01-09 14:54 UTC (permalink / raw)
To: dri-devel, David Airlie, andr2000, David Airlie, open list,
open list:DRM DRIVER FOR BOCHS VIRTUAL GPU
In-Reply-To: <20190109101044.GS21184@phenom.ffwll.local>
On Wed, Jan 09, 2019 at 11:10:44AM +0100, Daniel Vetter wrote:
> On Tue, Jan 08, 2019 at 12:25:19PM +0100, Gerd Hoffmann wrote:
> > The buffer object must be reserved before calling
> > ttm_bo_validate for pinning/unpinning.
> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>
> Seems a bit a bisect fumble in your series here: legacy kms code reserved
> the ttm bo before calling boch_bo_pin/unpin, your atomic code doesn't. I
> think pushing this into bochs_bo_pin/unpin makes sense for atomic, but to
> avoid bisect fail I think you need to have these temporarily in your
> cleanup/prepare_plane functions too.
I think I've sorted that. Have some other changes too, will probably
send v3 tomorrow.
> Looked through the entire series, this here is the only issue I think
> should be fixed before merging (making atomic_enable optional can be done
> as a follow-up if you feel like it). With that addressed on the series:
>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Thanks.
While being at it: I'm also looking at dma-buf export and import
support for the qemu drivers.
Right now both qxl and virtio have gem_prime_get_sg_table and
gem_prime_import_sg_table handlers which throw a WARN_ONCE() and return
an error.
If I understand things correctly it is valid to set all import/export
callbacks (prime_handle_to_fd, prime_fd_to_handle,
gem_prime_get_sg_table, gem_prime_import_sg_table) to NULL when not
supporting dma-buf import/export and still advertise DRIVER_PRIME to
indicate the other prime callbacks are supported (so generic fbdev
emulation can use gem_prime_vmap etc). Is that correct?
On exporting:
TTM_PL_TT should be easy, just pin the buffer, grab the pages list and
feed that into drm_prime_pages_to_sg. Didn't try yet though. Is that
approach correct?
Is it possible to export TTM_PL_VRAM objects (with backing storage being
a pci memory bar)? If so, how?
On importing:
Importing into TTM_PL_TT object looks easy again, at least when the
object is actually stored in RAM. What if not?
Importing into TTM_PL_VRAM: Impossible I think, without copying over
the data. Should that be done? If so, how? Or is it better to just
not support import then?
thanks,
Gerd
^ permalink raw reply
* Re: [PATCH v3 5/5] xfs: disable map_sync for virtio pmem
From: Darrick J. Wong @ 2019-01-09 16:26 UTC (permalink / raw)
To: Pankaj Gupta
Cc: jack, kvm, linux-nvdimm, qemu-devel, virtualization,
adilger.kernel, zwisler, eblake, dave.jiang, vishal.l.verma, mst,
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: <20190109144736.17452-6-pagupta@redhat.com>
On Wed, Jan 09, 2019 at 08:17:36PM +0530, Pankaj Gupta wrote:
> Virtio pmem provides asynchronous host page cache flush
> mechanism. we don't support 'MAP_SYNC' with virtio pmem
> and xfs.
>
> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> ---
> fs/xfs/xfs_file.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index e474250..eae4aa4 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1190,6 +1190,14 @@ xfs_file_mmap(
> if (!IS_DAX(file_inode(filp)) && (vma->vm_flags & VM_SYNC))
> return -EOPNOTSUPP;
>
> + /* We don't support synchronous mappings with guest direct access
> + * and virtio based host page cache mechanism.
> + */
> + if (IS_DAX(file_inode(filp)) && virtio_pmem_host_cache_enabled(
Echoing what Jan said, this ought to be some sort of generic function
that tells us whether or not memory mapped from the dax device will
always still be accessible even after a crash (i.e. supports MAP_SYNC).
What if the underlying file on the host is itself on pmem and can be
MAP_SYNC'd? Shouldn't the guest be able to use MAP_SYNC as well?
--D
> + xfs_find_daxdev_for_inode(file_inode(filp))) &&
> + (vma->vm_flags & VM_SYNC))
> + return -EOPNOTSUPP;
> +
> file_accessed(filp);
> vma->vm_ops = &xfs_file_vm_ops;
> if (IS_DAX(file_inode(filp)))
> --
> 2.9.3
>
^ permalink raw reply
* Re: [PATCH v1 1/2] virtio_pci: use queue idx instead of array idx to set up the vq
From: Christian Borntraeger @ 2019-01-09 16:29 UTC (permalink / raw)
To: Wei Wang, virtio-dev, linux-kernel, virtualization, kvm, mst,
cohuck
Cc: pbonzini, dgilbert
In-Reply-To: <1545963986-11280-2-git-send-email-wei.w.wang@intel.com>
Cc: stable@vger.kernel.org
Fixes: 86a559787e6f ("virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT")
On 28.12.2018 03:26, Wei Wang wrote:
> When find_vqs, there will be no vq[i] allocation if its corresponding
> names[i] is NULL. For example, the caller may pass in names[i] (i=4)
> with names[2] being NULL because the related feature bit is turned off,
> so technically there are 3 queues on the device, and name[4] should
> correspond to the 3rd queue on the device.
>
> So we use queue_idx as the queue index, which is increased only when the
> queue exists.
>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> ---
> drivers/virtio/virtio_pci_common.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 465a6f5..d0584c0 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -285,7 +285,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> u16 msix_vec;
> - int i, err, nvectors, allocated_vectors;
> + int i, err, nvectors, allocated_vectors, queue_idx = 0;
>
> vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL);
> if (!vp_dev->vqs)
> @@ -321,7 +321,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
> msix_vec = allocated_vectors++;
> else
> msix_vec = VP_MSIX_VQ_VECTOR;
> - vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i],
> + vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i],
> ctx ? ctx[i] : false,
> msix_vec);
> if (IS_ERR(vqs[i])) {
> @@ -356,7 +356,7 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned nvqs,
> const char * const names[], const bool *ctx)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> - int i, err;
> + int i, err, queue_idx = 0;
>
> vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL);
> if (!vp_dev->vqs)
> @@ -374,7 +374,7 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned nvqs,
> vqs[i] = NULL;
> continue;
> }
> - vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i],
> + vqs[i] = vp_setup_vq(vdev, queue_idx++, callbacks[i], names[i],
> ctx ? ctx[i] : false,
> VIRTIO_MSI_NO_VECTOR);
> if (IS_ERR(vqs[i])) {
>
^ permalink raw reply
* Re: [PATCH v1 2/2] virtio: don't allocate vqs when names[i] = NULL
From: Christian Borntraeger @ 2019-01-09 16:30 UTC (permalink / raw)
To: Wei Wang, virtio-dev, linux-kernel, virtualization, kvm, mst,
cohuck
Cc: pbonzini, dgilbert
In-Reply-To: <1545963986-11280-3-git-send-email-wei.w.wang@intel.com>
Cc: stable@vger.kernel.org
Fixes: 86a559787e6f ("virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT")
On 28.12.2018 03:26, Wei Wang wrote:
> Some vqs may not need to be allocated when their related feature bits
> are disabled. So callers may pass in such vqs with "names = NULL".
> Then we skip such vq allocations.
>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> ---
> drivers/misc/mic/vop/vop_main.c | 9 +++++++--
> drivers/remoteproc/remoteproc_virtio.c | 9 +++++++--
> drivers/s390/virtio/virtio_ccw.c | 12 +++++++++---
> drivers/virtio/virtio_mmio.c | 9 +++++++--
> 4 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/misc/mic/vop/vop_main.c b/drivers/misc/mic/vop/vop_main.c
> index 6b212c8..2bfa3a9 100644
> --- a/drivers/misc/mic/vop/vop_main.c
> +++ b/drivers/misc/mic/vop/vop_main.c
> @@ -394,16 +394,21 @@ static int vop_find_vqs(struct virtio_device *dev, unsigned nvqs,
> struct _vop_vdev *vdev = to_vopvdev(dev);
> struct vop_device *vpdev = vdev->vpdev;
> struct mic_device_ctrl __iomem *dc = vdev->dc;
> - int i, err, retry;
> + int i, err, retry, queue_idx = 0;
>
> /* We must have this many virtqueues. */
> if (nvqs > ioread8(&vdev->desc->num_vq))
> return -ENOENT;
>
> for (i = 0; i < nvqs; ++i) {
> + if (!names[i]) {
> + vqs[i] = NULL;
> + continue;
> + }
> +
> dev_dbg(_vop_dev(vdev), "%s: %d: %s\n",
> __func__, i, names[i]);
> - vqs[i] = vop_find_vq(dev, i, callbacks[i], names[i],
> + vqs[i] = vop_find_vq(dev, queue_idx++, callbacks[i], names[i],
> ctx ? ctx[i] : false);
> if (IS_ERR(vqs[i])) {
> err = PTR_ERR(vqs[i]);
> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
> index 183fc42..2d7cd344 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -153,10 +153,15 @@ static int rproc_virtio_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
> const bool * ctx,
> struct irq_affinity *desc)
> {
> - int i, ret;
> + int i, ret, queue_idx = 0;
>
> for (i = 0; i < nvqs; ++i) {
> - vqs[i] = rp_find_vq(vdev, i, callbacks[i], names[i],
> + if (!names[i]) {
> + vqs[i] = NULL;
> + continue;
> + }
> +
> + vqs[i] = rp_find_vq(vdev, queue_idx++, callbacks[i], names[i],
> ctx ? ctx[i] : false);
> if (IS_ERR(vqs[i])) {
> ret = PTR_ERR(vqs[i]);
> diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
> index fc9dbad..ae1d56d 100644
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -635,7 +635,7 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> {
> struct virtio_ccw_device *vcdev = to_vc_device(vdev);
> unsigned long *indicatorp = NULL;
> - int ret, i;
> + int ret, i, queue_idx = 0;
> struct ccw1 *ccw;
>
> ccw = kzalloc(sizeof(*ccw), GFP_DMA | GFP_KERNEL);
> @@ -643,8 +643,14 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> return -ENOMEM;
>
> for (i = 0; i < nvqs; ++i) {
> - vqs[i] = virtio_ccw_setup_vq(vdev, i, callbacks[i], names[i],
> - ctx ? ctx[i] : false, ccw);
> + if (!names[i]) {
> + vqs[i] = NULL;
> + continue;
> + }
> +
> + vqs[i] = virtio_ccw_setup_vq(vdev, queue_idx++, callbacks[i],
> + names[i], ctx ? ctx[i] : false,
> + ccw);
> if (IS_ERR(vqs[i])) {
> ret = PTR_ERR(vqs[i]);
> vqs[i] = NULL;
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 4cd9ea5..d9dd0f78 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -468,7 +468,7 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> {
> struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
> unsigned int irq = platform_get_irq(vm_dev->pdev, 0);
> - int i, err;
> + int i, err, queue_idx = 0;
>
> err = request_irq(irq, vm_interrupt, IRQF_SHARED,
> dev_name(&vdev->dev), vm_dev);
> @@ -476,7 +476,12 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> return err;
>
> for (i = 0; i < nvqs; ++i) {
> - vqs[i] = vm_setup_vq(vdev, i, callbacks[i], names[i],
> + if (!names[i]) {
> + vqs[i] = NULL;
> + continue;
> + }
> +
> + vqs[i] = vm_setup_vq(vdev, queue_idx++, callbacks[i], names[i],
> ctx ? ctx[i] : false);
> if (IS_ERR(vqs[i])) {
> vm_del_vqs(vdev);
>
^ permalink raw reply
* Re: [PATCH v3 3/5] libnvdimm: add nd_region buffered dax_dev flag
From: Dan Williams @ 2019-01-09 17:02 UTC (permalink / raw)
To: Pankaj Gupta
Cc: Jan Kara, KVM list, linux-nvdimm, Qemu Developers, virtualization,
Andreas Dilger, Ross Zwisler, Eric Blake, Dave Jiang,
Darrick J. Wong, Vishal L Verma, Michael S. Tsirkin,
Matthew Wilcox, Christoph Hellwig, Linux ACPI, jmoyer,
Nitesh Narayan Lal, Rik van Riel, Stefan Hajnoczi, Igor Mammedov,
lcapitulino, linux-ext4, Theodore Ts'o,
Xiao Guangrong <xiaoguangrong.eric@
In-Reply-To: <20190109135024.14093-4-pagupta@redhat.com>
On Wed, Jan 9, 2019 at 5:53 AM Pankaj Gupta <pagupta@redhat.com> wrote:
>
> This patch adds 'DAXDEV_BUFFERED' flag which is set
> for virtio pmem corresponding nd_region. This later
> is used to disable MAP_SYNC functionality for ext4
> & xfs filesystem.
>
> Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> ---
> drivers/dax/super.c | 17 +++++++++++++++++
> drivers/nvdimm/pmem.c | 3 +++
> drivers/nvdimm/region_devs.c | 7 +++++++
> drivers/virtio/pmem.c | 1 +
> include/linux/dax.h | 9 +++++++++
> include/linux/libnvdimm.h | 6 ++++++
> 6 files changed, 43 insertions(+)
>
> diff --git a/drivers/dax/super.c b/drivers/dax/super.c
> index 6e928f3..9128740 100644
> --- a/drivers/dax/super.c
> +++ b/drivers/dax/super.c
> @@ -167,6 +167,8 @@ enum dax_device_flags {
> DAXDEV_ALIVE,
> /* gate whether dax_flush() calls the low level flush routine */
> DAXDEV_WRITE_CACHE,
> + /* flag to disable MAP_SYNC for virtio based host page cache flush */
> + DAXDEV_BUFFERED,
> };
>
> /**
> @@ -335,6 +337,21 @@ bool dax_write_cache_enabled(struct dax_device *dax_dev)
> }
> EXPORT_SYMBOL_GPL(dax_write_cache_enabled);
>
> +void virtio_pmem_host_cache(struct dax_device *dax_dev, bool wc)
> +{
> + if (wc)
> + set_bit(DAXDEV_BUFFERED, &dax_dev->flags);
> + else
> + clear_bit(DAXDEV_BUFFERED, &dax_dev->flags);
> +}
> +EXPORT_SYMBOL_GPL(virtio_pmem_host_cache);
The "write_cache" property was structured this way because it can
conceivably change at runtime. The MAP_SYNC capability should be
static and never changed after init.
> +bool virtio_pmem_host_cache_enabled(struct dax_device *dax_dev)
> +{
> + return test_bit(DAXDEV_BUFFERED, &dax_dev->flags);
> +}
> +EXPORT_SYMBOL_GPL(virtio_pmem_host_cache_enabled);
Echoing Darrick and Jan this is should be a generic property of a
dax_device and not specific to virtio. I don't like the "buffered"
designation as that's not accurate. There may be hardware reasons why
a dax_device is not synchronous, like a requirement to flush a
write-pending queue or otherwise notify the device of new writes.
I would just have a dax_synchronous() helper and a DAXDEV_SYNC flag. I
would also modify alloc_dax() to take a flags argument so that the
capability can be instantiated when the dax_device is allocated.
^ permalink raw reply
* Re: [PATCH v2 15/15] drm/bochs: reserve bo for pin/unpin
From: Daniel Vetter @ 2019-01-09 17:35 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Oleksandr Andrushchenko, open list, dri-devel,
open list:DRM DRIVER FOR BOCHS VIRTUAL GPU, David Airlie,
David Airlie
In-Reply-To: <20190109145443.l5yus2pgvxcl4zbt@sirius.home.kraxel.org>
On Wed, Jan 9, 2019 at 3:54 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Wed, Jan 09, 2019 at 11:10:44AM +0100, Daniel Vetter wrote:
> > On Tue, Jan 08, 2019 at 12:25:19PM +0100, Gerd Hoffmann wrote:
> > > The buffer object must be reserved before calling
> > > ttm_bo_validate for pinning/unpinning.
> > >
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> >
> > Seems a bit a bisect fumble in your series here: legacy kms code reserved
> > the ttm bo before calling boch_bo_pin/unpin, your atomic code doesn't. I
> > think pushing this into bochs_bo_pin/unpin makes sense for atomic, but to
> > avoid bisect fail I think you need to have these temporarily in your
> > cleanup/prepare_plane functions too.
>
> I think I've sorted that. Have some other changes too, will probably
> send v3 tomorrow.
>
> > Looked through the entire series, this here is the only issue I think
> > should be fixed before merging (making atomic_enable optional can be done
> > as a follow-up if you feel like it). With that addressed on the series:
> >
> > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Thanks.
>
> While being at it: I'm also looking at dma-buf export and import
> support for the qemu drivers.
>
> Right now both qxl and virtio have gem_prime_get_sg_table and
> gem_prime_import_sg_table handlers which throw a WARN_ONCE() and return
> an error.
>
> If I understand things correctly it is valid to set all import/export
> callbacks (prime_handle_to_fd, prime_fd_to_handle,
> gem_prime_get_sg_table, gem_prime_import_sg_table) to NULL when not
> supporting dma-buf import/export and still advertise DRIVER_PRIME to
> indicate the other prime callbacks are supported (so generic fbdev
> emulation can use gem_prime_vmap etc). Is that correct?
I'm not sure how much that's a good idea ... Never thought about it
tbh. All the fbdev/dma-buf stuff has plenty of hacks and
inconsistencies still, so I guess we can't make it much worse really.
> On exporting:
>
> TTM_PL_TT should be easy, just pin the buffer, grab the pages list and
> feed that into drm_prime_pages_to_sg. Didn't try yet though. Is that
> approach correct?
>
> Is it possible to export TTM_PL_VRAM objects (with backing storage being
> a pci memory bar)? If so, how?
Not really in general. amdgpu upcasts to amdgpu_bo (if it's amgpu BO)
and then knows the internals so it can do a proper pci peer2peer
mapping. Or at least there's been lots of patches floating around to
make that happen.
I think other drivers migrate the bo out of VRAM.
> On importing:
>
> Importing into TTM_PL_TT object looks easy again, at least when the
> object is actually stored in RAM. What if not?
They are all supposed to be stored in RAM. Note that all current ttm
importers totally break the abstraction, by taking the sg list,
throwing the dma mapping away and assuming there's a struct page
backing it. Would be good if we could stop spreading that abuse - the
dma-buf interfaces have been modelled after the ttm bo interfaces, so
shouldn't be too hard to wire this up correctly.
> Importing into TTM_PL_VRAM: Impossible I think, without copying over
> the data. Should that be done? If so, how? Or is it better to just
> not support import then?
Hm, since you ask about TTM concepts and not what this means in terms
of dma-buf: As long as you upcast to the ttm_bo you can do whatever
you want to really. But with plain dma-buf this doesn't work right now
(least because ttm assumes it gets system RAM on import, in theory you
could put the peer2peer dma mapping into the sg list and it should
work).
-Daniel
>
> thanks,
> Gerd
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH v3 5/5] xfs: disable map_sync for virtio pmem
From: Pankaj Gupta @ 2019-01-09 18:08 UTC (permalink / raw)
To: Darrick J. Wong
Cc: jack, kvm, linux-nvdimm, qemu-devel, virtualization,
adilger kernel, zwisler, eblake, dave jiang, vishal l verma, mst,
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: <20190109162654.GW12689@magnolia>
> > Virtio pmem provides asynchronous host page cache flush
> > mechanism. we don't support 'MAP_SYNC' with virtio pmem
> > and xfs.
> >
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > ---
> > fs/xfs/xfs_file.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > index e474250..eae4aa4 100644
> > --- a/fs/xfs/xfs_file.c
> > +++ b/fs/xfs/xfs_file.c
> > @@ -1190,6 +1190,14 @@ xfs_file_mmap(
> > if (!IS_DAX(file_inode(filp)) && (vma->vm_flags & VM_SYNC))
> > return -EOPNOTSUPP;
> >
> > + /* We don't support synchronous mappings with guest direct access
> > + * and virtio based host page cache mechanism.
> > + */
> > + if (IS_DAX(file_inode(filp)) && virtio_pmem_host_cache_enabled(
>
> Echoing what Jan said, this ought to be some sort of generic function
> that tells us whether or not memory mapped from the dax device will
> always still be accessible even after a crash (i.e. supports MAP_SYNC).
o.k
>
> What if the underlying file on the host is itself on pmem and can be
> MAP_SYNC'd? Shouldn't the guest be able to use MAP_SYNC as well?
Guest MAP_SYNC on actual host pmem will sync guest metadata, as guest
writes are persistent on actual pmem. Host side Qemu MAP_SYNC enabling
work for pmem is in progress. It will make sure metadata at host side
is in consistent state after a crash or any other metadata corruption
operation.
For virtio-pmem, we are emulating pmem device over regular storage on
host side. Guest need to call fsync followed by write to make sure
guest metadata is in consistent state(or journalled). Host backing file
data & metadata will also be persistent after guest to host fsync.
Thanks,
Pankaj
^ permalink raw reply
* Re: [PATCH v3 3/5] libnvdimm: add nd_region buffered dax_dev flag
From: Pankaj Gupta @ 2019-01-09 18:21 UTC (permalink / raw)
To: Dan Williams
Cc: Jan Kara, KVM list, linux-nvdimm, Qemu Developers, virtualization,
Andreas Dilger, Ross Zwisler, Eric Blake, Dave Jiang,
Darrick J. Wong, Vishal L Verma, Michael S. Tsirkin,
Matthew Wilcox, Christoph Hellwig, Linux ACPI, jmoyer,
Nitesh Narayan Lal, Rik van Riel, Stefan Hajnoczi, Igor Mammedov,
lcapitulino, linux-ext4, Theodore Ts'o,
Xiao Guangrong <xiaoguangrong.eric@
In-Reply-To: <CAPcyv4jsa709BVPAx_BT5eFrODf-PUPOZ4aguDsjZVawVXTVHQ@mail.gmail.com>
> >
> > This patch adds 'DAXDEV_BUFFERED' flag which is set
> > for virtio pmem corresponding nd_region. This later
> > is used to disable MAP_SYNC functionality for ext4
> > & xfs filesystem.
> >
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > ---
> > drivers/dax/super.c | 17 +++++++++++++++++
> > drivers/nvdimm/pmem.c | 3 +++
> > drivers/nvdimm/region_devs.c | 7 +++++++
> > drivers/virtio/pmem.c | 1 +
> > include/linux/dax.h | 9 +++++++++
> > include/linux/libnvdimm.h | 6 ++++++
> > 6 files changed, 43 insertions(+)
> >
> > diff --git a/drivers/dax/super.c b/drivers/dax/super.c
> > index 6e928f3..9128740 100644
> > --- a/drivers/dax/super.c
> > +++ b/drivers/dax/super.c
> > @@ -167,6 +167,8 @@ enum dax_device_flags {
> > DAXDEV_ALIVE,
> > /* gate whether dax_flush() calls the low level flush routine */
> > DAXDEV_WRITE_CACHE,
> > + /* flag to disable MAP_SYNC for virtio based host page cache flush
> > */
> > + DAXDEV_BUFFERED,
> > };
> >
> > /**
> > @@ -335,6 +337,21 @@ bool dax_write_cache_enabled(struct dax_device
> > *dax_dev)
> > }
> > EXPORT_SYMBOL_GPL(dax_write_cache_enabled);
> >
> > +void virtio_pmem_host_cache(struct dax_device *dax_dev, bool wc)
> > +{
> > + if (wc)
> > + set_bit(DAXDEV_BUFFERED, &dax_dev->flags);
> > + else
> > + clear_bit(DAXDEV_BUFFERED, &dax_dev->flags);
> > +}
> > +EXPORT_SYMBOL_GPL(virtio_pmem_host_cache);
>
> The "write_cache" property was structured this way because it can
> conceivably change at runtime. The MAP_SYNC capability should be
> static and never changed after init.
o.k. Will change.
>
> > +bool virtio_pmem_host_cache_enabled(struct dax_device *dax_dev)
> > +{
> > + return test_bit(DAXDEV_BUFFERED, &dax_dev->flags);
> > +}
> > +EXPORT_SYMBOL_GPL(virtio_pmem_host_cache_enabled);
>
> Echoing Darrick and Jan this is should be a generic property of a
> dax_device and not specific to virtio. I don't like the "buffered"
> designation as that's not accurate. There may be hardware reasons why
> a dax_device is not synchronous, like a requirement to flush a
> write-pending queue or otherwise notify the device of new writes.
Agree.
>
> I would just have a dax_synchronous() helper and a DAXDEV_SYNC flag. I
> would also modify alloc_dax() to take a flags argument so that the
> capability can be instantiated when the dax_device is allocated.
o.k. Will make the change.
Thanks,
Pankaj
>
^ permalink raw reply
* Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
From: Christian Borntraeger @ 2019-01-09 18:22 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtio-dev, kvm, cohuck, linux-kernel, virtualization, pasic,
pbonzini, dgilbert
In-Reply-To: <20190109095048-mutt-send-email-mst@kernel.org>
On 09.01.2019 15:52, Michael S. Tsirkin wrote:
> On Wed, Jan 09, 2019 at 01:07:16PM +0100, Christian Borntraeger wrote:
>> On 09.01.2019 11:35, Wei Wang wrote:
>>> On 01/08/2019 04:46 PM, Christian Borntraeger wrote:
>>>>
>>>> On 08.01.2019 06:35, Wei Wang wrote:
>>>>> On 01/07/2019 09:49 PM, Christian Borntraeger wrote:
>>>>>> On 07.01.2019 08:01, Wei Wang wrote:
>>>>>>> virtio-ccw has deadlock issues with reading the config space inside the
>>>>>>> interrupt context, so we tweak the virtballoon_changed implementation
>>>>>>> by moving the config read operations into the related workqueue contexts.
>>>>>>> The config_read_bitmap is used as a flag to the workqueue callbacks
>>>>>>> about the related config fields that need to be read.
>>>>>>>
>>>>>>> The cmd_id_received is also renamed to cmd_id_received_cache, and
>>>>>>> the value should be obtained via virtio_balloon_cmd_id_received.
>>>>>>>
>>>>>>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>>>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>>>>>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>>>>>>> Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
>>>>>> Together with
>>>>>> virtio_pci: use queue idx instead of array idx to set up the vq
>>>>>> virtio: don't allocate vqs when names[i] = NULL
>>>>>>
>>>>>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>>> OK. I don't plan to send a new version of the above patches as no changes needed so far.
>>>>>
>>>>> Michael, if the above two patches look good to you, please help add the related tested-by
>>>>> and reviewed-by tags. Thanks.
>>>> Can we also make sure that
>>>>
>>>> virtio_pci: use queue idx instead of array idx to set up the vq
>>>> virtio: don't allocate vqs when names[i] = NULL
>>>>
>>>> also land in stable?
>>>>
>>>
>>> You could also send the request to stable after it gets merged to Linus' tree.
>>> The stable review committee will decide whether to take it.
>>>
>>> Please see Option 2:
>>>
>>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>>
>>
>> Those patches are not upstream yet, Correct?
>>
>> Michael,
>>
>> can you add the stable tag before submitting? If not, can you give me a heads up when doing the
>> pull request so that I can ping the stable folks.
>
> Can you reply to patches that you feel are needed on stable with just
>
> Cc: stable@vger.kernel.org
>
> in the message body?
>
> Then it's all automatically handled by ack attaching scripts.
Done. But this only works if those patches are not already part of a tree. I guess they have to go via
your tree, correct?
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation
From: Michael S. Tsirkin @ 2019-01-09 18:31 UTC (permalink / raw)
To: Christian Borntraeger
Cc: virtio-dev, kvm, cohuck, linux-kernel, virtualization, pasic,
pbonzini, dgilbert
In-Reply-To: <b67652e6-c77a-cf0c-c02e-1494645ade91@de.ibm.com>
On Wed, Jan 09, 2019 at 07:22:50PM +0100, Christian Borntraeger wrote:
>
> On 09.01.2019 15:52, Michael S. Tsirkin wrote:
> > On Wed, Jan 09, 2019 at 01:07:16PM +0100, Christian Borntraeger wrote:
> >> On 09.01.2019 11:35, Wei Wang wrote:
> >>> On 01/08/2019 04:46 PM, Christian Borntraeger wrote:
> >>>>
> >>>> On 08.01.2019 06:35, Wei Wang wrote:
> >>>>> On 01/07/2019 09:49 PM, Christian Borntraeger wrote:
> >>>>>> On 07.01.2019 08:01, Wei Wang wrote:
> >>>>>>> virtio-ccw has deadlock issues with reading the config space inside the
> >>>>>>> interrupt context, so we tweak the virtballoon_changed implementation
> >>>>>>> by moving the config read operations into the related workqueue contexts.
> >>>>>>> The config_read_bitmap is used as a flag to the workqueue callbacks
> >>>>>>> about the related config fields that need to be read.
> >>>>>>>
> >>>>>>> The cmd_id_received is also renamed to cmd_id_received_cache, and
> >>>>>>> the value should be obtained via virtio_balloon_cmd_id_received.
> >>>>>>>
> >>>>>>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >>>>>>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> >>>>>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> >>>>>>> Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
> >>>>>> Together with
> >>>>>> virtio_pci: use queue idx instead of array idx to set up the vq
> >>>>>> virtio: don't allocate vqs when names[i] = NULL
> >>>>>>
> >>>>>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >>>>> OK. I don't plan to send a new version of the above patches as no changes needed so far.
> >>>>>
> >>>>> Michael, if the above two patches look good to you, please help add the related tested-by
> >>>>> and reviewed-by tags. Thanks.
> >>>> Can we also make sure that
> >>>>
> >>>> virtio_pci: use queue idx instead of array idx to set up the vq
> >>>> virtio: don't allocate vqs when names[i] = NULL
> >>>>
> >>>> also land in stable?
> >>>>
> >>>
> >>> You could also send the request to stable after it gets merged to Linus' tree.
> >>> The stable review committee will decide whether to take it.
> >>>
> >>> Please see Option 2:
> >>>
> >>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> >>>
> >>
> >> Those patches are not upstream yet, Correct?
> >>
> >> Michael,
> >>
> >> can you add the stable tag before submitting? If not, can you give me a heads up when doing the
> >> pull request so that I can ping the stable folks.
> >
> > Can you reply to patches that you feel are needed on stable with just
> >
> > Cc: stable@vger.kernel.org
> >
> > in the message body?
> >
> > Then it's all automatically handled by ack attaching scripts.
>
> Done. But this only works if those patches are not already part of a tree. I guess they have to go via
> your tree, correct?
Yes. It works because I rebase my tree.
--
MST
^ permalink raw reply
* Re: [PATCH] virtio-ccw: wire up ->bus_name callback
From: Cornelia Huck @ 2019-01-09 18:39 UTC (permalink / raw)
To: Halil Pasic; +Cc: linux-s390, kvm, virtualization
In-Reply-To: <20190103141145.14554-1-cohuck@redhat.com>
On Thu, 3 Jan 2019 15:11:45 +0100
Cornelia Huck <cohuck@redhat.com> wrote:
> Return the bus id of the ccw proxy device. This makes 'ethtool -i'
> show a more useful value than 'virtio' in the bus-info field.
>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> drivers/s390/virtio/virtio_ccw.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
Queued.
^ permalink raw reply
* Re: [PATCH v2 15/15] drm/bochs: reserve bo for pin/unpin
From: Alex Deucher @ 2019-01-09 18:45 UTC (permalink / raw)
To: Daniel Vetter
Cc: Oleksandr Andrushchenko, open list, dri-devel,
open list:DRM DRIVER FOR BOCHS VIRTUAL GPU, David Airlie,
David Airlie
In-Reply-To: <CAKMK7uG0HCNso9rE9TivP2cVKMD1rNPC2wJH_+yu+CWGEh9XdQ@mail.gmail.com>
On Wed, Jan 9, 2019 at 12:36 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Jan 9, 2019 at 3:54 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Wed, Jan 09, 2019 at 11:10:44AM +0100, Daniel Vetter wrote:
> > > On Tue, Jan 08, 2019 at 12:25:19PM +0100, Gerd Hoffmann wrote:
> > > > The buffer object must be reserved before calling
> > > > ttm_bo_validate for pinning/unpinning.
> > > >
> > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > >
> > > Seems a bit a bisect fumble in your series here: legacy kms code reserved
> > > the ttm bo before calling boch_bo_pin/unpin, your atomic code doesn't. I
> > > think pushing this into bochs_bo_pin/unpin makes sense for atomic, but to
> > > avoid bisect fail I think you need to have these temporarily in your
> > > cleanup/prepare_plane functions too.
> >
> > I think I've sorted that. Have some other changes too, will probably
> > send v3 tomorrow.
> >
> > > Looked through the entire series, this here is the only issue I think
> > > should be fixed before merging (making atomic_enable optional can be done
> > > as a follow-up if you feel like it). With that addressed on the series:
> > >
> > > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > Thanks.
> >
> > While being at it: I'm also looking at dma-buf export and import
> > support for the qemu drivers.
> >
> > Right now both qxl and virtio have gem_prime_get_sg_table and
> > gem_prime_import_sg_table handlers which throw a WARN_ONCE() and return
> > an error.
> >
> > If I understand things correctly it is valid to set all import/export
> > callbacks (prime_handle_to_fd, prime_fd_to_handle,
> > gem_prime_get_sg_table, gem_prime_import_sg_table) to NULL when not
> > supporting dma-buf import/export and still advertise DRIVER_PRIME to
> > indicate the other prime callbacks are supported (so generic fbdev
> > emulation can use gem_prime_vmap etc). Is that correct?
>
> I'm not sure how much that's a good idea ... Never thought about it
> tbh. All the fbdev/dma-buf stuff has plenty of hacks and
> inconsistencies still, so I guess we can't make it much worse really.
>
> > On exporting:
> >
> > TTM_PL_TT should be easy, just pin the buffer, grab the pages list and
> > feed that into drm_prime_pages_to_sg. Didn't try yet though. Is that
> > approach correct?
> >
> > Is it possible to export TTM_PL_VRAM objects (with backing storage being
> > a pci memory bar)? If so, how?
>
> Not really in general. amdgpu upcasts to amdgpu_bo (if it's amgpu BO)
> and then knows the internals so it can do a proper pci peer2peer
> mapping. Or at least there's been lots of patches floating around to
> make that happen.
Here's Christian's WIP stuff for adding device memory support to dma-buf:
https://cgit.freedesktop.org/~deathsimple/linux/log/?h=p2p
Alex
>
> I think other drivers migrate the bo out of VRAM.
>
> > On importing:
> >
> > Importing into TTM_PL_TT object looks easy again, at least when the
> > object is actually stored in RAM. What if not?
>
> They are all supposed to be stored in RAM. Note that all current ttm
> importers totally break the abstraction, by taking the sg list,
> throwing the dma mapping away and assuming there's a struct page
> backing it. Would be good if we could stop spreading that abuse - the
> dma-buf interfaces have been modelled after the ttm bo interfaces, so
> shouldn't be too hard to wire this up correctly.
>
> > Importing into TTM_PL_VRAM: Impossible I think, without copying over
> > the data. Should that be done? If so, how? Or is it better to just
> > not support import then?
>
> Hm, since you ask about TTM concepts and not what this means in terms
> of dma-buf: As long as you upcast to the ttm_bo you can do whatever
> you want to really. But with plain dma-buf this doesn't work right now
> (least because ttm assumes it gets system RAM on import, in theory you
> could put the peer2peer dma mapping into the sg list and it should
> work).
> -Daniel
>
> >
> > thanks,
> > Gerd
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH v2 15/15] drm/bochs: reserve bo for pin/unpin
From: Gerd Hoffmann @ 2019-01-09 20:51 UTC (permalink / raw)
To: Daniel Vetter
Cc: Oleksandr Andrushchenko, open list, dri-devel,
open list:DRM DRIVER FOR BOCHS VIRTUAL GPU, David Airlie,
David Airlie
In-Reply-To: <CAKMK7uG0HCNso9rE9TivP2cVKMD1rNPC2wJH_+yu+CWGEh9XdQ@mail.gmail.com>
Hi,
> > If I understand things correctly it is valid to set all import/export
> > callbacks (prime_handle_to_fd, prime_fd_to_handle,
> > gem_prime_get_sg_table, gem_prime_import_sg_table) to NULL when not
> > supporting dma-buf import/export and still advertise DRIVER_PRIME to
> > indicate the other prime callbacks are supported (so generic fbdev
> > emulation can use gem_prime_vmap etc). Is that correct?
>
> I'm not sure how much that's a good idea ... Never thought about it
> tbh. All the fbdev/dma-buf stuff has plenty of hacks and
> inconsistencies still, so I guess we can't make it much worse really.
Setting prime_handle_to_fd + prime_fd_to_handle to NULL has the effect
that drm stops advertising DRM_PRIME_CAP_{IMPORT,EXPORT} to userspace.
Which looks better to me than telling userspace we support it then throw
errors unconditionally when userspace tries to use that.
> > Is it possible to export TTM_PL_VRAM objects (with backing storage being
> > a pci memory bar)? If so, how?
>
> Not really in general. amdgpu upcasts to amdgpu_bo (if it's amgpu BO)
> and then knows the internals so it can do a proper pci peer2peer
> mapping. Or at least there's been lots of patches floating around to
> make that happen.
That is limited to bo sharing between two amdgpu devices, correct?
> I think other drivers migrate the bo out of VRAM.
Well, that doesn't look too useful. bochs and qxl virtual hardware
can't access buffers outside VRAM. So, while I could migrate the
buffers to RAM (via memcpy) when exporting they would at the same time
become unusable for the GPU ...
> > On importing:
> >
> > Importing into TTM_PL_TT object looks easy again, at least when the
> > object is actually stored in RAM. What if not?
>
> They are all supposed to be stored in RAM. Note that all current ttm
> importers totally break the abstraction, by taking the sg list,
> throwing the dma mapping away and assuming there's a struct page
> backing it. Would be good if we could stop spreading that abuse - the
> dma-buf interfaces have been modelled after the ttm bo interfaces, so
> shouldn't be too hard to wire this up correctly.
Ok. With virtio-gpu (where objects are backed by RAM pages anyway)
wiring this up should be easy.
But given there is no correct sample code I can look at it would be cool
if you could give some more hints how this is supposed to work. The
gem_prime_import_sg_table() callback gets a sg list passed in after all,
so I probably would have tried to take the sg list too ...
> > Importing into TTM_PL_VRAM: Impossible I think, without copying over
> > the data. Should that be done? If so, how? Or is it better to just
> > not support import then?
>
> Hm, since you ask about TTM concepts and not what this means in terms
> of dma-buf:
Ok, more details on the quesion:
dma-buf: whatever the driver gets passed into the
gem_prime_import_sg_table() callback.
import into TTM_PL_VRAM: qemu driver which supports VRAM storage only
(bochs, qxl), so the buffer has to be stored there if we want do
something with it (like scanning out to a crtc).
> As long as you upcast to the ttm_bo you can do whatever
> you want to really.
Well, if the dma-buf comes from another device (say export vgem bo, then
try import into bochs/qxl/virtio) I can't upcast.
When the dma-buf comes from the same device drm_gem_prime_import_dev()
will notice and take a shortcut (skip import, just increase refcount
instead), so I don't have to worry about that case in the
gem_prime_import_sg_table() callback.
> But with plain dma-buf this doesn't work right now
> (least because ttm assumes it gets system RAM on import, in theory you
> could put the peer2peer dma mapping into the sg list and it should
> work).
Well, qemu display devices don't have peer2peer dma support.
So I guess the answer is "doesn't work".
cheers,
Gerd
^ permalink raw reply
* Re: [PATCH v2 15/15] drm/bochs: reserve bo for pin/unpin
From: Daniel Vetter @ 2019-01-09 21:45 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Oleksandr Andrushchenko, open list, dri-devel,
open list:DRM DRIVER FOR BOCHS VIRTUAL GPU, David Airlie,
David Airlie
In-Reply-To: <20190109205158.qx7a2gfyprbvpok5@sirius.home.kraxel.org>
On Wed, Jan 9, 2019 at 9:52 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> Hi,
>
> > > If I understand things correctly it is valid to set all import/export
> > > callbacks (prime_handle_to_fd, prime_fd_to_handle,
> > > gem_prime_get_sg_table, gem_prime_import_sg_table) to NULL when not
> > > supporting dma-buf import/export and still advertise DRIVER_PRIME to
> > > indicate the other prime callbacks are supported (so generic fbdev
> > > emulation can use gem_prime_vmap etc). Is that correct?
> >
> > I'm not sure how much that's a good idea ... Never thought about it
> > tbh. All the fbdev/dma-buf stuff has plenty of hacks and
> > inconsistencies still, so I guess we can't make it much worse really.
>
> Setting prime_handle_to_fd + prime_fd_to_handle to NULL has the effect
> that drm stops advertising DRM_PRIME_CAP_{IMPORT,EXPORT} to userspace.
>
> Which looks better to me than telling userspace we support it then throw
> errors unconditionally when userspace tries to use that.
>
> > > Is it possible to export TTM_PL_VRAM objects (with backing storage being
> > > a pci memory bar)? If so, how?
> >
> > Not really in general. amdgpu upcasts to amdgpu_bo (if it's amgpu BO)
> > and then knows the internals so it can do a proper pci peer2peer
> > mapping. Or at least there's been lots of patches floating around to
> > make that happen.
>
> That is limited to bo sharing between two amdgpu devices, correct?
>
> > I think other drivers migrate the bo out of VRAM.
>
> Well, that doesn't look too useful. bochs and qxl virtual hardware
> can't access buffers outside VRAM. So, while I could migrate the
> buffers to RAM (via memcpy) when exporting they would at the same time
> become unusable for the GPU ...
>
> > > On importing:
> > >
> > > Importing into TTM_PL_TT object looks easy again, at least when the
> > > object is actually stored in RAM. What if not?
> >
> > They are all supposed to be stored in RAM. Note that all current ttm
> > importers totally break the abstraction, by taking the sg list,
> > throwing the dma mapping away and assuming there's a struct page
> > backing it. Would be good if we could stop spreading that abuse - the
> > dma-buf interfaces have been modelled after the ttm bo interfaces, so
> > shouldn't be too hard to wire this up correctly.
>
> Ok. With virtio-gpu (where objects are backed by RAM pages anyway)
> wiring this up should be easy.
>
> But given there is no correct sample code I can look at it would be cool
> if you could give some more hints how this is supposed to work. The
> gem_prime_import_sg_table() callback gets a sg list passed in after all,
> so I probably would have tried to take the sg list too ...
I'm not a fan of that helper either, that's really the broken part
imo. i915 doesn't use it. It's a midlayer so that the nvidia blob can
avoid directly touching the EXPORT_SYMBOL_GPL dma-buf symbols, afaiui
there's really no other solid reason for it. What the new gem cma
helpers does is imo much better (it still uses the import_sg_table
midlayer, but oh well).
For ttm you'd need to make sure that all the various ttm cpu side
access functions also all go through the relevant dma-buf interfaces,
and not through the struct page list fished out of the sgt. That was
at least the idea, long ago.
> > > Importing into TTM_PL_VRAM: Impossible I think, without copying over
> > > the data. Should that be done? If so, how? Or is it better to just
> > > not support import then?
> >
> > Hm, since you ask about TTM concepts and not what this means in terms
> > of dma-buf:
>
> Ok, more details on the quesion:
>
> dma-buf: whatever the driver gets passed into the
> gem_prime_import_sg_table() callback.
>
> import into TTM_PL_VRAM: qemu driver which supports VRAM storage only
> (bochs, qxl), so the buffer has to be stored there if we want do
> something with it (like scanning out to a crtc).
>
> > As long as you upcast to the ttm_bo you can do whatever
> > you want to really.
>
> Well, if the dma-buf comes from another device (say export vgem bo, then
> try import into bochs/qxl/virtio) I can't upcast.
In that case you'll in practice only get system RAM, and you're not
allowed to move it (dma-buf is meant to be zero-copy after all). If
your hw can't scan these out directly, then userspace needs to arrange
for a buffer copy into a native buffer somehow (that's how Xorg prime
works at least I think). No idea whether your virtual gpus can make
use of that directly. You might also get some pci peer2peer range in
the future, but it's strictly opt-in (because there's too many dma-buf
importers that just blindly assume there's a struct page behind the
sgt).
> When the dma-buf comes from the same device drm_gem_prime_import_dev()
> will notice and take a shortcut (skip import, just increase refcount
> instead), so I don't have to worry about that case in the
> gem_prime_import_sg_table() callback.
You can also upcast if it's from the same driver, not just same device.
-Daniel
> > But with plain dma-buf this doesn't work right now
> > (least because ttm assumes it gets system RAM on import, in theory you
> > could put the peer2peer dma mapping into the sg list and it should
> > work).
>
> Well, qemu display devices don't have peer2peer dma support.
> So I guess the answer is "doesn't work".
>
> cheers,
> Gerd
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH v3 0/5] kvm "virtio pmem" device
From: Dave Chinner @ 2019-01-10 1:26 UTC (permalink / raw)
To: Pankaj Gupta
Cc: jack, kvm, linux-nvdimm, qemu-devel, virtualization,
adilger.kernel, zwisler, eblake, dave.jiang, darrick.wong,
vishal.l.verma, mst, 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: <20190109144736.17452-1-pagupta@redhat.com>
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.
Hmmmm. Sharing the host page cache direct into the guest VM. Sounds
like a good idea, but.....
This means the guest VM can now run timing attacks to observe host
side page cache residency, and depending on the implementation I'm
guessing that the guest will be able to control host side page
cache eviction, too (e.g. via discard or hole punch operations).
Which means this functionality looks to me like a new vector for
information leakage into and out of the guest VM via guest
controlled host page cache manipulation.
https://arxiv.org/pdf/1901.01161
I might be wrong, but if I'm not we're going to have to be very
careful about how guest VMs can access and manipulate host side
resources like the page cache.....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ 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