[PATCH] hax: Dynamic allocate vcpu state structure

[PATCH] hax: Dynamic allocate vcpu state structure

[WangBowen]

Dynamic allocating vcpu state structure according to smp value to be
more precise and safe. Previously it will allocate array of fixed size
HAX_MAX_VCPU.

This is achieved by using g_new0 to dynamic allocate the array. The
allocated size is obtained from smp.max_cpus in MachineState. Also, the
size is compared with HAX_MAX_VCPU when creating the vm. The reason for
chosing dynamic array over linked list is because the status is visited
by index all the time.

This will lead to QEMU checking whether the smp value is larger than the
HAX_MAX_VCPU when creating vm, if larger, the process will terminate,
otherwise it will allocate array of size smp to store the status.

Signed-off-by: WangBowen <bowen.wang@intel.com>
---
 target/i386/hax-all.c  | 25 +++++++++++++++++++------
 target/i386/hax-i386.h |  5 +++--
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
index a8b6e5aeb8..7ccd53a901 100644
--- a/target/i386/hax-all.c
+++ b/target/i386/hax-all.c
@@ -232,10 +232,10 @@ int hax_init_vcpu(CPUState *cpu)
     return ret;
 }
 
-struct hax_vm *hax_vm_create(struct hax_state *hax)
+struct hax_vm *hax_vm_create(struct hax_state *hax, int max_cpus)
 {
     struct hax_vm *vm;
-    int vm_id = 0, ret;
+    int vm_id = 0, ret, i;
 
     if (hax_invalid_fd(hax->fd)) {
         return NULL;
@@ -259,6 +259,17 @@ struct hax_vm *hax_vm_create(struct hax_state *hax)
         goto error;
     }
 
+    if (max_cpus > HAX_MAX_VCPU){
+        fprintf(stderr, "Failed to create vm, maximum possible VCPU number supported by QEMU is %d\n", HAX_MAX_VCPU);
+        goto error;
+    }
+
+    vm->numvcpus = max_cpus;
+    vm->vcpus = g_new0(struct hax_vcpu_state *, vm->numvcpus);
+    for(i = 0; i < vm->numvcpus; i++){
+        vm->vcpus[i]=NULL;
+    }
+
     hax->vm = vm;
     return vm;
 
@@ -272,12 +283,14 @@ int hax_vm_destroy(struct hax_vm *vm)
 {
     int i;
 
-    for (i = 0; i < HAX_MAX_VCPU; i++)
+    for (i = 0; i < vm->numvcpus; i++)
         if (vm->vcpus[i]) {
             fprintf(stderr, "VCPU should be cleaned before vm clean\n");
             return -1;
         }
     hax_close_fd(vm->fd);
+    vm->numvcpus = 0;
+    g_free(vm->vcpus);
     g_free(vm);
     hax_global.vm = NULL;
     return 0;
@@ -292,7 +305,7 @@ static void hax_handle_interrupt(CPUState *cpu, int mask)
     }
 }
 
-static int hax_init(ram_addr_t ram_size)
+static int hax_init(ram_addr_t ram_size, int max_cpus)
 {
     struct hax_state *hax = NULL;
     struct hax_qemu_version qversion;
@@ -324,7 +337,7 @@ static int hax_init(ram_addr_t ram_size)
         goto error;
     }
 
-    hax->vm = hax_vm_create(hax);
+    hax->vm = hax_vm_create(hax, max_cpus);
     if (!hax->vm) {
         fprintf(stderr, "Failed to create HAX VM\n");
         ret = -EINVAL;
@@ -352,7 +365,7 @@ static int hax_init(ram_addr_t ram_size)
 
 static int hax_accel_init(MachineState *ms)
 {
-    int ret = hax_init(ms->ram_size);
+    int ret = hax_init(ms->ram_size, (int)ms->smp.max_cpus);
 
     if (ret && (ret != -ENOSPC)) {
         fprintf(stderr, "No accelerator found.\n");
diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h
index 54e9d8b057..7d988f81da 100644
--- a/target/i386/hax-i386.h
+++ b/target/i386/hax-i386.h
@@ -47,7 +47,8 @@ struct hax_state {
 struct hax_vm {
     hax_fd fd;
     int id;
-    struct hax_vcpu_state *vcpus[HAX_MAX_VCPU];
+    int numvcpus;
+    struct hax_vcpu_state **vcpus;
 };
 
 #ifdef NEED_CPU_H
@@ -58,7 +59,7 @@ int valid_hax_tunnel_size(uint16_t size);
 /* Host specific functions */
 int hax_mod_version(struct hax_state *hax, struct hax_module_version *version);
 int hax_inject_interrupt(CPUArchState *env, int vector);
-struct hax_vm *hax_vm_create(struct hax_state *hax);
+struct hax_vm *hax_vm_create(struct hax_state *hax, int max_cpus);
 int hax_vcpu_run(struct hax_vcpu_state *vcpu);
 int hax_vcpu_create(int id);
 int hax_sync_vcpu_state(CPUArchState *env, struct vcpu_state_t *state,
-- 
2.24.1

Re: [PATCH] hax: Dynamic allocate vcpu state structure

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200406035016.609-1-bowen.wang@intel.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH] hax: Dynamic allocate vcpu state structure
Message-id: 20200406035016.609-1-bowen.wang@intel.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
0f29908 hax: Dynamic allocate vcpu state structure

=== OUTPUT BEGIN ===
ERROR: space required before the open brace '{'
#45: FILE: target/i386/hax-all.c:262:
+    if (max_cpus > HAX_MAX_VCPU){

ERROR: line over 90 characters
#46: FILE: target/i386/hax-all.c:263:
+        fprintf(stderr, "Failed to create vm, maximum possible VCPU number supported by QEMU is %d\n", HAX_MAX_VCPU);

ERROR: space required before the open brace '{'
#52: FILE: target/i386/hax-all.c:269:
+    for(i = 0; i < vm->numvcpus; i++){

ERROR: space required before the open parenthesis '('
#52: FILE: target/i386/hax-all.c:269:
+    for(i = 0; i < vm->numvcpus; i++){

ERROR: spaces required around that '=' (ctx:VxV)
#53: FILE: target/i386/hax-all.c:270:
+        vm->vcpus[i]=NULL;
                     ^

total: 5 errors, 0 warnings, 85 lines checked

Commit 0f299088bd75 (hax: Dynamic allocate vcpu state structure) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200406035016.609-1-bowen.wang@intel.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH] hax: Dynamic allocate vcpu state structure

[Philippe Mathieu-Daudé]

On 4/28/20 4:47 AM, Colin Xu wrote:
> 
> And this one. 3 patches for HAX.
> 
> Thanks in advance.
> -- 
> Best Regards,
> Colin Xu
> 
> On Mon, 20 Apr 2020, Colin Xu wrote:
> 
>>
>> Looks good to me.
>>
>> Reviewed-by: Colin Xu <colin.xu@intel.com>
>>
>> -- 
>> Best Regards,
>> Colin Xu
>>
>> On Mon, 6 Apr 2020, WangBowen wrote:
>>
>>> Dynamic allocating vcpu state structure according to smp value to be
>>> more precise and safe. Previously it will alloccate array of fixed size
>>> HAX_MAX_VCPU.
>>>
>>> This is achieved by using g_new0 to dynamic allocate the array. The
>>> allocated size is obtained from smp.max_cpus in MachineState. Also, the
>>> size is compared with HAX_MAX_VCPU when creating the vm. The reason for
>>> choosing dynamic array over linked list is because the status is visited
>>> by index all the time.
>>>
>>> This will lead to QEMU checking whether the smp value is larger than the
>>> HAX_MAX_VCPU when creating vm, if larger, the process will terminate,
>>> otherwise it will allocate array of size smp to store the status.
>>>
>>> Signed-off-by: WangBowen <bowen.wang@intel.com>
>>> ---
>>> target/i386/hax-all.c  | 25 +++++++++++++++++++------
>>> target/i386/hax-i386.h |  5 +++--
>>> 2 files changed, 22 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
>>> index a8b6e5aeb8..a22adec5da 100644
>>> --- a/target/i386/hax-all.c
>>> +++ b/target/i386/hax-all.c
>>> @@ -232,10 +232,10 @@ int hax_init_vcpu(CPUState *cpu)
>>>     return ret;
>>> }
>>>
>>> -struct hax_vm *hax_vm_create(struct hax_state *hax)
>>> +struct hax_vm *hax_vm_create(struct hax_state *hax, int max_cpus)
>>> {
>>>     struct hax_vm *vm;
>>> -    int vm_id = 0, ret;
>>> +    int vm_id = 0, ret, i;
>>>
>>>     if (hax_invalid_fd(hax->fd)) {
>>>         return NULL;
>>> @@ -259,6 +259,17 @@ struct hax_vm *hax_vm_create(struct hax_state *hax)
>>>         goto error;
>>>     }
>>>
>>> +    if (max_cpus > HAX_MAX_VCPU) {
>>> +        fprintf(stderr, "Maximum VCPU number QEMU supported is 
>>> %d\n", HAX_MAX_VCPU);
>>> +        goto error;
>>> +    }
>>> +

Move this check before the 'vm = g_new0(struct hax_vm, 1);' and simply 
return NULL, no need to goto error, else you are leaking vm->fd.

>>> +    vm->numvcpus = max_cpus;
>>> +    vm->vcpus = g_new0(struct hax_vcpu_state *, vm->numvcpus);
>>> +    for (i = 0; i < vm->numvcpus; i++) {
>>> +        vm->vcpus[i] = NULL;
>>> +    }
>>> +
>>>     hax->vm = vm;
>>>     return vm;
>>>
>>> @@ -272,12 +283,14 @@ int hax_vm_destroy(struct hax_vm *vm)
>>> {
>>>     int i;
>>>
>>> -    for (i = 0; i < HAX_MAX_VCPU; i++)
>>> +    for (i = 0; i < vm->numvcpus; i++)
>>>         if (vm->vcpus[i]) {
>>>             fprintf(stderr, "VCPU should be cleaned before vm clean\n");
>>>             return -1;
>>>         }
>>>     hax_close_fd(vm->fd);
>>> +    vm->numvcpus = 0;
>>> +    g_free(vm->vcpus);
>>>     g_free(vm);
>>>     hax_global.vm = NULL;
>>>     return 0;
>>> @@ -292,7 +305,7 @@ static void hax_handle_interrupt(CPUState *cpu, 
>>> int mask)
>>>     }
>>> }
>>>
>>> -static int hax_init(ram_addr_t ram_size)
>>> +static int hax_init(ram_addr_t ram_size, int max_cpus)
>>> {
>>>     struct hax_state *hax = NULL;
>>>     struct hax_qemu_version qversion;
>>> @@ -324,7 +337,7 @@ static int hax_init(ram_addr_t ram_size)
>>>         goto error;
>>>     }
>>>
>>> -    hax->vm = hax_vm_create(hax);
>>> +    hax->vm = hax_vm_create(hax, max_cpus);
>>>     if (!hax->vm) {
>>>         fprintf(stderr, "Failed to create HAX VM\n");
>>>         ret = -EINVAL;
>>> @@ -352,7 +365,7 @@ static int hax_init(ram_addr_t ram_size)
>>>
>>> static int hax_accel_init(MachineState *ms)
>>> {
>>> -    int ret = hax_init(ms->ram_size);
>>> +    int ret = hax_init(ms->ram_size, (int)ms->smp.max_cpus);
>>>
>>>     if (ret && (ret != -ENOSPC)) {
>>>         fprintf(stderr, "No accelerator found.\n");
>>> diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h
>>> index 54e9d8b057..7d988f81da 100644
>>> --- a/target/i386/hax-i386.h
>>> +++ b/target/i386/hax-i386.h
>>> @@ -47,7 +47,8 @@ struct hax_state {
>>> struct hax_vm {
>>>     hax_fd fd;
>>>     int id;
>>> -    struct hax_vcpu_state *vcpus[HAX_MAX_VCPU];
>>> +    int numvcpus;
>>> +    struct hax_vcpu_state **vcpus;
>>> };
>>>
>>> #ifdef NEED_CPU_H
>>> @@ -58,7 +59,7 @@ int valid_hax_tunnel_size(uint16_t size);
>>> /* Host specific functions */
>>> int hax_mod_version(struct hax_state *hax, struct hax_module_version 
>>> *version);
>>> int hax_inject_interrupt(CPUArchState *env, int vector);
>>> -struct hax_vm *hax_vm_create(struct hax_state *hax);
>>> +struct hax_vm *hax_vm_create(struct hax_state *hax, int max_cpus);
>>> int hax_vcpu_run(struct hax_vcpu_state *vcpu);
>>> int hax_vcpu_create(int id);
>>> int hax_sync_vcpu_state(CPUArchState *env, struct vcpu_state_t *state,
>>> -- 
>>> 2.24.1
>>>
>>>
>>
> 

Re: [PATCH] hax: Dynamic allocate vcpu state structure

[Colin Xu]

And this one. 3 patches for HAX.

Thanks in advance.
--
Best Regards,
Colin Xu

On Mon, 20 Apr 2020, Colin Xu wrote:

>
> Looks good to me.
>
> Reviewed-by: Colin Xu <colin.xu@intel.com>
>
> --
> Best Regards,
> Colin Xu
>
> On Mon, 6 Apr 2020, WangBowen wrote:
>
>> Dynamic allocating vcpu state structure according to smp value to be
>> more precise and safe. Previously it will alloccate array of fixed size
>> HAX_MAX_VCPU.
>> 
>> This is achieved by using g_new0 to dynamic allocate the array. The
>> allocated size is obtained from smp.max_cpus in MachineState. Also, the
>> size is compared with HAX_MAX_VCPU when creating the vm. The reason for
>> choosing dynamic array over linked list is because the status is visited
>> by index all the time.
>> 
>> This will lead to QEMU checking whether the smp value is larger than the
>> HAX_MAX_VCPU when creating vm, if larger, the process will terminate,
>> otherwise it will allocate array of size smp to store the status.
>> 
>> Signed-off-by: WangBowen <bowen.wang@intel.com>
>> ---
>> target/i386/hax-all.c  | 25 +++++++++++++++++++------
>> target/i386/hax-i386.h |  5 +++--
>> 2 files changed, 22 insertions(+), 8 deletions(-)
>> 
>> diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
>> index a8b6e5aeb8..a22adec5da 100644
>> --- a/target/i386/hax-all.c
>> +++ b/target/i386/hax-all.c
>> @@ -232,10 +232,10 @@ int hax_init_vcpu(CPUState *cpu)
>>     return ret;
>> }
>> 
>> -struct hax_vm *hax_vm_create(struct hax_state *hax)
>> +struct hax_vm *hax_vm_create(struct hax_state *hax, int max_cpus)
>> {
>>     struct hax_vm *vm;
>> -    int vm_id = 0, ret;
>> +    int vm_id = 0, ret, i;
>>
>>     if (hax_invalid_fd(hax->fd)) {
>>         return NULL;
>> @@ -259,6 +259,17 @@ struct hax_vm *hax_vm_create(struct hax_state *hax)
>>         goto error;
>>     }
>> 
>> +    if (max_cpus > HAX_MAX_VCPU) {
>> +        fprintf(stderr, "Maximum VCPU number QEMU supported is %d\n", 
>> HAX_MAX_VCPU);
>> +        goto error;
>> +    }
>> +
>> +    vm->numvcpus = max_cpus;
>> +    vm->vcpus = g_new0(struct hax_vcpu_state *, vm->numvcpus);
>> +    for (i = 0; i < vm->numvcpus; i++) {
>> +        vm->vcpus[i] = NULL;
>> +    }
>> +
>>     hax->vm = vm;
>>     return vm;
>> 
>> @@ -272,12 +283,14 @@ int hax_vm_destroy(struct hax_vm *vm)
>> {
>>     int i;
>> 
>> -    for (i = 0; i < HAX_MAX_VCPU; i++)
>> +    for (i = 0; i < vm->numvcpus; i++)
>>         if (vm->vcpus[i]) {
>>             fprintf(stderr, "VCPU should be cleaned before vm clean\n");
>>             return -1;
>>         }
>>     hax_close_fd(vm->fd);
>> +    vm->numvcpus = 0;
>> +    g_free(vm->vcpus);
>>     g_free(vm);
>>     hax_global.vm = NULL;
>>     return 0;
>> @@ -292,7 +305,7 @@ static void hax_handle_interrupt(CPUState *cpu, int 
>> mask)
>>     }
>> }
>> 
>> -static int hax_init(ram_addr_t ram_size)
>> +static int hax_init(ram_addr_t ram_size, int max_cpus)
>> {
>>     struct hax_state *hax = NULL;
>>     struct hax_qemu_version qversion;
>> @@ -324,7 +337,7 @@ static int hax_init(ram_addr_t ram_size)
>>         goto error;
>>     }
>> 
>> -    hax->vm = hax_vm_create(hax);
>> +    hax->vm = hax_vm_create(hax, max_cpus);
>>     if (!hax->vm) {
>>         fprintf(stderr, "Failed to create HAX VM\n");
>>         ret = -EINVAL;
>> @@ -352,7 +365,7 @@ static int hax_init(ram_addr_t ram_size)
>> 
>> static int hax_accel_init(MachineState *ms)
>> {
>> -    int ret = hax_init(ms->ram_size);
>> +    int ret = hax_init(ms->ram_size, (int)ms->smp.max_cpus);
>>
>>     if (ret && (ret != -ENOSPC)) {
>>         fprintf(stderr, "No accelerator found.\n");
>> diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h
>> index 54e9d8b057..7d988f81da 100644
>> --- a/target/i386/hax-i386.h
>> +++ b/target/i386/hax-i386.h
>> @@ -47,7 +47,8 @@ struct hax_state {
>> struct hax_vm {
>>     hax_fd fd;
>>     int id;
>> -    struct hax_vcpu_state *vcpus[HAX_MAX_VCPU];
>> +    int numvcpus;
>> +    struct hax_vcpu_state **vcpus;
>> };
>> 
>> #ifdef NEED_CPU_H
>> @@ -58,7 +59,7 @@ int valid_hax_tunnel_size(uint16_t size);
>> /* Host specific functions */
>> int hax_mod_version(struct hax_state *hax, struct hax_module_version 
>> *version);
>> int hax_inject_interrupt(CPUArchState *env, int vector);
>> -struct hax_vm *hax_vm_create(struct hax_state *hax);
>> +struct hax_vm *hax_vm_create(struct hax_state *hax, int max_cpus);
>> int hax_vcpu_run(struct hax_vcpu_state *vcpu);
>> int hax_vcpu_create(int id);
>> int hax_sync_vcpu_state(CPUArchState *env, struct vcpu_state_t *state,
>> -- 
>> 2.24.1
>> 
>> 
>

Re: [PATCH] hax: Dynamic allocate vcpu state structure

[Colin Xu]

Looks good to me.

Reviewed-by: Colin Xu <colin.xu@intel.com>

--
Best Regards,
Colin Xu

On Mon, 6 Apr 2020, WangBowen wrote:

> Dynamic allocating vcpu state structure according to smp value to be
> more precise and safe. Previously it will alloccate array of fixed size
> HAX_MAX_VCPU.
>
> This is achieved by using g_new0 to dynamic allocate the array. The
> allocated size is obtained from smp.max_cpus in MachineState. Also, the
> size is compared with HAX_MAX_VCPU when creating the vm. The reason for
> choosing dynamic array over linked list is because the status is visited
> by index all the time.
>
> This will lead to QEMU checking whether the smp value is larger than the
> HAX_MAX_VCPU when creating vm, if larger, the process will terminate,
> otherwise it will allocate array of size smp to store the status.
>
> Signed-off-by: WangBowen <bowen.wang@intel.com>
> ---
> target/i386/hax-all.c  | 25 +++++++++++++++++++------
> target/i386/hax-i386.h |  5 +++--
> 2 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
> index a8b6e5aeb8..a22adec5da 100644
> --- a/target/i386/hax-all.c
> +++ b/target/i386/hax-all.c
> @@ -232,10 +232,10 @@ int hax_init_vcpu(CPUState *cpu)
>     return ret;
> }
>
> -struct hax_vm *hax_vm_create(struct hax_state *hax)
> +struct hax_vm *hax_vm_create(struct hax_state *hax, int max_cpus)
> {
>     struct hax_vm *vm;
> -    int vm_id = 0, ret;
> +    int vm_id = 0, ret, i;
>
>     if (hax_invalid_fd(hax->fd)) {
>         return NULL;
> @@ -259,6 +259,17 @@ struct hax_vm *hax_vm_create(struct hax_state *hax)
>         goto error;
>     }
>
> +    if (max_cpus > HAX_MAX_VCPU) {
> +        fprintf(stderr, "Maximum VCPU number QEMU supported is %d\n", HAX_MAX_VCPU);
> +        goto error;
> +    }
> +
> +    vm->numvcpus = max_cpus;
> +    vm->vcpus = g_new0(struct hax_vcpu_state *, vm->numvcpus);
> +    for (i = 0; i < vm->numvcpus; i++) {
> +        vm->vcpus[i] = NULL;
> +    }
> +
>     hax->vm = vm;
>     return vm;
>
> @@ -272,12 +283,14 @@ int hax_vm_destroy(struct hax_vm *vm)
> {
>     int i;
>
> -    for (i = 0; i < HAX_MAX_VCPU; i++)
> +    for (i = 0; i < vm->numvcpus; i++)
>         if (vm->vcpus[i]) {
>             fprintf(stderr, "VCPU should be cleaned before vm clean\n");
>             return -1;
>         }
>     hax_close_fd(vm->fd);
> +    vm->numvcpus = 0;
> +    g_free(vm->vcpus);
>     g_free(vm);
>     hax_global.vm = NULL;
>     return 0;
> @@ -292,7 +305,7 @@ static void hax_handle_interrupt(CPUState *cpu, int mask)
>     }
> }
>
> -static int hax_init(ram_addr_t ram_size)
> +static int hax_init(ram_addr_t ram_size, int max_cpus)
> {
>     struct hax_state *hax = NULL;
>     struct hax_qemu_version qversion;
> @@ -324,7 +337,7 @@ static int hax_init(ram_addr_t ram_size)
>         goto error;
>     }
>
> -    hax->vm = hax_vm_create(hax);
> +    hax->vm = hax_vm_create(hax, max_cpus);
>     if (!hax->vm) {
>         fprintf(stderr, "Failed to create HAX VM\n");
>         ret = -EINVAL;
> @@ -352,7 +365,7 @@ static int hax_init(ram_addr_t ram_size)
>
> static int hax_accel_init(MachineState *ms)
> {
> -    int ret = hax_init(ms->ram_size);
> +    int ret = hax_init(ms->ram_size, (int)ms->smp.max_cpus);
>
>     if (ret && (ret != -ENOSPC)) {
>         fprintf(stderr, "No accelerator found.\n");
> diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h
> index 54e9d8b057..7d988f81da 100644
> --- a/target/i386/hax-i386.h
> +++ b/target/i386/hax-i386.h
> @@ -47,7 +47,8 @@ struct hax_state {
> struct hax_vm {
>     hax_fd fd;
>     int id;
> -    struct hax_vcpu_state *vcpus[HAX_MAX_VCPU];
> +    int numvcpus;
> +    struct hax_vcpu_state **vcpus;
> };
>
> #ifdef NEED_CPU_H
> @@ -58,7 +59,7 @@ int valid_hax_tunnel_size(uint16_t size);
> /* Host specific functions */
> int hax_mod_version(struct hax_state *hax, struct hax_module_version *version);
> int hax_inject_interrupt(CPUArchState *env, int vector);
> -struct hax_vm *hax_vm_create(struct hax_state *hax);
> +struct hax_vm *hax_vm_create(struct hax_state *hax, int max_cpus);
> int hax_vcpu_run(struct hax_vcpu_state *vcpu);
> int hax_vcpu_create(int id);
> int hax_sync_vcpu_state(CPUArchState *env, struct vcpu_state_t *state,
> -- 
> 2.24.1
>
>

[PATCH] hax: Dynamic allocate vcpu state structure

[WangBowen]

Dynamic allocating vcpu state structure according to smp value to be
more precise and safe. Previously it will alloccate array of fixed size
HAX_MAX_VCPU.

This is achieved by using g_new0 to dynamic allocate the array. The
allocated size is obtained from smp.max_cpus in MachineState. Also, the
size is compared with HAX_MAX_VCPU when creating the vm. The reason for
choosing dynamic array over linked list is because the status is visited
by index all the time.

This will lead to QEMU checking whether the smp value is larger than the
HAX_MAX_VCPU when creating vm, if larger, the process will terminate,
otherwise it will allocate array of size smp to store the status.

Signed-off-by: WangBowen <bowen.wang@intel.com>
---
 target/i386/hax-all.c  | 25 +++++++++++++++++++------
 target/i386/hax-i386.h |  5 +++--
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
index a8b6e5aeb8..a22adec5da 100644
--- a/target/i386/hax-all.c
+++ b/target/i386/hax-all.c
@@ -232,10 +232,10 @@ int hax_init_vcpu(CPUState *cpu)
     return ret;
 }
 
-struct hax_vm *hax_vm_create(struct hax_state *hax)
+struct hax_vm *hax_vm_create(struct hax_state *hax, int max_cpus)
 {
     struct hax_vm *vm;
-    int vm_id = 0, ret;
+    int vm_id = 0, ret, i;
 
     if (hax_invalid_fd(hax->fd)) {
         return NULL;
@@ -259,6 +259,17 @@ struct hax_vm *hax_vm_create(struct hax_state *hax)
         goto error;
     }
 
+    if (max_cpus > HAX_MAX_VCPU) {
+        fprintf(stderr, "Maximum VCPU number QEMU supported is %d\n", HAX_MAX_VCPU);
+        goto error;
+    }
+
+    vm->numvcpus = max_cpus;
+    vm->vcpus = g_new0(struct hax_vcpu_state *, vm->numvcpus);
+    for (i = 0; i < vm->numvcpus; i++) {
+        vm->vcpus[i] = NULL;
+    }
+
     hax->vm = vm;
     return vm;
 
@@ -272,12 +283,14 @@ int hax_vm_destroy(struct hax_vm *vm)
 {
     int i;
 
-    for (i = 0; i < HAX_MAX_VCPU; i++)
+    for (i = 0; i < vm->numvcpus; i++)
         if (vm->vcpus[i]) {
             fprintf(stderr, "VCPU should be cleaned before vm clean\n");
             return -1;
         }
     hax_close_fd(vm->fd);
+    vm->numvcpus = 0;
+    g_free(vm->vcpus);
     g_free(vm);
     hax_global.vm = NULL;
     return 0;
@@ -292,7 +305,7 @@ static void hax_handle_interrupt(CPUState *cpu, int mask)
     }
 }
 
-static int hax_init(ram_addr_t ram_size)
+static int hax_init(ram_addr_t ram_size, int max_cpus)
 {
     struct hax_state *hax = NULL;
     struct hax_qemu_version qversion;
@@ -324,7 +337,7 @@ static int hax_init(ram_addr_t ram_size)
         goto error;
     }
 
-    hax->vm = hax_vm_create(hax);
+    hax->vm = hax_vm_create(hax, max_cpus);
     if (!hax->vm) {
         fprintf(stderr, "Failed to create HAX VM\n");
         ret = -EINVAL;
@@ -352,7 +365,7 @@ static int hax_init(ram_addr_t ram_size)
 
 static int hax_accel_init(MachineState *ms)
 {
-    int ret = hax_init(ms->ram_size);
+    int ret = hax_init(ms->ram_size, (int)ms->smp.max_cpus);
 
     if (ret && (ret != -ENOSPC)) {
         fprintf(stderr, "No accelerator found.\n");
diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h
index 54e9d8b057..7d988f81da 100644
--- a/target/i386/hax-i386.h
+++ b/target/i386/hax-i386.h
@@ -47,7 +47,8 @@ struct hax_state {
 struct hax_vm {
     hax_fd fd;
     int id;
-    struct hax_vcpu_state *vcpus[HAX_MAX_VCPU];
+    int numvcpus;
+    struct hax_vcpu_state **vcpus;
 };
 
 #ifdef NEED_CPU_H
@@ -58,7 +59,7 @@ int valid_hax_tunnel_size(uint16_t size);
 /* Host specific functions */
 int hax_mod_version(struct hax_state *hax, struct hax_module_version *version);
 int hax_inject_interrupt(CPUArchState *env, int vector);
-struct hax_vm *hax_vm_create(struct hax_state *hax);
+struct hax_vm *hax_vm_create(struct hax_state *hax, int max_cpus);
 int hax_vcpu_run(struct hax_vcpu_state *vcpu);
 int hax_vcpu_create(int id);
 int hax_sync_vcpu_state(CPUArchState *env, struct vcpu_state_t *state,
-- 
2.24.1