[PATCH 5/6] 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 disconnection 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 when the SOS do the resuming from S3, there are some uncertain
bugs which unbind the usbfs for the USB devices, so the DM will lose
control and can't 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 | 220 +++++++++++++++++++++++++++++++++-------------
1 file changed, 158 insertions(+), 62 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index 2df7ce3..579021a 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -383,6 +383,7 @@ struct pci_xhci_vdev {
* is stored in native_assign_ports[x][y].
*/
int8_t *native_assign_ports[USB_NATIVE_NUM_BUS];
+ struct usb_native_devinfo *native_assign_info[USB_NATIVE_NUM_BUS];
struct timespec mf_prev_time; /* previous time of accessing MFINDEX */
};

@@ -475,36 +476,21 @@ 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 port;
int intr = 1;
+ int i, j;

xdev = hci_data;
- di = dev_data;

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

- de = pci_xhci_dev_create(xdev, di->priv_data);
- 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",
@@ -517,43 +503,53 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
goto errout;
}

+ assert(xdev->native_assign_info[di->bus]);
UPRINTF(LDBG, "%04x:%04x %d-%d belong to this vm.\r\n", di->vid,
di->pid, di->bus, di->port);

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

/* find free port */
- for (port = port_start; port < port_end; port++)
- if (!xdev->devices[port])
- break;
+ /* FIXME:
+ * Will find a decent way to deal with the relationship among Slot,
+ * Port and Device.
+ */
+ for (port = port_start; port < port_end; port++) {
+ if (xdev->devices[port])
+ continue;
+
+ for (i = 0; i < USB_NATIVE_NUM_BUS; ++i) {
+ if (!xdev->native_assign_ports[i])
+ continue;
+
+ for (j = 0; j < USB_NATIVE_NUM_PORT; ++j)
+ if (xdev->native_assign_ports[i][j] == port)
+ break;

- /* find free slot */
- for (slot = slot_start; slot < slot_end; slot++)
- if (!xdev->slots[slot])
+ if (j < USB_NATIVE_NUM_PORT)
+ break;
+ }
+ if (i >= USB_NATIVE_NUM_BUS)
break;
+ }

- if (port >= port_end || slot >= slot_end) {
- UPRINTF(LFTL, "no free resource: port %d slot %d\r\n",
- port, slot);
+ if (port >= port_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, "%X:%X %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_assign_ports[di->bus][di->port] = port;
+ xdev->native_assign_info[di->bus][di->port] = *di;

/* Trigger port change event for the arriving device */
if (pci_xhci_port_connect(xdev, port, di->speed, intr))
@@ -561,7 +557,6 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)

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

@@ -571,8 +566,8 @@ 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_dev *udev;
- uint8_t port, native_port;
- int intr = 1;
+ uint8_t port, slot, native_port;
+ int i, j, intr = 1;

assert(hci_data);
assert(dev_data);
@@ -601,6 +596,24 @@ 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;
+
+ /* FIXME: again, find a better relationship for Slot, Port and Device */
+ for (i = 0; i < USB_NATIVE_NUM_BUS; ++i) {
+ if (!xdev->native_assign_ports[i])
+ continue;
+
+ for (j = 0; j < USB_NATIVE_NUM_PORT; ++j)
+ if (xdev->native_assign_ports[i][j] == port)
+ break;
+
+ if (j < USB_NATIVE_NUM_PORT)
+ break;
+ }
+ assert(i < USB_NATIVE_NUM_BUS);
+ xdev->native_assign_ports[i][j] = -1;
UPRINTF(LDBG, "report virtual port %d status\r\n", port);
if (pci_xhci_port_disconnect(xdev, port, intr)) {
UPRINTF(LFTL, "fail to report event\r\n");
@@ -1405,21 +1418,61 @@ 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, j, port;

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;
+
+ for (i = 0; i < USB_NATIVE_NUM_BUS; ++i) {
+ if (!xdev->native_assign_ports[i])
+ continue;
+
+ for (j = 0; j < USB_NATIVE_NUM_PORT; ++j) {
+ if (xdev->native_assign_ports[i][j] > 0) {
+ port = xdev->native_assign_ports[i][j];
+ assert(port <= XHCI_MAX_DEVS);
+
+ if (!xdev->devices[port])
+ break;
}
}
+ if (j < USB_NATIVE_NUM_PORT)
+ break;
+ }
+
+ if (i >= USB_NATIVE_NUM_BUS) {
+ UPRINTF(LWRN, "unexpected Enable Slot commnad\r\n");
+ return -1;
+ }
+
+ assert(xdev->native_assign_info[i]);
+ di = &xdev->native_assign_info[i][j];
+
+ assert(di->priv_data);
+ dev = pci_xhci_dev_create(xdev, di->priv_data);
+ if (!dev) {
+ UPRINTF(LFTL, "fail to create device\r\n");
+ return -1;
+ }
+
+ port = xdev->native_assign_ports[i][j];
+ assert(port > 0);
+ assert(!xdev->devices[port]);
+
+ xdev->devices[port] = 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;
+ break;
+ }
+ }

UPRINTF(LDBG, "enable slot (error=%d) slot %u\r\n",
cmderr != XHCI_TRB_ERROR_SUCCESS, *slot);
@@ -1431,7 +1484,10 @@ static uint32_t
pci_xhci_cmd_disable_slot(struct pci_xhci_vdev *xdev, uint32_t slot)
{
struct pci_xhci_dev_emu *dev;
+ struct usb_devemu *ue;
+ uint8_t native_port, native_bus;
uint32_t cmderr;
+ void *ud;
int i;

UPRINTF(LDBG, "pci_xhci disable slot %u\r\n", slot);
@@ -1454,6 +1510,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)
@@ -1463,8 +1522,25 @@ 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);
+
+ ud = dev->dev_instance;
+ ue = dev->dev_ue;
+
+ assert(ud);
+ assert(ue);
+
+ ue->ue_info(ud, USB_INFO_BUS, &native_bus, sizeof(uint8_t));
+ ue->ue_info(ud, USB_INFO_PORT, &native_port, sizeof(uint8_t));
+
xdev->devices[i] = NULL;
xdev->slots[slot] = NULL;
+
+ assert(xdev->native_assign_ports[native_bus]);
+ assert(xdev->native_assign_ports[native_bus][native_port]);
+ UPRINTF(LINF, "disable slot %d for native device %d-%d"
+ "\r\n", slot, native_bus, native_port);
+
+ xdev->native_assign_ports[native_bus][native_port] = -1;
pci_xhci_dev_destroy(dev);
} else
UPRINTF(LWRN, "invalid slot %d\r\n", slot);
@@ -1632,7 +1708,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);
@@ -3352,8 +3431,10 @@ errout:
static int
pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
{
- int rc = 0, cnt;
+ int rc = 0, cnt, portcnt;
uint32_t port, bus;
+ int8_t *pports;
+ struct usb_native_devinfo *pinfo;

assert(xdev);
assert(opts);
@@ -3372,15 +3453,30 @@ 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(int8_t));
- if (!xdev->native_assign_ports[bus]) {
+ pports = xdev->native_assign_ports[bus];
+ pinfo = xdev->native_assign_info[bus];
+
+ /* those two pointers should always be consistant */
+ assert((!pports) == (!pinfo));
+
+ if (!pports) {
+ portcnt = USB_NATIVE_NUM_PORT;
+ pports = calloc(portcnt, sizeof(uint8_t));
+ pinfo = calloc(portcnt, sizeof(struct usb_native_devinfo));
+ if (!pports || !pinfo) {
rc = -3;
- goto errout;
+ goto release_out;
}
+
+ xdev->native_assign_ports[bus] = pports;
+ xdev->native_assign_info[bus] = pinfo;
}
xdev->native_assign_ports[bus][port] = -1;
+ return 0;
+
+release_out:
+ free(pinfo);
+ free(pports);

errout:
if (rc)
--
2.7.4

Join acrn-dev@lists.projectacrn.org to automatically receive all group messages.