Re: [PATCH v2 08/14] powerpc: Implement ARCH_HAS_KERNEL_FPU_SUPPORT

[Michael Ellerman]

Samuel Holland  writes:
> PowerPC provides an equivalent to the common kernel-mode FPU API, but in
> a different header and using different function names. The PowerPC API
> also requires a non-preemptible context. Add a wrapper header, and
> export the CFLAGS adjustments.
>
> Reviewed-by: Christoph Hellwig 
> Signed-off-by: Samuel Holland 
> ---
>
> (no changes since v1)
>
>  arch/powerpc/Kconfig   |  1 +
>  arch/powerpc/Makefile  |  5 -
>  arch/powerpc/include/asm/fpu.h | 28 
>  3 files changed, 33 insertions(+), 1 deletion(-)
>  create mode 100644 arch/powerpc/include/asm/fpu.h

Acked-by: Michael Ellerman  (powerpc)

cheers

Re: [RFC PATCH 10/12] drm/amd/display: Use ARCH_HAS_KERNEL_FPU_SUPPORT

[Michael Ellerman]

Samuel Holland  writes:
> On 2023-12-11 6:23 AM, Michael Ellerman wrote:
>> Hi Samuel,
>> 
>> Thanks for trying to clean all this up.
>> 
>> One problem below.
>> 
>> Samuel Holland  writes:
>>> Now that all previously-supported architectures select
>>> ARCH_HAS_KERNEL_FPU_SUPPORT, this code can depend on that symbol instead
>>> of the existing list of architectures. It can also take advantage of the
>>> common kernel-mode FPU API and method of adjusting CFLAGS.
>>>
>>> Signed-off-by: Samuel Holland 
>> ...
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c 
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
>>> index 4ae4720535a5..b64f917174ca 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
>>> @@ -87,20 +78,9 @@ void dc_fpu_begin(const char *function_name, const int 
>>> line)
>>> WARN_ON_ONCE(!in_task());
>>> preempt_disable();
>>> depth = __this_cpu_inc_return(fpu_recursion_depth);
>>> -
>>> if (depth == 1) {
>>> -#if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
>>> +   BUG_ON(!kernel_fpu_available());
>>> kernel_fpu_begin();
>>> -#elif defined(CONFIG_PPC64)
>>> -   if (cpu_has_feature(CPU_FTR_VSX_COMP))
>>> -   enable_kernel_vsx();
>>> -   else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP))
>>> -   enable_kernel_altivec();
>>  
>> Note altivec.
>> 
>>> -   else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE))
>>> -   enable_kernel_fp();
>>> -#elif defined(CONFIG_ARM64)
>>> -   kernel_neon_begin();
>>> -#endif
>>> }
>>>  
>>> TRACE_DCN_FPU(true, function_name, line, depth);
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile 
>>> b/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>> index ea7d60f9a9b4..5aad0f572ba3 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
>>> @@ -25,40 +25,8 @@
>>>  # It provides the general basic services required by other DAL
>>>  # subcomponents.
>>>  
>>> -ifdef CONFIG_X86
>>> -dml_ccflags-$(CONFIG_CC_IS_GCC) := -mhard-float
>>> -dml_ccflags := $(dml_ccflags-y) -msse
>>> -endif
>>> -
>>> -ifdef CONFIG_PPC64
>>> -dml_ccflags := -mhard-float -maltivec
>>> -endif
>> 
>> And altivec is enabled in the flags there.
>> 
>> That doesn't match your implementation for powerpc in patch 7, which
>> only deals with float.
>> 
>> I suspect the AMD driver actually doesn't need altivec enabled, but I
>> don't know that for sure. It compiles without it, but I don't have a GPU
>> to actually test. I've added Timothy on Cc who added the support for
>> powerpc to the driver originally, hopefully he has a test system.
>
> I tested this series on a POWER9 system with an AMD Radeon RX 6400 GPU (which
> requires this FPU code to initialize), and got functioning graphics output.

Awesome.

>> Anyway if that's true that it doesn't need altivec we should probably do
>> a lead-up patch that drops altivec from the AMD driver explicitly, eg.
>> as below.
>
> That makes sense to me. Do you want to provide your Signed-off-by so I can 
> send
> this patch with your authorship?

Yeah that'd be great. Patch below. Feel free to adjust the commit
message as you see fit.

cheers


>From c8a2862d2ebe76a023eceb3267fd85262925c0ba Mon Sep 17 00:00:00 2001
From: Michael Ellerman 
Date: Thu, 14 Dec 2023 15:39:05 +1100
Subject: [PATCH] drm/amd/display: Only use hard-float, not altivec on powerpc

The compiler flags enable altivec, but that is not required, hard-float
is sufficient for the code to build and function.

Drop altivec from the compiler flags and adjust the enable/disable code
to only enable FPU use.

Signed-off-by: Michael Ellerman 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c | 12 ++--
 drivers/gpu/drm/amd/display/dc/dml/Makefile|  2 +-
 drivers/gpu/drm/amd/display/dc/dml2/Makefile   |  2 +-
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
index 4ae4720535a5..0de16796466b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
@@ -92,11 +92,7 @@ void dc_fpu_begin(const char *function_name, const int line)
 #if defined(CON

Re: [RFC PATCH 10/12] drm/amd/display: Use ARCH_HAS_KERNEL_FPU_SUPPORT

[Michael Ellerman]

Hi Samuel,

Thanks for trying to clean all this up.

One problem below.

Samuel Holland  writes:
> Now that all previously-supported architectures select
> ARCH_HAS_KERNEL_FPU_SUPPORT, this code can depend on that symbol instead
> of the existing list of architectures. It can also take advantage of the
> common kernel-mode FPU API and method of adjusting CFLAGS.
>
> Signed-off-by: Samuel Holland 
...
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
> index 4ae4720535a5..b64f917174ca 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
> @@ -87,20 +78,9 @@ void dc_fpu_begin(const char *function_name, const int 
> line)
>   WARN_ON_ONCE(!in_task());
>   preempt_disable();
>   depth = __this_cpu_inc_return(fpu_recursion_depth);
> -
>   if (depth == 1) {
> -#if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
> + BUG_ON(!kernel_fpu_available());
>   kernel_fpu_begin();
> -#elif defined(CONFIG_PPC64)
> - if (cpu_has_feature(CPU_FTR_VSX_COMP))
> - enable_kernel_vsx();
> - else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP))
> - enable_kernel_altivec();
 
Note altivec.

> - else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE))
> - enable_kernel_fp();
> -#elif defined(CONFIG_ARM64)
> - kernel_neon_begin();
> -#endif
>   }
>  
>   TRACE_DCN_FPU(true, function_name, line, depth);
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile 
> b/drivers/gpu/drm/amd/display/dc/dml/Makefile
> index ea7d60f9a9b4..5aad0f572ba3 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
> @@ -25,40 +25,8 @@
>  # It provides the general basic services required by other DAL
>  # subcomponents.
>  
> -ifdef CONFIG_X86
> -dml_ccflags-$(CONFIG_CC_IS_GCC) := -mhard-float
> -dml_ccflags := $(dml_ccflags-y) -msse
> -endif
> -
> -ifdef CONFIG_PPC64
> -dml_ccflags := -mhard-float -maltivec
> -endif

And altivec is enabled in the flags there.

That doesn't match your implementation for powerpc in patch 7, which
only deals with float.

I suspect the AMD driver actually doesn't need altivec enabled, but I
don't know that for sure. It compiles without it, but I don't have a GPU
to actually test. I've added Timothy on Cc who added the support for
powerpc to the driver originally, hopefully he has a test system.

Anyway if that's true that it doesn't need altivec we should probably do
a lead-up patch that drops altivec from the AMD driver explicitly, eg.
as below.

cheers


diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
index 4ae4720535a5..0de16796466b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/dc_fpu.c
@@ -92,11 +92,7 @@ void dc_fpu_begin(const char *function_name, const int line)
 #if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
kernel_fpu_begin();
 #elif defined(CONFIG_PPC64)
-   if (cpu_has_feature(CPU_FTR_VSX_COMP))
-   enable_kernel_vsx();
-   else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP))
-   enable_kernel_altivec();
-   else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE))
+   if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE))
enable_kernel_fp();
 #elif defined(CONFIG_ARM64)
kernel_neon_begin();
@@ -125,11 +121,7 @@ void dc_fpu_end(const char *function_name, const int line)
 #if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH)
kernel_fpu_end();
 #elif defined(CONFIG_PPC64)
-   if (cpu_has_feature(CPU_FTR_VSX_COMP))
-   disable_kernel_vsx();
-   else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP))
-   disable_kernel_altivec();
-   else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE))
+   if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE))
disable_kernel_fp();
 #elif defined(CONFIG_ARM64)
kernel_neon_end();
diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile 
b/drivers/gpu/drm/amd/display/dc/dml/Makefile
index 6042a5a6a44f..554c39024a40 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
@@ -31,7 +31,7 @@ dml_ccflags := $(dml_ccflags-y) -msse
 endif
 
 ifdef CONFIG_PPC64
-dml_ccflags := -mhard-float -maltivec
+dml_ccflags := -mhard-float
 endif
 
 ifdef CONFIG_ARM64
diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile 
b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
index acff3449b8d7..7b51364084b5 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
@@ -30,7 +30,7 @@ dml2_ccflags := $(dml2_ccflags-y) 

Re: [PATCH] drm/amdgpu: drop the long-double-128 powerpc check/hack

[Michael Ellerman]

Christophe Leroy  writes:
> Le 31/03/2023 à 12:53, Michael Ellerman a écrit :
>> "Daniel Kolesa"  writes:
>>> Commit c653c591789b ("drm/amdgpu: Re-enable DCN for 64-bit powerpc")
>>> introduced this check as a workaround for the driver not building
>>> with toolchains that default to 64-bit long double.
>> ...
>>> In mainline, this work is now fully done, so this check is fully
>>> redundant and does not do anything except preventing AMDGPU DC
>>> from being built on systems such as those using musl libc. The
>>> last piece of work to enable this was commit c92b7fe0d92a
>>> ("drm/amd/display: move remaining FPU code to dml folder")
>>> and this has since been backported to 6.1 stable (in 6.1.7).
>>>
>>> Relevant issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2288
>> 
>> I looked to pick this up for 6.3 but was still seeing build errors with
>> some compilers. I assumed that was due to some fixes coming in
>> linux-next that I didn't have.
>> 
>> But applying the patch on v6.3-rc4 I still see build errors. This is
>> building allyesconfig with the kernel.org GCC 12.2.0 / binutils 2.39
>> toolchain:
>> 
>>powerpc64le-linux-gnu-ld: 
>> drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.o uses hard float, 
>> arch/powerpc/lib/test_emulate_step.o uses soft float
>>powerpc64le-linux-gnu-ld: failed to merge target specific data of file 
>> drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.o
>> 
>> etc.
>> 
>> All the conflicts are between test_emulate_step.o and some file in 
>> drivers/gpu/drm/amd/display/dc/dml.
>> 
>> So even with all the hard-float code isolated in the dml folder, we
>> still hit build errors, because allyesconfig wants to link those
>> hard-float using objects with soft-float objects from elsewhere in the
>> kernel.
>> 
>> It seems like the only workable fix is to force the kernel build to use
>> 128-bit long double. I'll send a patch doing that.
>> 
>
> Commit 78f0929884d4 ("powerpc/64: Always build with 128-bit long 
> double") I guess ?

Yes.

> Let's drop this patch from patchwork then.

Thanks.

cheers

Re: Fwd: Linux 6.3.1 + AMD RX 570 on ppc64le 4K: Kernel attempted to read user page (1128) - exploit attempt? (uid: 0)

[Michael Ellerman]

Bagas Sanjaya  writes:
> Hi,
>
> I notice a regression report on bugzilla ([1]). As many developers
> don't keep an eye on it, I decide to forward it by email.

Bugs filed against powerpc do get Cc'ed to linuxppc-dev, so we do see
them. Though that doesn't always mean we have time to fix them :)

I have a talos machine with an AMD GPU, but not this model, and mainline
is booting fine for me. So we'll need the original reporter to do some
more debugging.

Looks like there's a possible culprit identified on the gitlab issue:
https://gitlab.freedesktop.org/drm/amd/-/issues/2553

cheers

> Quoting from it:
>
>>  darkbasic 2023-05-10 13:36:37 UTC
>> 
>> I'm using Gentoo Linux on a Raptor CS Talos 2 ppc64le, GPU is an AMD RX 570. 
>> So far the past dozen of kernels (up to 6.2.14) worked flawlessly, but with 
>> 6.3.1 I don't get any video output and I get the following in journalctl:
>> 
>> May 10 15:09:01 talos2 kernel: Kernel attempted to read user page (1128) - 
>> exploit attempt? (uid: 0)
>> May 10 15:09:01 talos2 kernel: BUG: Unable to handle kernel data access on 
>> read at 0x1128
>> May 10 15:09:01 talos2 kernel: Faulting instruction address: 
>> 0xc0080d1a805c
>> May 10 15:09:01 talos2 kernel: Oops: Kernel access of bad area, sig: 11 [#1]
>> May 10 15:09:01 talos2 kernel: LE PAGE_SIZE=4K MMU=Radix SMP NR_CPUS=512 
>> NUMA PowerNV
>> May 10 15:09:01 talos2 kernel: Modules linked in: rfkill(+) 8021q garp mrp 
>> stp llc binfmt_misc amdgpu uvcvideo uvc videobuf2_vmalloc videobuf2_memops 
>> gpu_sched snd_hda_codec_hdmi i2c_algo_bit at24(+) videobuf2_v4l2 
>> drm_ttm_helper regmap_i2c videobuf2_common ttm snd_usb_audio drm_di>
>> May 10 15:09:01 talos2 kernel: CPU: 0 PID: 188 Comm: kworker/0:3 Not tainted 
>> 6.3.1-gentoo-dist #1
>> May 10 15:09:01 talos2 kernel: Hardware name: T2P9S01 REV 1.01 POWER9 
>> 0x4e1202 opal:skiboot-9858186 PowerNV
>> May 10 15:09:01 talos2 kernel: Workqueue: events_long 
>> drm_dp_check_and_send_link_address [drm_display_helper]
>> May 10 15:09:01 talos2 kernel: NIP:  c0080d1a805c LR: c0080d1a8018 
>> CTR: c0c87900
>> May 10 15:09:01 talos2 kernel: REGS: cbeb3370 TRAP: 0300   Not 
>> tainted  (6.3.1-gentoo-dist)
>> May 10 15:09:01 talos2 kernel: MSR:  9280b033 
>>   CR: 88048223  XER: 005a
>> May 10 15:09:01 talos2 kernel: CFAR: c0c87980 DAR: 1128 
>> DSISR: 4000 IRQMASK: 0 
>>GPR00: c0080d1a8018 cbeb3610 
>> c0080d690f00  
>>GPR04: 0002 c0080d6297c0 
>>  c0002a00b740 
>>GPR08:  1124 
>>  c0080d431560 
>>GPR12: c0c87900 c2a6b000 
>> c0170ad8 c0001a460310 
>>GPR16: 0045 c00022858388 
>> c00026000340 0001 
>>GPR20:  0001 
>> c000260001a0 4000 
>>GPR24: 4000 c0002610 
>> c000228580b8 fffd 
>>GPR28:  c000228580a0 
>> c00022856000 c00022858000 
>> May 10 15:09:01 talos2 kernel: NIP [c0080d1a805c] 
>> is_synaptics_cascaded_panamera+0x244/0x600 [amdgpu]
>> May 10 15:09:01 talos2 kernel: LR [c0080d1a8018] 
>> is_synaptics_cascaded_panamera+0x200/0x600 [amdgpu]
>> May 10 15:09:01 talos2 kernel: Call Trace:
>> May 10 15:09:01 talos2 kernel: [cbeb3610] [c0080d1a8018] 
>> is_synaptics_cascaded_panamera+0x200/0x600 [amdgpu] (unreliable)
>> May 10 15:09:01 talos2 kernel: [cbeb36d0] [c0080b7c2b18] 
>> drm_helper_probe_single_connector_modes+0x230/0x698 [drm_kms_helper]
>> May 10 15:09:01 talos2 kernel: [cbeb3810] [c0c57174] 
>> drm_client_modeset_probe+0x2b4/0x16c0
>> May 10 15:09:01 talos2 kernel: [cbeb3a10] [c0080b7c7a30] 
>> __drm_fb_helper_initial_config_and_unlock+0x68/0x640 [drm_kms_helper]
>> May 10 15:09:01 talos2 kernel: [cbeb3af0] [c0080b7c5b08] 
>> drm_fbdev_client_hotplug+0x40/0x1d0 [drm_kms_helper]
>> May 10 15:09:01 talos2 kernel: [cbeb3b70] [c0c55480] 
>> drm_client_dev_hotplug+0x120/0x1b0
>> May 10 15:09:01 talos2 kernel: [cbeb3c00] [c0080b7c1130] 
>> drm_kms_helper_hotplug_event+0x58/0x80 [drm_kms_helper]
>> May 10 15:09:01 talos2 kernel: [cbeb3c30] [c0080b80b298] 
>> drm_dp_check_and_send_link_address+0x330/0x3a0 [drm_display_helper]
>> May 10 15:09:01 talos2 kernel: [cbeb3cd0] [c0162d84] 
>> process_one_work+0x2f4/0x580
>> May 10 15:09:01 talos2 kernel: [cbeb3d70] [c01630b8] 
>> worker_thread+0xa8/0x600
>> May 10 15:09:01 talos2 kernel: [cbeb3e00] [c0170bf4] 
>> kthread+0x124/0x130
>> May 10 15:09:01 talos2 kernel: 

Re: [PATCH] powerpc/64: Always build with 128-bit long double

[Michael Ellerman]

On Tue, 04 Apr 2023 20:28:47 +1000, Michael Ellerman wrote:
> The amdgpu driver builds some of its code with hard-float enabled,
> whereas the rest of the kernel is built with soft-float.
> 
> When building with 64-bit long double, if soft-float and hard-float
> objects are linked together, the build fails due to incompatible ABI
> tags.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/64: Always build with 128-bit long double
  https://git.kernel.org/powerpc/c/78f0929884d4811c225fd2c57ecc602c84c07392

cheers

Re: In-flight collision: DRM_AMD_DC_DCN renaming

[Michael Ellerman]

Hi Lukas,

Lukas Bulwahn  writes:
> Dear Michael, dear Harry, dear Alex,
>
> The commit 4652ae7a51b7 ("drm/amd/display: Rename DCN config to FP")
> renames config DRM_AMD_DC_DCN to config DRM_AMD_DC_FP. The concurrent
> commit 78f0929884d4 ("powerpc/64: Always build with 128-bit long
> double") overrides the renaming change for the select in config
> DRM_AMD_DC, and this leads to selecting the non-existent
> DRM_AMD_DC_DCN.

The powerpc commit doesn't override the name change, in the powerpc tree
where it's applied the name change hasn't happened yet, see the diff:

diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index 0c9bd0a53e60..e36261d546af 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -8,7 +8,7 @@ config DRM_AMD_DC
depends on BROKEN || !CC_IS_CLANG || X86_64 || SPARC64 || ARM64
select SND_HDA_COMPONENT if SND_HDA_CORE
# !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752
-   select DRM_AMD_DC_DCN if (X86 || PPC_LONG_DOUBLE_128 || (ARM64 && 
KERNEL_MODE_NEON && !CC_IS_CLANG))
+   select DRM_AMD_DC_DCN if (X86 || (PPC64 && ALTIVEC) || (ARM64 && 
KERNEL_MODE_NEON && !CC_IS_CLANG))
help
  Choose this option if you want to use the new display engine
  support for AMDGPU. This adds required support for Vega and


The problem is that the resolution of the merge conflict in linux-next
is incorrect, it takes the powerpc change without taking into account
the rename from the amdgpu commit.

The correct resolution is:

diff --cc drivers/gpu/drm/amd/display/Kconfig
index e36261d546af,06b438217c61..
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@@ -8,7 -8,7 +8,7 @@@ config DRM_AMD_D
depends on BROKEN || !CC_IS_CLANG || X86_64 || SPARC64 || ARM64
select SND_HDA_COMPONENT if SND_HDA_CORE
# !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752
-   select DRM_AMD_DC_DCN if (X86 || (PPC64 && ALTIVEC) || (ARM64 && 
KERNEL_MODE_NEON && !CC_IS_CLANG))
 -  select DRM_AMD_DC_FP if (X86 || PPC64 || (ARM64 && KERNEL_MODE_NEON && 
!CC_IS_CLANG))
++  select DRM_AMD_DC_FP if (X86 || (PPC64 && ALTIVEC) || (ARM64 && 
KERNEL_MODE_NEON && !CC_IS_CLANG))
help
  Choose this option if you want to use the new display engine
  support for AMDGPU. This adds required support for Vega and


(Note that 4652ae7a51b7 incorrectly changed PPC_LONG_DOUBLE_128 to plain
 PPC64, which is why PPC_LONG_DOUBLE_128 doesn't appear in the diff above.)

Possibly the merge resoulution can be fixed in linux-next.

And ultimately the fix is for Linus to do the proper merge resolution
when he eventually merges the two trees.

cheers

Re: [PATCH] powerpc/64: Always build with 128-bit long double

[Michael Ellerman]

Hamza Mahfooz  writes:
> On 4/4/23 06:28, Michael Ellerman wrote:
>> The amdgpu driver builds some of its code with hard-float enabled,
>> whereas the rest of the kernel is built with soft-float.
>> 
>> When building with 64-bit long double, if soft-float and hard-float
>> objects are linked together, the build fails due to incompatible ABI
>> tags.
>> 
>> In the past there have been build errors in the amdgpu driver caused by
>> this, some of those were due to bad intermingling of soft & hard-float
>> code, but those issues have now all been fixed since commit c92b7fe0d92a
>> ("drm/amd/display: move remaining FPU code to dml folder").
>> 
>> However it's still possible for soft & hard-float objects to end up
>> linked together, if the amdgpu driver is built-in to the kernel along
>> with the test_emulate_step.c code, which uses soft-float. That happens
>> in an allyesconfig build.
>> 
>> Currently those build errors are avoided because the amdgpu driver is
>> gated on 128-bit long double being enabled. But that's not a detail the
>> amdgpu driver should need to be aware of, and if another driver starts
>> using hard-float the same problem would occur.
>> 
>> All versions of the 64-bit ABI specify that long-double is 128-bits.
>> However some compilers, notably the kernel.org ones, are built to use
>> 64-bit long double by default.
>> 
>> Apart from this issue of soft vs hard-float, the kernel doesn't care
>> what size long double is. In particular the kernel using 128-bit long
>> double doesn't impact userspace's ability to use 64-bit long double, as
>> musl does.
>> 
>> So always build the 64-bit kernel with 128-bit long double. That should
>> avoid any build errors due to the incompatible ABI tags. Excluding the
>> code that uses soft/hard-float, the vmlinux is identical with/without
>> the flag.
>> 
>> It does mean any code which is incorrectly intermingling soft &
>> hard-float code will build without error, so those bugs will need to be
>> caught by testing rather than at build time.
>> 
>> For more background see:
>>- commit d11219ad53dc ("amdgpu: disable powerpc support for the newer 
>> display engine")
>>- commit c653c591789b ("drm/amdgpu: Re-enable DCN for 64-bit powerpc")
>>- 
>> https://lore.kernel.org/r/dab9cbd8-2626-4b99-8098-31fe76397...@app.fastmail.com
>> 
>> Signed-off-by: Michael Ellerman 
>
> Reviewed-by: Hamza Mahfooz 

Thanks.

> If you'd prefer to have this go through the amdgpu branch, please let
> me know.

I think it makes more sense to go via the powerpc tree, it will get more
testing on powerpc that way.

cheers

Re: [PATCH] powerpc/64: Always build with 128-bit long double

[Michael Ellerman]

Segher Boessenkool  writes:
> On Tue, Apr 04, 2023 at 08:28:47PM +1000, Michael Ellerman wrote:
>> The amdgpu driver builds some of its code with hard-float enabled,
>> whereas the rest of the kernel is built with soft-float.
>> 
>> When building with 64-bit long double, if soft-float and hard-float
>> objects are linked together, the build fails due to incompatible ABI
>> tags.
>
>> Currently those build errors are avoided because the amdgpu driver is
>> gated on 128-bit long double being enabled. But that's not a detail the
>> amdgpu driver should need to be aware of, and if another driver starts
>> using hard-float the same problem would occur.
>
> Well.  The kernel driver either has no business using long double (or
> any other floating point even) at all, or it should know exactly what is
> used: double precision, double-double, or quadruple precision.  Both of
> the latter two are 128 bits.

In a perfect world ... :)

>> All versions of the 64-bit ABI specify that long-double is 128-bits.
>> However some compilers, notably the kernel.org ones, are built to use
>> 64-bit long double by default.
>
> Mea culpa, I suppose?  But builddall doesn't force 64 bit explicitly.
> I wonder how this happened?  Is it maybe a problem in the powerpc64le
> config in GCC itself?

Not blaming anyone, just one of those things that happens. The
toolchains the distros (Ubuntu/Fedora) build all seem to use 128, but
possibly that's because someone told them to configure them that way at
some point.

> I have a patch from summer last year (Arnd's
> toolchains are built without it) that does
> +   powerpc64le-*)  TARGET_GCC_CONF=--with-long-double-128
> Unfortunately I don't remember why I did that, and I never investigated
> what the deeper problem is :-/

Last summer (aka winter) is when we first discovered this issue with the
long double size being implicated.

See:
  https://git.kernel.org/torvalds/c/c653c591789b3acfa4bf6ae45d5af4f330e50a91

So I guess that's what prompted your patch?

> In either case, the kernel should always use specific types, not rely on
> the toolchain to pick a type that may or may not work.  The correct size
> floating point type alone is not enough, but it is a step in the right
> direction certainly.
>
> Reviewed-by: Segher Boessenkool 

Thanks.

cheers

[PATCH] powerpc/64: Always build with 128-bit long double

[Michael Ellerman]

The amdgpu driver builds some of its code with hard-float enabled,
whereas the rest of the kernel is built with soft-float.

When building with 64-bit long double, if soft-float and hard-float
objects are linked together, the build fails due to incompatible ABI
tags.

In the past there have been build errors in the amdgpu driver caused by
this, some of those were due to bad intermingling of soft & hard-float
code, but those issues have now all been fixed since commit c92b7fe0d92a
("drm/amd/display: move remaining FPU code to dml folder").

However it's still possible for soft & hard-float objects to end up
linked together, if the amdgpu driver is built-in to the kernel along
with the test_emulate_step.c code, which uses soft-float. That happens
in an allyesconfig build.

Currently those build errors are avoided because the amdgpu driver is
gated on 128-bit long double being enabled. But that's not a detail the
amdgpu driver should need to be aware of, and if another driver starts
using hard-float the same problem would occur.

All versions of the 64-bit ABI specify that long-double is 128-bits.
However some compilers, notably the kernel.org ones, are built to use
64-bit long double by default.

Apart from this issue of soft vs hard-float, the kernel doesn't care
what size long double is. In particular the kernel using 128-bit long
double doesn't impact userspace's ability to use 64-bit long double, as
musl does.

So always build the 64-bit kernel with 128-bit long double. That should
avoid any build errors due to the incompatible ABI tags. Excluding the
code that uses soft/hard-float, the vmlinux is identical with/without
the flag.

It does mean any code which is incorrectly intermingling soft &
hard-float code will build without error, so those bugs will need to be
caught by testing rather than at build time.

For more background see:
  - commit d11219ad53dc ("amdgpu: disable powerpc support for the newer display 
engine")
  - commit c653c591789b ("drm/amdgpu: Re-enable DCN for 64-bit powerpc")
  - 
https://lore.kernel.org/r/dab9cbd8-2626-4b99-8098-31fe76397...@app.fastmail.com

Signed-off-by: Michael Ellerman 
---
 arch/powerpc/Kconfig| 4 
 arch/powerpc/Makefile   | 1 +
 drivers/gpu/drm/amd/display/Kconfig | 2 +-
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index fc4e81dafca7..3fb2c2766139 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -291,10 +291,6 @@ config PPC
# Please keep this list sorted alphabetically.
#
 
-config PPC_LONG_DOUBLE_128
-   depends on PPC64 && ALTIVEC
-   def_bool $(success,test "$(shell,echo __LONG_DOUBLE_128__ | $(CC) -E -P 
-)" = 1)
-
 config PPC_BARRIER_NOSPEC
bool
default y
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 12447b2361e4..4343cca57cb3 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -133,6 +133,7 @@ endif
 endif
 CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcmodel=medium,$(call 
cc-option,-mminimal-toc))
 CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
+CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mlong-double-128)
 
 # Clang unconditionally reserves r2 on ppc32 and does not support the flag
 # https://bugs.llvm.org/show_bug.cgi?id=39555
diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index 0c9bd0a53e60..e36261d546af 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -8,7 +8,7 @@ config DRM_AMD_DC
depends on BROKEN || !CC_IS_CLANG || X86_64 || SPARC64 || ARM64
select SND_HDA_COMPONENT if SND_HDA_CORE
# !CC_IS_CLANG: https://github.com/ClangBuiltLinux/linux/issues/1752
-   select DRM_AMD_DC_DCN if (X86 || PPC_LONG_DOUBLE_128 || (ARM64 && 
KERNEL_MODE_NEON && !CC_IS_CLANG))
+   select DRM_AMD_DC_DCN if (X86 || (PPC64 && ALTIVEC) || (ARM64 && 
KERNEL_MODE_NEON && !CC_IS_CLANG))
help
  Choose this option if you want to use the new display engine
  support for AMDGPU. This adds required support for Vega and
-- 
2.39.2

Re: [PATCH] drm/amdgpu: drop the long-double-128 powerpc check/hack

[Michael Ellerman]

"Daniel Kolesa"  writes:
> Commit c653c591789b ("drm/amdgpu: Re-enable DCN for 64-bit powerpc")
> introduced this check as a workaround for the driver not building
> with toolchains that default to 64-bit long double.
...
> In mainline, this work is now fully done, so this check is fully
> redundant and does not do anything except preventing AMDGPU DC
> from being built on systems such as those using musl libc. The
> last piece of work to enable this was commit c92b7fe0d92a
> ("drm/amd/display: move remaining FPU code to dml folder")
> and this has since been backported to 6.1 stable (in 6.1.7).
>
> Relevant issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2288

I looked to pick this up for 6.3 but was still seeing build errors with
some compilers. I assumed that was due to some fixes coming in
linux-next that I didn't have.

But applying the patch on v6.3-rc4 I still see build errors. This is
building allyesconfig with the kernel.org GCC 12.2.0 / binutils 2.39
toolchain:

  powerpc64le-linux-gnu-ld: 
drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.o uses hard float, 
arch/powerpc/lib/test_emulate_step.o uses soft float
  powerpc64le-linux-gnu-ld: failed to merge target specific data of file 
drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.o

etc.

All the conflicts are between test_emulate_step.o and some file in 
drivers/gpu/drm/amd/display/dc/dml.

So even with all the hard-float code isolated in the dml folder, we
still hit build errors, because allyesconfig wants to link those
hard-float using objects with soft-float objects from elsewhere in the
kernel.

It seems like the only workable fix is to force the kernel build to use
128-bit long double. I'll send a patch doing that.

cheers

Re: [PATCH 1/7] mm/memory.c: Fix race when faulting a device private page

[Michael Ellerman]

Alistair Popple  writes:
> Michael Ellerman  writes:
>> Alistair Popple  writes:
>>> When the CPU tries to access a device private page the migrate_to_ram()
>>> callback associated with the pgmap for the page is called. However no
>>> reference is taken on the faulting page. Therefore a concurrent
>>> migration of the device private page can free the page and possibly the
>>> underlying pgmap. This results in a race which can crash the kernel due
>>> to the migrate_to_ram() function pointer becoming invalid. It also means
>>> drivers can't reliably read the zone_device_data field because the page
>>> may have been freed with memunmap_pages().
>>>
>>> Close the race by getting a reference on the page while holding the ptl
>>> to ensure it has not been freed. Unfortunately the elevated reference
>>> count will cause the migration required to handle the fault to fail. To
>>> avoid this failure pass the faulting page into the migrate_vma functions
>>> so that if an elevated reference count is found it can be checked to see
>>> if it's expected or not.
>>>
>>> Signed-off-by: Alistair Popple 
>>> ---
>>>  arch/powerpc/kvm/book3s_hv_uvmem.c   | 15 ++-
>>>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 17 +++--
>>>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.h |  2 +-
>>>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 11 +---
>>>  include/linux/migrate.h  |  8 ++-
>>>  lib/test_hmm.c   |  7 ++---
>>>  mm/memory.c  | 16 +++-
>>>  mm/migrate.c | 34 ++---
>>>  mm/migrate_device.c  | 18 +
>>>  9 files changed, 87 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c 
>>> b/arch/powerpc/kvm/book3s_hv_uvmem.c
>>> index 5980063..d4eacf4 100644
>>> --- a/arch/powerpc/kvm/book3s_hv_uvmem.c
>>> +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
>>> @@ -508,10 +508,10 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
...
>>> @@ -994,7 +997,7 @@ static vm_fault_t kvmppc_uvmem_migrate_to_ram(struct 
>>> vm_fault *vmf)
>>>
>>> if (kvmppc_svm_page_out(vmf->vma, vmf->address,
>>> vmf->address + PAGE_SIZE, PAGE_SHIFT,
>>> -   pvt->kvm, pvt->gpa))
>>> +   pvt->kvm, pvt->gpa, vmf->page))
>>> return VM_FAULT_SIGBUS;
>>> else
>>> return 0;
>>
>> I don't have a UV test system, but as-is it doesn't even compile :)
>
> Ugh, thanks. I did get as far as installing a PPC cross-compiler and
> building a kernel. Apparently I did not get as far as enabling
> CONFIG_PPC_UV :)

No worries, that's really on us. If we're going to keep the code in the
tree then it should really be enabled in at least one of our defconfigs.

cheers

Re: [PATCH 1/7] mm/memory.c: Fix race when faulting a device private page

[Michael Ellerman]

Alistair Popple  writes:
> When the CPU tries to access a device private page the migrate_to_ram()
> callback associated with the pgmap for the page is called. However no
> reference is taken on the faulting page. Therefore a concurrent
> migration of the device private page can free the page and possibly the
> underlying pgmap. This results in a race which can crash the kernel due
> to the migrate_to_ram() function pointer becoming invalid. It also means
> drivers can't reliably read the zone_device_data field because the page
> may have been freed with memunmap_pages().
>
> Close the race by getting a reference on the page while holding the ptl
> to ensure it has not been freed. Unfortunately the elevated reference
> count will cause the migration required to handle the fault to fail. To
> avoid this failure pass the faulting page into the migrate_vma functions
> so that if an elevated reference count is found it can be checked to see
> if it's expected or not.
>
> Signed-off-by: Alistair Popple 
> ---
>  arch/powerpc/kvm/book3s_hv_uvmem.c   | 15 ++-
>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 17 +++--
>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.h |  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 11 +---
>  include/linux/migrate.h  |  8 ++-
>  lib/test_hmm.c   |  7 ++---
>  mm/memory.c  | 16 +++-
>  mm/migrate.c | 34 ++---
>  mm/migrate_device.c  | 18 +
>  9 files changed, 87 insertions(+), 41 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c 
> b/arch/powerpc/kvm/book3s_hv_uvmem.c
> index 5980063..d4eacf4 100644
> --- a/arch/powerpc/kvm/book3s_hv_uvmem.c
> +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
> @@ -508,10 +508,10 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
>  static int __kvmppc_svm_page_out(struct vm_area_struct *vma,
>   unsigned long start,
>   unsigned long end, unsigned long page_shift,
> - struct kvm *kvm, unsigned long gpa)
> + struct kvm *kvm, unsigned long gpa, struct page *fault_page)
>  {
>   unsigned long src_pfn, dst_pfn = 0;
> - struct migrate_vma mig;
> + struct migrate_vma mig = { 0 };
>   struct page *dpage, *spage;
>   struct kvmppc_uvmem_page_pvt *pvt;
>   unsigned long pfn;
> @@ -525,6 +525,7 @@ static int __kvmppc_svm_page_out(struct vm_area_struct 
> *vma,
>   mig.dst = _pfn;
>   mig.pgmap_owner = _uvmem_pgmap;
>   mig.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
> + mig.fault_page = fault_page;
>  
>   /* The requested page is already paged-out, nothing to do */
>   if (!kvmppc_gfn_is_uvmem_pfn(gpa >> page_shift, kvm, NULL))
> @@ -580,12 +581,14 @@ static int __kvmppc_svm_page_out(struct vm_area_struct 
> *vma,
>  static inline int kvmppc_svm_page_out(struct vm_area_struct *vma,
> unsigned long start, unsigned long end,
> unsigned long page_shift,
> -   struct kvm *kvm, unsigned long gpa)
> +   struct kvm *kvm, unsigned long gpa,
> +   struct page *fault_page)
>  {
>   int ret;
>  
>   mutex_lock(>arch.uvmem_lock);
> - ret = __kvmppc_svm_page_out(vma, start, end, page_shift, kvm, gpa);
> + ret = __kvmppc_svm_page_out(vma, start, end, page_shift, kvm, gpa,
> + fault_page);
>   mutex_unlock(>arch.uvmem_lock);
>  
>   return ret;
> @@ -736,7 +739,7 @@ static int kvmppc_svm_page_in(struct vm_area_struct *vma,
>   bool pagein)
>  {
>   unsigned long src_pfn, dst_pfn = 0;
> - struct migrate_vma mig;
> + struct migrate_vma mig = { 0 };
>   struct page *spage;
>   unsigned long pfn;
>   struct page *dpage;
> @@ -994,7 +997,7 @@ static vm_fault_t kvmppc_uvmem_migrate_to_ram(struct 
> vm_fault *vmf)
>  
>   if (kvmppc_svm_page_out(vmf->vma, vmf->address,
>   vmf->address + PAGE_SIZE, PAGE_SHIFT,
> - pvt->kvm, pvt->gpa))
> + pvt->kvm, pvt->gpa, vmf->page))
>   return VM_FAULT_SIGBUS;
>   else
>   return 0;

I don't have a UV test system, but as-is it doesn't even compile :)

kvmppc_svm_page_out() is called via some paths other than the
migrate_to_ram callback.

I think it's correct to just pass fault_page = NULL when it's not called
from the migrate_to_ram callback?

Incremental diff below.

cheers


diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c 
b/arch/powerpc/kvm/book3s_hv_uvmem.c
index d4eacf410956..965c9e9e500b 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -637,7 +637,7 @@ void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot 
*slot,

Re: [PATCH] powerpc: export cpu_smallcore_map for modules

[Michael Ellerman]

Randy Dunlap  writes:
> Fix build error when CONFIG_DRM_AMDGPU=m:
>
> ERROR: modpost: "cpu_smallcore_map" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] 
> undefined!
>
> by exporting 'cpu_smallcore_map' just as other per_cpu
> symbols are exported.
>
> drivers/gpu/drm/amd/amdkfd/kfd_device.c calls cpu_smt_mask().
> This is an inline function on powerpc which references
> cpu_smallcore_map.
>
> Fixes: 425752c63b6f ("powerpc: Detect the presence of big-cores via "ibm, 
> thread-groups"")
> Fixes: 7bc913085765 ("drm/amdkfd: Try to schedule bottom half on same core")

That 2nd commit is not in mainline, only linux-next.

I don't mind merging this fix preemptively, but is that SHA stable?

cheers

Re: [PATCH] drm/amd/display: Fix a compilation failure on PowerPC caused by FPU code

[Michael Ellerman]

Rodrigo Siqueira  writes:
> We got a report from Stephen/Michael that the PowerPC build was failing
> with the following error:
>
> ld: drivers/gpu/drm/amd/display/dc/dml/display_mode_lib.o uses hard float, 
> drivers/gpu/drm/amd/display/dc/dcn30/dcn30_optc.o uses soft float
> ld: failed to merge target specific data of file 
> drivers/gpu/drm/amd/display/dc/dcn30/dcn30_optc.o
>
> This error happened because of the function optc3_set_vrr_m_const. This
> function expects a double as a parameter in a code that is not allowed
> to have FPU operations. After further investigation, it became clear
> that optc3_set_vrr_m_const was never invoked, so we can safely drop this
> function and fix the ld issue.
>
> Cc: Alex Deucher 
> Cc: Melissa Wen 
> Reported-by: Stephen Rothwell 
> Reported-by: Michael Ellerman 
> Signed-off-by: Rodrigo Siqueira 
> ---
>  drivers/gpu/drm/amd/display/dc/dcn30/dcn30_optc.c| 8 
>  drivers/gpu/drm/amd/display/dc/dcn30/dcn30_optc.h| 3 ---
>  drivers/gpu/drm/amd/display/dc/dcn32/dcn32_optc.c| 1 -
>  drivers/gpu/drm/amd/display/dc/inc/hw/timing_generator.h | 2 --
>  4 files changed, 14 deletions(-)

Thanks, that fixes the build issue for me.

Tested-by: Michael Ellerman 

cheers

Re: [PATCH] drm/amdgpu: Re-enable DCN for 64-bit powerpc

[Michael Ellerman]

Linus Torvalds  writes:
> On Mon, Jul 25, 2022 at 5:39 AM Michael Ellerman  wrote:
>>
>> Further digging shows that the build failures only occur with compilers
>> that default to 64-bit long double.
>
> Where the heck do we have 'long double' things anywhere in the kernel?

There's one or two uses, but not in any code that's relevant to this
issue AFAICS.

> I tried to grep for it, and failed miserably. I found some constants
> that would qualify, but they were in the v4l colorspaces-details.rst
> doc file.
>
> Strange.

It doesn't seem to matter if you use long double or not. It's just that
if the long double size is 64-bits the linker refuses to link a mixture
of soft/hard-float objects.

The 64-bit ABI says long double is 128-bits, so the compilers that are
using 64-bit long double are either not built correctly, or we are not
passing the correct flags to them.

There's an -mlong-double-128 flag which we can pass at build time which
seems to do the right thing, I will probably add that to the kernel
CFLAGS, but I want that to get a bit more testing.

cheers

Re: [PATCH] drm/amdgpu: Re-enable DCN for 64-bit powerpc

[Michael Ellerman]

Guenter Roeck  writes:
> On 7/25/22 13:42, Segher Boessenkool wrote:
>> On Mon, Jul 25, 2022 at 02:34:08PM -0500, Timothy Pearson wrote:
> Further digging shows that the build failures only occur with compilers
> that default to 64-bit long double.

 Where the heck do we have 'long double' things anywhere in the kernel?

 I tried to grep for it, and failed miserably. I found some constants
 that would qualify, but they were in the v4l colorspaces-details.rst
 doc file.

 Strange.
>>>
>>> We don't, at least not that I can see.  The affected code uses standard 
>>> doubles.
>>>
>>> What I'm wondering is if the compiler is getting confused between standard 
>>> and long doubles when they are both the same bit length...
>> 
>> The compiler emits the same code (DFmode things, double precision float)
>> in both cases, and it itself does not see any difference anymore fairly
>> early in the pipeline.  Compare to int and long on most 32-bit targets,
>> both are SImode, the compiler will not see different types anymore:
>> there *are* no types, except in the compiler frontend.
>> 
>> It only happens for powerpc64le things, and not for powerpc64 builds.
>> 
>> It is probably a GCC problem.  I don't see what forces the GCC build
>> here to use 64-bit long double either btw?  Compilers build via buildall
>> have all kinds of unnecessary things disabled, but not that, not
>> directly at least.
>> 
>
>  From what little documentation I can find, there appears to be
> "--with-long-double-128" and "--with-long-double-format=ieee".
> That looks like something that would need to be enabled, not disabled.
>
> FWIW, depending on compiler build options such as the above for kernel
> builds seems to be a little odd to me, and I am not sure I'd want to
> blame gcc if the kernel wants to be built with 128-bit floating point
> as default.

The kernel doesn't care what the size is, but ld refuses to link objects
built with soft/hard float if the long double size is 64-bits.

> At the very least, that should be documented somewhere,
> and if possible the kernel should refuse to build if the compiler build
> options don't meet the requirements.

The ABI says long double is 128-bits. So it's documented there :)

The kernel expects that passing `-m64 -mlittle-endian -mabi=elfv2` will
produce code that conforms to the 64-bit Little Endian ELFv2 ABI :D

But it seems those flags are not sufficient.

There is an -mlong-double-128 flag, which appears to do the right thing
regardless of how the compiler was built. I will probably add that to
the kernel CFLAGS, but that's not a change I want to do just before the
v5.19 release.

cheers

Re: [PATCH] amdgpu: re-enable DCN for ppc64le

[Michael Ellerman]

Dan Horák  writes:
> On Fri, 22 Jul 2022 22:32:06 +1000
> Michael Ellerman  wrote:
>> Dan Horák  writes:
>> > Commit d11219ad53dc disabled the DCN driver for all platforms that
>> > define PPC64 due long build issues during "make allmodconfig" using
>> > cross-compilation. Cross-compilation defaults to the ppc64_defconfig
>> > and thus big-endian toolchain configuration. The ppc64le platform uses a
>> > different ABI and doesn't suffer from the build issues.
>> 
>> Unfortunately it's a bit messier than that.
>> 
>> The build error occurs when the compiler is built to use a 64-bit long
>> double type.
>> 
>> The ppc64le ABI document says that long double should be 128-bits, but
>> there are ppc64le compilers out there that are configured to use 64-bit
>> long double, notably the kernel.org crosstool compilers.
>> 
>> So just testing for CPU_LITTLE_ENDIAN means we'll still get build errors
>> on those compilers.
>> 
>> But I think we can detect the long double size and key off that. Can you
>> test the patch below works for you?
>> 
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 7aa12e88c580..e9f8cd50af99 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -281,6 +281,9 @@ config PPC
>>  # Please keep this list sorted alphabetically.
>>  #
>>  
>> +config PCC_LONG_DOUBLE_128
>> +def_bool $(success,test "$(shell,echo __LONG_DOUBLE_128__ | $(CC) -E -P 
>> -)" = 1)
>
> ^^^ there is a typo s/PCC/PPC/ :-)

Oops, renamed it after testing :}

> with that fixed, it then defines AMD_DC_DCN on Fedora 36 with
> gcc-12.1.1-1.fc36.ppc64le and we should be OK.

Thanks. I'll send a proper patch.

cheers

[PATCH] drm/amdgpu: Re-enable DCN for 64-bit powerpc

[Michael Ellerman]

Commit d11219ad53dc ("amdgpu: disable powerpc support for the newer
display engine") disabled the DCN driver for all of powerpc due to
unresolved build failures with some compilers.

Further digging shows that the build failures only occur with compilers
that default to 64-bit long double.

Both the ppc64 and ppc64le ABIs define long double to be 128-bits, but
there are compilers in the wild that default to 64-bits. The compilers
provided by the major distros (Fedora, Ubuntu) default to 128-bits and
are not affected by the build failure.

There is a compiler flag to force 128-bit long double, which may be the
correct long term fix, but as an interim fix only allow building the DCN
driver if long double is 128-bits by default.

The bisection in commit d11219ad53dc must have gone off the rails at
some point, the build failure occurs all the way back to the original
commit that enabled DCN support on powerpc, at least with some
toolchains.

Depends-on: d11219ad53dc ("amdgpu: disable powerpc support for the newer 
display engine")
Fixes: 16a9dea110a6 ("amdgpu: Enable initial DCN support on POWER")
Signed-off-by: Michael Ellerman 
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2100
---
 arch/powerpc/Kconfig| 4 
 drivers/gpu/drm/amd/display/Kconfig | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

Alex, are you OK if I take this via the powerpc tree for v5.19?

cheers

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 7aa12e88c580..287cc2d4a4b3 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -281,6 +281,10 @@ config PPC
# Please keep this list sorted alphabetically.
#
 
+config PPC_LONG_DOUBLE_128
+   depends on PPC64
+   def_bool $(success,test "$(shell,echo __LONG_DOUBLE_128__ | $(CC) -E -P 
-)" = 1)
+
 config PPC_BARRIER_NOSPEC
bool
default y
diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index 0ba0598eba20..ec6771e87e73 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -6,7 +6,7 @@ config DRM_AMD_DC
bool "AMD DC - Enable new display engine"
default y
select SND_HDA_COMPONENT if SND_HDA_CORE
-   select DRM_AMD_DC_DCN if X86 && !(KCOV_INSTRUMENT_ALL && 
KCOV_ENABLE_COMPARISONS)
+   select DRM_AMD_DC_DCN if (X86 || PPC_LONG_DOUBLE_128) && 
!(KCOV_INSTRUMENT_ALL && KCOV_ENABLE_COMPARISONS)
help
  Choose this option if you want to use the new display engine
  support for AMDGPU. This adds required support for Vega and
-- 
2.35.3

Re: [PATCH] amdgpu: re-enable DCN for ppc64le

[Michael Ellerman]

Hi Dan,

[ Cc += linuxppc-dev  ]

Dan Horák  writes:
> Commit d11219ad53dc disabled the DCN driver for all platforms that
> define PPC64 due long build issues during "make allmodconfig" using
> cross-compilation. Cross-compilation defaults to the ppc64_defconfig
> and thus big-endian toolchain configuration. The ppc64le platform uses a
> different ABI and doesn't suffer from the build issues.

Unfortunately it's a bit messier than that.

The build error occurs when the compiler is built to use a 64-bit long
double type.

The ppc64le ABI document says that long double should be 128-bits, but
there are ppc64le compilers out there that are configured to use 64-bit
long double, notably the kernel.org crosstool compilers.

So just testing for CPU_LITTLE_ENDIAN means we'll still get build errors
on those compilers.

But I think we can detect the long double size and key off that. Can you
test the patch below works for you?

cheers


diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 7aa12e88c580..e9f8cd50af99 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -281,6 +281,9 @@ config PPC
# Please keep this list sorted alphabetically.
#
 
+config PCC_LONG_DOUBLE_128
+   def_bool $(success,test "$(shell,echo __LONG_DOUBLE_128__ | $(CC) -E -P 
-)" = 1)
+
 config PPC_BARRIER_NOSPEC
bool
default y
diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index b4029c0d5d8c..ec6771e87e73 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -6,7 +6,7 @@ config DRM_AMD_DC
bool "AMD DC - Enable new display engine"
default y
select SND_HDA_COMPONENT if SND_HDA_CORE
-   select DRM_AMD_DC_DCN if (X86 || PPC64) && !(KCOV_INSTRUMENT_ALL && 
KCOV_ENABLE_COMPARISONS)
+   select DRM_AMD_DC_DCN if (X86 || PPC_LONG_DOUBLE_128) && 
!(KCOV_INSTRUMENT_ALL && KCOV_ENABLE_COMPARISONS)
help
  Choose this option if you want to use the new display engine
  support for AMDGPU. This adds required support for Vega and

Re: [RFC] Upstreaming Linux for Nintendo Wii U

[Michael Ellerman]

Ash Logan  writes:
> Hello,

Hi Ash,

I can't really answer all your questions, but I can chime in on one or
two things ...

> - Right now I've made a new platform (like ps3) rather than joining the
> GameCube and Wii in embedded6xx, since that is marked as BROKEN_ON_SMP.
> The Wii U is a 3-core system, though a CPU bug[8] prevents existing
> userspaces working with it. Bit of a "cross that bridge when we get
> there" situation, though I'm reluctant to prevent that possibility by
> using a BROKEN_ON_SMP platform.

I'm happy for it to be a new platform. I'd almost prefer it to be a
separate platform, that way you can make changes in your platform code
without worrying (as much) about breaking other platforms.

> - Like the Wii before it, the Wii U has a small amount of RAM at address
> zero, a gap, then a large amount of RAM at a higher address. Instead of
> the "map everything and reserve the gap" approach of the Wii, we loop
> over each memblock and map only true RAM[9]. This seems to work, but as
> far as I can tell is unique amongst powerpc32 platforms, so it's worth
> pointing out. (Note: I've been told this doesn't work anymore after some
> KUAP changes[10], so this point might be moot; haven't investigated)

We'd need more detail on that I guess. Currently all the 32-bit
platforms use the flat memory model, which assumes RAM is a single
contiguous block. Though that doesn't mean it all has to be used or
mapped, like the Wii does. To properly support your layout you should be
using sparsemem, but it's possible that's more trouble than it's worth,
I'm not sure. How far apart are the low and high blocks of RAM, and what
are their sizes?

> - Due to the aformentioned DMA restrictions and possibly a fatal
> bytemasking bug on uncached mappings[11], I have been wondering if it'd
> be better to just give up on the SRAM at address 0 altogether and use it
> as VRAM or something, loading the kernel at a higher address.

Don't you have exceptions entering down at low addresses? Even so you
could possibly trampoline them up to the kernel at a high address.
 
> In terms of platform bringup, the key issue is whether to be embedded6xx
> or not and what output device to use. Beyond that it's just things like
> IRQ controller drivers, should be pretty straightforward. I think on our
> end, we'll start rebasing to 5.15 (LTS) and start sending patches from
> there. I know getting closer to HEAD is preferable, this project has
> just moved very slowly in the past and being on LTS has been a lifesaver.

As I said I'm happy for it to be a new platform. If there ends up being
a lot of shared code we can always refactor, but embedded6xx is only
~1500 LOC anyway.

One thing that has come up with previous console port submissions is the
requirement for patches to be signed off. The docs are here if you
aren't familiar with them:
  
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

Otherwise your plan sounds good to me, 4.19 is pretty old so getting up
to 5.15 would be a good start. Then submit whatever bits you can and
chip away at it.

cheers

Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()

[Michael Ellerman]

Christoph Hellwig  writes:
> On Wed, Sep 15, 2021 at 07:18:34PM +0200, Christophe Leroy wrote:
>> Could you please provide more explicit explanation why inlining such an
>> helper is considered as bad practice and messy ?
>
> Because now we get architectures to all subly differ.  Look at the mess
> for ioremap and the ioremap* variant.
>
> The only good reason to allow for inlines if if they are used in a hot
> path.  Which cc_platform_has is not, especially not on powerpc.

Yes I agree, it's not a hot path so it doesn't really matter, which is
why I Acked it.

I think it is possible to do both, share the declaration across arches
but also give arches flexibility to use an inline if they prefer, see
patch below.

I'm not suggesting we actually do that for this series now, but I think
it would solve the problem if we ever needed to in future.

cheers


diff --git a/arch/powerpc/platforms/pseries/cc_platform.c 
b/arch/powerpc/include/asm/cc_platform.h
similarity index 74%
rename from arch/powerpc/platforms/pseries/cc_platform.c
rename to arch/powerpc/include/asm/cc_platform.h
index e8021af83a19..6285c3c385a6 100644
--- a/arch/powerpc/platforms/pseries/cc_platform.c
+++ b/arch/powerpc/include/asm/cc_platform.h
@@ -7,13 +7,10 @@
  * Author: Tom Lendacky 
  */
 
-#include 
 #include 
-
-#include 
 #include 
 
-bool cc_platform_has(enum cc_attr attr)
+static inline bool arch_cc_platform_has(enum cc_attr attr)
 {
switch (attr) {
case CC_ATTR_MEM_ENCRYPT:
@@ -23,4 +20,3 @@ bool cc_platform_has(enum cc_attr attr)
return false;
}
 }
-EXPORT_SYMBOL_GPL(cc_platform_has);
diff --git a/arch/x86/include/asm/cc_platform.h 
b/arch/x86/include/asm/cc_platform.h
new file mode 100644
index ..0a4220697043
--- /dev/null
+++ b/arch/x86/include/asm/cc_platform.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_CC_PLATFORM_H_
+#define _ASM_X86_CC_PLATFORM_H_
+
+bool arch_cc_platform_has(enum cc_attr attr);
+
+#endif // _ASM_X86_CC_PLATFORM_H_
diff --git a/arch/x86/kernel/cc_platform.c b/arch/x86/kernel/cc_platform.c
index 3c9bacd3c3f3..77e8c3465979 100644
--- a/arch/x86/kernel/cc_platform.c
+++ b/arch/x86/kernel/cc_platform.c
@@ -11,11 +11,11 @@
 #include 
 #include 
 
-bool cc_platform_has(enum cc_attr attr)
+bool arch_cc_platform_has(enum cc_attr attr)
 {
if (sme_me_mask)
return amd_cc_platform_has(attr);
 
return false;
 }
-EXPORT_SYMBOL_GPL(cc_platform_has);
+EXPORT_SYMBOL_GPL(arch_cc_platform_has);
diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
index 253f3ea66cd8..f3306647c5d9 100644
--- a/include/linux/cc_platform.h
+++ b/include/linux/cc_platform.h
@@ -65,6 +65,8 @@ enum cc_attr {
 
 #ifdef CONFIG_ARCH_HAS_CC_PLATFORM
 
+#include 
+
 /**
  * cc_platform_has() - Checks if the specified cc_attr attribute is active
  * @attr: Confidential computing attribute to check
@@ -77,7 +79,10 @@ enum cc_attr {
  * * TRUE  - Specified Confidential Computing attribute is active
  * * FALSE - Specified Confidential Computing attribute is not active
  */
-bool cc_platform_has(enum cc_attr attr);
+static inline bool cc_platform_has(enum cc_attr attr)
+{
+   return arch_cc_platform_has(attr);
+}
 
 #else  /* !CONFIG_ARCH_HAS_CC_PLATFORM */
 

Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()

[Michael Ellerman]

Borislav Petkov  writes:
> On Wed, Sep 08, 2021 at 05:58:35PM -0500, Tom Lendacky wrote:
>> Introduce a powerpc version of the cc_platform_has() function. This will
>> be used to replace the powerpc mem_encrypt_active() implementation, so
>> the implementation will initially only support the CC_ATTR_MEM_ENCRYPT
>> attribute.
>> 
>> Cc: Michael Ellerman 
>> Cc: Benjamin Herrenschmidt 
>> Cc: Paul Mackerras 
>> Signed-off-by: Tom Lendacky 
>> ---
>>  arch/powerpc/platforms/pseries/Kconfig   |  1 +
>>  arch/powerpc/platforms/pseries/Makefile  |  2 ++
>>  arch/powerpc/platforms/pseries/cc_platform.c | 26 
>>  3 files changed, 29 insertions(+)
>>  create mode 100644 arch/powerpc/platforms/pseries/cc_platform.c
>
> Michael,
>
> can I get an ACK for the ppc bits to carry them through the tip tree
> pls?

Yeah.

I don't love it, a new C file and an out-of-line call to then call back
to a static inline that for most configuration will return false ... but
whatever :)

Acked-by: Michael Ellerman  (powerpc)


> Btw, on a related note, cross-compiling this throws the following error here:
>
> $ make 
> CROSS_COMPILE=/home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/powerpc64-linux-
>  V=1 ARCH=powerpc
>
> ...
>
> /home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/powerpc64-linux-gcc
>  -Wp,-MD,arch/powerpc/boot/.crt0.o.d -D__ASSEMBLY__ -Wall -Wundef 
> -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -O2 -msoft-float 
> -mno-altivec -mno-vsx -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc 
> -include ./include/linux/compiler_attributes.h -I./arch/powerpc/include 
> -I./arch/powerpc/include/generated  -I./include -I./arch/powerpc/include/uapi 
> -I./arch/powerpc/include/generated/uapi -I./include/uapi 
> -I./include/generated/uapi -include ./include/linux/compiler-version.h 
> -include ./include/linux/kconfig.h -m32 -isystem 
> /home/share/src/crosstool/gcc-9.4.0-nolibc/powerpc64-linux/bin/../lib/gcc/powerpc64-linux/9.4.0/include
>  -mbig-endian -nostdinc -c -o arch/powerpc/boot/crt0.o 
> arch/powerpc/boot/crt0.S
> In file included from :
> ././include/linux/compiler_attributes.h:62:5: warning: "__has_attribute" is 
> not defined, evaluates to 0 [-Wundef]
>62 | #if __has_attribute(__assume_aligned__)
>   | ^~~
> ././include/linux/compiler_attributes.h:62:20: error: missing binary operator 
> before token "("
>62 | #if __has_attribute(__assume_aligned__)
>   |^
> ././include/linux/compiler_attributes.h:88:5: warning: "__has_attribute" is 
> not defined, evaluates to 0 [-Wundef]
>88 | #if __has_attribute(__copy__)
>   | ^~~
> ...
>
> Known issue?

Yeah, fixed in mainline today, thanks for trying to cross compile :)

cheers

Re: [PATCH v2 04/12] powerpc/pseries/svm: Add a powerpc version of prot_guest_has()

[Michael Ellerman]

Tom Lendacky  writes:
> Introduce a powerpc version of the prot_guest_has() function. This will
> be used to replace the powerpc mem_encrypt_active() implementation, so
> the implementation will initially only support the PATTR_MEM_ENCRYPT
> attribute.
>
> Cc: Michael Ellerman 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/powerpc/include/asm/protected_guest.h | 30 ++
>  arch/powerpc/platforms/pseries/Kconfig |  1 +
>  2 files changed, 31 insertions(+)
>  create mode 100644 arch/powerpc/include/asm/protected_guest.h
>
> diff --git a/arch/powerpc/include/asm/protected_guest.h 
> b/arch/powerpc/include/asm/protected_guest.h
> new file mode 100644
> index ..ce55c2c7e534
> --- /dev/null
> +++ b/arch/powerpc/include/asm/protected_guest.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Protected Guest (and Host) Capability checks
> + *
> + * Copyright (C) 2021 Advanced Micro Devices, Inc.
> + *
> + * Author: Tom Lendacky 
> + */
> +
> +#ifndef _POWERPC_PROTECTED_GUEST_H
> +#define _POWERPC_PROTECTED_GUEST_H

Minor nit, we would usually use _ASM_POWERPC_PROTECTED_GUEST_H

Otherwise looks OK to me.

Acked-by: Michael Ellerman 

cheers

Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()

[Michael Ellerman]

On Tue, 9 Mar 2021 08:39:39 + (UTC), Christophe Leroy wrote:
> Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
> when CONFIG_VSX is not set, to avoid following build failure.
> 
>   CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
> In file included from 
> ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
>  from 
> ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
>  from 
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 
> 'dcn_bw_apply_registry_override':
> ./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit 
> declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? 
> [-Werror=implicit-function-declaration]
>64 |   enable_kernel_vsx(); \
>   |   ^
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in 
> expansion of macro 'DC_FP_START'
>   640 |  DC_FP_START();
>   |  ^~~
> ./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit 
> declaration of function 'disable_kernel_vsx'; did you mean 
> 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
>75 |   disable_kernel_vsx(); \
>   |   ^~
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in 
> expansion of macro 'DC_FP_END'
>   676 |  DC_FP_END();
>   |  ^
> cc1: some warnings being treated as errors
> make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] 
> Error 1

Applied to powerpc/fixes.

[1/1] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()
  https://git.kernel.org/powerpc/c/bd73758803c2eedc037c2268b65a19542a832594

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

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Michael Ellerman]

Hi Kyle,

KyleMahlkuch  writes:
> From: Kyle Mahlkuch 
>
> During kexec some adapters hit an EEH since they are not properly
> shut down in the radeon_pci_shutdown() function. Adding
> radeon_suspend_kms() fixes this issue.
> Enabled only on PPC because this patch causes issues on some other
> boards.

Which adapters hit the issues?

And do we know why they're not shut down correctly in
radeon_pci_shutdown()? That seems like the root cause no?


> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index 9e55076..4528f4d 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>  static void
>  radeon_pci_shutdown(struct pci_dev *pdev)
>  {
> +#ifdef CONFIG_PPC64
> + struct drm_device *ddev = pci_get_drvdata(pdev);
> +#endif

This local serves no real purpose and could be avoided, which would also
avoid this ifdef.

>   /* if we are running in a VM, make sure the device
>* torn down properly on reboot/shutdown
>*/
>   if (radeon_device_is_virtual())
>   radeon_pci_remove(pdev);
> +
> +#ifdef CONFIG_PPC64
> + /* Some adapters need to be suspended before a

AFAIK drm uses normal kernel comment style, so this should be:

/*
 * Some adapters need to be suspended before a
> +  * shutdown occurs in order to prevent an error
> +  * during kexec.
> +  * Make this power specific becauase it breaks
> +  * some non-power boards.
> +  */
> + radeon_suspend_kms(ddev, true, true, false);

ie, instead do:

radeon_suspend_kms(pci_get_drvdata(pdev), true, true, false);

> +#endif
>  }
>  
>  static int radeon_pmops_suspend(struct device *dev)
> -- 
> 1.8.3.1

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

Re: [PATCH v3 38/41] powerpc: convert put_page() to put_user_page*()

[Michael Ellerman]

John Hubbard  writes:
> On 8/7/19 10:42 PM, Michael Ellerman wrote:
>> Hi John,
>> 
>> john.hubb...@gmail.com writes:
>>> diff --git a/arch/powerpc/mm/book3s64/iommu_api.c 
>>> b/arch/powerpc/mm/book3s64/iommu_api.c
>>> index b056cae3388b..e126193ba295 100644
>>> --- a/arch/powerpc/mm/book3s64/iommu_api.c
>>> +++ b/arch/powerpc/mm/book3s64/iommu_api.c
>>> @@ -203,6 +202,7 @@ static void mm_iommu_unpin(struct 
>>> mm_iommu_table_group_mem_t *mem)
>>>  {
>>> long i;
>>> struct page *page = NULL;
>>> +   bool dirty = false;
>> 
>> I don't think you need that initialisation do you?
>> 
>
> Nope, it can go. Fixed locally, thanks.

Thanks.

> Did you get a chance to look at enough of the other bits to feel comfortable 
> with the patch, overall?

Mostly :) It's not really my area, but all the conversions looked
correct to me as best as I could tell.

So I'm fine for it to go in as part of the series:

Acked-by: Michael Ellerman  (powerpc)

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

Re: [PATCH v3 38/41] powerpc: convert put_page() to put_user_page*()

[Michael Ellerman]

Hi John,

john.hubb...@gmail.com writes:
> diff --git a/arch/powerpc/mm/book3s64/iommu_api.c 
> b/arch/powerpc/mm/book3s64/iommu_api.c
> index b056cae3388b..e126193ba295 100644
> --- a/arch/powerpc/mm/book3s64/iommu_api.c
> +++ b/arch/powerpc/mm/book3s64/iommu_api.c
> @@ -203,6 +202,7 @@ static void mm_iommu_unpin(struct 
> mm_iommu_table_group_mem_t *mem)
>  {
>   long i;
>   struct page *page = NULL;
> + bool dirty = false;

I don't think you need that initialisation do you?

>   if (!mem->hpas)
>   return;
> @@ -215,10 +215,9 @@ static void mm_iommu_unpin(struct 
> mm_iommu_table_group_mem_t *mem)
>   if (!page)
>   continue;
>  
> - if (mem->hpas[i] & MM_IOMMU_TABLE_GROUP_PAGE_DIRTY)
> - SetPageDirty(page);
> + dirty = mem->hpas[i] & MM_IOMMU_TABLE_GROUP_PAGE_DIRTY;
> - put_page(page);
> + put_user_pages_dirty_lock(, 1, dirty);
>   mem->hpas[i] = 0;
>   }
>  }

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

Re: [PATCH] fix double ;;s in code

[Michael Ellerman]

Daniel Vetter  writes:
> On Sun, Feb 18, 2018 at 11:00:56AM +0100, Christophe LEROY wrote:
>> Le 17/02/2018 à 22:19, Pavel Machek a écrit :
>> > 
>> > Fix double ;;'s in code.
>> > 
>> > Signed-off-by: Pavel Machek 
>> 
>> A summary of the files modified on top of the patch would help understand
>> the impact.
>> 
>> A maybe there should be one patch by area, eg one for each arch specific
>> modif and one for drivers/ and one for block/ ?
>
> Yeah, pls split this into one patch per area, with a suitable patch
> subject prefix. Look at git log of each file to get a feeling for what's
> the standard in each area.

This part is actually pretty annoying.

I hacked up a script (below) which seems to do a reasonable job in most
cases.

For this patch it gives:

  $ for f in $(git ls-files -m); do ./guess-prefix.py $f; done
  ARC: 
  ARC: 
  ARM: 
  arm64: ptrace: 
  KVM: PPC: 
  powerpc/powernv:
  x86/efi:
  block/sed-opal:
  clocksource: mips-gic: 
  clocksource/drivers/sun5i:
  drm/amd/display:
  drm/amd/powerplay:
  drm/msm/mdp5:
  drm: 
  iommu/vt-d:
  md: 
  soc: imx: gpc: 

I think those are correct except for:
 - "drm:" for "drivers/gpu/drm/scheduler" which has only a single commit.
 - "md:" for "drivers/md/raid1.c" which is tricked by inconsistent
usage of "md: raid1:" and "md/raid1:".

But that seems like a reasonable hit rate.

Another approach would be to have a file that defines for each subsystem
what the preferred style is, but that is likely to be a PITA to
maintain.

cheers


#!/usr/bin/python3

import sys
import re
from subprocess import check_output
from collections import Counter

if len(sys.argv) != 2:
print('Usage: %s ' % sys.argv[0], file=sys.stderr)
sys.exit(1)

fname = sys.argv[1]

cmd = ['git', 'log', '--format=%s', '-n', '100', fname]
output = check_output(cmd).decode('utf-8')

ignore = ['License', 'Merge']

# Ordered list of patterns
patterns = [
# Common style "foo/bar/baz: Fix the foo"
re.compile('^([\w\-_]+: )+'),
# Less common "foo bar baz: Fix the foo"
re.compile('^([\w\-_]+:? )+: ')
]

words = []
for line in output.splitlines():
prefix = line.split()[0]
for patt in patterns:
match = patt.search(line)
if match:
prefix = match.group(0)
break

if prefix in ignore:
continue

words.append(prefix)

# Warn if we didn't find many examples
if len(words) < 5:
print("Warning: only found %d previous commits to guess from for" % 
len(words),
  fname, file=sys.stderr)

counts = Counter(words)
print(counts.most_common(1)[0][0])
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx