Re: [Xen-devel] [PATCH 34/34] RFC x86: introduce directio virt cap
>>> On 21.08.18 at 18:02, wrote: > On Tue, Aug 21, 2018 at 09:40:07AM -0600, Jan Beulich wrote: >> >>> On 21.08.18 at 15:43, wrote: >> > On Tue, Aug 21, 2018 at 05:52:39AM -0600, Jan Beulich wrote: >> >> >>> On 17.08.18 at 17:12, wrote: >> >> > --- a/xen/arch/x86/sysctl.c >> >> > +++ b/xen/arch/x86/sysctl.c >> >> > @@ -92,8 +92,10 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi) >> >> > min(sizeof(pi->hw_cap), >> >> > sizeof(boot_cpu_data.x86_capability))); >> >> > if ( hvm_enabled ) >> >> > pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm; >> >> > -if ( iommu_enabled ) >> >> > +if ( hvm_enabled && iommu_enabled ) >> >> > pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm_directio; >> >> > +else if ( iommu_enabled ) >> >> > +pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio; >> >> > } >> >> >> >> At the sysctl layer I think you can, as suggested iirc by Roger, >> >> simply replace hvm_directio with directio (or iommu). For the >> >> "xl info" output, otoh, I'm afraid this doesn't hold, as people >> >> may parse for the string. Depending on how this would best >> >> be addressed in the tool stack, replacing the sysctl names may >> >> then no longer be the most suitable solution. >> > >> > In that case what do you think about the two flags this patch provides >> > on the toolstack level? >> > >> > Essentially we change slightly hvm_directio's meaning to mean "hvm && >> > iommu_enabled" while it previously mean "iommu_enabled", and then in the >> > absence of hvm_directio, add "directio" as an indication for >> > "iommu_enabled". >> >> I think when you mean to provide two flags, retaining the meaning >> of the pre-existing one might be desirable. But as said before - >> much depends on what the tool stack means to present to the user. >> The expectations on the current meaning on "xl info" output should >> not be broken. > > Previously "hvm" and "hvm_directio" always show up together. The > hvm_directio only case never showed up in practice -- I don't think > there had been systems with vt-d but not vt-x. > > My plan for xl info: > > pvhvm iommu flags in xl info > 0 0 0 n/a > 0 0 1 n/a > 0 1 0 hvm > 0 1 1 hvm hvm_directio > 1 0 0 NIL > 1 0 1 directio > 1 1 0 hvm > 1 1 1 hvm hvm_directio directio > > This retains sensible (though not completely identical) semantics for > hvm_directio. I think that ought to be fine. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 34/34] RFC x86: introduce directio virt cap
On Tue, Aug 21, 2018 at 09:40:07AM -0600, Jan Beulich wrote: > >>> On 21.08.18 at 15:43, wrote: > > On Tue, Aug 21, 2018 at 05:52:39AM -0600, Jan Beulich wrote: > >> >>> On 17.08.18 at 17:12, wrote: > >> > --- a/xen/arch/x86/sysctl.c > >> > +++ b/xen/arch/x86/sysctl.c > >> > @@ -92,8 +92,10 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi) > >> > min(sizeof(pi->hw_cap), > >> > sizeof(boot_cpu_data.x86_capability))); > >> > if ( hvm_enabled ) > >> > pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm; > >> > -if ( iommu_enabled ) > >> > +if ( hvm_enabled && iommu_enabled ) > >> > pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm_directio; > >> > +else if ( iommu_enabled ) > >> > +pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio; > >> > } > >> > >> At the sysctl layer I think you can, as suggested iirc by Roger, > >> simply replace hvm_directio with directio (or iommu). For the > >> "xl info" output, otoh, I'm afraid this doesn't hold, as people > >> may parse for the string. Depending on how this would best > >> be addressed in the tool stack, replacing the sysctl names may > >> then no longer be the most suitable solution. > > > > In that case what do you think about the two flags this patch provides > > on the toolstack level? > > > > Essentially we change slightly hvm_directio's meaning to mean "hvm && > > iommu_enabled" while it previously mean "iommu_enabled", and then in the > > absence of hvm_directio, add "directio" as an indication for > > "iommu_enabled". > > I think when you mean to provide two flags, retaining the meaning > of the pre-existing one might be desirable. But as said before - > much depends on what the tool stack means to present to the user. > The expectations on the current meaning on "xl info" output should > not be broken. Previously "hvm" and "hvm_directio" always show up together. The hvm_directio only case never showed up in practice -- I don't think there had been systems with vt-d but not vt-x. My plan for xl info: pv hvm iommu flags in xl info 0 0 0 n/a 0 0 1 n/a 0 1 0 hvm 0 1 1 hvm hvm_directio 1 0 0 NIL 1 0 1 directio 1 1 0 hvm 1 1 1 hvm hvm_directio directio This retains sensible (though not completely identical) semantics for hvm_directio. The term "directio" can also be "iommu" if that's preferable. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 34/34] RFC x86: introduce directio virt cap
>>> On 21.08.18 at 15:43, wrote: > On Tue, Aug 21, 2018 at 05:52:39AM -0600, Jan Beulich wrote: >> >>> On 17.08.18 at 17:12, wrote: >> > --- a/xen/arch/x86/sysctl.c >> > +++ b/xen/arch/x86/sysctl.c >> > @@ -92,8 +92,10 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi) >> > min(sizeof(pi->hw_cap), sizeof(boot_cpu_data.x86_capability))); >> > if ( hvm_enabled ) >> > pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm; >> > -if ( iommu_enabled ) >> > +if ( hvm_enabled && iommu_enabled ) >> > pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm_directio; >> > +else if ( iommu_enabled ) >> > +pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio; >> > } >> >> At the sysctl layer I think you can, as suggested iirc by Roger, >> simply replace hvm_directio with directio (or iommu). For the >> "xl info" output, otoh, I'm afraid this doesn't hold, as people >> may parse for the string. Depending on how this would best >> be addressed in the tool stack, replacing the sysctl names may >> then no longer be the most suitable solution. > > In that case what do you think about the two flags this patch provides > on the toolstack level? > > Essentially we change slightly hvm_directio's meaning to mean "hvm && > iommu_enabled" while it previously mean "iommu_enabled", and then in the > absence of hvm_directio, add "directio" as an indication for > "iommu_enabled". I think when you mean to provide two flags, retaining the meaning of the pre-existing one might be desirable. But as said before - much depends on what the tool stack means to present to the user. The expectations on the current meaning on "xl info" output should not be broken. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 34/34] RFC x86: introduce directio virt cap
On Tue, Aug 21, 2018 at 05:52:39AM -0600, Jan Beulich wrote: > >>> On 17.08.18 at 17:12, wrote: > > --- a/xen/arch/x86/sysctl.c > > +++ b/xen/arch/x86/sysctl.c > > @@ -92,8 +92,10 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi) > > min(sizeof(pi->hw_cap), sizeof(boot_cpu_data.x86_capability))); > > if ( hvm_enabled ) > > pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm; > > -if ( iommu_enabled ) > > +if ( hvm_enabled && iommu_enabled ) > > pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm_directio; > > +else if ( iommu_enabled ) > > +pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio; > > } > > At the sysctl layer I think you can, as suggested iirc by Roger, > simply replace hvm_directio with directio (or iommu). For the > "xl info" output, otoh, I'm afraid this doesn't hold, as people > may parse for the string. Depending on how this would best > be addressed in the tool stack, replacing the sysctl names may > then no longer be the most suitable solution. In that case what do you think about the two flags this patch provides on the toolstack level? Essentially we change slightly hvm_directio's meaning to mean "hvm && iommu_enabled" while it previously mean "iommu_enabled", and then in the absence of hvm_directio, add "directio" as an indication for "iommu_enabled". Wei. > > Jan > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 34/34] RFC x86: introduce directio virt cap
>>> On 17.08.18 at 17:12, wrote: > --- a/xen/arch/x86/sysctl.c > +++ b/xen/arch/x86/sysctl.c > @@ -92,8 +92,10 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi) > min(sizeof(pi->hw_cap), sizeof(boot_cpu_data.x86_capability))); > if ( hvm_enabled ) > pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm; > -if ( iommu_enabled ) > +if ( hvm_enabled && iommu_enabled ) > pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm_directio; > +else if ( iommu_enabled ) > +pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio; > } At the sysctl layer I think you can, as suggested iirc by Roger, simply replace hvm_directio with directio (or iommu). For the "xl info" output, otoh, I'm afraid this doesn't hold, as people may parse for the string. Depending on how this would best be addressed in the tool stack, replacing the sysctl names may then no longer be the most suitable solution. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 34/34] RFC x86: introduce directio virt cap
On Tue, Aug 21, 2018 at 12:40:22PM +0200, Roger Pau Monné wrote: > On Tue, Aug 21, 2018 at 11:25:58AM +0100, Wei Liu wrote: > > On Tue, Aug 21, 2018 at 10:32:18AM +0200, Roger Pau Monné wrote: > > > On Fri, Aug 17, 2018 at 04:12:52PM +0100, Wei Liu wrote: > > > > hvm_directio is set when iommu is enabled, but in fact iommu is not > > > > tied to HVM. In order to not break existing tools, expose a new flag > > > > directio for (iommu_enabled && !hvm_enabled). > > > > > > > > RFC This doesn't build at the moment. Do we care about that flag being > > > > inaccurate? > > > > > > I think there is no hardware out there with an IOMMU that don't have > > > virtualization extensions (ie: having VTd but not VTx for example), > > > but maybe I'm wrong. > > > > The question is whether it makes sense to expose the name "hvm_directio" > > at all when you can't run an HVM guest in the first place. > > > > Also iommu isn't an HVM only feature, PV guests can also make use of it > > if I understand correctly, hence the suggestion of "directio". > > Right, I see your point. > > Could we remove this hvm_directio artifact and just expose an iommu > capability? > > Since this is a sysctl I think we can change the interface without > issues, so I would just > s/XEN_SYSCTL_PHYSCAP_hvm_directio/XEN_SYSCTL_PHYSCAP_iommu/. > I agree and am very tempted at the moment. But let's wait to see if there is objection. Wei. > Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 34/34] RFC x86: introduce directio virt cap
On Tue, Aug 21, 2018 at 11:25:58AM +0100, Wei Liu wrote: > On Tue, Aug 21, 2018 at 10:32:18AM +0200, Roger Pau Monné wrote: > > On Fri, Aug 17, 2018 at 04:12:52PM +0100, Wei Liu wrote: > > > hvm_directio is set when iommu is enabled, but in fact iommu is not > > > tied to HVM. In order to not break existing tools, expose a new flag > > > directio for (iommu_enabled && !hvm_enabled). > > > > > > RFC This doesn't build at the moment. Do we care about that flag being > > > inaccurate? > > > > I think there is no hardware out there with an IOMMU that don't have > > virtualization extensions (ie: having VTd but not VTx for example), > > but maybe I'm wrong. > > The question is whether it makes sense to expose the name "hvm_directio" > at all when you can't run an HVM guest in the first place. > > Also iommu isn't an HVM only feature, PV guests can also make use of it > if I understand correctly, hence the suggestion of "directio". Right, I see your point. Could we remove this hvm_directio artifact and just expose an iommu capability? Since this is a sysctl I think we can change the interface without issues, so I would just s/XEN_SYSCTL_PHYSCAP_hvm_directio/XEN_SYSCTL_PHYSCAP_iommu/. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 34/34] RFC x86: introduce directio virt cap
On Tue, Aug 21, 2018 at 10:32:18AM +0200, Roger Pau Monné wrote: > On Fri, Aug 17, 2018 at 04:12:52PM +0100, Wei Liu wrote: > > hvm_directio is set when iommu is enabled, but in fact iommu is not > > tied to HVM. In order to not break existing tools, expose a new flag > > directio for (iommu_enabled && !hvm_enabled). > > > > RFC This doesn't build at the moment. Do we care about that flag being > > inaccurate? > > I think there is no hardware out there with an IOMMU that don't have > virtualization extensions (ie: having VTd but not VTx for example), > but maybe I'm wrong. The question is whether it makes sense to expose the name "hvm_directio" at all when you can't run an HVM guest in the first place. Also iommu isn't an HVM only feature, PV guests can also make use of it if I understand correctly, hence the suggestion of "directio". Wei. > > Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 34/34] RFC x86: introduce directio virt cap
On Fri, Aug 17, 2018 at 04:12:52PM +0100, Wei Liu wrote: > hvm_directio is set when iommu is enabled, but in fact iommu is not > tied to HVM. In order to not break existing tools, expose a new flag > directio for (iommu_enabled && !hvm_enabled). > > RFC This doesn't build at the moment. Do we care about that flag being > inaccurate? I think there is no hardware out there with an IOMMU that don't have virtualization extensions (ie: having VTd but not VTx for example), but maybe I'm wrong. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 34/34] RFC x86: introduce directio virt cap
hvm_directio is set when iommu is enabled, but in fact iommu is not tied to HVM. In order to not break existing tools, expose a new flag directio for (iommu_enabled && !hvm_enabled). RFC This doesn't build at the moment. Do we care about that flag being inaccurate? Signed-off-by: Wei Liu --- xen/arch/x86/sysctl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c index e704ed7..a8d64c5 100644 --- a/xen/arch/x86/sysctl.c +++ b/xen/arch/x86/sysctl.c @@ -92,8 +92,10 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi) min(sizeof(pi->hw_cap), sizeof(boot_cpu_data.x86_capability))); if ( hvm_enabled ) pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm; -if ( iommu_enabled ) +if ( hvm_enabled && iommu_enabled ) pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm_directio; +else if ( iommu_enabled ) +pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio; } long arch_do_sysctl( -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
