|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 08/14 v4] xen/arm: vpl011: Modify xenconsole to define and use a new console structure
On Tue, 6 Jun 2017, Bhupinder Thakur wrote:
> Xenconsole uses a domain structure which contains console specific fields.
> This
> patch defines a new console structure, which would be used by the xenconsole
> functions to perform console specific operations like reading/writing data
> from/to
> the console ring buffer or reading/writing data from/to console tty.
>
> This patch is in preparation to support multiple consoles to support vuart
> console.
>
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>
> ---
> CC: ij
> CC: wl
> CC: ss
> CC: jg
>
> Changes since v3:
> - The changes in xenconsole have been split into four patches. This is the
> first patch
> which modifies the xenconsole to use a new console structure.
>
> Changes since v2:
> - Defined a new function console_create_ring() which sets up the ring buffer
> and
> event channel a new console. domain_create_ring() uses this function to
> setup
> a console.
> - This patch does not contain vuart specific changes, which would be
> introduced in
> the next patch.
> - Changes for keeping the PV log file name unchanged.
>
> Changes since v1:
> - Split the domain struture to a separate console structure
> - Modified the functions to operate on the console struture
> - Replaced repetitive per console code with generic code
>
> tools/console/daemon/io.c | 226
> ++++++++++++++++++++++++++--------------------
> 1 file changed, 127 insertions(+), 99 deletions(-)
>
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index 947f13a..0402ddf 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -89,25 +89,30 @@ struct buffer {
> size_t max_capacity;
> };
>
> -struct domain {
> - int domid;
> +struct console {
> int master_fd;
> int master_pollfd_idx;
> int slave_fd;
> int log_fd;
> - bool is_dead;
> - unsigned last_seen;
> struct buffer buffer;
> - struct domain *next;
> char *conspath;
> int ring_ref;
> xenevtchn_port_or_error_t local_port;
> xenevtchn_port_or_error_t remote_port;
> + struct xencons_interface *interface;
> + struct domain *d;
> +};
> +
> +struct domain {
> + int domid;
> + bool is_dead;
> + unsigned last_seen;
> + struct domain *next;
> xenevtchn_handle *xce_handle;
> int xce_pollfd_idx;
> - struct xencons_interface *interface;
> int event_count;
> long long next_period;
> + struct console console;
> };
All the mechanical substitutions below look good. It remains to discuss
whether we should keep xce_handle, xce_pollfd_idx, event_count and
next_period in struct domain, see my comments on the other patches.
It is strange to count the number of events and the next_period on a
domain basis, when actually all the action is done at a console level.
> static struct domain *dom_head;
> @@ -160,9 +165,10 @@ static int write_with_timestamp(int fd, const char
> *data, size_t sz,
>
> static void buffer_append(struct domain *dom)
> {
> - struct buffer *buffer = &dom->buffer;
> + struct console *con = &dom->console;
> + struct buffer *buffer = &con->buffer;
> XENCONS_RING_IDX cons, prod, size;
> - struct xencons_interface *intf = dom->interface;
> + struct xencons_interface *intf = con->interface;
>
> cons = intf->out_cons;
> prod = intf->out_prod;
> @@ -187,22 +193,22 @@ static void buffer_append(struct domain *dom)
>
> xen_mb();
> intf->out_cons = cons;
> - xenevtchn_notify(dom->xce_handle, dom->local_port);
> + xenevtchn_notify(dom->xce_handle, con->local_port);
>
> /* Get the data to the logfile as early as possible because if
> * no one is listening on the console pty then it will fill up
> * and handle_tty_write will stop being called.
> */
> - if (dom->log_fd != -1) {
> + if (con->log_fd != -1) {
> int logret;
> if (log_time_guest) {
> logret = write_with_timestamp(
> - dom->log_fd,
> + con->log_fd,
> buffer->data + buffer->size - size,
> size, &log_time_guest_needts);
> } else {
> logret = write_all(
> - dom->log_fd,
> + con->log_fd,
> buffer->data + buffer->size - size,
> size);
> }
> @@ -338,14 +344,16 @@ static int create_domain_log(struct domain *dom)
>
> static void domain_close_tty(struct domain *dom)
> {
> - if (dom->master_fd != -1) {
> - close(dom->master_fd);
> - dom->master_fd = -1;
> + struct console *con = &dom->console;
> +
> + if (con->master_fd != -1) {
> + close(con->master_fd);
> + con->master_fd = -1;
> }
>
> - if (dom->slave_fd != -1) {
> - close(dom->slave_fd);
> - dom->slave_fd = -1;
> + if (con->slave_fd != -1) {
> + close(con->slave_fd);
> + con->slave_fd = -1;
> }
> }
>
> @@ -418,11 +426,12 @@ static int domain_create_tty(struct domain *dom)
> char *data;
> unsigned int len;
> struct termios term;
> + struct console *con = &dom->console;
>
> - assert(dom->slave_fd == -1);
> - assert(dom->master_fd == -1);
> + assert(con->slave_fd == -1);
> + assert(con->master_fd == -1);
>
> - if (openpty(&dom->master_fd, &dom->slave_fd, NULL, NULL, NULL) < 0) {
> + if (openpty(&con->master_fd, &con->slave_fd, NULL, NULL, NULL) < 0) {
> err = errno;
> dolog(LOG_ERR, "Failed to create tty for domain-%d "
> "(errno = %i, %s)",
> @@ -430,7 +439,7 @@ static int domain_create_tty(struct domain *dom)
> return 0;
> }
>
> - if (tcgetattr(dom->slave_fd, &term) < 0) {
> + if (tcgetattr(con->slave_fd, &term) < 0) {
> err = errno;
> dolog(LOG_ERR, "Failed to get tty attributes for domain-%d "
> "(errno = %i, %s)",
> @@ -438,7 +447,7 @@ static int domain_create_tty(struct domain *dom)
> goto out;
> }
> cfmakeraw(&term);
> - if (tcsetattr(dom->slave_fd, TCSANOW, &term) < 0) {
> + if (tcsetattr(con->slave_fd, TCSANOW, &term) < 0) {
> err = errno;
> dolog(LOG_ERR, "Failed to set tty attributes for domain-%d "
> "(errno = %i, %s)",
> @@ -446,7 +455,7 @@ static int domain_create_tty(struct domain *dom)
> goto out;
> }
>
> - if ((slave = ptsname(dom->master_fd)) == NULL) {
> + if ((slave = ptsname(con->master_fd)) == NULL) {
> err = errno;
> dolog(LOG_ERR, "Failed to get slave name for domain-%d "
> "(errno = %i, %s)",
> @@ -454,18 +463,18 @@ static int domain_create_tty(struct domain *dom)
> goto out;
> }
>
> - success = asprintf(&path, "%s/limit", dom->conspath) !=
> + success = asprintf(&path, "%s/limit", con->conspath) !=
> -1;
> if (!success)
> goto out;
> data = xs_read(xs, XBT_NULL, path, &len);
> if (data) {
> - dom->buffer.max_capacity = strtoul(data, 0, 0);
> + con->buffer.max_capacity = strtoul(data, 0, 0);
> free(data);
> }
> free(path);
>
> - success = (asprintf(&path, "%s/tty", dom->conspath) != -1);
> + success = (asprintf(&path, "%s/tty", con->conspath) != -1);
> if (!success)
> goto out;
> success = xs_write(xs, XBT_NULL, path, slave, strlen(slave));
> @@ -473,7 +482,7 @@ static int domain_create_tty(struct domain *dom)
> if (!success)
> goto out;
>
> - if (fcntl(dom->master_fd, F_SETFL, O_NONBLOCK) == -1)
> + if (fcntl(con->master_fd, F_SETFL, O_NONBLOCK) == -1)
> goto out;
>
> return 1;
> @@ -519,29 +528,32 @@ static int xs_gather(struct xs_handle *xs, const char
> *dir, ...)
>
> static void domain_unmap_interface(struct domain *dom)
> {
> - if (dom->interface == NULL)
> + struct console *con = &dom->console;
> +
> + if (con->interface == NULL)
> return;
> - if (xgt_handle && dom->ring_ref == -1)
> - xengnttab_unmap(xgt_handle, dom->interface, 1);
> + if (xgt_handle && con->ring_ref == -1)
> + xengnttab_unmap(xgt_handle, con->interface, 1);
> else
> - munmap(dom->interface, XC_PAGE_SIZE);
> - dom->interface = NULL;
> - dom->ring_ref = -1;
> + munmap(con->interface, XC_PAGE_SIZE);
> + con->interface = NULL;
> + con->ring_ref = -1;
> }
>
> static int domain_create_ring(struct domain *dom)
> {
> int err, remote_port, ring_ref, rc;
> char *type, path[PATH_MAX];
> + struct console *con = &dom->console;
>
> - err = xs_gather(xs, dom->conspath,
> + err = xs_gather(xs, con->conspath,
> "ring-ref", "%u", &ring_ref,
> "port", "%i", &remote_port,
> NULL);
> if (err)
> goto out;
>
> - snprintf(path, sizeof(path), "%s/type", dom->conspath);
> + snprintf(path, sizeof(path), "%s/type", con->conspath);
> type = xs_read(xs, XBT_NULL, path, NULL);
> if (type && strcmp(type, "xenconsoled") != 0) {
> free(type);
> @@ -550,41 +562,41 @@ static int domain_create_ring(struct domain *dom)
> free(type);
>
> /* If using ring_ref and it has changed, remap */
> - if (ring_ref != dom->ring_ref && dom->ring_ref != -1)
> + if (ring_ref != con->ring_ref && con->ring_ref != -1)
> domain_unmap_interface(dom);
>
> - if (!dom->interface && xgt_handle) {
> + if (!con->interface && xgt_handle) {
> /* Prefer using grant table */
> - dom->interface = xengnttab_map_grant_ref(xgt_handle,
> + con->interface = xengnttab_map_grant_ref(xgt_handle,
> dom->domid, GNTTAB_RESERVED_CONSOLE,
> PROT_READ|PROT_WRITE);
> - dom->ring_ref = -1;
> + con->ring_ref = -1;
> }
> - if (!dom->interface) {
> + if (!con->interface) {
> /* Fall back to xc_map_foreign_range */
> - dom->interface = xc_map_foreign_range(
> + con->interface = xc_map_foreign_range(
> xc, dom->domid, XC_PAGE_SIZE,
> PROT_READ|PROT_WRITE,
> (unsigned long)ring_ref);
> - if (dom->interface == NULL) {
> + if (con->interface == NULL) {
> err = EINVAL;
> goto out;
> }
> - dom->ring_ref = ring_ref;
> + con->ring_ref = ring_ref;
> }
>
> /* Go no further if port has not changed and we are still bound. */
> - if (remote_port == dom->remote_port) {
> + if (remote_port == con->remote_port) {
> xc_evtchn_status_t status = {
> .dom = DOMID_SELF,
> - .port = dom->local_port };
> + .port = con->local_port };
> if ((xc_evtchn_status(xc, &status) == 0) &&
> (status.status == EVTCHNSTAT_interdomain))
> goto out;
> }
>
> - dom->local_port = -1;
> - dom->remote_port = -1;
> + con->local_port = -1;
> + con->remote_port = -1;
> if (dom->xce_handle != NULL)
> xenevtchn_close(dom->xce_handle);
>
> @@ -605,22 +617,22 @@ static int domain_create_ring(struct domain *dom)
> dom->xce_handle = NULL;
> goto out;
> }
> - dom->local_port = rc;
> - dom->remote_port = remote_port;
> + con->local_port = rc;
> + con->remote_port = remote_port;
>
> - if (dom->master_fd == -1) {
> + if (con->master_fd == -1) {
> if (!domain_create_tty(dom)) {
> err = errno;
> xenevtchn_close(dom->xce_handle);
> dom->xce_handle = NULL;
> - dom->local_port = -1;
> - dom->remote_port = -1;
> + con->local_port = -1;
> + con->remote_port = -1;
> goto out;
> }
> }
>
> - if (log_guest && (dom->log_fd == -1))
> - dom->log_fd = create_domain_log(dom);
> + if (log_guest && (con->log_fd == -1))
> + con->log_fd = create_domain_log(dom);
>
> out:
> return err;
> @@ -630,16 +642,17 @@ static bool watch_domain(struct domain *dom, bool watch)
> {
> char domid_str[3 + MAX_STRLEN(dom->domid)];
> bool success;
> + struct console *con = &dom->console;
>
> snprintf(domid_str, sizeof(domid_str), "dom%u", dom->domid);
> if (watch) {
> - success = xs_watch(xs, dom->conspath, domid_str);
> + success = xs_watch(xs, con->conspath, domid_str);
> if (success)
> domain_create_ring(dom);
> else
> - xs_unwatch(xs, dom->conspath, domid_str);
> + xs_unwatch(xs, con->conspath, domid_str);
> } else {
> - success = xs_unwatch(xs, dom->conspath, domid_str);
> + success = xs_unwatch(xs, con->conspath, domid_str);
> }
>
> return success;
> @@ -651,6 +664,7 @@ static struct domain *create_domain(int domid)
> struct domain *dom;
> char *s;
> struct timespec ts;
> + struct console *con;
>
> if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) {
> dolog(LOG_ERR, "Cannot get time of day %s:%s:L%d",
> @@ -667,25 +681,26 @@ static struct domain *create_domain(int domid)
>
> dom->domid = domid;
>
> - dom->conspath = xs_get_domain_path(xs, dom->domid);
> - s = realloc(dom->conspath, strlen(dom->conspath) +
> + con = &dom->console;
> + con->conspath = xs_get_domain_path(xs, dom->domid);
> + s = realloc(con->conspath, strlen(con->conspath) +
> strlen("/console") + 1);
> if (s == NULL)
> goto out;
> - dom->conspath = s;
> - strcat(dom->conspath, "/console");
> + con->conspath = s;
> + strcat(con->conspath, "/console");
>
> - dom->master_fd = -1;
> - dom->master_pollfd_idx = -1;
> - dom->slave_fd = -1;
> - dom->log_fd = -1;
> + con->master_fd = -1;
> + con->master_pollfd_idx = -1;
> + con->slave_fd = -1;
> + con->log_fd = -1;
> dom->xce_pollfd_idx = -1;
>
> dom->next_period = ((long long)ts.tv_sec * 1000) + (ts.tv_nsec /
> 1000000) + RATE_LIMIT_PERIOD;
>
> - dom->ring_ref = -1;
> - dom->local_port = -1;
> - dom->remote_port = -1;
> + con->ring_ref = -1;
> + con->local_port = -1;
> + con->remote_port = -1;
>
> if (!watch_domain(dom, true))
> goto out;
> @@ -697,7 +712,7 @@ static struct domain *create_domain(int domid)
>
> return dom;
> out:
> - free(dom->conspath);
> + free(con->conspath);
> free(dom);
> return NULL;
> }
> @@ -729,18 +744,20 @@ static void remove_domain(struct domain *dom)
>
> static void cleanup_domain(struct domain *d)
> {
> + struct console *con = &d->console;
> +
> domain_close_tty(d);
>
> - if (d->log_fd != -1) {
> - close(d->log_fd);
> - d->log_fd = -1;
> + if (con->log_fd != -1) {
> + close(con->log_fd);
> + con->log_fd = -1;
> }
>
> - free(d->buffer.data);
> - d->buffer.data = NULL;
> + free(con->buffer.data);
> + con->buffer.data = NULL;
>
> - free(d->conspath);
> - d->conspath = NULL;
> + free(con->conspath);
> + con->conspath = NULL;
>
> remove_domain(d);
> }
> @@ -782,7 +799,8 @@ static void enum_domains(void)
>
> static int ring_free_bytes(struct domain *dom)
> {
> - struct xencons_interface *intf = dom->interface;
> + struct console *con = &dom->console;
> + struct xencons_interface *intf = con->interface;
> XENCONS_RING_IDX cons, prod, space;
>
> cons = intf->in_cons;
> @@ -812,7 +830,8 @@ static void handle_tty_read(struct domain *dom)
> ssize_t len = 0;
> char msg[80];
> int i;
> - struct xencons_interface *intf = dom->interface;
> + struct console *con = &dom->console;
> + struct xencons_interface *intf = con->interface;
> XENCONS_RING_IDX prod;
>
> if (dom->is_dead)
> @@ -825,7 +844,7 @@ static void handle_tty_read(struct domain *dom)
> if (len > sizeof(msg))
> len = sizeof(msg);
>
> - len = read(dom->master_fd, msg, len);
> + len = read(con->master_fd, msg, len);
> /*
> * Note: on Solaris, len == 0 means the slave closed, and this
> * is no problem, but Linux can't handle this usefully, so we
> @@ -841,7 +860,7 @@ static void handle_tty_read(struct domain *dom)
> }
> xen_wmb();
> intf->in_prod = prod;
> - xenevtchn_notify(dom->xce_handle, dom->local_port);
> + xenevtchn_notify(dom->xce_handle, con->local_port);
> } else {
> domain_close_tty(dom);
> shutdown_domain(dom);
> @@ -851,18 +870,19 @@ static void handle_tty_read(struct domain *dom)
> static void handle_tty_write(struct domain *dom)
> {
> ssize_t len;
> + struct console *con = &dom->console;
>
> if (dom->is_dead)
> return;
>
> - len = write(dom->master_fd, dom->buffer.data + dom->buffer.consumed,
> - dom->buffer.size - dom->buffer.consumed);
> + len = write(con->master_fd, con->buffer.data + con->buffer.consumed,
> + con->buffer.size - con->buffer.consumed);
> if (len < 1) {
> dolog(LOG_DEBUG, "Write failed on domain %d: %zd, %d\n",
> dom->domid, len, errno);
> domain_handle_broken_tty(dom, domain_is_valid(dom->domid));
> } else {
> - buffer_advance(&dom->buffer, len);
> + buffer_advance(&con->buffer, len);
> }
> }
>
> @@ -948,9 +968,11 @@ static void handle_log_reload(void)
> if (log_guest) {
> struct domain *d;
> for (d = dom_head; d; d = d->next) {
> - if (d->log_fd != -1)
> - close(d->log_fd);
> - d->log_fd = create_domain_log(d);
> + struct console *con = &d->console;
> +
> + if (con->log_fd != -1)
> + close(con->log_fd);
> + con->log_fd = create_domain_log(d);
> }
> }
>
> @@ -1059,6 +1081,8 @@ void handle_io(void)
> /* Re-calculate any event counter allowances & unblock
> domains with new allowance */
> for (d = dom_head; d; d = d->next) {
> + struct console *con = &d->console;
> +
> /* CS 16257:955ee4fa1345 introduces a 5ms fuzz
> * for select(), it is not clear poll() has
> * similar behavior (returning a couple of ms
> @@ -1068,13 +1092,15 @@ void handle_io(void)
> if ((now+5) > d->next_period) {
> d->next_period = now + RATE_LIMIT_PERIOD;
> if (d->event_count >= RATE_LIMIT_ALLOWANCE) {
> - (void)xenevtchn_unmask(d->xce_handle,
> d->local_port);
> + (void)xenevtchn_unmask(d->xce_handle,
> con->local_port);
> }
> d->event_count = 0;
> }
> }
>
> for (d = dom_head; d; d = d->next) {
> + struct console *con = &d->console;
> +
> if (d->event_count >= RATE_LIMIT_ALLOWANCE) {
> /* Determine if we're going to be the next time
> slice to expire */
> if (!next_timeout ||
> @@ -1082,25 +1108,25 @@ void handle_io(void)
> next_timeout = d->next_period;
> } else if (d->xce_handle != NULL) {
> if (discard_overflowed_data ||
> - !d->buffer.max_capacity ||
> - d->buffer.size < d->buffer.max_capacity) {
> + !con->buffer.max_capacity ||
> + con->buffer.size <
> con->buffer.max_capacity) {
> int evtchn_fd =
> xenevtchn_fd(d->xce_handle);
> d->xce_pollfd_idx = set_fds(evtchn_fd,
>
> POLLIN|POLLPRI);
> }
> }
>
> - if (d->master_fd != -1) {
> + if (con->master_fd != -1) {
> short events = 0;
> if (!d->is_dead && ring_free_bytes(d))
> events |= POLLIN;
>
> - if (!buffer_empty(&d->buffer))
> + if (!buffer_empty(&con->buffer))
> events |= POLLOUT;
>
> if (events)
> - d->master_pollfd_idx =
> - set_fds(d->master_fd,
> + con->master_pollfd_idx =
> + set_fds(con->master_fd,
> events|POLLPRI);
> }
> }
> @@ -1159,6 +1185,8 @@ void handle_io(void)
> }
>
> for (d = dom_head; d; d = n) {
> + struct console *con = &d->console;
> +
> n = d->next;
> if (d->event_count < RATE_LIMIT_ALLOWANCE) {
> if (d->xce_handle != NULL &&
> @@ -1170,22 +1198,22 @@ void handle_io(void)
> handle_ring_read(d);
> }
>
> - if (d->master_fd != -1 && d->master_pollfd_idx != -1) {
> - if (fds[d->master_pollfd_idx].revents &
> + if (con->master_fd != -1 && con->master_pollfd_idx !=
> -1) {
> + if (fds[con->master_pollfd_idx].revents &
> ~(POLLIN|POLLOUT|POLLPRI))
> domain_handle_broken_tty(d,
> domain_is_valid(d->domid));
> else {
> - if (fds[d->master_pollfd_idx].revents &
> + if (fds[con->master_pollfd_idx].revents
> &
> POLLIN)
> handle_tty_read(d);
> - if (fds[d->master_pollfd_idx].revents &
> + if (fds[con->master_pollfd_idx].revents
> &
> POLLOUT)
> handle_tty_write(d);
> }
> }
>
> - d->xce_pollfd_idx = d->master_pollfd_idx = -1;
> + d->xce_pollfd_idx = con->master_pollfd_idx = -1;
>
> if (d->last_seen != enum_pass)
> shutdown_domain(d);
> --
> 2.7.4
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |