NVDIMM Device and Persistent Memory development
 help / color / Atom feed
* [PATCH v2 2/2] virtio-pmem: Set DRIVER_OK status prior to creating pmem region
@ 2021-07-15 22:36 Taylor Stark
  2021-07-19 20:05 ` Pankaj Gupta
  0 siblings, 1 reply; 2+ messages in thread
From: Taylor Stark @ 2021-07-15 22:36 UTC (permalink / raw)
  To: dan.j.williams, vishal.l.verma, dave.jiang, ira.weiny
  Cc: nvdimm, apais, tyhicks, jamorris, benhill, sunilmut, grahamwo, tstark

Update virtio-pmem to call virtio_device_ready prior to creating the pmem
region. Otherwise, the guest may try to access the pmem region prior to
the DRIVER_OK status being set.

In the case of Hyper-V, the backing pmem file isn't mapped to the guest
until the DRIVER_OK status is set. Therefore, attempts to access the pmem
region can cause the guest to crash. Hyper-V could map the file earlier,
for example at VM creation, but we didn't want to pay the mapping cost if
the device is never used. Additionally, it felt weird to allow the guest
to access the region prior to the device fully coming online.

Signed-off-by: Taylor Stark <tstark@microsoft.com>
---
 drivers/nvdimm/virtio_pmem.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
index 43c1d835a449..ea9e111f3ea1 100644
--- a/drivers/nvdimm/virtio_pmem.c
+++ b/drivers/nvdimm/virtio_pmem.c
@@ -91,6 +91,11 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
 
 	dev_set_drvdata(&vdev->dev, vpmem->nvdimm_bus);
 
+	/* Online the device prior to creating a pmem region, to ensure that
+	 * the region is never touched while the device is offline.
+	 */
+	virtio_device_ready(vdev);
+
 	ndr_desc.res = &res;
 	ndr_desc.numa_node = nid;
 	ndr_desc.flush = async_pmem_flush;
@@ -105,6 +110,7 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
 	nd_region->provider_data = dev_to_virtio(nd_region->dev.parent->parent);
 	return 0;
 out_nd:
+	vdev->config->reset(vdev);
 	nvdimm_bus_unregister(vpmem->nvdimm_bus);
 out_vq:
 	vdev->config->del_vqs(vdev);
-- 
2.32.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2 2/2] virtio-pmem: Set DRIVER_OK status prior to creating pmem region
  2021-07-15 22:36 [PATCH v2 2/2] virtio-pmem: Set DRIVER_OK status prior to creating pmem region Taylor Stark
@ 2021-07-19 20:05 ` Pankaj Gupta
  0 siblings, 0 replies; 2+ messages in thread
From: Pankaj Gupta @ 2021-07-19 20:05 UTC (permalink / raw)
  To: Taylor Stark
  Cc: Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny, nvdimm, apais,
	tyhicks, jamorris, benhill, sunilmut, grahamwo, tstark,
	Michael S . Tsirkin

+CC Michael

> Update virtio-pmem to call virtio_device_ready prior to creating the pmem
> region. Otherwise, the guest may try to access the pmem region prior to
> the DRIVER_OK status being set.
>
> In the case of Hyper-V, the backing pmem file isn't mapped to the guest
> until the DRIVER_OK status is set. Therefore, attempts to access the pmem
> region can cause the guest to crash. Hyper-V could map the file earlier,
> for example at VM creation, but we didn't want to pay the mapping cost if
> the device is never used. Additionally, it felt weird to allow the guest
> to access the region prior to the device fully coming online.
>
> Signed-off-by: Taylor Stark <tstark@microsoft.com>
> ---
>  drivers/nvdimm/virtio_pmem.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
> index 43c1d835a449..ea9e111f3ea1 100644
> --- a/drivers/nvdimm/virtio_pmem.c
> +++ b/drivers/nvdimm/virtio_pmem.c
> @@ -91,6 +91,11 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
>
>         dev_set_drvdata(&vdev->dev, vpmem->nvdimm_bus);
>
> +       /* Online the device prior to creating a pmem region, to ensure that
> +        * the region is never touched while the device is offline.
> +        */
> +       virtio_device_ready(vdev);
> +
>         ndr_desc.res = &res;
>         ndr_desc.numa_node = nid;
>         ndr_desc.flush = async_pmem_flush;
> @@ -105,6 +110,7 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
>         nd_region->provider_data = dev_to_virtio(nd_region->dev.parent->parent);
>         return 0;
>  out_nd:
> +       vdev->config->reset(vdev);
>         nvdimm_bus_unregister(vpmem->nvdimm_bus);
>  out_vq:
>         vdev->config->del_vqs(vdev);
> --
> 2.32.0

Looks good to me, independent to the first patch.

Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 22:36 [PATCH v2 2/2] virtio-pmem: Set DRIVER_OK status prior to creating pmem region Taylor Stark
2021-07-19 20:05 ` Pankaj Gupta

NVDIMM Device and Persistent Memory development

Archives are clonable:
	git clone --mirror https://lore.kernel.org/nvdimm/0 nvdimm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 nvdimm nvdimm/ https://lore.kernel.org/nvdimm \
		nvdimm@lists.linux.dev
	public-inbox-index nvdimm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/dev.linux.lists.nvdimm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git