Re: [PATCH V3 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
Hi Olaf: Thanks for your review. On Fri, Feb 8, 2019 at 10:52 PM Olaf Hering wrote: > > On Thu, Feb 07, lantianyu1...@gmail.com wrote: > > > +++ b/drivers/iommu/Kconfig > > +config HYPERV_IOMMU > > + bool "Hyper-V x2APIC IRQ Handling" > > + depends on HYPERV > > + select IOMMU_API > > + help > > > Consider adding 'default HYPERV' like some other drivers already do it. > > Olaf Good suggestion and will update. Thanks. -- Best regards Tianyu Lan
Re: [PATCH V3 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
On Fri, Feb 8, 2019 at 1:15 AM Vitaly Kuznetsov wrote:
>
> lantianyu1...@gmail.com writes:
>
> > From: Lan Tianyu
> >
> > 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.
> >
> > Define 24 IO-APIC remapping entries because Hyper-V only expose one
> > single IO-APIC and one IO-APIC has 24 pins according IO-APIC spec(
> > https://pdos.csail.mit.edu/6.828/2016/readings/ia32/ioapic.pdf).
> >
> > Signed-off-by: Lan Tianyu
> > ---
> > Change since v2:
> >- Improve comment about why save IO-APIC entry in the irq chip data.
> >- Some code improvement.
> >- Improve statement in the IOMMU Kconfig.
> >
> > 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 | 194
> > ++
> > drivers/iommu/irq_remapping.c | 3 +
> > drivers/iommu/irq_remapping.h | 1 +
> > 5 files changed, 207 insertions(+)
> > create mode 100644 drivers/iommu/hyperv-iommu.c
> >
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index 45d7021..6090935 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 x2APIC IRQ Handling"
> > + depends on HYPERV
> > + select IOMMU_API
> > + help
> > + Stub IOMMU driver to handle IRQs as to allow Hyper-V Linux
> > + guests to run with x2APIC mode enabled.
> > +
> > 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 000..d8572c5
> > --- /dev/null
> > +++ b/drivers/iommu/hyperv-iommu.c
> > @@ -0,0 +1,194 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * Hyper-V stub IOMMU driver.
> > + *
> > + * Copyright (C) 2019, Microsoft, Inc.
> > + *
> > + * Author : Lan Tianyu
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "irq_remapping.h"
> > +
> > +#ifdef CONFIG_IRQ_REMAP
> > +
> > +/*
> > + * According 82093AA IO-APIC spec , IO APIC has a 24-entry Interrupt
> > + * Redirection Table. Hyper-V exposes one single IO-APIC and so define
> > + * 24 IO APIC remmapping entries.
> > + */
> > +#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;
> > + int ret;
> > +
> > + /* Return error If new irq affinity is out of ioapic_max_cpumask. */
> > + if (!cpumask_subset(mask, _max_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
Re: [PATCH V3 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
Hi Alex:
Thanks for your review.
On Fri, Feb 8, 2019 at 2:15 AM Alex Williamson
wrote:
>
> On Thu, 7 Feb 2019 23:33:48 +0800
> lantianyu1...@gmail.com wrote:
>
> > From: Lan Tianyu
> >
> > 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.
> >
> > Define 24 IO-APIC remapping entries because Hyper-V only expose one
> > single IO-APIC and one IO-APIC has 24 pins according IO-APIC spec(
> > https://pdos.csail.mit.edu/6.828/2016/readings/ia32/ioapic.pdf).
> >
> > Signed-off-by: Lan Tianyu
> > ---
> > Change since v2:
> >- Improve comment about why save IO-APIC entry in the irq chip data.
> >- Some code improvement.
> >- Improve statement in the IOMMU Kconfig.
> >
> > 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 | 194
> > ++
> > drivers/iommu/irq_remapping.c | 3 +
> > drivers/iommu/irq_remapping.h | 1 +
> > 5 files changed, 207 insertions(+)
> > create mode 100644 drivers/iommu/hyperv-iommu.c
> ...
> > diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> > new file mode 100644
> > index 000..d8572c5
> > --- /dev/null
> > +++ b/drivers/iommu/hyperv-iommu.c
> ...
> > +static int __init hyperv_prepare_irq_remapping(void)
> > +{
> > + struct fwnode_handle *fn;
> > + int i;
> > +
> > + if (!hypervisor_is_type(x86_hyper_type) ||
> > + !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,
> > + _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++)
> > + if (cpu_physical_id(i) < 256)
> > + cpumask_set_cpu(i, _max_cpumask);
>
> This looks sketchy. What if NR_CPUS is less than 256? Thanks,
Nice catch. I should check NR_CPUS here. Will update. Thanks.
--
Best regards
Tianyu Lan
Re: [PATCH V3 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
On Thu, Feb 07, lantianyu1...@gmail.com wrote: > +++ b/drivers/iommu/Kconfig > +config HYPERV_IOMMU > + bool "Hyper-V x2APIC IRQ Handling" > + depends on HYPERV > + select IOMMU_API > + help Consider adding 'default HYPERV' like some other drivers already do it. Olaf signature.asc Description: PGP signature
Re: [PATCH V3 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
On Thu, 7 Feb 2019 23:33:48 +0800
lantianyu1...@gmail.com wrote:
> From: Lan Tianyu
>
> 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.
>
> Define 24 IO-APIC remapping entries because Hyper-V only expose one
> single IO-APIC and one IO-APIC has 24 pins according IO-APIC spec(
> https://pdos.csail.mit.edu/6.828/2016/readings/ia32/ioapic.pdf).
>
> Signed-off-by: Lan Tianyu
> ---
> Change since v2:
>- Improve comment about why save IO-APIC entry in the irq chip data.
>- Some code improvement.
>- Improve statement in the IOMMU Kconfig.
>
> 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 | 194
> ++
> drivers/iommu/irq_remapping.c | 3 +
> drivers/iommu/irq_remapping.h | 1 +
> 5 files changed, 207 insertions(+)
> create mode 100644 drivers/iommu/hyperv-iommu.c
...
> diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> new file mode 100644
> index 000..d8572c5
> --- /dev/null
> +++ b/drivers/iommu/hyperv-iommu.c
...
> +static int __init hyperv_prepare_irq_remapping(void)
> +{
> + struct fwnode_handle *fn;
> + int i;
> +
> + if (!hypervisor_is_type(x86_hyper_type) ||
> + !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,
> + _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++)
> + if (cpu_physical_id(i) < 256)
> + cpumask_set_cpu(i, _max_cpumask);
This looks sketchy. What if NR_CPUS is less than 256? Thanks,
Alex
> +
> + return 0;
> +}
Re: [PATCH V3 2/3] HYPERV/IOMMU: Add Hyper-V stub IOMMU driver
lantianyu1...@gmail.com writes:
> From: Lan Tianyu
>
> 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.
>
> Define 24 IO-APIC remapping entries because Hyper-V only expose one
> single IO-APIC and one IO-APIC has 24 pins according IO-APIC spec(
> https://pdos.csail.mit.edu/6.828/2016/readings/ia32/ioapic.pdf).
>
> Signed-off-by: Lan Tianyu
> ---
> Change since v2:
>- Improve comment about why save IO-APIC entry in the irq chip data.
>- Some code improvement.
>- Improve statement in the IOMMU Kconfig.
>
> 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 | 194
> ++
> drivers/iommu/irq_remapping.c | 3 +
> drivers/iommu/irq_remapping.h | 1 +
> 5 files changed, 207 insertions(+)
> create mode 100644 drivers/iommu/hyperv-iommu.c
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 45d7021..6090935 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 x2APIC IRQ Handling"
> + depends on HYPERV
> + select IOMMU_API
> + help
> + Stub IOMMU driver to handle IRQs as to allow Hyper-V Linux
> + guests to run with x2APIC mode enabled.
> +
> 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 000..d8572c5
> --- /dev/null
> +++ b/drivers/iommu/hyperv-iommu.c
> @@ -0,0 +1,194 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Hyper-V stub IOMMU driver.
> + *
> + * Copyright (C) 2019, Microsoft, Inc.
> + *
> + * Author : Lan Tianyu
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include "irq_remapping.h"
> +
> +#ifdef CONFIG_IRQ_REMAP
> +
> +/*
> + * According 82093AA IO-APIC spec , IO APIC has a 24-entry Interrupt
> + * Redirection Table. Hyper-V exposes one single IO-APIC and so define
> + * 24 IO APIC remmapping entries.
> + */
> +#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;
> + int ret;
> +
> + /* Return error If new irq affinity is out of ioapic_max_cpumask. */
> + if (!cpumask_subset(mask, _max_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 ||
