[PATCH] iommu/hyper-v: Allow hyperv irq remapping without x2apic

[PATCH] iommu/hyper-v: Allow hyperv irq remapping without x2apic

[Nuno Das Neves]

If x2apic is not available, hyperv-iommu skips remapping
irqs. This breaks root partition which always needs irqs
remapped.

Fix this by allowing irq remapping regardless of x2apic,
and change hyperv_enable_irq_remapping() to return
IRQ_REMAP_XAPIC_MODE in case x2apic is missing.

Tested with root and non-root hyperv partitions.

Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
---
 drivers/iommu/Kconfig        | 6 +++---
 drivers/iommu/hyperv-iommu.c | 7 ++++---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index dc5f7a156ff5..cf7433652db0 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -474,13 +474,13 @@ config QCOM_IOMMU
 	  Support for IOMMU on certain Qualcomm SoCs.
 
 config HYPERV_IOMMU
-	bool "Hyper-V x2APIC IRQ Handling"
+	bool "Hyper-V IRQ Handling"
 	depends on HYPERV && X86
 	select IOMMU_API
 	default HYPERV
 	help
-	  Stub IOMMU driver to handle IRQs as to allow Hyper-V Linux
-	  guests to run with x2APIC mode enabled.
+	  Stub IOMMU driver to handle IRQs to support Hyper-V Linux
+	  guest and root partitions.
 
 config VIRTIO_IOMMU
 	tristate "Virtio IOMMU driver"
diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
index e190bb8c225c..abd1826a9e63 100644
--- a/drivers/iommu/hyperv-iommu.c
+++ b/drivers/iommu/hyperv-iommu.c
@@ -123,8 +123,7 @@ static int __init hyperv_prepare_irq_remapping(void)
 	const struct irq_domain_ops *ops;
 
 	if (!hypervisor_is_type(X86_HYPER_MS_HYPERV) ||
-	    x86_init.hyper.msi_ext_dest_id() ||
-	    !x2apic_supported())
+	    x86_init.hyper.msi_ext_dest_id())
 		return -ENODEV;
 
 	if (hv_root_partition) {
@@ -170,7 +169,9 @@ static int __init hyperv_prepare_irq_remapping(void)
 
 static int __init hyperv_enable_irq_remapping(void)
 {
-	return IRQ_REMAP_X2APIC_MODE;
+	if (x2apic_supported())
+		return IRQ_REMAP_X2APIC_MODE;
+	return IRQ_REMAP_XAPIC_MODE;
 }
 
 struct irq_remap_ops hyperv_irq_remap_ops = {
-- 
2.25.1

Re: [PATCH] iommu/hyper-v: Allow hyperv irq remapping without x2apic

[Stanislav Kinsburskii]

On Wed, Nov 09, 2022 at 11:07:33AM -0800, Nuno Das Neves wrote:
> If x2apic is not available, hyperv-iommu skips remapping
> irqs. This breaks root partition which always needs irqs
> remapped.
> 
> Fix this by allowing irq remapping regardless of x2apic,
> and change hyperv_enable_irq_remapping() to return
> IRQ_REMAP_XAPIC_MODE in case x2apic is missing.
> 
> Tested with root and non-root hyperv partitions.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
>  drivers/iommu/Kconfig        | 6 +++---
>  drivers/iommu/hyperv-iommu.c | 7 ++++---
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index dc5f7a156ff5..cf7433652db0 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -474,13 +474,13 @@ config QCOM_IOMMU
>  	  Support for IOMMU on certain Qualcomm SoCs.
>  
>  config HYPERV_IOMMU
> -	bool "Hyper-V x2APIC IRQ Handling"
> +	bool "Hyper-V IRQ Handling"
>  	depends on HYPERV && X86
>  	select IOMMU_API
>  	default HYPERV
>  	help
> -	  Stub IOMMU driver to handle IRQs as to allow Hyper-V Linux
> -	  guests to run with x2APIC mode enabled.
> +	  Stub IOMMU driver to handle IRQs to support Hyper-V Linux
> +	  guest and root partitions.
>  
>  config VIRTIO_IOMMU
>  	tristate "Virtio IOMMU driver"
> diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> index e190bb8c225c..abd1826a9e63 100644
> --- a/drivers/iommu/hyperv-iommu.c
> +++ b/drivers/iommu/hyperv-iommu.c
> @@ -123,8 +123,7 @@ static int __init hyperv_prepare_irq_remapping(void)
>  	const struct irq_domain_ops *ops;
>  
>  	if (!hypervisor_is_type(X86_HYPER_MS_HYPERV) ||
> -	    x86_init.hyper.msi_ext_dest_id() ||
> -	    !x2apic_supported())
> +	    x86_init.hyper.msi_ext_dest_id())
>  		return -ENODEV;
>  
>  	if (hv_root_partition) {
> @@ -170,7 +169,9 @@ static int __init hyperv_prepare_irq_remapping(void)
>  
>  static int __init hyperv_enable_irq_remapping(void)
>  {
> -	return IRQ_REMAP_X2APIC_MODE;
> +	if (x2apic_supported())
> +		return IRQ_REMAP_X2APIC_MODE;
> +	return IRQ_REMAP_XAPIC_MODE;
>  }
>  
>  struct irq_remap_ops hyperv_irq_remap_ops = {
> -- 
> 2.25.1

Reviewed-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>

Re: [PATCH] iommu/hyper-v: Allow hyperv irq remapping without x2apic

[Wei Liu]

Hi Tianyu

On Wed, Nov 09, 2022 at 11:07:33AM -0800, Nuno Das Neves wrote:
> If x2apic is not available, hyperv-iommu skips remapping
> irqs. This breaks root partition which always needs irqs
> remapped.
> 
> Fix this by allowing irq remapping regardless of x2apic,
> and change hyperv_enable_irq_remapping() to return
> IRQ_REMAP_XAPIC_MODE in case x2apic is missing.
> 

Do you remember why it was x2apic only?

We tested this patch on different VM SKUs and it worked fine. I'm just
wondering if there would be some subtle breakages that we couldn't
easily test.

Thanks,
Wei.

> Tested with root and non-root hyperv partitions.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
>  drivers/iommu/Kconfig        | 6 +++---
>  drivers/iommu/hyperv-iommu.c | 7 ++++---
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index dc5f7a156ff5..cf7433652db0 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -474,13 +474,13 @@ config QCOM_IOMMU
>  	  Support for IOMMU on certain Qualcomm SoCs.
>  
>  config HYPERV_IOMMU
> -	bool "Hyper-V x2APIC IRQ Handling"
> +	bool "Hyper-V IRQ Handling"
>  	depends on HYPERV && X86
>  	select IOMMU_API
>  	default HYPERV
>  	help
> -	  Stub IOMMU driver to handle IRQs as to allow Hyper-V Linux
> -	  guests to run with x2APIC mode enabled.
> +	  Stub IOMMU driver to handle IRQs to support Hyper-V Linux
> +	  guest and root partitions.
>  
>  config VIRTIO_IOMMU
>  	tristate "Virtio IOMMU driver"
> diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> index e190bb8c225c..abd1826a9e63 100644
> --- a/drivers/iommu/hyperv-iommu.c
> +++ b/drivers/iommu/hyperv-iommu.c
> @@ -123,8 +123,7 @@ static int __init hyperv_prepare_irq_remapping(void)
>  	const struct irq_domain_ops *ops;
>  
>  	if (!hypervisor_is_type(X86_HYPER_MS_HYPERV) ||
> -	    x86_init.hyper.msi_ext_dest_id() ||
> -	    !x2apic_supported())
> +	    x86_init.hyper.msi_ext_dest_id())
>  		return -ENODEV;
>  
>  	if (hv_root_partition) {
> @@ -170,7 +169,9 @@ static int __init hyperv_prepare_irq_remapping(void)
>  
>  static int __init hyperv_enable_irq_remapping(void)
>  {
> -	return IRQ_REMAP_X2APIC_MODE;
> +	if (x2apic_supported())
> +		return IRQ_REMAP_X2APIC_MODE;
> +	return IRQ_REMAP_XAPIC_MODE;
>  }
>  
>  struct irq_remap_ops hyperv_irq_remap_ops = {
> -- 
> 2.25.1
> 

RE: [PATCH] iommu/hyper-v: Allow hyperv irq remapping without x2apic

[Michael Kelley (LINUX)]

From: Wei Liu <wei.liu@kernel.org> Sent: Friday, November 11, 2022 8:33 AM
> 
> Hi Tianyu
> 
> On Wed, Nov 09, 2022 at 11:07:33AM -0800, Nuno Das Neves wrote:
> > If x2apic is not available, hyperv-iommu skips remapping
> > irqs. This breaks root partition which always needs irqs
> > remapped.
> >
> > Fix this by allowing irq remapping regardless of x2apic,
> > and change hyperv_enable_irq_remapping() to return
> > IRQ_REMAP_XAPIC_MODE in case x2apic is missing.
> >
> 
> Do you remember why it was x2apic only?
> 
> We tested this patch on different VM SKUs and it worked fine. I'm just
> wondering if there would be some subtle breakages that we couldn't
> easily test.
> 
> Thanks,
> Wei.

My recollection is that originally Hyper-V provided the x2apic in the
guest only when the number of vCPUs exceeded 255, and that was
the only case where IRQ remapping was needed.  The intent was to
not disturb the case where # of vCPUs was < 255 and the xapic is used.
I don't remember there being any potential for subtle breakages.

I think more recent versions of Hyper-V now provide the x2apic
in the guest in some cases when # of vCPUs is < 255.

Michael

> 
> > Tested with root and non-root hyperv partitions.
> >
> > Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> > ---
> >  drivers/iommu/Kconfig        | 6 +++---
> >  drivers/iommu/hyperv-iommu.c | 7 ++++---
> >  2 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index dc5f7a156ff5..cf7433652db0 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -474,13 +474,13 @@ config QCOM_IOMMU
> >  	  Support for IOMMU on certain Qualcomm SoCs.
> >
> >  config HYPERV_IOMMU
> > -	bool "Hyper-V x2APIC IRQ Handling"
> > +	bool "Hyper-V IRQ Handling"
> >  	depends on HYPERV && X86
> >  	select IOMMU_API
> >  	default HYPERV
> >  	help
> > -	  Stub IOMMU driver to handle IRQs as to allow Hyper-V Linux
> > -	  guests to run with x2APIC mode enabled.
> > +	  Stub IOMMU driver to handle IRQs to support Hyper-V Linux
> > +	  guest and root partitions.
> >
> >  config VIRTIO_IOMMU
> >  	tristate "Virtio IOMMU driver"
> > diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> > index e190bb8c225c..abd1826a9e63 100644
> > --- a/drivers/iommu/hyperv-iommu.c
> > +++ b/drivers/iommu/hyperv-iommu.c
> > @@ -123,8 +123,7 @@ static int __init hyperv_prepare_irq_remapping(void)
> >  	const struct irq_domain_ops *ops;
> >
> >  	if (!hypervisor_is_type(X86_HYPER_MS_HYPERV) ||
> > -	    x86_init.hyper.msi_ext_dest_id() ||
> > -	    !x2apic_supported())
> > +	    x86_init.hyper.msi_ext_dest_id())
> >  		return -ENODEV;
> >
> >  	if (hv_root_partition) {
> > @@ -170,7 +169,9 @@ static int __init hyperv_prepare_irq_remapping(void)
> >
> >  static int __init hyperv_enable_irq_remapping(void)
> >  {
> > -	return IRQ_REMAP_X2APIC_MODE;
> > +	if (x2apic_supported())
> > +		return IRQ_REMAP_X2APIC_MODE;
> > +	return IRQ_REMAP_XAPIC_MODE;
> >  }
> >
> >  struct irq_remap_ops hyperv_irq_remap_ops = {
> > --
> > 2.25.1
> >

Re: [PATCH] iommu/hyper-v: Allow hyperv irq remapping without x2apic

[Wei Liu]

On Fri, Nov 11, 2022 at 04:55:22PM +0000, Michael Kelley (LINUX) wrote:
> From: Wei Liu <wei.liu@kernel.org> Sent: Friday, November 11, 2022 8:33 AM
> > 
> > Hi Tianyu
> > 
> > On Wed, Nov 09, 2022 at 11:07:33AM -0800, Nuno Das Neves wrote:
> > > If x2apic is not available, hyperv-iommu skips remapping
> > > irqs. This breaks root partition which always needs irqs
> > > remapped.
> > >
> > > Fix this by allowing irq remapping regardless of x2apic,
> > > and change hyperv_enable_irq_remapping() to return
> > > IRQ_REMAP_XAPIC_MODE in case x2apic is missing.
> > >
> > 
> > Do you remember why it was x2apic only?
> > 
> > We tested this patch on different VM SKUs and it worked fine. I'm just
> > wondering if there would be some subtle breakages that we couldn't
> > easily test.
> > 
> > Thanks,
> > Wei.
> 
> My recollection is that originally Hyper-V provided the x2apic in the
> guest only when the number of vCPUs exceeded 255, and that was
> the only case where IRQ remapping was needed.  The intent was to
> not disturb the case where # of vCPUs was < 255 and the xapic is used.
> I don't remember there being any potential for subtle breakages.

Thanks for the information.

> 
> I think more recent versions of Hyper-V now provide the x2apic
> in the guest in some cases when # of vCPUs is < 255.
> 

On Azure the default for AMD SKUs is still xapic unless the number of
VCPUs exceeds 2XX (can't remember the exact number -- maybe it is 255).

Nuno, can you list the tests you've done? They will need to cover Linux
running as a normal guest on Azure and Hyper-V.

Thanks,
Wei.


> Michael
> 
> > 
> > > Tested with root and non-root hyperv partitions.
> > >
> > > Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> > > ---
> > >  drivers/iommu/Kconfig        | 6 +++---
> > >  drivers/iommu/hyperv-iommu.c | 7 ++++---
> > >  2 files changed, 7 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > > index dc5f7a156ff5..cf7433652db0 100644
> > > --- a/drivers/iommu/Kconfig
> > > +++ b/drivers/iommu/Kconfig
> > > @@ -474,13 +474,13 @@ config QCOM_IOMMU
> > >  	  Support for IOMMU on certain Qualcomm SoCs.
> > >
> > >  config HYPERV_IOMMU
> > > -	bool "Hyper-V x2APIC IRQ Handling"
> > > +	bool "Hyper-V IRQ Handling"
> > >  	depends on HYPERV && X86
> > >  	select IOMMU_API
> > >  	default HYPERV
> > >  	help
> > > -	  Stub IOMMU driver to handle IRQs as to allow Hyper-V Linux
> > > -	  guests to run with x2APIC mode enabled.
> > > +	  Stub IOMMU driver to handle IRQs to support Hyper-V Linux
> > > +	  guest and root partitions.
> > >
> > >  config VIRTIO_IOMMU
> > >  	tristate "Virtio IOMMU driver"
> > > diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> > > index e190bb8c225c..abd1826a9e63 100644
> > > --- a/drivers/iommu/hyperv-iommu.c
> > > +++ b/drivers/iommu/hyperv-iommu.c
> > > @@ -123,8 +123,7 @@ static int __init hyperv_prepare_irq_remapping(void)
> > >  	const struct irq_domain_ops *ops;
> > >
> > >  	if (!hypervisor_is_type(X86_HYPER_MS_HYPERV) ||
> > > -	    x86_init.hyper.msi_ext_dest_id() ||
> > > -	    !x2apic_supported())
> > > +	    x86_init.hyper.msi_ext_dest_id())
> > >  		return -ENODEV;
> > >
> > >  	if (hv_root_partition) {
> > > @@ -170,7 +169,9 @@ static int __init hyperv_prepare_irq_remapping(void)
> > >
> > >  static int __init hyperv_enable_irq_remapping(void)
> > >  {
> > > -	return IRQ_REMAP_X2APIC_MODE;
> > > +	if (x2apic_supported())
> > > +		return IRQ_REMAP_X2APIC_MODE;
> > > +	return IRQ_REMAP_XAPIC_MODE;
> > >  }
> > >
> > >  struct irq_remap_ops hyperv_irq_remap_ops = {
> > > --
> > > 2.25.1
> > >

RE: [PATCH] iommu/hyper-v: Allow hyperv irq remapping without x2apic

[Michael Kelley (LINUX)]

From: Wei Liu <wei.liu@kernel.org> Sent: Friday, November 11, 2022 9:27 AM
> 
> On Fri, Nov 11, 2022 at 04:55:22PM +0000, Michael Kelley (LINUX) wrote:
> > From: Wei Liu <wei.liu@kernel.org> Sent: Friday, November 11, 2022 8:33 AM
> > >
> > > Hi Tianyu
> > >
> > > On Wed, Nov 09, 2022 at 11:07:33AM -0800, Nuno Das Neves wrote:
> > > > If x2apic is not available, hyperv-iommu skips remapping
> > > > irqs. This breaks root partition which always needs irqs
> > > > remapped.
> > > >
> > > > Fix this by allowing irq remapping regardless of x2apic,
> > > > and change hyperv_enable_irq_remapping() to return
> > > > IRQ_REMAP_XAPIC_MODE in case x2apic is missing.
> > > >
> > >
> > > Do you remember why it was x2apic only?
> > >
> > > We tested this patch on different VM SKUs and it worked fine. I'm just
> > > wondering if there would be some subtle breakages that we couldn't
> > > easily test.
> > >
> > > Thanks,
> > > Wei.
> >
> > My recollection is that originally Hyper-V provided the x2apic in the
> > guest only when the number of vCPUs exceeded 255, and that was
> > the only case where IRQ remapping was needed.  The intent was to
> > not disturb the case where # of vCPUs was < 255 and the xapic is used.
> > I don't remember there being any potential for subtle breakages.
> 
> Thanks for the information.
> 
> >
> > I think more recent versions of Hyper-V now provide the x2apic
> > in the guest in some cases when # of vCPUs is < 255.
> >
> 
> On Azure the default for AMD SKUs is still xapic unless the number of
> VCPUs exceeds 2XX (can't remember the exact number -- maybe it is 255).

I don't think Azure has any VM sizes based on AMD processors with more
than 255 vCPUs.  All the >255 vCPUs VM sizes use Intel processors.

FWIW, I have a D2ds_v5 VM (2 CPUs & Intel processor) that shows an
x2apic instead of an xapic.  My memory is vague, but I think that's the
requirements to get an x2apic in a smaller VM:  must be a "v5" series and
must be Intel processor.

Michael

> 
> Nuno, can you list the tests you've done? They will need to cover Linux
> running as a normal guest on Azure and Hyper-V.
> 
> Thanks,
> Wei.
> 
> 
> > Michael
> >
> > >
> > > > Tested with root and non-root hyperv partitions.
> > > >
> > > > Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> > > > ---
> > > >  drivers/iommu/Kconfig        | 6 +++---
> > > >  drivers/iommu/hyperv-iommu.c | 7 ++++---
> > > >  2 files changed, 7 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > > > index dc5f7a156ff5..cf7433652db0 100644
> > > > --- a/drivers/iommu/Kconfig
> > > > +++ b/drivers/iommu/Kconfig
> > > > @@ -474,13 +474,13 @@ config QCOM_IOMMU
> > > >  	  Support for IOMMU on certain Qualcomm SoCs.
> > > >
> > > >  config HYPERV_IOMMU
> > > > -	bool "Hyper-V x2APIC IRQ Handling"
> > > > +	bool "Hyper-V IRQ Handling"
> > > >  	depends on HYPERV && X86
> > > >  	select IOMMU_API
> > > >  	default HYPERV
> > > >  	help
> > > > -	  Stub IOMMU driver to handle IRQs as to allow Hyper-V Linux
> > > > -	  guests to run with x2APIC mode enabled.
> > > > +	  Stub IOMMU driver to handle IRQs to support Hyper-V Linux
> > > > +	  guest and root partitions.
> > > >
> > > >  config VIRTIO_IOMMU
> > > >  	tristate "Virtio IOMMU driver"
> > > > diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
> > > > index e190bb8c225c..abd1826a9e63 100644
> > > > --- a/drivers/iommu/hyperv-iommu.c
> > > > +++ b/drivers/iommu/hyperv-iommu.c
> > > > @@ -123,8 +123,7 @@ static int __init hyperv_prepare_irq_remapping(void)
> > > >  	const struct irq_domain_ops *ops;
> > > >
> > > >  	if (!hypervisor_is_type(X86_HYPER_MS_HYPERV) ||
> > > > -	    x86_init.hyper.msi_ext_dest_id() ||
> > > > -	    !x2apic_supported())
> > > > +	    x86_init.hyper.msi_ext_dest_id())
> > > >  		return -ENODEV;
> > > >
> > > >  	if (hv_root_partition) {
> > > > @@ -170,7 +169,9 @@ static int __init hyperv_prepare_irq_remapping(void)
> > > >
> > > >  static int __init hyperv_enable_irq_remapping(void)
> > > >  {
> > > > -	return IRQ_REMAP_X2APIC_MODE;
> > > > +	if (x2apic_supported())
> > > > +		return IRQ_REMAP_X2APIC_MODE;
> > > > +	return IRQ_REMAP_XAPIC_MODE;
> > > >  }
> > > >
> > > >  struct irq_remap_ops hyperv_irq_remap_ops = {
> > > > --
> > > > 2.25.1
> > > >

Re: [PATCH] iommu/hyper-v: Allow hyperv irq remapping without x2apic

[Nuno Das Neves]

On 11/11/2022 9:58 AM, Michael Kelley (LINUX) wrote:
> From: Wei Liu <wei.liu@kernel.org> Sent: Friday, November 11, 2022 9:27 AM
>>
>> On Fri, Nov 11, 2022 at 04:55:22PM +0000, Michael Kelley (LINUX) wrote:
>>> From: Wei Liu <wei.liu@kernel.org> Sent: Friday, November 11, 2022 8:33 AM
>>>>
>>>> Hi Tianyu
>>>>
>>>> On Wed, Nov 09, 2022 at 11:07:33AM -0800, Nuno Das Neves wrote:
>>>>> If x2apic is not available, hyperv-iommu skips remapping
>>>>> irqs. This breaks root partition which always needs irqs
>>>>> remapped.
>>>>>
>>>>> Fix this by allowing irq remapping regardless of x2apic,
>>>>> and change hyperv_enable_irq_remapping() to return
>>>>> IRQ_REMAP_XAPIC_MODE in case x2apic is missing.
>>>>>
>>>>
>>>> Do you remember why it was x2apic only?
>>>>
>>>> We tested this patch on different VM SKUs and it worked fine. I'm just
>>>> wondering if there would be some subtle breakages that we couldn't
>>>> easily test.
>>>>
>>>> Thanks,
>>>> Wei.
>>>
>>> My recollection is that originally Hyper-V provided the x2apic in the
>>> guest only when the number of vCPUs exceeded 255, and that was
>>> the only case where IRQ remapping was needed.  The intent was to
>>> not disturb the case where # of vCPUs was < 255 and the xapic is used.
>>> I don't remember there being any potential for subtle breakages.
>>
>> Thanks for the information.
>>
>>>
>>> I think more recent versions of Hyper-V now provide the x2apic
>>> in the guest in some cases when # of vCPUs is < 255.
>>>
>>
>> On Azure the default for AMD SKUs is still xapic unless the number of
>> VCPUs exceeds 2XX (can't remember the exact number -- maybe it is 255).
> 
> I don't think Azure has any VM sizes based on AMD processors with more
> than 255 vCPUs.  All the >255 vCPUs VM sizes use Intel processors.
> 
> FWIW, I have a D2ds_v5 VM (2 CPUs & Intel processor) that shows an
> x2apic instead of an xapic.  My memory is vague, but I think that's the
> requirements to get an x2apic in a smaller VM:  must be a "v5" series and
> must be Intel processor.
> 
> Michael
>

Yes this seems to be the case, although I didn't realise it until now!
I sort of assumed since x2apic has been around so long, all the intel VMs
just had it enabled...

>>
>> Nuno, can you list the tests you've done? They will need to cover Linux
>> running as a normal guest on Azure and Hyper-V.
>>>> Thanks,
>> Wei.
>>

I've tested this patch on these Azure SKUs:
- Standard_D2S_v2 (intel xapic)
- Standard_D4ds_v4 (intel xapic)
- Standard_D4ds_v5 (intel x2apic)
- Standard_D4ads_v5 (amd xapic)

I've tested with linux Dom0 (nested hyperv root partition) and as a
regular L1 guest.

>>
>>> Michael
>>>
>>>>
>>>>> Tested with root and non-root hyperv partitions.
>>>>>
>>>>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>>>>> ---
>>>>>  drivers/iommu/Kconfig        | 6 +++---
>>>>>  drivers/iommu/hyperv-iommu.c | 7 ++++---
>>>>>  2 files changed, 7 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
>>>>> index dc5f7a156ff5..cf7433652db0 100644
>>>>> --- a/drivers/iommu/Kconfig
>>>>> +++ b/drivers/iommu/Kconfig
>>>>> @@ -474,13 +474,13 @@ config QCOM_IOMMU
>>>>>  	  Support for IOMMU on certain Qualcomm SoCs.
>>>>>
>>>>>  config HYPERV_IOMMU
>>>>> -	bool "Hyper-V x2APIC IRQ Handling"
>>>>> +	bool "Hyper-V IRQ Handling"
>>>>>  	depends on HYPERV && X86
>>>>>  	select IOMMU_API
>>>>>  	default HYPERV
>>>>>  	help
>>>>> -	  Stub IOMMU driver to handle IRQs as to allow Hyper-V Linux
>>>>> -	  guests to run with x2APIC mode enabled.
>>>>> +	  Stub IOMMU driver to handle IRQs to support Hyper-V Linux
>>>>> +	  guest and root partitions.
>>>>>
>>>>>  config VIRTIO_IOMMU
>>>>>  	tristate "Virtio IOMMU driver"
>>>>> diff --git a/drivers/iommu/hyperv-iommu.c b/drivers/iommu/hyperv-iommu.c
>>>>> index e190bb8c225c..abd1826a9e63 100644
>>>>> --- a/drivers/iommu/hyperv-iommu.c
>>>>> +++ b/drivers/iommu/hyperv-iommu.c
>>>>> @@ -123,8 +123,7 @@ static int __init hyperv_prepare_irq_remapping(void)
>>>>>  	const struct irq_domain_ops *ops;
>>>>>
>>>>>  	if (!hypervisor_is_type(X86_HYPER_MS_HYPERV) ||
>>>>> -	    x86_init.hyper.msi_ext_dest_id() ||
>>>>> -	    !x2apic_supported())
>>>>> +	    x86_init.hyper.msi_ext_dest_id())
>>>>>  		return -ENODEV;
>>>>>
>>>>>  	if (hv_root_partition) {
>>>>> @@ -170,7 +169,9 @@ static int __init hyperv_prepare_irq_remapping(void)
>>>>>
>>>>>  static int __init hyperv_enable_irq_remapping(void)
>>>>>  {
>>>>> -	return IRQ_REMAP_X2APIC_MODE;
>>>>> +	if (x2apic_supported())
>>>>> +		return IRQ_REMAP_X2APIC_MODE;
>>>>> +	return IRQ_REMAP_XAPIC_MODE;
>>>>>  }
>>>>>
>>>>>  struct irq_remap_ops hyperv_irq_remap_ops = {
>>>>> --
>>>>> 2.25.1
>>>>>

Re: [PATCH] iommu/hyper-v: Allow hyperv irq remapping without x2apic

[Wei Liu]

On Fri, Nov 11, 2022 at 02:53:59PM -0800, Nuno Das Neves wrote:
> On 11/11/2022 9:58 AM, Michael Kelley (LINUX) wrote:
> > From: Wei Liu <wei.liu@kernel.org> Sent: Friday, November 11, 2022 9:27 AM
[...]
> 
> I've tested this patch on these Azure SKUs:
> - Standard_D2S_v2 (intel xapic)
> - Standard_D4ds_v4 (intel xapic)
> - Standard_D4ds_v5 (intel x2apic)
> - Standard_D4ads_v5 (amd xapic)
> 
> I've tested with linux Dom0 (nested hyperv root partition) and as a
> regular L1 guest.
> 

Okay. I think your tests are good.

Michael, do you have any further concern?

Thanks,
Wei.

RE: [PATCH] iommu/hyper-v: Allow hyperv irq remapping without x2apic

[Michael Kelley (LINUX)]

From: Wei Liu <wei.liu@kernel.org> Sent: Monday, November 14, 2022 5:59 AM
> 
> On Fri, Nov 11, 2022 at 02:53:59PM -0800, Nuno Das Neves wrote:
> > On 11/11/2022 9:58 AM, Michael Kelley (LINUX) wrote:
> > > From: Wei Liu <wei.liu@kernel.org> Sent: Friday, November 11, 2022 9:27 AM
> [...]
> >
> > I've tested this patch on these Azure SKUs:
> > - Standard_D2S_v2 (intel xapic)
> > - Standard_D4ds_v4 (intel xapic)
> > - Standard_D4ds_v5 (intel x2apic)
> > - Standard_D4ads_v5 (amd xapic)
> >
> > I've tested with linux Dom0 (nested hyperv root partition) and as a
> > regular L1 guest.
> >
> 
> Okay. I think your tests are good.
> 
> Michael, do you have any further concern?
> 

If ms_hyperv_msi_ext_dest_id() returns "true", then
hyperv_prepare_irq_remapping() will still return -ENODEV and you
won't get interrupt remapping because it isn't needed, at least not
for guest VMs.  Is that what we want for the root partition?  Or does
ms_hyperv_msi_ext_dest_id() only return true in a guest partition,
and not in the root partition?  See commit d981059e13ff.

Michael

Re: [PATCH] iommu/hyper-v: Allow hyperv irq remapping without x2apic

[Nuno Das Neves]

On 11/14/2022 11:09 AM, Michael Kelley (LINUX) wrote:
> From: Wei Liu <wei.liu@kernel.org> Sent: Monday, November 14, 2022 5:59 AM
>>
>> On Fri, Nov 11, 2022 at 02:53:59PM -0800, Nuno Das Neves wrote:
>>> On 11/11/2022 9:58 AM, Michael Kelley (LINUX) wrote:
>>>> From: Wei Liu <wei.liu@kernel.org> Sent: Friday, November 11, 2022 9:27 AM
>> [...]
>>>
>>> I've tested this patch on these Azure SKUs:
>>> - Standard_D2S_v2 (intel xapic)
>>> - Standard_D4ds_v4 (intel xapic)
>>> - Standard_D4ds_v5 (intel x2apic)
>>> - Standard_D4ads_v5 (amd xapic)
>>>
>>> I've tested with linux Dom0 (nested hyperv root partition) and as a
>>> regular L1 guest.
>>>
>>
>> Okay. I think your tests are good.
>>
>> Michael, do you have any further concern?
>>
> 
> If ms_hyperv_msi_ext_dest_id() returns "true", then
> hyperv_prepare_irq_remapping() will still return -ENODEV and you
> won't get interrupt remapping because it isn't needed, at least not
> for guest VMs.  Is that what we want for the root partition?  Or does
> ms_hyperv_msi_ext_dest_id() only return true in a guest partition,
> and not in the root partition?  See commit d981059e13ff.
> 

I did some digging, and I *think* this function will always return "false"
in the root partition.

The cpuids (HYPERV_CPUID_VIRT_STACK_*) that determine the result of
ms_hyperv_msi_ext_dest_id() are implemented by the virtualization stack
in Azure, so for L1 guests it depends on that.

But, for nested root, the nested hypervisor controls which cpuids the
root partition sees, and VIRTUALIZATION_STACK_CPUID_INTERFACE is not in
that list.

I tested this too; if I boot the kernel with an L1 guest, I can see that
the HYPERV_CPUID_VIRT_STACK_INTERFACE contains the "VS#1" signature.
If I boot as L2 Root, the signature is not present.

I'm reasonably certain, but if I'm wrong we'll see the same breakage for
the same reason and we can fix it I guess.

RE: [PATCH] iommu/hyper-v: Allow hyperv irq remapping without x2apic

[Michael Kelley (LINUX)]

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Tuesday, November 15, 2022 5:25 PM
> 
> On 11/14/2022 11:09 AM, Michael Kelley (LINUX) wrote:
> > From: Wei Liu <wei.liu@kernel.org> Sent: Monday, November 14, 2022 5:59 AM
> >>
> >> On Fri, Nov 11, 2022 at 02:53:59PM -0800, Nuno Das Neves wrote:
> >>> On 11/11/2022 9:58 AM, Michael Kelley (LINUX) wrote:
> >>>> From: Wei Liu <wei.liu@kernel.org> Sent: Friday, November 11, 2022 9:27 AM
> >> [...]
> >>>
> >>> I've tested this patch on these Azure SKUs:
> >>> - Standard_D2S_v2 (intel xapic)
> >>> - Standard_D4ds_v4 (intel xapic)
> >>> - Standard_D4ds_v5 (intel x2apic)
> >>> - Standard_D4ads_v5 (amd xapic)
> >>>
> >>> I've tested with linux Dom0 (nested hyperv root partition) and as a
> >>> regular L1 guest.
> >>>
> >>
> >> Okay. I think your tests are good.
> >>
> >> Michael, do you have any further concern?
> >>
> >
> > If ms_hyperv_msi_ext_dest_id() returns "true", then
> > hyperv_prepare_irq_remapping() will still return -ENODEV and you
> > won't get interrupt remapping because it isn't needed, at least not
> > for guest VMs.  Is that what we want for the root partition?  Or does
> > ms_hyperv_msi_ext_dest_id() only return true in a guest partition,
> > and not in the root partition?  See commit d981059e13ff.
> >
> 
> I did some digging, and I *think* this function will always return "false"
> in the root partition.
> 
> The cpuids (HYPERV_CPUID_VIRT_STACK_*) that determine the result of
> ms_hyperv_msi_ext_dest_id() are implemented by the virtualization stack
> in Azure, so for L1 guests it depends on that.
> 
> But, for nested root, the nested hypervisor controls which cpuids the
> root partition sees, and VIRTUALIZATION_STACK_CPUID_INTERFACE is not in
> that list.
> 
> I tested this too; if I boot the kernel with an L1 guest, I can see that
> the HYPERV_CPUID_VIRT_STACK_INTERFACE contains the "VS#1" signature.
> If I boot as L2 Root, the signature is not present.
> 
> I'm reasonably certain, but if I'm wrong we'll see the same breakage for
> the same reason and we can fix it I guess.

Sounds good.  Please leave a comment somewhere in the code summarizing
what you found, and stating the expectation that ms_hyperv_msi_ext_dest_id()
returns "false" in the root partition.

Michael