[04/13] KVM: PPC: Ultravisor: Use UV_WRITE_PATE ucall to register a PATE

From: Michael Anderson <andmike@linux.ibm.com>

The Nest_MMU needs to know the address of the Partition table (PT).
However the PT is in secure memory, and nestMMU cannot access secure
memory.  Hence hypevisor will continue to use a Partition table of
its own. It will have PATE entries for HV and for Normal virtual
machines. The same entries are also in the UV's PT.  The HV's PT
is programmed with the nest MMU.

Suggested-by: Ryan Grimm <grimm@linux.vnet.ibm.com>
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
[device node name to ibm,ultravisor]
Signed-off-by: Michael Anderson <andmike@linux.ibm.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 arch/powerpc/include/asm/ucall-api.h        | 30 +++++++++++++++++++++++++++++
 arch/powerpc/include/uapi/asm/uapi_uvcall.h |  2 ++
 arch/powerpc/mm/hash_utils_64.c             |  4 +++-
 arch/powerpc/mm/pgtable-book3s64.c          | 29 ++++++++++++++++++++++++++--
 arch/powerpc/mm/pgtable-radix.c             | 10 +++++++---
 5 files changed, 69 insertions(+), 6 deletions(-)

On Tue, Jan 22, 2019 at 07:59:35AM -0800, Ram Pai wrote:
> From: Michael Anderson <andmike@linux.ibm.com>
> 
> The Nest_MMU needs to know the address of the Partition table (PT).
> However the PT is in secure memory, and nestMMU cannot access secure
> memory.  Hence hypevisor will continue to use a Partition table of
> its own. It will have PATE entries for HV and for Normal virtual
> machines. The same entries are also in the UV's PT.  The HV's PT
> is programmed with the nest MMU.

This isn't a good patch description.  It's confusing because it
doesn't start with the primary motivation of the patch - which is that
when running under an ultravisor, the ultravisor controls the real
partition table and has it in secure memory where the hypervisor can't
access it, and therefore we (the HV) have to do a ucall whenever we
want to update an entry.  Once you have explained that, then you can
explain the secondary aspect of the patch, which is that the HV still
keeps a copy of its view of the partition table in normal memory so
that the nest MMU can access it.

> Suggested-by: Ryan Grimm <grimm@linux.vnet.ibm.com>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> [device node name to ibm,ultravisor]
> Signed-off-by: Michael Anderson <andmike@linux.ibm.com>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>

If Michael Anderson wrote the patch and sent it to you, and you sent
it to the list, why is Maddy's signoff relevant?  Was the original
version of the patch actually written by Maddy?

> diff --git a/arch/powerpc/mm/pgtable-book3s64.c b/arch/powerpc/mm/pgtable-book3s64.c

Given how much this series is modifying in arch/powerpc outside of
arch/powerpc/kvm, next time you need to cc the whole series to
linuxppc-dev@ozlabs.org.

> +void mmu_partition_table_set_entry(unsigned int lpid, unsigned long dw0,
> +				  unsigned long dw1)
> +{
> +	pr_info("%s: SMF Regitered PATE for Hypervisor dw0 = 0x%lx dw1 = 0x%lx ", __func__, dw0, dw1);

Here you're printing an SMF message on all powernv systems, the vast
majority of which won't have any SMF or ultravisor capability.  That
seems unnecessary and confusing.

Also you have a spelling error in "Regitered".

Paul.

On Wed, Jan 23, 2019 at 10:48:20AM +1100, Paul Mackerras wrote:
> On Tue, Jan 22, 2019 at 07:59:35AM -0800, Ram Pai wrote:
> > From: Michael Anderson <andmike@linux.ibm.com>
> > 
> > The Nest_MMU needs to know the address of the Partition table (PT).
> > However the PT is in secure memory, and nestMMU cannot access secure
> > memory.  Hence hypevisor will continue to use a Partition table of
> > its own. It will have PATE entries for HV and for Normal virtual
> > machines. The same entries are also in the UV's PT.  The HV's PT
> > is programmed with the nest MMU.
> 
> This isn't a good patch description.  It's confusing because it
> doesn't start with the primary motivation of the patch - which is that
> when running under an ultravisor, the ultravisor controls the real
> partition table and has it in secure memory where the hypervisor can't
> access it, and therefore we (the HV) have to do a ucall whenever we
> want to update an entry.  Once you have explained that, then you can
> explain the secondary aspect of the patch, which is that the HV still
> keeps a copy of its view of the partition table in normal memory so
> that the nest MMU can access it.
> 
> > Suggested-by: Ryan Grimm <grimm@linux.vnet.ibm.com>
> > Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> > [device node name to ibm,ultravisor]
> > Signed-off-by: Michael Anderson <andmike@linux.ibm.com>
> > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> 
> If Michael Anderson wrote the patch and sent it to you, and you sent
> it to the list, why is Maddy's signoff relevant?  Was the original
> version of the patch actually written by Maddy?

Yes. this patch description needs a good amount work.  A couple of
related patches; some by Michael and some by me and some bug fixes by Maddy,
were merged  leading to this signoff cocktail and incoherent
description.

This was my first attempt to bring a logical order to
our internal set of patches, which I agree has some more way to go. :(

Thanks,
RP