diff mbox series

[v5,03/13] gunyah: Common types and error codes for Gunyah hypercalls

Message ID 20221011000840.289033-4-quic_eberman@quicinc.com
State Superseded
Headers show
Series Drivers for gunyah hypervisor | expand

Commit Message

Elliot Berman Oct. 11, 2022, 12:08 a.m. UTC
Add architecture-independent standard error codes, types, and macros for
Gunyah hypercalls.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 MAINTAINERS                  |  1 +
 include/asm-generic/gunyah.h | 74 ++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 include/asm-generic/gunyah.h

Comments

Elliot Berman Oct. 11, 2022, 6:21 p.m. UTC | #1
On 10/11/2022 12:21 AM, Greg Kroah-Hartman wrote:
> On Mon, Oct 10, 2022 at 05:08:30PM -0700, Elliot Berman wrote:
>> Add architecture-independent standard error codes, types, and macros for
>> Gunyah hypercalls.
>>
>> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
>> ---
>>   MAINTAINERS                  |  1 +
>>   include/asm-generic/gunyah.h | 74 ++++++++++++++++++++++++++++++++++++
>>   2 files changed, 75 insertions(+)
>>   create mode 100644 include/asm-generic/gunyah.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index ef6de7599d98..4fe8cec61551 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8886,6 +8886,7 @@ L:	linux-arm-msm@vger.kernel.org
>>   S:	Supported
>>   F:	Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
>>   F:	Documentation/virt/gunyah/
>> +F:	include/asm-generic/gunyah.h
>>   
>>   HABANALABS PCI DRIVER
>>   M:	Oded Gabbay <ogabbay@kernel.org>
>> diff --git a/include/asm-generic/gunyah.h b/include/asm-generic/gunyah.h
>> new file mode 100644
>> index 000000000000..64a02dd3b5ad
>> --- /dev/null
>> +++ b/include/asm-generic/gunyah.h
> 
> Why not include/linux/gunyah.h?  Why asm-generic?  This is not an
> architecture.
> 

My idea here is to differentiate between code that interacts with 
hypercalls and code that uses the abstractions provided on top of those 
hypercalls. include/asm-generic/gunyah.h contains 
architecture-independent definitions for hypercalls. Hypercalls are 
architecture-specific.

For instance, I wanted to avoid a header file that mixes the definitions 
for the message-queue mailbox with the hypercall definitions that the 
message-queue mailbox driver itself uses.

I can put it all in include/linux/gunyah.h and delineate with some clear 
comments, but I initially felt it would be better to have separate 
header file.

Thanks,
Elliot
Greg Kroah-Hartman Oct. 11, 2022, 6:48 p.m. UTC | #2
On Tue, Oct 11, 2022 at 11:21:36AM -0700, Elliot Berman wrote:
> 
> 
> On 10/11/2022 12:21 AM, Greg Kroah-Hartman wrote:
> > On Mon, Oct 10, 2022 at 05:08:30PM -0700, Elliot Berman wrote:
> > > Add architecture-independent standard error codes, types, and macros for
> > > Gunyah hypercalls.
> > > 
> > > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> > > ---
> > >   MAINTAINERS                  |  1 +
> > >   include/asm-generic/gunyah.h | 74 ++++++++++++++++++++++++++++++++++++
> > >   2 files changed, 75 insertions(+)
> > >   create mode 100644 include/asm-generic/gunyah.h
> > > 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index ef6de7599d98..4fe8cec61551 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -8886,6 +8886,7 @@ L:	linux-arm-msm@vger.kernel.org
> > >   S:	Supported
> > >   F:	Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
> > >   F:	Documentation/virt/gunyah/
> > > +F:	include/asm-generic/gunyah.h
> > >   HABANALABS PCI DRIVER
> > >   M:	Oded Gabbay <ogabbay@kernel.org>
> > > diff --git a/include/asm-generic/gunyah.h b/include/asm-generic/gunyah.h
> > > new file mode 100644
> > > index 000000000000..64a02dd3b5ad
> > > --- /dev/null
> > > +++ b/include/asm-generic/gunyah.h
> > 
> > Why not include/linux/gunyah.h?  Why asm-generic?  This is not an
> > architecture.
> > 
> 
> My idea here is to differentiate between code that interacts with hypercalls
> and code that uses the abstractions provided on top of those hypercalls.
> include/asm-generic/gunyah.h contains architecture-independent definitions
> for hypercalls. Hypercalls are architecture-specific.
> 
> For instance, I wanted to avoid a header file that mixes the definitions for
> the message-queue mailbox with the hypercall definitions that the
> message-queue mailbox driver itself uses.
> 
> I can put it all in include/linux/gunyah.h and delineate with some clear
> comments, but I initially felt it would be better to have separate header
> file.

Please put it all in one place, this is just one tiny driver and should
not abuse the asm-generic location at all, no one is only going to want
just this one file, they are going to need the whole thing or nothing.

thanks,

greg k-h
Trilok Soni Oct. 11, 2022, 6:50 p.m. UTC | #3
On 10/11/2022 11:48 AM, Greg Kroah-Hartman wrote:
> On Tue, Oct 11, 2022 at 11:21:36AM -0700, Elliot Berman wrote:
>>
>>
>> On 10/11/2022 12:21 AM, Greg Kroah-Hartman wrote:
>>> On Mon, Oct 10, 2022 at 05:08:30PM -0700, Elliot Berman wrote:
>>>> Add architecture-independent standard error codes, types, and macros for
>>>> Gunyah hypercalls.
>>>>
>>>> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
>>>> ---
>>>>    MAINTAINERS                  |  1 +
>>>>    include/asm-generic/gunyah.h | 74 ++++++++++++++++++++++++++++++++++++
>>>>    2 files changed, 75 insertions(+)
>>>>    create mode 100644 include/asm-generic/gunyah.h
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index ef6de7599d98..4fe8cec61551 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -8886,6 +8886,7 @@ L:	linux-arm-msm@vger.kernel.org
>>>>    S:	Supported
>>>>    F:	Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
>>>>    F:	Documentation/virt/gunyah/
>>>> +F:	include/asm-generic/gunyah.h
>>>>    HABANALABS PCI DRIVER
>>>>    M:	Oded Gabbay <ogabbay@kernel.org>
>>>> diff --git a/include/asm-generic/gunyah.h b/include/asm-generic/gunyah.h
>>>> new file mode 100644
>>>> index 000000000000..64a02dd3b5ad
>>>> --- /dev/null
>>>> +++ b/include/asm-generic/gunyah.h
>>>
>>> Why not include/linux/gunyah.h?  Why asm-generic?  This is not an
>>> architecture.
>>>
>>
>> My idea here is to differentiate between code that interacts with hypercalls
>> and code that uses the abstractions provided on top of those hypercalls.
>> include/asm-generic/gunyah.h contains architecture-independent definitions
>> for hypercalls. Hypercalls are architecture-specific.
>>
>> For instance, I wanted to avoid a header file that mixes the definitions for
>> the message-queue mailbox with the hypercall definitions that the
>> message-queue mailbox driver itself uses.
>>
>> I can put it all in include/linux/gunyah.h and delineate with some clear
>> comments, but I initially felt it would be better to have separate header
>> file.
> 
> Please put it all in one place, this is just one tiny driver and should
> not abuse the asm-generic location at all, no one is only going to want
> just this one file, they are going to need the whole thing or nothing.
> 

Let's say when we do the RISC-V port for Gunyah, we may need to move it 
back to asm-generic then?

---Trilok Soni
Greg Kroah-Hartman Oct. 11, 2022, 7:01 p.m. UTC | #4
On Tue, Oct 11, 2022 at 11:50:04AM -0700, Trilok Soni wrote:
> On 10/11/2022 11:48 AM, Greg Kroah-Hartman wrote:
> > On Tue, Oct 11, 2022 at 11:21:36AM -0700, Elliot Berman wrote:
> > > 
> > > 
> > > On 10/11/2022 12:21 AM, Greg Kroah-Hartman wrote:
> > > > On Mon, Oct 10, 2022 at 05:08:30PM -0700, Elliot Berman wrote:
> > > > > Add architecture-independent standard error codes, types, and macros for
> > > > > Gunyah hypercalls.
> > > > > 
> > > > > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> > > > > ---
> > > > >    MAINTAINERS                  |  1 +
> > > > >    include/asm-generic/gunyah.h | 74 ++++++++++++++++++++++++++++++++++++
> > > > >    2 files changed, 75 insertions(+)
> > > > >    create mode 100644 include/asm-generic/gunyah.h
> > > > > 
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index ef6de7599d98..4fe8cec61551 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -8886,6 +8886,7 @@ L:	linux-arm-msm@vger.kernel.org
> > > > >    S:	Supported
> > > > >    F:	Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
> > > > >    F:	Documentation/virt/gunyah/
> > > > > +F:	include/asm-generic/gunyah.h
> > > > >    HABANALABS PCI DRIVER
> > > > >    M:	Oded Gabbay <ogabbay@kernel.org>
> > > > > diff --git a/include/asm-generic/gunyah.h b/include/asm-generic/gunyah.h
> > > > > new file mode 100644
> > > > > index 000000000000..64a02dd3b5ad
> > > > > --- /dev/null
> > > > > +++ b/include/asm-generic/gunyah.h
> > > > 
> > > > Why not include/linux/gunyah.h?  Why asm-generic?  This is not an
> > > > architecture.
> > > > 
> > > 
> > > My idea here is to differentiate between code that interacts with hypercalls
> > > and code that uses the abstractions provided on top of those hypercalls.
> > > include/asm-generic/gunyah.h contains architecture-independent definitions
> > > for hypercalls. Hypercalls are architecture-specific.
> > > 
> > > For instance, I wanted to avoid a header file that mixes the definitions for
> > > the message-queue mailbox with the hypercall definitions that the
> > > message-queue mailbox driver itself uses.
> > > 
> > > I can put it all in include/linux/gunyah.h and delineate with some clear
> > > comments, but I initially felt it would be better to have separate header
> > > file.
> > 
> > Please put it all in one place, this is just one tiny driver and should
> > not abuse the asm-generic location at all, no one is only going to want
> > just this one file, they are going to need the whole thing or nothing.
> > 
> 
> Let's say when we do the RISC-V port for Gunyah, we may need to move it back
> to asm-generic then?

If that really happens and the things are arch-specific, yes, we can
worry about that then.  You know better than this, we only do what is
needed now.  We do not add code, or make splits like this, when it is
not needed today.

Keep it simple first, you want to get this merged first, and then you
can iterate on it to make it complex and messy :)

thanks,

greg k-h
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index ef6de7599d98..4fe8cec61551 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8886,6 +8886,7 @@  L:	linux-arm-msm@vger.kernel.org
 S:	Supported
 F:	Documentation/devicetree/bindings/firmware/gunyah-hypervisor.yaml
 F:	Documentation/virt/gunyah/
+F:	include/asm-generic/gunyah.h
 
 HABANALABS PCI DRIVER
 M:	Oded Gabbay <ogabbay@kernel.org>
diff --git a/include/asm-generic/gunyah.h b/include/asm-generic/gunyah.h
new file mode 100644
index 000000000000..64a02dd3b5ad
--- /dev/null
+++ b/include/asm-generic/gunyah.h
@@ -0,0 +1,74 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _ASM_GUNYAH_H
+#define _ASM_GUNYAH_H
+
+#include <linux/types.h>
+#include <linux/errno.h>
+
+/* Common Gunyah macros */
+#define GH_CAPID_INVAL	U64_MAX
+#define GH_VMID_ROOT_VM	0xff
+
+#define GH_ERROR_OK			0
+
+#define GH_ERROR_UNIMPLEMENTED		-1
+#define GH_ERROR_RETRY			-2
+
+#define GH_ERROR_ARG_INVAL		1
+#define GH_ERROR_ARG_SIZE		2
+#define GH_ERROR_ARG_ALIGN		3
+
+#define GH_ERROR_NOMEM			10
+
+#define GH_ERROR_ADDR_OVFL		20
+#define GH_ERROR_ADDR_UNFL		21
+#define GH_ERROR_ADDR_INVAL		22
+
+#define GH_ERROR_DENIED			30
+#define GH_ERROR_BUSY			31
+#define GH_ERROR_IDLE			32
+
+#define GH_ERROR_IRQ_BOUND		40
+#define GH_ERROR_IRQ_UNBOUND		41
+
+#define GH_ERROR_CSPACE_CAP_NULL	50
+#define GH_ERROR_CSPACE_CAP_REVOKED	51
+#define GH_ERROR_CSPACE_WRONG_OBJ_TYPE	52
+#define GH_ERROR_CSPACE_INSUF_RIGHTS	53
+#define GH_ERROR_CSPACE_FULL		54
+
+#define GH_ERROR_MSGQUEUE_EMPTY		60
+#define GH_ERROR_MSGQUEUE_FULL		61
+
+static inline int gh_remap_error(int 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:
+		return -EBUSY;
+	case GH_ERROR_IRQ_BOUND:
+	case GH_ERROR_IRQ_UNBOUND:
+	case GH_ERROR_MSGQUEUE_FULL:
+	case GH_ERROR_MSGQUEUE_EMPTY:
+		return -EPERM;
+	default:
+		return -EINVAL;
+	}
+}
+
+#endif