VMX posted interrupts support can now be excluded from x86 build along with
VMX code itself, but still we may want to keep the possibility to use
VT-d IOMMU driver in non-HVM setups.
So we guard vmx_pi_hooks_{assign/deassign} with some checks for such a case.

No functional change intended here.

Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
---
 xen/drivers/passthrough/vtd/iommu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index e13be244c1..ad78282250 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2772,7 +2772,7 @@ static int cf_check reassign_device_ownership(
 
     if ( !QUARANTINE_SKIP(target, pdev->arch.vtd.pgd_maddr) )
     {
-        if ( !has_arch_pdevs(target) )
+        if ( cpu_has_vmx && !has_arch_pdevs(target) )
             vmx_pi_hooks_assign(target);
 
 #ifdef CONFIG_PV
@@ -2806,7 +2806,7 @@ static int cf_check reassign_device_ownership(
     }
     if ( ret )
     {
-        if ( !has_arch_pdevs(target) )
+        if ( cpu_has_vmx && !has_arch_pdevs(target) )
             vmx_pi_hooks_deassign(target);
         return ret;
     }
@@ -2824,7 +2824,7 @@ static int cf_check reassign_device_ownership(
         write_unlock(&target->pci_lock);
     }
 
-    if ( !has_arch_pdevs(source) )
+    if ( cpu_has_vmx && !has_arch_pdevs(source) )
         vmx_pi_hooks_deassign(source);
 
     /*
-- 
2.25.1

On 15.05.2024 11:26, Sergiy Kibrik wrote:
> VMX posted interrupts support can now be excluded from x86 build along with
> VMX code itself, but still we may want to keep the possibility to use
> VT-d IOMMU driver in non-HVM setups.
> So we guard vmx_pi_hooks_{assign/deassign} with some checks for such a case.

But both function already have a stub each. Isn't is merely a matter of
changing when those stubs come into play?

Jan

16.05.24 15:15, Jan Beulich:
> On 15.05.2024 11:26, Sergiy Kibrik wrote:
>> VMX posted interrupts support can now be excluded from x86 build along with
>> VMX code itself, but still we may want to keep the possibility to use
>> VT-d IOMMU driver in non-HVM setups.
>> So we guard vmx_pi_hooks_{assign/deassign} with some checks for such a case.
> 
> But both function already have a stub each. Isn't is merely a matter of
> changing when those stubs come into play?
> 

indeed, we've got stubs already provided. Then this becomes a nice one 
line change.

   -Sergiy

On Wed, 15 May 2024, Sergiy Kibrik wrote:
> VMX posted interrupts support can now be excluded from x86 build along with
> VMX code itself, but still we may want to keep the possibility to use
> VT-d IOMMU driver in non-HVM setups.
> So we guard vmx_pi_hooks_{assign/deassign} with some checks for such a case.
> 
> No functional change intended here.
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>

I know that Andrew was keep on having a separate Kconfig option for
VT-D, separate from VMX. But still, couldn't we make the VT-D Kconfig
option depending on CONFIG_VMX?

To me, VT-D should require VMX, without VMX it should not be possible to
enable VT-D.

This comment goes in the same direction of my previous comment regarding
the vpmu: we are trying to make things more configurable and flexible
and that's good, but we don't necessary need to make all possible
combination work. VT-D without VMX is another one of those combination
that I would only enable after a customer asks.


> ---
>  xen/drivers/passthrough/vtd/iommu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
> index e13be244c1..ad78282250 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2772,7 +2772,7 @@ static int cf_check reassign_device_ownership(
>  
>      if ( !QUARANTINE_SKIP(target, pdev->arch.vtd.pgd_maddr) )
>      {
> -        if ( !has_arch_pdevs(target) )
> +        if ( cpu_has_vmx && !has_arch_pdevs(target) )
>              vmx_pi_hooks_assign(target);
>  
>  #ifdef CONFIG_PV
> @@ -2806,7 +2806,7 @@ static int cf_check reassign_device_ownership(
>      }
>      if ( ret )
>      {
> -        if ( !has_arch_pdevs(target) )
> +        if ( cpu_has_vmx && !has_arch_pdevs(target) )
>              vmx_pi_hooks_deassign(target);
>          return ret;
>      }
> @@ -2824,7 +2824,7 @@ static int cf_check reassign_device_ownership(
>          write_unlock(&target->pci_lock);
>      }
>  
> -    if ( !has_arch_pdevs(source) )
> +    if ( cpu_has_vmx && !has_arch_pdevs(source) )
>          vmx_pi_hooks_deassign(source);
>  
>      /*
> -- 
> 2.25.1
>

On 16.05.2024 02:54, Stefano Stabellini wrote:
> On Wed, 15 May 2024, Sergiy Kibrik wrote:
>> VMX posted interrupts support can now be excluded from x86 build along with
>> VMX code itself, but still we may want to keep the possibility to use
>> VT-d IOMMU driver in non-HVM setups.
>> So we guard vmx_pi_hooks_{assign/deassign} with some checks for such a case.
>>
>> No functional change intended here.
>>
>> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> 
> I know that Andrew was keep on having a separate Kconfig option for
> VT-D, separate from VMX. But still, couldn't we make the VT-D Kconfig
> option depending on CONFIG_VMX?
> 
> To me, VT-D should require VMX, without VMX it should not be possible to
> enable VT-D.
> 
> This comment goes in the same direction of my previous comment regarding
> the vpmu: we are trying to make things more configurable and flexible
> and that's good, but we don't necessary need to make all possible
> combination work. VT-D without VMX is another one of those combination
> that I would only enable after a customer asks.

Well. Imo again the configuration should be permitted. VMX and INTEL_IOMMU
ought to be default to INTEL, but permit being turned on/off in all cases.
(That's btw part of the reason why I continue to be unhappy with it being
INTEL where really INTEL_CPU was meant. If what is INTEL now would be
INTEL_CPU, INTEL could be an umbrella option for all three, or two if we
were to tie VMX to INTEL_CPU.)

Jan

On Thu, 16 May 2024, Jan Beulich wrote:
> On 16.05.2024 02:54, Stefano Stabellini wrote:
> > On Wed, 15 May 2024, Sergiy Kibrik wrote:
> >> VMX posted interrupts support can now be excluded from x86 build along with
> >> VMX code itself, but still we may want to keep the possibility to use
> >> VT-d IOMMU driver in non-HVM setups.
> >> So we guard vmx_pi_hooks_{assign/deassign} with some checks for such a case.
> >>
> >> No functional change intended here.
> >>
> >> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> > 
> > I know that Andrew was keep on having a separate Kconfig option for
> > VT-D, separate from VMX. But still, couldn't we make the VT-D Kconfig
> > option depending on CONFIG_VMX?
> > 
> > To me, VT-D should require VMX, without VMX it should not be possible to
> > enable VT-D.
> > 
> > This comment goes in the same direction of my previous comment regarding
> > the vpmu: we are trying to make things more configurable and flexible
> > and that's good, but we don't necessary need to make all possible
> > combination work. VT-D without VMX is another one of those combination
> > that I would only enable after a customer asks.
> 
> Well. Imo again the configuration should be permitted.

FYI Andrew said the same thing as you on Matrix, so I withdraw my
suggestion.