Re: [PATCH v13 16/24] virt: gunyah: Translate gh_rm_hyp_resource into gunyah_resource

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 6/5/2023 12:49 PM, Alex Elder wrote:
On 5/9/23 3:47 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>

Please zero the automatic variable in the place I suggest it.
I have two other comments/questions.  Otherwise, this looks good.

Reviewed-by: Alex Elder <elder@xxxxxxxxxx>

---

...

+struct gh_resource *gh_rm_alloc_resource(struct gh_rm *rm, struct gh_rm_hyp_resource *hyp_resource)
+{
+    struct gh_resource *ghrsc;
+    int ret;
+
+    ghrsc = kzalloc(sizeof(*ghrsc), GFP_KERNEL);
+    if (!ghrsc)
+        return NULL;
+
+    ghrsc->type = hyp_resource->type;
+    ghrsc->capid = le64_to_cpu(hyp_resource->cap_id);
+    ghrsc->irq = IRQ_NOTCONNECTED;
+    ghrsc->rm_label = le32_to_cpu(hyp_resource->resource_label);
+    if (hyp_resource->virq) {
+        struct gh_irq_chip_data irq_data = {
+            .gh_virq = le32_to_cpu(hyp_resource->virq),
+        };
+
+        ret = irq_domain_alloc_irqs(rm->irq_domain, 1, NUMA_NO_NODE, &irq_data);
+        if (ret < 0) {
+            dev_err(rm->dev,
+                "Failed to allocate interrupt for resource %d label: %d: %d\n",
+                ghrsc->type, ghrsc->rm_label, ghrsc->irq);

Is it reasonable to return in this case without indicating to the
caller that something is wrong?


I wasn't sure what to do here since this is unexpected edge case. Not returning would cause a client's "request_irq" to fail down the line if the client was interested in the irq. I had picked not to return since this error doesn't put us in an unrecoverable state. No one currently wants to try to recover from that error, so I'm really just deferring the real error handling until later.

I can return ret here.

+        } else {
+            ghrsc->irq = ret;
+        }
+    }
+
+    return ghrsc;

...



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux