Re: [PATCH RFC 0/8] console improvements

[Keir Fraser <keir@xen.org>]

On 11/09/2012 11:11, "Jan Beulich" <JBeulich@suse.com> wrote:

> This series adds support for an EHCI debug port based console, and
> does some other improvements and cleanup noticed to be desirable
> during the implementation of the former as follow-up.
> 
> 1: x86: allow early use of fixmaps
> 2: console: prepare for non-COMn port support
> 3: console: add EHCI debug port based serial console
> 4: serial: avoid fully initializing unused consoles
> 5: ns16550: MMIO adjustments
> 6: ns16550: PCI initialization adjustments
> 7: ns16550: command line parsing adjustments
> 8: drop tx_fifo_size
> 
> Note that this also requires a Dom0 kernel side enhancement,
> which I'm reproducing below for reference (intending to submit
> this only when the public interface change on the hypervisor
> side is approved, which in turn will take its time due to the
> current feature freeze).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Keir Fraser <keir@xen.org>

> --- a/drivers/usb/early/ehci-dbgp.c
> +++ b/drivers/usb/early/ehci-dbgp.c
> @@ -491,7 +491,7 @@ static int ehci_wait_for_port(int port);
>   * Return -ENODEV for any general failure
>   * Return -EIO if wait for port fails
>   */
> -int dbgp_external_startup(void)
> +static int _dbgp_external_startup(void)
>  {
> int devnum;
> struct usb_debug_descriptor dbgp_desc;
> @@ -613,6 +613,11 @@ err:
> goto try_again;
> return -ENODEV;
>  }
> +
> +int dbgp_external_startup(struct usb_hcd *hcd)
> +{
> + return xen_dbgp_external_startup(hcd) ?: _dbgp_external_startup();
> +}
>  EXPORT_SYMBOL_GPL(dbgp_external_startup);
>  
>  static int ehci_reset_port(int port)
> @@ -804,7 +809,7 @@ try_next_port:
> dbgp_ehci_status("ehci skip - already configured");
> }
>  
> - ret = dbgp_external_startup();
> + ret = _dbgp_external_startup();
> if (ret == -EIO)
> goto next_debug_port;
>  
> @@ -934,7 +939,7 @@ static void early_dbgp_write(struct cons
> ctrl = readl(&ehci_debug->control);
> if (!(ctrl & DBGP_ENABLED)) {
> dbgp_not_safe = 1;
> -   dbgp_external_startup();
> +   _dbgp_external_startup();
> } else {
> cmd |= CMD_RUN;
> writel(cmd, &ehci_regs->command);
> @@ -974,10 +979,14 @@ struct console early_dbgp_console = {
> .index = -1,
>  };
>  
> -int dbgp_reset_prep(void)
> +int dbgp_reset_prep(struct usb_hcd *hcd)
>  {
> + int ret = xen_dbgp_reset_prep(hcd);
> u32 ctrl;
>  
> + if (ret)
> +  return ret;
> +
> dbgp_not_safe = 1;
> if (!ehci_debug)
> return 0;
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -321,7 +321,7 @@ static int ehci_reset (struct ehci_hcd *
>  
> /* If the EHCI debug controller is active, special care must be
> * taken before and after a host controller reset */
> - if (ehci->debug && !dbgp_reset_prep())
> + if (ehci->debug && !dbgp_reset_prep(ehci_to_hcd(ehci)))
> ehci->debug = NULL;
>  
> command |= CMD_RESET;
> @@ -345,7 +345,7 @@ static int ehci_reset (struct ehci_hcd *
> tdi_reset (ehci);
>  
> if (ehci->debug)
> -  dbgp_external_startup();
> +  dbgp_external_startup(ehci_to_hcd(ehci));
>  
> ehci->port_c_suspend = ehci->suspended_ports =
> ehci->resuming_ports = 0;
> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -347,10 +347,10 @@ static int ehci_bus_resume (struct usb_h
> }
>  
> if (unlikely(ehci->debug)) {
> -  if (!dbgp_reset_prep())
> +  if (!dbgp_reset_prep(hcd))
> ehci->debug = NULL;
> else
> -   dbgp_external_startup();
> +   dbgp_external_startup(hcd);
> }
>  
> /* Ideally and we've got a real resume here, and no port's power
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -21,6 +21,7 @@ CFLAGS_features.o   := $(nostackp)
>  endif
>  
>  priv-$(CONFIG_PCI)   := pci.o
> +priv-$(CONFIG_USB_SUPPORT)  += dbgp.o
>  
>  obj-$(CONFIG_XEN)   += features.o $(xen-backend-y) $(xen-backend-m)
>  obj-$(CONFIG_XEN_PRIVILEGED_GUEST) += $(priv-y)
> @@ -37,7 +38,7 @@ obj-$(CONFIG_XEN_SYS_HYPERVISOR) += sys-
>  obj-$(CONFIG_XEN_PVHVM)   += platform-pci.o
>  obj-$(CONFIG_XEN_TMEM)   += tmem.o
>  obj-$(CONFIG_SWIOTLB_XEN)  += swiotlb-xen.o
> -obj-$(CONFIG_XEN_DOM0)   += pci.o
> +obj-$(CONFIG_XEN_DOM0)   += pci.o dbgp.o
>  obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/
>  obj-$(CONFIG_XEN_PRIVCMD)  += $(xen-privcmd_y)
>  obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o
> --- /dev/null
> +++ b/drivers/xen/dbgp.c
> @@ -0,0 +1,48 @@
> +#include <linux/pci.h>
> +#include <linux/usb.h>
> +#include <linux/usb/ehci_def.h>
> +#include <linux/usb/hcd.h>
> +#include <asm/xen/hypercall.h>
> +#include <xen/interface/physdev.h>
> +#include <xen/xen.h>
> +
> +static int xen_dbgp_op(struct usb_hcd *hcd, int op)
> +{
> + const struct device *ctrlr = hcd_to_bus(hcd)->controller;
> + struct physdev_dbgp_op dbgp;
> +
> + if (!xen_initial_domain())
> +  return 0;
> +
> + dbgp.op = op;
> +
> +#ifdef CONFIG_PCI
> + if (ctrlr->bus == &pci_bus_type) {
> +  const struct pci_dev *pdev = to_pci_dev(ctrlr);
> +
> +  dbgp.u.pci.seg = pci_domain_nr(pdev->bus);
> +  dbgp.u.pci.bus = pdev->bus->number;
> +  dbgp.u.pci.devfn = pdev->devfn;
> +  dbgp.bus = PHYSDEVOP_DBGP_BUS_PCI;
> + } else
> +#endif
> +  dbgp.bus = PHYSDEVOP_DBGP_BUS_UNKNOWN;
> +
> + return HYPERVISOR_physdev_op(PHYSDEVOP_dbgp_op, &dbgp);
> +}
> +
> +int xen_dbgp_reset_prep(struct usb_hcd *hcd)
> +{
> + return xen_dbgp_op(hcd, PHYSDEVOP_DBGP_RESET_PREPARE);
> +}
> +
> +int xen_dbgp_external_startup(struct usb_hcd *hcd)
> +{
> + return xen_dbgp_op(hcd, PHYSDEVOP_DBGP_RESET_DONE);
> +}
> +
> +#ifndef CONFIG_EARLY_PRINTK_DBGP
> +#include <linux/export.h>
> +EXPORT_SYMBOL_GPL(xen_dbgp_reset_prep);
> +EXPORT_SYMBOL_GPL(xen_dbgp_external_startup);
> +#endif
> --- a/include/linux/usb/ehci_def.h
> +++ b/include/linux/usb/ehci_def.h
> @@ -207,18 +207,35 @@ extern int __init early_dbgp_init(char *
>  extern struct console early_dbgp_console;
>  #endif /* CONFIG_EARLY_PRINTK_DBGP */
>  
> +struct usb_hcd;
> +
> +#if defined(CONFIG_XEN_PRIVILEGED_GUEST) || defined(CONFIG_XEN_DOM0)
> +extern int xen_dbgp_reset_prep(struct usb_hcd *);
> +extern int xen_dbgp_external_startup(struct usb_hcd *);
> +#else
> +static inline int xen_dbgp_reset_prep(struct usb_hcd *hcd)
> +{
> + return 1; /* Shouldn't this be 0? */
> +}
> +
> +static inline int xen_dbgp_external_startup(struct usb_hcd *hcd)
> +{
> + return -1;
> +}
> +#endif
> +
>  #ifdef CONFIG_EARLY_PRINTK_DBGP
>  /* Call backs from ehci host driver to ehci debug driver */
> -extern int dbgp_external_startup(void);
> -extern int dbgp_reset_prep(void);
> +extern int dbgp_external_startup(struct usb_hcd *);
> +extern int dbgp_reset_prep(struct usb_hcd *hcd);
>  #else
> -static inline int dbgp_reset_prep(void)
> +static inline int dbgp_reset_prep(struct usb_hcd *hcd)
>  {
> - return 1;
> + return xen_dbgp_reset_prep(hcd);
>  }
> -static inline int dbgp_external_startup(void)
> +static inline int dbgp_external_startup(struct usb_hcd *hcd)
>  {
> - return -1;
> + return xen_dbgp_external_startup(hcd);
>  }
>  #endif
>  
> --- a/include/xen/interface/physdev.h
> +++ b/include/xen/interface/physdev.h
> @@ -307,6 +307,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 **
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel