[PATCH 3/3] hv: vpci: fix pass-thru pcie device may access MSI-X BAR


Li, Fei1
 

Some pass-thru PCIe devices may access MSI-X BAR besides MSI-X Table Structure.
This patch would let the pass-thru PCIe device to access MSI-X BAR besides
MSI-X Table Structure directly.

Signed-off-by: Fei Li <fei1.li@...>
---
hypervisor/dm/vpci/vmsix.c | 51 ++++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/hypervisor/dm/vpci/vmsix.c b/hypervisor/dm/vpci/vmsix.c
index 213a33af2..e39c3818a 100644
--- a/hypervisor/dm/vpci/vmsix.c
+++ b/hypervisor/dm/vpci/vmsix.c
@@ -27,6 +27,7 @@
*/

#include <asm/guest/vm.h>
+#include <asm/io.h>
#include <errno.h>
#include <vpci.h>
#include <asm/guest/ept.h>
@@ -98,32 +99,46 @@ uint32_t rw_vmsix_table(struct pci_vdev *vdev, struct io_request *io_req)
struct msix_table_entry *entry;
uint32_t entry_offset, table_offset, index = CONFIG_MAX_MSIX_TABLE_NUM;
uint64_t offset;
+ void *hva;

- /* Must be full DWORD or full QWORD aligned. */
- if ((mmio->size == 4U) || (mmio->size == 8U)) {
+ if ((mmio->size <= 8U) && mem_aligned_check(mmio->address, mmio->size)) {
offset = mmio->address - vdev->msix.mmio_gpa;
if (msixtable_access(vdev, (uint32_t)offset)) {
-
- table_offset = (uint32_t)(offset - vdev->msix.table_offset);
- index = table_offset / MSIX_TABLE_ENTRY_SIZE;
-
- entry = &vdev->msix.table_entries[index];
- entry_offset = table_offset % MSIX_TABLE_ENTRY_SIZE;
-
- if (mmio->direction == ACRN_IOREQ_DIR_READ) {
- (void)memcpy_s(&mmio->value, (size_t)mmio->size,
- (void *)entry + entry_offset, (size_t)mmio->size);
+ /* Must be full DWORD or full QWORD aligned. */
+ if ((mmio->size == 4U) || (mmio->size == 8U)) {
+
+ table_offset = (uint32_t)(offset - vdev->msix.table_offset);
+ index = table_offset / MSIX_TABLE_ENTRY_SIZE;
+
+ entry = &vdev->msix.table_entries[index];
+ entry_offset = table_offset % MSIX_TABLE_ENTRY_SIZE;
+
+ if (mmio->direction == ACRN_IOREQ_DIR_READ) {
+ (void)memcpy_s(&mmio->value, (size_t)mmio->size,
+ (void *)entry + entry_offset, (size_t)mmio->size);
+ } else {
+ (void)memcpy_s((void *)entry + entry_offset, (size_t)mmio->size,
+ &mmio->value, (size_t)mmio->size);
+ }
} else {
- (void)memcpy_s((void *)entry + entry_offset, (size_t)mmio->size,
- &mmio->value, (size_t)mmio->size);
+ pr_err("%s, Only DWORD and QWORD are permitted", __func__);
}
} else {
- if (mmio->direction == ACRN_IOREQ_DIR_READ) {
- mmio->value = 0UL;
+ if (vdev->pdev != NULL) {
+ hva = hpa2hva(vdev->msix.mmio_hpa + (mmio->address - vdev->msix.mmio_gpa));
+ stac();
+ if (mmio->direction == ACRN_IOREQ_DIR_READ) {
+ mmio->value = mmio_read(hva, mmio->size);
+ } else {
+ mmio_write(hva, mmio->size, mmio->value);
+ }
+ clac();
+ } else {
+ if (mmio->direction == ACRN_IOREQ_DIR_READ) {
+ mmio->value = 0UL;
+ }
}
}
- } else {
- pr_err("%s, Only DWORD and QWORD are permitted", __func__);
}

return index;
--
2.34.1


Eddie Dong
 

-----Original Message-----
From: Li, Fei1 <fei1.li@...>
Sent: Tuesday, October 18, 2022 11:20 PM
To: acrn-dev@...
Cc: Dong, Eddie <eddie.dong@...>; Huang, Yonghua
<yonghua.huang@...>; Chen, Jason CJ <jason.cj.chen@...>
Subject: [PATCH 3/3] hv: vpci: fix pass-thru pcie device may access MSI-X BAR

Some pass-thru PCIe devices may access MSI-X BAR besides MSI-X Table
Structure.
This patch would let the pass-thru PCIe device to access MSI-X BAR besides
MSI-X Table Structure directly.
Revise the message.


Signed-off-by: Fei Li <fei1.li@...>
---
hypervisor/dm/vpci/vmsix.c | 51 ++++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/hypervisor/dm/vpci/vmsix.c b/hypervisor/dm/vpci/vmsix.c index
213a33af2..e39c3818a 100644
--- a/hypervisor/dm/vpci/vmsix.c
+++ b/hypervisor/dm/vpci/vmsix.c
@@ -27,6 +27,7 @@
*/

#include <asm/guest/vm.h>
+#include <asm/io.h>
#include <errno.h>
#include <vpci.h>
#include <asm/guest/ept.h>
@@ -98,32 +99,46 @@ uint32_t rw_vmsix_table(struct pci_vdev *vdev,
struct io_request *io_req)
struct msix_table_entry *entry;
uint32_t entry_offset, table_offset, index =
CONFIG_MAX_MSIX_TABLE_NUM;
uint64_t offset;
+ void *hva;

- /* Must be full DWORD or full QWORD aligned. */
- if ((mmio->size == 4U) || (mmio->size == 8U)) {
+ if ((mmio->size <= 8U) && mem_aligned_check(mmio->address,
+mmio->size)) {
offset = mmio->address - vdev->msix.mmio_gpa;
if (msixtable_access(vdev, (uint32_t)offset)) {
-
- table_offset = (uint32_t)(offset - vdev-
msix.table_offset);
- index = table_offset / MSIX_TABLE_ENTRY_SIZE;
-
- entry = &vdev->msix.table_entries[index];
- entry_offset = table_offset %
MSIX_TABLE_ENTRY_SIZE;
-
- if (mmio->direction == ACRN_IOREQ_DIR_READ) {
- (void)memcpy_s(&mmio->value,
(size_t)mmio->size,
- (void *)entry + entry_offset,
(size_t)mmio->size);
+ /* Must be full DWORD or full QWORD aligned. */
+ if ((mmio->size == 4U) || (mmio->size == 8U)) {
+
+ table_offset = (uint32_t)(offset - vdev-
msix.table_offset);
+ index = table_offset /
MSIX_TABLE_ENTRY_SIZE;
+
+ entry = &vdev->msix.table_entries[index];
+ entry_offset = table_offset %
MSIX_TABLE_ENTRY_SIZE;
+
+ if (mmio->direction ==
ACRN_IOREQ_DIR_READ) {
+ (void)memcpy_s(&mmio->value,
(size_t)mmio->size,
+ (void *)entry + entry_offset,
(size_t)mmio->size);
+ } else {
+ (void)memcpy_s((void *)entry +
entry_offset, (size_t)mmio->size,
+ &mmio->value,
(size_t)mmio->size);
+ }
} else {
- (void)memcpy_s((void *)entry + entry_offset,
(size_t)mmio->size,
- &mmio->value, (size_t)mmio->size);
+ pr_err("%s, Only DWORD and QWORD are
permitted", __func__);
}
} else {
- if (mmio->direction == ACRN_IOREQ_DIR_READ) {
- mmio->value = 0UL;
+ if (vdev->pdev != NULL) {
+ hva = hpa2hva(vdev->msix.mmio_hpa +
(mmio->address - vdev->msix.mmio_gpa));
The previous code assume the entire bar is MSIX table registers, this is how "struct pci_msix" defines.
Now the assumption changes. We need to redefine struct pci_msix, or it is actually not a pci_msix, but a bar containing msix...


+ stac();
+ if (mmio->direction ==
ACRN_IOREQ_DIR_READ) {
+ mmio->value = mmio_read(hva,
mmio->size);
+ } else {
+ mmio_write(hva, mmio->size, mmio-
value);
+ }
+ clac();
+ } else {
+ if (mmio->direction ==
ACRN_IOREQ_DIR_READ) {
+ mmio->value = 0UL;
+ }
}
}
- } else {
- pr_err("%s, Only DWORD and QWORD are permitted",
__func__);
}

return index;
--
2.34.1


Li, Fei1
 

On 2022-10-20 at 01:18:05 +0800, Dong, Eddie wrote:


-----Original Message-----
From: Li, Fei1 <fei1.li@...>
Sent: Tuesday, October 18, 2022 11:20 PM
To: acrn-dev@...
Cc: Dong, Eddie <eddie.dong@...>; Huang, Yonghua
<yonghua.huang@...>; Chen, Jason CJ <jason.cj.chen@...>
Subject: [PATCH 3/3] hv: vpci: fix pass-thru pcie device may access MSI-X BAR

Some pass-thru PCIe devices may access MSI-X BAR besides MSI-X Table
Structure.
This patch would let the pass-thru PCIe device to access MSI-X BAR besides
MSI-X Table Structure directly.
Revise the message.

The MSI-X Table BAR
+----------------------------------------------------------------------------------------+
| | MSI-X Table | MSI-X Table | | MSI-X Table | |
| Part 1 | Structure | Structure | ... | Structure | Part 2 |
| | Entry 0 | Entry 1 | | Entry (N - 1) | |
+----------------------------------------------------------------------------------------+
| | |
| | |
| | |
MSI-X Table BAR Base MSI-X Table BAR Base + MSI-X Table BAR Offset MSI-X Table BAR Base + MSI-X Table BAR Size - 1

If (N * sizeof (MSI-X Table Structure 0)) is less than 4K, the pass-thru PCIe device
may access the Part 2 in its MSI-X Table BAR;
If MSI-X Table BAR Offset is not 4K aligned, the pass-thru PCIe device
may access the Part 1 in its MSI-X Table BAR;



Signed-off-by: Fei Li <fei1.li@...>
---
hypervisor/dm/vpci/vmsix.c | 51 ++++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/hypervisor/dm/vpci/vmsix.c b/hypervisor/dm/vpci/vmsix.c index
213a33af2..e39c3818a 100644
--- a/hypervisor/dm/vpci/vmsix.c
+++ b/hypervisor/dm/vpci/vmsix.c
@@ -27,6 +27,7 @@
*/

#include <asm/guest/vm.h>
+#include <asm/io.h>
#include <errno.h>
#include <vpci.h>
#include <asm/guest/ept.h>
@@ -98,32 +99,46 @@ uint32_t rw_vmsix_table(struct pci_vdev *vdev,
struct io_request *io_req)
struct msix_table_entry *entry;
uint32_t entry_offset, table_offset, index =
CONFIG_MAX_MSIX_TABLE_NUM;
uint64_t offset;
+ void *hva;

- /* Must be full DWORD or full QWORD aligned. */
- if ((mmio->size == 4U) || (mmio->size == 8U)) {
+ if ((mmio->size <= 8U) && mem_aligned_check(mmio->address,
+mmio->size)) {
offset = mmio->address - vdev->msix.mmio_gpa;
if (msixtable_access(vdev, (uint32_t)offset)) {
-
- table_offset = (uint32_t)(offset - vdev-
msix.table_offset);
- index = table_offset / MSIX_TABLE_ENTRY_SIZE;
-
- entry = &vdev->msix.table_entries[index];
- entry_offset = table_offset %
MSIX_TABLE_ENTRY_SIZE;
-
- if (mmio->direction == ACRN_IOREQ_DIR_READ) {
- (void)memcpy_s(&mmio->value,
(size_t)mmio->size,
- (void *)entry + entry_offset,
(size_t)mmio->size);
+ /* Must be full DWORD or full QWORD aligned. */
+ if ((mmio->size == 4U) || (mmio->size == 8U)) {
+
+ table_offset = (uint32_t)(offset - vdev-
msix.table_offset);
+ index = table_offset /
MSIX_TABLE_ENTRY_SIZE;
+
+ entry = &vdev->msix.table_entries[index];
+ entry_offset = table_offset %
MSIX_TABLE_ENTRY_SIZE;
+
+ if (mmio->direction ==
ACRN_IOREQ_DIR_READ) {
+ (void)memcpy_s(&mmio->value,
(size_t)mmio->size,
+ (void *)entry + entry_offset,
(size_t)mmio->size);
+ } else {
+ (void)memcpy_s((void *)entry +
entry_offset, (size_t)mmio->size,
+ &mmio->value,
(size_t)mmio->size);
+ }
} else {
- (void)memcpy_s((void *)entry + entry_offset,
(size_t)mmio->size,
- &mmio->value, (size_t)mmio->size);
+ pr_err("%s, Only DWORD and QWORD are
permitted", __func__);
}
} else {
- if (mmio->direction == ACRN_IOREQ_DIR_READ) {
- mmio->value = 0UL;
+ if (vdev->pdev != NULL) {
+ hva = hpa2hva(vdev->msix.mmio_hpa +
(mmio->address - vdev->msix.mmio_gpa));
The previous code assume the entire bar is MSIX table registers, this is how "struct pci_msix" defines.
Now the assumption changes. We need to redefine struct pci_msix, or it is actually not a pci_msix, but a bar containing msix...


+ stac();
+ if (mmio->direction ==
ACRN_IOREQ_DIR_READ) {
+ mmio->value = mmio_read(hva,
mmio->size);
+ } else {
+ mmio_write(hva, mmio->size, mmio-
value);
+ }
+ clac();
+ } else {
+ if (mmio->direction ==
ACRN_IOREQ_DIR_READ) {
+ mmio->value = 0UL;
+ }
}
}
- } else {
- pr_err("%s, Only DWORD and QWORD are permitted",
__func__);
}

return index;
--
2.34.1


Eddie Dong
 

Some pass-thru PCIe devices may access MSI-X BAR besides MSI-X Table
Structure.
This patch would let the pass-thru PCIe device to access MSI-X BAR
besides MSI-X Table Structure directly.
Revise the message.

The MSI-X Table BAR
+----------------------------------------------------------------------------------------+
| | MSI-X Table | MSI-X Table | | MSI-X Table | |
| Part 1 | Structure | Structure | ... | Structure | Part 2 |
| | Entry 0 | Entry 1 | | Entry (N - 1) | |
+----------------------------------------------------------------------------------------+
| | |
| | |
| | |
MSI-X Table BAR Base MSI-X Table BAR Base + MSI-X Table BAR Offset MSI-
X Table BAR Base + MSI-X Table BAR Size - 1

If (N * sizeof (MSI-X Table Structure 0)) is less than 4K, the pass-thru PCIe
device may access the Part 2 in its MSI-X Table BAR; If MSI-X Table BAR Offset
is not 4K aligned, the pass-thru PCIe device may access the Part 1 in its MSI-X
Table BAR;
The MSIX-x table registers are located in one of the PCIe BAR, and is intercept and emulated by ACRN.
However, the bar may contain registers other than MSI-X table registers, which is trapped by ACRN as a whole, in the granularity of entire bar registers.
ACRN need to forward the access to real hardware.

But, this may be over-trap. See next

+ } else {
+ (void)memcpy_s((void *)entry +
entry_offset, (size_t)mmio->size,
+ &mmio->value,
(size_t)mmio->size);
+ }
} else {
- (void)memcpy_s((void *)entry + entry_offset,
(size_t)mmio->size,
- &mmio->value, (size_t)mmio->size);
+ pr_err("%s, Only DWORD and QWORD are
permitted", __func__);
}
} else {
- if (mmio->direction == ACRN_IOREQ_DIR_READ) {
- mmio->value = 0UL;
+ if (vdev->pdev != NULL) {
+ hva = hpa2hva(vdev->msix.mmio_hpa +
(mmio->address - vdev->msix.mmio_gpa));
The previous code assume the entire bar is MSIX table registers, this is how
"struct pci_msix" defines.
Now the assumption changes. We need to redefine struct pci_msix, or it is
actually not a pci_msix, but a bar containing msix...
We need to re-design the structure. We may not trap the entire PCI bar.
Instead, we only need to trap the MSI-X table registers. If the Table registers are 4KB aligned (base and size), that is great!

If not, we have to trap a little bit more registers, and forward the access to real hardware for those additional registers.

I hope the MSI-x table can be 4KB aligned. Please double check.


Li, Fei1
 

On 2022-10-21 at 01:14:27 +0800, Dong, Eddie wrote:
Some pass-thru PCIe devices may access MSI-X BAR besides MSI-X Table
Structure.
This patch would let the pass-thru PCIe device to access MSI-X BAR
besides MSI-X Table Structure directly.
Revise the message.

The MSI-X Table BAR
+----------------------------------------------------------------------------------------+
| | MSI-X Table | MSI-X Table | | MSI-X Table | |
| Part 1 | Structure | Structure | ... | Structure | Part 2 |
| | Entry 0 | Entry 1 | | Entry (N - 1) | |
+----------------------------------------------------------------------------------------+
| | |
| | |
| | |
MSI-X Table BAR Base MSI-X Table BAR Base + MSI-X Table BAR Offset MSI-
X Table BAR Base + MSI-X Table BAR Size - 1

If (N * sizeof (MSI-X Table Structure 0)) is less than 4K, the pass-thru PCIe
device may access the Part 2 in its MSI-X Table BAR; If MSI-X Table BAR Offset
is not 4K aligned, the pass-thru PCIe device may access the Part 1 in its MSI-X
Table BAR;
The MSIX-x table registers are located in one of the PCIe BAR, and is intercept and emulated by ACRN.
However, the bar may contain registers other than MSI-X table registers, which is trapped by ACRN as a whole, in the granularity of entire bar registers.
Hi Eddie

Sorry for mislead you that ACRN would trap the whole MSIX-x table in cover letter.
ACRN only trap the MXI-X Table Structure parts in granularity of 4K, not all the MSI-X Table BAR.


ACRN need to forward the access to real hardware.

But, this may be over-trap. See next

+ } else {
+ (void)memcpy_s((void *)entry +
entry_offset, (size_t)mmio->size,
+ &mmio->value,
(size_t)mmio->size);
+ }
} else {
- (void)memcpy_s((void *)entry + entry_offset,
(size_t)mmio->size,
- &mmio->value, (size_t)mmio->size);
+ pr_err("%s, Only DWORD and QWORD are
permitted", __func__);
}
} else {
- if (mmio->direction == ACRN_IOREQ_DIR_READ) {
- mmio->value = 0UL;
+ if (vdev->pdev != NULL) {
+ hva = hpa2hva(vdev->msix.mmio_hpa +
(mmio->address - vdev->msix.mmio_gpa));
The previous code assume the entire bar is MSIX table registers, this is how
"struct pci_msix" defines.
Now the assumption changes. We need to redefine struct pci_msix, or it is
actually not a pci_msix, but a bar containing msix...
We need to re-design the structure. We may not trap the entire PCI bar.
Sorry for mislead you, ACRN didn't want to trap the entire PCI MSI-X Table bar
Instead, we only need to trap the MSI-X table registers. If the Table registers are 4KB aligned (base and size), that is great!

If not, we have to trap a little bit more registers, and forward the access to real hardware for those additional registers.
yes, that's what this patch does. (forward the access to real hardware)

I hope the MSI-x table can be 4KB aligned. Please double check.
first: PCIe MMIO BAR address may be not 4KB aligned, PCIe Spec only suggests that.

all address spaces used are a power of two in size and are naturally aligned.
Functions are free to consume more address space than required, but decoding
down to a 4 KB space for memory is suggested for Functions that need less
than that amount.

second:
MSI-X Table Offset may be not 4K aligned



MSI-X Table Offset = 2K (SIZE = MSI-X Table Structure Entry SIZE * MSI-X Vector Count)
|
|
+--------------------------------------------------------------------+
| | MSI-X Table | |
| | Structure Entries | |
+--------------------------------------------------------------------+
MSI-X Table BAR Base MSI-X Table Structure

or
MSI-X Table structure and MSI-X PBA structure are in a 4K page in the same BAR.
(The MSI-X Table and MSI-X PBA are permitted to co-reside within a naturally
aligned 4-KB address range, though they must not overlap with each other.)


MSI-X Table Offset = 4K (SIZE <= 2K) MSI-X PBA Offset = 6K
| |
| |
+---------------------------------------------------------------------------------------------+
| | MSI-X Table | MSI-X PBA |
| | Structure Entries | Structure Entries |
+---------------------------------------------------------------------------------------------+
MSI-X Table BAR Base MSI-X Table Structure MSI-X PBA Structure


So we can't guarantee the MSI-X structure doesn't share any naturally aligned 4-KB address range with
MSI-X PBA structure or registers not related to MSI-X.

Thanks.