| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <564F2F49.8090801@linux.intel.com>
Date: Fri, 20 Nov 2015 16:33:45 +0200
From: Mathias Nyman <mathias.nyman@...ux.intel.com>
To: Lu Baolu <baolu.lu@...ux.intel.com>,
Mathias Nyman <mathias.nyman@...el.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC: linux-usb@...r.kernel.org, x86@...nel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 01/12] usb: xhci: add sysfs file for xHCI debug port
On 17.11.2015 08:38, Lu Baolu wrote:
> This patch adds a sysfs file for users to check 1) whether the debug
> capability is implemented by hardware; 2) if supported, which state
> does it stay at.
>
> With a host that supports debug port, a file named "debug_port_state"
> will be created under the device sysfs directory. Reading this file
> will show users the state (disabled, enabled or configured) of the
> debug port.
>
> With a host that does NOT support debug port, "debug_port_state" file
> won't be created.
>
> Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
> ---
> .../ABI/testing/sysfs-bus-pci-drivers-xhci_hcd | 23 +++++++
> drivers/usb/host/Makefile | 2 +-
> drivers/usb/host/xhci-ext-caps.h | 14 +++-
> drivers/usb/host/xhci-sysfs.c | 80 ++++++++++++++++++++++
> drivers/usb/host/xhci.c | 4 ++
> drivers/usb/host/xhci.h | 4 ++
> 6 files changed, 125 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> create mode 100644 drivers/usb/host/xhci-sysfs.c
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> new file mode 100644
> index 0000000..dd3e722
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-pci-drivers-xhci_hcd
> @@ -0,0 +1,23 @@
> +What: /sys/bus/pci/drivers/xhci_hcd/.../debug_port_state
> +Date: November 2015
> +KernelVersion: 4.3.0
> +Contact: Lu Baolu <baolu.lu@...ux.intel.com>
> +Description:
> + This file is designed for users to check the state of a
> + USB3 debug port. On a machine which supports USB3 debug
> + port, this file will be created. Reading this file will
> + show the state (disabled, enabled or configured) of the
> + debug port. On a machine that doesn't support USB3 debug
> + port, this file doesn't exist.
> +
> + The state of a debug port could be:
> + 1) disabled: The debug port is not enabled and the root
> + port has been switched to xHCI host as a normal
> + root port.
> + 2) enabled: The debug port is enabled. The debug port
> + has been assigned to debug capability. The debug
> + capability is able to handle the control requests
> + defined in USB3 spec.
> + 3) configured: The debug port has been enumerated by the
> + debug host as a debug device. The debug port is
> + in use now.
How much will this sysfs file be used, It looks more like debugging info needed while
developing xhci debug port capability.
Would it be enough to add something like "supports DbC" in one of the
dev_info() lines at xhci driver load if capability is supported?
This sysfs file will only be visible after xhci is loaded.
I understood that we would like to use the debug port capability even if xhci driver is not used,
or then for earlyprintk before xhci is loaded.
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index e7558ab..810c304 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -12,7 +12,7 @@ fhci-$(CONFIG_FHCI_DEBUG) += fhci-dbg.o
>
> xhci-hcd-y := xhci.o xhci-mem.o
> xhci-hcd-y += xhci-ring.o xhci-hub.o xhci-dbg.o
> -xhci-hcd-y += xhci-trace.o
> +xhci-hcd-y += xhci-trace.o xhci-sysfs.o
>
> xhci-plat-hcd-y := xhci-plat.o
> ifneq ($(CONFIG_USB_XHCI_MVEBU), )
> diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
> index 9fe3225..12c87e5 100644
> --- a/drivers/usb/host/xhci-ext-caps.h
> +++ b/drivers/usb/host/xhci-ext-caps.h
> @@ -49,8 +49,15 @@
> #define XHCI_EXT_CAPS_PM 3
> #define XHCI_EXT_CAPS_VIRT 4
> #define XHCI_EXT_CAPS_ROUTE 5
> -/* IDs 6-9 reserved */
> +#define XHCI_EXT_CAPS_LOCALMEM 6
> +/* IDs 7-9 reserved */
> #define XHCI_EXT_CAPS_DEBUG 10
> +/* IDs 192-255 vendor specific */
> +#define XHCI_EXT_CAPS_VEN_START 192
> +#define XHCI_EXT_CAPS_VEN_END 255
The CAPS_VEN_END is probably not needed,
all capabilities end at 255 (8 bit)
Perhaps just use the EXT_MAX_CAPID defined later
> +#define XHCI_EXT_CAPS_VENDOR(p) (((p) >= XHCI_EXT_CAPS_VEN_START) && \
> + ((p) <= XHCI_EXT_CAPS_VEN_END))
> +#define XHCI_EXT_MAX_CAPID XHCI_EXT_CAPS_VEN_END
> /* USB Legacy Support Capability - section 7.1.1 */
> #define XHCI_HC_BIOS_OWNED (1 << 16)
> #define XHCI_HC_OS_OWNED (1 << 24)
> @@ -73,6 +80,11 @@
> #define XHCI_HLC (1 << 19)
> #define XHCI_BLC (1 << 20)
>
> +/* Debug capability - section 7.6.8 */
> +#define XHCI_DBC_DCCTRL 0x20
> +#define XHCI_DBC_DCCTRL_DCR (1 << 0)
> +#define XHCI_DBC_DCCTRL_DCE (1 << 31)
> +
> /* command register values to disable interrupts and halt the HC */
> /* start/stop HC execution - do not write unless HC is halted*/
> #define XHCI_CMD_RUN (1 << 0)
> diff --git a/drivers/usb/host/xhci-sysfs.c b/drivers/usb/host/xhci-sysfs.c
> new file mode 100644
> index 0000000..365858f
> --- /dev/null
> +++ b/drivers/usb/host/xhci-sysfs.c
> @@ -0,0 +1,80 @@
> +/*
> + * sysfs interface for xHCI host controller driver
> + *
> + * Copyright (C) 2015 Intel Corp.
> + *
> + * Author: Lu Baolu <baolu.lu@...ux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/kernel.h>
> +
> +#include "xhci.h"
> +
> +/*
> + * Return the register offset of a extended capability specified
> + * by @cap_id. Return 0 if @cap_id capability is not supported or
> + * in error cases.
> + */
> +static int get_extended_capability_offset(struct xhci_hcd *xhci,
> + int cap_id)
> +{
> + int offset;
> + void __iomem *base = (void __iomem *) xhci->cap_regs;
> + struct usb_hcd *hcd = xhci_to_hcd(xhci);
> +
> + offset = xhci_find_next_cap_offset(base, XHCI_HCC_PARAMS_OFFSET);
> + if (!HCD_HW_ACCESSIBLE(hcd) || !offset)
> + return 0;
> +
> + return xhci_find_ext_cap_by_id(base, offset, cap_id);
> +}
After looking at how we are parsing the extended capabilities in xhci-mem.c and
pci-quirks.c I see that the current helpers are not that useful.
A new, generic one is needed.
I send a rework out shortly, this way we can avoid adding one more capability
parsing helper for this case.
-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists