[XEN][PATCH 3/8] xen/arm: move vcpu_switch_to_aarch64_mode() in arm64

Grygorii Strashko posted 8 patches 3 months, 1 week ago
[XEN][PATCH 3/8] xen/arm: move vcpu_switch_to_aarch64_mode() in arm64
Posted by Grygorii Strashko 3 months, 1 week ago
From: Grygorii Strashko <grygorii_strashko@epam.com>

The vcpu_switch_to_aarch64_mode() is Arm64 specific, so move it in Arm64.
As part of this change:
- introduce arm32/arm64 domain.h headers and include them in asm/domain.h
basing on CONFIG_ARM_xx;
- declare vcpu_switch_to_aarch64_mode() for arm64;
- add vcpu_switch_to_aarch64_mode() as empty macro for arm32.

Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
---
 xen/arch/arm/arm64/domain.c             |  5 +++++
 xen/arch/arm/domain.c                   |  5 -----
 xen/arch/arm/include/asm/arm32/domain.h | 17 +++++++++++++++++
 xen/arch/arm/include/asm/arm64/domain.h | 22 ++++++++++++++++++++++
 xen/arch/arm/include/asm/domain.h       |  3 ++-
 5 files changed, 46 insertions(+), 6 deletions(-)
 create mode 100644 xen/arch/arm/include/asm/arm32/domain.h
 create mode 100644 xen/arch/arm/include/asm/arm64/domain.h

diff --git a/xen/arch/arm/arm64/domain.c b/xen/arch/arm/arm64/domain.c
index dd1909892995..1e78986b5a7b 100644
--- a/xen/arch/arm/arm64/domain.c
+++ b/xen/arch/arm/arm64/domain.c
@@ -55,6 +55,11 @@ void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
 #undef C
 }
 
+void vcpu_switch_to_aarch64_mode(struct vcpu *v)
+{
+    v->arch.hcr_el2 |= HCR_RW;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index bbd4a764c696..e785278cdbd7 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -605,11 +605,6 @@ void arch_vcpu_destroy(struct vcpu *v)
     free_xenheap_pages(v->arch.stack, STACK_ORDER);
 }
 
-void vcpu_switch_to_aarch64_mode(struct vcpu *v)
-{
-    v->arch.hcr_el2 |= HCR_RW;
-}
-
 int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
 {
     unsigned int max_vcpus;
diff --git a/xen/arch/arm/include/asm/arm32/domain.h b/xen/arch/arm/include/asm/arm32/domain.h
new file mode 100644
index 000000000000..4d1251e9c128
--- /dev/null
+++ b/xen/arch/arm/include/asm/arm32/domain.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef ARM_ARM32_DOMAIN_H
+#define ARM_ARM32_DOMAIN_H
+
+#define vcpu_switch_to_aarch64_mode(v)
+
+#endif /* ARM_ARM32_DOMAIN_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/include/asm/arm64/domain.h b/xen/arch/arm/include/asm/arm64/domain.h
new file mode 100644
index 000000000000..b5f1177d2508
--- /dev/null
+++ b/xen/arch/arm/include/asm/arm64/domain.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef ARM_ARM64_DOMAIN_H
+#define ARM_ARM64_DOMAIN_H
+
+/*
+ * Set guest execution state to AArch64 (EL1) for selected vcpu
+ *
+ * @vcpu: pointer to the vcpu structure
+ */
+void vcpu_switch_to_aarch64_mode(struct vcpu *v);
+
+#endif /* ARM_ARM64_DOMAIN_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index a3487ca71303..fa258eb8d359 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -24,9 +24,11 @@ enum domain_type {
 };
 #define is_32bit_domain(d) ((d)->arch.type == DOMAIN_32BIT)
 #define is_64bit_domain(d) ((d)->arch.type == DOMAIN_64BIT)
+#include <asm/arm64/domain.h>
 #else
 #define is_32bit_domain(d) (1)
 #define is_64bit_domain(d) (0)
+#include <asm/arm64/domain.h>
 #endif
 
 /*
@@ -246,7 +248,6 @@ struct arch_vcpu
 }  __cacheline_aligned;
 
 void vcpu_show_registers(struct vcpu *v);
-void vcpu_switch_to_aarch64_mode(struct vcpu *v);
 
 /*
  * Due to the restriction of GICv3, the number of vCPUs in AFF0 is
-- 
2.34.1
Re: [XEN][PATCH 3/8] xen/arm: move vcpu_switch_to_aarch64_mode() in arm64
Posted by Julien Grall 3 months, 1 week ago
Hi,

On 23/07/2025 08:58, Grygorii Strashko wrote:
> From: Grygorii Strashko <grygorii_strashko@epam.com>
> 
> The vcpu_switch_to_aarch64_mode() is Arm64 specific, so move it in Arm64.
> As part of this change:
> - introduce arm32/arm64 domain.h headers and include them in asm/domain.h
> basing on CONFIG_ARM_xx;
> - declare vcpu_switch_to_aarch64_mode() for arm64;
> - add vcpu_switch_to_aarch64_mode() as empty macro for arm32.
> 
> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
> ---
>   xen/arch/arm/arm64/domain.c             |  5 +++++
>   xen/arch/arm/domain.c                   |  5 -----
>   xen/arch/arm/include/asm/arm32/domain.h | 17 +++++++++++++++++
>   xen/arch/arm/include/asm/arm64/domain.h | 22 ++++++++++++++++++++++
>   xen/arch/arm/include/asm/domain.h       |  3 ++-
>   5 files changed, 46 insertions(+), 6 deletions(-)
>   create mode 100644 xen/arch/arm/include/asm/arm32/domain.h
>   create mode 100644 xen/arch/arm/include/asm/arm64/domain.h
> 
> diff --git a/xen/arch/arm/arm64/domain.c b/xen/arch/arm/arm64/domain.c
> index dd1909892995..1e78986b5a7b 100644
> --- a/xen/arch/arm/arm64/domain.c
> +++ b/xen/arch/arm/arm64/domain.c
> @@ -55,6 +55,11 @@ void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
>   #undef C
>   }
>   
> +void vcpu_switch_to_aarch64_mode(struct vcpu *v)
> +{
> +    v->arch.hcr_el2 |= HCR_RW;
> +}

Strictly speaking arm/domain.c is GPLv2-or-later. But arm64/domain.c 
doesn't have a license. So it would default to GPLv2-only. There have 
been argument in the past on whether we would re-license code from 
GPLv2-or-later to GPLv2-only. But this was never concluded. So I am not 
entirely sure what to do with this change...

Bertrand, Michal,  Stefano?

> +
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index bbd4a764c696..e785278cdbd7 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -605,11 +605,6 @@ void arch_vcpu_destroy(struct vcpu *v)
>       free_xenheap_pages(v->arch.stack, STACK_ORDER);
>   }
>   
> -void vcpu_switch_to_aarch64_mode(struct vcpu *v)
> -{
> -    v->arch.hcr_el2 |= HCR_RW;
> -}
> -
>   int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>   {
>       unsigned int max_vcpus;
> diff --git a/xen/arch/arm/include/asm/arm32/domain.h b/xen/arch/arm/include/asm/arm32/domain.h
> new file mode 100644
> index 000000000000..4d1251e9c128
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/arm32/domain.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef ARM_ARM32_DOMAIN_H
> +#define ARM_ARM32_DOMAIN_H
> +
> +#define vcpu_switch_to_aarch64_mode(v)

I think you want to "consume" v. IOW (void)(v).  That said, we tend to 
prefer using a static inline whenever it is possible. Have you tried it?

> +
> +#endif /* ARM_ARM32_DOMAIN_H */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/include/asm/arm64/domain.h b/xen/arch/arm/include/asm/arm64/domain.h
> new file mode 100644
> index 000000000000..b5f1177d2508
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/arm64/domain.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef ARM_ARM64_DOMAIN_H
> +#define ARM_ARM64_DOMAIN_H
> +
> +/*
> + * Set guest execution state to AArch64 (EL1) for selected vcpu
> + *
> + * @vcpu: pointer to the vcpu structure
> + */
> +void vcpu_switch_to_aarch64_mode(struct vcpu *v);
> +
> +#endif /* ARM_ARM64_DOMAIN_H */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
> index a3487ca71303..fa258eb8d359 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -24,9 +24,11 @@ enum domain_type {
>   };
>   #define is_32bit_domain(d) ((d)->arch.type == DOMAIN_32BIT)
>   #define is_64bit_domain(d) ((d)->arch.type == DOMAIN_64BIT)
> +#include <asm/arm64/domain.h>
>   #else
>   #define is_32bit_domain(d) (1)
>   #define is_64bit_domain(d) (0)
> +#include <asm/arm64/domain.h>
>   #endif
>   
>   /*
> @@ -246,7 +248,6 @@ struct arch_vcpu
>   }  __cacheline_aligned;
>   
>   void vcpu_show_registers(struct vcpu *v);
> -void vcpu_switch_to_aarch64_mode(struct vcpu *v);
>   
>   /*
>    * Due to the restriction of GICv3, the number of vCPUs in AFF0 is

Cheers,

-- 
Julien Grall
Re: [XEN][PATCH 3/8] xen/arm: move vcpu_switch_to_aarch64_mode() in arm64
Posted by Grygorii Strashko 3 months, 1 week ago

On 23.07.25 12:22, Julien Grall wrote:
> Hi,
> 
> On 23/07/2025 08:58, Grygorii Strashko wrote:
>> From: Grygorii Strashko <grygorii_strashko@epam.com>
>>
>> The vcpu_switch_to_aarch64_mode() is Arm64 specific, so move it in Arm64.
>> As part of this change:
>> - introduce arm32/arm64 domain.h headers and include them in asm/domain.h
>> basing on CONFIG_ARM_xx;
>> - declare vcpu_switch_to_aarch64_mode() for arm64;
>> - add vcpu_switch_to_aarch64_mode() as empty macro for arm32.
>>
>> Signed-off-by: Grygorii Strashko <grygorii_strashko@epam.com>
>> ---
>>   xen/arch/arm/arm64/domain.c             |  5 +++++
>>   xen/arch/arm/domain.c                   |  5 -----
>>   xen/arch/arm/include/asm/arm32/domain.h | 17 +++++++++++++++++
>>   xen/arch/arm/include/asm/arm64/domain.h | 22 ++++++++++++++++++++++
>>   xen/arch/arm/include/asm/domain.h       |  3 ++-
>>   5 files changed, 46 insertions(+), 6 deletions(-)
>>   create mode 100644 xen/arch/arm/include/asm/arm32/domain.h
>>   create mode 100644 xen/arch/arm/include/asm/arm64/domain.h
>>
>> diff --git a/xen/arch/arm/arm64/domain.c b/xen/arch/arm/arm64/domain.c
>> index dd1909892995..1e78986b5a7b 100644
>> --- a/xen/arch/arm/arm64/domain.c
>> +++ b/xen/arch/arm/arm64/domain.c
>> @@ -55,6 +55,11 @@ void vcpu_regs_user_to_hyp(struct vcpu *vcpu,
>>   #undef C
>>   }
>> +void vcpu_switch_to_aarch64_mode(struct vcpu *v)
>> +{
>> +    v->arch.hcr_el2 |= HCR_RW;
>> +}
> 
> Strictly speaking arm/domain.c is GPLv2-or-later. But arm64/domain.c doesn't have a license.
  So it would default to GPLv2-only. There have been argument in the past on whether we would
  re-license code from GPLv2-or-later to GPLv2-only. But this was never concluded. So I am not entirely sure what to do with this change...
  
> Bertrand, Michal,  Stefano?
> 
>> +
>>   /*
>>    * Local variables:
>>    * mode: C
>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>> index bbd4a764c696..e785278cdbd7 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -605,11 +605,6 @@ void arch_vcpu_destroy(struct vcpu *v)
>>       free_xenheap_pages(v->arch.stack, STACK_ORDER);
>>   }
>> -void vcpu_switch_to_aarch64_mode(struct vcpu *v)
>> -{
>> -    v->arch.hcr_el2 |= HCR_RW;
>> -}
>> -
>>   int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>>   {
>>       unsigned int max_vcpus;
>> diff --git a/xen/arch/arm/include/asm/arm32/domain.h b/xen/arch/arm/include/asm/arm32/domain.h
>> new file mode 100644
>> index 000000000000..4d1251e9c128
>> --- /dev/null
>> +++ b/xen/arch/arm/include/asm/arm32/domain.h
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#ifndef ARM_ARM32_DOMAIN_H
>> +#define ARM_ARM32_DOMAIN_H
>> +
>> +#define vcpu_switch_to_aarch64_mode(v)
> 
> I think you want to "consume" v. IOW (void)(v).  That said, we tend to prefer using a static inline whenever it is possible. Have you tried it?

will do static inline.

Thank you.

-- 
Best regards,
-grygorii