Re: [PATCH] drm/amdgpu: SR-IOV data exchange between PF&VF

["Christian König" <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Am 09.10.2017 um 12:29 schrieb Horace Chen:
> SR-IOV need to exchange some data between PF&VF through shared VRAM
>
> PF will copy some necessary firmware and information to the shared
> VRAM. It also requires some information from VF. PF will send a
> key through mailbox2 to help guest calculate checksum so that it can
> verify whether the data is correct.
>
> So check the data on the specified offset of the shared VRAM, if the
> checksum is right, read values from it and write some VF information
> next to the data from PF.
>
> Signed-off-by: Horace Chen <horace.chen@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   3 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   |  75 ++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h   | 185 +++++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c      |   6 +
>   4 files changed, 269 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index a964241..2275427 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2343,6 +2343,9 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   	if (r)
>   		DRM_ERROR("ib ring test failed (%d).\n", r);
>   
> +	if (amdgpu_sriov_vf(adev))
> +		amdgpu_virt_init_data_exchange(adev);
> +
>   	amdgpu_fbdev_init(adev);
>   
>   	r = amdgpu_pm_sysfs_init(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index ab05121..d877fe8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -274,3 +274,78 @@ void amdgpu_virt_free_mm_table(struct amdgpu_device *adev)
>   			      (void *)&adev->virt.mm_table.cpu_addr);
>   	adev->virt.mm_table.gpu_addr = 0;
>   }
> +
> +
> +int amdgpu_virt_fw_reserve_get_checksum(void *obj,
> +					unsigned long obj_size,
> +					unsigned int key,
> +					unsigned int chksum)
> +{
> +	unsigned int ret = key;
> +	unsigned long i = 0;
> +	unsigned char *pos;
> +
> +	pos = (char *)obj;
> +	//calculate checksum
> +	for (i = 0; i < obj_size; ++i)
> +		ret += *(pos + i);
> +	//minus the chksum itself
> +	pos = (char *)&chksum;
> +	for (i = 0; i < sizeof(chksum); ++i)
> +		ret -= *(pos + i);
> +	return ret;
> +}
> +
> +void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev)
> +{
> +	uint32_t pf2vf_ver = 0;
> +	uint32_t pf2vf_size = 0;
> +	uint32_t checksum = 0;
> +	char *str;
> +
> +	adev->virt.fw_reserve.p_pf2vf = NULL;
> +	adev->virt.fw_reserve.p_vf2pf = NULL;
> +
> +	if (adev->fw_vram_usage.va != NULL) {
> +		adev->virt.fw_reserve.p_pf2vf =
> +			(struct amdgim_pf2vf_info_header *)(
> +			adev->fw_vram_usage.va +
> +			KB_TO_BYTE(AMDGIM_DATAEXCHANGE_OFFSET_KB)),
> +		pf2vf_ver = adev->virt.fw_reserve.p_pf2vf->version;
> +		AMDGPU_FW_VRAM_PF2VF_READ(adev, header.size, &pf2vf_size);
> +		AMDGPU_FW_VRAM_PF2VF_READ(adev, checksum, &checksum);
> +
> +		/*pf2vf message must be in 4K*/
> +		if (pf2vf_size > 0 && pf2vf_size < KB_TO_BYTE(4)) {
> +			if (amdgpu_virt_fw_reserve_get_checksum(
> +				adev->virt.fw_reserve.p_pf2vf, pf2vf_size,
> +				adev->virt.fw_reserve.checksum_key,
> +				checksum) == checksum){
> +				adev->virt.fw_reserve.p_vf2pf =
> +					((void *)adev->virt.fw_reserve.p_pf2vf +
> +					pf2vf_size);
> +				memset((void *)adev->virt.fw_reserve.p_vf2pf, 0,
> +					sizeof(amdgim_vf2pf_info));
> +				AMDGPU_FW_VRAM_VF2PF_WRITE(adev, header.version,
> +					AMDGPU_FW_VRAM_VF2PF_VER);
> +				AMDGPU_FW_VRAM_VF2PF_WRITE(adev, header.size,
> +					sizeof(amdgim_vf2pf_info));
> +				AMDGPU_FW_VRAM_VF2PF_READ(adev, driver_version,
> +					&str);
> +				if (THIS_MODULE->version != NULL)
> +					strcpy(str, THIS_MODULE->version);
> +				else
> +					strcpy(str, "N/A");
> +				AMDGPU_FW_VRAM_VF2PF_WRITE(adev, driver_cert,
> +					0);
> +				AMDGPU_FW_VRAM_VF2PF_WRITE(adev, checksum,
> +					amdgpu_virt_fw_reserve_get_checksum(
> +					adev->virt.fw_reserve.p_vf2pf,
> +					pf2vf_size,
> +					adev->virt.fw_reserve.checksum_key, 0));
> +			}
> +		}
> +	}
> +}
> +
> +
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> index e5fd0ff..13e1fda 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> @@ -58,6 +58,186 @@ struct amdgpu_virt_ops {
>   	void (*trans_msg)(struct amdgpu_device *adev, u32 req, u32 data1, u32 data2, u32 data3);
>   };
>   
> +/*
> + * Firmware Reserve Frame buffer
> + */
> +struct amdgpu_virt_fw_reserve {
> +	struct amdgim_pf2vf_info_header *p_pf2vf;
> +	struct amdgim_vf2pf_info_header *p_vf2pf;
> +	unsigned int checksum_key;
> +};
> +/*
> + * Defination between PF and VF
> + */
> +#pragma pack(push)
> +#pragma pack(4)

IIRC we don't use pragmas in kernel code. Instead use the __attribute__ 
on the affected structures.

Apart from that at least the style looks good to me, but I can't judge 
technical correctness.

Christian.

> +
> +#define AMDGIM_DATAEXCHANGE_OFFSET_KB           (64)
> +#define KB_TO_BYTE(x)                           ((x) << 10)
> +#define BYTE_TO_KB(x)                           ((x) >> 10)
> +
> +#define AMDGIM_GET_STRUCTURE_RESERVED_SIZE(total, u8, u16, u32, u64) \
> +		(total - (((u8)+3) / 4 + ((u16)+1) / 2 + (u32) + (u64)*2))
> +
> +enum AMDGIM_FEATURE_FLAG {
> +	/* GIM supports feature of Error log collecting */
> +	AMDGIM_FEATURE_ERROR_LOG_COLLECT = 0x1,
> +	/* GIM supports feature of loading uCodes */
> +	AMDGIM_FEATURE_GIM_LOAD_UCODES   = 0x2,
> +};
> +
> +struct amdgim_pf2vf_info_header {
> +	/* the total structure size in byte. */
> +	uint32_t size;
> +	/* version of this structure, written by the GIM */
> +	uint32_t version;
> +};
> +struct  amdgim_pf2vf_info_v1 {
> +	/* header contains size and version */
> +	struct amdgim_pf2vf_info_header header;
> +	/* max_width * max_height */
> +	unsigned int uvd_enc_max_pixels_count;
> +	/* 16x16 pixels/sec, codec independent */
> +	unsigned int uvd_enc_max_bandwidth;
> +	/* max_width * max_height */
> +	unsigned int vce_enc_max_pixels_count;
> +	/* 16x16 pixels/sec, codec independent */
> +	unsigned int vce_enc_max_bandwidth;
> +	/* MEC FW position in kb from the start of visible frame buffer */
> +	unsigned int mecfw_kboffset;
> +	/* The features flags of the GIM driver supports. */
> +	unsigned int feature_flags;
> +	/* use private key from mailbox 2 to create chueksum */
> +	unsigned int checksum;
> +};
> +
> +struct  amdgim_pf2vf_info_v2 {
> +	/* header contains size and version */
> +	struct amdgim_pf2vf_info_header header;
> +	/* use private key from mailbox 2 to create chueksum */
> +	unsigned int checksum;
> +	/* The features flags of the GIM driver supports. */
> +	unsigned int feature_flags;
> +	/* max_width * max_height */
> +	unsigned int uvd_enc_max_pixels_count;
> +	/* 16x16 pixels/sec, codec independent */
> +	unsigned int uvd_enc_max_bandwidth;
> +	/* max_width * max_height */
> +	unsigned int vce_enc_max_pixels_count;
> +	/* 16x16 pixels/sec, codec independent */
> +	unsigned int vce_enc_max_bandwidth;
> +	/* MEC FW position in kb from the start of VF visible frame buffer */
> +	unsigned int mecfw_kboffset;
> +	/* MEC FW size in KB */
> +	unsigned int mecfw_ksize;
> +	/* UVD FW position in kb from the start of VF visible frame buffer */
> +	unsigned int uvdfw_kboffset;
> +	/* UVD FW size in KB */
> +	unsigned int uvdfw_ksize;
> +	/* VCE FW position in kb from the start of VF visible frame buffer */
> +	unsigned int vcefw_kboffset;
> +	/* VCE FW size in KB */
> +	unsigned int vcefw_ksize;
> +	unsigned int reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256, 0, 0, (9 + sizeof(struct amdgim_pf2vf_info_header)/sizeof(uint32_t)), 3)];
> +};
> +
> +
> +struct amdgim_vf2pf_info_header {
> +	/* the total structure size in byte. */
> +	uint32_t size;
> +	/*version of this structure, written by the guest */
> +	uint32_t version;
> +};
> +
> +struct amdgim_vf2pf_info_v1 {
> +	/* header contains size and version */
> +	struct amdgim_vf2pf_info_header header;
> +	/* driver version */
> +	char driver_version[64];
> +	/* driver certification, 1=WHQL, 0=None */
> +	unsigned int driver_cert;
> +	/* guest OS type and version: need a define */
> +	unsigned int os_info;
> +	/* in the unit of 1M */
> +	unsigned int fb_usage;
> +	/* guest gfx engine usage percentage */
> +	unsigned int gfx_usage;
> +	/* guest gfx engine health percentage */
> +	unsigned int gfx_health;
> +	/* guest compute engine usage percentage */
> +	unsigned int compute_usage;
> +	/* guest compute engine health percentage */
> +	unsigned int compute_health;
> +	/* guest vce engine usage percentage. 0xffff means N/A. */
> +	unsigned int vce_enc_usage;
> +	/* guest vce engine health percentage. 0xffff means N/A. */
> +	unsigned int vce_enc_health;
> +	/* guest uvd engine usage percentage. 0xffff means N/A. */
> +	unsigned int uvd_enc_usage;
> +	/* guest uvd engine usage percentage. 0xffff means N/A. */
> +	unsigned int uvd_enc_health;
> +	unsigned int checksum;
> +};
> +
> +struct amdgim_vf2pf_info_v2 {
> +	/* header contains size and version */
> +	struct amdgim_vf2pf_info_header header;
> +	unsigned int checksum;
> +	/* driver version */
> +	char driver_version[64];
> +	/* driver certification, 1=WHQL, 0=None */
> +	unsigned int driver_cert;
> +	/* guest OS type and version: need a define */
> +	unsigned int os_info;
> +	/* in the unit of 1M */
> +	unsigned int fb_usage;
> +	/* guest gfx engine usage percentage */
> +	unsigned int gfx_usage;
> +	/* guest gfx engine health percentage */
> +	unsigned int gfx_health;
> +	/* guest compute engine usage percentage */
> +	unsigned int compute_usage;
> +	/* guest compute engine health percentage */
> +	unsigned int compute_health;
> +	/* guest vce engine usage percentage. 0xffff means N/A. */
> +	unsigned int vce_enc_usage;
> +	/* guest vce engine health percentage. 0xffff means N/A. */
> +	unsigned int vce_enc_health;
> +	/* guest uvd engine usage percentage. 0xffff means N/A. */
> +	unsigned int uvd_enc_usage;
> +	/* guest uvd engine usage percentage. 0xffff means N/A. */
> +	unsigned int uvd_enc_health;
> +	unsigned int reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256, 64, 0, (12 + sizeof(struct amdgim_vf2pf_info_header)/sizeof(uint32_t)), 0)];
> +};
> +
> +#define AMDGPU_FW_VRAM_VF2PF_VER 2
> +typedef struct amdgim_vf2pf_info_v2 amdgim_vf2pf_info ;
> +
> +#define AMDGPU_FW_VRAM_VF2PF_WRITE(adev, field, val) \
> +	do { \
> +		((amdgim_vf2pf_info *)adev->virt.fw_reserve.p_vf2pf)->field = (val); \
> +	} while (0)
> +
> +#define AMDGPU_FW_VRAM_VF2PF_READ(adev, field, val) \
> +	do { \
> +		(*val) = ((amdgim_vf2pf_info *)adev->virt.fw_reserve.p_vf2pf)->field; \
> +	} while (0)
> +
> +#define AMDGPU_FW_VRAM_PF2VF_READ(adev, field, val) \
> +	do { \
> +		if (!adev->virt.fw_reserve.p_pf2vf) \
> +			*(val) = 0; \
> +		else { \
> +			if (adev->virt.fw_reserve.p_pf2vf->version == 1) \
> +				*(val) = ((struct amdgim_pf2vf_info_v1 *)adev->virt.fw_reserve.p_pf2vf)->field; \
> +			if (adev->virt.fw_reserve.p_pf2vf->version == 2) \
> +				*(val) = ((struct amdgim_pf2vf_info_v2 *)adev->virt.fw_reserve.p_pf2vf)->field; \
> +		} \
> +	} while (0)
> +
> +#pragma pack(pop)
> +
> +
>   /* GPU virtualization */
>   struct amdgpu_virt {
>   	uint32_t			caps;
> @@ -72,6 +252,7 @@ struct amdgpu_virt {
>   	struct amdgpu_mm_table		mm_table;
>   	const struct amdgpu_virt_ops	*ops;
>   	struct amdgpu_vf_error_buffer   vf_errors;
> +	struct amdgpu_virt_fw_reserve	fw_reserve;
>   };
>   
>   #define AMDGPU_CSA_SIZE    (8 * 1024)
> @@ -114,5 +295,9 @@ int amdgpu_virt_reset_gpu(struct amdgpu_device *adev);
>   int amdgpu_sriov_gpu_reset(struct amdgpu_device *adev, struct amdgpu_job *job);
>   int amdgpu_virt_alloc_mm_table(struct amdgpu_device *adev);
>   void amdgpu_virt_free_mm_table(struct amdgpu_device *adev);
> +int amdgpu_virt_fw_reserve_get_checksum(void *obj, unsigned long obj_size,
> +					unsigned int key,
> +					unsigned int chksum);
> +void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev);
>   
>   #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
> index 2812d88..b4906d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
> @@ -183,6 +183,12 @@ static int xgpu_ai_send_access_requests(struct amdgpu_device *adev,
>   			pr_err("Doesn't get READY_TO_ACCESS_GPU from pf, give up\n");
>   			return r;
>   		}
> +		/* Retrieve checksum from mailbox2 */
> +		if (req == IDH_REQ_GPU_INIT_ACCESS) {
> +			adev->virt.fw_reserve.checksum_key =
> +				RREG32_NO_KIQ(SOC15_REG_OFFSET(NBIO, 0,
> +					mmBIF_BX_PF0_MAILBOX_MSGBUF_RCV_DW2));
> +		}
>   	}
>   
>   	return 0;


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx