Re: [PATCH] add POWER Virtual Management Channel driver

[Steven Royer <seroyer@linux.vnet.ibm.com>]

On 2016-02-17 16:31, Greg Kroah-Hartman wrote:
> On Wed, Feb 17, 2016 at 03:18:26PM -0600, Steven Royer wrote:
>> On 2016-02-16 16:18, Greg Kroah-Hartman wrote:
>> >On Tue, Feb 16, 2016 at 02:43:13PM -0600, Steven Royer wrote:
>> >>From: Steven Royer <seroyer@us.ibm.com>
>> >>
>> >>The ibmvmc driver is a device driver for the POWER Virtual Management
>> >>Channel virtual adapter on the PowerVM platform.  It is used to
>> >>communicate with the hypervisor for virtualization management.  It
>> >>provides both request/response and asynchronous message support through
>> >>the /dev/ibmvmc node.
>> >
>> >What is the protocol for that device node?
>> The protocol is not currently published.  I am pushing on getting it
>> published, but that process will take time.  If you have a PowerVM 
>> system
>> with NovaLink, it would not be hard to reverse engineer it...  If you 
>> don't
>> have a PowerVM system, then this driver isn't interesting anyway...
> 
> You can't just expect us to review this code without at least having a
> clue as to how it is supposed to work?
There are two layers to the protocol.  The first layer is the only layer 
that the driver actually cares about.  The second layer is just a 
payload that is between the application and the hypervisor and can 
change independently from the kernel/driver (this is what is transported 
over the /dev/ibmvmc node).  The first layer technically is published in 
the PAPR (appendix G), but it is not trivial for most people to access 
online.  I'll put together some documentation that describes that first 
layer of the protocol in my next revision of the patch.  In many 
respects, the interface between driver and hypervisor is similar to 
ibmvscsi.  Both are CRQ based devices.  ibmvmc is actually a little 
closer to the old ibmvstgt driver since it is a CRQ server device.
> 
>> >Where is the documentation here?  Why does this have to be a character
>> >device?  Why can't it fit in with other drivers of this type?
>> This is a character device for historical reasons.  The short version 
>> is
>> that this driver is a clean-room rewrite of an AIX driver which made 
>> it a
>> character device.  The user space application was ported from AIX to 
>> Linux
>> and it is convenient to have the AIX and Linux drivers match behavior 
>> where
>> possible.
> 
> Note that we don't let random userspace applications dictate kernel api
> decisions, please make the best choice for this interface without being
> influenced by AIX.
That is fair.  I actually started down the path of using a block 
interface early on, and I ran into some complications that made it seem 
less desirable than character.  Specifically the interface has variable 
length messages from 32 bytes to 4kb (mostly closer to 32 bytes than 4kb 
per message) and I was worried about overhead of dealing with all the 
zeros in the majority of the messages.  I've never made a block 
interface before and it's entirely possible (likely) I missed something 
obvious.  I'll revisit before I post the next revision.
> 
>> >>+/*
>> >>+ * IBM Power Systems Virtual Management Channel Support.
>> >>+ *
>> >>+ * Copyright (c) 2004, 2016 IBM Corp.
>> >>+ *   Dave Engebretsen engebret@us.ibm.com
>> >>+ *   Steven Royer seroyer@linux.vnet.ibm.com
>> >>+ *   Adam Reznechek adreznec@linux.vnet.ibm.com
>> >>+ *
>> >>+ * This program is free software; you can redistribute it and/or
>> >>+ * modify it under the terms of the GNU General Public License
>> >>+ * as published by the Free Software Foundation; either version 2
>> >>+ * of the License, or (at your option) any later version.
>> >
>> >I have to ask, but do you really mean "or any later version"?
>> This actually matches closely to other similar PowerVM virtual device
>> drivers, like ibmvscsi or ibmveth.
> 
> That did not answer the question, picking a license in a cargo-cult
> manner is not a wise decision :(
This is boilerplate for IBM provided PowerVM drivers.  So yes, I did 
mean this.