From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH] xen/pciif: Clarify what values go in op->err and op->result. Date: Wed, 15 Apr 2015 14:01:10 -0400 Message-ID: <20150415180110.GC15256@l.oracle.com> References: <1427813912-6593-1-git-send-email-konrad.wilk@oracle.com> <1427817923.2115.177.camel@citrix.com> <20150331162907.GA15146@l.oracle.com> <1429113916.15516.346.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1429113916.15516.346.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: ian.jackson@eu.citrix.com, tim@xen.org, keir@xen.org, jbeulich@suse.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Wed, Apr 15, 2015 at 05:05:16PM +0100, Ian Campbell wrote: > On Tue, 2015-03-31 at 12:29 -0400, Konrad Rzeszutek Wilk wrote: > > On Tue, Mar 31, 2015 at 05:05:23PM +0100, Ian Campbell wrote: > > > On Tue, 2015-03-31 at 10:58 -0400, Konrad Rzeszutek Wilk wrote: > > > > The earlier comment says that errno values go in op->err. > > > > However all implementations (NetBSD, Linux) of the most > > > > common operations use XEN_PCI_ERR_* instead of -EXX values. > > > > > > > > The exception is the xen-pciback in Linux code when doing > > > > XEN_PCI_OP_enable_msix can stash the -EXX in op->result > > > > and in op->err. > > > > > > i.e. both of them contain the same thing? How unhelpful! > > > > > > What would be the impact of "correcting" ->result to do the right thing? > > > (as documented below after this patch). > > > > Ugh. The frontend (Linux) first checks op->err. If it is non-zero > > then it returns op->err back up. If op->err is zero > > but op->result is non-zero, then it returns op->result up the > > stack. > > > > The 'stack' differs depending on what XEN_PCI_OP it is. > > > > For XEN_PCI_OP_conf_read and XEN_PCI_OP_conf_write > > it expects 'err' to contain XEN_PCI_ERR* values. And it converts them. > > > > In upstream Linux: > > The XEN_PCI_OP_enable_msix it expects 'err' to contain > > -EXX values. Which means that whoever called 'pci_enable_msi_range' will > > get the 'err' value. > > > > In Linux 2.6.18, if 'err' has any value it will convert all of them > > to '-EINVAL'. > > > > For XEN_PCI_OP_enable_msi if 'err' has any value it will convert > > all of them to -EINVAL. > > > > For XEN_PCI_OP_disable_msix and XEN_PCI_OP_disable_msi it just > > reports the value. > > > > NetBSD only implements XEN_PCI_OP_conf_write and XEN_PCI_OP_conf_read. > > > > It looks to me that the upstream Linux kernel frontend driver needs > > to do what the linux-2.6.18 does (return -EINVAL if there are any errors). > > So what are the next steps? Patches to other things? What about this > one, should I expect a new version, drop it or apply it? I neglected to mention that the upstream frontend driver will end up converting an uint32_t to int, with the end result that the error is actually 0xffffffffa (or such). Which means that it really does not matter what (-EXX or XEN_PCI_ERR_*) or where (op->err or op->result) the backend stashes it as the frontend screws it up. Which makes me comfortable in proposing this patch that mandates op->err to use XEN_PCI_ERR_* and stick in op->result -EXX if the opcode wants it. It won't affect how the frontend deals with it as with even that change it will still return 0xfffff.. on failures. In short, I will repost this patch and include this long rant in it. > >