Re: [Qemu-ppc] [PATCH v4 0/8] ISA 3.00 KVM guest support

[David Gibson]

On Thu, Mar 16, 2017 at 05:08:12PM +1100, Sam Bobroff wrote:
> 
> ISA 3.00 KVM guest support
> 
> Notes for v4:
> 
> * Patch "spapr: Small cleanup of PPC MMU enums" has already been accepted,
>   so it's dropped.
> * Patches "scripts/update-linux-headers.sh: refactor extra files" and
>   "scripts/update-linux-headers.sh: add new files for ARM" have been posted to
>   qemu-devel separately so are dropped here.
> * Patch "Update headers using update-linux-headers.sh" is no longer necessary
>   because a header update has been done upstream, so it is dropped.
> 
> Because KVM will soon provide the necessary infrastructure for KVM guests to
> run on POWER9 CPUs, we can now start exploiting this new functionality from
> QEMU. See:
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-January/153433.html
> 
> This set aims to support only the following scenarios:
> * A POWER9 host running in radix mode, running a guest in radix mode.
> * A POWER9 host running in hash mode, running a guest in hash mode.
> * A POWER9 host running in hash mode, running a guest in legacy(+) mode.
> (+) Where legacy means that the guest does not support ISA 3.00.
> 
> Hash or radix mode for the host is controlled via the "disable_radix" kernel
> command line parameter: the host will use radix unless disable_radix is given.
> For the guest it should be automatically selected to match the host.
> 
> Bad legacy guests: There are some recent kernels (e.g. 4.9) that will, when 
> run
> as a KVM guest and if the ibm,pa-features entry in the device tree has the
> Radix MMU bit set, attempt to initialize the MMU as if they were a host (which
> will cause them to crash). To avoid exposing this problem, the Radix MMU bit
> is removed from ibm,pa-features when a legacy guest is detected.
> 
> Notes:
> * This set is based on David Gibson's ppc-for-2.10 branch, available
> in his tree on github at https://github.com/dgibson/qemu.git .
> * This set sits on top of Suraj's recent patch set titled "target/ppc:
> Implement POWER9 pseries TCG legacy support", archved here:
> https://lists.gnu.org/archive/html/qemu-ppc/2017-03/msg00004.html .
> * I have merged my patch that implements with h_register_process_table()
> with the patches Suraj's work that implement the same function for TCG (to 
> make
> integration easier). The patches are now:
>     * target/ppc: Add new H-CALL shells for in memory table translation
>     * target/ppc: Implement H_REGISTER_PROCESS_TABLE H_CALL
> * The set itself and Suraj's related work are availbe in a branch called 
> p9-cas,
> from my tree on github at https://github.com/sambltc/qemu.git .

This is basically good, but there are enough nits that I'd like a
respin (most of the problems I'd just fix during merge, but some of
the compile failures are a bit more fiddly).

For future reference, before sending can you, as a matter of course:
   * Compile test for all targets (use ./configure with no parameters)
   * Do the above on both ppc and x86 hosts
   * Run checkpatch.pl on all patches

> 
> Patch set changelog follows:
> 
> ====== Version 3 -> version 4: ======
> 
> Patch 1/8: spapr: Add ibm,processor-radix-AP-encodings to the device tree
> * Don't clear ppc->radix_page_info when not using KVM.
> 
> Patch 3/8: spapr: Only setup HPT if necessary
> * Fixed typo in title.
> * Reworked to track that no HPT has been allocated using the GR bit of PATB.
> 
> Patch 5/8: target/ppc: Implement H_REGISTER_PROCESS_TABLE H_CALL
> * h_register_process_table(): Moved bit ops out of macro definitions and
>   associated code reorganization.
> 
> Patch 7/8: spapr: Enable ISA 3.0 MMU mode selection via CAS
> * spapr_populate_pa_features(): Removed explicit array length specifier. It
>   would be nice toadd a QEMU_BUILD_BUG_ON() in it's place but it's not 
> possible
>   because the pa_features array can't be const.
> 
> ====== Version 2 -> version 3: ======
> 
> Patch 2/13: spapr: Small cleanup of PPC MMU enums
> * Slight changes due to reordering patches.
> * Fixed additional uses of POWERPC_MMU_3_00 caused by reordering.
> 
> Patch 8/13: spapr: Only setup HTP if necessary.
> * in spapr_setup_hpt_and_vrma() replaced MACHINE(qdev_get_machine()) with 
> MACHINE(spapr).
> 
> Patch 9/13: target/ppc: Add new H-CALL shells for in memory table translation
> * Brought in from Suraj's work.
> 
> Patch 10/13: target/ppc: Implement H_REGISTER_PROCESS_TABLE H_CALL
> * KVM-only implementation of h_register_process_table() integrated into the 
> TCG version.
> 
> Patch 12/13: spapr: Enable ISA 3.0 MMU mode selection via CAS
> * Added some terse documentation for the ibm,pa_features bits for ISA 3.00.
> * Shortened the error reports for guest termination a little.
> 
> Patch 13/13: spapr: Workaround for broken radix guests
> * Legacy guest detection now uses the ISA 3.00 bit in option vector 1.
> 
> ====== Version 1 -> version 2: ======
> 
> Patch 4/14: scripts/update-linux-headers.sh: refactor extra files
> I've factored the script to make it easier to add new files.
> 
> Patch 5/14: scripts/update-linux-headers.sh: add new files for ARM
> * Added the two new arm headers.
> 
> Patch 6/14: Move virtio_mmio.h to fix update-linux-headers.sh
> * FWIW, here's one way of fixing it.
> 
> Patch 7/14: Update headers using update-linux-headers.sh
> * Added information about where the headers came from.
> 
> Patch 8/14: spapr: Add ibm,processor-radix-AP-encodings to the device tree
> * ppc_radix_page_info now kept in native format, conversion to BE done when 
> adding to the device tree.
> * radix_page_info moved into the CPU class, cleaning up some code.
> 
> Patch 9/14: target-ppc: support KVM_CAP_PPC_MMU_RADIX, KVM_CAP_PPC_MMU_HASH_V3
> * cap_mmu_hash renamed to cap_mmu_hash_v3.
> 
> Patch 10/14: spapr: Only setup HTP if necessary.
> * This patch has been mostly rewritten to move the late HPT allocation to CAS.
> This allows a guest to start in radix mode (when it's in real mode) and then
> change to hash, even if it is a legacy guest and will not call
> h_register_process_table().
> * Added an exported function to spapr.c to perform HPT allocation and adjust
> the vrma if necessary. This makes it possible to allocate the HPT from
> h_client_architecture_support() in spapr_hcall.c.
> 
> Patch 11/14: spapr: Add h_register_process_table() hypercall
> * I haven't addressed review comments for this patch because it overlaps with
> Suraj's implementation of the same function and we'll work together to
> integrate them.
> 
> Patch 13/14: spapr: Enable ISA 3.0 MMU mode selection via CAS
> * Unused bits removed.
> * Logic and bit definitions changed due to architectural change.
> * Cleanly terminate QEMU if the guest requests an unavailable mode (as 
> required
>   by the new architecture).
> * Legacy guest workaround moved to it's own patch.
> * I'm sorry for the bitfield constants in spapr_dt_ov5_platform_support() but
>   there don't seem to be convienent macros for converting an option vector
>   specifier (OV_BIT(x,y)) into a byte-mask. I'm open to suggestions.
> 
> 
> Sam Bobroff (6):
>   spapr: Add ibm,processor-radix-AP-encodings to the device tree
>   target-ppc: support KVM_CAP_PPC_MMU_RADIX, KVM_CAP_PPC_MMU_HASH_V3
>   spapr: Only setup HPT if necessary
>   spapr: move spapr_populate_pa_features()
>   spapr: Enable ISA 3.0 MMU mode selection via CAS
>   spapr: Workaround for broken radix guests
> 
> Suraj Jitindar Singh (2):
>   target/ppc: Add new H-CALL shells for in memory table translation
>   target/ppc: Implement H_REGISTER_PROCESS_TABLE H_CALL
> 
>  hw/ppc/spapr.c              | 228 
> ++++++++++++++++++++++++++++++--------------
>  hw/ppc/spapr_hcall.c        | 179 +++++++++++++++++++++++++++++++++-
>  include/hw/ppc/spapr.h      |   6 ++
>  include/hw/ppc/spapr_ovec.h |   8 ++
>  include/sysemu/kvm.h        |   1 +
>  target/ppc/cpu-qom.h        |   1 +
>  target/ppc/cpu.h            |   4 +
>  target/ppc/kvm.c            |  62 ++++++++++++
>  target/ppc/kvm_ppc.h        |  14 +++
>  9 files changed, 427 insertions(+), 76 deletions(-)
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature