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

Paolo Bonzini
2014-06-18T15:00:47Z

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