Date   

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


[PATCH v2 0/8] USB xHCI S3 support

Wu, Xiaoguang
 

This patchset enable the USB xHCI S3 feature.

Xiaoguang Wu (8):
DM USB: xHCI: fix an xHCI issue to enable UOS s3 feature
DM USB: xHCI: refine xHCI PORTSC Register related functions
DM USB: introduce struct usb_native_devinfo
DM USB: xHCI: limit bus and port numbers of xHCI
DM USB: xHCI: refine port assignment logic
DM USB: xHCI: code cleanup: change variable name
DM USB: xHCI: change flow of creation of virtual USB device
DM USB: xHCI: enable xHCI SOS S3 support

devicemodel/hw/pci/xhci.c | 305 ++++++++++++++++++++--------------
devicemodel/hw/platform/usb_pmapper.c | 95 +++++------
devicemodel/include/usb_core.h | 14 +-
devicemodel/include/usb_pmapper.h | 9 +-
4 files changed, 242 insertions(+), 181 deletions(-)

--
2.7.4


[PATCH 8/8] hv: add gva check for the case gva is from instruction decode

Yin, Fengwei <fengwei.yin@...>
 

For the instructions other than MOVS and STO, the gva comes
from instruction decoding. Apply the gva check for this type
of gva and inject exception to guest accordingly during
instruction decoding phase.

Signed-off-by: Yin Fengwei <fengwei.yin@...>
---
hypervisor/arch/x86/guest/instr_emul.c | 87 ++++++++++++++++++++++++++
1 file changed, 87 insertions(+)

diff --git a/hypervisor/arch/x86/guest/instr_emul.c b/hypervisor/arch/x86/guest/instr_emul.c
index 49a25ded..f1f23ffa 100644
--- a/hypervisor/arch/x86/guest/instr_emul.c
+++ b/hypervisor/arch/x86/guest/instr_emul.c
@@ -2173,6 +2173,91 @@ static int instr_check_di_si(struct vcpu *vcpu,
}
}

+static int instr_check_gva(struct vcpu *vcpu, struct instr_emul_ctxt *emul_ctxt,
+ enum vm_cpu_mode cpu_mode)
+{
+ int ret = 0;
+ uint64_t base, segbase, idx, gva, gpa;
+ uint32_t err_code;
+ enum cpu_reg_name seg;
+ struct instr_emul_vie *vie = &emul_ctxt->vie;
+
+ base = 0UL;
+ if (vie->base_register != CPU_REG_LAST) {
+ base = vm_get_register(vcpu, vie->base_register);
+
+ /* RIP relative addressing starts from the
+ * following instruction
+ */
+ if (vie->base_register == CPU_REG_RIP)
+ base += vie->num_processed;
+
+ }
+
+ idx = 0UL;
+ if (vie->index_register != CPU_REG_LAST) {
+ idx = vm_get_register(vcpu, vie->index_register);
+ }
+
+ /* "Specifying a Segment Selector" of SDM Vol1 3.7.4
+ *
+ * In legacy IA-32 mode, when ESP or EBP register is used as
+ * base, the SS segment is default segment.
+ *
+ * All data references, except when relative to stack or
+ * string destination, DS is default segment.
+ *
+ * segment override could overwrite the default segment
+ *
+ * 64bit mode, segmentation is generally disabled. The
+ * exception are FS and GS.
+ */
+ if (vie->seg_override != 0U) {
+ seg = vie->segment_register;
+ } else if ((vie->base_register == CPU_REG_RSP) ||
+ (vie->base_register == CPU_REG_RBP)) {
+ seg = CPU_REG_SS;
+ } else {
+ seg = CPU_REG_DS;
+ }
+
+ if ((cpu_mode == CPU_MODE_64BIT) && (seg != CPU_REG_FS) &&
+ (seg != CPU_REG_GS)) {
+ segbase = 0UL;
+ } else {
+ struct seg_desc desc;
+
+ vm_get_seg_desc(seg, &desc);
+
+ segbase = desc.base;
+ }
+
+ gva = segbase + base + vie->scale * idx + vie->displacement;
+
+ if (vie_canonical_check(cpu_mode, gva) != 0) {
+ if (seg == CPU_REG_SS) {
+ vcpu_inject_ss(vcpu);
+ } else {
+ vcpu_inject_gp(vcpu, 0U);
+ }
+ return -EFAULT;
+ }
+
+ err_code = (vcpu->req.reqs.mmio.direction == REQUEST_WRITE) ?
+ PAGE_FAULT_WR_FLAG : 0U;
+
+ ret = gva2gpa(vcpu, gva, &gpa, &err_code);
+ if (ret < 0) {
+ if (ret == -EFAULT) {
+ vcpu_inject_pf(vcpu, gva,
+ err_code);
+ }
+ return ret;
+ }
+
+ return 0;
+}
+
int decode_instruction(struct vcpu *vcpu)
{
struct instr_emul_ctxt *emul_ctxt;
@@ -2219,6 +2304,8 @@ int decode_instruction(struct vcpu *vcpu)
retval = instr_check_di_si(vcpu, emul_ctxt);
if (retval < 0)
return retval;
+ } else {
+ instr_check_gva(vcpu, emul_ctxt, cpu_mode);
}

return emul_ctxt->vie.opsize;
--
2.17.0


[PATCH 7/8] hv: add failure case check for MOVS/STO

Yin, Fengwei <fengwei.yin@...>
 

MOVS and STO instruction get the operand line address from DI
and SI. This patch adds line address check after instruction
decoding is done. Inject exception accordingly if check fails.

We add check during instruction decoding phase. So we could
assume the following instruction emulation will always success.

Signed-off-by: Yin Fengwei <fengwei.yin@...>
---
hypervisor/arch/x86/guest/instr_emul.c | 52 ++++++++++++++++++++++----
1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/hypervisor/arch/x86/guest/instr_emul.c b/hypervisor/arch/x86/guest/instr_emul.c
index 2012945c..49a25ded 100644
--- a/hypervisor/arch/x86/guest/instr_emul.c
+++ b/hypervisor/arch/x86/guest/instr_emul.c
@@ -55,7 +55,7 @@
#define VIE_OP_F_IMM8 (1U << 1) /* 8-bit immediate operand */
#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_GVA_IN_DI_SI (1U << 4) /* GVA from (R|E)SI/DI */
#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] = {
@@ -121,19 +121,19 @@ static const struct instr_emul_vie_op one_byte_opcodes[256] = {
},
[0xA4] = {
.op_type = VIE_OP_TYPE_MOVS,
- .op_flags = VIE_OP_F_NO_MODRM | VIE_OP_F_NO_GLA_VERIFICATION
+ .op_flags = VIE_OP_F_NO_MODRM | VIE_OP_F_GVA_IN_DI_SI
},
[0xA5] = {
.op_type = VIE_OP_TYPE_MOVS,
- .op_flags = VIE_OP_F_NO_MODRM | VIE_OP_F_NO_GLA_VERIFICATION
+ .op_flags = VIE_OP_F_NO_MODRM | VIE_OP_F_GVA_IN_DI_SI
},
[0xAA] = {
.op_type = VIE_OP_TYPE_STOS,
- .op_flags = VIE_OP_F_NO_MODRM | VIE_OP_F_NO_GLA_VERIFICATION
+ .op_flags = VIE_OP_F_NO_MODRM | VIE_OP_F_GVA_IN_DI_SI
},
[0xAB] = {
.op_type = VIE_OP_TYPE_STOS,
- .op_flags = VIE_OP_F_NO_MODRM | VIE_OP_F_NO_GLA_VERIFICATION
+ .op_flags = VIE_OP_F_NO_MODRM | VIE_OP_F_GVA_IN_DI_SI
},
[0xC6] = {
/* XXX Group 11 extended opcode - not just MOV */
@@ -969,7 +969,7 @@ static int emulate_movs(struct vcpu *vcpu, struct instr_emul_vie *vie)
{
uint64_t dstaddr, srcaddr;
uint64_t rcx, rdi, rsi, rflags;
- int error, fault, repeat;
+ int error, repeat;
uint8_t opsize;
enum cpu_reg_name seg;

@@ -1549,7 +1549,6 @@ static int emulate_bittest(struct vcpu *vcpu, struct instr_emul_vie *vie)

static int vmm_emulate_instruction(struct instr_emul_ctxt *ctxt)
{
- struct vm_guest_paging *paging = &ctxt->paging;
struct instr_emul_vie *vie = &ctxt->vie;
struct vcpu *vcpu = ctxt->vcpu;
int error;
@@ -2146,6 +2145,34 @@ static int local_decode_instruction(enum vm_cpu_mode cpu_mode,
return 0;
}

+/* for instruction MOVS/STO, check the gva gotten from DI/SI. */
+static int instr_check_di_si(struct vcpu *vcpu,
+ struct instr_emul_ctxt *emul_ctxt)
+{
+ int ret;
+ struct instr_emul_vie *vie = &emul_ctxt->vie;
+ uint64_t gva;
+ enum cpu_reg_name seg;
+
+ ret = get_gva_di_si_check(vcpu, vie->addrsize, PROT_WRITE,
+ CPU_REG_ES, CPU_REG_RDI, &gva);
+
+ if (ret < 0) {
+ return -EFAULT;
+ }
+
+ if (vie->op.op_type == VIE_OP_TYPE_MOVS) {
+ seg = (vie->seg_override != 0U) ?
+ (vie->segment_register) : CPU_REG_DS;
+ ret = get_gva_di_si_check(vcpu, vie->addrsize, PROT_READ,
+ seg, CPU_REG_RSI, &gva);
+
+ if (ret < 0) {
+ return -EFAULT;
+ }
+ }
+}
+
int decode_instruction(struct vcpu *vcpu)
{
struct instr_emul_ctxt *emul_ctxt;
@@ -2183,6 +2210,17 @@ int decode_instruction(struct vcpu *vcpu)
return -EFAULT;
}

+ /*
+ * We do operand check in instruction decode phase and
+ * inject exception accordingly. In late instruction
+ * emulation, it will always sucess.
+ */
+ if (emul_ctxt->vie.op.op_flags & VIE_OP_F_GVA_IN_DI_SI) {
+ retval = instr_check_di_si(vcpu, emul_ctxt);
+ if (retval < 0)
+ return retval;
+ }
+
return emul_ctxt->vie.opsize;
}

--
2.17.0


[PATCH 6/8] hv: add new function to get gva for MOVS/STO instruction

Yin, Fengwei <fengwei.yin@...>
 

Drop the get_gla function and add
- get_gva_di_si_nocheck
get gva from ES:DI and DS(other segment):SI without
check faulure case
- get_gva_di_si_check
get gva from ES:DI and DS(other segment):SI with failure
case checked

Signed-off-by: Yin Fengwei <fengwei.yin@...>
---
hypervisor/arch/x86/guest/instr_emul.c | 116 ++++++++++++++++---------
1 file changed, 76 insertions(+), 40 deletions(-)

diff --git a/hypervisor/arch/x86/guest/instr_emul.c b/hypervisor/arch/x86/guest/instr_emul.c
index b2485c48..2012945c 100644
--- a/hypervisor/arch/x86/guest/instr_emul.c
+++ b/hypervisor/arch/x86/guest/instr_emul.c
@@ -470,7 +470,6 @@ static int vie_calculate_gla(enum vm_cpu_mode cpu_mode, enum cpu_reg_name seg,
uint64_t firstoff, segbase;
uint64_t offset = offset_arg;
uint8_t glasize;
- uint32_t type;

firstoff = offset;
glasize = (cpu_mode == CPU_MODE_64BIT) ? 8U: 4U;
@@ -876,52 +875,97 @@ static int emulate_movx(struct vcpu *vcpu, struct instr_emul_vie *vie)
return error;
}

+/**
+ * @pre only called by instruction emulation and check was done during
+ * instruction decode
+ *
+ * @remark This function can only be called in instruction emulation and
+ * suppose always success because the check was done during instruction
+ * decode.
+ *
+ * It's only used by MOVS/STO
+ */
+static void get_gva_di_si_nocheck(struct vcpu *vcpu, uint8_t addrsize,
+ enum cpu_reg_name seg, enum cpu_reg_name gpr, uint64_t *gva)
+{
+ uint64_t val;
+ struct seg_desc desc;
+ enum vm_cpu_mode cpu_mode;
+
+ val = vm_get_register(vcpu, gpr);
+ vm_get_seg_desc(seg, &desc);
+ cpu_mode = get_vcpu_mode(vcpu);
+
+ (void)vie_calculate_gla(cpu_mode, seg, &desc, val, addrsize, gva);
+
+ return;
+}
+
/*
- * Helper function to calculate and validate a linear address.
+ * @pre only called during instruction decode phase
+ *
+ * @remark This function get gva from ES:DI and DS(other segment):SI. And
+ * do check the failure condition and inject exception to guest accordingly.
+ *
+ * It's only used by MOVS/STO
*/
-static int get_gla(struct vcpu *vcpu, __unused struct instr_emul_vie *vie,
- struct vm_guest_paging *paging,
- uint8_t opsize, uint8_t addrsize, uint32_t prot, enum cpu_reg_name seg,
- enum cpu_reg_name gpr, uint64_t *gla, int *fault)
+static int get_gva_di_si_check(struct vcpu *vcpu, uint8_t addrsize,
+ uint32_t prot, enum cpu_reg_name seg, enum cpu_reg_name gpr,
+ uint64_t *gva)
{
int ret;
+ uint32_t err_code;
struct seg_desc desc;
- uint64_t cr0, val, rflags;
+ enum vm_cpu_mode cpu_mode;
+ uint64_t val, gpa;

- cr0 = vm_get_register(vcpu, CPU_REG_CR0);
- rflags = vm_get_register(vcpu, CPU_REG_RFLAGS);
val = vm_get_register(vcpu, gpr);
vm_get_seg_desc(seg, &desc);
+ cpu_mode = get_vcpu_mode(vcpu);

- if (vie_calculate_gla(paging->cpu_mode, seg, &desc, val,
- addrsize, gla) != 0) {
- if (seg == CPU_REG_SS) {
- pr_err("TODO: inject ss exception");
- } else {
- pr_err("TODO: inject gp exception");
+ if (!is_desc_valid(&desc, prot)) {
+ goto exception_inject;
+ }
+
+ if (cpu_mode == CPU_MODE_64BIT) {
+ if ((addrsize != 4U) && (addrsize != 8U)) {
+ goto exception_inject;
+ }
+ } else {
+ if ((addrsize != 2U) && (addrsize != 4U)) {
+ goto exception_inject;
}
- goto guest_fault;
}

- if (vie_canonical_check(paging->cpu_mode, *gla) != 0) {
- if (seg == CPU_REG_SS) {
- pr_err("TODO: inject ss exception");
- } else {
- pr_err("TODO: inject gp exception");
+ if (vie_calculate_gla(cpu_mode, seg, &desc, val, addrsize, gva) != 0) {
+ goto exception_inject;
+ }
+
+ if (vie_canonical_check(cpu_mode, *gva) != 0) {
+ goto exception_inject;
+ }
+
+ err_code = (prot == PROT_WRITE) ? PAGE_FAULT_WR_FLAG : 0U;
+ ret = gva2gpa(vcpu, (uint64_t)gva, &gpa, &err_code);
+ if (ret < 0) {
+ if (ret == -EFAULT) {
+ vcpu_inject_pf(vcpu, (uint64_t)gva, err_code);
}
- goto guest_fault;
+ return ret;
}

- *fault = 0;
return 0;

-guest_fault:
- *fault = 1;
- return ret;
+exception_inject:
+ if (seg == CPU_REG_SS) {
+ vcpu_inject_ss(vcpu);
+ } else {
+ vcpu_inject_gp(vcpu, 0U);
+ }
+ return -EFAULT;
}

-static int emulate_movs(struct vcpu *vcpu, struct instr_emul_vie *vie,
- struct vm_guest_paging *paging)
+static int emulate_movs(struct vcpu *vcpu, struct instr_emul_vie *vie)
{
uint64_t dstaddr, srcaddr;
uint64_t rcx, rdi, rsi, rflags;
@@ -955,18 +999,10 @@ static int emulate_movs(struct vcpu *vcpu, struct instr_emul_vie *vie,
}

seg = (vie->seg_override != 0U) ? (vie->segment_register) : CPU_REG_DS;
- error = get_gla(vcpu, vie, paging, opsize, vie->addrsize,
- PROT_READ, seg, CPU_REG_RSI, &srcaddr, &fault);
- if ((error != 0) || (fault != 0)) {
- goto done;
- }

- error = get_gla(vcpu, vie, paging, opsize, vie->addrsize,
- PROT_WRITE, CPU_REG_ES, CPU_REG_RDI, &dstaddr,
- &fault);
- if ((error != 0) || (fault != 0)) {
- goto done;
- }
+ get_gva_di_si_nocheck(vcpu, vie->addrsize, seg, CPU_REG_RSI, &srcaddr);
+ get_gva_di_si_nocheck(vcpu, vie->addrsize, CPU_REG_ES, CPU_REG_RDI,
+ &dstaddr);

(void)memcpy_s((void *)dstaddr, 16U, (void *)srcaddr, opsize);

@@ -1536,7 +1572,7 @@ static int vmm_emulate_instruction(struct instr_emul_ctxt *ctxt)
error = emulate_movx(vcpu, vie);
break;
case VIE_OP_TYPE_MOVS:
- error = emulate_movs(vcpu, vie, paging);
+ error = emulate_movs(vcpu, vie);
break;
case VIE_OP_TYPE_STOS:
error = emulate_stos(vcpu, vie);
--
2.17.0


[PATCH 5/8] hv: move check out of vie_calculate_gla

Yin, Fengwei <fengwei.yin@...>
 

We will do check only during instruction decode phase.
vie_calculate_gla will be called both in instruction emulation
phase, we move the check out of vie_calculate_gla.

Signed-off-by: Yin Fengwei <fengwei.yin@...>
---
hypervisor/arch/x86/guest/instr_emul.c | 82 ++++++++++++--------------
1 file changed, 37 insertions(+), 45 deletions(-)

diff --git a/hypervisor/arch/x86/guest/instr_emul.c b/hypervisor/arch/x86/guest/instr_emul.c
index 65bdecef..b2485c48 100644
--- a/hypervisor/arch/x86/guest/instr_emul.c
+++ b/hypervisor/arch/x86/guest/instr_emul.c
@@ -421,6 +421,40 @@ static int vie_canonical_check(enum vm_cpu_mode cpu_mode, uint64_t gla)
}
}

+static bool is_desc_valid(struct seg_desc *desc, uint32_t prot)
+{
+ uint32_t type;
+
+ /* The descriptor type must indicate a code/data segment. */
+ type = SEG_DESC_TYPE(desc->access);
+ if (type < 16 || type > 31) {
+ return false;
+ }
+
+ if ((prot & PROT_READ) != 0U) {
+ /* #GP on a read access to a exec-only code segment */
+ if ((type & 0xAU) == 0x8U) {
+ return false;
+ }
+ }
+
+ if ((prot & PROT_WRITE) != 0U) {
+ /*
+ * #GP on a write access to a code segment or a
+ * read-only data segment.
+ */
+ if ((type & 0x8U) != 0U) { /* code segment */
+ return false;
+ }
+
+ if ((type & 0xAU) == 0U) { /* read-only data seg */
+ return false;
+ }
+ }
+
+ return true;
+}
+
/*
*@pre seg must be segment register index
*@pre length_arg must be 1, 2, 4 or 8
@@ -431,7 +465,7 @@ static int vie_canonical_check(enum vm_cpu_mode cpu_mode, uint64_t gla)
*/
static int vie_calculate_gla(enum vm_cpu_mode cpu_mode, enum cpu_reg_name seg,
struct seg_desc *desc, uint64_t offset_arg, uint8_t addrsize,
- uint32_t prot, uint64_t *gla)
+ uint64_t *gla)
{
uint64_t firstoff, segbase;
uint64_t offset = offset_arg;
@@ -439,49 +473,7 @@ static int vie_calculate_gla(enum vm_cpu_mode cpu_mode, enum cpu_reg_name seg,
uint32_t type;

firstoff = offset;
- if (cpu_mode == CPU_MODE_64BIT) {
- if (addrsize != 4U && addrsize != 8U) {
- pr_dbg("%s: invalid addr size %d for cpu mode %d",
- __func__, addrsize, cpu_mode);
- return -1;
- }
- glasize = 8U;
- } else {
- if (addrsize != 2U && addrsize != 4U) {
- pr_dbg("%s: invalid addr size %d for cpu mode %d",
- __func__, addrsize, cpu_mode);
- return -1;
- }
- glasize = 4U;
-
- /* The descriptor type must indicate a code/data segment. */
- type = SEG_DESC_TYPE(desc->access);
- if (type < 16 || type > 31) {
- /*TODO: Inject #GP */
- return -1;
- }
-
- if ((prot & PROT_READ) != 0U) {
- /* #GP on a read access to a exec-only code segment */
- if ((type & 0xAU) == 0x8U) {
- return -1;
- }
- }
-
- if ((prot & PROT_WRITE) != 0U) {
- /*
- * #GP on a write access to a code segment or a
- * read-only data segment.
- */
- if ((type & 0x8U) != 0U) { /* code segment */
- return -1;
- }
-
- if ((type & 0xAU) == 0U) { /* read-only data seg */
- return -1;
- }
- }
- }
+ glasize = (cpu_mode == CPU_MODE_64BIT) ? 8U: 4U;

/*
* In 64-bit mode all segments except %fs and %gs have a segment
@@ -902,7 +894,7 @@ static int get_gla(struct vcpu *vcpu, __unused struct instr_emul_vie *vie,
vm_get_seg_desc(seg, &desc);

if (vie_calculate_gla(paging->cpu_mode, seg, &desc, val,
- addrsize, prot, gla) != 0) {
+ addrsize, gla) != 0) {
if (seg == CPU_REG_SS) {
pr_err("TODO: inject ss exception");
} else {
--
2.17.0


[PATCH 4/8] hv: remove unnecessary check for gva

Yin, Fengwei <fengwei.yin@...>
 

According to SDM vol3 25.1.1
With VMX enabled, following exception will be handled by guest
without trigger VM exit:
- faults based on privilege level
- general protection due to relevent segment being unusable
- general protection due to offset beyond limit of relevent segment
- alignment check exception

ACRN always assume VMX is enabled. So we don't need to these check
in instruction emulation. But we need to do page fault related check.

Signed-off-by: Yin Fengwei <fengwei.yin@...>
---
hypervisor/arch/x86/guest/instr_emul.c | 79 +++-----------------------
1 file changed, 7 insertions(+), 72 deletions(-)

diff --git a/hypervisor/arch/x86/guest/instr_emul.c b/hypervisor/arch/x86/guest/instr_emul.c
index ab9340e1..65bdecef 100644
--- a/hypervisor/arch/x86/guest/instr_emul.c
+++ b/hypervisor/arch/x86/guest/instr_emul.c
@@ -401,18 +401,6 @@ static void get_guest_paging_info(struct vcpu *vcpu, struct instr_emul_ctxt *emu
emul_ctxt->paging.paging_mode = get_vcpu_paging_mode(vcpu);
}

-static int vie_alignment_check(uint8_t cpl, uint8_t size, uint64_t cr0,
- uint64_t rflags, uint64_t gla)
-{
- pr_dbg("Checking alignment with cpl: %hhu, addrsize: %hhu", cpl, size);
-
- if (cpl < 3U || (cr0 & CR0_AM) == 0UL || (rflags & PSL_AC) == 0UL) {
- return 0;
- }
-
- return ((gla & (size - 1U)) != 0UL) ? 1 : 0;
-}
-
static int vie_canonical_check(enum vm_cpu_mode cpu_mode, uint64_t gla)
{
uint64_t mask;
@@ -442,12 +430,11 @@ static int vie_canonical_check(enum vm_cpu_mode cpu_mode, uint64_t gla)
*return -1 - on failure
*/
static int vie_calculate_gla(enum vm_cpu_mode cpu_mode, enum cpu_reg_name seg,
- struct seg_desc *desc, uint64_t offset_arg, uint8_t length_arg,
- uint8_t addrsize, uint32_t prot, uint64_t *gla)
+ struct seg_desc *desc, uint64_t offset_arg, uint8_t addrsize,
+ uint32_t prot, uint64_t *gla)
{
- uint64_t firstoff, low_limit, high_limit, segbase;
+ uint64_t firstoff, segbase;
uint64_t offset = offset_arg;
- uint8_t length = length_arg;
uint8_t glasize;
uint32_t type;

@@ -466,25 +453,6 @@ static int vie_calculate_gla(enum vm_cpu_mode cpu_mode, enum cpu_reg_name seg,
return -1;
}
glasize = 4U;
- /*
- * If the segment selector is loaded with a NULL selector
- * then the descriptor is unusable and attempting to use
- * it results in a #GP(0).
- */
- if (SEG_DESC_UNUSABLE(desc->access)) {
- return -1;
- }
-
- /*
- * The processor generates a #NP exception when a segment
- * register is loaded with a selector that points to a
- * descriptor that is not present. If this was the case then
- * it would have been checked before the VM-exit.
- */
- if (SEG_DESC_PRESENT(desc->access) != 0) {
- /* TODO: Inject #NP */
- return -1;
- }

/* The descriptor type must indicate a code/data segment. */
type = SEG_DESC_TYPE(desc->access);
@@ -513,30 +481,6 @@ static int vie_calculate_gla(enum vm_cpu_mode cpu_mode, enum cpu_reg_name seg,
return -1;
}
}
-
- /*
- * 'desc->limit' is fully expanded taking granularity into
- * account.
- */
- if ((type & 0xCU) == 0x4U) {
- /* expand-down data segment */
- low_limit = desc->limit + 1U;
- high_limit = SEG_DESC_DEF32(desc->access) ?
- 0xffffffffU : 0xffffU;
- } else {
- /* code segment or expand-up data segment */
- low_limit = 0U;
- high_limit = desc->limit;
- }
-
- while (length > 0U) {
- offset &= size2mask[addrsize];
- if (offset < low_limit || offset > high_limit) {
- return -1;
- }
- offset++;
- length--;
- }
}

/*
@@ -948,6 +892,7 @@ static int get_gla(struct vcpu *vcpu, __unused struct instr_emul_vie *vie,
uint8_t opsize, uint8_t addrsize, uint32_t prot, enum cpu_reg_name seg,
enum cpu_reg_name gpr, uint64_t *gla, int *fault)
{
+ int ret;
struct seg_desc desc;
uint64_t cr0, val, rflags;

@@ -956,13 +901,11 @@ static int get_gla(struct vcpu *vcpu, __unused struct instr_emul_vie *vie,
val = vm_get_register(vcpu, gpr);
vm_get_seg_desc(seg, &desc);

- if (vie_calculate_gla(paging->cpu_mode, seg, &desc, val, opsize,
- addrsize, prot, gla) != 0) {
+ if (vie_calculate_gla(paging->cpu_mode, seg, &desc, val,
+ addrsize, prot, gla) != 0) {
if (seg == CPU_REG_SS) {
- /*vm_inject_ss(vcpu, 0);*/
pr_err("TODO: inject ss exception");
} else {
- /*vm_inject_gp(vcpu);*/
pr_err("TODO: inject gp exception");
}
goto guest_fault;
@@ -970,27 +913,19 @@ static int get_gla(struct vcpu *vcpu, __unused struct instr_emul_vie *vie,

if (vie_canonical_check(paging->cpu_mode, *gla) != 0) {
if (seg == CPU_REG_SS) {
- /*vm_inject_ss(vcpu, 0);*/
pr_err("TODO: inject ss exception");
} else {
- /*vm_inject_gp(vcpu);*/
pr_err("TODO: inject gp exception");
}
goto guest_fault;
}

- if (vie_alignment_check(paging->cpl, opsize, cr0, rflags, *gla) != 0) {
- /*vm_inject_ac(vcpu, 0);*/
- pr_err("TODO: inject ac exception");
- goto guest_fault;
- }
-
*fault = 0;
return 0;

guest_fault:
*fault = 1;
- return 0;
+ return ret;
}

static int emulate_movs(struct vcpu *vcpu, struct instr_emul_vie *vie,
--
2.17.0


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

Yin, Fengwei <fengwei.yin@...>
 

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


[PATCH 3/8] hv: fix use without initialization build error

Yin, Fengwei <fengwei.yin@...>
 

Fix two use without initialization build error in instr_emul.c

Signed-off-by: Yin Fengwei <fengwei.yin@...>
---
hypervisor/arch/x86/guest/instr_emul.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hypervisor/arch/x86/guest/instr_emul.c b/hypervisor/arch/x86/guest/instr_emul.c
index d62c6eaa..ab9340e1 100644
--- a/hypervisor/arch/x86/guest/instr_emul.c
+++ b/hypervisor/arch/x86/guest/instr_emul.c
@@ -326,7 +326,7 @@ static uint32_t get_vmcs_field(enum cpu_reg_name ident)
*/
static uint64_t vm_get_register(struct vcpu *vcpu, enum cpu_reg_name reg)
{
- uint64_t reg_val;
+ uint64_t reg_val = 0;

if ((reg >= CPU_REG_GENERAL_FIRST) && (reg <= CPU_REG_GENERAL_LAST)) {
reg_val = vcpu_get_gpreg(vcpu, reg);
@@ -379,7 +379,7 @@ static void vm_set_register(struct vcpu *vcpu, enum cpu_reg_name reg,
*/
static void vm_get_seg_desc(enum cpu_reg_name seg, struct seg_desc *desc)
{
- struct seg_desc tdesc;
+ struct seg_desc tdesc = {0UL, 0U, 0U};

/* tdesc->access != 0xffffffffU in this function */
encode_vmcs_seg_desc(seg, &tdesc);
--
2.17.0