LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v5 2/7] powerpc/kernel: Add ucall_norets() ultravisor call handler
From: Claudio Carvalho @ 2019-08-22  1:26 UTC (permalink / raw)
  To: Segher Boessenkool, Michael Ellerman
  Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
	Bharata B Rao, linuxppc-dev, Ryan Grimm, Sukadev Bhattiprolu,
	Guerney Hunt, Thiago Bauermann
In-Reply-To: <20190814183432.GG31406@gate.crashing.org>


On 8/14/19 3:34 PM, Segher Boessenkool wrote:
> On Wed, Aug 14, 2019 at 08:46:15PM +1000, Michael Ellerman wrote:
>> Claudio Carvalho <cclaudio@linux.ibm.com> writes:
>>> +_GLOBAL(ucall_norets)
>>> +EXPORT_SYMBOL_GPL(ucall_norets)
>>> +	mfcr	r0
>>> +	stw	r0,8(r1)
>>> +
>>> +	sc	2		/* Invoke the ultravisor */
>>> +
>>> +	lwz	r0,8(r1)
>>> +	mtcrf	0xff,r0
>>> +	blr			/* Return r3 = status */
>> Paulus points that we shouldn't need to save CR here. Our caller will
>> have already saved it if it needed to, and we don't use CR in this
>> function so we don't need to save it.
>>
>> That's assuming the Ultravisor follows the hcall ABI in which CR2-4 are
>> non-volatile (PAPR § 14.5.3).
> And assuming the ultravisor already clears (or sets, or whatever) all CR
> fields it does not want to leak the contents of (which it also should,
> of course).

Thanks Segher. We are working on that in the ultravisor source code.

Claudio.


>
>> I know plpar_hcall_norets() does save CR, but it shouldn't need to, that
>> seems to be historical. aka. no one knows why it does it but it always
>> has.
>
> Segher
>

^ permalink raw reply

* Re: [PATCH v5 4/7] powerpc/mm: Use UV_WRITE_PATE ucall to register a PATE
From: Claudio Carvalho @ 2019-08-22  1:33 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev
  Cc: Ryan Grimm, Madhavan Srinivasan, Michael Anderson, Ram Pai,
	kvm-ppc, Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu,
	Guerney Hunt, Thiago Bauermann
In-Reply-To: <871rxo7zif.fsf@concordia.ellerman.id.au>


On 8/14/19 8:33 AM, Michael Ellerman wrote:
> 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).

Yes, I agree. I applied that to next patchset version.


>
>>  {
>> -	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.

True. I dropped the two changes above in the next patchset version.

Thanks,
Claudio


>
>>  			     "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
>

^ permalink raw reply

* Re: [PATCH v6 1/7] kvmppc: Driver to manage pages of secure guest
From: Bharata B Rao @ 2019-08-22  3:29 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
	paulus, sukadev, linuxppc-dev, hch
In-Reply-To: <87ftlwwn9a.fsf@morokweng.localdomain>

On Tue, Aug 20, 2019 at 12:04:33AM -0300, Thiago Jung Bauermann wrote:
> 
> Hello Bharata,
> 
> I have just a couple of small comments.
> 
> Bharata B Rao <bharata@linux.ibm.com> writes:
> 
> > +/*
> > + * Get a free device PFN from the pool
> > + *
> > + * Called when a normal page is moved to secure memory (UV_PAGE_IN). Device
> > + * PFN will be used to keep track of the secure page on HV side.
> > + *
> > + * @rmap here is the slot in the rmap array that corresponds to @gpa.
> > + * Thus a non-zero rmap entry indicates that the corresonding guest
> 
> Typo: corresponding
> 
> > +static u64 kvmppc_get_secmem_size(void)
> > +{
> > +	struct device_node *np;
> > +	int i, len;
> > +	const __be32 *prop;
> > +	u64 size = 0;
> > +
> > +	np = of_find_node_by_path("/ibm,ultravisor/ibm,uv-firmware");
> > +	if (!np)
> > +		goto out;
> 
> I believe that in general we try to avoid hard-coding the path when a
> node is accessed and searched instead via its compatible property.

Thanks, will switch to of_find_compatible_node().

Regards,
Bharata.


^ permalink raw reply

* [PATCH v6 0/7] kvmppc: Paravirtualize KVM to support ultravisor
From: Claudio Carvalho @ 2019-08-22  3:48 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, Claudio Carvalho,
	kvm-ppc, Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu,
	Guerney Hunt, Thiago Bauermann

Protected Execution Facility (PEF) is an architectural change for POWER 9
that enables Secure Virtual Machines (SVMs). When enabled, PEF adds a new
higher privileged mode, called Ultravisor mode, to POWER architecture.
Along with the new mode there is new firmware called the Protected
Execution Ultravisor (or Ultravisor for short). Ultravisor mode is the
highest privileged mode in POWER architecture.

The Ultravisor calls allow the SVMs and Hypervisor to request services from
the Ultravisor such as accessing a register or memory region that can only
be accessed when running in Ultravisor-privileged mode.

This patch set adds support for Ultravisor calls and do some preparation
for running secure guests.

---
Changelog:
---

v5->v6:

- Patch "Documentation/powerpc: Ultravisor API"
  - Added chip version and PEF RFC number as suggested by mpe.
  - Applied Fabiano Rosas' suggestions.
  - Updated the commit message.

- Patch "powerpc/kernel: Add ucall_norets() ultravisor call handler"
  - Dropped CR save/restore from ucall_norets.
  - Updated the commit message.

- Patch "powerpc/mm: Use UV_WRITE_PATE ucall to register a PATE"
  - Replaced the "old_patb0" arg in flush_partition() by "bool radix".
  - Better documented mmu_partition_table_set_entry() to address mpe's
    questions about the copy of the partition table.
  - Updated the commit message.

- Patch "powerpc/mm: Write to PTCR only if ultravisor disabled"
  - Moved reg.h changes to ultravisor.h.
  - Replaced the try_set_ptcr macro by static inline set_ptcr_when_no_uv().
  - Updated the commit message.

- Rebased to powerpc/next

v4->v5:

- New patch "Documentation/powerpc: Ultravisor API"

- Patch "v4: KVM: PPC: Ultravisor: Add generic ultravisor call handler":
  - Made global the ucall_norets symbol without adding it to the TOC.
  - Implemented ucall_norets() rather than ucall().
  - Defined the ucall_norets in "asm/asm-prototypes.h" for symbol
    versioning.
  - Renamed to "powerpc/kernel: Add ucall_norets() ultravisor call
    handler".
  - Updated the commit message.

- Patch "v4: powerpc: Introduce FW_FEATURE_ULTRAVISOR":
  - Changed to scan for a node that is compatible with "ibm,ultravisor"
  - Renamed to "powerpc/powernv: Introduce FW_FEATURE_ULTRAVISOR".
  - Updated the commit message.

- Patch "v4: KVM: PPC: Ultravisor: Restrict flush of the partition tlb
  cache":
  - Merged into "v4: ... Use UV_WRITE_PATE ucall to register a PATE".

- Patch "v4: KVM: PPC: Ultravisor: Use UV_WRITE_PATE ucall to register a
  PATE":
  - Added back the missing "ptesync" instruction in flush_partition().
  - Updated source code comments for the partition table creation.
  - Factored out "powerpc/mm: Write to PTCR only if ultravisor disabled".
  - Cleaned up the code a bit.
  - Renamed to "powerpc/mm: Use UV_WRITE_PATE ucall to register a PATE".
  - Updated the commit message.

- Patch "v4: KVM: PPC: Ultravisor: Restrict LDBAR access":
  - Dropped the change that skips loading the IMC driver if ultravisor
    enabled because skiboot will remove the IMC devtree nodes if
    ultravisor enabled.
  - Dropped the BEGIN_{END_}FW_FTR_SECTION_NESTED in power8 code.
  - Renamed to "powerpc/powernv: Access LDBAR only if ultravisor
    disabled".
  - Updated the commit message.

- Patch "v4: KVM: PPC: Ultravisor: Enter a secure guest":
  - Openned "LOAD_REG_IMMEDIATE(r3, UV_RETURN)" to save instructions
  - Used R2, rather than R11, to pass synthesized interrupts in
    UV_RETURN ucall.
  - Dropped the change that preserves the MSR[S] bit in
    "kvmppc_msr_interrupt" because that is done by the ultravisor.
  - Hoisted up the load of R6 and R7 to before "bne ret_to_ultra".
  - Cleaned up the code a bit.
  - Renamed to "powerpc/kvm: Use UV_RETURN ucall to return to ultravisor".
  - Updated the commit message.

- Patch "v4: KVM: PPC: Ultravisor: Check for MSR_S during hv_reset_msr":
  - Dropped from the patch set because "kvm_arch->secure_guest" rather
    than MSR[S] is used to determine if we need to return to the
    ultravisor.

- Patch "v4: KVM: PPC: Ultravisor: Introduce the MSR_S bit":
  - Moved to the patch set "Secure Virtual Machine Enablement" posted by
    Thiago Bauermann. MSR[S] is no longer needed in this patch set.

- Rebased to powerpc/next

v3->v4:

- Patch "KVM: PPC: Ultravisor: Add PPC_UV config option":
  - Moved to the patchset "kvmppc: HMM driver to manage pages of secure
    guest" v5 that will be posted by Bharata Rao.

- Patch "powerpc: Introduce FW_FEATURE_ULTRAVISOR":
  - Changed to depend only on CONFIG_PPC_POWERNV.

- Patch "KVM: PPC: Ultravisor: Add generic ultravisor call handler":
  - Fixed whitespaces in ucall.S and in ultravisor-api.h.
  - Changed to depend only on CONFIG_PPC_POWERNV.
  - Changed the ucall wrapper to pass the ucall number in R3.

- Patch "KVM: PPC: Ultravisor: Use UV_WRITE_PATE ucall to register a
  PATE:
  - Changed to depend only on CONFIG_PPC_POWERNV.

- Patch "KVM: PPC: Ultravisor: Restrict LDBAR access":
  - Fixed comment in opal-imc.c to be "Disable IMC devices, when
    Ultravisor is enabled.
  - Fixed signed-off-by.

- Patch "KVM: PPC: Ultravisor: Enter a secure guest":
  - Changed the UV_RETURN assembly call to save the actual R3 in
    R0 for the ultravisor and pass the UV_RETURN call number in R3.

- Patch "KVM: PPC: Ultravisor: Check for MSR_S during hv_reset_msr":
  - Fixed commit message.

- Rebased to powerpc/next.

Claudio Carvalho (4):
  powerpc/kernel: Add ucall_norets() ultravisor call handler
  powerpc/powernv: Introduce FW_FEATURE_ULTRAVISOR
  powerpc/mm: Write to PTCR only if ultravisor disabled
  powerpc/powernv: Access LDBAR only if ultravisor disabled

Michael Anderson (1):
  powerpc/mm: Use UV_WRITE_PATE ucall to register a PATE

Sukadev Bhattiprolu (2):
  Documentation/powerpc: Ultravisor API
  powerpc/kvm: Use UV_RETURN ucall to return to ultravisor

 Documentation/powerpc/ultravisor.rst        | 1057 +++++++++++++++++++
 arch/powerpc/include/asm/asm-prototypes.h   |   11 +
 arch/powerpc/include/asm/firmware.h         |    5 +-
 arch/powerpc/include/asm/kvm_host.h         |    1 +
 arch/powerpc/include/asm/ultravisor-api.h   |   29 +
 arch/powerpc/include/asm/ultravisor.h       |   34 +
 arch/powerpc/kernel/Makefile                |    1 +
 arch/powerpc/kernel/asm-offsets.c           |    1 +
 arch/powerpc/kernel/prom.c                  |    4 +
 arch/powerpc/kernel/ucall.S                 |   14 +
 arch/powerpc/kvm/book3s_hv_rmhandlers.S     |   39 +-
 arch/powerpc/mm/book3s64/hash_utils.c       |    5 +-
 arch/powerpc/mm/book3s64/pgtable.c          |   52 +-
 arch/powerpc/mm/book3s64/radix_pgtable.c    |    8 +-
 arch/powerpc/platforms/powernv/Makefile     |    1 +
 arch/powerpc/platforms/powernv/idle.c       |    6 +-
 arch/powerpc/platforms/powernv/ultravisor.c |   24 +
 17 files changed, 1263 insertions(+), 29 deletions(-)
 create mode 100644 Documentation/powerpc/ultravisor.rst
 create mode 100644 arch/powerpc/include/asm/ultravisor-api.h
 create mode 100644 arch/powerpc/include/asm/ultravisor.h
 create mode 100644 arch/powerpc/kernel/ucall.S
 create mode 100644 arch/powerpc/platforms/powernv/ultravisor.c

-- 
2.20.1


^ permalink raw reply

* [PATCH v6 1/7] Documentation/powerpc: Ultravisor API
From: Claudio Carvalho @ 2019-08-22  3:48 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, Claudio Carvalho,
	kvm-ppc, Bharata B Rao, Ryan Grimm, Ram Pai, Sukadev Bhattiprolu,
	Sukadev Bhattiprolu, Guerney Hunt, Thiago Bauermann
In-Reply-To: <20190822034838.27876-1-cclaudio@linux.ibm.com>

From: Sukadev Bhattiprolu <sukadev@linux.ibm.com>

Protected Execution Facility (PEF) is an architectural change for
POWER 9 that enables Secure Virtual Machines (SVMs). When enabled,
PEF adds a new higher privileged mode, called Ultravisor mode, to POWER
architecture. Along with the new mode there is new firmware called the
Protected Execution Ultravisor (or Ultravisor for short).

POWER 9 DD2.3 chips (PVR=0x004e1203) or greater will be PEF-capable.

Attached documentation provides an overview of PEF and defines the API
for various interfaces that must be implemented in the Ultravisor
firmware as well as in the KVM Hypervisor.

Based on input from Mike Anderson, Thiago Bauermann, Claudio Carvalho,
Ben Herrenschmidt, Guerney Hunt, Paul Mackerras.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
Signed-off-by: Ram Pai <linuxram@linux.ibm.com>
Signed-off-by: Guerney Hunt <gdhh@linux.ibm.com>
Reviewed-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Reviewed-by: Michael Anderson <andmike@linux.ibm.com>
Reviewed-by: Thiago Bauermann <bauerman@linux.ibm.com>
Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
---
 Documentation/powerpc/ultravisor.rst | 1057 ++++++++++++++++++++++++++
 1 file changed, 1057 insertions(+)
 create mode 100644 Documentation/powerpc/ultravisor.rst

diff --git a/Documentation/powerpc/ultravisor.rst b/Documentation/powerpc/ultravisor.rst
new file mode 100644
index 000000000000..94a149f34ec3
--- /dev/null
+++ b/Documentation/powerpc/ultravisor.rst
@@ -0,0 +1,1057 @@
+.. SPDX-License-Identifier: GPL-2.0
+.. _ultravisor:
+
+============================
+Protected Execution Facility
+============================
+
+.. contents::
+    :depth: 3
+
+.. sectnum::
+    :depth: 3
+
+Protected Execution Facility
+############################
+
+    Protected Execution Facility (PEF) is an architectural change for
+    POWER 9 that enables Secure Virtual Machines (SVMs). DD2.3 chips
+    (PVR=0x004e1203) or greater will be PEF-capable. A new ISA release
+    will include the PEF RFC02487 changes.
+
+    When enabled, PEF adds a new higher privileged mode, called Ultravisor
+    mode, to POWER architecture. Along with the new mode there is new
+    firmware called the Protected Execution Ultravisor (or Ultravisor
+    for short). Ultravisor mode is the highest privileged mode in POWER
+    architecture.
+
+	+------------------+
+	| Privilege States |
+	+==================+
+	|  Problem         |
+	+------------------+
+	|  Supervisor      |
+	+------------------+
+	|  Hypervisor      |
+	+------------------+
+	|  Ultravisor      |
+	+------------------+
+
+    PEF protects SVMs from the hypervisor, privileged users, and other
+    VMs in the system. SVMs are protected while at rest and can only be
+    executed by an authorized machine. All virtual machines utilize
+    hypervisor services. The Ultravisor filters calls between the SVMs
+    and the hypervisor to assure that information does not accidentally
+    leak. All hypercalls except H_RANDOM are reflected to the hypervisor.
+    H_RANDOM is not reflected to prevent the hypervisor from influencing
+    random values in the SVM.
+
+    To support this there is a refactoring of the ownership of resources
+    in the CPU. Some of the resources which were previously hypervisor
+    privileged are now ultravisor privileged.
+
+Hardware
+========
+
+    The hardware changes include the following:
+
+    * There is a new bit in the MSR that determines whether the current
+      process is running in secure mode, MSR(S) bit 41. MSR(S)=1, process
+      is in secure mode, MSR(s)=0 process is in normal mode.
+
+    * The MSR(S) bit can only be set by the Ultravisor.
+
+    * HRFID cannot be used to set the MSR(S) bit. If the hypervisor needs
+      to return to a SVM it must use an ultracall. It can determine if
+      the VM it is returning to is secure.
+
+    * There is a new Ultravisor privileged register, SMFCTRL, which has an
+      enable/disable bit SMFCTRL(E).
+
+    * The privilege of a process is now determined by three MSR bits,
+      MSR(S, HV, PR). In each of the tables below the modes are listed
+      from least privilege to highest privilege. The higher privilege
+      modes can access all the resources of the lower privilege modes.
+
+      **Secure Mode MSR Settings**
+
+      +---+---+---+---------------+
+      | S | HV| PR|Privilege      |
+      +===+===+===+===============+
+      | 1 | 0 | 1 | Problem       |
+      +---+---+---+---------------+
+      | 1 | 0 | 0 | Privileged(OS)|
+      +---+---+---+---------------+
+      | 1 | 1 | 0 | Ultravisor    |
+      +---+---+---+---------------+
+      | 1 | 1 | 1 | Reserved      |
+      +---+---+---+---------------+
+
+      **Normal Mode MSR Settings**
+
+      +---+---+---+---------------+
+      | S | HV| PR|Privilege      |
+      +===+===+===+===============+
+      | 0 | 0 | 1 | Problem       |
+      +---+---+---+---------------+
+      | 0 | 0 | 0 | Privileged(OS)|
+      +---+---+---+---------------+
+      | 0 | 1 | 0 | Hypervisor    |
+      +---+---+---+---------------+
+      | 0 | 1 | 1 | Problem (Host)|
+      +---+---+---+---------------+
+
+    * Memory is partitioned into secure and normal memory. Only processes
+      that are running in secure mode can access secure memory.
+
+    * The hardware does not allow anything that is not running secure to
+      access secure memory. This means that the Hypervisor cannot access
+      the memory of the SVM without using an ultracall (asking the
+      Ultravisor). The Ultravisor will only allow the hypervisor to see
+      the SVM memory encrypted.
+
+    * I/O systems are not allowed to directly address secure memory. This
+      limits the SVMs to virtual I/O only.
+
+    * The architecture allows the SVM to share pages of memory with the
+      hypervisor that are not protected with encryption. However, this
+      sharing must be initiated by the SVM.
+
+    * When a process is running in secure mode all hypercalls
+      (syscall lev=1) go to the Ultravisor.
+
+    * When a process is in secure mode all interrupts go to the
+      Ultravisor.
+
+    * The following resources have become Ultravisor privileged and
+      require an Ultravisor interface to manipulate:
+
+      * Processor configurations registers (SCOMs).
+
+      * Stop state information.
+
+      * The debug registers CIABR, DAWR, and DAWRX when SMFCTRL(D) is set.
+        If SMFCTRL(D) is not set they do not work in secure mode. When set,
+        reading and writing requires an Ultravisor call, otherwise that
+        will cause a Hypervisor Emulation Assistance interrupt.
+
+      * PTCR and partition table entries (partition table is in secure
+        memory). An attempt to write to PTCR will cause a Hypervisor
+        Emulation Assitance interrupt.
+
+      * LDBAR (LD Base Address Register) and IMC (In-Memory Collection)
+        non-architected registers. An attempt to write to them will cause a
+        Hypervisor Emulation Assistance interrupt.
+
+      * Paging for an SVM, sharing of memory with Hypervisor for an SVM.
+        (Including Virtual Processor Area (VPA) and virtual I/O).
+
+
+Software/Microcode
+==================
+
+    The software changes include:
+
+    * SVMs are created from normal VM using (open source) tooling supplied
+      by IBM.
+
+    * All SVMs start as normal VMs and utilize an ultracall, UV_ESM
+      (Enter Secure Mode), to make the transition.
+
+    * When the UV_ESM ultracall is made the Ultravisor copies the VM into
+      secure memory, decrypts the verification information, and checks the
+      integrity of the SVM. If the integrity check passes the Ultravisor
+      passes control in secure mode.
+
+    * The verification information includes the pass phrase for the
+      encrypted disk associated with the SVM. This pass phrase is given
+      to the SVM when requested.
+
+    * The Ultravisor is not involved in protecting the encrypted disk of
+      the SVM while at rest.
+
+    * For external interrupts the Ultravisor saves the state of the SVM,
+      and reflects the interrupt to the hypervisor for processing.
+      For hypercalls, the Ultravisor inserts neutral state into all
+      registers not needed for the hypercall then reflects the call to
+      the hypervisor for processing. The H_RANDOM hypercall is performed
+      by the Ultravisor and not reflected.
+
+    * For virtual I/O to work bounce buffering must be done.
+
+    * The Ultravisor uses AES (IAPM) for protection of SVM memory. IAPM
+      is a mode of AES that provides integrity and secrecy concurrently.
+
+    * The movement of data between normal and secure pages is coordinated
+      with the Ultravisor by a new HMM plug-in in the Hypervisor.
+
+    The Ultravisor offers new services to the hypervisor and SVMs. These
+    are accessed through ultracalls.
+
+Terminology
+===========
+
+    * Hypercalls: special system calls used to request services from
+      Hypervisor.
+
+    * Normal memory: Memory that is accessible to Hypervisor.
+
+    * Normal page: Page backed by normal memory and available to
+      Hypervisor.
+
+    * Shared page: A page backed by normal memory and available to both
+      the Hypervisor/QEMU and the SVM (i.e page has mappings in SVM and
+      Hypervisor/QEMU).
+
+    * Secure memory: Memory that is accessible only to Ultravisor and
+      SVMs.
+
+    * Secure page: Page backed by secure memory and only available to
+      Ultravisor and SVM.
+
+    * SVM: Secure Virtual Machine.
+
+    * Ultracalls: special system calls used to request services from
+      Ultravisor.
+
+
+Ultravisor calls API
+####################
+
+    This section describes Ultravisor calls (ultracalls) needed to
+    support Secure Virtual Machines (SVM)s and Paravirtualized KVM. The
+    ultracalls allow the SVMs and Hypervisor to request services from the
+    Ultravisor such as accessing a register or memory region that can only
+    be accessed when running in Ultravisor-privileged mode.
+
+    The specific service needed from an ultracall is specified in register
+    R3 (the first parameter to the ultracall). Other parameters to the
+    ultracall, if any, are specified in registers R4 through R12.
+
+    Return value of all ultracalls is in register R3. Other output values
+    from the ultracall, if any, are returned in registers R4 through R12.
+    The only exception to this register usage is the ``UV_RETURN``
+    ultracall described below.
+
+    Each ultracall returns specific error codes, applicable in the context
+    of the ultracall. However, like with the PowerPC Architecture Platform
+    Reference (PAPR), if no specific error code is defined for a
+    particular situation, then the ultracall will fallback to an erroneous
+    parameter-position based code. i.e U_PARAMETER, U_P2, U_P3 etc
+    depending on the ultracall parameter that may have caused the error.
+
+    Some ultracalls involve transferring a page of data between Ultravisor
+    and Hypervisor.  Secure pages that are transferred from secure memory
+    to normal memory may be encrypted using dynamically generated keys.
+    When the secure pages are transferred back to secure memory, they may
+    be decrypted using the same dynamically generated keys. Generation and
+    management of these keys will be covered in a separate document.
+
+    For now this only covers ultracalls currently implemented and being
+    used by Hypervisor and SVMs but others can be added here when it
+    makes sense.
+
+    The full specification for all hypercalls/ultracalls will eventually
+    be made available in the public/OpenPower version of the PAPR
+    specification.
+
+    **Note**
+
+    If PEF is not enabled, the ultracalls will be redirected to the
+    Hypervisor which must handle/fail the calls.
+
+Ultracalls used by Hypervisor
+=============================
+
+    This section describes the virtual memory management ultracalls used
+    by the Hypervisor to manage SVMs.
+
+UV_PAGE_OUT
+-----------
+
+    Encrypt and move the contents of a page from secure memory to normal
+    memory.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t ultracall(const uint64_t UV_PAGE_OUT,
+		uint16_t lpid,		/* LPAR ID */
+		uint64_t dest_ra,	/* real address of destination page */
+		uint64_t src_gpa,	/* source guest-physical-address */
+		uint8_t  flags,		/* flags */
+		uint64_t order)		/* page size order */
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* U_SUCCESS	on success.
+	* U_PARAMETER	if ``lpid`` is invalid.
+	* U_P2 		if ``dest_ra`` is invalid.
+	* U_P3		if the ``src_gpa`` address is invalid.
+	* U_P4		if any bit in the ``flags`` is unrecognized
+	* U_P5		if the ``order`` parameter is unsupported.
+	* U_FUNCTION	if functionality is not supported.
+	* U_BUSY	if page cannot be currently paged-out.
+
+Description
+~~~~~~~~~~~
+
+    Encrypt the contents of a secure-page and make it available to
+    Hypervisor in a normal page.
+
+    By default, the source page is unmapped from the SVM's partition-
+    scoped page table. But the Hypervisor can provide a hint to the
+    Ultravisor to retain the page mapping by setting the ``UV_SNAPSHOT``
+    flag in ``flags`` parameter.
+
+    If the source page is already a shared page the call returns
+    U_SUCCESS, without doing anything.
+
+Use cases
+~~~~~~~~~
+
+    #. QEMU attempts to access an address belonging to the SVM but the
+       page frame for that address is not mapped into QEMU's address
+       space. In this case, the Hypervisor will allocate a page frame,
+       map it into QEMU's address space and issue the ``UV_PAGE_OUT``
+       call to retrieve the encrypted contents of the page.
+
+    #. When Ultravisor runs low on secure memory and it needs to page-out
+       an LRU page. In this case, Ultravisor will issue the
+       ``H_SVM_PAGE_OUT`` hypercall to the Hypervisor. The Hypervisor will
+       then allocate a normal page and issue the ``UV_PAGE_OUT`` ultracall
+       and the Ultravisor will encrypt and move the contents of the secure
+       page into the normal page.
+
+    #. When Hypervisor accesses SVM data, the Hypervisor requests the
+       Ultravisor to transfer the corresponding page into a insecure page,
+       which the Hypervisor can access. The data in the normal page will
+       be encrypted though.
+
+UV_PAGE_IN
+----------
+
+    Move the contents of a page from normal memory to secure memory.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t ultracall(const uint64_t UV_PAGE_IN,
+		uint16_t lpid,		/* the LPAR ID */
+		uint64_t src_ra,	/* source real address of page */
+		uint64_t dest_gpa,	/* destination guest physical address */
+		uint64_t flags,		/* flags */
+		uint64_t order)		/* page size order */
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* U_SUCCESS	on success.
+	* U_BUSY	if page cannot be currently paged-in.
+	* U_FUNCTION	if functionality is not supported
+	* U_PARAMETER	if ``lpid`` is invalid.
+	* U_P2 		if ``src_ra`` is invalid.
+	* U_P3		if the ``dest_gpa`` address is invalid.
+	* U_P4		if any bit in the ``flags`` is unrecognized
+	* U_P5		if the ``order`` parameter is unsupported.
+
+Description
+~~~~~~~~~~~
+
+    Move the contents of the page identified by ``src_ra`` from normal
+    memory to secure memory and map it to the guest physical address
+    ``dest_gpa``.
+
+    If `dest_gpa` refers to a shared address, map the page into the
+    partition-scoped page-table of the SVM.  If `dest_gpa` is not shared,
+    copy the contents of the page into the corresponding secure page.
+    Depending on the context, decrypt the page before being copied.
+
+    The caller provides the attributes of the page through the ``flags``
+    parameter. Valid values for ``flags`` are:
+
+	* CACHE_INHIBITED
+	* CACHE_ENABLED
+	* WRITE_PROTECTION
+
+    The Hypervisor must pin the page in memory before making
+    ``UV_PAGE_IN`` ultracall.
+
+Use cases
+~~~~~~~~~
+
+    #. When a normal VM switches to secure mode, all its pages residing
+       in normal memory, are moved into secure memory.
+
+    #. When an SVM requests to share a page with Hypervisor the Hypervisor
+       allocates a page and informs the Ultravisor.
+
+    #. When an SVM accesses a secure page that has been paged-out,
+       Ultravisor invokes the Hypervisor to locate the page. After
+       locating the page, the Hypervisor uses UV_PAGE_IN to make the
+       page available to Ultravisor.
+
+UV_PAGE_INVAL
+-------------
+
+    Invalidate the Ultravisor mapping of a page.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t ultracall(const uint64_t UV_PAGE_INVAL,
+		uint16_t lpid,		/* the LPAR ID */
+		uint64_t guest_pa,	/* destination guest-physical-address */
+		uint64_t order)		/* page size order */
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* U_SUCCESS	on success.
+	* U_PARAMETER	if ``lpid`` is invalid.
+	* U_P2 		if ``guest_pa`` is invalid (or corresponds to a secure
+                        page mapping).
+	* U_P3		if the ``order`` is invalid.
+	* U_FUNCTION	if functionality is not supported.
+	* U_BUSY	if page cannot be currently invalidated.
+
+Description
+~~~~~~~~~~~
+
+    This ultracall informs Ultravisor that the page mapping in Hypervisor
+    corresponding to the given guest physical address has been invalidated
+    and that the Ultravisor should not access the page. If the specified
+    ``guest_pa`` corresponds to a secure page, Ultravisor will ignore the
+    attempt to invalidate the page and return U_P2.
+
+Use cases
+~~~~~~~~~
+
+    #. When a shared page is unmapped from the QEMU's page table, possibly
+       because it is paged-out to disk, Ultravisor needs to know that the
+       page should not be accessed from its side too.
+
+
+UV_WRITE_PATE
+-------------
+
+    Validate and write the partition table entry (PATE) for a given
+    partition.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t ultracall(const uint64_t UV_WRITE_PATE,
+		uint32_t lpid,		/* the LPAR ID */
+		uint64_t dw0		/* the first double word to write */
+		uint64_t dw1)		/* the second double word to write */
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* U_SUCCESS	on success.
+	* U_BUSY	if PATE cannot be currently written to.
+	* U_FUNCTION	if functionality is not supported.
+	* U_PARAMETER	if ``lpid`` is invalid.
+	* U_P2 		if ``dw0`` is invalid.
+	* U_P3		if the ``dw1`` address is invalid.
+	* U_PERMISSION	if the Hypervisor is attempting to change the PATE
+			of a secure virtual machine or if called from a
+			context other than Hypervisor.
+
+Description
+~~~~~~~~~~~
+
+    Validate and write a LPID and its partition-table-entry for the given
+    LPID.  If the LPID is already allocated and initialized, this call
+    results in changing the partition table entry.
+
+Use cases
+~~~~~~~~~
+
+    #. The Partition table resides in Secure memory and its entries,
+       called PATE (Partition Table Entries), point to the partition-
+       scoped page tables for the Hypervisor as well as each of the
+       virtual machines (both secure and normal). The Hypervisor
+       operates in partition 0 and its partition-scoped page tables
+       reside in normal memory.
+
+    #. This ultracall allows the Hypervisor to register the partition-
+       scoped and process-scoped page table entries for the Hypervisor
+       and other partitions (virtual machines) with the Ultravisor.
+
+    #. If the value of the PATE for an existing partition (VM) changes,
+       the TLB cache for the partition is flushed.
+
+    #. The Hypervisor is responsible for allocating LPID. The LPID and
+       its PATE entry are registered together.  The Hypervisor manages
+       the PATE entries for a normal VM and can change the PATE entry
+       anytime. Ultravisor manages the PATE entries for an SVM and
+       Hypervisor is not allowed to modify them.
+
+UV_RETURN
+---------
+
+    Return control from the Hypervisor back to the Ultravisor after
+    processing an hypercall or interrupt that was forwarded (aka
+    *reflected*) to the Hypervisor.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t ultracall(const uint64_t UV_RETURN)
+
+Return values
+~~~~~~~~~~~~~
+
+     This call never returns to Hypervisor on success.  It returns
+     U_INVALID if ultracall is not made from a Hypervisor context.
+
+Description
+~~~~~~~~~~~
+
+    When an SVM makes an hypercall or incurs some other exception, the
+    Ultravisor usually forwards (aka *reflects*) the exceptions to the
+    Hypervisor.  After processing the exception, Hypervisor uses the
+    ``UV_RETURN`` ultracall to return control back to the SVM.
+
+    The expected register state on entry to this ultracall is:
+
+    * Non-volatile registers are restored to their original values.
+    * If returning from an hypercall, register R0 contains the return
+      value (**unlike other ultracalls**) and, registers R4 through R12
+      contain any output values of the hypercall.
+    * R3 contains the ultracall number, i.e UV_RETURN.
+    * If returning with a synthesized interrupt, R2 contains the
+      synthesized interrupt number.
+
+Use cases
+~~~~~~~~~
+
+    #. Ultravisor relies on the Hypervisor to provide several services to
+       the SVM such as processing hypercall and other exceptions. After
+       processing the exception, Hypervisor uses UV_RETURN to return
+       control back to the Ultravisor.
+
+    #. Hypervisor has to use this ultracall to return control to the SVM.
+
+
+UV_REGISTER_MEM_SLOT
+--------------------
+
+    Register an SVM address-range with specified properties.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t ultracall(const uint64_t UV_REGISTER_MEM_SLOT,
+		uint64_t lpid,		/* LPAR ID of the SVM */
+		uint64_t start_gpa,	/* start guest physical address */
+		uint64_t size,		/* size of address range in bytes */
+		uint64_t flags		/* reserved for future expansion */
+		uint16_t slotid)	/* slot identifier */
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* U_SUCCESS	on success.
+	* U_PARAMETER	if ``lpid`` is invalid.
+	* U_P2 		if ``start_gpa`` is invalid.
+	* U_P3		if ``size`` is invalid.
+	* U_P4		if any bit in the ``flags`` is unrecognized.
+	* U_P5		if the ``slotid`` parameter is unsupported.
+	* U_PERMISSION	if called from context other than Hypervisor.
+	* U_FUNCTION	if functionality is not supported.
+
+
+Description
+~~~~~~~~~~~
+
+    Register a memory range for an SVM.  The memory range starts at the
+    guest physical address ``start_gpa`` and is ``size`` bytes long.
+
+Use cases
+~~~~~~~~~
+
+
+    #. When a virtual machine goes secure, all the memory slots managed by
+       the Hypervisor move into secure memory. The Hypervisor iterates
+       through each of memory slots, and registers the slot with
+       Ultravisor.  Hypervisor may discard some slots such as those used
+       for firmware (SLOF).
+
+    #. When new memory is hot-plugged, a new memory slot gets registered.
+
+
+UV_UNREGISTER_MEM_SLOT
+----------------------
+
+    Unregister an SVM address-range that was previously registered using
+    UV_REGISTER_MEM_SLOT.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t ultracall(const uint64_t UV_UNREGISTER_MEM_SLOT,
+		uint64_t lpid,		/* LPAR ID of the SVM */
+		uint64_t slotid)	/* reservation slotid */
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* U_SUCCESS	on success.
+	* U_FUNCTION	if functionality is not supported.
+	* U_PARAMETER	if ``lpid`` is invalid.
+	* U_P2 		if ``slotid`` is invalid.
+	* U_PERMISSION	if called from context other than Hypervisor.
+
+Description
+~~~~~~~~~~~
+
+    Release the memory slot identified by ``slotid`` and free any
+    resources allocated towards the reservation.
+
+Use cases
+~~~~~~~~~
+
+    #. Memory hot-remove.
+
+
+UV_SVM_TERMINATE
+----------------
+
+    Terminate an SVM and release its resources.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t ultracall(const uint64_t UV_SVM_TERMINATE,
+		uint64_t lpid,		/* LPAR ID of the SVM */)
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* U_SUCCESS	on success.
+	* U_FUNCTION	if functionality is not supported.
+	* U_PARAMETER	if ``lpid`` is invalid.
+	* U_INVALID	if VM is not secure.
+	* U_PERMISSION  if not called from a Hypervisor context.
+
+Description
+~~~~~~~~~~~
+
+    Terminate an SVM and release all its resources.
+
+Use cases
+~~~~~~~~~
+
+    #. Called by Hypervisor when terminating an SVM.
+
+
+Ultracalls used by SVM
+======================
+
+UV_SHARE_PAGE
+-------------
+
+    Share a set of guest physical pages with the Hypervisor.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t ultracall(const uint64_t UV_SHARE_PAGE,
+		uint64_t gfn,	/* guest page frame number */
+		uint64_t num)	/* number of pages of size PAGE_SIZE */
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* U_SUCCESS	on success.
+	* U_FUNCTION	if functionality is not supported.
+	* U_INVALID	if the VM is not secure.
+	* U_PARAMETER	if ``gfn`` is invalid.
+	* U_P2 		if ``num`` is invalid.
+
+Description
+~~~~~~~~~~~
+
+    Share the ``num`` pages starting at guest physical frame number ``gfn``
+    with the Hypervisor. Assume page size is PAGE_SIZE bytes. Zero the
+    pages before returning.
+
+    If the address is already backed by a secure page, unmap the page and
+    back it with an insecure page, with the help of the Hypervisor. If it
+    is not backed by any page yet, mark the PTE as insecure and back it
+    with an insecure page when the address is accessed. If it is already
+    backed by an insecure page, zero the page and return.
+
+Use cases
+~~~~~~~~~
+
+    #. The Hypervisor cannot access the SVM pages since they are backed by
+       secure pages. Hence an SVM must explicitly request Ultravisor for
+       pages it can share with Hypervisor.
+
+    #. Shared pages are needed to support virtio and Virtual Processor Area
+       (VPA) in SVMs.
+
+
+UV_UNSHARE_PAGE
+---------------
+
+    Restore a shared SVM page to its initial state.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t ultracall(const uint64_t UV_UNSHARE_PAGE,
+		uint64_t gfn,	/* guest page frame number */
+		uint73 num)	/* number of pages of size PAGE_SIZE*/
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* U_SUCCESS	on success.
+	* U_FUNCTION	if functionality is not supported.
+	* U_INVALID	if VM is not secure.
+	* U_PARAMETER	if ``gfn`` is invalid.
+	* U_P2 		if ``num`` is invalid.
+
+Description
+~~~~~~~~~~~
+
+    Stop sharing ``num`` pages starting at ``gfn`` with the Hypervisor.
+    Assume that the page size is PAGE_SIZE. Zero the pages before
+    returning.
+
+    If the address is already backed by an insecure page, unmap the page
+    and back it with a secure page. Inform the Hypervisor to release
+    reference to its shared page. If the address is not backed by a page
+    yet, mark the PTE as secure and back it with a secure page when that
+    address is accessed. If it is already backed by an secure page zero
+    the page and return.
+
+Use cases
+~~~~~~~~~
+
+    #. The SVM may decide to unshare a page from the Hypervisor.
+
+
+UV_UNSHARE_ALL_PAGES
+--------------------
+
+    Unshare all pages the SVM has shared with Hypervisor.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t ultracall(const uint64_t UV_UNSHARE_ALL_PAGES)
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* U_SUCCESS	on success.
+	* U_FUNCTION	if functionality is not supported.
+	* U_INVAL	if VM is not secure.
+
+Description
+~~~~~~~~~~~
+
+    Unshare all shared pages from the Hypervisor. All unshared pages are
+    zeroed on return. Only pages explicitly shared by the SVM with the
+    Hypervisor (using UV_SHARE_PAGE ultracall) are unshared. Ultravisor
+    may internally share some pages with the Hypervisor without explicit
+    request from the SVM.  These pages will not be unshared by this
+    ultracall.
+
+Use cases
+~~~~~~~~~
+
+    #. This call is needed when ``kexec`` is used to boot a different
+       kernel. It may also be needed during SVM reset.
+
+UV_ESM
+------
+
+    Secure the virtual machine (*enter secure mode*).
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t ultracall(const uint64_t UV_ESM,
+		uint64_t esm_blob_addr,	/* location of the ESM blob */
+		unint64_t fdt)		/* Flattened device tree */
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* U_SUCCESS	on success (including if VM is already secure).
+	* U_FUNCTION	if functionality is not supported.
+	* U_INVALID	if VM is not secure.
+	* U_PARAMETER	if ``esm_blob_addr`` is invalid.
+	* U_P2 		if ``fdt`` is invalid.
+	* U_PERMISSION	if any integrity checks fail.
+	* U_RETRY	insufficient memory to create SVM.
+	* U_NO_KEY	symmetric key unavailable.
+
+Description
+~~~~~~~~~~~
+
+    Secure the virtual machine. On successful completion, return
+    control to the virtual machine at the address specified in the
+    ESM blob.
+
+Use cases
+~~~~~~~~~
+
+    #. A normal virtual machine can choose to switch to a secure mode.
+
+Hypervisor Calls API
+####################
+
+    This document describes the Hypervisor calls (hypercalls) that are
+    needed to support the Ultravisor. Hypercalls are services provided by
+    the Hypervisor to virtual machines and Ultravisor.
+
+    Register usage for these hypercalls is identical to that of the other
+    hypercalls defined in the Power Architecture Platform Reference (PAPR)
+    document.  i.e on input, register R3 identifies the specific service
+    that is being requested and registers R4 through R11 contain
+    additional parameters to the hypercall, if any. On output, register
+    R3 contains the return value and registers R4 through R9 contain any
+    other output values from the hypercall.
+
+    This document only covers hypercalls currently implemented/planned
+    for Ultravisor usage but others can be added here when it makes sense.
+
+    The full specification for all hypercalls/ultracalls will eventually
+    be made available in the public/OpenPower version of the PAPR
+    specification.
+
+Hypervisor calls to support Ultravisor
+======================================
+
+    Following are the set of hypercalls needed to support Ultravisor.
+
+H_SVM_INIT_START
+----------------
+
+    Begin the process of converting a normal virtual machine into an SVM.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t hypercall(const uint64_t H_SVM_INIT_START)
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* H_SUCCESS	 on success.
+
+Description
+~~~~~~~~~~~
+
+    Initiate the process of securing a virtual machine. This involves
+    coordinating with the Ultravisor, using ultracalls, to allocate
+    resources in the Ultravisor for the new SVM, transferring the VM's
+    pages from normal to secure memory etc. When the process is
+    completed, Ultravisor issues the H_SVM_INIT_DONE hypercall.
+
+Use cases
+~~~~~~~~~
+
+     #. Ultravisor uses this hypercall to inform Hypervisor that a VM
+        has initiated the process of switching to secure mode.
+
+
+H_SVM_INIT_DONE
+---------------
+
+    Complete the process of securing an SVM.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t hypercall(const uint64_t H_SVM_INIT_DONE)
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* H_SUCCESS 		on success.
+	* H_UNSUPPORTED		if called from the wrong context (e.g.
+				from an SVM or before an H_SVM_INIT_START
+				hypercall).
+
+Description
+~~~~~~~~~~~
+
+    Complete the process of securing a virtual machine. This call must
+    be made after a prior call to ``H_SVM_INIT_START`` hypercall.
+
+Use cases
+~~~~~~~~~
+
+    On successfully securing a virtual machine, the Ultravisor informs
+    Hypervisor about it. Hypervisor can use this call to finish setting
+    up its internal state for this virtual machine.
+
+
+H_SVM_PAGE_IN
+-------------
+
+    Move the contents of a page from normal memory to secure memory.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t hypercall(const uint64_t H_SVM_PAGE_IN,
+		uint64_t guest_pa,	/* guest-physical-address */
+		uint64_t flags,		/* flags */
+		uint64_t order)		/* page size order */
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* H_SUCCESS	on success.
+	* H_PARAMETER	if ``guest_pa`` is invalid.
+	* H_P2		if ``flags`` is invalid.
+	* H_P3		if ``order`` of page is invalid.
+
+Description
+~~~~~~~~~~~
+
+    Retrieve the content of the page, belonging to the VM at the specified
+    guest physical address.
+
+    Only valid value(s) in ``flags`` are:
+
+        * H_PAGE_IN_SHARED which indicates that the page is to be shared
+	  with the Ultravisor.
+
+	* H_PAGE_IN_NONSHARED indicates that the UV is not anymore
+          interested in the page. Applicable if the page is a shared page.
+
+    The ``order`` parameter must correspond to the configured page size.
+
+Use cases
+~~~~~~~~~
+
+    #. When a normal VM becomes a secure VM (using the UV_ESM ultracall),
+       the Ultravisor uses this hypercall to move contents of each page of
+       the VM from normal memory to secure memory.
+
+    #. Ultravisor uses this hypercall to ask Hypervisor to provide a page
+       in normal memory that can be shared between the SVM and Hypervisor.
+
+    #. Ultravisor uses this hypercall to page-in a paged-out page. This
+       can happen when the SVM touches a paged-out page.
+
+    #. If SVM wants to disable sharing of pages with Hypervisor, it can
+       inform Ultravisor to do so. Ultravisor will then use this hypercall
+       and inform Hypervisor that it has released access to the normal
+       page.
+
+H_SVM_PAGE_OUT
+---------------
+
+    Move the contents of the page to normal memory.
+
+Syntax
+~~~~~~
+
+.. code-block:: c
+
+	uint64_t hypercall(const uint64_t H_SVM_PAGE_OUT,
+		uint64_t guest_pa,	/* guest-physical-address */
+		uint64_t flags,		/* flags (currently none) */
+		uint64_t order)		/* page size order */
+
+Return values
+~~~~~~~~~~~~~
+
+    One of the following values:
+
+	* H_SUCCESS	on success.
+	* H_PARAMETER	if ``guest_pa`` is invalid.
+	* H_P2		if ``flags`` is invalid.
+	* H_P3		if ``order`` is invalid.
+
+Description
+~~~~~~~~~~~
+
+    Move the contents of the page identified by ``guest_pa`` to normal
+    memory.
+
+    Currently ``flags`` is unused and must be set to 0. The ``order``
+    parameter must correspond to the configured page size.
+
+Use cases
+~~~~~~~~~
+
+    #. If Ultravisor is running low on secure pages, it can move the
+       contents of some secure pages, into normal pages using this
+       hypercall. The content will be encrypted.
+
+References
+##########
+
+.. [1] `Supporting Protected Computing on IBM Power Architecture <https://developer.ibm.com/articles/l-support-protected-computing/>`_
-- 
2.20.1


^ permalink raw reply related

* [PATCH v6 2/7] powerpc/kernel: Add ucall_norets() ultravisor call handler
From: Claudio Carvalho @ 2019-08-22  3:48 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, Claudio Carvalho,
	kvm-ppc, Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu,
	Guerney Hunt, Thiago Bauermann
In-Reply-To: <20190822034838.27876-1-cclaudio@linux.ibm.com>

The ultracalls (ucalls for short) allow the Secure Virtual Machines
(SVM)s and hypervisor to request services from the ultravisor such as
accessing a register or memory region that can only be accessed when
running in ultravisor-privileged mode.

This patch adds the ucall_norets() ultravisor call handler.

The specific service needed from an ucall is specified in register
R3 (the first parameter to the ucall). Other parameters to the
ucall, if any, are specified in registers R4 through R12.

Return value of all ucalls is in register R3. Other output values
from the ucall, if any, are returned in registers R4 through R12.

Each ucall returns specific error codes, applicable in the context
of the ucall. However, like with the PowerPC Architecture Platform
Reference (PAPR), if no specific error code is defined for a particular
situation, then the ucall will fallback to an erroneous
parameter-position based code. i.e U_PARAMETER, U_P2, U_P3 etc depending
on the ucall parameter that may have caused the error.

Every host kernel (powernv) needs to be able to do ucalls in case it
ends up being run in a machine with ultravisor enabled. Otherwise, the
kernel may crash early in boot trying to access ultravisor resources,
for instance, trying to set the partition table entry 0. Secure guests
also need to be able to do ucalls and its kernel may not have
CONFIG_PPC_POWERNV=y. For that reason, the ucall.S file is placed under
arch/powerpc/kernel.

If ultravisor is not enabled, the ucalls will be redirected to the
hypervisor which must handle/fail the call.

Thanks to inputs from Ram Pai and Michael Anderson.

Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>

---
Ultravisor call support for secure guests is being proposed as part of
the patchset "Secure Virtual Machine Enablement" posted by Thiago
Bauermann.
---
 arch/powerpc/include/asm/asm-prototypes.h | 11 +++++++++++
 arch/powerpc/include/asm/ultravisor-api.h | 23 +++++++++++++++++++++++
 arch/powerpc/kernel/Makefile              |  1 +
 arch/powerpc/kernel/ucall.S               | 14 ++++++++++++++
 4 files changed, 49 insertions(+)
 create mode 100644 arch/powerpc/include/asm/ultravisor-api.h
 create mode 100644 arch/powerpc/kernel/ucall.S

diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index ec1c97a8e8cb..e698f48cbc6d 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -15,6 +15,7 @@
 #include <asm/epapr_hcalls.h>
 #include <asm/dcr.h>
 #include <asm/mmu_context.h>
+#include <asm/ultravisor-api.h>
 
 #include <uapi/asm/ucontext.h>
 
@@ -34,6 +35,16 @@ extern struct static_key hcall_tracepoint_key;
 void __trace_hcall_entry(unsigned long opcode, unsigned long *args);
 void __trace_hcall_exit(long opcode, long retval, unsigned long *retbuf);
 
+/* Ultravisor */
+#ifdef CONFIG_PPC_POWERNV
+long ucall_norets(unsigned long opcode, ...);
+#else
+static inline long ucall_norets(unsigned long opcode, ...)
+{
+	return U_NOT_AVAILABLE;
+}
+#endif
+
 /* OPAL */
 int64_t __opal_call(int64_t a0, int64_t a1, int64_t a2, int64_t a3,
 		    int64_t a4, int64_t a5, int64_t a6, int64_t a7,
diff --git a/arch/powerpc/include/asm/ultravisor-api.h b/arch/powerpc/include/asm/ultravisor-api.h
new file mode 100644
index 000000000000..88ffa78f9d61
--- /dev/null
+++ b/arch/powerpc/include/asm/ultravisor-api.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Ultravisor API.
+ *
+ * Copyright 2019, IBM Corporation.
+ *
+ */
+#ifndef _ASM_POWERPC_ULTRAVISOR_API_H
+#define _ASM_POWERPC_ULTRAVISOR_API_H
+
+#include <asm/hvcall.h>
+
+/* Return codes */
+#define U_FUNCTION		H_FUNCTION
+#define U_NOT_AVAILABLE		H_NOT_AVAILABLE
+#define U_P2			H_P2
+#define U_P3			H_P3
+#define U_P4			H_P4
+#define U_P5			H_P5
+#define U_PARAMETER		H_PARAMETER
+#define U_SUCCESS		H_SUCCESS
+
+#endif /* _ASM_POWERPC_ULTRAVISOR_API_H */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index ea0c69236789..c6c4ea240b2a 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -156,6 +156,7 @@ endif
 
 obj-$(CONFIG_EPAPR_PARAVIRT)	+= epapr_paravirt.o epapr_hcalls.o
 obj-$(CONFIG_KVM_GUEST)		+= kvm.o kvm_emul.o
+obj-$(CONFIG_PPC_POWERNV)	+= ucall.o
 
 # Disable GCOV, KCOV & sanitizers in odd or sensitive code
 GCOV_PROFILE_prom_init.o := n
diff --git a/arch/powerpc/kernel/ucall.S b/arch/powerpc/kernel/ucall.S
new file mode 100644
index 000000000000..07296bc39166
--- /dev/null
+++ b/arch/powerpc/kernel/ucall.S
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Generic code to perform an ultravisor call.
+ *
+ * Copyright 2019, IBM Corporation.
+ *
+ */
+#include <asm/ppc_asm.h>
+#include <asm/export.h>
+
+_GLOBAL(ucall_norets)
+EXPORT_SYMBOL_GPL(ucall_norets)
+	sc	2	/* Invoke the ultravisor */
+	blr		/* Return r3 = status */
-- 
2.20.1


^ permalink raw reply related

* [PATCH v6 3/7] powerpc/powernv: Introduce FW_FEATURE_ULTRAVISOR
From: Claudio Carvalho @ 2019-08-22  3:48 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, Claudio Carvalho,
	kvm-ppc, Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu,
	Guerney Hunt, Thiago Bauermann
In-Reply-To: <20190822034838.27876-1-cclaudio@linux.ibm.com>

In PEF enabled systems, some of the resources which were previously
hypervisor privileged are now ultravisor privileged and controlled by
the ultravisor firmware.

This adds FW_FEATURE_ULTRAVISOR to indicate if PEF is enabled.

The host kernel can use FW_FEATURE_ULTRAVISOR, for instance, to skip
accessing resources (e.g. PTCR and LDBAR) in case PEF is enabled.

Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
[ andmike: Device node name to "ibm,ultravisor" ]
Signed-off-by: Michael Anderson <andmike@linux.ibm.com>
---
 arch/powerpc/include/asm/firmware.h         |  5 +++--
 arch/powerpc/include/asm/ultravisor.h       | 14 ++++++++++++
 arch/powerpc/kernel/prom.c                  |  4 ++++
 arch/powerpc/platforms/powernv/Makefile     |  1 +
 arch/powerpc/platforms/powernv/ultravisor.c | 24 +++++++++++++++++++++
 5 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/include/asm/ultravisor.h
 create mode 100644 arch/powerpc/platforms/powernv/ultravisor.c

diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index faeca8b76c8c..b3e214a97f3a 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -50,6 +50,7 @@
 #define FW_FEATURE_DRC_INFO	ASM_CONST(0x0000000800000000)
 #define FW_FEATURE_BLOCK_REMOVE ASM_CONST(0x0000001000000000)
 #define FW_FEATURE_PAPR_SCM 	ASM_CONST(0x0000002000000000)
+#define FW_FEATURE_ULTRAVISOR	ASM_CONST(0x0000004000000000)
 
 #ifndef __ASSEMBLY__
 
@@ -68,9 +69,9 @@ enum {
 		FW_FEATURE_TYPE1_AFFINITY | FW_FEATURE_PRRN |
 		FW_FEATURE_HPT_RESIZE | FW_FEATURE_DRMEM_V2 |
 		FW_FEATURE_DRC_INFO | FW_FEATURE_BLOCK_REMOVE |
-		FW_FEATURE_PAPR_SCM,
+		FW_FEATURE_PAPR_SCM | FW_FEATURE_ULTRAVISOR,
 	FW_FEATURE_PSERIES_ALWAYS = 0,
-	FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL,
+	FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL | FW_FEATURE_ULTRAVISOR,
 	FW_FEATURE_POWERNV_ALWAYS = 0,
 	FW_FEATURE_PS3_POSSIBLE = FW_FEATURE_LPAR | FW_FEATURE_PS3_LV1,
 	FW_FEATURE_PS3_ALWAYS = FW_FEATURE_LPAR | FW_FEATURE_PS3_LV1,
diff --git a/arch/powerpc/include/asm/ultravisor.h b/arch/powerpc/include/asm/ultravisor.h
new file mode 100644
index 000000000000..dc6e1ea198f2
--- /dev/null
+++ b/arch/powerpc/include/asm/ultravisor.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Ultravisor definitions
+ *
+ * Copyright 2019, IBM Corporation.
+ *
+ */
+#ifndef _ASM_POWERPC_ULTRAVISOR_H
+#define _ASM_POWERPC_ULTRAVISOR_H
+
+int early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
+				  int depth, void *data);
+
+#endif	/* _ASM_POWERPC_ULTRAVISOR_H */
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 7159e791a70d..5828f1c81dc9 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -55,6 +55,7 @@
 #include <asm/firmware.h>
 #include <asm/dt_cpu_ftrs.h>
 #include <asm/drmem.h>
+#include <asm/ultravisor.h>
 
 #include <mm/mmu_decl.h>
 
@@ -702,6 +703,9 @@ void __init early_init_devtree(void *params)
 #ifdef CONFIG_PPC_POWERNV
 	/* Some machines might need OPAL info for debugging, grab it now. */
 	of_scan_flat_dt(early_init_dt_scan_opal, NULL);
+
+	/* Scan tree for ultravisor feature */
+	of_scan_flat_dt(early_init_dt_scan_ultravisor, NULL);
 #endif
 
 #ifdef CONFIG_FA_DUMP
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index 69a3aefa905b..49186f0985a4 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -4,6 +4,7 @@ obj-y			+= idle.o opal-rtc.o opal-nvram.o opal-lpc.o opal-flash.o
 obj-y			+= rng.o opal-elog.o opal-dump.o opal-sysparam.o opal-sensor.o
 obj-y			+= opal-msglog.o opal-hmi.o opal-power.o opal-irqchip.o
 obj-y			+= opal-kmsg.o opal-powercap.o opal-psr.o opal-sensor-groups.o
+obj-y			+= ultravisor.o
 
 obj-$(CONFIG_SMP)	+= smp.o subcore.o subcore-asm.o
 obj-$(CONFIG_PCI)	+= pci.o pci-ioda.o npu-dma.o pci-ioda-tce.o
diff --git a/arch/powerpc/platforms/powernv/ultravisor.c b/arch/powerpc/platforms/powernv/ultravisor.c
new file mode 100644
index 000000000000..02ac57b4bded
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/ultravisor.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Ultravisor high level interfaces
+ *
+ * Copyright 2019, IBM Corporation.
+ *
+ */
+#include <linux/init.h>
+#include <linux/printk.h>
+#include <linux/of_fdt.h>
+
+#include <asm/ultravisor.h>
+#include <asm/firmware.h>
+
+int __init early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
+					 int depth, void *data)
+{
+	if (!of_flat_dt_is_compatible(node, "ibm,ultravisor"))
+		return 0;
+
+	powerpc_firmware_features |= FW_FEATURE_ULTRAVISOR;
+	pr_debug("Ultravisor detected!\n");
+	return 1;
+}
-- 
2.20.1


^ permalink raw reply related

* [PATCH v6 4/7] powerpc/mm: Use UV_WRITE_PATE ucall to register a PATE
From: Claudio Carvalho @ 2019-08-22  3:48 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Ryan Grimm, Madhavan Srinivasan, Michael Anderson, Ram Pai,
	Claudio Carvalho, kvm-ppc, Bharata B Rao, Ryan Grimm,
	Sukadev Bhattiprolu, Guerney Hunt, Thiago Bauermann
In-Reply-To: <20190822034838.27876-1-cclaudio@linux.ibm.com>

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

When Ultravisor (UV) is enabled, the partition table is stored in secure
memory and can only be accessed via the UV. The Hypervisor (HV) however
maintains a copy of the partition table in normal memory to allow Nest MMU
translations to occur (for normal VMs). The HV copy includes partition
table entries (PATE)s for secure VMs which would currently be unused
(Nest MMU translations cannot access secure memory) but they would be
needed as we add functionality.

This patch adds the UV_WRITE_PATE ucall which is used to update the PATE
for a VM (both normal and secure) when Ultravisor is enabled.

Signed-off-by: Michael Anderson <andmike@linux.ibm.com>
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
[ cclaudio: Write the PATE in HV's table before doing that in UV's ]
Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Reviewed-by: Ryan Grimm <grimm@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/ultravisor-api.h |  5 +++
 arch/powerpc/include/asm/ultravisor.h     |  8 ++++
 arch/powerpc/mm/book3s64/pgtable.c        | 50 +++++++++++++++++------
 3 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/include/asm/ultravisor-api.h b/arch/powerpc/include/asm/ultravisor-api.h
index 88ffa78f9d61..8cd49abff4f3 100644
--- a/arch/powerpc/include/asm/ultravisor-api.h
+++ b/arch/powerpc/include/asm/ultravisor-api.h
@@ -11,6 +11,7 @@
 #include <asm/hvcall.h>
 
 /* Return codes */
+#define U_BUSY			H_BUSY
 #define U_FUNCTION		H_FUNCTION
 #define U_NOT_AVAILABLE		H_NOT_AVAILABLE
 #define U_P2			H_P2
@@ -18,6 +19,10 @@
 #define U_P4			H_P4
 #define U_P5			H_P5
 #define U_PARAMETER		H_PARAMETER
+#define U_PERMISSION		H_PERMISSION
 #define U_SUCCESS		H_SUCCESS
 
+/* opcodes */
+#define UV_WRITE_PATE			0xF104
+
 #endif /* _ASM_POWERPC_ULTRAVISOR_API_H */
diff --git a/arch/powerpc/include/asm/ultravisor.h b/arch/powerpc/include/asm/ultravisor.h
index dc6e1ea198f2..6fe1f365dec8 100644
--- a/arch/powerpc/include/asm/ultravisor.h
+++ b/arch/powerpc/include/asm/ultravisor.h
@@ -8,7 +8,15 @@
 #ifndef _ASM_POWERPC_ULTRAVISOR_H
 #define _ASM_POWERPC_ULTRAVISOR_H
 
+#include <asm/asm-prototypes.h>
+#include <asm/ultravisor-api.h>
+
 int early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
 				  int depth, void *data);
 
+static inline int uv_register_pate(u64 lpid, u64 dw0, u64 dw1)
+{
+	return ucall_norets(UV_WRITE_PATE, lpid, dw0, dw1);
+}
+
 #endif	/* _ASM_POWERPC_ULTRAVISOR_H */
diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
index 7d0e0d0d22c4..4173f6931009 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -12,6 +12,8 @@
 #include <asm/tlb.h>
 #include <asm/trace.h>
 #include <asm/powernv.h>
+#include <asm/firmware.h>
+#include <asm/ultravisor.h>
 
 #include <mm/mmu_decl.h>
 #include <trace/events/thp.h>
@@ -209,21 +211,10 @@ 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)
+static void flush_partition(unsigned int lpid, bool radix)
 {
-	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) {
+	if (radix) {
 		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) : :
@@ -237,6 +228,39 @@ void mmu_partition_table_set_entry(unsigned int lpid, unsigned long dw0,
 	/* 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);
+
+	/*
+	 * When ultravisor is enabled, the partition table is stored in secure
+	 * memory and can only be accessed doing an ultravisor call. However, we
+	 * maintain a copy of the partition table in normal memory to allow Nest
+	 * MMU translations to occur (for normal VMs).
+	 *
+	 * Therefore, here we always update partition_tb, regardless of whether
+	 * we are running under an ultravisor or not.
+	 */
+	partition_tb[lpid].patb0 = cpu_to_be64(dw0);
+	partition_tb[lpid].patb1 = cpu_to_be64(dw1);
+
+	/*
+	 * If ultravisor is enabled, we do an ultravisor call to register the
+	 * partition table entry (PATE), which also do a global flush of TLBs
+	 * and partition table caches for the lpid. Otherwise, just do the
+	 * flush. The type of flush (hash or radix) depends on what the previous
+	 * use of the partition ID was, not the new use.
+	 */
+	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 & PATB_HR));
+	}
+}
 EXPORT_SYMBOL_GPL(mmu_partition_table_set_entry);
 
 static pmd_t *get_pmd_from_cache(struct mm_struct *mm)
-- 
2.20.1


^ permalink raw reply related

* [PATCH v6 5/7] powerpc/mm: Write to PTCR only if ultravisor disabled
From: Claudio Carvalho @ 2019-08-22  3:48 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, Claudio Carvalho,
	kvm-ppc, Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu,
	Guerney Hunt, Thiago Bauermann
In-Reply-To: <20190822034838.27876-1-cclaudio@linux.ibm.com>

In ultravisor enabled systems, PTCR becomes ultravisor privileged only
for writing and an attempt to write to it will cause a Hypervisor
Emulation Assitance interrupt.

This patch uses the set_ptcr_when_no_uv() function to restrict PTCR
writing to only when ultravisor is disabled.

Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
---
 arch/powerpc/include/asm/ultravisor.h    | 12 ++++++++++++
 arch/powerpc/mm/book3s64/hash_utils.c    |  5 +++--
 arch/powerpc/mm/book3s64/pgtable.c       |  2 +-
 arch/powerpc/mm/book3s64/radix_pgtable.c |  8 +++++---
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/ultravisor.h b/arch/powerpc/include/asm/ultravisor.h
index 6fe1f365dec8..d7aa97aa7834 100644
--- a/arch/powerpc/include/asm/ultravisor.h
+++ b/arch/powerpc/include/asm/ultravisor.h
@@ -10,10 +10,22 @@
 
 #include <asm/asm-prototypes.h>
 #include <asm/ultravisor-api.h>
+#include <asm/firmware.h>
 
 int early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
 				  int depth, void *data);
 
+/*
+ * In ultravisor enabled systems, PTCR becomes ultravisor privileged only for
+ * writing and an attempt to write to it will cause a Hypervisor Emulation
+ * Assistance interrupt.
+ */
+static inline void set_ptcr_when_no_uv(u64 val)
+{
+	if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
+		mtspr(SPRN_PTCR, val);
+}
+
 static inline int uv_register_pate(u64 lpid, u64 dw0, u64 dw1)
 {
 	return ucall_norets(UV_WRITE_PATE, lpid, dw0, dw1);
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index c3bfef08dcf8..4d0bb194ed70 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -62,6 +62,7 @@
 #include <asm/ps3.h>
 #include <asm/pte-walk.h>
 #include <asm/asm-prototypes.h>
+#include <asm/ultravisor.h>
 
 #include <mm/mmu_decl.h>
 
@@ -1076,8 +1077,8 @@ void hash__early_init_mmu_secondary(void)
 		if (!cpu_has_feature(CPU_FTR_ARCH_300))
 			mtspr(SPRN_SDR1, _SDR1);
 		else
-			mtspr(SPRN_PTCR,
-			      __pa(partition_tb) | (PATB_SIZE_SHIFT - 12));
+			set_ptcr_when_no_uv(__pa(partition_tb) |
+					    (PATB_SIZE_SHIFT - 12));
 	}
 	/* Initialize SLB */
 	slb_initialize();
diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
index 4173f6931009..01a7570c10e0 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -207,7 +207,7 @@ void __init mmu_partition_table_init(void)
 	 * 64 K size.
 	 */
 	ptcr = __pa(partition_tb) | (PATB_SIZE_SHIFT - 12);
-	mtspr(SPRN_PTCR, ptcr);
+	set_ptcr_when_no_uv(ptcr);
 	powernv_set_nmmu_ptcr(ptcr);
 }
 
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index 2204d8eeb784..ec3560b96177 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -27,6 +27,7 @@
 #include <asm/sections.h>
 #include <asm/trace.h>
 #include <asm/uaccess.h>
+#include <asm/ultravisor.h>
 
 #include <trace/events/thp.h>
 
@@ -650,8 +651,9 @@ void radix__early_init_mmu_secondary(void)
 		lpcr = mfspr(SPRN_LPCR);
 		mtspr(SPRN_LPCR, lpcr | LPCR_UPRT | LPCR_HR);
 
-		mtspr(SPRN_PTCR,
-		      __pa(partition_tb) | (PATB_SIZE_SHIFT - 12));
+		set_ptcr_when_no_uv(__pa(partition_tb) |
+				    (PATB_SIZE_SHIFT - 12));
+
 		radix_init_amor();
 	}
 
@@ -667,7 +669,7 @@ void radix__mmu_cleanup_all(void)
 	if (!firmware_has_feature(FW_FEATURE_LPAR)) {
 		lpcr = mfspr(SPRN_LPCR);
 		mtspr(SPRN_LPCR, lpcr & ~LPCR_UPRT);
-		mtspr(SPRN_PTCR, 0);
+		set_ptcr_when_no_uv(0);
 		powernv_set_nmmu_ptcr(0);
 		radix__flush_tlb_all();
 	}
-- 
2.20.1


^ permalink raw reply related

* [PATCH v6 6/7] powerpc/powernv: Access LDBAR only if ultravisor disabled
From: Claudio Carvalho @ 2019-08-22  3:48 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, Claudio Carvalho,
	kvm-ppc, Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu,
	Guerney Hunt, Thiago Bauermann
In-Reply-To: <20190822034838.27876-1-cclaudio@linux.ibm.com>

LDBAR is a per-thread SPR populated and used by the thread-imc pmu
driver to dump the data counter into memory. It contains memory along
with few other configuration bits. LDBAR is populated and enabled only
when any of the thread imc pmu events are monitored.

In ultravisor enabled systems, LDBAR becomes ultravisor privileged and
an attempt to write to it will cause a Hypervisor Emulation Assistance
interrupt.

In ultravisor enabled systems, the ultravisor is responsible to maintain
the LDBAR (e.g. save and restore it).

This restricts LDBAR access to only when ultravisor is disabled.

Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Reviewed-by: Ram Pai <linuxram@us.ibm.com>
Reviewed-by: Ryan Grimm <grimm@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/idle.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 09f49eed7fb8..78599bca66c2 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -675,7 +675,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
 		sprs.ptcr	= mfspr(SPRN_PTCR);
 		sprs.rpr	= mfspr(SPRN_RPR);
 		sprs.tscr	= mfspr(SPRN_TSCR);
-		sprs.ldbar	= mfspr(SPRN_LDBAR);
+		if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
+			sprs.ldbar = mfspr(SPRN_LDBAR);
 
 		sprs_saved = true;
 
@@ -789,7 +790,8 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
 	mtspr(SPRN_MMCR0,	sprs.mmcr0);
 	mtspr(SPRN_MMCR1,	sprs.mmcr1);
 	mtspr(SPRN_MMCR2,	sprs.mmcr2);
-	mtspr(SPRN_LDBAR,	sprs.ldbar);
+	if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))
+		mtspr(SPRN_LDBAR, sprs.ldbar);
 
 	mtspr(SPRN_SPRG3,	local_paca->sprg_vdso);
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH v6 7/7] powerpc/kvm: Use UV_RETURN ucall to return to ultravisor
From: Claudio Carvalho @ 2019-08-22  3:48 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, Claudio Carvalho,
	kvm-ppc, Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu,
	Guerney Hunt, Thiago Bauermann
In-Reply-To: <20190822034838.27876-1-cclaudio@linux.ibm.com>

From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>

When an SVM makes an hypercall or incurs some other exception, the
Ultravisor usually forwards (a.k.a. reflects) the exceptions to the
Hypervisor. After processing the exception, Hypervisor uses the
UV_RETURN ultracall to return control back to the SVM.

The expected register state on entry to this ultracall is:

* Non-volatile registers are restored to their original values.
* If returning from an hypercall, register R0 contains the return value
  (unlike other ultracalls) and, registers R4 through R12 contain any
  output values of the hypercall.
* R3 contains the ultracall number, i.e UV_RETURN.
* If returning with a synthesized interrupt, R2 contains the
  synthesized interrupt number.

Thanks to input from Paul Mackerras, Ram Pai and Mike Anderson.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
---
 arch/powerpc/include/asm/kvm_host.h       |  1 +
 arch/powerpc/include/asm/ultravisor-api.h |  1 +
 arch/powerpc/kernel/asm-offsets.c         |  1 +
 arch/powerpc/kvm/book3s_hv_rmhandlers.S   | 39 +++++++++++++++++++----
 4 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index e6e5f59aaa97..4bb552d639b8 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -283,6 +283,7 @@ struct kvm_arch {
 	cpumask_t cpu_in_guest;
 	u8 radix;
 	u8 fwnmi_enabled;
+	u8 secure_guest;
 	bool threads_indep;
 	bool nested_enable;
 	pgd_t *pgtable;
diff --git a/arch/powerpc/include/asm/ultravisor-api.h b/arch/powerpc/include/asm/ultravisor-api.h
index 8cd49abff4f3..6a0f9c74f959 100644
--- a/arch/powerpc/include/asm/ultravisor-api.h
+++ b/arch/powerpc/include/asm/ultravisor-api.h
@@ -24,5 +24,6 @@
 
 /* opcodes */
 #define UV_WRITE_PATE			0xF104
+#define UV_RETURN			0xF11C
 
 #endif /* _ASM_POWERPC_ULTRAVISOR_API_H */
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 4ccb6b3a7fbd..484f54dab247 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -506,6 +506,7 @@ int main(void)
 	OFFSET(KVM_VRMA_SLB_V, kvm, arch.vrma_slb_v);
 	OFFSET(KVM_RADIX, kvm, arch.radix);
 	OFFSET(KVM_FWNMI, kvm, arch.fwnmi_enabled);
+	OFFSET(KVM_SECURE_GUEST, kvm, arch.secure_guest);
 	OFFSET(VCPU_DSISR, kvm_vcpu, arch.shregs.dsisr);
 	OFFSET(VCPU_DAR, kvm_vcpu, arch.shregs.dar);
 	OFFSET(VCPU_VPA, kvm_vcpu, arch.vpa.pinned_addr);
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 07181d0dfcb7..9a05b0d932ef 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -29,6 +29,7 @@
 #include <asm/asm-compat.h>
 #include <asm/feature-fixups.h>
 #include <asm/cpuidle.h>
+#include <asm/ultravisor-api.h>
 
 /* Sign-extend HDEC if not on POWER9 */
 #define EXTEND_HDEC(reg)			\
@@ -1085,16 +1086,10 @@ BEGIN_FTR_SECTION
 END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 
 	ld	r5, VCPU_LR(r4)
-	ld	r6, VCPU_CR(r4)
 	mtlr	r5
-	mtcr	r6
 
 	ld	r1, VCPU_GPR(R1)(r4)
-	ld	r2, VCPU_GPR(R2)(r4)
-	ld	r3, VCPU_GPR(R3)(r4)
 	ld	r5, VCPU_GPR(R5)(r4)
-	ld	r6, VCPU_GPR(R6)(r4)
-	ld	r7, VCPU_GPR(R7)(r4)
 	ld	r8, VCPU_GPR(R8)(r4)
 	ld	r9, VCPU_GPR(R9)(r4)
 	ld	r10, VCPU_GPR(R10)(r4)
@@ -1112,10 +1107,42 @@ BEGIN_FTR_SECTION
 	mtspr	SPRN_HDSISR, r0
 END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
 
+	ld	r6, VCPU_KVM(r4)
+	lbz	r7, KVM_SECURE_GUEST(r6)
+	cmpdi	r7, 0
+	ld	r6, VCPU_GPR(R6)(r4)
+	ld	r7, VCPU_GPR(R7)(r4)
+	bne	ret_to_ultra
+
+	lwz	r0, VCPU_CR(r4)
+	mtcr	r0
+
 	ld	r0, VCPU_GPR(R0)(r4)
+	ld	r2, VCPU_GPR(R2)(r4)
+	ld	r3, VCPU_GPR(R3)(r4)
 	ld	r4, VCPU_GPR(R4)(r4)
 	HRFI_TO_GUEST
 	b	.
+/*
+ * Use UV_RETURN ultracall to return control back to the Ultravisor after
+ * processing an hypercall or interrupt that was forwarded (a.k.a. reflected)
+ * to the Hypervisor.
+ *
+ * All registers have already been loaded, except:
+ *   R0 = hcall result
+ *   R2 = SRR1, so UV can detect a synthesized interrupt (if any)
+ *   R3 = UV_RETURN
+ */
+ret_to_ultra:
+	lwz	r0, VCPU_CR(r4)
+	mtcr	r0
+
+	ld	r0, VCPU_GPR(R3)(r4)
+	mfspr	r2, SPRN_SRR1
+	li	r3, 0
+	ori	r3, r3, UV_RETURN
+	ld	r4, VCPU_GPR(R4)(r4)
+	sc	2
 
 /*
  * Enter the guest on a P9 or later system where we have exactly
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH 1/2] net/ibmvnic: unlock rtnl_lock in reset so linkwatch_event can run
From: David Miller @ 2019-08-22  4:06 UTC (permalink / raw)
  To: julietk; +Cc: netdev, linuxppc-dev
In-Reply-To: <20190820213120.19880-1-julietk@linux.vnet.ibm.com>

From: Juliet Kim <julietk@linux.vnet.ibm.com>
Date: Tue, 20 Aug 2019 17:31:19 -0400

> Commit a5681e20b541 ("net/ibmnvic: Fix deadlock problem in reset") 
> made the change to hold the RTNL lock during a reset to avoid deadlock 
> but linkwatch_event is fired during the reset and needs the RTNL lock.  
> That keeps linkwatch_event process from proceeding until the reset 
> is complete. The reset process cannot tolerate the linkwatch_event 
> processing after reset completes, so release the RTNL lock during the 
> process to allow a chance for linkwatch_event to run during reset. 
> This does not guarantee that the linkwatch_event will be processed as 
> soon as link state changes, but is an improvement over the current code
> where linkwatch_event processing is always delayed, which prevents 
> transmissions on the device from being deactivated leading transmit 
> watchdog timer to time-out. 
> 
> Release the RTNL lock before link state change and re-acquire after 
> the link state change to allow linkwatch_event to grab the RTNL lock 
> and run during the reset.
> 
> Fixes: a5681e20b541 ("net/ibmnvic: Fix deadlock problem in reset")
> Signed-off-by: Juliet Kim <julietk@linux.vnet.ibm.com>

Conditional locking, especialy such extensive use of conditional
locking as is being done here, is strongly discouraged and is always
indicative of bad design.

Please try to rework this change such that the code paths that want
to lock things a certain way are %100 segregated functionally into
different code paths and functions.

Or feel free to find a cleaner way to fix this.

^ permalink raw reply

* Re: [PATCH v2 1/4] powerpc/powernv: Add OPAL API interface to access secure variable
From: Oliver O'Halloran @ 2019-08-22  5:02 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, linux-kernel, Mimi Zohar,
	Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer, George Wilson
In-Reply-To: <1566400103-18201-2-git-send-email-nayna@linux.ibm.com>

On Wed, 2019-08-21 at 11:08 -0400, Nayna Jain wrote:
> The X.509 certificates trusted by the platform and required to secure boot
> the OS kernel are wrapped in secure variables, which are controlled by
> OPAL.
> 
> This patch adds firmware/kernel interface to read and write OPAL secure
> variables based on the unique key.
> 
> This support can be enabled using CONFIG_OPAL_SECVAR.
> 
> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/opal-api.h          |   5 +-
>  arch/powerpc/include/asm/opal.h              |   6 ++
>  arch/powerpc/include/asm/secvar.h            |  55 ++++++++++
>  arch/powerpc/kernel/Makefile                 |   2 +-
>  arch/powerpc/kernel/secvar-ops.c             |  25 +++++
>  arch/powerpc/platforms/powernv/Kconfig       |   6 ++
>  arch/powerpc/platforms/powernv/Makefile      |   1 +
>  arch/powerpc/platforms/powernv/opal-call.c   |   3 +
>  arch/powerpc/platforms/powernv/opal-secvar.c | 102 +++++++++++++++++++
>  arch/powerpc/platforms/powernv/opal.c        |   5 +
>  10 files changed, 208 insertions(+), 2 deletions(-)
>  create mode 100644 arch/powerpc/include/asm/secvar.h
>  create mode 100644 arch/powerpc/kernel/secvar-ops.c
>  create mode 100644 arch/powerpc/platforms/powernv/opal-secvar.c
> 
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index 383242eb0dea..b238b4f26c5b 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -208,7 +208,10 @@
>  #define OPAL_HANDLE_HMI2			166
>  #define	OPAL_NX_COPROC_INIT			167
>  #define OPAL_XIVE_GET_VP_STATE			170
> -#define OPAL_LAST				170
> +#define OPAL_SECVAR_GET                         173
> +#define OPAL_SECVAR_GET_NEXT                    174
> +#define OPAL_SECVAR_ENQUEUE_UPDATE              175
> +#define OPAL_LAST                               175
>  
>  #define QUIESCE_HOLD			1 /* Spin all calls at entry */
>  #define QUIESCE_REJECT			2 /* Fail all calls with OPAL_BUSY */
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 57bd029c715e..247adec2375f 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -388,6 +388,12 @@ void opal_powercap_init(void);
>  void opal_psr_init(void);
>  void opal_sensor_groups_init(void);

Put these with the rest of the OPAL API wrapper prototypes
(i.e. just after opal_nx_coproc_init()) rather than with the
internal functions.
 
> > +extern int opal_secvar_get(uint64_t k_key, uint64_t k_key_len,
> +			   uint64_t k_data, uint64_t k_data_size);
> +extern int opal_secvar_get_next(uint64_t k_key, uint64_t k_key_len,
> +				uint64_t k_key_size);
> +extern int opal_secvar_enqueue_update(uint64_t k_key, uint64_t k_key_len,
> +				      uint64_t k_data, uint64_t k_data_size);

Everything in asm/opal.h is intended for consumption by the kernel, so
use a useful kernel type (or annotation) rather than blank uint64_t for
the parameters that are actually pointers. You should also ditch the k_
prefix since it doesn't make much sense having it inside the kernel.

As a general comment, don't use extern on function prototypes. They're
extern by default and, more importantly, it's contrary to the normal
kernel style.

>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* _ASM_POWERPC_OPAL_H */
> diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
> new file mode 100644
> index 000000000000..645654456265
> --- /dev/null
> +++ b/arch/powerpc/include/asm/secvar.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * PowerPC secure variable operations.
> + *
> + * Copyright (C) 2019 IBM Corporation
> + * Author: Nayna Jain <nayna@linux.ibm.com>
> + *
> + */
> +#ifndef SECVAR_OPS_H
> +#define SECVAR_OPS_H
> +
> +#include<linux/types.h>
> +#include<linux/errno.h>
missing space

> +
> +struct secvar_operations {
> +	int (*get_variable)(const char *key, unsigned long key_len, u8 *data,
> +			    unsigned long *data_size);
> +	int (*get_next_variable)(const char *key, unsigned long *key_len,
> +				 unsigned long keysize);
> +	int (*set_variable)(const char *key, unsigned long key_len, u8 *data,
> +			    unsigned long data_size);
> +};


Calling them requires writing code like:

	secvar_ops->get_variable(blah);

Why not shorten it to:
	secvar_ops->get(blah);


> +#ifdef CONFIG_PPC_SECURE_BOOT
> +
> +extern void set_secvar_ops(struct secvar_operations *ops);
> +extern struct secvar_operations *get_secvar_ops(void);
> +
> +#else
> +
> +static inline void set_secvar_ops(struct secvar_operations *ops)
> +{
> +}
> +
> +static inline struct secvar_operations *get_secvar_ops(void)
> +{
> +	return NULL;
> +}
> +
> +#endif
> +#ifdef CONFIG_OPAL_SECVAR
> +
> +extern int secvar_init(void);
> +
> +#else
> +
> +static inline int secvar_init(void)
> +{
> +	return -EINVAL;
> +}
> +
> +#endif

The init function is always going to be platform specific so having an
opal-specific secvar_init() in a generic header doesn't make much sense
to me.

> +
> +#endif
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 520b1c814197..9041563f1c74 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -157,7 +157,7 @@ endif
>  obj-$(CONFIG_EPAPR_PARAVIRT)	+= epapr_paravirt.o epapr_hcalls.o
>  obj-$(CONFIG_KVM_GUEST)		+= kvm.o kvm_emul.o
>  
> -obj-$(CONFIG_PPC_SECURE_BOOT)	+= secboot.o ima_arch.o
> +obj-$(CONFIG_PPC_SECURE_BOOT)	+= secboot.o ima_arch.o secvar-ops.o
>  
>  # Disable GCOV, KCOV & sanitizers in odd or sensitive code
>  GCOV_PROFILE_prom_init.o := n
> diff --git a/arch/powerpc/kernel/secvar-ops.c b/arch/powerpc/kernel/secvar-ops.c
> new file mode 100644
> index 000000000000..198222499848
> --- /dev/null
> +++ b/arch/powerpc/kernel/secvar-ops.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 IBM Corporation
> + * Author: Nayna Jain <nayna@linux.ibm.com>
> + *
> + * secvar-ops.c
> + *      - initialize secvar operations for PowerPC Secureboot
> + */
> +

> +#include<stddef.h>
> +#include<asm/secvar.h>
missing space

> +
> +static struct secvar_operations *secvars_ops;
should be const

> +
> +void set_secvar_ops(struct secvar_operations *ops)
> +{
> +	if (!ops)
> +		secvars_ops = NULL;
> +	secvars_ops = ops;
> +}
> +
> +struct secvar_operations *get_secvar_ops(void)
> +{
> +	return secvars_ops;
> +}

Is this really any better than just making the ops pointer global?

> diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig
> index 850eee860cf2..65b060539b5c 100644
> --- a/arch/powerpc/platforms/powernv/Kconfig
> +++ b/arch/powerpc/platforms/powernv/Kconfig
> @@ -47,3 +47,9 @@ config PPC_VAS
>  	  VAS adapters are found in POWER9 based systems.
>  
>  	  If unsure, say N.
> +
> +config OPAL_SECVAR
> +	bool "OPAL Secure Variables"
> +	depends on PPC_POWERNV
> +	help
> +	  This enables the kernel to access OPAL secure variables.
> diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
> index da2e99efbd04..6651c742e530 100644
> --- a/arch/powerpc/platforms/powernv/Makefile
> +++ b/arch/powerpc/platforms/powernv/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
>  obj-$(CONFIG_PPC_MEMTRACE)	+= memtrace.o
>  obj-$(CONFIG_PPC_VAS)	+= vas.o vas-window.o vas-debug.o
>  obj-$(CONFIG_OCXL_BASE)	+= ocxl.o
> +obj-$(CONFIG_OPAL_SECVAR) += opal-secvar.o
> diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
> index 29ca523c1c79..93106e867924 100644
> --- a/arch/powerpc/platforms/powernv/opal-call.c
> +++ b/arch/powerpc/platforms/powernv/opal-call.c
> @@ -287,3 +287,6 @@ OPAL_CALL(opal_pci_set_pbcq_tunnel_bar,		OPAL_PCI_SET_PBCQ_TUNNEL_BAR);
>  OPAL_CALL(opal_sensor_read_u64,			OPAL_SENSOR_READ_U64);
>  OPAL_CALL(opal_sensor_group_enable,		OPAL_SENSOR_GROUP_ENABLE);
>  OPAL_CALL(opal_nx_coproc_init,			OPAL_NX_COPROC_INIT);
> +OPAL_CALL(opal_secvar_get,                     OPAL_SECVAR_GET);
> +OPAL_CALL(opal_secvar_get_next,                 OPAL_SECVAR_GET_NEXT);
> +OPAL_CALL(opal_secvar_enqueue_update,           OPAL_SECVAR_ENQUEUE_UPDATE);
> diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c b/arch/powerpc/platforms/powernv/opal-secvar.c
> new file mode 100644
> index 000000000000..b0f97cea7675
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/opal-secvar.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PowerNV code for secure variables
> + *
> + * Copyright (C) 2019 IBM Corporation
> + * Author: Claudio Carvalho <cclaudio@linux.ibm.com>
> + *
> + * APIs to access secure variables managed by OPAL.
> + *
> + */
> +
> +#define pr_fmt(fmt) "secvar: "fmt
> +
> +#include <linux/types.h>
> +#include <asm/opal.h>
> +#include <asm/secvar.h>
> +
> +static bool is_opal_secvar_supported(void)
> +{
> +	static bool opal_secvar_supported;
> +	static bool initialized;
> +
> +	if (initialized)
> +		return opal_secvar_supported;
> +
> +	if (!opal_check_token(OPAL_SECVAR_GET)
> +			|| !opal_check_token(OPAL_SECVAR_GET_NEXT)
> +			|| !opal_check_token(OPAL_SECVAR_ENQUEUE_UPDATE)) {

> +		pr_err("OPAL doesn't support secure variables\n");

This should only print an error if OPAL has advertised support for
secure variables through the DT, but doesn't support the OPAL
calls. Otherwise we'll get a spurious error message on any system
running currently released firmware.

> +		opal_secvar_supported = false;
> +	} else {
> +		opal_secvar_supported = true;
> +	}
> +
> +	initialized = true;
> +
> +	return opal_secvar_supported;
> +}
> +
> +static int opal_get_variable(const char *key, unsigned long ksize,
> +			     u8 *data, unsigned long *dsize)
> +{
> +	int rc;
> +
> +	if (!is_opal_secvar_supported())
> +		return OPAL_UNSUPPORTED;

This should be -ENXIO or -ENOSUPP. OPAL_UNSUPPORTED is an OPAL return
code, not a kernel one. That said, if the firmware doesn't support
secure variables we should never be calling this function anyway since
the ops pointer is never set.

> +
> +	if (dsize)
> +		*dsize = cpu_to_be64(*dsize);
> +
> +	rc = opal_secvar_get(__pa(key), ksize,
> +			__pa(data), __pa(dsize));
> +
> +	if (dsize)
> +		*dsize = be64_to_cpu(*dsize);
> +
> +	return rc;
> +}
> +
> +static int opal_get_next_variable(const char *key, unsigned long *keylen,
> +				  unsigned long keysize)
> +{
> +	int rc;
> +
> +	if (!is_opal_secvar_supported())
> +		return OPAL_UNSUPPORTED;
> +
> +	if (keylen)
> +		*keylen = cpu_to_be64(*keylen);
> +
> +	rc = opal_secvar_get_next(__pa(key), __pa(keylen), keysize);
> +
> +	if (keylen)
> +		*keylen = be64_to_cpu(*keylen);
> +
> +	return rc;
> +}
> +
> +static int opal_set_variable(const char *key, unsigned long ksize, u8 *data,
> +			     unsigned long dsize)
> +{
> +	int rc;
> +
> +	if (!is_opal_secvar_supported())
> +		return OPAL_UNSUPPORTED;
> +
> +	rc = opal_secvar_enqueue_update(__pa(key), ksize, __pa(data), dsize);
> +
> +	return rc;
> +}
> +

> +static struct secvar_operations secvar_ops = {
> +	.get_variable = opal_get_variable,
> +	.get_next_variable = opal_get_next_variable,
> +	.set_variable = opal_set_variable,
> +};

should be const

> +int secvar_init(void)
> +{
> +	set_secvar_ops(&secvar_ops);
> +	return 0;
> +}

Rename this to opal_secvar_init() and put the prototype in
arch/powerpc/platforms/powernv/powernv.h

> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index aba443be7daa..ffe6f1cf0830 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -32,6 +32,8 @@
>  #include <asm/mce.h>
>  #include <asm/imc-pmu.h>
>  #include <asm/bug.h>
> +#include <asm/secvar.h>
> +#include <asm/secboot.h>
>  
>  #include "powernv.h"
>  
> @@ -988,6 +990,9 @@ static int __init opal_init(void)
>  	/* Initialise OPAL Power control interface */
>  	opal_power_control_init();
>  
> +	if (is_powerpc_secvar_supported())
> +		secvar_init();
> +

The usual pattern here is to have the init function check for support
internally.

Also, is_powerpc_secvar_supported() doesn't appear to be defined
anywhere. Is that supposed to be is_opal_secvar_supported()? Or is this
series supposed to be applied on top of another series?

>  	return 0;
>  }
>  machine_subsys_initcall(powernv, opal_init);


^ permalink raw reply

* Re: [RFC PATCH] powerpc: Convert ____flush_dcache_icache_phys() to C
From: Christophe Leroy @ 2019-08-22  5:06 UTC (permalink / raw)
  To: Alastair D'Silva, Segher Boessenkool, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <fcc94e7f347c3759a1698444239a7250b22cde7d.camel@au1.ibm.com>



Le 22/08/2019 à 02:27, Alastair D'Silva a écrit :
> On Wed, 2019-08-21 at 22:27 +0200, Christophe Leroy wrote:
>>
>> Le 20/08/2019 à 06:36, Alastair D'Silva a écrit :
>>> On Fri, 2019-08-16 at 15:52 +0000, Christophe Leroy wrote:
>>
>> [...]
>>
>>>
>>> Thanks Christophe,
>>>
>>> I'm trying a somewhat different approach that requires less
>>> knowledge
>>> of assembler. Handling of CPU_FTR_COHERENT_ICACHE is outside this
>>> function. The code below is not a patch as my tree is a bit messy,
>>> sorry:
>>
>> Can we be 100% sure that GCC won't add any code accessing some
>> global
>> data or stack while the Data MMU is OFF ?
>>
>> Christophe
>>
> 
> +mpe
> 
> I'm not sure how we would go about making such a guarantee, but I've
> tied every variable used to a register and addr is passed in a
> register, so there is no stack usage, and every call in there only
> operates on it's operands.
> 
> The calls to the inline cache helpers (for the PPC32 case) are all
> constants, so I can't see a reasonable scenario where there would be a
> function call and reordered to after the DR bit is turned off, but I
> guess if we want to be paranoid, we could always add an mb() call
> before the DR bit is manipulated to prevent the compiler from
> reordering across the section where the data MMU is disabled.
> 
> 

Anyway, I think the benefit of converting that function to C is pretty 
small. flush_dcache_range() and friends were converted to C mainly in 
order to inline them. But this __flush_dcache_icache_phys() is too big 
to be worth inlining, yet small and stable enough to remain in assembly 
for the time being.

So I suggest you keep it aside your series for now, just move 
PURGE_PREFETCHED_INS inside it directly as it will be the only remaining 
user of it.

Christophe

^ permalink raw reply

* Re: [PATCH v2 2/4] powerpc: expose secure variables to userspace via sysfs
From: Oliver O'Halloran @ 2019-08-22  5:18 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, linux-kernel, Mimi Zohar,
	Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer, George Wilson
In-Reply-To: <1566400103-18201-3-git-send-email-nayna@linux.ibm.com>

On Wed, 2019-08-21 at 11:08 -0400, Nayna Jain wrote:
> PowerNV secure variables, which store the keys used for OS kernel
> verification, are managed by the firmware. These secure variables need to
> be accessed by the userspace for addition/deletion of the certificates.
> 
> This patch adds the sysfs interface to expose secure variables for PowerNV
> secureboot. The users shall use this interface for manipulating
> the keys stored in the secure variables.
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
>  Documentation/ABI/testing/sysfs-secvar |  27 ++++
>  arch/powerpc/Kconfig                   |   9 ++
>  arch/powerpc/kernel/Makefile           |   1 +
>  arch/powerpc/kernel/secvar-sysfs.c     | 210 +++++++++++++++++++++++++
>  4 files changed, 247 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-secvar
>  create mode 100644 arch/powerpc/kernel/secvar-sysfs.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
> new file mode 100644
> index 000000000000..68f0e03d873d
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-secvar
> @@ -0,0 +1,27 @@
> +What:		/sys/firmware/secvar
> +Date:		August 2019
> +Contact:	Nayna Jain <nayna@linux.ibm.com>
> +Description:
> +		This directory exposes interfaces for interacting with
> +		the secure variables managed by OPAL firmware.
> +
> +		This is only for the powerpc/powernv platform.
> +
> +		Directory:
> +		vars:		This directory lists all the variables that
> +				are supported by the OPAL. The variables are
> +				represented in the form of directories with
> +				their variable names. The variable name is
> +				unique and is in ASCII representation. The data
> +				and size can be determined by reading their
> +				respective attribute files.
> +
> +		Each variable directory has the following files:
> +		name:		An ASCII representation of the variable name
> +		data:		A read-only file containing the value of the
> +				variable
> +		size:		An integer representation of the size of the
> +				content of the variable. In other works, it
> +				represents the size of the data
> +		update:		A write-only file that is used to submit the new
> +				value for the variable.
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 42109682b727..b4bdf77837b2 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -925,6 +925,15 @@ config PPC_SECURE_BOOT
>  	  allows user to enable OS Secure Boot on PowerPC systems that
>  	  have firmware secure boot support.
>  
> +config SECVAR_SYSFS
> +        tristate "Enable sysfs interface for POWER secure variables"
> +        depends on PPC_SECURE_BOOT
> +        help
> +          POWER secure variables are managed and controlled by firmware.
> +          These variables are exposed to userspace via sysfs to enable
> +          read/write operations on these variables. Say Y if you have
> +	  secure boot enabled and want to expose variables to userspace.
> +
>  endmenu
>  
>  config ISA_DMA_API
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 9041563f1c74..4ea7b738c3a3 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -158,6 +158,7 @@ obj-$(CONFIG_EPAPR_PARAVIRT)	+= epapr_paravirt.o epapr_hcalls.o
>  obj-$(CONFIG_KVM_GUEST)		+= kvm.o kvm_emul.o
>  
>  obj-$(CONFIG_PPC_SECURE_BOOT)	+= secboot.o ima_arch.o secvar-ops.o
> +obj-$(CONFIG_SECVAR_SYSFS)     += secvar-sysfs.o
>  
>  # Disable GCOV, KCOV & sanitizers in odd or sensitive code
>  GCOV_PROFILE_prom_init.o := n
> diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c
> new file mode 100644
> index 000000000000..e46986bb29a0
> --- /dev/null
> +++ b/arch/powerpc/kernel/secvar-sysfs.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 IBM Corporation <nayna@linux.ibm.com>
> + *
> + * This code exposes secure variables to user via sysfs
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/compat.h>
> +#include <linux/string.h>
> +#include <asm/opal.h>
> +#include <asm/secvar.h>
> +
> +//Approximating it for now, it is bound to change.
> +#define VARIABLE_MAX_SIZE  32000

this needs to be communicated from the secvar backend, maybe via a
field in the ops structure?

> +
> +static struct kobject *powerpc_kobj;
Call it secvar_kobj or something.

> +static struct secvar_operations *secvarops;
Ah, the old I-can't-believe-it's-not-global trick.

> +struct kset *secvar_kset;
shouldn't that be static too?

> +
> +static ssize_t name_show(struct kobject *kobj, struct kobj_attribute *attr,
> +			 char *buf)
> +{
> +	return sprintf(buf, "%s", kobj->name);
> +}
> +
> +static ssize_t size_show(struct kobject *kobj, struct kobj_attribute *attr,
> +			 char *buf)
> +{
> +	unsigned long dsize;
> +	int rc;
> +
> +	rc = secvarops->get_variable(kobj->name, strlen(kobj->name) + 1, NULL,
> +				     &dsize);
> +	if (rc) {
> +		pr_err("Error retrieving variable size %d\n", rc);
> +		return rc;
> +	}
> +
> +	rc = sprintf(buf, "%ld", dsize);
> +
> +	return rc;
> +}
> +
> +static ssize_t data_read(struct file *filep, struct kobject *kobj,
> +			 struct bin_attribute *attr, char *buf, loff_t off,
> +			 size_t count)
> +{
> +	unsigned long dsize;
> +	int rc;
> +	char *data;
> +
> +	rc = secvarops->get_variable(kobj->name, strlen(kobj->name) + 1, NULL,
> +				     &dsize);
> +	if (rc) {
> +		pr_err("Error getting variable size %d\n", rc);
> +		return rc;
> +	}
> +	pr_debug("dsize is %ld\n", dsize);
> +
> +	data = kzalloc(dsize, GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	rc = secvarops->get_variable(kobj->name, strlen(kobj->name)+1, data,
> +				     &dsize);
> +	if (rc) {
> +		pr_err("Error getting variable %d\n", rc);
> +		goto data_fail;
> +	}
> +
> +	rc = memory_read_from_buffer(buf, count, &off, data, dsize);
> +
> +data_fail:
> +	kfree(data);
> +	return rc;
> +}
> +
> +static ssize_t update_write(struct file *filep, struct kobject *kobj,
> +			    struct bin_attribute *attr, char *buf, loff_t off,
> +			    size_t count)
> +{
> +	int rc;
> +
> +	pr_debug("count is %ld\n", count);
> +	rc = secvarops->set_variable(kobj->name, strlen(kobj->name)+1, buf,
> +				     count);
> +	if (rc) {
> +		pr_err("Error setting the variable %s\n", kobj->name);
> +		return rc;
> +	}
> +
> +	return count;
> +}
> +
> +static struct kobj_attribute name_attr =
> +__ATTR(name, 0444, name_show, NULL);
> +
> +static struct kobj_attribute size_attr =
> +__ATTR(size, 0444, size_show, NULL);
> +

> +static struct bin_attribute data_attr = {
> +	.attr = {.name = "data", .mode = 0444},
> +	.size = VARIABLE_MAX_SIZE,
> +	.read = data_read,
> +};

Should they be globally readable? If efivars is globally readable I'm
happy to follow that example, but mpe might have opinions.

> +
> +
> +static struct bin_attribute update_attr = {
> +	.attr = {.name = "update", .mode = 0200},
> +	.size = VARIABLE_MAX_SIZE,
> +	.write = update_write,
> +};
> +
> +static struct bin_attribute  *secvar_bin_attrs[] = {
> +	&data_attr,
> +	&update_attr,
> +	NULL,
> +};
> +
> +static struct attribute *secvar_attrs[] = {
> +	&name_attr.attr,
> +	&size_attr.attr,
> +	NULL,
> +};
> +
> +const struct attribute_group secvar_attr_group = {
> +	.attrs = secvar_attrs,
> +	.bin_attrs = secvar_bin_attrs,
> +};
> +
> +int secvar_sysfs_load(void)
> +{
> +
> +	char *name;
> +	unsigned long namesize;
> +	struct kobject *kobj;
> +	int status;
> +	int rc = 0;
> +
> +	name = kzalloc(1024, GFP_KERNEL);
> +	if (!name)
> +		return -ENOMEM;

Where'd the 1024 restriction on the length of the variable name come
from? is that enforced by firmware? If so, how does firmware
communicate the limited key length?

> +
> +	do {
> +

> +		status = secvarops->get_next_variable(name, &namesize, 1024);
does namesize need to be initialised for the first call?

> +		if (status != OPAL_SUCCESS)
> +			break;

You might want to differentiate between the error case and the "no
extra variables" case. Come to think of it, since the point of
abstracting secvar ops is to make this code indepdendent of the backend
why are we checking for OPAL_SUCCESS? The ops functions should be
returning linux return code (EIO, etc) rather than OPAL codes.

> +
> +		pr_info("name is %s\n", name);
pr_info?

> +		kobj = kobject_create_and_add(name, &(secvar_kset->kobj));
> +		if (kobj) {
> +			rc = sysfs_create_group(kobj, &secvar_attr_group);
> +			if (rc)
> +				pr_err("Error creating attributes for %s variable\n",
> +				name);
> +		} else {
> +			pr_err("Error creating sysfs entry for %s variable\n",
> +				name);
> +			rc = -EINVAL;
> +		}
> +
> +	} while ((status == OPAL_SUCCESS) && (rc == 0));

Checking status here isn't needed here since we checked it above and
broken out of the loop.

> +
> +	kfree(name);
> +	return rc;
> +}
> +
> +int secvar_sysfs_init(void)
> +{
> +	powerpc_kobj = kobject_create_and_add("secvar", firmware_kobj);
> +	if (!powerpc_kobj) {
> +		pr_err("secvar: Failed to create firmware kobj\n");
> +		return -ENODEV;
> +	}
> +
> +	secvar_kset = kset_create_and_add("vars", NULL, powerpc_kobj);
> +	if (!secvar_kset) {
> +		pr_err("secvar: sysfs kobject registration failed.\n");
> +		return -ENODEV;
> +	}
> +
> +	secvarops = get_secvar_ops();
> +	if (!secvarops) {
> +		kobject_put(powerpc_kobj);
> +		pr_err("secvar: failed to retrieve secvar operations.\n");
> +		return -ENODEV;
> +	}

Not having secvar support isn't an error. IMO checking if the ops are
defined is the first thing you should be doing. If we don't have a
defined set of ops then we don't need to do anything else.

> +
> +	secvar_sysfs_load();
> +	pr_info("Secure variables sysfs initialized");
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(secvar_sysfs_init);
> +
> +static void secvar_sysfs_exit(void)
> +{
> +	kobject_put(powerpc_kobj);
> +}
> +EXPORT_SYMBOL_GPL(secvar_sysfs_exit);
> +
> +module_init(secvar_sysfs_init);
> +module_exit(secvar_sysfs_exit);
> +

> +MODULE_AUTHOR("Nayna Jain<nayna@linux.ibm.com>");
needs a space between your name and the opening '<' of the email
> +MODULE_DESCRIPTION("sysfs interface to POWER secure variables");
> +MODULE_LICENSE("GPL");


^ permalink raw reply

* Re: [PATCH v2 1/4] powerpc/powernv: Add OPAL API interface to access secure variable
From: Oliver O'Halloran @ 2019-08-22  5:41 UTC (permalink / raw)
  To: Nayna Jain, linuxppc-dev, linux-efi, linux-integrity
  Cc: Ard Biesheuvel, Eric Ricther, Linux Kernel Mailing List,
	Mimi Zohar, Claudio Carvalho, Matthew Garret, Greg Kroah-Hartman,
	Paul Mackerras, Jeremy Kerr, Elaine Palmer, George Wilson
In-Reply-To: <eda9210b56ab220519642d272079eeca60a18265.camel@gmail.com>

On Thu, Aug 22, 2019 at 3:02 PM Oliver O'Halloran <oohall@gmail.com> wrote:
>
> On Wed, 2019-08-21 at 11:08 -0400, Nayna Jain wrote:
> > diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> > index aba443be7daa..ffe6f1cf0830 100644
> > --- a/arch/powerpc/platforms/powernv/opal.c
> > +++ b/arch/powerpc/platforms/powernv/opal.c
> > @@ -32,6 +32,8 @@
> >  #include <asm/mce.h>
> >  #include <asm/imc-pmu.h>
> >  #include <asm/bug.h>
> > +#include <asm/secvar.h>
> > +#include <asm/secboot.h>
> >
> >  #include "powernv.h"
> >
> > @@ -988,6 +990,9 @@ static int __init opal_init(void)
> >       /* Initialise OPAL Power control interface */
> >       opal_power_control_init();
> >
> > +     if (is_powerpc_secvar_supported())
> > +             secvar_init();
> > +
>
> The usual pattern here is to have the init function check for support
> internally.
>
> Also, is_powerpc_secvar_supported() doesn't appear to be defined
> anywhere. Is that supposed to be is_opal_secvar_supported()? Or is this
> series supposed to be applied on top of another series?

To answer my own question, yes it depends on the series at [1] which
adds IMA support. Turns out actually reading the cover letter helps,
who knew.

That said, I'm still not entirely sure about this. The implementation
of is_powerpc_secvar_supported() in [2] parses the DT and seems to
assume the DT bindings that OPAL produces. Are those common with the
DT bindings produced by OF when running on pseries?

[1] http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=125961
[2] http://patchwork.ozlabs.org/patch/1149257/

>
> >       return 0;
> >  }
> >  machine_subsys_initcall(powernv, opal_init);
>

^ permalink raw reply

* [linux-next][PPC][bisected c7d8b7][gcc 6.4.1] build error at drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:1471
From: Abdul Haleem @ 2019-08-22  5:46 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: jgg, Stephen Rothwell, rcampbell, linux-next, linux-kernel

Greeting's

Today's linux-next kernel 5.3.0-rc5-next-20190820 failed to build on my
powerpc machine

Build errors:
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c: In function amdgpu_exit:
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:1471:2: error: implicit
declaration of function mmu_notifier_synchronize
[-Werror=implicit-function-declaration]
  mmu_notifier_synchronize();
  ^~~~~~~~~~~~~~~~~~~~~~~~ 
cc1: some warnings being treated as errors
make[4]: *** [drivers/gpu/drm/amd/amdgpu/amdgpu_drv.o] Error 1
make[3]: *** [drivers/gpu/drm/amd/amdgpu] Error 2

It was introduced with commit c7d8b7 (hmm: use mmu_notifier_get/put for
'struct hmm')

Reverting the commit fixes the issue.

-- 
Regard's

Abdul Haleem
IBM Linux Technology Centre




^ permalink raw reply

* RE: [RFC PATCH] powerpc: Convert ____flush_dcache_icache_phys() to C
From: Alastair D'Silva @ 2019-08-22  5:47 UTC (permalink / raw)
  To: Christophe Leroy, Segher Boessenkool, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <c22dfd19-872a-afe1-40c6-82e11b0af9e1@c-s.fr>

On Thu, 2019-08-22 at 07:06 +0200, Christophe Leroy wrote:
> 
> Le 22/08/2019 à 02:27, Alastair D'Silva a écrit :
> > On Wed, 2019-08-21 at 22:27 +0200, Christophe Leroy wrote:
> > > Le 20/08/2019 à 06:36, Alastair D'Silva a écrit :
> > > > On Fri, 2019-08-16 at 15:52 +0000, Christophe Leroy wrote:
> > > 
> > > [...]
> > > 
> > > > Thanks Christophe,
> > > > 
> > > > I'm trying a somewhat different approach that requires less
> > > > knowledge
> > > > of assembler. Handling of CPU_FTR_COHERENT_ICACHE is outside
> > > > this
> > > > function. The code below is not a patch as my tree is a bit
> > > > messy,
> > > > sorry:
> > > 
> > > Can we be 100% sure that GCC won't add any code accessing some
> > > global
> > > data or stack while the Data MMU is OFF ?
> > > 
> > > Christophe
> > > 
> > 
> > +mpe
> > 
> > I'm not sure how we would go about making such a guarantee, but
> > I've
> > tied every variable used to a register and addr is passed in a
> > register, so there is no stack usage, and every call in there only
> > operates on it's operands.
> > 
> > The calls to the inline cache helpers (for the PPC32 case) are all
> > constants, so I can't see a reasonable scenario where there would
> > be a
> > function call and reordered to after the DR bit is turned off, but
> > I
> > guess if we want to be paranoid, we could always add an mb() call
> > before the DR bit is manipulated to prevent the compiler from
> > reordering across the section where the data MMU is disabled.
> > 
> > 
> 
> Anyway, I think the benefit of converting that function to C is
> pretty 
> small. flush_dcache_range() and friends were converted to C mainly
> in 
> order to inline them. But this __flush_dcache_icache_phys() is too
> big 
> to be worth inlining, yet small and stable enough to remain in
> assembly 
> for the time being.
> 
I disagree on this point, after converting it to C, using
44x/currituck.defconfig, the compiler definitely will inline it (noting
that there is only 1 caller of it):

00000134 <flush_dcache_icache_page>:
 134:   94 21 ff f0     stwu    r1,-16(r1)
 138:   3d 20 00 00     lis     r9,0
 13c:   81 29 00 00     lwz     r9,0(r9)
 140:   7c 08 02 a6     mflr    r0
 144:   38 81 00 0c     addi    r4,r1,12
 148:   90 01 00 14     stw     r0,20(r1)
 14c:   91 21 00 0c     stw     r9,12(r1)
 150:   48 00 00 01     bl      150 <flush_dcache_icache_page+0x1c>
 154:   39 00 00 20     li      r8,32
 158:   39 43 10 00     addi    r10,r3,4096
 15c:   7c 69 1b 78     mr      r9,r3
 160:   7d 09 03 a6     mtctr   r8
 164:   7c 00 48 6c     dcbst   0,r9
 168:   39 29 00 80     addi    r9,r9,128
 16c:   42 00 ff f8     bdnz    164 <flush_dcache_icache_page+0x30>
 170:   7c 00 04 ac     hwsync
 174:   7c 69 1b 78     mr      r9,r3
 178:   7c 00 4f ac     icbi    0,r9
 17c:   39 29 00 80     addi    r9,r9,128
 180:   7f 8a 48 40     cmplw   cr7,r10,r9
 184:   40 9e ff f4     bne     cr7,178 <flush_dcache_icache_page+0x44>
 188:   7c 00 04 ac     hwsync
 18c:   4c 00 01 2c     isync
 190:   80 01 00 14     lwz     r0,20(r1)
 194:   38 21 00 10     addi    r1,r1,16
 198:   7c 08 03 a6     mtlr    r0
 19c:   48 00 00 00     b       19c <flush_dcache_icache_page+0x68>


> So I suggest you keep it aside your series for now, just move 
> PURGE_PREFETCHED_INS inside it directly as it will be the only
> remaining 
> user of it.
> 
> Christophe

-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819


^ permalink raw reply

* Re: [PATCH 1/3] powerpc/sriov: Remove VF eeh_dev state when disabling SR-IOV
From: Sam Bobroff @ 2019-08-22  5:49 UTC (permalink / raw)
  To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <20190821062655.19735-1-oohall@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2389 bytes --]

On Wed, Aug 21, 2019 at 04:26:53PM +1000, Oliver O'Halloran wrote:
> When disabling virtual functions on an SR-IOV adapter we currently do not
> correctly remove the EEH state for the now-dead virtual functions. When
> removing the pci_dn that was created for the VF when SR-IOV was enabled
> we free the corresponding eeh_dev without removing it from the child device
> list of the eeh_pe that contained it. This can result in crashes due to the
> use-after-free.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> No Fixes: here since I'm not sure if the commit that added this actually
> introduced the bug. EEH is amazing.

Yep.

> I suspect backporting this would cause more problems than it solves since
> reliably replicating the crash required enabling memory poisoning and
> hacking a device driver to remove the PCI error handling callbacks so
> the EEH fallback path (which removes and re-probes PCI devices)
> would be used.

I gave this a quick test with some added instrumentation, and I can see
that the new code is used during VF removal and it doesn't cause any new
problems. I agree that even if it's difficult to trigger, it was
definitely a bug.

Reviewed-by: Sam Bobroff <sbobroff@linux.ibm.com>
Tested-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
>  arch/powerpc/kernel/pci_dn.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index 6556b57..795c4e3 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -244,9 +244,22 @@ void remove_dev_pci_data(struct pci_dev *pdev)
>  				continue;
>  
>  #ifdef CONFIG_EEH
> -			/* Release EEH device for the VF */
> +			/*
> +			 * Release EEH state for this VF. The PCI core
> +			 * has already torn down the pci_dev for this VF, but
> +			 * we're responsible to removing the eeh_dev since it
> +			 * has the same lifetime as the pci_dn that spawned it.
> +			 */
>  			edev = pdn_to_eeh_dev(pdn);
>  			if (edev) {
> +				/*
> +				 * We allocate pci_dn's for the totalvfs count,
> +				 * but only only the vfs that were activated
> +				 * have a configured PE.
> +				 */
> +				if (edev->pe)
> +					eeh_rmv_from_parent_pe(edev);
> +
>  				pdn->edev = NULL;
>  				kfree(edev);
>  			}
> -- 
> 2.9.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v3 1/2] powerpc/powernv: Enhance opal message read interface
From: Oliver O'Halloran @ 2019-08-22  5:51 UTC (permalink / raw)
  To: Vasant Hegde, linuxppc-dev; +Cc: Mahesh Salgaonkar, Jeremy Kerr
In-Reply-To: <20190821081335.20103-1-hegdevasant@linux.vnet.ibm.com>

On Wed, 2019-08-21 at 13:43 +0530, Vasant Hegde wrote:
> Use "opal-msg-size" device tree property to allocate memory for "opal_msg".
> 
> Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> Cc: Jeremy Kerr <jk@ozlabs.org>
> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> ---
> Changes in v3:
>   - Call BUG_ON, if we fail to allocate memory during init.
> 
> -Vasant
> 
>  arch/powerpc/platforms/powernv/opal.c | 29 ++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index aba443be7daa..4f1f68f568bf 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -58,6 +58,8 @@ static DEFINE_SPINLOCK(opal_write_lock);
>  static struct atomic_notifier_head opal_msg_notifier_head[OPAL_MSG_TYPE_MAX];
>  static uint32_t opal_heartbeat;
>  static struct task_struct *kopald_tsk;
> +static struct opal_msg *opal_msg;
> +static uint64_t opal_msg_size;
>  
>  void opal_configure_cores(void)
>  {
> @@ -271,14 +273,9 @@ static void opal_message_do_notify(uint32_t msg_type, void *msg)
>  static void opal_handle_message(void)
>  {
>  	s64 ret;
> -	/*
> -	 * TODO: pre-allocate a message buffer depending on opal-msg-size
> -	 * value in /proc/device-tree.
> -	 */
> -	static struct opal_msg msg;
>  	u32 type;
>  
> -	ret = opal_get_msg(__pa(&msg), sizeof(msg));
> +	ret = opal_get_msg(__pa(opal_msg), opal_msg_size);
>  	/* No opal message pending. */
>  	if (ret == OPAL_RESOURCE)
>  		return;
> @@ -290,14 +287,14 @@ static void opal_handle_message(void)
>  		return;
>  	}
>  
> -	type = be32_to_cpu(msg.msg_type);
> +	type = be32_to_cpu(opal_msg->msg_type);
>  
>  	/* Sanity check */
>  	if (type >= OPAL_MSG_TYPE_MAX) {
>  		pr_warn_once("%s: Unknown message type: %u\n", __func__, type);
>  		return;
>  	}
> -	opal_message_do_notify(type, (void *)&msg);
> +	opal_message_do_notify(type, (void *)opal_msg);
>  }
>  
>  static irqreturn_t opal_message_notify(int irq, void *data)
> @@ -306,9 +303,21 @@ static irqreturn_t opal_message_notify(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> -static int __init opal_message_init(void)
> +static int __init opal_message_init(struct device_node *opal_node)
>  {
>  	int ret, i, irq;

> +	const __be32 *val;
> +
> +	val = of_get_property(opal_node, "opal-msg-size", NULL);
> +	if (val)
> +		opal_msg_size = be32_to_cpup(val);

Use of_property_read_u32()

> +
> +	/* If opal-msg-size property is not available then use default size */
> +	if (!opal_msg_size)
> +		opal_msg_size = sizeof(struct opal_msg);
> +
> +	opal_msg = kmalloc(opal_msg_size, GFP_KERNEL);

> +	BUG_ON(opal_msg == NULL);

Seems questionable. Why not fall back to using a staticly allocated
struct opal_msg? Or re-try the allocation with the size limited to
sizeof(struct opal_msg)?

>  	for (i = 0; i < OPAL_MSG_TYPE_MAX; i++)
>  		ATOMIC_INIT_NOTIFIER_HEAD(&opal_msg_notifier_head[i]);
> @@ -910,7 +919,7 @@ static int __init opal_init(void)
>  	}
>  
>  	/* Initialise OPAL messaging system */
> -	opal_message_init();
> +	opal_message_init(opal_node);
>  
>  	/* Initialise OPAL asynchronous completion interface */
>  	opal_async_comp_init();


^ permalink raw reply

* [linux-next][PPC][bisected c7d8b7][gcc 6.4.1] build error at drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:1471
From: Abdul Haleem @ 2019-08-22  5:53 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Stephen Rothwell, jgg, linux-next, rcampbell, linux-kernel

Greeting's

Today's linux-next kernel 5.3.0-rc5-next-20190820 failed to build on my
powerpc machine

Build errors:
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c: In function amdgpu_exit:
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:1471:2: error: implicit
declaration of function mmu_notifier_synchronize
[-Werror=implicit-function-declaration]
  mmu_notifier_synchronize();
  ^~~~~~~~~~~~~~~~~~~~~~~~ 
cc1: some warnings being treated as errors
make[4]: *** [drivers/gpu/drm/amd/amdgpu/amdgpu_drv.o] Error 1
make[3]: *** [drivers/gpu/drm/amd/amdgpu] Error 2

It was introduced with commit c7d8b7 (hmm: use mmu_notifier_get/put for
'struct hmm')

error disappears when commit is reverted.

-- 
Regard's

Abdul Haleem
IBM Linux Technology Centre




^ permalink raw reply

* Re: [PATCH] crypto: vmx/xts - use fallback for ciphertext stealing
From: Herbert Xu @ 2019-08-22  5:57 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: nayna, ebiggers, pfsmorigo, linux-crypto, leitao, linuxppc-dev
In-Reply-To: <20190816140625.27053-1-ard.biesheuvel@linaro.org>

On Fri, Aug 16, 2019 at 05:06:24PM +0300, Ard Biesheuvel wrote:
> For correctness and compliance with the XTS-AES specification, we are
> adding support for ciphertext stealing to XTS implementations, even
> though no use cases are known that will be enabled by this.
> 
> Since the Power8 implementation already has a fallback skcipher standby
> for other purposes, let's use it for this purpose as well. If ciphertext
> stealing use cases ever become a bottleneck, we can always revisit this.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/crypto/vmx/aes_xts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 3/3] powerpc/pcidn: Warn when sriov pci_dn management is used incorrectly
From: Sam Bobroff @ 2019-08-22  5:59 UTC (permalink / raw)
  To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <20190821062655.19735-3-oohall@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]

On Wed, Aug 21, 2019 at 04:26:55PM +1000, Oliver O'Halloran wrote:
> These functions can only be used on a SR-IOV capable physical function and
> they're only called in pcibios_sriov_enable / disable. Make them emit a
> warning in the future if they're used incorrectly and remove the dead
> code that checks if the device is a VF.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

Looks good, but you might want to consider using WARN_ON_ONCE() just in
case it gets hit a lot.

Reviewed-by: Sam Bobroff <sbobroff@linux.ibm.com>

> ---
>  arch/powerpc/kernel/pci_dn.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index 24da1d8..69dafc3 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -158,8 +158,8 @@ struct pci_dn *add_sriov_vf_pdns(struct pci_dev *pdev)
>  	int i;
>  
>  	/* Only support IOV for now */
> -	if (!pdev->is_physfn)
> -		return pci_get_pdn(pdev);
> +	if (WARN_ON(!pdev->is_physfn))
> +		return NULL;
>  
>  	/* Check if VFs have been populated */
>  	pdn = pci_get_pdn(pdev);
> @@ -199,19 +199,8 @@ void remove_sriov_vf_pdns(struct pci_dev *pdev)
>  	struct pci_dn *pdn, *tmp;
>  	int i;
>  
> -	/*
> -	 * VF and VF PE are created/released dynamically, so we need to
> -	 * bind/unbind them.  Otherwise the VF and VF PE would be mismatched
> -	 * when re-enabling SR-IOV.
> -	 */
> -	if (pdev->is_virtfn) {
> -		pdn = pci_get_pdn(pdev);
> -		pdn->pe_number = IODA_INVALID_PE;
> -		return;
> -	}
> -
>  	/* Only support IOV PF for now */
> -	if (!pdev->is_physfn)
> +	if (WARN_ON(!pdev->is_physfn))
>  		return;
>  
>  	/* Check if VFs have been populated */
> -- 
> 2.9.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH 2/3] powerpc/pcidn: Make VF pci_dn management CONFIG_PCI_IOV specific
From: Sam Bobroff @ 2019-08-22  5:59 UTC (permalink / raw)
  To: Oliver O'Halloran; +Cc: linuxppc-dev
In-Reply-To: <20190821062655.19735-2-oohall@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 5882 bytes --]

On Wed, Aug 21, 2019 at 04:26:54PM +1000, Oliver O'Halloran wrote:
> The powerpc PCI code requires that a pci_dn structure exists for all
> devices in the system. This is fine for real devices since at boot a pci_dn
> is created for each PCI device in the DT and it's fine for hotplugged devices
> since the hotplug slot driver will manage the pci_dn's devices in hotplug
> slots. For SR-IOV, we need the platform / pcibios to manage the pci_dn for
> virtual functions since firmware is unaware of VFs, and they aren't
> "hot plugged" in the traditional sense.
> 
> Management of the pci_dn is handled by the, poorly named, functions:
> add_pci_dev_data() and remove_pci_dev_data(). The entire body of these
> functions is #ifdef`ed around CONFIG_PCI_IOV and they cannot be used
> in any other context, so make them only available when CONFIG_PCI_IOV
> is selected, and rename them to reflect their actual usage rather than
> having them masquerade as generic code.
> 
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

Nice cleanup,

Reviewed-by: Sam Bobroff <sbobroff@linux.ibm.com>

> ---
>  arch/powerpc/include/asm/pci-bridge.h     |  7 +++++--
>  arch/powerpc/kernel/pci_dn.c              | 15 +++++----------
>  arch/powerpc/platforms/powernv/pci-ioda.c |  4 ++--
>  arch/powerpc/platforms/pseries/pci.c      |  4 ++--
>  4 files changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index ea6ec65..69f4cb3 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -223,12 +223,15 @@ struct pci_dn {
>  extern struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
>  					   int devfn);
>  extern struct pci_dn *pci_get_pdn(struct pci_dev *pdev);
> -extern struct pci_dn *add_dev_pci_data(struct pci_dev *pdev);
> -extern void remove_dev_pci_data(struct pci_dev *pdev);
>  extern struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
>  					       struct device_node *dn);
>  extern void pci_remove_device_node_info(struct device_node *dn);
>  
> +#ifdef CONFIG_PCI_IOV
> +struct pci_dn *add_sriov_vf_pdns(struct pci_dev *pdev);
> +void remove_sriov_vf_pdns(struct pci_dev *pdev);
> +#endif
> +
>  static inline int pci_device_from_OF_node(struct device_node *np,
>  					  u8 *bus, u8 *devfn)
>  {
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index 795c4e3..24da1d8 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -125,7 +125,7 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
>  }
>  
>  #ifdef CONFIG_PCI_IOV
> -static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
> +static struct pci_dn *add_one_sriov_vf_pdn(struct pci_dn *parent,
>  					   int vf_index,
>  					   int busno, int devfn)
>  {
> @@ -151,11 +151,9 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>  
>  	return pdn;
>  }
> -#endif
>  
> -struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
> +struct pci_dn *add_sriov_vf_pdns(struct pci_dev *pdev)
>  {
> -#ifdef CONFIG_PCI_IOV
>  	struct pci_dn *parent, *pdn;
>  	int i;
>  
> @@ -176,7 +174,7 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>  	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
>  		struct eeh_dev *edev __maybe_unused;
>  
> -		pdn = add_one_dev_pci_data(parent, i,
> +		pdn = add_one_sriov_vf_pdn(parent, i,
>  					   pci_iov_virtfn_bus(pdev, i),
>  					   pci_iov_virtfn_devfn(pdev, i));
>  		if (!pdn) {
> @@ -192,14 +190,11 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>  		edev->physfn = pdev;
>  #endif /* CONFIG_EEH */
>  	}
> -#endif /* CONFIG_PCI_IOV */
> -
>  	return pci_get_pdn(pdev);
>  }
>  
> -void remove_dev_pci_data(struct pci_dev *pdev)
> +void remove_sriov_vf_pdns(struct pci_dev *pdev)
>  {
> -#ifdef CONFIG_PCI_IOV
>  	struct pci_dn *parent;
>  	struct pci_dn *pdn, *tmp;
>  	int i;
> @@ -271,8 +266,8 @@ void remove_dev_pci_data(struct pci_dev *pdev)
>  			kfree(pdn);
>  		}
>  	}
> -#endif /* CONFIG_PCI_IOV */
>  }
> +#endif /* CONFIG_PCI_IOV */
>  
>  struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
>  					struct device_node *dn)
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index d8080558d0..f1fa489 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -1719,14 +1719,14 @@ int pnv_pcibios_sriov_disable(struct pci_dev *pdev)
>  	pnv_pci_sriov_disable(pdev);
>  
>  	/* Release PCI data */
> -	remove_dev_pci_data(pdev);
> +	remove_sriov_vf_pdns(pdev);
>  	return 0;
>  }
>  
>  int pnv_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>  {
>  	/* Allocate PCI data */
> -	add_dev_pci_data(pdev);
> +	add_sriov_vf_pdns(pdev);
>  
>  	return pnv_pci_sriov_enable(pdev, num_vfs);
>  }
> diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
> index 7228309..911534b 100644
> --- a/arch/powerpc/platforms/pseries/pci.c
> +++ b/arch/powerpc/platforms/pseries/pci.c
> @@ -192,7 +192,7 @@ int pseries_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>  int pseries_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>  {
>  	/* Allocate PCI data */
> -	add_dev_pci_data(pdev);
> +	add_sriov_vf_pdns(pdev);
>  	return pseries_pci_sriov_enable(pdev, num_vfs);
>  }
>  
> @@ -204,7 +204,7 @@ int pseries_pcibios_sriov_disable(struct pci_dev *pdev)
>  	/* Releasing pe_num_map */
>  	kfree(pdn->pe_num_map);
>  	/* Release PCI data */
> -	remove_dev_pci_data(pdev);
> +	remove_sriov_vf_pdns(pdev);
>  	pci_vf_drivers_autoprobe(pdev, true);
>  	return 0;
>  }
> -- 
> 2.9.5
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [linux-next][PPC][bisected c7d8b7][gcc 6.4.1] build error at drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:1471
From: Stephen Rothwell @ 2019-08-22  6:00 UTC (permalink / raw)
  To: Abdul Haleem; +Cc: rcampbell, jgg, linux-next, linuxppc-dev, linux-kernel
In-Reply-To: <1566452811.526.7.camel@abdul.in.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

Hi Abdul,

On Thu, 22 Aug 2019 11:16:51 +0530 Abdul Haleem <abdhalee@linux.vnet.ibm.com> wrote:
>
> Today's linux-next kernel 5.3.0-rc5-next-20190820 failed to build on my
> powerpc machine
> 
> Build errors:
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c: In function amdgpu_exit:
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c:1471:2: error: implicit
> declaration of function mmu_notifier_synchronize
> [-Werror=implicit-function-declaration]
>   mmu_notifier_synchronize();
>   ^~~~~~~~~~~~~~~~~~~~~~~~ 
> cc1: some warnings being treated as errors
> make[4]: *** [drivers/gpu/drm/amd/amdgpu/amdgpu_drv.o] Error 1
> make[3]: *** [drivers/gpu/drm/amd/amdgpu] Error 2
> 
> It was introduced with commit c7d8b7 (hmm: use mmu_notifier_get/put for
> 'struct hmm')

This should have been fixed in next-20190821.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox