Re: [XEN PATCH v1 01/15] x86: introduce AMD-V and Intel VT-x Kconfig options

[Sergiy Kibrik <sergiy_kibrik@xxxxxxxx>]

18.04.24 14:16, Jan Beulich:
> On 16.04.2024 08:20, Sergiy Kibrik wrote:
> > From: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
> >
> > Introduce two new Kconfig options, SVM and VMX, to allow code
> > specific to each virtualization technology to be separated and, when not
> > required, stripped.
> > CONFIG_SVM will be used to enable virtual machine extensions on platforms that
> > implement the AMD Virtualization Technology (AMD-V).
> > CONFIG_VMX will be used to enable virtual machine extensions on platforms that
> > implement the Intel Virtualization Technology (Intel VT-x).
> >
> > Both features depend on HVM support.
> >
> > Since, at this point, disabling any of them would cause Xen to not compile,
> > the options are enabled by default if HVM and are not selectable by the user.
> >
> > No functional change intended.
> >
> > Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
> > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> In the subject tag you say v1, but I think this is a re-post? In which
> case and with it having been a long time since then it would be even
> more so important that a proper revision log is present in each individual
> patch.
well, this patch isn't plain resend, it does have changes since rfc 
version (actually all of them do, hence V1 tag). Anyway, individual 
change log per patch would make things easier a bit, I agree.
> > --- a/xen/arch/x86/Kconfig
> > +++ b/xen/arch/x86/Kconfig
> > @@ -120,6 +120,12 @@ config HVM
> >   
> >   	  If unsure, say Y.
> >   
> > +config SVM
> > +       def_bool y if HVM
> > +
> > +config VMX
> > +       def_bool y if HVM
> It was probably me to have requested this form, but meanwhile I've learned
> (on the Linux side) that newer kconfig is capable to dealing with
>
> config SVM
>         def_bool HVM
>
> config VMX
>         def_bool HVM
>
> quite fine (i.e. no longer leaving around useless "# CONFIG_... is not set"
> when (in this case) HVM=n. Please double check with the kconfig we use
> whether that's the case there, too, and simplify if so.
seems to work, thanks for pointing this out

> > --- a/xen/arch/x86/mm/Makefile
> > +++ b/xen/arch/x86/mm/Makefile
> > @@ -10,6 +10,7 @@ obj-$(CONFIG_MEM_SHARING) += mem_sharing.o
> >   obj-$(CONFIG_HVM) += nested.o
> >   obj-$(CONFIG_HVM) += p2m.o
> >   obj-y += p2m-basic.o
> > -obj-$(CONFIG_HVM) += p2m-ept.o p2m-pod.o p2m-pt.o
> > +obj-$(CONFIG_HVM) += p2m-pod.o p2m-pt.o
> > +obj-$(CONFIG_VMX) += p2m-ept.o
> Please can these be kept in alphabetical order?
yes, sure

  -Sergiy