| Message ID | 20170718145329.22717-1-eggi.innovations@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 18/07/17 15:53, Felix Schmoll wrote: > Implement reading from PV console. Making use of polling. > > Signed-off-by: Felix Schmoll <eggi.innovations@gmail.com> > --- > common/console.c | 22 ++++++++++++++++++++++ > include/xtf/console.h | 2 ++ > 2 files changed, 24 insertions(+) > > diff --git a/common/console.c b/common/console.c > index 7cb2361..5895ccc 100644 > --- a/common/console.c > +++ b/common/console.c > @@ -7,6 +7,8 @@ > #include <xtf/libc.h> > #include <xtf/traps.h> > > +#include <xen/sched.h> Why is this needed? > + > /* > * Output functions, registered if/when available. > * Possibilities: > @@ -45,6 +47,26 @@ static size_t pv_console_write_some(const char *buf, size_t len) > return s; > } > > +extern shared_info_t shared_info; This extern isn't needed. > +size_t pv_console_read(char *buf, size_t len) > +{ > + size_t s = 0; > + uint32_t cons, prod; > + > + while ( !test_and_clear_bit(pv_evtchn, shared_info.evtchn_pending) || > + (pv_ring->in_cons == pv_ring->in_prod) ) > + hypercall_poll(pv_evtchn); > + > + cons = pv_ring->in_cons, prod = LOAD_ACQUIRE(&pv_ring->in_prod); > + > + while ( (s < len) && (0 < (prod - cons)) ) > + buf[s++] = pv_ring->in[cons++ & (sizeof(pv_ring->in) - 1)]; > + > + STORE_RELEASE(&pv_ring->in_cons, cons); > + > + return s; > +} > + > /* > * Write some data into the pv ring, synchronously waiting for all data to be > * consumed. > diff --git a/include/xtf/console.h b/include/xtf/console.h > index 2a93c06..9b3f85d 100644 > --- a/include/xtf/console.h > +++ b/include/xtf/console.h > @@ -25,6 +25,8 @@ void init_pv_console(xencons_interface_t *ring, > void vprintk(const char *fmt, va_list args) __printf(1, 0); > void printk(const char *fmt, ...) __printf(1, 2); > > +size_t pv_console_read(char *buf, size_t len); What behaviour do you intend this function to have wrt partial strings? ~Andrew > + > #endif /* XTF_CONSOLE_H */ > > /*
2017-07-18 17:04 GMT+02:00 Andrew Cooper <andrew.cooper3@citrix.com>: > On 18/07/17 15:53, Felix Schmoll wrote: > > Implement reading from PV console. Making use of polling. > > > > Signed-off-by: Felix Schmoll <eggi.innovations@gmail.com> > > --- > > common/console.c | 22 ++++++++++++++++++++++ > > include/xtf/console.h | 2 ++ > > 2 files changed, 24 insertions(+) > > > > diff --git a/common/console.c b/common/console.c > > index 7cb2361..5895ccc 100644 > > --- a/common/console.c > > +++ b/common/console.c > > @@ -7,6 +7,8 @@ > > #include <xtf/libc.h> > > #include <xtf/traps.h> > > > > +#include <xen/sched.h> > > Why is this needed? > > Apparently it's not; will be removed in the next version of this patch. > + > > /* > > * Output functions, registered if/when available. > > * Possibilities: > > @@ -45,6 +47,26 @@ static size_t pv_console_write_some(const char *buf, > size_t len) > > return s; > > } > > > > +extern shared_info_t shared_info; > > This extern isn't needed. > > > +size_t pv_console_read(char *buf, size_t len) > > +{ > > + size_t s = 0; > > + uint32_t cons, prod; > > + > > + while ( !test_and_clear_bit(pv_evtchn, shared_info.evtchn_pending) > || > > + (pv_ring->in_cons == pv_ring->in_prod) ) > > + hypercall_poll(pv_evtchn); > > + > > + cons = pv_ring->in_cons, prod = LOAD_ACQUIRE(&pv_ring->in_prod); > > + > > + while ( (s < len) && (0 < (prod - cons)) ) > > + buf[s++] = pv_ring->in[cons++ & (sizeof(pv_ring->in) - 1)]; > > + > > + STORE_RELEASE(&pv_ring->in_cons, cons); > > + > > + return s; > > +} > > + > > /* > > * Write some data into the pv ring, synchronously waiting for all data > to be > > * consumed. > > diff --git a/include/xtf/console.h b/include/xtf/console.h > > index 2a93c06..9b3f85d 100644 > > --- a/include/xtf/console.h > > +++ b/include/xtf/console.h > > @@ -25,6 +25,8 @@ void init_pv_console(xencons_interface_t *ring, > > void vprintk(const char *fmt, va_list args) __printf(1, 0); > > void printk(const char *fmt, ...) __printf(1, 2); > > > > +size_t pv_console_read(char *buf, size_t len); > > What behaviour do you intend this function to have wrt partial strings? > > I'd prefer the function to be as simple as possible. If a specific output is expected that should be handled by the caller. The function name could be adjusted to pv_console_read_some to reflect that better, in symmetry to pv_console_write_some. That one isn't public though, which is why I didn't do it initially. Alternatively one could adopt a more well-known function such as readline, as was done with printk in regard to printf. > ~Andrew > > > + > > #endif /* XTF_CONSOLE_H */ > > > > /* > >
On 19/07/17 09:36, Felix Schmoll wrote: > > > 2017-07-18 17:04 GMT+02:00 Andrew Cooper <andrew.cooper3@citrix.com > <mailto:andrew.cooper3@citrix.com>>: > > On 18/07/17 15:53, Felix Schmoll wrote: > > Implement reading from PV console. Making use of polling. > > > > Signed-off-by: Felix Schmoll <eggi.innovations@gmail.com > <mailto:eggi.innovations@gmail.com>> > > --- > > common/console.c | 22 ++++++++++++++++++++++ > > include/xtf/console.h | 2 ++ > > 2 files changed, 24 insertions(+) > > > > diff --git a/common/console.c b/common/console.c > > index 7cb2361..5895ccc 100644 > > --- a/common/console.c > > +++ b/common/console.c > > @@ -7,6 +7,8 @@ > > #include <xtf/libc.h> > > #include <xtf/traps.h> > > > > +#include <xen/sched.h> > > Why is this needed? > > > Apparently it's not; will be removed in the next version of this patch. > > > + > > /* > > * Output functions, registered if/when available. > > * Possibilities: > > @@ -45,6 +47,26 @@ static size_t pv_console_write_some(const > char *buf, size_t len) > > return s; > > } > > > > +extern shared_info_t shared_info; > > This extern isn't needed. > > > +size_t pv_console_read(char *buf, size_t len) > > +{ > > + size_t s = 0; > > + uint32_t cons, prod; > > + > > + while ( !test_and_clear_bit(pv_evtchn, > shared_info.evtchn_pending) || > > + (pv_ring->in_cons == pv_ring->in_prod) ) > > + hypercall_poll(pv_evtchn); > > + > > + cons = pv_ring->in_cons, prod = > LOAD_ACQUIRE(&pv_ring->in_prod); > > + > > + while ( (s < len) && (0 < (prod - cons)) ) > > + buf[s++] = pv_ring->in[cons++ & (sizeof(pv_ring->in) - 1)]; > > + > > + STORE_RELEASE(&pv_ring->in_cons, cons); > > + > > + return s; > > +} > > + > > /* > > * Write some data into the pv ring, synchronously waiting for > all data to be > > * consumed. > > diff --git a/include/xtf/console.h b/include/xtf/console.h > > index 2a93c06..9b3f85d 100644 > > --- a/include/xtf/console.h > > +++ b/include/xtf/console.h > > @@ -25,6 +25,8 @@ void init_pv_console(xencons_interface_t *ring, > > void vprintk(const char *fmt, va_list args) __printf(1, 0); > > void printk(const char *fmt, ...) __printf(1, 2); > > > > +size_t pv_console_read(char *buf, size_t len); > > What behaviour do you intend this function to have wrt partial > strings? > > > I'd prefer the function to be as simple as possible. If a specific > output is expected that should be handled by the caller. The function > name could be adjusted to pv_console_read_some to reflect that better, > in symmetry to pv_console_write_some. That one isn't public though, > which is why I didn't do it initially. > > Alternatively one could adopt a more well-known function such as > readline, as was done with printk in regard to printf. I am not asking you to change the function (although naming it pv_console_read_some() would be an improvement). There should however be a description (like the other pv_console_*() functions) of what the intended behaviour is. This function in particular is more complicated than its write counterpart. ~Andrew
diff --git a/common/console.c b/common/console.c index 7cb2361..5895ccc 100644 --- a/common/console.c +++ b/common/console.c @@ -7,6 +7,8 @@ #include <xtf/libc.h> #include <xtf/traps.h> +#include <xen/sched.h> + /* * Output functions, registered if/when available. * Possibilities: @@ -45,6 +47,26 @@ static size_t pv_console_write_some(const char *buf, size_t len) return s; } +extern shared_info_t shared_info; +size_t pv_console_read(char *buf, size_t len) +{ + size_t s = 0; + uint32_t cons, prod; + + while ( !test_and_clear_bit(pv_evtchn, shared_info.evtchn_pending) || + (pv_ring->in_cons == pv_ring->in_prod) ) + hypercall_poll(pv_evtchn); + + cons = pv_ring->in_cons, prod = LOAD_ACQUIRE(&pv_ring->in_prod); + + while ( (s < len) && (0 < (prod - cons)) ) + buf[s++] = pv_ring->in[cons++ & (sizeof(pv_ring->in) - 1)]; + + STORE_RELEASE(&pv_ring->in_cons, cons); + + return s; +} + /* * Write some data into the pv ring, synchronously waiting for all data to be * consumed. diff --git a/include/xtf/console.h b/include/xtf/console.h index 2a93c06..9b3f85d 100644 --- a/include/xtf/console.h +++ b/include/xtf/console.h @@ -25,6 +25,8 @@ void init_pv_console(xencons_interface_t *ring, void vprintk(const char *fmt, va_list args) __printf(1, 0); void printk(const char *fmt, ...) __printf(1, 2); +size_t pv_console_read(char *buf, size_t len); + #endif /* XTF_CONSOLE_H */ /*
Implement reading from PV console. Making use of polling. Signed-off-by: Felix Schmoll <eggi.innovations@gmail.com> --- common/console.c | 22 ++++++++++++++++++++++ include/xtf/console.h | 2 ++ 2 files changed, 24 insertions(+)