[PATCH V2 0/3] x86/Hyper-V/IOMMU: Add Hyper-V IOMMU driver to support x2apic mode

[PATCH V2 0/3] x86/Hyper-V/IOMMU: Add Hyper-V IOMMU driver to support x2apic mode

[lantianyu1986]

From: Lan Tianyu <Tianyu.Lan@microsoft.com>

On the bare metal, enabling X2APIC mode requires interrupt remapping
function which helps to deliver irq to cpu with 32-bit APIC ID.
Hyper-V doesn't provide interrupt remapping function so far and Hyper-V
MSI protocol already supports to deliver interrupt to the CPU whose
virtual processor index is more than 255. IO-APIC interrupt still has
8-bit APIC ID limitation.

This patchset is to add Hyper-V stub IOMMU driver in order to enable
X2APIC mode successfully in Hyper-V Linux guest. The driver returns X2APIC
interrupt remapping capability when X2APIC mode is available. X2APIC
destination mode is set to physical by PATCH 1 when X2APIC is available.
Hyper-V IOMMU driver will scan cpu 0~255 and set cpu into IO-APIC MAX cpu
affinity cpumask if its APIC ID is 8-bit. Driver creates a Hyper-V irq domain
to limit IO-APIC interrupts' affinity and make sure cpus assigned with IO-APIC
interrupt are in the scope of IO-APIC MAX cpu affinity.

Lan Tianyu (3):
  x86/Hyper-V: Set x2apic destination mode to physical when x2apic is   
     available
  HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
  MAINTAINERS: Add Hyper-V IOMMU driver into Hyper-V CORE AND DRIVERS
    scope

 MAINTAINERS                    |   1 +
 arch/x86/kernel/cpu/mshyperv.c |  10 +++
 drivers/iommu/Kconfig          |   8 ++
 drivers/iommu/Makefile         |   1 +
 drivers/iommu/hyperv-iommu.c   | 193 +++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/irq_remapping.c  |   3 +
 drivers/iommu/irq_remapping.h  |   1 +
 7 files changed, 217 insertions(+)
 create mode 100644 drivers/iommu/hyperv-iommu.c

-- 
2.7.4

[PATCH V2 1/3] x86/Hyper-V: Set x2apic destination mode to physical when x2apic is available

[lantianyu1986]

From: Lan Tianyu <Tianyu.Lan@microsoft.com>

Hyper-V doesn't provide irq remapping for IO-APIC. To enable x2apic,
set x2apic destination mode to physcial mode when x2apic is available
and Hyper-V IOMMU driver makes sure cpus assigned with IO-APIC irqs have
8-bit APIC id.

Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
---
Change since v1:
       - Remove redundant extern for x2apic_phys
---
 arch/x86/kernel/cpu/mshyperv.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index e81a2db..4bd6d90 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -328,6 +328,16 @@ static void __init ms_hyperv_init_platform(void)
 # ifdef CONFIG_SMP
 	smp_ops.smp_prepare_boot_cpu = hv_smp_prepare_boot_cpu;
 # endif
+
+/*
+ * Hyper-V doesn't provide irq remapping for IO-APIC. To enable x2apic,
+ * set x2apic destination mode to physcial mode when x2apic is available
+ * and Hyper-V IOMMU driver makes sure cpus assigned with IO-APIC irqs
+ * have 8-bit APIC id.
+ */
+	if (IS_ENABLED(CONFIG_HYPERV_IOMMU) && x2apic_supported())
+		x2apic_phys = 1;
+
 #endif
 }
 
-- 
2.7.4

RE: [PATCH V2 1/3] x86/Hyper-V: Set x2apic destination mode to physical when x2apic is available

[Michael Kelley]

From: lantianyu1986@gmail.com <lantianyu1986@gmail.com>  Sent: Saturday, February 2, 2019 5:15 AM
> 
> Hyper-V doesn't provide irq remapping for IO-APIC. To enable x2apic,
> set x2apic destination mode to physcial mode when x2apic is available
> and Hyper-V IOMMU driver makes sure cpus assigned with IO-APIC irqs have
> 8-bit APIC id.
> 
> Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
> ---
> Change since v1:
>        - Remove redundant extern for x2apic_phys
> ---
>  arch/x86/kernel/cpu/mshyperv.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index e81a2db..4bd6d90 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -328,6 +328,16 @@ static void __init ms_hyperv_init_platform(void)
>  # ifdef CONFIG_SMP
>  	smp_ops.smp_prepare_boot_cpu = hv_smp_prepare_boot_cpu;
>  # endif
> +
> +/*
> + * Hyper-V doesn't provide irq remapping for IO-APIC. To enable x2apic,
> + * set x2apic destination mode to physcial mode when x2apic is available
> + * and Hyper-V IOMMU driver makes sure cpus assigned with IO-APIC irqs
> + * have 8-bit APIC id.
> + */

Per comment from Dan Carpenter on v1 of this patch, the above comment
block should be indented one tab to line up with the "if" statement below.

Michael

> +	if (IS_ENABLED(CONFIG_HYPERV_IOMMU) && x2apic_supported())
> +		x2apic_phys = 1;
> +
>  #endif
>  }
> 
> --
> 2.7.4

Re: [PATCH V2 1/3] x86/Hyper-V: Set x2apic destination mode to physical when x2apic is available

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]

Hi Lan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on iommu/next]
[also build test ERROR on v5.0-rc4 next-20190201]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/lantianyu1986-gmail-com/x86-Hyper-V-IOMMU-Add-Hyper-V-IOMMU-driver-to-support-x2apic-mode/20190204-132009
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: x86_64-randconfig-x005-201905 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-14) 8.2.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   arch/x86//kernel/cpu/mshyperv.c: In function 'ms_hyperv_init_platform':
>> arch/x86//kernel/cpu/mshyperv.c:339:3: error: 'x2apic_phys' undeclared (first use in this function); did you mean 'x2apic_mode'?
      x2apic_phys = 1;
      ^~~~~~~~~~~
      x2apic_mode
   arch/x86//kernel/cpu/mshyperv.c:339:3: note: each undeclared identifier is reported only once for each function it appears in

vim +339 arch/x86//kernel/cpu/mshyperv.c

   331	
   332	/*
   333	 * Hyper-V doesn't provide irq remapping for IO-APIC. To enable x2apic,
   334	 * set x2apic destination mode to physcial mode when x2apic is available
   335	 * and Hyper-V IOMMU driver makes sure cpus assigned with IO-APIC irqs
   336	 * have 8-bit APIC id.
   337	 */
   338		if (IS_ENABLED(CONFIG_HYPERV_IOMMU) && x2apic_supported())
 > 339			x2apic_phys = 1;
   340	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35413 bytes --]

[-- Attachment #3: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH V2 1/3] x86/Hyper-V: Set x2apic destination mode to physical when x2apic is available

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1659 bytes --]

Hi Lan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on iommu/next]
[also build test ERROR on v5.0-rc4 next-20190204]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/lantianyu1986-gmail-com/x86-Hyper-V-IOMMU-Add-Hyper-V-IOMMU-driver-to-support-x2apic-mode/20190204-132009
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: i386-randconfig-a3-02040849 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86/kernel/cpu/mshyperv.c: In function 'ms_hyperv_init_platform':
>> arch/x86/kernel/cpu/mshyperv.c:339:3: error: 'x2apic_phys' undeclared (first use in this function)
      x2apic_phys = 1;
      ^
   arch/x86/kernel/cpu/mshyperv.c:339:3: note: each undeclared identifier is reported only once for each function it appears in

vim +/x2apic_phys +339 arch/x86/kernel/cpu/mshyperv.c

   331	
   332	/*
   333	 * Hyper-V doesn't provide irq remapping for IO-APIC. To enable x2apic,
   334	 * set x2apic destination mode to physcial mode when x2apic is available
   335	 * and Hyper-V IOMMU driver makes sure cpus assigned with IO-APIC irqs
   336	 * have 8-bit APIC id.
   337	 */
   338		if (IS_ENABLED(CONFIG_HYPERV_IOMMU) && x2apic_supported())
 > 339			x2apic_phys = 1;
   340	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32527 bytes --]

[-- Attachment #3: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH V2 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver

[lantianyu1986]

From: Lan Tianyu <Tianyu.Lan@microsoft.com>

On the bare metal, enabling X2APIC mode requires interrupt remapping
function which helps to deliver irq to cpu with 32-bit APIC ID.
Hyper-V doesn't provide interrupt remapping function so far and Hyper-V
MSI protocol already supports to deliver interrupt to the CPU whose
virtual processor index is more than 255. IO-APIC interrupt still has
8-bit APIC ID limitation.

This patch is to add Hyper-V stub IOMMU driver in order to enable
X2APIC mode successfully in Hyper-V Linux guest. The driver returns X2APIC
interrupt remapping capability when X2APIC mode is available. Otherwise,
it creates a Hyper-V irq domain to limit IO-APIC interrupts' affinity
and make sure cpus assigned with IO-APIC interrupt have 8-bit APIC ID.

Signed-off-by: Lan Tianyu <Tianyu.Lan@microsoft.com>
---
Change since v1:
      - Remove unused pr_fmt
      - Make ioapic_ir_domain as static variable
      - Remove unused variables cfg and entry in the hyperv_irq_remapping_alloc()
      - Fix comments
---
 drivers/iommu/Kconfig         |   8 ++
 drivers/iommu/Makefile        |   1 +
 drivers/iommu/hyperv-iommu.c  | 193 ++++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/irq_remapping.c |   3 +
 drivers/iommu/irq_remapping.h |   1 +
 5 files changed, 206 insertions(+)
 create mode 100644 drivers/iommu/hyperv-iommu.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 45d7021..9bdaa7b 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -437,4 +437,12 @@ config QCOM_IOMMU
 	help
 	  Support for IOMMU on certain Qualcomm SoCs.
 
+config HYPERV_IOMMU
+	bool "Hyper-V IRQ Remapping Support"
+	depends on HYPERV
+	select IOMMU_API
+	help
+	    Hyper-V stub IOMMU driver provides IRQ Remapping capability
+	    to run Linux guest with X2APIC mode on Hyper-V.
+
 endif # IOMMU_SUPPORT
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index a158a68..8c71a15 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
 obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
 obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
 obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o
+obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o
diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
new file mode 100644
index 0000000..59d8f96
--- /dev/null
+++ b/drivers/iommu/hyperv-iommu.c
@@ -0,0 +1,193 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Hyper-V stub IOMMU driver.
+ *
+ * Copyright (C) 2019, Microsoft, Inc.
+ *
+ * Author : Lan Tianyu <Tianyu.Lan@microsoft.com>
+ */
+
+#include <linux/types.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/iommu.h>
+#include <linux/module.h>
+
+#include <asm/hw_irq.h>
+#include <asm/io_apic.h>
+#include <asm/irq_remapping.h>
+#include <asm/hypervisor.h>
+
+#include "irq_remapping.h"
+
+/*
+ * According 82093AA IO-APIC spec , IO APIC has a 24-entry Interrupt
+ * Redirection Table.
+ */
+#define IOAPIC_REMAPPING_ENTRY 24
+
+static cpumask_t ioapic_max_cpumask = { CPU_BITS_NONE };
+static struct irq_domain *ioapic_ir_domain;
+
+static int hyperv_ir_set_affinity(struct irq_data *data,
+		const struct cpumask *mask, bool force)
+{
+	struct irq_data *parent = data->parent_data;
+	struct irq_cfg *cfg = irqd_cfg(data);
+	struct IO_APIC_route_entry *entry;
+	cpumask_t cpumask;
+	int ret;
+
+	cpumask_andnot(&cpumask, mask, &ioapic_max_cpumask);
+
+	/* Return error If new irq affinity is out of ioapic_max_cpumask. */
+	if (!cpumask_empty(&cpumask))
+		return -EINVAL;
+
+	ret = parent->chip->irq_set_affinity(parent, mask, force);
+	if (ret < 0 || ret == IRQ_SET_MASK_OK_DONE)
+		return ret;
+
+	entry = data->chip_data;
+	entry->dest = cfg->dest_apicid;
+	entry->vector = cfg->vector;
+	send_cleanup_vector(cfg);
+
+	return 0;
+}
+
+static struct irq_chip hyperv_ir_chip = {
+	.name			= "HYPERV-IR",
+	.irq_ack		= apic_ack_irq,
+	.irq_set_affinity	= hyperv_ir_set_affinity,
+};
+
+static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
+				     unsigned int virq, unsigned int nr_irqs,
+				     void *arg)
+{
+	struct irq_alloc_info *info = arg;
+	struct irq_data *irq_data;
+	struct irq_desc *desc;
+	int ret = 0;
+
+	if (!info || info->type != X86_IRQ_ALLOC_TYPE_IOAPIC || nr_irqs > 1)
+		return -EINVAL;
+
+	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
+	if (ret < 0)
+		return ret;
+
+	irq_data = irq_domain_get_irq_data(domain, virq);
+	if (!irq_data) {
+		irq_domain_free_irqs_common(domain, virq, nr_irqs);
+		return -EINVAL;
+	}
+
+	irq_data->chip = &hyperv_ir_chip;
+
+	/*
+	 * IOAPIC entry pointer is saved in chip_data to allow
+	 * hyperv_irq_remappng_activate()/hyperv_ir_set_affinity() to set
+	 * vector and dest_apicid. cfg->vector and cfg->dest_apicid are
+	 * ignorred when IRQ remapping is enabled. See ioapic_configure_entry().
+	 */
+	irq_data->chip_data = info->ioapic_entry;
+
+	/*
+	 * Hypver-V IO APIC irq affinity should be in the scope of
+	 * ioapic_max_cpumask because no irq remapping support.
+	 */
+	desc = irq_data_to_desc(irq_data);
+	cpumask_and(desc->irq_common_data.affinity,
+			desc->irq_common_data.affinity,
+			&ioapic_max_cpumask);
+
+	return 0;
+}
+
+static void hyperv_irq_remapping_free(struct irq_domain *domain,
+				 unsigned int virq, unsigned int nr_irqs)
+{
+	irq_domain_free_irqs_common(domain, virq, nr_irqs);
+}
+
+static int hyperv_irq_remappng_activate(struct irq_domain *domain,
+			  struct irq_data *irq_data, bool reserve)
+{
+	struct irq_cfg *cfg = irqd_cfg(irq_data);
+	struct IO_APIC_route_entry *entry = irq_data->chip_data;
+
+	entry->dest = cfg->dest_apicid;
+	entry->vector = cfg->vector;
+
+	return 0;
+}
+
+static struct irq_domain_ops hyperv_ir_domain_ops = {
+	.alloc = hyperv_irq_remapping_alloc,
+	.free = hyperv_irq_remapping_free,
+	.activate = hyperv_irq_remappng_activate,
+};
+
+static int __init hyperv_prepare_irq_remapping(void)
+{
+	struct fwnode_handle *fn;
+	u32 apic_id;
+	int i;
+
+	if (x86_hyper_type != X86_HYPER_MS_HYPERV ||
+	    !x2apic_supported())
+		return -ENODEV;
+
+	fn = irq_domain_alloc_named_id_fwnode("HYPERV-IR", 0);
+	if (!fn)
+		return -ENOMEM;
+
+	ioapic_ir_domain =
+		irq_domain_create_hierarchy(arch_get_ir_parent_domain(),
+				0, IOAPIC_REMAPPING_ENTRY, fn,
+				&hyperv_ir_domain_ops, NULL);
+
+	irq_domain_free_fwnode(fn);
+
+	/*
+	 * Hyper-V doesn't provide irq remapping function for
+	 * IO-APIC and so IO-APIC only accepts 8-bit APIC ID.
+	 * Cpu's APIC ID is read from ACPI MADT table and APIC IDs
+	 * in the MADT table on Hyper-v are sorted monotonic increasingly.
+	 * APIC ID reflects cpu topology. There maybe some APIC ID
+	 * gaps when cpu number in a socket is not power of two. Prepare
+	 * max cpu affinity for IOAPIC irqs. Scan cpu 0-255 and set cpu
+	 * into ioapic_max_cpumask if its APIC ID is less than 256.
+	 */
+	for (i = 0; i < 256; i++) {
+		apic_id = cpu_physical_id(i);
+		if (apic_id > 255)
+			continue;
+
+		cpumask_set_cpu(i, &ioapic_max_cpumask);
+	}
+
+	return 0;
+}
+
+static int __init hyperv_enable_irq_remapping(void)
+{
+	return IRQ_REMAP_X2APIC_MODE;
+}
+
+static struct irq_domain *hyperv_get_ir_irq_domain(struct irq_alloc_info *info)
+{
+	if (info->type == X86_IRQ_ALLOC_TYPE_IOAPIC)
+		return ioapic_ir_domain;
+	else
+		return NULL;
+}
+
+struct irq_remap_ops hyperv_irq_remap_ops = {
+	.prepare		= hyperv_prepare_irq_remapping,
+	.enable			= hyperv_enable_irq_remapping,
+	.get_ir_irq_domain	= hyperv_get_ir_irq_domain,
+};
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index b94ebd4..81cf290 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -103,6 +103,9 @@ int __init irq_remapping_prepare(void)
 	else if (IS_ENABLED(CONFIG_AMD_IOMMU) &&
 		 amd_iommu_irq_ops.prepare() == 0)
 		remap_ops = &amd_iommu_irq_ops;
+	else if (IS_ENABLED(CONFIG_HYPERV_IOMMU) &&
+		 hyperv_irq_remap_ops.prepare() == 0)
+		remap_ops = &hyperv_irq_remap_ops;
 	else
 		return -ENOSYS;
 
diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
index 0afef6e..f8609e9 100644
--- a/drivers/iommu/irq_remapping.h
+++ b/drivers/iommu/irq_remapping.h
@@ -64,6 +64,7 @@ struct irq_remap_ops {
 
 extern struct irq_remap_ops intel_irq_remap_ops;
 extern struct irq_remap_ops amd_iommu_irq_ops;
+extern struct irq_remap_ops hyperv_irq_remap_ops;
 
 #else  /* CONFIG_IRQ_REMAP */
 
-- 
2.7.4

RE: [PATCH V2 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver

[Michael Kelley]

From: lantianyu1986@gmail.com <lantianyu1986@gmail.com> Sent: Saturday, February 2, 2019 5:15 AM
> 
> +/*
> + * According 82093AA IO-APIC spec , IO APIC has a 24-entry Interrupt
> + * Redirection Table.
> + */
> +#define IOAPIC_REMAPPING_ENTRY 24

The other unstated assumption here is that Hyper-v guest VMs
have only a single IOAPIC, regardless of the size of the VM.
Perhaps that should be stated in the comment explaining why
there are 24 entries?

> +
> +static cpumask_t ioapic_max_cpumask = { CPU_BITS_NONE };
> +static struct irq_domain *ioapic_ir_domain;
> +
> +static int hyperv_ir_set_affinity(struct irq_data *data,
> +		const struct cpumask *mask, bool force)
> +{
> +	struct irq_data *parent = data->parent_data;
> +	struct irq_cfg *cfg = irqd_cfg(data);
> +	struct IO_APIC_route_entry *entry;
> +	cpumask_t cpumask;
> +	int ret;
> +
> +	cpumask_andnot(&cpumask, mask, &ioapic_max_cpumask);
> +
> +	/* Return error If new irq affinity is out of ioapic_max_cpumask. */
> +	if (!cpumask_empty(&cpumask))
> +		return -EINVAL;

The above two cpumask functions can be combined in a single
call to cpumask_subset().  This has the nice property that determining
whether the cpus in "mask" are a subset of the cpus in "ioapic_max_cpumask"
is exactly what this code is trying to do. :-)  And it gets rid of the local
cpumask variable and the associated compiler warnings about stack frame
size.

> +
> +	ret = parent->chip->irq_set_affinity(parent, mask, force);
> +	if (ret < 0 || ret == IRQ_SET_MASK_OK_DONE)
> +		return ret;
> +
> +	entry = data->chip_data;
> +	entry->dest = cfg->dest_apicid;
> +	entry->vector = cfg->vector;
> +	send_cleanup_vector(cfg);
> +
> +	return 0;
> +}
> +
> +static struct irq_chip hyperv_ir_chip = {
> +	.name			= "HYPERV-IR",
> +	.irq_ack		= apic_ack_irq,
> +	.irq_set_affinity	= hyperv_ir_set_affinity,
> +};
> +
> +static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
> +				     unsigned int virq, unsigned int nr_irqs,
> +				     void *arg)
> +{
> +	struct irq_alloc_info *info = arg;
> +	struct irq_data *irq_data;
> +	struct irq_desc *desc;
> +	int ret = 0;
> +
> +	if (!info || info->type != X86_IRQ_ALLOC_TYPE_IOAPIC || nr_irqs > 1)
> +		return -EINVAL;
> +
> +	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
> +	if (ret < 0)
> +		return ret;
> +
> +	irq_data = irq_domain_get_irq_data(domain, virq);
> +	if (!irq_data) {
> +		irq_domain_free_irqs_common(domain, virq, nr_irqs);
> +		return -EINVAL;
> +	}
> +
> +	irq_data->chip = &hyperv_ir_chip;
> +
> +	/*
> +	 * IOAPIC entry pointer is saved in chip_data to allow
> +	 * hyperv_irq_remappng_activate()/hyperv_ir_set_affinity() to set
> +	 * vector and dest_apicid. cfg->vector and cfg->dest_apicid are
> +	 * ignorred when IRQ remapping is enabled. See ioapic_configure_entry().
 
Spelling: "ignored".

I saw Vitaly previous comments, and I still don't understand this comment. :-(
Is IRQ remapping considered to be enabled by this IOMMU driver, such that
cfg->vector and cfg->dest_apicid are ignored?  Or is the "when IRQ remapping
is enabled" a statement about some future enhancement?

> +	 */
> +	irq_data->chip_data = info->ioapic_entry;
> +
> +	/*
> +	 * Hypver-V IO APIC irq affinity should be in the scope of
> +	 * ioapic_max_cpumask because no irq remapping support.
> +	 */
> +	desc = irq_data_to_desc(irq_data);
> +	cpumask_and(desc->irq_common_data.affinity,
> +			desc->irq_common_data.affinity,
> +			&ioapic_max_cpumask);
> +
> +	return 0;
> +}
> +
> +static void hyperv_irq_remapping_free(struct irq_domain *domain,
> +				 unsigned int virq, unsigned int nr_irqs)
> +{
> +	irq_domain_free_irqs_common(domain, virq, nr_irqs);
> +}
> +
> +static int hyperv_irq_remappng_activate(struct irq_domain *domain,
> +			  struct irq_data *irq_data, bool reserve)
> +{
> +	struct irq_cfg *cfg = irqd_cfg(irq_data);
> +	struct IO_APIC_route_entry *entry = irq_data->chip_data;
> +
> +	entry->dest = cfg->dest_apicid;
> +	entry->vector = cfg->vector;
> +
> +	return 0;
> +}
> +
> +static struct irq_domain_ops hyperv_ir_domain_ops = {
> +	.alloc = hyperv_irq_remapping_alloc,
> +	.free = hyperv_irq_remapping_free,
> +	.activate = hyperv_irq_remappng_activate,
> +};
> +
> +static int __init hyperv_prepare_irq_remapping(void)
> +{
> +	struct fwnode_handle *fn;
> +	u32 apic_id;
> +	int i;
> +
> +	if (x86_hyper_type != X86_HYPER_MS_HYPERV ||

The function hypervisor_is_type() exists for doing the above test.
See include/asm/hypervisor.h

> +	    !x2apic_supported())
> +		return -ENODEV;
> +
> +	fn = irq_domain_alloc_named_id_fwnode("HYPERV-IR", 0);
> +	if (!fn)
> +		return -ENOMEM;
> +
> +	ioapic_ir_domain =
> +		irq_domain_create_hierarchy(arch_get_ir_parent_domain(),
> +				0, IOAPIC_REMAPPING_ENTRY, fn,
> +				&hyperv_ir_domain_ops, NULL);
> +
> +	irq_domain_free_fwnode(fn);
> +
> +	/*
> +	 * Hyper-V doesn't provide irq remapping function for
> +	 * IO-APIC and so IO-APIC only accepts 8-bit APIC ID.
> +	 * Cpu's APIC ID is read from ACPI MADT table and APIC IDs
> +	 * in the MADT table on Hyper-v are sorted monotonic increasingly.
> +	 * APIC ID reflects cpu topology. There maybe some APIC ID
> +	 * gaps when cpu number in a socket is not power of two. Prepare
> +	 * max cpu affinity for IOAPIC irqs. Scan cpu 0-255 and set cpu
> +	 * into ioapic_max_cpumask if its APIC ID is less than 256.
> +	 */
> +	for (i = 0; i < 256; i++) {
> +		apic_id = cpu_physical_id(i);
> +		if (apic_id > 255)
> +			continue;
> +
> +		cpumask_set_cpu(i, &ioapic_max_cpumask);
> +	}

The above for loop would be more compact as:

	for (i=0; i <256; i++)
		if (cpu_physical_id(i)  < 256)
			cpumask_set_cpu(i, &ioapic_max_cpumask);

> +
> +	return 0;
> +}
> +

RE: [PATCH V2 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver

[Michael Kelley]

From: lantianyu1986@gmail.com <lantianyu1986@gmail.com> Sent: Saturday, February 2, 2019 5:15 AM

I have a couple more comments ....

> 
> +config HYPERV_IOMMU
> +	bool "Hyper-V IRQ Remapping Support"
> +	depends on HYPERV
> +	select IOMMU_API
> +	help
> +	    Hyper-V stub IOMMU driver provides IRQ Remapping capability
> +	    to run Linux guest with X2APIC mode on Hyper-V.
> +
> +

I'm a little concerned about the terminology here.  The comments and
commit messages for these patches all say that Hyper-V guests don't
have interrupt remapping support.  And we don't really *need* interrupt
remapping support because all the interrupts that should be nicely spread
out across all vCPUs (i.e., the MSI interrupts for PCI pass-thru devices) are
handled via a hypercall in pci-hyperv.c, and not via the virtual IOAPIC.  So
we have this stub IOMMU driver that doesn't actually do interrupt remapping.
It just prevents assigning the very small number of non-performance sensitive
IOAPIC interrupts to a CPU with an APIC ID above 255.

With that background, describing this feature as "Hyper-V IRQ Remapping
Support" seems incorrect, and similarly in the "help" description.  Finding
good terminology for this situation is hard.  But how about narrowing the
focus to x2APIC handling:

	bool "Hyper-V x2APIC IRQ Handling"
	...
	help
	   Stub IOMMU driver to handle IRQs as to allow Hyper-V Linux
	   guests to run with x2APIC mode enabled


> +static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
> +				     unsigned int virq, unsigned int nr_irqs,
> +				     void *arg)
> +{
> +	struct irq_alloc_info *info = arg;
> +	struct irq_data *irq_data;
> +	struct irq_desc *desc;
> +	int ret = 0;
> +
> +	if (!info || info->type != X86_IRQ_ALLOC_TYPE_IOAPIC || nr_irqs > 1)
> +		return -EINVAL;
> +
> +	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
> +	if (ret < 0)
> +		return ret;
> +
> +	irq_data = irq_domain_get_irq_data(domain, virq);
> +	if (!irq_data) {
> +		irq_domain_free_irqs_common(domain, virq, nr_irqs);
> +		return -EINVAL;
> +	}
> +
> +	irq_data->chip = &hyperv_ir_chip;
> +
> +	/*
> +	 * IOAPIC entry pointer is saved in chip_data to allow
> +	 * hyperv_irq_remappng_activate()/hyperv_ir_set_affinity() to set
> +	 * vector and dest_apicid. cfg->vector and cfg->dest_apicid are
> +	 * ignorred when IRQ remapping is enabled. See ioapic_configure_entry().
> +	 */
> +	irq_data->chip_data = info->ioapic_entry;
> +
> +	/*
> +	 * Hypver-V IO APIC irq affinity should be in the scope of
> +	 * ioapic_max_cpumask because no irq remapping support.
> +	 */
> +	desc = irq_data_to_desc(irq_data);
> +	cpumask_and(desc->irq_common_data.affinity,
> +			desc->irq_common_data.affinity,
> +			&ioapic_max_cpumask);

The intent of this cpumask_and() call is to ensure that IOAPIC interrupts
are initially assigned to a CPU with APIC ID < 256.   But do we know that
the initial value of desc->irq_common_data.affinity is such that the result
of the cpumask_and() will not be the empty set?  My impression is that
these local IOAPIC interrupts are assigned an initial affinity mask with all
CPUs set, in which case this will work just fine.  But I'm not sure if that
is guaranteed.

An alternative would be to set the affinity to ioapic_max_cpumask and
overwrite whatever might have been previously specified.  But I don't know
if that's really better.

> +
> +	return 0;
> +}

Re: [PATCH V2 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 9732 bytes --]

Hi Lan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on iommu/next]
[also build test ERROR on v5.0-rc4]
[cannot apply to next-20190204]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/lantianyu1986-gmail-com/x86-Hyper-V-IOMMU-Add-Hyper-V-IOMMU-driver-to-support-x2apic-mode/20190204-132009
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: i386-allmodconfig (attached as .config)
compiler: gcc-8 (Debian 8.2.0-14) 8.2.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   drivers/iommu/hyperv-iommu.c: In function 'hyperv_prepare_irq_remapping':
>> drivers/iommu/hyperv-iommu.c:149:31: error: implicit declaration of function 'arch_get_ir_parent_domain'; did you mean 'arch_init_msi_domain'? [-Werror=implicit-function-declaration]
      irq_domain_create_hierarchy(arch_get_ir_parent_domain(),
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~
                                  arch_init_msi_domain
>> drivers/iommu/hyperv-iommu.c:149:31: warning: passing argument 1 of 'irq_domain_create_hierarchy' makes pointer from integer without a cast [-Wint-conversion]
      irq_domain_create_hierarchy(arch_get_ir_parent_domain(),
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from arch/x86/include/asm/irqdomain.h:5,
                    from arch/x86/include/asm/irq_remapping.h:25,
                    from drivers/iommu/hyperv-iommu.c:19:
   include/linux/irqdomain.h:434:27: note: expected 'struct irq_domain *' but argument is of type 'int'
    extern struct irq_domain *irq_domain_create_hierarchy(struct irq_domain *parent,
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iommu/hyperv-iommu.c: At top level:
>> drivers/iommu/hyperv-iommu.c:189:8: error: variable 'hyperv_irq_remap_ops' has initializer but incomplete type
    struct irq_remap_ops hyperv_irq_remap_ops = {
           ^~~~~~~~~~~~~
>> drivers/iommu/hyperv-iommu.c:190:3: error: 'struct irq_remap_ops' has no member named 'prepare'
     .prepare  = hyperv_prepare_irq_remapping,
      ^~~~~~~
>> drivers/iommu/hyperv-iommu.c:190:14: warning: excess elements in struct initializer
     .prepare  = hyperv_prepare_irq_remapping,
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iommu/hyperv-iommu.c:190:14: note: (near initialization for 'hyperv_irq_remap_ops')
>> drivers/iommu/hyperv-iommu.c:191:3: error: 'struct irq_remap_ops' has no member named 'enable'
     .enable   = hyperv_enable_irq_remapping,
      ^~~~~~
   drivers/iommu/hyperv-iommu.c:191:14: warning: excess elements in struct initializer
     .enable   = hyperv_enable_irq_remapping,
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iommu/hyperv-iommu.c:191:14: note: (near initialization for 'hyperv_irq_remap_ops')
>> drivers/iommu/hyperv-iommu.c:192:3: error: 'struct irq_remap_ops' has no member named 'get_ir_irq_domain'
     .get_ir_irq_domain = hyperv_get_ir_irq_domain,
      ^~~~~~~~~~~~~~~~~
   drivers/iommu/hyperv-iommu.c:192:23: warning: excess elements in struct initializer
     .get_ir_irq_domain = hyperv_get_ir_irq_domain,
                          ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iommu/hyperv-iommu.c:192:23: note: (near initialization for 'hyperv_irq_remap_ops')
>> drivers/iommu/hyperv-iommu.c:189:22: error: storage size of 'hyperv_irq_remap_ops' isn't known
    struct irq_remap_ops hyperv_irq_remap_ops = {
                         ^~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +149 drivers/iommu/hyperv-iommu.c

    16	
    17	#include <asm/hw_irq.h>
    18	#include <asm/io_apic.h>
  > 19	#include <asm/irq_remapping.h>
    20	#include <asm/hypervisor.h>
    21	
    22	#include "irq_remapping.h"
    23	
    24	/*
    25	 * According 82093AA IO-APIC spec , IO APIC has a 24-entry Interrupt
    26	 * Redirection Table.
    27	 */
    28	#define IOAPIC_REMAPPING_ENTRY 24
    29	
    30	static cpumask_t ioapic_max_cpumask = { CPU_BITS_NONE };
    31	static struct irq_domain *ioapic_ir_domain;
    32	
    33	static int hyperv_ir_set_affinity(struct irq_data *data,
    34			const struct cpumask *mask, bool force)
    35	{
    36		struct irq_data *parent = data->parent_data;
    37		struct irq_cfg *cfg = irqd_cfg(data);
    38		struct IO_APIC_route_entry *entry;
    39		cpumask_t cpumask;
    40		int ret;
    41	
    42		cpumask_andnot(&cpumask, mask, &ioapic_max_cpumask);
    43	
    44		/* Return error If new irq affinity is out of ioapic_max_cpumask. */
    45		if (!cpumask_empty(&cpumask))
    46			return -EINVAL;
    47	
    48		ret = parent->chip->irq_set_affinity(parent, mask, force);
    49		if (ret < 0 || ret == IRQ_SET_MASK_OK_DONE)
    50			return ret;
    51	
    52		entry = data->chip_data;
    53		entry->dest = cfg->dest_apicid;
    54		entry->vector = cfg->vector;
    55		send_cleanup_vector(cfg);
    56	
    57		return 0;
    58	}
    59	
    60	static struct irq_chip hyperv_ir_chip = {
    61		.name			= "HYPERV-IR",
    62		.irq_ack		= apic_ack_irq,
    63		.irq_set_affinity	= hyperv_ir_set_affinity,
    64	};
    65	
    66	static int hyperv_irq_remapping_alloc(struct irq_domain *domain,
    67					     unsigned int virq, unsigned int nr_irqs,
    68					     void *arg)
    69	{
    70		struct irq_alloc_info *info = arg;
    71		struct irq_data *irq_data;
    72		struct irq_desc *desc;
    73		int ret = 0;
    74	
    75		if (!info || info->type != X86_IRQ_ALLOC_TYPE_IOAPIC || nr_irqs > 1)
    76			return -EINVAL;
    77	
    78		ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
    79		if (ret < 0)
    80			return ret;
    81	
    82		irq_data = irq_domain_get_irq_data(domain, virq);
    83		if (!irq_data) {
    84			irq_domain_free_irqs_common(domain, virq, nr_irqs);
    85			return -EINVAL;
    86		}
    87	
    88		irq_data->chip = &hyperv_ir_chip;
    89	
    90		/*
    91		 * IOAPIC entry pointer is saved in chip_data to allow
    92		 * hyperv_irq_remappng_activate()/hyperv_ir_set_affinity() to set
    93		 * vector and dest_apicid. cfg->vector and cfg->dest_apicid are
    94		 * ignorred when IRQ remapping is enabled. See ioapic_configure_entry().
    95		 */
    96		irq_data->chip_data = info->ioapic_entry;
    97	
    98		/*
    99		 * Hypver-V IO APIC irq affinity should be in the scope of
   100		 * ioapic_max_cpumask because no irq remapping support.
   101		 */
   102		desc = irq_data_to_desc(irq_data);
   103		cpumask_and(desc->irq_common_data.affinity,
   104				desc->irq_common_data.affinity,
   105				&ioapic_max_cpumask);
   106	
   107		return 0;
   108	}
   109	
   110	static void hyperv_irq_remapping_free(struct irq_domain *domain,
   111					 unsigned int virq, unsigned int nr_irqs)
   112	{
   113		irq_domain_free_irqs_common(domain, virq, nr_irqs);
   114	}
   115	
   116	static int hyperv_irq_remappng_activate(struct irq_domain *domain,
   117				  struct irq_data *irq_data, bool reserve)
   118	{
   119		struct irq_cfg *cfg = irqd_cfg(irq_data);
   120		struct IO_APIC_route_entry *entry = irq_data->chip_data;
   121	
   122		entry->dest = cfg->dest_apicid;
   123		entry->vector = cfg->vector;
   124	
   125		return 0;
   126	}
   127	
   128	static struct irq_domain_ops hyperv_ir_domain_ops = {
   129		.alloc = hyperv_irq_remapping_alloc,
   130		.free = hyperv_irq_remapping_free,
   131		.activate = hyperv_irq_remappng_activate,
   132	};
   133	
   134	static int __init hyperv_prepare_irq_remapping(void)
   135	{
   136		struct fwnode_handle *fn;
   137		u32 apic_id;
   138		int i;
   139	
   140		if (x86_hyper_type != X86_HYPER_MS_HYPERV ||
   141		    !x2apic_supported())
   142			return -ENODEV;
   143	
   144		fn = irq_domain_alloc_named_id_fwnode("HYPERV-IR", 0);
   145		if (!fn)
   146			return -ENOMEM;
   147	
   148		ioapic_ir_domain =
 > 149			irq_domain_create_hierarchy(arch_get_ir_parent_domain(),
   150					0, IOAPIC_REMAPPING_ENTRY, fn,
   151					&hyperv_ir_domain_ops, NULL);
   152	
   153		irq_domain_free_fwnode(fn);
   154	
   155		/*
   156		 * Hyper-V doesn't provide irq remapping function for
   157		 * IO-APIC and so IO-APIC only accepts 8-bit APIC ID.
   158		 * Cpu's APIC ID is read from ACPI MADT table and APIC IDs
   159		 * in the MADT table on Hyper-v are sorted monotonic increasingly.
   160		 * APIC ID reflects cpu topology. There maybe some APIC ID
   161		 * gaps when cpu number in a socket is not power of two. Prepare
   162		 * max cpu affinity for IOAPIC irqs. Scan cpu 0-255 and set cpu
   163		 * into ioapic_max_cpumask if its APIC ID is less than 256.
   164		 */
   165		for (i = 0; i < 256; i++) {
   166			apic_id = cpu_physical_id(i);
   167			if (apic_id > 255)
   168				continue;
   169	
   170			cpumask_set_cpu(i, &ioapic_max_cpumask);
   171		}
   172	
   173		return 0;
   174	}
   175	
   176	static int __init hyperv_enable_irq_remapping(void)
   177	{
   178		return IRQ_REMAP_X2APIC_MODE;
   179	}
   180	
   181	static struct irq_domain *hyperv_get_ir_irq_domain(struct irq_alloc_info *info)
   182	{
   183		if (info->type == X86_IRQ_ALLOC_TYPE_IOAPIC)
   184			return ioapic_ir_domain;
   185		else
   186			return NULL;
   187	}
   188	
 > 189	struct irq_remap_ops hyperv_irq_remap_ops = {
 > 190		.prepare		= hyperv_prepare_irq_remapping,
 > 191		.enable			= hyperv_enable_irq_remapping,
 > 192		.get_ir_irq_domain	= hyperv_get_ir_irq_domain,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66583 bytes --]