Date   

Re: [PATCH v2 1/3] HV: Add the emulation of CPUID with 0x16 leaf

Zhao, Yakui
 

-----Original Message-----
From: acrn-dev@... [mailto:acrn-dev@...]
On Behalf Of Eddie Dong
Sent: Monday, August 13, 2018 4:04 PM
To: acrn-dev@...; Xu, Anthony <anthony.xu@...>
Subject: Re: [acrn-dev] [PATCH v2 1/3] HV: Add the emulation of CPUID with
0x16 leaf

Hi Yakui,

ACRN doesn't emulate CPUID >=0x80000000U at all.
So for CPUID >=0x80000000U, ACRN just read from hardware in
guest_cpuid().
init_vcpuid_entry doesn't need to handle CPUID >=0x80000000U.
After Eddie's suggestion is followed to add the limit check(>=0x15
CPUID), the mentioned check is not needed any more.

I keep them so that the init_vcpuid_entry still can return the zero
entry for the out-of-range CPUID input.

If you think that the check is redundant, I can remove them.
If it is physical out of ranger, returning what hardware returns is better than
fixed-zero.

We want the code to be slim to save FuSa effort.
OK. I will remove them.


Thx Eddie


Re: [PATCH] HV: hv_main: Add #ifdef HV_DEBUG before debug specific code #ifdef

Eddie Dong
 

In general we should do in different patch for bug fix. But this one is simple for those compile option --- this looks like MISRAC patches, we can do together to save

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Kaige Fu
Sent: Monday, August 13, 2018 5:18 PM
To: acrn-dev@...
Subject: Re: [acrn-dev] [PATCH] HV: hv_main: Add #ifdef HV_DEBUG before
debug specific code

On 08-13 Mon 08:27, Eddie Dong wrote:

On 08-13 Mon 07:59, Eddie Dong wrote:
This is part of SBUF component, what kind of compile option do we
use?
Let us use same one.
shell command "vmexit" is the only one user of vmexit_time/cnt and
we use the get_vmexit_profile, enclosed by HV_DEBUG, to access these
variables.

Ok then.


BTW, hcall side also needs this kind of compile option.
I can't follow here. Can you make it more clear.
The sbuf has a hypercall, we need compile option to cover all sbuf code...
Absolutely, we should do it. This should be done in another patch, right?




Re: [PATCH v3] HV: fix "missing for discarded return value" for vm related api

Eddie Dong
 

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Victor Sun
Sent: Friday, August 10, 2018 11:32 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH v3] HV: fix "missing for discarded return value"
for vm related api

- add handler if prepare_vm0() failed;

- remove assert in start_vm() and return -1 if failed;

changelog:
v2 -> v3: replace panic with pr_fatal and return directly
if prepare_vm0() failed;
v1 -> v2: panic if prepare_vm0() failed instead of reboot system;

Signed-off-by: Victor Sun <victor.sun@...>
---
hypervisor/arch/x86/cpu.c | 9 ++++++---
hypervisor/arch/x86/guest/vm.c | 7 ++++---
hypervisor/common/hypercall.c | 3 +--
3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/hypervisor/arch/x86/cpu.c b/hypervisor/arch/x86/cpu.c index
02cbaec..6944592 100644
--- a/hypervisor/arch/x86/cpu.c
+++ b/hypervisor/arch/x86/cpu.c
@@ -563,9 +563,12 @@ static void bsp_boot_post(void)

exec_vmxon_instr(BOOT_CPU_ID);

- prepare_vm(BOOT_CPU_ID);
-
- default_idle();
+ if (prepare_vm0() == 0) {
Why you want to change from prepare_vm to prepare_vm0? This is obvious issue.

+ default_idle();
+ } else {
+ pr_fatal("Prepare VM0 failed!");
+ return;
+ }

/* Control should not come here */
cpu_dead(BOOT_CPU_ID);
diff --git a/hypervisor/arch/x86/guest/vm.c
b/hypervisor/arch/x86/guest/vm.c index 09ceadb..09d9fc0 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -331,7 +331,9 @@ int start_vm(struct vm *vm)

/* Only start BSP (vid = 0) and let BSP start other APs */
vcpu = vcpu_from_vid(vm, 0U);
- ASSERT(vcpu != NULL, "vm%d, vcpu0", vm->vm_id);
+ if (vcpu == NULL) {
+ return -1;
Why -1 ?

+ }
schedule_vcpu(vcpu);

return 0;
@@ -358,8 +360,7 @@ int reset_vm(struct vm *vm)

vioapic_reset(vm->arch_vm.virt_ioapic);

- start_vm(vm);
- return 0;
+ return (start_vm(vm));
I remember FuSa wants to split them into 2 lines of instructions.
}

/**
diff --git a/hypervisor/common/hypercall.c
b/hypervisor/common/hypercall.c index f8efb35..7489b99 100644
--- a/hypervisor/common/hypercall.c
+++ b/hypervisor/common/hypercall.c
@@ -280,8 +280,7 @@ int32_t hcall_reset_vm(uint16_t vmid)
if ((target_vm == NULL) || is_vm0(target_vm))
return -1;

- reset_vm(target_vm);
- return 0;
+ return (reset_vm(target_vm));
}

int32_t hcall_assert_irqline(struct vm *vm, uint16_t vmid, uint64_t param)
--
2.7.4



Re: [PATCH] HV: hv_main: Add #ifdef HV_DEBUG before debug specific code #ifdef

Kaige Fu
 

On 08-13 Mon 08:27, Eddie Dong wrote:

On 08-13 Mon 07:59, Eddie Dong wrote:
This is part of SBUF component, what kind of compile option do we use?
Let us use same one.
shell command "vmexit" is the only one user of vmexit_time/cnt and we use
the get_vmexit_profile, enclosed by HV_DEBUG, to access these variables.
Ok then.


BTW, hcall side also needs this kind of compile option.
I can't follow here. Can you make it more clear.
The sbuf has a hypercall, we need compile option to cover all sbuf code...
Absolutely, we should do it. This should be done in another patch, right?




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

Yu Wang
 

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

On 18-08-13 16:48:30, Wu, Xiaoguang wrote:
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@...>
---
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




Re: [PATCH v3 1/1] vbs: fix virtio_vq_index_get func handling of multi VQ concurrent request.

Yu Wang
 

On 18-08-13 00:25:12, Ong Hock Yu wrote:
Under multiple VQ use case, it is possible to have concurrent requests.
Added support to return multiple vq index from all vcpu.

Signed-off-by: Ong Hock Yu <ong.hock.yu@...>
---
drivers/vbs/vbs.c | 14 ++++++++------
drivers/vbs/vbs_rng.c | 2 +-
include/linux/vbs/vbs.h | 8 ++++++--
3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/vbs/vbs.c b/drivers/vbs/vbs.c
index 65b8a0bbe3d3..a2f263cb91c0 100644
--- a/drivers/vbs/vbs.c
+++ b/drivers/vbs/vbs.c
@@ -148,9 +148,11 @@ long virtio_dev_deregister(struct virtio_dev_info *dev)
return 0;
}

-int virtio_vq_index_get(struct virtio_dev_info *dev, unsigned long *ioreqs_map)
+int virtio_vqs_index_get(struct virtio_dev_info *dev,
+ unsigned long *ioreqs_map,
+ int *vqs_index, int max_vqs_index)
{
- int val = -1;
+ int idx = 0;
struct vhm_request *req;
int vcpu;

@@ -159,7 +161,7 @@ int virtio_vq_index_get(struct virtio_dev_info *dev, unsigned long *ioreqs_map)
return -EINVAL;
}

- while (1) {
+ while (idx < max_vqs_index) {
vcpu = find_first_bit(ioreqs_map, dev->_ctx.max_vcpu);
if (vcpu == dev->_ctx.max_vcpu)
break;
@@ -179,9 +181,9 @@ int virtio_vq_index_get(struct virtio_dev_info *dev, unsigned long *ioreqs_map)
pr_debug("%s: write request! type %d\n",
__func__, req->type);
if (dev->io_range_type == PIO_RANGE)
- val = req->reqs.pio_request.value;
+ vqs_index[idx++] = req->reqs.pio_request.value;
else
- val = req->reqs.mmio_request.value;
+ vqs_index[idx++] = req->reqs.mmio_request.value;
}
smp_mb();
atomic_set(&req->processed, REQ_STATE_COMPLETE);
@@ -189,7 +191,7 @@ int virtio_vq_index_get(struct virtio_dev_info *dev, unsigned long *ioreqs_map)
}
}
Haven't fix the v2 comment... Please add one log followed v2 suggestion


- return val;
+ return idx;
}

static long virtio_vqs_info_set(struct virtio_dev_info *dev,
diff --git a/drivers/vbs/vbs_rng.c b/drivers/vbs/vbs_rng.c
index fd2bb27af66e..c5e28cc12c55 100644
--- a/drivers/vbs/vbs_rng.c
+++ b/drivers/vbs/vbs_rng.c
@@ -268,7 +268,7 @@ static int handle_kick(int client_id, unsigned long *ioreqs_map)
return -EINVAL;
}

- val = virtio_vq_index_get(&rng->dev, ioreqs_map);
+ virtio_vqs_index_get(&rng->dev, ioreqs_map, &val, 1);

if (val >= 0)
handle_vq_kick(rng, val);
diff --git a/include/linux/vbs/vbs.h b/include/linux/vbs/vbs.h
index 30df8ebf68a0..964bacac865c 100644
--- a/include/linux/vbs/vbs.h
+++ b/include/linux/vbs/vbs.h
@@ -262,7 +262,7 @@ long virtio_dev_register(struct virtio_dev_info *dev);
long virtio_dev_deregister(struct virtio_dev_info *dev);

/**
- * virtio_vq_index_get - get virtqueue index that frontend kicks
+ * virtio_vqs_index_get - get virtqueue indexes that frontend kicks
*
* This API is normally called in the VBS-K device's callback
* function, to get value write to the "kick" register from
@@ -270,10 +270,14 @@ long virtio_dev_deregister(struct virtio_dev_info *dev);
*
* @dev: Pointer to VBS-K device data struct
* @ioreqs_map: requests bitmap need to handle, provided by VHM
+ * @vqs_index: array to store the vq indexes
+ * @max_vqs_index: size of vqs_index array
*
* Return: >=0 on virtqueue index, <0 on error
*/
-int virtio_vq_index_get(struct virtio_dev_info *dev, unsigned long *ioreqs_map);
+int virtio_vqs_index_get(struct virtio_dev_info *dev,
+ unsigned long *ioreqs_map,
+ int *vqs_index, int max_vqs_index);

/**
* virtio_dev_reset - reset a VBS-K device
--
2.17.0




Re: [PATCH] tools: acrntrace: Add ring buffer mode

Geoffroy Van Cutsem
 

Can you please also modify the documentation (README.rst [1]) to reflect these changes?

[1] https://github.com/projectacrn/acrn-hypervisor/blob/master/tools/acrntrace/README.rst

Thanks,
Geoffroy

-----Original Message-----
From: acrn-dev@... [mailto:acrn-dev@...]
On Behalf Of Zhipeng Gong
Sent: Monday, August 13, 2018 10:20 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH] tools: acrntrace: Add ring buffer mode

When running longevity test and capture acrntrace, generated acrntrace files
sizes are too big.
Sometimes we don't care very old trace. This patch adds ring buffer mode,
fixes acrntrace file size and overwrites old trace with new trace.

Signed-off-by: Zhipeng Gong <zhipeng.gong@...>
---
tools/acrntrace/acrntrace.c | 44
+++++++++++++++++++++++++++++++++++++++++---
tools/acrntrace/acrntrace.h | 1 +
2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/tools/acrntrace/acrntrace.c b/tools/acrntrace/acrntrace.c index
39fda35..371ffaa 100644
--- a/tools/acrntrace/acrntrace.c
+++ b/tools/acrntrace/acrntrace.c
@@ -37,16 +37,19 @@ static char trace_file_dir[TRACE_FILE_DIR_LEN];

static reader_struct *reader;
static int pcpu_num = 0;
+static int ring_buffer_size = 0;

static void display_usage(void)
{
printf("acrntrace - tool to collect ACRN trace data\n"
- "[Usage] acrntrace [-i] [period in msec] [-ch]\n\n"
+ "[Usage] acrntrace [-i] [period in msec] [-r] [ring buffer size] [-
ch]\n\n"
"[Options]\n"
"\t-h: print this message\n"
"\t-i: period_in_ms: specify polling interval [1-999]\n"
"\t-t: max time to capture trace data (in second)\n"
- "\t-c: clear the buffered old data\n");
+ "\t-c: clear the buffered old data\n"
+ "\t-r: ring_buffer_size (in MB): enable ring buffer mode,"
+ " where new trace overwrites old trace when full\n");
}

static void timer_handler(union sigval sv) @@ -113,6 +116,15 @@ static int
parse_opt(int argc, char *argv[])
timeout = ret;
pr_dbg("Capture trace data for at most %ds\n", ret);
break;
+ case 'r':
+ ret = atoi(optarg);
+ if (ret <= 0) {
+ pr_err("'-r' require integer greater than 0\n");
+ return -EINVAL;
+ }
+ ring_buffer_size = ret * 1024 * 1024;
+ pr_dbg("Ring buffer size is %dM\n", ret);
+ break;
case 'c':
flags |= FLAG_CLEAR_BUF;
break;
@@ -206,6 +218,15 @@ static void reader_fn(param_t * param)
trace_ev_t e;
struct statvfs stat;
uint64_t freespace;
+ int pos = 0;
+
+ if (ring_buffer_size != 0) {
+ param->buffer = malloc(ring_buffer_size);
+ if (!param->buffer) {
+ perror("Failed to allocate ring buffer\n");
+ return;
+ }
+ }

pr_dbg("reader thread[%lu] created for FILE*[0x%p]\n",
pthread_self(), fp);
@@ -219,7 +240,13 @@ static void reader_fn(param_t * param)

while (1) {
do {
- ret = sbuf_write(fd, sbuf);
+ if (ring_buffer_size != 0) {
+ ret = sbuf_get(sbuf, param->buffer + pos);
+ pos += sbuf->ele_size;
+ if (pos + sbuf->ele_size > ring_buffer_size)
+ pos = 0;
+ } else
+ ret = sbuf_write(fd, sbuf);
} while (ret > 0);

usleep(period);
@@ -284,6 +311,17 @@ static void destory_reader(reader_struct * reader)
reader->thrd = 0;
}

+ if (ring_buffer_size != 0 && reader->param.buffer) {
+ int ret;
+
+ ret = write(reader->param.trace_fd, reader->param.buffer,
+ ring_buffer_size);
+ if (ret != ring_buffer_size) {
+ perror("Failed to write ring buffer\n");
+ }
+ free(reader->param.buffer);
+ }
+
if (reader->param.sbuf) {
munmap(reader->param.sbuf, MMAP_SIZE);
reader->param.sbuf = NULL;
diff --git a/tools/acrntrace/acrntrace.h b/tools/acrntrace/acrntrace.h index
ea54f4d..6615157 100644
--- a/tools/acrntrace/acrntrace.h
+++ b/tools/acrntrace/acrntrace.h
@@ -76,6 +76,7 @@ typedef struct {
int trace_fd;
shared_buf_t *sbuf;
pthread_mutex_t *sbuf_lock;
+ uint8_t *buffer;
} param_t;

typedef struct {
--
2.7.4



Re: [PATCH 1/2] vhm: add ioeventfd support for ACRN hypervisor service module

Jie Deng
 

On 2018/8/13 14:40, Shuo Liu wrote:
+
+static struct acrn_vhm_ioeventfd *vhm_ioeventfd_match(
+ struct vhm_ioeventfd_info *info,
+ u64 addr, u64 data, int length, int type)
+{
+ struct acrn_vhm_ioeventfd *p = NULL;
+
+ list_for_each_entry(p, &info->ioeventfds, list) {
+ if (p->type == type &&
+ p->addr == addr &&
+ p->length == length &&
+ (p->data == data || p->wildcard))
+ return p;
Is it better to be "(p->wildcard || p->data == data)" ?


[PATCH 4/6] 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 assgned to UOS.
The logic is zero reserve and nonzero assignement.

In this patch, the meaning of native_assign_ports is expand to the
following logic:
native_assign_ports[x][y] = 0: native port x-y is not assigned.
native_assign_ports[x][y] < 0: native port x-y is assigned but
currently no virtual device attached.
native_assign_ports[x][y] > 0: native paort x-y is assigned and
virtual device attached to virtual port whose number
is stored in native_assign_ports[x][y].

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

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index f12126b..2df7ce3 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -373,7 +373,16 @@ 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];
+
+ /* native_assign_ports is used to record the assigment information:
+ * native_assign_ports[x][y] = 0: native port x-y is not assigned.
+ * native_assign_ports[x][y] < 0: native port x-y is assigned but
+ * currently no virtual device attached.
+ * native_assign_ports[x][y] > 0: native paort x-y is assigned and
+ * virtual device attached to virtual port whose number
+ * is stored in native_assign_ports[x][y].
+ */
+ int8_t *native_assign_ports[USB_NATIVE_NUM_BUS];
struct timespec mf_prev_time; /* previous time of accessing MFINDEX */
};

@@ -3365,14 +3374,14 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)

if (!xdev->native_assign_ports[bus]) {
xdev->native_assign_ports[bus] = calloc(USB_NATIVE_NUM_PORT,
- sizeof(uint8_t));
+ sizeof(int8_t));
if (!xdev->native_assign_ports[bus]) {
rc = -3;
goto errout;
}
}
+ xdev->native_assign_ports[bus][port] = -1;

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


[PATCH 6/6] 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 | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index 579021a..501de01 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: "
@@ -550,6 +551,8 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)

xdev->native_assign_ports[di->bus][di->port] = port;
xdev->native_assign_info[di->bus][di->port] = *di;
+ if (vm_get_suspend_mode() != VM_SUSPEND_NONE)
+ intr = 0;

/* Trigger port change event for the arriving device */
if (pci_xhci_port_connect(xdev, port, di->speed, intr))
@@ -614,6 +617,17 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
}
assert(i < USB_NATIVE_NUM_BUS);
xdev->native_assign_ports[i][j] = -1;
+
+ 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);
+ intr = 0;
+ }
+
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");
--
2.7.4


[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


[PATCH 3/6] 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 dificulties 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 | 31 ++++++++++++-------------------
devicemodel/hw/platform/usb_pmapper.c | 14 +++++++++++++-
devicemodel/include/usb_core.h | 10 ++++++++++
3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index 490219a..f12126b 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -469,23 +469,22 @@ 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;
int intr = 1;

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->priv_data);
if (!de) {
UPRINTF(LFTL, "fail to create device\r\n");
return -1;
@@ -499,26 +498,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;
@@ -550,11 +543,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_port_connect(xdev, port, native_speed, intr))
+ if (pci_xhci_port_connect(xdev, port, di->speed, intr))
UPRINTF(LFTL, "fail to report port event\n");

return 0;
diff --git a/devicemodel/hw/platform/usb_pmapper.c b/devicemodel/hw/platform/usb_pmapper.c
index 9f57f1c..a9c8608 100644
--- a/devicemodel/hw/platform/usb_pmapper.c
+++ b/devicemodel/hw/platform/usb_pmapper.c
@@ -1020,6 +1020,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 +1030,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,
--
2.7.4


[PATCH 1/6] 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@...>
---
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 2/6] 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 | 76 ++++++++++++++++++++++-------------------------
1 file changed, 35 insertions(+), 41 deletions(-)

diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index 792d930..490219a 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -439,11 +439,14 @@ static void pci_xhci_update_ep_ring(struct pci_xhci_vdev *xdev,
struct xhci_endp_ctx *ep_ctx,
uint32_t streamid, uint64_t ringaddr,
int ccs);
-static void pci_xhci_init_port(struct pci_xhci_vdev *xdev, int portn);
+static void pci_xhci_port_init(struct pci_xhci_vdev *xdev, int portn);
+static int pci_xhci_port_connect(struct pci_xhci_vdev *xdev, int port,
+ int usb_speed, int intr);
+static int pci_xhci_port_disconnect(struct pci_xhci_vdev *xdev, int port,
+ int 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,8 @@ 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;
+ int intr = 1;

xdev = hci_data;

@@ -498,6 +503,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 +549,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_port_connect(xdev, port, native_speed, intr))
UPRINTF(LFTL, "fail to report port event\n");

return 0;
@@ -566,6 +570,7 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
struct pci_xhci_dev_emu *edev;
struct usb_dev *udev;
uint8_t port, native_port;
+ int intr = 1;

assert(hci_data);
assert(dev_data);
@@ -595,7 +600,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_port_disconnect(xdev, port, intr)) {
UPRINTF(LFTL, "fail to report event\r\n");
return -1;
}
@@ -784,32 +789,29 @@ pci_xhci_convert_speed(int lspeed)
}

static int
-pci_xhci_port_chg(struct pci_xhci_vdev *xdev, int port, int conn)
+pci_xhci_port_change(struct pci_xhci_vdev *xdev, int port, int usb_speed,
+ int conn, int 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 (!intr)
+ return 0;
+
/* make an event for the guest OS */
pci_xhci_set_evtrb(&evtrb,
port,
@@ -825,6 +827,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_port_connect(struct pci_xhci_vdev *xdev, int port, int usb_speed,
+ int intr)
+{
+ return pci_xhci_port_change(xdev, port, usb_speed, 1, intr);
+}
+
+static int
+pci_xhci_port_disconnect(struct pci_xhci_vdev *xdev, int port, int intr)
+{
+ /* for disconnect, the speed is useless */
+ return pci_xhci_port_change(xdev, port, 0, 0, intr);
+}
+
static void
pci_xhci_set_evtrb(struct xhci_trb *evtrb, uint64_t port, uint32_t errcode,
uint32_t evtype)
@@ -3206,32 +3222,10 @@ 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)
+pci_xhci_port_init(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
@@ -3567,7 +3561,7 @@ pci_xhci_parse_opts(struct pci_xhci_vdev *xdev, char *opts)

/* do not use the zero index element */
for (i = 1; i <= XHCI_MAX_DEVS; i++)
- pci_xhci_init_port(xdev, i);
+ pci_xhci_port_init(xdev, i);

errout:
if (rc) {
--
2.7.4


[PATCH 0/6] USB xHCI S3 support

Wu, Xiaoguang
 

This patchset enable the USB xHCI S3 feature.

Xiaoguang Wu (6):
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: refine port assignment logic
DM USB: xHCI: change flow of creation of virtual USB device
DM USB: xHCI: enable xHCI SOS S3 support

devicemodel/hw/pci/xhci.c | 341 ++++++++++++++++++++++------------
devicemodel/hw/platform/usb_pmapper.c | 14 +-
devicemodel/include/usb_core.h | 10 +
3 files changed, 245 insertions(+), 120 deletions(-)

--
2.7.4


Re: [PATCH v3] HV: fix "missing for discarded return value" for vm related api

Victor Sun
 

Hi Eddie/Anthony,

Do you have any comments on this one?

On 8/10/2018 11:31 AM, Victor Sun wrote:
- add handler if prepare_vm0() failed;

- remove assert in start_vm() and return -1 if failed;

changelog:
v2 -> v3: replace panic with pr_fatal and return directly
if prepare_vm0() failed;
v1 -> v2: panic if prepare_vm0() failed instead of reboot system;

Signed-off-by: Victor Sun <victor.sun@...>
---
hypervisor/arch/x86/cpu.c | 9 ++++++---
hypervisor/arch/x86/guest/vm.c | 7 ++++---
hypervisor/common/hypercall.c | 3 +--
3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/hypervisor/arch/x86/cpu.c b/hypervisor/arch/x86/cpu.c
index 02cbaec..6944592 100644
--- a/hypervisor/arch/x86/cpu.c
+++ b/hypervisor/arch/x86/cpu.c
@@ -563,9 +563,12 @@ static void bsp_boot_post(void)
exec_vmxon_instr(BOOT_CPU_ID);
- prepare_vm(BOOT_CPU_ID);
-
- default_idle();
+ if (prepare_vm0() == 0) {
+ default_idle();
+ } else {
+ pr_fatal("Prepare VM0 failed!");
+ return;
+ }
/* Control should not come here */
cpu_dead(BOOT_CPU_ID);
diff --git a/hypervisor/arch/x86/guest/vm.c b/hypervisor/arch/x86/guest/vm.c
index 09ceadb..09d9fc0 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -331,7 +331,9 @@ int start_vm(struct vm *vm)
/* Only start BSP (vid = 0) and let BSP start other APs */
vcpu = vcpu_from_vid(vm, 0U);
- ASSERT(vcpu != NULL, "vm%d, vcpu0", vm->vm_id);
+ if (vcpu == NULL) {
+ return -1;
+ }
schedule_vcpu(vcpu);
return 0;
@@ -358,8 +360,7 @@ int reset_vm(struct vm *vm)
vioapic_reset(vm->arch_vm.virt_ioapic);
- start_vm(vm);
- return 0;
+ return (start_vm(vm));
}
/**
diff --git a/hypervisor/common/hypercall.c b/hypervisor/common/hypercall.c
index f8efb35..7489b99 100644
--- a/hypervisor/common/hypercall.c
+++ b/hypervisor/common/hypercall.c
@@ -280,8 +280,7 @@ int32_t hcall_reset_vm(uint16_t vmid)
if ((target_vm == NULL) || is_vm0(target_vm))
return -1;
- reset_vm(target_vm);
- return 0;
+ return (reset_vm(target_vm));
}
int32_t hcall_assert_irqline(struct vm *vm, uint16_t vmid, uint64_t param)


Re: [PATCH] hv:Fix misra-c return value violations for vioapic APIs

Eddie Dong
 

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Mingqiang Chi
Sent: Monday, August 13, 2018 3:47 PM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH] hv:Fix misra-c return value violations for vioapic
APIs

From: Mingqiang Chi <mingqiang.chi@...>

--Changed 4 APIs to void type
vioapic_set_irqstate
vioapic_assert_irq
vioapic_deassert_irq
vioapic_pulse_irq

Signed-off-by: Mingqiang Chi <mingqiang.chi@...>
---
hypervisor/common/hypercall.c | 6 +++---
hypervisor/dm/vioapic.c | 20 +++++++++-----------
hypervisor/include/arch/x86/guest/vioapic.h | 6 +++---
3 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/hypervisor/common/hypercall.c
b/hypervisor/common/hypercall.c index f8efb35..110b3b1 100644
--- a/hypervisor/common/hypercall.c
+++ b/hypervisor/common/hypercall.c
@@ -109,13 +109,13 @@ handle_vioapic_irqline(struct vm *vm, uint32_t irq,
enum irq_mode mode)

switch (mode) {
case IRQ_ASSERT:
- ret = vioapic_assert_irq(vm, irq);
+ vioapic_assert_irq(vm, irq);
break;
case IRQ_DEASSERT:
- ret = vioapic_deassert_irq(vm, irq);
+ vioapic_deassert_irq(vm, irq);
break;
case IRQ_PULSE:
- ret = vioapic_pulse_irq(vm, irq);
+ vioapic_pulse_irq(vm, irq);
break;
default:
/*
diff --git a/hypervisor/dm/vioapic.c b/hypervisor/dm/vioapic.c index
4692094..3724be0 100644
--- a/hypervisor/dm/vioapic.c
+++ b/hypervisor/dm/vioapic.c
@@ -150,14 +150,15 @@ enum irqstate {
IRQSTATE_PULSE
};

-static int
+static void
vioapic_set_irqstate(struct vm *vm, uint32_t irq, enum irqstate irqstate) {
struct vioapic *vioapic;
uint32_t pin = irq;

if (pin >= vioapic_pincount(vm)) {
- return -EINVAL;
+ pr_err("invalid vioapic pin number %hhu", pin);
+ return;
This is not equivalent... I think, we have to make it as a pre-assumption, by doing so. Please check with Junjie.

}

vioapic = vm_ioapic(vm);
@@ -178,26 +179,23 @@ vioapic_set_irqstate(struct vm *vm, uint32_t irq,
enum irqstate irqstate)
panic("vioapic_set_irqstate: invalid irqstate %d", irqstate);
}
VIOAPIC_UNLOCK(vioapic);
-
- return 0;
}

-int
+void
vioapic_assert_irq(struct vm *vm, uint32_t irq) {
- return vioapic_set_irqstate(vm, irq, IRQSTATE_ASSERT);
+ vioapic_set_irqstate(vm, irq, IRQSTATE_ASSERT);
}

-int
-vioapic_deassert_irq(struct vm *vm, uint32_t irq)
+void vioapic_deassert_irq(struct vm *vm, uint32_t irq)
{
- return vioapic_set_irqstate(vm, irq, IRQSTATE_DEASSERT);
+ vioapic_set_irqstate(vm, irq, IRQSTATE_DEASSERT);
}

-int
+void
vioapic_pulse_irq(struct vm *vm, uint32_t irq) {
- return vioapic_set_irqstate(vm, irq, IRQSTATE_PULSE);
+ vioapic_set_irqstate(vm, irq, IRQSTATE_PULSE);
}

/*
diff --git a/hypervisor/include/arch/x86/guest/vioapic.h
b/hypervisor/include/arch/x86/guest/vioapic.h
index 98ffc9e..29bb540 100644
--- a/hypervisor/include/arch/x86/guest/vioapic.h
+++ b/hypervisor/include/arch/x86/guest/vioapic.h
@@ -41,9 +41,9 @@ struct vioapic *vioapic_init(struct vm *vm);
void vioapic_cleanup(struct vioapic *vioapic);
void vioapic_reset(struct vioapic *vioapic);

-int vioapic_assert_irq(struct vm *vm, uint32_t irq);
-int vioapic_deassert_irq(struct vm *vm, uint32_t irq);
-int vioapic_pulse_irq(struct vm *vm, uint32_t irq);
+void vioapic_assert_irq(struct vm *vm, uint32_t irq);
+void vioapic_deassert_irq(struct vm *vm, uint32_t irq);
+void vioapic_pulse_irq(struct vm *vm, uint32_t irq);
void vioapic_update_tmr(struct vcpu *vcpu);

void vioapic_mmio_write(struct vm *vm, uint64_t gpa, uint32_t wval);
--
2.7.4



Re: [PATCH] HV: handle trusty on vm reset

Victor Sun
 

HI Eddie, do you mean misra violation that mingqiang addressed?

If so, please give me a ACK in my V2 patch.

BR,

Victor

On 8/13/2018 4:24 PM, Eddie Dong wrote:
LGTM, with one issue to fix.

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Victor Sun
Sent: Monday, August 13, 2018 3:47 PM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH] HV: handle trusty on vm reset

- clear run context when reset vcpu;

- destroy trusty without erase trusty memory when reset vm;

Signed-off-by: Sun Victor <victor.sun@...>
Signed-off-by: Yin Fengwei <fengwei.yin@...>
---
hypervisor/arch/x86/guest/vcpu.c | 7 +++++++
hypervisor/arch/x86/guest/vm.c | 2 ++
2 files changed, 9 insertions(+)

diff --git a/hypervisor/arch/x86/guest/vcpu.c
b/hypervisor/arch/x86/guest/vcpu.c
index 9dc0241..a8a9f0e 100644
--- a/hypervisor/arch/x86/guest/vcpu.c
+++ b/hypervisor/arch/x86/guest/vcpu.c
@@ -396,6 +396,7 @@ void destroy_vcpu(struct vcpu *vcpu)
*/
void reset_vcpu(struct vcpu *vcpu)
{
+ int i;
struct acrn_vlapic *vlapic;

pr_dbg("vcpu%hu reset", vcpu->vcpu_id); @@ -419,6 +420,12 @@ void
reset_vcpu(struct vcpu *vcpu)
vcpu->arch_vcpu.inject_event_pending = false;
(void)memset(vcpu->arch_vcpu.vmcs, 0U, CPU_PAGE_SIZE);

+ for (i = 0; i < NR_WORLD; i++) {
+ memset(&vcpu->arch_vcpu.contexts[i], 0,
+ sizeof(struct run_context));
+ }
+ vcpu->arch_vcpu.cur_context = NORMAL_WORLD;
+
vlapic = vcpu->arch_vcpu.vlapic;
vlapic_reset(vlapic);
}
diff --git a/hypervisor/arch/x86/guest/vm.c
b/hypervisor/arch/x86/guest/vm.c index 4bfda8e..f2efbf9 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -366,6 +366,8 @@ int reset_vm(struct vm *vm)
}

vioapic_reset(vm->arch_vm.virt_ioapic);
+ destroy_secure_world(vm, false);
+ vm->sworld_control.flag.active = 0UL;

start_vm(vm);
return 0;
--
2.7.4




Re: [PATCH v2] HV: handle trusty on vm reset

Eddie Dong
 

Acked-by: Eddie Dong <eddie.dong@...>

-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Victor Sun
Sent: Monday, August 13, 2018 4:12 PM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH v2] HV: handle trusty on vm reset

- clear run context when reset vcpu;

- destroy trusty without erase trusty memory when reset vm;

changelog:
v1 -> v2: fix misra violation on calling memset();

Signed-off-by: Sun Victor <victor.sun@...>
Signed-off-by: Yin Fengwei <fengwei.yin@...>
---
hypervisor/arch/x86/guest/vcpu.c | 7 +++++++
hypervisor/arch/x86/guest/vm.c | 2 ++
2 files changed, 9 insertions(+)

diff --git a/hypervisor/arch/x86/guest/vcpu.c
b/hypervisor/arch/x86/guest/vcpu.c
index 9dc0241..a6746d2 100644
--- a/hypervisor/arch/x86/guest/vcpu.c
+++ b/hypervisor/arch/x86/guest/vcpu.c
@@ -396,6 +396,7 @@ void destroy_vcpu(struct vcpu *vcpu)
*/
void reset_vcpu(struct vcpu *vcpu)
{
+ int i;
struct acrn_vlapic *vlapic;

pr_dbg("vcpu%hu reset", vcpu->vcpu_id); @@ -419,6 +420,12 @@ void
reset_vcpu(struct vcpu *vcpu)
vcpu->arch_vcpu.inject_event_pending = false;
(void)memset(vcpu->arch_vcpu.vmcs, 0U, CPU_PAGE_SIZE);

+ for (i = 0; i < NR_WORLD; i++) {
+ (void)memset(&vcpu->arch_vcpu.contexts[i], 0U,
+ sizeof(struct run_context));
+ }
+ vcpu->arch_vcpu.cur_context = NORMAL_WORLD;
+
vlapic = vcpu->arch_vcpu.vlapic;
vlapic_reset(vlapic);
}
diff --git a/hypervisor/arch/x86/guest/vm.c
b/hypervisor/arch/x86/guest/vm.c index 4bfda8e..f2efbf9 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -366,6 +366,8 @@ int reset_vm(struct vm *vm)
}

vioapic_reset(vm->arch_vm.virt_ioapic);
+ destroy_secure_world(vm, false);
+ vm->sworld_control.flag.active = 0UL;

start_vm(vm);
return 0;
--
2.7.4



Re: [PATCH] HV: hv_main: Add #ifdef HV_DEBUG before debug specific code #ifdef

Eddie Dong
 


On 08-13 Mon 07:59, Eddie Dong wrote:
This is part of SBUF component, what kind of compile option do we use?
Let us use same one.
shell command "vmexit" is the only one user of vmexit_time/cnt and we use
the get_vmexit_profile, enclosed by HV_DEBUG, to access these variables.
Ok then.


BTW, hcall side also needs this kind of compile option.
I can't follow here. Can you make it more clear.
The sbuf has a hypercall, we need compile option to cover all sbuf code...