linux-kernel - Re: [PATCH 3/5] Intel MIC Host Driver Changes for Virtio Devices.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87li4fqy3h.fsf@rustcorp.com.au>
Date:	Tue, 06 Aug 2013 13:17:46 +0930
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Sudeep Dutt <sudeep.dutt@...el.com>,
	"Michael S. Tsirkin" <mst@...hat.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Arnd Bergmann <arnd@...db.de>, Rob Landley <rob@...dley.net>,
	linux-kernel@...r.kernel.org,
	virtualization@...ts.linux-foundation.org,
	linux-doc@...r.kernel.org, Nikhil Rao <nikhil.rao@...el.com>,
	Ashutosh Dixit <ashutosh.dixit@...el.com>,
	Caz Yokoyama <Caz.Yokoyama@...el.com>,
	Dasaratharaman Chandramouli 
	<dasaratharaman.chandramouli@...el.com>,
	Harshavardhan R Kharche <harshavardhan.r.kharche@...el.com>,
	"Yaozu \(Eddie\) Dong" <eddie.dong@...el.com>,
	Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@...el.com>,
	Sudeep Dutt <sudeep.dutt@...el.com>
Subject: Re: [PATCH 3/5] Intel MIC Host Driver Changes for Virtio Devices.

Sudeep Dutt <sudeep.dutt@...el.com> writes:
> On Mon, 2013-07-29 at 10:05 +0300, Michael S. Tsirkin wrote: 
>> On Wed, Jul 24, 2013 at 08:31:34PM -0700, Sudeep Dutt wrote:
>> > From: Ashutosh Dixit <ashutosh.dixit@...el.com>
>> > 
>> > This patch introduces the host "Virtio over PCIe" interface for
>> > Intel MIC. It allows creating user space backends on the host and
>> > instantiating virtio devices for them on the Intel MIC card. A character
>> > device per MIC is exposed with IOCTL, mmap and poll callbacks. This allows
>> > the user space backend to:
>> > (a) add/remove a virtio device via a device page.
>> > (b) map (R/O) virtio rings and device page to user space.
>> > (c) poll for availability of data.
>> > (d) copy a descriptor or entire descriptor chain to/from the card.
>> > (e) modify virtio configuration.
>> > (f) handle virtio device reset.
>> > The buffers are copied over using CPU copies for this initial patch
>> > and host initiated MIC DMA support is planned for future patches.
>> > The avail and desc virtio rings are in host memory and the used ring
>> > is in card memory to maximize writes across PCIe for performance.
>> > 
>> > Co-author: Sudeep Dutt <sudeep.dutt@...el.com>
>> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@...el.com>
>> > Signed-off-by: Caz Yokoyama <Caz.Yokoyama@...el.com>
>> > Signed-off-by: Dasaratharaman Chandramouli <dasaratharaman.chandramouli@...el.com>
>> > Signed-off-by: Nikhil Rao <nikhil.rao@...el.com>
>> > Signed-off-by: Harshavardhan R Kharche <harshavardhan.r.kharche@...el.com>
>> > Signed-off-by: Sudeep Dutt <sudeep.dutt@...el.com>
>> > Acked-by: Yaozu (Eddie) Dong <eddie.dong@...el.com>
>> > Reviewed-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@...el.com>
>> 
>> I decided to look at the security and ordering of ring accesses.
>> Doing a quick look, I think I found some issues, see comments below.
>> If it were possible to reuse existing ring handling code,
>> such issues would go away automatically.
>> Which brings me to the next question: have you looked at reusing
>> some code under drivers/vhost for host side processing?
>> If not, you probably should.
>> Is code in vringh.c generic enough to support your use-case,
>> and if not what exactly are the issues preventing this?
>> 
>> Thanks,
>> 
> We had implemented our custom MIC vring host access logic before the
> VRINGH infrastructure was merged to mainline in v3.10. Based on your
> feedback, we have a proof of concept implemented this week, by reusing
> the VRINGH infrastructure and it works nicely for us!

Nice!  Good suggestion MST, thanks for the plug :)

> One of our goals is to issue the  buffer transfers using DMA with future
> patches. The CPU copy in our current patches is also slightly different
> compared to VRINGH since we are copying from card buffers to user space
> and vice versa. In order to do meet these goals, we are obtaining the
> next available descriptor via vringh_getdesc_kern(..), then triggering
> the copy (CPU or eventually DMA) via a custom MIC API and then
> publishing the descriptor via vringh_complete_kern(..). Are there any
> plans of enhancing VRINGH to allow overriding the xfer mechanism in
> vringh_iov_xfer(..)? This will allow drivers with custom xfer routines
> to reuse APIs like vringh_iov_push_kern(..) and vringh_iov_pull_kern(..)
> as well. That said, the existing VRINGH infrastructure is generic enough
> for our use case as is today.

We'll have to look at exposing the internals.  vringh_iov_xfer() works
well because it's internal and inlined.  It'll be much easier to
evaluate when we're dealing with specific patches.

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/