+xen-devel (sent it on private by mistake)
On Mon, 17 Mar 2025, 09:29 Jan Beulich, <jbeul...@suse.com> wrote:
> On 17.03.2025 10:19, Alejandro Vallejo wrote:
> > Hi,
> >
> > I'm surprised this isn't already possible. Neat!
> >
> > On Mon, 17 Mar 2025, 07:19 Sergiy Kibrik, <sergiy_kib...@epam.com>
> wrote:
> >
> >> Add config option HVM_VIRIDIAN that covers viridian code within HVM.
> >> Calls to viridian functions guarded by is_viridian_domain() and related
> >> macros.
> >> Having this option may be beneficial by reducing code footprint for
> systems
> >> that are not using Hyper-V.
> >>
> >> Signed-off-by: Sergiy Kibrik <sergiy_kib...@epam.com>
> >> ---
> >> xen/arch/x86/Kconfig | 5 +++++
> >> xen/arch/x86/hvm/Makefile | 2 +-
> >> xen/arch/x86/hvm/hvm.c | 27 ++++++++++++++++++---------
> >> xen/arch/x86/hvm/vlapic.c | 11 +++++++----
> >> xen/arch/x86/include/asm/hvm/domain.h | 4 ++--
> >> xen/arch/x86/include/asm/hvm/hvm.h | 3 ++-
> >> xen/arch/x86/include/asm/hvm/vcpu.h | 3 ++-
> >> 7 files changed, 37 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> >> index 6e41bc0fb4..34f9b79d98 100644
> >> --- a/xen/arch/x86/Kconfig
> >> +++ b/xen/arch/x86/Kconfig
> >> @@ -348,6 +348,11 @@ config HYPERV_GUEST
> >>
> >> endif
> >>
> >> +config HVM_VIRIDIAN
> >> + bool "Viridian enlightenments support" if EXPERT
> >> + depends on HVM
> >> + default y
> >> +
> >>
> >
> >
> > I don't see why this should be gated by EXPERT, provided a
> > suitable (now absent) help message was to exist explaining
> > what it does in plain simple words.
> >
> > For the title, I'd say it needs to properly state it refers to
> > enlightenments for guests, rather than enlightenments for
> > Xen itself when running under Hyper-V. As it is, it sounds
> > ambiguous (Maybe "Hyper-V enlighnments for guests"?).
>
> I'm slightly puzzled: Here you're worried about ambiguity, yet then ...
>
> > On a personal nitpicky preference note, I'd say HVM_VIRIDIAN sounds
> > rather redundant, and I think just VIRIDIAN works just as well
> > while being shorter.
>
> ... you suggest to introduce ambiguity here. I'd expect VIRIDIAN alone
> to cover whatever enlightenments Xen might want to use itself, when
> run on top of Viridian.
>
> Jan
>
For the define name, I did say it it was a matter of preference rather
than a worry. I'm perfectly happy with HVM_VIRIDIAN :)
CONFIG_{HVM_}VIRIDIAN would only be seen in code, where we use that
name exclusively for enlightenments given to a guest, AFAIK, so I don't
see as much ambiguity. Doesn't matter much either way.
However,
Non-Xen-developers rely on nconfig/menuconfig titles and descriptions
alone to make decisions, so not providing adequate background (even
if under EXPERT) seems less than great.
Cheers,
Alejandro
