On Fri, Mar 3, 2017 at 11:45 AM, 'Sergiy Kibrik' via OSv Development < [email protected]> wrote:
> Complete driver to properly work with xl console utility, i.e. full > read/write > access. Driver is extended to work with shared structure (ring) and event > channel. > > Signed-off-by: Sergiy Kibrik <[email protected]> > --- > drivers/xenconsole.cc | 65 ++++++++++++++++++++++++++++++ > +++++++++++++++++---- > drivers/xenconsole.hh | 14 +++++++++-- > 2 files changed, 72 insertions(+), 7 deletions(-) > > diff --git a/drivers/xenconsole.cc b/drivers/xenconsole.cc > index fe7c5bf..6ddac85 100644 > --- a/drivers/xenconsole.cc > +++ b/drivers/xenconsole.cc > @@ -5,32 +5,87 @@ > * BSD license as described in the LICENSE file in the top-level > directory. > */ > > +#define XENHVM //FIXME hack to reveal hvm_get_parameter() > #include <bsd/porting/netport.h> /* __dead2 defined here */ > +#include <bsd/porting/bus.h> > +#include <machine/xen/xen-os.h> > #include <xen/hypervisor.h> > - > +#include <xen/interface/hvm/params.h> > +#include <xen/interface/io/console.h> > +#include <xen/xen_intr.h> > +#include <xen/evtchn.h> > +#include <osv/mmio.hh> > #include "xenconsole.hh" > > namespace console { > > +XEN_Console:: > +XEN_Console::XEN_Console() > + : _interface(0) > + , _evtchn(-1) > + , _irq(0) > + , _pfn(0) > +{} > + > +void XEN_Console::handle_intr() > +{ > + _thread->wake(); > +} > + > void XEN_Console::write(const char *str, size_t len) { > - HYPERVISOR_console_write(str, len); > + assert(len > 0); > + if (!_interface) { > + HYPERVISOR_console_write(str, len); > + return; > + } > + > + XENCONS_RING_IDX cons, prod; > + prod = _interface->out_prod; > + cons = _interface->out_cons; > + mb(); > + while (len-- > 0) > + _interface->out[MASK_XENCONS_IDX(prod++, _interface->out)] = > *str++; > It looks like your implementation has no flow control, and if you write too quickly before Xen empties the buffer, you abort instead of waiting for the buffer to be empties. I guess this is fine as a first implementation, but please add a FIXME on this limitation. > + wmb(); > + assert(prod - cons <= sizeof(_interface->out)); > I guess this assert() is related to my question about. I am curious why test it here instead of up-front, using len? > + _interface->out_prod = prod; > } > > void XEN_Console::dev_start() > { > + _pfn = hvm_get_parameter(HVM_PARAM_CONSOLE_PFN); > + _evtchn = hvm_get_parameter(HVM_PARAM_CONSOLE_EVTCHN); > + > + if (!_pfn || !_evtchn) > + throw std::runtime_error("fail to get console params"); > + > + if (bind_caller_port_to_irqhandler(_evtchn, "xenconsole", > + XEN_Console::console_intr, > + static_cast<void*>(this), > + INTR_TYPE_MISC, &_irq) != 0) > + throw std::runtime_error("fail to bind evtchn"); > + > + _interface = (xencons_interface*)mmio_map(_pfn << PAGE_SHIFT, > PAGE_SIZE); > } > > void XEN_Console::flush() > { > + notify_remote_via_evtchn(_evtchn); > } > > bool XEN_Console::input_ready() > { > - return false; /*TODO: support input */ > + mb(); > What does this memory barrier, with nothing above it, do? > + return _interface->in_cons != _interface->in_prod; > } > > char XEN_Console::readch() { > - return '\0'; /*TODO: support input */ > + XENCONS_RING_IDX cons; > + char c; > + assert(_interface); > + cons = _interface->in_cons; > + c = _interface->in[MASK_XENCONS_IDX(cons, _interface->in)]; > I think you should gracefully handle the case where you had a false wakeup and called readch() when nothing is available, and return 0 in this case (as you can see, LineDiscipline::read_poll handles 0 being returned). I guess you need to check if cons is actually beyond prod? > + mb(); > + _interface->in_cons = cons + 1; > + return c; > } > - > } > diff --git a/drivers/xenconsole.hh b/drivers/xenconsole.hh > index ec43893..ca6d8cf 100644 > --- a/drivers/xenconsole.hh > +++ b/drivers/xenconsole.hh > @@ -9,13 +9,14 @@ > #define XEN_CONSOLE_HH > > #include "console-driver.hh" > -#include "exceptions.hh" > -#include <osv/interrupt.hh> > +#include <xen/interface/xen.h> > +#include <xen/interface/grant_table.h> > > namespace console { > > class XEN_Console : public console_driver { > public: > + XEN_Console(); > virtual void write(const char *str, size_t len); > virtual void flush(); > virtual bool input_ready(); > @@ -24,6 +25,15 @@ public: > private: > virtual void dev_start(); > virtual const char *thread_name() { return "xen-input"; } > + void handle_intr(); > + static void console_intr(void *arg) { > + static_cast<XEN_Console*>(arg)->handle_intr(); > + } > + > + struct xencons_interface *_interface; > + int _evtchn; > + unsigned int _irq; > + unsigned long _pfn; > }; > > } > -- > 2.7.4 > > -- > You received this message because you are subscribed to the Google Groups > "OSv Development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
