diff mbox series

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

Message ID 20240514224508.212318-6-romank@linux.microsoft.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series arm64/hyperv: Support Virtual Trust Level Boot | expand

Commit Message

Roman Kisel May 14, 2024, 10:43 p.m. UTC
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(+)

Comments

Krzysztof Kozlowski May 15, 2024, 7:47 a.m. UTC | #1
On 15/05/2024 00:43, Roman Kisel wrote:
> 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);

Where is the binding for this?

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

Not related and not really helpful. Simple entry/exit tracking is
provided already by tracing.


Best regards,
Krzysztof
Saurabh Singh Sengar May 15, 2024, 9:42 a.m. UTC | #2
On Tue, May 14, 2024 at 03:43:52PM -0700, Roman Kisel wrote:
> The vmbus driver uses ACPI for interrupt assignment on

In subject use the prefix "Drivers: hv: vmbus:".
It is preferred to us "VMbus/VMBus" instead of "vmbus" for all the
commit message and comments.

> 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);
> +		return -ENODEV;
> +	}
> +
> +	vmbus_irq = irq;
> +	vmbus_interrupt = desc->irq_data.hwirq;
> +	pr_debug("VMBus virq %d, hwirq %d\n", vmbus_irq, vmbus_interrupt);
> +
> +	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");
> +
>  	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
>
Michael Kelley May 15, 2024, 1:44 p.m. UTC | #3
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
>
Roman Kisel May 15, 2024, 4:31 p.m. UTC | #4
On 5/15/2024 2:42 AM, Saurabh Singh Sengar wrote:
> On Tue, May 14, 2024 at 03:43:52PM -0700, Roman Kisel wrote:
>> The vmbus driver uses ACPI for interrupt assignment on
> 
> In subject use the prefix "Drivers: hv: vmbus:".
> It is preferred to us "VMbus/VMBus" instead of "vmbus" for all the
> commit message and comments.
> 
I can see "Drivers: hv: vmbus:" as a convention from the data[1], will 
update the commit message per that.

The recent Michael Kelly's patchset where the Hyper-V documentation is 
updated to use "VMBus" instead of the incorrect one (iiuc) "VMbus" 
compels me to use "VMBus" out of the options you have enumerated, in the 
commit description.

[1]
The data mining device was

git log --all --grep='Drivers: hv: vmbus:'

and variations thereof.

>> 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);
>> +		return -ENODEV;
>> +	}
>> +
>> +	vmbus_irq = irq;
>> +	vmbus_interrupt = desc->irq_data.hwirq;
>> +	pr_debug("VMBus virq %d, hwirq %d\n", vmbus_irq, vmbus_interrupt);
>> +
>> +	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");
>> +
>>   	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
>>
Roman Kisel May 15, 2024, 5:05 p.m. UTC | #5
On 5/15/2024 12:47 AM, Krzysztof Kozlowski wrote:
> On 15/05/2024 00:43, Roman Kisel wrote:
>> 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);
> 
> Where is the binding for this?
> 
Have not added to 
Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml, my bad. Will 
update the file.

>> +	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);
>> +		return -ENODEV;
>> +	}
>> +
>> +	vmbus_irq = irq;
>> +	vmbus_interrupt = desc->irq_data.hwirq;
>> +	pr_debug("VMBus virq %d, hwirq %d\n", vmbus_irq, vmbus_interrupt);
>> +
>> +	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");
> 
> Not related and not really helpful. Simple entry/exit tracking is
> provided already by tracing.
> 
True. Will remove.

> 
> Best regards,
> Krzysztof
Roman Kisel May 15, 2024, 6:21 p.m. UTC | #6
On 5/15/2024 6:44 AM, Michael Kelley wrote:
> 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".
> 
Yep, thanks!

>> +		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".
> 
Appreciate the indispensable insight! I'll straighten this out.

>> +
>> +	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.
> 
True. Will remove.

>>   	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
>>
>
kernel test robot May 16, 2024, 2:40 a.m. UTC | #7
Hi Roman,

kernel test robot noticed the following build errors:

[auto build test ERROR on f2580a907e5c0e8fc9354fd095b011301c64f949]

url:    https://github.com/intel-lab-lkp/linux/commits/Roman-Kisel/arm64-hyperv-Support-DeviceTree/20240515-064749
base:   f2580a907e5c0e8fc9354fd095b011301c64f949
patch link:    https://lore.kernel.org/r/20240514224508.212318-6-romank%40linux.microsoft.com
patch subject: [PATCH v2 5/6] drivers/hv/vmbus: Get the irq number from DeviceTree
config: arm64-randconfig-r053-20240515 (https://download.01.org/0day-ci/archive/20240516/202405161034.f4v8i4eI-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project d3455f4ddd16811401fa153298fadd2f59f6914e)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240516/202405161034.f4v8i4eI-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405161034.f4v8i4eI-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt7622-hif.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt7622-aud.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8186-img.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8186-ipe.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8186-mdp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8186-vdec.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8188-img.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8188-ipe.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8188-vdec.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8188-vdo0.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8188-vdo1.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8188-venc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8188-vpp0.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8188-vpp1.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-apusys_pll.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-cam.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-ccu.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-img.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-imp_iic_wrap.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-ipe.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-scp_adsp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-vdec.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-vdo0.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-vdo1.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-vpp0.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-vpp1.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/mediatek/clk-mt8195-wpe.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/meson/meson-clkc-utils.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/meson/meson-aoclk.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/meson/meson-eeclk.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/meson/a1-pll.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/meson/gxbb.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/meson/gxbb-aoclk.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/meson/s4-peripherals.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/qcom/lpass-gfm-sm8250.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/qcom/videocc-sdm845.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/versatile/clk-vexpress-osc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/clk/clk-gate_test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/soc/amlogic/meson-clk-measure.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/char/hw_random/omap-rng.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/char/hw_random/omap3-rom-rng.o
WARNING: modpost: drivers/char/hw_random/mxc-rnga: section mismatch in reference: mxc_rnga_driver+0x10 (section: .data) -> mxc_rnga_remove (section: .exit.text)
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/char/hw_random/cavium-rng.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/char/hw_random/cavium-rng-vf.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/gpu/drm/panel/panel-abt-y030xx067a.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/gpu/drm/bridge/lontium-lt9611uxc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/gpu/drm/bridge/sii9234.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/gpu/drm/tiny/bochs.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/base/regmap/regmap-kunit.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/base/regmap/regmap-ram.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/base/regmap/regmap-raw-ram.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/base/regmap/regmap-spmi.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mfd/arizona.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mfd/vexpress-sysreg.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/dax/dax.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/core/cxl_core.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_acpi.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_port.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/spi/spi-fsl-lib.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/firewire/firewire-uapi-test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/uio/uio_aec.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/auxdisplay/hd44780_common.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/pcmcia/i82092.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/input/misc/soc_button_array.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/input/tests/input_test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/rtc/lib_test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/i2c/busses/i2c-ccgx-ucsi.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/dvb-frontends/au8522_decoder.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/v4l2-core/v4l2-async.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/media/v4l2-core/v4l2-fwnode.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hwmon/corsair-cpro.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mmc/core/pwrseq_emmc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mmc/host/tmio_mmc_core.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/leds/flash/leds-rt4505.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-belkin.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-evision.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-google-stadiaff.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-gyration.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-ite.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-kensington.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-kye.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-microsoft.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-saitek.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-sjoy.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-speedlink.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-topseed.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-twinhan.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-xinmo.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-zpff.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-zydacron.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/devfreq/governor_powersave.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/devfreq/governor_userspace.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/perf/arm-ccn.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/perf/fsl_imx8_ddr_perf.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/perf/marvell_cn10k_ddr_pmu.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fsi/fsi-master-aspeed.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/spmi/spmi-pmic-arb.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/iio/adc/xilinx-ams.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/iio/buffer/kfifo_buf.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/siox/siox-bus-gpio.o
>> ERROR: modpost: "irq_to_desc" [drivers/hv/hv_vmbus.ko] undefined!
Rob Herring (Arm) May 17, 2024, 5:14 p.m. UTC | #8
On Tue, May 14, 2024 at 5:45 PM Roman Kisel <romank@linux.microsoft.com> wrote:
>
> 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>

If you are using this header in a driver, you are doing it wrong. We
have common functions which work on both ACPI or DT, so use them if
you have a need to support both.

Though my first question on a binding will be the same as on every
'hypervisor binding'.  Why can't you make your hypervisor interfaces
discoverable? It's all s/w, not some h/w device which is fixed.

Rob
Roman Kisel May 20, 2024, 7:25 p.m. UTC | #9
On 5/17/2024 10:14 AM, Rob Herring wrote:
> On Tue, May 14, 2024 at 5:45 PM Roman Kisel <romank@linux.microsoft.com> wrote:
>>
>> 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>
> 
> If you are using this header in a driver, you are doing it wrong. We
> have common functions which work on both ACPI or DT, so use them if
> you have a need to support both.
> 
Understood, thank you! I'll look more for the examples. If you happen to 
have in mind the place where the idiomatic/more preferred approach is 
used, please let me know, would owe you a great debt of gratitude.


> Though my first question on a binding will be the same as on every
> 'hypervisor binding'.  Why can't you make your hypervisor interfaces
> discoverable? It's all s/w, not some h/w device which is fixed.
> 
I've taken a look at the related art. AWS's Firecracker, Intel's Cloud 
Hypervisor, Google's CrosVM, QEmu allow the guest use the 
well-established battle-tested generic approaches (ACPI, 
DeviceTree/OpenFirmware) of describing the virtual hardware and its 
resources rather than making the guest use their own specific 
interfaces. That holds true for the s/w devices like 
"vcpu-stall-detector" and VirtIO that do not have counterparts built as 
hardware, too.

Here, the guest needs to set up VMBus (the intra-partition communication 
transport) to be able to talk to the host partition. Receiving a message 
needs an interrupt service routine attached to the interrupt injected 
into the guest virtual processor, and DeviceTree lets provide the 
interrupt number. If a custom interface were used here, it'd look less 
expected due to others relying on ACPI and DT for configuring virtual 
devices and busses. A specialized interface would add more code (new 
code) instead of relying on the approach that is widely used.


> Rob
diff mbox series

Patch

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