[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1] x86: make Viridian support optional


  • To: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
  • From: Sergiy Kibrik <sergiy_kibrik@xxxxxxxx>
  • Date: Thu, 20 Mar 2025 11:39:54 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=KsMPvtpM3ETFU/HIoo6pamiLwx+41JgJET6xVqOUrVE=; b=y6sE5LX7DuOmRn+can99rAf6Slq0lJL8HreH8jizArwVm20/Bt8vu3t+0fOekgi3sJFzOL7QutSBMjiPTAi1M0cnoSGR8qKjEtA+Kg6fzg/3Xzo/0Dpzv8rsbkVIJ33ypHJh25tMle9xFpAyp9kV/yL9NxWKDWJ/VwFZidpW6Yb9SYd10OZDwRxm/nEETK/vCqKycjePkusEHA/qhDvqDuX3fvYSRZiDrHdzB4TaLWlakV/cmdjJVkfxJoT+tRonJGwRGjCeEYpZXbvHgB55Gxiu67Kw7j5d9JicyZJF2h0VYo3Nj0fnKd73LGcyULrqxMFqWH/IIF/pRHtOFW472Q==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=f3dWIeI9bU0wqDQFHQm7q9gG5XYFO3UCRuYvqSMXfY2/CM5Zu15SxOLcE88umFJ3CXtxwjKT+8NBEKJtHKkqkGQn8KkqDRYKJzepu7zHoVVCKfvJlOq3HEX/HMWdD3KsB4VPCDyMMnqJlhPPp/F/Khl52ND/SV61LmCOAV7Gwhmkb1NAQ76m79n5K3oKTVq9+jV7xxCbXXfFDC3uwTVfMSBFOuMWRtKN6dMCZ1w0JSKdYQkXBkgDog5DUiAYzbj3qntmIWbEwQiKHHF9NMHrvcUTGiOnQ7fRxjIg6Z3HFqlGNVofwA36+sebdC5vD7M1s03M4+PYb51nWmEz8OTEKw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=epam.com;
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • Delivery-date: Thu, 20 Mar 2025 09:40:14 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

hi Alejandro,

17.03.25 11:19, Alejandro Vallejo:
[..]
      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.

The option is intended primarily for fine-tuned systems optimized for particular platform and mode of operation. As for generic systems (e.g. distributions) whey wouldn't want to disable it anyway.

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"?).


Agree, "Hyper-V enlighnments for guests" is better title.
As for help message, would the one below be sufficient?:

 help
   Support optimizations for Hyper-V guests such as faster hypercalls,
   efficient timer and interrupt handling, and enhanced paravirtualized
   I/O. This is to improve performance and compatibility of Windows VMs.

   If unsure, say Y.


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.


this is rather to highlight the fact that the code is part of HVM support, a bit of self-documenting

[..]
    diff --git a/xen/arch/x86/include/asm/hvm/vcpu.h
    b/xen/arch/x86/include/asm/hvm/vcpu.h
    index 196fed6d5d..bac35ec47a 100644
    --- a/xen/arch/x86/include/asm/hvm/vcpu.h
    +++ b/xen/arch/x86/include/asm/hvm/vcpu.h
    @@ -171,8 +171,9 @@ struct hvm_vcpu {

          /* Pending hw/sw interrupt (.vector = -1 means nothing
    pending). */
          struct x86_event     inject_event;
    -
    +#ifdef CONFIG_HVM_VIRIDIAN
          struct viridian_vcpu *viridian;
    +#endif
      };

      #endif /* __ASM_X86_HVM_VCPU_H__ */


nit: I suspect the code would be far less cluttered with "if viridian..." if the
init/deinit/etc functions had dummy versions of those functions when
!HVM_VIRIDIAN in the header.


as Jan explained some time ago [1] it's preferable to compile as much as possible in all build configuration. Besides most of calls to viridian code are already guarded by is_viridian_domain() & not actually require stubs.

 -Sergiy

[1] https://lore.kernel.org/xen-devel/36708a3f-2664-4b04-9f5d-f115d362d100@xxxxxxxx/



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.