From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4D5FCC04AB4 for ; Thu, 16 May 2019 07:49:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 2721C20862 for ; Thu, 16 May 2019 07:49:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726948AbfEPHtH (ORCPT ); Thu, 16 May 2019 03:49:07 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55050 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726513AbfEPHtH (ORCPT ); Thu, 16 May 2019 03:49:07 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 394FD10C94; Thu, 16 May 2019 07:49:06 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 12CC660600; Thu, 16 May 2019 07:49:06 +0000 (UTC) Received: from zmail21.collab.prod.int.phx2.redhat.com (zmail21.collab.prod.int.phx2.redhat.com [10.5.83.24]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 8AF211806B11; Thu, 16 May 2019 07:49:05 +0000 (UTC) Date: Thu, 16 May 2019 03:49:05 -0400 (EDT) From: Pankaj Gupta To: David Hildenbrand Cc: linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, kvm@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-acpi@vger.kernel.org, qemu-devel@nongnu.org, linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org, mst@redhat.com, dan j williams , zwisler@kernel.org, vishal l verma , dave jiang , jasowang@redhat.com, willy@infradead.org, rjw@rjwysocki.net, hch@infradead.org, lenb@kernel.org, jack@suse.cz, tytso@mit.edu, adilger kernel , darrick wong , lcapitulino@redhat.com, kwolf@redhat.com, imammedo@redhat.com, jmoyer@redhat.com, nilal@redhat.com, riel@surriel.com, stefanha@redhat.com, aarcange@redhat.com, david@fromorbit.com, cohuck@redhat.com, xiaoguangrong eric , pbonzini@redhat.com, kilobyte@angband.pl, yuval shaia , jstaron@google.com Message-ID: <1441757090.29182398.1557992945173.JavaMail.zimbra@redhat.com> In-Reply-To: <9f6b1d8e-ef90-7d8b-56da-61a426953ba3@redhat.com> References: <20190514145422.16923-1-pagupta@redhat.com> <20190514145422.16923-3-pagupta@redhat.com> <9f6b1d8e-ef90-7d8b-56da-61a426953ba3@redhat.com> Subject: Re: [PATCH v9 2/7] virtio-pmem: Add virtio pmem driver MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [10.65.16.97, 10.4.195.30] Thread-Topic: virtio-pmem: Add virtio pmem driver Thread-Index: au4HbQEd/vcK+r9shsETeRHx6YmHRw== X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Thu, 16 May 2019 07:49:06 +0000 (UTC) Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org > > > + vpmem->vdev = vdev; > > + vdev->priv = vpmem; > > + err = init_vq(vpmem); > > + if (err) { > > + dev_err(&vdev->dev, "failed to initialize virtio pmem vq's\n"); > > + 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; > > nit: " - 1;" Sure. > > > + vpmem->nd_desc.provider_name = "virtio-pmem"; > > + vpmem->nd_desc.module = THIS_MODULE; > > + > > + vpmem->nvdimm_bus = nvdimm_bus_register(&vdev->dev, > > + &vpmem->nd_desc); > > + if (!vpmem->nvdimm_bus) { > > + dev_err(&vdev->dev, "failed to register device with nvdimm_bus\n"); > > + err = -ENXIO; > > + goto out_vq; > > + } > > + > > + dev_set_drvdata(&vdev->dev, vpmem->nvdimm_bus); > > + > > + ndr_desc.res = &res; > > + ndr_desc.numa_node = nid; > > + ndr_desc.flush = async_pmem_flush; > > + set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags); > > + set_bit(ND_REGION_ASYNC, &ndr_desc.flags); > > + nd_region = nvdimm_pmem_region_create(vpmem->nvdimm_bus, &ndr_desc); > > + if (!nd_region) { > > + dev_err(&vdev->dev, "failed to create nvdimm region\n"); > > + err = -ENXIO; > > + goto out_nd; > > + } > > + nd_region->provider_data = dev_to_virtio(nd_region->dev.parent->parent); > > + return 0; > > +out_nd: > > + nvdimm_bus_unregister(vpmem->nvdimm_bus); > > +out_vq: > > + vdev->config->del_vqs(vdev); > > +out_err: > > + return err; > > +} > > + > > +static void virtio_pmem_remove(struct virtio_device *vdev) > > +{ > > + struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(&vdev->dev); > > + > > + nvdimm_bus_unregister(nvdimm_bus); > > + vdev->config->del_vqs(vdev); > > + vdev->config->reset(vdev); > > +} > > + > > +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/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h > > new file mode 100644 > > index 000000000000..ab1da877575d > > --- /dev/null > > +++ b/drivers/nvdimm/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 > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +struct virtio_pmem_request { > > + /* Host return status corresponding to flush request */ > > + int ret; > > + > > + /* command name*/ > > + char name[16]; > > So ... why are we sending string commands and expect native-endianess > integers and don't define a proper request/response structure + request > types in include/uapi/linux/virtio_pmem.h like > > struct virtio_pmem_resp { > __virtio32 ret; > } > > #define VIRTIO_PMEM_REQ_TYPE_FLUSH 1 > struct virtio_pmem_req { > __virtio16 type; > } > > ... and this way we also define a proper endianess format for exchange > and keep it extensible Done the suggested change. Thank you, Pankaj > > @MST, what's your take on this? > > > -- > > Thanks, > > David / dhildenb >