RE: [PATCH v2 5/6] drivers/hv/vmbus: Get the irq number from DeviceTree

Michael Kelley
2024-05-15T13:44:54Z

From: Roman Kisel <romank@linux.microsoft.com> Sent: Tuesday, May 14, 2024 3:44 PM
>
> The vmbus driver uses ACPI for interrupt assignment on
> arm64 hence it won't function in the VTL mode where only
> DeviceTree can be used.
>
> Update the vmbus driver to discover interrupt configuration
> via DeviceTree.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---
> drivers/hv/vmbus_drv.c | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index e25223cee3ab..52f01bd1c947 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -36,6 +36,7 @@
> #include <linux/syscore_ops.h>
> #include <linux/dma-map-ops.h>
> #include <linux/pci.h>
> +#include <linux/of_irq.h>
> #include <clocksource/hyperv_timer.h>
> #include <asm/mshyperv.h>
> #include "hyperv_vmbus.h"
> @@ -2316,6 +2317,34 @@ static int vmbus_acpi_add(struct platform_device *pdev)
> }
> #endif
>
> +static int __maybe_unused vmbus_of_set_irq(struct device_node *np)
> +{
> + struct irq_desc *desc;
> + int irq;
> +
> + irq = of_irq_get(np, 0);
> + if (irq == 0) {
> + pr_err("VMBus interrupt mapping failure\n");
> + return -EINVAL;
> + }
> + if (irq < 0) {
> + pr_err("VMBus interrupt data can't be read from DeviceTree, error %d\n", irq);
> + return irq;
> + }
> +
> + desc = irq_to_desc(irq);
> + if (!desc) {
> + pr_err("VMBus interrupt description can't be found for virq %d\n", irq);

s/description/descriptor/

Or maybe slightly more compact overall: "No interrupt descriptor for VMBus virq %d\n".

> + return -ENODEV;
> + }
> +
> + vmbus_irq = irq;
> + vmbus_interrupt = desc->irq_data.hwirq;
> + pr_debug("VMBus virq %d, hwirq %d\n", vmbus_irq, vmbus_interrupt);

How does device DMA cache coherency get handled in the DeviceTree case on
arm64? For vmbus_acpi_add(), there's code to look at the _CCA flag, which is
required in ACPI for arm64. (There's also code to handle the Hyper-V bug where
_CCA is omitted.) I don't know DeviceTree, but is there a similar flag to indicate
device cache coherency? Of course, Hyper-V always assumes DMA cache
coherency, and that's a valid assumption for the server-class systems that
would run Hyper-V VMs on arm64. But the Linux DMA subsystem needs to be
told, and vmbus_dma_configure() needs to propagate the setting from the
VMBus device to the child VMBus devices. Everything still works if the Linux
DMA subsystem isn't told, but you end up with a perf hit. The DMA code
defaults to "not coherent" on arm64, and you'll get a lot of high-cost cache
coherency maintenance done by the CPU when it is unnecessary. This issue
doesn't arise on x86 since the architecture requires DMA cache coherency, and
the Linux default is "coherent".

> +
> + return 0;
> +}
> +
> static int vmbus_device_add(struct platform_device *pdev)
> {
> struct resource **cur_res = &hyperv_mmio;
> @@ -2324,12 +2353,20 @@ static int vmbus_device_add(struct platform_device *pdev)
> struct device_node *np = pdev->dev.of_node;
> int ret;
>
> + pr_debug("VMBus is present in DeviceTree\n");
> +

I'm not clear on how interpret this debug message. Reaching this point in the code
path just means that acpi_disabled is "true". DeviceTree hasn't yet been searched to
see if VMBus is found.

> hv_dev = &pdev->dev;
>
> ret = of_range_parser_init(&parser, np);
> if (ret)
> return ret;
>
> +#ifndef HYPERVISOR_CALLBACK_VECTOR
> + ret = vmbus_of_set_irq(np);
> + if (ret)
> + return ret;
> +#endif
> +
> for_each_of_range(&parser, &range) {
> struct resource *res;
>
> --
> 2.45.0
>