Re: [PATCH v5 00/12] KVM Support for MIPS32 Processors

[Paolo Bonzini <pbonzini@redhat.com>]

Il 18/06/2014 00:10, James Hogan ha scritto:
> The patchset depends on v4 of "target-mips: implement UserLocal
> Register". I'm aiming for QEMU 2.1, hopefully it isn't too late to get
> some final review.
>
> Thanks to everybody who has already taken part in review.
>
> This patchset implements KVM support for MIPS32 processors, using Trap &
> Emulation.
>
> In KVM mode, CPU virtualization is handled via the kvm kernel module,
> while system and I/O virtualization leverage the Malta model already
> present in QEMU.
>
> Both Guest kernel and Guest Userspace execute in UM. The Guest address
> space is as folows:
> Guest User address space:   0x00000000 -> 0x40000000
> Guest Kernel Unmapped:      0x40000000 -> 0x60000000
> Guest Kernel Mapped:        0x60000000 -> 0x80000000
>
> As a result, Guest Usermode virtual memory is limited to 1GB.
>
> KVM support (by trap and emulate) was added to the Linux kernel in
> v3.10. This patchset partly depends on MIPS KVM work which will land in
> v3.16 (for example to save/restore the state of various registers and
> the KVM Count/Compare timer).
>
> Changes in v5:
>
> Changes addressing review comments from v4 patchset, and to use the MIPS
> KVM timer API added in v3.16.
>
> A git tag for this version of the patchset can also be found on github:
> https://github.com/jahogan/qemu-kvm-mips.git kvm-mips-v5
>
>  - Rebase on master + v4 of "target-mips: implement UserLocal Register".
>  - New patch ([01/12] target-mips: Reset CPU timer consistently) to
>    address timer reset behaviour (reported by Paolo Bonzini).
>  - New patch ([08/12] target-mips: Call kvm_mips_reset_vcpu() from
>    mips_cpu_reset()) and rename kvm_arch_reset_vcpu to
>    kvm_mips_reset_vcpu, based on commit 50a2c6e55fa2 (kvm: reset state
>    from the CPU's reset method).
>  - KSEG0 doesn't actually change size, so fix mask in
>    cpu_mips_kseg0_to_phys() (patch 3) and use that instead of having the
>    KVM specific cpu_mips_kvm_um_kseg0_to_phys() (patch 10).
>  - Fix typo in patch 9 subject (s/interupts/interrupts/).
>  - Rename kvm_mips_te_{put,get}_cp0_registers() functions to drop the
>    "te_" since they're not really specific to T&E.
>  - Pass level through from kvm_arch_put_registers() to
>    kvm_mips_put_cp0_registers() rather than hard coding it to
>    KVM_PUT_FULL_STATE.
>  - Fix KVM_REG_MIPS_CP0_* definitions to set KVM_REG_MIPS and
>    KVM_REG_SIZE_U32/KVM_REG_SIZE_U64 (using a macro).
>  - Remove unused KVM_REG_MIPS_CP0_* definitions for now.
>  - Correct type of kvm_mips_{get,put}_one_{,ul}reg() reg_id argument to
>    uint64_t. Various high bits must be set to disambiguate the
>    architecture and register size.
>  - Simplify register access functions slightly.
>  - Add register accessors for always-64-bit registers (rather than ulong
>    registers). These are needed for virtual KVM registers for
>    controlling the KVM Compare/Count timer.
>  - Save and restore KVM timer state with the rest of the state, and also
>    when VM clock is started or stopped. When the KVM timer state is
>    restored (or VM clock restarted) it is resumed with the stored count
>    at the monotonic time when the VM clock was last stopped. If the VM
>    clock hasn't been stopped it resumes from the monotonic time when the
>    state was saved (i.e. as if the timer was never stopped).
>    Changes since RFC patch on kernel KVM thread "[PATCH v2 00/23] MIPS:
>                  KVM: Fixes and guest timer rewrite"):
>     - Simplified, removing extra state for storing VM time of
>       save/restore, at the cost of losing/gaining time when VM gets
>       stopped and started (Paolo Bonzini).
>  - Save and restore the UserLocal and HWREna CP0 registers.
>  - Improve get/put KVM register error handling with DPRINTFs and fall
>    through so that getting/putting of all the registers is attempted
>    even if one of them fails due to being unimplemented in the kernel.
>
> Changes in v4:
>
> Changes mostly addressing a few review comments from v3 patchset.
>
> A git tag for this version of the patchset can also be found on github:
> https://github.com/jahogan/qemu-kvm-mips.git kvm-mips-v4
>
>  - Rebase on v2.0.0-rc0.
>  - Use int32_t instead of int32 (which is for softfloat) in kvm register
>    accessors (Andreas Färber).
>  - Use uint64_t instead of __u64 (which is really just for kernel
>    headers) in the kvm register accessors (Andreas Färber).
>  - Cast pointer to uintptr_t rather than target_ulong in kvm register
>    accessors.
>  - Remove some redundant casts in kvm register accessors.
>  - Add MAINTAINERS entry for MIPS KVM.
>
> Changes in v3:
>
> Changes mostly addressing review comments from v2 patchset.
>
> A git tag for this version of the patchset can also be found on github:
> https://github.com/jahogan/qemu-kvm-mips.git kvm-mips-v3
>
>  - Remove "target-mips: Set target page size to 16K in KVM mode". It
>    should actually work fine with 4k TARGET_PAGE_SIZE as long as there
>    is no cache aliasing or both host and guest kernels are configured to
>    a sufficient page size to avoid aliasing (which the kernel
>    arch/mips/kvm/00README.txt alludes to anyway).
>  - Rewrote kvm sigmask patch to allow sigmask length to be set by
>    kvm_arch_init(), so that MIPS can set it to 16 as it has 128 signals.
>    This is better than cluttering kvm-all.c with TARGET_* ifdefs (Peter
>    Maydell).
>  - Set sigmask length to 16 from kvm_arch_init() since MIPS Linux has
>    128 signals. This is better than cluttering kvm_all.c with TARGET_*
>    ifdefs (Peter Maydell).
>  - s/dprintf/DPRINTF/ (Andreas Färber).
>  - Use "cs" rather than "cpu" or "env" for CPUState variable names
>    (Andreas Färber).
>  - Use CPUMIPSState rather than CPUArchState (Andreas Färber).
>  - Pass MIPSCPU to cpu_mips_io_interrupts_pending() rather than
>    CPUMIPSState (Andreas Färber).
>  - Remove spurious parentheses around cpu_mips_io_interrupts_pending()
>    call (Andreas Färber).
>  - Pass MIPSCPU to kvm_mips_set_[ipi_]interrupt (Andreas Färber).
>  - Make use of error_report (Andreas Färber) and clean up error messages
>    a little to include __func__.
>  - Remove inline kvm_mips_{put,get}_one_[ul]reg() declarations from
>    kvm_mips.h. They're only used in target-mips/kvm.c anyway.
>  - Make kvm_arch_{put,get}_registers static within target-mips/kvm.c and
>    remove from kvm_mips.h.
>  - Remove unnecessary includes from Malta patch, especially linux/kvm.h
>    which isn't a good idea on non-Linux (Peter Maydell).
>
> Changes in v2:
>
> This patchset is based on Sanjay Lal's V1 patchset from 2nd March 2013:
> https://patchwork.kernel.org/project/kvm/list/?submitter=51991&state=*&q=qemu-devel
>
> I think I've addressed all the V1 feedback. The other main change is the
> removal of the boot-CPS ROM code binary blob and GIC/SMP support since
> it's all slightly orthogonal to KVM support. Instead the existing
> minimal bootloader code for Malta has been updated to work with KVM T&E.
>
> A git tag for this version of the patchset can also be found on github:
> https://github.com/jahogan/qemu-kvm-mips.git kvm-mips-v2
>
>  - Expand commit messages
>  - Rebase on v1.7.0
>  - Misc checkpatch and other cleanups
>  - Some interrupt bug fixes from Yann Le Du <ledu@kymasys.com>
>  - Add get/set register functionality from Yann Le Du <ledu@kymasys.com>
>  - Use new 64 bit compatible ABI from Cavium from Sanjay Lal
>    <sanjayl@kymasys.com>
>  - Add dummy kvm_arch_init_irq_routing()
>    The common KVM code insists on calling kvm_arch_init_irq_routing() as
>    soon as it sees kernel header support for it (regardless of whether
>    QEMU supports it). Provide a dummy function to satisfy this.
>  - Remove request_interrupt_window code (Peter Maydell)
>  - Remove #ifdef CONFIG_KVM where guarded by kvm_enabled() already
>  - Removal of cps / GIC / SMP support
>  - Minimal bootloader modified to execute safely from RAM
>  - Create asm-mips symlink using generic code and move above default
>    case (Peter Maydell)
>  - Remove redundant check of target_name = cpu = mips
>  - Remove mipsel cross compilation fix, which is now fixed by commit
>    61cc919f73ea (configure: detect endian via compile test)
>  - Add translation of guest kernel segments to allow an attached gdb to
>    see kernel memory correctly
>
> James Hogan (7):
>   target-mips: Reset CPU timer consistently
>   target-mips: get_physical_address: Add defines for segment bases
>   target-mips: get_physical_address: Add KVM awareness
>   kvm: Allow arch to set sigmask length
>   target-mips: Call kvm_mips_reset_vcpu() from mips_cpu_reset()
>   hw/mips: malta: Add KVM support
>   MAINTAINERS: Add entry for MIPS KVM
>
> Sanjay Lal (5):
>   hw/mips/cputimer: Don't start periodic timer in KVM mode
>   hw/mips: Add API to convert KVM guest KSEG0 <-> GPA
>   target-mips: kvm: Add main KVM support for MIPS
>   hw/mips: In KVM mode, inject IRQ2 (I/O) interrupts via ioctls
>   target-mips: Enable KVM support in build system
>
>  MAINTAINERS               |   5 +
>  configure                 |   6 +-
>  hw/mips/addr.c            |   7 +-
>  hw/mips/cputimer.c        |  18 +-
>  hw/mips/mips_int.c        |  11 +
>  hw/mips/mips_malta.c      |  73 +++--
>  include/hw/mips/cpudevs.h |   2 +
>  include/sysemu/kvm.h      |   2 +
>  kvm-all.c                 |  11 +-
>  target-mips/Makefile.objs |   1 +
>  target-mips/cpu.c         |   8 +
>  target-mips/helper.c      |  51 +++-
>  target-mips/kvm.c         | 683 ++++++++++++++++++++++++++++++++++++++++++++++
>  target-mips/kvm_mips.h    |  26 ++
>  target-mips/translate.c   |   2 +
>  15 files changed, 866 insertions(+), 40 deletions(-)
>  create mode 100644 target-mips/kvm.c
>  create mode 100644 target-mips/kvm_mips.h
>

Thanks, it's a very clean patch set.  I'll leave a few days for Aurelien 
to comment, and then apply to uq/master.

Paolo