Re: [PATCH RFC 3/9] console: add EHCI debug port based serial console

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

> +static void __init ehci_dbgp_init_preirq(struct serial_port *port)
> +{
> +    struct ehci_dbgp *dbgp = port->uart;
> +    u32 debug_port, offset;
> +    void __iomem *ehci_bar;
> +
> +    debug_port = pci_conf_read32(0, dbgp->bus, dbgp->slot, dbgp->func,
> +                                 dbgp->cap);
> +    offset = (debug_port >> 16) & 0xfff;
> +
> +    /* double check if the mem space is enabled */
> +    dbgp->pci_cr = pci_conf_read8(0, dbgp->bus, dbgp->slot, dbgp->func,
> +                                  PCI_COMMAND);
> +    if ( !(dbgp->pci_cr & PCI_COMMAND_MEMORY) )
> +    {
> +        dbgp->pci_cr |= PCI_COMMAND_MEMORY;
> +        pci_conf_write16(0, dbgp->bus, dbgp->slot, dbgp->func, PCI_COMMAND,
> +                         dbgp->pci_cr);
> +        dbgp_printk("MMIO for EHCI enabled\n");
> +    }
> +
> +    /*
> +     * FIXME I don't have the bar size so just guess PAGE_SIZE is more
> +     * than enough.  1k is the biggest that was seen.
> +     */
> +    set_fixmap_nocache(FIX_EHCI_DBGP, dbgp->bar_val);

Should this have dbgp->bar_val & PAGE_MASK ?

> +    ehci_bar = (void __iomem *)fix_to_virt(FIX_EHCI_DBGP);
> +    ehci_bar += dbgp->bar_val & ~PAGE_MASK;
> +    dbgp_printk("ehci_bar: %p\n", ehci_bar);
> +
> +    dbgp->ehci_caps = ehci_bar;
> +    dbgp->ehci_regs = ehci_bar +
> +                      HC_LENGTH(readl(&dbgp->ehci_caps->hc_capbase));
> +    dbgp->ehci_debug = ehci_bar + offset;
> +
> +    detect_set_debug_port(dbgp);
> +
> +    if ( ehci_dbgp_setup_preirq(dbgp) )
> +        ehci_dbgp_status(dbgp, "ehci_dbgp_init_preirq complete");
> +
> +    port->tx_fifo_size = DBGP_MAX_PACKET;
> +    dbgp->lock = &port->tx_lock;
> +}
> +
> +static void ehci_dbgp_setup_postirq(struct ehci_dbgp *dbgp)
> +{
> +    set_timer(&dbgp->timer, NOW() + MILLISECS(1));
> +}
> +
> +static void __init ehci_dbgp_init_postirq(struct serial_port *port)
> +{
> +    struct ehci_dbgp *dbgp = port->uart;
> +
> +    if ( !dbgp->ehci_debug )
> +        return;
> +
> +    serial_async_transmit(port);
> +
> +    init_timer(&dbgp->timer, ehci_dbgp_poll, port, 0);
> +
> +    ehci_dbgp_setup_postirq(dbgp);
> +}
> +
> +static int ehci_dbgp_check_release(struct ehci_dbgp *dbgp)
> +{
> +    struct ehci_dbg_port __iomem *ehci_debug = dbgp->ehci_debug;
> +    u32 ctrl;
> +    unsigned int i;
> +
> +    if ( !ehci_debug )
> +        return 0;
> +
> +    for ( i = 0; i < DBGP_MAX_PACKET; ++i )
> +        if ( dbgp->out.buf[i] )
> +            return 1;
> +
> +    /*
> +     * This means the console is not initialized, or should get shutdown
> +     * so as to allow for reuse of the USB device, which means it is time
> +     * to shutdown the USB debug port.
> +     */
> +    printk(XENLOG_INFO "Releasing EHCI debug port at %02x:%02x.%u\n",
> +           dbgp->bus, dbgp->slot, dbgp->func);
> +
> +    kill_timer(&dbgp->timer);
> +    dbgp->ehci_debug = NULL;
> +
> +    ctrl = readl(&ehci_debug->control);
> +    if ( ctrl & DBGP_ENABLED )
> +    {
> +        ctrl &= ~DBGP_CLAIM;
> +        writel(ctrl, &ehci_debug->control);
> +    }
> +
> +    return 0;
> +}
> +
> +static void __init ehci_dbgp_endboot(struct serial_port *port)
> +{
> +    ehci_dbgp_check_release(port->uart);

Would it make sense to debug dom0 access to this EHCI bar?

> +}
> +
> +static void ehci_dbgp_suspend(struct serial_port *port)
> +{
> +    struct ehci_dbgp *dbgp = port->uart;
> +
> +    if ( !dbgp->ehci_debug )
> +        return;
> +
> +    stop_timer(&dbgp->timer);
> +    dbgp->timer.expires = 0;
> +
> +    dbgp->pci_cr = pci_conf_read16(0, dbgp->bus, dbgp->slot, dbgp->func,
> +                                   PCI_COMMAND);
> +
> +    dbgp->state = dbgp_unsafe;
> +}
> +
> +static void ehci_dbgp_resume(struct serial_port *port)
> +{
> +    struct ehci_dbgp *dbgp = port->uart;
> +
> +    if ( !dbgp->ehci_debug )
> +        return;
> +
> +    pci_conf_write32(0, dbgp->bus, dbgp->slot, dbgp->func, dbgp->bar,
> +                     dbgp->bar_val);
> +    pci_conf_write16(0, dbgp->bus, dbgp->slot, dbgp->func,
> +                     PCI_COMMAND, dbgp->pci_cr);
> +
> +    ehci_dbgp_setup_preirq(dbgp);
> +    ehci_dbgp_setup_postirq(dbgp);
> +}
> +
> +static struct uart_driver __read_mostly ehci_dbgp_driver = {
> +    .init_preirq  = ehci_dbgp_init_preirq,
> +    .init_postirq = ehci_dbgp_init_postirq,
> +    .endboot      = ehci_dbgp_endboot,
> +    .suspend      = ehci_dbgp_suspend,
> +    .resume       = ehci_dbgp_resume,
> +    .tx_empty     = ehci_dbgp_tx_empty,
> +    .putc         = ehci_dbgp_putc,
> +    .flush        = ehci_dbgp_flush,
> +    .getc         = ehci_dbgp_getc
> +};
> +
> +static struct ehci_dbgp ehci_dbgp = { .state = dbgp_unsafe, .phys_port = 1 };
> +
> +static char __initdata opt_dbgp[30];
> +string_param("dbgp", opt_dbgp);
> +
> +void __init ehci_dbgp_init(void)
> +{
> +    struct ehci_dbgp *dbgp = &ehci_dbgp;
> +    u32 debug_port, offset, bar_val;
> +    const char *e;
> +
> +    if ( strncmp(opt_dbgp, "ehci", 4) )
> +        return;
> +
> +    if ( isdigit(opt_dbgp[4]) || !opt_dbgp[4] )
> +    {
> +        unsigned int num = 0;
> +
> +        if ( opt_dbgp[4] )
> +            simple_strtoul(opt_dbgp + 4, &e, 10);
> +
> +        dbgp->cap = find_dbgp(dbgp, num);
> +        if ( !dbgp->cap )
> +            return;
> +
> +        dbgp_printk("Found EHCI debug port on %02x:%02x.%u\n",
> +                    dbgp->bus, dbgp->slot, dbgp->func);
> +    }
> +    else if ( strncmp(opt_dbgp + 4, "@pci", 4) == 0 )
> +    {
> +        unsigned long val = simple_strtoul(opt_dbgp + 8, &e, 16);
> +
> +        dbgp->bus = val;
> +        if ( dbgp->bus != val || *e != ':' )
> +            return;
> +
> +        val = simple_strtoul(e + 1, &e, 16);
> +        if ( PCI_SLOT(PCI_DEVFN(val, 0)) != val || *e != '.' )
> +            return;
> +        dbgp->slot = val;
> +
> +        val = simple_strtoul(e + 1, &e, 16);
> +        if ( PCI_FUNC(PCI_DEVFN(0, val)) != val || *e )
> +            return;
> +        dbgp->func = val;
> +
> +        if ( !pci_device_detect(0, dbgp->bus, dbgp->slot, dbgp->func) )
> +            return;
> +
> +        dbgp->cap = __find_dbgp(dbgp->bus, dbgp->slot, dbgp->func);
> +        if ( !dbgp->cap )
> +            return;
> +
> +        dbgp_printk("Using EHCI debug port on %02x:%02x.%u\n",
> +                    dbgp->bus, dbgp->slot, dbgp->func);
> +    }
> +    else
> +        return;
> +
> +    debug_port = pci_conf_read32(0, dbgp->bus, dbgp->slot, dbgp->func,
> +                                 dbgp->cap);
> +    dbgp->bar = (debug_port >> 29) & 0x7;
> +    dbgp->bar = ((dbgp->bar - 1) * 4) + PCI_BASE_ADDRESS_0;
> +    offset = (debug_port >> 16) & 0xfff;
> +    dbgp_printk("bar: %02x offset: %03x\n", dbgp->bar, offset);
> +    if ( dbgp->bar < PCI_BASE_ADDRESS_0 || dbgp->bar > PCI_BASE_ADDRESS_5 )
> +    {
> +        dbgp_printk("unsupported/invalid bar\n");
> +        return;
> +    }
> +
> +    dbgp->bar_val = bar_val = pci_conf_read32(0, dbgp->bus, dbgp->slot,
> +                                              dbgp->func, dbgp->bar);
> +    dbgp_printk("bar_val: %08x\n", bar_val);
> +    if ( bar_val & ~PCI_BASE_ADDRESS_MEM_MASK )
> +    {
> +        dbgp_printk("only simple 32-bit MMIO BARs supported\n");
> +        return;
> +    }
> +    bar_val &= PCI_BASE_ADDRESS_MEM_MASK;
> +    if ( !bar_val || !(bar_val + (bar_val & -bar_val)) )
> +    {
> +        dbgp_printk("firmware initialization of MMIO BAR required\n");
> +        return;
> +    }
> +
> +    serial_register_uart(SERHND_DBGP, &ehci_dbgp_driver, dbgp);
> +}
> +
> +int dbgp_op(const struct physdev_dbgp_op *op)
> +{
> +    if ( !ehci_dbgp.ehci_debug )
> +        return 0;
> +
> +    switch ( op->bus )
> +    {
> +    case PHYSDEVOP_DBGP_BUS_UNKNOWN:
> +        break;
> +    case PHYSDEVOP_DBGP_BUS_PCI:
> +        if ( op->u.pci.seg || ehci_dbgp.bus != op->u.pci.bus ||
> +            PCI_DEVFN(ehci_dbgp.slot, ehci_dbgp.func) != op->u.pci.devfn )
> +    default:
> +            return 0;
> +        break;
> +    }
> +
> +    switch ( op->op )
> +    {
> +    case PHYSDEVOP_DBGP_RESET_PREPARE:a


Oh, you have a handoff protocol!

So how does this work without using this? Meaning if one uses Xen hypervisor
EHCI with a pvops kernel that does not implement this?

> +        spin_lock_irq(ehci_dbgp.lock);
> +        ehci_dbgp.state = dbgp_unsafe;
> +        dbgp_wait_until_complete(&ehci_dbgp, NULL);
> +        spin_unlock_irq(ehci_dbgp.lock);
> +
> +        return ehci_dbgp_check_release(&ehci_dbgp);
> +
> +    case PHYSDEVOP_DBGP_RESET_DONE:
> +        return ehci_dbgp_external_startup(&ehci_dbgp) ?: 1;

Oh, this is new. With the ?: it just passes on the return value from
ehci_dbgp_external_startup?


> +    }
> +
> +    return -ENOSYS;
> +}
> --- a/xen/drivers/char/serial.c
> +++ b/xen/drivers/char/serial.c
> @@ -265,6 +265,14 @@ int __init serial_parse_handle(char *con
>  {
>      int handle;
>  
> +    if ( !strncmp(conf, "dbgp", 4) && (!conf[4] || conf[4] == ',') )
> +    {
> +        if ( !com[SERHND_DBGP].driver )
> +            goto fail;
> +
> +        return SERHND_DBGP | SERHND_COOKED;
> +    }
> +
>      if ( strncmp(conf, "com", 3) )
>          goto fail;
>  
> --- a/xen/include/asm-x86/fixmap.h
> +++ b/xen/include/asm-x86/fixmap.h
> @@ -36,7 +36,15 @@
>   * from the end of virtual memory backwards.
>   */
>  enum fixed_addresses {
> -    FIX_RESERVED, /* Index 0 is reserved since fix_to_virt(0) > FIXADDR_TOP. */
> +    /* Index 0 is reserved since fix_to_virt(0) == FIXADDR_TOP. */
> +    FIX_RESERVED,
> +    /*
> +     * Indexes using the page tables set up before entering __start_xen()
> +     * must be among the first (L1_PAGETABLE_ENTRIES - 1) entries.
> +     * These are generally those needed by the various console drivers.
> +     */
> +    FIX_EHCI_DBGP,
> +    /* Everything else should go further down. */
>  #ifdef __i386__
>      FIX_PAE_HIGHMEM_0,
>      FIX_PAE_HIGHMEM_END = FIX_PAE_HIGHMEM_0 + NR_CPUS-1,
> --- a/xen/include/public/physdev.h
> +++ b/xen/include/public/physdev.h
> @@ -312,6 +312,24 @@ struct physdev_pci_device {
>  typedef struct physdev_pci_device physdev_pci_device_t;
>  DEFINE_XEN_GUEST_HANDLE(physdev_pci_device_t);
>  
> +#define PHYSDEVOP_DBGP_RESET_PREPARE    1
> +#define PHYSDEVOP_DBGP_RESET_DONE       2
> +
> +#define PHYSDEVOP_DBGP_BUS_UNKNOWN      0
> +#define PHYSDEVOP_DBGP_BUS_PCI          1
> +
> +#define PHYSDEVOP_dbgp_op               29
> +struct physdev_dbgp_op {
> +    /* IN */
> +    uint8_t op;
> +    uint8_t bus;
> +    union {
> +        struct physdev_pci_device pci;
> +    } u;
> +};
> +typedef struct physdev_dbgp_op physdev_dbgp_op_t;
> +DEFINE_XEN_GUEST_HANDLE(physdev_dbgp_op_t);
> +
>  /*
>   * Notify that some PIRQ-bound event channels have been unmasked.
>   * ** This command is obsolete since interface version 0x00030202 and is **
> --- a/xen/include/xen/serial.h
> +++ b/xen/include/xen/serial.h
> @@ -69,9 +69,10 @@ struct uart_driver {
>  };
>  
>  /* 'Serial handles' are composed from the following fields. */
> -#define SERHND_IDX      (3<<0) /* COM1 or COM2?                           */
> +#define SERHND_IDX      (3<<0) /* COM1, COM2, or DBGP?                    */
>  # define SERHND_COM1    (0<<0)
>  # define SERHND_COM2    (1<<0)
> +# define SERHND_DBGP    (2<<0)
>  #define SERHND_HI       (1<<2) /* Mux/demux each transferred char by MSB. */
>  #define SERHND_LO       (1<<3) /* Ditto, except that the MSB is cleared.  */
>  #define SERHND_COOKED   (1<<4) /* Newline/carriage-return translation?    */
> @@ -142,9 +143,13 @@ struct ns16550_defaults {
>      unsigned long io_base; /* default io_base address */
>  };
>  void ns16550_init(int index, struct ns16550_defaults *defaults);
> +void ehci_dbgp_init(void);
>  
>  void pl011_init(int index, unsigned long register_base_address);
>  
> +struct physdev_dbgp_op;
> +int dbgp_op(const struct physdev_dbgp_op *);
> +
>  /* Baud rate was pre-configured before invoking the UART driver. */
>  #define BAUD_AUTO (-1)
>  
> 
>