[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v12 7/8] Add IOREQ_TYPE_VMWARE_PORT



On Sat, Jun 27, 2015 at 07:27:44PM -0400, Don Slutz wrote:
> From: Don Slutz <dslutz@xxxxxxxxxxx>
> 
> This adds synchronization of the 6 vcpu registers (only 32bits of
> them) that vmport.c needs between Xen and QEMU.
> 
> This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
> fetch and put these 6 vcpu registers used by the code in vmport.c
> and vmmouse.c
> 
> In the tools, enable usage of QEMU's vmport code.
> 
> The currently most useful VMware port support that QEMU has is the
> VMware mouse support.  Xorg included a VMware mouse support that
> uses absolute mode.  This make using a mouse in X11 much nicer.
> 
> Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> CC: Don Slutz <don.slutz@xxxxxxxxx>
> ---
> v12:
>   Rebase changes.
> 
>   Pass size to vmport_check_port() -- required if overlap
>   I.E. inl on port 0x5657, 0x5656, 0x5655, 0x5659, 0x565a,
>   and 0x565b.
> 
>   Move define of vmport_check_port() into this patch from ring3
>   patch.
> 
> v11:
>   No change
> 
> v10:
>   These literals should become an enum.
>     I don't think the invalidate type is needed.
>     Code handling "case X86EMUL_UNHANDLEABLE:" in emulate.c
>     is unclear.
>     Comment about "special' range of 1" is not clear.
> 
> 
> v9:
>   New code was presented as an RFC before this.
> 
>   Paul Durrant sugested I add support for other IOREQ types
>   to HVMOP_map_io_range_to_ioreq_server.
>     I have done this.
> 
>  tools/libxc/xc_hvm_build_x86.c   |   5 +-
>  tools/libxl/libxl_dm.c           |   2 +
>  xen/arch/x86/hvm/emulate.c       |  83 +++++++++++++++---
>  xen/arch/x86/hvm/hvm.c           | 182 
> ++++++++++++++++++++++++++++++++++-----
>  xen/arch/x86/hvm/io.c            |  16 ++++
>  xen/arch/x86/hvm/vmware/vmport.c |  13 +++
>  xen/include/asm-x86/hvm/domain.h |   3 +-
>  xen/include/asm-x86/hvm/hvm.h    |   2 +
>  xen/include/public/hvm/hvm_op.h  |   5 ++
>  xen/include/public/hvm/ioreq.h   |  17 ++++
>  xen/include/public/hvm/params.h  |   4 +-
>  11 files changed, 293 insertions(+), 39 deletions(-)
> 
> diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
> index 003ea06..feebbf7 100644
> --- a/tools/libxc/xc_hvm_build_x86.c
> +++ b/tools/libxc/xc_hvm_build_x86.c
> @@ -46,7 +46,8 @@
>  #define SPECIALPAGE_IOREQ    5
>  #define SPECIALPAGE_IDENT_PT 6
>  #define SPECIALPAGE_CONSOLE  7
> -#define NR_SPECIAL_PAGES     8
> +#define SPECIALPAGE_VMPORT_REGS 8
> +#define NR_SPECIAL_PAGES     9
>  #define special_pfn(x) (0xff000u - NR_SPECIAL_PAGES + (x))
>  
>  #define NR_IOREQ_SERVER_PAGES 8
> @@ -610,6 +611,8 @@ static int setup_guest(xc_interface *xch,
>                       special_pfn(SPECIALPAGE_BUFIOREQ));
>      xc_hvm_param_set(xch, dom, HVM_PARAM_IOREQ_PFN,
>                       special_pfn(SPECIALPAGE_IOREQ));
> +    xc_hvm_param_set(xch, dom, HVM_PARAM_VMPORT_REGS_PFN,
> +                     special_pfn(SPECIALPAGE_VMPORT_REGS));
>      xc_hvm_param_set(xch, dom, HVM_PARAM_CONSOLE_PFN,
>                       special_pfn(SPECIALPAGE_CONSOLE));
>      xc_hvm_param_set(xch, dom, HVM_PARAM_PAGING_RING_PFN,
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 975996d..966ca46 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -831,6 +831,8 @@ static int libxl__build_device_model_args_new(libxl__gc 
> *gc,
>                                              machinearg, max_ram_below_4g);
>              }
>          }
> +        if (libxl_defbool_val(c_info->vmware_port))
> +            machinearg = GCSPRINTF("%s,vmport=on", machinearg);
>          flexarray_append(dm_args, machinearg);
>          for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
>              flexarray_append(dm_args, b_info->extra_hvm[i]);
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index fe5661d..51a97d5 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -169,33 +169,88 @@ static int hvmemul_do_io(
>              vio->io_state = HVMIO_handle_mmio_awaiting_completion;
>          break;
>      case X86EMUL_UNHANDLEABLE:
> -    {
> -        struct hvm_ioreq_server *s =
> -            hvm_select_ioreq_server(curr->domain, &p);
> -
> -        /* If there is no suitable backing DM, just ignore accesses */
> -        if ( !s )
> +        if ( vmport_check_port(p.addr, p.size) )

I would have made this !. Thought if Jan or Andrew said to make it that
way - then please ignore me.

That would mean you could also change the function to be 'is_port_vmport' or
such.

>          {
> -            hvm_complete_assist_req(&p);
> -            rc = X86EMUL_OKAY;
> -            vio->io_state = HVMIO_none;
> +            struct hvm_ioreq_server *s =
> +                hvm_select_ioreq_server(curr->domain, &p);
> +
> +            /* If there is no suitable backing DM, just ignore accesses */
> +            if ( !s )
> +            {
> +                hvm_complete_assist_req(&p);
> +                rc = X86EMUL_OKAY;
> +                vio->io_state = HVMIO_none;
> +            }
> +            else
> +            {
> +                rc = hvm_send_assist_req(s, &p);
> +                if ( rc != X86EMUL_RETRY )
> +                    vio->io_state = HVMIO_none;
> +                else if ( data_is_addr || dir == IOREQ_WRITE )
> +                    rc = X86EMUL_OKAY;
> +            }
>          }
>          else
>          {
> -            rc = hvm_send_assist_req(s, &p);
> -            if ( rc != X86EMUL_RETRY )
> -                vio->io_state = HVMIO_none;
> -            else if ( data_is_addr || dir == IOREQ_WRITE )
> +            struct hvm_ioreq_server *s;
> +            vmware_regs_t *vr;
> +
> +            BUILD_BUG_ON(sizeof(ioreq_t) < sizeof(vmware_regs_t));
> +
> +            p.type = IOREQ_TYPE_VMWARE_PORT;
> +            s = hvm_select_ioreq_server(curr->domain, &p);
> +            vr = get_vmport_regs_any(s, curr);
> +
> +            /*
> +             * If there is no suitable backing DM, just ignore accesses.  If
> +             * we do not have access to registers to pass to QEMU, just
> +             * ignore access.
> +             */
> +            if ( !s || !vr )
> +            {
> +                hvm_complete_assist_req(&p);
>                  rc = X86EMUL_OKAY;
> +                vio->io_state = HVMIO_none;
> +            }
> +            else
> +            {
> +                struct cpu_user_regs *regs = guest_cpu_user_regs();
> +
> +                p.data = regs->rax;
> +                vr->ebx = regs->_ebx;
> +                vr->ecx = regs->_ecx;
> +                vr->edx = regs->_edx;
> +                vr->esi = regs->_esi;
> +                vr->edi = regs->_edi;
> +
> +                vio->io_state = HVMIO_handle_pio_awaiting_completion;
> +                rc = hvm_send_assist_req(s, &p);
> +                if ( rc != X86EMUL_RETRY )
> +                {
> +                    vio->io_state = HVMIO_none;
> +                    if ( rc == X86EMUL_OKAY )
> +                        rc = X86EMUL_RETRY;
> +                }
> +            }
>          }
>          break;
> -    }
>      default:
>          BUG();
>      }
>  
>      if ( rc != X86EMUL_OKAY )
> +    {
> +        /*
> +         * If rc is X86EMUL_RETRY and vio->io_state is
> +         * HVMIO_handle_pio_awaiting_completion, then were are of

were are? Perhaps 'were'?
> +         * type IOREQ_TYPE_VMWARE_PORT, so completion in
> +         * hvm_io_assist() with no re-emulation required

.. is required.

> +         */
> +        if ( rc == X86EMUL_RETRY &&
> +             vio->io_state == HVMIO_handle_pio_awaiting_completion )
> +            rc = X86EMUL_OKAY;
>          return rc;
> +    }
>  
>   finish_access:
>      if ( dir == IOREQ_READ )
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index f8ec170..ce8cd09 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -394,6 +394,47 @@ static ioreq_t *get_ioreq(struct hvm_ioreq_server *s, 
> struct vcpu *v)
>      return &p->vcpu_ioreq[v->vcpu_id];
>  }
>  
> +static vmware_regs_t *get_vmport_regs_one(struct hvm_ioreq_server *s,
> +                                          struct vcpu *v)
> +{
> +    struct hvm_ioreq_vcpu *sv;
> +
> +    list_for_each_entry ( sv,
> +                          &s->ioreq_vcpu_list,
> +                          list_entry )

Could it be just made it one nice line?

.. snip..

> @@ -2507,6 +2637,13 @@ struct hvm_ioreq_server 
> *hvm_select_ioreq_server(struct domain *d,
>              }
>  
>              break;
> +        case IOREQ_TYPE_VMWARE_PORT:
> +        case IOREQ_TYPE_TIMEOFFSET:
> +            /* The 'special' range of [1,1] is checked for being enabled */

Missing full stop.

> +            if ( rangeset_contains_singleton(r, 1) )
> +                return s;
> +
> +            break;
>          }
>      }
>  
> @@ -2669,6 +2806,7 @@ void hvm_complete_assist_req(ioreq_t *p)
>      case IOREQ_TYPE_PCI_CONFIG:
>          ASSERT_UNREACHABLE();
>          break;
> +    case IOREQ_TYPE_VMWARE_PORT:
>      case IOREQ_TYPE_COPY:
>      case IOREQ_TYPE_PIO:
>          if ( p->dir == IOREQ_READ )
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index c0964ec..c1379bf 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -192,6 +192,22 @@ void hvm_io_assist(ioreq_t *p)
>          (void)handle_mmio();
>          break;
>      case HVMIO_handle_pio_awaiting_completion:
> +        if ( p->type == IOREQ_TYPE_VMWARE_PORT )
> +        {
> +            vmware_regs_t *vr = get_vmport_regs_any(NULL, curr);
> +
> +            if ( vr )
> +            {
> +                struct cpu_user_regs *regs = guest_cpu_user_regs();
> +
> +                /* Only change the 32bit part of the register */

Missing full stop.

> +                regs->_ebx = vr->ebx;
> +                regs->_ecx = vr->ecx;
> +                regs->_edx = vr->edx;
> +                regs->_esi = vr->esi;
> +                regs->_edi = vr->edi;
> +            }
> +        }
>          if ( vio->io_size == 4 ) /* Needs zero extension. */
>              guest_cpu_user_regs()->rax = (uint32_t)p->data;
>          else
> diff --git a/xen/arch/x86/hvm/vmware/vmport.c 
> b/xen/arch/x86/hvm/vmware/vmport.c
> index f24d8e3..5e14402 100644
> --- a/xen/arch/x86/hvm/vmware/vmport.c
> +++ b/xen/arch/x86/hvm/vmware/vmport.c
> @@ -137,6 +137,19 @@ void vmport_register(struct domain *d)
>      register_portio_handler(d, BDOOR_PORT, 4, vmport_ioport);
>  }
>  
> +int vmport_check_port(unsigned int port, unsigned int bytes)
> +{
> +    struct vcpu *curr = current;
> +    struct domain *currd = curr->domain;
> +
> +    if ( port + bytes > BDOOR_PORT && port < BDOOR_PORT + 4 &&
> +         is_hvm_domain(currd) &&
> +         currd->arch.hvm_domain.is_vmware_port_enabled ) {

The '{' should be on its own line.

> +        return 0;
> +    }
> +    return 1;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-x86/hvm/domain.h 
> b/xen/include/asm-x86/hvm/domain.h
> index 726b977..a6c312f 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -48,7 +48,7 @@ struct hvm_ioreq_vcpu {
>      evtchn_port_t    ioreq_evtchn;
>  };
>  
> -#define NR_IO_RANGE_TYPES (HVMOP_IO_RANGE_PCI + 1)
> +#define NR_IO_RANGE_TYPES (HVMOP_IO_RANGE_VMWARE_PORT + 1)
>  #define MAX_NR_IO_RANGES  256
>  
>  struct hvm_ioreq_server {
> @@ -63,6 +63,7 @@ struct hvm_ioreq_server {
>      ioservid_t             id;
>      struct hvm_ioreq_page  ioreq;
>      struct list_head       ioreq_vcpu_list;
> +    struct hvm_ioreq_page  vmport_ioreq;
>      struct hvm_ioreq_page  bufioreq;
>  
>      /* Lock to serialize access to buffered ioreq ring */
> diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
> index 394c230..78752b4 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -523,6 +523,8 @@ extern bool_t opt_hvm_fep;
>  #endif
>  
>  void vmport_register(struct domain *d);
> +int vmport_check_port(unsigned int port, unsigned int bytes);
> +vmware_regs_t *get_vmport_regs_any(struct hvm_ioreq_server *s, struct vcpu 
> *v);
>  
>  #endif /* __ASM_X86_HVM_HVM_H__ */
>  
> diff --git a/xen/include/public/hvm/hvm_op.h b/xen/include/public/hvm/hvm_op.h
> index 9b84e84..4d10eee 100644
> --- a/xen/include/public/hvm/hvm_op.h
> +++ b/xen/include/public/hvm/hvm_op.h
> @@ -321,6 +321,9 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_get_ioreq_server_info_t);
>   *
>   * NOTE: unless an emulation request falls entirely within a range mapped
>   * by a secondary emulator, it will not be passed to that emulator.
> + *
> + * NOTE: The 'special' range of [1,1] is what is checked for on
> + * TIMEOFFSET and VMWARE_PORT.
>   */
>  #define HVMOP_map_io_range_to_ioreq_server 19
>  #define HVMOP_unmap_io_range_from_ioreq_server 20
> @@ -331,6 +334,8 @@ struct xen_hvm_io_range {
>  # define HVMOP_IO_RANGE_PORT   0 /* I/O port range */
>  # define HVMOP_IO_RANGE_MEMORY 1 /* MMIO range */
>  # define HVMOP_IO_RANGE_PCI    2 /* PCI segment/bus/dev/func range */
> +# define HVMOP_IO_RANGE_TIMEOFFSET 7 /* TIMEOFFSET special range */
> +# define HVMOP_IO_RANGE_VMWARE_PORT 9 /* VMware port special range */
>      uint64_aligned_t start, end; /* IN - inclusive start and end of range */
>  };
>  typedef struct xen_hvm_io_range xen_hvm_io_range_t;
> diff --git a/xen/include/public/hvm/ioreq.h b/xen/include/public/hvm/ioreq.h
> index 2e5809b..2f326cf 100644
> --- a/xen/include/public/hvm/ioreq.h
> +++ b/xen/include/public/hvm/ioreq.h
> @@ -37,6 +37,7 @@
>  #define IOREQ_TYPE_PCI_CONFIG   2
>  #define IOREQ_TYPE_TIMEOFFSET   7
>  #define IOREQ_TYPE_INVALIDATE   8 /* mapcache */
> +#define IOREQ_TYPE_VMWARE_PORT  9 /* pio + vmport registers */
>  
>  /*
>   * VMExit dispatcher should cooperate with instruction decoder to
> @@ -48,6 +49,8 @@
>   * 
>   * 63....48|47..40|39..35|34..32|31........0
>   * SEGMENT |BUS   |DEV   |FN    |OFFSET
> + *
> + * For I/O type IOREQ_TYPE_VMWARE_PORT also use the vmware_regs.
>   */
>  struct ioreq {
>      uint64_t addr;          /* physical address */
> @@ -66,11 +69,25 @@ struct ioreq {
>  };
>  typedef struct ioreq ioreq_t;
>  
> +struct vmware_regs {
> +    uint32_t esi;
> +    uint32_t edi;
> +    uint32_t ebx;
> +    uint32_t ecx;
> +    uint32_t edx;
> +};
> +typedef struct vmware_regs vmware_regs_t;
> +
>  struct shared_iopage {
>      struct ioreq vcpu_ioreq[1];
>  };
>  typedef struct shared_iopage shared_iopage_t;
>  
> +struct shared_vmport_iopage {
> +    struct vmware_regs vcpu_vmport_regs[1];
> +};
> +typedef struct shared_vmport_iopage shared_vmport_iopage_t;
> +
>  struct buf_ioreq {
>      uint8_t  type;   /* I/O type                    */
>      uint8_t  pad:1;
> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
> index 7c73089..130eba9 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -50,6 +50,8 @@
>  #define HVM_PARAM_PAE_ENABLED  4
>  
>  #define HVM_PARAM_IOREQ_PFN    5
> +/* Extra vmport PFN. */

s/Extra/optional/ ?

> +#define HVM_PARAM_VMPORT_REGS_PFN 35
>  
>  #define HVM_PARAM_BUFIOREQ_PFN 6
>  #define HVM_PARAM_BUFIOREQ_EVTCHN 26
> @@ -187,6 +189,6 @@
>  /* Location of the VM Generation ID in guest physical address space. */
>  #define HVM_PARAM_VM_GENERATION_ID_ADDR 34
>  
> -#define HVM_NR_PARAMS          35
> +#define HVM_NR_PARAMS          36
>  
>  #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
> -- 
> 1.8.3.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.