* [PATCH V2] drm/amdgpu: support reserve bad page for virt
@ 2020-06-04 12:36 Stanley.Yang
2020-06-05 2:24 ` Chen, Guchun
2020-06-05 3:00 ` Zhou1, Tao
0 siblings, 2 replies; 6+ messages in thread
From: Stanley.Yang @ 2020-06-04 12:36 UTC (permalink / raw)
To: amd-gfx
Cc: Guchun.Chen, Tao.Zhou1, Stanley.Yang, Monk.Liu, John.Clements,
Dennis.Li, Hawking.Zhang
Changed from V1:
rename same functions name, only init ras error handler data for
supported asic.
Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
Change-Id: Ia0ad9453ac3ac929f95c73cbee5b7a8fc42a9816
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 172 +++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 30 +++-
3 files changed, 201 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1df28b7bf22e..668ad0e35160 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2326,6 +2326,9 @@ static int amdgpu_device_ip_fini(struct amdgpu_device *adev)
{
int i, r;
+ if (amdgpu_sriov_vf(adev) && adev->virt.ras_init_done)
+ amdgpu_virt_release_ras_err_handler_data(adev);
+
amdgpu_ras_pre_fini(adev);
if (adev->gmc.xgmi.num_physical_nodes > 1)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index bab9286021a7..174fcb8c8b57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -26,6 +26,7 @@
#include <drm/drm_drv.h>
#include "amdgpu.h"
+#include "amdgpu_ras.h"
bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev)
{
@@ -255,12 +256,169 @@ int amdgpu_virt_fw_reserve_get_checksum(void *obj,
return ret;
}
+static int amdgpu_virt_init_ras_err_handler_data(struct amdgpu_device *adev)
+{
+ struct amdgpu_virt *virt = &adev->virt;
+ struct amdgpu_virt_ras_err_handler_data **data = &virt->virt_eh_data;
+ /* GPU will be marked bad on host if bp count more then 10,
+ * so alloc 512 is enough.
+ */
+ unsigned int align_space = 512;
+ void *bps = NULL;
+ struct amdgpu_bo **bps_bo = NULL;
+
+ *data = kmalloc(sizeof(struct amdgpu_virt_ras_err_handler_data), GFP_KERNEL);
+ if (!*data)
+ return -ENOMEM;
+
+ bps = kmalloc(align_space * sizeof((*data)->bps), GFP_KERNEL);
+ bps_bo = kmalloc(align_space * sizeof((*data)->bps_bo), GFP_KERNEL);
+
+ if (!bps || !bps_bo) {
+ kfree(bps);
+ kfree(bps_bo);
+ return -ENOMEM;
+ }
+
+ (*data)->bps = bps;
+ (*data)->bps_bo = bps_bo;
+ (*data)->count = 0;
+ (*data)->last_reserved = 0;
+
+ virt->ras_init_done = true;
+
+ return 0;
+}
+
+static void amdgpu_virt_ras_release_bp(struct amdgpu_device *adev)
+{
+ struct amdgpu_virt *virt = &adev->virt;
+ struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
+ struct amdgpu_bo *bo;
+ int i;
+
+ if (!data)
+ return;
+
+ for (i = data->last_reserved - 1; i >= 0; i--) {
+ bo = data->bps_bo[i];
+ amdgpu_bo_free_kernel(&bo, NULL, NULL);
+ data->bps_bo[i] = bo;
+ data->last_reserved = i;
+ }
+}
+
+void amdgpu_virt_release_ras_err_handler_data(struct amdgpu_device *adev)
+{
+ struct amdgpu_virt *virt = &adev->virt;
+ struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
+
+ virt->ras_init_done = false;
+
+ if (!data)
+ return;
+
+ amdgpu_virt_ras_release_bp(adev);
+
+ kfree(data->bps);
+ kfree(data->bps_bo);
+ kfree(data);
+ virt->virt_eh_data = NULL;
+}
+
+static void amdgpu_virt_ras_add_bps(struct amdgpu_device *adev,
+ struct eeprom_table_record *bps, int pages)
+{
+ struct amdgpu_virt *virt = &adev->virt;
+ struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
+
+ if (!data)
+ return;
+
+ memcpy(&data->bps[data->count], bps, pages * sizeof(*data->bps));
+ data->count += pages;
+}
+
+static void amdgpu_virt_ras_reserve_bps(struct amdgpu_device *adev)
+{
+ struct amdgpu_virt *virt = &adev->virt;
+ struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
+ struct amdgpu_bo *bo = NULL;
+ uint64_t bp;
+ int i;
+
+ if (!data)
+ return;
+
+ for (i = data->last_reserved; i < data->count; i++) {
+ bp = data->bps[i].retired_page;
+
+ /* There are two cases of reserve error should be ignored:
+ * 1) a ras bad page has been allocated (used by someone);
+ * 2) a ras bad page has been reserved (duplicate error injection
+ * for one page);
+ */
+ if (amdgpu_bo_create_kernel_at(adev, bp << AMDGPU_GPU_PAGE_SHIFT,
+ AMDGPU_GPU_PAGE_SIZE,
+ AMDGPU_GEM_DOMAIN_VRAM,
+ &bo, NULL))
+ DRM_DEBUG("RAS WARN: [SRV-IO] reserve vram for retired page %llx fail\n", bp);
+
+ data->bps_bo[i] = bo;
+ data->last_reserved = i + 1;
+ bo = NULL;
+ }
+}
+
+static bool amdgpu_virt_ras_check_bad_page(struct amdgpu_device *adev,
+ uint64_t retired_page)
+{
+ struct amdgpu_virt *virt = &adev->virt;
+ struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
+ int i;
+
+ if (!data)
+ return true;
+
+ for (i = 0; i < data->count; i++)
+ if (retired_page == data->bps[i].retired_page)
+ return true;
+
+ return false;
+}
+
+static void amdgpu_virt_add_bad_page(struct amdgpu_device *adev,
+ uint64_t bp_block_offset, uint32_t bp_block_size)
+{
+ struct eeprom_table_record bp;
+ uint64_t retired_page;
+ uint32_t bp_idx, bp_cnt;
+
+ if (bp_block_size) {
+ bp_cnt = bp_block_size / sizeof(uint64_t);
+ for (bp_idx = 0; bp_idx < bp_cnt; bp_idx++) {
+ retired_page = *(uint64_t *)(adev->fw_vram_usage.va +
+ bp_block_offset + bp_idx * sizeof(uint64_t));
+ bp.retired_page = retired_page;
+
+ if (amdgpu_virt_ras_check_bad_page(adev, retired_page))
+ continue;
+
+ amdgpu_virt_ras_add_bps(adev, &bp, 1);
+
+ amdgpu_virt_ras_reserve_bps(adev);
+ }
+ }
+}
+
void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev)
{
uint32_t pf2vf_size = 0;
uint32_t checksum = 0;
uint32_t checkval;
char *str;
+ uint64_t bp_block_offset = 0;
+ uint32_t bp_block_size = 0;
adev->virt.fw_reserve.p_pf2vf = NULL;
adev->virt.fw_reserve.p_vf2pf = NULL;
@@ -275,6 +433,20 @@ void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev)
/* pf2vf message must be in 4K */
if (pf2vf_size > 0 && pf2vf_size < 4096) {
+ if (adev->virt.fw_reserve.p_pf2vf->version == 2) {
+ struct amdgim_pf2vf_info_v2 *pf2vf_v2 = NULL;
+
+ pf2vf_v2 = (struct amdgim_pf2vf_info_v2 *)adev->virt.fw_reserve.p_pf2vf;
+ bp_block_offset = ((uint64_t)pf2vf_v2->bp_block_offset_L & 0xFFFFFFFF) |
+ ((((uint64_t)pf2vf_v2->bp_block_offset_H) << 32) & 0xFFFFFFFF00000000);
+ bp_block_size = pf2vf_v2->bp_block_size;
+
+ if (bp_block_size && !adev->virt.ras_init_done)
+ amdgpu_virt_init_ras_err_handler_data(adev);
+
+ amdgpu_virt_add_bad_page(adev, bp_block_offset, bp_block_size);
+ }
+
checkval = amdgpu_virt_fw_reserve_get_checksum(
adev->virt.fw_reserve.p_pf2vf, pf2vf_size,
adev->virt.fw_reserve.checksum_key, checksum);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
index b90e822cebd7..f826945989c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
@@ -143,19 +143,27 @@ struct amdgim_pf2vf_info_v2 {
uint32_t vce_enc_max_pixels_count;
/* 16x16 pixels/sec, codec independent */
uint32_t vce_enc_max_bandwidth;
+ /* Bad pages block position in BYTE */
+ uint32_t bp_block_offset_L;
+ uint32_t bp_block_offset_H;
+ /* Bad pages block size in BYTE */
+ uint32_t bp_block_size;
/* MEC FW position in kb from the start of VF visible frame buffer */
- uint64_t mecfw_kboffset;
+ uint32_t mecfw_kboffset_L;
+ uint32_t mecfw_kboffset_H;
/* MEC FW size in KB */
uint32_t mecfw_ksize;
/* UVD FW position in kb from the start of VF visible frame buffer */
- uint64_t uvdfw_kboffset;
+ uint32_t uvdfw_kboffset_L;
+ uint32_t uvdfw_kboffset_H;
/* UVD FW size in KB */
uint32_t uvdfw_ksize;
/* VCE FW position in kb from the start of VF visible frame buffer */
- uint64_t vcefw_kboffset;
+ uint32_t vcefw_kboffset_L;
+ uint32_t vcefw_kboffset_H;
/* VCE FW size in KB */
uint32_t vcefw_ksize;
- uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256, 0, 0, (9 + sizeof(struct amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)), 3)];
+ uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256, 0, 0, (18 + sizeof(struct amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)), 0)];
} __aligned(4);
@@ -254,6 +262,17 @@ typedef struct amdgim_vf2pf_info_v2 amdgim_vf2pf_info ;
} \
} while (0)
+struct amdgpu_virt_ras_err_handler_data {
+ /* point to bad page records array */
+ struct eeprom_table_record *bps;
+ /* point to reserved bo array */
+ struct amdgpu_bo **bps_bo;
+ /* the count of entries */
+ int count;
+ /* last reserved entry's index + 1 */
+ int last_reserved;
+};
+
/* GPU virtualization */
struct amdgpu_virt {
uint32_t caps;
@@ -272,6 +291,8 @@ struct amdgpu_virt {
uint32_t reg_access_mode;
int req_init_data_ver;
bool tdr_debug;
+ struct amdgpu_virt_ras_err_handler_data *virt_eh_data;
+ bool ras_init_done;
};
#define amdgpu_sriov_enabled(adev) \
@@ -323,6 +344,7 @@ 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_release_ras_err_handler_data(struct amdgpu_device *adev);
void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev);
void amdgpu_detect_virtualization(struct amdgpu_device *adev);
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH V2] drm/amdgpu: support reserve bad page for virt
2020-06-04 12:36 [PATCH V2] drm/amdgpu: support reserve bad page for virt Stanley.Yang
@ 2020-06-05 2:24 ` Chen, Guchun
2020-06-05 3:09 ` Yang, Stanley
2020-06-05 3:00 ` Zhou1, Tao
1 sibling, 1 reply; 6+ messages in thread
From: Chen, Guchun @ 2020-06-05 2:24 UTC (permalink / raw)
To: Yang, Stanley, amd-gfx
Cc: Zhou1, Tao, Yang, Stanley, Liu, Monk, Clements, John, Li, Dennis,
Zhang, Hawking
[AMD Public Use]
Please see my comments with prefix [Guchun].
Regards,
Guchun
-----Original Message-----
From: Stanley.Yang <Stanley.Yang@amd.com>
Sent: Thursday, June 4, 2020 8:36 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Chen, Guchun <Guchun.Chen@amd.com>; Liu, Monk <Monk.Liu@amd.com>; Clements, John <John.Clements@amd.com>; Zhou1, Tao <Tao.Zhou1@amd.com>; Li, Dennis <Dennis.Li@amd.com>; Yang, Stanley <Stanley.Yang@amd.com>
Subject: [PATCH V2] drm/amdgpu: support reserve bad page for virt
Changed from V1:
rename same functions name, only init ras error handler data for
supported asic.
[Guchun] 'same' is one typo? It should be some..
Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
Change-Id: Ia0ad9453ac3ac929f95c73cbee5b7a8fc42a9816
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 172 +++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 30 +++-
3 files changed, 201 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 1df28b7bf22e..668ad0e35160 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2326,6 +2326,9 @@ static int amdgpu_device_ip_fini(struct amdgpu_device *adev) {
int i, r;
+ if (amdgpu_sriov_vf(adev) && adev->virt.ras_init_done)
+ amdgpu_virt_release_ras_err_handler_data(adev);
+
amdgpu_ras_pre_fini(adev);
if (adev->gmc.xgmi.num_physical_nodes > 1) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index bab9286021a7..174fcb8c8b57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -26,6 +26,7 @@
#include <drm/drm_drv.h>
#include "amdgpu.h"
+#include "amdgpu_ras.h"
bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev) { @@ -255,12 +256,169 @@ int amdgpu_virt_fw_reserve_get_checksum(void *obj,
return ret;
}
+static int amdgpu_virt_init_ras_err_handler_data(struct amdgpu_device
+*adev) {
+ struct amdgpu_virt *virt = &adev->virt;
+ struct amdgpu_virt_ras_err_handler_data **data = &virt->virt_eh_data;
+ /* GPU will be marked bad on host if bp count more then 10,
+ * so alloc 512 is enough.
+ */
+ unsigned int align_space = 512;
+ void *bps = NULL;
+ struct amdgpu_bo **bps_bo = NULL;
+
+ *data = kmalloc(sizeof(struct amdgpu_virt_ras_err_handler_data), GFP_KERNEL);
+ if (!*data)
+ return -ENOMEM;
+
+ bps = kmalloc(align_space * sizeof((*data)->bps), GFP_KERNEL);
+ bps_bo = kmalloc(align_space * sizeof((*data)->bps_bo), GFP_KERNEL);
+
+ if (!bps || !bps_bo) {
+ kfree(bps);
+ kfree(bps_bo);
[Guchun]It's needed to release *data as well to prevent memory leak?
+ return -ENOMEM;
+ }
+
+ (*data)->bps = bps;
+ (*data)->bps_bo = bps_bo;
+ (*data)->count = 0;
+ (*data)->last_reserved = 0;
+
+ virt->ras_init_done = true;
+
+ return 0;
+}
+
+static void amdgpu_virt_ras_release_bp(struct amdgpu_device *adev) {
+ struct amdgpu_virt *virt = &adev->virt;
+ struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
+ struct amdgpu_bo *bo;
+ int i;
+
+ if (!data)
+ return;
+
+ for (i = data->last_reserved - 1; i >= 0; i--) {
+ bo = data->bps_bo[i];
+ amdgpu_bo_free_kernel(&bo, NULL, NULL);
+ data->bps_bo[i] = bo;
+ data->last_reserved = i;
+ }
+}
+
+void amdgpu_virt_release_ras_err_handler_data(struct amdgpu_device
+*adev) {
+ struct amdgpu_virt *virt = &adev->virt;
+ struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
+
+ virt->ras_init_done = false;
+
+ if (!data)
+ return;
+
+ amdgpu_virt_ras_release_bp(adev);
+
+ kfree(data->bps);
+ kfree(data->bps_bo);
+ kfree(data);
+ virt->virt_eh_data = NULL;
+}
+
+static void amdgpu_virt_ras_add_bps(struct amdgpu_device *adev,
+ struct eeprom_table_record *bps, int pages) {
+ struct amdgpu_virt *virt = &adev->virt;
+ struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
+
+ if (!data)
+ return;
+
+ memcpy(&data->bps[data->count], bps, pages * sizeof(*data->bps));
+ data->count += pages;
+}
+
+static void amdgpu_virt_ras_reserve_bps(struct amdgpu_device *adev) {
+ struct amdgpu_virt *virt = &adev->virt;
+ struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
+ struct amdgpu_bo *bo = NULL;
+ uint64_t bp;
+ int i;
+
+ if (!data)
+ return;
+
+ for (i = data->last_reserved; i < data->count; i++) {
+ bp = data->bps[i].retired_page;
+
+ /* There are two cases of reserve error should be ignored:
+ * 1) a ras bad page has been allocated (used by someone);
+ * 2) a ras bad page has been reserved (duplicate error injection
+ * for one page);
+ */
+ if (amdgpu_bo_create_kernel_at(adev, bp << AMDGPU_GPU_PAGE_SHIFT,
+ AMDGPU_GPU_PAGE_SIZE,
+ AMDGPU_GEM_DOMAIN_VRAM,
+ &bo, NULL))
+ DRM_DEBUG("RAS WARN: [SRV-IO] reserve vram for retired page %llx
+fail\n", bp);
+
+ data->bps_bo[i] = bo;
+ data->last_reserved = i + 1;
+ bo = NULL;
+ }
+}
+
+static bool amdgpu_virt_ras_check_bad_page(struct amdgpu_device *adev,
+ uint64_t retired_page)
+{
+ struct amdgpu_virt *virt = &adev->virt;
+ struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
+ int i;
+
+ if (!data)
+ return true;
+
+ for (i = 0; i < data->count; i++)
+ if (retired_page == data->bps[i].retired_page)
+ return true;
+
+ return false;
+}
+
+static void amdgpu_virt_add_bad_page(struct amdgpu_device *adev,
+ uint64_t bp_block_offset, uint32_t bp_block_size) {
+ struct eeprom_table_record bp;
+ uint64_t retired_page;
+ uint32_t bp_idx, bp_cnt;
+
+ if (bp_block_size) {
+ bp_cnt = bp_block_size / sizeof(uint64_t);
+ for (bp_idx = 0; bp_idx < bp_cnt; bp_idx++) {
+ retired_page = *(uint64_t *)(adev->fw_vram_usage.va +
+ bp_block_offset + bp_idx * sizeof(uint64_t));
+ bp.retired_page = retired_page;
+
+ if (amdgpu_virt_ras_check_bad_page(adev, retired_page))
+ continue;
+
+ amdgpu_virt_ras_add_bps(adev, &bp, 1);
+
+ amdgpu_virt_ras_reserve_bps(adev);
+ }
+ }
+}
+
void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev) {
uint32_t pf2vf_size = 0;
uint32_t checksum = 0;
uint32_t checkval;
char *str;
+ uint64_t bp_block_offset = 0;
+ uint32_t bp_block_size = 0;
adev->virt.fw_reserve.p_pf2vf = NULL;
adev->virt.fw_reserve.p_vf2pf = NULL;
@@ -275,6 +433,20 @@ void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev)
/* pf2vf message must be in 4K */
if (pf2vf_size > 0 && pf2vf_size < 4096) {
+ if (adev->virt.fw_reserve.p_pf2vf->version == 2) {
+ struct amdgim_pf2vf_info_v2 *pf2vf_v2 = NULL;
[Guchun]Move declare of struct amdgim_pf2vf_info_v2 *pf2vf_v2 = NULL to the first beginning of this function.
+ pf2vf_v2 = (struct amdgim_pf2vf_info_v2 *)adev->virt.fw_reserve.p_pf2vf;
+ bp_block_offset = ((uint64_t)pf2vf_v2->bp_block_offset_L & 0xFFFFFFFF) |
+ ((((uint64_t)pf2vf_v2->bp_block_offset_H) << 32) & 0xFFFFFFFF00000000);
+ bp_block_size = pf2vf_v2->bp_block_size;
+
+ if (bp_block_size && !adev->virt.ras_init_done)
+ amdgpu_virt_init_ras_err_handler_data(adev);
+
+ amdgpu_virt_add_bad_page(adev, bp_block_offset, bp_block_size);
+ }
+
checkval = amdgpu_virt_fw_reserve_get_checksum(
adev->virt.fw_reserve.p_pf2vf, pf2vf_size,
adev->virt.fw_reserve.checksum_key, checksum); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
index b90e822cebd7..f826945989c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
@@ -143,19 +143,27 @@ struct amdgim_pf2vf_info_v2 {
uint32_t vce_enc_max_pixels_count;
/* 16x16 pixels/sec, codec independent */
uint32_t vce_enc_max_bandwidth;
+ /* Bad pages block position in BYTE */
+ uint32_t bp_block_offset_L;
+ uint32_t bp_block_offset_H;
+ /* Bad pages block size in BYTE */
+ uint32_t bp_block_size;
/* MEC FW position in kb from the start of VF visible frame buffer */
- uint64_t mecfw_kboffset;
+ uint32_t mecfw_kboffset_L;
+ uint32_t mecfw_kboffset_H;
/* MEC FW size in KB */
uint32_t mecfw_ksize;
/* UVD FW position in kb from the start of VF visible frame buffer */
- uint64_t uvdfw_kboffset;
+ uint32_t uvdfw_kboffset_L;
+ uint32_t uvdfw_kboffset_H;
/* UVD FW size in KB */
uint32_t uvdfw_ksize;
/* VCE FW position in kb from the start of VF visible frame buffer */
- uint64_t vcefw_kboffset;
+ uint32_t vcefw_kboffset_L;
+ uint32_t vcefw_kboffset_H;
/* VCE FW size in KB */
uint32_t vcefw_ksize;
- uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256, 0, 0, (9 + sizeof(struct amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)), 3)];
+ uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256, 0, 0, (18 +
+sizeof(struct amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)), 0)];
} __aligned(4);
@@ -254,6 +262,17 @@ typedef struct amdgim_vf2pf_info_v2 amdgim_vf2pf_info ;
} \
} while (0)
+struct amdgpu_virt_ras_err_handler_data {
+ /* point to bad page records array */
+ struct eeprom_table_record *bps;
+ /* point to reserved bo array */
+ struct amdgpu_bo **bps_bo;
+ /* the count of entries */
+ int count;
+ /* last reserved entry's index + 1 */
+ int last_reserved;
+};
+
/* GPU virtualization */
struct amdgpu_virt {
uint32_t caps;
@@ -272,6 +291,8 @@ struct amdgpu_virt {
uint32_t reg_access_mode;
int req_init_data_ver;
bool tdr_debug;
+ struct amdgpu_virt_ras_err_handler_data *virt_eh_data;
+ bool ras_init_done;
};
#define amdgpu_sriov_enabled(adev) \
@@ -323,6 +344,7 @@ 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_release_ras_err_handler_data(struct amdgpu_device
+*adev);
void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev); void amdgpu_detect_virtualization(struct amdgpu_device *adev);
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH V2] drm/amdgpu: support reserve bad page for virt
2020-06-04 12:36 [PATCH V2] drm/amdgpu: support reserve bad page for virt Stanley.Yang
2020-06-05 2:24 ` Chen, Guchun
@ 2020-06-05 3:00 ` Zhou1, Tao
2020-06-05 3:40 ` Yang, Stanley
1 sibling, 1 reply; 6+ messages in thread
From: Zhou1, Tao @ 2020-06-05 3:00 UTC (permalink / raw)
To: Yang, Stanley, amd-gfx
Cc: Chen, Guchun, Yang, Stanley, Liu, Monk, Clements, John, Li,
Dennis, Zhang, Hawking
[AMD Public Use]
> -----Original Message-----
> From: Stanley.Yang <Stanley.Yang@amd.com>
> Sent: 2020年6月4日 20:36
> To: amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Chen, Guchun
> <Guchun.Chen@amd.com>; Liu, Monk <Monk.Liu@amd.com>; Clements,
> John <John.Clements@amd.com>; Zhou1, Tao <Tao.Zhou1@amd.com>; Li,
> Dennis <Dennis.Li@amd.com>; Yang, Stanley <Stanley.Yang@amd.com>
> Subject: [PATCH V2] drm/amdgpu: support reserve bad page for virt
>
> Changed from V1:
> rename same functions name, only init ras error handler data for
> supported asic.
>
> Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
> Change-Id: Ia0ad9453ac3ac929f95c73cbee5b7a8fc42a9816
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 172
> +++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 30 +++-
> 3 files changed, 201 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 1df28b7bf22e..668ad0e35160 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2326,6 +2326,9 @@ static int amdgpu_device_ip_fini(struct
> amdgpu_device *adev) {
> int i, r;
>
> + if (amdgpu_sriov_vf(adev) && adev->virt.ras_init_done)
> + amdgpu_virt_release_ras_err_handler_data(adev);
> +
> amdgpu_ras_pre_fini(adev);
>
> if (adev->gmc.xgmi.num_physical_nodes > 1) diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index bab9286021a7..174fcb8c8b57 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -26,6 +26,7 @@
> #include <drm/drm_drv.h>
>
> #include "amdgpu.h"
> +#include "amdgpu_ras.h"
>
> bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev) { @@ -
> 255,12 +256,169 @@ int amdgpu_virt_fw_reserve_get_checksum(void *obj,
> return ret;
> }
>
> +static int amdgpu_virt_init_ras_err_handler_data(struct amdgpu_device
> +*adev) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data **data = &virt-
> >virt_eh_data;
> + /* GPU will be marked bad on host if bp count more then 10,
> + * so alloc 512 is enough.
> + */
> + unsigned int align_space = 512;
> + void *bps = NULL;
> + struct amdgpu_bo **bps_bo = NULL;
> +
> + *data = kmalloc(sizeof(struct amdgpu_virt_ras_err_handler_data),
> GFP_KERNEL);
> + if (!*data)
> + return -ENOMEM;
> +
> + bps = kmalloc(align_space * sizeof((*data)->bps), GFP_KERNEL);
> + bps_bo = kmalloc(align_space * sizeof((*data)->bps_bo),
> GFP_KERNEL);
> +
> + if (!bps || !bps_bo) {
> + kfree(bps);
> + kfree(bps_bo);
> + return -ENOMEM;
> + }
> +
> + (*data)->bps = bps;
> + (*data)->bps_bo = bps_bo;
> + (*data)->count = 0;
> + (*data)->last_reserved = 0;
> +
> + virt->ras_init_done = true;
> +
> + return 0;
> +}
> +
> +static void amdgpu_virt_ras_release_bp(struct amdgpu_device *adev) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> + struct amdgpu_bo *bo;
> + int i;
> +
> + if (!data)
> + return;
> +
> + for (i = data->last_reserved - 1; i >= 0; i--) {
> + bo = data->bps_bo[i];
> + amdgpu_bo_free_kernel(&bo, NULL, NULL);
> + data->bps_bo[i] = bo;
> + data->last_reserved = i;
> + }
> +}
> +
> +void amdgpu_virt_release_ras_err_handler_data(struct amdgpu_device
> +*adev) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> +
> + virt->ras_init_done = false;
> +
> + if (!data)
> + return;
> +
> + amdgpu_virt_ras_release_bp(adev);
> +
> + kfree(data->bps);
> + kfree(data->bps_bo);
> + kfree(data);
> + virt->virt_eh_data = NULL;
> +}
> +
> +static void amdgpu_virt_ras_add_bps(struct amdgpu_device *adev,
> + struct eeprom_table_record *bps, int pages) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> +
> + if (!data)
> + return;
> +
> + memcpy(&data->bps[data->count], bps, pages * sizeof(*data->bps));
> + data->count += pages;
> +}
> +
> +static void amdgpu_virt_ras_reserve_bps(struct amdgpu_device *adev) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> + struct amdgpu_bo *bo = NULL;
> + uint64_t bp;
> + int i;
> +
> + if (!data)
> + return;
> +
> + for (i = data->last_reserved; i < data->count; i++) {
> + bp = data->bps[i].retired_page;
> +
> + /* There are two cases of reserve error should be ignored:
> + * 1) a ras bad page has been allocated (used by someone);
> + * 2) a ras bad page has been reserved (duplicate error
> injection
> + * for one page);
> + */
> + if (amdgpu_bo_create_kernel_at(adev, bp <<
> AMDGPU_GPU_PAGE_SHIFT,
> + AMDGPU_GPU_PAGE_SIZE,
> + AMDGPU_GEM_DOMAIN_VRAM,
> + &bo, NULL))
> + DRM_DEBUG("RAS WARN: [SRV-IO] reserve vram for
> retired page %llx
> +fail\n", bp);
[Tao] SRIOV? Typo?
> +
> + data->bps_bo[i] = bo;
> + data->last_reserved = i + 1;
> + bo = NULL;
> + }
> +}
> +
> +static bool amdgpu_virt_ras_check_bad_page(struct amdgpu_device *adev,
> + uint64_t retired_page)
> +{
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> + int i;
> +
> + if (!data)
> + return true;
> +
> + for (i = 0; i < data->count; i++)
> + if (retired_page == data->bps[i].retired_page)
> + return true;
> +
> + return false;
> +}
> +
> +static void amdgpu_virt_add_bad_page(struct amdgpu_device *adev,
> + uint64_t bp_block_offset, uint32_t bp_block_size) {
> + struct eeprom_table_record bp;
> + uint64_t retired_page;
> + uint32_t bp_idx, bp_cnt;
> +
> + if (bp_block_size) {
> + bp_cnt = bp_block_size / sizeof(uint64_t);
> + for (bp_idx = 0; bp_idx < bp_cnt; bp_idx++) {
> + retired_page = *(uint64_t *)(adev-
> >fw_vram_usage.va +
> + bp_block_offset + bp_idx *
> sizeof(uint64_t));
> + bp.retired_page = retired_page;
> +
> + if (amdgpu_virt_ras_check_bad_page(adev,
> retired_page))
> + continue;
> +
> + amdgpu_virt_ras_add_bps(adev, &bp, 1);
> +
> + amdgpu_virt_ras_reserve_bps(adev);
> + }
> + }
> +}
> +
> void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev) {
> uint32_t pf2vf_size = 0;
> uint32_t checksum = 0;
> uint32_t checkval;
> char *str;
> + uint64_t bp_block_offset = 0;
> + uint32_t bp_block_size = 0;
>
> adev->virt.fw_reserve.p_pf2vf = NULL;
> adev->virt.fw_reserve.p_vf2pf = NULL;
> @@ -275,6 +433,20 @@ void amdgpu_virt_init_data_exchange(struct
> amdgpu_device *adev)
>
> /* pf2vf message must be in 4K */
> if (pf2vf_size > 0 && pf2vf_size < 4096) {
> + if (adev->virt.fw_reserve.p_pf2vf->version == 2) {
> + struct amdgim_pf2vf_info_v2 *pf2vf_v2 =
> NULL;
> +
> + pf2vf_v2 = (struct amdgim_pf2vf_info_v2
> *)adev->virt.fw_reserve.p_pf2vf;
> + bp_block_offset = ((uint64_t)pf2vf_v2-
> >bp_block_offset_L & 0xFFFFFFFF) |
> + ((((uint64_t)pf2vf_v2-
> >bp_block_offset_H) << 32) & 0xFFFFFFFF00000000);
> + bp_block_size = pf2vf_v2->bp_block_size;
> +
> + if (bp_block_size && !adev-
> >virt.ras_init_done)
> +
> amdgpu_virt_init_ras_err_handler_data(adev);
> +
> + amdgpu_virt_add_bad_page(adev,
> bp_block_offset, bp_block_size);
[Tao] We can add "if (adev->virt.ras_init_done)" before it to skip add bad page handle quickly in init failed case.
> + }
> +
> checkval = amdgpu_virt_fw_reserve_get_checksum(
> adev->virt.fw_reserve.p_pf2vf, pf2vf_size,
> adev->virt.fw_reserve.checksum_key,
> checksum); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> index b90e822cebd7..f826945989c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> @@ -143,19 +143,27 @@ struct amdgim_pf2vf_info_v2 {
> uint32_t vce_enc_max_pixels_count;
> /* 16x16 pixels/sec, codec independent */
> uint32_t vce_enc_max_bandwidth;
> + /* Bad pages block position in BYTE */
> + uint32_t bp_block_offset_L;
> + uint32_t bp_block_offset_H;
> + /* Bad pages block size in BYTE */
> + uint32_t bp_block_size;
> /* MEC FW position in kb from the start of VF visible frame buffer */
> - uint64_t mecfw_kboffset;
> + uint32_t mecfw_kboffset_L;
> + uint32_t mecfw_kboffset_H;
> /* MEC FW size in KB */
> uint32_t mecfw_ksize;
> /* UVD FW position in kb from the start of VF visible frame buffer */
> - uint64_t uvdfw_kboffset;
> + uint32_t uvdfw_kboffset_L;
> + uint32_t uvdfw_kboffset_H;
> /* UVD FW size in KB */
> uint32_t uvdfw_ksize;
> /* VCE FW position in kb from the start of VF visible frame buffer */
> - uint64_t vcefw_kboffset;
> + uint32_t vcefw_kboffset_L;
> + uint32_t vcefw_kboffset_H;
> /* VCE FW size in KB */
> uint32_t vcefw_ksize;
> - uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256, 0,
> 0, (9 + sizeof(struct amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)), 3)];
> + uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256, 0,
> 0, (18 +
> +sizeof(struct amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)), 0)];
> } __aligned(4);
>
>
> @@ -254,6 +262,17 @@ typedef struct amdgim_vf2pf_info_v2
> amdgim_vf2pf_info ;
> } \
> } while (0)
>
> +struct amdgpu_virt_ras_err_handler_data {
> + /* point to bad page records array */
> + struct eeprom_table_record *bps;
> + /* point to reserved bo array */
> + struct amdgpu_bo **bps_bo;
> + /* the count of entries */
> + int count;
> + /* last reserved entry's index + 1 */
> + int last_reserved;
> +};
> +
> /* GPU virtualization */
> struct amdgpu_virt {
> uint32_t caps;
> @@ -272,6 +291,8 @@ struct amdgpu_virt {
> uint32_t reg_access_mode;
> int req_init_data_ver;
> bool tdr_debug;
> + struct amdgpu_virt_ras_err_handler_data *virt_eh_data;
> + bool ras_init_done;
> };
>
> #define amdgpu_sriov_enabled(adev) \
> @@ -323,6 +344,7 @@ 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_release_ras_err_handler_data(struct amdgpu_device
> +*adev);
> void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev); void
> amdgpu_detect_virtualization(struct amdgpu_device *adev);
>
> --
> 2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH V2] drm/amdgpu: support reserve bad page for virt
2020-06-05 2:24 ` Chen, Guchun
@ 2020-06-05 3:09 ` Yang, Stanley
2020-06-05 3:24 ` Zhang, Hawking
0 siblings, 1 reply; 6+ messages in thread
From: Yang, Stanley @ 2020-06-05 3:09 UTC (permalink / raw)
To: Chen, Guchun, amd-gfx
Cc: Zhou1, Tao, Clements, John, Li, Dennis, Liu, Monk, Zhang, Hawking
[AMD Public Use]
Thanks GuChun,
Will fix potential memory leak and typo.
Regards,
Stanley
> -----Original Message-----
> From: Chen, Guchun <Guchun.Chen@amd.com>
> Sent: Friday, June 5, 2020 10:24 AM
> To: Yang, Stanley <Stanley.Yang@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Liu, Monk
> <Monk.Liu@amd.com>; Clements, John <John.Clements@amd.com>; Zhou1,
> Tao <Tao.Zhou1@amd.com>; Li, Dennis <Dennis.Li@amd.com>; Yang,
> Stanley <Stanley.Yang@amd.com>
> Subject: RE: [PATCH V2] drm/amdgpu: support reserve bad page for virt
>
> [AMD Public Use]
>
> Please see my comments with prefix [Guchun].
>
> Regards,
> Guchun
>
> -----Original Message-----
> From: Stanley.Yang <Stanley.Yang@amd.com>
> Sent: Thursday, June 4, 2020 8:36 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Chen, Guchun
> <Guchun.Chen@amd.com>; Liu, Monk <Monk.Liu@amd.com>; Clements,
> John <John.Clements@amd.com>; Zhou1, Tao <Tao.Zhou1@amd.com>; Li,
> Dennis <Dennis.Li@amd.com>; Yang, Stanley <Stanley.Yang@amd.com>
> Subject: [PATCH V2] drm/amdgpu: support reserve bad page for virt
>
> Changed from V1:
> rename same functions name, only init ras error handler data for
> supported asic.
> [Guchun] 'same' is one typo? It should be some..
>
>
> Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
> Change-Id: Ia0ad9453ac3ac929f95c73cbee5b7a8fc42a9816
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 172
> +++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 30 +++-
> 3 files changed, 201 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 1df28b7bf22e..668ad0e35160 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2326,6 +2326,9 @@ static int amdgpu_device_ip_fini(struct
> amdgpu_device *adev) {
> int i, r;
>
> + if (amdgpu_sriov_vf(adev) && adev->virt.ras_init_done)
> + amdgpu_virt_release_ras_err_handler_data(adev);
> +
> amdgpu_ras_pre_fini(adev);
>
> if (adev->gmc.xgmi.num_physical_nodes > 1) diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index bab9286021a7..174fcb8c8b57 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -26,6 +26,7 @@
> #include <drm/drm_drv.h>
>
> #include "amdgpu.h"
> +#include "amdgpu_ras.h"
>
> bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev) { @@ -
> 255,12 +256,169 @@ int amdgpu_virt_fw_reserve_get_checksum(void *obj,
> return ret;
> }
>
> +static int amdgpu_virt_init_ras_err_handler_data(struct amdgpu_device
> +*adev) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data **data = &virt-
> >virt_eh_data;
> + /* GPU will be marked bad on host if bp count more then 10,
> + * so alloc 512 is enough.
> + */
> + unsigned int align_space = 512;
> + void *bps = NULL;
> + struct amdgpu_bo **bps_bo = NULL;
> +
> + *data = kmalloc(sizeof(struct amdgpu_virt_ras_err_handler_data),
> GFP_KERNEL);
> + if (!*data)
> + return -ENOMEM;
> +
> + bps = kmalloc(align_space * sizeof((*data)->bps), GFP_KERNEL);
> + bps_bo = kmalloc(align_space * sizeof((*data)->bps_bo),
> GFP_KERNEL);
> +
> + if (!bps || !bps_bo) {
> + kfree(bps);
> + kfree(bps_bo);
> [Guchun]It's needed to release *data as well to prevent memory leak?
>
>
> + return -ENOMEM;
> + }
> +
> + (*data)->bps = bps;
> + (*data)->bps_bo = bps_bo;
> + (*data)->count = 0;
> + (*data)->last_reserved = 0;
> +
> + virt->ras_init_done = true;
> +
> + return 0;
> +}
> +
> +static void amdgpu_virt_ras_release_bp(struct amdgpu_device *adev) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> + struct amdgpu_bo *bo;
> + int i;
> +
> + if (!data)
> + return;
> +
> + for (i = data->last_reserved - 1; i >= 0; i--) {
> + bo = data->bps_bo[i];
> + amdgpu_bo_free_kernel(&bo, NULL, NULL);
> + data->bps_bo[i] = bo;
> + data->last_reserved = i;
> + }
> +}
> +
> +void amdgpu_virt_release_ras_err_handler_data(struct amdgpu_device
> +*adev) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> +
> + virt->ras_init_done = false;
> +
> + if (!data)
> + return;
> +
> + amdgpu_virt_ras_release_bp(adev);
> +
> + kfree(data->bps);
> + kfree(data->bps_bo);
> + kfree(data);
> + virt->virt_eh_data = NULL;
> +}
> +
> +static void amdgpu_virt_ras_add_bps(struct amdgpu_device *adev,
> + struct eeprom_table_record *bps, int pages) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> +
> + if (!data)
> + return;
> +
> + memcpy(&data->bps[data->count], bps, pages * sizeof(*data->bps));
> + data->count += pages;
> +}
> +
> +static void amdgpu_virt_ras_reserve_bps(struct amdgpu_device *adev) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> + struct amdgpu_bo *bo = NULL;
> + uint64_t bp;
> + int i;
> +
> + if (!data)
> + return;
> +
> + for (i = data->last_reserved; i < data->count; i++) {
> + bp = data->bps[i].retired_page;
> +
> + /* There are two cases of reserve error should be ignored:
> + * 1) a ras bad page has been allocated (used by someone);
> + * 2) a ras bad page has been reserved (duplicate error
> injection
> + * for one page);
> + */
> + if (amdgpu_bo_create_kernel_at(adev, bp <<
> AMDGPU_GPU_PAGE_SHIFT,
> + AMDGPU_GPU_PAGE_SIZE,
> + AMDGPU_GEM_DOMAIN_VRAM,
> + &bo, NULL))
> + DRM_DEBUG("RAS WARN: [SRV-IO] reserve vram for
> retired page %llx
> +fail\n", bp);
> +
> + data->bps_bo[i] = bo;
> + data->last_reserved = i + 1;
> + bo = NULL;
> + }
> +}
> +
> +static bool amdgpu_virt_ras_check_bad_page(struct amdgpu_device
> *adev,
> + uint64_t retired_page)
> +{
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> + int i;
> +
> + if (!data)
> + return true;
> +
> + for (i = 0; i < data->count; i++)
> + if (retired_page == data->bps[i].retired_page)
> + return true;
> +
> + return false;
> +}
> +
> +static void amdgpu_virt_add_bad_page(struct amdgpu_device *adev,
> + uint64_t bp_block_offset, uint32_t bp_block_size) {
> + struct eeprom_table_record bp;
> + uint64_t retired_page;
> + uint32_t bp_idx, bp_cnt;
> +
> + if (bp_block_size) {
> + bp_cnt = bp_block_size / sizeof(uint64_t);
> + for (bp_idx = 0; bp_idx < bp_cnt; bp_idx++) {
> + retired_page = *(uint64_t *)(adev-
> >fw_vram_usage.va +
> + bp_block_offset + bp_idx *
> sizeof(uint64_t));
> + bp.retired_page = retired_page;
> +
> + if (amdgpu_virt_ras_check_bad_page(adev,
> retired_page))
> + continue;
> +
> + amdgpu_virt_ras_add_bps(adev, &bp, 1);
> +
> + amdgpu_virt_ras_reserve_bps(adev);
> + }
> + }
> +}
> +
> void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev) {
> uint32_t pf2vf_size = 0;
> uint32_t checksum = 0;
> uint32_t checkval;
> char *str;
> + uint64_t bp_block_offset = 0;
> + uint32_t bp_block_size = 0;
>
> adev->virt.fw_reserve.p_pf2vf = NULL;
> adev->virt.fw_reserve.p_vf2pf = NULL;
> @@ -275,6 +433,20 @@ void amdgpu_virt_init_data_exchange(struct
> amdgpu_device *adev)
>
> /* pf2vf message must be in 4K */
> if (pf2vf_size > 0 && pf2vf_size < 4096) {
> + if (adev->virt.fw_reserve.p_pf2vf->version == 2) {
> + struct amdgim_pf2vf_info_v2 *pf2vf_v2 =
> NULL;
> [Guchun]Move declare of struct amdgim_pf2vf_info_v2 *pf2vf_v2 = NULL to
> the first beginning of this function.
>
>
> + pf2vf_v2 = (struct amdgim_pf2vf_info_v2
> *)adev->virt.fw_reserve.p_pf2vf;
> + bp_block_offset = ((uint64_t)pf2vf_v2-
> >bp_block_offset_L & 0xFFFFFFFF) |
> + ((((uint64_t)pf2vf_v2-
> >bp_block_offset_H) << 32) & 0xFFFFFFFF00000000);
> + bp_block_size = pf2vf_v2->bp_block_size;
> +
> + if (bp_block_size && !adev-
> >virt.ras_init_done)
> +
> amdgpu_virt_init_ras_err_handler_data(adev);
> +
> + amdgpu_virt_add_bad_page(adev,
> bp_block_offset, bp_block_size);
> + }
> +
> checkval = amdgpu_virt_fw_reserve_get_checksum(
> adev->virt.fw_reserve.p_pf2vf, pf2vf_size,
> adev->virt.fw_reserve.checksum_key,
> checksum); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> index b90e822cebd7..f826945989c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> @@ -143,19 +143,27 @@ struct amdgim_pf2vf_info_v2 {
> uint32_t vce_enc_max_pixels_count;
> /* 16x16 pixels/sec, codec independent */
> uint32_t vce_enc_max_bandwidth;
> + /* Bad pages block position in BYTE */
> + uint32_t bp_block_offset_L;
> + uint32_t bp_block_offset_H;
> + /* Bad pages block size in BYTE */
> + uint32_t bp_block_size;
> /* MEC FW position in kb from the start of VF visible frame buffer */
> - uint64_t mecfw_kboffset;
> + uint32_t mecfw_kboffset_L;
> + uint32_t mecfw_kboffset_H;
> /* MEC FW size in KB */
> uint32_t mecfw_ksize;
> /* UVD FW position in kb from the start of VF visible frame buffer */
> - uint64_t uvdfw_kboffset;
> + uint32_t uvdfw_kboffset_L;
> + uint32_t uvdfw_kboffset_H;
> /* UVD FW size in KB */
> uint32_t uvdfw_ksize;
> /* VCE FW position in kb from the start of VF visible frame buffer */
> - uint64_t vcefw_kboffset;
> + uint32_t vcefw_kboffset_L;
> + uint32_t vcefw_kboffset_H;
> /* VCE FW size in KB */
> uint32_t vcefw_ksize;
> - uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256,
> 0, 0, (9 + sizeof(struct amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)),
> 3)];
> + uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256,
> 0, 0, (18 +
> +sizeof(struct amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)), 0)];
> } __aligned(4);
>
>
> @@ -254,6 +262,17 @@ typedef struct amdgim_vf2pf_info_v2
> amdgim_vf2pf_info ;
> } \
> } while (0)
>
> +struct amdgpu_virt_ras_err_handler_data {
> + /* point to bad page records array */
> + struct eeprom_table_record *bps;
> + /* point to reserved bo array */
> + struct amdgpu_bo **bps_bo;
> + /* the count of entries */
> + int count;
> + /* last reserved entry's index + 1 */
> + int last_reserved;
> +};
> +
> /* GPU virtualization */
> struct amdgpu_virt {
> uint32_t caps;
> @@ -272,6 +291,8 @@ struct amdgpu_virt {
> uint32_t reg_access_mode;
> int req_init_data_ver;
> bool tdr_debug;
> + struct amdgpu_virt_ras_err_handler_data *virt_eh_data;
> + bool ras_init_done;
> };
>
> #define amdgpu_sriov_enabled(adev) \
> @@ -323,6 +344,7 @@ 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_release_ras_err_handler_data(struct amdgpu_device
> +*adev);
> void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev); void
> amdgpu_detect_virtualization(struct amdgpu_device *adev);
>
> --
> 2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH V2] drm/amdgpu: support reserve bad page for virt
2020-06-05 3:09 ` Yang, Stanley
@ 2020-06-05 3:24 ` Zhang, Hawking
0 siblings, 0 replies; 6+ messages in thread
From: Zhang, Hawking @ 2020-06-05 3:24 UTC (permalink / raw)
To: Yang, Stanley, Chen, Guchun, amd-gfx
Cc: Zhou1, Tao, Clements, John, Li, Dennis, Liu, Monk
[AMD Public Use]
I would not suggest to explicitly call out SRIOV in the kernel message. That's just confusing people. It doesn't matter the message share the same format with bare-metal one -- We haven't make a unified amdgpu driver to support both host and guest for bare-metal and sriov use case. So when the function amdgpu_virt_ras_reserve_bps was invoked, it simply indicates currently you are running from a guest VM.
Regards,
Hawking
-----Original Message-----
From: Yang, Stanley <Stanley.Yang@amd.com>
Sent: Friday, June 5, 2020 11:09
To: Chen, Guchun <Guchun.Chen@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Liu, Monk <Monk.Liu@amd.com>; Clements, John <John.Clements@amd.com>; Zhou1, Tao <Tao.Zhou1@amd.com>; Li, Dennis <Dennis.Li@amd.com>
Subject: RE: [PATCH V2] drm/amdgpu: support reserve bad page for virt
[AMD Public Use]
Thanks GuChun,
Will fix potential memory leak and typo.
Regards,
Stanley
> -----Original Message-----
> From: Chen, Guchun <Guchun.Chen@amd.com>
> Sent: Friday, June 5, 2020 10:24 AM
> To: Yang, Stanley <Stanley.Yang@amd.com>;
> amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Liu, Monk
> <Monk.Liu@amd.com>; Clements, John <John.Clements@amd.com>; Zhou1, Tao
> <Tao.Zhou1@amd.com>; Li, Dennis <Dennis.Li@amd.com>; Yang, Stanley
> <Stanley.Yang@amd.com>
> Subject: RE: [PATCH V2] drm/amdgpu: support reserve bad page for virt
>
> [AMD Public Use]
>
> Please see my comments with prefix [Guchun].
>
> Regards,
> Guchun
>
> -----Original Message-----
> From: Stanley.Yang <Stanley.Yang@amd.com>
> Sent: Thursday, June 4, 2020 8:36 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Chen, Guchun
> <Guchun.Chen@amd.com>; Liu, Monk <Monk.Liu@amd.com>; Clements, John
> <John.Clements@amd.com>; Zhou1, Tao <Tao.Zhou1@amd.com>; Li, Dennis
> <Dennis.Li@amd.com>; Yang, Stanley <Stanley.Yang@amd.com>
> Subject: [PATCH V2] drm/amdgpu: support reserve bad page for virt
>
> Changed from V1:
> rename same functions name, only init ras error handler data for
> supported asic.
> [Guchun] 'same' is one typo? It should be some..
>
>
> Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
> Change-Id: Ia0ad9453ac3ac929f95c73cbee5b7a8fc42a9816
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 172
> +++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 30 +++-
> 3 files changed, 201 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 1df28b7bf22e..668ad0e35160 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2326,6 +2326,9 @@ static int amdgpu_device_ip_fini(struct
> amdgpu_device *adev) {
> int i, r;
>
> + if (amdgpu_sriov_vf(adev) && adev->virt.ras_init_done)
> + amdgpu_virt_release_ras_err_handler_data(adev);
> +
> amdgpu_ras_pre_fini(adev);
>
> if (adev->gmc.xgmi.num_physical_nodes > 1) diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index bab9286021a7..174fcb8c8b57 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -26,6 +26,7 @@
> #include <drm/drm_drv.h>
>
> #include "amdgpu.h"
> +#include "amdgpu_ras.h"
>
> bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev) { @@ -
> 255,12 +256,169 @@ int amdgpu_virt_fw_reserve_get_checksum(void *obj,
> return ret;
> }
>
> +static int amdgpu_virt_init_ras_err_handler_data(struct amdgpu_device
> +*adev) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data **data = &virt-
> >virt_eh_data;
> + /* GPU will be marked bad on host if bp count more then 10,
> + * so alloc 512 is enough.
> + */
> + unsigned int align_space = 512;
> + void *bps = NULL;
> + struct amdgpu_bo **bps_bo = NULL;
> +
> + *data = kmalloc(sizeof(struct amdgpu_virt_ras_err_handler_data),
> GFP_KERNEL);
> + if (!*data)
> + return -ENOMEM;
> +
> + bps = kmalloc(align_space * sizeof((*data)->bps), GFP_KERNEL);
> + bps_bo = kmalloc(align_space * sizeof((*data)->bps_bo),
> GFP_KERNEL);
> +
> + if (!bps || !bps_bo) {
> + kfree(bps);
> + kfree(bps_bo);
> [Guchun]It's needed to release *data as well to prevent memory leak?
>
>
> + return -ENOMEM;
> + }
> +
> + (*data)->bps = bps;
> + (*data)->bps_bo = bps_bo;
> + (*data)->count = 0;
> + (*data)->last_reserved = 0;
> +
> + virt->ras_init_done = true;
> +
> + return 0;
> +}
> +
> +static void amdgpu_virt_ras_release_bp(struct amdgpu_device *adev) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> + struct amdgpu_bo *bo;
> + int i;
> +
> + if (!data)
> + return;
> +
> + for (i = data->last_reserved - 1; i >= 0; i--) {
> + bo = data->bps_bo[i];
> + amdgpu_bo_free_kernel(&bo, NULL, NULL);
> + data->bps_bo[i] = bo;
> + data->last_reserved = i;
> + }
> +}
> +
> +void amdgpu_virt_release_ras_err_handler_data(struct amdgpu_device
> +*adev) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> +
> + virt->ras_init_done = false;
> +
> + if (!data)
> + return;
> +
> + amdgpu_virt_ras_release_bp(adev);
> +
> + kfree(data->bps);
> + kfree(data->bps_bo);
> + kfree(data);
> + virt->virt_eh_data = NULL;
> +}
> +
> +static void amdgpu_virt_ras_add_bps(struct amdgpu_device *adev,
> + struct eeprom_table_record *bps, int pages) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> +
> + if (!data)
> + return;
> +
> + memcpy(&data->bps[data->count], bps, pages * sizeof(*data->bps));
> + data->count += pages;
> +}
> +
> +static void amdgpu_virt_ras_reserve_bps(struct amdgpu_device *adev) {
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> + struct amdgpu_bo *bo = NULL;
> + uint64_t bp;
> + int i;
> +
> + if (!data)
> + return;
> +
> + for (i = data->last_reserved; i < data->count; i++) {
> + bp = data->bps[i].retired_page;
> +
> + /* There are two cases of reserve error should be ignored:
> + * 1) a ras bad page has been allocated (used by someone);
> + * 2) a ras bad page has been reserved (duplicate error
> injection
> + * for one page);
> + */
> + if (amdgpu_bo_create_kernel_at(adev, bp <<
> AMDGPU_GPU_PAGE_SHIFT,
> + AMDGPU_GPU_PAGE_SIZE,
> + AMDGPU_GEM_DOMAIN_VRAM,
> + &bo, NULL))
> + DRM_DEBUG("RAS WARN: [SRV-IO] reserve vram for
> retired page %llx
> +fail\n", bp);
> +
> + data->bps_bo[i] = bo;
> + data->last_reserved = i + 1;
> + bo = NULL;
> + }
> +}
> +
> +static bool amdgpu_virt_ras_check_bad_page(struct amdgpu_device
> *adev,
> + uint64_t retired_page)
> +{
> + struct amdgpu_virt *virt = &adev->virt;
> + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> + int i;
> +
> + if (!data)
> + return true;
> +
> + for (i = 0; i < data->count; i++)
> + if (retired_page == data->bps[i].retired_page)
> + return true;
> +
> + return false;
> +}
> +
> +static void amdgpu_virt_add_bad_page(struct amdgpu_device *adev,
> + uint64_t bp_block_offset, uint32_t bp_block_size) {
> + struct eeprom_table_record bp;
> + uint64_t retired_page;
> + uint32_t bp_idx, bp_cnt;
> +
> + if (bp_block_size) {
> + bp_cnt = bp_block_size / sizeof(uint64_t);
> + for (bp_idx = 0; bp_idx < bp_cnt; bp_idx++) {
> + retired_page = *(uint64_t *)(adev-
> >fw_vram_usage.va +
> + bp_block_offset + bp_idx *
> sizeof(uint64_t));
> + bp.retired_page = retired_page;
> +
> + if (amdgpu_virt_ras_check_bad_page(adev,
> retired_page))
> + continue;
> +
> + amdgpu_virt_ras_add_bps(adev, &bp, 1);
> +
> + amdgpu_virt_ras_reserve_bps(adev);
> + }
> + }
> +}
> +
> void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev) {
> uint32_t pf2vf_size = 0;
> uint32_t checksum = 0;
> uint32_t checkval;
> char *str;
> + uint64_t bp_block_offset = 0;
> + uint32_t bp_block_size = 0;
>
> adev->virt.fw_reserve.p_pf2vf = NULL;
> adev->virt.fw_reserve.p_vf2pf = NULL; @@ -275,6 +433,20 @@ void
> amdgpu_virt_init_data_exchange(struct
> amdgpu_device *adev)
>
> /* pf2vf message must be in 4K */
> if (pf2vf_size > 0 && pf2vf_size < 4096) {
> + if (adev->virt.fw_reserve.p_pf2vf->version == 2) {
> + struct amdgim_pf2vf_info_v2 *pf2vf_v2 =
> NULL;
> [Guchun]Move declare of struct amdgim_pf2vf_info_v2 *pf2vf_v2 = NULL
> to the first beginning of this function.
>
>
> + pf2vf_v2 = (struct amdgim_pf2vf_info_v2
> *)adev->virt.fw_reserve.p_pf2vf;
> + bp_block_offset = ((uint64_t)pf2vf_v2-
> >bp_block_offset_L & 0xFFFFFFFF) |
> + ((((uint64_t)pf2vf_v2-
> >bp_block_offset_H) << 32) & 0xFFFFFFFF00000000);
> + bp_block_size = pf2vf_v2->bp_block_size;
> +
> + if (bp_block_size && !adev-
> >virt.ras_init_done)
> +
> amdgpu_virt_init_ras_err_handler_data(adev);
> +
> + amdgpu_virt_add_bad_page(adev,
> bp_block_offset, bp_block_size);
> + }
> +
> checkval = amdgpu_virt_fw_reserve_get_checksum(
> adev->virt.fw_reserve.p_pf2vf, pf2vf_size,
> adev->virt.fw_reserve.checksum_key,
> checksum); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> index b90e822cebd7..f826945989c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> @@ -143,19 +143,27 @@ struct amdgim_pf2vf_info_v2 {
> uint32_t vce_enc_max_pixels_count;
> /* 16x16 pixels/sec, codec independent */
> uint32_t vce_enc_max_bandwidth;
> + /* Bad pages block position in BYTE */
> + uint32_t bp_block_offset_L;
> + uint32_t bp_block_offset_H;
> + /* Bad pages block size in BYTE */
> + uint32_t bp_block_size;
> /* MEC FW position in kb from the start of VF visible frame buffer */
> - uint64_t mecfw_kboffset;
> + uint32_t mecfw_kboffset_L;
> + uint32_t mecfw_kboffset_H;
> /* MEC FW size in KB */
> uint32_t mecfw_ksize;
> /* UVD FW position in kb from the start of VF visible frame buffer */
> - uint64_t uvdfw_kboffset;
> + uint32_t uvdfw_kboffset_L;
> + uint32_t uvdfw_kboffset_H;
> /* UVD FW size in KB */
> uint32_t uvdfw_ksize;
> /* VCE FW position in kb from the start of VF visible frame buffer */
> - uint64_t vcefw_kboffset;
> + uint32_t vcefw_kboffset_L;
> + uint32_t vcefw_kboffset_H;
> /* VCE FW size in KB */
> uint32_t vcefw_ksize;
> - uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256,
> 0, 0, (9 + sizeof(struct
> amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)),
> 3)];
> + uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256,
> 0, 0, (18 +
> +sizeof(struct amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)),
> +0)];
> } __aligned(4);
>
>
> @@ -254,6 +262,17 @@ typedef struct amdgim_vf2pf_info_v2
> amdgim_vf2pf_info ;
> } \
> } while (0)
>
> +struct amdgpu_virt_ras_err_handler_data {
> + /* point to bad page records array */
> + struct eeprom_table_record *bps;
> + /* point to reserved bo array */
> + struct amdgpu_bo **bps_bo;
> + /* the count of entries */
> + int count;
> + /* last reserved entry's index + 1 */
> + int last_reserved;
> +};
> +
> /* GPU virtualization */
> struct amdgpu_virt {
> uint32_t caps;
> @@ -272,6 +291,8 @@ struct amdgpu_virt {
> uint32_t reg_access_mode;
> int req_init_data_ver;
> bool tdr_debug;
> + struct amdgpu_virt_ras_err_handler_data *virt_eh_data;
> + bool ras_init_done;
> };
>
> #define amdgpu_sriov_enabled(adev) \
> @@ -323,6 +344,7 @@ 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_release_ras_err_handler_data(struct amdgpu_device
> +*adev);
> void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev);
> void amdgpu_detect_virtualization(struct amdgpu_device *adev);
>
> --
> 2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH V2] drm/amdgpu: support reserve bad page for virt
2020-06-05 3:00 ` Zhou1, Tao
@ 2020-06-05 3:40 ` Yang, Stanley
0 siblings, 0 replies; 6+ messages in thread
From: Yang, Stanley @ 2020-06-05 3:40 UTC (permalink / raw)
To: Zhou1, Tao, amd-gfx, Zhang, Hawking
Cc: Clements, John, Liu, Monk, Chen, Guchun, Li, Dennis
[-- Attachment #1.1: Type: text/plain, Size: 15511 bytes --]
[AMD Public Use]
Hi Tao,
Thanks for your suggestion and reply inline.
Regards,
Stanley
> -----Original Message-----
> From: Zhou1, Tao <Tao.Zhou1@amd.com>
> Sent: Friday, June 5, 2020 11:00 AM
> To: Yang, Stanley <Stanley.Yang@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Chen, Guchun
> <Guchun.Chen@amd.com>; Liu, Monk <Monk.Liu@amd.com>; Clements,
> John <John.Clements@amd.com>; Li, Dennis <Dennis.Li@amd.com>; Yang,
> Stanley <Stanley.Yang@amd.com>
> Subject: RE: [PATCH V2] drm/amdgpu: support reserve bad page for virt
>
> [AMD Public Use]
>
>
>
> > -----Original Message-----
> > From: Stanley.Yang <Stanley.Yang@amd.com>
> > Sent: 2020年6月4日 20:36
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Chen, Guchun
> > <Guchun.Chen@amd.com>; Liu, Monk <Monk.Liu@amd.com>; Clements,
> John
> > <John.Clements@amd.com>; Zhou1, Tao <Tao.Zhou1@amd.com>; Li,
> Dennis
> > <Dennis.Li@amd.com>; Yang, Stanley <Stanley.Yang@amd.com>
> > Subject: [PATCH V2] drm/amdgpu: support reserve bad page for virt
> >
> > Changed from V1:
> > rename same functions name, only init ras error handler data for
> > supported asic.
> >
> > Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
> > Change-Id: Ia0ad9453ac3ac929f95c73cbee5b7a8fc42a9816
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +
> > drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 172
> > +++++++++++++++++++++
> > drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 30 +++-
> > 3 files changed, 201 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 1df28b7bf22e..668ad0e35160 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -2326,6 +2326,9 @@ static int amdgpu_device_ip_fini(struct
> > amdgpu_device *adev) {
> > int i, r;
> >
> > + if (amdgpu_sriov_vf(adev) && adev->virt.ras_init_done)
> > + amdgpu_virt_release_ras_err_handler_data(adev);
> > +
> > amdgpu_ras_pre_fini(adev);
> >
> > if (adev->gmc.xgmi.num_physical_nodes > 1) diff --git
> > a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > index bab9286021a7..174fcb8c8b57 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > @@ -26,6 +26,7 @@
> > #include <drm/drm_drv.h>
> >
> > #include "amdgpu.h"
> > +#include "amdgpu_ras.h"
> >
> > bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev) { @@ -
> > 255,12 +256,169 @@ int amdgpu_virt_fw_reserve_get_checksum(void
> *obj,
> > return ret;
> > }
> >
> > +static int amdgpu_virt_init_ras_err_handler_data(struct amdgpu_device
> > +*adev) {
> > + struct amdgpu_virt *virt = &adev->virt;
> > + struct amdgpu_virt_ras_err_handler_data **data = &virt-
> > >virt_eh_data;
> > + /* GPU will be marked bad on host if bp count more then 10,
> > + * so alloc 512 is enough.
> > + */
> > + unsigned int align_space = 512;
> > + void *bps = NULL;
> > + struct amdgpu_bo **bps_bo = NULL;
> > +
> > + *data = kmalloc(sizeof(struct amdgpu_virt_ras_err_handler_data),
> > GFP_KERNEL);
> > + if (!*data)
> > + return -ENOMEM;
> > +
> > + bps = kmalloc(align_space * sizeof((*data)->bps), GFP_KERNEL);
> > + bps_bo = kmalloc(align_space * sizeof((*data)->bps_bo),
> > GFP_KERNEL);
> > +
> > + if (!bps || !bps_bo) {
> > + kfree(bps);
> > + kfree(bps_bo);
> > + return -ENOMEM;
> > + }
> > +
> > + (*data)->bps = bps;
> > + (*data)->bps_bo = bps_bo;
> > + (*data)->count = 0;
> > + (*data)->last_reserved = 0;
> > +
> > + virt->ras_init_done = true;
> > +
> > + return 0;
> > +}
> > +
> > +static void amdgpu_virt_ras_release_bp(struct amdgpu_device *adev) {
> > + struct amdgpu_virt *virt = &adev->virt;
> > + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> > + struct amdgpu_bo *bo;
> > + int i;
> > +
> > + if (!data)
> > + return;
> > +
> > + for (i = data->last_reserved - 1; i >= 0; i--) {
> > + bo = data->bps_bo[i];
> > + amdgpu_bo_free_kernel(&bo, NULL, NULL);
> > + data->bps_bo[i] = bo;
> > + data->last_reserved = i;
> > + }
> > +}
> > +
> > +void amdgpu_virt_release_ras_err_handler_data(struct amdgpu_device
> > +*adev) {
> > + struct amdgpu_virt *virt = &adev->virt;
> > + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> > +
> > + virt->ras_init_done = false;
> > +
> > + if (!data)
> > + return;
> > +
> > + amdgpu_virt_ras_release_bp(adev);
> > +
> > + kfree(data->bps);
> > + kfree(data->bps_bo);
> > + kfree(data);
> > + virt->virt_eh_data = NULL;
> > +}
> > +
> > +static void amdgpu_virt_ras_add_bps(struct amdgpu_device *adev,
> > + struct eeprom_table_record *bps, int pages) {
> > + struct amdgpu_virt *virt = &adev->virt;
> > + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> > +
> > + if (!data)
> > + return;
> > +
> > + memcpy(&data->bps[data->count], bps, pages * sizeof(*data->bps));
> > + data->count += pages;
> > +}
> > +
> > +static void amdgpu_virt_ras_reserve_bps(struct amdgpu_device *adev) {
> > + struct amdgpu_virt *virt = &adev->virt;
> > + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> > + struct amdgpu_bo *bo = NULL;
> > + uint64_t bp;
> > + int i;
> > +
> > + if (!data)
> > + return;
> > +
> > + for (i = data->last_reserved; i < data->count; i++) {
> > + bp = data->bps[i].retired_page;
> > +
> > + /* There are two cases of reserve error should be ignored:
> > + * 1) a ras bad page has been allocated (used by someone);
> > + * 2) a ras bad page has been reserved (duplicate error
> > injection
> > + * for one page);
> > + */
> > + if (amdgpu_bo_create_kernel_at(adev, bp <<
> > AMDGPU_GPU_PAGE_SHIFT,
> > + AMDGPU_GPU_PAGE_SIZE,
> > + AMDGPU_GEM_DOMAIN_VRAM,
> > + &bo, NULL))
> > + DRM_DEBUG("RAS WARN: [SRV-IO] reserve vram for
> > retired page %llx
> > +fail\n", bp);
>
> [Tao] SRIOV? Typo?
[Yang, Stanley] :
Will keep the same message with bare-metal according to @Zhang, Hawking<mailto:Hawking.Zhang@amd.com>'s suggestion.
>
> > +
> > + data->bps_bo[i] = bo;
> > + data->last_reserved = i + 1;
> > + bo = NULL;
> > + }
> > +}
> > +
> > +static bool amdgpu_virt_ras_check_bad_page(struct amdgpu_device
> *adev,
> > + uint64_t retired_page)
> > +{
> > + struct amdgpu_virt *virt = &adev->virt;
> > + struct amdgpu_virt_ras_err_handler_data *data = virt->virt_eh_data;
> > + int i;
> > +
> > + if (!data)
> > + return true;
> > +
> > + for (i = 0; i < data->count; i++)
> > + if (retired_page == data->bps[i].retired_page)
> > + return true;
> > +
> > + return false;
> > +}
> > +
> > +static void amdgpu_virt_add_bad_page(struct amdgpu_device *adev,
> > + uint64_t bp_block_offset, uint32_t bp_block_size) {
> > + struct eeprom_table_record bp;
> > + uint64_t retired_page;
> > + uint32_t bp_idx, bp_cnt;
> > +
> > + if (bp_block_size) {
> > + bp_cnt = bp_block_size / sizeof(uint64_t);
> > + for (bp_idx = 0; bp_idx < bp_cnt; bp_idx++) {
> > + retired_page = *(uint64_t *)(adev-
> > >fw_vram_usage.va +
> > + bp_block_offset + bp_idx *
> > sizeof(uint64_t));
> > + bp.retired_page = retired_page;
> > +
> > + if (amdgpu_virt_ras_check_bad_page(adev,
> > retired_page))
> > + continue;
> > +
> > + amdgpu_virt_ras_add_bps(adev, &bp, 1);
> > +
> > + amdgpu_virt_ras_reserve_bps(adev);
> > + }
> > + }
> > +}
> > +
> > void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev) {
> > uint32_t pf2vf_size = 0;
> > uint32_t checksum = 0;
> > uint32_t checkval;
> > char *str;
> > + uint64_t bp_block_offset = 0;
> > + uint32_t bp_block_size = 0;
> >
> > adev->virt.fw_reserve.p_pf2vf = NULL;
> > adev->virt.fw_reserve.p_vf2pf = NULL; @@ -275,6 +433,20 @@ void
> > amdgpu_virt_init_data_exchange(struct
> > amdgpu_device *adev)
> >
> > /* pf2vf message must be in 4K */
> > if (pf2vf_size > 0 && pf2vf_size < 4096) {
> > + if (adev->virt.fw_reserve.p_pf2vf->version == 2) {
> > + struct amdgim_pf2vf_info_v2 *pf2vf_v2 =
> > NULL;
> > +
> > + pf2vf_v2 = (struct amdgim_pf2vf_info_v2
> > *)adev->virt.fw_reserve.p_pf2vf;
> > + bp_block_offset = ((uint64_t)pf2vf_v2-
> > >bp_block_offset_L & 0xFFFFFFFF) |
> > + ((((uint64_t)pf2vf_v2-
> > >bp_block_offset_H) << 32) & 0xFFFFFFFF00000000);
> > + bp_block_size = pf2vf_v2->bp_block_size;
> > +
> > + if (bp_block_size && !adev-
> > >virt.ras_init_done)
> > +
> > amdgpu_virt_init_ras_err_handler_data(adev);
> > +
> > + amdgpu_virt_add_bad_page(adev,
> > bp_block_offset, bp_block_size);
>
> [Tao] We can add "if (adev->virt.ras_init_done)" before it to skip add bad
> page handle quickly in init failed case.
>
[Yang, Stanley] :
Yes, add this judgement is better.
>
> > + }
> > +
> > checkval = amdgpu_virt_fw_reserve_get_checksum(
> > adev->virt.fw_reserve.p_pf2vf, pf2vf_size,
> > adev->virt.fw_reserve.checksum_key,
> > checksum); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > index b90e822cebd7..f826945989c7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > @@ -143,19 +143,27 @@ struct amdgim_pf2vf_info_v2 {
> > uint32_t vce_enc_max_pixels_count;
> > /* 16x16 pixels/sec, codec independent */
> > uint32_t vce_enc_max_bandwidth;
> > + /* Bad pages block position in BYTE */
> > + uint32_t bp_block_offset_L;
> > + uint32_t bp_block_offset_H;
> > + /* Bad pages block size in BYTE */
> > + uint32_t bp_block_size;
> > /* MEC FW position in kb from the start of VF visible frame buffer */
> > - uint64_t mecfw_kboffset;
> > + uint32_t mecfw_kboffset_L;
> > + uint32_t mecfw_kboffset_H;
> > /* MEC FW size in KB */
> > uint32_t mecfw_ksize;
> > /* UVD FW position in kb from the start of VF visible frame buffer */
> > - uint64_t uvdfw_kboffset;
> > + uint32_t uvdfw_kboffset_L;
> > + uint32_t uvdfw_kboffset_H;
> > /* UVD FW size in KB */
> > uint32_t uvdfw_ksize;
> > /* VCE FW position in kb from the start of VF visible frame buffer */
> > - uint64_t vcefw_kboffset;
> > + uint32_t vcefw_kboffset_L;
> > + uint32_t vcefw_kboffset_H;
> > /* VCE FW size in KB */
> > uint32_t vcefw_ksize;
> > - uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256,
> 0,
> > 0, (9 + sizeof(struct
> > amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)), 3)];
> > + uint32_t reserved[AMDGIM_GET_STRUCTURE_RESERVED_SIZE(256,
> 0,
> > 0, (18 +
> > +sizeof(struct amd_sriov_msg_pf2vf_info_header)/sizeof(uint32_t)),
> > +0)];
> > } __aligned(4);
> >
> >
> > @@ -254,6 +262,17 @@ typedef struct amdgim_vf2pf_info_v2
> > amdgim_vf2pf_info ;
> > } \
> > } while (0)
> >
> > +struct amdgpu_virt_ras_err_handler_data {
> > + /* point to bad page records array */
> > + struct eeprom_table_record *bps;
> > + /* point to reserved bo array */
> > + struct amdgpu_bo **bps_bo;
> > + /* the count of entries */
> > + int count;
> > + /* last reserved entry's index + 1 */
> > + int last_reserved;
> > +};
> > +
> > /* GPU virtualization */
> > struct amdgpu_virt {
> > uint32_t caps;
> > @@ -272,6 +291,8 @@ struct amdgpu_virt {
> > uint32_t reg_access_mode;
> > int req_init_data_ver;
> > bool tdr_debug;
> > + struct amdgpu_virt_ras_err_handler_data *virt_eh_data;
> > + bool ras_init_done;
> > };
> >
> > #define amdgpu_sriov_enabled(adev) \
> > @@ -323,6 +344,7 @@ 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_release_ras_err_handler_data(struct amdgpu_device
> > +*adev);
> > void amdgpu_virt_init_data_exchange(struct amdgpu_device *adev);
> > void amdgpu_detect_virtualization(struct amdgpu_device *adev);
> >
> > --
> > 2.17.1
[-- Attachment #1.2: Type: text/html, Size: 49050 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-05 3:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-04 12:36 [PATCH V2] drm/amdgpu: support reserve bad page for virt Stanley.Yang
2020-06-05 2:24 ` Chen, Guchun
2020-06-05 3:09 ` Yang, Stanley
2020-06-05 3:24 ` Zhang, Hawking
2020-06-05 3:00 ` Zhou1, Tao
2020-06-05 3:40 ` Yang, Stanley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).