Re: [PATCH v9 24/27] virt: gunyah: Add proxy-scheduled vCPUs

[Elliot Berman]

On 2/7/2023 6:43 AM, Srinivas Kandagatla wrote:
> On 20/01/2023 22:46, Elliot Berman wrote:
> > Gunyah allows host virtual machines to schedule guest virtual machines
> > and handle their MMIO accesses. vCPUs are presented to the host as a
> > Gunyah resource and represented to userspace as a Gunyah VM function.
> >
> > Creating the vcpu VM function will create a file descriptor that:
> >   - can run an ioctl: GH_VCPU_RUN to schedule the guest vCPU until the
> >     next interrupt occurs on the host or when the guest vCPU can no
> >     longer be run.
> >   - can be mmap'd to share a gh_vcpu_run structure which can look up the
> >     reason why GH_VCPU_RUN returned and provide return values for MMIO
> >     access.
> >
> > Co-developed-by: Prakruthi Deepak Heragu <quic_pheragu@xxxxxxxxxxx>
> > Signed-off-by: Prakruthi Deepak Heragu <quic_pheragu@xxxxxxxxxxx>
> > Signed-off-by: Elliot Berman <quic_eberman@xxxxxxxxxxx>
> > ---
> >   Documentation/virt/gunyah/vm-manager.rst |  30 +-
> >   arch/arm64/gunyah/gunyah_hypercall.c     |  28 ++
> >   drivers/virt/gunyah/Kconfig              |  11 +
> >   drivers/virt/gunyah/Makefile             |   2 +
> >   drivers/virt/gunyah/gunyah_vcpu.c        | 358 +++++++++++++++++++++++
> >   drivers/virt/gunyah/vm_mgr.c             |  25 ++
> >   drivers/virt/gunyah/vm_mgr.h             |   1 +
> >   include/linux/gunyah.h                   |   7 +
> >   include/uapi/linux/gunyah.h              |  30 ++
> >   9 files changed, 490 insertions(+), 2 deletions(-)
> >   create mode 100644 drivers/virt/gunyah/gunyah_vcpu.c
> ...
>
>
> > diff --git a/include/uapi/linux/gunyah.h b/include/uapi/linux/gunyah.h
> > index ec8da6fde045..b4afb11f538a 100644
> > --- a/include/uapi/linux/gunyah.h
> > +++ b/include/uapi/linux/gunyah.h
> > @@ -53,9 +53,14 @@ struct gh_vm_dtb_config {
> >   #define GUNYAH_FUNCTION_NAME_SIZE        32
> >   #define GUNYAH_FUNCTION_MAX_ARG_SIZE        1024
> > +struct gh_fn_vcpu_arg {
> > +    __u32 vcpu_id;
> > +};
> > +
> >   struct gh_vm_function {
> >       char name[GUNYAH_FUNCTION_NAME_SIZE];
> >       union {
> > +        struct gh_device_vcpu_arg vcpu;
> >           char data[GUNYAH_FUNCTION_MAX_ARG_SIZE];
> >       };
> >   };
> > @@ -63,4 +68,29 @@ struct gh_vm_function {
> >   #define GH_VM_ADD_FUNCTION    _IOW(GH_IOCTL_TYPE, 0x4, struct 
> > gh_vm_function)
> >   #define GH_VM_REMOVE_FUNCTION    _IOW(GH_IOCTL_TYPE, 0x7, struct 
> > gh_vm_function)
> > +/* for GH_VCPU_RUN, returned by mmap(vcpu_fd, offset=0) */
> > +struct gh_vcpu_run {
> this looks unused, I dont see any reference to this.

This structure gets mapped into userspace from the vCPU file descriptor, 
similar principle to KVM.

Thanks,
Elliot

> > +    /* in */
> > +    __u8 immediate_exit;
> > +    __u8 padding1[7];
> > +
> > +    /* out */
> > +#define GH_VM_EXIT_UNKNOWN            0
> > +#define GH_VM_EXIT_MMIO               1
> > +    __u32 exit_reason;
>
> If this is ment to go in any of the ioctl arguments then this suffers 
> same issue of padding.
>
> > +
> > +    union {
> > +        /* GH_VM_EXIT_MMIO */
> > +        struct {
> > +            __u64 phys_addr;
> > +            __u8  data[8];
> > +            __u32 len;
> > +            __u8  is_write;
> > +        } mmio;
> > +    };
> > +};
> > +
> > +#define GH_VCPU_RUN        _IO(GH_IOCTL_TYPE, 0x5)
> > +#define GH_VCPU_MMAP_SIZE    _IO(GH_IOCTL_TYPE, 0x6)
> > +
> >   #endif