[Xen-devel,13/25,v6] xen/arm: vpl011: Add a new add_console_evtchn_fd function in xenconsole

Message ID 1500296815-10243-14-git-send-email-bhupinder.thakur@linaro.org
State Superseded
Headers show
Series
  • SBSA UART emulation support in Xen
Related show

Commit Message

Bhupinder Thakur July 17, 2017, 1:06 p.m.
This patch introduces a new add_console_evtchn_fd function. This
function adds the console event channel FD to list of polled FDs.

Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
---
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>

Changes since v5:
- Split this change in a separate patch.

 tools/console/daemon/io.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

Comments

Wei Liu July 18, 2017, 11:52 a.m. | #1
On Mon, Jul 17, 2017 at 06:36:43PM +0530, Bhupinder Thakur wrote:
> This patch introduces a new add_console_evtchn_fd function. This
> function adds the console event channel FD to list of polled FDs.
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>
> ---
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> 
> Changes since v5:
> - Split this change in a separate patch.
> 
>  tools/console/daemon/io.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index e4882e2..dc96203 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -1048,6 +1048,27 @@ static void reset_fds(void)
>  		memset(fds, 0, sizeof(struct pollfd) * current_array_size);
>  }
>  
> +static void add_console_evtchn_fd(struct console *con, void *data)

There is no need to use void *. Just use long long * here.

Or, you can avoid using pointer by returning the new timeout

  next_timeout = maybe_add_console_evtchn_fd(con, next_timeout);

Up to you.

> +{
> +	long long next_timeout = *((long long *)data);
> +
> +	if (con->event_count >= RATE_LIMIT_ALLOWANCE) {
> +		/* Determine if we're going to be the next time slice to expire */
> +		if (!next_timeout ||
> +		    con->next_period < next_timeout)
> +			next_timeout = con->next_period;
> +	} else if (con->xce_handle != NULL) {
> +			if (buffer_available(con))
> +			{
> +				int evtchn_fd = xenevtchn_fd(con->xce_handle);
> +				con->xce_pollfd_idx = set_fds(evtchn_fd,
> +							      POLLIN|POLLPRI);
> +			}
> +		}

Indentation is wrong.

And please add maybe_ prefix because it doesn't always add the fd.
Bhupinder Thakur July 20, 2017, 1:55 p.m. | #2
Hi Wei,


>>
>> +static void add_console_evtchn_fd(struct console *con, void *data)
>
> There is no need to use void *. Just use long long * here.

Since this function is going to be passed in later patches as an
argument to iter functions,
I wanted to keep the type generic so that I could type cast it as
required. Or I could change
the parameter type to long long * for all iterator functions.

>
> Or, you can avoid using pointer by returning the new timeout
>
>   next_timeout = maybe_add_console_evtchn_fd(con, next_timeout);
>
> Up to you.
>
I need to pass the next_timeout goes as an input/output parameter to
add_console_evtchnfd()
which selects the minimum timeout among all the domains.

>> +{
>> +     long long next_timeout = *((long long *)data);
>> +
>> +     if (con->event_count >= RATE_LIMIT_ALLOWANCE) {
>> +             /* Determine if we're going to be the next time slice to expire */
>> +             if (!next_timeout ||
>> +                 con->next_period < next_timeout)
>> +                     next_timeout = con->next_period;
>> +     } else if (con->xce_handle != NULL) {
>> +                     if (buffer_available(con))
>> +                     {
>> +                             int evtchn_fd = xenevtchn_fd(con->xce_handle);
>> +                             con->xce_pollfd_idx = set_fds(evtchn_fd,
>> +                                                           POLLIN|POLLPRI);
>> +                     }
>> +             }
>
> Indentation is wrong.
>
I will correct the indentation.

> And please add maybe_ prefix because it doesn't always add the fd.

I will add the maybe_ prefix.

Regards,
Bhupinder

Patch

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index e4882e2..dc96203 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -1048,6 +1048,27 @@  static void reset_fds(void)
 		memset(fds, 0, sizeof(struct pollfd) * current_array_size);
 }
 
+static void add_console_evtchn_fd(struct console *con, void *data)
+{
+	long long next_timeout = *((long long *)data);
+
+	if (con->event_count >= RATE_LIMIT_ALLOWANCE) {
+		/* Determine if we're going to be the next time slice to expire */
+		if (!next_timeout ||
+		    con->next_period < next_timeout)
+			next_timeout = con->next_period;
+	} else if (con->xce_handle != NULL) {
+			if (buffer_available(con))
+			{
+				int evtchn_fd = xenevtchn_fd(con->xce_handle);
+				con->xce_pollfd_idx = set_fds(evtchn_fd,
+							      POLLIN|POLLPRI);
+			}
+		}
+
+	*((long long *)data) = next_timeout;
+}
+
 void handle_io(void)
 {
 	int ret;
@@ -1125,18 +1146,7 @@  void handle_io(void)
 		for (d = dom_head; d; d = d->next) {
 			struct console *con = &d->console;
 
-			if (con->event_count >= RATE_LIMIT_ALLOWANCE) {
-				/* Determine if we're going to be the next time slice to expire */
-				if (!next_timeout ||
-				    con->next_period < next_timeout)
-					next_timeout = con->next_period;
-			} else if (con->xce_handle != NULL) {
-			        if (buffer_available(con)) {
-					int evtchn_fd = xenevtchn_fd(con->xce_handle);
-					con->xce_pollfd_idx = set_fds(evtchn_fd,
-								    POLLIN|POLLPRI);
-				}
-			}
+			add_console_evtchn_fd(con, (void *)&next_timeout);
 
 			if (con->master_fd != -1) {
 				short events = 0;