Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 4/5] ocxl: Add functions to map/unmap LPC memory
To:     "Alastair D'Silva" <alastair@au1.ibm.com>, alastair@d-silva.org
Cc:     Andrew Donnellan <ajd@linux.ibm.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-kernel@vger.kernel.org, Nicholas Piggin <npiggin@gmail.com>,
        Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>,
        Masahiro Yamada <yamada.masahiro@socionext.com>,
        Anju T Sudhakar <anju@linux.vnet.ibm.com>,
        Paul Mackerras <paulus@samba.org>,
        =?UTF-8?Q?C=c3=a9dric_Le_Goater?= <clg@kaod.org>,
        Vaibhav Jain <vaibhav@linux.ibm.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        linuxppc-dev@lists.ozlabs.org,
        Allison Randal <allison@lohutok.net>,
        David Gibson <david@gibson.dropbear.id.au>
References: <20190917014307.30485-1-alastair@au1.ibm.com>
 <20190917014307.30485-5-alastair@au1.ibm.com>
From:   Frederic Barrat <fbarrat@linux.ibm.com>
Date:   Mon, 23 Sep 2019 13:39:43 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.1.0
MIME-Version: 1.0
In-Reply-To: <20190917014307.30485-5-alastair@au1.ibm.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Message-Id: <b790c190-4800-6e2a-0d2b-e89f04d14dd9@linux.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
> index 2874811a4398..9e303a5f4d85 100644
> --- a/drivers/misc/ocxl/link.c
> +++ b/drivers/misc/ocxl/link.c
> @@ -738,7 +738,7 @@ int ocxl_link_add_lpc_mem(void *link_handle, u64 size)
>   }
>   EXPORT_SYMBOL_GPL(ocxl_link_add_lpc_mem);
>   
> -u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
> +u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev)
>   {
>   	struct ocxl_link *link = (struct ocxl_link *) link_handle;
>   

A bit of a nitpick, but is there any specific reason to rename with 
"online" suffix? I'm discovering it myself, but with memory hotplug, 
"onlining" seems to refer to the second, a.k.a logical memory hotplug 
phase (as described in Documentation/admin-guide/mm/memory-hotplug.rst). 
We'll need to worry about it, but the function here is really doing the 
first phase, a.k.a physical memory hotplug.

   Fred


> @@ -759,7 +759,7 @@ u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev)
>   	return link->lpc_mem;
>   }
>   
> -void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev)
> +void ocxl_link_lpc_offline(void *link_handle, struct pci_dev *pdev)
>   {
>   	struct ocxl_link *link = (struct ocxl_link *) link_handle;
>   
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index db2647a90fc8..5656a4aab5b7 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -52,6 +52,12 @@ struct ocxl_afu {
>   	void __iomem *global_mmio_ptr;
>   	u64 pp_mmio_start;
>   	void *private;
> +	u64 lpc_base_addr; /* Covers both LPC & special purpose memory */
> +	struct bin_attribute attr_global_mmio;
> +	struct bin_attribute attr_lpc_mem;
> +	struct resource lpc_res;
> +	struct bin_attribute attr_special_purpose_mem;
> +	struct resource special_purpose_res;
>   };
>   
>   enum ocxl_context_status {
> @@ -170,7 +176,7 @@ extern u64 ocxl_link_get_lpc_mem_sz(void *link_handle);
>    * @link_handle: The OpenCAPI link handle
>    * @pdev: A device that is on the link
>    */
> -u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
> +u64 ocxl_link_lpc_online(void *link_handle, struct pci_dev *pdev);
>   
>   /**
>    * Release the LPC memory device for an OpenCAPI device
> @@ -181,6 +187,6 @@ u64 ocxl_link_lpc_map(void *link_handle, struct pci_dev *pdev);
>    * @link_handle: The OpenCAPI link handle
>    * @pdev: A device that is on the link
>    */
> -void ocxl_link_lpc_release(void *link_handle, struct pci_dev *pdev);
> +void ocxl_link_lpc_offline(void *link_handle, struct pci_dev *pdev);
>   
>   #endif /* _OCXL_INTERNAL_H_ */
> diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> index 06dd5839e438..a1897737908d 100644
> --- a/include/misc/ocxl.h
> +++ b/include/misc/ocxl.h
> @@ -212,6 +212,24 @@ int ocxl_irq_set_handler(struct ocxl_context *ctx, int irq_id,
>   
>   // AFU Metadata
>   
> +/**
> + * Map the LPC system & special purpose memory for an AFU
> + *
> + * Do not call this during device discovery, as there may me multiple
> + * devices on a link, and the memory is mapped for the whole link, not
> + * just one device. It should only be called after all devices have
> + * registered their memory on the link.
> + *
> + * afu: The AFU that has the LPC memory to map
> + */
> +extern int ocxl_map_lpc_mem(struct ocxl_afu *afu);
> +
> +/**
> + * Get the physical address range of LPC memory for an AFU
> + * afu: The AFU associated with the LPC memory
> + */
> +extern struct resource *ocxl_afu_lpc_mem(struct ocxl_afu *afu);
> +
>   /**
>    * Get a pointer to the config for an AFU
>    *
>