[Skiboot] [PATCH 16/16] [PATCH 16/16] opencapi5: Add support for OpenCAPI Persistent Memory devices.

[Frederic Barrat]

On 20/08/2021 11:45, Christophe Lombard wrote:
> Lowest Point of Coherency (LPC) memory allows the host to access memory on
> an OpenCAPI device.
> 
> When the P10 chip accesses memory addresses on the AFU, the Real Address
> on the PowerBus must hit a BAR in the PAU such as GPU-Memory BAR. The BAR
> defines the range of Real Addresses that represent AFU memory.
> 
> The two existing OPAL calls, OPAL_NPU_MEM_ALLOC and OPAL_NPU_MEM_RELEASE
> are used to manage the AFU momory.
> 
> Signed-off-by: Christophe Lombard <clombard at linux.vnet.ibm.com>
> ---
>   hw/npu-opal.c      | 35 +++++++++++++++++
>   hw/npu2-opencapi.c | 18 ++-------
>   hw/pau.c           | 93 ++++++++++++++++++++++++++++++++++++++++++++++
>   hw/phys-map.c      |  3 ++
>   include/npu2.h     |  3 ++
>   include/pau-regs.h |  8 ++++
>   include/pau.h      |  4 ++
>   7 files changed, 149 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/npu-opal.c b/hw/npu-opal.c
> index 50aa8675..0f0b7bbe 100644
> --- a/hw/npu-opal.c
> +++ b/hw/npu-opal.c
> @@ -252,3 +252,38 @@ static int64_t opal_npu_tl_set(uint64_t phb_id, uint32_t bdfn,
>   	return rc;
>   }
>   opal_call(OPAL_NPU_TL_SET, opal_npu_tl_set, 5);
> +
> +static int64_t opal_npu_mem_alloc(uint64_t phb_id, uint32_t bdfn,
> +				  uint64_t size, uint64_t *bar)
> +{
> +	struct phb *phb = pci_get_phb(phb_id);
> +	int64_t rc = OPAL_SUCCESS;
> +
> +	if (!phb)
> +		return OPAL_PARAMETER;
> +
> +	if (phb->phb_type == phb_type_npu_v2_opencapi)
> +		rc = npu2_opencapi_mem_alloc(phb, bdfn, size, bar);
> +	else if (phb->phb_type == phb_type_pau_opencapi)
> +		rc = pau_opencapi_mem_alloc(phb, bdfn, size, bar);
> +
> +	return rc;

As mentioned in the very first patch, the call succeeds if the phb is 
not of type opencapi.
Why not code all those opal calls with the model of opal_npu_map_lpar() ?


> diff --git a/hw/phys-map.c b/hw/phys-map.c
> index 7b44fc61..68d7cd0d 100644
> --- a/hw/phys-map.c
> +++ b/hw/phys-map.c
> @@ -32,6 +32,9 @@ static const struct phys_map_entry phys_map_table_p10[] = {
> 
>   	/* TODO: Figure out GPU memory */
> 
> +	{ OCAPI_MEM,	   0, 0x0002000000000000ull, 0x0000040000000000ull },
> +	{ OCAPI_MEM,	   1, 0x0002004000000000ull, 0x0000040000000000ull },
> +


The 2 ranges overlap (and could be adjusted for P10).
We need a comment to explain that we use a hole in the non-mirrored 
memory range, like for P9. However, there's a lot more space available 
on P10 by allocating a new topology ID.

   Fred