On Fri, Mar 3, 2017 at 11:45 AM, 'Sergiy Kibrik' via OSv Development <
osv-dev@googlegroups.com> 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 <sergiy.kib...@globallogic.com>
> ---
> 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 osv-dev+unsubscr...@googlegroups.com.
> 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 osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
