[PATCH v2 2/2] hv: refine the sworld memory allocate


chenli.wei
 

From: Chenli Wei <chenli.wei@...>

The current code uses a predefined sworld memory array to reserve memory
for trusty VMs, and assume all post launched VMs are trusty VM which is
not correct.

This patch statically reserved memory just for trusty VMs and save 16M
memory for every non trusty VM.

v1-->v2:
1. code format refine and function rename

Signed-off-by: Chenli Wei <chenli.wei@...>
---
hypervisor/arch/x86/guest/vm.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/hypervisor/arch/x86/guest/vm.c b/hypervisor/arch/x86/guest/vm.c
index bdd31d3fa..6fbc2c322 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -48,7 +48,7 @@
/* Local variables */

/* pre-assumption: TRUSTY_RAM_SIZE is 2M aligned */
-static struct page post_user_vm_sworld_memory[MAX_POST_VM_NUM][TRUSTY_RAM_SIZE >> PAGE_SHIFT] __aligned(MEM_2M);
+static struct page post_user_vm_sworld_memory[MAX_TRUSTY_VM_NUM][TRUSTY_RAM_SIZE >> PAGE_SHIFT] __aligned(MEM_2M);

static struct acrn_vm vm_array[CONFIG_MAX_VM_NUM] __aligned(PAGE_SIZE);

@@ -635,6 +635,26 @@ void prepare_vm_identical_memmap(struct acrn_vm *vm, uint16_t e820_entry_type, u
}
}

+int32_t get_sworld_vm_index(uint16_t vm_id)
+{
+ int16_t i;
+ int32_t page_idx = 0;
+ struct acrn_vm_config *vm_config;
+
+ for (i = 0; i < vm_id; i++) {
+ vm_config = get_vm_config(i);
+ if ((vm_config->guest_flags & GUEST_FLAG_SECURE_WORLD_ENABLED) != 0U) {
+ page_idx += 1;
+ }
+ }
+
+ if (page_idx >= (int32_t)MAX_TRUSTY_VM_NUM) {
+ pr_err("Can't find sworld memory for vm id: %d", vm_id);
+ page_idx = -1;
+ }
+ return page_idx;
+}
+
/**
* @pre vm_id < CONFIG_MAX_VM_NUM && vm_config != NULL && rtn_vm != NULL
* @pre vm->state == VM_POWERED_OFF
@@ -667,12 +687,14 @@ int32_t create_vm(uint16_t vm_id, uint64_t pcpu_bitmap, struct acrn_vm_config *v
vm->sworld_control.flag.supported = 1U;
}
if (vm->sworld_control.flag.supported != 0UL) {
- uint16_t service_vm_id = (get_service_vm())->vm_id;
- uint16_t page_idx = vmid_2_rel_vmid(service_vm_id, vm_id) - 1U;
+ int32_t page_idx = get_sworld_vm_index(vm_id);

- ept_add_mr(vm, (uint64_t *)vm->arch_vm.nworld_eptp,
- hva2hpa(post_user_vm_sworld_memory[page_idx]),
- TRUSTY_EPT_REBASE_GPA, TRUSTY_RAM_SIZE, EPT_WB | EPT_RWX);
+ if (page_idx >= 0)
+ {
+ ept_add_mr(vm, (uint64_t *)vm->arch_vm.nworld_eptp,
+ hva2hpa(post_user_vm_sworld_memory[page_idx]),
+ TRUSTY_EPT_REBASE_GPA, TRUSTY_RAM_SIZE, EPT_WB | EPT_RWX);
+ }
}
if (vm_config->name[0] == '\0') {
/* if VM name is not configured, specify with VM ID */
--
2.25.1


Eddie Dong
 

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of chenli.wei
Sent: Thursday, September 29, 2022 9:16 AM
To: Sun, Victor <victor.sun@...>; Mao, Junjie <junjie.mao@...>;
acrn-dev@...
Cc: Wei, Chenli <chenli.wei@...>
Subject: [acrn-dev] [PATCH v2 2/2] hv: refine the sworld memory allocate

From: Chenli Wei <chenli.wei@...>

The current code uses a predefined sworld memory array to reserve memory
for trusty VMs, and assume all post launched VMs are trusty VM which is not
correct.

This patch statically reserved memory just for trusty VMs and save 16M
memory for every non trusty VM.

v1-->v2:
1. code format refine and function rename

Signed-off-by: Chenli Wei <chenli.wei@...>
---
hypervisor/arch/x86/guest/vm.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/hypervisor/arch/x86/guest/vm.c
b/hypervisor/arch/x86/guest/vm.c index bdd31d3fa..6fbc2c322 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -48,7 +48,7 @@
/* Local variables */

/* pre-assumption: TRUSTY_RAM_SIZE is 2M aligned */ -static struct page
post_user_vm_sworld_memory[MAX_POST_VM_NUM][TRUSTY_RAM_SIZE >>
PAGE_SHIFT] __aligned(MEM_2M);
+static struct page
+post_user_vm_sworld_memory[MAX_TRUSTY_VM_NUM][TRUSTY_RAM_SIZ
E >>
+PAGE_SHIFT] __aligned(MEM_2M);

static struct acrn_vm vm_array[CONFIG_MAX_VM_NUM]
__aligned(PAGE_SIZE);

@@ -635,6 +635,26 @@ void prepare_vm_identical_memmap(struct
acrn_vm *vm, uint16_t e820_entry_type, u
}
}

+int32_t get_sworld_vm_index(uint16_t vm_id) {
+ int16_t i;
+ int32_t page_idx = 0;
Use vm_idx.

+ struct acrn_vm_config *vm_config;
+
+ for (i = 0; i < vm_id; i++) {
+ vm_config = get_vm_config(i);
+ if ((vm_config->guest_flags &
GUEST_FLAG_SECURE_WORLD_ENABLED) != 0U) {
+ page_idx += 1;
+ }
+ }
+
+ if (page_idx >= (int32_t)MAX_TRUSTY_VM_NUM) {
+ pr_err("Can't find sworld memory for vm id: %d", vm_id);
+ page_idx = -1;
+ }
+ return page_idx;
+}
+
/**
* @pre vm_id < CONFIG_MAX_VM_NUM && vm_config != NULL &&
rtn_vm != NULL
* @pre vm->state == VM_POWERED_OFF
@@ -667,12 +687,14 @@ int32_t create_vm(uint16_t vm_id, uint64_t
pcpu_bitmap, struct acrn_vm_config *v
vm->sworld_control.flag.supported = 1U;
}
if (vm->sworld_control.flag.supported != 0UL) {
- uint16_t service_vm_id = (get_service_vm())->vm_id;
- uint16_t page_idx = vmid_2_rel_vmid(service_vm_id,
vm_id) - 1U;
+ int32_t page_idx = get_sworld_vm_index(vm_id);

- ept_add_mr(vm, (uint64_t *)vm-
arch_vm.nworld_eptp,
-
hva2hpa(post_user_vm_sworld_memory[page_idx]),
- TRUSTY_EPT_REBASE_GPA,
TRUSTY_RAM_SIZE, EPT_WB | EPT_RWX);
+ if (page_idx >= 0)
+ {
+ ept_add_mr(vm, (uint64_t *)vm-
arch_vm.nworld_eptp,
+
hva2hpa(post_user_vm_sworld_memory[page_idx]),
+ TRUSTY_EPT_REBASE_GPA,
TRUSTY_RAM_SIZE, EPT_WB | EPT_RWX);
+ }
For "else" case, how it will be?
I thought create_vm should fail and stop to continue.

}
if (vm_config->name[0] == '\0') {
/* if VM name is not configured, specify with VM ID
*/
--
2.25.1





chenli.wei
 

On 9/30/2022 4:40 AM, Eddie Dong wrote:

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of chenli.wei
Sent: Thursday, September 29, 2022 9:16 AM
To: Sun, Victor <victor.sun@...>; Mao, Junjie <junjie.mao@...>;
acrn-dev@...
Cc: Wei, Chenli <chenli.wei@...>
Subject: [acrn-dev] [PATCH v2 2/2] hv: refine the sworld memory allocate

From: Chenli Wei <chenli.wei@...>

The current code uses a predefined sworld memory array to reserve memory
for trusty VMs, and assume all post launched VMs are trusty VM which is not
correct.

This patch statically reserved memory just for trusty VMs and save 16M
memory for every non trusty VM.

v1-->v2:
1. code format refine and function rename

Signed-off-by: Chenli Wei <chenli.wei@...>
---
hypervisor/arch/x86/guest/vm.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/hypervisor/arch/x86/guest/vm.c
b/hypervisor/arch/x86/guest/vm.c index bdd31d3fa..6fbc2c322 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -48,7 +48,7 @@
/* Local variables */

/* pre-assumption: TRUSTY_RAM_SIZE is 2M aligned */ -static struct page
post_user_vm_sworld_memory[MAX_POST_VM_NUM][TRUSTY_RAM_SIZE >>
PAGE_SHIFT] __aligned(MEM_2M);
+static struct page
+post_user_vm_sworld_memory[MAX_TRUSTY_VM_NUM][TRUSTY_RAM_SIZ
E >>
+PAGE_SHIFT] __aligned(MEM_2M);

static struct acrn_vm vm_array[CONFIG_MAX_VM_NUM]
__aligned(PAGE_SIZE);

@@ -635,6 +635,26 @@ void prepare_vm_identical_memmap(struct
acrn_vm *vm, uint16_t e820_entry_type, u
}
}

+int32_t get_sworld_vm_index(uint16_t vm_id) {
+ int16_t i;
+ int32_t page_idx = 0;
Use vm_idx.
Done

+ struct acrn_vm_config *vm_config;
+
+ for (i = 0; i < vm_id; i++) {
+ vm_config = get_vm_config(i);
+ if ((vm_config->guest_flags &
GUEST_FLAG_SECURE_WORLD_ENABLED) != 0U) {
+ page_idx += 1;
+ }
+ }
+
+ if (page_idx >= (int32_t)MAX_TRUSTY_VM_NUM) {
+ pr_err("Can't find sworld memory for vm id: %d", vm_id);
+ page_idx = -1;
+ }
+ return page_idx;
+}
+
/**
* @pre vm_id < CONFIG_MAX_VM_NUM && vm_config != NULL &&
rtn_vm != NULL
* @pre vm->state == VM_POWERED_OFF
@@ -667,12 +687,14 @@ int32_t create_vm(uint16_t vm_id, uint64_t
pcpu_bitmap, struct acrn_vm_config *v
vm->sworld_control.flag.supported = 1U;
}
if (vm->sworld_control.flag.supported != 0UL) {
- uint16_t service_vm_id = (get_service_vm())->vm_id;
- uint16_t page_idx = vmid_2_rel_vmid(service_vm_id,
vm_id) - 1U;
+ int32_t page_idx = get_sworld_vm_index(vm_id);

- ept_add_mr(vm, (uint64_t *)vm-
arch_vm.nworld_eptp,
-
hva2hpa(post_user_vm_sworld_memory[page_idx]),
- TRUSTY_EPT_REBASE_GPA,
TRUSTY_RAM_SIZE, EPT_WB | EPT_RWX);
+ if (page_idx >= 0)
+ {
+ ept_add_mr(vm, (uint64_t *)vm-
arch_vm.nworld_eptp,
+
hva2hpa(post_user_vm_sworld_memory[page_idx]),
+ TRUSTY_EPT_REBASE_GPA,
TRUSTY_RAM_SIZE, EPT_WB | EPT_RWX);
+ }
For "else" case, how it will be?
I thought create_vm should fail and stop to continue.
Yes.

The trusty  VM can't run on the ACRN without swrold memory, so I will add "else" to set error status and stop the create_vm.

}
if (vm_config->name[0] == '\0') {
/* if VM name is not configured, specify with VM ID
*/
--
2.25.1







Eddie Dong
 

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of chenli.wei
Sent: Thursday, September 29, 2022 8:13 PM
To: acrn-dev@...; Sun, Victor <victor.sun@...>; Mao,
Junjie <junjie.mao@...>
Cc: Wei, Chenli <chenli.wei@...>
Subject: Re: [acrn-dev] [PATCH v2 2/2] hv: refine the sworld memory allocate


On 9/30/2022 4:40 AM, Eddie Dong wrote:

-----Original Message-----
From: acrn-dev@... <acrn-dev@...>
On Behalf Of chenli.wei
Sent: Thursday, September 29, 2022 9:16 AM
To: Sun, Victor <victor.sun@...>; Mao, Junjie
<junjie.mao@...>; acrn-dev@...
Cc: Wei, Chenli <chenli.wei@...>
Subject: [acrn-dev] [PATCH v2 2/2] hv: refine the sworld memory
allocate

From: Chenli Wei <chenli.wei@...>

The current code uses a predefined sworld memory array to reserve
memory for trusty VMs, and assume all post launched VMs are trusty VM
which is not correct.

This patch statically reserved memory just for trusty VMs and save
16M memory for every non trusty VM.

v1-->v2:
1. code format refine and function rename

Signed-off-by: Chenli Wei <chenli.wei@...>
---
hypervisor/arch/x86/guest/vm.c | 34 ++++++++++++++++++++++++++++----
--
1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/hypervisor/arch/x86/guest/vm.c
b/hypervisor/arch/x86/guest/vm.c index bdd31d3fa..6fbc2c322 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -48,7 +48,7 @@
/* Local variables */

/* pre-assumption: TRUSTY_RAM_SIZE is 2M aligned */ -static struct
page
post_user_vm_sworld_memory[MAX_POST_VM_NUM][TRUSTY_RAM_SIZE >>
PAGE_SHIFT] __aligned(MEM_2M);
+static struct page
+post_user_vm_sworld_memory[MAX_TRUSTY_VM_NUM][TRUSTY_RAM_SIZ
E >>
+PAGE_SHIFT] __aligned(MEM_2M);

static struct acrn_vm vm_array[CONFIG_MAX_VM_NUM]
__aligned(PAGE_SIZE);

@@ -635,6 +635,26 @@ void prepare_vm_identical_memmap(struct
acrn_vm *vm, uint16_t e820_entry_type, u
}
}

+int32_t get_sworld_vm_index(uint16_t vm_id) {
+ int16_t i;
+ int32_t page_idx = 0;
Use vm_idx.
Done

+ struct acrn_vm_config *vm_config;
+
+ for (i = 0; i < vm_id; i++) {
+ vm_config = get_vm_config(i);
+ if ((vm_config->guest_flags &
GUEST_FLAG_SECURE_WORLD_ENABLED) != 0U) {
+ page_idx += 1;
+ }
+ }
+
+ if (page_idx >= (int32_t)MAX_TRUSTY_VM_NUM) {

BTW, this API assumes the current VM (w/ vm_id) is a VM with secure world. Right?
Do we need to double check this as well?


+ pr_err("Can't find sworld memory for vm id: %d", vm_id);
+ page_idx = -1;
+ }
+ return page_idx;
+}
+
/**
* @pre vm_id < CONFIG_MAX_VM_NUM && vm_config != NULL &&
rtn_vm !=
NULL
* @pre vm->state == VM_POWERED_OFF @@ -667,12 +687,14 @@
int32_t
create_vm(uint16_t vm_id, uint64_t pcpu_bitmap, struct acrn_vm_config
*v
vm->sworld_control.flag.supported = 1U;
}
if (vm->sworld_control.flag.supported != 0UL) {
- uint16_t service_vm_id = (get_service_vm())->vm_id;
- uint16_t page_idx = vmid_2_rel_vmid(service_vm_id,
vm_id) - 1U;
+ int32_t page_idx = get_sworld_vm_index(vm_id);

- ept_add_mr(vm, (uint64_t *)vm-
arch_vm.nworld_eptp,
-
hva2hpa(post_user_vm_sworld_memory[page_idx]),
- TRUSTY_EPT_REBASE_GPA,
TRUSTY_RAM_SIZE, EPT_WB | EPT_RWX);
+ if (page_idx >= 0)
+ {
+ ept_add_mr(vm, (uint64_t *)vm-
arch_vm.nworld_eptp,
+
hva2hpa(post_user_vm_sworld_memory[page_idx]),
+ TRUSTY_EPT_REBASE_GPA,
TRUSTY_RAM_SIZE, EPT_WB | EPT_RWX);
+ }
For "else" case, how it will be?
I thought create_vm should fail and stop to continue.
Yes.

The trusty  VM can't run on the ACRN without swrold memory, so I will add
"else" to set error status and stop the create_vm.

}
if (vm_config->name[0] == '\0') {
/* if VM name is not configured, specify with VM ID
*/
--
2.25.1










chenli.wei
 

On 9/30/2022 1:05 PM, Eddie Dong wrote:

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of chenli.wei
Sent: Thursday, September 29, 2022 8:13 PM
To: acrn-dev@...; Sun, Victor <victor.sun@...>; Mao,
Junjie <junjie.mao@...>
Cc: Wei, Chenli <chenli.wei@...>
Subject: Re: [acrn-dev] [PATCH v2 2/2] hv: refine the sworld memory allocate


On 9/30/2022 4:40 AM, Eddie Dong wrote:
-----Original Message-----
From: acrn-dev@... <acrn-dev@...>
On Behalf Of chenli.wei
Sent: Thursday, September 29, 2022 9:16 AM
To: Sun, Victor <victor.sun@...>; Mao, Junjie
<junjie.mao@...>; acrn-dev@...
Cc: Wei, Chenli <chenli.wei@...>
Subject: [acrn-dev] [PATCH v2 2/2] hv: refine the sworld memory
allocate

From: Chenli Wei <chenli.wei@...>

The current code uses a predefined sworld memory array to reserve
memory for trusty VMs, and assume all post launched VMs are trusty VM
which is not correct.

This patch statically reserved memory just for trusty VMs and save
16M memory for every non trusty VM.

v1-->v2:
1. code format refine and function rename

Signed-off-by: Chenli Wei <chenli.wei@...>
---
hypervisor/arch/x86/guest/vm.c | 34 ++++++++++++++++++++++++++++----
--
1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/hypervisor/arch/x86/guest/vm.c
b/hypervisor/arch/x86/guest/vm.c index bdd31d3fa..6fbc2c322 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -48,7 +48,7 @@
/* Local variables */

/* pre-assumption: TRUSTY_RAM_SIZE is 2M aligned */ -static struct
page
post_user_vm_sworld_memory[MAX_POST_VM_NUM][TRUSTY_RAM_SIZE >>
PAGE_SHIFT] __aligned(MEM_2M);
+static struct page
+post_user_vm_sworld_memory[MAX_TRUSTY_VM_NUM][TRUSTY_RAM_SIZ
E >>
+PAGE_SHIFT] __aligned(MEM_2M);

static struct acrn_vm vm_array[CONFIG_MAX_VM_NUM]
__aligned(PAGE_SIZE);

@@ -635,6 +635,26 @@ void prepare_vm_identical_memmap(struct
acrn_vm *vm, uint16_t e820_entry_type, u
}
}

+int32_t get_sworld_vm_index(uint16_t vm_id) {
+ int16_t i;
+ int32_t page_idx = 0;
Use vm_idx.
Done
+ struct acrn_vm_config *vm_config;
+
+ for (i = 0; i < vm_id; i++) {
+ vm_config = get_vm_config(i);
+ if ((vm_config->guest_flags &
GUEST_FLAG_SECURE_WORLD_ENABLED) != 0U) {
+ page_idx += 1;
+ }
+ }
+
+ if (page_idx >= (int32_t)MAX_TRUSTY_VM_NUM) {
BTW, this API assumes the current VM (w/ vm_id) is a VM with secure world. Right?
Do we need to double check this as well?
There is a trusty VM check before call this interface.

If we add some trusty VM check in this function, we should decide whether pop an error message which will stop the create_vm after we add some logic to check the return value.



+ pr_err("Can't find sworld memory for vm id: %d", vm_id);
+ page_idx = -1;
+ }
+ return page_idx;
+}
+
/**
* @pre vm_id < CONFIG_MAX_VM_NUM && vm_config != NULL &&
rtn_vm !=
NULL
* @pre vm->state == VM_POWERED_OFF @@ -667,12 +687,14 @@
int32_t
create_vm(uint16_t vm_id, uint64_t pcpu_bitmap, struct acrn_vm_config
*v
vm->sworld_control.flag.supported = 1U;
}
if (vm->sworld_control.flag.supported != 0UL) {
- uint16_t service_vm_id = (get_service_vm())->vm_id;
- uint16_t page_idx = vmid_2_rel_vmid(service_vm_id,
vm_id) - 1U;
+ int32_t page_idx = get_sworld_vm_index(vm_id);

- ept_add_mr(vm, (uint64_t *)vm-
arch_vm.nworld_eptp,
-
hva2hpa(post_user_vm_sworld_memory[page_idx]),
- TRUSTY_EPT_REBASE_GPA,
TRUSTY_RAM_SIZE, EPT_WB | EPT_RWX);
+ if (page_idx >= 0)
+ {
+ ept_add_mr(vm, (uint64_t *)vm-
arch_vm.nworld_eptp,
+
hva2hpa(post_user_vm_sworld_memory[page_idx]),
+ TRUSTY_EPT_REBASE_GPA,
TRUSTY_RAM_SIZE, EPT_WB | EPT_RWX);
+ }
For "else" case, how it will be?
I thought create_vm should fail and stop to continue.
Yes.

The trusty  VM can't run on the ACRN without swrold memory, so I will add
"else" to set error status and stop the create_vm.

}
if (vm_config->name[0] == '\0') {
/* if VM name is not configured, specify with VM ID
*/
--
2.25.1










Eddie Dong
 

+ if (page_idx >= (int32_t)MAX_TRUSTY_VM_NUM) {
BTW, this API assumes the current VM (w/ vm_id) is a VM with secure world.
Right?
Do we need to double check this as well?
There is a trusty VM check before call this interface.
Then, put an @pre comments


If we add some trusty VM check in this function, we should decide whether
pop an error message which will stop the create_vm after we add some logic
to check the return value.