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 <address@hidden>
>
> --
> 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 <address@hidden>
> > ---
> > 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