diff mbox series

[v3,kvmtool,08/32] pci: Fix ioport allocation size

Message ID 20200326152438.6218-9-alexandru.elisei@arm.com (mailing list archive)
State New, archived
Headers show
Series Add reassignable BARs and PCIE 1.1 support | expand

Commit Message

Alexandru Elisei March 26, 2020, 3:24 p.m. UTC 
From: Julien Thierry <julien.thierry@arm.com>

The PCI Local Bus Specification, Rev. 3.0,
Section 6.2.5.1. "Address Maps" states:
"Devices that map control functions into I/O Space must not consume more
than 256 bytes per I/O Base Address register."

Yet all the PCI devices allocate IO ports of IOPORT_SIZE (= 1024 bytes).

Fix this by having PCI devices use 256 bytes ports for IO BARs.

There is no hard requirement on the size of the memory region described
by memory BARs. Since BAR 1 is supposed to offer the same functionality as
IO ports, let's make its size match BAR 0.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
[Added rationale for changing BAR1 size to PCI_IO_SIZE]
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 hw/vesa.c            |  4 ++--
 include/kvm/ioport.h |  1 -
 pci.c                |  2 +-
 virtio/pci.c         | 15 +++++++--------
 4 files changed, 10 insertions(+), 12 deletions(-)

Comments

Andre Przywara March 30, 2020, 9:27 a.m. UTC | #1 
On 26/03/2020 15:24, Alexandru Elisei wrote:
> From: Julien Thierry <julien.thierry@arm.com>

Hi,

> The PCI Local Bus Specification, Rev. 3.0,
> Section 6.2.5.1. "Address Maps" states:
> "Devices that map control functions into I/O Space must not consume more
> than 256 bytes per I/O Base Address register."
> 
> Yet all the PCI devices allocate IO ports of IOPORT_SIZE (= 1024 bytes).
> 
> Fix this by having PCI devices use 256 bytes ports for IO BARs.
> 
> There is no hard requirement on the size of the memory region described
> by memory BARs. Since BAR 1 is supposed to offer the same functionality as
> IO ports, let's make its size match BAR 0.
> 
> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
> [Added rationale for changing BAR1 size to PCI_IO_SIZE]
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>

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

Cheers,
Andre

> ---
>  hw/vesa.c            |  4 ++--
>  include/kvm/ioport.h |  1 -
>  pci.c                |  2 +-
>  virtio/pci.c         | 15 +++++++--------
>  4 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/vesa.c b/hw/vesa.c
> index 24fb46faad3b..d8d91aa9c873 100644
> --- a/hw/vesa.c
> +++ b/hw/vesa.c
> @@ -63,8 +63,8 @@ struct framebuffer *vesa__init(struct kvm *kvm)
>  
>  	if (!kvm->cfg.vnc && !kvm->cfg.sdl && !kvm->cfg.gtk)
>  		return NULL;
> -	r = pci_get_io_port_block(IOPORT_SIZE);
> -	r = ioport__register(kvm, r, &vesa_io_ops, IOPORT_SIZE, NULL);
> +	r = pci_get_io_port_block(PCI_IO_SIZE);
> +	r = ioport__register(kvm, r, &vesa_io_ops, PCI_IO_SIZE, NULL);
>  	if (r < 0)
>  		return ERR_PTR(r);
>  
> diff --git a/include/kvm/ioport.h b/include/kvm/ioport.h
> index b10fcd5b4412..8c86b7151f25 100644
> --- a/include/kvm/ioport.h
> +++ b/include/kvm/ioport.h
> @@ -14,7 +14,6 @@
>  
>  /* some ports we reserve for own use */
>  #define IOPORT_DBG			0xe0
> -#define IOPORT_SIZE			0x400
>  
>  struct kvm;
>  
> diff --git a/pci.c b/pci.c
> index 80b5c5d3d7f3..b6892d974c08 100644
> --- a/pci.c
> +++ b/pci.c
> @@ -20,7 +20,7 @@ static u16 io_port_blocks		= PCI_IOPORT_START;
>  
>  u16 pci_get_io_port_block(u32 size)
>  {
> -	u16 port = ALIGN(io_port_blocks, IOPORT_SIZE);
> +	u16 port = ALIGN(io_port_blocks, PCI_IO_SIZE);
>  
>  	io_port_blocks = port + size;
>  	return port;
> diff --git a/virtio/pci.c b/virtio/pci.c
> index d73414abde05..eeb5b5efa6e1 100644
> --- a/virtio/pci.c
> +++ b/virtio/pci.c
> @@ -421,7 +421,7 @@ static void virtio_pci__io_mmio_callback(struct kvm_cpu *vcpu,
>  {
>  	struct virtio_pci *vpci = ptr;
>  	int direction = is_write ? KVM_EXIT_IO_OUT : KVM_EXIT_IO_IN;
> -	u16 port = vpci->port_addr + (addr & (IOPORT_SIZE - 1));
> +	u16 port = vpci->port_addr + (addr & (PCI_IO_SIZE - 1));
>  
>  	kvm__emulate_io(vcpu, port, data, direction, len, 1);
>  }
> @@ -435,17 +435,16 @@ int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
>  	vpci->kvm = kvm;
>  	vpci->dev = dev;
>  
> -	BUILD_BUG_ON(!is_power_of_two(IOPORT_SIZE));
>  	BUILD_BUG_ON(!is_power_of_two(PCI_IO_SIZE));
>  
> -	r = pci_get_io_port_block(IOPORT_SIZE);
> -	r = ioport__register(kvm, r, &virtio_pci__io_ops, IOPORT_SIZE, vdev);
> +	r = pci_get_io_port_block(PCI_IO_SIZE);
> +	r = ioport__register(kvm, r, &virtio_pci__io_ops, PCI_IO_SIZE, vdev);
>  	if (r < 0)
>  		return r;
>  	vpci->port_addr = (u16)r;
>  
> -	vpci->mmio_addr = pci_get_mmio_block(IOPORT_SIZE);
> -	r = kvm__register_mmio(kvm, vpci->mmio_addr, IOPORT_SIZE, false,
> +	vpci->mmio_addr = pci_get_mmio_block(PCI_IO_SIZE);
> +	r = kvm__register_mmio(kvm, vpci->mmio_addr, PCI_IO_SIZE, false,
>  			       virtio_pci__io_mmio_callback, vpci);
>  	if (r < 0)
>  		goto free_ioport;
> @@ -475,8 +474,8 @@ int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
>  							| PCI_BASE_ADDRESS_SPACE_MEMORY),
>  		.status			= cpu_to_le16(PCI_STATUS_CAP_LIST),
>  		.capabilities		= (void *)&vpci->pci_hdr.msix - (void *)&vpci->pci_hdr,
> -		.bar_size[0]		= cpu_to_le32(IOPORT_SIZE),
> -		.bar_size[1]		= cpu_to_le32(IOPORT_SIZE),
> +		.bar_size[0]		= cpu_to_le32(PCI_IO_SIZE),
> +		.bar_size[1]		= cpu_to_le32(PCI_IO_SIZE),
>  		.bar_size[2]		= cpu_to_le32(PCI_IO_SIZE*2),
>  	};
>  
>
diff mbox series

Patch

diff --git a/hw/vesa.c b/hw/vesa.c
index 24fb46faad3b..d8d91aa9c873 100644
--- a/hw/vesa.c
+++ b/hw/vesa.c
@@ -63,8 +63,8 @@  struct framebuffer *vesa__init(struct kvm *kvm)
 
 	if (!kvm->cfg.vnc && !kvm->cfg.sdl && !kvm->cfg.gtk)
 		return NULL;
-	r = pci_get_io_port_block(IOPORT_SIZE);
-	r = ioport__register(kvm, r, &vesa_io_ops, IOPORT_SIZE, NULL);
+	r = pci_get_io_port_block(PCI_IO_SIZE);
+	r = ioport__register(kvm, r, &vesa_io_ops, PCI_IO_SIZE, NULL);
 	if (r < 0)
 		return ERR_PTR(r);
 
diff --git a/include/kvm/ioport.h b/include/kvm/ioport.h
index b10fcd5b4412..8c86b7151f25 100644
--- a/include/kvm/ioport.h
+++ b/include/kvm/ioport.h
@@ -14,7 +14,6 @@ 
 
 /* some ports we reserve for own use */
 #define IOPORT_DBG			0xe0
-#define IOPORT_SIZE			0x400
 
 struct kvm;
 
diff --git a/pci.c b/pci.c
index 80b5c5d3d7f3..b6892d974c08 100644
--- a/pci.c
+++ b/pci.c
@@ -20,7 +20,7 @@  static u16 io_port_blocks		= PCI_IOPORT_START;
 
 u16 pci_get_io_port_block(u32 size)
 {
-	u16 port = ALIGN(io_port_blocks, IOPORT_SIZE);
+	u16 port = ALIGN(io_port_blocks, PCI_IO_SIZE);
 
 	io_port_blocks = port + size;
 	return port;
diff --git a/virtio/pci.c b/virtio/pci.c
index d73414abde05..eeb5b5efa6e1 100644
--- a/virtio/pci.c
+++ b/virtio/pci.c
@@ -421,7 +421,7 @@  static void virtio_pci__io_mmio_callback(struct kvm_cpu *vcpu,
 {
 	struct virtio_pci *vpci = ptr;
 	int direction = is_write ? KVM_EXIT_IO_OUT : KVM_EXIT_IO_IN;
-	u16 port = vpci->port_addr + (addr & (IOPORT_SIZE - 1));
+	u16 port = vpci->port_addr + (addr & (PCI_IO_SIZE - 1));
 
 	kvm__emulate_io(vcpu, port, data, direction, len, 1);
 }
@@ -435,17 +435,16 @@  int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 	vpci->kvm = kvm;
 	vpci->dev = dev;
 
-	BUILD_BUG_ON(!is_power_of_two(IOPORT_SIZE));
 	BUILD_BUG_ON(!is_power_of_two(PCI_IO_SIZE));
 
-	r = pci_get_io_port_block(IOPORT_SIZE);
-	r = ioport__register(kvm, r, &virtio_pci__io_ops, IOPORT_SIZE, vdev);
+	r = pci_get_io_port_block(PCI_IO_SIZE);
+	r = ioport__register(kvm, r, &virtio_pci__io_ops, PCI_IO_SIZE, vdev);
 	if (r < 0)
 		return r;
 	vpci->port_addr = (u16)r;
 
-	vpci->mmio_addr = pci_get_mmio_block(IOPORT_SIZE);
-	r = kvm__register_mmio(kvm, vpci->mmio_addr, IOPORT_SIZE, false,
+	vpci->mmio_addr = pci_get_mmio_block(PCI_IO_SIZE);
+	r = kvm__register_mmio(kvm, vpci->mmio_addr, PCI_IO_SIZE, false,
 			       virtio_pci__io_mmio_callback, vpci);
 	if (r < 0)
 		goto free_ioport;
@@ -475,8 +474,8 @@  int virtio_pci__init(struct kvm *kvm, void *dev, struct virtio_device *vdev,
 							| PCI_BASE_ADDRESS_SPACE_MEMORY),
 		.status			= cpu_to_le16(PCI_STATUS_CAP_LIST),
 		.capabilities		= (void *)&vpci->pci_hdr.msix - (void *)&vpci->pci_hdr,
-		.bar_size[0]		= cpu_to_le32(IOPORT_SIZE),
-		.bar_size[1]		= cpu_to_le32(IOPORT_SIZE),
+		.bar_size[0]		= cpu_to_le32(PCI_IO_SIZE),
+		.bar_size[1]		= cpu_to_le32(PCI_IO_SIZE),
 		.bar_size[2]		= cpu_to_le32(PCI_IO_SIZE*2),
 	};