| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <D89D60253BB73A4E8C62F9FD18A939CA01031F98@storexdag02.amd.com>
Date: Sat, 12 Jul 2014 00:37:49 +0000
From: "Bridgman, John" <John.Bridgman@....com>
To: Jerome Glisse <j.glisse@...il.com>,
Oded Gabbay <oded.gabbay@...il.com>
CC: David Airlie <airlied@...ux.ie>,
"Deucher, Alexander" <Alexander.Deucher@....com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"Lewycky, Andrew" <Andrew.Lewycky@....com>,
Joerg Roedel <joro@...tes.org>,
"Gabbay, Oded" <Oded.Gabbay@....com>,
"Koenig, Christian" <Christian.Koenig@....com>
Subject: RE: [PATCH 07/83] drm/radeon: Add kfd-->kgd interface of locking
srbm_gfx_cntl register
>-----Original Message-----
>From: Bridgman, John
>Sent: Friday, July 11, 2014 1:48 PM
>To: 'Jerome Glisse'; Oded Gabbay
>Cc: David Airlie; Deucher, Alexander; linux-kernel@...r.kernel.org; dri-
>devel@...ts.freedesktop.org; Lewycky, Andrew; Joerg Roedel; Gabbay, Oded;
>Koenig, Christian
>Subject: RE: [PATCH 07/83] drm/radeon: Add kfd-->kgd interface of locking
>srbm_gfx_cntl register
>
>Checking... we shouldn't need to call the lock from kfd any more.We should
>be able to do any required locking in radeon kgd code.
Confirmed. The locking functions are removed from the interface in commit 82 :
[PATCH 82/83] drm/radeon: Remove lock functions from kfd2kgd interface
There is an elegant symmetry there, but yeah we need to find a way to make this less awkward to review without screwing up all the work you've done so far. It's not obvious how to do that though. I looked at squashing into a smaller number of big commits earlier on but unless we completely rip the code out and recreate from scratch I don't see anything better than :
- a few foundation commits
- a big code dump that covers everything up to ~patch 54 (with 71 squashed in)
- remaining commits squashed a bit to combine fixes with initial code
Is that what you had in mind when you said ~10 big commits ? Our feeling was that the need to skip over the original scheduler would make it more like "one really big commit and 10-20 smaller ones", and I think we all felt that the "big code dump" required to skip over the original scheduler would be a non-starter.
I guess there is another option, and maybe that's what you had in mind -- breaking the "big code dump" into smaller commits would be possible if we were willing to not have working code until we got to the equivalent of ~patch 54 (+71) when all the new scheduler bits were in. Maybe that would still be an improvement ?
Thanks,
JB
>
>>-----Original Message-----
>>From: Jerome Glisse [mailto:j.glisse@...il.com]
>>Sent: Friday, July 11, 2014 12:35 PM
>>To: Oded Gabbay
>>Cc: David Airlie; Deucher, Alexander; linux-kernel@...r.kernel.org;
>>dri- devel@...ts.freedesktop.org; Bridgman, John; Lewycky, Andrew;
>>Joerg Roedel; Gabbay, Oded; Koenig, Christian
>>Subject: Re: [PATCH 07/83] drm/radeon: Add kfd-->kgd interface of
>>locking srbm_gfx_cntl register
>>
>>On Fri, Jul 11, 2014 at 12:50:07AM +0300, Oded Gabbay wrote:
>>> This patch adds a new interface to kfd2kgd_calls structure, which
>>> allows the kfd to lock and unlock the srbm_gfx_cntl register
>>
>>Why does kfd needs to lock this register if kfd can not access any of
>>those register ? This sounds broken to me, exposing a driver internal
>>mutex to another driver is not something i am fan of.
>>
>>Cheers,
>>Jérôme
>>
>>>
>>> Signed-off-by: Oded Gabbay <oded.gabbay@....com>
>>> ---
>>> drivers/gpu/drm/radeon/radeon_kfd.c | 20 ++++++++++++++++++++
>>> include/linux/radeon_kfd.h | 4 ++++
>>> 2 files changed, 24 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/radeon_kfd.c
>>> b/drivers/gpu/drm/radeon/radeon_kfd.c
>>> index 66ee36b..594020e 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_kfd.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_kfd.c
>>> @@ -43,6 +43,10 @@ static void unkmap_mem(struct kgd_dev *kgd,
>struct
>>> kgd_mem *mem);
>>>
>>> static uint64_t get_vmem_size(struct kgd_dev *kgd);
>>>
>>> +static void lock_srbm_gfx_cntl(struct kgd_dev *kgd); static void
>>> +unlock_srbm_gfx_cntl(struct kgd_dev *kgd);
>>> +
>>> +
>>> static const struct kfd2kgd_calls kfd2kgd = {
>>> .allocate_mem = allocate_mem,
>>> .free_mem = free_mem,
>>> @@ -51,6 +55,8 @@ static const struct kfd2kgd_calls kfd2kgd = {
>>> .kmap_mem = kmap_mem,
>>> .unkmap_mem = unkmap_mem,
>>> .get_vmem_size = get_vmem_size,
>>> + .lock_srbm_gfx_cntl = lock_srbm_gfx_cntl,
>>> + .unlock_srbm_gfx_cntl = unlock_srbm_gfx_cntl,
>>> };
>>>
>>> static const struct kgd2kfd_calls *kgd2kfd; @@ -233,3 +239,17 @@
>>> static uint64_t get_vmem_size(struct kgd_dev *kgd)
>>>
>>> return rdev->mc.real_vram_size;
>>> }
>>> +
>>> +static void lock_srbm_gfx_cntl(struct kgd_dev *kgd) {
>>> + struct radeon_device *rdev = (struct radeon_device *)kgd;
>>> +
>>> + mutex_lock(&rdev->srbm_mutex);
>>> +}
>>> +
>>> +static void unlock_srbm_gfx_cntl(struct kgd_dev *kgd) {
>>> + struct radeon_device *rdev = (struct radeon_device *)kgd;
>>> +
>>> + mutex_unlock(&rdev->srbm_mutex);
>>> +}
>>> diff --git a/include/linux/radeon_kfd.h b/include/linux/radeon_kfd.h
>>> index c7997d4..40b691c 100644
>>> --- a/include/linux/radeon_kfd.h
>>> +++ b/include/linux/radeon_kfd.h
>>> @@ -81,6 +81,10 @@ struct kfd2kgd_calls {
>>> void (*unkmap_mem)(struct kgd_dev *kgd, struct kgd_mem *mem);
>>>
>>> uint64_t (*get_vmem_size)(struct kgd_dev *kgd);
>>> +
>>> + /* SRBM_GFX_CNTL mutex */
>>> + void (*lock_srbm_gfx_cntl)(struct kgd_dev *kgd);
>>> + void (*unlock_srbm_gfx_cntl)(struct kgd_dev *kgd);
>>> };
>>>
>>> bool kgd2kfd_init(unsigned interface_version,
>>> --
>>> 1.9.1
>>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists