From mboxrd@z Thu Jan 1 00:00:00 1970 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=0+ML9D8t0qHLLc0IkFSgl4VquOhXBGW3wDjpNV1Dr4Y=; b=GzOoXN6R0YwtVm9WdlSPJSM79+9WNpzWlc67UUXi1x5ICXqL5P8zbyYvxOUWNrR3mS Y09eTbzgjxUe62qDkZeRSOe5PVpcNs2Ep67LSc+NWFFRm9NOq+3gytkRw3odi8Rdnwbb eoMKKx/J/LN5PUy0AN+ppxn4boRqV7zZB7rsSviODQ0IILPUmb/KtqGN8Q976XfTuA8X bpukzVrFMjgDQNkjp45VoE0UkGa3JhidUSyp5RXJtLMf4S0U+bY5TtnUW440sNaUD5DH wr1++BD1e5cF3k2dNUFrzNgtpEhAiufcK4LIGX78nUfBHb0B14uLdZ9JTJCet78R6Tzn MG6g== MIME-Version: 1.0 References: <20200807195526.426056-1-vgoyal@redhat.com> <20200807195526.426056-5-vgoyal@redhat.com> <20200810100327-mutt-send-email-mst@kernel.org> <20200810145019.GB455528@redhat.com> In-Reply-To: <20200810145019.GB455528@redhat.com> From: Gurchetan Singh Date: Thu, 13 Aug 2020 19:51:56 -0700 Message-ID: Content-Type: multipart/alternative; boundary="000000000000bd1d2c05accd8162" Subject: Re: [Virtio-fs] [PATCH v2 04/20] virtio: Implement get_shm_region for PCI transport List-Id: Development discussions about virtio-fs List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vivek Goyal Cc: kbuild test robot , kvm@vger.kernel.org, miklos@szeredi.hu, "Michael S. Tsirkin" , open list , virtio-fs@redhat.com, linux-fsdevel@vger.kernel.org --000000000000bd1d2c05accd8162 Content-Type: text/plain; charset="UTF-8" On Mon, Aug 10, 2020 at 7:50 AM Vivek Goyal wrote: > On Mon, Aug 10, 2020 at 10:05:17AM -0400, Michael S. Tsirkin wrote: > > On Fri, Aug 07, 2020 at 03:55:10PM -0400, Vivek Goyal wrote: > > > From: Sebastien Boeuf > > > > > > On PCI the shm regions are found using capability entries; > > > find a region by searching for the capability. > > > > > > Signed-off-by: Sebastien Boeuf > > > Signed-off-by: Dr. David Alan Gilbert > > > Signed-off-by: kbuild test robot > > > Cc: kvm@vger.kernel.org > > > Cc: "Michael S. Tsirkin" > > > > Acked-by: Michael S. Tsirkin > > > > [..] > > > +static bool vp_get_shm_region(struct virtio_device *vdev, > > > + struct virtio_shm_region *region, u8 id) > > > +{ > > > + struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > > + struct pci_dev *pci_dev = vp_dev->pci_dev; > > > + u8 bar; > > > + u64 offset, len; > > > + phys_addr_t phys_addr; > > > + size_t bar_len; > > > + > > > + if (!virtio_pci_find_shm_cap(pci_dev, id, &bar, &offset, &len)) { > > > + return false; > > > + } > > > + > > > + phys_addr = pci_resource_start(pci_dev, bar); > > > + bar_len = pci_resource_len(pci_dev, bar); > > > + > > > + if ((offset + len) < offset) { > > > + dev_err(&pci_dev->dev, "%s: cap offset+len overflow > detected\n", > > > + __func__); > > > + return false; > > > + } > > > + > > > + if (offset + len > bar_len) { > > > + dev_err(&pci_dev->dev, "%s: bar shorter than cap > offset+len\n", > > > + __func__); > > > + return false; > > > + } > > > > Maybe move this to a common header so the checks can be reused by > > other transports? Can be a patch on top. > > Will do as patch on top once these patches get merged. > There's also some minor checkpatch complaints. I fixed them with the series I sent out with virtio-gpu. > > Thanks > Vivek > > --000000000000bd1d2c05accd8162 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Mon, Aug 10, 2020 at 7:50 AM Vivek= Goyal <vgoyal@redhat.com> w= rote:
On Mon, Au= g 10, 2020 at 10:05:17AM -0400, Michael S. Tsirkin wrote:
> On Fri, Aug 07, 2020 at 03:55:10PM -0400, Vivek Goyal wrote:
> > From: Sebastien Boeuf <sebastien.boeuf@intel.com>
> >
> > On PCI the shm regions are found using capability entries;
> > find a region by searching for the capability.
> >
> > Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Signed-off-by: kbuild test robot <lkp@intel.com>
> > Cc: kvm@= vger.kernel.org
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>

[..]
> > +static bool vp_get_shm_region(struct virtio_device *vdev,
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0struct virtio_shm_region *region, u8 id)
> > +{
> > +=C2=A0 =C2=A0struct virtio_pci_device *vp_dev =3D to_vp_device(v= dev);
> > +=C2=A0 =C2=A0struct pci_dev *pci_dev =3D vp_dev->pci_dev;
> > +=C2=A0 =C2=A0u8 bar;
> > +=C2=A0 =C2=A0u64 offset, len;
> > +=C2=A0 =C2=A0phys_addr_t phys_addr;
> > +=C2=A0 =C2=A0size_t bar_len;
> > +
> > +=C2=A0 =C2=A0if (!virtio_pci_find_shm_cap(pci_dev, id, &bar,= &offset, &len)) {
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return false;
> > +=C2=A0 =C2=A0}
> > +
> > +=C2=A0 =C2=A0phys_addr =3D pci_resource_start(pci_dev, bar);
> > +=C2=A0 =C2=A0bar_len =3D pci_resource_len(pci_dev, bar);
> > +
> > +=C2=A0 =C2=A0if ((offset + len) < offset) {
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev_err(&pci_dev-&g= t;dev, "%s: cap offset+len overflow detected\n",
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0__func__);
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return false;
> > +=C2=A0 =C2=A0}
> > +
> > +=C2=A0 =C2=A0if (offset + len > bar_len) {
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev_err(&pci_dev-&g= t;dev, "%s: bar shorter than cap offset+len\n",
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0__func__);
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return false;
> > +=C2=A0 =C2=A0}
>
> Maybe move this to a common header so the checks can be reused by
> other transports? Can be a patch on top.

Will do as patch on top once these patches get merged.

There's also some minor checkpatch complaints.=C2=A0 I= fixed them with the series I sent out with virtio-gpu.
=C2=A0

Thanks
Vivek

--000000000000bd1d2c05accd8162--