On 3/31/2023 7:26 AM, Alex Elder wrote:
On 3/3/23 7:06 PM, Elliot Berman wrote:
When booting a Gunyah virtual machine, the host VM may gain capabilities
to interact with resources for the guest virtual machine. Examples of
such resources are vCPUs or message queues. To use those resources, we
need to translate the RM response into a gunyah_resource structure which
are useful to Linux drivers. Presently, Linux drivers need only to know
the type of resource, the capability ID, and an interrupt.
On ARM64 systems, the interrupt reported by Gunyah is the GIC interrupt
ID number and always a SPI.
Signed-off-by: Elliot Berman <quic_eberman@xxxxxxxxxxx>
Several comments here, nothing major. -Alex
---
arch/arm64/include/asm/gunyah.h | 23 +++++
drivers/virt/gunyah/rsc_mgr.c | 163 +++++++++++++++++++++++++++++++-
include/linux/gunyah.h | 4 +
include/linux/gunyah_rsc_mgr.h | 3 +
4 files changed, 192 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/gunyah.h
diff --git a/arch/arm64/include/asm/gunyah.h
b/arch/arm64/include/asm/gunyah.h
new file mode 100644
index 000000000000..64cfb964efee
--- /dev/null
+++ b/arch/arm64/include/asm/gunyah.h
@@ -0,0 +1,23 @@
+/* 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_
Maybe just one _ at the beginning and none at the end?
Follow the same convention across all your header files.
(Maybe you're looking at other files in the same directory
as this one, but that's not consistent.)
+
+#include <linux/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+static inline int arch_gh_fill_irq_fwspec_params(u32 virq, struct
irq_fwspec *fwspec)
+{
+ if (virq < 32 || virq > 1019)
+ return -EINVAL;
What is special about VIRQs greater than 1019 (minus 32)?
It's probably documented somewhere but it's worth adding a
comment here to explain the check.
You would know better than I, but could/should the caller
be responsible for this check? (Not a big deal.)
I think definitely not the caller should be responsible for this check.
On arm systems, the IRQ # that is returned is the hwirq # for the GIC.
Presently, Gunyah only gives us SPI interrupts [32,1019] so I've written
a translation to a SPI fwspec.
+
+ fwspec->param_count = 3;
+ fwspec->param[0] = GIC_SPI;
+ fwspec->param[1] = virq - 32;
And why is 32 subtracted?
GIC driver expects the SPI #, not the hwirq #.
+ fwspec->param[2] = IRQ_TYPE_EDGE_RISING;
+ return 0;
+}
+
+#endif
diff --git a/drivers/virt/gunyah/rsc_mgr.c
b/drivers/virt/gunyah/rsc_mgr.c
index d7ce692d0067..383be5ac0f44 100644
--- a/drivers/virt/gunyah/rsc_mgr.c
+++ b/drivers/virt/gunyah/rsc_mgr.c
@@ -17,6 +17,8 @@
#include <linux/platform_device.h>
#include <linux/miscdevice.h>
+#include <asm/gunyah.h>
+
#include "rsc_mgr.h"
#include "vm_mgr.h"
@@ -132,6 +134,7 @@ struct gh_rm_connection {
* @send_lock: synchronization to allow only one request to be sent
at a time
* @nh: notifier chain for clients interested in RM notification
messages
* @miscdev: /dev/gunyah
+ * @irq_domain: Domain to translate Gunyah hwirqs to Linux irqs
*/
struct gh_rm {
struct device *dev;
@@ -150,6 +153,7 @@ struct gh_rm {
struct blocking_notifier_head nh;
struct miscdevice miscdev;
+ struct irq_domain *irq_domain;
};
/**
@@ -190,6 +194,134 @@ static inline int gh_rm_remap_error(enum
gh_rm_error rm_error)
}
}
+struct gh_irq_chip_data {
+ u32 gh_virq;
+};
+
+static struct irq_chip gh_rm_irq_chip = {
+ .name = "Gunyah",
+ .irq_enable = irq_chip_enable_parent,
+ .irq_disable = irq_chip_disable_parent,
+ .irq_ack = irq_chip_ack_parent,
+ .irq_mask = irq_chip_mask_parent,
+ .irq_mask_ack = irq_chip_mask_ack_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_eoi = irq_chip_eoi_parent,
+ .irq_set_affinity = irq_chip_set_affinity_parent,
+ .irq_set_type = irq_chip_set_type_parent,
+ .irq_set_wake = irq_chip_set_wake_parent,
+ .irq_set_vcpu_affinity = irq_chip_set_vcpu_affinity_parent,
+ .irq_retrigger = irq_chip_retrigger_hierarchy,
+ .irq_get_irqchip_state = irq_chip_get_parent_state,
+ .irq_set_irqchip_state = irq_chip_set_parent_state,
+ .flags = IRQCHIP_SET_TYPE_MASKED |
+ IRQCHIP_SKIP_SET_WAKE |
+ IRQCHIP_MASK_ON_SUSPEND,
+};
+
+static int gh_rm_irq_domain_alloc(struct irq_domain *d, unsigned int
virq, unsigned int nr_irqs,
+ void *arg)
+{
+ struct gh_irq_chip_data *chip_data, *spec = arg;
+ struct irq_fwspec parent_fwspec;
+ struct gh_rm *rm = d->host_data;
+ u32 gh_virq = spec->gh_virq;
+ int ret;
+
+ if (nr_irqs != 1 || gh_virq == U32_MAX)
Does U32_MAX have special meaning? Why are you checking for it?
Whatever it is, you should explain why this is invalid here.
This was holdover from deprecated Gunyah code. Since there are new
features/version checks it's not possible for Linux to encounter these
values. I've dropped it in v12.
Thanks,
Elliot
