Re: [Nouveau] [PATCH v2 1/3] drm/nouveau: chan: use struct nvif_mclass
For the whole series:
Reviewed-by: Lyude Paul
On Mon, 2023-10-02 at 15:46 +0200, Danilo Krummrich wrote:
> Use actual struct nvif_mclass instead of identical anonymous struct.
>
> Signed-off-by: Danilo Krummrich
> ---
> drivers/gpu/drm/nouveau/nouveau_chan.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
> b/drivers/gpu/drm/nouveau/nouveau_chan.c
> index 1fd5ccf41128..dffbee59be6a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_chan.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
> @@ -259,10 +259,7 @@ static int
> nouveau_channel_ctor(struct nouveau_drm *drm, struct nvif_device *device,
> bool priv, u64 runm,
>struct nouveau_channel **pchan)
> {
> - static const struct {
> - s32 oclass;
> - int version;
> - } hosts[] = {
> + const struct nvif_mclass hosts[] = {
> { AMPERE_CHANNEL_GPFIFO_B, 0 },
> { AMPERE_CHANNEL_GPFIFO_A, 0 },
> { TURING_CHANNEL_GPFIFO_A, 0 },
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Re: [PATCH 0/9] drm: Annotate structs with __counted_by
On Mon, Oct 02, 2023 at 08:11:41PM +0200, Christian König wrote: > Am 02.10.23 um 20:08 schrieb Kees Cook: > > On Mon, Oct 02, 2023 at 08:01:57PM +0200, Christian König wrote: > > > Am 02.10.23 um 18:53 schrieb Kees Cook: > > > > On Mon, Oct 02, 2023 at 11:06:19AM -0400, Alex Deucher wrote: > > > > > On Mon, Oct 2, 2023 at 5:20 AM Christian König > > > > > wrote: > > > > > > Am 29.09.23 um 21:33 schrieb Kees Cook: > > > > > > > On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: > > > > > > > > This is a batch of patches touching drm for preparing for the > > > > > > > > coming > > > > > > > > implementation by GCC and Clang of the __counted_by attribute. > > > > > > > > Flexible > > > > > > > > array members annotated with __counted_by can have their > > > > > > > > accesses > > > > > > > > bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS > > > > > > > > (for array > > > > > > > > indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > > > > > > > > functions). > > > > > > > > > > > > > > > > As found with Coccinelle[1], add __counted_by to structs that > > > > > > > > would > > > > > > > > benefit from the annotation. > > > > > > > > > > > > > > > > [...] > > > > > > > Since this got Acks, I figure I should carry it in my tree. Let > > > > > > > me know > > > > > > > if this should go via drm instead. > > > > > > > > > > > > > > Applied to for-next/hardening, thanks! > > > > > > > > > > > > > > [1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table > > > > > > > with __counted_by > > > > > > > https://git.kernel.org/kees/c/a6046ac659d6 > > > > > > STOP! In a follow up discussion Alex and I figured out that this > > > > > > won't work. > > > > I'm so confused; from the discussion I saw that Alex said both instances > > > > were false positives? > > > > > > > > > > The value in the structure is byte swapped based on some firmware > > > > > > endianness which not necessary matches the CPU endianness. > > > > > SMU10 is APU only so the endianess of the SMU firmware and the CPU > > > > > will always match. > > > > Which I think is what is being said here? > > > > > > > > > > Please revert that one from going upstream if it's already on it's > > > > > > way. > > > > > > > > > > > > And because of those reasons I strongly think that patches like this > > > > > > should go through the DRM tree :) > > > > Sure, that's fine -- please let me know. It was others Acked/etc. Who > > > > should carry these patches? > > > Probably best if the relevant maintainer pick them up individually. > > > > > > Some of those structures are filled in by firmware/hardware and only the > > > maintainers can judge if that value actually matches what the compiler > > > needs. > > > > > > We have cases where individual bits are used as flags or when the size is > > > byte swapped etc... > > > > > > Even Alex and I didn't immediately say how and where that field is > > > actually > > > used and had to dig that up. That's where the confusion came from. > > Okay, I've dropped them all from my tree. Several had Acks/Reviews, so > > hopefully those can get picked up for the DRM tree? > > I will pick those up to go through drm-misc-next. > > Going to ping maintainers once more when I'm not sure if stuff is correct or > not. Sounds great; thanks! -Kees -- Kees Cook
Re: [PATCH 0/9] drm: Annotate structs with __counted_by
Am 02.10.23 um 20:08 schrieb Kees Cook: On Mon, Oct 02, 2023 at 08:01:57PM +0200, Christian König wrote: Am 02.10.23 um 18:53 schrieb Kees Cook: On Mon, Oct 02, 2023 at 11:06:19AM -0400, Alex Deucher wrote: On Mon, Oct 2, 2023 at 5:20 AM Christian König wrote: Am 29.09.23 um 21:33 schrieb Kees Cook: On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: This is a batch of patches touching drm for preparing for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by to structs that would benefit from the annotation. [...] Since this got Acks, I figure I should carry it in my tree. Let me know if this should go via drm instead. Applied to for-next/hardening, thanks! [1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by https://git.kernel.org/kees/c/a6046ac659d6 STOP! In a follow up discussion Alex and I figured out that this won't work. I'm so confused; from the discussion I saw that Alex said both instances were false positives? The value in the structure is byte swapped based on some firmware endianness which not necessary matches the CPU endianness. SMU10 is APU only so the endianess of the SMU firmware and the CPU will always match. Which I think is what is being said here? Please revert that one from going upstream if it's already on it's way. And because of those reasons I strongly think that patches like this should go through the DRM tree :) Sure, that's fine -- please let me know. It was others Acked/etc. Who should carry these patches? Probably best if the relevant maintainer pick them up individually. Some of those structures are filled in by firmware/hardware and only the maintainers can judge if that value actually matches what the compiler needs. We have cases where individual bits are used as flags or when the size is byte swapped etc... Even Alex and I didn't immediately say how and where that field is actually used and had to dig that up. That's where the confusion came from. Okay, I've dropped them all from my tree. Several had Acks/Reviews, so hopefully those can get picked up for the DRM tree? I will pick those up to go through drm-misc-next. Going to ping maintainers once more when I'm not sure if stuff is correct or not. Christian. Thanks! -Kees
Re: [Nouveau] [PATCH 0/9] drm: Annotate structs with __counted_by
On Mon, Oct 02, 2023 at 08:01:57PM +0200, Christian König wrote: > Am 02.10.23 um 18:53 schrieb Kees Cook: > > On Mon, Oct 02, 2023 at 11:06:19AM -0400, Alex Deucher wrote: > > > On Mon, Oct 2, 2023 at 5:20 AM Christian König > > > wrote: > > > > Am 29.09.23 um 21:33 schrieb Kees Cook: > > > > > On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: > > > > > > This is a batch of patches touching drm for preparing for the coming > > > > > > implementation by GCC and Clang of the __counted_by attribute. > > > > > > Flexible > > > > > > array members annotated with __counted_by can have their accesses > > > > > > bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for > > > > > > array > > > > > > indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > > > > > > functions). > > > > > > > > > > > > As found with Coccinelle[1], add __counted_by to structs that would > > > > > > benefit from the annotation. > > > > > > > > > > > > [...] > > > > > Since this got Acks, I figure I should carry it in my tree. Let me > > > > > know > > > > > if this should go via drm instead. > > > > > > > > > > Applied to for-next/hardening, thanks! > > > > > > > > > > [1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with > > > > > __counted_by > > > > > https://git.kernel.org/kees/c/a6046ac659d6 > > > > STOP! In a follow up discussion Alex and I figured out that this won't > > > > work. > > I'm so confused; from the discussion I saw that Alex said both instances > > were false positives? > > > > > > The value in the structure is byte swapped based on some firmware > > > > endianness which not necessary matches the CPU endianness. > > > SMU10 is APU only so the endianess of the SMU firmware and the CPU > > > will always match. > > Which I think is what is being said here? > > > > > > Please revert that one from going upstream if it's already on it's way. > > > > > > > > And because of those reasons I strongly think that patches like this > > > > should go through the DRM tree :) > > Sure, that's fine -- please let me know. It was others Acked/etc. Who > > should carry these patches? > > Probably best if the relevant maintainer pick them up individually. > > Some of those structures are filled in by firmware/hardware and only the > maintainers can judge if that value actually matches what the compiler > needs. > > We have cases where individual bits are used as flags or when the size is > byte swapped etc... > > Even Alex and I didn't immediately say how and where that field is actually > used and had to dig that up. That's where the confusion came from. Okay, I've dropped them all from my tree. Several had Acks/Reviews, so hopefully those can get picked up for the DRM tree? Thanks! -Kees -- Kees Cook
Re: [PATCH 0/9] drm: Annotate structs with __counted_by
Am 02.10.23 um 18:53 schrieb Kees Cook: On Mon, Oct 02, 2023 at 11:06:19AM -0400, Alex Deucher wrote: On Mon, Oct 2, 2023 at 5:20 AM Christian König wrote: Am 29.09.23 um 21:33 schrieb Kees Cook: On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: This is a batch of patches touching drm for preparing for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by to structs that would benefit from the annotation. [...] Since this got Acks, I figure I should carry it in my tree. Let me know if this should go via drm instead. Applied to for-next/hardening, thanks! [1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by https://git.kernel.org/kees/c/a6046ac659d6 STOP! In a follow up discussion Alex and I figured out that this won't work. I'm so confused; from the discussion I saw that Alex said both instances were false positives? The value in the structure is byte swapped based on some firmware endianness which not necessary matches the CPU endianness. SMU10 is APU only so the endianess of the SMU firmware and the CPU will always match. Which I think is what is being said here? Please revert that one from going upstream if it's already on it's way. And because of those reasons I strongly think that patches like this should go through the DRM tree :) Sure, that's fine -- please let me know. It was others Acked/etc. Who should carry these patches? Probably best if the relevant maintainer pick them up individually. Some of those structures are filled in by firmware/hardware and only the maintainers can judge if that value actually matches what the compiler needs. We have cases where individual bits are used as flags or when the size is byte swapped etc... Even Alex and I didn't immediately say how and where that field is actually used and had to dig that up. That's where the confusion came from. Regards, Christian. Thanks! -Kees Regards, Christian. [2/9] drm/amdgpu/discovery: Annotate struct ip_hw_instance with __counted_by https://git.kernel.org/kees/c/4df33089b46f [3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by https://git.kernel.org/kees/c/ffd3f823bdf6 [4/9] drm/msm/dpu: Annotate struct dpu_hw_intr with __counted_by https://git.kernel.org/kees/c/2de35a989b76 [5/9] drm/nouveau/pm: Annotate struct nvkm_perfdom with __counted_by https://git.kernel.org/kees/c/188aeb08bfaa [6/9] drm/vc4: Annotate struct vc4_perfmon with __counted_by https://git.kernel.org/kees/c/59a54dc896c3 [7/9] drm/virtio: Annotate struct virtio_gpu_object_array with __counted_by https://git.kernel.org/kees/c/5cd476de33af [8/9] drm/vmwgfx: Annotate struct vmw_surface_dirty with __counted_by https://git.kernel.org/kees/c/b426f2e5356a [9/9] drm/v3d: Annotate struct v3d_perfmon with __counted_by https://git.kernel.org/kees/c/dc662fa1b0e4 Take care,
Re: [PATCH 0/9] drm: Annotate structs with __counted_by
On Mon, Oct 02, 2023 at 11:06:19AM -0400, Alex Deucher wrote: > On Mon, Oct 2, 2023 at 5:20 AM Christian König > wrote: > > > > Am 29.09.23 um 21:33 schrieb Kees Cook: > > > On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: > > >> This is a batch of patches touching drm for preparing for the coming > > >> implementation by GCC and Clang of the __counted_by attribute. Flexible > > >> array members annotated with __counted_by can have their accesses > > >> bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array > > >> indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). > > >> > > >> As found with Coccinelle[1], add __counted_by to structs that would > > >> benefit from the annotation. > > >> > > >> [...] > > > Since this got Acks, I figure I should carry it in my tree. Let me know > > > if this should go via drm instead. > > > > > > Applied to for-next/hardening, thanks! > > > > > > [1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with > > > __counted_by > > >https://git.kernel.org/kees/c/a6046ac659d6 > > > > STOP! In a follow up discussion Alex and I figured out that this won't work. I'm so confused; from the discussion I saw that Alex said both instances were false positives? > > > > The value in the structure is byte swapped based on some firmware > > endianness which not necessary matches the CPU endianness. > > SMU10 is APU only so the endianess of the SMU firmware and the CPU > will always match. Which I think is what is being said here? > > Please revert that one from going upstream if it's already on it's way. > > > > And because of those reasons I strongly think that patches like this > > should go through the DRM tree :) Sure, that's fine -- please let me know. It was others Acked/etc. Who should carry these patches? Thanks! -Kees > > > > Regards, > > Christian. > > > > > [2/9] drm/amdgpu/discovery: Annotate struct ip_hw_instance with > > > __counted_by > > >https://git.kernel.org/kees/c/4df33089b46f > > > [3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by > > >https://git.kernel.org/kees/c/ffd3f823bdf6 > > > [4/9] drm/msm/dpu: Annotate struct dpu_hw_intr with __counted_by > > >https://git.kernel.org/kees/c/2de35a989b76 > > > [5/9] drm/nouveau/pm: Annotate struct nvkm_perfdom with __counted_by > > >https://git.kernel.org/kees/c/188aeb08bfaa > > > [6/9] drm/vc4: Annotate struct vc4_perfmon with __counted_by > > >https://git.kernel.org/kees/c/59a54dc896c3 > > > [7/9] drm/virtio: Annotate struct virtio_gpu_object_array with > > > __counted_by > > >https://git.kernel.org/kees/c/5cd476de33af > > > [8/9] drm/vmwgfx: Annotate struct vmw_surface_dirty with __counted_by > > >https://git.kernel.org/kees/c/b426f2e5356a > > > [9/9] drm/v3d: Annotate struct v3d_perfmon with __counted_by > > >https://git.kernel.org/kees/c/dc662fa1b0e4 > > > > > > Take care, > > > > > -- Kees Cook
Re: [Nouveau] [PATCH drm-misc-next v5 4/6] drm/gpuvm: track/lock/validate external/evicted objects
Hi Danilo,
kernel test robot noticed the following build warnings:
[auto build test WARNING on a4ead6e37e3290cff399e2598d75e98777b69b37]
url:
https://github.com/intel-lab-lkp/linux/commits/Danilo-Krummrich/drm-gpuvm-add-common-dma-resv-per-struct-drm_gpuvm/20230929-031831
base: a4ead6e37e3290cff399e2598d75e98777b69b37
patch link:
https://lore.kernel.org/r/20230928191624.13703-5-dakr%40redhat.com
patch subject: [PATCH drm-misc-next v5 4/6] drm/gpuvm: track/lock/validate
external/evicted objects
reproduce:
(https://download.01.org/0day-ci/archive/20231002/202310022331.lpoa8krt-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202310022331.lpoa8krt-...@intel.com/
All warnings (new ones prefixed by >>):
>> ./include/drm/drm_gpuvm.h:563: warning: Function parameter or member
>> 'vm_exec' not described in 'drm_gpuvm_exec_unlock'
>> ./include/drm/drm_gpuvm.h:563: warning: expecting prototype for
>> drm_gpuvm_lock(). Prototype was for drm_gpuvm_exec_unlock() instead
>> ./include/drm/drm_gpuvm.h:601: warning: expecting prototype for
>> drm_gpuvm_exec_resv_add_fence(). Prototype was for drm_gpuvm_exec_validate()
>> instead
vim +563 ./include/drm/drm_gpuvm.h
527
528 int drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
529struct drm_exec *exec,
530unsigned int num_fences);
531
532 int drm_gpuvm_prepare_range(struct drm_gpuvm *gpuvm,
533 struct drm_exec *exec,
534 u64 addr, u64 range,
535 unsigned int num_fences);
536
537 int drm_gpuvm_exec_lock(struct drm_gpuvm_exec *vm_exec,
538 unsigned int num_fences,
539 bool interruptible);
540
541 int drm_gpuvm_exec_lock_array(struct drm_gpuvm_exec *vm_exec,
542struct drm_gem_object **objs,
543unsigned int num_objs,
544unsigned int num_fences,
545bool interruptible);
546
547 int drm_gpuvm_exec_lock_range(struct drm_gpuvm_exec *vm_exec,
548u64 addr, u64 range,
549unsigned int num_fences,
550bool interruptible);
551
552 /**
553 * drm_gpuvm_lock() - lock all dma-resv of all assoiciated BOs
554 * @gpuvm: the _gpuvm
555 *
556 * Releases all dma-resv locks of all _gem_objects previously
acquired
557 * through drm_gpuvm_lock() or its variants.
558 *
559 * Returns: 0 on success, negative error code on failure.
560 */
561 static inline void
562 drm_gpuvm_exec_unlock(struct drm_gpuvm_exec *vm_exec)
> 563 {
564 drm_exec_fini(_exec->exec);
565 }
566
567 int drm_gpuvm_validate(struct drm_gpuvm *gpuvm, struct drm_exec *exec);
568 void drm_gpuvm_resv_add_fence(struct drm_gpuvm *gpuvm,
569struct drm_exec *exec,
570struct dma_fence *fence,
571enum dma_resv_usage private_usage,
572enum dma_resv_usage extobj_usage);
573
574 /**
575 * drm_gpuvm_exec_resv_add_fence()
576 * @vm_exec: the _gpuvm_exec abstraction
577 * @fence: fence to add
578 * @private_usage: private dma-resv usage
579 * @extobj_usage: extobj dma-resv usage
580 *
581 * See drm_gpuvm_resv_add_fence().
582 */
583 static inline void
584 drm_gpuvm_exec_resv_add_fence(struct drm_gpuvm_exec *vm_exec,
585struct dma_fence *fence,
586enum dma_resv_usage private_usage,
587enum dma_resv_usage extobj_usage)
588 {
589 drm_gpuvm_resv_add_fence(vm_exec->vm, _exec->exec, fence,
590 private_usage, extobj_usage);
591 }
592
593 /**
594 * drm_gpuvm_exec_resv_add_fence()
595 * @vm_exec: the _gpuvm_exec abstraction
596 *
597 * See drm_gpuvm_validate().
598 */
599 static inline int
600 drm_gpuvm_exec_validate(struct drm_gpuvm_exec *vm_exec)
> 601 {
602 return drm_gpuvm_validate(vm_exec->vm, _exec->exec);
603 }
604
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 0/9] drm: Annotate structs with __counted_by
On Mon, Oct 2, 2023 at 5:20 AM Christian König wrote: > > Am 29.09.23 um 21:33 schrieb Kees Cook: > > On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: > >> This is a batch of patches touching drm for preparing for the coming > >> implementation by GCC and Clang of the __counted_by attribute. Flexible > >> array members annotated with __counted_by can have their accesses > >> bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array > >> indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). > >> > >> As found with Coccinelle[1], add __counted_by to structs that would > >> benefit from the annotation. > >> > >> [...] > > Since this got Acks, I figure I should carry it in my tree. Let me know > > if this should go via drm instead. > > > > Applied to for-next/hardening, thanks! > > > > [1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with > > __counted_by > >https://git.kernel.org/kees/c/a6046ac659d6 > > STOP! In a follow up discussion Alex and I figured out that this won't work. > > The value in the structure is byte swapped based on some firmware > endianness which not necessary matches the CPU endianness. SMU10 is APU only so the endianess of the SMU firmware and the CPU will always match. Alex > > Please revert that one from going upstream if it's already on it's way. > > And because of those reasons I strongly think that patches like this > should go through the DRM tree :) > > Regards, > Christian. > > > [2/9] drm/amdgpu/discovery: Annotate struct ip_hw_instance with __counted_by > >https://git.kernel.org/kees/c/4df33089b46f > > [3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by > >https://git.kernel.org/kees/c/ffd3f823bdf6 > > [4/9] drm/msm/dpu: Annotate struct dpu_hw_intr with __counted_by > >https://git.kernel.org/kees/c/2de35a989b76 > > [5/9] drm/nouveau/pm: Annotate struct nvkm_perfdom with __counted_by > >https://git.kernel.org/kees/c/188aeb08bfaa > > [6/9] drm/vc4: Annotate struct vc4_perfmon with __counted_by > >https://git.kernel.org/kees/c/59a54dc896c3 > > [7/9] drm/virtio: Annotate struct virtio_gpu_object_array with __counted_by > >https://git.kernel.org/kees/c/5cd476de33af > > [8/9] drm/vmwgfx: Annotate struct vmw_surface_dirty with __counted_by > >https://git.kernel.org/kees/c/b426f2e5356a > > [9/9] drm/v3d: Annotate struct v3d_perfmon with __counted_by > >https://git.kernel.org/kees/c/dc662fa1b0e4 > > > > Take care, > > >
[Nouveau] [PATCH v2 2/3] drm/nouveau: chan: use channel class definitions
Use channel class definitions instead of magic numbers.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/nouveau_chan.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
b/drivers/gpu/drm/nouveau/nouveau_chan.c
index dffbee59be6a..ac56f4689ee3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -442,9 +442,11 @@ nouveau_channel_init(struct nouveau_channel *chan, u32
vram, u32 gart)
}
/* initialise dma tracking parameters */
- switch (chan->user.oclass & 0x00ff) {
- case 0x006b:
- case 0x006e:
+ switch (chan->user.oclass) {
+ case NV03_CHANNEL_DMA:
+ case NV10_CHANNEL_DMA:
+ case NV17_CHANNEL_DMA:
+ case NV40_CHANNEL_DMA:
chan->user_put = 0x40;
chan->user_get = 0x44;
chan->dma.max = (0x1 / 4) - 2;
--
2.41.0
[Nouveau] [PATCH v2 3/3] drm/nouveau: exec: report max pushs through getparam
Report the maximum number of IBs that can be pushed with a single
DRM_IOCTL_NOUVEAU_EXEC through DRM_IOCTL_NOUVEAU_GETPARAM.
While the maximum number of IBs per ring might vary between chipsets,
the kernel will make sure that userspace can only push a fraction of the
maximum number of IBs per ring per job, such that we avoid a situation
where there's only a single job occupying the ring, which could
potentially lead to the ring run dry.
Using DRM_IOCTL_NOUVEAU_GETPARAM to report the maximum number of IBs
that can be pushed with a single DRM_IOCTL_NOUVEAU_EXEC implies that
all channels of a given device have the same ring size.
Acked-by: Faith Ekstrand
Signed-off-by: Danilo Krummrich
---
Changes in V2
=
- consider the extra slot required by a job's HW fence
---
drivers/gpu/drm/nouveau/nouveau_abi16.c | 21 +
drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_dma.h | 3 +++
drivers/gpu/drm/nouveau/nouveau_exec.c | 7 ---
drivers/gpu/drm/nouveau/nouveau_exec.h | 10 ++
include/uapi/drm/nouveau_drm.h | 10 ++
6 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c
b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index 30afbec9e3b1..2edd7bb13fae 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -31,6 +31,7 @@
#include "nouveau_drv.h"
#include "nouveau_dma.h"
+#include "nouveau_exec.h"
#include "nouveau_gem.h"
#include "nouveau_chan.h"
#include "nouveau_abi16.h"
@@ -183,6 +184,20 @@ nouveau_abi16_fini(struct nouveau_abi16 *abi16)
cli->abi16 = NULL;
}
+static inline int
+getparam_dma_ib_max(struct nvif_device *device)
+{
+ const struct nvif_mclass dmas[] = {
+ { NV03_CHANNEL_DMA, 0 },
+ { NV10_CHANNEL_DMA, 0 },
+ { NV17_CHANNEL_DMA, 0 },
+ { NV40_CHANNEL_DMA, 0 },
+ {}
+ };
+
+ return nvif_mclass(>object, dmas) < 0 ? NV50_DMA_IB_MAX : 0;
+}
+
int
nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
{
@@ -247,6 +262,12 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
case NOUVEAU_GETPARAM_GRAPH_UNITS:
getparam->value = nvkm_gr_units(gr);
break;
+ case NOUVEAU_GETPARAM_EXEC_PUSH_MAX: {
+ int ib_max = getparam_dma_ib_max(device);
+
+ getparam->value = nouveau_exec_push_max_from_ib_max(ib_max);
+ break;
+ }
default:
NV_PRINTK(dbg, cli, "unknown parameter %lld\n",
getparam->param);
return -EINVAL;
diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
b/drivers/gpu/drm/nouveau/nouveau_chan.c
index ac56f4689ee3..c3c2ab887978 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -456,7 +456,7 @@ nouveau_channel_init(struct nouveau_channel *chan, u32
vram, u32 gart)
chan->user_get = 0x44;
chan->user_get_hi = 0x60;
chan->dma.ib_base = 0x1 / 4;
- chan->dma.ib_max = (0x02000 / 8) - 1;
+ chan->dma.ib_max = NV50_DMA_IB_MAX;
chan->dma.ib_put = 0;
chan->dma.ib_free = chan->dma.ib_max - chan->dma.ib_put;
chan->dma.max = chan->dma.ib_base;
diff --git a/drivers/gpu/drm/nouveau/nouveau_dma.h
b/drivers/gpu/drm/nouveau/nouveau_dma.h
index 1744d95b233e..c52cda82353e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dma.h
+++ b/drivers/gpu/drm/nouveau/nouveau_dma.h
@@ -49,6 +49,9 @@ void nv50_dma_push(struct nouveau_channel *, u64 addr, u32
length,
/* Maximum push buffer size. */
#define NV50_DMA_PUSH_MAX_LENGTH 0x7f
+/* Maximum IBs per ring. */
+#define NV50_DMA_IB_MAX ((0x02000 / 8) - 1)
+
/* Object handles - for stuff that's doesn't use handle == oclass. */
enum {
NvDmaFB = 0x8002,
diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.c
b/drivers/gpu/drm/nouveau/nouveau_exec.c
index ba6913a3efb6..7a40509fa7ea 100644
--- a/drivers/gpu/drm/nouveau/nouveau_exec.c
+++ b/drivers/gpu/drm/nouveau/nouveau_exec.c
@@ -346,7 +346,7 @@ nouveau_exec_ioctl_exec(struct drm_device *dev,
struct nouveau_channel *chan = NULL;
struct nouveau_exec_job_args args = {};
struct drm_nouveau_exec *req = data;
- int ret = 0;
+ int push_max, ret = 0;
if (unlikely(!abi16))
return -ENOMEM;
@@ -371,9 +371,10 @@ nouveau_exec_ioctl_exec(struct drm_device *dev,
if (!chan->dma.ib_max)
return nouveau_abi16_put(abi16, -ENOSYS);
- if (unlikely(req->push_count > NOUVEAU_GEM_MAX_PUSH)) {
+ push_max = nouveau_exec_push_max_from_ib_max(chan->dma.ib_max);
+ if (unlikely(req->push_count > push_max)) {
NV_PRINTK(err, cli, "pushbuf push count exceeds limit: %d max
%d\n",
-req->push_count,
[Nouveau] [PATCH v2 1/3] drm/nouveau: chan: use struct nvif_mclass
Use actual struct nvif_mclass instead of identical anonymous struct.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/nouveau/nouveau_chan.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
b/drivers/gpu/drm/nouveau/nouveau_chan.c
index 1fd5ccf41128..dffbee59be6a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -259,10 +259,7 @@ static int
nouveau_channel_ctor(struct nouveau_drm *drm, struct nvif_device *device, bool
priv, u64 runm,
struct nouveau_channel **pchan)
{
- static const struct {
- s32 oclass;
- int version;
- } hosts[] = {
+ const struct nvif_mclass hosts[] = {
{ AMPERE_CHANNEL_GPFIFO_B, 0 },
{ AMPERE_CHANNEL_GPFIFO_A, 0 },
{ TURING_CHANNEL_GPFIFO_A, 0 },
--
2.41.0
Re: [PATCH 0/9] drm: Annotate structs with __counted_by
Am 29.09.23 um 21:33 schrieb Kees Cook: On Fri, 22 Sep 2023 10:32:05 -0700, Kees Cook wrote: This is a batch of patches touching drm for preparing for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by to structs that would benefit from the annotation. [...] Since this got Acks, I figure I should carry it in my tree. Let me know if this should go via drm instead. Applied to for-next/hardening, thanks! [1/9] drm/amd/pm: Annotate struct smu10_voltage_dependency_table with __counted_by https://git.kernel.org/kees/c/a6046ac659d6 STOP! In a follow up discussion Alex and I figured out that this won't work. The value in the structure is byte swapped based on some firmware endianness which not necessary matches the CPU endianness. Please revert that one from going upstream if it's already on it's way. And because of those reasons I strongly think that patches like this should go through the DRM tree :) Regards, Christian. [2/9] drm/amdgpu/discovery: Annotate struct ip_hw_instance with __counted_by https://git.kernel.org/kees/c/4df33089b46f [3/9] drm/i915/selftests: Annotate struct perf_series with __counted_by https://git.kernel.org/kees/c/ffd3f823bdf6 [4/9] drm/msm/dpu: Annotate struct dpu_hw_intr with __counted_by https://git.kernel.org/kees/c/2de35a989b76 [5/9] drm/nouveau/pm: Annotate struct nvkm_perfdom with __counted_by https://git.kernel.org/kees/c/188aeb08bfaa [6/9] drm/vc4: Annotate struct vc4_perfmon with __counted_by https://git.kernel.org/kees/c/59a54dc896c3 [7/9] drm/virtio: Annotate struct virtio_gpu_object_array with __counted_by https://git.kernel.org/kees/c/5cd476de33af [8/9] drm/vmwgfx: Annotate struct vmw_surface_dirty with __counted_by https://git.kernel.org/kees/c/b426f2e5356a [9/9] drm/v3d: Annotate struct v3d_perfmon with __counted_by https://git.kernel.org/kees/c/dc662fa1b0e4 Take care,
Re: [Nouveau] [PATCH drm-misc-next v5 3/6] drm/gpuvm: add an abstraction for a VM / BO combination
Hi Danilo,
kernel test robot noticed the following build warnings:
[auto build test WARNING on a4ead6e37e3290cff399e2598d75e98777b69b37]
url:
https://github.com/intel-lab-lkp/linux/commits/Danilo-Krummrich/drm-gpuvm-add-common-dma-resv-per-struct-drm_gpuvm/20230929-031831
base: a4ead6e37e3290cff399e2598d75e98777b69b37
patch link:
https://lore.kernel.org/r/20230928191624.13703-4-dakr%40redhat.com
patch subject: [PATCH drm-misc-next v5 3/6] drm/gpuvm: add an abstraction for a
VM / BO combination
reproduce:
(https://download.01.org/0day-ci/archive/20231002/202310021416.3jqeztqg-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202310021416.3jqeztqg-...@intel.com/
All warnings (new ones prefixed by >>):
>> ./include/drm/drm_gpuvm.h:464: warning: Function parameter or member 'vm'
>> not described in 'drm_gpuvm_bo'
vim +464 ./include/drm/drm_gpuvm.h
427
428 /**
429 * @gpuvm: The _gpuvm the @obj is mapped in.
430 */
431 struct drm_gpuvm *vm;
432
433 /**
434 * @obj: The _gem_object being mapped in the @gpuvm.
435 */
436 struct drm_gem_object *obj;
437
438 /**
439 * @kref: The reference count for this _gpuvm_bo.
440 */
441 struct kref kref;
442
443 /**
444 * @list: Structure containing all _heads.
445 */
446 struct {
447 /**
448 * @gpuva: The list of linked _gpuvas.
449 */
450 struct list_head gpuva;
451
452 /**
453 * @entry: Structure containing all _heads serving
as
454 * entry.
455 */
456 struct {
457 /**
458 * @gem: List entry to attach to the
_gem_objects
459 * gpuva list.
460 */
461 struct list_head gem;
462 } entry;
463 } list;
> 464 };
465
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
