From: Andre Przywara <andre.przywara@arm.com>
To: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: kvm@vger.kernel.org, will@kernel.org,
julien.thierry.kdev@gmail.com, sami.mujawar@arm.com,
lorenzo.pieralisi@arm.com
Subject: Re: [PATCH kvmtool 07/16] pci: Fix ioport allocation size
Date: Tue, 28 Jan 2020 18:26:50 +0000 [thread overview]
Message-ID: <20200128182650.12b4430b@donnerap.cambridge.arm.com> (raw)
In-Reply-To: <20191125103033.22694-8-alexandru.elisei@arm.com>
On Mon, 25 Nov 2019 10:30:24 +0000
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
Hi,
> 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. However, the region must be big enough to hold the
> virtio common interface described in [1], which is 20 bytes, and other
> MSI-X and/or device specific configuration. To be consistent, let's also
> limit the memory region described by BAR1 to 256. This is the same size
> used by BAR2 for each of the two MSI-X vectors.
So the I/O port size is surely fine, QEMU seems to get away with 64 or even 32 bytes.
But QEMU also reports a memory region size of 4K, is that something we need to consider?
> [1] VIRTIO Version 1.0 Committee Specification 04, section 4.4.8.
I think that should read section 4.1.4.8.
The rest looks OK.
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Cheers,
Andre
> Cc: julien.thierry.kdev@gmail.com
> 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(-)
>
> diff --git a/hw/vesa.c b/hw/vesa.c
> index 70ab59974f76..0191e9264666 100644
> --- a/hw/vesa.c
> +++ b/hw/vesa.c
> @@ -62,8 +62,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 32a07335a765..b4677434c50c 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),
> };
>
next prev parent reply other threads:[~2020-01-28 18:27 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-25 10:30 [PATCH kvmtool 00/16] Add writable BARs and PCIE 1.1 support Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 01/16] Makefile: Use correct objcopy binary when cross-compiling for x86_64 Alexandru Elisei
2019-11-27 18:23 ` Andre Przywara
2019-11-25 10:30 ` [PATCH kvmtool 02/16] pci: Fix BAR resource sizing arbitration Alexandru Elisei
2019-11-27 18:24 ` Andre Przywara
2019-11-28 9:37 ` Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 03/16] Remove pci-shmem device Alexandru Elisei
2019-11-27 18:24 ` Andre Przywara
2019-11-25 10:30 ` [PATCH kvmtool 04/16] Check that a PCI device's memory size is power of two Alexandru Elisei
2019-11-27 18:25 ` Andre Przywara
2020-01-15 12:43 ` Alexandru Elisei
2020-01-15 14:07 ` Andre Przywara
2020-01-15 15:00 ` Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 05/16] arm: pci.c: Advertise only PCI bus 0 in the DT Alexandru Elisei
2019-11-28 17:43 ` Andre Przywara
2020-01-15 14:49 ` Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 06/16] ioport: pci: Move port allocations to PCI devices Alexandru Elisei
2020-01-28 18:25 ` Andre Przywara
2020-01-29 10:07 ` Alexandru Elisei
2020-01-29 10:29 ` Andre Przywara
2019-11-25 10:30 ` [PATCH kvmtool 07/16] pci: Fix ioport allocation size Alexandru Elisei
2020-01-28 18:26 ` Andre Przywara [this message]
2020-01-29 11:11 ` Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 08/16] arm/pci: Fix PCI IO region Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 09/16] arm/pci: Do not use first PCI IO space bytes for devices Alexandru Elisei
2019-12-02 12:15 ` Lorenzo Pieralisi
2020-01-15 15:08 ` Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 10/16] virtio/pci: Make memory and IO BARs independent Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 11/16] virtio/pci: Ignore MMIO and I/O accesses when they are disabled Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 12/16] Use independent read/write locks for ioport and mmio Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 13/16] vfio: Add support for BAR configuration Alexandru Elisei
2019-11-29 17:05 ` Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 14/16] virtio/pci: " Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 15/16] arm/fdt: Remove 'linux,pci-probe-only' property Alexandru Elisei
2019-11-25 10:30 ` [PATCH kvmtool 16/16] Add PCI Express 1.1 support Alexandru Elisei
2019-11-28 17:41 ` [PATCH kvmtool 00/16] Add writable BARs and PCIE " Lorenzo Pieralisi
2020-01-15 15:10 ` Alexandru Elisei
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200128182650.12b4430b@donnerap.cambridge.arm.com \
--to=andre.przywara@arm.com \
--cc=alexandru.elisei@arm.com \
--cc=julien.thierry.kdev@gmail.com \
--cc=kvm@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=sami.mujawar@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).