[PATCH v2 kvmtool 0/7] vfio/pci: Fix MSIX table and PBA size allocation

[PATCH v2 kvmtool 0/7] vfio/pci: Fix MSIX table and PBA size allocation

[Alexandru Elisei]

This series is meant to rework the way the MSIX table and PBA are allocated
to prevent situations where the size allocated by kvmtool is larger than
the size of the BAR that holds them.

Patches 1-3 are fixes for stuff I found when I was investing a bug
triggered by the incorrect sizing of the table and PBA.

Patch 4 is a preparatory patch.

Patch 5 is the proper fix. More details in the commit message.

Patch 6 is there to make it easier to catch such errors if the code
regresses.

Patch 7 is an optimization for guests with larger page sizes than the host.


Testing
=======

For v2:

On an AMD Seattle, host page size 4k and 64k, guest page size 4k and 64k,
tested device passthrough with a Realtek RTL8168 NIC.


For v1:

On an AMD Seattle, host page size 4k and 64k, guest page size 4k and 64k
(so 4 tests in total for each device). Tried device passthrough with a
Realtek RTL8168 NIC and with a Intel 82574L NIC (not at the same time). No
issues encountered, MSIX table and PBA BAR has the same layout as the
physical device.

On an AMD Ryzen 3900x, host and guest 4k pages, I've assigned an Intel
82574L NIC to the guest. Realtek RTL8168 doesn't work without emulating a
PCI Express bus because the drivers falls back to CSI for device
configuration. Everything works in the guest, the shared BAR has the same
layout in the guest as the physical device.


Changelog
=========

Since v1:

* Rebased on top of dc6646192057 ("arm64: Be more permissive when parsing the
  kernel header").
* Added reviewed-by tags, many thanks.
* Removed comment about aligning table and pba sizes to PAGE_SIZE in
  vfio_pci_create_msix_table, as per Andre's suggestion.
* Added a newline after local variable declaration in
  vfio_pci_create_msix_table.
* Align MSIX table and PBA to the maximum architectural page size in #7
  ("vfio/pci: Fix MSIX table and PBA size allocation"), as suggested by Andre.

Alexandru Elisei (7):
  arm/gicv2m: Set errno when gicv2_update_routing() fails
  vfio/pci.c: Remove double include for assert.h
  pci: Fix pci_dev_* print macros
  vfio/pci: Rename PBA offset in device descriptor to fd_offset
  vfio/pci: Rework MSIX table and PBA physical size allocation
  vfio/pci: Print an error when offset is outside of the MSIX table or
    PBA
  vfio/pci: Align MSIX Table and PBA size to guest maximum page size

 arm/aarch32/include/kvm/kvm-arch.h |  4 ++
 arm/aarch64/include/kvm/kvm-arch.h |  4 ++
 arm/gicv2m.c                       | 10 ++--
 include/kvm/pci.h                  | 10 ++--
 include/kvm/vfio.h                 |  3 +-
 mips/include/kvm/kvm-arch.h        |  3 ++
 powerpc/include/kvm/kvm-arch.h     |  3 ++
 vfio/pci.c                         | 86 +++++++++++++++++++-----------
 x86/include/kvm/kvm-arch.h         |  3 ++
 9 files changed, 84 insertions(+), 42 deletions(-)

-- 
2.20.1

[PATCH v2 kvmtool 1/7] arm/gicv2m: Set errno when gicv2_update_routing() fails

[Alexandru Elisei]

In case of an error when updating the routing table entries,
irq__update_msix_route() uses perror to print an error message.
gicv2m_update_routing() doesn't set errno, and instead returns the value
that errno should have had, which can lead to failure messages like this:

KVM_SET_GSI_ROUTING: Success

Set errno in gicv2m_update_routing() to avoid such messages in the future.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/gicv2m.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arm/gicv2m.c b/arm/gicv2m.c
index d7e6398..b47ada8 100644
--- a/arm/gicv2m.c
+++ b/arm/gicv2m.c
@@ -42,16 +42,18 @@ static int gicv2m_update_routing(struct kvm *kvm,
 {
 	int spi;
 
-	if (entry->type != KVM_IRQ_ROUTING_MSI)
-		return -EINVAL;
+	if (entry->type != KVM_IRQ_ROUTING_MSI) {
+		errno = EINVAL;
+		return -errno;
+	}
 
 	if (!entry->u.msi.address_hi && !entry->u.msi.address_lo)
 		return 0;
 
 	spi = entry->u.msi.data & GICV2M_SPI_MASK;
 	if (spi < v2m.first_spi || spi >= v2m.first_spi + v2m.num_spis) {
-		pr_err("invalid SPI number %d", spi);
-		return -EINVAL;
+		errno = EINVAL;
+		return -errno;
 	}
 
 	v2m.spis[spi - v2m.first_spi] = entry->gsi;
-- 
2.20.1

[PATCH v2 kvmtool 2/7] vfio/pci.c: Remove double include for assert.h

[Alexandru Elisei]

assert.h is included twice, keep only one instance.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 vfio/pci.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/vfio/pci.c b/vfio/pci.c
index ea33fd6..10ff99e 100644
--- a/vfio/pci.c
+++ b/vfio/pci.c
@@ -10,8 +10,6 @@
 #include <sys/resource.h>
 #include <sys/time.h>
 
-#include <assert.h>
-
 /* Some distros don't have the define. */
 #ifndef PCI_CAP_EXP_RC_ENDPOINT_SIZEOF_V1
 #define PCI_CAP_EXP_RC_ENDPOINT_SIZEOF_V1	12
-- 
2.20.1

[PATCH v2 kvmtool 3/7] pci: Fix pci_dev_* print macros

[Alexandru Elisei]

Evaluate the "pci_hdr" argument before attempting to deference a field.
This fixes cryptic errors like this one, which came about during a
debugging session:

vfio/pci.c: In function 'vfio_pci_bar_activate':
include/kvm/pci.h:18:40: error: invalid type argument of '->' (have 'struct pci_device_header')
  pr_warning("[%04x:%04x] " fmt, pci_hdr->vendor_id, pci_hdr->device_id, ##__VA_ARGS__)
                                        ^~
vfio/pci.c:482:3: note: in expansion of macro 'pci_dev_warn'
   pci_dev_warn(&vdev->pci.hdr, "%s: BAR4\n", __func__);

This is caused by the operator precedence rules in C, where pointer
deference via "->" has a higher precedence than taking the address with the
ampersand symbol. When the macro is substituted, it becomes
&vdev->pci.hdr->vendor_id and it dereferences vdev->pci.hdr, which is not a
pointer, instead of dereferencing &vdev->pci.hdr, which is a pointer, and
quite likely what the author intended.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 include/kvm/pci.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/kvm/pci.h b/include/kvm/pci.h
index 0f2d5bb..d6eb398 100644
--- a/include/kvm/pci.h
+++ b/include/kvm/pci.h
@@ -13,15 +13,15 @@
 #include "kvm/kvm-arch.h"
 
 #define pci_dev_err(pci_hdr, fmt, ...) \
-	pr_err("[%04x:%04x] " fmt, pci_hdr->vendor_id, pci_hdr->device_id, ##__VA_ARGS__)
+	pr_err("[%04x:%04x] " fmt, (pci_hdr)->vendor_id, (pci_hdr)->device_id, ##__VA_ARGS__)
 #define pci_dev_warn(pci_hdr, fmt, ...) \
-	pr_warning("[%04x:%04x] " fmt, pci_hdr->vendor_id, pci_hdr->device_id, ##__VA_ARGS__)
+	pr_warning("[%04x:%04x] " fmt, (pci_hdr)->vendor_id, (pci_hdr)->device_id, ##__VA_ARGS__)
 #define pci_dev_info(pci_hdr, fmt, ...) \
-	pr_info("[%04x:%04x] " fmt, pci_hdr->vendor_id, pci_hdr->device_id, ##__VA_ARGS__)
+	pr_info("[%04x:%04x] " fmt, (pci_hdr)->vendor_id, (pci_hdr)->device_id, ##__VA_ARGS__)
 #define pci_dev_dbg(pci_hdr, fmt, ...) \
-	pr_debug("[%04x:%04x] " fmt, pci_hdr->vendor_id, pci_hdr->device_id, ##__VA_ARGS__)
+	pr_debug("[%04x:%04x] " fmt, (pci_hdr)->vendor_id, (pci_hdr)->device_id, ##__VA_ARGS__)
 #define pci_dev_die(pci_hdr, fmt, ...) \
-	die("[%04x:%04x] " fmt, pci_hdr->vendor_id, pci_hdr->device_id, ##__VA_ARGS__)
+	die("[%04x:%04x] " fmt, (pci_hdr)->vendor_id, (pci_hdr)->device_id, ##__VA_ARGS__)
 
 /*
  * PCI Configuration Mechanism #1 I/O ports. See Section 3.7.4.1.
-- 
2.20.1

[PATCH v2 kvmtool 4/7] vfio/pci: Rename PBA offset in device descriptor to fd_offset

[Alexandru Elisei]

The MSI-X capability defines a PBA offset, which is the offset of the PBA
array in the BAR that holds the array.

kvmtool uses the field "pba_offset" in struct msix_cap (which represents
the MSIX capability) to refer to the [PBA offset:BAR] field of the
capability; and the field "offset" in the struct vfio_pci_msix_pba to refer
to offset of the PBA array in the device descriptor created by the VFIO
driver.

As we're getting ready to add yet another field that represents an offset
to struct vfio_pci_msix_pba, try to avoid ambiguities by renaming the
struct's "offset" field to "fd_offset".

No functional change intended.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 include/kvm/vfio.h | 2 +-
 vfio/pci.c         | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/kvm/vfio.h b/include/kvm/vfio.h
index 28223cf..8cdf04f 100644
--- a/include/kvm/vfio.h
+++ b/include/kvm/vfio.h
@@ -48,7 +48,7 @@ struct vfio_pci_msix_table {
 
 struct vfio_pci_msix_pba {
 	size_t				size;
-	off_t				offset; /* in VFIO device fd */
+	off_t				fd_offset; /* in VFIO device fd */
 	unsigned int			bar;
 	u32				guest_phys_addr;
 };
diff --git a/vfio/pci.c b/vfio/pci.c
index 10ff99e..cc18311 100644
--- a/vfio/pci.c
+++ b/vfio/pci.c
@@ -256,7 +256,7 @@ static void vfio_pci_msix_pba_access(struct kvm_cpu *vcpu, u64 addr, u8 *data,
 	 * TODO: emulate PBA. Hardware MSI-X is never masked, so reading the PBA
 	 * is completely useless here. Note that Linux doesn't use PBA.
 	 */
-	if (pread(vdev->fd, data, len, pba->offset + offset) != (ssize_t)len)
+	if (pread(vdev->fd, data, len, pba->fd_offset + offset) != (ssize_t)len)
 		vfio_dev_err(vdev, "cannot access MSIX PBA\n");
 }
 
@@ -815,8 +815,8 @@ static int vfio_pci_fixup_cfg_space(struct vfio_device *vdev)
 	if (msix) {
 		/* Add a shortcut to the PBA region for the MMIO handler */
 		int pba_index = VFIO_PCI_BAR0_REGION_INDEX + pdev->msix_pba.bar;
-		pdev->msix_pba.offset = vdev->regions[pba_index].info.offset +
-					(msix->pba_offset & PCI_MSIX_PBA_OFFSET);
+		pdev->msix_pba.fd_offset = vdev->regions[pba_index].info.offset +
+					   (msix->pba_offset & PCI_MSIX_PBA_OFFSET);
 
 		/* Tidy up the capability */
 		msix->table_offset &= PCI_MSIX_TABLE_BIR;
-- 
2.20.1

[PATCH v2 kvmtool 5/7] vfio/pci: Rework MSIX table and PBA physical size allocation

[Alexandru Elisei]

When creating the MSIX table and PBA, kvmtool rounds up the table and
pending bit array sizes to the host's page size. Unfortunately, when doing
that, it doesn't take into account that the new size can exceed the device
BAR size, leading to hard to diagnose errors for certain configurations.

One theoretical example: PBA and table in the same 4k BAR, host's page size
is 4k. In this case, table->size = 4k, pba->size = 4k, map_size = 4k, which
means that pba->guest_phys_addr = table->guest_phys_addr + 4k, which is
outside of the 4k MMIO range allocated for both structures.

Another example, this time a real-world error that I encountered: happens
with a 64k host booting a 4k guest, an RTL8168 PCIE NIC assigned to the
guest. In this case, kvmtool sets table->size = 64k (because it's rounded
to the host's page size) and pba->size = 64k.

Truncated output of lspci -vv on the host:

01:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 06)
	Subsystem: TP-LINK Technologies Co., Ltd. TG-3468 Gigabit PCI Express Network Adapter
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin A routed to IRQ 255
	Region 0: I/O ports at 1000 [size=256]
	Region 2: Memory at 40000000 (64-bit, non-prefetchable) [size=4K]
	Region 4: Memory at 100000000 (64-bit, prefetchable) [size=16K]
	[..]
	Capabilities: [b0] MSI-X: Enable- Count=4 Masked-
		Vector table: BAR=4 offset=00000000
		PBA: BAR=4 offset=00000800
	[..]

When booting the guest:

[..]
[    0.207444] pci-host-generic 40000000.pci: host bridge /pci ranges:
[    0.208564] pci-host-generic 40000000.pci:       IO 0x0000000000..0x000000ffff -> 0x0000000000
[    0.209857] pci-host-generic 40000000.pci:      MEM 0x0050000000..0x007fffffff -> 0x0050000000
[    0.211184] pci-host-generic 40000000.pci: ECAM at [mem 0x40000000-0x4fffffff] for [bus 00]
[    0.212625] pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00
[    0.213647] pci_bus 0000:00: root bus resource [bus 00]
[    0.214429] pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
[    0.215355] pci_bus 0000:00: root bus resource [mem 0x50000000-0x7fffffff]
[    0.216676] pci 0000:00:00.0: [10ec:8168] type 00 class 0x020000
[    0.223771] pci 0000:00:00.0: reg 0x10: [io  0x6200-0x62ff]
[    0.239765] pci 0000:00:00.0: reg 0x18: [mem 0x50010000-0x50010fff]
[    0.244595] pci 0000:00:00.0: reg 0x20: [mem 0x50000000-0x50003fff]
[    0.246331] pci 0000:00:01.0: [1af4:1000] type 00 class 0x020000
[    0.247278] pci 0000:00:01.0: reg 0x10: [io  0x6300-0x63ff]
[    0.248212] pci 0000:00:01.0: reg 0x14: [mem 0x50020000-0x500200ff]
[    0.249172] pci 0000:00:01.0: reg 0x18: [mem 0x50020400-0x500207ff]
[    0.250450] pci 0000:00:02.0: [1af4:1001] type 00 class 0x018000
[    0.251392] pci 0000:00:02.0: reg 0x10: [io  0x6400-0x64ff]
[    0.252351] pci 0000:00:02.0: reg 0x14: [mem 0x50020800-0x500208ff]
[    0.253312] pci 0000:00:02.0: reg 0x18: [mem 0x50020c00-0x50020fff]
[    0.254760] pci 0000:00:00.0: BAR 4: assigned [mem 0x50000000-0x50003fff] (1)
[    0.255805] pci 0000:00:00.0: BAR 2: assigned [mem 0x50004000-0x50004fff] (2)
  Warning: [10ec:8168] Error activating emulation for BAR 2
  Warning: [10ec:8168] Error activating emulation for BAR 2
[    0.260432] pci 0000:00:01.0: BAR 2: assigned [mem 0x50005000-0x500053ff]
  Warning: [1af4:1000] Error activating emulation for BAR 2
  Warning: [1af4:1000] Error activating emulation for BAR 2
[    0.261469] pci 0000:00:02.0: BAR 2: assigned [mem 0x50005400-0x500057ff]
  Warning: [1af4:1001] Error activating emulation for BAR 2
  Warning: [1af4:1001] Error activating emulation for BAR 2
[    0.262499] pci 0000:00:00.0: BAR 0: assigned [io  0x1000-0x10ff]
[    0.263415] pci 0000:00:01.0: BAR 0: assigned [io  0x1100-0x11ff]
[    0.264462] pci 0000:00:01.0: BAR 1: assigned [mem 0x50005800-0x500058ff]
  Warning: [1af4:1000] Error activating emulation for BAR 1
  Warning: [1af4:1000] Error activating emulation for BAR 1
[    0.265481] pci 0000:00:02.0: BAR 0: assigned [io  0x1200-0x12ff]
[    0.266397] pci 0000:00:02.0: BAR 1: assigned [mem 0x50005900-0x500059ff]
  Warning: [1af4:1001] Error activating emulation for BAR 1
  Warning: [1af4:1001] Error activating emulation for BAR 1
[    0.267892] EINJ: ACPI disabled.
[    0.269922] virtio-pci 0000:00:01.0: virtio_pci: leaving for legacy driver
[    0.271118] virtio-pci 0000:00:02.0: virtio_pci: leaving for legacy driver
[    0.274122] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[    0.275930] printk: console [ttyS0] disabled
[    0.276669] 1000000.U6_16550A: ttyS0 at MMIO 0x1000000 (irq = 13, base_baud = 115200) is a 16550A
[    0.278058] printk: console [ttyS0] enabled
[    0.278058] printk: console [ttyS0] enabled
[    0.279304] printk: bootconsole [ns16550a0] disabled
[    0.279304] printk: bootconsole [ns16550a0] disabled
[    0.281252] 1001000.U6_16550A: ttyS1 at MMIO 0x1001000 (irq = 14, base_baud = 115200) is a 16550A
[    0.282842] 1002000.U6_16550A: ttyS2 at MMIO 0x1002000 (irq = 15, base_baud = 115200) is a 16550A
[    0.284611] 1003000.U6_16550A: ttyS3 at MMIO 0x1003000 (irq = 16, base_baud = 115200) is a 16550A
[    0.286094] SuperH (H)SCI(F) driver initialized
[    0.286868] msm_serial: driver initialized
[    0.287890] [drm] radeon kernel modesetting enabled.
[    0.288826] cacheinfo: Unable to detect cache hierarchy for CPU 0
[    0.293321] loop: module loaded
KVM_SET_GSI_ROUTING: Invalid argument

At (1), the guest writes 0x50000000 into BAR 4 of the NIC (which holds
the MSIX table and PBA), expecting that will cover only 16k of address
space (the BAR size), up to 0x50003fff, inclusive. On the host side, in
vfio_pci_bar_activate(), kvmtool will actually register for MMIO
emulation the region 0x50000000-0x5000ffff (64k in total) for the MSIX
table and 0x50010000-0x5001ffff (another 64k) for the PBA (kvmtool set
table->size and pba->size to 64k when it aligned them to the host's page
size).

Then at step (2), the guest writes the next available address (from its
point of view) into BAR 2 of the NIC, which is 0x50004000. On the host
side, the PCI emulation layer will search all the regions that overlap with
the BAR address range (0x50004000-0x50004fff) and will find none because,
just like the guest, it uses the BAR size to check for overlaps. When
vfio_pci_bar_activate() is reached, kvmtool will try to register memory for
this region, but it is already registered for the MSIX table emulation and
fails.

The same scenario repeats for every following memory BAR, because the MSIX
table and PBA use memory from 0x50000000 to 0x5001ffff.

The error at the end, which finally terminates the VM, is caused by the
guest trying to write to a totally different BAR, which vfio-pci
interpretes as a write to MSI-X table because it falls in the 64k region
that was registered for emulation. The IRQ ID is not a valid SPI number and
gicv2m_update_routing() returns an error (and sets errno to EINVAL).

Fix this by aligning the table and PBA size to 8 bytes to allow for
qword accesses, like PCI 3.0 mandates.

For the sake of simplicity, the PBA offset in a BAR, in case of a shared
BAR, is kept the same as the offset of the physical device. One hopes that
the device respects the recommendations set forth in PCI LOCAL BUS
SPECIFICATION, REV. 3.0, section "MSI-X Capability and Table Structures"

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 include/kvm/vfio.h |  1 +
 vfio/pci.c         | 69 +++++++++++++++++++++++++++-------------------
 2 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/include/kvm/vfio.h b/include/kvm/vfio.h
index 8cdf04f..764ab9b 100644
--- a/include/kvm/vfio.h
+++ b/include/kvm/vfio.h
@@ -50,6 +50,7 @@ struct vfio_pci_msix_pba {
 	size_t				size;
 	off_t				fd_offset; /* in VFIO device fd */
 	unsigned int			bar;
+	u32				bar_offset; /* in the shared BAR */
 	u32				guest_phys_addr;
 };
 
diff --git a/vfio/pci.c b/vfio/pci.c
index cc18311..582aedd 100644
--- a/vfio/pci.c
+++ b/vfio/pci.c
@@ -502,7 +502,7 @@ static int vfio_pci_bar_activate(struct kvm *kvm,
 
 	if (has_msix && (u32)bar_num == pba->bar) {
 		if (pba->bar == table->bar)
-			pba->guest_phys_addr = table->guest_phys_addr + table->size;
+			pba->guest_phys_addr = table->guest_phys_addr + pba->bar_offset;
 		else
 			pba->guest_phys_addr = region->guest_phys_addr;
 		ret = kvm__register_mmio(kvm, pba->guest_phys_addr,
@@ -815,15 +815,21 @@ static int vfio_pci_fixup_cfg_space(struct vfio_device *vdev)
 	if (msix) {
 		/* Add a shortcut to the PBA region for the MMIO handler */
 		int pba_index = VFIO_PCI_BAR0_REGION_INDEX + pdev->msix_pba.bar;
+		u32 pba_bar_offset = msix->pba_offset & PCI_MSIX_PBA_OFFSET;
+
 		pdev->msix_pba.fd_offset = vdev->regions[pba_index].info.offset +
-					   (msix->pba_offset & PCI_MSIX_PBA_OFFSET);
+					   pba_bar_offset;
 
 		/* Tidy up the capability */
 		msix->table_offset &= PCI_MSIX_TABLE_BIR;
-		msix->pba_offset &= PCI_MSIX_PBA_BIR;
-		if (pdev->msix_table.bar == pdev->msix_pba.bar)
-			msix->pba_offset |= pdev->msix_table.size &
-					    PCI_MSIX_PBA_OFFSET;
+		if (pdev->msix_table.bar == pdev->msix_pba.bar) {
+			/* Keep the same offset as the MSIX cap. */
+			pdev->msix_pba.bar_offset = pba_bar_offset;
+		} else {
+			/* PBA is at the start of the BAR. */
+			msix->pba_offset &= PCI_MSIX_PBA_BIR;
+			pdev->msix_pba.bar_offset = 0;
+		}
 	}
 
 	/* Install our fake Configuration Space */
@@ -892,12 +898,11 @@ static int vfio_pci_create_msix_table(struct kvm *kvm, struct vfio_device *vdev)
 	table->bar = msix->table_offset & PCI_MSIX_TABLE_BIR;
 	pba->bar = msix->pba_offset & PCI_MSIX_TABLE_BIR;
 
-	/*
-	 * KVM needs memory regions to be multiple of and aligned on PAGE_SIZE.
-	 */
 	nr_entries = (msix->ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
-	table->size = ALIGN(nr_entries * PCI_MSIX_ENTRY_SIZE, PAGE_SIZE);
-	pba->size = ALIGN(DIV_ROUND_UP(nr_entries, 64), PAGE_SIZE);
+
+	/* MSIX table and PBA must support QWORD accesses. */
+	table->size = ALIGN(nr_entries * PCI_MSIX_ENTRY_SIZE, 8);
+	pba->size = ALIGN(DIV_ROUND_UP(nr_entries, 64), 8);
 
 	entries = calloc(nr_entries, sizeof(struct vfio_pci_msi_entry));
 	if (!entries)
@@ -911,23 +916,8 @@ static int vfio_pci_create_msix_table(struct kvm *kvm, struct vfio_device *vdev)
 		return ret;
 	if (!info.size)
 		return -EINVAL;
-	map_size = info.size;
-
-	if (table->bar != pba->bar) {
-		ret = vfio_pci_get_region_info(vdev, pba->bar, &info);
-		if (ret)
-			return ret;
-		if (!info.size)
-			return -EINVAL;
-		map_size += info.size;
-	}
 
-	/*
-	 * To ease MSI-X cap configuration in case they share the same BAR,
-	 * collapse table and pending array. The size of the BAR regions must be
-	 * powers of two.
-	 */
-	map_size = ALIGN(map_size, PAGE_SIZE);
+	map_size = ALIGN(info.size, PAGE_SIZE);
 	table->guest_phys_addr = pci_get_mmio_block(map_size);
 	if (!table->guest_phys_addr) {
 		pr_err("cannot allocate MMIO space");
@@ -943,7 +933,30 @@ static int vfio_pci_create_msix_table(struct kvm *kvm, struct vfio_device *vdev)
 	 * between MSI-X table and PBA. For the sake of isolation, create a
 	 * virtual PBA.
 	 */
-	pba->guest_phys_addr = table->guest_phys_addr + table->size;
+	if (table->bar == pba->bar) {
+		u32 pba_bar_offset = msix->pba_offset & PCI_MSIX_PBA_OFFSET;
+
+		/* Sanity checks. */
+		if (table->size > pba_bar_offset)
+			die("MSIX table overlaps with PBA");
+		if (pba_bar_offset + pba->size > info.size)
+			die("PBA exceeds the size of the region");
+		pba->guest_phys_addr = table->guest_phys_addr + pba_bar_offset;
+	} else {
+		ret = vfio_pci_get_region_info(vdev, pba->bar, &info);
+		if (ret)
+			return ret;
+		if (!info.size)
+			return -EINVAL;
+
+		map_size = ALIGN(info.size, PAGE_SIZE);
+		pba->guest_phys_addr = pci_get_mmio_block(map_size);
+		if (!pba->guest_phys_addr) {
+			pr_err("cannot allocate MMIO space");
+			ret = -ENOMEM;
+			goto out_free;
+		}
+	}
 
 	pdev->msix.entries = entries;
 	pdev->msix.nr_entries = nr_entries;
-- 
2.20.1

[PATCH v2 kvmtool 6/7] vfio/pci: Print an error when offset is outside of the MSIX table or PBA

[Alexandru Elisei]

Now that we keep track of the real size of MSIX table and PBA, print an
error when the guest tries to write to an offset which is not inside the
correct regions.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 vfio/pci.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/vfio/pci.c b/vfio/pci.c
index 582aedd..a08352d 100644
--- a/vfio/pci.c
+++ b/vfio/pci.c
@@ -249,6 +249,11 @@ static void vfio_pci_msix_pba_access(struct kvm_cpu *vcpu, u64 addr, u8 *data,
 	u64 offset = addr - pba->guest_phys_addr;
 	struct vfio_device *vdev = container_of(pdev, struct vfio_device, pci);
 
+	if (offset >= pba->size) {
+		vfio_dev_err(vdev, "access outside of the MSIX PBA");
+		return;
+	}
+
 	if (is_write)
 		return;
 
@@ -269,6 +274,10 @@ static void vfio_pci_msix_table_access(struct kvm_cpu *vcpu, u64 addr, u8 *data,
 	struct vfio_device *vdev = container_of(pdev, struct vfio_device, pci);
 
 	u64 offset = addr - pdev->msix_table.guest_phys_addr;
+	if (offset >= pdev->msix_table.size) {
+		vfio_dev_err(vdev, "access outside of the MSI-X table");
+		return;
+	}
 
 	size_t vector = offset / PCI_MSIX_ENTRY_SIZE;
 	off_t field = offset % PCI_MSIX_ENTRY_SIZE;
-- 
2.20.1

[PATCH v2 kvmtool 7/7] vfio/pci: Align MSIX Table and PBA size to guest maximum page size

[Alexandru Elisei]

When allocating MMIO space for the MSI-X table, kvmtool rounds the
allocation to the host's page size to make it as easy as possible for the
guest to map the table to a page, if it wants to (and doesn't do BAR
reassignment, like the x86 architecture for example). However, the host's
page size can differ from the guest's on architectures which support
multiple page sizes. For example, arm64 supports three different page size,
and it is possible for the host to be using 4k pages, while the guest is
using 64k pages.

To make sure the allocation is always aligned to a guest's page size, round
it up to the maximum architectural page size. Do the same for the pending
bit array if it lives in its own BAR.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/aarch32/include/kvm/kvm-arch.h | 4 ++++
 arm/aarch64/include/kvm/kvm-arch.h | 4 ++++
 mips/include/kvm/kvm-arch.h        | 3 +++
 powerpc/include/kvm/kvm-arch.h     | 3 +++
 vfio/pci.c                         | 6 ++++--
 x86/include/kvm/kvm-arch.h         | 3 +++
 6 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/arm/aarch32/include/kvm/kvm-arch.h b/arm/aarch32/include/kvm/kvm-arch.h
index a772bb1..bee2fc2 100644
--- a/arm/aarch32/include/kvm/kvm-arch.h
+++ b/arm/aarch32/include/kvm/kvm-arch.h
@@ -1,10 +1,14 @@
 #ifndef KVM__KVM_ARCH_H
 #define KVM__KVM_ARCH_H
 
+#include <linux/sizes.h>
+
 #define kvm__arch_get_kern_offset(...)	0x8000
 
 #define ARM_MAX_MEMORY(...)	ARM_LOMAP_MAX_MEMORY
 
+#define MAX_PAGE_SIZE	SZ_4K
+
 #include "arm-common/kvm-arch.h"
 
 #endif /* KVM__KVM_ARCH_H */
diff --git a/arm/aarch64/include/kvm/kvm-arch.h b/arm/aarch64/include/kvm/kvm-arch.h
index 159567b..5e5ee41 100644
--- a/arm/aarch64/include/kvm/kvm-arch.h
+++ b/arm/aarch64/include/kvm/kvm-arch.h
@@ -1,6 +1,8 @@
 #ifndef KVM__KVM_ARCH_H
 #define KVM__KVM_ARCH_H
 
+#include <linux/sizes.h>
+
 struct kvm;
 unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm, int fd);
 int kvm__arch_get_ipa_limit(struct kvm *kvm);
@@ -21,6 +23,8 @@ int kvm__arch_get_ipa_limit(struct kvm *kvm);
 	max_ram;							\
 })
 
+#define MAX_PAGE_SIZE	SZ_64K
+
 #include "arm-common/kvm-arch.h"
 
 #endif /* KVM__KVM_ARCH_H */
diff --git a/mips/include/kvm/kvm-arch.h b/mips/include/kvm/kvm-arch.h
index fdc09d8..e2f048a 100644
--- a/mips/include/kvm/kvm-arch.h
+++ b/mips/include/kvm/kvm-arch.h
@@ -1,6 +1,7 @@
 #ifndef KVM__KVM_ARCH_H
 #define KVM__KVM_ARCH_H
 
+#include <linux/sizes.h>
 
 /*
  * Guest memory map is:
@@ -36,6 +37,8 @@
 
 #define VIRTIO_DEFAULT_TRANS(kvm)	VIRTIO_PCI
 
+#define MAX_PAGE_SIZE		SZ_64K
+
 #include <stdbool.h>
 
 #include "linux/types.h"
diff --git a/powerpc/include/kvm/kvm-arch.h b/powerpc/include/kvm/kvm-arch.h
index 26d440b..8eeca50 100644
--- a/powerpc/include/kvm/kvm-arch.h
+++ b/powerpc/include/kvm/kvm-arch.h
@@ -13,6 +13,7 @@
 
 #include <stdbool.h>
 #include <linux/types.h>
+#include <linux/sizes.h>
 #include <time.h>
 
 /*
@@ -48,6 +49,8 @@
 
 #define KVM_IOEVENTFD_HAS_PIO		0
 
+#define MAX_PAGE_SIZE			SZ_256K
+
 #define VIRTIO_DEFAULT_TRANS(kvm)	VIRTIO_PCI
 
 struct spapr_phb;
diff --git a/vfio/pci.c b/vfio/pci.c
index a08352d..78f5ca5 100644
--- a/vfio/pci.c
+++ b/vfio/pci.c
@@ -1,3 +1,5 @@
+#include "linux/sizes.h"
+
 #include "kvm/irq.h"
 #include "kvm/kvm.h"
 #include "kvm/kvm-cpu.h"
@@ -926,7 +928,7 @@ static int vfio_pci_create_msix_table(struct kvm *kvm, struct vfio_device *vdev)
 	if (!info.size)
 		return -EINVAL;
 
-	map_size = ALIGN(info.size, PAGE_SIZE);
+	map_size = ALIGN(info.size, MAX_PAGE_SIZE);
 	table->guest_phys_addr = pci_get_mmio_block(map_size);
 	if (!table->guest_phys_addr) {
 		pr_err("cannot allocate MMIO space");
@@ -958,7 +960,7 @@ static int vfio_pci_create_msix_table(struct kvm *kvm, struct vfio_device *vdev)
 		if (!info.size)
 			return -EINVAL;
 
-		map_size = ALIGN(info.size, PAGE_SIZE);
+		map_size = ALIGN(info.size, MAX_PAGE_SIZE);
 		pba->guest_phys_addr = pci_get_mmio_block(map_size);
 		if (!pba->guest_phys_addr) {
 			pr_err("cannot allocate MMIO space");
diff --git a/x86/include/kvm/kvm-arch.h b/x86/include/kvm/kvm-arch.h
index 85cd336..d8a7312 100644
--- a/x86/include/kvm/kvm-arch.h
+++ b/x86/include/kvm/kvm-arch.h
@@ -5,6 +5,7 @@
 
 #include <stdbool.h>
 #include <linux/types.h>
+#include <linux/sizes.h>
 #include <time.h>
 
 /*
@@ -30,6 +31,8 @@
 
 #define KVM_IOEVENTFD_HAS_PIO	1
 
+#define MAX_PAGE_SIZE		SZ_4K
+
 #define VIRTIO_DEFAULT_TRANS(kvm)	VIRTIO_PCI
 
 struct kvm_arch {
-- 
2.20.1

Re: [PATCH v2 kvmtool 5/7] vfio/pci: Rework MSIX table and PBA physical size allocation

[Andre Przywara]

On Tue, 12 Oct 2021 14:25:08 +0100
Alexandru Elisei <alexandru.elisei@arm.com> wrote:

> When creating the MSIX table and PBA, kvmtool rounds up the table and
> pending bit array sizes to the host's page size. Unfortunately, when doing
> that, it doesn't take into account that the new size can exceed the device
> BAR size, leading to hard to diagnose errors for certain configurations.
> 
> [ ...]

> Fix this by aligning the table and PBA size to 8 bytes to allow for
> qword accesses, like PCI 3.0 mandates.
> 
> For the sake of simplicity, the PBA offset in a BAR, in case of a shared
> BAR, is kept the same as the offset of the physical device. One hopes that
> the device respects the recommendations set forth in PCI LOCAL BUS
> SPECIFICATION, REV. 3.0, section "MSI-X Capability and Table Structures"

Thanks for the changes!

> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  include/kvm/vfio.h |  1 +
>  vfio/pci.c         | 69 +++++++++++++++++++++++++++-------------------
>  2 files changed, 42 insertions(+), 28 deletions(-)
> 
> diff --git a/include/kvm/vfio.h b/include/kvm/vfio.h
> index 8cdf04f..764ab9b 100644
> --- a/include/kvm/vfio.h
> +++ b/include/kvm/vfio.h
> @@ -50,6 +50,7 @@ struct vfio_pci_msix_pba {
>  	size_t				size;
>  	off_t				fd_offset; /* in VFIO device fd */
>  	unsigned int			bar;
> +	u32				bar_offset; /* in the shared BAR */
>  	u32				guest_phys_addr;
>  };
>  
> diff --git a/vfio/pci.c b/vfio/pci.c
> index cc18311..582aedd 100644
> --- a/vfio/pci.c
> +++ b/vfio/pci.c
> @@ -502,7 +502,7 @@ static int vfio_pci_bar_activate(struct kvm *kvm,
>  
>  	if (has_msix && (u32)bar_num == pba->bar) {
>  		if (pba->bar == table->bar)
> -			pba->guest_phys_addr = table->guest_phys_addr + table->size;
> +			pba->guest_phys_addr = table->guest_phys_addr + pba->bar_offset;
>  		else
>  			pba->guest_phys_addr = region->guest_phys_addr;
>  		ret = kvm__register_mmio(kvm, pba->guest_phys_addr,
> @@ -815,15 +815,21 @@ static int vfio_pci_fixup_cfg_space(struct vfio_device *vdev)
>  	if (msix) {
>  		/* Add a shortcut to the PBA region for the MMIO handler */
>  		int pba_index = VFIO_PCI_BAR0_REGION_INDEX + pdev->msix_pba.bar;
> +		u32 pba_bar_offset = msix->pba_offset & PCI_MSIX_PBA_OFFSET;
> +
>  		pdev->msix_pba.fd_offset = vdev->regions[pba_index].info.offset +
> -					   (msix->pba_offset & PCI_MSIX_PBA_OFFSET);
> +					   pba_bar_offset;
>  
>  		/* Tidy up the capability */
>  		msix->table_offset &= PCI_MSIX_TABLE_BIR;
> -		msix->pba_offset &= PCI_MSIX_PBA_BIR;
> -		if (pdev->msix_table.bar == pdev->msix_pba.bar)
> -			msix->pba_offset |= pdev->msix_table.size &
> -					    PCI_MSIX_PBA_OFFSET;
> +		if (pdev->msix_table.bar == pdev->msix_pba.bar) {
> +			/* Keep the same offset as the MSIX cap. */
> +			pdev->msix_pba.bar_offset = pba_bar_offset;
> +		} else {
> +			/* PBA is at the start of the BAR. */
> +			msix->pba_offset &= PCI_MSIX_PBA_BIR;
> +			pdev->msix_pba.bar_offset = 0;
> +		}
>  	}
>  
>  	/* Install our fake Configuration Space */
> @@ -892,12 +898,11 @@ static int vfio_pci_create_msix_table(struct kvm *kvm, struct vfio_device *vdev)
>  	table->bar = msix->table_offset & PCI_MSIX_TABLE_BIR;
>  	pba->bar = msix->pba_offset & PCI_MSIX_TABLE_BIR;
>  
> -	/*
> -	 * KVM needs memory regions to be multiple of and aligned on PAGE_SIZE.
> -	 */
>  	nr_entries = (msix->ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
> -	table->size = ALIGN(nr_entries * PCI_MSIX_ENTRY_SIZE, PAGE_SIZE);
> -	pba->size = ALIGN(DIV_ROUND_UP(nr_entries, 64), PAGE_SIZE);
> +
> +	/* MSIX table and PBA must support QWORD accesses. */
> +	table->size = ALIGN(nr_entries * PCI_MSIX_ENTRY_SIZE, 8);
> +	pba->size = ALIGN(DIV_ROUND_UP(nr_entries, 64), 8);
>  
>  	entries = calloc(nr_entries, sizeof(struct vfio_pci_msi_entry));
>  	if (!entries)
> @@ -911,23 +916,8 @@ static int vfio_pci_create_msix_table(struct kvm *kvm, struct vfio_device *vdev)
>  		return ret;
>  	if (!info.size)
>  		return -EINVAL;
> -	map_size = info.size;
> -
> -	if (table->bar != pba->bar) {
> -		ret = vfio_pci_get_region_info(vdev, pba->bar, &info);
> -		if (ret)
> -			return ret;
> -		if (!info.size)
> -			return -EINVAL;
> -		map_size += info.size;
> -	}
>  
> -	/*
> -	 * To ease MSI-X cap configuration in case they share the same BAR,
> -	 * collapse table and pending array. The size of the BAR regions must be
> -	 * powers of two.
> -	 */
> -	map_size = ALIGN(map_size, PAGE_SIZE);
> +	map_size = ALIGN(info.size, PAGE_SIZE);
>  	table->guest_phys_addr = pci_get_mmio_block(map_size);
>  	if (!table->guest_phys_addr) {
>  		pr_err("cannot allocate MMIO space");
> @@ -943,7 +933,30 @@ static int vfio_pci_create_msix_table(struct kvm *kvm, struct vfio_device *vdev)
>  	 * between MSI-X table and PBA. For the sake of isolation, create a
>  	 * virtual PBA.
>  	 */
> -	pba->guest_phys_addr = table->guest_phys_addr + table->size;
> +	if (table->bar == pba->bar) {
> +		u32 pba_bar_offset = msix->pba_offset & PCI_MSIX_PBA_OFFSET;
> +
> +		/* Sanity checks. */
> +		if (table->size > pba_bar_offset)
> +			die("MSIX table overlaps with PBA");
> +		if (pba_bar_offset + pba->size > info.size)
> +			die("PBA exceeds the size of the region");
> +		pba->guest_phys_addr = table->guest_phys_addr + pba_bar_offset;
> +	} else {
> +		ret = vfio_pci_get_region_info(vdev, pba->bar, &info);
> +		if (ret)
> +			return ret;
> +		if (!info.size)
> +			return -EINVAL;
> +
> +		map_size = ALIGN(info.size, PAGE_SIZE);
> +		pba->guest_phys_addr = pci_get_mmio_block(map_size);
> +		if (!pba->guest_phys_addr) {
> +			pr_err("cannot allocate MMIO space");
> +			ret = -ENOMEM;
> +			goto out_free;
> +		}
> +	}
>  
>  	pdev->msix.entries = entries;
>  	pdev->msix.nr_entries = nr_entries;

Re: [PATCH v2 kvmtool 7/7] vfio/pci: Align MSIX Table and PBA size to guest maximum page size

[Andre Przywara]

On Tue, 12 Oct 2021 14:25:10 +0100
Alexandru Elisei <alexandru.elisei@arm.com> wrote:

> When allocating MMIO space for the MSI-X table, kvmtool rounds the
> allocation to the host's page size to make it as easy as possible for the
> guest to map the table to a page, if it wants to (and doesn't do BAR
> reassignment, like the x86 architecture for example). However, the host's
> page size can differ from the guest's on architectures which support
> multiple page sizes. For example, arm64 supports three different page size,
> and it is possible for the host to be using 4k pages, while the guest is
> using 64k pages.
> 
> To make sure the allocation is always aligned to a guest's page size, round
> it up to the maximum architectural page size. Do the same for the pending
> bit array if it lives in its own BAR.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks for adding MAX_PAGE_SIZE, looks alright now.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  arm/aarch32/include/kvm/kvm-arch.h | 4 ++++
>  arm/aarch64/include/kvm/kvm-arch.h | 4 ++++
>  mips/include/kvm/kvm-arch.h        | 3 +++
>  powerpc/include/kvm/kvm-arch.h     | 3 +++
>  vfio/pci.c                         | 6 ++++--
>  x86/include/kvm/kvm-arch.h         | 3 +++
>  6 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/arm/aarch32/include/kvm/kvm-arch.h b/arm/aarch32/include/kvm/kvm-arch.h
> index a772bb1..bee2fc2 100644
> --- a/arm/aarch32/include/kvm/kvm-arch.h
> +++ b/arm/aarch32/include/kvm/kvm-arch.h
> @@ -1,10 +1,14 @@
>  #ifndef KVM__KVM_ARCH_H
>  #define KVM__KVM_ARCH_H
>  
> +#include <linux/sizes.h>
> +
>  #define kvm__arch_get_kern_offset(...)	0x8000
>  
>  #define ARM_MAX_MEMORY(...)	ARM_LOMAP_MAX_MEMORY
>  
> +#define MAX_PAGE_SIZE	SZ_4K
> +
>  #include "arm-common/kvm-arch.h"
>  
>  #endif /* KVM__KVM_ARCH_H */
> diff --git a/arm/aarch64/include/kvm/kvm-arch.h b/arm/aarch64/include/kvm/kvm-arch.h
> index 159567b..5e5ee41 100644
> --- a/arm/aarch64/include/kvm/kvm-arch.h
> +++ b/arm/aarch64/include/kvm/kvm-arch.h
> @@ -1,6 +1,8 @@
>  #ifndef KVM__KVM_ARCH_H
>  #define KVM__KVM_ARCH_H
>  
> +#include <linux/sizes.h>
> +
>  struct kvm;
>  unsigned long long kvm__arch_get_kern_offset(struct kvm *kvm, int fd);
>  int kvm__arch_get_ipa_limit(struct kvm *kvm);
> @@ -21,6 +23,8 @@ int kvm__arch_get_ipa_limit(struct kvm *kvm);
>  	max_ram;							\
>  })
>  
> +#define MAX_PAGE_SIZE	SZ_64K
> +
>  #include "arm-common/kvm-arch.h"
>  
>  #endif /* KVM__KVM_ARCH_H */
> diff --git a/mips/include/kvm/kvm-arch.h b/mips/include/kvm/kvm-arch.h
> index fdc09d8..e2f048a 100644
> --- a/mips/include/kvm/kvm-arch.h
> +++ b/mips/include/kvm/kvm-arch.h
> @@ -1,6 +1,7 @@
>  #ifndef KVM__KVM_ARCH_H
>  #define KVM__KVM_ARCH_H
>  
> +#include <linux/sizes.h>
>  
>  /*
>   * Guest memory map is:
> @@ -36,6 +37,8 @@
>  
>  #define VIRTIO_DEFAULT_TRANS(kvm)	VIRTIO_PCI
>  
> +#define MAX_PAGE_SIZE		SZ_64K
> +
>  #include <stdbool.h>
>  
>  #include "linux/types.h"
> diff --git a/powerpc/include/kvm/kvm-arch.h b/powerpc/include/kvm/kvm-arch.h
> index 26d440b..8eeca50 100644
> --- a/powerpc/include/kvm/kvm-arch.h
> +++ b/powerpc/include/kvm/kvm-arch.h
> @@ -13,6 +13,7 @@
>  
>  #include <stdbool.h>
>  #include <linux/types.h>
> +#include <linux/sizes.h>
>  #include <time.h>
>  
>  /*
> @@ -48,6 +49,8 @@
>  
>  #define KVM_IOEVENTFD_HAS_PIO		0
>  
> +#define MAX_PAGE_SIZE			SZ_256K
> +
>  #define VIRTIO_DEFAULT_TRANS(kvm)	VIRTIO_PCI
>  
>  struct spapr_phb;
> diff --git a/vfio/pci.c b/vfio/pci.c
> index a08352d..78f5ca5 100644
> --- a/vfio/pci.c
> +++ b/vfio/pci.c
> @@ -1,3 +1,5 @@
> +#include "linux/sizes.h"
> +
>  #include "kvm/irq.h"
>  #include "kvm/kvm.h"
>  #include "kvm/kvm-cpu.h"
> @@ -926,7 +928,7 @@ static int vfio_pci_create_msix_table(struct kvm *kvm, struct vfio_device *vdev)
>  	if (!info.size)
>  		return -EINVAL;
>  
> -	map_size = ALIGN(info.size, PAGE_SIZE);
> +	map_size = ALIGN(info.size, MAX_PAGE_SIZE);
>  	table->guest_phys_addr = pci_get_mmio_block(map_size);
>  	if (!table->guest_phys_addr) {
>  		pr_err("cannot allocate MMIO space");
> @@ -958,7 +960,7 @@ static int vfio_pci_create_msix_table(struct kvm *kvm, struct vfio_device *vdev)
>  		if (!info.size)
>  			return -EINVAL;
>  
> -		map_size = ALIGN(info.size, PAGE_SIZE);
> +		map_size = ALIGN(info.size, MAX_PAGE_SIZE);
>  		pba->guest_phys_addr = pci_get_mmio_block(map_size);
>  		if (!pba->guest_phys_addr) {
>  			pr_err("cannot allocate MMIO space");
> diff --git a/x86/include/kvm/kvm-arch.h b/x86/include/kvm/kvm-arch.h
> index 85cd336..d8a7312 100644
> --- a/x86/include/kvm/kvm-arch.h
> +++ b/x86/include/kvm/kvm-arch.h
> @@ -5,6 +5,7 @@
>  
>  #include <stdbool.h>
>  #include <linux/types.h>
> +#include <linux/sizes.h>
>  #include <time.h>
>  
>  /*
> @@ -30,6 +31,8 @@
>  
>  #define KVM_IOEVENTFD_HAS_PIO	1
>  
> +#define MAX_PAGE_SIZE		SZ_4K
> +
>  #define VIRTIO_DEFAULT_TRANS(kvm)	VIRTIO_PCI
>  
>  struct kvm_arch {

Re: [PATCH v2 kvmtool 0/7] vfio/pci: Fix MSIX table and PBA size allocation

[Will Deacon]

On Tue, 12 Oct 2021 14:25:03 +0100, Alexandru Elisei wrote:
> This series is meant to rework the way the MSIX table and PBA are allocated
> to prevent situations where the size allocated by kvmtool is larger than
> the size of the BAR that holds them.
> 
> Patches 1-3 are fixes for stuff I found when I was investing a bug
> triggered by the incorrect sizing of the table and PBA.
> 
> [...]

Applied to kvmtool (master), thanks!

[1/7] arm/gicv2m: Set errno when gicv2_update_routing() fails
      https://git.kernel.org/will/kvmtool/c/e3b0ade2de2b
[2/7] vfio/pci.c: Remove double include for assert.h
      https://git.kernel.org/will/kvmtool/c/3d3dca077ae2
[3/7] pci: Fix pci_dev_* print macros
      https://git.kernel.org/will/kvmtool/c/34bfe5f632a4
[4/7] vfio/pci: Rename PBA offset in device descriptor to fd_offset
      https://git.kernel.org/will/kvmtool/c/5f44d5d6e45f
[5/7] vfio/pci: Rework MSIX table and PBA physical size allocation
      https://git.kernel.org/will/kvmtool/c/f93acc042fbd
[6/7] vfio/pci: Print an error when offset is outside of the MSIX table or PBA
      https://git.kernel.org/will/kvmtool/c/b20d6e302940
[7/7] vfio/pci: Align MSIX Table and PBA size to guest maximum page size
      https://git.kernel.org/will/kvmtool/c/39181fc6429f

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev