mbox series

[kvmtool,v3,00/22] Unify I/O port and MMIO trap handling

Message ID 20210315153350.19988-1-andre.przywara@arm.com (mailing list archive)
Headers show
Series Unify I/O port and MMIO trap handling | expand

Message

Andre Przywara March 15, 2021, 3:33 p.m. UTC
Hi,

this version is addressing Alexandru's comments, fixing mostly minor
issues in the naming scheme. The biggest change is to keep the
ioport__read/ioport_write wrappers for the serial device.
For more details see the changelog below.
==============

At the moment we use two separate code paths to handle exits for
KVM_EXIT_IO (ioport.c) and KVM_EXIT_MMIO (mmio.c), even though they
are semantically very similar. Because the trap handler callback routine
is different, devices need to decide on one conduit or need to provide
different handler functions for both of them.

This is not only unnecessary code duplication, but makes switching
devices from I/O port to MMIO a tedious task, even though there is no
real difference between the two, especially on ARM and PowerPC.

For ARM we aim at providing a flexible memory layout, and also have
trouble with the UART and RTC device overlapping with the PCI I/O area,
so it seems indicated to tackle this once and for all.

The first three patches do some cleanup, to simplify things later.

Patch 04/22 lays the groundwork, by extending mmio.c to be able to also
register I/O port trap handlers, using the same callback prototype as
we use for MMIO.

The next 14 patches then convert devices that use the I/O port
interface over to the new joint interface. This requires to rework
the trap handler routine to adhere to the same prototype as the existing
MMIO handlers. For most devices this is done in two steps: a first to
introduce the reworked handler routine, and a second to switch to the new
joint registration routine. For some devices the first step is trivial,
so it's done in one patch.

Patch 19/22 then retires the old I/O port interface, by removing ioport.c
and friends.
Patch 20/22 uses the opportunity to clean up the memory map description,
also declares a new region (from 16MB on), where the final two patches
switch the UART and the RTC device to. They are now registered
on the MMIO "bus", when running on ARM or arm64. This moves them away
from the first 64KB, so they are not in the PCI I/O area anymore.

Please have a look and comment!

Cheers,
Andre

Changelog v2 .. v3:
- use _io as function prefix for x86 I/O port devices
- retain ioport__{read,write}8() wrappers for serial device
- fix memory map ASCII art
- fix serial base declaration
- minor nit fixes
- add Reviewed-by: tags

Changelog v1 .. v2:
- rework memory map definition
- add explicit debug output for debug I/O port
- add explicit check for MMIO coalescing on I/O ports
- drop usage of ioport__{read,write}8() from serial
- drop explicit I/O port cleanup routine (to mimic MMIO operation)
- add comment for IOTRAP_BUS_MASK
- minor cleanups / formatting changes


Andre Przywara (22):
  ioport: Remove ioport__setup_arch()
  hw/serial: Use device abstraction for FDT generator function
  ioport: Retire .generate_fdt_node functionality
  mmio: Extend handling to include ioport emulation
  hw/i8042: Clean up data types
  hw/i8042: Refactor trap handler
  hw/i8042: Switch to new trap handlers
  x86/ioport: Refactor trap handlers
  x86/ioport: Switch to new trap handlers
  hw/rtc: Refactor trap handlers
  hw/rtc: Switch to new trap handler
  hw/vesa: Switch trap handling to use MMIO handler
  hw/serial: Refactor trap handler
  hw/serial: Switch to new trap handlers
  vfio: Refactor ioport trap handler
  vfio: Switch to new ioport trap handlers
  virtio: Switch trap handling to use MMIO handler
  pci: Switch trap handling to use MMIO handler
  Remove ioport specific routines
  arm: Reorganise and document memory map
  hw/serial: ARM/arm64: Use MMIO at higher addresses
  hw/rtc: ARM/arm64: Use MMIO at higher addresses

 Makefile                          |   1 -
 arm/include/arm-common/kvm-arch.h |  47 ++++--
 arm/ioport.c                      |   5 -
 hw/i8042.c                        |  94 +++++-------
 hw/rtc.c                          |  91 ++++++------
 hw/serial.c                       | 126 +++++++++++-----
 hw/vesa.c                         |  19 +--
 include/kvm/i8042.h               |   1 -
 include/kvm/ioport.h              |  32 ----
 include/kvm/kvm.h                 |  49 ++++++-
 ioport.c                          | 235 ------------------------------
 mips/kvm.c                        |   5 -
 mmio.c                            |  65 +++++++--
 pci.c                             |  82 +++--------
 powerpc/ioport.c                  |   6 -
 vfio/core.c                       |  50 ++++---
 virtio/pci.c                      |  46 ++----
 x86/ioport.c                      | 108 +++++++-------
 18 files changed, 421 insertions(+), 641 deletions(-)
 delete mode 100644 ioport.c

Comments

Alexandru Elisei March 17, 2021, 5:44 p.m. UTC | #1
Hi Will, Julien,

On 3/15/21 3:33 PM, Andre Przywara wrote:
> Hi,
>
> this version is addressing Alexandru's comments, fixing mostly minor
> issues in the naming scheme. The biggest change is to keep the
> ioport__read/ioport_write wrappers for the serial device.
> For more details see the changelog below.
> ==============
>
> At the moment we use two separate code paths to handle exits for
> KVM_EXIT_IO (ioport.c) and KVM_EXIT_MMIO (mmio.c), even though they
> are semantically very similar. Because the trap handler callback routine
> is different, devices need to decide on one conduit or need to provide
> different handler functions for both of them.
>
> This is not only unnecessary code duplication, but makes switching
> devices from I/O port to MMIO a tedious task, even though there is no
> real difference between the two, especially on ARM and PowerPC.
>
> For ARM we aim at providing a flexible memory layout, and also have
> trouble with the UART and RTC device overlapping with the PCI I/O area,
> so it seems indicated to tackle this once and for all.
>
> The first three patches do some cleanup, to simplify things later.
>
> Patch 04/22 lays the groundwork, by extending mmio.c to be able to also
> register I/O port trap handlers, using the same callback prototype as
> we use for MMIO.
>
> The next 14 patches then convert devices that use the I/O port
> interface over to the new joint interface. This requires to rework
> the trap handler routine to adhere to the same prototype as the existing
> MMIO handlers. For most devices this is done in two steps: a first to
> introduce the reworked handler routine, and a second to switch to the new
> joint registration routine. For some devices the first step is trivial,
> so it's done in one patch.
>
> Patch 19/22 then retires the old I/O port interface, by removing ioport.c
> and friends.
> Patch 20/22 uses the opportunity to clean up the memory map description,
> also declares a new region (from 16MB on), where the final two patches
> switch the UART and the RTC device to. They are now registered
> on the MMIO "bus", when running on ARM or arm64. This moves them away
> from the first 64KB, so they are not in the PCI I/O area anymore.

I have reviewed the series and everything looks fine to me and ready to be merged.
I have also ran the following tests:

- On my x86_64 desktop, I ran a guest with --sdl, to exercise the vesa device.

- On a rockpro64, I ran kvm-unit-tests for arm64 and arm (kvmtool was compiled for
arm64); I also ran Linux guests using 4k and 64k pages with and without --force-pci.

- On a Seattle machine, I did PCI passthrough for an Intel 82574L network card and
ran Linux guests using 4k and 64k pages with and without --force-pci.

- On an odroid-c4 (4 x Cortex-A55), I ran Linux guests using 4k, 16k and 64k pages
with and without --force-pci.

With this series merged, everything will be in place to bring back the patch that
adds PCI Express 1.1 support for arm/arm64 [1]. The patch was previously dropped
because the RTC and UART were overlapping with the PCI I/O space and EDK2 doesn't
not understand PCI I/O bus addresses above 64k, but this series fixes that by
moving the addresses of the two devices.

[1] https://www.spinics.net/lists/kvm/msg211304.html

Thanks,
Alex
Will Deacon March 18, 2021, 10:04 a.m. UTC | #2
On Mon, 15 Mar 2021 15:33:28 +0000, Andre Przywara wrote:
> this version is addressing Alexandru's comments, fixing mostly minor
> issues in the naming scheme. The biggest change is to keep the
> ioport__read/ioport_write wrappers for the serial device.
> For more details see the changelog below.
> ==============
> 
> At the moment we use two separate code paths to handle exits for
> KVM_EXIT_IO (ioport.c) and KVM_EXIT_MMIO (mmio.c), even though they
> are semantically very similar. Because the trap handler callback routine
> is different, devices need to decide on one conduit or need to provide
> different handler functions for both of them.
> 
> [...]

Applied to kvmtool (master), thanks!

[01/22] ioport: Remove ioport__setup_arch()
        https://git.kernel.org/will/kvmtool/c/97531eb2ca70
[02/22] hw/serial: Use device abstraction for FDT generator function
        https://git.kernel.org/will/kvmtool/c/a81be31eee6e
[03/22] ioport: Retire .generate_fdt_node functionality
        https://git.kernel.org/will/kvmtool/c/9bc7e2ce343e
[04/22] mmio: Extend handling to include ioport emulation
        https://git.kernel.org/will/kvmtool/c/96f0c86ce221
[05/22] hw/i8042: Clean up data types
        https://git.kernel.org/will/kvmtool/c/fc7696277b29
[06/22] hw/i8042: Refactor trap handler
        https://git.kernel.org/will/kvmtool/c/f7ef3dc0cd28
[07/22] hw/i8042: Switch to new trap handlers
        https://git.kernel.org/will/kvmtool/c/d24bedb1cc9a
[08/22] x86/ioport: Refactor trap handlers
        https://git.kernel.org/will/kvmtool/c/82304999f936
[09/22] x86/ioport: Switch to new trap handlers
        https://git.kernel.org/will/kvmtool/c/3adbcb235020
[10/22] hw/rtc: Refactor trap handlers
        https://git.kernel.org/will/kvmtool/c/8c45f36430bd
[11/22] hw/rtc: Switch to new trap handler
        https://git.kernel.org/will/kvmtool/c/123ee474b97b
[12/22] hw/vesa: Switch trap handling to use MMIO handler
        https://git.kernel.org/will/kvmtool/c/38ae332ffcec
[13/22] hw/serial: Refactor trap handler
        https://git.kernel.org/will/kvmtool/c/47a510600e08
[14/22] hw/serial: Switch to new trap handlers
        https://git.kernel.org/will/kvmtool/c/59866df60b4b
[15/22] vfio: Refactor ioport trap handler
        https://git.kernel.org/will/kvmtool/c/a4a0dac75469
[16/22] vfio: Switch to new ioport trap handlers
        https://git.kernel.org/will/kvmtool/c/579bc61f8798
[17/22] virtio: Switch trap handling to use MMIO handler
        https://git.kernel.org/will/kvmtool/c/205eaa794be7
[18/22] pci: Switch trap handling to use MMIO handler
        https://git.kernel.org/will/kvmtool/c/1f56b9d10a28
[19/22] Remove ioport specific routines
        https://git.kernel.org/will/kvmtool/c/7e19cb54a7cc
[20/22] arm: Reorganise and document memory map
        https://git.kernel.org/will/kvmtool/c/f01cc778bd65
[21/22] hw/serial: ARM/arm64: Use MMIO at higher addresses
        https://git.kernel.org/will/kvmtool/c/45b4968e0de1
[22/22] hw/rtc: ARM/arm64: Use MMIO at higher addresses
        https://git.kernel.org/will/kvmtool/c/382eaad7b695

Cheers,