Re: [PATCH v10 03/26] gunyah: Common types and error codes for Gunyah hypercalls

From: Elliot Berman
Date: Wed Mar 01 2023 - 20:41:46 EST




On 2/23/2023 1:58 PM, Alex Elder wrote:
On 2/14/23 3:12 PM, Elliot Berman wrote:
Add architecture-independent standard error codes, types, and macros for
Gunyah hypercalls.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
Signed-off-by: Elliot Berman <quic_eberman@xxxxxxxxxxx>
---
  include/linux/gunyah.h | 82 ++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 82 insertions(+)
  create mode 100644 include/linux/gunyah.h

diff --git a/include/linux/gunyah.h b/include/linux/gunyah.h
new file mode 100644
index 000000000000..59ef4c735ae8
--- /dev/null
+++ b/include/linux/gunyah.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _LINUX_GUNYAH_H
+#define _LINUX_GUNYAH_H
+
+#include <linux/errno.h>
+#include <linux/limits.h>
+
+/******************************************************************************/
+/* Common arch-independent definitions for Gunyah hypercalls                  */
+#define GH_CAPID_INVAL    U64_MAX
+#define GH_VMID_ROOT_VM    0xff
+
+enum gh_error {
+    GH_ERROR_OK            = 0,
+    GH_ERROR_UNIMPLEMENTED        = -1,
+    GH_ERROR_RETRY            = -2,

Do you expect this type to have a particular size?
Since you specify negative values, it matters, and
it's possible that this forces it to be a 4-byte value
(though I'm not sure what the rules are).  In other
words, UNIMPLEMENTED could conceivably have value 0xff
or 0xffffffff.  I'm not even sure you can tell whether
an enum is interpreted as signed or unsigned.

I'm not a C expert, but my understanding is that enums are signed. Gunyah will be returning a signed 64-bit register, however there's no intention to go beyond 32 bits of error codes since we want to work on 32-bit architectures.


It's not usually a good thing to do, but this *could*
be a case where you do a typedef to represent this as
a signed value of a certain bit width.  (But don't do
that unless someone else says that's worth doing.)

                    -Alex

+
+    GH_ERROR_ARG_INVAL        = 1,
+    GH_ERROR_ARG_SIZE        = 2,
+    GH_ERROR_ARG_ALIGN        = 3,
+
+    GH_ERROR_NOMEM            = 10,
+
+    GH_ERROR_ADDR_OVFL        = 20,
+    GH_ERROR_ADDR_UNFL        = 21,
+    GH_ERROR_ADDR_INVAL        = 22,
+
+    GH_ERROR_DENIED            = 30,
+    GH_ERROR_BUSY            = 31,
+    GH_ERROR_IDLE            = 32,
+
+    GH_ERROR_IRQ_BOUND        = 40,
+    GH_ERROR_IRQ_UNBOUND        = 41,
+
+    GH_ERROR_CSPACE_CAP_NULL    = 50,
+    GH_ERROR_CSPACE_CAP_REVOKED    = 51,
+    GH_ERROR_CSPACE_WRONG_OBJ_TYPE    = 52,
+    GH_ERROR_CSPACE_INSUF_RIGHTS    = 53,
+    GH_ERROR_CSPACE_FULL        = 54,
+
+    GH_ERROR_MSGQUEUE_EMPTY        = 60,
+    GH_ERROR_MSGQUEUE_FULL        = 61,
+};
+
+/**
+ * gh_remap_error() - Remap Gunyah hypervisor errors into a Linux error code
+ * @gh_error: Gunyah hypercall return value
+ */
+static inline int gh_remap_error(enum gh_error gh_error)
+{
+    switch (gh_error) {
+    case GH_ERROR_OK:
+        return 0;
+    case GH_ERROR_NOMEM:
+        return -ENOMEM;
+    case GH_ERROR_DENIED:
+    case GH_ERROR_CSPACE_CAP_NULL:
+    case GH_ERROR_CSPACE_CAP_REVOKED:
+    case GH_ERROR_CSPACE_WRONG_OBJ_TYPE:
+    case GH_ERROR_CSPACE_INSUF_RIGHTS:
+    case GH_ERROR_CSPACE_FULL:
+        return -EACCES;
+    case GH_ERROR_BUSY:
+    case GH_ERROR_IDLE:
+    case GH_ERROR_IRQ_BOUND:
+    case GH_ERROR_IRQ_UNBOUND:
+    case GH_ERROR_MSGQUEUE_FULL:
+    case GH_ERROR_MSGQUEUE_EMPTY:

Is an empty message queue really busy?


Changed to -EIO.

+        return -EBUSY;
+    case GH_ERROR_UNIMPLEMENTED:
+    case GH_ERROR_RETRY:
+        return -EOPNOTSUPP;
+    default:
+        return -EINVAL;
+    }
+}
+
+#endif