[Xen-devel] [PATCH v2] xen/pciif: Clarify what values go in op->err and op->result.

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

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 (upstream & XenClassic)
code when doing XEN_PCI_OP_enable_msix can stash the -EXX in op->result
and in op->err, but they are also the only ones implementing this
operation.

Here is how it works right now with the XEN_PCI_OP:

- XEN_PCI_OP_conf_read and XEN_PCI_OP_conf_write
  it expects 'err' to contain XEN_PCI_ERR* values. And it converts them
  as it sees fit to -Exx.
  Note that NetBSD only implements XEN_PCI_OP_conf_write and
  XEN_PCI_OP_conf_read.

- For XEN_PCI_OP_enable_msi if 'err' has any value it will convert
  all of them to -EINVAL (Linux).

- For XEN_PCI_OP_disable_msix and XEN_PCI_OP_disable_msi it just
  reports the value (printk) and discards the 'err'.

- The XEN_PCI_OP_enable_msix differs on the frontend (classic Linux
  vs upstream).
  In Linux classic, if 'err' has any value it will convert all of them
  to '-EINVAL'.
  In Linux upstream it will convert the 'err' to uint32_t and pass it
  back up (to 'pci_enable_msi_range'). However due to the casting
  errors it ends up being 0xffffffffa (or such) and is useless.

  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 or ignores it.

Which means this patch will not break existing implementations and mandating
op->err to use XEN_PCI_ERR_* and stick in op->result -EXX if the
opcode wants it is the step in the right direction.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
v2: Update the commit with the discovery.
---
 xen/include/public/io/pciif.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/include/public/io/pciif.h b/xen/include/public/io/pciif.h
index a4ba13c..535963a 100644
--- a/xen/include/public/io/pciif.h
+++ b/xen/include/public/io/pciif.h
@@ -71,7 +71,7 @@ struct xen_pci_op {
     /* IN: what action to perform: XEN_PCI_OP_* */
     uint32_t cmd;
 
-    /* OUT: will contain an error number (if any) from errno.h */
+    /* OUT: will contain an XEN_PCI_ERR_* value. */
     int32_t err;
 
     /* IN: which device to touch */
@@ -83,7 +83,9 @@ struct xen_pci_op {
     int32_t offset;
     int32_t size;
 
-    /* IN/OUT: Contains the result after a READ or the value to WRITE */
+    /* IN/OUT: Contains the result after a READ or the value to WRITE.
+     * If the err does not have XEN_PCI_ERR_success, depending on
+     *  XEN_PCI_OP_* might have the errno value. */
     uint32_t value;
     /* IN: Contains extra infor for this operation */
     uint32_t info;
-- 
2.1.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel