| Message ID | 73072e5b2ec40ad28d4bcfb9bb0870f3838bb726.1715761386.git.Sergiy_Kibrik@epam.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: make cpu virtualization support configurable | expand |
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 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
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.
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
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); /*
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(-)