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

Re: [XEN][PATCH 2/8] xen/arm: move vcpu_switch_to_aarch64_mode() in arch_vcpu_create()


  • To: Julien Grall <julien@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
  • Date: Thu, 24 Jul 2025 16:54:24 +0300
  • 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=hR+YD69o2p6Gx+zoy1dZDtoyc9SchK1dKTpVtGAeE70=; b=uQN9a3+smBuBFtZXYGalMGoFPvNuzeNq8BZOgplx/+QI6VrTXddmf+4IGYiefNIrmz5r5MT15WyvHHp0CTvOaIp7kmIV5tE+yzkIFoZWbGUz2AIy3k+i3hbWhggAiEAZ0/LLXDCkdyc66wb8rmC/KfZN2LIW2A5isYh2VV5vhoGndNx5bw/h8UINHQSJLl5fzRsCBqw4u8XhVbAEeEb08ByAck5DHsdaw5ccVa+eFbyfu64QnVJhrD5ztQ/JhhRrcXSUaN/UNg7XNxnq+G9JZK94mzo10s2mB6XALrkMWXzVBijvdeU4nb/Ays6lNIRf1RlxEYFjTZgbFrV0koApKw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=Y1e6y+zsSHVqFBVsDGbk88sSIzf+wZFNdGpTfyzbK5MQv3/rTTMiWnehPB/+P7QZXSKCzqP22eOisIaZHIxWKQhRVtcSsxQ+TFXwvEyHIlI7BGXSC5LrfkKUo0bgXRUFTV0Tk4Imu0qP6eOGrAVsMNFw8uDfclIBS6qwUniosnI1gzo/P7+YL8fw7znsH2uHQBTt8pFY4fa4nBzdeseP3RhbxbrD0ieqbDxzP0SqI/dBRUwnrutPxoDm+11qfmZft5wGZVWxQ5MqF7zXejgYczeSQmGaaf6L7C6/Xqq3BelkfDTkA9jBAOw6ZC7vcP3cqIBdPTmIrCi79X/WUOyqqQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=epam.com;
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>
  • Delivery-date: Thu, 24 Jul 2025 13:54:37 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Julien, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks for your comment.

On 23.07.25 14:09, Julien Grall wrote:
Hi,

On 23/07/2025 11:19, Grygorii Strashko wrote:
On 23.07.25 12:16, Julien Grall wrote:
On 23/07/2025 08:58, Grygorii Strashko wrote:
From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>

Move vcpu_switch_to_aarch64_mode() in arch_vcpu_create() callback instead
of calling it manually from few different places after vcpu_create().

Before doing above ensure vcpu0 is created after kernel_probe() is done and
domain's guest execution mode (32-bit/64-bit) is set for dom0 and dom0less
domains.

The commit message doesn't mention anything about domains created by the 
toolstack. In this case, from my understanding, the switch to 64- bit domain 
happens *after* the vCPUs are created.

At the moment, I think this is probably ok to call...


Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
---
  xen/arch/arm/domain.c                    |  3 +++
  xen/arch/arm/domain_build.c              | 13 +++++--------
  xen/common/device-tree/dom0less-build.c  |  6 +++---
  xen/include/asm-generic/dom0less-build.h |  2 +-
  4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 79a144e61be9..bbd4a764c696 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -586,6 +586,9 @@ int arch_vcpu_create(struct vcpu *v)
      if ( get_ssbd_state() == ARM_SSBD_RUNTIME )
          v->arch.cpu_info->flags |= CPUINFO_WORKAROUND_2_FLAG;
+    if ( is_64bit_domain(v->domain) )
+        vcpu_switch_to_aarch64_mode(v);

... this function here because I *think* it would be NOP. But this feels really 
fragile.

The toolstack configures domain and vcpus through XEN_DOMCTL_set_address_size 
on Arm64:
- toolstack creates domain and parses kernel binary: domain created with 
DOMAIN_32BIT mode by default
- toolstack creates vcpus (still 32 bit mode): libxl__build_pre()- 
 >xc_domain_max_vcpus()
- toolstack switches domain mode depending on kernel binary type: 
libxl__build_dom()->xc_dom_boot_mem_init(),
   which triggers XEN_DOMCTL_set_address_size hypercall.
   Xen: arm64: switches domain mode and re-configures vcpus: 
subarch_do_domctl()->set_address_size()

So, this patch does not affect toolstack path, only optimizes Xen boots a bit.

Thanks for providing more detaisl. I am not sure what you mean by optimize. It 
reduces the number of places where vcpu_switch_to_aarch64_mode() is called, but 
there should be no difference in term of boot time.


Also, during Xen boot or by toolstack - the domain is always created before 
it's type is even known, which, in turn,
is based on guest binary which is parsed later during domain configuration 
stage.

What you are describing is the current situation. But this doesn't tell me 
*why* we can't provide the type when the domain is created.


I can add note in commit message "This patch doesn't affect on the toolstack 
Arm64 domain creation path as toolstack always
re-configures domain mode and vcpus through XEN_DOMCTL_set_address_size hypercall 
during domain configuration stage"

Well, as I wrote before, I find this code extremely fragile. And you so far, 
you don't seem to have address this concern in your reply. In fact...



If the desire is to make 32-bit domain optional on Arm64. Then I think it would 
be better to pass the domain type when the domain
is created (IOW add an extra flags to XEN_DOMCTL_createdomain). This will 
require more work, but it will be a lot more robust.

... I proposed what I think is a better alternative. Did you consider it?


Yes, sorry for delay. I've been considering it and tried to study and estimate 
it, at least preliminary.
 ...It's definitely more work.

I'd be appreciated if you could clarify some points, please:

- Am understood correctly that proposal is to add "XEN_DOMCTL_CDF_is_32bit" flag 
which will be passed in struct xen_domctl_createdomain->flags?
  (or "XEN_DOMCTL_CDF_is_32bit_mode")

  [Arm] The Arm64 specific enum domain_type and (d)->arch.type can be dropped 
(use XEN_DOMCTL_CDF_is_32bit instead)
  [x86] the d->arch.pv.is_32bit can be dropped (use XEN_DOMCTL_CDF_is_32bit 
instead)

- is corresponding XEN_DOMINF_is_32bit needed?

- Assumption XEN_DOMCTL_set_address_size will become obsolete finally. Right?

After studying the topic, I have below thought regarding the requested change.
(I might be missing/misunderstanding smth, so will be appreciated for any 
advice or guidance to the right direction.
 Also I'm not very familiar with x86, sorry.)

The goal:
- domain mode (32bit/64bit) should be determined by probing guest binary before 
creating domain;
- domain mode (32bit/64bit) should be set during domain creation 
(XEN_DOMCTL_createdomain) using new flag XEN_DOMCTL_CDF_is_32bit.

Possible steps:
1) Introduce XEN_DOMCTL_CDF_is_32bit flag;
2) Arm: re-work regular dom0 creation code;
   challenges: cross references kernel_info vs domain.
3) Arm: re-work dom0les boot mode;
4) x86_pv: re-work regular PV dom0 creation code;
5) x86_hvm: ???;
6) toolstack: re-work domain creation, so guest binary probed and domain mode 
determined before creating domain;
   challenges: running "bootloader" to get guest binaries seems the most 
difficult part.
7) de-scope XEN_DOMCTL_set_address_size;

The toolstack behavior is left unchanged until step 6.

Hence the amount of work is significant It's preferred to be done in 
independent phases (series) to
easy review and testing process.

I'd try to come up with patches for items 1-3 as the first phase.

Thank you.

--
Best regards,
-grygorii



 


Rackspace

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