[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


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




Wu, Xiaoguang
 

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

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

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

How about:
VPORT_FREE
VPORT_ASSIGNED
VPORT_CONNECTED

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

ok, will change, thanks.



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

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

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