[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH QEMU-XEN v6 4/8] xen: Switch uses of xc_map_fore
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH QEMU-XEN v6 4/8] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_pages |
Date: |
Fri, 4 Dec 2015 15:26:22 +0000 |
User-agent: |
Alpine 2.02 (DEB 1266 2009-07-14) |
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 <address@hidden>
Reviewed-by: Stefano Stabellini <address@hidden>
> 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
>
- [Qemu-devel] [Minios-devel] [PATCH v6 0/<VARIOUS>] Begin to disentangle libxenctrl and provide some stable libraries, Ian Campbell, 2015/12/03
- [Qemu-devel] [PATCH QEMU-XEN v6 0/8] Begin to disentangle libxenctrl and provide some stable libraries, Ian Campbell, 2015/12/03
- [Qemu-devel] [PATCH QEMU-XEN v6 1/8] xen_console: correctly cleanup primary console on teardown., Ian Campbell, 2015/12/03
- [Qemu-devel] [PATCH QEMU-XEN v6 2/8] xen: Switch to libxenevtchn interface for compat shims., Ian Campbell, 2015/12/03
- [Qemu-devel] [PATCH QEMU-XEN v6 7/8] xen: domainbuild: reopen libxenctrl interface after forking for domain watcher., Ian Campbell, 2015/12/03
- [Qemu-devel] [PATCH QEMU-XEN v6 3/8] xen: Switch to libxengnttab interface for compat shims., Ian Campbell, 2015/12/03
- [Qemu-devel] [PATCH QEMU-XEN v6 6/8] xen: Use stable library interfaces when they are available., Ian Campbell, 2015/12/03
- [Qemu-devel] [PATCH QEMU-XEN v6 4/8] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_pages, Ian Campbell, 2015/12/03
- Re: [Qemu-devel] [PATCH QEMU-XEN v6 4/8] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_pages, Stefano Stabellini, 2015/12/11
- Re: [Qemu-devel] [PATCH QEMU-XEN v6 4/8] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_pages, Ian Campbell, 2015/12/11
- Re: [Qemu-devel] [PATCH QEMU-XEN v6 4/8] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_pages, Stefano Stabellini, 2015/12/11
[Qemu-devel] [PATCH QEMU-XEN v6 5/8] xen: Switch uses of xc_map_foreign_{pages, bulk} to use libxenforeignmemory API., Ian Campbell, 2015/12/03
[Qemu-devel] [PATCH QEMU-XEN v6 8/8] xen: make it possible to build without the Xen PV domain builder, Ian Campbell, 2015/12/03
Re: [Qemu-devel] [Minios-devel] [PATCH v6 0/<VARIOUS>] Begin to disentangle libxenctrl and provide some stable libraries, Ian Campbell, 2015/12/09