linux-kernel - Re: [PATCH v2 1/2] bus: mc-bus: Add support for mapping shareable portals

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cf672a75-0f44-0202-b8f4-000fac80e97a@nxp.com>
Date:   Mon, 8 Apr 2019 09:20:30 +0000
From:   Laurentiu Tudor <laurentiu.tudor@....com>
To:     Roy Pledge <roy.pledge@....com>,
        "stuyoder@...il.com" <stuyoder@...il.com>,
        Leo Li <leoyang.li@....com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 1/2] bus: mc-bus: Add support for mapping shareable
 portals

Hi Roy,

On 05.04.2019 17:41, Roy Pledge wrote:
> Starting with v5 of NXP QBMan devices the hardware supports using
> regular cacheable/shareable memory as the backing store for the
> portals.
> 
> This patch adds support for the new portal mode by switching to
> use the DPRC get object region v2 command which returns both
> a base address and offset for the portal memory. The new portal
> region is identified as shareable through the addition of a new
> flag.
> 
> Signed-off-by: Roy Pledge <roy.pledge@....com>

Looks good to me:

Reviewed-by: Laurentiu Tudor <laurentiu.tudor@....com>

---
Best Regards, Laurentiu

> ---
>   drivers/bus/fsl-mc/dprc.c           | 30 +++++++++++++++++++++++++++---
>   drivers/bus/fsl-mc/fsl-mc-bus.c     | 15 +++++++++++++--
>   drivers/bus/fsl-mc/fsl-mc-private.h | 17 +++++++++++++++--
>   3 files changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/bus/fsl-mc/dprc.c b/drivers/bus/fsl-mc/dprc.c
> index 1c3f621..0fe3f52 100644
> --- a/drivers/bus/fsl-mc/dprc.c
> +++ b/drivers/bus/fsl-mc/dprc.c
> @@ -443,11 +443,31 @@ int dprc_get_obj_region(struct fsl_mc_io *mc_io,
>   	struct fsl_mc_command cmd = { 0 };
>   	struct dprc_cmd_get_obj_region *cmd_params;
>   	struct dprc_rsp_get_obj_region *rsp_params;
> +	u16 major_ver, minor_ver;
>   	int err;
>   
>   	/* prepare command */
> -	cmd.header = mc_encode_cmd_header(DPRC_CMDID_GET_OBJ_REG,
> -					  cmd_flags, token);
> +	err = dprc_get_api_version(mc_io, 0,
> +				     &major_ver,
> +				     &minor_ver);
> +	if (err)
> +		return err;
> +
> +	/**
> +	 * MC API version 6.3 introduced a new field to the region
> +	 * descriptor: base_address. If the older API is in use then the base
> +	 * address is set to zero to indicate it needs to be obtained elsewhere
> +	 * (typically the device tree).
> +	 */
> +	if (major_ver > 6 || (major_ver == 6 && minor_ver >= 3))
> +		cmd.header =
> +			mc_encode_cmd_header(DPRC_CMDID_GET_OBJ_REG_V2,
> +					     cmd_flags, token);
> +	else
> +		cmd.header =
> +			mc_encode_cmd_header(DPRC_CMDID_GET_OBJ_REG,
> +					     cmd_flags, token);
> +
>   	cmd_params = (struct dprc_cmd_get_obj_region *)cmd.params;
>   	cmd_params->obj_id = cpu_to_le32(obj_id);
>   	cmd_params->region_index = region_index;
> @@ -461,8 +481,12 @@ int dprc_get_obj_region(struct fsl_mc_io *mc_io,
>   
>   	/* retrieve response parameters */
>   	rsp_params = (struct dprc_rsp_get_obj_region *)cmd.params;
> -	region_desc->base_offset = le64_to_cpu(rsp_params->base_addr);
> +	region_desc->base_offset = le64_to_cpu(rsp_params->base_offset);
>   	region_desc->size = le32_to_cpu(rsp_params->size);
> +	if (major_ver > 6 || (major_ver == 6 && minor_ver >= 3))
> +		region_desc->base_address = le64_to_cpu(rsp_params->base_addr);
> +	else
> +		region_desc->base_address = 0;
>   
>   	return 0;
>   }
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index f0404c6..5c9bf2e 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -487,10 +487,19 @@ static int fsl_mc_device_get_mmio_regions(struct fsl_mc_device *mc_dev,
>   				"dprc_get_obj_region() failed: %d\n", error);
>   			goto error_cleanup_regions;
>   		}
> -
> -		error = translate_mc_addr(mc_dev, mc_region_type,
> +		/*
> +		 * Older MC only returned region offset and no base address
> +		 * If base address is in the region_desc use it otherwise
> +		 * revert to old mechanism
> +		 */
> +		if (region_desc.base_address)
> +			regions[i].start = region_desc.base_address +
> +						region_desc.base_offset;
> +		else
> +			error = translate_mc_addr(mc_dev, mc_region_type,
>   					  region_desc.base_offset,
>   					  &regions[i].start);
> +
>   		if (error < 0) {
>   			dev_err(parent_dev,
>   				"Invalid MC offset: %#x (for %s.%d\'s region %d)\n",
> @@ -504,6 +513,8 @@ static int fsl_mc_device_get_mmio_regions(struct fsl_mc_device *mc_dev,
>   		regions[i].flags = IORESOURCE_IO;
>   		if (region_desc.flags & DPRC_REGION_CACHEABLE)
>   			regions[i].flags |= IORESOURCE_CACHEABLE;
> +		if (region_desc.flags & DPRC_REGION_SHAREABLE)
> +			regions[i].flags |= IORESOURCE_MEM;
>   	}
>   
>   	mc_dev->regions = regions;
> diff --git a/drivers/bus/fsl-mc/fsl-mc-private.h b/drivers/bus/fsl-mc/fsl-mc-private.h
> index ea11b4f..020fcc0 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-private.h
> +++ b/drivers/bus/fsl-mc/fsl-mc-private.h
> @@ -79,9 +79,11 @@ int dpmcp_reset(struct fsl_mc_io *mc_io,
>   
>   /* DPRC command versioning */
>   #define DPRC_CMD_BASE_VERSION			1
> +#define DPRC_CMD_2ND_VERSION			2
>   #define DPRC_CMD_ID_OFFSET			4
>   
>   #define DPRC_CMD(id)	(((id) << DPRC_CMD_ID_OFFSET) | DPRC_CMD_BASE_VERSION)
> +#define DPRC_CMD_V2(id)	(((id) << DPRC_CMD_ID_OFFSET) | DPRC_CMD_2ND_VERSION)
>   
>   /* DPRC command IDs */
>   #define DPRC_CMDID_CLOSE                        DPRC_CMD(0x800)
> @@ -100,6 +102,7 @@ int dpmcp_reset(struct fsl_mc_io *mc_io,
>   #define DPRC_CMDID_GET_OBJ_COUNT                DPRC_CMD(0x159)
>   #define DPRC_CMDID_GET_OBJ                      DPRC_CMD(0x15A)
>   #define DPRC_CMDID_GET_OBJ_REG                  DPRC_CMD(0x15E)
> +#define DPRC_CMDID_GET_OBJ_REG_V2               DPRC_CMD_V2(0x15E)
>   #define DPRC_CMDID_SET_OBJ_IRQ                  DPRC_CMD(0x15F)
>   
>   struct dprc_cmd_open {
> @@ -199,9 +202,16 @@ struct dprc_rsp_get_obj_region {
>   	/* response word 0 */
>   	__le64 pad;
>   	/* response word 1 */
> -	__le64 base_addr;
> +	__le64 base_offset;
>   	/* response word 2 */
>   	__le32 size;
> +	__le32 pad2;
> +	/* response word 3 */
> +	__le32 flags;
> +	__le32 pad3;
> +	/* response word 4 */
> +	/* base_addr may be zero if older MC firmware is used */
> +	__le64 base_addr;
>   };
>   
>   struct dprc_cmd_set_obj_irq {
> @@ -334,6 +344,7 @@ int dprc_set_obj_irq(struct fsl_mc_io *mc_io,
>   /* Region flags */
>   /* Cacheable - Indicates that region should be mapped as cacheable */
>   #define DPRC_REGION_CACHEABLE	0x00000001
> +#define DPRC_REGION_SHAREABLE	0x00000002
>   
>   /**
>    * enum dprc_region_type - Region type
> @@ -342,7 +353,8 @@ int dprc_set_obj_irq(struct fsl_mc_io *mc_io,
>    */
>   enum dprc_region_type {
>   	DPRC_REGION_TYPE_MC_PORTAL,
> -	DPRC_REGION_TYPE_QBMAN_PORTAL
> +	DPRC_REGION_TYPE_QBMAN_PORTAL,
> +	DPRC_REGION_TYPE_QBMAN_MEM_BACKED_PORTAL
>   };
>   
>   /**
> @@ -360,6 +372,7 @@ struct dprc_region_desc {
>   	u32 size;
>   	u32 flags;
>   	enum dprc_region_type type;
> +	u64 base_address;
>   };
>   
>   int dprc_get_obj_region(struct fsl_mc_io *mc_io,
>