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
toggle quoted message
Show quoted text
-----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
|
|
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.
|
|
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.
|
|