From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Paul Durrant <paul.durrant@citrix.com>
Cc: xen-devel@lists.xen.org
Subject: Re: [RFC PATCH 3/5] ioreq-server: on-demand creation of ioreq server
Date: Thu, 30 Jan 2014 15:21:50 +0000 [thread overview]
Message-ID: <52EA6E0E.8020200@citrix.com> (raw)
In-Reply-To: <1391091590-5454-4-git-send-email-paul.durrant@citrix.com>
On 30/01/14 14:19, Paul Durrant wrote:
> This patch only creates the ioreq server when the legacy HVM parameters
> are touched by an emulator. It also lays some groundwork for supporting
> multiple IOREQ servers. For instance, it introduces ioreq server reference
> counting which is not strictly necessary at this stage but will become so
> when ioreq servers can be destroyed prior the domain dying.
>
> There is a significant change in the layout of the special pages reserved
> in xc_hvm_build_x86.c. This is so that we can 'grow' them downwards without
> moving pages such as the xenstore page when building a domain that can
> support more than one emulator.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> tools/libxc/xc_hvm_build_x86.c | 41 ++--
> xen/arch/x86/hvm/hvm.c | 409 ++++++++++++++++++++++++++------------
> xen/include/asm-x86/hvm/domain.h | 3 +-
> 3 files changed, 314 insertions(+), 139 deletions(-)
>
> diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
> index 77bd365..f24f2a1 100644
> --- a/tools/libxc/xc_hvm_build_x86.c
> +++ b/tools/libxc/xc_hvm_build_x86.c
> @@ -41,13 +41,12 @@
> #define SPECIALPAGE_PAGING 0
> #define SPECIALPAGE_ACCESS 1
> #define SPECIALPAGE_SHARING 2
> -#define SPECIALPAGE_BUFIOREQ 3
> -#define SPECIALPAGE_XENSTORE 4
> -#define SPECIALPAGE_IOREQ 5
> -#define SPECIALPAGE_IDENT_PT 6
> -#define SPECIALPAGE_CONSOLE 7
> -#define NR_SPECIAL_PAGES 8
> -#define special_pfn(x) (0xff000u - NR_SPECIAL_PAGES + (x))
> +#define SPECIALPAGE_XENSTORE 3
> +#define SPECIALPAGE_IDENT_PT 4
> +#define SPECIALPAGE_CONSOLE 5
> +#define SPECIALPAGE_IOREQ 6
> +#define NR_SPECIAL_PAGES SPECIALPAGE_IOREQ + 2 /* ioreq server needs 2 pages */
> +#define special_pfn(x) (0xff000u - (x))
>
> static int modules_init(struct xc_hvm_build_args *args,
> uint64_t vend, struct elf_binary *elf,
> @@ -112,7 +111,7 @@ static void build_hvm_info(void *hvm_info_page, uint64_t mem_size,
> /* Memory parameters. */
> hvm_info->low_mem_pgend = lowmem_end >> PAGE_SHIFT;
> hvm_info->high_mem_pgend = highmem_end >> PAGE_SHIFT;
> - hvm_info->reserved_mem_pgstart = special_pfn(0);
> + hvm_info->reserved_mem_pgstart = special_pfn(0) - NR_SPECIAL_PAGES;
>
> /* Finish with the checksum. */
> for ( i = 0, sum = 0; i < hvm_info->length; i++ )
> @@ -463,6 +462,24 @@ static int setup_guest(xc_interface *xch,
> munmap(hvm_info_page, PAGE_SIZE);
>
> /* Allocate and clear special pages. */
> +
> + DPRINTF("%d SPECIAL PAGES:\n"
> + " PAGING: %"PRI_xen_pfn"\n"
> + " ACCESS: %"PRI_xen_pfn"\n"
> + " SHARING: %"PRI_xen_pfn"\n"
> + " STORE: %"PRI_xen_pfn"\n"
> + " IDENT_PT: %"PRI_xen_pfn"\n"
> + " CONSOLE: %"PRI_xen_pfn"\n"
> + " IOREQ: %"PRI_xen_pfn"\n",
> + NR_SPECIAL_PAGES,
> + (xen_pfn_t)special_pfn(SPECIALPAGE_PAGING),
> + (xen_pfn_t)special_pfn(SPECIALPAGE_ACCESS),
> + (xen_pfn_t)special_pfn(SPECIALPAGE_SHARING),
> + (xen_pfn_t)special_pfn(SPECIALPAGE_XENSTORE),
> + (xen_pfn_t)special_pfn(SPECIALPAGE_IDENT_PT),
> + (xen_pfn_t)special_pfn(SPECIALPAGE_CONSOLE),
> + (xen_pfn_t)special_pfn(SPECIALPAGE_IOREQ));
> +
I realise I am being quite picky here, but from a daemon point of view
trying to log to facilities like syslog, multi-line single debugging
messages are a pain. Would it be possible to do this as 8 DPRINTF()s?
> for ( i = 0; i < NR_SPECIAL_PAGES; i++ )
> {
> xen_pfn_t pfn = special_pfn(i);
> @@ -478,10 +495,6 @@ static int setup_guest(xc_interface *xch,
>
> xc_set_hvm_param(xch, dom, HVM_PARAM_STORE_PFN,
> special_pfn(SPECIALPAGE_XENSTORE));
> - xc_set_hvm_param(xch, dom, HVM_PARAM_BUFIOREQ_PFN,
> - special_pfn(SPECIALPAGE_BUFIOREQ));
> - xc_set_hvm_param(xch, dom, HVM_PARAM_IOREQ_PFN,
> - special_pfn(SPECIALPAGE_IOREQ));
> xc_set_hvm_param(xch, dom, HVM_PARAM_CONSOLE_PFN,
> special_pfn(SPECIALPAGE_CONSOLE));
> xc_set_hvm_param(xch, dom, HVM_PARAM_PAGING_RING_PFN,
> @@ -490,6 +503,10 @@ static int setup_guest(xc_interface *xch,
> special_pfn(SPECIALPAGE_ACCESS));
> xc_set_hvm_param(xch, dom, HVM_PARAM_SHARING_RING_PFN,
> special_pfn(SPECIALPAGE_SHARING));
> + xc_set_hvm_param(xch, dom, HVM_PARAM_IOREQ_PFN,
> + special_pfn(SPECIALPAGE_IOREQ));
> + xc_set_hvm_param(xch, dom, HVM_PARAM_BUFIOREQ_PFN,
> + special_pfn(SPECIALPAGE_IOREQ) - 1);
>
> /*
> * Identity-map page table is required for running with CR0.PG=0 when
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index a0eaadb..d9874fb 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -352,24 +352,9 @@ static ioreq_t *get_ioreq(struct hvm_ioreq_server *s, int id)
> return &p->vcpu_ioreq[id];
> }
>
> -void hvm_do_resume(struct vcpu *v)
> +static void hvm_wait_on_io(struct domain *d, ioreq_t *p)
> {
> - struct hvm_ioreq_server *s;
> - ioreq_t *p;
> -
> - check_wakeup_from_wait();
> -
> - if ( is_hvm_vcpu(v) )
> - pt_restore_timer(v);
> -
> - s = v->arch.hvm_vcpu.ioreq_server;
> - v->arch.hvm_vcpu.ioreq_server = NULL;
> -
> - if ( !s )
> - goto check_inject_trap;
> -
> /* NB. Optimised for common case (p->state == STATE_IOREQ_NONE). */
> - p = get_ioreq(s, v->vcpu_id);
> while ( p->state != STATE_IOREQ_NONE )
> {
> switch ( p->state )
> @@ -385,12 +370,32 @@ void hvm_do_resume(struct vcpu *v)
> break;
> default:
> gdprintk(XENLOG_ERR, "Weird HVM iorequest state %d.\n", p->state);
> - domain_crash(v->domain);
> + domain_crash(d);
> return; /* bail */
> }
> }
> +}
> +
> +void hvm_do_resume(struct vcpu *v)
> +{
> + struct domain *d = v->domain;
> + struct hvm_ioreq_server *s;
> +
> + check_wakeup_from_wait();
> +
> + if ( is_hvm_vcpu(v) )
> + pt_restore_timer(v);
> +
> + s = v->arch.hvm_vcpu.ioreq_server;
> + v->arch.hvm_vcpu.ioreq_server = NULL;
> +
> + if ( s )
> + {
> + ioreq_t *p = get_ioreq(s, v->vcpu_id);
> +
> + hvm_wait_on_io(d, p);
> + }
>
> - check_inject_trap:
> /* Inject pending hw/sw trap */
> if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
> {
> @@ -399,11 +404,13 @@ void hvm_do_resume(struct vcpu *v)
> }
> }
>
> -static void hvm_init_ioreq_page(
> - struct domain *d, struct hvm_ioreq_page *iorp)
> +static void hvm_init_ioreq_page(struct hvm_ioreq_server *s, int buf)
> {
> + struct hvm_ioreq_page *iorp;
> +
> + iorp = ( buf ) ? &s->buf_ioreq : &s->ioreq;
> +
Brackets are redundant.
> spin_lock_init(&iorp->lock);
> - domain_pause(d);
> }
>
> void destroy_ring_for_helper(
> @@ -419,16 +426,13 @@ void destroy_ring_for_helper(
> }
> }
>
> -static void hvm_destroy_ioreq_page(
> - struct domain *d, struct hvm_ioreq_page *iorp)
> +static void hvm_destroy_ioreq_page(struct hvm_ioreq_server *s, int buf)
> {
> - spin_lock(&iorp->lock);
> + struct hvm_ioreq_page *iorp;
>
> - ASSERT(d->is_dying);
> + iorp = ( buf ) ? &s->buf_ioreq : &s->ioreq;
>
> destroy_ring_for_helper(&iorp->va, iorp->page);
> -
> - spin_unlock(&iorp->lock);
> }
>
> int prepare_ring_for_helper(
> @@ -476,8 +480,10 @@ int prepare_ring_for_helper(
> }
>
> static int hvm_set_ioreq_page(
> - struct domain *d, struct hvm_ioreq_page *iorp, unsigned long gmfn)
> + struct hvm_ioreq_server *s, int buf, unsigned long gmfn)
> {
> + struct domain *d = s->domain;
> + struct hvm_ioreq_page *iorp;
> struct page_info *page;
> void *va;
> int rc;
> @@ -485,22 +491,17 @@ static int hvm_set_ioreq_page(
> if ( (rc = prepare_ring_for_helper(d, gmfn, &page, &va)) )
> return rc;
>
> - spin_lock(&iorp->lock);
> + iorp = ( buf ) ? &s->buf_ioreq : &s->ioreq;
>
> if ( (iorp->va != NULL) || d->is_dying )
> {
> - destroy_ring_for_helper(&iorp->va, iorp->page);
> - spin_unlock(&iorp->lock);
> + destroy_ring_for_helper(&va, page);
> return -EINVAL;
> }
>
> iorp->va = va;
> iorp->page = page;
>
> - spin_unlock(&iorp->lock);
> -
> - domain_unpause(d);
> -
> return 0;
> }
>
> @@ -544,38 +545,6 @@ static int handle_pvh_io(
> return X86EMUL_OKAY;
> }
>
> -static int hvm_init_ioreq_server(struct domain *d)
> -{
> - struct hvm_ioreq_server *s;
> - int i;
> -
> - s = xzalloc(struct hvm_ioreq_server);
> - if ( !s )
> - return -ENOMEM;
> -
> - s->domain = d;
> -
> - for ( i = 0; i < MAX_HVM_VCPUS; i++ )
> - s->ioreq_evtchn[i] = -1;
> - s->buf_ioreq_evtchn = -1;
> -
> - hvm_init_ioreq_page(d, &s->ioreq);
> - hvm_init_ioreq_page(d, &s->buf_ioreq);
> -
> - d->arch.hvm_domain.ioreq_server = s;
> - return 0;
> -}
> -
> -static void hvm_deinit_ioreq_server(struct domain *d)
> -{
> - struct hvm_ioreq_server *s = d->arch.hvm_domain.ioreq_server;
> -
> - hvm_destroy_ioreq_page(d, &s->ioreq);
> - hvm_destroy_ioreq_page(d, &s->buf_ioreq);
> -
> - xfree(s);
> -}
> -
> static void hvm_update_ioreq_server_evtchn(struct hvm_ioreq_server *s)
> {
> struct domain *d = s->domain;
> @@ -637,6 +606,152 @@ static void hvm_ioreq_server_remove_vcpu(struct hvm_ioreq_server *s, struct vcpu
> }
> }
>
> +static int hvm_create_ioreq_server(struct domain *d, domid_t domid)
> +{
> + struct hvm_ioreq_server *s;
> + int i;
> + unsigned long pfn;
> + struct vcpu *v;
> + int rc;
i and rc can be declared together.
> +
> + spin_lock(&d->arch.hvm_domain.ioreq_server_lock);
> +
> + rc = -EEXIST;
> + if ( d->arch.hvm_domain.ioreq_server != NULL )
> + goto fail_exist;
> +
> + gdprintk(XENLOG_INFO, "%s: %d\n", __func__, d->domain_id);
> +
> + rc = -ENOMEM;
> + s = xzalloc(struct hvm_ioreq_server);
> + if ( !s )
> + goto fail_alloc;
> +
> + s->domain = d;
> + s->domid = domid;
> +
> + for ( i = 0; i < MAX_HVM_VCPUS; i++ )
> + s->ioreq_evtchn[i] = -1;
> + s->buf_ioreq_evtchn = -1;
> +
> + /* Initialize shared pages */
> + pfn = d->arch.hvm_domain.params[HVM_PARAM_IOREQ_PFN];
> +
> + hvm_init_ioreq_page(s, 0);
> + if ( (rc = hvm_set_ioreq_page(s, 0, pfn)) < 0 )
> + goto fail_set_ioreq;
> +
> + pfn = d->arch.hvm_domain.params[HVM_PARAM_BUFIOREQ_PFN];
> +
> + hvm_init_ioreq_page(s, 1);
> + if ( (rc = hvm_set_ioreq_page(s, 1, pfn)) < 0 )
> + goto fail_set_buf_ioreq;
> +
> + for_each_vcpu ( d, v )
> + {
> + if ( (rc = hvm_ioreq_server_add_vcpu(s, v)) < 0 )
> + goto fail_add_vcpu;
> + }
> +
> + d->arch.hvm_domain.ioreq_server = s;
> +
> + spin_unlock(&d->arch.hvm_domain.ioreq_server_lock);
> +
> + return 0;
> +
> +fail_add_vcpu:
> + for_each_vcpu ( d, v )
> + hvm_ioreq_server_remove_vcpu(s, v);
> + hvm_destroy_ioreq_page(s, 1);
> +fail_set_buf_ioreq:
> + hvm_destroy_ioreq_page(s, 0);
> +fail_set_ioreq:
> + xfree(s);
> +fail_alloc:
> +fail_exist:
> + spin_unlock(&d->arch.hvm_domain.ioreq_server_lock);
> + return rc;
> +}
> +
> +static void hvm_destroy_ioreq_server(struct domain *d)
> +{
> + struct hvm_ioreq_server *s;
> + struct vcpu *v;
> +
> + spin_lock(&d->arch.hvm_domain.ioreq_server_lock);
> +
> + gdprintk(XENLOG_INFO, "%s: %d\n", __func__, d->domain_id);
> +
> + s = d->arch.hvm_domain.ioreq_server;
> + if ( !s )
> + goto done;
> +
> + domain_pause(d);
> +
> + d->arch.hvm_domain.ioreq_server = NULL;
> +
> + for_each_vcpu ( d, v )
> + hvm_ioreq_server_remove_vcpu(s, v);
> +
> + hvm_destroy_ioreq_page(s, 1);
> + hvm_destroy_ioreq_page(s, 0);
> +
> + xfree(s);
> +
> + domain_unpause(d);
> +
> +done:
> + spin_unlock(&d->arch.hvm_domain.ioreq_server_lock);
> +}
> +
> +static int hvm_get_ioreq_server_buf_port(struct domain *d, evtchn_port_t *port)
> +{
> + struct hvm_ioreq_server *s;
> + int rc;
> +
> + spin_lock(&d->arch.hvm_domain.ioreq_server_lock);
> +
> + s = d->arch.hvm_domain.ioreq_server;
> +
> + rc = -ENOENT;
> + if ( !s )
> + goto done;
> +
> + *port = s->buf_ioreq_evtchn;
> + rc = 0;
> +
> +done:
> + spin_unlock(&d->arch.hvm_domain.ioreq_server_lock);
> +
> + return rc;
> +}
> +
> +static int hvm_get_ioreq_server_pfn(struct domain *d, int buf, xen_pfn_t *pfn)
> +{
> + struct hvm_ioreq_server *s;
> + int rc;
> +
> + spin_lock(&d->arch.hvm_domain.ioreq_server_lock);
> +
> + s = d->arch.hvm_domain.ioreq_server;
> +
> + rc = -ENOENT;
> + if ( !s )
> + goto done;
> +
> + if ( buf )
> + *pfn = d->arch.hvm_domain.params[HVM_PARAM_BUFIOREQ_PFN];
> + else
> + *pfn = d->arch.hvm_domain.params[HVM_PARAM_IOREQ_PFN];
This can be reduced and use "params[buf ? HVM_PARAM_BUFIOREQ_PFN :
HVM_PARAM_IOREQ_PFN]", although that is perhaps not as clear.
> +
> + rc = 0;
> +
> +done:
> + spin_unlock(&d->arch.hvm_domain.ioreq_server_lock);
> +
> + return rc;
> +}
> +
> static int hvm_replace_event_channel(struct vcpu *v, domid_t remote_domid,
> int *p_port)
> {
> @@ -652,13 +767,24 @@ static int hvm_replace_event_channel(struct vcpu *v, domid_t remote_domid,
> return 0;
> }
>
> -static int hvm_set_ioreq_server_domid(struct hvm_ioreq_server *s, domid_t domid)
> +static int hvm_set_ioreq_server_domid(struct domain *d, domid_t domid)
> {
> - struct domain *d = s->domain;
> + struct hvm_ioreq_server *s;
> struct vcpu *v;
> int rc = 0;
>
> domain_pause(d);
> + spin_lock(&d->arch.hvm_domain.ioreq_server_lock);
> +
> + s = d->arch.hvm_domain.ioreq_server;
> +
> + rc = -ENOENT;
> + if ( !s )
> + goto done;
> +
> + rc = 0;
> + if ( s->domid == domid )
> + goto done;
>
> if ( d->vcpu[0] )
> {
> @@ -680,31 +806,11 @@ static int hvm_set_ioreq_server_domid(struct hvm_ioreq_server *s, domid_t domid)
>
> done:
> domain_unpause(d);
> + spin_unlock(&d->arch.hvm_domain.ioreq_server_lock);
Mismatched order of pause/unpause and lock/unlock pairs. The unlock
should ideally be before the unpause.
>
> return rc;
> }
>
> -static int hvm_set_ioreq_server_pfn(struct hvm_ioreq_server *s, unsigned long pfn)
> -{
> - struct domain *d = s->domain;
> - int rc;
> -
> - rc = hvm_set_ioreq_page(d, &s->ioreq, pfn);
> - if ( rc < 0 )
> - return rc;
> -
> - hvm_update_ioreq_server_evtchn(s);
> -
> - return 0;
> -}
> -
> -static int hvm_set_ioreq_server_buf_pfn(struct hvm_ioreq_server *s, unsigned long pfn)
> -{
> - struct domain *d = s->domain;
> -
> - return hvm_set_ioreq_page(d, &s->buf_ioreq, pfn);
> -}
> -
> int hvm_domain_initialise(struct domain *d)
> {
> int rc;
> @@ -732,6 +838,7 @@ int hvm_domain_initialise(struct domain *d)
>
> }
>
> + spin_lock_init(&d->arch.hvm_domain.ioreq_server_lock);
> spin_lock_init(&d->arch.hvm_domain.irq_lock);
> spin_lock_init(&d->arch.hvm_domain.uc_lock);
>
> @@ -772,20 +879,14 @@ int hvm_domain_initialise(struct domain *d)
>
> rtc_init(d);
>
> - rc = hvm_init_ioreq_server(d);
> - if ( rc != 0 )
> - goto fail2;
> -
> register_portio_handler(d, 0xe9, 1, hvm_print_line);
>
> rc = hvm_funcs.domain_initialise(d);
> if ( rc != 0 )
> - goto fail3;
> + goto fail2;
>
> return 0;
>
> - fail3:
> - hvm_deinit_ioreq_server(d);
> fail2:
> rtc_deinit(d);
> stdvga_deinit(d);
> @@ -809,7 +910,7 @@ void hvm_domain_relinquish_resources(struct domain *d)
> if ( hvm_funcs.nhvm_domain_relinquish_resources )
> hvm_funcs.nhvm_domain_relinquish_resources(d);
>
> - hvm_deinit_ioreq_server(d);
> + hvm_destroy_ioreq_server(d);
>
> msixtbl_pt_cleanup(d);
>
> @@ -1364,11 +1465,16 @@ int hvm_vcpu_initialise(struct vcpu *v)
> && (rc = nestedhvm_vcpu_initialise(v)) < 0 ) /* teardown: nestedhvm_vcpu_destroy */
> goto fail5;
>
> + spin_lock(&d->arch.hvm_domain.ioreq_server_lock);
> s = d->arch.hvm_domain.ioreq_server;
> + spin_unlock(&d->arch.hvm_domain.ioreq_server_lock);
>
> - rc = hvm_ioreq_server_add_vcpu(s, v);
> - if ( rc < 0 )
> - goto fail6;
> + if ( s )
> + {
> + rc = hvm_ioreq_server_add_vcpu(s, v);
> + if ( rc < 0 )
> + goto fail6;
> + }
>
> if ( v->vcpu_id == 0 )
> {
> @@ -1404,9 +1510,14 @@ int hvm_vcpu_initialise(struct vcpu *v)
> void hvm_vcpu_destroy(struct vcpu *v)
> {
> struct domain *d = v->domain;
> - struct hvm_ioreq_server *s = d->arch.hvm_domain.ioreq_server;
> + struct hvm_ioreq_server *s;
> +
> + spin_lock(&d->arch.hvm_domain.ioreq_server_lock);
> + s = d->arch.hvm_domain.ioreq_server;
> + spin_unlock(&d->arch.hvm_domain.ioreq_server_lock);
>
> - hvm_ioreq_server_remove_vcpu(s, v);
> + if ( s )
> + hvm_ioreq_server_remove_vcpu(s, v);
>
> nestedhvm_vcpu_destroy(v);
>
> @@ -1459,7 +1570,10 @@ int hvm_buffered_io_send(ioreq_t *p)
> /* Ensure buffered_iopage fits in a page */
> BUILD_BUG_ON(sizeof(buffered_iopage_t) > PAGE_SIZE);
>
> + spin_lock(&d->arch.hvm_domain.ioreq_server_lock);
> s = d->arch.hvm_domain.ioreq_server;
> + spin_unlock(&d->arch.hvm_domain.ioreq_server_lock);
> +
> if ( !s )
> return 0;
>
> @@ -1532,20 +1646,12 @@ int hvm_buffered_io_send(ioreq_t *p)
> return 1;
> }
>
> -bool_t hvm_send_assist_req(struct vcpu *v, ioreq_t *proto_p)
> +static bool_t hvm_send_assist_req_to_server(struct hvm_ioreq_server *s,
> + struct vcpu *v,
> + ioreq_t *proto_p)
> {
> struct domain *d = v->domain;
> - struct hvm_ioreq_server *s;
> - ioreq_t *p;
> -
> - if ( unlikely(!vcpu_start_shutdown_deferral(v)) )
> - return 0; /* implicitly bins the i/o operation */
> -
> - s = d->arch.hvm_domain.ioreq_server;
> - if ( !s )
> - return 0;
> -
> - p = get_ioreq(s, v->vcpu_id);
> + ioreq_t *p = get_ioreq(s, v->vcpu_id);
>
> if ( unlikely(p->state != STATE_IOREQ_NONE) )
> {
> @@ -1578,6 +1684,26 @@ bool_t hvm_send_assist_req(struct vcpu *v, ioreq_t *proto_p)
> return 1;
> }
>
> +bool_t hvm_send_assist_req(struct vcpu *v, ioreq_t *p)
> +{
> + struct domain *d = v->domain;
> + struct hvm_ioreq_server *s;
> +
> + ASSERT(v->arch.hvm_vcpu.ioreq_server == NULL);
> +
> + if ( unlikely(!vcpu_start_shutdown_deferral(v)) )
> + return 0;
> +
> + spin_lock(&d->arch.hvm_domain.ioreq_server_lock);
> + s = d->arch.hvm_domain.ioreq_server;
> + spin_unlock(&d->arch.hvm_domain.ioreq_server_lock);
What is the purpose of talking the server lock just to read the
ioreq_server pointer?
> +
> + if ( !s )
> + return 0;
> +
> + return hvm_send_assist_req_to_server(s, v, p);
> +}
> +
> void hvm_hlt(unsigned long rflags)
> {
> struct vcpu *curr = current;
> @@ -4172,7 +4298,6 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
> case HVMOP_get_param:
> {
> struct xen_hvm_param a;
> - struct hvm_ioreq_server *s;
> struct domain *d;
> struct vcpu *v;
>
> @@ -4198,20 +4323,12 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
> if ( rc )
> goto param_fail;
>
> - s = d->arch.hvm_domain.ioreq_server;
> -
> if ( op == HVMOP_set_param )
> {
> rc = 0;
>
> switch ( a.index )
> {
> - case HVM_PARAM_IOREQ_PFN:
> - rc = hvm_set_ioreq_server_pfn(s, a.value);
> - break;
> - case HVM_PARAM_BUFIOREQ_PFN:
> - rc = hvm_set_ioreq_server_buf_pfn(s, a.value);
> - break;
> case HVM_PARAM_CALLBACK_IRQ:
> hvm_set_callback_via(d, a.value);
> hvm_latch_shinfo_size(d);
> @@ -4265,7 +4382,9 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
> if ( a.value == DOMID_SELF )
> a.value = curr_d->domain_id;
>
> - rc = hvm_set_ioreq_server_domid(s, a.value);
> + rc = hvm_create_ioreq_server(d, a.value);
> + if ( rc == -EEXIST )
> + rc = hvm_set_ioreq_server_domid(d, a.value);
> break;
> case HVM_PARAM_ACPI_S_STATE:
> /* Not reflexive, as we must domain_pause(). */
> @@ -4360,8 +4479,46 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
> {
> switch ( a.index )
> {
> + case HVM_PARAM_IOREQ_PFN:
> + case HVM_PARAM_BUFIOREQ_PFN:
> case HVM_PARAM_BUFIOREQ_EVTCHN:
> - a.value = s->buf_ioreq_evtchn;
> + /* May need to create server */
> + rc = hvm_create_ioreq_server(d, curr_d->domain_id);
> + if ( rc != 0 && rc != -EEXIST )
> + goto param_fail;
> +
> + switch ( a.index )
> + {
> + case HVM_PARAM_IOREQ_PFN: {
> + xen_pfn_t pfn;
> +
> + if ( (rc = hvm_get_ioreq_server_pfn(d, 0, &pfn)) < 0 )
> + goto param_fail;
> +
> + a.value = pfn;
> + break;
> + }
> + case HVM_PARAM_BUFIOREQ_PFN: {
> + xen_pfn_t pfn;
> +
> + if ( (rc = hvm_get_ioreq_server_pfn(d, 1, &pfn)) < 0 )
> + goto param_fail;
> +
> + a.value = pfn;
> + break;
> + }
> + case HVM_PARAM_BUFIOREQ_EVTCHN: {
> + evtchn_port_t port;
> +
> + if ( (rc = hvm_get_ioreq_server_buf_port(d, &port)) < 0 )
> + goto param_fail;
> +
> + a.value = port;
> + break;
> + }
> + default:
> + BUG();
> + }
> break;
> case HVM_PARAM_ACPI_S_STATE:
> a.value = d->arch.hvm_domain.is_s3_suspended ? 3 : 0;
> diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
> index 4c039f8..e750ef0 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -52,6 +52,8 @@ struct hvm_ioreq_server {
>
> struct hvm_domain {
> struct hvm_ioreq_server *ioreq_server;
> + spinlock_t ioreq_server_lock;
> +
> struct pl_time pl_time;
>
> struct hvm_io_handler *io_handler;
> @@ -106,4 +108,3 @@ struct hvm_domain {
> #define hap_enabled(d) ((d)->arch.hvm_domain.hap_enabled)
>
> #endif /* __ASM_X86_HVM_DOMAIN_H__ */
> -
Spurious whitespace change
~Andrew
next prev parent reply other threads:[~2014-01-30 15:21 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-30 14:19 [RFC PATCH 1/5] Support for running secondary emulators Paul Durrant
2014-01-30 14:19 ` [RFC PATCH 1/5] ioreq-server: centralize access to ioreq structures Paul Durrant
2014-01-30 14:32 ` Andrew Cooper
2014-01-30 14:35 ` Paul Durrant
2014-02-07 4:53 ` Matt Wilson
2014-02-07 9:24 ` Paul Durrant
2014-01-30 14:19 ` [RFC PATCH 2/5] ioreq-server: create basic ioreq server abstraction Paul Durrant
2014-01-30 15:03 ` Andrew Cooper
2014-01-30 15:17 ` Paul Durrant
2014-01-30 14:19 ` [RFC PATCH 3/5] ioreq-server: on-demand creation of ioreq server Paul Durrant
2014-01-30 15:21 ` Andrew Cooper [this message]
2014-01-30 15:32 ` Paul Durrant
2014-01-30 14:19 ` [RFC PATCH 4/5] ioreq-server: add support for multiple servers Paul Durrant
2014-01-30 15:46 ` Andrew Cooper
2014-01-30 15:56 ` Paul Durrant
2014-01-30 14:19 ` [RFC PATCH 5/5] ioreq-server: bring the PCI hotplug controller implementation into Xen Paul Durrant
2014-01-30 15:55 ` Andrew Cooper
2014-01-30 16:06 ` Paul Durrant
2014-01-30 16:38 ` Jan Beulich
2014-01-30 16:42 ` Paul Durrant
2014-01-30 14:23 ` [RFC PATCH 1/5] Support for running secondary emulators Paul Durrant
2014-03-01 22:24 ` Matt Wilson
2014-03-03 13:34 ` Paul Durrant
2014-03-03 22:41 ` Matt Wilson
2014-03-04 10:11 ` Paul Durrant
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52EA6E0E.8020200@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=paul.durrant@citrix.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.