All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "amd-gfx list" <amd-gfx@lists.freedesktop.org>,
	"Greg KH" <gregkh@linuxfoundation.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	"Qiang Yu" <yuq825@gmail.com>,
	"Alex Deucher" <Alexander.Deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>
Subject: Re: [PATCH v4 11/14] drm/amdgpu: Guard against write accesses after device removal
Date: Tue, 19 Jan 2021 13:22:18 -0500	[thread overview]
Message-ID: <8ed4a153-d503-e704-0a0d-3422877e50fa@amd.com> (raw)
In-Reply-To: <CAKMK7uHCzBpaC2YypKeQwbJiT0JG2Hq7V0BC5yC88f9nqgxUiw@mail.gmail.com>


On 1/19/21 1:05 PM, Daniel Vetter wrote:
> On Tue, Jan 19, 2021 at 4:35 PM Andrey Grodzovsky
> <Andrey.Grodzovsky@amd.com> wrote:
>> There is really no other way according to this article
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.net%2FArticles%2F767885%2F&amp;data=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C7a1f5ae6a06f4661d47708d8bca4cb32%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637466763278674162%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=QupsglO9WRuis8XRLBFIhl6miTXVOdAnk8oP4BfSclQ%3D&amp;reserved=0
>>
>> "A perfect solution seems nearly impossible though; we cannot acquire a mutex on
>> the user
>> to prevent them from yanking a device and we cannot check for a presence change
>> after every
>> device access for performance reasons. "
>>
>> But I assumed srcu_read_lock should be pretty seamless performance wise, no ?
> The read side is supposed to be dirt cheap, the write side is were we
> just stall for all readers to eventually complete on their own.
> Definitely should be much cheaper than mmio read, on the mmio write
> side it might actually hurt a bit. Otoh I think those don't stall the
> cpu by default when they're timing out, so maybe if the overhead is
> too much for those, we could omit them?
>
> Maybe just do a small microbenchmark for these for testing, with a
> register that doesn't change hw state. So with and without
> drm_dev_enter/exit, and also one with the hw plugged out so that we
> have actual timeouts in the transactions.
> -Daniel


So say writing in a loop to some harmless scratch register for many times both 
for plugged
and unplugged case and measure total time delta ?

Andrey


>
>> The other solution would be as I suggested to keep all the device IO ranges
>> reserved and system
>> memory pages unfreed until the device is finalized in the driver but Daniel said
>> this would upset the PCI layer (the MMIO ranges reservation part).
>>
>> Andrey
>>
>>
>>
>>
>> On 1/19/21 3:55 AM, Christian König wrote:
>>> Am 18.01.21 um 22:01 schrieb Andrey Grodzovsky:
>>>> This should prevent writing to memory or IO ranges possibly
>>>> already allocated for other uses after our device is removed.
>>> Wow, that adds quite some overhead to every register access. I'm not sure we
>>> can do this.
>>>
>>> Christian.
>>>
>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 57 ++++++++++++++++++++++++
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    |  9 ++++
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c    | 53 +++++++++++++---------
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h    |  3 ++
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   | 70 ++++++++++++++++++++++++++++++
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h   | 49 ++-------------------
>>>>    drivers/gpu/drm/amd/amdgpu/psp_v11_0.c     | 16 ++-----
>>>>    drivers/gpu/drm/amd/amdgpu/psp_v12_0.c     |  8 +---
>>>>    drivers/gpu/drm/amd/amdgpu/psp_v3_1.c      |  8 +---
>>>>    9 files changed, 184 insertions(+), 89 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> index e99f4f1..0a9d73c 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -72,6 +72,8 @@
>>>>      #include <linux/iommu.h>
>>>>    +#include <drm/drm_drv.h>
>>>> +
>>>>    MODULE_FIRMWARE("amdgpu/vega10_gpu_info.bin");
>>>>    MODULE_FIRMWARE("amdgpu/vega12_gpu_info.bin");
>>>>    MODULE_FIRMWARE("amdgpu/raven_gpu_info.bin");
>>>> @@ -404,13 +406,21 @@ uint8_t amdgpu_mm_rreg8(struct amdgpu_device *adev,
>>>> uint32_t offset)
>>>>     */
>>>>    void amdgpu_mm_wreg8(struct amdgpu_device *adev, uint32_t offset, uint8_t
>>>> value)
>>>>    {
>>>> +    int idx;
>>>> +
>>>>        if (adev->in_pci_err_recovery)
>>>>            return;
>>>>    +
>>>> +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>>        if (offset < adev->rmmio_size)
>>>>            writeb(value, adev->rmmio + offset);
>>>>        else
>>>>            BUG();
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> @@ -427,9 +437,14 @@ void amdgpu_device_wreg(struct amdgpu_device *adev,
>>>>                uint32_t reg, uint32_t v,
>>>>                uint32_t acc_flags)
>>>>    {
>>>> +    int idx;
>>>> +
>>>>        if (adev->in_pci_err_recovery)
>>>>            return;
>>>>    +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>>        if ((reg * 4) < adev->rmmio_size) {
>>>>            if (!(acc_flags & AMDGPU_REGS_NO_KIQ) &&
>>>>                amdgpu_sriov_runtime(adev) &&
>>>> @@ -444,6 +459,8 @@ void amdgpu_device_wreg(struct amdgpu_device *adev,
>>>>        }
>>>>          trace_amdgpu_device_wreg(adev->pdev->device, reg, v);
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /*
>>>> @@ -454,9 +471,14 @@ void amdgpu_device_wreg(struct amdgpu_device *adev,
>>>>    void amdgpu_mm_wreg_mmio_rlc(struct amdgpu_device *adev,
>>>>                     uint32_t reg, uint32_t v)
>>>>    {
>>>> +    int idx;
>>>> +
>>>>        if (adev->in_pci_err_recovery)
>>>>            return;
>>>>    +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>>        if (amdgpu_sriov_fullaccess(adev) &&
>>>>            adev->gfx.rlc.funcs &&
>>>>            adev->gfx.rlc.funcs->is_rlcg_access_range) {
>>>> @@ -465,6 +487,8 @@ void amdgpu_mm_wreg_mmio_rlc(struct amdgpu_device *adev,
>>>>        } else {
>>>>            writel(v, ((void __iomem *)adev->rmmio) + (reg * 4));
>>>>        }
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> @@ -499,15 +523,22 @@ u32 amdgpu_io_rreg(struct amdgpu_device *adev, u32 reg)
>>>>     */
>>>>    void amdgpu_io_wreg(struct amdgpu_device *adev, u32 reg, u32 v)
>>>>    {
>>>> +    int idx;
>>>> +
>>>>        if (adev->in_pci_err_recovery)
>>>>            return;
>>>>    +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>>        if ((reg * 4) < adev->rio_mem_size)
>>>>            iowrite32(v, adev->rio_mem + (reg * 4));
>>>>        else {
>>>>            iowrite32((reg * 4), adev->rio_mem + (mmMM_INDEX * 4));
>>>>            iowrite32(v, adev->rio_mem + (mmMM_DATA * 4));
>>>>        }
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> @@ -544,14 +575,21 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device *adev, u32
>>>> index)
>>>>     */
>>>>    void amdgpu_mm_wdoorbell(struct amdgpu_device *adev, u32 index, u32 v)
>>>>    {
>>>> +    int idx;
>>>> +
>>>>        if (adev->in_pci_err_recovery)
>>>>            return;
>>>>    +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>>        if (index < adev->doorbell.num_doorbells) {
>>>>            writel(v, adev->doorbell.ptr + index);
>>>>        } else {
>>>>            DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
>>>>        }
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> @@ -588,14 +626,21 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device *adev,
>>>> u32 index)
>>>>     */
>>>>    void amdgpu_mm_wdoorbell64(struct amdgpu_device *adev, u32 index, u64 v)
>>>>    {
>>>> +    int idx;
>>>> +
>>>>        if (adev->in_pci_err_recovery)
>>>>            return;
>>>>    +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>>        if (index < adev->doorbell.num_doorbells) {
>>>>            atomic64_set((atomic64_t *)(adev->doorbell.ptr + index), v);
>>>>        } else {
>>>>            DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
>>>>        }
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> @@ -682,6 +727,10 @@ void amdgpu_device_indirect_wreg(struct amdgpu_device
>>>> *adev,
>>>>        unsigned long flags;
>>>>        void __iomem *pcie_index_offset;
>>>>        void __iomem *pcie_data_offset;
>>>> +    int idx;
>>>> +
>>>> +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>>          spin_lock_irqsave(&adev->pcie_idx_lock, flags);
>>>>        pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
>>>> @@ -692,6 +741,8 @@ void amdgpu_device_indirect_wreg(struct amdgpu_device *adev,
>>>>        writel(reg_data, pcie_data_offset);
>>>>        readl(pcie_data_offset);
>>>>        spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> @@ -711,6 +762,10 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device
>>>> *adev,
>>>>        unsigned long flags;
>>>>        void __iomem *pcie_index_offset;
>>>>        void __iomem *pcie_data_offset;
>>>> +    int idx;
>>>> +
>>>> +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>>          spin_lock_irqsave(&adev->pcie_idx_lock, flags);
>>>>        pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
>>>> @@ -727,6 +782,8 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device
>>>> *adev,
>>>>        writel((u32)(reg_data >> 32), pcie_data_offset);
>>>>        readl(pcie_data_offset);
>>>>        spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>>>> index fe1a39f..1beb4e6 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>>>> @@ -31,6 +31,8 @@
>>>>    #include "amdgpu_ras.h"
>>>>    #include "amdgpu_xgmi.h"
>>>>    +#include <drm/drm_drv.h>
>>>> +
>>>>    /**
>>>>     * amdgpu_gmc_get_pde_for_bo - get the PDE for a BO
>>>>     *
>>>> @@ -98,6 +100,10 @@ int amdgpu_gmc_set_pte_pde(struct amdgpu_device *adev,
>>>> void *cpu_pt_addr,
>>>>    {
>>>>        void __iomem *ptr = (void *)cpu_pt_addr;
>>>>        uint64_t value;
>>>> +    int idx;
>>>> +
>>>> +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return 0;
>>>>          /*
>>>>         * The following is for PTE only. GART does not have PDEs.
>>>> @@ -105,6 +111,9 @@ int amdgpu_gmc_set_pte_pde(struct amdgpu_device *adev,
>>>> void *cpu_pt_addr,
>>>>        value = addr & 0x0000FFFFFFFFF000ULL;
>>>>        value |= flags;
>>>>        writeq(value, ptr + (gpu_page_idx * 8));
>>>> +
>>>> +    drm_dev_exit(idx);
>>>> +
>>>>        return 0;
>>>>    }
>>>>    diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>>> index 523d22d..89e2bfe 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>>> @@ -37,6 +37,8 @@
>>>>      #include "amdgpu_ras.h"
>>>>    +#include <drm/drm_drv.h>
>>>> +
>>>>    static int psp_sysfs_init(struct amdgpu_device *adev);
>>>>    static void psp_sysfs_fini(struct amdgpu_device *adev);
>>>>    @@ -248,7 +250,7 @@ psp_cmd_submit_buf(struct psp_context *psp,
>>>>               struct psp_gfx_cmd_resp *cmd, uint64_t fence_mc_addr)
>>>>    {
>>>>        int ret;
>>>> -    int index;
>>>> +    int index, idx;
>>>>        int timeout = 2000;
>>>>        bool ras_intr = false;
>>>>        bool skip_unsupport = false;
>>>> @@ -256,6 +258,9 @@ psp_cmd_submit_buf(struct psp_context *psp,
>>>>        if (psp->adev->in_pci_err_recovery)
>>>>            return 0;
>>>>    +    if (!drm_dev_enter(&psp->adev->ddev, &idx))
>>>> +        return 0;
>>>> +
>>>>        mutex_lock(&psp->mutex);
>>>>          memset(psp->cmd_buf_mem, 0, PSP_CMD_BUFFER_SIZE);
>>>> @@ -266,8 +271,7 @@ psp_cmd_submit_buf(struct psp_context *psp,
>>>>        ret = psp_ring_cmd_submit(psp, psp->cmd_buf_mc_addr, fence_mc_addr,
>>>> index);
>>>>        if (ret) {
>>>>            atomic_dec(&psp->fence_value);
>>>> -        mutex_unlock(&psp->mutex);
>>>> -        return ret;
>>>> +        goto exit;
>>>>        }
>>>>          amdgpu_asic_invalidate_hdp(psp->adev, NULL);
>>>> @@ -307,8 +311,8 @@ psp_cmd_submit_buf(struct psp_context *psp,
>>>>                 psp->cmd_buf_mem->cmd_id,
>>>>                 psp->cmd_buf_mem->resp.status);
>>>>            if (!timeout) {
>>>> -            mutex_unlock(&psp->mutex);
>>>> -            return -EINVAL;
>>>> +            ret = -EINVAL;
>>>> +            goto exit;
>>>>            }
>>>>        }
>>>>    @@ -316,8 +320,10 @@ psp_cmd_submit_buf(struct psp_context *psp,
>>>>            ucode->tmr_mc_addr_lo = psp->cmd_buf_mem->resp.fw_addr_lo;
>>>>            ucode->tmr_mc_addr_hi = psp->cmd_buf_mem->resp.fw_addr_hi;
>>>>        }
>>>> -    mutex_unlock(&psp->mutex);
>>>>    +exit:
>>>> +    mutex_unlock(&psp->mutex);
>>>> +    drm_dev_exit(idx);
>>>>        return ret;
>>>>    }
>>>>    @@ -354,8 +360,7 @@ static int psp_load_toc(struct psp_context *psp,
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>        /* Copy toc to psp firmware private buffer */
>>>> -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->toc_start_addr, psp->toc_bin_size);
>>>> +    psp_copy_fw(psp, psp->toc_start_addr, psp->toc_bin_size);
>>>>          psp_prep_load_toc_cmd_buf(cmd, psp->fw_pri_mc_addr, psp->toc_bin_size);
>>>>    @@ -570,8 +575,7 @@ static int psp_asd_load(struct psp_context *psp)
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->asd_start_addr, psp->asd_ucode_size);
>>>> +    psp_copy_fw(psp, psp->asd_start_addr, psp->asd_ucode_size);
>>>>          psp_prep_asd_load_cmd_buf(cmd, psp->fw_pri_mc_addr,
>>>>                      psp->asd_ucode_size);
>>>> @@ -726,8 +730,7 @@ static int psp_xgmi_load(struct psp_context *psp)
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->ta_xgmi_start_addr, psp->ta_xgmi_ucode_size);
>>>> +    psp_copy_fw(psp, psp->ta_xgmi_start_addr, psp->ta_xgmi_ucode_size);
>>>>          psp_prep_ta_load_cmd_buf(cmd,
>>>>                     psp->fw_pri_mc_addr,
>>>> @@ -982,8 +985,7 @@ static int psp_ras_load(struct psp_context *psp)
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->ta_ras_start_addr, psp->ta_ras_ucode_size);
>>>> +    psp_copy_fw(psp, psp->ta_ras_start_addr, psp->ta_ras_ucode_size);
>>>>          psp_prep_ta_load_cmd_buf(cmd,
>>>>                     psp->fw_pri_mc_addr,
>>>> @@ -1219,8 +1221,7 @@ static int psp_hdcp_load(struct psp_context *psp)
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->ta_hdcp_start_addr,
>>>> +    psp_copy_fw(psp, psp->ta_hdcp_start_addr,
>>>>               psp->ta_hdcp_ucode_size);
>>>>          psp_prep_ta_load_cmd_buf(cmd,
>>>> @@ -1366,8 +1367,7 @@ static int psp_dtm_load(struct psp_context *psp)
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->ta_dtm_start_addr, psp->ta_dtm_ucode_size);
>>>> +    psp_copy_fw(psp, psp->ta_dtm_start_addr, psp->ta_dtm_ucode_size);
>>>>          psp_prep_ta_load_cmd_buf(cmd,
>>>>                     psp->fw_pri_mc_addr,
>>>> @@ -1507,8 +1507,7 @@ static int psp_rap_load(struct psp_context *psp)
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->ta_rap_start_addr, psp->ta_rap_ucode_size);
>>>> +    psp_copy_fw(psp, psp->ta_rap_start_addr, psp->ta_rap_ucode_size);
>>>>          psp_prep_ta_load_cmd_buf(cmd,
>>>>                     psp->fw_pri_mc_addr,
>>>> @@ -2778,6 +2777,20 @@ static ssize_t psp_usbc_pd_fw_sysfs_write(struct
>>>> device *dev,
>>>>        return count;
>>>>    }
>>>>    +void psp_copy_fw(struct psp_context *psp, uint8_t *start_addr, uint32_t
>>>> bin_size)
>>>> +{
>>>> +    int idx;
>>>> +
>>>> +    if (!drm_dev_enter(&psp->adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>> +    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> +    memcpy(psp->fw_pri_buf, start_addr, bin_size);
>>>> +
>>>> +    drm_dev_exit(idx);
>>>> +}
>>>> +
>>>> +
>>>>    static DEVICE_ATTR(usbc_pd_fw, S_IRUGO | S_IWUSR,
>>>>               psp_usbc_pd_fw_sysfs_read,
>>>>               psp_usbc_pd_fw_sysfs_write);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>>>> index da250bc..ac69314 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>>>> @@ -400,4 +400,7 @@ int psp_init_ta_microcode(struct psp_context *psp,
>>>>                  const char *chip_name);
>>>>    int psp_get_fw_attestation_records_addr(struct psp_context *psp,
>>>>                        uint64_t *output_ptr);
>>>> +
>>>> +void psp_copy_fw(struct psp_context *psp, uint8_t *start_addr, uint32_t
>>>> bin_size);
>>>> +
>>>>    #endif
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>>> index 1a612f5..d656494 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>>> @@ -35,6 +35,8 @@
>>>>    #include "amdgpu.h"
>>>>    #include "atom.h"
>>>>    +#include <drm/drm_drv.h>
>>>> +
>>>>    /*
>>>>     * Rings
>>>>     * Most engines on the GPU are fed via ring buffers.  Ring
>>>> @@ -463,3 +465,71 @@ int amdgpu_ring_test_helper(struct amdgpu_ring *ring)
>>>>        ring->sched.ready = !r;
>>>>        return r;
>>>>    }
>>>> +
>>>> +void amdgpu_ring_clear_ring(struct amdgpu_ring *ring)
>>>> +{
>>>> +    int idx;
>>>> +    int i = 0;
>>>> +
>>>> +    if (!drm_dev_enter(&ring->adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>> +    while (i <= ring->buf_mask)
>>>> +        ring->ring[i++] = ring->funcs->nop;
>>>> +
>>>> +    drm_dev_exit(idx);
>>>> +
>>>> +}
>>>> +
>>>> +void amdgpu_ring_write(struct amdgpu_ring *ring, uint32_t v)
>>>> +{
>>>> +    int idx;
>>>> +
>>>> +    if (!drm_dev_enter(&ring->adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>> +    if (ring->count_dw <= 0)
>>>> +        DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
>>>> +    ring->ring[ring->wptr++ & ring->buf_mask] = v;
>>>> +    ring->wptr &= ring->ptr_mask;
>>>> +    ring->count_dw--;
>>>> +
>>>> +    drm_dev_exit(idx);
>>>> +}
>>>> +
>>>> +void amdgpu_ring_write_multiple(struct amdgpu_ring *ring,
>>>> +                          void *src, int count_dw)
>>>> +{
>>>> +    unsigned occupied, chunk1, chunk2;
>>>> +    void *dst;
>>>> +    int idx;
>>>> +
>>>> +    if (!drm_dev_enter(&ring->adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>> +    if (unlikely(ring->count_dw < count_dw))
>>>> +        DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
>>>> +
>>>> +    occupied = ring->wptr & ring->buf_mask;
>>>> +    dst = (void *)&ring->ring[occupied];
>>>> +    chunk1 = ring->buf_mask + 1 - occupied;
>>>> +    chunk1 = (chunk1 >= count_dw) ? count_dw: chunk1;
>>>> +    chunk2 = count_dw - chunk1;
>>>> +    chunk1 <<= 2;
>>>> +    chunk2 <<= 2;
>>>> +
>>>> +    if (chunk1)
>>>> +        memcpy(dst, src, chunk1);
>>>> +
>>>> +    if (chunk2) {
>>>> +        src += chunk1;
>>>> +        dst = (void *)ring->ring;
>>>> +        memcpy(dst, src, chunk2);
>>>> +    }
>>>> +
>>>> +    ring->wptr += count_dw;
>>>> +    ring->wptr &= ring->ptr_mask;
>>>> +    ring->count_dw -= count_dw;
>>>> +
>>>> +    drm_dev_exit(idx);
>>>> +}
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> index accb243..f90b81f 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> @@ -300,53 +300,12 @@ static inline void
>>>> amdgpu_ring_set_preempt_cond_exec(struct amdgpu_ring *ring,
>>>>        *ring->cond_exe_cpu_addr = cond_exec;
>>>>    }
>>>>    -static inline void amdgpu_ring_clear_ring(struct amdgpu_ring *ring)
>>>> -{
>>>> -    int i = 0;
>>>> -    while (i <= ring->buf_mask)
>>>> -        ring->ring[i++] = ring->funcs->nop;
>>>> -
>>>> -}
>>>> -
>>>> -static inline void amdgpu_ring_write(struct amdgpu_ring *ring, uint32_t v)
>>>> -{
>>>> -    if (ring->count_dw <= 0)
>>>> -        DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
>>>> -    ring->ring[ring->wptr++ & ring->buf_mask] = v;
>>>> -    ring->wptr &= ring->ptr_mask;
>>>> -    ring->count_dw--;
>>>> -}
>>>> +void amdgpu_ring_clear_ring(struct amdgpu_ring *ring);
>>>>    -static inline void amdgpu_ring_write_multiple(struct amdgpu_ring *ring,
>>>> -                          void *src, int count_dw)
>>>> -{
>>>> -    unsigned occupied, chunk1, chunk2;
>>>> -    void *dst;
>>>> -
>>>> -    if (unlikely(ring->count_dw < count_dw))
>>>> -        DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
>>>> -
>>>> -    occupied = ring->wptr & ring->buf_mask;
>>>> -    dst = (void *)&ring->ring[occupied];
>>>> -    chunk1 = ring->buf_mask + 1 - occupied;
>>>> -    chunk1 = (chunk1 >= count_dw) ? count_dw: chunk1;
>>>> -    chunk2 = count_dw - chunk1;
>>>> -    chunk1 <<= 2;
>>>> -    chunk2 <<= 2;
>>>> +void amdgpu_ring_write(struct amdgpu_ring *ring, uint32_t v);
>>>>    -    if (chunk1)
>>>> -        memcpy(dst, src, chunk1);
>>>> -
>>>> -    if (chunk2) {
>>>> -        src += chunk1;
>>>> -        dst = (void *)ring->ring;
>>>> -        memcpy(dst, src, chunk2);
>>>> -    }
>>>> -
>>>> -    ring->wptr += count_dw;
>>>> -    ring->wptr &= ring->ptr_mask;
>>>> -    ring->count_dw -= count_dw;
>>>> -}
>>>> +void amdgpu_ring_write_multiple(struct amdgpu_ring *ring,
>>>> +                          void *src, int count_dw);
>>>>      int amdgpu_ring_test_helper(struct amdgpu_ring *ring);
>>>>    diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>>>> index bd4248c..b3ce5be 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>>>> @@ -269,10 +269,8 @@ static int psp_v11_0_bootloader_load_kdb(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy PSP KDB binary to memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->kdb_start_addr, psp->kdb_bin_size);
>>>> +    psp_copy_fw(psp, psp->kdb_start_addr, psp->kdb_bin_size);
>>>>          /* Provide the PSP KDB to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> @@ -302,10 +300,8 @@ static int psp_v11_0_bootloader_load_spl(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy PSP SPL binary to memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->spl_start_addr, psp->spl_bin_size);
>>>> +    psp_copy_fw(psp, psp->spl_start_addr, psp->spl_bin_size);
>>>>          /* Provide the PSP SPL to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> @@ -335,10 +331,8 @@ static int psp_v11_0_bootloader_load_sysdrv(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy PSP System Driver binary to memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->sys_start_addr, psp->sys_bin_size);
>>>> +    psp_copy_fw(psp, psp->sys_start_addr, psp->sys_bin_size);
>>>>          /* Provide the sys driver to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> @@ -371,10 +365,8 @@ static int psp_v11_0_bootloader_load_sos(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy Secure OS binary to PSP memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->sos_start_addr, psp->sos_bin_size);
>>>> +    psp_copy_fw(psp, psp->sos_start_addr, psp->sos_bin_size);
>>>>          /* Provide the PSP secure OS to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
>>>> index c4828bd..618e5b6 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
>>>> @@ -138,10 +138,8 @@ static int psp_v12_0_bootloader_load_sysdrv(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy PSP System Driver binary to memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->sys_start_addr, psp->sys_bin_size);
>>>> +    psp_copy_fw(psp, psp->sys_start_addr, psp->sys_bin_size);
>>>>          /* Provide the sys driver to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> @@ -179,10 +177,8 @@ static int psp_v12_0_bootloader_load_sos(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy Secure OS binary to PSP memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->sos_start_addr, psp->sos_bin_size);
>>>> +    psp_copy_fw(psp, psp->sos_start_addr, psp->sos_bin_size);
>>>>          /* Provide the PSP secure OS to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
>>>> b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
>>>> index f2e725f..d0a6cccd 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
>>>> @@ -102,10 +102,8 @@ static int psp_v3_1_bootloader_load_sysdrv(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy PSP System Driver binary to memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->sys_start_addr, psp->sys_bin_size);
>>>> +    psp_copy_fw(psp, psp->sys_start_addr, psp->sys_bin_size);
>>>>          /* Provide the sys driver to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> @@ -143,10 +141,8 @@ static int psp_v3_1_bootloader_load_sos(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy Secure OS binary to PSP memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->sos_start_addr, psp->sos_bin_size);
>>>> +    psp_copy_fw(psp, psp->sos_start_addr, psp->sos_bin_size);
>>>>          /* Provide the PSP secure OS to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Rob Herring" <robh@kernel.org>,
	"amd-gfx list" <amd-gfx@lists.freedesktop.org>,
	"Greg KH" <gregkh@linuxfoundation.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	"Anholt, Eric" <eric@anholt.net>,
	"Pekka Paalanen" <ppaalanen@gmail.com>,
	"Qiang Yu" <yuq825@gmail.com>,
	"Alex Deucher" <Alexander.Deucher@amd.com>,
	"Wentland, Harry" <Harry.Wentland@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Lucas Stach" <l.stach@pengutronix.de>
Subject: Re: [PATCH v4 11/14] drm/amdgpu: Guard against write accesses after device removal
Date: Tue, 19 Jan 2021 13:22:18 -0500	[thread overview]
Message-ID: <8ed4a153-d503-e704-0a0d-3422877e50fa@amd.com> (raw)
In-Reply-To: <CAKMK7uHCzBpaC2YypKeQwbJiT0JG2Hq7V0BC5yC88f9nqgxUiw@mail.gmail.com>


On 1/19/21 1:05 PM, Daniel Vetter wrote:
> On Tue, Jan 19, 2021 at 4:35 PM Andrey Grodzovsky
> <Andrey.Grodzovsky@amd.com> wrote:
>> There is really no other way according to this article
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.net%2FArticles%2F767885%2F&amp;data=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C7a1f5ae6a06f4661d47708d8bca4cb32%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637466763278674162%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=QupsglO9WRuis8XRLBFIhl6miTXVOdAnk8oP4BfSclQ%3D&amp;reserved=0
>>
>> "A perfect solution seems nearly impossible though; we cannot acquire a mutex on
>> the user
>> to prevent them from yanking a device and we cannot check for a presence change
>> after every
>> device access for performance reasons. "
>>
>> But I assumed srcu_read_lock should be pretty seamless performance wise, no ?
> The read side is supposed to be dirt cheap, the write side is were we
> just stall for all readers to eventually complete on their own.
> Definitely should be much cheaper than mmio read, on the mmio write
> side it might actually hurt a bit. Otoh I think those don't stall the
> cpu by default when they're timing out, so maybe if the overhead is
> too much for those, we could omit them?
>
> Maybe just do a small microbenchmark for these for testing, with a
> register that doesn't change hw state. So with and without
> drm_dev_enter/exit, and also one with the hw plugged out so that we
> have actual timeouts in the transactions.
> -Daniel


So say writing in a loop to some harmless scratch register for many times both 
for plugged
and unplugged case and measure total time delta ?

Andrey


>
>> The other solution would be as I suggested to keep all the device IO ranges
>> reserved and system
>> memory pages unfreed until the device is finalized in the driver but Daniel said
>> this would upset the PCI layer (the MMIO ranges reservation part).
>>
>> Andrey
>>
>>
>>
>>
>> On 1/19/21 3:55 AM, Christian König wrote:
>>> Am 18.01.21 um 22:01 schrieb Andrey Grodzovsky:
>>>> This should prevent writing to memory or IO ranges possibly
>>>> already allocated for other uses after our device is removed.
>>> Wow, that adds quite some overhead to every register access. I'm not sure we
>>> can do this.
>>>
>>> Christian.
>>>
>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 57 ++++++++++++++++++++++++
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    |  9 ++++
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c    | 53 +++++++++++++---------
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h    |  3 ++
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   | 70 ++++++++++++++++++++++++++++++
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h   | 49 ++-------------------
>>>>    drivers/gpu/drm/amd/amdgpu/psp_v11_0.c     | 16 ++-----
>>>>    drivers/gpu/drm/amd/amdgpu/psp_v12_0.c     |  8 +---
>>>>    drivers/gpu/drm/amd/amdgpu/psp_v3_1.c      |  8 +---
>>>>    9 files changed, 184 insertions(+), 89 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> index e99f4f1..0a9d73c 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -72,6 +72,8 @@
>>>>      #include <linux/iommu.h>
>>>>    +#include <drm/drm_drv.h>
>>>> +
>>>>    MODULE_FIRMWARE("amdgpu/vega10_gpu_info.bin");
>>>>    MODULE_FIRMWARE("amdgpu/vega12_gpu_info.bin");
>>>>    MODULE_FIRMWARE("amdgpu/raven_gpu_info.bin");
>>>> @@ -404,13 +406,21 @@ uint8_t amdgpu_mm_rreg8(struct amdgpu_device *adev,
>>>> uint32_t offset)
>>>>     */
>>>>    void amdgpu_mm_wreg8(struct amdgpu_device *adev, uint32_t offset, uint8_t
>>>> value)
>>>>    {
>>>> +    int idx;
>>>> +
>>>>        if (adev->in_pci_err_recovery)
>>>>            return;
>>>>    +
>>>> +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>>        if (offset < adev->rmmio_size)
>>>>            writeb(value, adev->rmmio + offset);
>>>>        else
>>>>            BUG();
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> @@ -427,9 +437,14 @@ void amdgpu_device_wreg(struct amdgpu_device *adev,
>>>>                uint32_t reg, uint32_t v,
>>>>                uint32_t acc_flags)
>>>>    {
>>>> +    int idx;
>>>> +
>>>>        if (adev->in_pci_err_recovery)
>>>>            return;
>>>>    +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>>        if ((reg * 4) < adev->rmmio_size) {
>>>>            if (!(acc_flags & AMDGPU_REGS_NO_KIQ) &&
>>>>                amdgpu_sriov_runtime(adev) &&
>>>> @@ -444,6 +459,8 @@ void amdgpu_device_wreg(struct amdgpu_device *adev,
>>>>        }
>>>>          trace_amdgpu_device_wreg(adev->pdev->device, reg, v);
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /*
>>>> @@ -454,9 +471,14 @@ void amdgpu_device_wreg(struct amdgpu_device *adev,
>>>>    void amdgpu_mm_wreg_mmio_rlc(struct amdgpu_device *adev,
>>>>                     uint32_t reg, uint32_t v)
>>>>    {
>>>> +    int idx;
>>>> +
>>>>        if (adev->in_pci_err_recovery)
>>>>            return;
>>>>    +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>>        if (amdgpu_sriov_fullaccess(adev) &&
>>>>            adev->gfx.rlc.funcs &&
>>>>            adev->gfx.rlc.funcs->is_rlcg_access_range) {
>>>> @@ -465,6 +487,8 @@ void amdgpu_mm_wreg_mmio_rlc(struct amdgpu_device *adev,
>>>>        } else {
>>>>            writel(v, ((void __iomem *)adev->rmmio) + (reg * 4));
>>>>        }
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> @@ -499,15 +523,22 @@ u32 amdgpu_io_rreg(struct amdgpu_device *adev, u32 reg)
>>>>     */
>>>>    void amdgpu_io_wreg(struct amdgpu_device *adev, u32 reg, u32 v)
>>>>    {
>>>> +    int idx;
>>>> +
>>>>        if (adev->in_pci_err_recovery)
>>>>            return;
>>>>    +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>>        if ((reg * 4) < adev->rio_mem_size)
>>>>            iowrite32(v, adev->rio_mem + (reg * 4));
>>>>        else {
>>>>            iowrite32((reg * 4), adev->rio_mem + (mmMM_INDEX * 4));
>>>>            iowrite32(v, adev->rio_mem + (mmMM_DATA * 4));
>>>>        }
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> @@ -544,14 +575,21 @@ u32 amdgpu_mm_rdoorbell(struct amdgpu_device *adev, u32
>>>> index)
>>>>     */
>>>>    void amdgpu_mm_wdoorbell(struct amdgpu_device *adev, u32 index, u32 v)
>>>>    {
>>>> +    int idx;
>>>> +
>>>>        if (adev->in_pci_err_recovery)
>>>>            return;
>>>>    +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>>        if (index < adev->doorbell.num_doorbells) {
>>>>            writel(v, adev->doorbell.ptr + index);
>>>>        } else {
>>>>            DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
>>>>        }
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> @@ -588,14 +626,21 @@ u64 amdgpu_mm_rdoorbell64(struct amdgpu_device *adev,
>>>> u32 index)
>>>>     */
>>>>    void amdgpu_mm_wdoorbell64(struct amdgpu_device *adev, u32 index, u64 v)
>>>>    {
>>>> +    int idx;
>>>> +
>>>>        if (adev->in_pci_err_recovery)
>>>>            return;
>>>>    +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>>        if (index < adev->doorbell.num_doorbells) {
>>>>            atomic64_set((atomic64_t *)(adev->doorbell.ptr + index), v);
>>>>        } else {
>>>>            DRM_ERROR("writing beyond doorbell aperture: 0x%08x!\n", index);
>>>>        }
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> @@ -682,6 +727,10 @@ void amdgpu_device_indirect_wreg(struct amdgpu_device
>>>> *adev,
>>>>        unsigned long flags;
>>>>        void __iomem *pcie_index_offset;
>>>>        void __iomem *pcie_data_offset;
>>>> +    int idx;
>>>> +
>>>> +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>>          spin_lock_irqsave(&adev->pcie_idx_lock, flags);
>>>>        pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
>>>> @@ -692,6 +741,8 @@ void amdgpu_device_indirect_wreg(struct amdgpu_device *adev,
>>>>        writel(reg_data, pcie_data_offset);
>>>>        readl(pcie_data_offset);
>>>>        spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> @@ -711,6 +762,10 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device
>>>> *adev,
>>>>        unsigned long flags;
>>>>        void __iomem *pcie_index_offset;
>>>>        void __iomem *pcie_data_offset;
>>>> +    int idx;
>>>> +
>>>> +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return;
>>>>          spin_lock_irqsave(&adev->pcie_idx_lock, flags);
>>>>        pcie_index_offset = (void __iomem *)adev->rmmio + pcie_index * 4;
>>>> @@ -727,6 +782,8 @@ void amdgpu_device_indirect_wreg64(struct amdgpu_device
>>>> *adev,
>>>>        writel((u32)(reg_data >> 32), pcie_data_offset);
>>>>        readl(pcie_data_offset);
>>>>        spin_unlock_irqrestore(&adev->pcie_idx_lock, flags);
>>>> +
>>>> +    drm_dev_exit(idx);
>>>>    }
>>>>      /**
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>>>> index fe1a39f..1beb4e6 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
>>>> @@ -31,6 +31,8 @@
>>>>    #include "amdgpu_ras.h"
>>>>    #include "amdgpu_xgmi.h"
>>>>    +#include <drm/drm_drv.h>
>>>> +
>>>>    /**
>>>>     * amdgpu_gmc_get_pde_for_bo - get the PDE for a BO
>>>>     *
>>>> @@ -98,6 +100,10 @@ int amdgpu_gmc_set_pte_pde(struct amdgpu_device *adev,
>>>> void *cpu_pt_addr,
>>>>    {
>>>>        void __iomem *ptr = (void *)cpu_pt_addr;
>>>>        uint64_t value;
>>>> +    int idx;
>>>> +
>>>> +    if (!drm_dev_enter(&adev->ddev, &idx))
>>>> +        return 0;
>>>>          /*
>>>>         * The following is for PTE only. GART does not have PDEs.
>>>> @@ -105,6 +111,9 @@ int amdgpu_gmc_set_pte_pde(struct amdgpu_device *adev,
>>>> void *cpu_pt_addr,
>>>>        value = addr & 0x0000FFFFFFFFF000ULL;
>>>>        value |= flags;
>>>>        writeq(value, ptr + (gpu_page_idx * 8));
>>>> +
>>>> +    drm_dev_exit(idx);
>>>> +
>>>>        return 0;
>>>>    }
>>>>    diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>>> index 523d22d..89e2bfe 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>>> @@ -37,6 +37,8 @@
>>>>      #include "amdgpu_ras.h"
>>>>    +#include <drm/drm_drv.h>
>>>> +
>>>>    static int psp_sysfs_init(struct amdgpu_device *adev);
>>>>    static void psp_sysfs_fini(struct amdgpu_device *adev);
>>>>    @@ -248,7 +250,7 @@ psp_cmd_submit_buf(struct psp_context *psp,
>>>>               struct psp_gfx_cmd_resp *cmd, uint64_t fence_mc_addr)
>>>>    {
>>>>        int ret;
>>>> -    int index;
>>>> +    int index, idx;
>>>>        int timeout = 2000;
>>>>        bool ras_intr = false;
>>>>        bool skip_unsupport = false;
>>>> @@ -256,6 +258,9 @@ psp_cmd_submit_buf(struct psp_context *psp,
>>>>        if (psp->adev->in_pci_err_recovery)
>>>>            return 0;
>>>>    +    if (!drm_dev_enter(&psp->adev->ddev, &idx))
>>>> +        return 0;
>>>> +
>>>>        mutex_lock(&psp->mutex);
>>>>          memset(psp->cmd_buf_mem, 0, PSP_CMD_BUFFER_SIZE);
>>>> @@ -266,8 +271,7 @@ psp_cmd_submit_buf(struct psp_context *psp,
>>>>        ret = psp_ring_cmd_submit(psp, psp->cmd_buf_mc_addr, fence_mc_addr,
>>>> index);
>>>>        if (ret) {
>>>>            atomic_dec(&psp->fence_value);
>>>> -        mutex_unlock(&psp->mutex);
>>>> -        return ret;
>>>> +        goto exit;
>>>>        }
>>>>          amdgpu_asic_invalidate_hdp(psp->adev, NULL);
>>>> @@ -307,8 +311,8 @@ psp_cmd_submit_buf(struct psp_context *psp,
>>>>                 psp->cmd_buf_mem->cmd_id,
>>>>                 psp->cmd_buf_mem->resp.status);
>>>>            if (!timeout) {
>>>> -            mutex_unlock(&psp->mutex);
>>>> -            return -EINVAL;
>>>> +            ret = -EINVAL;
>>>> +            goto exit;
>>>>            }
>>>>        }
>>>>    @@ -316,8 +320,10 @@ psp_cmd_submit_buf(struct psp_context *psp,
>>>>            ucode->tmr_mc_addr_lo = psp->cmd_buf_mem->resp.fw_addr_lo;
>>>>            ucode->tmr_mc_addr_hi = psp->cmd_buf_mem->resp.fw_addr_hi;
>>>>        }
>>>> -    mutex_unlock(&psp->mutex);
>>>>    +exit:
>>>> +    mutex_unlock(&psp->mutex);
>>>> +    drm_dev_exit(idx);
>>>>        return ret;
>>>>    }
>>>>    @@ -354,8 +360,7 @@ static int psp_load_toc(struct psp_context *psp,
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>        /* Copy toc to psp firmware private buffer */
>>>> -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->toc_start_addr, psp->toc_bin_size);
>>>> +    psp_copy_fw(psp, psp->toc_start_addr, psp->toc_bin_size);
>>>>          psp_prep_load_toc_cmd_buf(cmd, psp->fw_pri_mc_addr, psp->toc_bin_size);
>>>>    @@ -570,8 +575,7 @@ static int psp_asd_load(struct psp_context *psp)
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->asd_start_addr, psp->asd_ucode_size);
>>>> +    psp_copy_fw(psp, psp->asd_start_addr, psp->asd_ucode_size);
>>>>          psp_prep_asd_load_cmd_buf(cmd, psp->fw_pri_mc_addr,
>>>>                      psp->asd_ucode_size);
>>>> @@ -726,8 +730,7 @@ static int psp_xgmi_load(struct psp_context *psp)
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->ta_xgmi_start_addr, psp->ta_xgmi_ucode_size);
>>>> +    psp_copy_fw(psp, psp->ta_xgmi_start_addr, psp->ta_xgmi_ucode_size);
>>>>          psp_prep_ta_load_cmd_buf(cmd,
>>>>                     psp->fw_pri_mc_addr,
>>>> @@ -982,8 +985,7 @@ static int psp_ras_load(struct psp_context *psp)
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->ta_ras_start_addr, psp->ta_ras_ucode_size);
>>>> +    psp_copy_fw(psp, psp->ta_ras_start_addr, psp->ta_ras_ucode_size);
>>>>          psp_prep_ta_load_cmd_buf(cmd,
>>>>                     psp->fw_pri_mc_addr,
>>>> @@ -1219,8 +1221,7 @@ static int psp_hdcp_load(struct psp_context *psp)
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->ta_hdcp_start_addr,
>>>> +    psp_copy_fw(psp, psp->ta_hdcp_start_addr,
>>>>               psp->ta_hdcp_ucode_size);
>>>>          psp_prep_ta_load_cmd_buf(cmd,
>>>> @@ -1366,8 +1367,7 @@ static int psp_dtm_load(struct psp_context *psp)
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->ta_dtm_start_addr, psp->ta_dtm_ucode_size);
>>>> +    psp_copy_fw(psp, psp->ta_dtm_start_addr, psp->ta_dtm_ucode_size);
>>>>          psp_prep_ta_load_cmd_buf(cmd,
>>>>                     psp->fw_pri_mc_addr,
>>>> @@ -1507,8 +1507,7 @@ static int psp_rap_load(struct psp_context *psp)
>>>>        if (!cmd)
>>>>            return -ENOMEM;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -    memcpy(psp->fw_pri_buf, psp->ta_rap_start_addr, psp->ta_rap_ucode_size);
>>>> +    psp_copy_fw(psp, psp->ta_rap_start_addr, psp->ta_rap_ucode_size);
>>>>          psp_prep_ta_load_cmd_buf(cmd,
>>>>                     psp->fw_pri_mc_addr,
>>>> @@ -2778,6 +2777,20 @@ static ssize_t psp_usbc_pd_fw_sysfs_write(struct
>>>> device *dev,
>>>>        return count;
>>>>    }
>>>>    +void psp_copy_fw(struct psp_context *psp, uint8_t *start_addr, uint32_t
>>>> bin_size)
>>>> +{
>>>> +    int idx;
>>>> +
>>>> +    if (!drm_dev_enter(&psp->adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>> +    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> +    memcpy(psp->fw_pri_buf, start_addr, bin_size);
>>>> +
>>>> +    drm_dev_exit(idx);
>>>> +}
>>>> +
>>>> +
>>>>    static DEVICE_ATTR(usbc_pd_fw, S_IRUGO | S_IWUSR,
>>>>               psp_usbc_pd_fw_sysfs_read,
>>>>               psp_usbc_pd_fw_sysfs_write);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>>>> index da250bc..ac69314 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>>>> @@ -400,4 +400,7 @@ int psp_init_ta_microcode(struct psp_context *psp,
>>>>                  const char *chip_name);
>>>>    int psp_get_fw_attestation_records_addr(struct psp_context *psp,
>>>>                        uint64_t *output_ptr);
>>>> +
>>>> +void psp_copy_fw(struct psp_context *psp, uint8_t *start_addr, uint32_t
>>>> bin_size);
>>>> +
>>>>    #endif
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>>> index 1a612f5..d656494 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>>>> @@ -35,6 +35,8 @@
>>>>    #include "amdgpu.h"
>>>>    #include "atom.h"
>>>>    +#include <drm/drm_drv.h>
>>>> +
>>>>    /*
>>>>     * Rings
>>>>     * Most engines on the GPU are fed via ring buffers.  Ring
>>>> @@ -463,3 +465,71 @@ int amdgpu_ring_test_helper(struct amdgpu_ring *ring)
>>>>        ring->sched.ready = !r;
>>>>        return r;
>>>>    }
>>>> +
>>>> +void amdgpu_ring_clear_ring(struct amdgpu_ring *ring)
>>>> +{
>>>> +    int idx;
>>>> +    int i = 0;
>>>> +
>>>> +    if (!drm_dev_enter(&ring->adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>> +    while (i <= ring->buf_mask)
>>>> +        ring->ring[i++] = ring->funcs->nop;
>>>> +
>>>> +    drm_dev_exit(idx);
>>>> +
>>>> +}
>>>> +
>>>> +void amdgpu_ring_write(struct amdgpu_ring *ring, uint32_t v)
>>>> +{
>>>> +    int idx;
>>>> +
>>>> +    if (!drm_dev_enter(&ring->adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>> +    if (ring->count_dw <= 0)
>>>> +        DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
>>>> +    ring->ring[ring->wptr++ & ring->buf_mask] = v;
>>>> +    ring->wptr &= ring->ptr_mask;
>>>> +    ring->count_dw--;
>>>> +
>>>> +    drm_dev_exit(idx);
>>>> +}
>>>> +
>>>> +void amdgpu_ring_write_multiple(struct amdgpu_ring *ring,
>>>> +                          void *src, int count_dw)
>>>> +{
>>>> +    unsigned occupied, chunk1, chunk2;
>>>> +    void *dst;
>>>> +    int idx;
>>>> +
>>>> +    if (!drm_dev_enter(&ring->adev->ddev, &idx))
>>>> +        return;
>>>> +
>>>> +    if (unlikely(ring->count_dw < count_dw))
>>>> +        DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
>>>> +
>>>> +    occupied = ring->wptr & ring->buf_mask;
>>>> +    dst = (void *)&ring->ring[occupied];
>>>> +    chunk1 = ring->buf_mask + 1 - occupied;
>>>> +    chunk1 = (chunk1 >= count_dw) ? count_dw: chunk1;
>>>> +    chunk2 = count_dw - chunk1;
>>>> +    chunk1 <<= 2;
>>>> +    chunk2 <<= 2;
>>>> +
>>>> +    if (chunk1)
>>>> +        memcpy(dst, src, chunk1);
>>>> +
>>>> +    if (chunk2) {
>>>> +        src += chunk1;
>>>> +        dst = (void *)ring->ring;
>>>> +        memcpy(dst, src, chunk2);
>>>> +    }
>>>> +
>>>> +    ring->wptr += count_dw;
>>>> +    ring->wptr &= ring->ptr_mask;
>>>> +    ring->count_dw -= count_dw;
>>>> +
>>>> +    drm_dev_exit(idx);
>>>> +}
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> index accb243..f90b81f 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>>> @@ -300,53 +300,12 @@ static inline void
>>>> amdgpu_ring_set_preempt_cond_exec(struct amdgpu_ring *ring,
>>>>        *ring->cond_exe_cpu_addr = cond_exec;
>>>>    }
>>>>    -static inline void amdgpu_ring_clear_ring(struct amdgpu_ring *ring)
>>>> -{
>>>> -    int i = 0;
>>>> -    while (i <= ring->buf_mask)
>>>> -        ring->ring[i++] = ring->funcs->nop;
>>>> -
>>>> -}
>>>> -
>>>> -static inline void amdgpu_ring_write(struct amdgpu_ring *ring, uint32_t v)
>>>> -{
>>>> -    if (ring->count_dw <= 0)
>>>> -        DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
>>>> -    ring->ring[ring->wptr++ & ring->buf_mask] = v;
>>>> -    ring->wptr &= ring->ptr_mask;
>>>> -    ring->count_dw--;
>>>> -}
>>>> +void amdgpu_ring_clear_ring(struct amdgpu_ring *ring);
>>>>    -static inline void amdgpu_ring_write_multiple(struct amdgpu_ring *ring,
>>>> -                          void *src, int count_dw)
>>>> -{
>>>> -    unsigned occupied, chunk1, chunk2;
>>>> -    void *dst;
>>>> -
>>>> -    if (unlikely(ring->count_dw < count_dw))
>>>> -        DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
>>>> -
>>>> -    occupied = ring->wptr & ring->buf_mask;
>>>> -    dst = (void *)&ring->ring[occupied];
>>>> -    chunk1 = ring->buf_mask + 1 - occupied;
>>>> -    chunk1 = (chunk1 >= count_dw) ? count_dw: chunk1;
>>>> -    chunk2 = count_dw - chunk1;
>>>> -    chunk1 <<= 2;
>>>> -    chunk2 <<= 2;
>>>> +void amdgpu_ring_write(struct amdgpu_ring *ring, uint32_t v);
>>>>    -    if (chunk1)
>>>> -        memcpy(dst, src, chunk1);
>>>> -
>>>> -    if (chunk2) {
>>>> -        src += chunk1;
>>>> -        dst = (void *)ring->ring;
>>>> -        memcpy(dst, src, chunk2);
>>>> -    }
>>>> -
>>>> -    ring->wptr += count_dw;
>>>> -    ring->wptr &= ring->ptr_mask;
>>>> -    ring->count_dw -= count_dw;
>>>> -}
>>>> +void amdgpu_ring_write_multiple(struct amdgpu_ring *ring,
>>>> +                          void *src, int count_dw);
>>>>      int amdgpu_ring_test_helper(struct amdgpu_ring *ring);
>>>>    diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>>>> index bd4248c..b3ce5be 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>>>> @@ -269,10 +269,8 @@ static int psp_v11_0_bootloader_load_kdb(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy PSP KDB binary to memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->kdb_start_addr, psp->kdb_bin_size);
>>>> +    psp_copy_fw(psp, psp->kdb_start_addr, psp->kdb_bin_size);
>>>>          /* Provide the PSP KDB to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> @@ -302,10 +300,8 @@ static int psp_v11_0_bootloader_load_spl(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy PSP SPL binary to memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->spl_start_addr, psp->spl_bin_size);
>>>> +    psp_copy_fw(psp, psp->spl_start_addr, psp->spl_bin_size);
>>>>          /* Provide the PSP SPL to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> @@ -335,10 +331,8 @@ static int psp_v11_0_bootloader_load_sysdrv(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy PSP System Driver binary to memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->sys_start_addr, psp->sys_bin_size);
>>>> +    psp_copy_fw(psp, psp->sys_start_addr, psp->sys_bin_size);
>>>>          /* Provide the sys driver to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> @@ -371,10 +365,8 @@ static int psp_v11_0_bootloader_load_sos(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy Secure OS binary to PSP memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->sos_start_addr, psp->sos_bin_size);
>>>> +    psp_copy_fw(psp, psp->sos_start_addr, psp->sos_bin_size);
>>>>          /* Provide the PSP secure OS to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
>>>> index c4828bd..618e5b6 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
>>>> @@ -138,10 +138,8 @@ static int psp_v12_0_bootloader_load_sysdrv(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy PSP System Driver binary to memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->sys_start_addr, psp->sys_bin_size);
>>>> +    psp_copy_fw(psp, psp->sys_start_addr, psp->sys_bin_size);
>>>>          /* Provide the sys driver to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> @@ -179,10 +177,8 @@ static int psp_v12_0_bootloader_load_sos(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy Secure OS binary to PSP memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->sos_start_addr, psp->sos_bin_size);
>>>> +    psp_copy_fw(psp, psp->sos_start_addr, psp->sos_bin_size);
>>>>          /* Provide the PSP secure OS to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
>>>> b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
>>>> index f2e725f..d0a6cccd 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
>>>> @@ -102,10 +102,8 @@ static int psp_v3_1_bootloader_load_sysdrv(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy PSP System Driver binary to memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->sys_start_addr, psp->sys_bin_size);
>>>> +    psp_copy_fw(psp, psp->sys_start_addr, psp->sys_bin_size);
>>>>          /* Provide the sys driver to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>>>> @@ -143,10 +141,8 @@ static int psp_v3_1_bootloader_load_sos(struct
>>>> psp_context *psp)
>>>>        if (ret)
>>>>            return ret;
>>>>    -    memset(psp->fw_pri_buf, 0, PSP_1_MEG);
>>>> -
>>>>        /* Copy Secure OS binary to PSP memory */
>>>> -    memcpy(psp->fw_pri_buf, psp->sos_start_addr, psp->sos_bin_size);
>>>> +    psp_copy_fw(psp, psp->sos_start_addr, psp->sos_bin_size);
>>>>          /* Provide the PSP secure OS to bootloader */
>>>>        WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_36,
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  reply	other threads:[~2021-01-19 18:34 UTC|newest]

Thread overview: 196+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-18 21:01 [PATCH v4 00/14] RFC Support hot device unplug in amdgpu Andrey Grodzovsky
2021-01-18 21:01 ` Andrey Grodzovsky
2021-01-18 21:01 ` [PATCH v4 01/14] drm/ttm: Remap all page faults to per process dummy page Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-18 21:48   ` Alex Deucher
2021-01-18 21:48     ` Alex Deucher
2021-01-19  8:41   ` Christian König
2021-01-19  8:41     ` Christian König
2021-01-19 13:56   ` Daniel Vetter
2021-01-19 13:56     ` Daniel Vetter
2021-01-25 15:28     ` Andrey Grodzovsky
2021-01-25 15:28       ` Andrey Grodzovsky
2021-01-27 14:29       ` Andrey Grodzovsky
2021-01-27 14:29         ` Andrey Grodzovsky
2021-02-02 14:21         ` Daniel Vetter
2021-02-02 14:21           ` Daniel Vetter
2021-01-18 21:01 ` [PATCH v4 02/14] drm: Unamp the entire device address space on device unplug Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-18 21:01 ` [PATCH v4 03/14] drm/ttm: Expose ttm_tt_unpopulate for driver use Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-18 21:01 ` [PATCH v4 04/14] drm/sched: Cancel and flush all oustatdning jobs before finish Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-18 21:49   ` Alex Deucher
2021-01-18 21:49     ` Alex Deucher
2021-01-19  8:42   ` Christian König
2021-01-19  8:42     ` Christian König
2021-01-19  9:50     ` Christian König
2021-01-19  9:50       ` Christian König
2021-01-18 21:01 ` [PATCH v4 05/14] drm/amdgpu: Split amdgpu_device_fini into early and late Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-19  8:45   ` Christian König
2021-01-19  8:45     ` Christian König
2021-01-18 21:01 ` [PATCH v4 06/14] drm/amdgpu: Add early fini callback Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-18 21:01 ` [PATCH v4 07/14] drm/amdgpu: Register IOMMU topology notifier per device Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-18 21:52   ` Alex Deucher
2021-01-18 21:52     ` Alex Deucher
2021-01-19  8:48   ` Christian König
2021-01-19  8:48     ` Christian König
2021-01-19 13:45     ` Daniel Vetter
2021-01-19 13:45       ` Daniel Vetter
2021-01-19 21:21       ` Andrey Grodzovsky
2021-01-19 21:21         ` Andrey Grodzovsky
2021-01-19 22:01         ` Daniel Vetter
2021-01-19 22:01           ` Daniel Vetter
2021-01-20  4:21           ` Andrey Grodzovsky
2021-01-20  4:21             ` Andrey Grodzovsky
2021-01-20  8:38             ` Daniel Vetter
2021-01-20  8:38               ` Daniel Vetter
     [not found]               ` <1a5f7ccb-1f91-91be-1cb1-e7cb43ac2c13@amd.com>
2021-01-21 10:48                 ` Daniel Vetter
2021-01-21 10:48                   ` Daniel Vetter
2021-01-20  5:01     ` Andrey Grodzovsky
2021-01-20  5:01       ` Andrey Grodzovsky
2021-01-20 19:38       ` Andrey Grodzovsky
2021-01-20 19:38         ` Andrey Grodzovsky
2021-01-21 10:42         ` Christian König
2021-01-21 10:42           ` Christian König
2021-01-18 21:01 ` [PATCH v4 08/14] drm/amdgpu: Fix a bunch of sdma code crash post device unplug Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-19  8:51   ` Christian König
2021-01-19  8:51     ` Christian König
2021-01-18 21:01 ` [PATCH v4 09/14] drm/amdgpu: Remap all page faults to per process dummy page Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-19  8:52   ` Christian König
2021-01-19  8:52     ` Christian König
2021-01-18 21:01 ` [PATCH v4 10/14] dmr/amdgpu: Move some sysfs attrs creation to default_attr Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-19  7:34   ` Greg KH
2021-01-19  7:34     ` Greg KH
2021-01-19 16:36     ` Andrey Grodzovsky
2021-01-19 16:36       ` Andrey Grodzovsky
2021-01-19 17:47       ` Greg KH
2021-01-19 17:47         ` Greg KH
2021-01-19 19:04         ` Alex Deucher
2021-01-19 19:04           ` Alex Deucher
2021-01-19 19:16           ` Andrey Grodzovsky
2021-01-19 19:16             ` Andrey Grodzovsky
2021-01-19 19:41           ` Greg KH
2021-01-19 19:41             ` Greg KH
2021-01-19  8:53   ` Christian König
2021-01-19  8:53     ` Christian König
2021-01-18 21:01 ` [PATCH v4 11/14] drm/amdgpu: Guard against write accesses after device removal Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-19  8:55   ` Christian König
2021-01-19  8:55     ` Christian König
2021-01-19 15:35     ` Andrey Grodzovsky
2021-01-19 15:35       ` Andrey Grodzovsky
2021-01-19 15:39       ` Christian König
2021-01-19 15:39         ` Christian König
2021-01-19 18:05       ` Daniel Vetter
2021-01-19 18:05         ` Daniel Vetter
2021-01-19 18:22         ` Andrey Grodzovsky [this message]
2021-01-19 18:22           ` Andrey Grodzovsky
2021-01-19 18:59           ` Christian König
2021-01-19 18:59             ` Christian König
2021-01-19 19:16             ` Andrey Grodzovsky
2021-01-19 19:16               ` Andrey Grodzovsky
2021-01-20 19:34               ` Andrey Grodzovsky
2021-01-20 19:34                 ` Andrey Grodzovsky
2021-01-28 17:23             ` Andrey Grodzovsky
2021-01-28 17:23               ` Andrey Grodzovsky
2021-01-29 15:16               ` Christian König
2021-01-29 15:16                 ` Christian König
2021-01-29 17:35                 ` Andrey Grodzovsky
2021-01-29 17:35                   ` Andrey Grodzovsky
2021-01-29 19:25                   ` Christian König
2021-01-29 19:25                     ` Christian König
2021-02-05 16:22                     ` Andrey Grodzovsky
2021-02-05 16:22                       ` Andrey Grodzovsky
2021-02-05 22:10                       ` Daniel Vetter
2021-02-05 22:10                         ` Daniel Vetter
2021-02-05 23:09                         ` Andrey Grodzovsky
2021-02-05 23:09                           ` Andrey Grodzovsky
2021-02-06 14:18                           ` Daniel Vetter
2021-02-06 14:18                             ` Daniel Vetter
2021-02-07 21:28                         ` Andrey Grodzovsky
2021-02-07 21:28                           ` Andrey Grodzovsky
2021-02-07 21:50                           ` Daniel Vetter
2021-02-07 21:50                             ` Daniel Vetter
2021-02-08  9:37                             ` Christian König
2021-02-08  9:37                               ` Christian König
2021-02-08  9:48                               ` Daniel Vetter
2021-02-08  9:48                                 ` Daniel Vetter
2021-02-08 10:03                                 ` Christian König
2021-02-08 10:03                                   ` Christian König
2021-02-08 10:11                                   ` Daniel Vetter
2021-02-08 10:11                                     ` Daniel Vetter
2021-02-08 13:59                                     ` Christian König
2021-02-08 13:59                                       ` Christian König
2021-02-08 16:23                                       ` Daniel Vetter
2021-02-08 16:23                                         ` Daniel Vetter
2021-02-08 22:15                                         ` Andrey Grodzovsky
2021-02-08 22:15                                           ` Andrey Grodzovsky
2021-02-09  7:58                                           ` Christian König
2021-02-09  7:58                                             ` Christian König
2021-02-09 14:30                                             ` Andrey Grodzovsky
2021-02-09 14:30                                               ` Andrey Grodzovsky
2021-02-09 15:40                                               ` Christian König
2021-02-09 15:40                                                 ` Christian König
2021-02-10 22:01                                                 ` Andrey Grodzovsky
2021-02-10 22:01                                                   ` Andrey Grodzovsky
2021-02-12 15:00                                                   ` Andrey Grodzovsky
2021-02-12 15:00                                                     ` Andrey Grodzovsky
2021-02-08 22:09                               ` Andrey Grodzovsky
2021-02-08 22:09                                 ` Andrey Grodzovsky
2021-02-09  8:27                                 ` Christian König
2021-02-09  8:27                                   ` Christian König
2021-02-09  9:46                                   ` Daniel Vetter
2021-02-09  9:46                                     ` Daniel Vetter
2021-01-18 21:01 ` [PATCH v4 12/14] drm/scheduler: Job timeout handler returns status Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-19  7:53   ` Christian König
2021-01-19  7:53     ` Christian König
2021-01-19 17:47     ` Luben Tuikov
2021-01-19 17:47       ` Luben Tuikov
2021-01-19 18:53       ` Christian König
2021-01-19 18:53         ` Christian König
2021-01-18 21:01 ` [PATCH v4 13/14] drm/sched: Make timeout timer rearm conditional Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-18 21:01 ` [PATCH v4 14/14] drm/amdgpu: Prevent any job recoveries after device is unplugged Andrey Grodzovsky
2021-01-18 21:01   ` Andrey Grodzovsky
2021-01-19 14:16 ` [PATCH v4 00/14] RFC Support hot device unplug in amdgpu Daniel Vetter
2021-01-19 14:16   ` Daniel Vetter
2021-01-19 17:31   ` Andrey Grodzovsky
2021-01-19 17:31     ` Andrey Grodzovsky
2021-01-19 18:08     ` Daniel Vetter
2021-01-19 18:08       ` Daniel Vetter
2021-01-19 18:18       ` Andrey Grodzovsky
2021-01-19 18:18         ` Andrey Grodzovsky
2021-01-20  9:05         ` Daniel Vetter
2021-01-20  9:05           ` Daniel Vetter
2021-01-20 14:19           ` Andrey Grodzovsky
2021-01-20 14:19             ` Andrey Grodzovsky
2021-01-20 15:59             ` Daniel Vetter
2021-01-20 15:59               ` Daniel Vetter
2021-02-08  5:59               ` Andrey Grodzovsky
2021-02-08  5:59                 ` Andrey Grodzovsky
2021-02-08  7:27                 ` Daniel Vetter
2021-02-08  7:27                   ` Daniel Vetter
2021-02-09  4:01                   ` Andrey Grodzovsky
2021-02-09  4:01                     ` Andrey Grodzovsky
2021-02-09  9:50                     ` Daniel Vetter
2021-02-09  9:50                       ` Daniel Vetter
2021-02-09 15:34                       ` Andrey Grodzovsky
2021-02-09 15:34                         ` Andrey Grodzovsky
2021-02-18 20:03                       ` Andrey Grodzovsky
2021-02-18 20:03                         ` Andrey Grodzovsky
2021-02-19 10:24                         ` Daniel Vetter
2021-02-19 10:24                           ` Daniel Vetter
2021-02-24 16:30                           ` Andrey Grodzovsky
2021-02-24 16:30                             ` Andrey Grodzovsky
2021-02-25 10:25                             ` Daniel Vetter
2021-02-25 10:25                               ` Daniel Vetter
2021-02-25 16:12                               ` Andrey Grodzovsky
2021-02-25 16:12                                 ` Andrey Grodzovsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8ed4a153-d503-e704-0a0d-3422877e50fa@amd.com \
    --to=andrey.grodzovsky@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=yuq825@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.