| Message ID | 1487676368-22356-10-git-send-email-bhupinder.thakur@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | pl011 emulation support in Xen | expand |
On Tue, Feb 21, 2017 at 04:56:06PM +0530, Bhupinder Thakur wrote: > Modfication in domain_create_ring(): > - Bind to the vpl011 event channel obtained from the xen store as a new parameter > - Map the PFN to its address space to be used as IN/OUT ring buffers. It obtains > the PFN from the xen store as a new parameter > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> > --- > tools/console/daemon/io.c | 128 +++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 110 insertions(+), 18 deletions(-) > > diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c > index 7e6a886..b1aa615 100644 > --- a/tools/console/daemon/io.c > +++ b/tools/console/daemon/io.c > @@ -101,13 +101,18 @@ struct domain { > struct domain *next; > char *conspath; > int ring_ref; > + int vpl011_ring_ref; > xenevtchn_port_or_error_t local_port; > xenevtchn_port_or_error_t remote_port; > + xenevtchn_port_or_error_t vpl011_local_port; > + xenevtchn_port_or_error_t vpl011_remote_port; > xenevtchn_handle *xce_handle; > int xce_pollfd_idx; > struct xencons_interface *interface; > + struct xencons_interface *vpl011_interface; > int event_count; > long long next_period; > + bool vpl011_initialized; > }; > > static struct domain *dom_head; > @@ -529,9 +534,58 @@ static void domain_unmap_interface(struct domain *dom) > dom->ring_ref = -1; > } > > +static void domain_unmap_vpl011_interface(struct domain *dom) > +{ > + if ( dom->vpl011_interface == NULL ) > + return; > + > + if ( xgt_handle && dom->vpl011_ring_ref == -1 ) > + xengnttab_unmap(xgt_handle, dom->vpl011_interface, 1); This code uses its own styleguide, which means: - Use tabs instead of spaces. And only one here (and in the above return) - No spaces between ( ). > + else > + munmap(dom->vpl011_interface, XC_PAGE_SIZE); > + dom->vpl011_interface = NULL; > + dom->vpl011_ring_ref = -1; > +} Hm on second thought, this looks like domain_unmap_interface. Can you just replicate it to be like it? Or better yet. Could you change the domain_unmap_interface to accept two parameters: a void pointer to the interface and a pointer to the ring_ref? That way you could do: domain_unmap_interface(&dom->interface, &dom->ring_ref); domain_unmap_interface(&dom->vpl011_interface, &dom->vpl011_ring_ref); And you would reuse the function? > + > +int bind_event_channel(struct domain *dom, int new_rport, int *lport, int *rport) > +{ > + int err = 0, rc; > + > + /* Go no further if port has not changed and we are still bound. */ > + if ( new_rport == *rport ) { Again, no spaces here. > + xc_evtchn_status_t status = { > + .dom = DOMID_SELF, > + .port = *lport }; > + if ((xc_evtchn_status(xc, &status) == 0) && Shouldn't this have an tab in front it? It is part of the 'if (new_rport == *rport)' conditional after all. > + (status.status == EVTCHNSTAT_interdomain)) > + goto out; > + } > + > + /* initialize the ports */ Please remove this comment > + *lport = -1; > + *rport = -1; > + > + /* bind to new remote port */ And this one.. > + rc = xenevtchn_bind_interdomain(dom->xce_handle, > + dom->domid, new_rport); Huh? that should have been moved to be under (. > + > + if ( rc == -1 ) { Wrong style guide. No spaces. > + err = errno; > + xenevtchn_close(dom->xce_handle); > + dom->xce_handle = NULL; > + goto out; > + } > + > + /* store new local and remote event channel ports */ Please remove this comment. > + *lport = rc; > + *rport = new_rport; > +out: > + return err; > +} > + > static int domain_create_ring(struct domain *dom) > { > - int err, remote_port, ring_ref, rc; > + int err, remote_port, ring_ref, vpl011_remote_port, vpl011_ring_ref; > char *type, path[PATH_MAX]; > > err = xs_gather(xs, dom->conspath, > @@ -541,6 +595,20 @@ static int domain_create_ring(struct domain *dom) > if (err) > goto out; > > + /* > + * if the vpl011 parameters are not available or are not initialized > + * the vpl011 console is not available I would just change this to: /* vpl011 is optional. */ > + */ > + err = xs_gather(xs, dom->conspath, > + "vpl011-ring-ref", "%u", &vpl011_ring_ref, > + "vpl011-port", "%i", &vpl011_remote_port, How come you don't use %u for both? > + NULL); > + > + if ( err || vpl011_ring_ref == -1 ) > + dom->vpl011_initialized = false; > + else > + dom->vpl011_initialized = true; > + > snprintf(path, sizeof(path), "%s/type", dom->conspath); > type = xs_read(xs, XBT_NULL, path, NULL); > if (type && strcmp(type, "xenconsoled") != 0) { > @@ -553,6 +621,12 @@ static int domain_create_ring(struct domain *dom) > if (ring_ref != dom->ring_ref && dom->ring_ref != -1) > domain_unmap_interface(dom); > > + /* If using vpl011 ring_ref and it has changed, remap */ > + if ( dom->vpl011_initialized && > + vpl011_ring_ref != dom->vpl011_ring_ref && > + dom->vpl011_ring_ref != -1 ) Something is off here. Or maybe it is my editor. But I would think that the condtionals should be on the same line. Also, no spaces here. > + domain_unmap_vpl011_interface(dom); > + > if (!dom->interface && xgt_handle) { > /* Prefer using grant table */ > dom->interface = xengnttab_map_grant_ref(xgt_handle, > @@ -560,6 +634,8 @@ static int domain_create_ring(struct domain *dom) > PROT_READ|PROT_WRITE); > dom->ring_ref = -1; > } > + > + /* map PV console ring buffer */ ?? Please remove that. > if (!dom->interface) { > /* Fall back to xc_map_foreign_range */ > dom->interface = xc_map_foreign_range( > @@ -573,18 +649,21 @@ static int domain_create_ring(struct domain *dom) > dom->ring_ref = ring_ref; > } > > - /* Go no further if port has not changed and we are still bound. */ > - if (remote_port == dom->remote_port) { > - xc_evtchn_status_t status = { > - .dom = DOMID_SELF, > - .port = dom->local_port }; > - if ((xc_evtchn_status(xc, &status) == 0) && > - (status.status == EVTCHNSTAT_interdomain)) > + /* map vpl011 console ring buffer */ s/map/Map/ and also add period at the end please. > + if ( dom->vpl011_initialized && !dom->vpl011_interface ) { > + > + /* Fall back to xc_map_foreign_range */ > + dom->vpl011_interface = xc_map_foreign_range( > + xc, dom->domid, XC_PAGE_SIZE, > + PROT_READ|PROT_WRITE, > + (unsigned long)vpl011_ring_ref); What happend here? Is my editor mixed up? I would think that those would a bit shifted over? > + if ( dom->vpl011_interface == NULL ) { No spaces .. > + err = EINVAL; > goto out; > + } > + dom->vpl011_ring_ref = vpl011_ring_ref; > } > > - dom->local_port = -1; > - dom->remote_port = -1; > if (dom->xce_handle != NULL) > xenevtchn_close(dom->xce_handle); > > @@ -596,17 +675,24 @@ static int domain_create_ring(struct domain *dom) > goto out; > } > > - rc = xenevtchn_bind_interdomain(dom->xce_handle, > - dom->domid, remote_port); > - > - if (rc == -1) { > - err = errno; > + /* bind PV console channel */ > + err = bind_event_channel(dom, remote_port, &dom->local_port, &dom->remote_port); > + if (err) > + { { is on the same line as if (err) > xenevtchn_close(dom->xce_handle); > - dom->xce_handle = NULL; Why? We are still closing it.. > goto out; > } > - dom->local_port = rc; > - dom->remote_port = remote_port; > + > + /* bind vpl011 console channel */ Uppercase please and period. > + if ( dom->vpl011_initialized ) > + { { is on the same line as if in this code. > + err = bind_event_channel(dom, vpl011_remote_port, &dom->vpl011_local_port, &dom->vpl011_remote_port); > + if (err) > + { Ditto. > + xenevtchn_close(dom->xce_handle); No xce_handle = NULL? > + goto out; > + } > + } > > if (dom->master_fd == -1) { > if (!domain_create_tty(dom)) { > @@ -615,6 +701,9 @@ static int domain_create_ring(struct domain *dom) > dom->xce_handle = NULL; > dom->local_port = -1; > dom->remote_port = -1; > + dom->vpl011_local_port = -1; > + dom->vpl011_remote_port = -1; > + dom->vpl011_initialized = false; > goto out; > } > } > @@ -684,8 +773,11 @@ static struct domain *create_domain(int domid) > dom->next_period = ((long long)ts.tv_sec * 1000) + (ts.tv_nsec / 1000000) + RATE_LIMIT_PERIOD; > > dom->ring_ref = -1; > + dom->vpl011_ring_ref = -1; > dom->local_port = -1; > dom->remote_port = -1; > + dom->vpl011_local_port = -1; > + dom->vpl011_remote_port = -1; > > if (!watch_domain(dom, true)) > goto out; > -- > 2.7.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c index 7e6a886..b1aa615 100644 --- a/tools/console/daemon/io.c +++ b/tools/console/daemon/io.c @@ -101,13 +101,18 @@ struct domain { struct domain *next; char *conspath; int ring_ref; + int vpl011_ring_ref; xenevtchn_port_or_error_t local_port; xenevtchn_port_or_error_t remote_port; + xenevtchn_port_or_error_t vpl011_local_port; + xenevtchn_port_or_error_t vpl011_remote_port; xenevtchn_handle *xce_handle; int xce_pollfd_idx; struct xencons_interface *interface; + struct xencons_interface *vpl011_interface; int event_count; long long next_period; + bool vpl011_initialized; }; static struct domain *dom_head; @@ -529,9 +534,58 @@ static void domain_unmap_interface(struct domain *dom) dom->ring_ref = -1; } +static void domain_unmap_vpl011_interface(struct domain *dom) +{ + if ( dom->vpl011_interface == NULL ) + return; + + if ( xgt_handle && dom->vpl011_ring_ref == -1 ) + xengnttab_unmap(xgt_handle, dom->vpl011_interface, 1); + else + munmap(dom->vpl011_interface, XC_PAGE_SIZE); + dom->vpl011_interface = NULL; + dom->vpl011_ring_ref = -1; +} + +int bind_event_channel(struct domain *dom, int new_rport, int *lport, int *rport) +{ + int err = 0, rc; + + /* Go no further if port has not changed and we are still bound. */ + if ( new_rport == *rport ) { + xc_evtchn_status_t status = { + .dom = DOMID_SELF, + .port = *lport }; + if ((xc_evtchn_status(xc, &status) == 0) && + (status.status == EVTCHNSTAT_interdomain)) + goto out; + } + + /* initialize the ports */ + *lport = -1; + *rport = -1; + + /* bind to new remote port */ + rc = xenevtchn_bind_interdomain(dom->xce_handle, + dom->domid, new_rport); + + if ( rc == -1 ) { + err = errno; + xenevtchn_close(dom->xce_handle); + dom->xce_handle = NULL; + goto out; + } + + /* store new local and remote event channel ports */ + *lport = rc; + *rport = new_rport; +out: + return err; +} + static int domain_create_ring(struct domain *dom) { - int err, remote_port, ring_ref, rc; + int err, remote_port, ring_ref, vpl011_remote_port, vpl011_ring_ref; char *type, path[PATH_MAX]; err = xs_gather(xs, dom->conspath, @@ -541,6 +595,20 @@ static int domain_create_ring(struct domain *dom) if (err) goto out; + /* + * if the vpl011 parameters are not available or are not initialized + * the vpl011 console is not available + */ + err = xs_gather(xs, dom->conspath, + "vpl011-ring-ref", "%u", &vpl011_ring_ref, + "vpl011-port", "%i", &vpl011_remote_port, + NULL); + + if ( err || vpl011_ring_ref == -1 ) + dom->vpl011_initialized = false; + else + dom->vpl011_initialized = true; + snprintf(path, sizeof(path), "%s/type", dom->conspath); type = xs_read(xs, XBT_NULL, path, NULL); if (type && strcmp(type, "xenconsoled") != 0) { @@ -553,6 +621,12 @@ static int domain_create_ring(struct domain *dom) if (ring_ref != dom->ring_ref && dom->ring_ref != -1) domain_unmap_interface(dom); + /* If using vpl011 ring_ref and it has changed, remap */ + if ( dom->vpl011_initialized && + vpl011_ring_ref != dom->vpl011_ring_ref && + dom->vpl011_ring_ref != -1 ) + domain_unmap_vpl011_interface(dom); + if (!dom->interface && xgt_handle) { /* Prefer using grant table */ dom->interface = xengnttab_map_grant_ref(xgt_handle, @@ -560,6 +634,8 @@ static int domain_create_ring(struct domain *dom) PROT_READ|PROT_WRITE); dom->ring_ref = -1; } + + /* map PV console ring buffer */ if (!dom->interface) { /* Fall back to xc_map_foreign_range */ dom->interface = xc_map_foreign_range( @@ -573,18 +649,21 @@ static int domain_create_ring(struct domain *dom) dom->ring_ref = ring_ref; } - /* Go no further if port has not changed and we are still bound. */ - if (remote_port == dom->remote_port) { - xc_evtchn_status_t status = { - .dom = DOMID_SELF, - .port = dom->local_port }; - if ((xc_evtchn_status(xc, &status) == 0) && - (status.status == EVTCHNSTAT_interdomain)) + /* map vpl011 console ring buffer */ + if ( dom->vpl011_initialized && !dom->vpl011_interface ) { + + /* Fall back to xc_map_foreign_range */ + dom->vpl011_interface = xc_map_foreign_range( + xc, dom->domid, XC_PAGE_SIZE, + PROT_READ|PROT_WRITE, + (unsigned long)vpl011_ring_ref); + if ( dom->vpl011_interface == NULL ) { + err = EINVAL; goto out; + } + dom->vpl011_ring_ref = vpl011_ring_ref; } - dom->local_port = -1; - dom->remote_port = -1; if (dom->xce_handle != NULL) xenevtchn_close(dom->xce_handle); @@ -596,17 +675,24 @@ static int domain_create_ring(struct domain *dom) goto out; } - rc = xenevtchn_bind_interdomain(dom->xce_handle, - dom->domid, remote_port); - - if (rc == -1) { - err = errno; + /* bind PV console channel */ + err = bind_event_channel(dom, remote_port, &dom->local_port, &dom->remote_port); + if (err) + { xenevtchn_close(dom->xce_handle); - dom->xce_handle = NULL; goto out; } - dom->local_port = rc; - dom->remote_port = remote_port; + + /* bind vpl011 console channel */ + if ( dom->vpl011_initialized ) + { + err = bind_event_channel(dom, vpl011_remote_port, &dom->vpl011_local_port, &dom->vpl011_remote_port); + if (err) + { + xenevtchn_close(dom->xce_handle); + goto out; + } + } if (dom->master_fd == -1) { if (!domain_create_tty(dom)) { @@ -615,6 +701,9 @@ static int domain_create_ring(struct domain *dom) dom->xce_handle = NULL; dom->local_port = -1; dom->remote_port = -1; + dom->vpl011_local_port = -1; + dom->vpl011_remote_port = -1; + dom->vpl011_initialized = false; goto out; } } @@ -684,8 +773,11 @@ static struct domain *create_domain(int domid) dom->next_period = ((long long)ts.tv_sec * 1000) + (ts.tv_nsec / 1000000) + RATE_LIMIT_PERIOD; dom->ring_ref = -1; + dom->vpl011_ring_ref = -1; dom->local_port = -1; dom->remote_port = -1; + dom->vpl011_local_port = -1; + dom->vpl011_remote_port = -1; if (!watch_domain(dom, true)) goto out;
Modfication in domain_create_ring(): - Bind to the vpl011 event channel obtained from the xen store as a new parameter - Map the PFN to its address space to be used as IN/OUT ring buffers. It obtains the PFN from the xen store as a new parameter Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> --- tools/console/daemon/io.c | 128 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 110 insertions(+), 18 deletions(-)