From: Michael Ellerman <mpe@ellerman•id.au>
To: Claudio Carvalho <cclaudio@linux•ibm.com>, linuxppc-dev@ozlabs•org
Cc: Ryan Grimm <grimm@linux•vnet.ibm.com>,
Madhavan Srinivasan <maddy@linux•vnet.ibm.com>,
Michael Anderson <andmike@linux•ibm.com>,
Ram Pai <linuxram@us•ibm.com>,
Claudio Carvalho <cclaudio@linux•ibm.com>,
kvm-ppc@vger•kernel.org, Bharata B Rao <bharata@linux•ibm.com>,
Ryan Grimm <grimm@linux•ibm.com>,
Sukadev Bhattiprolu <sukadev@linux•vnet.ibm.com>,
Guerney Hunt <gdhh@linux•ibm.com>,
Thiago Bauermann <bauerman@linux•ibm.com>
Subject: Re: [PATCH v5 4/7] powerpc/mm: Use UV_WRITE_PATE ucall to register a PATE
Date: Wed, 14 Aug 2019 21:33:12 +1000 [thread overview]
Message-ID: <871rxo7zif.fsf@concordia.ellerman.id.au> (raw)
In-Reply-To: <20190808040555.2371-5-cclaudio@linux.ibm.com>
Hi Claudio,
Claudio Carvalho <cclaudio@linux•ibm.com> writes:
> From: Michael Anderson <andmike@linux•ibm.com>
>
> In ultravisor enabled systems, the ultravisor creates and maintains the
> partition table in secure memory where the hypervisor cannot access, and
^
which?
> therefore, the hypervisor have to do the UV_WRITE_PATE ucall whenever it
^ ^
has a
> wants to set a partition table entry (PATE).
>
> This patch adds the UV_WRITE_PATE ucall and uses it to set a PATE if
> ultravisor is enabled. Additionally, this also also keeps a copy of the
> partition table because the nestMMU does not have access to secure
> memory. Such copy has entries for nonsecure and hypervisor partition.
I'm having trouble parsing the last sentence there.
Or at least it doesn't seem to match the code, or I don't understand
either the code or the comment. More below.
> diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
> index 85bc81abd286..033731f5dbaa 100644
> --- a/arch/powerpc/mm/book3s64/pgtable.c
> +++ b/arch/powerpc/mm/book3s64/pgtable.c
> @@ -213,34 +223,50 @@ void __init mmu_partition_table_init(void)
> powernv_set_nmmu_ptcr(ptcr);
> }
>
> -void mmu_partition_table_set_entry(unsigned int lpid, unsigned long dw0,
> - unsigned long dw1)
> +/*
> + * Global flush of TLBs and partition table caches for this lpid. The type of
> + * flush (hash or radix) depends on what the previous use of this partition ID
> + * was, not the new use.
> + */
> +static void flush_partition(unsigned int lpid, unsigned long old_patb0)
A nicer API would be for the 2nd param to be a "bool radix", and have
the caller worry about the fact that it comes from (patb0 & PATB_HR).
> {
> - unsigned long old = be64_to_cpu(partition_tb[lpid].patb0);
> -
> - partition_tb[lpid].patb0 = cpu_to_be64(dw0);
> - partition_tb[lpid].patb1 = cpu_to_be64(dw1);
> -
> - /*
> - * Global flush of TLBs and partition table caches for this lpid.
> - * The type of flush (hash or radix) depends on what the previous
> - * use of this partition ID was, not the new use.
> - */
> asm volatile("ptesync" : : : "memory");
> - if (old & PATB_HR) {
> - asm volatile(PPC_TLBIE_5(%0,%1,2,0,1) : :
> + if (old_patb0 & PATB_HR) {
> + asm volatile(PPC_TLBIE_5(%0, %1, 2, 0, 1) : :
> "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
> - asm volatile(PPC_TLBIE_5(%0,%1,2,1,1) : :
> + asm volatile(PPC_TLBIE_5(%0, %1, 2, 1, 1) : :
That looks like an unrelated whitespace change.
> "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
> trace_tlbie(lpid, 0, TLBIEL_INVAL_SET_LPID, lpid, 2, 0, 1);
> } else {
> - asm volatile(PPC_TLBIE_5(%0,%1,2,0,0) : :
> + asm volatile(PPC_TLBIE_5(%0, %1, 2, 0, 0) : :
Ditto.
> "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
> trace_tlbie(lpid, 0, TLBIEL_INVAL_SET_LPID, lpid, 2, 0, 0);
> }
> /* do we need fixup here ?*/
> asm volatile("eieio; tlbsync; ptesync" : : : "memory");
> }
> +
> +void mmu_partition_table_set_entry(unsigned int lpid, unsigned long dw0,
> + unsigned long dw1)
> +{
> + unsigned long old = be64_to_cpu(partition_tb[lpid].patb0);
> +
> + partition_tb[lpid].patb0 = cpu_to_be64(dw0);
> + partition_tb[lpid].patb1 = cpu_to_be64(dw1);
ie. here we always update the copy of the partition table, regardless of
whether we're running under an ultravisor or not. So the copy is a
complete copy isn't it?
> + /*
> + * In ultravisor enabled systems, the ultravisor maintains the partition
> + * table in secure memory where we don't have access, therefore, we have
> + * to do a ucall to set an entry.
> + */
> + if (firmware_has_feature(FW_FEATURE_ULTRAVISOR)) {
> + uv_register_pate(lpid, dw0, dw1);
> + pr_info("PATE registered by ultravisor: dw0 = 0x%lx, dw1 = 0x%lx\n",
> + dw0, dw1);
> + } else {
> + flush_partition(lpid, old);
> + }
What is different is whether we flush or not.
And don't we still need to do the flush for the nestMMU? I assume we're
saying the ultravisor will broadcast a flush for us, which will also
handle the nestMMU case?
cheers
next prev parent reply other threads:[~2019-08-14 11:35 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-08 4:05 [PATCH v5 0/7] kvmppc: Paravirtualize KVM to support ultravisor Claudio Carvalho
2019-08-08 4:05 ` [PATCH v5 1/7] Documentation/powerpc: Ultravisor API Claudio Carvalho
2019-08-09 12:45 ` Michael Ellerman
2019-08-21 22:30 ` Claudio Carvalho
2019-08-12 15:58 ` Fabiano Rosas
2019-08-21 22:01 ` Claudio Carvalho
2019-08-08 4:05 ` [PATCH v5 2/7] powerpc/kernel: Add ucall_norets() ultravisor call handler Claudio Carvalho
2019-08-14 10:46 ` Michael Ellerman
2019-08-14 18:34 ` Segher Boessenkool
2019-08-22 1:26 ` Claudio Carvalho
2019-08-22 1:24 ` Claudio Carvalho
2019-08-08 4:05 ` [PATCH v5 3/7] powerpc/powernv: Introduce FW_FEATURE_ULTRAVISOR Claudio Carvalho
2019-08-08 4:05 ` [PATCH v5 4/7] powerpc/mm: Use UV_WRITE_PATE ucall to register a PATE Claudio Carvalho
2019-08-14 11:33 ` Michael Ellerman [this message]
2019-08-21 0:04 ` Sukadev Bhattiprolu
2019-08-22 1:33 ` Claudio Carvalho
2019-08-08 4:05 ` [PATCH v5 5/7] powerpc/mm: Write to PTCR only if ultravisor disabled Claudio Carvalho
2019-08-14 12:04 ` Michael Ellerman
2019-08-08 4:05 ` [PATCH v5 6/7] powerpc/powernv: Access LDBAR " Claudio Carvalho
2019-08-08 4:05 ` [PATCH v5 7/7] powerpc/kvm: Use UV_RETURN ucall to return to ultravisor Claudio Carvalho
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871rxo7zif.fsf@concordia.ellerman.id.au \
--to=mpe@ellerman$(echo .)id.au \
--cc=andmike@linux$(echo .)ibm.com \
--cc=bauerman@linux$(echo .)ibm.com \
--cc=bharata@linux$(echo .)ibm.com \
--cc=cclaudio@linux$(echo .)ibm.com \
--cc=gdhh@linux$(echo .)ibm.com \
--cc=grimm@linux$(echo .)ibm.com \
--cc=grimm@linux$(echo .)vnet.ibm.com \
--cc=kvm-ppc@vger$(echo .)kernel.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=linuxram@us$(echo .)ibm.com \
--cc=maddy@linux$(echo .)vnet.ibm.com \
--cc=sukadev@linux$(echo .)vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox