From: Grygorii Strashko <grygorii_strashko@epam.com>
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.
Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
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);
+
return rc;
fail:
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d91a71acfd3b..af7e9d830ae1 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1885,10 +1885,6 @@ int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
printk("SVE is not available for 32-bit domain\n");
return -EINVAL;
}
-
- if ( is_64bit_domain(d) )
- vcpu_switch_to_aarch64_mode(v);
-
#endif
/*
@@ -1941,9 +1937,6 @@ int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
printk("Failed to allocate d%dv%d\n", d->domain_id, i);
break;
}
-
- if ( is_64bit_domain(d) )
- vcpu_switch_to_aarch64_mode(d->vcpu[i]);
}
domain_update_node_affinity(d);
@@ -1995,9 +1988,13 @@ int __init construct_hwdom(struct kernel_info *kinfo,
iommu_hwdom_init(d);
#ifdef CONFIG_ARM_64
- /* type must be set before allocate_memory */
+ /* type must be set before allocate_memory or create cpu */
d->arch.type = kinfo->arch.type;
#endif
+
+ if ( vcpu_create(d, 0) == NULL )
+ panic("Error creating domain 0 vcpu0\n");
+
find_gnttab_region(d, kinfo);
if ( is_domain_direct_mapped(d) )
allocate_memory_11(d, kinfo);
diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c
index efa846da2a55..f02ce6c776de 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -771,9 +771,6 @@ static int __init construct_domU(struct domain *d,
else if ( rc == 0 && !strcmp(dom0less_enhanced, "no-xenstore") )
kinfo.dom0less_feature = DOM0LESS_ENHANCED_NO_XS;
- if ( vcpu_create(d, 0) == NULL )
- return -ENOMEM;
-
d->max_pages = ((paddr_t)mem * SZ_1K) >> PAGE_SHIFT;
kinfo.bd.d = d;
@@ -792,6 +789,9 @@ static int __init construct_domU(struct domain *d,
}
else
{
+ if ( vcpu_create(d, 0) == NULL )
+ return -ENOMEM;
+
if ( !dt_find_property(node, "xen,static-mem", NULL) )
allocate_memory(d, &kinfo);
else if ( !is_domain_direct_mapped(d) )
diff --git a/xen/include/asm-generic/dom0less-build.h b/xen/include/asm-generic/dom0less-build.h
index 6b80ffbd8679..13616975b3ca 100644
--- a/xen/include/asm-generic/dom0less-build.h
+++ b/xen/include/asm-generic/dom0less-build.h
@@ -59,7 +59,7 @@ int make_arch_nodes(struct kernel_info *kinfo);
/*
* Set domain type from struct kernel_info which defines guest Execution
* State 32-bit/64-bit (for Arm AArch32/AArch64).
- * The domain type must be set before allocate_memory.
+ * The domain type must be set before allocate_memory or create vcpus.
*
* @d: pointer to the domain structure.
* @kinfo: pointer to the kinfo structure.
--
2.34.1Hi, On 23/07/2025 08:58, Grygorii Strashko wrote: > From: Grygorii Strashko <grygorii_strashko@epam.com> > > 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@epam.com> > --- > 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. 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. Cheers, -- Julien Grall
On 23.07.25 12:16, Julien Grall wrote: > Hi, > > On 23/07/2025 08:58, Grygorii Strashko wrote: >> From: Grygorii Strashko <grygorii_strashko@epam.com> >> >> 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@epam.com> >> --- >> 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. 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. 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" > > 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. -- Best regards, -grygorii
On 23/07/2025 11:19 am, Grygorii Strashko wrote: > > > On 23.07.25 12:16, Julien Grall wrote: >> Hi, >> >> On 23/07/2025 08:58, Grygorii Strashko wrote: >>> From: Grygorii Strashko <grygorii_strashko@epam.com> >>> >>> 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@epam.com> >>> --- >>> 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. > > 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. This is an error which has existed in Xen since the outset. ARM inherited it from x86 PV (albeit the opposite way around). It is literally backwards to create a VM in one mode, do some setup, then decide "no actually I want it in the other mode". For both x86 PV, and ARM it seems, parsing the kernel first and choosing the right mode(s) at create time would be a substantial improvement. As a note, x86 HVM has no concept of 64bit existing without 32bit. ~Andrew
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@epam.com> >>> >>> 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@epam.com> >>> --- >>> 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? Cheers, -- Julien Grall
Hi Julien, Andrew Cooper <andrew.cooper3@citrix.com>
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@epam.com>
>>>>
>>>> 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@epam.com>
>>>> ---
>>>> 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
© 2016 - 2025 Red Hat, Inc.