Re: [Xen-devel] [PATCH RFC v1 34/74] x86/guest: add PV console code

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.01.18 at 14:05, <wei.liu2@xxxxxxxxxx> wrote:
> --- /dev/null
> +++ b/xen/drivers/char/xen_pv_console.c
> @@ -0,0 +1,198 @@
> +/******************************************************************************
> + * drivers/char/xen_pv_console.c
> + *
> + * A frontend driver for Xen's PV console.
> + * Can be used when Xen is running on top of Xen in pv-in-pvh mode.
> + * (Linux's name for this is hvc console)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Copyright (c) 2017 Citrix Systems Ltd.
> + */
> +
> +#include <xen/lib.h>
> +#include <xen/hypercall.h>
> +#include <xen/pv_console.h>
> +
> +#include <asm/fixmap.h>
> +#include <asm/guest.h>
> +
> +#include <public/io/console.h>
> +
> +static struct xencons_interface *cons_ring;
> +static evtchn_port_t cons_evtchn;
> +static serial_rx_fn cons_rx_handler;
> +static DEFINE_SPINLOCK(tx_lock);
> +
> +void __init pv_console_init(void)
> +{
> +    long r;
> +    uint64_t raw_pfn = 0, raw_evtchn = 0;
> +
> +    if ( !xen_guest )
> +    {
> +        printk("PV console init failed: xen_guest mode is not active!\n");
> +        return;
> +    }
> +
> +    r = xen_hypercall_hvm_get_param(HVM_PARAM_CONSOLE_PFN, &raw_pfn);
> +    if ( r < 0 )
> +        goto error;
> +
> +    r = xen_hypercall_hvm_get_param(HVM_PARAM_CONSOLE_EVTCHN, &raw_evtchn);
> +    if ( r < 0 )
> +        goto error;
> +
> +    set_fixmap(FIX_PV_CONSOLE, raw_pfn << PAGE_SHIFT);
> +    cons_ring = (struct xencons_interface *)fix_to_virt(FIX_PV_CONSOLE);

Pointless cast with the earlier return type change.

> +    cons_evtchn = raw_evtchn;
> +
> +    printk("Initialised PV console at 0x%p with pfn %#lx and evtchn %#x\n",

Does %#p not work?

> +void __init pv_console_set_rx_handler(serial_rx_fn fn)
> +{
> +    cons_rx_handler = fn;
> +}

Especially this and ...

> +size_t pv_console_rx(struct cpu_user_regs *regs)
> +{
> +    char c;
> +    XENCONS_RING_IDX cons, prod;
> +    size_t recv = 0;
> +
> +    if ( !cons_ring )
> +        return 0;
> +
> +    /* TODO: move this somewhere */
> +    if ( !test_bit(cons_evtchn, XEN_shared_info->evtchn_pending) )
> +        return 0;

... the need for this and ...

> +    prod = ACCESS_ONCE(cons_ring->in_prod);
> +    cons = cons_ring->in_cons;
> +    /* Get pointers before reading the ring */
> +    smp_rmb();
> +
> +    ASSERT((prod - cons) <= sizeof(cons_ring->in));
> +
> +    while ( cons != prod )
> +    {
> +        c = cons_ring->in[MASK_XENCONS_IDX(cons++, cons_ring->in)];
> +        if ( cons_rx_handler )
> +            cons_rx_handler(c, regs);
> +        recv++;
> +    }
> +
> +    /* No need for a mem barrier because every character was already 
> consumed */
> +    barrier();
> +    ACCESS_ONCE(cons_ring->in_cons) = cons;
> +    notify_daemon();
> +
> +    clear_bit(cons_evtchn, XEN_shared_info->evtchn_pending);

... this at this layer are very hard to judge about with all the code
here being dead for the moment. Can't this driver be modeled like
any other of the UART drivers, surfacing the accessors through
struct uart_driver (and making the ad-hoc call sites in the next
patch [mostly] unnecessary)?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel