Re: [PATCH v2] printk: Add a short description string to kmsg_dump()

[Michael Ellerman]

Jocelyn Falempe  writes:
> kmsg_dump doesn't forward the panic reason string to the kmsg_dumper
> callback.
> This patch adds a new struct kmsg_dump_detail, that will hold the
> reason and description, and pass it to the dump() callback.
>
> To avoid updating all kmsg_dump() call, it adds a kmsg_dump_desc()
> function and a macro for backward compatibility.
>
> I've written this for drm_panic, but it can be useful for other
> kmsg_dumper.
> It allows to see the panic reason, like "sysrq triggered crash"
> or "VFS: Unable to mount root fs on " on the drm panic screen.
>
> v2:
>  * Use a struct kmsg_dump_detail to hold the reason and description
>pointer, for more flexibility if we want to add other parameters.
>(Kees Cook)
>  * Fix powerpc/nvram_64 build, as I didn't update the forward
>declaration of oops_to_nvram()
>
> Signed-off-by: Jocelyn Falempe 
> ---
>  arch/powerpc/kernel/nvram_64.c     |  8 ++++
>  arch/powerpc/platforms/powernv/opal-kmsg.c |  4 ++--

Acked-by: Michael Ellerman  (powerpc)

cheers

Re: [PATCH v2] printk: Add a short description string to kmsg_dump()

[Michael Ellerman]

Jocelyn Falempe  writes:
> On 02/07/2024 14:26, Jocelyn Falempe wrote:
>> kmsg_dump doesn't forward the panic reason string to the kmsg_dumper
>> callback.
>> This patch adds a new struct kmsg_dump_detail, that will hold the
>> reason and description, and pass it to the dump() callback.
>> 
>> To avoid updating all kmsg_dump() call, it adds a kmsg_dump_desc()
>> function and a macro for backward compatibility.
>> 
>> I've written this for drm_panic, but it can be useful for other
>> kmsg_dumper.
>> It allows to see the panic reason, like "sysrq triggered crash"
>> or "VFS: Unable to mount root fs on " on the drm panic screen.
>> 
>> v2:
>>   * Use a struct kmsg_dump_detail to hold the reason and description
>> pointer, for more flexibility if we want to add other parameters.
>> (Kees Cook)
>>   * Fix powerpc/nvram_64 build, as I didn't update the forward
>> declaration of oops_to_nvram()
>> 
>> Signed-off-by: Jocelyn Falempe 
>> ---
>>   arch/powerpc/kernel/nvram_64.c |  8 
>>   arch/powerpc/platforms/powernv/opal-kmsg.c |  4 ++--
>>   arch/um/kernel/kmsg_dump.c |  2 +-
>>   drivers/gpu/drm/drm_panic.c|  4 ++--
>>   drivers/hv/hv_common.c |  4 ++--
>>   drivers/mtd/mtdoops.c  |  2 +-
>>   fs/pstore/platform.c   | 10 +-
>>   include/linux/kmsg_dump.h  | 22 +++---
>>   kernel/panic.c |  2 +-
>>   kernel/printk/printk.c | 11 ---
>>   10 files changed, 45 insertions(+), 24 deletions(-)
>
...
> Gentle ping, I need reviews from powerpc, usermod linux, mtd, pstore and 
> hyperv, to be able to push it in the drm-misc tree.

For a simple mechanical change like that you don't need reviews from
every subsystem. As long as it's posted to each subsystem and there's
been a bit of time for folks to see it, and the build robots to build
it, that should be sufficient. Otherwise you could be waiting forever.

cheers

Re: (subset) [PATCH 00/34] address all -Wunused-const warnings

[Michael Ellerman]

On Wed, 03 Apr 2024 10:06:18 +0200, Arnd Bergmann wrote:
> Compilers traditionally warn for unused 'static' variables, but not
> if they are constant. The reason here is a custom for C++ programmers
> to define named constants as 'static const' variables in header files
> instead of using macros or enums.
> 
> In W=1 builds, we get warnings only static const variables in C
> files, but not in headers, which is a good compromise, but this still
> produces warning output in at least 30 files. These warnings are
> almost all harmless, but also trivial to fix, and there is no
> good reason to warn only about the non-const variables being unused.
> 
> [...]

Applied to powerpc/next.

[01/34] powerpc/fsl-soc: hide unused const variable

https://git.kernel.org/powerpc/c/01acaf3aa75e1641442cc23d8fe0a7bb4226efb1

cheers

Re: [PATCH v3 15/15] powerpc: Add support for suppressing warning backtraces

[Michael Ellerman]

Guenter Roeck  writes:
> Add name of functions triggering warning backtraces to the __bug_table
> object section to enable support for suppressing WARNING backtraces.
>
> To limit image size impact, the pointer to the function name is only added
> to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
> CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
> parameter is replaced with a (dummy) NULL parameter to avoid an image size
> increase due to unused __func__ entries (this is necessary because __func__
> is not a define but a virtual variable).
>
> Tested-by: Linux Kernel Functional Testing 
> Acked-by: Dan Carpenter 
> Cc: Michael Ellerman 
> Signed-off-by: Guenter Roeck 
> ---
> v2:
> - Rebased to v6.9-rc1
> - Added Tested-by:, Acked-by:, and Reviewed-by: tags
> - Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
> v3:
> - Rebased to v6.9-rc2
>
>  arch/powerpc/include/asm/bug.h | 37 +-
>  1 file changed, 28 insertions(+), 9 deletions(-)

I ran it through some build and boot tests, LGTM.

Acked-by: Michael Ellerman  (powerpc)

cheers

Re: [PATCH 00/14] Add support for suppressing warning backtraces

[Michael Ellerman]

Guenter Roeck  writes:
> On 3/14/24 07:37, Guenter Roeck wrote:
>> On 3/14/24 06:36, Geert Uytterhoeven wrote:
>>> Hi Günter,
>>>
>>> On Tue, Mar 12, 2024 at 6:03 PM Guenter Roeck  wrote:
 Some unit tests intentionally trigger warning backtraces by passing bad
 parameters to kernel API functions. Such unit tests typically check the
 return value from such calls, not the existence of the warning backtrace.

 Such intentionally generated warning backtraces are neither desirable
 nor useful for a number of reasons.
 - They can result in overlooked real problems.
 - A warning that suddenly starts to show up in unit tests needs to be
    investigated and has to be marked to be ignored, for example by
    adjusting filter scripts. Such filters are ad-hoc because there is
    no real standard format for warnings. On top of that, such filter
    scripts would require constant maintenance.

 One option to address problem would be to add messages such as "expected
 warning backtraces start / end here" to the kernel log.  However, that
 would again require filter scripts, it might result in missing real
 problematic warning backtraces triggered while the test is running, and
 the irrelevant backtrace(s) would still clog the kernel log.

 Solve the problem by providing a means to identify and suppress specific
 warning backtraces while executing test code. Support suppressing multiple
 backtraces while at the same time limiting changes to generic code to the
 absolute minimum. Architecture specific changes are kept at minimum by
 retaining function names only if both CONFIG_DEBUG_BUGVERBOSE and
 CONFIG_KUNIT are enabled.

 The first patch of the series introduces the necessary infrastructure.
 The second patch introduces support for counting suppressed backtraces.
 This capability is used in patch three to implement unit tests.
 Patch four documents the new API.
 The next two patches add support for suppressing backtraces in drm_rect
 and dev_addr_lists unit tests. These patches are intended to serve as
 examples for the use of the functionality introduced with this series.
 The remaining patches implement the necessary changes for all
 architectures with GENERIC_BUG support.
>>>
>>> Thanks for your series!
>>>
>>> I gave it a try on m68k, just running backtrace-suppression-test,
>>> and that seems to work fine.
>>>
 Design note:
    Function pointers are only added to the __bug_table section if both
    CONFIG_KUNIT and CONFIG_DEBUG_BUGVERBOSE are enabled to avoid image
    size increases if CONFIG_KUNIT=n. There would be some benefits to
    adding those pointers all the time (reduced complexity, ability to
    display function names in BUG/WARNING messages). That change, if
    desired, can be made later.
>>>
>>> Unfortunately this also increases kernel size in the CONFIG_KUNIT=m
>>> case (ca. 80 KiB for atari_defconfig), making it less attractive to have
>>> kunit and all tests enabled as modules in my standard kernel.
>>>
>> 
>> Good point. Indeed, it does. I wanted to avoid adding a configuration option,
>> but maybe I should add it after all. How about something like this ?
>> 
>> +config KUNIT_SUPPRESS_BACKTRACE
>> +   bool "KUnit - Enable backtrace suppression"
>> +   default y
>> +   help
>> + Enable backtrace suppression for KUnit. If enabled, backtraces
>> + generated intentionally by KUnit tests can be suppressed. Disable
>> + to reduce kernel image size if image size is more important than
>> + suppression of backtraces generated by KUnit tests.
>
> Any more comments / feedback on this ? Otherwise I'll introduce the
> above configuration option in v2 of the series.
>
> In this context, any suggestions if it should be enabled or disabled by
> default ? I personally think it would be more important to be able to
> suppress backtraces, but I understand that others may not be willing to
> accept a ~1% image size increase with CONFIG_KUNIT=m unless
> KUNIT_SUPPRESS_BACKTRACE is explicitly disabled.

Please enable it by default.

There are multiple CI systems that will benefit from it, whereas the
number of users enabling KUNIT in severely spaced constrainted
environments is surely small - perhaps just Geert ;).

cheers

Re: [PATCH v3 3/3] arch/powerpc: Remove from backlight code

[Michael Ellerman]

Thomas Zimmermann  writes:
> Replace  with a forward declaration in  to
> resolve an unnecessary dependency. Remove pmac_backlight_curve_lookup()
> and struct fb_info from source and header files. The function and the
> framebuffer struct are unused. No functional changes.
>
> v3:
>   * Add Fixes tag (Christophe)
>   * fix typos in commit message (Jani)
>
> Signed-off-by: Thomas Zimmermann 
> Fixes: d565dd3b0824 ("[PATCH] powerpc: More via-pmu backlight fixes")
> Reviewed-by: Jani Nikula 
> ---
>  arch/powerpc/include/asm/backlight.h|  5 ++--
>  arch/powerpc/platforms/powermac/backlight.c | 26 -
>  2 files changed, 2 insertions(+), 29 deletions(-)

Acked-by: Michael Ellerman  (powerpc)

cheers

Re: [PATCH] powerpc: include linux/backlight.h from asm/backlight.h

[Michael Ellerman]

Jani Nikula  writes:
> On Mon, 04 Mar 2024, Jani Nikula  wrote:
>> Removal of the backlight include from fb.h uncovered an implicit
>> dependency in powerpc asm/backlight.h. Add the explicit include.
>>
>> Reported-by: Naresh Kamboju 
>> Closes: 
>> https://lore.kernel.org/r/ca+g9fysak5tbqqxfc2w4ohlga0cbthmxbeq8qayfxtu75yi...@mail.gmail.com
>> Fixes: 11b4eedfc87d ("fbdev: Do not include  in header")
>> Cc: Thomas Zimmermann 
>> Cc: Helge Deller 
>> Cc: linux-fb...@vger.kernel.org
>> Signed-off-by: Jani Nikula 
>>
>> ---
>>
>> Not even compile tested!
>
> Naresh, please try this patch!
>
> Michael, if this is fine by you, ack to merge via the drm subsystem
> along with the regressing commit?

I prefer Thomas' patch, but I'm fine with either to go in via drm.

cheers

Re: [PATCH] powerpc: include linux/backlight.h from asm/backlight.h

[Michael Ellerman]

Christophe Leroy  writes:
> Le 04/03/2024 à 11:32, Thomas Zimmermann a écrit :
>> Hi
>> 
>> Am 04.03.24 um 10:55 schrieb Jani Nikula:
>>> Removal of the backlight include from fb.h uncovered an implicit
>>> dependency in powerpc asm/backlight.h. Add the explicit include.
>>>
>>> Reported-by: Naresh Kamboju 
>>> Closes: 
>>> https://lore.kernel.org/r/ca+g9fysak5tbqqxfc2w4ohlga0cbthmxbeq8qayfxtu75yi...@mail.gmail.com
>>> Fixes: 11b4eedfc87d ("fbdev: Do not include  in 
>>> header")
>>> Cc: Thomas Zimmermann 
>>> Cc: Helge Deller 
>>> Cc: linux-fb...@vger.kernel.org
>>> Signed-off-by: Jani Nikula 
>>>
>>> ---
>>>
>>> Not even compile tested!
>> 
>> That's one of the cases that's hard to catch unless you get the config 
>> right.
>> 
>>> ---
>>>   arch/powerpc/include/asm/backlight.h | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/powerpc/include/asm/backlight.h 
>>> b/arch/powerpc/include/asm/backlight.h
>>> index 1b5eab62ed04..275d5bb9aa04 100644
>>> --- a/arch/powerpc/include/asm/backlight.h
>>> +++ b/arch/powerpc/include/asm/backlight.h
>>> @@ -10,6 +10,7 @@
>>>   #define __ASM_POWERPC_BACKLIGHT_H
>>>   #ifdef __KERNEL__
>>> +#include 
>> 
>> Thanks, but I think this should go directly into chipsfb.c. I would have 
>> provided a patch already, if our mail server didn't have issues this 
>> morning. Let me try again.
>
> asm/backlight.h needs it for struct backlight_device
>
> At least if you don't want to include linux/backlight.h in 
> asm/backlight.h, then you need a forward declaration of struct 
> backlight_device;

It's preferable for asm headers not to include linux headers (to avoid
loops), so a forward declaration would be better IMHO.

cheers

[PATCH] fbdev/mb862xxfb: Fix defined but not used error

[Michael Ellerman]

socrates_gc_mode is defined at the top-level but then only used inside
an #ifdef CONFIG_FB_MB862XX_LIME, leading to an error with some configs:

  drivers/video/fbdev/mb862xx/mb862xxfbdrv.c:36:31: error: ‘socrates_gc_mode’ 
defined but not used
 36 | static struct mb862xx_gc_mode socrates_gc_mode = {

Fix it by moving socrates_gc_mode inside that ifdef, immediately prior
to the only function where it's used.

Signed-off-by: Michael Ellerman 
---
 drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c 
b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
index 7c402e9fd7a9..baec312d7b33 100644
--- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
+++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
@@ -32,15 +32,6 @@
 #define CARMINE_MEM_SIZE   0x800
 #define DRV_NAME   "mb862xxfb"
 
-#if defined(CONFIG_SOCRATES)
-static struct mb862xx_gc_mode socrates_gc_mode = {
-   /* Mode for Prime View PM070WL4 TFT LCD Panel */
-   { "800x480", 45, 800, 480, 4, 86, 42, 33, 10, 128, 2, 0, 0, 0 },
-   /* 16 bits/pixel, 16MB, 133MHz, SDRAM memory mode value */
-   16, 0x100, GC_CCF_COT_133, 0x4157ba63
-};
-#endif
-
 /* Helpers */
 static inline int h_total(struct fb_var_screeninfo *var)
 {
@@ -666,6 +657,15 @@ static int mb862xx_gdc_init(struct mb862xxfb_par *par)
return 0;
 }
 
+#if defined(CONFIG_SOCRATES)
+static struct mb862xx_gc_mode socrates_gc_mode = {
+   /* Mode for Prime View PM070WL4 TFT LCD Panel */
+   { "800x480", 45, 800, 480, 4, 86, 42, 33, 10, 128, 2, 0, 0, 0 },
+   /* 16 bits/pixel, 16MB, 133MHz, SDRAM memory mode value */
+   16, 0x100, GC_CCF_COT_133, 0x4157ba63
+};
+#endif
+
 static int of_platform_mb862xx_probe(struct platform_device *ofdev)
 {
struct device_node *np = ofdev->dev.of_node;
-- 
2.43.2

Re: [PATCH v2] drivers/ps3: select VIDEO to provide cmdline functions

[Michael Ellerman]

Thomas Zimmermann  writes:
> Am 07.02.24 um 17:13 schrieb Randy Dunlap:
>> When VIDEO is not set, there is a build error. Fix that by selecting
>> VIDEO for PS3_PS3AV.
>>
>> ERROR: modpost: ".video_get_options" [drivers/ps3/ps3av_mod.ko] undefined!
>>
>> Fixes: dae7fbf43fd0 ("driver/ps3: Include  for mode 
>> parsing")
>> Fixes: a3b6792e990d ("video/cmdline: Introduce CONFIG_VIDEO for video= 
>> parameter")
>> Cc: Michael Ellerman 
>> Cc: Nicholas Piggin 
>> Cc: Christophe Leroy 
>> Cc: Aneesh Kumar K.V 
>> Cc: Naveen N. Rao 
>> Cc: linuxppc-...@lists.ozlabs.org
>> Cc: Thomas Zimmermann 
>> Cc: Geoff Levand 
>> Acked-by: Geoff Levand 
>> Cc: linux-fb...@vger.kernel.org
>> Cc: dri-devel@lists.freedesktop.org
>> Signed-off-by: Randy Dunlap 
>
> Reviewed-by: Thomas Zimmermann 

Can you take it via whatever tree the CONFIG_VIDEO patch is in?

Acked-by: Michael Ellerman 

cheers

Re: (subset) [PATCH 00/22] -Wmissing-prototype warning fixes

[Michael Ellerman]

On Wed, 08 Nov 2023 13:58:21 +0100, Arnd Bergmann wrote:
> I slightly dropped the ball on this since last sending the series in
> August, but a number of warning fixes have made it into the kernel in
> the meantime, both from my earlier submission and from architecture
> maintainers.
> 
> I have none patches that remain from the previous submission, with
> two of them reworked according to comments. The additional patches
> are from more testing across architectures and configurations that
> I had previously missed.
> 
> [...]

Applied to powerpc/next.

[17/22] powerpc: ps3: move udbg_shutdown_ps3gelic prototype

https://git.kernel.org/powerpc/c/04c40eed3f7ac48ddaf20104489510e743a53c47
[18/22] powerpc: pasemi: mark pas_shutdown() static

https://git.kernel.org/powerpc/c/0c9a768de64d24e38e27652b8c273725ccc31916
[19/22] powerpc: powermac: mark smp_psurge_{give,take}_timebase static

https://git.kernel.org/powerpc/c/afb36ac386783d2ef2ed839293c03fd06f470be0

cheers

Re: (subset) [PATCH v5 0/5] ppc, fbdev: Clean up fbdev mmap helper

[Michael Ellerman]

On Fri, 22 Sep 2023 10:04:54 +0200, Thomas Zimmermann wrote:
> Clean up and rename fb_pgprotect() to work without struct file. Then
> refactor the implementation for PowerPC. This change has been discussed
> at [1] in the context of refactoring fbdev's mmap code.
> 
> The first two patches update fbdev and replace fbdev's fb_pgprotect()
> with pgprot_framebuffer() on all architectures. The new helper's stream-
> lined interface enables more refactoring within fbdev's mmap
> implementation.
> 
> [...]

Patches 3-5 applied to powerpc/fixes.

[3/5] arch/powerpc: Remove trailing whitespaces
  https://git.kernel.org/powerpc/c/322948c3198cf80e7c10d953ddad24ebd85757cd
[4/5] arch/powerpc: Remove file parameter from phys_mem_access_prot code
  https://git.kernel.org/powerpc/c/1f92a844c35e483c00bab8a7b7d39c555ee799d8
[5/5] arch/powerpc: Call internal __phys_mem_access_prot() in fbdev code
  https://git.kernel.org/powerpc/c/deebe5f607d7f72f83c41163191ad0c1c4356385

cheers

Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes

[Michael Ellerman]

Christophe Leroy  writes:
> Le 09/11/2023 à 11:18, Michael Ellerman a écrit :
>> "Arnd Bergmann"  writes:
>>> On Wed, Nov 8, 2023, at 19:31, Christophe Leroy wrote:
>>>> Le 08/11/2023 à 13:58, Arnd Bergmann a écrit :
>>>
>>>> powerpc has functions doing more or less the same, they are called
>>>> __c_kernel_clock_gettime() and alike with their prototypes siting in
>>>> arch/powerpc/include/asm/vdso/gettimeofday.h
>>>>
>>>> Should those prototypes be moved to include/vdso/gettime.h too and
>>>> eventually renamed, or are they considered too powerpc specific ?
>>>
>>> I don't actually know, my initial interpretation was that
>>> these function names are part of the user ABI for the vdso,
>>> but I never looked closely enough at how vdso works to
>>> be sure what the actual ABI is.
>> 
>> AFAIK the ABI is just the symbols we export, as defined in the linker
>> script:
>> 
>> /*
>>   * This controls what symbols we export from the DSO.
>>   */
>> VERSION
>> {
>>  VDSO_VERSION_STRING {
>>  global:
>>  __kernel_get_syscall_map;
>>  __kernel_gettimeofday;
>>  __kernel_clock_gettime;
>>  __kernel_clock_getres;
>>  __kernel_get_tbfreq;
>>  __kernel_sync_dicache;
>>  __kernel_sigtramp_rt64;
>>  __kernel_getcpu;
>>  __kernel_time;
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/kernel/vdso/vdso64.lds.S?h=v6.6&#

Re: [PATCH 15/22] arch: vdso: consolidate gettime prototypes

[Michael Ellerman]

"Arnd Bergmann"  writes:
> On Wed, Nov 8, 2023, at 19:31, Christophe Leroy wrote:
>> Le 08/11/2023 à 13:58, Arnd Bergmann a écrit :
>
>> powerpc has functions doing more or less the same, they are called 
>> __c_kernel_clock_gettime() and alike with their prototypes siting in 
>> arch/powerpc/include/asm/vdso/gettimeofday.h
>>
>> Should those prototypes be moved to include/vdso/gettime.h too and 
>> eventually renamed, or are they considered too powerpc specific ?
>
> I don't actually know, my initial interpretation was that
> these function names are part of the user ABI for the vdso,
> but I never looked closely enough at how vdso works to
> be sure what the actual ABI is.

AFAIK the ABI is just the symbols we export, as defined in the linker
script:

/*
 * This controls what symbols we export from the DSO.
 */
VERSION
{
VDSO_VERSION_STRING {
global:
__kernel_get_syscall_map;
__kernel_gettimeofday;
__kernel_clock_gettime;
__kernel_clock_getres;
__kernel_get_tbfreq;
__kernel_sync_dicache;
__kernel_sigtramp_rt64;
__kernel_getcpu;
__kernel_time;

Re: [PATCH v5 0/5] ppc, fbdev: Clean up fbdev mmap helper

[Michael Ellerman]

Thomas Zimmermann  writes:
> FYI, I intent to merge patches 1 and 2 of this patchset into 
> drm-misc-next. The updates for PowerPC can be merged through PPC trees 
> later. Let me know if this does not work for you.

Hi Thomas,

Sorry for the late reply, I was on leave.

Yeah that's fine.

cheers

> Am 22.09.23 um 10:04 schrieb Thomas Zimmermann:
>> Clean up and rename fb_pgprotect() to work without struct file. Then
>> refactor the implementation for PowerPC. This change has been discussed
>> at [1] in the context of refactoring fbdev's mmap code.
>> 
>> The first two patches update fbdev and replace fbdev's fb_pgprotect()
>> with pgprot_framebuffer() on all architectures. The new helper's stream-
>> lined interface enables more refactoring within fbdev's mmap
>> implementation.
>> 
>> Patches 3 to 5 adapt PowerPC's internal interfaces to provide
>> phys_mem_access_prot() that works without struct file. Neither the
>> architecture code or fbdev helpers need the parameter.
>> 
>> v5:
>>  * improve commit descriptions (Javier)
>>  * add missing tags (Geert)
>> v4:
>>  * fix commit message (Christophe)
>> v3:
>>  * rename fb_pgrotect() to pgprot_framebuffer() (Arnd)
>> v2:
>>  * reorder patches to simplify merging (Michael)
>> 
>> [1] 
>> https://lore.kernel.org/linuxppc-dev/5501ba80-bdb0-6344-16b0-0466a950f...@suse.com/
>> 
>> Thomas Zimmermann (5):
>>fbdev: Avoid file argument in fb_pgprotect()
>>fbdev: Replace fb_pgprotect() with pgprot_framebuffer()
>>arch/powerpc: Remove trailing whitespaces
>>arch/powerpc: Remove file parameter from phys_mem_access_prot code
>>arch/powerpc: Call internal __phys_mem_access_prot() in fbdev code
>> 
>>   arch/ia64/include/asm/fb.h| 15 +++
>>   arch/m68k/include/asm/fb.h| 19 ++-
>>   arch/mips/include/asm/fb.h| 11 +--
>>   arch/powerpc/include/asm/book3s/pgtable.h | 10 --
>>   arch/powerpc/include/asm/fb.h | 13 +
>>   arch/powerpc/include/asm/machdep.h| 13 ++---
>>   arch/powerpc/include/asm/nohash/pgtable.h | 10 --
>>   arch/powerpc/include/asm/pci.h|  4 +---
>>   arch/powerpc/kernel/pci-common.c  |  3 +--
>>   arch/powerpc/mm/mem.c |  8 
>>   arch/sparc/include/asm/fb.h   | 15 +--
>>   arch/x86/include/asm/fb.h | 10 ++
>>   arch/x86/video/fbdev.c| 15 ---
>>   drivers/video/fbdev/core/fb_chrdev.c  |  3 ++-
>>   include/asm-generic/fb.h  | 12 ++--
>>   15 files changed, 86 insertions(+), 75 deletions(-)
>> 
>> 
>> base-commit: f8d21cb17a99b75862196036bb4bb93ee9637b74
>
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)

Re: [PATCH 00/11] add missing of_node_put

[Michael Ellerman]

On Thu, 07 Sep 2023 11:55:10 +0200, Julia Lawall wrote:
> Add of_node_put on a break out of an of_node loop.
> 

Patches 3 and 6 applied to powerpc/next.

[03/11] powerpc/powermac: add missing of_node_put

https://git.kernel.org/powerpc/c/a59e9eb25216eb1dc99e14fc31b76aa648d79540
[06/11] powerpc/kexec_file: add missing of_node_put

https://git.kernel.org/powerpc/c/06b627c1236216ac1239c5e1afcc75359af3fb72

cheers

Re: [PATCH 0/4] ppc, fbdev: Clean up fbdev mmap helper

[Michael Ellerman]

Thomas Zimmermann  writes:
> Refactor fb_pgprotect() in PowerPC to work without struct file. Then
> clean up and rename fb_pgprotect(). This change has been discussed at
> [1] in the context of refactoring fbdev's mmap code.
>
> The first three patches adapt PowerPC's internal interfaces to
> provide a phys_mem_access_prot() that works without struct file. Neither
> the architecture code or fbdev helpers need the parameter.
>
> Patch 4 replaces fbdev's fb_pgprotect() with fb_pgprot_device() on
> all architectures. The new helper with its stream-lined interface
> enables more refactoring within fbdev's mmap implementation.

The content of this series is OK, but the way it's structured makes it a
real headache to merge, because it's mostly powerpc changes and then a
dependant cross architecture patch at the end.

It would be simpler if patch 4 was first and just passed file=NULL to
the powerpc helper, with an explanation that it's unused and will be
dropped in a future cleanup.

We could then put the first patch (previously patch 4) in a topic branch
that is shared between the powerpc tree and the fbdev tree, and then the
powerpc changes could be staged on top of that through the powerpc tree.

cheers

Re: [PATCH v2 1/9] vgacon: rework Kconfig dependencies

[Michael Ellerman]

Arnd Bergmann  writes:
> From: Arnd Bergmann 
>
> The list of dependencies here is phrased as an opt-out, but this is missing
> a lot of architectures that don't actually support VGA consoles, and some
> of the entries are stale:
>
>  - powerpc used to support VGA consoles in the old arch/ppc codebase, but
>the merged arch/powerpc never did

Not disputing this, but how did you come to that conclusion? I grepped
around and couldn't convince myself whether it can work on powerpc or
not. ie. currently it's possible to enable CONFIG_VGA_CONSOLE and
powerpc does have a struct screen_info defined which seems like it would
allow vgacon_startup() to complete.

My only concern is that someone could be using it with Qemu?

cheers

Re: build failure from drm/ttm commit now in mainline (was: linux-next: build failure after merge of the drm tree)

[Michael Ellerman]

Thorsten Leemhuis  writes:
> Lo!
>
> Sometimes the regression tracker runs into regressions himself... :-D
>
> On 11.04.23 08:47, Stephen Rothwell wrote:
>> 
>> After merging the drm tree, today's linux-next build (powerpc
>> allyesconfig) failed like this:
>> 
>> drivers/gpu/drm/ttm/ttm_pool.c:73:29: error: variably modified 
>> 'global_write_combined' at file scope
>>73 | static struct ttm_pool_type global_write_combined[TTM_DIM_ORDER];
>>   | ^
>> drivers/gpu/drm/ttm/ttm_pool.c:74:29: error: variably modified 
>> 'global_uncached' at file scope
>>74 | static struct ttm_pool_type global_uncached[TTM_DIM_ORDER];
>>   | ^~~
>> drivers/gpu/drm/ttm/ttm_pool.c:76:29: error: variably modified 
>> 'global_dma32_write_combined' at file scope
>>76 | static struct ttm_pool_type 
>> global_dma32_write_combined[TTM_DIM_ORDER];
>>   | ^~~
>> drivers/gpu/drm/ttm/ttm_pool.c:77:29: error: variably modified 
>> 'global_dma32_uncached' at file scope
>>77 | static struct ttm_pool_type global_dma32_uncached[TTM_DIM_ORDER];
>>   | ^
>> 
>> Caused by commit
>> 
>>   322458c2bb1a ("drm/ttm: Reduce the number of used allocation orders for 
>> TTM pages")
>> 
>> PMD_SHIFT is not necessarily a constant on ppc (see
>> arch/powerpc/include/asm/book3s/64/pgtable.h).
>> 
>> I have reverted that commit for today.
>
> Did anyone look into this?

Christian sent a revert:
  56e51681246e ("drm/ttm: revert "Reduce the number of used allocation orders 
for TTM pages"")

Which has been in linux-next since the 21st. But I guess it missed being
in the initial DRM pull request.

See also other thread:
  https://lore.kernel.org/all/ZEljCgVFnDl%2FC+l3@debian/

Where Christian says the revert is "on the way" to Linus.

cheers

Re: mainline build failure due to 322458c2bb1a ("drm/ttm: Reduce the number of used allocation orders for TTM pages")

[Michael Ellerman]

Linus Torvalds  writes:
> On Wed, Apr 26, 2023 at 10:44 AM Sudip Mukherjee (Codethink)
>  wrote:
>>
>> drivers/gpu/drm/ttm/ttm_pool.c:73:29: error: variably modified 
>> 'global_write_combined' at file scope
>>73 | static struct ttm_pool_type global_write_combined[TTM_DIM_ORDER];
>>   | ^
>
> Ugh.
>
> This is because we have
>
>   #define TTM_DIM_ORDER (__TTM_DIM_ORDER <= MAX_ORDER ?
> __TTM_DIM_ORDER : MAX_ORDER)
>
> which looks perfectly fine as a constant ("pick the smaller of
> MAX_ORDER and __TTM_DIM_ORDER").
>
> But:
>
>   #define __TTM_DIM_ORDER (TTM_MAX_ORDER + 1)
>   #define TTM_MAX_ORDER (PMD_SHIFT - PAGE_SHIFT)
>
> which still _looks_ fine, but  on 64-bit powerpc, we then have
>
>#define PTE_INDEX_SIZE  __pte_index_size
>
> so that __TTM_DIM_ORDER constant isn't actually a constant at all.
..
>
> It's a bit sad how that macro that _looks_ like a constant (and is one
> pretty much everywhere else) isn't actually constant on powerpc, but
> looking around it looks fairly unavoidable.

Yeah, it allows us to build a single kernel that can choose at runtime
whether it uses the HPT or Radix MMU. The page table geometry is
different between the MMUs because they support a different sized huge
page for THP.

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 v2 15/19] arch/powerpc: Implement with generic helpers

[Michael Ellerman]

Thomas Zimmermann  writes:
> Replace the architecture's fb_is_primary_device() with the generic
> one from . No functional changes.
>
> Signed-off-by: Thomas Zimmermann 
> Cc: Michael Ellerman 
> Cc: Nicholas Piggin 
> Cc: Christophe Leroy 
> ---
>  arch/powerpc/include/asm/fb.h | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)

Looks fine.

Acked-by: Michael Ellerman  (powerpc)

cheers

> diff --git a/arch/powerpc/include/asm/fb.h b/arch/powerpc/include/asm/fb.h
> index 6541ab77c5b9..5f1a2e5f7654 100644
> --- a/arch/powerpc/include/asm/fb.h
> +++ b/arch/powerpc/include/asm/fb.h
> @@ -2,8 +2,8 @@
>  #ifndef _ASM_FB_H_
>  #define _ASM_FB_H_
>  
> -#include 
>  #include 
> +
>  #include 
>  
>  static inline void fb_pgprotect(struct file *file, struct vm_area_struct 
> *vma,
> @@ -13,10 +13,8 @@ static inline void fb_pgprotect(struct file *file, struct 
> vm_area_struct *vma,
>vma->vm_end - vma->vm_start,
>vma->vm_page_prot);
>  }
> +#define fb_pgprotect fb_pgprotect
>  
> -static inline int fb_is_primary_device(struct fb_info *info)
> -{
> - return 0;
> -}
> +#include 
>  
>  #endif /* _ASM_FB_H_ */
> -- 
> 2.40.0

Re: linux-next: manual merge of the drm tree with the powerpc tree

[Michael Ellerman]

Stephen Rothwell  writes:
> Hi all,
>
> Today's linux-next merge of the drm tree got a conflict in:
>
>   drivers/gpu/drm/amd/display/Kconfig
>
> between commit:
>
>   78f0929884d4 ("powerpc/64: Always build with 128-bit long double")
>
> from the powerpc tree and commit:
>
>   4652ae7a51b7 ("drm/amd/display: Rename DCN config to FP")
>
> from the drm tree.

> I fixed it up (I used the powerpc version - with "(PPC64 && ALTIVEC)")
> and can carry the fix as necessary.

Thanks, that's the right resolution.

Not much we can do to avoid that conflict, we'll just have to tell Linus
about it at pull request time.

cheers

Re: [PATCH v2 01/16] slab: Remove __malloc attribute from realloc functions

[Michael Ellerman]

Kees Cook  writes:
> On Wed, Sep 28, 2022 at 09:26:15AM +0200, Geert Uytterhoeven wrote:
>> On Fri, Sep 23, 2022 at 10:35 PM Kees Cook  wrote:
>> > The __malloc attribute should not be applied to "realloc" functions, as
>> > the returned pointer may alias the storage of the prior pointer. Instead
>> > of splitting __malloc from __alloc_size, which would be a huge amount of
>> > churn, just create __realloc_size for the few cases where it is needed.
>> >
>> > Additionally removes the conditional test for __alloc_size__, which is
>> > always defined now.
>> >
>> > Cc: Christoph Lameter 
>> > Cc: Pekka Enberg 
>> > Cc: David Rientjes 
>> > Cc: Joonsoo Kim 
>> > Cc: Andrew Morton 
>> > Cc: Vlastimil Babka 
>> > Cc: Roman Gushchin 
>> > Cc: Hyeonggon Yoo <42.hye...@gmail.com>
>> > Cc: Marco Elver 
>> > Cc: linux...@kvack.org
>> > Signed-off-by: Kees Cook 
>> 
>> Thanks for your patch, which is now commit 63caa04ec60583b1 ("slab:
>> Remove __malloc attribute from realloc functions") in next-20220927.
>> 
>> nore...@ellerman.id.au reported all gcc8-based builds to fail
>> (e.g. [1], more at [2]):
>> 
>> In file included from :
>> ./include/linux/percpu.h: In function ‘__alloc_reserved_percpu’:
>> ././include/linux/compiler_types.h:279:30: error: expected
>> declaration specifiers before ‘__alloc_size__’
>>  #define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc
>>   ^~
>> ./include/linux/percpu.h:120:74: note: in expansion of macro 
>> ‘__alloc_size’
>> [...]
>> 
>> It's building fine with e.g. gcc-9 (which is my usual m68k cross-compiler).
>> Reverting this commit on next-20220927 fixes the issue.
>> 
>> [1] http://kisskb.ellerman.id.au/kisskb/buildresult/14803908/
>> [2] 
>> http://kisskb.ellerman.id.au/kisskb/head/1bd8b75fe6adeaa89d02968bdd811ffe708cf839/
>
> Eek! Thanks for letting me know. I'm confused about this --
> __alloc_size__ wasn't optional in compiler_attributes.h -- but obviously
> I broke something! I'll go figure this out.

This fixes it for me.

cheers

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index f141a6f6b9f6..0717534f8364 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -275,8 +275,13 @@ struct ftrace_likely_data {
  * be performing a _reallocation_, as that may alias the existing pointer.
  * For these, use __realloc_size().
  */
-#define __alloc_size(x, ...)   __alloc_size__(x, ## __VA_ARGS__) __malloc
-#define __realloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__)
+#ifdef __alloc_size__
+# define __alloc_size(x, ...)  __alloc_size__(x, ## __VA_ARGS__) __malloc
+# define __realloc_size(x, ...)__alloc_size__(x, ## __VA_ARGS__)
+#else
+# define __alloc_size(x, ...)  __malloc
+# define __realloc_size(x, ...)
+#endif
 
 #ifndef asm_volatile_goto
 #define asm_volatile_goto(x...) asm goto(x)

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 v3 1/4] video: fbdev: offb: Include missing linux/platform_device.h

[Michael Ellerman]

On Fri, 8 Jul 2022 09:11:05 +0200, Christophe Leroy wrote:
> A lot of drivers were getting platform and of headers
> indirectly via headers like asm/pci.h or asm/prom.h
> 
> Most of them were fixed during 5.19 cycle but a newissue was
> introduced by commit 52b1b46c39ae ("of: Create platform devices
> for OF framebuffers")
> 
> [...]

Applied to powerpc/next.

[1/4] video: fbdev: offb: Include missing linux/platform_device.h
  https://git.kernel.org/powerpc/c/ebef8abc963b9e537c0a0d619dd8faf1b8f2b183
[2/4] scsi: cxlflash: Include missing linux/irqdomain.h
  https://git.kernel.org/powerpc/c/61657dcd528b75cd196adaf56890124c13953c8d
[3/4] powerpc: Remove asm/prom.h from asm/mpc52xx.h and asm/pci.h
  https://git.kernel.org/powerpc/c/4d5c5bad51935482437528f7fa4dffdcb3330d8b
[4/4] powerpc: Finally remove unnecessary headers from asm/prom.h
  https://git.kernel.org/powerpc/c/36afe68714d45edf34430d28e3dc787425ad8b22

cheers

Re: [PATCH v2 00/10] drm: Add driver for PowerPC OF displays

[Michael Ellerman]

Thomas Zimmermann  writes:
> (was: drm: Add driverof PowerPC OF displays)
>
> PowerPC's Open Firmware offers a simple display buffer for graphics
> output. Add ofdrm, a DRM driver for the device. As with the existing
> simpledrm driver, the graphics hardware is pre-initialized by the
> firmware. The driver only provides blitting, no actual DRM modesetting
> is possible.

Hi Thomas,

I tried to test this on a 32-bit ppc Mac Mini but didn't have much luck.
But I'm probably doing something wrong because I'm a graphics noob.

The machine normally uses CONFIG_DRM_RADEON, so I turned that off, and
turned DRM_OFDRM on.

When I boot I get boot messages but only one screen worth, the messages
don't scroll at all, which is unusual. But I'm not sure if that's
related to ofdrm or something else.

The machine does come up, I can login via SSH. Is there some way to
start X to exercise the driver from an SSH login?

cheers

Re: [PATCH v2] drm/nouveau/bios: Rename prom_init() and friends functions

[Michael Ellerman]

Lyude Paul  writes:
> Alright, dim's pushing it now. sorry about that!

Thanks, I see it in linux-next now.

cheers

> On Wed, 2022-06-08 at 16:48 -0400, Lyude Paul wrote:
>> Whoops, it totally may have. Thank you for the reminder, I will double check
>> and make sure this gets pushed today
>> 
>> On Tue, 2022-06-07 at 23:00 +1000, Michael Ellerman wrote:
>> > Lyude Paul  writes:
>> > > Reviewed-by: Lyude Paul 
>> > > 
>> > > Will push to drm-misc-next
>> > 
>> > I don't see this patch in mainline or drm-misc-next, did it get lost?
>> > 
>> > cheers
>> > 
>> > > On Sat, 2022-03-19 at 11:27 +0100, Christophe Leroy wrote:
>> > > > While working at fixing powerpc headers, I ended up with the
>> > > > following error.
>> > > > 
>> > > > drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.c:48:1:
>> > > > error:
>> > > > conflicting types for 'prom_init'; have 'void *(struct nvkm_bios *,
>> > > > const
>> > > > char *)'
>> > > > make[5]: *** [scripts/Makefile.build:288:
>> > > > drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.o] Error 1
>> > > > 
>> > > > powerpc and a few other architectures have a prom_init() global
>> > > > function.
>> > > > One day or another it will conflict with the one in shadowrom.c
>> > > > 
>> > > > Those being static, they can easily be renamed. Do it.
>> > > > 
>> > > > While at it, also rename the ops structure as 'nvbios_prom' instead of
>> > > > 'nvbios_rom' in order to make it clear that it refers to the
>> > > > NV_PROM device.
>> > > > 
>> > > > Signed-off-by: Christophe Leroy 
>> > > > ---
>> > > > v2: using nvbios_prom prefix instead of nvbios_rom. Changed structure
>> > > > name
>> > > > to keep things consistant.
>> > > > 
>> > > >  drivers/gpu/drm/nouveau/nvkm/subdev/bios/priv.h    |  2 +-
>> > > >  drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c  |  2 +-
>> > > >  .../gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.c   | 14 +++--
>> > > > -
>> > > >  3 files changed, 9 insertions(+), 9 deletions(-)
>> > > > 
>> > > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/priv.h
>> > > > b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/priv.h
>> > > > index fac1bff1311b..cfa8a0c356dd 100644
>> > > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/priv.h
>> > > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/priv.h
>> > > > @@ -19,7 +19,7 @@ struct nvbios_source {
>> > > >  int nvbios_extend(struct nvkm_bios *, u32 length);
>> > > >  int nvbios_shadow(struct nvkm_bios *);
>> > > >  
>> > > > -extern const struct nvbios_source nvbios_rom;
>> > > > +extern const struct nvbios_source nvbios_prom;
>> > > >  extern const struct nvbios_source nvbios_ramin;
>> > > >  extern const struct nvbios_source nvbios_acpi_fast;
>> > > >  extern const struct nvbios_source nvbios_acpi_slow;
>> > > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
>> > > > b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
>> > > > index 4b571cc6bc70..19188683c8fc 100644
>> > > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
>> > > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
>> > > > @@ -171,7 +171,7 @@ nvbios_shadow(struct nvkm_bios *bios)
>> > > > struct shadow mthds[] = {
>> > > > { 0, _of },
>> > > > { 0, _ramin },
>> > > > -   { 0, _rom },
>> > > > +   { 0, _prom },
>> > > > { 0, _acpi_fast },
>> > > > { 4, _acpi_slow },
>> > > > { 1, _pcirom },
>> > > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.c
>> > > > b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.c
>> > > > index ffa4b395220a..39144ceb117b 100644
>> > > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.c
>> > > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.c
>> > > > @@ -25,7 +25,7 @@
>> > &

Re: [PATCH v2] drm/nouveau/bios: Rename prom_init() and friends functions

[Michael Ellerman]

Lyude Paul  writes:
> Reviewed-by: Lyude Paul 
>
> Will push to drm-misc-next

I don't see this patch in mainline or drm-misc-next, did it get lost?

cheers

> On Sat, 2022-03-19 at 11:27 +0100, Christophe Leroy wrote:
>> While working at fixing powerpc headers, I ended up with the
>> following error.
>> 
>> drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.c:48:1: error:
>> conflicting types for 'prom_init'; have 'void *(struct nvkm_bios *, const
>> char *)'
>> make[5]: *** [scripts/Makefile.build:288:
>> drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.o] Error 1
>> 
>> powerpc and a few other architectures have a prom_init() global function.
>> One day or another it will conflict with the one in shadowrom.c
>> 
>> Those being static, they can easily be renamed. Do it.
>> 
>> While at it, also rename the ops structure as 'nvbios_prom' instead of
>> 'nvbios_rom' in order to make it clear that it refers to the
>> NV_PROM device.
>> 
>> Signed-off-by: Christophe Leroy 
>> ---
>> v2: using nvbios_prom prefix instead of nvbios_rom. Changed structure name
>> to keep things consistant.
>> 
>>  drivers/gpu/drm/nouveau/nvkm/subdev/bios/priv.h    |  2 +-
>>  drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c  |  2 +-
>>  .../gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.c   | 14 +++---
>>  3 files changed, 9 insertions(+), 9 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/priv.h
>> b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/priv.h
>> index fac1bff1311b..cfa8a0c356dd 100644
>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/priv.h
>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/priv.h
>> @@ -19,7 +19,7 @@ struct nvbios_source {
>>  int nvbios_extend(struct nvkm_bios *, u32 length);
>>  int nvbios_shadow(struct nvkm_bios *);
>>  
>> -extern const struct nvbios_source nvbios_rom;
>> +extern const struct nvbios_source nvbios_prom;
>>  extern const struct nvbios_source nvbios_ramin;
>>  extern const struct nvbios_source nvbios_acpi_fast;
>>  extern const struct nvbios_source nvbios_acpi_slow;
>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
>> b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
>> index 4b571cc6bc70..19188683c8fc 100644
>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
>> @@ -171,7 +171,7 @@ nvbios_shadow(struct nvkm_bios *bios)
>> struct shadow mthds[] = {
>> { 0, _of },
>> { 0, _ramin },
>> -   { 0, _rom },
>> +   { 0, _prom },
>> { 0, _acpi_fast },
>> { 4, _acpi_slow },
>> { 1, _pcirom },
>> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.c
>> b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.c
>> index ffa4b395220a..39144ceb117b 100644
>> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.c
>> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.c
>> @@ -25,7 +25,7 @@
>>  #include 
>>  
>>  static u32
>> -prom_read(void *data, u32 offset, u32 length, struct nvkm_bios *bios)
>> +nvbios_prom_read(void *data, u32 offset, u32 length, struct nvkm_bios
>> *bios)
>>  {
>> struct nvkm_device *device = data;
>> u32 i;
>> @@ -38,14 +38,14 @@ prom_read(void *data, u32 offset, u32 length, struct
>> nvkm_bios *bios)
>>  }
>>  
>>  static void
>> -prom_fini(void *data)
>> +nvbios_prom_fini(void *data)
>>  {
>> struct nvkm_device *device = data;
>> nvkm_pci_rom_shadow(device->pci, true);
>>  }
>>  
>>  static void *
>> -prom_init(struct nvkm_bios *bios, const char *name)
>> +nvbios_prom_init(struct nvkm_bios *bios, const char *name)
>>  {
>> struct nvkm_device *device = bios->subdev.device;
>> if (device->card_type == NV_40 && device->chipset >= 0x4c)
>> @@ -55,10 +55,10 @@ prom_init(struct nvkm_bios *bios, const char *name)
>>  }
>>  
>>  const struct nvbios_source
>> -nvbios_rom = {
>> +nvbios_prom = {
>> .name = "PROM",
>> -   .init = prom_init,
>> -   .fini = prom_fini,
>> -   .read = prom_read,
>> +   .init = nvbios_prom_init,
>> +   .fini = nvbios_prom_fini,
>> +   .read = nvbios_prom_read,
>> .rw = false,
>>  };
>
> -- 
> Cheers,
>  Lyude Paul (she/her)
>  Software Engineer at Red Hat

Re: [PATCH] video: fbdev: use memset_io() instead of memset()

[Michael Ellerman]

On Wed, 15 Sep 2021 15:34:35 +0200, Christophe Leroy wrote:
> While investigating a lockup at startup on Powerbook 3400C, it was
> identified that the fbdev driver generates alignment exception at
> startup:
> 
>   --- interrupt: 600 at memset+0x60/0xc0
>   NIP:  c0021414 LR: c03fc49c CTR: 7fff
>   REGS: ca021c10 TRAP: 0600   Tainted: GW  
> (5.14.2-pmac-00727-g12a41fa69492)
>   MSR:  9032   CR: 44008442  XER: 2100
>   DAR: cab80020 DSISR: 00017c07
>   GPR00: 0007 ca021cd0 c14412e0 cab8  0010 cab8001c 
> 0004
>   GPR08: 0010 7fff   84008442  c0006fb4 
> 
>   GPR16:        
> 0010
>   GPR24:  8180 0320 c15fa400 c14d1878  c14d1800 
> c094e19c
>   NIP [c0021414] memset+0x60/0xc0
>   LR [c03fc49c] chipsfb_pci_init+0x160/0x580
>   --- interrupt: 600
>   [ca021cd0] [c03fc46c] chipsfb_pci_init+0x130/0x580 (unreliable)
>   [ca021d20] [c03a3a70] pci_device_probe+0xf8/0x1b8
>   [ca021d50] [c043d584] really_probe.part.0+0xac/0x388
>   [ca021d70] [c043d914] __driver_probe_device+0xb4/0x170
>   [ca021d90] [c043da18] driver_probe_device+0x48/0x144
>   [ca021dc0] [c043e318] __driver_attach+0x11c/0x1c4
>   [ca021de0] [c043ad30] bus_for_each_dev+0x88/0xf0
>   [ca021e10] [c043c724] bus_add_driver+0x190/0x22c
>   [ca021e40] [c043ee94] driver_register+0x9c/0x170
>   [ca021e60] [c0006c28] do_one_initcall+0x54/0x1ec
>   [ca021ed0] [c08246e4] kernel_init_freeable+0x1c0/0x270
>   [ca021f10] [c0006fdc] kernel_init+0x28/0x11c
>   [ca021f30] [c0017148] ret_from_kernel_thread+0x14/0x1c
>   Instruction dump:
>   7d4601a4 39490777 7d4701a4 39490888 7d4801a4 39490999 7d4901a4 39290aaa
>   7d2a01a4 4c00012c 4bfffe88 0fe0 <4bfffe80> 9421fff0 38210010 
> 48001970
> 
> [...]

Applied to powerpc/next.

[1/1] video: fbdev: use memset_io() instead of memset()
  https://git.kernel.org/powerpc/c/f2719b26ae27282c145202ffd656d5ff1fe737cc

cheers

Re: [PATCH] video: fbdev: use memset_io() instead of memset()

[Michael Ellerman]

Christophe Leroy  writes:
> While investigating a lockup at startup on Powerbook 3400C, it was
> identified that the fbdev driver generates alignment exception at
> startup:
...
>
> Use fb_memset() instead of memset(). fb_memset() is defined as
> memset_io() for powerpc.
>
> Fixes: 8c8709334cec ("[PATCH] ppc32: Remove CONFIG_PMAC_PBOOK")
> Reported-by: Stan Johnson 
> Cc: Finn Thain 
> Signed-off-by: Christophe Leroy 
> ---
>  drivers/video/fbdev/chipsfb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Looks like drivers/video/fbdev is orphaned, so I'll pick this up via
powerpc.

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 57/63] powerpc/signal32: Use struct_group() to zero spe regs

[Michael Ellerman]

Kees Cook  writes:
> On Fri, Aug 20, 2021 at 05:49:35PM +1000, Michael Ellerman wrote:
>> Kees Cook  writes:
>> > In preparation for FORTIFY_SOURCE performing compile-time and run-time
>> > field bounds checking for memset(), avoid intentionally writing across
>> > neighboring fields.
>> >
>> > Add a struct_group() for the spe registers so that memset() can correctly 
>> > reason
>> > about the size:
>> >
>> >In function 'fortify_memset_chk',
>> >inlined from 'restore_user_regs.part.0' at 
>> > arch/powerpc/kernel/signal_32.c:539:3:
>> >>> include/linux/fortify-string.h:195:4: error: call to 
>> >>> '__write_overflow_field' declared with attribute warning: detected write 
>> >>> beyond size of field (1st parameter); maybe use struct_group()? 
>> >>> [-Werror=attribute-warning]
>> >  195 |__write_overflow_field();
>> >  |^~~~
>> >
>> > Cc: Michael Ellerman 
>> > Cc: Benjamin Herrenschmidt 
>> > Cc: Paul Mackerras 
>> > Cc: Christophe Leroy 
>> > Cc: Sudeep Holla 
>> > Cc: linuxppc-...@lists.ozlabs.org
>> > Reported-by: kernel test robot 
>> > Signed-off-by: Kees Cook 
>> > ---
>> >  arch/powerpc/include/asm/processor.h | 6 --
>> >  arch/powerpc/kernel/signal_32.c  | 6 +++---
>> >  2 files changed, 7 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/arch/powerpc/include/asm/processor.h 
>> > b/arch/powerpc/include/asm/processor.h
>> > index f348e564f7dd..05dc567cb9a8 100644
>> > --- a/arch/powerpc/include/asm/processor.h
>> > +++ b/arch/powerpc/include/asm/processor.h
>> > @@ -191,8 +191,10 @@ struct thread_struct {
>> >int used_vsr;   /* set if process has used VSX */
>> >  #endif /* CONFIG_VSX */
>> >  #ifdef CONFIG_SPE
>> > -  unsigned long   evr[32];/* upper 32-bits of SPE regs */
>> > -  u64 acc;/* Accumulator */
>> > +  struct_group(spe,
>> > +  unsigned long   evr[32];/* upper 32-bits of SPE regs */
>> > +  u64 acc;/* Accumulator */
>> > +  );
>> >unsigned long   spefscr;/* SPE & eFP status */
>> >unsigned long   spefscr_last;   /* SPEFSCR value on last prctl
>> >   call or trap return */
>> > diff --git a/arch/powerpc/kernel/signal_32.c 
>> > b/arch/powerpc/kernel/signal_32.c
>> > index 0608581967f0..77b86caf5c51 100644
>> > --- a/arch/powerpc/kernel/signal_32.c
>> > +++ b/arch/powerpc/kernel/signal_32.c
>> > @@ -532,11 +532,11 @@ static long restore_user_regs(struct pt_regs *regs,
>> >regs_set_return_msr(regs, regs->msr & ~MSR_SPE);
>> >if (msr & MSR_SPE) {
>> >/* restore spe registers from the stack */
>> > -  unsafe_copy_from_user(current->thread.evr, >mc_vregs,
>> > -ELF_NEVRREG * sizeof(u32), failed);
>> > +  unsafe_copy_from_user(>thread.spe, >mc_vregs,
>> > +sizeof(current->thread.spe), failed);
>> 
>> This makes me nervous, because the ABI is that we copy ELF_NEVRREG *
>> sizeof(u32) bytes, not whatever sizeof(current->thread.spe) happens to
>> be.
>> 
>> ie. if we use sizeof an inadvertent change to the fields in
>> thread_struct could change how many bytes we copy out to userspace,
>> which would be an ABI break.
>> 
>> And that's not that hard to do, because it's not at all obvious that the
>> size and layout of fields in thread_struct affects the user ABI.
>> 
>> At the same time we don't want to copy the right number of bytes but
>> the wrong content, so from that point of view using sizeof is good :)
>> 
>> The way we handle it in ptrace is to have BUILD_BUG_ON()s to verify that
>> things match up, so maybe we should do that here too.
>> 
>> ie. add:
>> 
>>  BUILD_BUG_ON(sizeof(current->thread.spe) == ELF_NEVRREG * sizeof(u32));
>> 
>> Not sure if you are happy doing that as part of this patch. I can always
>> do it later if not.
>
> Sounds good to me; I did that in a few other cases in the series where
> the relationships between things seemed tenuous. :) I'll add this (as
> !=) in v3.

Thanks.

cheers

Re: [PATCH v2 57/63] powerpc/signal32: Use struct_group() to zero spe regs

[Michael Ellerman]

Christophe Leroy  writes:
> Le 20/08/2021 à 09:49, Michael Ellerman a écrit :
>> Kees Cook  writes:
>>> In preparation for FORTIFY_SOURCE performing compile-time and run-time
>>> field bounds checking for memset(), avoid intentionally writing across
>>> neighboring fields.
>>>
>>> Add a struct_group() for the spe registers so that memset() can correctly 
>>> reason
>>> about the size:
>>>
>>> In function 'fortify_memset_chk',
>>> inlined from 'restore_user_regs.part.0' at 
>>> arch/powerpc/kernel/signal_32.c:539:3:
>>>>> include/linux/fortify-string.h:195:4: error: call to 
>>>>> '__write_overflow_field' declared with attribute warning: detected write 
>>>>> beyond size of field (1st parameter); maybe use struct_group()? 
>>>>> [-Werror=attribute-warning]
>>>   195 |__write_overflow_field();
>>>   |^~~~
>>>
>>> Cc: Michael Ellerman 
>>> Cc: Benjamin Herrenschmidt 
>>> Cc: Paul Mackerras 
>>> Cc: Christophe Leroy 
>>> Cc: Sudeep Holla 
>>> Cc: linuxppc-...@lists.ozlabs.org
>>> Reported-by: kernel test robot 
>>> Signed-off-by: Kees Cook 
>>> ---
>>>   arch/powerpc/include/asm/processor.h | 6 --
>>>   arch/powerpc/kernel/signal_32.c  | 6 +++---
>>>   2 files changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/processor.h 
>>> b/arch/powerpc/include/asm/processor.h
>>> index f348e564f7dd..05dc567cb9a8 100644
>>> --- a/arch/powerpc/include/asm/processor.h
>>> +++ b/arch/powerpc/include/asm/processor.h
>>> @@ -191,8 +191,10 @@ struct thread_struct {
>>> int used_vsr;   /* set if process has used VSX */
>>>   #endif /* CONFIG_VSX */
>>>   #ifdef CONFIG_SPE
>>> -   unsigned long   evr[32];/* upper 32-bits of SPE regs */
>>> -   u64 acc;/* Accumulator */
>>> +   struct_group(spe,
>>> +   unsigned long   evr[32];/* upper 32-bits of SPE regs */
>>> +   u64 acc;/* Accumulator */
>>> +   );
>>> unsigned long   spefscr;/* SPE & eFP status */
>>> unsigned long   spefscr_last;   /* SPEFSCR value on last prctl
>>>call or trap return */
>>> diff --git a/arch/powerpc/kernel/signal_32.c 
>>> b/arch/powerpc/kernel/signal_32.c
>>> index 0608581967f0..77b86caf5c51 100644
>>> --- a/arch/powerpc/kernel/signal_32.c
>>> +++ b/arch/powerpc/kernel/signal_32.c
>>> @@ -532,11 +532,11 @@ static long restore_user_regs(struct pt_regs *regs,
>>> regs_set_return_msr(regs, regs->msr & ~MSR_SPE);
>>> if (msr & MSR_SPE) {
>>> /* restore spe registers from the stack */
>>> -   unsafe_copy_from_user(current->thread.evr, >mc_vregs,
>>> - ELF_NEVRREG * sizeof(u32), failed);
>>> +   unsafe_copy_from_user(>thread.spe, >mc_vregs,
>>> + sizeof(current->thread.spe), failed);
>> 
>> This makes me nervous, because the ABI is that we copy ELF_NEVRREG *
>> sizeof(u32) bytes, not whatever sizeof(current->thread.spe) happens to
>> be.
>> 
>> ie. if we use sizeof an inadvertent change to the fields in
>> thread_struct could change how many bytes we copy out to userspace,
>> which would be an ABI break.
>> 
>> And that's not that hard to do, because it's not at all obvious that the
>> size and layout of fields in thread_struct affects the user ABI.
>> 
>> At the same time we don't want to copy the right number of bytes but
>> the wrong content, so from that point of view using sizeof is good :)
>> 
>> The way we handle it in ptrace is to have BUILD_BUG_ON()s to verify that
>> things match up, so maybe we should do that here too.
>> 
>> ie. add:
>> 
>>  BUILD_BUG_ON(sizeof(current->thread.spe) == ELF_NEVRREG * sizeof(u32));
>
> You mean != I guess ?

Gah. Yes I do :)

cheers

Re: [PATCH v2 57/63] powerpc/signal32: Use struct_group() to zero spe regs

[Michael Ellerman]

Kees Cook  writes:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memset(), avoid intentionally writing across
> neighboring fields.
>
> Add a struct_group() for the spe registers so that memset() can correctly 
> reason
> about the size:
>
>In function 'fortify_memset_chk',
>inlined from 'restore_user_regs.part.0' at 
> arch/powerpc/kernel/signal_32.c:539:3:
>>> include/linux/fortify-string.h:195:4: error: call to 
>>> '__write_overflow_field' declared with attribute warning: detected write 
>>> beyond size of field (1st parameter); maybe use struct_group()? 
>>> [-Werror=attribute-warning]
>  195 |__write_overflow_field();
>  |^~~~
>
> Cc: Michael Ellerman 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: Christophe Leroy 
> Cc: Sudeep Holla 
> Cc: linuxppc-...@lists.ozlabs.org
> Reported-by: kernel test robot 
> Signed-off-by: Kees Cook 
> ---
>  arch/powerpc/include/asm/processor.h | 6 --
>  arch/powerpc/kernel/signal_32.c  | 6 +++---
>  2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/processor.h 
> b/arch/powerpc/include/asm/processor.h
> index f348e564f7dd..05dc567cb9a8 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -191,8 +191,10 @@ struct thread_struct {
>   int used_vsr;   /* set if process has used VSX */
>  #endif /* CONFIG_VSX */
>  #ifdef CONFIG_SPE
> - unsigned long   evr[32];/* upper 32-bits of SPE regs */
> - u64 acc;/* Accumulator */
> + struct_group(spe,
> + unsigned long   evr[32];/* upper 32-bits of SPE regs */
> + u64 acc;/* Accumulator */
> + );
>   unsigned long   spefscr;/* SPE & eFP status */
>   unsigned long   spefscr_last;   /* SPEFSCR value on last prctl
>  call or trap return */
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index 0608581967f0..77b86caf5c51 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -532,11 +532,11 @@ static long restore_user_regs(struct pt_regs *regs,
>   regs_set_return_msr(regs, regs->msr & ~MSR_SPE);
>   if (msr & MSR_SPE) {
>   /* restore spe registers from the stack */
> - unsafe_copy_from_user(current->thread.evr, >mc_vregs,
> -   ELF_NEVRREG * sizeof(u32), failed);
> + unsafe_copy_from_user(>thread.spe, >mc_vregs,
> +   sizeof(current->thread.spe), failed);

This makes me nervous, because the ABI is that we copy ELF_NEVRREG *
sizeof(u32) bytes, not whatever sizeof(current->thread.spe) happens to
be.

ie. if we use sizeof an inadvertent change to the fields in
thread_struct could change how many bytes we copy out to userspace,
which would be an ABI break.

And that's not that hard to do, because it's not at all obvious that the
size and layout of fields in thread_struct affects the user ABI.

At the same time we don't want to copy the right number of bytes but
the wrong content, so from that point of view using sizeof is good :)

The way we handle it in ptrace is to have BUILD_BUG_ON()s to verify that
things match up, so maybe we should do that here too.

ie. add:

BUILD_BUG_ON(sizeof(current->thread.spe) == ELF_NEVRREG * sizeof(u32));


Not sure if you are happy doing that as part of this patch. I can always
do it later if not.

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 58/64] powerpc: Split memset() to avoid multi-field overflow

[Michael Ellerman]

Kees Cook  writes:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memset(), avoid intentionally writing across
> neighboring fields.
>
> Instead of writing across a field boundary with memset(), move the call
> to just the array, and an explicit zeroing of the prior field.
>
> Signed-off-by: Kees Cook 
> ---
>  drivers/macintosh/smu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/macintosh/smu.c b/drivers/macintosh/smu.c
> index 94fb63a7b357..59ce431da7ef 100644
> --- a/drivers/macintosh/smu.c
> +++ b/drivers/macintosh/smu.c
> @@ -848,7 +848,8 @@ int smu_queue_i2c(struct smu_i2c_cmd *cmd)
>   cmd->read = cmd->info.devaddr & 0x01;
>   switch(cmd->info.type) {
>   case SMU_I2C_TRANSFER_SIMPLE:
> - memset(>info.sublen, 0, 4);
> + cmd->info.sublen = 0;
> + memset(>info.subaddr, 0, 3);
>   break;
>   case SMU_I2C_TRANSFER_COMBINED:
>   cmd->info.devaddr &= 0xfe;
> -- 
> 2.30.2

Reviewed-by: Michael Ellerman 

cheers

Re: [PATCH 1/2] vfio/pci: remove vfio_pci_nvlink2

[Michael Ellerman]

Alex Williamson  writes:
> On Mon, 12 Apr 2021 19:41:41 +1000
> Michael Ellerman  wrote:
>
>> Alex Williamson  writes:
>> > On Fri, 26 Mar 2021 07:13:10 +0100
>> > Christoph Hellwig  wrote:
>> >  
>> >> This driver never had any open userspace (which for VFIO would include
>> >> VM kernel drivers) that use it, and thus should never have been added
>> >> by our normal userspace ABI rules.
>> >> 
>> >> Signed-off-by: Christoph Hellwig 
>> >> Acked-by: Greg Kroah-Hartman 
>> >> ---
>> >>  drivers/vfio/pci/Kconfig|   6 -
>> >>  drivers/vfio/pci/Makefile   |   1 -
>> >>  drivers/vfio/pci/vfio_pci.c |  18 -
>> >>  drivers/vfio/pci/vfio_pci_nvlink2.c | 490 
>> >>  drivers/vfio/pci/vfio_pci_private.h |  14 -
>> >>  include/uapi/linux/vfio.h   |  38 +--
>> >>  6 files changed, 4 insertions(+), 563 deletions(-)
>> >>  delete mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c  
>> >
>> > Hearing no objections, applied to vfio next branch for v5.13.  Thanks,  
>> 
>> Looks like you only took patch 1?
>> 
>> I can't take patch 2 on its own, that would break the build.
>> 
>> Do you want to take both patches? There's currently no conflicts against
>> my tree. It's possible one could appear before the v5.13 merge window,
>> though it would probably just be something minor.
>> 
>> Or I could apply both patches to my tree, which means patch 1 would
>> appear as two commits in the git history, but that's not a big deal.
>
> I've already got a conflict in my next branch with patch 1, so it's
> best to go through my tree.  Seems like a shared branch would be
> easiest to allow you to merge and manage potential conflicts against
> patch 2, I've pushed a branch here:
>
> https://github.com/awilliam/linux-vfio.git v5.13/vfio/nvlink

Thanks.

My next is based on rc2, so I won't pull that in directly, because I
don't want to pull all of rc6 in with it.

I'll put it in a topic branch and merge it into my next after my first
pull has gone to Linus.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] vfio/pci: remove vfio_pci_nvlink2

[Michael Ellerman]

Alex Williamson  writes:
> On Fri, 26 Mar 2021 07:13:10 +0100
> Christoph Hellwig  wrote:
>
>> This driver never had any open userspace (which for VFIO would include
>> VM kernel drivers) that use it, and thus should never have been added
>> by our normal userspace ABI rules.
>> 
>> Signed-off-by: Christoph Hellwig 
>> Acked-by: Greg Kroah-Hartman 
>> ---
>>  drivers/vfio/pci/Kconfig|   6 -
>>  drivers/vfio/pci/Makefile   |   1 -
>>  drivers/vfio/pci/vfio_pci.c |  18 -
>>  drivers/vfio/pci/vfio_pci_nvlink2.c | 490 
>>  drivers/vfio/pci/vfio_pci_private.h |  14 -
>>  include/uapi/linux/vfio.h   |  38 +--
>>  6 files changed, 4 insertions(+), 563 deletions(-)
>>  delete mode 100644 drivers/vfio/pci/vfio_pci_nvlink2.c
>
> Hearing no objections, applied to vfio next branch for v5.13.  Thanks,

Looks like you only took patch 1?

I can't take patch 2 on its own, that would break the build.

Do you want to take both patches? There's currently no conflicts against
my tree. It's possible one could appear before the v5.13 merge window,
though it would probably just be something minor.

Or I could apply both patches to my tree, which means patch 1 would
appear as two commits in the git history, but that's not a big deal.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/13] powerpc/powernv: remove get_cxl_module

[Michael Ellerman]

Christoph Hellwig  writes:
> The static inline get_cxl_module function is entirely unused since commit
> 8bf6b91a5125a ("Revert "powerpc/powernv: Add support for the cxl kernel
> api on the real phb"), so remove it.
>
> Signed-off-by: Christoph Hellwig 
> Reviewed-by: Andrew Donnellan 
> ---
>  arch/powerpc/platforms/powernv/pci-cxl.c | 22 --
>  1 file changed, 22 deletions(-)

Acked-by: Michael Ellerman 

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] powerpc/ps3: make system bus's remove and shutdown callbacks return void

[Michael Ellerman]

Uwe Kleine-König  writes:
> Hello Michael,
>
> On Sat, Nov 28, 2020 at 09:48:30AM +0100, Takashi Iwai wrote:
>> On Thu, 26 Nov 2020 17:59:50 +0100,
>> Uwe Kleine-König wrote:
>> > 
>> > The driver core ignores the return value of struct device_driver::remove
>> > because there is only little that can be done. For the shutdown callback
>> > it's ps3_system_bus_shutdown() which ignores the return value.
>> > 
>> > To simplify the quest to make struct device_driver::remove return void,
>> > let struct ps3_system_bus_driver::remove return void, too. All users
>> > already unconditionally return 0, this commit makes it obvious that
>> > returning an error code is a bad idea and ensures future users behave
>> > accordingly.
>> > 
>> > Signed-off-by: Uwe Kleine-König 
>> 
>> For the sound bit:
>> Acked-by: Takashi Iwai 
>
> assuming that you are the one who will apply this patch: Note that it
> depends on patch 1 that Takashi already applied to his tree. So you
> either have to wait untils patch 1 appears in some tree that you merge
> before applying, or you have to take patch 1, too. (With Takashi
> optinally dropping it then.)

Thanks. I've picked up both patches.

If Takashi doesn't want to rebase his tree to drop patch 1 that's OK, it
will just arrive in mainline via two paths, but git should handle it.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] video: fbdev: controlfb: Fix build for COMPILE_TEST=y && PPC_PMAC=n

[Michael Ellerman]

On Fri, 21 Aug 2020 20:49:10 +1000, Michael Ellerman wrote:
> The build is currently broken, if COMPILE_TEST=y and PPC_PMAC=n:
> 
>   linux/drivers/video/fbdev/controlfb.c: In function 
> ‘control_set_hardware’:
>   linux/drivers/video/fbdev/controlfb.c:276:2: error: implicit declaration of 
> function ‘btext_update_display’
> 276 |  btext_update_display(p->frame_buffer_phys + CTRLFB_OFF,
> |  ^~~~
> 
> [...]

Applied to powerpc/fixes.

[1/1] video: fbdev: controlfb: Fix build for COMPILE_TEST=y && PPC_PMAC=n
  https://git.kernel.org/powerpc/c/4d618b9f3fcab84e9ec28c180de46fb2c929d096

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] video: fbdev: controlfb: Fix build for COMPILE_TEST=y && PPC_PMAC=n

[Michael Ellerman]

The build is currently broken, if COMPILE_TEST=y and PPC_PMAC=n:

  linux/drivers/video/fbdev/controlfb.c: In function ‘control_set_hardware’:
  linux/drivers/video/fbdev/controlfb.c:276:2: error: implicit declaration of 
function ‘btext_update_display’
276 |  btext_update_display(p->frame_buffer_phys + CTRLFB_OFF,
|  ^~~~

Fix it by including btext.h whenever CONFIG_BOOTX_TEXT is enabled.

Fixes: a07a63b0e24d ("video: fbdev: controlfb: add COMPILE_TEST support")
Signed-off-by: Michael Ellerman 
---
 drivers/video/fbdev/controlfb.c | 2 ++
 1 file changed, 2 insertions(+)

Does anyone mind if I apply this via the powerpc tree for v5.9?

It would be nice to get the build clean.

cheers

diff --git a/drivers/video/fbdev/controlfb.c b/drivers/video/fbdev/controlfb.c
index 9c4f1be856ec..547abeb39f87 100644
--- a/drivers/video/fbdev/controlfb.c
+++ b/drivers/video/fbdev/controlfb.c
@@ -49,6 +49,8 @@
 #include 
 #ifdef CONFIG_PPC_PMAC
 #include 
+#endif
+#ifdef CONFIG_BOOTX_TEXT
 #include 
 #endif
 
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms

[Michael Ellerman]

On Mon, 30 Mar 2020 15:32:15 +0200, Michal Simek wrote:
> recently we wanted to update xilinx intc driver and we found that function
> which we wanted to remove is still wired by ancient Xilinx PowerPC
> platforms. Here is the thread about it.
> https://lore.kernel.org/linux-next/48d3232d-0f1d-42ea-3109-f44bbabfa...@xilinx.com/
> 
> I have been talking about it internally and there is no interest in these
> platforms and it is also orphan for quite a long time. None is really
> running/testing these platforms regularly that's why I think it makes sense
> to remove them also with drivers which are specific to this platform.
> 
> [...]

Applied to powerpc/next.

[1/2] sound: ac97: Remove sound driver for ancient platform
  https://git.kernel.org/powerpc/c/f16dca3e30c14aff545a834a7c1a1bb02b9edb48
[2/2] powerpc: Remove Xilinx PPC405/PPC440 support
  https://git.kernel.org/powerpc/c/7ade8495dcfd788a76e6877c9ea86f5207369ea4

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms

[Michael Ellerman]

Christophe Leroy  writes:
> Hi,
>
> Le 03/06/2020 à 10:10, Michal Simek a écrit :
>> Hi Michael,
>> 
>> On 26. 05. 20 15:44, Michael Ellerman wrote:
>>> Michal Simek  writes:
>>>> Hi Michael,
>>>>
>>>> On 01. 04. 20 13:30, Michal Simek wrote:
>>>>> On 01. 04. 20 12:38, Takashi Iwai wrote:
>>>>>> On Wed, 01 Apr 2020 12:35:16 +0200,
>>>>>> Michael Ellerman wrote:
>>>>>>>
>>>>>>> Michal Simek  writes:
>>>>>>>> On 01. 04. 20 4:07, Michael Ellerman wrote:
>>>>>>>>> Michal Simek  writes:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> recently we wanted to update xilinx intc driver and we found that 
>>>>>>>>>> function
>>>>>>>>>> which we wanted to remove is still wired by ancient Xilinx PowerPC
>>>>>>>>>> platforms. Here is the thread about it.
>>>>>>>>>> https://lore.kernel.org/linux-next/48d3232d-0f1d-42ea-3109-f44bbabfa...@xilinx.com/
>>>>>>>>>>
>>>>>>>>>> I have been talking about it internally and there is no interest in 
>>>>>>>>>> these
>>>>>>>>>> platforms and it is also orphan for quite a long time. None is really
>>>>>>>>>> running/testing these platforms regularly that's why I think it 
>>>>>>>>>> makes sense
>>>>>>>>>> to remove them also with drivers which are specific to this platform.
>>>>>>>>>>
>>>>>>>>>> U-Boot support was removed in 2017 without anybody complain about it
>>>>>>>>>> https://github.com/Xilinx/u-boot-xlnx/commit/98f705c9cefdfdba62c069821bbba10273a0a8ed
>>>>>>>>>>
>>>>>>>>>> Based on current ppc/next.
>>>>>>>>>>
>>>>>>>>>> If anyone has any objection about it, please let me know.
>>>>>>>>>
>>>>>>>>> Thanks for taking the time to find all this code and remove it.
>>>>>>>>>
>>>>>>>>> I'm not going to take this series for v5.7, it was posted too close to
>>>>>>>>> the merge window, and doing so wouldn't give people much time to 
>>>>>>>>> object,
>>>>>>>>> especially given people are distracted at the moment.
>>>>>>>>>
>>>>>>>>> I'm happy to take it for v5.8, assuming there's no major objections.
>>>>>>>>
>>>>>>>> Sure. Just to let you know Christophe Leroy included this patch in his
>>>>>>>> series about ppc405 removal. It should be the same.
>>>>>>>>
>>>>>>>> If you don't want to take that alsa patch I can send it separately and
>>>>>>>> this patch can be taken from his series. I don't really mind but please
>>>>>>>> let me know what way you prefer.
>>>>>>>
>>>>>>> It's better to keep it all together, so I'm happy take the alsa patch as
>>>>>>> well, it's already been acked.
>>>>
>>>> Can you please take this series? I know that there is v5 from Christophe
>>>> which has this 1/2 as 1/13. But I need this alsa patch too and I would
>>>> like to close this because it is around for almost 2 months and none
>>>> raised a concern about removing just these Xilinx platforms.
>>>
>>> Sorry I meant to reply to your last mail.
>>>
>>> I have Christophe's series in my testing branch, planning for it to be
>>> in v5.8.
>>>
>>> Even if the rest of his series doesn't make it for some reason, as you
>>> say the Xilinx removal is uncontroversial so I'll keep that in.
>>>
>>> I forgot about the sound patch, I'll pick that up as well.
>> 
>> I took a look at your
>> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git repo
>> and I can't see any branch with my patches.
>> Also was checking linux-next and my patches are also not there.
>> That's why I am curious if this will be go v5.8 in MW.
>
> I see them in 
> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git in 
> next-test branch.

Which == the next branch, which is what will go into v5.8, all going
well.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms

[Michael Ellerman]

Michal Simek  writes:
> Hi Michael,
>
> On 01. 04. 20 13:30, Michal Simek wrote:
>> On 01. 04. 20 12:38, Takashi Iwai wrote:
>>> On Wed, 01 Apr 2020 12:35:16 +0200,
>>> Michael Ellerman wrote:
>>>>
>>>> Michal Simek  writes:
>>>>> On 01. 04. 20 4:07, Michael Ellerman wrote:
>>>>>> Michal Simek  writes:
>>>>>>> Hi,
>>>>>>>
>>>>>>> recently we wanted to update xilinx intc driver and we found that 
>>>>>>> function
>>>>>>> which we wanted to remove is still wired by ancient Xilinx PowerPC
>>>>>>> platforms. Here is the thread about it.
>>>>>>> https://lore.kernel.org/linux-next/48d3232d-0f1d-42ea-3109-f44bbabfa...@xilinx.com/
>>>>>>>
>>>>>>> I have been talking about it internally and there is no interest in 
>>>>>>> these
>>>>>>> platforms and it is also orphan for quite a long time. None is really
>>>>>>> running/testing these platforms regularly that's why I think it makes 
>>>>>>> sense
>>>>>>> to remove them also with drivers which are specific to this platform.
>>>>>>>
>>>>>>> U-Boot support was removed in 2017 without anybody complain about it
>>>>>>> https://github.com/Xilinx/u-boot-xlnx/commit/98f705c9cefdfdba62c069821bbba10273a0a8ed
>>>>>>>
>>>>>>> Based on current ppc/next.
>>>>>>>
>>>>>>> If anyone has any objection about it, please let me know.
>>>>>>
>>>>>> Thanks for taking the time to find all this code and remove it.
>>>>>>
>>>>>> I'm not going to take this series for v5.7, it was posted too close to
>>>>>> the merge window, and doing so wouldn't give people much time to object,
>>>>>> especially given people are distracted at the moment.
>>>>>>
>>>>>> I'm happy to take it for v5.8, assuming there's no major objections.
>>>>>
>>>>> Sure. Just to let you know Christophe Leroy included this patch in his
>>>>> series about ppc405 removal. It should be the same.
>>>>>
>>>>> If you don't want to take that alsa patch I can send it separately and
>>>>> this patch can be taken from his series. I don't really mind but please
>>>>> let me know what way you prefer.
>>>>
>>>> It's better to keep it all together, so I'm happy take the alsa patch as
>>>> well, it's already been acked.
>
> Can you please take this series? I know that there is v5 from Christophe
> which has this 1/2 as 1/13. But I need this alsa patch too and I would
> like to close this because it is around for almost 2 months and none
> raised a concern about removing just these Xilinx platforms.

Sorry I meant to reply to your last mail.

I have Christophe's series in my testing branch, planning for it to be
in v5.8.

Even if the rest of his series doesn't make it for some reason, as you
say the Xilinx removal is uncontroversial so I'll keep that in.

I forgot about the sound patch, I'll pick that up as well.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms

[Michael Ellerman]

Christophe Leroy  writes:
> Le 21/05/2020 à 09:02, Michael Ellerman a écrit :
>> Arnd Bergmann  writes:
>>> +On Wed, Apr 8, 2020 at 2:04 PM Michael Ellerman  
>>> wrote:
>>>> Benjamin Herrenschmidt  writes:
>>>>> On Fri, 2020-04-03 at 15:59 +1100, Michael Ellerman wrote:
>>>>>> Benjamin Herrenschmidt  writes:
>>>>> IBM still put 40x cores inside POWER chips no ?
>>>>
>>>> Oh yeah that's true. I guess most folks don't know that, or that they
>>>> run RHEL on them.
>>>
>>> Is there a reason for not having those dts files in mainline then?
>>> If nothing else, it would document what machines are still being
>>> used with future kernels.
>> 
>> Sorry that part was a joke :D  Those chips don't run Linux.
>> 
>
> Nice to know :)
>
> What's the plan then, do we still want to keep 40x in the kernel ?

I guess we keep it for now.

Perhaps we mark it BROKEN for a few releases and see if anyone
complains?

> If yes, is it ok to drop the oldies anyway as done in my series 
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=172630 ?

Yeah let's do it. I would love to get rid of that horrible
PPC405_ERR77() sprinkled all through our atomics.

> (Note that this series will conflict with my series on hugepages on 8xx 
> due to the PTE_ATOMIC_UPDATES stuff. I can rebase the 40x modernisation 
> series on top of the 8xx hugepages series if it is worth it)

Yeah if you can rebase that would be great.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms

[Michael Ellerman]

Arnd Bergmann  writes:
> +On Wed, Apr 8, 2020 at 2:04 PM Michael Ellerman  wrote:
>> Benjamin Herrenschmidt  writes:
>> > On Fri, 2020-04-03 at 15:59 +1100, Michael Ellerman wrote:
>> >> Benjamin Herrenschmidt  writes:
>> > IBM still put 40x cores inside POWER chips no ?
>>
>> Oh yeah that's true. I guess most folks don't know that, or that they
>> run RHEL on them.
>
> Is there a reason for not having those dts files in mainline then?
> If nothing else, it would document what machines are still being
> used with future kernels.

Sorry that part was a joke :D  Those chips don't run Linux.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drivers/powerpc: Replace _ALIGN_UP() by ALIGN()

[Michael Ellerman]

On Mon, 20 Apr 2020 18:36:34 + (UTC), Christophe Leroy wrote:
> _ALIGN_UP() is specific to powerpc
> ALIGN() is generic and does the same
> 
> Replace _ALIGN_UP() by ALIGN()

Applied to powerpc/next.

[1/5] drivers/powerpc: Replace _ALIGN_UP() by ALIGN()
  https://git.kernel.org/powerpc/c/7bfc3c84cbf5167d943cff9b3d2619dab0b7894c
[2/5] powerpc: Replace _ALIGN_DOWN() by ALIGN_DOWN()
  https://git.kernel.org/powerpc/c/e96d904ede6756641563d27daa746875b478a6c8
[3/5] powerpc: Replace _ALIGN_UP() by ALIGN()
  https://git.kernel.org/powerpc/c/b711531641038f3ff3723914f3d5ba79848d347e
[4/5] powerpc: Replace _ALIGN() by ALIGN()
  https://git.kernel.org/powerpc/c/d3f3d3bf76cfb04e73436a15e3987d3573e7523a
[5/5] powerpc: Remove _ALIGN_UP(), _ALIGN_DOWN() and _ALIGN()
  https://git.kernel.org/powerpc/c/4cdb2da654033d76e1b1cb4ac427d9193dce816b

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] powerpc/configs: replace deprecated riva/nvidia with nouveau

[Michael Ellerman]

Emil Velikov  writes:
> As mentioned in earlier commit, the riva and nvidia fbdev drivers have
> seen no love over the years, are short on features and overall below par
>
> Users are encouraged to switch to the nouveau drm driver instead.
>
> v2: Split configs to separate patch, enable nouveau (Bartlomiej)
>
> Cc: Antonino Daplas 
> Cc: Bartlomiej Zolnierkiewicz 
> Cc: linux-fb...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: Michael Ellerman 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: linuxppc-...@lists.ozlabs.org
> Signed-off-by: Emil Velikov 
> Acked-by: Daniel Vetter  (v1)
> ---
> Hi all unless, there are objections I would prefer to merge this via
> the drm tree.

Have you tested that the resulting kernels work on the relevant
hardware?

The old drivers may be crufty but they presumably have been tested by
people and at least somewhat work.

So I'd be inclined to leave the defconfigs alone until someone can test
that the new driver works at all.

I gave it a quick spin on a G5 I have access to and dmesg has a bunch of
errors in it (see below). I can't actually tell if the display is
working because the machine is remote, and I can't go and check it at
the moment because the office is closed.

cheers


[  128.563251] nouveau :f0:10.0: NVIDIA NV34 (034900b1)
[  128.565720] nouveau :f0:10.0: bios: version 04.34.20.18.00
[  128.570994] nouveau :f0:10.0: bios: OOB 1 0cdd 0cdd
[  128.571702] nouveau :f0:10.0: bios: OOB 1 0ce1 0ce1
[  128.571723] nouveau :f0:10.0: bios: OOB 1 0cde 0cde
[  128.571731] nouveau :f0:10.0: bios: OOB 1 0cdf 0cdf
[  128.572438] nouveau :f0:10.0: bios: OOB 1 0ce5 0ce5
[  128.572459] nouveau :f0:10.0: bios: OOB 1 0ce2 0ce2
[  128.572468] nouveau :f0:10.0: bios: OOB 1 0ce3 0ce3
[  128.579187] nouveau :f0:10.0: bios: OOB 1 0ce9 0ce9
[  128.579210] nouveau :f0:10.0: bios: OOB 1 0ce6 0ce6
[  128.579219] nouveau :f0:10.0: bios: OOB 1 0ce7 0ce7
[  128.580230] nouveau :f0:10.0: bios: OOB 1 0ced 0ced
[  128.580252] nouveau :f0:10.0: bios: OOB 1 0cea 0cea
[  128.580261] nouveau :f0:10.0: bios: OOB 1 0ceb 0ceb
[  128.581543] nouveau :f0:10.0: bios: OOB 1 0cf1 0cf1
[  128.581567] nouveau :f0:10.0: bios: OOB 1 0cee 0cee
[  128.581576] nouveau :f0:10.0: bios: OOB 1 0cef 0cef
[  128.582456] nouveau :f0:10.0: bios: OOB 1 0cf5 0cf5
[  128.582479] nouveau :f0:10.0: bios: OOB 1 0cf2 0cf2
[  128.582487] nouveau :f0:10.0: bios: OOB 1 0cf3 0cf3
[  128.583314] nouveau :f0:10.0: bios: OOB 1 0cf9 0cf9
[  128.583336] nouveau :f0:10.0: bios: OOB 1 0cf6 0cf6
[  128.583344] nouveau :f0:10.0: bios: OOB 1 0cf7 0cf7
[  128.584186] nouveau :f0:10.0: bios: OOB 1 0cfd 0cfd
[  128.584208] nouveau :f0:10.0: bios: OOB 1 0cfa 0cfa
[  128.584217] nouveau :f0:10.0: bios: OOB 1 0cfb 0cfb
[  128.585635] nouveau :f0:10.0: bios: OOB 1 0d01 0d01
[  128.585658] nouveau :f0:10.0: bios: OOB 1 0cfe 0cfe
[  128.585666] nouveau :f0:10.0: bios: OOB 1 0cff 0cff
[  128.589405] nouveau :f0:10.0: bios: OOB 1 0d05 0d05
[  128.589428] nouveau :f0:10.0: bios: OOB 1 0d02 0d02
[  128.589436] nouveau :f0:10.0: bios: OOB 1 0d03 0d03
[  128.590316] nouveau :f0:10.0: bios: OOB 1 0d09 0d09
[  128.590338] nouveau :f0:10.0: bios: OOB 1 0d06 0d06
[  128.590346] nouveau :f0:10.0: bios: OOB 1 0d07 0d07
[  128.591173] nouveau :f0:10.0: bios: OOB 1 0d0d 0d0d
[  128.591196] nouveau :f0:10.0: bios: OOB 1 0d0a 0d0a
[  128.591205] nouveau :f0:10.0: bios: OOB 1 0d0b 0d0b
[  128.593513] nouveau :f0:10.0: tmr: unknown input clock freq
[  128.595886] nouveau :f0:10.0: fb: 64 MiB DDR1
[  128.609034] [TTM] Zone  kernel: Available graphics memory: 500010 KiB
[  128.609058] [TTM] Initializing pool allocator
[  128.609106] nouveau :f0:10.0: DRM: VRAM: 63 MiB
[  128.609115] nouveau :f0:10.0: DRM: GART: 128 MiB
[  128.609134] nouveau :f0:10.0: DRM: BMP version 5.38
[  128.609144] nouveau :f0:10.0: DRM: DCB version 2.2
[  128.609155] nouveau :f0:10.0: DRM: DCB outp 00: 01000122 0004
[  128.609168] nouveau :f0:10.0: DRM: DCB outp 01: 02010200 11b088b8
[  128.609177] nouveau :f0:10.0: DRM: DCB outp 02: 02010201 11b00703
[  128.609222] nouveau :f0:10.0: DRM: Loading NV17 power sequencing 
microcode
[  128.610947] nouveau :f0:10.0: DRM: MM: using M2MF for buffer copies
[  128.613371] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[  128.615065] nouveau :f0:10.0: DRM: Setting dpms mode 3 on TV encoder 
(output 2)
[  128.707050] nouveau :f0:10.0: TV-1: EDID is invalid:
[  128.

Re: [PATCH 06/11] powerpc/xmon: constify sysrq_key_op

[Michael Ellerman]

Emil Velikov  writes:
> With earlier commits, the API no longer discards the const-ness of the
> sysrq_key_op. As such we can add the notation.
>
> Cc: Greg Kroah-Hartman 
> Cc: Jiri Slaby 
> Cc: linux-ker...@vger.kernel.org
> Cc: Michael Ellerman 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: linuxppc-...@lists.ozlabs.org
> Signed-off-by: Emil Velikov 
> ---
> Please keep me in the CC list, as I'm not subscribed to the list.
>
> IMHO it would be better if this gets merged this via the tty tree.

Fine by me.

Acked-by: Michael Ellerman 

cheers

> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 7af840c0fc93..0d8ca5c9f131 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -3842,7 +3842,7 @@ static void sysrq_handle_xmon(int key)
>   xmon_init(0);
>  }
>  
> -static struct sysrq_key_op sysrq_xmon_op = {
> +static const struct sysrq_key_op sysrq_xmon_op = {
>   .handler =  sysrq_handle_xmon,
>   .help_msg = "xmon(x)",
>   .action_msg =   "Entering xmon",
> -- 
> 2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V1 00/10] Remove duplicated kmap code

[Michael Ellerman]

ira.we...@intel.com writes:
> From: Ira Weiny 
>
> The kmap infrastructure has been copied almost verbatim to every architecture.
> This series consolidates obvious duplicated code by defining core functions
> which call into the architectures only when needed.
>
> Some of the k[un]map_atomic() implementations have some similarities but the
> similarities were not sufficient to warrant further changes.
>
> In addition we remove a duplicate implementation of kmap() in DRM.
>
> Testing was done by 0day to cover all the architectures I can't readily
> build/test.

I threw some powerpc builds at it and they all passed, so LGTM.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms

[Michael Ellerman]

Benjamin Herrenschmidt  writes:
> On Fri, 2020-04-03 at 15:59 +1100, Michael Ellerman wrote:
>> Benjamin Herrenschmidt  writes:
>> > On Tue, 2020-03-31 at 16:30 +1100, Michael Ellerman wrote:
>> > > I have no attachment to 40x, and I'd certainly be happy to have
>> > > less
>> > > code in the tree, we struggle to keep even the modern platforms
>> > > well
>> > > maintained.
>> > > 
>> > > At the same time I don't want to render anyone's hardware
>> > > obsolete
>> > > unnecessarily. But if there's really no one using 40x then we
>> > > should
>> > > remove it, it could well be broken already.
>> > > 
>> > > So I guess post a series to do the removal and we'll see if
>> > > anyone
>> > > speaks up.
>> > 
>> > We shouldn't remove 40x completely. Just remove the Xilinx 405
>> > stuff.
>> 
>> Congratulations on becoming the 40x maintainer!
>
> Didn't I give you my last 40x system ? :-)

Probably, but my desk is nearly as messy as yours so it's probably
buried under some even more obscure hardware :P

> IBM still put 40x cores inside POWER chips no ?

Oh yeah that's true. I guess most folks don't know that, or that they
run RHEL on them.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms

[Michael Ellerman]

Benjamin Herrenschmidt  writes:
> On Tue, 2020-03-31 at 16:30 +1100, Michael Ellerman wrote:
>> I have no attachment to 40x, and I'd certainly be happy to have less
>> code in the tree, we struggle to keep even the modern platforms well
>> maintained.
>> 
>> At the same time I don't want to render anyone's hardware obsolete
>> unnecessarily. But if there's really no one using 40x then we should
>> remove it, it could well be broken already.
>> 
>> So I guess post a series to do the removal and we'll see if anyone
>> speaks up.
>
> We shouldn't remove 40x completely. Just remove the Xilinx 405 stuff.

Congratulations on becoming the 40x maintainer!

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms

[Michael Ellerman]

Michal Simek  writes:
> On 01. 04. 20 4:07, Michael Ellerman wrote:
>> Michal Simek  writes:
>>> Hi,
>>>
>>> recently we wanted to update xilinx intc driver and we found that function
>>> which we wanted to remove is still wired by ancient Xilinx PowerPC
>>> platforms. Here is the thread about it.
>>> https://lore.kernel.org/linux-next/48d3232d-0f1d-42ea-3109-f44bbabfa...@xilinx.com/
>>>
>>> I have been talking about it internally and there is no interest in these
>>> platforms and it is also orphan for quite a long time. None is really
>>> running/testing these platforms regularly that's why I think it makes sense
>>> to remove them also with drivers which are specific to this platform.
>>>
>>> U-Boot support was removed in 2017 without anybody complain about it
>>> https://github.com/Xilinx/u-boot-xlnx/commit/98f705c9cefdfdba62c069821bbba10273a0a8ed
>>>
>>> Based on current ppc/next.
>>>
>>> If anyone has any objection about it, please let me know.
>> 
>> Thanks for taking the time to find all this code and remove it.
>> 
>> I'm not going to take this series for v5.7, it was posted too close to
>> the merge window, and doing so wouldn't give people much time to object,
>> especially given people are distracted at the moment.
>> 
>> I'm happy to take it for v5.8, assuming there's no major objections.
>
> Sure. Just to let you know Christophe Leroy included this patch in his
> series about ppc405 removal. It should be the same.
>
> If you don't want to take that alsa patch I can send it separately and
> this patch can be taken from his series. I don't really mind but please
> let me know what way you prefer.

It's better to keep it all together, so I'm happy take the alsa patch as
well, it's already been acked.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms

[Michael Ellerman]

Michal Simek  writes:
> Hi,
>
> recently we wanted to update xilinx intc driver and we found that function
> which we wanted to remove is still wired by ancient Xilinx PowerPC
> platforms. Here is the thread about it.
> https://lore.kernel.org/linux-next/48d3232d-0f1d-42ea-3109-f44bbabfa...@xilinx.com/
>
> I have been talking about it internally and there is no interest in these
> platforms and it is also orphan for quite a long time. None is really
> running/testing these platforms regularly that's why I think it makes sense
> to remove them also with drivers which are specific to this platform.
>
> U-Boot support was removed in 2017 without anybody complain about it
> https://github.com/Xilinx/u-boot-xlnx/commit/98f705c9cefdfdba62c069821bbba10273a0a8ed
>
> Based on current ppc/next.
>
> If anyone has any objection about it, please let me know.

Thanks for taking the time to find all this code and remove it.

I'm not going to take this series for v5.7, it was posted too close to
the merge window, and doing so wouldn't give people much time to object,
especially given people are distracted at the moment.

I'm happy to take it for v5.8, assuming there's no major objections.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms

[Michael Ellerman]

Christophe Leroy  writes:
> Le 27/03/2020 à 15:14, Andy Shevchenko a écrit :
>> On Fri, Mar 27, 2020 at 02:22:55PM +0100, Arnd Bergmann wrote:
>>> On Fri, Mar 27, 2020 at 2:15 PM Andy Shevchenko
>>>  wrote:
 On Fri, Mar 27, 2020 at 03:10:26PM +0200, Andy Shevchenko wrote:
> On Fri, Mar 27, 2020 at 01:54:33PM +0100, Arnd Bergmann wrote:
>> On Fri, Mar 27, 2020 at 1:12 PM Michal Simek  
>> wrote:
>> ...
>> 
>> It does raise a follow-up question about ppc40x though: is it time to
>> retire all of it?
>
> Who knows?
>
> I have in possession nice WD My Book Live, based on this architecture, 
> and I
> won't it gone from modern kernel support. OTOH I understand that amount 
> of real
> users not too big.

 +Cc: Christian Lamparter, whom I owe for that WD box.
>>>
>>> According to https://openwrt.org/toh/wd/mybooklive, that one is based on
>>> APM82181/ppc464, so it is about several generations newer than what I
>>> asked about (ppc40x).
>>>
> Ah, and I have Amiga board, but that one is being used only for testing, 
> so,
> I don't care much.
>>>
>>> I think there are a couple of ppc440 based Amiga boards, but again, not 405
>>> to my knowledge.
>> 
>> Ah, you are right. No objections from ppc40x removal!
>
> Removing 40x would help cleaning things a bit. For instance 40x is the 
> last platform still having PTE_ATOMIC_UPDATES. So if we can remove 40x 
> we can get rid of PTE_ATOMIC_UPDATES completely.
>
> If no one objects, I can prepare a series to drop support for 40x 
> completely.
>
> Michael, any thought ?

I have no attachment to 40x, and I'd certainly be happy to have less
code in the tree, we struggle to keep even the modern platforms well
maintained.

At the same time I don't want to render anyone's hardware obsolete
unnecessarily. But if there's really no one using 40x then we should
remove it, it could well be broken already.

So I guess post a series to do the removal and we'll see if anyone
speaks up.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RESEND PATCH v2 1/9] iomap: Constify ioreadX() iomem argument (as in generic implementation)

[Michael Ellerman]

Krzysztof Kozlowski  writes:
> diff --git a/arch/powerpc/kernel/iomap.c b/arch/powerpc/kernel/iomap.c
> index 5ac84efc6ede..9fe4fb3b08aa 100644
> --- a/arch/powerpc/kernel/iomap.c
> +++ b/arch/powerpc/kernel/iomap.c
> @@ -15,23 +15,23 @@
>   * Here comes the ppc64 implementation of the IOMAP 
>   * interfaces.
>   */
> -unsigned int ioread8(void __iomem *addr)
> +unsigned int ioread8(const void __iomem *addr)
>  {
>   return readb(addr);
>  }
> -unsigned int ioread16(void __iomem *addr)
> +unsigned int ioread16(const void __iomem *addr)
>  {
>   return readw(addr);
>  }
> -unsigned int ioread16be(void __iomem *addr)
> +unsigned int ioread16be(const void __iomem *addr)
>  {
>   return readw_be(addr);
>  }
> -unsigned int ioread32(void __iomem *addr)
> +unsigned int ioread32(const void __iomem *addr)
>  {
>   return readl(addr);
>  }
> -unsigned int ioread32be(void __iomem *addr)
> +unsigned int ioread32be(const void __iomem *addr)
>  {
>   return readl_be(addr);
>  }
> @@ -41,27 +41,27 @@ EXPORT_SYMBOL(ioread16be);
>  EXPORT_SYMBOL(ioread32);
>  EXPORT_SYMBOL(ioread32be);
>  #ifdef __powerpc64__
> -u64 ioread64(void __iomem *addr)
> +u64 ioread64(const void __iomem *addr)
>  {
>   return readq(addr);
>  }
> -u64 ioread64_lo_hi(void __iomem *addr)
> +u64 ioread64_lo_hi(const void __iomem *addr)
>  {
>   return readq(addr);
>  }
> -u64 ioread64_hi_lo(void __iomem *addr)
> +u64 ioread64_hi_lo(const void __iomem *addr)
>  {
>   return readq(addr);
>  }
> -u64 ioread64be(void __iomem *addr)
> +u64 ioread64be(const void __iomem *addr)
>  {
>   return readq_be(addr);
>  }
> -u64 ioread64be_lo_hi(void __iomem *addr)
> +u64 ioread64be_lo_hi(const void __iomem *addr)
>  {
>   return readq_be(addr);
>  }
> -u64 ioread64be_hi_lo(void __iomem *addr)
> +u64 ioread64be_hi_lo(const void __iomem *addr)
>  {
>   return readq_be(addr);
>  }
> @@ -139,15 +139,15 @@ EXPORT_SYMBOL(iowrite64be_hi_lo);
>   * FIXME! We could make these do EEH handling if we really
>   * wanted. Not clear if we do.
>   */
> -void ioread8_rep(void __iomem *addr, void *dst, unsigned long count)
> +void ioread8_rep(const void __iomem *addr, void *dst, unsigned long count)
>  {
>   readsb(addr, dst, count);
>  }
> -void ioread16_rep(void __iomem *addr, void *dst, unsigned long count)
> +void ioread16_rep(const void __iomem *addr, void *dst, unsigned long count)
>  {
>   readsw(addr, dst, count);
>  }
> -void ioread32_rep(void __iomem *addr, void *dst, unsigned long count)
> +void ioread32_rep(const void __iomem *addr, void *dst, unsigned long count)
>  {
>   readsl(addr, dst, count);
>  }

This looks OK to me.

Acked-by: Michael Ellerman  (powerpc)

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH -next] drivers/video/fbdev: remove set but not used variable 'size'

[Michael Ellerman]

Bartlomiej Zolnierkiewicz  writes:
> On 10/17/2018 01:52 AM, Michael Ellerman wrote:
>> YueHaibing  writes:
>> 
>>> Fixes gcc '-Wunused-but-set-variable' warning:
>>>
>>> drivers/video/fbdev/chipsfb.c: In function 'chipsfb_pci_init':
>>> drivers/video/fbdev/chipsfb.c:352:22: warning:
>>>  variable 'size' set but not used [-Wunused-but-set-variable]
>>>
>>> It not used any more after commit
>>> 8c8709334cec ("[PATCH] ppc32: Remove CONFIG_PMAC_PBOOK")
>>>
>>> Signed-off-by: YueHaibing 
>>> ---
>>>  drivers/video/fbdev/chipsfb.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>> 
>> Looks good, thanks.
>> 
>> Should I merge this? Or otherwise:
>> 
>> Acked-by: Michael Ellerman 
>
> I've queued the patch to fbdev tree for v5.1 w/ slightly modified patch
> summary ("fbdev: chipsfb: ...") add patch description (use Fixes: tag),
> thanks.

Thanks.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH] drm/ttm: force cached mappings for system RAM on ARM

[Michael Ellerman]

Hi Will,

Will Deacon  writes:
> [+ BenH and MPE]
>
> On Mon, Jan 14, 2019 at 07:21:08PM +, Koenig, Christian wrote:
>> Am 14.01.19 um 20:13 schrieb Will Deacon:
...
>
>> > The Arm architecture (and others including Power afaiu) doesn't
>> > guarantee coherency when memory is accessed using mismatched cacheability
>> > attributes.
...
>
>> As far as I know Power doesn't really supports un-cached memory at all, 
>> except for a very very old and odd configuration with AGP.
>
> Hopefully Michael/Ben can elaborate here, but I was under the (possibly
> mistaken) impression that mismatched attributes could cause a machine-check
> on Power.

That's what I've always been told, but I can't actually find where it's
documented, I'll keep searching.

But you're right that mixing cached / uncached is not really supported,
and probably results in a machine check or worse.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V2] mm: Replace all open encodings for NUMA_NO_NODE

[Michael Ellerman]

Anshuman Khandual  writes:
> At present there are multiple places where invalid node number is encoded
> as -1. Even though implicitly understood it is always better to have macros
> in there. Replace these open encodings for an invalid node number with the
> global macro NUMA_NO_NODE. This helps remove NUMA related assumptions like
> 'invalid node' from various places redirecting them to a common definition.
>
> Signed-off-by: Anshuman Khandual 
> ---
> Changes in V2:
>
> - Added inclusion of 'numa.h' header at various places per Andrew
> - Updated 'dev_to_node' to use NUMA_NO_NODE instead per Vinod
>
> Changes in V1: (https://lkml.org/lkml/2018/11/23/485)
>
> - Dropped OCFS2 changes per Joseph
> - Dropped media/video drivers changes per Hans
>
> RFC - https://patchwork.kernel.org/patch/10678035/
>
> Build tested this with multiple cross compiler options like alpha, sparc,
> arm64, x86, powerpc, powerpc64le etc with their default config which might
> not have compiled tested all driver related changes. I will appreciate
> folks giving this a test in their respective build environment.
>
> All these places for replacement were found by running the following grep
> patterns on the entire kernel code. Please let me know if this might have
> missed some instances. This might also have replaced some false positives.
> I will appreciate suggestions, inputs and review.
>
> 1. git grep "nid == -1"
> 2. git grep "node == -1"
> 3. git grep "nid = -1"
> 4. git grep "node = -1"
>
>  arch/alpha/include/asm/topology.h |  3 ++-
>  arch/ia64/kernel/numa.c   |  2 +-
>  arch/ia64/mm/discontig.c  |  6 +++---
>  arch/ia64/sn/kernel/io_common.c   |  3 ++-
>  arch/powerpc/include/asm/pci-bridge.h |  3 ++-
>  arch/powerpc/kernel/paca.c|  3 ++-
>  arch/powerpc/kernel/pci-common.c  |  3 ++-
>  arch/powerpc/mm/numa.c| 14 +++---
>  arch/powerpc/platforms/powernv/memtrace.c |  5 +++--

These powerpc changes all look fine.

Acked-by: Michael Ellerman 


cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Crash on FSL Book3E due to pte_pgprot()? (was Re: [PATCH v3 12/24] powerpc/mm: use pte helpers in generic code)

[Michael Ellerman]

Christophe Leroy  writes:

> Get rid of platform specific _PAGE_ in powerpc common code and
> use helpers instead.
>
> mm/dump_linuxpagetables.c will be handled separately
>
> Reviewed-by: Aneesh Kumar K.V 
> Signed-off-by: Christophe Leroy 
> ---
>  arch/powerpc/include/asm/book3s/32/pgtable.h |  9 +++--
>  arch/powerpc/include/asm/nohash/32/pgtable.h | 12 
>  arch/powerpc/include/asm/nohash/pgtable.h|  3 +--
>  arch/powerpc/mm/pgtable.c| 21 +++--
>  arch/powerpc/mm/pgtable_32.c | 15 ---
>  arch/powerpc/mm/pgtable_64.c | 14 +++---
>  arch/powerpc/xmon/xmon.c | 12 +++-
>  7 files changed, 41 insertions(+), 45 deletions(-)

So turns out this patch *also* breaks my p5020ds :)

Even with patch 4 merged, see next.

It's the same crash:

  pcieport 2000:00:00.0: AER enabled with IRQ 480
  Unable to handle kernel paging request for data at address 0x88008008
  Faulting instruction address: 0xc00192cc
  Oops: Kernel access of bad area, sig: 11 [#1]
  BE SMP NR_CPUS=24 CoreNet Generic
  Modules linked in:
  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc3-gcc7x-g98c847323b3a #1
  NIP:  c00192cc LR: c05d0f9c CTR: 0010
  REGS: c000f31bb400 TRAP: 0300   Not tainted  
(4.19.0-rc3-gcc7x-g98c847323b3a)
  MSR:  80029000   CR: 24000224  XER: 
  DEAR: 88008008 ESR: 0080 IRQMASK: 0 
  GPR00: c05d0f84 c000f31bb688 c117dc00 88008008 
  GPR04:  0040 0ffbff241010 c000f31b8000 
  GPR08:  0010  c12d4710 
  GPR12: 84000422 c12ff000 c0002774  
  GPR16:     
  GPR20:     
  GPR24:   88008008 c00089a8 
  GPR28: c000f3576400 c000f3576410 0040 c12ecc98 
  NIP [c00192cc] ._memset_io+0x6c/0x9c
  LR [c05d0f9c] .fsl_qman_probe+0x198/0x928
  Call Trace:
  [c000f31bb688] [c05d0f84] .fsl_qman_probe+0x180/0x928 (unreliable)
  [c000f31bb728] [c06432ec] .platform_drv_probe+0x60/0xb4
  [c000f31bb7a8] [c064083c] .really_probe+0x294/0x35c
  [c000f31bb848] [c0640d2c] .__driver_attach+0x148/0x14c
  [c000f31bb8d8] [c063d7dc] .bus_for_each_dev+0xb0/0x118
  [c000f31bb988] [c063ff28] .driver_attach+0x34/0x4c
  [c000f31bba08] [c063f648] .bus_add_driver+0x174/0x2bc
  [c000f31bbaa8] [c06418bc] .driver_register+0x90/0x180
  [c000f31bbb28] [c0643270] .__platform_driver_register+0x60/0x7c
  [c000f31bbba8] [c0ee2a70] .fsl_qman_driver_init+0x24/0x38
  [c000f31bbc18] [c00023fc] .do_one_initcall+0x64/0x2b8
  [c000f31bbcf8] [c0e9f480] .kernel_init_freeable+0x3a8/0x494
  [c000f31bbda8] [c0002798] .kernel_init+0x24/0x148
  [c000f31bbe28] [c9e8] .ret_from_kernel_thread+0x58/0x70
  Instruction dump:
  4e800020 2ba50003 40dd003c 3925fffc 5488402e 7929f082 7d082378 39290001 
  550a801e 7d2903a6 7d4a4378 794a0020 <9143> 38630004 4200fff8 70a50003 


Comparing a working vs broken kernel, it seems to boil down to the fact
that we're filtering out more PTE bits now that we use pte_pgprot() in
ioremap_prot().

With the old code we get:
  ioremap_prot: addr 0xff80 flags 0x241215
  ioremap_prot: addr 0xff80 flags 0x241215
  map_kernel_page: ea 0x88008008 pa 0xff80 pte 0xff800241215


And now we get:
  ioremap_prot: addr 0xff80 flags 0x241215 pte 0x241215
  ioremap_prot: addr 0xff80 pte 0x241215
  ioremap_prot: addr 0xff80 prot 0x241014
  map_kernel_page: ea 0x88008008 pa 0xff80 pte 0xff800241014

So we're losing 0x201, which for nohash book3e is:

  #define _PAGE_PRESENT 0x01 /* software: pte contains a translation */
  #define _PAGE_PSIZE_4K0x000200


I haven't worked out if it's one or both of those that matter.

The question is what's the right way to fix it? Should pte_pgprot() not
be filtering those bits out on book3e?

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH -next] drivers/video/fbdev: remove set but not used variable 'size'

[Michael Ellerman]

YueHaibing  writes:

> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/video/fbdev/chipsfb.c: In function 'chipsfb_pci_init':
> drivers/video/fbdev/chipsfb.c:352:22: warning:
>  variable 'size' set but not used [-Wunused-but-set-variable]
>
> It not used any more after commit
> 8c8709334cec ("[PATCH] ppc32: Remove CONFIG_PMAC_PBOOK")
>
> Signed-off-by: YueHaibing 
> ---
>  drivers/video/fbdev/chipsfb.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Looks good, thanks.

Should I merge this? Or otherwise:

Acked-by: Michael Ellerman 

cheers

> diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c
> index 40182ed..ca549e1 100644
> --- a/drivers/video/fbdev/chipsfb.c
> +++ b/drivers/video/fbdev/chipsfb.c
> @@ -349,7 +349,7 @@ static void init_chips(struct fb_info *p, unsigned long 
> addr)
>  static int chipsfb_pci_init(struct pci_dev *dp, const struct pci_device_id 
> *ent)
>  {
>   struct fb_info *p;
> - unsigned long addr, size;
> + unsigned long addr;
>   unsigned short cmd;
>   int rc = -ENODEV;
>  
> @@ -361,7 +361,6 @@ static int chipsfb_pci_init(struct pci_dev *dp, const 
> struct pci_device_id *ent)
>   if ((dp->resource[0].flags & IORESOURCE_MEM) == 0)
>   goto err_disable;
>   addr = pci_resource_start(dp, 0);
> - size = pci_resource_len(dp, 0);
>   if (addr == 0)
>   goto err_disable;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 06/24] powerpc/mm: properly set PAGE_KERNEL flags in ioremap()

[Michael Ellerman]

LEROY Christophe  writes:
> Michael Ellerman  a écrit :
>> Michael Ellerman  writes:
>>> Christophe Leroy  writes:
>>>
>>>> Set PAGE_KERNEL directly in the caller and do not rely on a
>>>> hack adding PAGE_KERNEL flags when _PAGE_PRESENT is not set.
>>>>
>>>> As already done for PPC64, use pgprot_cache() helpers instead of
>>>> _PAGE_XXX flags in PPC32 ioremap() derived functions.
>>>>
>>>> Signed-off-by: Christophe Leroy 
>>>
>>> Something in here is breaking my p5020ds (both 32-bit and 64-bit):
>>
>> Oh duh.
>>
>> That's because I didn't take patch 4.
>>
>> It didn't have any acks, but I guess I'll just merge it rather than
>> breaking things.
>
> Yes indeed. Maybe should I have followed it more carrefully to ensure  
> it gets an ack.

That's OK, I should have paid more attention to what it was actually
doing, I thought it was just a cleanup.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 06/24] powerpc/mm: properly set PAGE_KERNEL flags in ioremap()

[Michael Ellerman]

Christophe Leroy  writes:

> Set PAGE_KERNEL directly in the caller and do not rely on a
> hack adding PAGE_KERNEL flags when _PAGE_PRESENT is not set.
>
> As already done for PPC64, use pgprot_cache() helpers instead of
> _PAGE_XXX flags in PPC32 ioremap() derived functions.
>
> Signed-off-by: Christophe Leroy 

Something in here is breaking my p5020ds (both 32-bit and 64-bit):

# first bad commit: [cb9220cc18345a2a1ec287752e576a2c88fa4a2b] powerpc/mm: 
properly set PAGE_KERNEL flags in ioremap()

64-bit:

  io scheduler mq-deadline registered
  io scheduler kyber registered
  pcieport :00:00.0: AER enabled with IRQ 482
  pcieport 2000:00:00.0: AER enabled with IRQ 480
  Unable to handle kernel paging request for data at address 0x88008008
  Faulting instruction address: 0xc00152e4
  Oops: Kernel access of bad area, sig: 11 [#1]
  BE SMP NR_CPUS=32 CoreNet Generic
  Modules linked in:
  CPU: 1 PID: 1 Comm: swapper/0 Not tainted 
4.19.0-rc3-gcc-7.3.1-00135-gcb9220cc1834 #16
  NIP:  c00152e4 LR: c05173b8 CTR: 0010
  REGS: c000f30eb420 TRAP: 0300   Not tainted  
(4.19.0-rc3-gcc-7.3.1-00135-gcb9220cc1834)
  MSR:  80029000   CR: 24000224  XER: 
  DEAR: 88008008 ESR: 0080 IRQMASK: 0 
  GPR00: c05173a0 c000f30eb6a0 c0f86e00 88008008 
  GPR04:  0040 0ffbff04 0008 
  GPR08:  0010  0080 
  GPR12: 84000422 c00037c0 c000263c  
  GPR16:     
  GPR20:     
  GPR24: c0dbe0a0 c0d41bf8 88008008 c00089a8 
  GPR28: c000f3492400 c000f3492410 0040 c0ff0a18 
  NIP [c00152e4] ._memset_io+0x6c/0x9c
  LR [c05173b8] .fsl_qman_probe+0x188/0x918
  Call Trace:
  [c000f30eb6a0] [c05173a0] .fsl_qman_probe+0x170/0x918 (unreliable)
  [c000f30eb740] [c05797dc] .platform_drv_probe+0x58/0xac
  [c000f30eb7c0] [c05772b0] .really_probe+0x2a8/0x380
  [c000f30eb860] [c05776f0] .__driver_attach+0x138/0x13c
  [c000f30eb8f0] [c0574550] .bus_for_each_dev+0x9c/0x110
  [c000f30eb9a0] [c0576a18] .driver_attach+0x24/0x38
  [c000f30eba10] [c05761ec] .bus_add_driver+0x16c/0x2ac
  [c000f30ebab0] [c0578194] .driver_register+0x88/0x178
  [c000f30ebb30] [c0579770] .__platform_driver_register+0x48/0x5c
  [c000f30ebba0] [c0d85744] .fsl_qman_driver_init+0x1c/0x30
  [c000f30ebc10] [c0002374] .do_one_initcall+0x70/0x258
  [c000f30ebcf0] [c0d4a244] .kernel_init_freeable+0x340/0x43c
  [c000f30ebdb0] [c0002658] .kernel_init+0x1c/0x130
  [c000f30ebe30] [c9e4] .ret_from_kernel_thread+0x58/0x74
  Instruction dump:
  4e800020 2ba50003 40dd003c 3925fffc 5488402e 7929f082 7d082378 39290001 
  550a801e 7d2903a6 7d4a4378 794a0020 <9143> 38630004 4200fff8 70a50003 
  ---[ end trace 372a57fd67efb6fe ]---

32 bit:

  [1.076133] pcieport 2000:02:00.0: AER enabled with IRQ 480
  [1.082106] Unable to handle kernel paging request for data at address 
0xf110
  [1.089488] Faulting instruction address: 0xc0011c80
  [1.094437] Oops: Kernel access of bad area, sig: 11 [#1]
  [1.099816] BE SMP NR_CPUS=24 CoreNet Generic
  [1.104157] Modules linked in:
  [1.107197] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
4.19.0-rc3-gcc7x-g8f0c636b0542 #1
  [1.115181] NIP:  c0011c80 LR: c058f970 CTR: 0010
  [1.120216] REGS: e8107c80 TRAP: 0300   Not tainted  
(4.19.0-rc3-gcc7x-g8f0c636b0542)
  [1.128026] MSR:  00029002   CR: 24000284  XER: 
  [1.134190] DEAR: f110 ESR: 0080 
  [1.134190] GPR00: c058f958 e8107d30 e8108000 f110  0040 
e8107cd8 e8107d0c 
  [1.134190] GPR08:  0010  ff00 24000282  
c0003340  
  [1.134190] GPR16:     c0d64410 c0edb34c 
c0ec6700 0007 
  [1.134190] GPR24: c0ef f110 efffcab8 c0ef e8231c00 e8231c10 
0040003f c101d290 
  [1.171519] NIP [c0011c80] _memset_io+0x90/0xd0
  [1.176030] LR [c058f970] fsl_qman_probe+0x190/0x8c0
  [1.180975] Call Trace:
  [1.183410] [e8107d30] [c001f6c0] ioremap_prot+0x40/0x50 (unreliable)
  [1.189830] [e8107d40] [c058f958] fsl_qman_probe+0x178/0x8c0
  [1.195475] [e8107d70] [c0600894] platform_drv_probe+0x54/0xb0
  [1.201287] [e8107d90] [c05fe15c] really_probe+0x28c/0x350
  [1.206756] [e8107dc0] [c05fe73c] __driver_attach+0x12c/0x130
  [1.212485] [e8107de0] [c05fb5a8] bus_for_each_dev+0x98/0x110
  [1.218213] [e8107e10] [c05fd048] bus_add_driver+0x158/0x2b0
  [1.223855] [e8107e40] [c05ff1c4] 

Re: [v3,01/24] powerpc/32: Add ioremap_wt() and ioremap_coherent()

[Michael Ellerman]

On Tue, 2018-10-09 at 13:51:33 UTC, Christophe Leroy wrote:
> Other arches have ioremap_wt() to map IO areas write-through.
> Implement it on PPC as well in order to avoid drivers using
> __ioremap(_PAGE_WRITETHRU)
> 
> Also implement ioremap_coherent() to avoid drivers using
> __ioremap(_PAGE_COHERENT)
> 
> Signed-off-by: Christophe Leroy 

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/86c391bd5f47101acf1f3e0abd9fe0

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 06/24] powerpc/mm: properly set PAGE_KERNEL flags in ioremap()

[Michael Ellerman]

Michael Ellerman  writes:

> Christophe Leroy  writes:
>
>> Set PAGE_KERNEL directly in the caller and do not rely on a
>> hack adding PAGE_KERNEL flags when _PAGE_PRESENT is not set.
>>
>> As already done for PPC64, use pgprot_cache() helpers instead of
>> _PAGE_XXX flags in PPC32 ioremap() derived functions.
>>
>> Signed-off-by: Christophe Leroy 
>
> Something in here is breaking my p5020ds (both 32-bit and 64-bit):

Oh duh.

That's because I didn't take patch 4.

It didn't have any acks, but I guess I'll just merge it rather than
breaking things.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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])
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3, 3/6] dt: booting-without-of: DT fix s/#interrupt-cell/#interrupt-cells/

[Michael Ellerman]

On Fri, 2017-06-02 at 12:38:46 UTC, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven 
> Acked-by: Rob Herring 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/4be4119d1fbd93c44d5c639735c312

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3, 4/6] powerpc: dts: acadia: DT fix s/#interrupts-parent/#interrupt-parent/

[Michael Ellerman]

On Fri, 2017-06-02 at 12:38:47 UTC, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven 
> Acked-by: Rob Herring 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/3d2d4339cc326c427638daa67e264d

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] powerpc/asm/cacheflush: Cleanup cacheflush function params

[Michael Ellerman]

Geert Uytterhoeven <ge...@linux-m68k.org> writes:

> On Thu, Jul 20, 2017 at 1:43 PM, Michael Ellerman <m...@ellerman.id.au> wrote:
>> Matt Brown <matthew.brown@gmail.com> writes:
>>> The cacheflush prototypes currently use start and stop values and each
>>> call requires typecasting the address to an unsigned long.
>>> This patch changes the cacheflush prototypes to follow the x86 style of
>>> using a base and size values, with base being a void pointer.
>>>
>>> All callers of the cacheflush functions, including drivers, have been
>>> modified to conform to the new prototypes.
>>>
>>> The 64 bit cacheflush functions which were implemented in assembly code
>>> (flush_dcache_range, flush_inval_dcache_range) have been translated into
>>> C for readability and coherence.
>
>>> --- a/arch/powerpc/include/asm/cacheflush.h
>>> +++ b/arch/powerpc/include/asm/cacheflush.h
>>> @@ -51,13 +51,13 @@ static inline void __flush_dcache_icache_phys(unsigned 
>>> long physaddr)
>>>   * Write any modified data cache blocks out to memory and invalidate them.
>>>   * Does not invalidate the corresponding instruction cache blocks.
>>>   */
>>> -static inline void flush_dcache_range(unsigned long start, unsigned long 
>>> stop)
>>> +static inline void flush_dcache_range(void *start, unsigned long size)
>>>  {
>>> - void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
>>> - unsigned long size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 
>>> 1);
>>> + void *addr = (void *)((u32)start & ~(L1_CACHE_BYTES - 1));
>>
>> unsigned long would be nicer than u32.
>
> Indeed. That would make this work on ppc64, too.
> After which ppc64 has an identical copy (u64 = unsigned long on ppc64) below?

That was Matt's homework to notice that ;)

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] powerpc/asm/cacheflush: Cleanup cacheflush function params

[Michael Ellerman]

Hi Matt,

Thanks for tackling this mess.

Matt Brown  writes:
> The cacheflush prototypes currently use start and stop values and each
> call requires typecasting the address to an unsigned long.
> This patch changes the cacheflush prototypes to follow the x86 style of
> using a base and size values, with base being a void pointer.
>
> All callers of the cacheflush functions, including drivers, have been
> modified to conform to the new prototypes.
>
> The 64 bit cacheflush functions which were implemented in assembly code
> (flush_dcache_range, flush_inval_dcache_range) have been translated into
> C for readability and coherence.
>
> Signed-off-by: Matt Brown 
> ---
>  arch/powerpc/include/asm/cacheflush.h| 47 +
>  arch/powerpc/kernel/misc_64.S| 52 
> 
>  arch/powerpc/mm/dma-noncoherent.c| 15 
>  arch/powerpc/platforms/512x/mpc512x_shared.c | 10 +++---
>  arch/powerpc/platforms/85xx/smp.c|  6 ++--
>  arch/powerpc/sysdev/dart_iommu.c |  5 +--
>  drivers/ata/pata_bf54x.c |  3 +-
>  drivers/char/agp/uninorth-agp.c  |  6 ++--
>  drivers/gpu/drm/drm_cache.c  |  3 +-
>  drivers/macintosh/smu.c  | 15 
>  drivers/mmc/host/bfin_sdh.c  |  3 +-
>  drivers/mtd/nand/bf5xx_nand.c|  6 ++--
>  drivers/soc/fsl/qbman/dpaa_sys.h |  2 +-
>  drivers/soc/fsl/qbman/qman_ccsr.c|  3 +-
>  drivers/spi/spi-bfin5xx.c| 10 +++---
>  drivers/tty/serial/mpsc.c| 46 
>  drivers/usb/musb/blackfin.c  |  6 ++--

I think you want to trim that to powerpc only drivers for now at least.

> diff --git a/arch/powerpc/include/asm/cacheflush.h 
> b/arch/powerpc/include/asm/cacheflush.h
> index 11843e3..b8f04c3 100644
> --- a/arch/powerpc/include/asm/cacheflush.h
> +++ b/arch/powerpc/include/asm/cacheflush.h
> @@ -51,13 +51,13 @@ static inline void __flush_dcache_icache_phys(unsigned 
> long physaddr)
>   * Write any modified data cache blocks out to memory and invalidate them.
>   * Does not invalidate the corresponding instruction cache blocks.
>   */
> -static inline void flush_dcache_range(unsigned long start, unsigned long 
> stop)
> +static inline void flush_dcache_range(void *start, unsigned long size)
>  {
> - void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
> - unsigned long size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
> + void *addr = (void *)((u32)start & ~(L1_CACHE_BYTES - 1));

unsigned long would be nicer than u32.

And ALIGN_DOWN() should work here I think.

> + unsigned long len = size + (L1_CACHE_BYTES - 1);

And ALIGN?

> @@ -83,22 +83,39 @@ static inline void clean_dcache_range(unsigned long 
> start, unsigned long stop)
>   * to invalidate the cache so the PPC core doesn't get stale data
>   * from the CPM (no cache snooping here :-).
>   */
> -static inline void invalidate_dcache_range(unsigned long start,
> -unsigned long stop)
> +static inline void invalidate_dcache_range(void *start, unsigned long size)
>  {
> - void *addr = (void *)(start & ~(L1_CACHE_BYTES - 1));
> - unsigned long size = stop - (unsigned long)addr + (L1_CACHE_BYTES - 1);
> + void *addr = (void *)((u32)start & ~(L1_CACHE_BYTES - 1));
> + unsigned long len = size + (L1_CACHE_SHIFT - 1);
>   unsigned long i;
>  
> - for (i = 0; i < size >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
> + for (i = 0; i < len >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
>   dcbi(addr);
>   mb();   /* sync */
>  }
>  
>  #endif /* CONFIG_PPC32 */
>  #ifdef CONFIG_PPC64
> -extern void flush_dcache_range(unsigned long start, unsigned long stop);
> -extern void flush_inval_dcache_range(unsigned long start, unsigned long 
> stop);
> +static inline void flush_dcache_range(void *start, unsigned long size)
> +{
> + void *addr = (void *)((u64)start & ~(L1_CACHE_BYTES - 1));
> + unsigned long len = size + (L1_CACHE_BYTES - 1);
> + unsigned long i;
> +
> + for (i = 0; i < len >> L1_CACHE_SHIFT; i++, addr += L1_CACHE_BYTES)
> + dcbf(addr);
> + mb();   /* sync */
> +}

I'd probably prefer a precursor patch to do the asm -> C conversion, but
I guess that's a pain because then you have to implement both the old
and new logic in C.

Also L1_CACHE_SHIFT is not necessarily == DCACHEL1BLOCKSIZE.

Finally it would be good to see what code the compiler generates out of
this, and see how it compares to the asm version. Not because it's
particularly performance critical (hopefully) but just so we know.

> diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c 
> b/arch/powerpc/platforms/512x/mpc512x_shared.c
> index 6b4f4cb..0f3a7d9 100644
> --- 

Re: [PATCH 21/44] powerpc: implement ->mapping_error

[Michael Ellerman]

Christoph Hellwig <h...@lst.de> writes:

> DMA_ERROR_CODE is going to go away, so don't rely on it.  Instead
> define a ->mapping_error method for all IOMMU based dma operation
> instances.  The direct ops don't ever return an error and don't
> need a ->mapping_error method.
>
> Signed-off-by: Christoph Hellwig <h...@lst.de>
> ---
>  arch/powerpc/include/asm/dma-mapping.h |  4 
>  arch/powerpc/include/asm/iommu.h   |  4 
>  arch/powerpc/kernel/dma-iommu.c|  6 ++
>  arch/powerpc/kernel/iommu.c| 28 ++--
>  arch/powerpc/platforms/cell/iommu.c|  1 +
>  arch/powerpc/platforms/pseries/vio.c   |  3 ++-
>  6 files changed, 27 insertions(+), 19 deletions(-)

I also see:

  arch/powerpc/kernel/dma.c:const struct dma_map_ops dma_direct_ops = {

Which you mentioned can't fail.

  arch/powerpc/platforms/pseries/ibmebus.c:static const struct dma_map_ops 
ibmebus_dma_ops = {

Which can't fail.

And:

  arch/powerpc/platforms/powernv/npu-dma.c:static const struct dma_map_ops 
dma_npu_ops = {
  arch/powerpc/platforms/ps3/system-bus.c:static const struct dma_map_ops 
ps3_sb_dma_ops = {
  arch/powerpc/platforms/ps3/system-bus.c:static const struct dma_map_ops 
ps3_ioc0_dma_ops = {

All of which look like they definitely can fail, but return 0 on error
and don't implement ->mapping_error.

So I guess I'm acking this and adding a TODO to fix up the NPU code at
least, the ps3 code is probably better left alone these days.

Acked-by: Michael Ellerman <m...@ellerman.id.au>

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/10] mm: replace access_process_vm() write parameter with gup_flags

[Michael Ellerman]

Lorenzo Stoakes  writes:

> diff --git a/arch/powerpc/kernel/ptrace32.c b/arch/powerpc/kernel/ptrace32.c
> index f52b7db3..010b7b3 100644
> --- a/arch/powerpc/kernel/ptrace32.c
> +++ b/arch/powerpc/kernel/ptrace32.c
> @@ -74,7 +74,7 @@ long compat_arch_ptrace(struct task_struct *child, 
> compat_long_t request,
>   break;
>  
>   copied = access_process_vm(child, (u64)addrOthers, ,
> - sizeof(tmp), 0);
> + sizeof(tmp), FOLL_FORCE);
>   if (copied != sizeof(tmp))
>   break;
>   ret = put_user(tmp, (u32 __user *)data);

LGTM.

> @@ -179,7 +179,8 @@ long compat_arch_ptrace(struct task_struct *child, 
> compat_long_t request,
>   break;
>   ret = 0;
>   if (access_process_vm(child, (u64)addrOthers, ,
> - sizeof(tmp), 1) == sizeof(tmp))
> + sizeof(tmp),
> + FOLL_FORCE | FOLL_WRITE) == sizeof(tmp))
>   break;
>   ret = -EIO;
>   break;

If you're respinning this anyway, can you format that as:

if (access_process_vm(child, (u64)addrOthers, , sizeof(tmp),
  FOLL_FORCE | FOLL_WRITE) == sizeof(tmp))
break;

I realise you probably deliberately didn't do that to make the diff clearer.

Either way:

Acked-by: Michael Ellerman  (powerpc)


cheers

[PATCH 1/7] selftest: sync: basic tests for sw_sync framework

[Michael Ellerman]

Emilio López  writes:
> El 22/09/16 a las 06:43, Michael Ellerman escribió:
>> Emilio López  writes:
>>
>> Please don't include the *kernel* headers, they're really not meant to
>> be used in userspace programs :)
>>
>>> +CFLAGS += -I../../../../usr/include/
>>
>> That is the correct place to get them from. They'll have been put there
>> by 'make headers_install'.
>
> My inspiration here has been tools/testing/selftests/memfd/Makefile, 
> which does it this way. If I only include the ones on usr then it 
> doesn't build, as there's no sync_file.h available, even after running 
> make headers_install. How am I supposed to use the ioctls from there?

It looks like it's missing from include/uapi/linux/Kbuild, you need to
add it to the list of exported headers:

diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index dd604395606b..40411b4ff012 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -397,6 +397,7 @@ header-y += stddef.h
 header-y += string.h
 header-y += suspend_ioctls.h
 header-y += swab.h
+header-y += sync_file.h
 header-y += synclink.h
 header-y += sysctl.h
 header-y += sysinfo.h


cheers

[PATCH 1/7] selftest: sync: basic tests for sw_sync framework

[Michael Ellerman]

Emilio López  writes:

> These tests are based on the libsync test suite from Android.
> This commit lays the ground for future tests, as well as includes
> tests for a variety of basic allocation commands.

Hi Emilio,

Just a few comments on the Makefile.

> diff --git a/tools/testing/selftests/sync/Makefile 
> b/tools/testing/selftests/sync/Makefile
> new file mode 100644
> index 000..f67827f
> --- /dev/null
> +++ b/tools/testing/selftests/sync/Makefile
> @@ -0,0 +1,24 @@
> +CC = $(CROSS_COMPILE)gcc

lib.mk does that for you.

> +CFLAGS := -O2 -g -std=gnu89 -pthread -Wall -Wextra

It's more polite to just add to CFLAGS rather than defining it from
scratch. That way a user can add CFLAGS via setting them in the
environment.

ie. this would be:

CFLAGS += -O2 -g -std=gnu89 -pthread -Wall -Wextra

or:

CFLAGS := -O2 -g -std=gnu89 -pthread -Wall -Wextra $(CFLAGS)


> +CFLAGS += -I../../../../include/uapi/

Please don't include the unprocessed uapi headers, they are not meant to
be directly included in userspace programs.

They even say so:

include/uapi/linux/types.h:#warning "Attempt to use kernel headers from user 
space, see http://kernelnewbies.org/KernelHeaders;

> +CFLAGS += -I../../../../include/

Please don't include the *kernel* headers, they're really not meant to
be used in userspace programs :)

> +CFLAGS += -I../../../../usr/include/

That is the correct place to get them from. They'll have been put there
by 'make headers_install'.

> +CFLAGS += -I../../../../drivers/dma-buf/

That's also a bit fishy.

> +LDFLAGS += -pthread
> +
> +TEST_PROGS = sync_test
> +
> +all: $(TEST_PROGS)
> +
> +include ../lib.mk
> +
> +SRC = sync_test.o sync.o

SRC would usually point to .c files, .o's would be in OBJS or something
similar. Though it's not that important obviously.

Just listing the .c files in SRC should work, make knows how to turn
them into .o's for you.

> +TESTS += sync_alloc.o

And similarly there.

So I think you could just use:

SRC := sync_test.c sync.c sync_alloc.c

Or if you actually just want to list all the .c files in the directory
then this would also work:

SRC := $(wildcard *.c)

> +sync_test: $(SRC) $(TESTS)
> +
> +.PHONY: clean

lib.mk did that for you.

> +
> +clean:
> + $(RM) sync_test $(SRC) $(TESTS)

If you redefined SRC above to be the actual sources then you obviously
don't want to clean them, so here you would just use *.o.

cheers

[PATCH] drivers: depend on instead of select BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO

[Michael Ellerman]

On Wed, 2014-10-29 at 09:54 +0200, Jani Nikula wrote:
> On Wed, 29 Oct 2014, Michael Ellerman  wrote:
> > On Tue, 2014-10-28 at 13:29 -0700, Randy Dunlap wrote:
> >> On 10/27/14 06:13, Tomi Valkeinen wrote:
> >> > I also think the 'depends on BACKLIGHT_CLASS_DEVICE ||
> >> > BACKLIGHT_CLASS_DEVICE=n' pattern is quite... interesting (i.e. sounds
> >> > like a hack to me =).
> >> 
> >> It does exactly what is needed and it is used in many places in kernel
> >> Kconfig files.
> >
> > Is there any reason you can't do:
> >
> >   depends on BACKLIGHT_CLASS_DEVICE != m
> 
> That's not the same thing. The FOO || FOO=n allows for all options, but
> forbids it being a module when the option depending on it is
> built-in.

OK right. Because "BAR depends on FOO" is short for "depends on FOO=y || FOO=m",
but also adds the implicit condition that if FOO=m then BAR must also be m.

Thanks for clueing me in.

cheers

[PATCH] drivers: depend on instead of select BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO

[Michael Ellerman]

On Tue, 2014-10-28 at 13:29 -0700, Randy Dunlap wrote:
> On 10/27/14 06:13, Tomi Valkeinen wrote:
> > I also think the 'depends on BACKLIGHT_CLASS_DEVICE ||
> > BACKLIGHT_CLASS_DEVICE=n' pattern is quite... interesting (i.e. sounds
> > like a hack to me =).
> 
> It does exactly what is needed and it is used in many places in kernel
> Kconfig files.

Is there any reason you can't do:

  depends on BACKLIGHT_CLASS_DEVICE != m

cheers

Re: [PATCHv3 1/2] ppc64: perform proper max_bus_speed detection

[Michael Ellerman]

On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote:
 On pseries machines the detection for max_bus_speed should be done
 through an OpenFirmware property. This patch adds a function to perform this
 detection and a hook to perform dynamic adding of the function only for
 pseries.

This fails to build for me on ppc64_defconfig, with:

arch/powerpc/include/asm/machdep.h:111:5: error: 'struct pci_host_bridge' 
declared inside parameter list [-Werror]


Presumably you tested it using some other defconfig?

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCHv3 1/2] ppc64: perform proper max_bus_speed detection

[Michael Ellerman]

On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote:
> On pseries machines the detection for max_bus_speed should be done
> through an OpenFirmware property. This patch adds a function to perform this
> detection and a hook to perform dynamic adding of the function only for
> pseries.

This fails to build for me on ppc64_defconfig, with:

arch/powerpc/include/asm/machdep.h:111:5: error: 'struct pci_host_bridge' 
declared inside parameter list [-Werror]


Presumably you tested it using some other defconfig?

cheers

[PATCHv3 1/2] ppc64: perform proper max_bus_speed detection

[Michael Ellerman]

On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote:
> On pseries machines the detection for max_bus_speed should be done
> through an OpenFirmware property. This patch adds a function to perform this
> detection and a hook to perform dynamic adding of the function only for
> pseries.

The crucial detail you didn't mention is that pcibios_root_bridge_prepare()
already exists as a weak function in the PCI code and is called from
pci_create_root_bus().

> diff --git a/arch/powerpc/platforms/pseries/setup.c 
> b/arch/powerpc/platforms/pseries/setup.c
> index 8bcc9ca..15796b5 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -430,6 +430,8 @@ static void pSeries_machine_kexec(struct kimage *image)
>  }
>  #endif
>  
> +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge);
> +

Don't do that, put it in a header where it belongs.

cheers

Re: [PATCHv3 1/2] ppc64: perform proper max_bus_speed detection

[Michael Ellerman]

On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote:
 On pseries machines the detection for max_bus_speed should be done
 through an OpenFirmware property. This patch adds a function to perform this
 detection and a hook to perform dynamic adding of the function only for
 pseries.

The crucial detail you didn't mention is that pcibios_root_bridge_prepare()
already exists as a weak function in the PCI code and is called from
pci_create_root_bus().

 diff --git a/arch/powerpc/platforms/pseries/setup.c 
 b/arch/powerpc/platforms/pseries/setup.c
 index 8bcc9ca..15796b5 100644
 --- a/arch/powerpc/platforms/pseries/setup.c
 +++ b/arch/powerpc/platforms/pseries/setup.c
 @@ -430,6 +430,8 @@ static void pSeries_machine_kexec(struct kimage *image)
  }
  #endif
  
 +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge);
 +

Don't do that, put it in a header where it belongs.

cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel