Re: [PATCH kvmtool 00/21] Unify I/O port and MMIO trap handling

Alexandru Elisei
2021-02-10T17:45:31Z

Hi Andre,

On 12/10/20 2:28 PM, Andre Przywara wrote:
> 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/21 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/21 then retires the old I/O port interface, by removing ioport.c
> and friends.
>
> The final two patches switch the UART and the RTC device over to register
> on the MMIO "bus", when running on ARM or arm64. This changes the
> addresses to be at 16MB, so they are not in the PCI I/O area anymore.
>
> Admittedly this goal can be achieved much simpler, by just having the
> first three patches, and some more changes and ifdef's in the last two,
> but I figured it would be good to clean up the I/O port mess for good.

This is a very good idea, I thought more than once as I was working on other
things that there really is no reason to differentiate between ioport and mmio
emulation.

Thanks,

Alex

>
> Please have a look and comment!
>
> Cheers,
> Andre
>
> Andre Przywara (21):
> 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
> hw/serial: ARM/arm64: Use MMIO at higher addresses
> hw/rtc: ARM/arm64: Use MMIO at higher addresses
>
> Makefile | 1 -
> arm/ioport.c | 5 -
> hw/i8042.c | 88 ++++++----------
> hw/rtc.c | 91 ++++++++---------
> hw/serial.c | 166 +++++++++++++++++++-----------
> hw/vesa.c | 19 +---
> include/kvm/i8042.h | 1 -
> include/kvm/ioport.h | 25 -----
> include/kvm/kvm.h | 42 +++++++-
> ioport.c | 235 -------------------------------------------
> mips/kvm.c | 5 -
> mmio.c | 59 +++++++++--
> pci.c | 82 +++++----------
> powerpc/ioport.c | 6 --
> vfio/core.c | 50 ++++-----
> virtio/pci.c | 42 ++------
> x86/ioport.c | 106 +++++++++----------
> 17 files changed, 385 insertions(+), 638 deletions(-)
> delete mode 100644 ioport.c
>