Re: [Xen-devel] [PATCH QEMU-XEN v6 4/8] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_pages

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 3 Dec 2015, Ian Campbell wrote:
> In Xen 4.7 we are refactoring parts libxenctrl into a number of
> separate libraries which will provide backward and forward API and ABI
> compatiblity.
> 
> One such library will be libxenforeignmemory which provides access to
> privileged foreign mappings and which will provide an interface
> equivalent to xc_map_foreign_{pages,bulk}.
> 
> In preparation for this switch all uses of xc_map_foreign_range to
> xc_map_foreign_pages. This is trivial because size was always
> XC_PAGE_SIZE so the necessary adjustments are trivial:
> 
>   * Pass &mfn (an array of length 1) instead of mfn. The function
>     takes a pointer to const, so there is no possibily of mfn changing
>     due to this change.
>   * Pass nr_pages=1 instead of size=XC_PAGE_SIZE
> 
> There is one wrinkle in xen_console.c:con_initialise() where
> con->ring_ref is an int but can in some code paths (when !xendev->dev)
> be treated as an mfn. I think this is an existing latent truncation
> hazard on platforms where xen_pfn_t is 64-bit and int is 32-bit (e.g.
> amd64, both arm* variants). I'm unsure under what circumstances
> xendev->dev can be NULL or if anything elsewhere ensures the value
> fits into an int. For now I just use a temporary xen_pfn_t to in
> effect upcast the pointer from int* to xen_pfn_t*.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Reviewed-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>


> v6: Switch to xc_map_foreign_pages rather than _bulk. Switching to
>     _bulk without checking the value of err[0] risked missing errors.
>     The new xenforeignmemory_map coming later in this series will
>     DTRT with err==NULL.
> 
>     Dropped Stefano's Reviewed-by due to this change.
> 
> v4: Ran checkpatch, fixed all errors
>     Mention the const-ness of the mfn array in the commit log
> ---
>  hw/char/xen_console.c |  8 ++++----
>  hw/display/xenfb.c    |  5 ++---
>  xen-hvm.c             | 14 +++++++-------
>  3 files changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index 8c06c19..56f1b1a 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -228,10 +228,10 @@ static int con_initialise(struct XenDevice *xendev)
>       con->buffer.max_capacity = limit;
>  
>      if (!xendev->dev) {
> -        con->sring = xc_map_foreign_range(xen_xc, con->xendev.dom,
> -                                          XC_PAGE_SIZE,
> -                                          PROT_READ|PROT_WRITE,
> -                                          con->ring_ref);
> +        xen_pfn_t mfn = con->ring_ref;
> +        con->sring = xc_map_foreign_pages(xen_xc, con->xendev.dom,
> +                                         PROT_READ|PROT_WRITE,
> +                                         &mfn, 1);
>      } else {
>          con->sring = xengnttab_map_grant_ref(xendev->gnttabdev, 
> con->xendev.dom,
>                                               con->ring_ref,
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 5e324ef..c96d974 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -104,9 +104,8 @@ static int common_bind(struct common *c)
>      if (xenstore_read_fe_int(&c->xendev, "event-channel", 
> &c->xendev.remote_port) == -1)
>       return -1;
>  
> -    c->page = xc_map_foreign_range(xen_xc, c->xendev.dom,
> -                                XC_PAGE_SIZE,
> -                                PROT_READ | PROT_WRITE, mfn);
> +    c->page = xc_map_foreign_pages(xen_xc, c->xendev.dom,
> +                                   PROT_READ | PROT_WRITE, &mfn, 1);
>      if (c->page == NULL)
>       return -1;
>  
> diff --git a/xen-hvm.c b/xen-hvm.c
> index 6680782..2f4e109 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -1240,8 +1240,9 @@ int xen_hvm_init(PCMachineState *pcms,
>      DPRINTF("buffered io page at pfn %lx\n", bufioreq_pfn);
>      DPRINTF("buffered io evtchn is %x\n", bufioreq_evtchn);
>  
> -    state->shared_page = xc_map_foreign_range(xen_xc, xen_domid, 
> XC_PAGE_SIZE,
> -                                              PROT_READ|PROT_WRITE, 
> ioreq_pfn);
> +    state->shared_page = xc_map_foreign_pages(xen_xc, xen_domid,
> +                                              PROT_READ|PROT_WRITE,
> +                                              &ioreq_pfn, 1);
>      if (state->shared_page == NULL) {
>          hw_error("map shared IO page returned error %d handle=" 
> XC_INTERFACE_FMT,
>                   errno, xen_xc);
> @@ -1251,8 +1252,8 @@ int xen_hvm_init(PCMachineState *pcms,
>      if (!rc) {
>          DPRINTF("shared vmport page at pfn %lx\n", ioreq_pfn);
>          state->shared_vmport_page =
> -            xc_map_foreign_range(xen_xc, xen_domid, XC_PAGE_SIZE,
> -                                 PROT_READ|PROT_WRITE, ioreq_pfn);
> +            xc_map_foreign_pages(xen_xc, xen_domid, PROT_READ|PROT_WRITE,
> +                                 &ioreq_pfn, 1);
>          if (state->shared_vmport_page == NULL) {
>              hw_error("map shared vmport IO page returned error %d handle="
>                       XC_INTERFACE_FMT, errno, xen_xc);
> @@ -1261,10 +1262,9 @@ int xen_hvm_init(PCMachineState *pcms,
>          hw_error("get vmport regs pfn returned error %d, rc=%d", errno, rc);
>      }
>  
> -    state->buffered_io_page = xc_map_foreign_range(xen_xc, xen_domid,
> -                                                   XC_PAGE_SIZE,
> +    state->buffered_io_page = xc_map_foreign_pages(xen_xc, xen_domid,
>                                                     PROT_READ|PROT_WRITE,
> -                                                   bufioreq_pfn);
> +                                                   &bufioreq_pfn, 1);
>      if (state->buffered_io_page == NULL) {
>          hw_error("map buffered IO page returned error %d", errno);
>      }
> -- 
> 2.1.4
> 

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