Date   

Re: [PATCH 2/8] hv: extend the decode_modrm

Xu, Anthony
 

-----Original Message-----
From: acrn-dev@... [mailto:acrn-dev@...]
On Behalf Of Yin, Fengwei
Sent: Tuesday, August 14, 2018 6:04 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH 2/8] hv: extend the decode_modrm

If rm show there is no SIB following rm field, we should get
base_register info from rm.
---
hypervisor/arch/x86/guest/instr_emul.c | 42 +++++++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/hypervisor/arch/x86/guest/instr_emul.c
b/hypervisor/arch/x86/guest/instr_emul.c
index 222fa033..d62c6eaa 100644
--- a/hypervisor/arch/x86/guest/instr_emul.c
+++ b/hypervisor/arch/x86/guest/instr_emul.c
@@ -1652,6 +1652,11 @@ static int vie_init(struct instr_emul_vie *vie, struct
vcpu *vcpu)

(void)memset(vie, 0U, sizeof(struct instr_emul_vie));

+ /* init register fields in vie. */
+ vie->base_register = CPU_REG_LAST;
+ vie->index_register = CPU_REG_LAST;
+ vie->segment_register = CPU_REG_LAST;
+
err_code = PAGE_FAULT_ID_FLAG;
ret = copy_from_gva(vcpu, vie->inst, guest_rip_gva,
inst_len, &err_code, &fault_addr);
@@ -1877,6 +1882,41 @@ static int decode_modrm(struct instr_emul_vie
*vie, enum vm_cpu_mode cpu_mode)

vie->reg |= (vie->rex_r << 3);

+ /* SIB */
+ if (vie->mod != VIE_MOD_DIRECT && vie->rm == VIE_RM_SIB) {
+ goto done;
+ }
+
+ vie->base_register = vie->rm;
+
+ switch (vie->mod) {
+ case VIE_MOD_INDIRECT_DISP8:
+ vie->disp_bytes = 1U;
+ break;
+ case VIE_MOD_INDIRECT_DISP32:
+ vie->disp_bytes = 4U;
+ break;
decode_sib has the same logic, can we consolidate them?





+ case VIE_MOD_INDIRECT:
+ if (vie->rm == VIE_RM_DISP32) {
+ vie->disp_bytes = 4U;
+ /*
+ * Table 2-7. RIP-Relative Addressing
+ *
+ * In 64-bit mode mod=00 r/m=101 implies [rip] + disp32
+ * whereas in compatibility mode it just implies disp32.
+ */
+
+ if (cpu_mode == CPU_MODE_64BIT) {
+ vie->base_register = CPU_REG_RIP;
+ }
+ else {
+ vie->base_register = CPU_REG_LAST;
+ }
+ }
+ break;
+ }
+
+done:
vie_advance(vie);

return 0;
@@ -1953,7 +1993,7 @@ static int decode_sib(struct instr_emul_vie *vie)
}

/* 'scale' makes sense only in the context of an index register */
- if (vie->index_register <= CPU_REG_LAST) {
+ if (vie->index_register < CPU_REG_LAST) {
vie->scale = 1U << vie->ss;
}

--
2.17.0



Re: [PATCH 1/8] hv: add lock prefix check for exception

Xu, Anthony
 

Does this trigger GP in guest before triggering VM_Exit?

Anthony

-----Original Message-----
From: acrn-dev@... [mailto:acrn-dev@...]
On Behalf Of Yin, Fengwei
Sent: Tuesday, August 14, 2018 6:04 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH 1/8] hv: add lock prefix check for exception

According to SDM Vol2 - Instruction Set Reference:
Some instructions can't work with prefix lock or can't work in
some situations (destination is not a memory operand).

We add the lock prefix check for the instructions we support and
inject UD if lock shouldn't be used.

Signed-off-by: Yin Fengwei <fengwei.yin@...>
---
hypervisor/arch/x86/guest/instr_emul.c | 34 ++++++++++++++++++++++----
hypervisor/arch/x86/guest/instr_emul.h | 3 ++-
2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/hypervisor/arch/x86/guest/instr_emul.c
b/hypervisor/arch/x86/guest/instr_emul.c
index 521f0d36..222fa033 100644
--- a/hypervisor/arch/x86/guest/instr_emul.c
+++ b/hypervisor/arch/x86/guest/instr_emul.c
@@ -56,20 +56,24 @@
#define VIE_OP_F_MOFFSET (1U << 2) /* 16/32/64-bit immediate
moffset */
#define VIE_OP_F_NO_MODRM (1U << 3)
#define VIE_OP_F_NO_GLA_VERIFICATION (1U << 4)
+#define VIE_OP_F_NO_LOCK (1U << 5) /* UD if lock prefix is used
*/

static const struct instr_emul_vie_op two_byte_opcodes[256] = {
[0xB6] = {
.op_type = VIE_OP_TYPE_MOVZX,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0xB7] = {
.op_type = VIE_OP_TYPE_MOVZX,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0xBA] = {
.op_type = VIE_OP_TYPE_BITTEST,
- .op_flags = VIE_OP_F_IMM8,
+ .op_flags = VIE_OP_F_IMM8 | VIE_OP_F_NO_LOCK,
},
[0xBE] = {
.op_type = VIE_OP_TYPE_MOVSX,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
};

@@ -79,32 +83,41 @@ static const struct instr_emul_vie_op
one_byte_opcodes[256] = {
},
[0x2B] = {
.op_type = VIE_OP_TYPE_SUB,
+ .op_flags = VIE_OP_F_NO_LOCK, /* 0x2B has register as dst */
},
[0x39] = {
.op_type = VIE_OP_TYPE_CMP,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x3B] = {
.op_type = VIE_OP_TYPE_CMP,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x88] = {
.op_type = VIE_OP_TYPE_MOV,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x89] = {
.op_type = VIE_OP_TYPE_MOV,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x8A] = {
.op_type = VIE_OP_TYPE_MOV,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x8B] = {
.op_type = VIE_OP_TYPE_MOV,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0xA1] = {
.op_type = VIE_OP_TYPE_MOV,
- .op_flags = VIE_OP_F_MOFFSET | VIE_OP_F_NO_MODRM,
+ .op_flags = VIE_OP_F_MOFFSET | VIE_OP_F_NO_MODRM |
+ VIE_OP_F_NO_LOCK,
},
[0xA3] = {
.op_type = VIE_OP_TYPE_MOV,
- .op_flags = VIE_OP_F_MOFFSET | VIE_OP_F_NO_MODRM,
+ .op_flags = VIE_OP_F_MOFFSET | VIE_OP_F_NO_MODRM |
+ VIE_OP_F_NO_LOCK,
},
[0xA4] = {
.op_type = VIE_OP_TYPE_MOVS,
@@ -125,14 +138,15 @@ static const struct instr_emul_vie_op
one_byte_opcodes[256] = {
[0xC6] = {
/* XXX Group 11 extended opcode - not just MOV */
.op_type = VIE_OP_TYPE_MOV,
- .op_flags = VIE_OP_F_IMM8,
+ .op_flags = VIE_OP_F_IMM8 | VIE_OP_F_NO_LOCK,
},
[0xC7] = {
.op_type = VIE_OP_TYPE_MOV,
- .op_flags = VIE_OP_F_IMM,
+ .op_flags = VIE_OP_F_IMM | VIE_OP_F_NO_LOCK,
},
[0x23] = {
.op_type = VIE_OP_TYPE_AND,
+ .op_flags = VIE_OP_F_NO_LOCK, /* 0x23 has register as dst */
},
[0x80] = {
/* Group 1 extended opcode */
@@ -151,9 +165,11 @@ static const struct instr_emul_vie_op
one_byte_opcodes[256] = {
},
[0x84] = {
.op_type = VIE_OP_TYPE_TEST,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x85] = {
.op_type = VIE_OP_TYPE_TEST,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x08] = {
.op_type = VIE_OP_TYPE_OR,
@@ -1714,6 +1730,8 @@ static int decode_prefixes(struct instr_emul_vie *vie,
vie->repz_present = 1U;
} else if (x == 0xF2U) {
vie->repnz_present = 1U;
+ } else if (x == 0xF0U) {
+ vie->lock_prefix = 1U;
} else if (segment_override(x, &vie->segment_register)) {
vie->seg_override = 1U;
} else {
@@ -2114,6 +2132,12 @@ static int local_decode_instruction(enum
vm_cpu_mode cpu_mode,
return -1;
}

+ /* If lock is used with instruction doesn't allow lock,
+ * inject invalid opcode exception UD to guest.
+ */
+ if (vie->lock_prefix && (vie->op.op_flags & VIE_OP_F_NO_LOCK))
+ return -1;
+
vie->decoded = 1U; /* success */

return 0;
diff --git a/hypervisor/arch/x86/guest/instr_emul.h
b/hypervisor/arch/x86/guest/instr_emul.h
index 6132332e..a7a20bdb 100644
--- a/hypervisor/arch/x86/guest/instr_emul.h
+++ b/hypervisor/arch/x86/guest/instr_emul.h
@@ -103,7 +103,8 @@ struct instr_emul_vie {
repnz_present:1, /* REPNE/REPNZ prefix */
opsize_override:1, /* Operand size override */
addrsize_override:1, /* Address size override */
- seg_override:1; /* Segment override */
+ seg_override:1, /* Segment override */
+ lock_prefix:1; /* has LOCK prefix */

uint8_t mod:2, /* ModRM byte */
reg:4,
--
2.17.0



Re: [PATCH v2 8/8] DM USB: xHCI: enable xHCI SOS S3 support

Wu, Xiaoguang
 

On 18-08-14 23:17:02, Yu Wang wrote:
On 18-08-14 21:39:14, Wu, Xiaoguang wrote:
This patch enable the support for SOS S3 from the perspective
of USB xHCI.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index ec215a0..3509fbd 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -91,6 +91,7 @@
#include "pci_core.h"
#include "xhci.h"
#include "usb_pmapper.h"
+#include "vmmapi.h"

#undef LOG_TAG
#define LOG_TAG "xHCI: "
@@ -485,6 +486,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
struct usb_native_devinfo *di;
int vport_start, vport_end;
int port;
+ int need_intr = 1;

xdev = hci_data;

@@ -534,8 +536,11 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
xdev->port_map_tbl[di->bus][di->port] =
PT_MAKE_STATE(PT_ASSIGNED_WITH_NEW_DEV, port);

+ if (vm_get_suspend_mode() != VM_SUSPEND_NONE)
+ need_intr = 0;
+
/* Trigger port change event for the arriving device */
- if (pci_xhci_connect_port(xdev, port, di->speed, 1))
+ if (pci_xhci_connect_port(xdev, port, di->speed, need_intr))
UPRINTF(LFTL, "fail to report port event\n");

return 0;
@@ -552,6 +557,7 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
struct usb_dev *udev;
uint8_t port, slot, native_port;
uint8_t status;
+ int need_intr = 1;

assert(hci_data);
assert(dev_data);
@@ -594,8 +600,18 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
xdev->port_map_tbl[di.bus][di.port] = PT_MAKE_STATE(
PT_ASSIGNED_WITH_NO_DEV, port);

+ if (vm_get_suspend_mode() != VM_SUSPEND_NONE) {
+ XHCI_PORTREG_PTR(xdev, port)->portsc &= ~(XHCI_PS_CSC |
+ XHCI_PS_CCS | XHCI_PS_PED | XHCI_PS_PP);
+ edev->dev_slotstate = XHCI_ST_DISABLED;
+ xdev->devices[port] = NULL;
+ xdev->slots[slot] = NULL;
+ pci_xhci_dev_destroy(edev);
+ need_intr = 0;
+ }
Please add one TODO. We need revisit this logic in future. This patch is
like one band-aid fix.

xgwu:

okey, will change, thanks.


+
UPRINTF(LDBG, "report virtual port %d status\r\n", port);
- if (pci_xhci_disconnect_port(xdev, port, 1)) {
+ if (pci_xhci_disconnect_port(xdev, port, need_intr)) {
UPRINTF(LFTL, "fail to report event\r\n");
return -1;
}
--
2.7.4





Re: [PATCH v2 7/8] DM USB: xHCI: change flow of creation of virtual USB device

Wu, Xiaoguang
 

On 18-08-14 23:15:19, Yu Wang wrote:
On 18-08-14 21:39:13, Wu, Xiaoguang wrote:
The xHCI emulation greatly depends on the user space library libusb
which is based on the usbfs module in Linux kernel. The libusb will
bind usbfs to physical USB device which makes hardware control over
libusb in user space possible.

The pci_xhci_dev_create is called in pci_xhci_native_usb_dev_conn_cb
which is a callback function triggered by physical USB device plugging.
This function will bind the physical USB device to usbfs in SOS, which
we depend to create the communication between UOS xHCI driver with
physical USB device.

This design will fail if the reconnection happened in the SOS, which
will bind class driver to the physical USB device instead of usbfs,
hence the libusb device handle in DM is invalid.

Currently, the native S3 will disable the vbus for all xHCI ports and
re-drive during S3 resume. This behavior cause native USB driver unbind
the usbfs and bind to related class driver, then made the DM lost
control and failed to continue emulation.

To fix this issue, place the pci_xhci_dev_create in the function
pci_xhci_cmd_enable_slot. According to the xHCI spec 4.5.3 Figure 10,
the UOS always send Enable Slot command when a device is attached or
recovered from errors (by Disable Slot command). So every time the SOS
can't resuming normally or some unexpected disconnection happens, this
desigen will always survive by Disable Slot and Enable Slot command
series from UOS xHCI driver.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 197 ++++++++++++++++++++++++++++++----------------
1 file changed, 130 insertions(+), 67 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index c5fce1e..ec215a0 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -373,7 +373,10 @@ struct pci_xhci_vdev {
int (*excap_write)(struct pci_xhci_vdev *, uint64_t, uint64_t);
int usb2_port_start;
int usb3_port_start;
- uint8_t port_map_tbl[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
+
+ uint16_t port_map_tbl[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
+ struct usb_native_devinfo
+ native_dev_info[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
struct timespec mf_prev_time; /* previous time of accessing MFINDEX */
};

@@ -385,8 +388,16 @@ struct pci_xhci_vdev {
#define XHCI_GADDR(xdev, a) paddr_guest2host((xdev)->dev->vmctx, (a), \
XHCI_PADDR_SZ - ((a) & (XHCI_PADDR_SZ-1)))

+/* port mapping status */
#define PT_NOT_ASSIGNED (0)
-#define PT_ASSIGNED_WITH_NO_DEV (-1)
+#define PT_ASSIGNED_WITH_NO_DEV (1)
+#define PT_ASSIGNED_WITH_NEW_DEV (2)
+#define PT_ASSIGNED_WITH_EMU_DEV (3)
Just like the previous comment. These names are not good. Please change
to:
VPORT_FREE
VPORT_ASSIGNED
VPORT_CONNECTED
VPORT_EMULATED
xgwu:

ok, will change. thanks.

+
+/* helpers for get port mapping information */
+#define PT_GET_NUMBER(state) (state & 0xFF)
+#define PT_GET_STATUS(state) ((state >> 8) & 0xFF)
+#define PT_MAKE_STATE(status, num) (((status & 0xFF) << 8) | (num & 0xFF))
Don't use PT. The PT in hypervisor is means passthrough... It is easy
confused..

#define VPORT_NUM(val) (val & 0xFF)
#define VPORT_STATE(val) ((val >> 8) & 0xFF)
#define VPORT_SET_CONFIG(state, num) (((state & 0xFF) << 8) | (num & 0xFF))
xgwu:

will change to:

VPORT_GET_NUMBER
VPORT_GET_STATUS
VPORT_MAKE_STATE ---> this one just 'make', do not 'set'

this means STATE = STATUS + NUMBER




struct pci_xhci_option_elem {
char *parse_opt;
@@ -470,83 +481,58 @@ static struct pci_xhci_option_elem xhci_option_table[] = {
static int
pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
{
- struct pci_xhci_dev_emu *de;
struct pci_xhci_vdev *xdev;
- struct usb_devemu *ue;
struct usb_native_devinfo *di;
- int port_start, port_end;
- int slot_start, slot_end;
- int port, slot;
- void *ud;
+ int vport_start, vport_end;
+ int port;

xdev = hci_data;
- di = dev_data;

assert(xdev);
assert(dev_data);
assert(xdev->devices);
assert(xdev->slots);

- de = pci_xhci_dev_create(xdev, di);
- if (!de) {
- UPRINTF(LFTL, "fail to create device\r\n");
- return -1;
- }
-
- /* find free port and slot for the new usb device */
- ud = de->dev_instance;
- ue = de->dev_ue;
-
- assert(ud);
- assert(ue);
+ di = dev_data;

/* print physical information about new device */
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
di->vid, di->pid, di->bus, di->port);

- if (xdev->port_map_tbl[di->bus][di->port] == PT_NOT_ASSIGNED) {
+ if (PT_GET_STATUS(xdev->port_map_tbl[di->bus][di->port]) ==
+ PT_NOT_ASSIGNED) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
"\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
}
-
UPRINTF(LDBG, "%04x:%04x %d-%d belong to this vm.\r\n", di->vid,
di->pid, di->bus, di->port);

- if (di->bcd < 0x300)
- port_start = xdev->usb2_port_start;
- else
- port_start = xdev->usb3_port_start;
-
- slot_start = 1;
- port_end = port_start + (XHCI_MAX_DEVS / 2);
- slot_end = XHCI_MAX_SLOTS;
+ if (di->bcd < 0x300) {
+ vport_start = xdev->usb2_port_start;
+ vport_end = vport_start + (XHCI_MAX_DEVS / 2);
+ } else {
+ vport_start = xdev->usb3_port_start;
+ vport_end = vport_start + (XHCI_MAX_DEVS / 2);
+ }

/* find free port */
- for (port = port_start; port < port_end; port++)
+ for (port = vport_start; port < vport_end; port++)
if (!xdev->devices[port])
break;

- /* find free slot */
- for (slot = slot_start; slot < slot_end; slot++)
- if (!xdev->slots[slot])
- break;
-
- if (port >= port_end || slot >= slot_end) {
- UPRINTF(LFTL, "no free resource: port %d slot %d\r\n",
- port, slot);
+ if (port >= vport_end) {
+ UPRINTF(LFTL, "no free virtual port for native device %d-%d"
+ "\r\n", di->bus, di->port);
goto errout;
}

- /* use index of devices as port number */
- xdev->devices[port] = de;
- xdev->slots[slot] = de;
- xdev->ndevices++;
+ UPRINTF(LDBG, "%04X:%04X %d-%d is attached to virtual port %d.\r\n",
+ di->vid, di->pid, di->bus, di->port, port);

- pci_xhci_reset_slot(xdev, slot);
- UPRINTF(LDBG, "%X:%X %d-%d locates in slot %d port %d.\r\n",
- di->vid, di->pid, di->bus, di->port,
- slot, port);
+ xdev->native_dev_info[di->bus][di->port] = *di;
+ xdev->port_map_tbl[di->bus][di->port] =
+ PT_MAKE_STATE(PT_ASSIGNED_WITH_NEW_DEV, port);

/* Trigger port change event for the arriving device */
if (pci_xhci_connect_port(xdev, port, di->speed, 1))
@@ -554,7 +540,6 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)

return 0;
errout:
- pci_xhci_dev_destroy(de);
return -1;
}

@@ -563,8 +548,10 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
{
struct pci_xhci_vdev *xdev;
struct pci_xhci_dev_emu *edev;
+ struct usb_native_devinfo di;
struct usb_dev *udev;
- uint8_t port, native_port;
+ uint8_t port, slot, native_port;
+ uint8_t status;

assert(hci_data);
assert(dev_data);
@@ -584,8 +571,10 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
continue;

udev = edev->dev_instance;
- if (udev->info.port == native_port)
+ if (udev->info.port == native_port) {
+ di = udev->info;
break;
+ }
}

if (port == XHCI_MAX_DEVS + 1) {
@@ -593,6 +582,18 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
return -1;
}

+ for (slot = 1; slot < XHCI_MAX_SLOTS; ++slot)
+ if (xdev->slots[slot] == edev)
+ break;
+
+ assert(slot < USB_NATIVE_NUM_BUS);
+
+ status = PT_GET_STATUS(xdev->port_map_tbl[di.bus][di.port]);
+ assert(status == PT_ASSIGNED_WITH_EMU_DEV ||
+ status == PT_ASSIGNED_WITH_NEW_DEV);
Let's unify all status to state.. Don't mix them..

+ xdev->port_map_tbl[di.bus][di.port] = PT_MAKE_STATE(
+ PT_ASSIGNED_WITH_NO_DEV, port);
+
UPRINTF(LDBG, "report virtual port %d status\r\n", port);
if (pci_xhci_disconnect_port(xdev, port, 1)) {
UPRINTF(LFTL, "fail to report event\r\n");
@@ -1393,28 +1394,67 @@ done:
return err;
}

+static struct usb_native_devinfo *
+pci_xhci_find_native_devinfo(struct pci_xhci_vdev *xdev)
+{
+ int i, j;
+
+ assert(xdev);
+ for (i = 0; i < USB_NATIVE_NUM_BUS; ++i)
+ for (j = 0; j < USB_NATIVE_NUM_PORT; ++j)
+ if (PT_GET_STATUS(xdev->port_map_tbl[i][j]) ==
+ PT_ASSIGNED_WITH_NEW_DEV)
+ return &xdev->native_dev_info[i][j];
+
+ return NULL;
+}
+
static uint32_t
pci_xhci_cmd_enable_slot(struct pci_xhci_vdev *xdev, uint32_t *slot)
{
struct pci_xhci_dev_emu *dev;
- uint32_t cmderr;
- int i;
+ uint32_t cmderr;
+ struct usb_native_devinfo *di;
+ int i, vport;

cmderr = XHCI_TRB_ERROR_NO_SLOTS;
- if (xdev->portregs != NULL)
- for (i = 1; i <= XHCI_MAX_SLOTS; i++) {
- dev = XHCI_SLOTDEV_PTR(xdev, i);
- if (dev && dev->dev_slotstate == XHCI_ST_DISABLED) {
- *slot = i;
- dev->dev_slotstate = XHCI_ST_ENABLED;
- cmderr = XHCI_TRB_ERROR_SUCCESS;
- dev->hci.hci_address = i;
- break;
- }
+
+ di = pci_xhci_find_native_devinfo(xdev);
+ if (!di) {
+ UPRINTF(LWRN, "unexpected Enable Slot commnad\r\n");
+ return -1;
+ }
+
+ assert(di->priv_data);
+ dev = pci_xhci_dev_create(xdev, di);
+ if (!dev) {
+ UPRINTF(LFTL, "fail to create device\r\n");
+ return -1;
+ }
+
+ vport = PT_GET_NUMBER(xdev->port_map_tbl[di->bus][di->port]);
+ assert(vport > 0);
+ assert(!xdev->devices[vport]);
+
+ xdev->devices[vport] = dev;
+ xdev->ndevices++;
+
+ for (i = 1; i <= XHCI_MAX_SLOTS; i++) {
+ if (XHCI_SLOTDEV_PTR(xdev, i) == NULL) {
+ xdev->slots[i] = dev;
+ *slot = i;
+ dev->dev_slotstate = XHCI_ST_ENABLED;
+ cmderr = XHCI_TRB_ERROR_SUCCESS;
+ dev->hci.hci_address = i;
+ xdev->port_map_tbl[di->bus][di->port] =
+ PT_MAKE_STATE(PT_ASSIGNED_WITH_EMU_DEV, vport);
+ break;
}
+ }

- UPRINTF(LDBG, "enable slot (error=%d) slot %u\r\n",
- cmderr != XHCI_TRB_ERROR_SUCCESS, *slot);
+ UPRINTF(LDBG, "enable slot (error=%d) slot %u for native device "
+ "%d-%d\r\n", cmderr != XHCI_TRB_ERROR_SUCCESS, *slot,
+ di->bus, di->port);

return cmderr;
}
@@ -1423,6 +1463,8 @@ static uint32_t
pci_xhci_cmd_disable_slot(struct pci_xhci_vdev *xdev, uint32_t slot)
{
struct pci_xhci_dev_emu *dev;
+ struct usb_dev *udev;
+ struct usb_native_devinfo *di;
uint32_t cmderr;
int i;

@@ -1446,6 +1488,9 @@ pci_xhci_cmd_disable_slot(struct pci_xhci_vdev *xdev, uint32_t slot)
cmderr = XHCI_TRB_ERROR_SUCCESS;
/* TODO: reset events and endpoints */
}
+ } else {
+ UPRINTF(LDBG, "disable NULL device, slot %d\r\n", slot);
+ goto done;
}

for (i = 1; i <= XHCI_MAX_DEVS; ++i)
@@ -1455,8 +1500,20 @@ pci_xhci_cmd_disable_slot(struct pci_xhci_vdev *xdev, uint32_t slot)
if (i <= XHCI_MAX_DEVS && XHCI_PORTREG_PTR(xdev, i)) {
XHCI_PORTREG_PTR(xdev, i)->portsc &= ~(XHCI_PS_CSC |
XHCI_PS_CCS | XHCI_PS_PED | XHCI_PS_PP);
+
+ udev = dev->dev_instance;
+ assert(udev);
+
xdev->devices[i] = NULL;
xdev->slots[slot] = NULL;
+
+ di = &udev->info;
+ xdev->port_map_tbl[di->bus][di->port] =
+ PT_MAKE_STATE(PT_ASSIGNED_WITH_NO_DEV, 0);
+
+ UPRINTF(LINF, "disable slot %d for native device %d-%d"
+ "\r\n", slot, di->bus, di->port);
+
pci_xhci_dev_destroy(dev);
} else
UPRINTF(LWRN, "invalid slot %d\r\n", slot);
@@ -1624,7 +1681,10 @@ pci_xhci_cmd_config_ep(struct pci_xhci_vdev *xdev,
UPRINTF(LDBG, "config_ep slot %u\r\n", slot);

dev = XHCI_SLOTDEV_PTR(xdev, slot);
- assert(dev != NULL);
+ if (dev == NULL) {
+ cmderr = XHCI_TRB_ERROR_SLOT_NOT_ON;
+ goto done;
+ }

if ((trb->dwTrb3 & XHCI_TRB_3_DCEP_BIT) != 0) {
UPRINTF(LDBG, "config_ep - deconfigure ep slot %u\r\n", slot);
@@ -3364,7 +3424,10 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
goto errout;
}

- xdev->port_map_tbl[bus][port] = PT_ASSIGNED_WITH_NO_DEV;
+ xdev->port_map_tbl[bus][port] =
+ PT_MAKE_STATE(PT_ASSIGNED_WITH_NO_DEV, 0);
+ return 0;
+
errout:
if (rc)
UPRINTF(LWRN, "%s fails, rc=%d\r\n", __func__, rc);
--
2.7.4





Re: [PATCH v2 5/8] DM USB: xHCI: refine port assignment logic

Wu, Xiaoguang
 

On 18-08-14 22:59:18, Yu Wang wrote:
On 18-08-14 21:39:11, Wu, Xiaoguang wrote:
The variable native_assign_ports in struct pci_xhci_vdev is used
to record wether certain root hub port in SOS is assigned to UOS.
The logic uses zero to express 'not assigned' and nonzero to express
'assigned'. In this patch, use macro to replace number to express
better.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index 14009a8..b5d8555 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -384,6 +384,10 @@ struct pci_xhci_vdev {
#define XHCI_HALTED(xdev) ((xdev)->opregs.usbsts & XHCI_STS_HCH)
#define XHCI_GADDR(xdev, a) paddr_guest2host((xdev)->dev->vmctx, (a), \
XHCI_PADDR_SZ - ((a) & (XHCI_PADDR_SZ-1)))
+
+#define PT_NOT_ASSIGNED (0)
+#define PT_ASSIGNED_WITH_NO_DEV (-1)
The name is not good.

How about:
VPORT_FREE
VPORT_ASSIGNED
VPORT_CONNECTED

In your case, the VPORT_CONNECTED is > 0.
xgwu:

ok, will change, thanks.



+
struct pci_xhci_option_elem {
char *parse_opt;
int (*parse_fn)(struct pci_xhci_vdev *, char *);
@@ -500,7 +504,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
di->vid, di->pid, di->bus, di->port);

- if (!xdev->native_assign_ports[di->bus][di->port]) {
+ if (xdev->native_assign_ports[di->bus][di->port] == PT_NOT_ASSIGNED) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
"\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
@@ -3360,7 +3364,7 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
goto errout;
}

- xdev->native_assign_ports[bus][port] = 1;
+ xdev->native_assign_ports[bus][port] = PT_ASSIGNED_WITH_NO_DEV;
errout:
if (rc)
UPRINTF(LWRN, "%s fails, rc=%d\r\n", __func__, rc);
--
2.7.4





Re: [PATCH v2 8/8] DM USB: xHCI: enable xHCI SOS S3 support

Yu Wang
 

On 18-08-14 21:39:14, Wu, Xiaoguang wrote:
This patch enable the support for SOS S3 from the perspective
of USB xHCI.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index ec215a0..3509fbd 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -91,6 +91,7 @@
#include "pci_core.h"
#include "xhci.h"
#include "usb_pmapper.h"
+#include "vmmapi.h"

#undef LOG_TAG
#define LOG_TAG "xHCI: "
@@ -485,6 +486,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
struct usb_native_devinfo *di;
int vport_start, vport_end;
int port;
+ int need_intr = 1;

xdev = hci_data;

@@ -534,8 +536,11 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
xdev->port_map_tbl[di->bus][di->port] =
PT_MAKE_STATE(PT_ASSIGNED_WITH_NEW_DEV, port);

+ if (vm_get_suspend_mode() != VM_SUSPEND_NONE)
+ need_intr = 0;
+
/* Trigger port change event for the arriving device */
- if (pci_xhci_connect_port(xdev, port, di->speed, 1))
+ if (pci_xhci_connect_port(xdev, port, di->speed, need_intr))
UPRINTF(LFTL, "fail to report port event\n");

return 0;
@@ -552,6 +557,7 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
struct usb_dev *udev;
uint8_t port, slot, native_port;
uint8_t status;
+ int need_intr = 1;

assert(hci_data);
assert(dev_data);
@@ -594,8 +600,18 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
xdev->port_map_tbl[di.bus][di.port] = PT_MAKE_STATE(
PT_ASSIGNED_WITH_NO_DEV, port);

+ if (vm_get_suspend_mode() != VM_SUSPEND_NONE) {
+ XHCI_PORTREG_PTR(xdev, port)->portsc &= ~(XHCI_PS_CSC |
+ XHCI_PS_CCS | XHCI_PS_PED | XHCI_PS_PP);
+ edev->dev_slotstate = XHCI_ST_DISABLED;
+ xdev->devices[port] = NULL;
+ xdev->slots[slot] = NULL;
+ pci_xhci_dev_destroy(edev);
+ need_intr = 0;
+ }
Please add one TODO. We need revisit this logic in future. This patch is
like one band-aid fix.

+
UPRINTF(LDBG, "report virtual port %d status\r\n", port);
- if (pci_xhci_disconnect_port(xdev, port, 1)) {
+ if (pci_xhci_disconnect_port(xdev, port, need_intr)) {
UPRINTF(LFTL, "fail to report event\r\n");
return -1;
}
--
2.7.4




Re: [PATCH v2 7/8] DM USB: xHCI: change flow of creation of virtual USB device

Yu Wang
 

On 18-08-14 21:39:13, Wu, Xiaoguang wrote:
The xHCI emulation greatly depends on the user space library libusb
which is based on the usbfs module in Linux kernel. The libusb will
bind usbfs to physical USB device which makes hardware control over
libusb in user space possible.

The pci_xhci_dev_create is called in pci_xhci_native_usb_dev_conn_cb
which is a callback function triggered by physical USB device plugging.
This function will bind the physical USB device to usbfs in SOS, which
we depend to create the communication between UOS xHCI driver with
physical USB device.

This design will fail if the reconnection happened in the SOS, which
will bind class driver to the physical USB device instead of usbfs,
hence the libusb device handle in DM is invalid.

Currently, the native S3 will disable the vbus for all xHCI ports and
re-drive during S3 resume. This behavior cause native USB driver unbind
the usbfs and bind to related class driver, then made the DM lost
control and failed to continue emulation.

To fix this issue, place the pci_xhci_dev_create in the function
pci_xhci_cmd_enable_slot. According to the xHCI spec 4.5.3 Figure 10,
the UOS always send Enable Slot command when a device is attached or
recovered from errors (by Disable Slot command). So every time the SOS
can't resuming normally or some unexpected disconnection happens, this
desigen will always survive by Disable Slot and Enable Slot command
series from UOS xHCI driver.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 197 ++++++++++++++++++++++++++++++----------------
1 file changed, 130 insertions(+), 67 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index c5fce1e..ec215a0 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -373,7 +373,10 @@ struct pci_xhci_vdev {
int (*excap_write)(struct pci_xhci_vdev *, uint64_t, uint64_t);
int usb2_port_start;
int usb3_port_start;
- uint8_t port_map_tbl[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
+
+ uint16_t port_map_tbl[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
+ struct usb_native_devinfo
+ native_dev_info[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
struct timespec mf_prev_time; /* previous time of accessing MFINDEX */
};

@@ -385,8 +388,16 @@ struct pci_xhci_vdev {
#define XHCI_GADDR(xdev, a) paddr_guest2host((xdev)->dev->vmctx, (a), \
XHCI_PADDR_SZ - ((a) & (XHCI_PADDR_SZ-1)))

+/* port mapping status */
#define PT_NOT_ASSIGNED (0)
-#define PT_ASSIGNED_WITH_NO_DEV (-1)
+#define PT_ASSIGNED_WITH_NO_DEV (1)
+#define PT_ASSIGNED_WITH_NEW_DEV (2)
+#define PT_ASSIGNED_WITH_EMU_DEV (3)
Just like the previous comment. These names are not good. Please change
to:
VPORT_FREE
VPORT_ASSIGNED
VPORT_CONNECTED
VPORT_EMULATED

+
+/* helpers for get port mapping information */
+#define PT_GET_NUMBER(state) (state & 0xFF)
+#define PT_GET_STATUS(state) ((state >> 8) & 0xFF)
+#define PT_MAKE_STATE(status, num) (((status & 0xFF) << 8) | (num & 0xFF))
Don't use PT. The PT in hypervisor is means passthrough... It is easy
confused..

#define VPORT_NUM(val) (val & 0xFF)
#define VPORT_STATE(val) ((val >> 8) & 0xFF)
#define VPORT_SET_CONFIG(state, num) (((state & 0xFF) << 8) | (num & 0xFF))


struct pci_xhci_option_elem {
char *parse_opt;
@@ -470,83 +481,58 @@ static struct pci_xhci_option_elem xhci_option_table[] = {
static int
pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
{
- struct pci_xhci_dev_emu *de;
struct pci_xhci_vdev *xdev;
- struct usb_devemu *ue;
struct usb_native_devinfo *di;
- int port_start, port_end;
- int slot_start, slot_end;
- int port, slot;
- void *ud;
+ int vport_start, vport_end;
+ int port;

xdev = hci_data;
- di = dev_data;

assert(xdev);
assert(dev_data);
assert(xdev->devices);
assert(xdev->slots);

- de = pci_xhci_dev_create(xdev, di);
- if (!de) {
- UPRINTF(LFTL, "fail to create device\r\n");
- return -1;
- }
-
- /* find free port and slot for the new usb device */
- ud = de->dev_instance;
- ue = de->dev_ue;
-
- assert(ud);
- assert(ue);
+ di = dev_data;

/* print physical information about new device */
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
di->vid, di->pid, di->bus, di->port);

- if (xdev->port_map_tbl[di->bus][di->port] == PT_NOT_ASSIGNED) {
+ if (PT_GET_STATUS(xdev->port_map_tbl[di->bus][di->port]) ==
+ PT_NOT_ASSIGNED) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
"\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
}
-
UPRINTF(LDBG, "%04x:%04x %d-%d belong to this vm.\r\n", di->vid,
di->pid, di->bus, di->port);

- if (di->bcd < 0x300)
- port_start = xdev->usb2_port_start;
- else
- port_start = xdev->usb3_port_start;
-
- slot_start = 1;
- port_end = port_start + (XHCI_MAX_DEVS / 2);
- slot_end = XHCI_MAX_SLOTS;
+ if (di->bcd < 0x300) {
+ vport_start = xdev->usb2_port_start;
+ vport_end = vport_start + (XHCI_MAX_DEVS / 2);
+ } else {
+ vport_start = xdev->usb3_port_start;
+ vport_end = vport_start + (XHCI_MAX_DEVS / 2);
+ }

/* find free port */
- for (port = port_start; port < port_end; port++)
+ for (port = vport_start; port < vport_end; port++)
if (!xdev->devices[port])
break;

- /* find free slot */
- for (slot = slot_start; slot < slot_end; slot++)
- if (!xdev->slots[slot])
- break;
-
- if (port >= port_end || slot >= slot_end) {
- UPRINTF(LFTL, "no free resource: port %d slot %d\r\n",
- port, slot);
+ if (port >= vport_end) {
+ UPRINTF(LFTL, "no free virtual port for native device %d-%d"
+ "\r\n", di->bus, di->port);
goto errout;
}

- /* use index of devices as port number */
- xdev->devices[port] = de;
- xdev->slots[slot] = de;
- xdev->ndevices++;
+ UPRINTF(LDBG, "%04X:%04X %d-%d is attached to virtual port %d.\r\n",
+ di->vid, di->pid, di->bus, di->port, port);

- pci_xhci_reset_slot(xdev, slot);
- UPRINTF(LDBG, "%X:%X %d-%d locates in slot %d port %d.\r\n",
- di->vid, di->pid, di->bus, di->port,
- slot, port);
+ xdev->native_dev_info[di->bus][di->port] = *di;
+ xdev->port_map_tbl[di->bus][di->port] =
+ PT_MAKE_STATE(PT_ASSIGNED_WITH_NEW_DEV, port);

/* Trigger port change event for the arriving device */
if (pci_xhci_connect_port(xdev, port, di->speed, 1))
@@ -554,7 +540,6 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)

return 0;
errout:
- pci_xhci_dev_destroy(de);
return -1;
}

@@ -563,8 +548,10 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
{
struct pci_xhci_vdev *xdev;
struct pci_xhci_dev_emu *edev;
+ struct usb_native_devinfo di;
struct usb_dev *udev;
- uint8_t port, native_port;
+ uint8_t port, slot, native_port;
+ uint8_t status;

assert(hci_data);
assert(dev_data);
@@ -584,8 +571,10 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
continue;

udev = edev->dev_instance;
- if (udev->info.port == native_port)
+ if (udev->info.port == native_port) {
+ di = udev->info;
break;
+ }
}

if (port == XHCI_MAX_DEVS + 1) {
@@ -593,6 +582,18 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
return -1;
}

+ for (slot = 1; slot < XHCI_MAX_SLOTS; ++slot)
+ if (xdev->slots[slot] == edev)
+ break;
+
+ assert(slot < USB_NATIVE_NUM_BUS);
+
+ status = PT_GET_STATUS(xdev->port_map_tbl[di.bus][di.port]);
+ assert(status == PT_ASSIGNED_WITH_EMU_DEV ||
+ status == PT_ASSIGNED_WITH_NEW_DEV);
Let's unify all status to state.. Don't mix them..

+ xdev->port_map_tbl[di.bus][di.port] = PT_MAKE_STATE(
+ PT_ASSIGNED_WITH_NO_DEV, port);
+
UPRINTF(LDBG, "report virtual port %d status\r\n", port);
if (pci_xhci_disconnect_port(xdev, port, 1)) {
UPRINTF(LFTL, "fail to report event\r\n");
@@ -1393,28 +1394,67 @@ done:
return err;
}

+static struct usb_native_devinfo *
+pci_xhci_find_native_devinfo(struct pci_xhci_vdev *xdev)
+{
+ int i, j;
+
+ assert(xdev);
+ for (i = 0; i < USB_NATIVE_NUM_BUS; ++i)
+ for (j = 0; j < USB_NATIVE_NUM_PORT; ++j)
+ if (PT_GET_STATUS(xdev->port_map_tbl[i][j]) ==
+ PT_ASSIGNED_WITH_NEW_DEV)
+ return &xdev->native_dev_info[i][j];
+
+ return NULL;
+}
+
static uint32_t
pci_xhci_cmd_enable_slot(struct pci_xhci_vdev *xdev, uint32_t *slot)
{
struct pci_xhci_dev_emu *dev;
- uint32_t cmderr;
- int i;
+ uint32_t cmderr;
+ struct usb_native_devinfo *di;
+ int i, vport;

cmderr = XHCI_TRB_ERROR_NO_SLOTS;
- if (xdev->portregs != NULL)
- for (i = 1; i <= XHCI_MAX_SLOTS; i++) {
- dev = XHCI_SLOTDEV_PTR(xdev, i);
- if (dev && dev->dev_slotstate == XHCI_ST_DISABLED) {
- *slot = i;
- dev->dev_slotstate = XHCI_ST_ENABLED;
- cmderr = XHCI_TRB_ERROR_SUCCESS;
- dev->hci.hci_address = i;
- break;
- }
+
+ di = pci_xhci_find_native_devinfo(xdev);
+ if (!di) {
+ UPRINTF(LWRN, "unexpected Enable Slot commnad\r\n");
+ return -1;
+ }
+
+ assert(di->priv_data);
+ dev = pci_xhci_dev_create(xdev, di);
+ if (!dev) {
+ UPRINTF(LFTL, "fail to create device\r\n");
+ return -1;
+ }
+
+ vport = PT_GET_NUMBER(xdev->port_map_tbl[di->bus][di->port]);
+ assert(vport > 0);
+ assert(!xdev->devices[vport]);
+
+ xdev->devices[vport] = dev;
+ xdev->ndevices++;
+
+ for (i = 1; i <= XHCI_MAX_SLOTS; i++) {
+ if (XHCI_SLOTDEV_PTR(xdev, i) == NULL) {
+ xdev->slots[i] = dev;
+ *slot = i;
+ dev->dev_slotstate = XHCI_ST_ENABLED;
+ cmderr = XHCI_TRB_ERROR_SUCCESS;
+ dev->hci.hci_address = i;
+ xdev->port_map_tbl[di->bus][di->port] =
+ PT_MAKE_STATE(PT_ASSIGNED_WITH_EMU_DEV, vport);
+ break;
}
+ }

- UPRINTF(LDBG, "enable slot (error=%d) slot %u\r\n",
- cmderr != XHCI_TRB_ERROR_SUCCESS, *slot);
+ UPRINTF(LDBG, "enable slot (error=%d) slot %u for native device "
+ "%d-%d\r\n", cmderr != XHCI_TRB_ERROR_SUCCESS, *slot,
+ di->bus, di->port);

return cmderr;
}
@@ -1423,6 +1463,8 @@ static uint32_t
pci_xhci_cmd_disable_slot(struct pci_xhci_vdev *xdev, uint32_t slot)
{
struct pci_xhci_dev_emu *dev;
+ struct usb_dev *udev;
+ struct usb_native_devinfo *di;
uint32_t cmderr;
int i;

@@ -1446,6 +1488,9 @@ pci_xhci_cmd_disable_slot(struct pci_xhci_vdev *xdev, uint32_t slot)
cmderr = XHCI_TRB_ERROR_SUCCESS;
/* TODO: reset events and endpoints */
}
+ } else {
+ UPRINTF(LDBG, "disable NULL device, slot %d\r\n", slot);
+ goto done;
}

for (i = 1; i <= XHCI_MAX_DEVS; ++i)
@@ -1455,8 +1500,20 @@ pci_xhci_cmd_disable_slot(struct pci_xhci_vdev *xdev, uint32_t slot)
if (i <= XHCI_MAX_DEVS && XHCI_PORTREG_PTR(xdev, i)) {
XHCI_PORTREG_PTR(xdev, i)->portsc &= ~(XHCI_PS_CSC |
XHCI_PS_CCS | XHCI_PS_PED | XHCI_PS_PP);
+
+ udev = dev->dev_instance;
+ assert(udev);
+
xdev->devices[i] = NULL;
xdev->slots[slot] = NULL;
+
+ di = &udev->info;
+ xdev->port_map_tbl[di->bus][di->port] =
+ PT_MAKE_STATE(PT_ASSIGNED_WITH_NO_DEV, 0);
+
+ UPRINTF(LINF, "disable slot %d for native device %d-%d"
+ "\r\n", slot, di->bus, di->port);
+
pci_xhci_dev_destroy(dev);
} else
UPRINTF(LWRN, "invalid slot %d\r\n", slot);
@@ -1624,7 +1681,10 @@ pci_xhci_cmd_config_ep(struct pci_xhci_vdev *xdev,
UPRINTF(LDBG, "config_ep slot %u\r\n", slot);

dev = XHCI_SLOTDEV_PTR(xdev, slot);
- assert(dev != NULL);
+ if (dev == NULL) {
+ cmderr = XHCI_TRB_ERROR_SLOT_NOT_ON;
+ goto done;
+ }

if ((trb->dwTrb3 & XHCI_TRB_3_DCEP_BIT) != 0) {
UPRINTF(LDBG, "config_ep - deconfigure ep slot %u\r\n", slot);
@@ -3364,7 +3424,10 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
goto errout;
}

- xdev->port_map_tbl[bus][port] = PT_ASSIGNED_WITH_NO_DEV;
+ xdev->port_map_tbl[bus][port] =
+ PT_MAKE_STATE(PT_ASSIGNED_WITH_NO_DEV, 0);
+ return 0;
+
errout:
if (rc)
UPRINTF(LWRN, "%s fails, rc=%d\r\n", __func__, rc);
--
2.7.4




Re: [PATCH v2 6/8] DM USB: xHCI: code cleanup: change variable name

Yu Wang
 

Acked-by: Yu Wang <yu1.wang@...>

On 18-08-14 21:39:12, Wu, Xiaoguang wrote:
Replace 'native_assign_ports' with 'port_map_tbl' to be more accurate
for the role of this variable plays.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index b5d8555..c5fce1e 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -373,7 +373,7 @@ struct pci_xhci_vdev {
int (*excap_write)(struct pci_xhci_vdev *, uint64_t, uint64_t);
int usb2_port_start;
int usb3_port_start;
- uint8_t native_assign_ports[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
+ uint8_t port_map_tbl[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
struct timespec mf_prev_time; /* previous time of accessing MFINDEX */
};

@@ -504,7 +504,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
di->vid, di->pid, di->bus, di->port);

- if (xdev->native_assign_ports[di->bus][di->port] == PT_NOT_ASSIGNED) {
+ if (xdev->port_map_tbl[di->bus][di->port] == PT_NOT_ASSIGNED) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
"\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
@@ -3364,7 +3364,7 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
goto errout;
}

- xdev->native_assign_ports[bus][port] = PT_ASSIGNED_WITH_NO_DEV;
+ xdev->port_map_tbl[bus][port] = PT_ASSIGNED_WITH_NO_DEV;
errout:
if (rc)
UPRINTF(LWRN, "%s fails, rc=%d\r\n", __func__, rc);
--
2.7.4




Re: [PATCH v2 5/8] DM USB: xHCI: refine port assignment logic

Yu Wang
 

On 18-08-14 21:39:11, Wu, Xiaoguang wrote:
The variable native_assign_ports in struct pci_xhci_vdev is used
to record wether certain root hub port in SOS is assigned to UOS.
The logic uses zero to express 'not assigned' and nonzero to express
'assigned'. In this patch, use macro to replace number to express
better.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index 14009a8..b5d8555 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -384,6 +384,10 @@ struct pci_xhci_vdev {
#define XHCI_HALTED(xdev) ((xdev)->opregs.usbsts & XHCI_STS_HCH)
#define XHCI_GADDR(xdev, a) paddr_guest2host((xdev)->dev->vmctx, (a), \
XHCI_PADDR_SZ - ((a) & (XHCI_PADDR_SZ-1)))
+
+#define PT_NOT_ASSIGNED (0)
+#define PT_ASSIGNED_WITH_NO_DEV (-1)
The name is not good.

How about:
VPORT_FREE
VPORT_ASSIGNED
VPORT_CONNECTED

In your case, the VPORT_CONNECTED is > 0.

+
struct pci_xhci_option_elem {
char *parse_opt;
int (*parse_fn)(struct pci_xhci_vdev *, char *);
@@ -500,7 +504,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
di->vid, di->pid, di->bus, di->port);

- if (!xdev->native_assign_ports[di->bus][di->port]) {
+ if (xdev->native_assign_ports[di->bus][di->port] == PT_NOT_ASSIGNED) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
"\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
@@ -3360,7 +3364,7 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
goto errout;
}

- xdev->native_assign_ports[bus][port] = 1;
+ xdev->native_assign_ports[bus][port] = PT_ASSIGNED_WITH_NO_DEV;
errout:
if (rc)
UPRINTF(LWRN, "%s fails, rc=%d\r\n", __func__, rc);
--
2.7.4




Re: [PATCH v2 4/8] DM USB: xHCI: limit bus and port numbers of xHCI

Yu Wang
 

Acked-by: Yu Wang <yu1.wang@...>

On 18-08-14 21:39:10, Wu, Xiaoguang wrote:
Currently the maximum number of bus and port for xHCI are
both set to 255, it is theoretically possible but in fact
not neccessary. This patch changes those two values to be
more proper: 4 buses and 20 ports.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 14 ++------------
devicemodel/include/usb_core.h | 4 ++--
2 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index b50d435..14009a8 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -373,7 +373,7 @@ struct pci_xhci_vdev {
int (*excap_write)(struct pci_xhci_vdev *, uint64_t, uint64_t);
int usb2_port_start;
int usb3_port_start;
- uint8_t *native_assign_ports[USB_NATIVE_NUM_BUS];
+ uint8_t native_assign_ports[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
struct timespec mf_prev_time; /* previous time of accessing MFINDEX */
};

@@ -500,8 +500,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
di->vid, di->pid, di->bus, di->port);

- if (!xdev->native_assign_ports[di->bus] ||
- !xdev->native_assign_ports[di->bus][di->port]) {
+ if (!xdev->native_assign_ports[di->bus][di->port]) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
"\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
@@ -3361,15 +3360,6 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
goto errout;
}

- if (!xdev->native_assign_ports[bus]) {
- xdev->native_assign_ports[bus] = calloc(USB_NATIVE_NUM_PORT,
- sizeof(uint8_t));
- if (!xdev->native_assign_ports[bus]) {
- rc = -3;
- goto errout;
- }
- }
-
xdev->native_assign_ports[bus][port] = 1;
errout:
if (rc)
diff --git a/devicemodel/include/usb_core.h b/devicemodel/include/usb_core.h
index 8727b8b..add4cb4 100644
--- a/devicemodel/include/usb_core.h
+++ b/devicemodel/include/usb_core.h
@@ -223,8 +223,8 @@ enum USB_ERRCODE {
#define NATIVE_USBSYS_DEVDIR "/sys/bus/usb/devices"
#define NATIVE_USB2_SPEED "480"
#define NATIVE_USB3_SPEED "5000"
-#define USB_NATIVE_NUM_PORT 255
-#define USB_NATIVE_NUM_BUS 255
+#define USB_NATIVE_NUM_PORT 20
+#define USB_NATIVE_NUM_BUS 4

extern int usb_log_level;
inline int usb_get_log_level(void) { return usb_log_level; }
--
2.7.4




Re: [PATCH v2 3/8] DM USB: introduce struct usb_native_devinfo

Yu Wang
 

Acked-by: Yu Wang <yu1.wang@...>

On 18-08-14 21:39:09, Wu, Xiaoguang wrote:
Current design cannot get physical USB device information without
the creation of pci_xhci_dev_emu. This brings some difficulties in
certain situations, hence struct usb_native_devinfo is introduced
to describe neccessary information to solve this trouble.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 33 +++++-------
devicemodel/hw/platform/usb_pmapper.c | 95 ++++++++++++++++++-----------------
devicemodel/include/usb_core.h | 10 ++++
devicemodel/include/usb_pmapper.h | 9 +---
4 files changed, 73 insertions(+), 74 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index f18660c..b50d435 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -469,22 +469,21 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
struct pci_xhci_dev_emu *de;
struct pci_xhci_vdev *xdev;
struct usb_devemu *ue;
+ struct usb_native_devinfo *di;
int port_start, port_end;
int slot_start, slot_end;
int port, slot;
void *ud;
- uint8_t native_bus, native_pid, native_port;
- uint16_t native_vid;
- int native_speed;

xdev = hci_data;
+ di = dev_data;

assert(xdev);
assert(dev_data);
assert(xdev->devices);
assert(xdev->slots);

- de = pci_xhci_dev_create(xdev, dev_data);
+ de = pci_xhci_dev_create(xdev, di);
if (!de) {
UPRINTF(LFTL, "fail to create device\r\n");
return -1;
@@ -498,26 +497,20 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
assert(ue);

/* print physical information about new device */
- ue->ue_info(ud, USB_INFO_BUS, &native_bus, sizeof(native_bus));
- ue->ue_info(ud, USB_INFO_PORT, &native_port, sizeof(native_port));
- ue->ue_info(ud, USB_INFO_VID, &native_vid, sizeof(native_vid));
- ue->ue_info(ud, USB_INFO_PID, &native_pid, sizeof(native_pid));
- ue->ue_info(ud, USB_INFO_SPEED, &native_speed, sizeof(native_speed));
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
- native_vid, native_pid, native_bus, native_port);
+ di->vid, di->pid, di->bus, di->port);

- if (!xdev->native_assign_ports[native_bus] ||
- !xdev->native_assign_ports[native_bus][native_port]) {
+ if (!xdev->native_assign_ports[di->bus] ||
+ !xdev->native_assign_ports[di->bus][di->port]) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
- "\r\n", native_vid, native_pid, native_bus,
- native_port);
+ "\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
}

- UPRINTF(LDBG, "%04x:%04x %d-%d belong to this vm.\r\n", native_vid,
- native_pid, native_bus, native_port);
+ UPRINTF(LDBG, "%04x:%04x %d-%d belong to this vm.\r\n", di->vid,
+ di->pid, di->bus, di->port);

- if (ue->ue_usbver == 2)
+ if (di->bcd < 0x300)
port_start = xdev->usb2_port_start;
else
port_start = xdev->usb3_port_start;
@@ -549,11 +542,11 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)

pci_xhci_reset_slot(xdev, slot);
UPRINTF(LDBG, "%X:%X %d-%d locates in slot %d port %d.\r\n",
- native_vid, native_pid, native_bus, native_port,
+ di->vid, di->pid, di->bus, di->port,
slot, port);

/* Trigger port change event for the arriving device */
- if (pci_xhci_connect_port(xdev, port, native_speed, 1))
+ if (pci_xhci_connect_port(xdev, port, di->speed, 1))
UPRINTF(LFTL, "fail to report port event\n");

return 0;
@@ -588,7 +581,7 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
continue;

udev = edev->dev_instance;
- if (udev->port == native_port)
+ if (udev->info.port == native_port)
break;
}

diff --git a/devicemodel/hw/platform/usb_pmapper.c b/devicemodel/hw/platform/usb_pmapper.c
index 9f57f1c..2aedf28 100644
--- a/devicemodel/hw/platform/usb_pmapper.c
+++ b/devicemodel/hw/platform/usb_pmapper.c
@@ -383,7 +383,7 @@ usb_dev_update_ep(struct usb_dev *udev)
int i, j;

assert(udev);
- if (libusb_get_active_config_descriptor(udev->ldev, &cfg))
+ if (libusb_get_active_config_descriptor(udev->info.priv_data, &cfg))
return;

for (i = 0; i < cfg->bNumInterfaces; i++) {
@@ -409,13 +409,12 @@ usb_dev_native_toggle_if(struct usb_dev *udev, int claim)

assert(udev);
assert(udev->handle);
- assert(udev->ldev);
assert(claim == 1 || claim == 0);

- b = udev->bus;
- p = udev->port;
+ b = udev->info.bus;
+ p = udev->info.port;

- r = libusb_get_active_config_descriptor(udev->ldev, &config);
+ r = libusb_get_active_config_descriptor(udev->info.priv_data, &config);
if (r) {
UPRINTF(LWRN, "%d-%d: can't get config\r\n", b, p);
return -1;
@@ -458,13 +457,12 @@ usb_dev_native_toggle_if_drivers(struct usb_dev *udev, int attach)

assert(udev);
assert(udev->handle);
- assert(udev->ldev);
assert(attach == 1 || attach == 0);

- b = udev->bus;
- p = udev->port;
+ b = udev->info.bus;
+ p = udev->info.port;

- r = libusb_get_active_config_descriptor(udev->ldev, &config);
+ r = libusb_get_active_config_descriptor(udev->info.priv_data, &config);
if (r) {
UPRINTF(LWRN, "%d-%d: can't get config\r\n", b, p);
return -1;
@@ -503,7 +501,6 @@ usb_dev_set_config(struct usb_dev *udev, struct usb_data_xfer *xfer, int config)
struct libusb_config_descriptor *cfg;

assert(udev);
- assert(udev->ldev);
assert(udev->handle);

/*
@@ -521,7 +518,7 @@ usb_dev_set_config(struct usb_dev *udev, struct usb_data_xfer *xfer, int config)
}

/* claim all the interfaces of this configuration */
- rc = libusb_get_active_config_descriptor(udev->ldev, &cfg);
+ rc = libusb_get_active_config_descriptor(udev->info.priv_data, &cfg);
if (rc) {
UPRINTF(LWRN, "fail to get config rc %d\r\n", rc);
goto err2;
@@ -545,7 +542,8 @@ err1:
usb_dev_native_toggle_if(udev, 0);
libusb_free_config_descriptor(cfg);
err2:
- UPRINTF(LWRN, "%d-%d: fail to set config\r\n", udev->bus, udev->port);
+ UPRINTF(LWRN, "%d-%d: fail to set config\r\n", udev->info.bus,
+ udev->info.port);
xfer->status = USB_ERR_STALLED;
}

@@ -560,8 +558,8 @@ usb_dev_set_if(struct usb_dev *udev, int iface, int alt, struct usb_data_xfer
if (iface >= USB_NUM_INTERFACE)
goto errout;

- UPRINTF(LDBG, "%d-%d set if, iface %d alt %d\r\n", udev->bus,
- udev->port, iface, alt);
+ UPRINTF(LDBG, "%d-%d set if, iface %d alt %d\r\n", udev->info.bus,
+ udev->info.port, iface, alt);

if (libusb_set_interface_alt_setting(udev->handle, iface, alt))
goto errout;
@@ -578,7 +576,7 @@ usb_dev_set_if(struct usb_dev *udev, int iface, int alt, struct usb_data_xfer
errout:
xfer->status = USB_ERR_STALLED;
UPRINTF(LDBG, "%d-%d fail to set if, iface %d alt %d\r\n",
- udev->bus, udev->port, iface, alt);
+ udev->info.bus, udev->info.port, iface, alt);
}

static struct usb_data_xfer_block *
@@ -773,7 +771,7 @@ usb_dev_request(void *pdata, struct usb_data_xfer *xfer)
assert(udev);

xfer->status = USB_ERR_NORMAL_COMPLETION;
- if (!udev->ldev || !xfer->ureq) {
+ if (!udev->info.priv_data || !xfer->ureq) {
UPRINTF(LWRN, "invalid request\r\n");
xfer->status = USB_ERR_IOERROR;
goto out;
@@ -861,22 +859,18 @@ void *
usb_dev_init(void *pdata, char *opt)
{
struct usb_dev *udev = NULL;
- struct libusb_device *ldev;
struct libusb_device_descriptor desc;
- uint8_t bus, port;
- int speed, ver;
+ struct usb_native_devinfo *di;
+ int ver;

assert(pdata);

- ldev = pdata;
- speed = libusb_get_device_speed(ldev);
- port = libusb_get_port_number(ldev);
- bus = libusb_get_bus_number(ldev);
- libusb_get_device_descriptor(ldev, &desc);
+ di = pdata;
+ libusb_get_device_descriptor(di->priv_data, &desc);
UPRINTF(LINF, "Found USB device: %d-%d\r\nPID(0x%X), VID(0x%X) CLASS"
- "(0x%X) SUBCLASS(0x%X) BCD(0x%X) SPEED(%d)\r\n", bus,
- port, desc.idProduct, desc.idVendor, desc.bDeviceClass,
- desc.bDeviceSubClass, desc.bcdUSB, speed);
+ "(0x%X) SUBCLASS(0x%X) BCD(0x%X) SPEED(%d)\r\n",
+ di->bus, di->port, di->pid, di->vid, desc.bDeviceClass,
+ desc.bDeviceSubClass, di->bcd, di->speed);

/* allocate and populate udev */
udev = calloc(1, sizeof(struct usb_dev));
@@ -884,7 +878,7 @@ usb_dev_init(void *pdata, char *opt)
goto errout;

/* this is a root hub */
- if (port == 0)
+ if (di->port == 0)
goto errout;

switch (desc.bcdUSB) {
@@ -908,17 +902,12 @@ usb_dev_init(void *pdata, char *opt)
goto errout;
}

- udev->speed = speed;
- udev->ldev = ldev;
+ udev->info = *di;
udev->version = ver;
udev->handle = NULL;
- udev->port = port;
- udev->bus = bus;
- udev->pid = desc.idProduct;
- udev->vid = desc.idVendor;

/* configure physical device through libusb library */
- if (libusb_open(udev->ldev, &udev->handle)) {
+ if (libusb_open(udev->info.priv_data, &udev->handle)) {
UPRINTF(LWRN, "fail to open device.\r\n");
goto errout;
}
@@ -973,25 +962,25 @@ usb_dev_info(void *pdata, int type, void *value, int size)
pv = &udev->version;
break;
case USB_INFO_SPEED:
- sz = sizeof(udev->speed);
- udev->speed = libusb_speed_to_usb_speed(udev->speed);
- pv = &udev->speed;
+ sz = sizeof(udev->info.speed);
+ udev->info.speed = libusb_speed_to_usb_speed(udev->info.speed);
+ pv = &udev->info.speed;
break;
case USB_INFO_BUS:
- sz = sizeof(udev->bus);
- pv = &udev->bus;
+ sz = sizeof(udev->info.bus);
+ pv = &udev->info.bus;
break;
case USB_INFO_PORT:
- sz = sizeof(udev->port);
- pv = &udev->port;
+ sz = sizeof(udev->info.port);
+ pv = &udev->info.port;
break;
case USB_INFO_VID:
- sz = sizeof(udev->vid);
- pv = &udev->vid;
+ sz = sizeof(udev->info.vid);
+ pv = &udev->info.vid;
break;
case USB_INFO_PID:
- sz = sizeof(udev->pid);
- pv = &udev->pid;
+ sz = sizeof(udev->info.pid);
+ pv = &udev->info.pid;
break;
default:
return -1;
@@ -1020,6 +1009,9 @@ static int
usb_dev_native_sys_conn_cb(struct libusb_context *ctx, struct libusb_device
*ldev, libusb_hotplug_event event, void *pdata)
{
+ struct libusb_device_descriptor d;
+ struct usb_native_devinfo di;
+
UPRINTF(LDBG, "connect event\r\n");

if (!ctx || !ldev) {
@@ -1027,8 +1019,17 @@ usb_dev_native_sys_conn_cb(struct libusb_context *ctx, struct libusb_device
return -1;
}

+ libusb_get_device_descriptor(ldev, &d);
+ di.bus = libusb_get_bus_number(ldev);
+ di.speed = libusb_get_device_speed(ldev);
+ di.port = libusb_get_port_number(ldev);
+ di.pid = d.idProduct;
+ di.vid = d.idVendor;
+ di.bcd = d.bcdUSB;
+ di.priv_data = ldev;
+
if (g_ctx.conn_cb)
- g_ctx.conn_cb(g_ctx.hci_data, ldev);
+ g_ctx.conn_cb(g_ctx.hci_data, &di);

return 0;
}
diff --git a/devicemodel/include/usb_core.h b/devicemodel/include/usb_core.h
index 3a9648a..8727b8b 100644
--- a/devicemodel/include/usb_core.h
+++ b/devicemodel/include/usb_core.h
@@ -163,6 +163,16 @@ struct usb_data_xfer {
pthread_mutex_t mtx;
};

+struct usb_native_devinfo {
+ int speed;
+ uint8_t bus;
+ uint8_t port;
+ uint16_t bcd;
+ uint16_t pid;
+ uint16_t vid;
+ void *priv_data;
+};
+
enum USB_ERRCODE {
USB_ACK,
USB_NAK,
diff --git a/devicemodel/include/usb_pmapper.h b/devicemodel/include/usb_pmapper.h
index 144c68f..43cb09f 100644
--- a/devicemodel/include/usb_pmapper.h
+++ b/devicemodel/include/usb_pmapper.h
@@ -36,14 +36,10 @@ struct usb_dev_ep {

struct usb_dev {
/* physical device info */
+ struct usb_native_devinfo info;
int addr;
int version;
- int speed;
int configuration;
- uint8_t port;
- uint8_t bus;
- uint8_t pid;
- uint16_t vid;

/* interface info */
int if_num;
@@ -55,8 +51,7 @@ struct usb_dev {
struct usb_dev_ep epo[USB_NUM_ENDPOINT];

/* libusb data */
- libusb_device *ldev;
- libusb_device_handle *handle;
+ libusb_device_handle *handle;
};

/*
--
2.7.4




Re: [PATCH v2 2/8] DM USB: xHCI: refine xHCI PORTSC Register related functions

Yu Wang
 

Acked-by: Yu Wang <yu1.wang@...>

On 18-08-14 21:39:08, Wu, Xiaoguang wrote:
PORTSC (Port Status and Control Register) register play a very
important role in USB sub-system. This patch is used to refine
related manipulation functions.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 68 +++++++++++++++++++++--------------------------
1 file changed, 30 insertions(+), 38 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index 792d930..f18660c 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -440,10 +440,13 @@ static void pci_xhci_update_ep_ring(struct pci_xhci_vdev *xdev,
uint32_t streamid, uint64_t ringaddr,
int ccs);
static void pci_xhci_init_port(struct pci_xhci_vdev *xdev, int portn);
+static int pci_xhci_connect_port(struct pci_xhci_vdev *xdev, int port,
+ int usb_speed, int need_intr);
+static int pci_xhci_disconnect_port(struct pci_xhci_vdev *xdev, int port,
+ int need_intr);
static struct pci_xhci_dev_emu *pci_xhci_dev_create(struct pci_xhci_vdev *
xdev, void *dev_data);
static void pci_xhci_dev_destroy(struct pci_xhci_dev_emu *de);
-static int pci_xhci_port_chg(struct pci_xhci_vdev *xdev, int port, int conn);
static void pci_xhci_set_evtrb(struct xhci_trb *evtrb, uint64_t port,
uint32_t errcode, uint32_t evtype);
static int pci_xhci_xfer_complete(struct pci_xhci_vdev *xdev,
@@ -472,6 +475,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
void *ud;
uint8_t native_bus, native_pid, native_port;
uint16_t native_vid;
+ int native_speed;

xdev = hci_data;

@@ -498,6 +502,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
ue->ue_info(ud, USB_INFO_PORT, &native_port, sizeof(native_port));
ue->ue_info(ud, USB_INFO_VID, &native_vid, sizeof(native_vid));
ue->ue_info(ud, USB_INFO_PID, &native_pid, sizeof(native_pid));
+ ue->ue_info(ud, USB_INFO_SPEED, &native_speed, sizeof(native_speed));
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
native_vid, native_pid, native_bus, native_port);

@@ -543,14 +548,12 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
xdev->ndevices++;

pci_xhci_reset_slot(xdev, slot);
- pci_xhci_init_port(xdev, port);
-
UPRINTF(LDBG, "%X:%X %d-%d locates in slot %d port %d.\r\n",
native_vid, native_pid, native_bus, native_port,
slot, port);

/* Trigger port change event for the arriving device */
- if (pci_xhci_port_chg(xdev, port, 1))
+ if (pci_xhci_connect_port(xdev, port, native_speed, 1))
UPRINTF(LFTL, "fail to report port event\n");

return 0;
@@ -595,7 +598,7 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
}

UPRINTF(LDBG, "report virtual port %d status\r\n", port);
- if (pci_xhci_port_chg(xdev, port, 0)) {
+ if (pci_xhci_disconnect_port(xdev, port, 1)) {
UPRINTF(LFTL, "fail to report event\r\n");
return -1;
}
@@ -784,32 +787,29 @@ pci_xhci_convert_speed(int lspeed)
}

static int
-pci_xhci_port_chg(struct pci_xhci_vdev *xdev, int port, int conn)
+pci_xhci_change_port(struct pci_xhci_vdev *xdev, int port, int usb_speed,
+ int conn, int need_intr)
{
int speed, error;
struct xhci_trb evtrb;
struct pci_xhci_portregs *reg;
- struct pci_xhci_dev_emu *dev;

assert(xdev != NULL);

reg = XHCI_PORTREG_PTR(xdev, port);
- dev = XHCI_DEVINST_PTR(xdev, port);
- if (!dev || !dev->dev_ue || !reg) {
- UPRINTF(LWRN, "find nullptr with port %d\r\n", port);
- return -1;
- }
-
if (conn == 0) {
reg->portsc &= ~XHCI_PS_CCS;
reg->portsc |= (XHCI_PS_CSC |
XHCI_PS_PLS_SET(UPS_PORT_LS_RX_DET));
} else {
- speed = pci_xhci_convert_speed(dev->dev_ue->ue_usbspeed);
+ speed = pci_xhci_convert_speed(usb_speed);
reg->portsc = XHCI_PS_CCS | XHCI_PS_PP | XHCI_PS_CSC;
reg->portsc |= XHCI_PS_SPEED_SET(speed);
}

+ if (!need_intr)
+ return 0;
+
/* make an event for the guest OS */
pci_xhci_set_evtrb(&evtrb,
port,
@@ -825,6 +825,20 @@ pci_xhci_port_chg(struct pci_xhci_vdev *xdev, int port, int conn)
return (error == XHCI_TRB_ERROR_SUCCESS) ? 0 : -1;
}

+static int
+pci_xhci_connect_port(struct pci_xhci_vdev *xdev, int port, int usb_speed,
+ int intr)
+{
+ return pci_xhci_change_port(xdev, port, usb_speed, 1, intr);
+}
+
+static int
+pci_xhci_disconnect_port(struct pci_xhci_vdev *xdev, int port, int intr)
+{
+ /* for disconnect, the speed is useless */
+ return pci_xhci_change_port(xdev, port, 0, 0, intr);
+}
+
static void
pci_xhci_set_evtrb(struct xhci_trb *evtrb, uint64_t port, uint32_t errcode,
uint32_t evtype)
@@ -3208,30 +3222,8 @@ pci_xhci_reset_port(struct pci_xhci_vdev *xdev, int portn, int warm)
static void
pci_xhci_init_port(struct pci_xhci_vdev *xdev, int portn)
{
- struct pci_xhci_portregs *port;
- struct pci_xhci_dev_emu *dev;
-
- port = XHCI_PORTREG_PTR(xdev, portn);
- dev = XHCI_DEVINST_PTR(xdev, portn);
- if (dev) {
- port->portsc = XHCI_PS_CCS | /* connected */
- XHCI_PS_PP; /* port power */
-
- if (dev->dev_ue->ue_usbver == 2) {
- port->portsc |= XHCI_PS_PLS_SET(UPS_PORT_LS_POLL) |
- XHCI_PS_SPEED_SET(dev->dev_ue->ue_usbspeed);
- } else {
- port->portsc |= XHCI_PS_PLS_SET(UPS_PORT_LS_U0) |
- XHCI_PS_PED | /* enabled */
- XHCI_PS_SPEED_SET(dev->dev_ue->ue_usbspeed);
- }
-
- UPRINTF(LDBG, "Init port %d 0x%x\n", portn, port->portsc);
- } else {
- port->portsc = XHCI_PS_PLS_SET(UPS_PORT_LS_RX_DET) | XHCI_PS_PP;
- UPRINTF(LDBG, "Init empty port %d 0x%x\n",
- portn, port->portsc);
- }
+ XHCI_PORTREG_PTR(xdev, portn)->portsc =
+ XHCI_PS_PLS_SET(UPS_PORT_LS_RX_DET) | XHCI_PS_PP;
}

static int
--
2.7.4




[PATCH v2 8/8] DM USB: xHCI: enable xHCI SOS S3 support

Wu, Xiaoguang
 

This patch enable the support for SOS S3 from the perspective
of USB xHCI.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index ec215a0..3509fbd 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -91,6 +91,7 @@
#include "pci_core.h"
#include "xhci.h"
#include "usb_pmapper.h"
+#include "vmmapi.h"

#undef LOG_TAG
#define LOG_TAG "xHCI: "
@@ -485,6 +486,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
struct usb_native_devinfo *di;
int vport_start, vport_end;
int port;
+ int need_intr = 1;

xdev = hci_data;

@@ -534,8 +536,11 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
xdev->port_map_tbl[di->bus][di->port] =
PT_MAKE_STATE(PT_ASSIGNED_WITH_NEW_DEV, port);

+ if (vm_get_suspend_mode() != VM_SUSPEND_NONE)
+ need_intr = 0;
+
/* Trigger port change event for the arriving device */
- if (pci_xhci_connect_port(xdev, port, di->speed, 1))
+ if (pci_xhci_connect_port(xdev, port, di->speed, need_intr))
UPRINTF(LFTL, "fail to report port event\n");

return 0;
@@ -552,6 +557,7 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
struct usb_dev *udev;
uint8_t port, slot, native_port;
uint8_t status;
+ int need_intr = 1;

assert(hci_data);
assert(dev_data);
@@ -594,8 +600,18 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
xdev->port_map_tbl[di.bus][di.port] = PT_MAKE_STATE(
PT_ASSIGNED_WITH_NO_DEV, port);

+ if (vm_get_suspend_mode() != VM_SUSPEND_NONE) {
+ XHCI_PORTREG_PTR(xdev, port)->portsc &= ~(XHCI_PS_CSC |
+ XHCI_PS_CCS | XHCI_PS_PED | XHCI_PS_PP);
+ edev->dev_slotstate = XHCI_ST_DISABLED;
+ xdev->devices[port] = NULL;
+ xdev->slots[slot] = NULL;
+ pci_xhci_dev_destroy(edev);
+ need_intr = 0;
+ }
+
UPRINTF(LDBG, "report virtual port %d status\r\n", port);
- if (pci_xhci_disconnect_port(xdev, port, 1)) {
+ if (pci_xhci_disconnect_port(xdev, port, need_intr)) {
UPRINTF(LFTL, "fail to report event\r\n");
return -1;
}
--
2.7.4


[PATCH v2 7/8] DM USB: xHCI: change flow of creation of virtual USB device

Wu, Xiaoguang
 

The xHCI emulation greatly depends on the user space library libusb
which is based on the usbfs module in Linux kernel. The libusb will
bind usbfs to physical USB device which makes hardware control over
libusb in user space possible.

The pci_xhci_dev_create is called in pci_xhci_native_usb_dev_conn_cb
which is a callback function triggered by physical USB device plugging.
This function will bind the physical USB device to usbfs in SOS, which
we depend to create the communication between UOS xHCI driver with
physical USB device.

This design will fail if the reconnection happened in the SOS, which
will bind class driver to the physical USB device instead of usbfs,
hence the libusb device handle in DM is invalid.

Currently, the native S3 will disable the vbus for all xHCI ports and
re-drive during S3 resume. This behavior cause native USB driver unbind
the usbfs and bind to related class driver, then made the DM lost
control and failed to continue emulation.

To fix this issue, place the pci_xhci_dev_create in the function
pci_xhci_cmd_enable_slot. According to the xHCI spec 4.5.3 Figure 10,
the UOS always send Enable Slot command when a device is attached or
recovered from errors (by Disable Slot command). So every time the SOS
can't resuming normally or some unexpected disconnection happens, this
desigen will always survive by Disable Slot and Enable Slot command
series from UOS xHCI driver.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 197 ++++++++++++++++++++++++++++++----------------
1 file changed, 130 insertions(+), 67 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index c5fce1e..ec215a0 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -373,7 +373,10 @@ struct pci_xhci_vdev {
int (*excap_write)(struct pci_xhci_vdev *, uint64_t, uint64_t);
int usb2_port_start;
int usb3_port_start;
- uint8_t port_map_tbl[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
+
+ uint16_t port_map_tbl[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
+ struct usb_native_devinfo
+ native_dev_info[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
struct timespec mf_prev_time; /* previous time of accessing MFINDEX */
};

@@ -385,8 +388,16 @@ struct pci_xhci_vdev {
#define XHCI_GADDR(xdev, a) paddr_guest2host((xdev)->dev->vmctx, (a), \
XHCI_PADDR_SZ - ((a) & (XHCI_PADDR_SZ-1)))

+/* port mapping status */
#define PT_NOT_ASSIGNED (0)
-#define PT_ASSIGNED_WITH_NO_DEV (-1)
+#define PT_ASSIGNED_WITH_NO_DEV (1)
+#define PT_ASSIGNED_WITH_NEW_DEV (2)
+#define PT_ASSIGNED_WITH_EMU_DEV (3)
+
+/* helpers for get port mapping information */
+#define PT_GET_NUMBER(state) (state & 0xFF)
+#define PT_GET_STATUS(state) ((state >> 8) & 0xFF)
+#define PT_MAKE_STATE(status, num) (((status & 0xFF) << 8) | (num & 0xFF))

struct pci_xhci_option_elem {
char *parse_opt;
@@ -470,83 +481,58 @@ static struct pci_xhci_option_elem xhci_option_table[] = {
static int
pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
{
- struct pci_xhci_dev_emu *de;
struct pci_xhci_vdev *xdev;
- struct usb_devemu *ue;
struct usb_native_devinfo *di;
- int port_start, port_end;
- int slot_start, slot_end;
- int port, slot;
- void *ud;
+ int vport_start, vport_end;
+ int port;

xdev = hci_data;
- di = dev_data;

assert(xdev);
assert(dev_data);
assert(xdev->devices);
assert(xdev->slots);

- de = pci_xhci_dev_create(xdev, di);
- if (!de) {
- UPRINTF(LFTL, "fail to create device\r\n");
- return -1;
- }
-
- /* find free port and slot for the new usb device */
- ud = de->dev_instance;
- ue = de->dev_ue;
-
- assert(ud);
- assert(ue);
+ di = dev_data;

/* print physical information about new device */
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
di->vid, di->pid, di->bus, di->port);

- if (xdev->port_map_tbl[di->bus][di->port] == PT_NOT_ASSIGNED) {
+ if (PT_GET_STATUS(xdev->port_map_tbl[di->bus][di->port]) ==
+ PT_NOT_ASSIGNED) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
"\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
}
-
UPRINTF(LDBG, "%04x:%04x %d-%d belong to this vm.\r\n", di->vid,
di->pid, di->bus, di->port);

- if (di->bcd < 0x300)
- port_start = xdev->usb2_port_start;
- else
- port_start = xdev->usb3_port_start;
-
- slot_start = 1;
- port_end = port_start + (XHCI_MAX_DEVS / 2);
- slot_end = XHCI_MAX_SLOTS;
+ if (di->bcd < 0x300) {
+ vport_start = xdev->usb2_port_start;
+ vport_end = vport_start + (XHCI_MAX_DEVS / 2);
+ } else {
+ vport_start = xdev->usb3_port_start;
+ vport_end = vport_start + (XHCI_MAX_DEVS / 2);
+ }

/* find free port */
- for (port = port_start; port < port_end; port++)
+ for (port = vport_start; port < vport_end; port++)
if (!xdev->devices[port])
break;

- /* find free slot */
- for (slot = slot_start; slot < slot_end; slot++)
- if (!xdev->slots[slot])
- break;
-
- if (port >= port_end || slot >= slot_end) {
- UPRINTF(LFTL, "no free resource: port %d slot %d\r\n",
- port, slot);
+ if (port >= vport_end) {
+ UPRINTF(LFTL, "no free virtual port for native device %d-%d"
+ "\r\n", di->bus, di->port);
goto errout;
}

- /* use index of devices as port number */
- xdev->devices[port] = de;
- xdev->slots[slot] = de;
- xdev->ndevices++;
+ UPRINTF(LDBG, "%04X:%04X %d-%d is attached to virtual port %d.\r\n",
+ di->vid, di->pid, di->bus, di->port, port);

- pci_xhci_reset_slot(xdev, slot);
- UPRINTF(LDBG, "%X:%X %d-%d locates in slot %d port %d.\r\n",
- di->vid, di->pid, di->bus, di->port,
- slot, port);
+ xdev->native_dev_info[di->bus][di->port] = *di;
+ xdev->port_map_tbl[di->bus][di->port] =
+ PT_MAKE_STATE(PT_ASSIGNED_WITH_NEW_DEV, port);

/* Trigger port change event for the arriving device */
if (pci_xhci_connect_port(xdev, port, di->speed, 1))
@@ -554,7 +540,6 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)

return 0;
errout:
- pci_xhci_dev_destroy(de);
return -1;
}

@@ -563,8 +548,10 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
{
struct pci_xhci_vdev *xdev;
struct pci_xhci_dev_emu *edev;
+ struct usb_native_devinfo di;
struct usb_dev *udev;
- uint8_t port, native_port;
+ uint8_t port, slot, native_port;
+ uint8_t status;

assert(hci_data);
assert(dev_data);
@@ -584,8 +571,10 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
continue;

udev = edev->dev_instance;
- if (udev->info.port == native_port)
+ if (udev->info.port == native_port) {
+ di = udev->info;
break;
+ }
}

if (port == XHCI_MAX_DEVS + 1) {
@@ -593,6 +582,18 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
return -1;
}

+ for (slot = 1; slot < XHCI_MAX_SLOTS; ++slot)
+ if (xdev->slots[slot] == edev)
+ break;
+
+ assert(slot < USB_NATIVE_NUM_BUS);
+
+ status = PT_GET_STATUS(xdev->port_map_tbl[di.bus][di.port]);
+ assert(status == PT_ASSIGNED_WITH_EMU_DEV ||
+ status == PT_ASSIGNED_WITH_NEW_DEV);
+ xdev->port_map_tbl[di.bus][di.port] = PT_MAKE_STATE(
+ PT_ASSIGNED_WITH_NO_DEV, port);
+
UPRINTF(LDBG, "report virtual port %d status\r\n", port);
if (pci_xhci_disconnect_port(xdev, port, 1)) {
UPRINTF(LFTL, "fail to report event\r\n");
@@ -1393,28 +1394,67 @@ done:
return err;
}

+static struct usb_native_devinfo *
+pci_xhci_find_native_devinfo(struct pci_xhci_vdev *xdev)
+{
+ int i, j;
+
+ assert(xdev);
+ for (i = 0; i < USB_NATIVE_NUM_BUS; ++i)
+ for (j = 0; j < USB_NATIVE_NUM_PORT; ++j)
+ if (PT_GET_STATUS(xdev->port_map_tbl[i][j]) ==
+ PT_ASSIGNED_WITH_NEW_DEV)
+ return &xdev->native_dev_info[i][j];
+
+ return NULL;
+}
+
static uint32_t
pci_xhci_cmd_enable_slot(struct pci_xhci_vdev *xdev, uint32_t *slot)
{
struct pci_xhci_dev_emu *dev;
- uint32_t cmderr;
- int i;
+ uint32_t cmderr;
+ struct usb_native_devinfo *di;
+ int i, vport;

cmderr = XHCI_TRB_ERROR_NO_SLOTS;
- if (xdev->portregs != NULL)
- for (i = 1; i <= XHCI_MAX_SLOTS; i++) {
- dev = XHCI_SLOTDEV_PTR(xdev, i);
- if (dev && dev->dev_slotstate == XHCI_ST_DISABLED) {
- *slot = i;
- dev->dev_slotstate = XHCI_ST_ENABLED;
- cmderr = XHCI_TRB_ERROR_SUCCESS;
- dev->hci.hci_address = i;
- break;
- }
+
+ di = pci_xhci_find_native_devinfo(xdev);
+ if (!di) {
+ UPRINTF(LWRN, "unexpected Enable Slot commnad\r\n");
+ return -1;
+ }
+
+ assert(di->priv_data);
+ dev = pci_xhci_dev_create(xdev, di);
+ if (!dev) {
+ UPRINTF(LFTL, "fail to create device\r\n");
+ return -1;
+ }
+
+ vport = PT_GET_NUMBER(xdev->port_map_tbl[di->bus][di->port]);
+ assert(vport > 0);
+ assert(!xdev->devices[vport]);
+
+ xdev->devices[vport] = dev;
+ xdev->ndevices++;
+
+ for (i = 1; i <= XHCI_MAX_SLOTS; i++) {
+ if (XHCI_SLOTDEV_PTR(xdev, i) == NULL) {
+ xdev->slots[i] = dev;
+ *slot = i;
+ dev->dev_slotstate = XHCI_ST_ENABLED;
+ cmderr = XHCI_TRB_ERROR_SUCCESS;
+ dev->hci.hci_address = i;
+ xdev->port_map_tbl[di->bus][di->port] =
+ PT_MAKE_STATE(PT_ASSIGNED_WITH_EMU_DEV, vport);
+ break;
}
+ }

- UPRINTF(LDBG, "enable slot (error=%d) slot %u\r\n",
- cmderr != XHCI_TRB_ERROR_SUCCESS, *slot);
+ UPRINTF(LDBG, "enable slot (error=%d) slot %u for native device "
+ "%d-%d\r\n", cmderr != XHCI_TRB_ERROR_SUCCESS, *slot,
+ di->bus, di->port);

return cmderr;
}
@@ -1423,6 +1463,8 @@ static uint32_t
pci_xhci_cmd_disable_slot(struct pci_xhci_vdev *xdev, uint32_t slot)
{
struct pci_xhci_dev_emu *dev;
+ struct usb_dev *udev;
+ struct usb_native_devinfo *di;
uint32_t cmderr;
int i;

@@ -1446,6 +1488,9 @@ pci_xhci_cmd_disable_slot(struct pci_xhci_vdev *xdev, uint32_t slot)
cmderr = XHCI_TRB_ERROR_SUCCESS;
/* TODO: reset events and endpoints */
}
+ } else {
+ UPRINTF(LDBG, "disable NULL device, slot %d\r\n", slot);
+ goto done;
}

for (i = 1; i <= XHCI_MAX_DEVS; ++i)
@@ -1455,8 +1500,20 @@ pci_xhci_cmd_disable_slot(struct pci_xhci_vdev *xdev, uint32_t slot)
if (i <= XHCI_MAX_DEVS && XHCI_PORTREG_PTR(xdev, i)) {
XHCI_PORTREG_PTR(xdev, i)->portsc &= ~(XHCI_PS_CSC |
XHCI_PS_CCS | XHCI_PS_PED | XHCI_PS_PP);
+
+ udev = dev->dev_instance;
+ assert(udev);
+
xdev->devices[i] = NULL;
xdev->slots[slot] = NULL;
+
+ di = &udev->info;
+ xdev->port_map_tbl[di->bus][di->port] =
+ PT_MAKE_STATE(PT_ASSIGNED_WITH_NO_DEV, 0);
+
+ UPRINTF(LINF, "disable slot %d for native device %d-%d"
+ "\r\n", slot, di->bus, di->port);
+
pci_xhci_dev_destroy(dev);
} else
UPRINTF(LWRN, "invalid slot %d\r\n", slot);
@@ -1624,7 +1681,10 @@ pci_xhci_cmd_config_ep(struct pci_xhci_vdev *xdev,
UPRINTF(LDBG, "config_ep slot %u\r\n", slot);

dev = XHCI_SLOTDEV_PTR(xdev, slot);
- assert(dev != NULL);
+ if (dev == NULL) {
+ cmderr = XHCI_TRB_ERROR_SLOT_NOT_ON;
+ goto done;
+ }

if ((trb->dwTrb3 & XHCI_TRB_3_DCEP_BIT) != 0) {
UPRINTF(LDBG, "config_ep - deconfigure ep slot %u\r\n", slot);
@@ -3364,7 +3424,10 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
goto errout;
}

- xdev->port_map_tbl[bus][port] = PT_ASSIGNED_WITH_NO_DEV;
+ xdev->port_map_tbl[bus][port] =
+ PT_MAKE_STATE(PT_ASSIGNED_WITH_NO_DEV, 0);
+ return 0;
+
errout:
if (rc)
UPRINTF(LWRN, "%s fails, rc=%d\r\n", __func__, rc);
--
2.7.4


[PATCH v2 6/8] DM USB: xHCI: code cleanup: change variable name

Wu, Xiaoguang
 

Replace 'native_assign_ports' with 'port_map_tbl' to be more accurate
for the role of this variable plays.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index b5d8555..c5fce1e 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -373,7 +373,7 @@ struct pci_xhci_vdev {
int (*excap_write)(struct pci_xhci_vdev *, uint64_t, uint64_t);
int usb2_port_start;
int usb3_port_start;
- uint8_t native_assign_ports[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
+ uint8_t port_map_tbl[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
struct timespec mf_prev_time; /* previous time of accessing MFINDEX */
};

@@ -504,7 +504,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
di->vid, di->pid, di->bus, di->port);

- if (xdev->native_assign_ports[di->bus][di->port] == PT_NOT_ASSIGNED) {
+ if (xdev->port_map_tbl[di->bus][di->port] == PT_NOT_ASSIGNED) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
"\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
@@ -3364,7 +3364,7 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
goto errout;
}

- xdev->native_assign_ports[bus][port] = PT_ASSIGNED_WITH_NO_DEV;
+ xdev->port_map_tbl[bus][port] = PT_ASSIGNED_WITH_NO_DEV;
errout:
if (rc)
UPRINTF(LWRN, "%s fails, rc=%d\r\n", __func__, rc);
--
2.7.4


[PATCH v2 4/8] DM USB: xHCI: limit bus and port numbers of xHCI

Wu, Xiaoguang
 

Currently the maximum number of bus and port for xHCI are
both set to 255, it is theoretically possible but in fact
not neccessary. This patch changes those two values to be
more proper: 4 buses and 20 ports.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 14 ++------------
devicemodel/include/usb_core.h | 4 ++--
2 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index b50d435..14009a8 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -373,7 +373,7 @@ struct pci_xhci_vdev {
int (*excap_write)(struct pci_xhci_vdev *, uint64_t, uint64_t);
int usb2_port_start;
int usb3_port_start;
- uint8_t *native_assign_ports[USB_NATIVE_NUM_BUS];
+ uint8_t native_assign_ports[USB_NATIVE_NUM_BUS][USB_NATIVE_NUM_PORT];
struct timespec mf_prev_time; /* previous time of accessing MFINDEX */
};

@@ -500,8 +500,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
di->vid, di->pid, di->bus, di->port);

- if (!xdev->native_assign_ports[di->bus] ||
- !xdev->native_assign_ports[di->bus][di->port]) {
+ if (!xdev->native_assign_ports[di->bus][di->port]) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
"\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
@@ -3361,15 +3360,6 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
goto errout;
}

- if (!xdev->native_assign_ports[bus]) {
- xdev->native_assign_ports[bus] = calloc(USB_NATIVE_NUM_PORT,
- sizeof(uint8_t));
- if (!xdev->native_assign_ports[bus]) {
- rc = -3;
- goto errout;
- }
- }
-
xdev->native_assign_ports[bus][port] = 1;
errout:
if (rc)
diff --git a/devicemodel/include/usb_core.h b/devicemodel/include/usb_core.h
index 8727b8b..add4cb4 100644
--- a/devicemodel/include/usb_core.h
+++ b/devicemodel/include/usb_core.h
@@ -223,8 +223,8 @@ enum USB_ERRCODE {
#define NATIVE_USBSYS_DEVDIR "/sys/bus/usb/devices"
#define NATIVE_USB2_SPEED "480"
#define NATIVE_USB3_SPEED "5000"
-#define USB_NATIVE_NUM_PORT 255
-#define USB_NATIVE_NUM_BUS 255
+#define USB_NATIVE_NUM_PORT 20
+#define USB_NATIVE_NUM_BUS 4

extern int usb_log_level;
inline int usb_get_log_level(void) { return usb_log_level; }
--
2.7.4


[PATCH v2 3/8] DM USB: introduce struct usb_native_devinfo

Wu, Xiaoguang
 

Current design cannot get physical USB device information without
the creation of pci_xhci_dev_emu. This brings some difficulties in
certain situations, hence struct usb_native_devinfo is introduced
to describe neccessary information to solve this trouble.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 33 +++++-------
devicemodel/hw/platform/usb_pmapper.c | 95 ++++++++++++++++++-----------------
devicemodel/include/usb_core.h | 10 ++++
devicemodel/include/usb_pmapper.h | 9 +---
4 files changed, 73 insertions(+), 74 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index f18660c..b50d435 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -469,22 +469,21 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
struct pci_xhci_dev_emu *de;
struct pci_xhci_vdev *xdev;
struct usb_devemu *ue;
+ struct usb_native_devinfo *di;
int port_start, port_end;
int slot_start, slot_end;
int port, slot;
void *ud;
- uint8_t native_bus, native_pid, native_port;
- uint16_t native_vid;
- int native_speed;

xdev = hci_data;
+ di = dev_data;

assert(xdev);
assert(dev_data);
assert(xdev->devices);
assert(xdev->slots);

- de = pci_xhci_dev_create(xdev, dev_data);
+ de = pci_xhci_dev_create(xdev, di);
if (!de) {
UPRINTF(LFTL, "fail to create device\r\n");
return -1;
@@ -498,26 +497,20 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
assert(ue);

/* print physical information about new device */
- ue->ue_info(ud, USB_INFO_BUS, &native_bus, sizeof(native_bus));
- ue->ue_info(ud, USB_INFO_PORT, &native_port, sizeof(native_port));
- ue->ue_info(ud, USB_INFO_VID, &native_vid, sizeof(native_vid));
- ue->ue_info(ud, USB_INFO_PID, &native_pid, sizeof(native_pid));
- ue->ue_info(ud, USB_INFO_SPEED, &native_speed, sizeof(native_speed));
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
- native_vid, native_pid, native_bus, native_port);
+ di->vid, di->pid, di->bus, di->port);

- if (!xdev->native_assign_ports[native_bus] ||
- !xdev->native_assign_ports[native_bus][native_port]) {
+ if (!xdev->native_assign_ports[di->bus] ||
+ !xdev->native_assign_ports[di->bus][di->port]) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
- "\r\n", native_vid, native_pid, native_bus,
- native_port);
+ "\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
}

- UPRINTF(LDBG, "%04x:%04x %d-%d belong to this vm.\r\n", native_vid,
- native_pid, native_bus, native_port);
+ UPRINTF(LDBG, "%04x:%04x %d-%d belong to this vm.\r\n", di->vid,
+ di->pid, di->bus, di->port);

- if (ue->ue_usbver == 2)
+ if (di->bcd < 0x300)
port_start = xdev->usb2_port_start;
else
port_start = xdev->usb3_port_start;
@@ -549,11 +542,11 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)

pci_xhci_reset_slot(xdev, slot);
UPRINTF(LDBG, "%X:%X %d-%d locates in slot %d port %d.\r\n",
- native_vid, native_pid, native_bus, native_port,
+ di->vid, di->pid, di->bus, di->port,
slot, port);

/* Trigger port change event for the arriving device */
- if (pci_xhci_connect_port(xdev, port, native_speed, 1))
+ if (pci_xhci_connect_port(xdev, port, di->speed, 1))
UPRINTF(LFTL, "fail to report port event\n");

return 0;
@@ -588,7 +581,7 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
continue;

udev = edev->dev_instance;
- if (udev->port == native_port)
+ if (udev->info.port == native_port)
break;
}

diff --git a/devicemodel/hw/platform/usb_pmapper.c b/devicemodel/hw/platform/usb_pmapper.c
index 9f57f1c..2aedf28 100644
--- a/devicemodel/hw/platform/usb_pmapper.c
+++ b/devicemodel/hw/platform/usb_pmapper.c
@@ -383,7 +383,7 @@ usb_dev_update_ep(struct usb_dev *udev)
int i, j;

assert(udev);
- if (libusb_get_active_config_descriptor(udev->ldev, &cfg))
+ if (libusb_get_active_config_descriptor(udev->info.priv_data, &cfg))
return;

for (i = 0; i < cfg->bNumInterfaces; i++) {
@@ -409,13 +409,12 @@ usb_dev_native_toggle_if(struct usb_dev *udev, int claim)

assert(udev);
assert(udev->handle);
- assert(udev->ldev);
assert(claim == 1 || claim == 0);

- b = udev->bus;
- p = udev->port;
+ b = udev->info.bus;
+ p = udev->info.port;

- r = libusb_get_active_config_descriptor(udev->ldev, &config);
+ r = libusb_get_active_config_descriptor(udev->info.priv_data, &config);
if (r) {
UPRINTF(LWRN, "%d-%d: can't get config\r\n", b, p);
return -1;
@@ -458,13 +457,12 @@ usb_dev_native_toggle_if_drivers(struct usb_dev *udev, int attach)

assert(udev);
assert(udev->handle);
- assert(udev->ldev);
assert(attach == 1 || attach == 0);

- b = udev->bus;
- p = udev->port;
+ b = udev->info.bus;
+ p = udev->info.port;

- r = libusb_get_active_config_descriptor(udev->ldev, &config);
+ r = libusb_get_active_config_descriptor(udev->info.priv_data, &config);
if (r) {
UPRINTF(LWRN, "%d-%d: can't get config\r\n", b, p);
return -1;
@@ -503,7 +501,6 @@ usb_dev_set_config(struct usb_dev *udev, struct usb_data_xfer *xfer, int config)
struct libusb_config_descriptor *cfg;

assert(udev);
- assert(udev->ldev);
assert(udev->handle);

/*
@@ -521,7 +518,7 @@ usb_dev_set_config(struct usb_dev *udev, struct usb_data_xfer *xfer, int config)
}

/* claim all the interfaces of this configuration */
- rc = libusb_get_active_config_descriptor(udev->ldev, &cfg);
+ rc = libusb_get_active_config_descriptor(udev->info.priv_data, &cfg);
if (rc) {
UPRINTF(LWRN, "fail to get config rc %d\r\n", rc);
goto err2;
@@ -545,7 +542,8 @@ err1:
usb_dev_native_toggle_if(udev, 0);
libusb_free_config_descriptor(cfg);
err2:
- UPRINTF(LWRN, "%d-%d: fail to set config\r\n", udev->bus, udev->port);
+ UPRINTF(LWRN, "%d-%d: fail to set config\r\n", udev->info.bus,
+ udev->info.port);
xfer->status = USB_ERR_STALLED;
}

@@ -560,8 +558,8 @@ usb_dev_set_if(struct usb_dev *udev, int iface, int alt, struct usb_data_xfer
if (iface >= USB_NUM_INTERFACE)
goto errout;

- UPRINTF(LDBG, "%d-%d set if, iface %d alt %d\r\n", udev->bus,
- udev->port, iface, alt);
+ UPRINTF(LDBG, "%d-%d set if, iface %d alt %d\r\n", udev->info.bus,
+ udev->info.port, iface, alt);

if (libusb_set_interface_alt_setting(udev->handle, iface, alt))
goto errout;
@@ -578,7 +576,7 @@ usb_dev_set_if(struct usb_dev *udev, int iface, int alt, struct usb_data_xfer
errout:
xfer->status = USB_ERR_STALLED;
UPRINTF(LDBG, "%d-%d fail to set if, iface %d alt %d\r\n",
- udev->bus, udev->port, iface, alt);
+ udev->info.bus, udev->info.port, iface, alt);
}

static struct usb_data_xfer_block *
@@ -773,7 +771,7 @@ usb_dev_request(void *pdata, struct usb_data_xfer *xfer)
assert(udev);

xfer->status = USB_ERR_NORMAL_COMPLETION;
- if (!udev->ldev || !xfer->ureq) {
+ if (!udev->info.priv_data || !xfer->ureq) {
UPRINTF(LWRN, "invalid request\r\n");
xfer->status = USB_ERR_IOERROR;
goto out;
@@ -861,22 +859,18 @@ void *
usb_dev_init(void *pdata, char *opt)
{
struct usb_dev *udev = NULL;
- struct libusb_device *ldev;
struct libusb_device_descriptor desc;
- uint8_t bus, port;
- int speed, ver;
+ struct usb_native_devinfo *di;
+ int ver;

assert(pdata);

- ldev = pdata;
- speed = libusb_get_device_speed(ldev);
- port = libusb_get_port_number(ldev);
- bus = libusb_get_bus_number(ldev);
- libusb_get_device_descriptor(ldev, &desc);
+ di = pdata;
+ libusb_get_device_descriptor(di->priv_data, &desc);
UPRINTF(LINF, "Found USB device: %d-%d\r\nPID(0x%X), VID(0x%X) CLASS"
- "(0x%X) SUBCLASS(0x%X) BCD(0x%X) SPEED(%d)\r\n", bus,
- port, desc.idProduct, desc.idVendor, desc.bDeviceClass,
- desc.bDeviceSubClass, desc.bcdUSB, speed);
+ "(0x%X) SUBCLASS(0x%X) BCD(0x%X) SPEED(%d)\r\n",
+ di->bus, di->port, di->pid, di->vid, desc.bDeviceClass,
+ desc.bDeviceSubClass, di->bcd, di->speed);

/* allocate and populate udev */
udev = calloc(1, sizeof(struct usb_dev));
@@ -884,7 +878,7 @@ usb_dev_init(void *pdata, char *opt)
goto errout;

/* this is a root hub */
- if (port == 0)
+ if (di->port == 0)
goto errout;

switch (desc.bcdUSB) {
@@ -908,17 +902,12 @@ usb_dev_init(void *pdata, char *opt)
goto errout;
}

- udev->speed = speed;
- udev->ldev = ldev;
+ udev->info = *di;
udev->version = ver;
udev->handle = NULL;
- udev->port = port;
- udev->bus = bus;
- udev->pid = desc.idProduct;
- udev->vid = desc.idVendor;

/* configure physical device through libusb library */
- if (libusb_open(udev->ldev, &udev->handle)) {
+ if (libusb_open(udev->info.priv_data, &udev->handle)) {
UPRINTF(LWRN, "fail to open device.\r\n");
goto errout;
}
@@ -973,25 +962,25 @@ usb_dev_info(void *pdata, int type, void *value, int size)
pv = &udev->version;
break;
case USB_INFO_SPEED:
- sz = sizeof(udev->speed);
- udev->speed = libusb_speed_to_usb_speed(udev->speed);
- pv = &udev->speed;
+ sz = sizeof(udev->info.speed);
+ udev->info.speed = libusb_speed_to_usb_speed(udev->info.speed);
+ pv = &udev->info.speed;
break;
case USB_INFO_BUS:
- sz = sizeof(udev->bus);
- pv = &udev->bus;
+ sz = sizeof(udev->info.bus);
+ pv = &udev->info.bus;
break;
case USB_INFO_PORT:
- sz = sizeof(udev->port);
- pv = &udev->port;
+ sz = sizeof(udev->info.port);
+ pv = &udev->info.port;
break;
case USB_INFO_VID:
- sz = sizeof(udev->vid);
- pv = &udev->vid;
+ sz = sizeof(udev->info.vid);
+ pv = &udev->info.vid;
break;
case USB_INFO_PID:
- sz = sizeof(udev->pid);
- pv = &udev->pid;
+ sz = sizeof(udev->info.pid);
+ pv = &udev->info.pid;
break;
default:
return -1;
@@ -1020,6 +1009,9 @@ static int
usb_dev_native_sys_conn_cb(struct libusb_context *ctx, struct libusb_device
*ldev, libusb_hotplug_event event, void *pdata)
{
+ struct libusb_device_descriptor d;
+ struct usb_native_devinfo di;
+
UPRINTF(LDBG, "connect event\r\n");

if (!ctx || !ldev) {
@@ -1027,8 +1019,17 @@ usb_dev_native_sys_conn_cb(struct libusb_context *ctx, struct libusb_device
return -1;
}

+ libusb_get_device_descriptor(ldev, &d);
+ di.bus = libusb_get_bus_number(ldev);
+ di.speed = libusb_get_device_speed(ldev);
+ di.port = libusb_get_port_number(ldev);
+ di.pid = d.idProduct;
+ di.vid = d.idVendor;
+ di.bcd = d.bcdUSB;
+ di.priv_data = ldev;
+
if (g_ctx.conn_cb)
- g_ctx.conn_cb(g_ctx.hci_data, ldev);
+ g_ctx.conn_cb(g_ctx.hci_data, &di);

return 0;
}
diff --git a/devicemodel/include/usb_core.h b/devicemodel/include/usb_core.h
index 3a9648a..8727b8b 100644
--- a/devicemodel/include/usb_core.h
+++ b/devicemodel/include/usb_core.h
@@ -163,6 +163,16 @@ struct usb_data_xfer {
pthread_mutex_t mtx;
};

+struct usb_native_devinfo {
+ int speed;
+ uint8_t bus;
+ uint8_t port;
+ uint16_t bcd;
+ uint16_t pid;
+ uint16_t vid;
+ void *priv_data;
+};
+
enum USB_ERRCODE {
USB_ACK,
USB_NAK,
diff --git a/devicemodel/include/usb_pmapper.h b/devicemodel/include/usb_pmapper.h
index 144c68f..43cb09f 100644
--- a/devicemodel/include/usb_pmapper.h
+++ b/devicemodel/include/usb_pmapper.h
@@ -36,14 +36,10 @@ struct usb_dev_ep {

struct usb_dev {
/* physical device info */
+ struct usb_native_devinfo info;
int addr;
int version;
- int speed;
int configuration;
- uint8_t port;
- uint8_t bus;
- uint8_t pid;
- uint16_t vid;

/* interface info */
int if_num;
@@ -55,8 +51,7 @@ struct usb_dev {
struct usb_dev_ep epo[USB_NUM_ENDPOINT];

/* libusb data */
- libusb_device *ldev;
- libusb_device_handle *handle;
+ libusb_device_handle *handle;
};

/*
--
2.7.4


[PATCH v2 5/8] DM USB: xHCI: refine port assignment logic

Wu, Xiaoguang
 

The variable native_assign_ports in struct pci_xhci_vdev is used
to record wether certain root hub port in SOS is assigned to UOS.
The logic uses zero to express 'not assigned' and nonzero to express
'assigned'. In this patch, use macro to replace number to express
better.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index 14009a8..b5d8555 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -384,6 +384,10 @@ struct pci_xhci_vdev {
#define XHCI_HALTED(xdev) ((xdev)->opregs.usbsts & XHCI_STS_HCH)
#define XHCI_GADDR(xdev, a) paddr_guest2host((xdev)->dev->vmctx, (a), \
XHCI_PADDR_SZ - ((a) & (XHCI_PADDR_SZ-1)))
+
+#define PT_NOT_ASSIGNED (0)
+#define PT_ASSIGNED_WITH_NO_DEV (-1)
+
struct pci_xhci_option_elem {
char *parse_opt;
int (*parse_fn)(struct pci_xhci_vdev *, char *);
@@ -500,7 +504,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
di->vid, di->pid, di->bus, di->port);

- if (!xdev->native_assign_ports[di->bus][di->port]) {
+ if (xdev->native_assign_ports[di->bus][di->port] == PT_NOT_ASSIGNED) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
"\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
@@ -3360,7 +3364,7 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
goto errout;
}

- xdev->native_assign_ports[bus][port] = 1;
+ xdev->native_assign_ports[bus][port] = PT_ASSIGNED_WITH_NO_DEV;
errout:
if (rc)
UPRINTF(LWRN, "%s fails, rc=%d\r\n", __func__, rc);
--
2.7.4


[PATCH v2 2/8] DM USB: xHCI: refine xHCI PORTSC Register related functions

Wu, Xiaoguang
 

PORTSC (Port Status and Control Register) register play a very
important role in USB sub-system. This patch is used to refine
related manipulation functions.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 68 +++++++++++++++++++++--------------------------
1 file changed, 30 insertions(+), 38 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index 792d930..f18660c 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -440,10 +440,13 @@ static void pci_xhci_update_ep_ring(struct pci_xhci_vdev *xdev,
uint32_t streamid, uint64_t ringaddr,
int ccs);
static void pci_xhci_init_port(struct pci_xhci_vdev *xdev, int portn);
+static int pci_xhci_connect_port(struct pci_xhci_vdev *xdev, int port,
+ int usb_speed, int need_intr);
+static int pci_xhci_disconnect_port(struct pci_xhci_vdev *xdev, int port,
+ int need_intr);
static struct pci_xhci_dev_emu *pci_xhci_dev_create(struct pci_xhci_vdev *
xdev, void *dev_data);
static void pci_xhci_dev_destroy(struct pci_xhci_dev_emu *de);
-static int pci_xhci_port_chg(struct pci_xhci_vdev *xdev, int port, int conn);
static void pci_xhci_set_evtrb(struct xhci_trb *evtrb, uint64_t port,
uint32_t errcode, uint32_t evtype);
static int pci_xhci_xfer_complete(struct pci_xhci_vdev *xdev,
@@ -472,6 +475,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
void *ud;
uint8_t native_bus, native_pid, native_port;
uint16_t native_vid;
+ int native_speed;

xdev = hci_data;

@@ -498,6 +502,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
ue->ue_info(ud, USB_INFO_PORT, &native_port, sizeof(native_port));
ue->ue_info(ud, USB_INFO_VID, &native_vid, sizeof(native_vid));
ue->ue_info(ud, USB_INFO_PID, &native_pid, sizeof(native_pid));
+ ue->ue_info(ud, USB_INFO_SPEED, &native_speed, sizeof(native_speed));
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
native_vid, native_pid, native_bus, native_port);

@@ -543,14 +548,12 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
xdev->ndevices++;

pci_xhci_reset_slot(xdev, slot);
- pci_xhci_init_port(xdev, port);
-
UPRINTF(LDBG, "%X:%X %d-%d locates in slot %d port %d.\r\n",
native_vid, native_pid, native_bus, native_port,
slot, port);

/* Trigger port change event for the arriving device */
- if (pci_xhci_port_chg(xdev, port, 1))
+ if (pci_xhci_connect_port(xdev, port, native_speed, 1))
UPRINTF(LFTL, "fail to report port event\n");

return 0;
@@ -595,7 +598,7 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
}

UPRINTF(LDBG, "report virtual port %d status\r\n", port);
- if (pci_xhci_port_chg(xdev, port, 0)) {
+ if (pci_xhci_disconnect_port(xdev, port, 1)) {
UPRINTF(LFTL, "fail to report event\r\n");
return -1;
}
@@ -784,32 +787,29 @@ pci_xhci_convert_speed(int lspeed)
}

static int
-pci_xhci_port_chg(struct pci_xhci_vdev *xdev, int port, int conn)
+pci_xhci_change_port(struct pci_xhci_vdev *xdev, int port, int usb_speed,
+ int conn, int need_intr)
{
int speed, error;
struct xhci_trb evtrb;
struct pci_xhci_portregs *reg;
- struct pci_xhci_dev_emu *dev;

assert(xdev != NULL);

reg = XHCI_PORTREG_PTR(xdev, port);
- dev = XHCI_DEVINST_PTR(xdev, port);
- if (!dev || !dev->dev_ue || !reg) {
- UPRINTF(LWRN, "find nullptr with port %d\r\n", port);
- return -1;
- }
-
if (conn == 0) {
reg->portsc &= ~XHCI_PS_CCS;
reg->portsc |= (XHCI_PS_CSC |
XHCI_PS_PLS_SET(UPS_PORT_LS_RX_DET));
} else {
- speed = pci_xhci_convert_speed(dev->dev_ue->ue_usbspeed);
+ speed = pci_xhci_convert_speed(usb_speed);
reg->portsc = XHCI_PS_CCS | XHCI_PS_PP | XHCI_PS_CSC;
reg->portsc |= XHCI_PS_SPEED_SET(speed);
}

+ if (!need_intr)
+ return 0;
+
/* make an event for the guest OS */
pci_xhci_set_evtrb(&evtrb,
port,
@@ -825,6 +825,20 @@ pci_xhci_port_chg(struct pci_xhci_vdev *xdev, int port, int conn)
return (error == XHCI_TRB_ERROR_SUCCESS) ? 0 : -1;
}

+static int
+pci_xhci_connect_port(struct pci_xhci_vdev *xdev, int port, int usb_speed,
+ int intr)
+{
+ return pci_xhci_change_port(xdev, port, usb_speed, 1, intr);
+}
+
+static int
+pci_xhci_disconnect_port(struct pci_xhci_vdev *xdev, int port, int intr)
+{
+ /* for disconnect, the speed is useless */
+ return pci_xhci_change_port(xdev, port, 0, 0, intr);
+}
+
static void
pci_xhci_set_evtrb(struct xhci_trb *evtrb, uint64_t port, uint32_t errcode,
uint32_t evtype)
@@ -3208,30 +3222,8 @@ pci_xhci_reset_port(struct pci_xhci_vdev *xdev, int portn, int warm)
static void
pci_xhci_init_port(struct pci_xhci_vdev *xdev, int portn)
{
- struct pci_xhci_portregs *port;
- struct pci_xhci_dev_emu *dev;
-
- port = XHCI_PORTREG_PTR(xdev, portn);
- dev = XHCI_DEVINST_PTR(xdev, portn);
- if (dev) {
- port->portsc = XHCI_PS_CCS | /* connected */
- XHCI_PS_PP; /* port power */
-
- if (dev->dev_ue->ue_usbver == 2) {
- port->portsc |= XHCI_PS_PLS_SET(UPS_PORT_LS_POLL) |
- XHCI_PS_SPEED_SET(dev->dev_ue->ue_usbspeed);
- } else {
- port->portsc |= XHCI_PS_PLS_SET(UPS_PORT_LS_U0) |
- XHCI_PS_PED | /* enabled */
- XHCI_PS_SPEED_SET(dev->dev_ue->ue_usbspeed);
- }
-
- UPRINTF(LDBG, "Init port %d 0x%x\n", portn, port->portsc);
- } else {
- port->portsc = XHCI_PS_PLS_SET(UPS_PORT_LS_RX_DET) | XHCI_PS_PP;
- UPRINTF(LDBG, "Init empty port %d 0x%x\n",
- portn, port->portsc);
- }
+ XHCI_PORTREG_PTR(xdev, portn)->portsc =
+ XHCI_PS_PLS_SET(UPS_PORT_LS_RX_DET) | XHCI_PS_PP;
}

static int
--
2.7.4


[PATCH v2 1/8] DM USB: xHCI: fix an xHCI issue to enable UOS s3 feature

Wu, Xiaoguang
 

Current DM design use two variables to do the indexing of xHCI
Event Ring: er_enq_idx and er_events_cnt. They are members of
the struct pci_xhci_rtsregs.

In UOS, during the process of xHCI resuming, the xHCI driver
will restore the ERSTBA (Event Ring Segment Table Base Address)
register to be the value before suspending. And at this point,
the old DM implementation will set both er_enq_idx and
er_events_cnt to be zero, so the DM will access the Event Ring
from the start position in the buffer. But at the same time the
UOS xHCI driver still wants to access the old position in the
Event Ring before suspending, which will result of unexpected
errors.

Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
Acked-by: Yu Wang <yu1.wang@...>
---
devicemodel/hw/pci/xhci.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index da0c42a..792d930 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -2732,9 +2732,6 @@ pci_xhci_rtsregs_write(struct pci_xhci_vdev *xdev,
rts->erst_p = XHCI_GADDR(xdev,
xdev->rtsregs.erstba_p->qwEvrsTablePtr & ~0x3FUL);

- rts->er_enq_idx = 0;
- rts->er_events_cnt = 0;
-
UPRINTF(LDBG, "wr erstba erst (%p) ptr 0x%lx, sz %u\r\n",
rts->erstba_p,
rts->erstba_p->qwEvrsTablePtr,
--
2.7.4