LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Bug 204371] BUG kmalloc-4k (Tainted: G        W        ): Object padding overwritten
From: bugzilla-daemon @ 2019-08-15 17:56 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-204371-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=204371

--- Comment #30 from Erhard F. (erhard_f@mailbox.org) ---
On Thu, 15 Aug 2019 17:11:36 +0000
bugzilla-daemon@bugzilla.kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=204371
> 
> --- Comment #29 from Christophe Leroy (christophe.leroy@c-s.fr) ---
> Looks good. Does it work better ?
Had some trouble getting the dmesg. With these modifications the btrfs module
hiccups as soon as it gets loaded, during btrfs selftests:

[...]
[  167.258266] Btrfs loaded, crc32c=crc32c-generic, debug=on
[  167.259388] BTRFS: selftest: sectorsize: 4096  nodesize: 4096
[  167.259602] BTRFS: selftest: running btrfs free space cache tests
[  167.259943] BTRFS: selftest: running extent only tests
[  167.260201] BTRFS: selftest: running bitmap only tests
[  167.260501] BTRFS: selftest: running bitmap and extent tests
[  167.260963] WARNING: CPU: 0 PID: 266 at mm/slub.c:1846
___slab_alloc.constprop.75+0x2ac/0x380
[  167.261277] Modules linked in: btrfs(+) auth_rpcgss nfsv4 dns_resolver nfs
lockd grace sunrpc input_leds joydev b43legacy led_class hid_generic mac80211
usbhid hid snd_aoa_codec_tas snd_aoa_fabric_layout snd_aoa cfg80211 rfkill
libarc4 evdev ohci_pci xor zstd_decompress zstd_compress zlib_deflate radeon
raid6_pq zlib_inflate therm_windtunnel ehci_pci ohci_hcd hwmon i2c_algo_bit
ehci_hcd backlight drm_kms_helper sungem firewire_ohci syscopyarea sungem_phy
sysfillrect sr_mod firewire_core sysimgblt fb_sys_fops cdrom crc_itu_t
snd_aoa_i2sbus snd_aoa_soundbus ttm snd_pcm usbcore snd_timer drm usb_common
snd soundcore ssb uninorth_agp drm_panel_orientation_quirks agpgart lzo
lzo_compress lzo_decompress zram zsmalloc
[  167.263795] CPU: 0 PID: 266 Comm: modprobe Tainted: G        W        
5.3.0-rc4+ #3
[  167.264074] NIP:  c0196ddc LR: c0196dd4 CTR: c019711c
[  167.264236] REGS: ecde9a70 TRAP: 0700   Tainted: G        W         
(5.3.0-rc4+)
[  167.264488] MSR:  00021032 <ME,IR,DR,RI>  CR: 28224222  XER: 00000000
[  167.264709] 
               GPR00: c0196dd4 ecde9b28 eb353380 00000000 ef3d3600 0003000e
00000000 0007000f 
               GPR08: 00000001 00000001 ef42488c ecde9b28 48244222 00a9eff4
00a64e74 00000004 
               GPR16: 00000000 ee800810 00000000 c0800000 00000000 ee800824
f203de58 c07fea34 
               GPR24: 00210d00 ef424888 00000d40 ee800800 ef3d3600 ee8032e0
00000000 eedb1af8 
[  167.265857] NIP [c0196ddc] ___slab_alloc.constprop.75+0x2ac/0x380
[  167.266052] LR [c0196dd4] ___slab_alloc.constprop.75+0x2a4/0x380
[  167.266248] Call Trace:
[  167.266308] [ecde9b28] [c0196dd4] ___slab_alloc.constprop.75+0x2a4/0x380
(unreliable)
[  167.266567] [ecde9bb8] [c0196ef0] __slab_alloc.constprop.74+0x40/0x6c
[  167.266776] [ecde9be8] [c0197198] kmem_cache_alloc_trace+0x7c/0x1a0
[  167.267321] [ecde9c28] [f203de58] test_add_free_space_entry+0xf0/0x214
[btrfs]
[  167.267672] [ecde9c78] [f207e238] btrfs_test_free_space_cache+0x918/0x1308
[btrfs]
[  167.268012] [ecde9cd8] [f207ad3c] btrfs_run_sanity_tests+0x8c/0x144 [btrfs]
[  167.268327] [ecde9d08] [f1678cd0] init_btrfs_fs+0xd4/0x12c [btrfs]
[  167.268536] [ecde9d28] [c00052cc] do_one_initcall+0x54/0x288
[  167.282204] [ecde9d98] [c00bebf8] do_init_module+0x60/0x1dc
[  167.295710] [ecde9dc8] [c00c0ad4] load_module+0x1ca4/0x1e18
[  167.308943] [ecde9ea8] [c00c0df4] sys_finit_module+0x98/0xb8
[  167.322086] [ecde9f38] [c0014274] ret_from_syscall+0x0/0x34
[  167.335151] --- interrupt: c01 at 0x8ed2c4
                   LR = 0xa757c4
[  167.361356] Instruction dump:
[  167.374279] 7e048378 7f83e378 60e70001 90e10030 4bffda0d 2f830000 41beff20
7f84e378 
[  167.387497] 7f63db78 4bffdd35 7e090034 5529d97e <0f090000> 2f900000 41beff00
7e527a14 
[  167.400820] irq event stamp: 0
[  167.414008] hardirqs last  enabled at (0): [<00000000>] 0x0
[  167.427196] hardirqs last disabled at (0): [<c0032f0c>]
copy_process+0x474/0x1368
[  167.440311] softirqs last  enabled at (0): [<c0032f0c>]
copy_process+0x474/0x1368
[  167.453198] softirqs last disabled at (0): [<00000000>] 0x0
[  167.465906] ---[ end trace 74450a6aa18e595d ]---
[  167.481091] BTRFS: selftest: running space stealing from bitmap to extent
tests
[  167.496220] BTRFS: selftest: running extent buffer operation tests
[  167.510905] BTRFS: selftest: running btrfs_split_item tests
[  167.525961] BTRFS: selftest: running extent I/O tests
[  167.540569] BTRFS: selftest: running find delalloc tests
[  168.104731] BTRFS: selftest: running find_first_clear_extent_bit test
[  168.119119] BTRFS: selftest: running extent buffer bitmap tests
[  168.463591] BTRFS: selftest: running inode tests
[  168.477316] BTRFS: selftest: running btrfs_get_extent tests
[  168.491784] BTRFS: selftest: running hole first btrfs_get_extent test
[  168.506234] BTRFS: selftest: running outstanding_extents tests
[  168.520925] BTRFS: selftest: running qgroup tests
[  168.535039] BTRFS: selftest: running qgroup add/remove tests
[  168.549344] BTRFS: selftest: running qgroup multiple refs test
[  168.563713] BTRFS: selftest: running free space tree tests
[  168.706099] BTRFS: selftest: sectorsize: 4096  nodesize: 8192
[  168.718674] BTRFS: selftest: running btrfs free space cache tests
[  168.731349] BTRFS: selftest: running extent only tests
[  168.744380] BTRFS: selftest: running bitmap only tests
[  168.757505] BTRFS: selftest: running bitmap and extent tests
[  168.770615] WARNING: CPU: 0 PID: 266 at mm/slub.c:1846
___slab_alloc.constprop.75+0x2ac/0x380
[  168.783738] Modules linked in: btrfs(+) auth_rpcgss nfsv4 dns_resolver nfs
lockd grace sunrpc input_leds joydev b43legacy led_class hid_generic mac80211
usbhid hid snd_aoa_codec_tas snd_aoa_fabric_layout snd_aoa cfg80211 rfkill
libarc4 evdev ohci_pci xor zstd_decompress zstd_compress zlib_deflate radeon
raid6_pq zlib_inflate therm_windtunnel ehci_pci ohci_hcd hwmon i2c_algo_bit
ehci_hcd backlight drm_kms_helper sungem firewire_ohci syscopyarea sungem_phy
sysfillrect sr_mod firewire_core sysimgblt fb_sys_fops cdrom crc_itu_t
snd_aoa_i2sbus snd_aoa_soundbus ttm snd_pcm usbcore snd_timer drm usb_common
snd soundcore ssb uninorth_agp drm_panel_orientation_quirks agpgart lzo
lzo_compress lzo_decompress zram zsmalloc
[  168.828086] CPU: 0 PID: 266 Comm: modprobe Tainted: G        W        
5.3.0-rc4+ #3
[  168.843577] NIP:  c0196ddc LR: c0196dd4 CTR: c019711c
[  168.859054] REGS: ecde9a70 TRAP: 0700   Tainted: G        W         
(5.3.0-rc4+)
[  168.874772] MSR:  00021032 <ME,IR,DR,RI>  CR: 28224242  XER: 00000000
[  168.890402] 
               GPR00: c0196dd4 ecde9b28 eb353380 00000000 ef3d3960 0001000e
00000000 0007000f 
               GPR08: 00000001 00000001 ef432578 ecde9b28 48244242 00a9eff4
00a64e74 00000006 
               GPR16: 00000000 ee800810 00000000 c0800000 00000000 ee800824
f203de58 c07fea34 
               GPR24: 00210d00 ef432574 00000d40 ee800800 ef3d3960 ee8032e0
00000000 eedb1af8 
[  168.960711] NIP [c0196ddc] ___slab_alloc.constprop.75+0x2ac/0x380
[  168.974148] LR [c0196dd4] ___slab_alloc.constprop.75+0x2a4/0x380
[  168.987536] Call Trace:
[  169.000772] [ecde9b28] [c0196dd4] ___slab_alloc.constprop.75+0x2a4/0x380
(unreliable)
[  169.014236] [ecde9bb8] [c0196ef0] __slab_alloc.constprop.74+0x40/0x6c
[  169.027634] [ecde9be8] [c0197198] kmem_cache_alloc_trace+0x7c/0x1a0
[  169.041326] [ecde9c28] [f203de58] test_add_free_space_entry+0xf0/0x214
[btrfs]
[  169.054891] [ecde9c78] [f207dee4] btrfs_test_free_space_cache+0x5c4/0x1308
[btrfs]
[  169.068328] [ecde9cd8] [f207ad3c] btrfs_run_sanity_tests+0x8c/0x144 [btrfs]
[  169.081776] [ecde9d08] [f1678cd0] init_btrfs_fs+0xd4/0x12c [btrfs]
[  169.095039] [ecde9d28] [c00052cc] do_one_initcall+0x54/0x288
[  169.108161] [ecde9d98] [c00bebf8] do_init_module+0x60/0x1dc
[  169.121179] [ecde9dc8] [c00c0ad4] load_module+0x1ca4/0x1e18
[  169.134179] [ecde9ea8] [c00c0df4] sys_finit_module+0x98/0xb8
[  169.147133] [ecde9f38] [c0014274] ret_from_syscall+0x0/0x34
[  169.159933] --- interrupt: c01 at 0x8ed2c4
                   LR = 0xa757c4
[  169.185350] Instruction dump:
[  169.197936] 7e048378 7f83e378 60e70001 90e10030 4bffda0d 2f830000 41beff20
7f84e378 
[  169.210814] 7f63db78 4bffdd35 7e090034 5529d97e <0f090000> 2f900000 41beff00
7e527a14 
[  169.223824] irq event stamp: 0
[  169.236770] hardirqs last  enabled at (0): [<00000000>] 0x0
[  169.249906] hardirqs last disabled at (0): [<c0032f0c>]
copy_process+0x474/0x1368
[  169.263077] softirqs last  enabled at (0): [<c0032f0c>]
copy_process+0x474/0x1368
[  169.276147] softirqs last disabled at (0): [<00000000>] 0x0
[  169.289173] ---[ end trace 74450a6aa18e595e ]---
[  169.305352] WARNING: CPU: 0 PID: 266 at mm/slub.c:1846
___slab_alloc.constprop.75+0x2ac/0x380
[  169.318278] Modules linked in: btrfs(+) auth_rpcgss nfsv4 dns_resolver nfs
lockd grace sunrpc input_leds joydev b43legacy led_class hid_generic mac80211
usbhid hid snd_aoa_codec_tas snd_aoa_fabric_layout snd_aoa cfg80211 rfkill
libarc4 evdev ohci_pci xor zstd_decompress zstd_compress zlib_deflate radeon
raid6_pq zlib_inflate therm_windtunnel ehci_pci ohci_hcd hwmon i2c_algo_bit
ehci_hcd backlight drm_kms_helper sungem firewire_ohci syscopyarea sungem_phy
sysfillrect sr_mod firewire_core sysimgblt fb_sys_fops cdrom crc_itu_t
snd_aoa_i2sbus snd_aoa_soundbus ttm snd_pcm usbcore snd_timer drm usb_common
snd soundcore ssb uninorth_agp drm_panel_orientation_quirks agpgart lzo
lzo_compress lzo_decompress zram zsmalloc
[  169.363085] CPU: 0 PID: 266 Comm: modprobe Tainted: G        W        
5.3.0-rc4+ #3
[  169.378753] NIP:  c0196ddc LR: c0196dd4 CTR: c019711c
[  169.394382] REGS: ecde9a70 TRAP: 0700   Tainted: G        W         
(5.3.0-rc4+)
[  169.410156] MSR:  00021032 <ME,IR,DR,RI>  CR: 28224222  XER: 00000000
[  169.425900] 
               GPR00: c0196dd4 ecde9b28 eb353380 00000000 ef3d3ba0 0001000e
00000000 0007000f 
               GPR08: 00000001 00000001 ef188f3c ecde9b28 48244222 00a9eff4
00a64e74 00000006 
               GPR16: 00000000 ee800810 00000000 c0800000 00000000 ee800824
f203de58 c07fea34 
               GPR24: 00210d00 ef188f38 00000d40 ee800800 ef3d3ba0 ee8032e0
00000000 eedb1af8 
[  169.506070] NIP [c0196ddc] ___slab_alloc.constprop.75+0x2ac/0x380
[  169.522457] LR [c0196dd4] ___slab_alloc.constprop.75+0x2a4/0x380
[  169.538919] Call Trace:
[  169.555301] [ecde9b28] [c0196dd4] ___slab_alloc.constprop.75+0x2a4/0x380
(unreliable)
[  169.572079] [ecde9bb8] [c0196ef0] __slab_alloc.constprop.74+0x40/0x6c
[  169.588847] [ecde9be8] [c0197198] kmem_cache_alloc_trace+0x7c/0x1a0
[  169.605861] [ecde9c28] [f203de58] test_add_free_space_entry+0xf0/0x214
[btrfs]
[  169.623013] [ecde9c78] [f207e08c] btrfs_test_free_space_cache+0x76c/0x1308
[btrfs]
[  169.640288] [ecde9cd8] [f207ad3c] btrfs_run_sanity_tests+0x8c/0x144 [btrfs]
[  169.657589] [ecde9d08] [f1678cd0] init_btrfs_fs+0xd4/0x12c [btrfs]
[  169.674812] [ecde9d28] [c00052cc] do_one_initcall+0x54/0x288
[  169.692019] [ecde9d98] [c00bebf8] do_init_module+0x60/0x1dc
[  169.709242] [ecde9dc8] [c00c0ad4] load_module+0x1ca4/0x1e18
[  169.726533] [ecde9ea8] [c00c0df4] sys_finit_module+0x98/0xb8
[  169.743904] [ecde9f38] [c0014274] ret_from_syscall+0x0/0x34
[  169.761110] --- interrupt: c01 at 0x8ed2c4
                   LR = 0xa757c4
[  169.793422] Instruction dump:
[  169.808483] 7e048378 7f83e378 60e70001 90e10030 4bffda0d 2f830000 41beff20
7f84e378 
[  169.823241] 7f63db78 4bffdd35 7e090034 5529d97e <0f090000> 2f900000 41beff00
7e527a14 
[  169.838004] irq event stamp: 0
[  169.852643] hardirqs last  enabled at (0): [<00000000>] 0x0
[  169.867316] hardirqs last disabled at (0): [<c0032f0c>]
copy_process+0x474/0x1368
[  169.882061] softirqs last  enabled at (0): [<c0032f0c>]
copy_process+0x474/0x1368
[  169.896525] softirqs last disabled at (0): [<00000000>] 0x0
[  169.910840] ---[ end trace 74450a6aa18e595f ]---
[  169.928722] BTRFS: selftest: running space stealing from bitmap to extent
tests
[  169.943052] WARNING: CPU: 1 PID: 266 at mm/slub.c:1846
___slab_alloc.constprop.75+0x2ac/0x380
[  169.957190] Modules linked in: btrfs(+) auth_rpcgss nfsv4 dns_resolver nfs
lockd grace sunrpc input_leds joydev b43legacy led_class hid_generic mac80211
usbhid hid snd_aoa_codec_tas snd_aoa_fabric_layout snd_aoa cfg80211 rfkill
libarc4 evdev ohci_pci xor zstd_decompress zstd_compress zlib_deflate radeon
raid6_pq zlib_inflate therm_windtunnel ehci_pci ohci_hcd hwmon i2c_algo_bit
ehci_hcd backlight drm_kms_helper sungem firewire_ohci syscopyarea sungem_phy
sysfillrect sr_mod firewire_core sysimgblt fb_sys_fops cdrom crc_itu_t
snd_aoa_i2sbus snd_aoa_soundbus ttm snd_pcm usbcore snd_timer drm usb_common
snd soundcore ssb uninorth_agp drm_panel_orientation_quirks agpgart lzo
lzo_compress lzo_decompress zram zsmalloc
[  170.003934] CPU: 1 PID: 266 Comm: modprobe Tainted: G        W        
5.3.0-rc4+ #3
[  170.019968] NIP:  c0196ddc LR: c0196dd4 CTR: c019711c
[  170.035984] REGS: ecde9a70 TRAP: 0700   Tainted: G        W         
(5.3.0-rc4+)
[  170.052020] MSR:  00021032 <ME,IR,DR,RI>  CR: 28224242  XER: 00000000
[  170.068087] 
               GPR00: c0196dd4 ecde9b28 eb353380 00000000 ef3d3cc0 0004000e
00000000 0007000f 
               GPR08: 00000001 00000001 ef188eac ecde9b28 48244242 00a9eff4
00a64e74 00000003 
               GPR16: 00000000 ee800810 00000000 c0800000 00000000 ee800824
f203de58 c07fea34 
               GPR24: 00210d00 ef188ea8 00000d40 ee800800 ef3d3cc0 ee8032e0
00000000 eedceaf8 
[  170.149103] NIP [c0196ddc] ___slab_alloc.constprop.75+0x2ac/0x380
[  170.165571] LR [c0196dd4] ___slab_alloc.constprop.75+0x2a4/0x380
[  170.182082] Call Trace:
[  170.198482] [ecde9b28] [c0196dd4] ___slab_alloc.constprop.75+0x2a4/0x380
(unreliable)
[  170.215245] [ecde9bb8] [c0196ef0] __slab_alloc.constprop.74+0x40/0x6c
[  170.232091] [ecde9be8] [c0197198] kmem_cache_alloc_trace+0x7c/0x1a0
[  170.249175] [ecde9c28] [f203de58] test_add_free_space_entry+0xf0/0x214
[btrfs]
[  170.266059] [ecde9c78] [f207e44c] btrfs_test_free_space_cache+0xb2c/0x1308
[btrfs]
[  170.282918] [ecde9cd8] [f207ad3c] btrfs_run_sanity_tests+0x8c/0x144 [btrfs]
[  170.299700] [ecde9d08] [f1678cd0] init_btrfs_fs+0xd4/0x12c [btrfs]
[  170.316388] [ecde9d28] [c00052cc] do_one_initcall+0x54/0x288
[  170.333159] [ecde9d98] [c00bebf8] do_init_module+0x60/0x1dc
[  170.349916] [ecde9dc8] [c00c0ad4] load_module+0x1ca4/0x1e18
[  170.366610] [ecde9ea8] [c00c0df4] sys_finit_module+0x98/0xb8
[  170.383179] [ecde9f38] [c0014274] ret_from_syscall+0x0/0x34
[  170.399101] --- interrupt: c01 at 0x8ed2c4
                   LR = 0xa757c4
[  170.428916] Instruction dump:
[  170.442826] 7e048378 7f83e378 60e70001 90e10030 4bffda0d 2f830000 41beff20
7f84e378 
[  170.456986] 7f63db78 4bffdd35 7e090034 5529d97e <0f090000> 2f900000 41beff00
7e527a14 
[  170.471190] irq event stamp: 0
[  170.485188] hardirqs last  enabled at (0): [<00000000>] 0x0
[  170.499283] hardirqs last disabled at (0): [<c0032f0c>]
copy_process+0x474/0x1368
[  170.513322] softirqs last  enabled at (0): [<c0032f0c>]
copy_process+0x474/0x1368
[  170.527207] softirqs last disabled at (0): [<00000000>] 0x0
[  170.540920] ---[ end trace 74450a6aa18e5960 ]---
[  170.558438] ------------[ cut here ]------------
[  170.572459] kernel BUG at mm/slub.c:3952!
[  170.586496] Oops: Exception in kernel mode, sig: 5 [#1]
[  170.600564] BE PAGE_SIZE=4K MMU=Hash SMP NR_CPUS=2 PowerMac
[  170.614556] Modules linked in: btrfs(+) auth_rpcgss nfsv4 dns_resolver nfs
lockd grace sunrpc input_leds joydev b43legacy led_class hid_generic mac80211
usbhid hid snd_aoa_codec_tas snd_aoa_fabric_layout snd_aoa cfg80211 rfkill
libarc4 evdev ohci_pci xor zstd_decompress zstd_compress zlib_deflate radeon
raid6_pq zlib_inflate therm_windtunnel ehci_pci ohci_hcd hwmon i2c_algo_bit
ehci_hcd backlight drm_kms_helper sungem firewire_ohci syscopyarea sungem_phy
sysfillrect sr_mod firewire_core sysimgblt fb_sys_fops cdrom crc_itu_t
snd_aoa_i2sbus snd_aoa_soundbus ttm snd_pcm usbcore snd_timer drm usb_common
snd soundcore ssb uninorth_agp drm_panel_orientation_quirks agpgart lzo
lzo_compress lzo_decompress zram zsmalloc
[  170.662386] CPU: 0 PID: 266 Comm: modprobe Tainted: G        W        
5.3.0-rc4+ #3
[  170.678893] NIP:  c0198228 LR: c0198204 CTR: c01981a4
[  170.695309] REGS: ecde9b10 TRAP: 0700   Tainted: G        W         
(5.3.0-rc4+)
[  170.711791] MSR:  00029032 <EE,ME,IR,DR,RI>  CR: 28242424  XER: 00000000
[  170.728343] 
               GPR00: f207aa60 ecde9bc8 eb353380 ef3d3a80 c07fe5f8 c01600dc
00003c40 00000000 
               GPR08: 00001032 00000001 00000000 ecde9c18 c01981a4 00a9eff4
00a64e74 c0710cd4 
               GPR16: f16a1fac 000011ad 00000001 00000124 c00bbeac c0e7e55c
00000001 ea8ed15c 
               GPR24: ea8ed16c f207aa60 f209a370 f209a343 00002000 c07fe5f8
ea9a6458 ef3d3a80 
[  170.812601] NIP [c0198228] kfree+0x84/0x29c
[  170.829631] LR [c0198204] kfree+0x60/0x29c
[  170.846753] Call Trace:
[  170.863878] [ecde9bc8] [f20d0000] test_error+0xd44/0xfffba69c [btrfs]
(unreliable)
[  170.881178] [ecde9c18] [f207aa60] btrfs_free_dummy_fs_info+0x168/0x1e0
[btrfs]
[  170.898408] [ecde9c78] [f207da3c] btrfs_test_free_space_cache+0x11c/0x1308
[btrfs]
[  170.915026] [ecde9cd8] [f207ad3c] btrfs_run_sanity_tests+0x8c/0x144 [btrfs]
[  170.931323] [ecde9d08] [f1678cd0] init_btrfs_fs+0xd4/0x12c [btrfs]
[  170.948575] [ecde9d28] [c00052cc] do_one_initcall+0x54/0x288
[  170.965784] [ecde9d98] [c00bebf8] do_init_module+0x60/0x1dc
[  170.982897] [ecde9dc8] [c00c0ad4] load_module+0x1ca4/0x1e18
[  170.999911] [ecde9ea8] [c00c0df4] sys_finit_module+0x98/0xb8
[  171.016767] [ecde9f38] [c0014274] ret_from_syscall+0x0/0x34
[  171.032967] --- interrupt: c01 at 0x8ed2c4
                   LR = 0xa757c4
[  171.063206] Instruction dump:
[  171.077290] 4bffade1 7c7f1b78 4bffadbd 81230000 71290200 40a200f8 813f0000
552987ff 
[  171.091622] 4082000c 813f0004 552907fe 69290001 <0f090000> 7fe3fb78 4bffadcd
7c641b78 
[  171.106020] ---[ end trace 74450a6aa18e5961 ]---

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply

* Re: [PATCH v5 06/18] compat_ioctl: move WDIOC handling into wdt drivers
From: Guenter Roeck @ 2019-08-15 18:10 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-hwmon, linux-rtc, Alexandre Belloni, Jean Delvare,
	Anatolij Gustschin, linux-um, linux-kernel, Nicolas Ferre,
	Ludovic Desroches, Florian Fainelli, bcm-kernel-feedback-list,
	viro, linux-fsdevel, openipmi-developer, Wim Van Sebroeck,
	linuxppc-dev, linux-arm-kernel, linux-watchdog
In-Reply-To: <20190814205245.121691-1-arnd@arndb.de>

On Wed, Aug 14, 2019 at 10:49:18PM +0200, Arnd Bergmann wrote:
> All watchdog drivers implement the same set of ioctl commands, and
> fortunately all of them are compatible between 32-bit and 64-bit
> architectures.
> 
> Modern drivers always go through drivers/watchdog/wdt.c as an abstraction
> layer, but older ones implement their own file_operations on a character
> device for this.
> 
> Move the handling from fs/compat_ioctl.c into the individual drivers.
> 
> Note that most of the legacy drivers will never be used on 64-bit
> hardware, because they are for an old 32-bit SoC implementation, but
> doing them all at once is safer than trying to guess which ones do
> or do not need the compat_ioctl handling.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

This patch doesn't seem to have a useful base (or at least git says so).
It does not apply to mainline nor to my own watchdog-next branch.
I assume you plan to apply the entire series together. Please not
that there will be conflicts against watchdog-next when you do so.

Guenter

^ permalink raw reply

* Re: [PATCH 0/6] drm+dma: cache support for arm, etc
From: Koenig, Christian @ 2019-08-15 18:21 UTC (permalink / raw)
  To: Christoph Hellwig, Rob Clark
  Cc: Kate Stewart, Masayoshi Mizuma, Maciej W. Rozycki, Eric Biggers,
	Catalin Marinas, Imre Deak, dri-devel, Chris Wilson,
	Masahiro Yamada, Benjamin Gaignard, Mauro Carvalho Chehab,
	Will Deacon, Emil Velikov, Sharma, Deepak, Paul Burton,
	Mike Rapoport, Geert Uytterhoeven,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), Daniel Vetter,
	open list:MIPS, Linus Walleij, Robin Murphy,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Joerg Roedel,
	Arnd Bergmann, Anshuman Khandual, Hauke Mehrtens,
	Jesper Dangaard Brouer, Wolfram Sang (Renesas),
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Alexios Zavras,
	Russell King, Doug Anderson, Thomas Gleixner, Sean Paul,
	Allison Randal, Enrico Weigelt, Ard Biesheuvel,
	Greg Kroah-Hartman, open list, Rob Clark, Souptick Joarder,
	Andrew Morton, open list:DRM DRIVER FOR MSM ADRENO GPU
In-Reply-To: <20190815175346.GA19839@lst.de>

Am 15.08.19 um 19:53 schrieb Christoph Hellwig:
> On Thu, Aug 15, 2019 at 06:54:39AM -0700, Rob Clark wrote:
>> On Wed, Aug 14, 2019 at 11:51 PM Christoph Hellwig <hch@lst.de> wrote:
>>> As said before I don't think these low-level helpers are the
>>> right API to export, but even if they did you'd just cover a tiny
>>> subset of the architectures.
>> Are you thinking instead something like:
>>
>> void dma_sync_sg_for_{cpu,device}(struct device *dev, struct scatterlist *sgl,
>>                                    int nents, enum dma_data_direction dir)
>> {
>>      for_each_sg(sgl, sg, nents, i) {
>>          arch_sync_dma_for_..(dev, sg_phys(sg), sg->length, dir);
>>      }
>> }
>> EXPORT_SYMBOL_GPL(dma_sync_sg_for_..)
>>
>> or did you have something else in mind?
> No.  We really need an interface thay says please give me uncached
> memory (for some definition of uncached that includes that grapics
> drivers call write combine), and then let the architecture do the right
> thing.  Basically dma_alloc_coherent with DMA_ATTR_NO_KERNEL_MAPPING
> is superficially close to what you want, except that the way the drm
> drivers work you can't actually use it.

The terminology graphics driver use is USWC (Uncached Speculative Write 
Combine).

Essentially it is a leftover from the AGP days where the graphics 
devices couldn't snoop the CPU caches.

Nowadays they either don't want to snoop the CPU caches for performance 
reasons.

Or because of special requirements that certain areas of system memory 
are not cached for certain functionality.

For example the "VRAM" on AMD GPUs which are integrated into the CPU is 
just stolen system memory. Then you can scanout your picture to the 
display from this memory or "normal" system memory, but only if the 
system memory is mapped as USWC.

> The reason for that is if we can we really need to not create another
> uncachable alias, but instead change the page attributes in place.
> On x86 we can and must do that for example, and based on the
> conversation with Will arm64 could do that fairly easily.  arm32 can
> right now only do that for CMA, though.
>
> The big question is what API do we want.  I had a pretty similar
> discussion with Christian on doing such an allocation for amdgpu,
> where the device normally is cache coherent, but they actually want
> to turn it into non-coherent by using PCIe unsnooped transactions.
>
> Here is my high level plan, which still has a few lose end:
>
>   (1) provide a new API:
>
> 	struct page *dma_alloc_pages(struct device *dev, unsigned nr_pages,
> 			gfp_t gfp, unsigned long flags);
> 	void dma_free_pages(struct device *dev, unsigned nr_pages,
> 			unsigned long flags);
>
>       These give you back page backed memory that is guaranteed to be
>       addressable by the device (no swiotlb or similar).  The memory can
>       then be mapped using dma_map*, including unmap and dma_sync to
>       bounce ownership around.  This is the replacement for the current
>       dma_alloc_attrs with DMA_ATTR_NON_CONSISTENT API, that is rather
>       badly defined.
>
>   (2) Add support for DMA_ATTR_NO_KERNEL_MAPPING to this new API instead
>       of dma_alloc_attrs.  The initial difference with that flag is just
>       that we allow highmem, but in the future we could also unmap this
>       memory from the kernel linear mapping entirely on architectures
>       where we can easily do that.

Mhm, why would we want to do this?

>
>   (3) Add a dma_pages_map/dma_pages_unmap or similar API that allows you
>       to get a kernel mapping for parts or all of a
>       DMA_ATTR_NO_KERNEL_MAPPING allocation.  This is to replace things
>       like your open-coded vmap in msm (or similarly elsewhere in dma-buf
>       providers).
>
>   (4) Add support for a DMA_ATTR_UNCACHABLE flags (or similar) to the new
>       API, that maps the pages as uncachable iff they have a kernel
>       mapping, including invalidating the caches at time of this page
>       attribute change (or creation of a new mapping).  This API will fail
>       if the architecture does not allow in-place remapping.  Note that for
>       arm32 we could always dip into the CMA pool if one is present to not
>       fail.  We'll also need some helper to map from the DMA_ATTR_* flags
>       to a pgprot for mapping the page to userspace.  There is also a few
>       other weird bits here, e.g. on architectures like mips that use an
>       uncached segment we'll have to fail use with the plain
>       DMA_ATTR_UNCACHABLE flag, but it could be supported with
>       DMA_ATTR_UNCACHABLE | DMA_ATTR_NO_KERNEL_MAPPING.
>
> I was hoping to get most of this done for this merge window, but I'm
> probably lucky if I get at least parts done.  Too much distraction.

Thanks again for taking care of this,
Christian.

>
>> Hmm, not entirely sure why.. you should be on the cc list for each
>> individual patch.
> They finally made it, although even with the delay they only ended up
> in the spam mailbox.  I still can't see them on the various mailing
> lists.


^ permalink raw reply

* Re: [PATCH 0/6] drm+dma: cache support for arm, etc
From: Christoph Hellwig @ 2019-08-15 18:27 UTC (permalink / raw)
  To: Koenig, Christian
  Cc: Kate Stewart, Masayoshi Mizuma, Maciej W. Rozycki, Eric Biggers,
	Catalin Marinas, Imre Deak, dri-devel, Chris Wilson,
	Masahiro Yamada, Benjamin Gaignard, Mauro Carvalho Chehab,
	Will Deacon, Christoph Hellwig, Emil Velikov, Rob Clark,
	Paul Burton, Mike Rapoport, Geert Uytterhoeven,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), Daniel Vetter,
	open list:MIPS, Linus Walleij, Robin Murphy,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Sharma, Deepak,
	Joerg Roedel, Arnd Bergmann, Anshuman Khandual, Hauke Mehrtens,
	Jesper Dangaard Brouer, Wolfram Sang (Renesas),
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Alexios Zavras,
	Russell King, Doug Anderson, Thomas Gleixner, Sean Paul,
	Allison Randal, Ard Biesheuvel, Enrico Weigelt, open list,
	Rob Clark, Souptick Joarder, Greg Kroah-Hartman, Andrew Morton,
	open list:DRM DRIVER FOR MSM ADRENO GPU
In-Reply-To: <e3f73b3c-49d5-3c19-dfff-0a24b4617e50@amd.com>

On Thu, Aug 15, 2019 at 06:21:00PM +0000, Koenig, Christian wrote:
> >   (2) Add support for DMA_ATTR_NO_KERNEL_MAPPING to this new API instead
> >       of dma_alloc_attrs.  The initial difference with that flag is just
> >       that we allow highmem, but in the future we could also unmap this
> >       memory from the kernel linear mapping entirely on architectures
> >       where we can easily do that.
> 
> Mhm, why would we want to do this?

To avoid the CPU misspeculating into this memory.  For example NVMe SSDs
have a feature called host memory buffer that is a lot like your stolen
main ram for the GPU case.  We currently hand the SSD a
DMA_ATTR_NO_KERNEL_MAPPING allocation if it requests such a buffer.  If
possible we'd really like to make sure no speculative execution bug
(or intentional attacker with a kernel exploit for that matter) can easily
access that memory.

^ permalink raw reply

* [Bug 204371] BUG kmalloc-4k (Tainted: G        W        ): Object padding overwritten
From: bugzilla-daemon @ 2019-08-15 18:33 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-204371-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=204371

--- Comment #31 from Christophe Leroy (christophe.leroy@c-s.fr) ---
Problem 1: test_add_free_space_entry() contains a kzalloc() to allocate a
bitmap. That's the problem.


Problem 2: btrfs_free_dummy_fs_info() has 3 kfree(). Need to know which one is
creating your last warning (kernel BUG at mm/slub.c:3952!)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply

* Re: [PATCH 0/6] drm+dma: cache support for arm, etc
From: Christoph Hellwig @ 2019-08-15 18:52 UTC (permalink / raw)
  To: Koenig, Christian
  Cc: Kate Stewart, Masayoshi Mizuma, Maciej W. Rozycki, Eric Biggers,
	Catalin Marinas, Imre Deak, dri-devel, Chris Wilson,
	Masahiro Yamada, Benjamin Gaignard, Mauro Carvalho Chehab,
	Will Deacon, Christoph Hellwig, Emil Velikov, Rob Clark,
	Paul Burton, Mike Rapoport, Geert Uytterhoeven,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), Daniel Vetter,
	open list:MIPS, Linus Walleij, Robin Murphy,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Sharma, Deepak,
	Joerg Roedel, Arnd Bergmann, Anshuman Khandual, Hauke Mehrtens,
	Jesper Dangaard Brouer, Wolfram Sang (Renesas),
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Alexios Zavras,
	Russell King, Doug Anderson, Thomas Gleixner, Sean Paul,
	Allison Randal, Ard Biesheuvel, Enrico Weigelt, open list,
	Rob Clark, Souptick Joarder, Greg Kroah-Hartman, Andrew Morton,
	open list:DRM DRIVER FOR MSM ADRENO GPU
In-Reply-To: <215e5cb7-0fcf-48db-a656-817054dde420@email.android.com>

On Thu, Aug 15, 2019 at 06:48:39PM +0000, Koenig, Christian wrote:
> Well, for the graphics case I absolutely need to keep the linear kernel mapping. Because for certain use cases the memory is accessed by the kernel all the time as well.

Then don't set DMA_ATTR_NO_KERNEL_MAPPING.  At least for x86 and arm64
we should be able to support uncached allocations easily even without
that, and I plan to support that for your use case.  But if the driver
is explicitly saying it doesn't want a permanent kernel mapping it makes
sense to (optionally if the architecture can easily do it) unmap the
memory from the kernel linear mapping.

^ permalink raw reply

* Re: [PATCH v5 1/4] nvdimm: Consider probe return -EOPNOTSUPP as success
From: Dan Williams @ 2019-08-15 19:54 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: Linux MM, linuxppc-dev, linux-nvdimm
In-Reply-To: <CAPcyv4jmxKPkTh0_Bbu2tRXm4vcBHonZJ6UcKrOBnPGCG2_i1A@mail.gmail.com>

On Tue, Aug 13, 2019 at 9:22 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> Hi Aneesh, logic looks correct but there are some cleanups I'd like to
> see and a lead-in patch that I attached.
>
> I've started prefixing nvdimm patches with:
>
>     libnvdimm/$component:
>
> ...since this patch mostly impacts the pmem driver lets prefix it
> "libnvdimm/pmem: "
>
> On Fri, Aug 9, 2019 at 12:45 AM Aneesh Kumar K.V
> <aneesh.kumar@linux.ibm.com> wrote:
> >
> > This patch add -EOPNOTSUPP as return from probe callback to
>
> s/This patch add/Add/
>
> No need to say "this patch" it's obviously a patch.
>
> > indicate we were not able to initialize a namespace due to pfn superblock
> > feature/version mismatch. We want to consider this a probe success so that
> > we can create new namesapce seed and there by avoid marking the failed
> > namespace as the seed namespace.
>
> Please replace usage of "we" with the exact agent involved as which
> "we" is being referred to gets confusing for the reader.
>
> i.e. "indicate that the pmem driver was not..." "The nvdimm core wants
> to consider this...".
>
> >
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> > ---
> >  drivers/nvdimm/bus.c  |  2 +-
> >  drivers/nvdimm/pmem.c | 26 ++++++++++++++++++++++----
> >  2 files changed, 23 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
> > index 798c5c4aea9c..16c35e6446a7 100644
> > --- a/drivers/nvdimm/bus.c
> > +++ b/drivers/nvdimm/bus.c
> > @@ -95,7 +95,7 @@ static int nvdimm_bus_probe(struct device *dev)
> >         rc = nd_drv->probe(dev);
> >         debug_nvdimm_unlock(dev);
> >
> > -       if (rc == 0)
> > +       if (rc == 0 || rc == -EOPNOTSUPP)
> >                 nd_region_probe_success(nvdimm_bus, dev);
>
> This now makes the nd_region_probe_success() helper obviously misnamed
> since it now wants to take actions on non-probe success. I attached a
> lead-in cleanup that you can pull into your series that renames that
> routine to nd_region_advance_seeds().
>
> When you rebase this needs a comment about why EOPNOTSUPP has special handling.
>
> >         else
> >                 nd_region_disable(nvdimm_bus, dev);
> > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> > index 4c121dd03dd9..3f498881dd28 100644
> > --- a/drivers/nvdimm/pmem.c
> > +++ b/drivers/nvdimm/pmem.c
> > @@ -490,6 +490,7 @@ static int pmem_attach_disk(struct device *dev,
> >
> >  static int nd_pmem_probe(struct device *dev)
> >  {
> > +       int ret;
> >         struct nd_namespace_common *ndns;
> >
> >         ndns = nvdimm_namespace_common_probe(dev);
> > @@ -505,12 +506,29 @@ static int nd_pmem_probe(struct device *dev)
> >         if (is_nd_pfn(dev))
> >                 return pmem_attach_disk(dev, ndns);
> >
> > -       /* if we find a valid info-block we'll come back as that personality */
> > -       if (nd_btt_probe(dev, ndns) == 0 || nd_pfn_probe(dev, ndns) == 0
> > -                       || nd_dax_probe(dev, ndns) == 0)
>
> Similar need for an updated comment here to explain the special
> translation of error codes.
>
> > +       ret = nd_btt_probe(dev, ndns);
> > +       if (ret == 0)
> >                 return -ENXIO;
> > +       else if (ret == -EOPNOTSUPP)
>
> Are there cases where the btt driver needs to return EOPNOTSUPP? I'd
> otherwise like to keep this special casing constrained to the pfn /
> dax info block cases.

In fact I think EOPNOTSUPP is only something that the device-dax case
would be concerned with because that's the only interface that
attempts to guarantee a given mapping granularity.

^ permalink raw reply

* Re: next take at setting up a dma mask by default for platform devices
From: Alan Stern @ 2019-08-15 14:39 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Gavin Li, Shawn Guo, Fabio Estevam, linux-arch, Michal Simek,
	Maxime Chevallier, NXP Linux Team, Mathias Nyman, Sascha Hauer,
	Minas Harutyunyan, Olav Kongas, Bin Liu, linux-arm-kernel,
	Laurentiu Tudor, Geoff Levand, Greg Kroah-Hartman, linux-usb,
	linux-kernel, Tony Prisk, iommu, Pengutronix Kernel Team,
	linuxppc-dev
In-Reply-To: <20190815132531.GA12036@lst.de>

On Thu, 15 Aug 2019, Christoph Hellwig wrote:

> On Thu, Aug 15, 2019 at 03:23:18PM +0200, Greg Kroah-Hartman wrote:
> > I've taken the first 2 patches for 5.3-final.  Given that patch 3 needs
> > to be fixed, I'll wait for a respin of these before considering them.
> 
> I have a respun version ready, but I'd really like to hear some
> comments from usb developers about the approach before spamming
> everyone again..

I didn't see any problems with your approach at first glance; it looked 
like a good idea.

Alan Stern


^ permalink raw reply

* Re: [PATCH v5 3/4] mm/nvdimm: Use correct #defines instead of open coding
From: Dan Williams @ 2019-08-15 21:05 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: Linux MM, linuxppc-dev, linux-nvdimm
In-Reply-To: <20190809074520.27115-4-aneesh.kumar@linux.ibm.com>

On Fri, Aug 9, 2019 at 12:45 AM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> Use PAGE_SIZE instead of SZ_4K and sizeof(struct page) instead of 64.
> If we have a kernel built with different struct page size the previous
> patch should handle marking the namespace disabled.

Each of these changes carry independent non-overlapping regression
risk, so lets split them into separate patches. Others might

> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  drivers/nvdimm/label.c          | 2 +-
>  drivers/nvdimm/namespace_devs.c | 6 +++---
>  drivers/nvdimm/pfn_devs.c       | 3 ++-
>  drivers/nvdimm/region_devs.c    | 8 ++++----
>  4 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
> index 73e197babc2f..7ee037063be7 100644
> --- a/drivers/nvdimm/label.c
> +++ b/drivers/nvdimm/label.c
> @@ -355,7 +355,7 @@ static bool slot_valid(struct nvdimm_drvdata *ndd,
>
>         /* check that DPA allocations are page aligned */
>         if ((__le64_to_cpu(nd_label->dpa)
> -                               | __le64_to_cpu(nd_label->rawsize)) % SZ_4K)
> +                               | __le64_to_cpu(nd_label->rawsize)) % PAGE_SIZE)

The UEFI label specification has no concept of PAGE_SIZE, so this
check is a pure Linux-ism. There's no strict requirement why
slot_valid() needs to check for page alignment and it would seem to
actively hurt cross-page-size compatibility, so let's delete the check
and rely on checksum validation.

>                 return false;
>
>         /* check checksum */
> diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
> index a16e52251a30..a9c76df12cb9 100644
> --- a/drivers/nvdimm/namespace_devs.c
> +++ b/drivers/nvdimm/namespace_devs.c
> @@ -1006,10 +1006,10 @@ static ssize_t __size_store(struct device *dev, unsigned long long val)
>                 return -ENXIO;
>         }
>
> -       div_u64_rem(val, SZ_4K * nd_region->ndr_mappings, &remainder);
> +       div_u64_rem(val, PAGE_SIZE * nd_region->ndr_mappings, &remainder);
>         if (remainder) {
> -               dev_dbg(dev, "%llu is not %dK aligned\n", val,
> -                               (SZ_4K * nd_region->ndr_mappings) / SZ_1K);
> +               dev_dbg(dev, "%llu is not %ldK aligned\n", val,
> +                               (PAGE_SIZE * nd_region->ndr_mappings) / SZ_1K);
>                 return -EINVAL;

Yes, looks good, but this deserves its own independent patch.

>         }
>
> diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c
> index 37e96811c2fc..c1d9be609322 100644
> --- a/drivers/nvdimm/pfn_devs.c
> +++ b/drivers/nvdimm/pfn_devs.c
> @@ -725,7 +725,8 @@ static int nd_pfn_init(struct nd_pfn *nd_pfn)
>                  * when populating the vmemmap. This *should* be equal to
>                  * PMD_SIZE for most architectures.
>                  */
> -               offset = ALIGN(start + SZ_8K + 64 * npfns, align) - start;
> +               offset = ALIGN(start + SZ_8K + sizeof(struct page) * npfns,

I'd prefer if this was not dynamic and was instead set to the maximum
size of 'struct page' across all archs just to enhance cross-arch
compatibility. I think that answer is '64'.
> +                              align) - start;
>         } else if (nd_pfn->mode == PFN_MODE_RAM)
>                 offset = ALIGN(start + SZ_8K, align) - start;
>         else
> diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> index af30cbe7a8ea..20e265a534f8 100644
> --- a/drivers/nvdimm/region_devs.c
> +++ b/drivers/nvdimm/region_devs.c
> @@ -992,10 +992,10 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
>                 struct nd_mapping_desc *mapping = &ndr_desc->mapping[i];
>                 struct nvdimm *nvdimm = mapping->nvdimm;
>
> -               if ((mapping->start | mapping->size) % SZ_4K) {
> -                       dev_err(&nvdimm_bus->dev, "%s: %s mapping%d is not 4K aligned\n",
> -                                       caller, dev_name(&nvdimm->dev), i);
> -
> +               if ((mapping->start | mapping->size) % PAGE_SIZE) {
> +                       dev_err(&nvdimm_bus->dev,
> +                               "%s: %s mapping%d is not %ld aligned\n",
> +                               caller, dev_name(&nvdimm->dev), i, PAGE_SIZE);
>                         return NULL;
>                 }
>
> --
> 2.21.0
>

^ permalink raw reply

* Re: [PATCH 0/6] drm+dma: cache support for arm, etc
From: Rob Clark @ 2019-08-15 13:54 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Kate Stewart, Masayoshi Mizuma, Maciej W. Rozycki, Eric Biggers,
	Catalin Marinas, Imre Deak, dri-devel, Chris Wilson,
	Masahiro Yamada, Benjamin Gaignard, Mauro Carvalho Chehab,
	Will Deacon, Emil Velikov, Deepak Sharma, Paul Burton,
	Mike Rapoport, Geert Uytterhoeven,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), Daniel Vetter,
	open list:MIPS, Linus Walleij, Robin Murphy,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Joerg Roedel,
	Arnd Bergmann, Anshuman Khandual, Hauke Mehrtens,
	Jesper Dangaard Brouer, Wolfram Sang (Renesas),
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Alexios Zavras,
	Russell King, Doug Anderson, Thomas Gleixner, Sean Paul,
	Allison Randal, Enrico Weigelt, Ard Biesheuvel,
	Greg Kroah-Hartman, open list, Rob Clark, Souptick Joarder,
	Andrew Morton, open list:DRM DRIVER FOR MSM ADRENO GPU
In-Reply-To: <20190815065117.GA23761@lst.de>

On Wed, Aug 14, 2019 at 11:51 PM Christoph Hellwig <hch@lst.de> wrote:
>
> As said before I don't think these low-level helpers are the
> right API to export, but even if they did you'd just cover a tiny
> subset of the architectures.

Are you thinking instead something like:

void dma_sync_sg_for_{cpu,device}(struct device *dev, struct scatterlist *sgl,
                                  int nents, enum dma_data_direction dir)
{
    for_each_sg(sgl, sg, nents, i) {
        arch_sync_dma_for_..(dev, sg_phys(sg), sg->length, dir);
    }
}
EXPORT_SYMBOL_GPL(dma_sync_sg_for_..)

or did you have something else in mind?

I guess something like this would avoid figuring out *which* archs
actually build drm..


> Also to distil the previous thread - if you remap memory to uncached
> the helper to use is arch_dma_prep_coherent, which does a writeback+
> invalidate everywhere, and there is no need to clean up after a
> long-term uncached mapping.  We might still get speculations into
> that area, if we don't remap the direct mapping, but it isn't like
> invalidting that just before freeing the memory is going to help
> anyone.

hmm, IIUC the aarch64 cache instructions, what I'm doing now is equiv
to what I would get with dma_map_sg(DMA_BIDIRECTIONAL) and
arch_dma_prep_coherent() is equiv to what I'd get w/ DMA_FROM_DEVICE
(but a single pass instead of separate inv+clean passes)..

but I can respin this with a single dma_prep_coherent_sg() which uses
arch_dma_prep_coherent()..

> Also it seems like patches 5 and 6 are missing in my inbox.

Hmm, not entirely sure why.. you should be on the cc list for each
individual patch.

But here is the patchwork link if you want to see them:

https://patchwork.freedesktop.org/series/65211/

BR,
-R

^ permalink raw reply

* Re: 5.2.7 kernel doesn't boot on G5
From: Christian Marillat @ 2019-08-15 17:13 UTC (permalink / raw)
  To: Mathieu Malaterre; +Cc: marillat, linuxppc-dev
In-Reply-To: <CA+7wUsw5eTdwJG3UytWr9CajVhpUkyOGufmvUvqQJoEWq4nWhQ@mail.gmail.com>

On 15 août 2019 19:05, Mathieu Malaterre <malat@debian.org> wrote:

> Does that ring a bell to anyone here ? Thanks

Din't reply to all. Second reply :

Nothing. Fans turn 100% after 1 or 2 minutes.

Christian

^ permalink raw reply

* Re: 5.2.7 kernel doesn't boot on G5
From: Christian Marillat @ 2019-08-15 17:48 UTC (permalink / raw)
  To: christophe leroy; +Cc: Mathieu Malaterre, marillat, linuxppc-dev
In-Reply-To: <a84c86b3-4c6c-f7a2-ad3f-6e075e6ebe25@c-s.fr>

On 15 août 2019 19:29, christophe leroy <christophe.leroy@c-s.fr> wrote:

> Le 15/08/2019 à 19:05, Mathieu Malaterre a écrit :
>> Does that ring a bell to anyone here ? Thanks
>
> Apparently that's 5.2.0, not 5.2.7

Yes, 5.2.7 is the Debian package version. Sorry for the mistake.

Christian

^ permalink raw reply

* Re: [PATCH 0/6] drm+dma: cache support for arm, etc
From: Koenig, Christian @ 2019-08-15 18:48 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Kate Stewart, Masayoshi Mizuma, Maciej W. Rozycki, Eric Biggers,
	Catalin Marinas, Imre Deak, dri-devel, Chris Wilson,
	Masahiro Yamada, Benjamin Gaignard, Mauro Carvalho Chehab,
	Will Deacon, Emil Velikov, Rob Clark, Paul Burton, Mike Rapoport,
	Geert Uytterhoeven,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE), Daniel Vetter,
	open list:MIPS, Linus Walleij, Robin Murphy,
	open list:DRM DRIVER FOR MSM ADRENO GPU, Sharma, Deepak,
	Joerg Roedel, Arnd Bergmann, Anshuman Khandual, Hauke Mehrtens,
	Jesper Dangaard Brouer, Wolfram Sang (Renesas),
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Alexios Zavras,
	Russell King, Doug Anderson, Thomas Gleixner, Sean Paul,
	Allison Randal, Enrico Weigelt, Ard Biesheuvel,
	Greg Kroah-Hartman, open list, Rob Clark, Souptick Joarder,
	Andrew Morton, open list:DRM DRIVER FOR MSM ADRENO GPU

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

Sorry if the style is messed up, replying from my phone.

Am 15.08.2019 20:27 schrieb Christoph Hellwig <hch@lst.de>:
On Thu, Aug 15, 2019 at 06:21:00PM +0000, Koenig, Christian wrote:
> >   (2) Add support for DMA_ATTR_NO_KERNEL_MAPPING to this new API instead
> >       of dma_alloc_attrs.  The initial difference with that flag is just
> >       that we allow highmem, but in the future we could also unmap this
> >       memory from the kernel linear mapping entirely on architectures
> >       where we can easily do that.
>
> Mhm, why would we want to do this?

To avoid the CPU misspeculating into this memory.  For example NVMe SSDs
have a feature called host memory buffer that is a lot like your stolen
main ram for the GPU case.  We currently hand the SSD a
DMA_ATTR_NO_KERNEL_MAPPING allocation if it requests such a buffer.  If
possible we'd really like to make sure no speculative execution bug
(or intentional attacker with a kernel exploit for that matter) can easily
access that memory.

Well, for the graphics case I absolutely need to keep the linear kernel mapping. Because for certain use cases the memory is accessed by the kernel all the time as well.

Why should accessing uncached memory be more of a security problem than accessing cached?

Regards,
Christian


[-- Attachment #2: Type: text/html, Size: 2507 bytes --]

^ permalink raw reply

* Re: [PATCH v1 2/4] soc: fsl: guts: Add definition for LS1028A
From: Li Yang @ 2019-08-15 22:40 UTC (permalink / raw)
  To: Yinbo Zhu
  Cc: Mark Rutland, Ulf Hansson, Catalin Marinas, Will Deacon,
	Adrian Hunter, Catalin Horghidan, linux-mmc,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rajesh Bhagat, Alison Wang, Ashish Kumar, Claudiu Manoil,
	Rob Herring, Vabhav Sharma,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Amit Jain, Alex Marginean, lkml, Rajat Srivastava, Yangbo Lu,
	Jiafei Pan, linuxppc-dev, Xiaobo Xie
In-Reply-To: <20190814072649.8237-2-yinbo.zhu@nxp.com>

On Wed, Aug 14, 2019 at 2:26 AM Yinbo Zhu <yinbo.zhu@nxp.com> wrote:
>
> Adding compatible string "ls1028a-dcfg" to initialize guts driver
> for ls1028 and SoC die attribute definition for LS1028A
>
> Signed-off-by: Yinbo Zhu <yinbo.zhu@nxp.com>

Applied for next.

> ---
>  drivers/soc/fsl/guts.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index 1ef8068c8dd3..34810f9bb2ee 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -102,6 +102,11 @@ static const struct fsl_soc_die_attr fsl_soc_die[] = {
>           .svr          = 0x87360000,
>           .mask         = 0xff3f0000,
>         },
> +       /* Die: LS1028A, SoC: LS1028A */
> +       { .die          = "LS1028A",
> +         .svr          = 0x870b0000,
> +         .mask         = 0xff3f0000,
> +       },
>         { },
>  };
>
> @@ -224,6 +229,7 @@ static const struct of_device_id fsl_guts_of_match[] = {
>         { .compatible = "fsl,ls1012a-dcfg", },
>         { .compatible = "fsl,ls1046a-dcfg", },
>         { .compatible = "fsl,lx2160a-dcfg", },
> +       { .compatible = "fsl,ls1028a-dcfg", },
>         {}
>  };
>  MODULE_DEVICE_TABLE(of, fsl_guts_of_match);
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH v3 0/7] soc/fsl/qbman: Enable Kexec for DPAA1 devices
From: Li Yang @ 2019-08-15 23:31 UTC (permalink / raw)
  To: Roy Pledge
  Cc: Madalin-cristian Bucur, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Laurentiu Tudor
In-Reply-To: <1564690599-29713-1-git-send-email-roy.pledge@nxp.com>

On Thu, Aug 1, 2019 at 3:20 PM Roy Pledge <roy.pledge@nxp.com> wrote:
>
> Most DPAA1 devices do not support a soft reset which is an issue if
> Kexec starts a new kernel. This patch series allows Kexec to function
> by detecting that the QBMan device was previously initialized.
>
> The patches fix some issues with device cleanup as well as ensuring
> that the location of the QBMan private memories has not changed
> after the execution of the Kexec.
>
> Changes since v1:
>         - Removed a bug fix and sent it separately to ease backporting
> Changes since v2:
>         - Expliciitly flush FQD memory from cache on PPC before unmapping
>
> Roy Pledge (7):
>   soc/fsl/qbman: Rework QBMan private memory setup
>   soc/fsl/qbman: Cleanup buffer pools if BMan was initialized prior to
>     bootup
>   soc/fsl/qbman: Cleanup QMan queues if device was already initialized
>   soc/fsl/qbman: Fix drain_mr_fqni()
>   soc/fsl/qbman: Disable interrupts during portal recovery
>   soc/fsl/qbman: Fixup qman_shutdown_fq()
>   soc/fsl/qbman: Update device tree with reserved memory

Series applied for next.  Thanks!

>
>  drivers/soc/fsl/qbman/bman.c        | 17 ++++----
>  drivers/soc/fsl/qbman/bman_ccsr.c   | 36 +++++++++++++++-
>  drivers/soc/fsl/qbman/bman_portal.c | 18 +++++++-
>  drivers/soc/fsl/qbman/bman_priv.h   |  5 +++
>  drivers/soc/fsl/qbman/dpaa_sys.c    | 63 ++++++++++++++++------------
>  drivers/soc/fsl/qbman/qman.c        | 83 +++++++++++++++++++++++++++++--------
>  drivers/soc/fsl/qbman/qman_ccsr.c   | 68 +++++++++++++++++++++++++++---
>  drivers/soc/fsl/qbman/qman_portal.c | 18 +++++++-
>  drivers/soc/fsl/qbman/qman_priv.h   |  8 ++++
>  9 files changed, 255 insertions(+), 61 deletions(-)
>
> --
> 2.7.4
>

^ permalink raw reply

* Re: [PATCH v3 11/16] powerpc/pseries/svm: Export guest SVM status to user space via sysfs
From: Thiago Jung Bauermann @ 2019-08-16  0:49 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Anshuman Khandual, Alexey Kardashevskiy, Mike Anderson, Ram Pai,
	linux-kernel, Claudio Carvalho, Ryan Grimm, Paul Mackerras,
	linuxppc-dev, Christoph Hellwig
In-Reply-To: <87sgq36ivk.fsf@concordia.ellerman.id.au>


Michael Ellerman <mpe@ellerman.id.au> writes:

> Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
>> Michael Ellerman <mpe@ellerman.id.au> writes:
>>> Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
>>>> From: Ryan Grimm <grimm@linux.vnet.ibm.com>
>>>> User space might want to know it's running in a secure VM.  It can't do
>>>> a mfmsr because mfmsr is a privileged instruction.
>>>>
>>>> The solution here is to create a cpu attribute:
>>>>
>>>> /sys/devices/system/cpu/svm
>>>>
>>>> which will read 0 or 1 based on the S bit of the guest's CPU 0.
>>>
>>> Why CPU 0?
>>>
>>> If we have different CPUs running with different MSR_S then something
>>> has gone badly wrong, no?
>>
>> Yes, that would be very bad.
>>
>>> So can't we just read the MSR on whatever CPU the sysfs code happens to
>>> run on.
>>
>> Good point. I made the change in the patch below.
>
> The patch looks good. Although, it raises the question of whether it
> should be an attribute of the CPU at all.
>
> I guess there's not obviously anywhere better for it.

Ok. TBH this patch is not as urgent as the others. It was added so that
tests have an easy way to tell if they're in an SVM. I can leave it out
for now to figure out if there's a better place for this information.

> Still you should document the attribute in Documentation/ABI/testing/sysfs-devices-system-cpu

Indedd, will do that.

--
Thiago Jung Bauermann
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH v3 08/16] powerpc/pseries/svm: Use shared memory for LPPACA structures
From: Thiago Jung Bauermann @ 2019-08-16  0:52 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Anshuman Khandual, Alexey Kardashevskiy, Mike Anderson, Ram Pai,
	linux-kernel, Claudio Carvalho, Paul Mackerras, Paul E. McKenney,
	linuxppc-dev, Christoph Hellwig, Anshuman Khandual
In-Reply-To: <87tvakqap7.fsf@concordia.ellerman.id.au>


Michael Ellerman <mpe@ellerman.id.au> writes:

> Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
>> Michael Ellerman <mpe@ellerman.id.au> writes:
>>> Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
>>>> From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>>>>
>>>> LPPACA structures need to be shared with the host. Hence they need to be in
>>>> shared memory. Instead of allocating individual chunks of memory for a
>>>> given structure from memblock, a contiguous chunk of memory is allocated
>>>> and then converted into shared memory. Subsequent allocation requests will
>>>> come from the contiguous chunk which will be always shared memory for all
>>>> structures.
>>>>
>>>> While we are able to use a kmem_cache constructor for the Debug Trace Log,
>>>> LPPACAs are allocated very early in the boot process (before SLUB is
>>>> available) so we need to use a simpler scheme here.
>>>>
>>>> Introduce helper is_svm_platform() which uses the S bit of the MSR to tell
>>>> whether we're running as a secure guest.
>>>>
>>>> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>>>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>>>> ---
>>>>  arch/powerpc/include/asm/svm.h | 26 ++++++++++++++++++++
>>>>  arch/powerpc/kernel/paca.c     | 43 +++++++++++++++++++++++++++++++++-
>>>>  2 files changed, 68 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
>>>> new file mode 100644
>>>> index 000000000000..fef3740f46a6
>>>> --- /dev/null
>>>> +++ b/arch/powerpc/include/asm/svm.h
>>>> @@ -0,0 +1,26 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>>> +/*
>>>> + * SVM helper functions
>>>> + *
>>>> + * Copyright 2019 Anshuman Khandual, IBM Corporation.
>>>
>>> Are we sure this copyright date is correct?
>>
>> I may be confused about which year the copyright refers to. I thought it
>> was the year when the patch was committed. If it is the first time the
>> patch was published then this one should be 2018.
>
> I'm not a lawyer etc. but AIUI the date above is about the authorship,
> ie. when it was originally written, not when it was published or
> committed.
>
> In general I don't think it matters too much, but in this case I'm
> pretty sure Anshuman can't have possibly written it in 2019 on behalf of
> IBM :)
>
> So we can either change the date to 2018, or drop his name and just say
> it's copyright 2019 by IBM.

I think it's better to change the date to 2018. The same should be done
for svm.c, svm.h and mem_encrypt.h. I'll send a new patch series with
the correction.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center


^ permalink raw reply

* Re: [PATCH] powerpc: Allow flush_(inval_)dcache_range to work across ranges >4GB
From: Michael Ellerman @ 2019-08-16  1:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Alastair D'Silva
  Cc: linux-kernel, stable, Paul Mackerras, alastair, Thomas Gleixner,
	linuxppc-dev, Allison Randal
In-Reply-To: <20190815071924.GA26670@kroah.com>

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Thu, Aug 15, 2019 at 02:55:42PM +1000, Alastair D'Silva wrote:
>> From: Alastair D'Silva <alastair@d-silva.org>
>> 
>> Heads Up: This patch cannot be submitted to Linus's tree, as the affected
>> assembler functions have already been converted to C.

That was done in upstream commit:

22e9c88d486a ("powerpc/64: reuse PPC32 static inline flush_dcache_range()")

Which is a larger change that we don't want to backport. This patch is a
minimal fix for stable trees.


>> When calling flush_(inval_)dcache_range with a size >4GB, we were masking
>> off the upper 32 bits, so we would incorrectly flush a range smaller
>> than intended.
>> 
>> This patch replaces the 32 bit shifts with 64 bit ones, so that
>> the full size is accounted for.
>> 
>> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>> ---
>>  arch/powerpc/kernel/misc_64.S | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)

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

> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
>
> </formletter>

Hi Greg,

This is "option 3", submit the patch directly, and the patch "deviates
from the original upstream patch" because the upstream patch was a
wholesale conversion from asm to C.

This patch applies cleanly to v4.14 and v4.19.

The change log should have mentioned which upstream patch it is not a
backport of, is there anything else we should have done differently to
avoid the formletter bot :)

cheers

^ permalink raw reply

* Re: [PATCH 3/6] powerpc: Convert flush_icache_range & friends to C
From: Alastair D'Silva @ 2019-08-16  1:44 UTC (permalink / raw)
  To: christophe leroy
  Cc: Michal Hocko, Greg Kroah-Hartman, David Hildenbrand, linux-kernel,
	Nicholas Piggin, Mike Rapoport, Qian Cai, Paul Mackerras,
	Thomas Gleixner, linuxppc-dev, Andrew Morton, Allison Randal
In-Reply-To: <8a86bccf-ae4d-6d2c-72b1-db136cec9d10@c-s.fr>

On Thu, 2019-08-15 at 09:29 +0200, christophe leroy wrote:
> 
> Le 15/08/2019 à 06:10, Alastair D'Silva a écrit :
> > From: Alastair D'Silva <alastair@d-silva.org>
> > 
> > Similar to commit 22e9c88d486a
> > ("powerpc/64: reuse PPC32 static inline flush_dcache_range()")
> > this patch converts flush_icache_range() to C, and reimplements the
> > following functions as wrappers around it:
> > __flush_dcache_icache
> > __flush_dcache_icache_phys
> 
> Not sure you can do that for __flush_dcache_icache_phys(), see
> detailed 
> comments below
> 
> > This was done as we discovered a long-standing bug where the length
> > of the
> > range was truncated due to using a 32 bit shift instead of a 64 bit
> > one.
> > 
> > By converting these functions to C, it becomes easier to maintain.
> > 
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> >   arch/powerpc/include/asm/cache.h      |  26 +++---
> >   arch/powerpc/include/asm/cacheflush.h |  32 ++++---
> >   arch/powerpc/kernel/misc_32.S         | 117 -------------------
> > -------
> >   arch/powerpc/kernel/misc_64.S         |  97 ---------------------
> >   arch/powerpc/mm/mem.c                 |  71 +++++++++++++++-
> >   5 files changed, 102 insertions(+), 241 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/cache.h
> > b/arch/powerpc/include/asm/cache.h
> > index f852d5cd746c..728f154204db 100644
> > --- a/arch/powerpc/include/asm/cache.h
> > +++ b/arch/powerpc/include/asm/cache.h
> > @@ -98,20 +98,7 @@ static inline u32 l1_icache_bytes(void)
> >   #endif
> >   #endif /* ! __ASSEMBLY__ */
> >   
> > -#if defined(__ASSEMBLY__)
> > -/*
> > - * For a snooping icache, we still need a dummy icbi to purge all
> > the
> > - * prefetched instructions from the ifetch buffers. We also need a
> > sync
> > - * before the icbi to order the the actual stores to memory that
> > might
> > - * have modified instructions with the icbi.
> > - */
> > -#define PURGE_PREFETCHED_INS	\
> > -	sync;			\
> > -	icbi	0,r3;		\
> > -	sync;			\
> > -	isync
> 
> Is this still used anywhere now ?
No, this patch removes all users of it.

> > -
> > -#else
> > +#if !defined(__ASSEMBLY__)
> >   #define __read_mostly
> > __attribute__((__section__(".data..read_mostly")))
> >   
> >   #ifdef CONFIG_PPC_BOOK3S_32
> > @@ -145,6 +132,17 @@ static inline void dcbst(void *addr)
> >   {
> >   	__asm__ __volatile__ ("dcbst %y0" : : "Z"(*(u8 *)addr) :
> > "memory");
> >   }
> > +
> > +static inline void icbi(void *addr)
> > +{
> > +	__asm__ __volatile__ ("icbi 0, %0" : : "r"(addr) : "memory");
> > +}
> > +
> > +static inline void iccci(void)
> > +{
> > +	__asm__ __volatile__ ("iccci 0, r0");
> 
> I think you need the "memory" clobber too here.
> 
Thanks, I'll add that.

> > +}
> > +
> >   #endif /* !__ASSEMBLY__ */
> >   #endif /* __KERNEL__ */
> >   #endif /* _ASM_POWERPC_CACHE_H */
> > diff --git a/arch/powerpc/include/asm/cacheflush.h
> > b/arch/powerpc/include/asm/cacheflush.h
> > index ed57843ef452..4c3377aff8ed 100644
> > --- a/arch/powerpc/include/asm/cacheflush.h
> > +++ b/arch/powerpc/include/asm/cacheflush.h
> > @@ -42,24 +42,18 @@ extern void flush_dcache_page(struct page
> > *page);
> >   #define flush_dcache_mmap_lock(mapping)		do { } while
> > (0)
> >   #define flush_dcache_mmap_unlock(mapping)	do { } while (0)
> >   
> > -extern void flush_icache_range(unsigned long, unsigned long);
> > +void flush_icache_range(unsigned long start, unsigned long stop);
> >   extern void flush_icache_user_range(struct vm_area_struct *vma,
> >   				    struct page *page, unsigned long
> > addr,
> >   				    int len);
> > -extern void __flush_dcache_icache(void *page_va);
> >   extern void flush_dcache_icache_page(struct page *page);
> > -#if defined(CONFIG_PPC32) && !defined(CONFIG_BOOKE)
> > -extern void __flush_dcache_icache_phys(unsigned long physaddr);
> > -#else
> > -static inline void __flush_dcache_icache_phys(unsigned long
> > physaddr)
> > -{
> > -	BUG();
> > -}
> > -#endif
> >   
> > -/*
> > - * Write any modified data cache blocks out to memory and
> > invalidate them.
> > +/**
> > + * flush_dcache_range(): Write any modified data cache blocks out
> > to memory and invalidate them.
> >    * Does not invalidate the corresponding instruction cache
> > blocks.
> > + *
> > + * @start: the start address
> > + * @stop: the stop address (exclusive)
> >    */
> >   static inline void flush_dcache_range(unsigned long start,
> > unsigned long stop)
> >   {
> > @@ -82,6 +76,20 @@ static inline void flush_dcache_range(unsigned
> > long start, unsigned long stop)
> >   		isync();
> >   }
> >   
> > +/**
> > + * __flush_dcache_icache(): Flush a particular page from the data
> > cache to RAM.
> > + * Note: this is necessary because the instruction cache does
> > *not*
> > + * snoop from the data cache.
> > + *
> > + * @page: the address of the page to flush
> > + */
> > +static inline void __flush_dcache_icache(void *page)
> > +{
> > +	unsigned long page_addr = (unsigned long)page;
> 
> The function is small enough to call this addr instead of page_addr 
> without ambiguity.
Ok

> 
> > +
> > +	flush_icache_range(page_addr, page_addr + PAGE_SIZE);
> 
> Finally, I think that's not so simple. For the 44x, if
> MMU_FTR_TYPE_44x 
> is set, that's just a clean_dcache_range().
> If that feature is not set, it looks like a standard 
> flush_icache_range() but without the special CONFIG_4xx handling
> with 
> iccci we have in flush_icache_range()
> 
Hmm, I'll have to hack at this a bit more.

<snip>
-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819


^ permalink raw reply

* RE: [PATCH 4/6] powerpc: Chunk calls to flush_dcache_range in arch_*_memory
From: Alastair D'Silva @ 2019-08-16  1:45 UTC (permalink / raw)
  To: christophe leroy
  Cc: Greg Kroah-Hartman, David Hildenbrand, linux-kernel,
	Nicholas Piggin, Mike Rapoport, Qian Cai, Paul Mackerras,
	Thomas Gleixner, linuxppc-dev, Andrew Morton, Allison Randal
In-Reply-To: <366f94bd-c61b-d454-b202-d3feb81a14c3@c-s.fr>

On Thu, 2019-08-15 at 09:36 +0200, christophe leroy wrote:
> 
> Le 15/08/2019 à 06:10, Alastair D'Silva a écrit :
> > From: Alastair D'Silva <alastair@d-silva.org>
> > 
> > When presented with large amounts of memory being hotplugged
> > (in my test case, ~890GB), the call to flush_dcache_range takes
> > a while (~50 seconds), triggering RCU stalls.
> > 
> > This patch breaks up the call into 16GB chunks, calling
> > cond_resched() inbetween to allow the scheduler to run.
> 
> Is 16GB small enough ? If 890GB takes 50s, 16GB still takes about 1s.
> I'd use 1GB chuncks to remain below 100ms.
> 
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> >   arch/powerpc/mm/mem.c | 16 ++++++++++++++--
> >   1 file changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> > index 5400da87a804..fb0d5e9aa11b 100644
> > --- a/arch/powerpc/mm/mem.c
> > +++ b/arch/powerpc/mm/mem.c
> > @@ -104,11 +104,14 @@ int __weak remove_section_mapping(unsigned
> > long start, unsigned long end)
> >   	return -ENODEV;
> >   }
> >   
> > +#define FLUSH_CHUNK_SIZE (16ull * 1024ull * 1024ull * 1024ull)
> 
> Can we use SZ_16GB ?
Sure, I'll go with 1GB as you recommended above

> > +
> >   int __ref arch_add_memory(int nid, u64 start, u64 size,
> >   			struct mhp_restrictions *restrictions)
> >   {
> >   	unsigned long start_pfn = start >> PAGE_SHIFT;
> >   	unsigned long nr_pages = size >> PAGE_SHIFT;
> > +	unsigned long i;
> >   	int rc;
> >   
> >   	resize_hpt_for_hotplug(memblock_phys_mem_size());
> > @@ -120,7 +123,11 @@ int __ref arch_add_memory(int nid, u64 start,
> > u64 size,
> >   			start, start + size, rc);
> >   		return -EFAULT;
> >   	}
> > -	flush_dcache_range(start, start + size);
> > +
> > +	for (i = 0; i < size; i += FLUSH_CHUNK_SIZE) {
> > +		flush_dcache_range(start + i, min(start + size, start +
> > i + FLUSH_CHUNK_SIZE));
> 
> Isn't the line a bit long (I have not checked).
> 
> > +		cond_resched();
> > +	}
> >   
> >   	return __add_pages(nid, start_pfn, nr_pages, restrictions);
> >   }
> > @@ -131,13 +138,18 @@ void __ref arch_remove_memory(int nid, u64
> > start, u64 size,
> >   	unsigned long start_pfn = start >> PAGE_SHIFT;
> >   	unsigned long nr_pages = size >> PAGE_SHIFT;
> >   	struct page *page = pfn_to_page(start_pfn) +
> > vmem_altmap_offset(altmap);
> > +	unsigned long i;
> >   	int ret;
> >   
> >   	__remove_pages(page_zone(page), start_pfn, nr_pages, altmap);
> >   
> >   	/* Remove htab bolted mappings for this section of memory */
> >   	start = (unsigned long)__va(start);
> > -	flush_dcache_range(start, start + size);
> > +	for (i = 0; i < size; i += FLUSH_CHUNK_SIZE) {
> > +		flush_dcache_range(start + i, min(start + size, start +
> > i + FLUSH_CHUNK_SIZE));
> > +		cond_resched();
> > +	}
> > +
> >   	ret = remove_section_mapping(start, start + size);
> >   	WARN_ON_ONCE(ret);
> >   
> > 
> 
> Christophe
> 
> ---
> L'absence de virus dans ce courrier électronique a été vérifiée par
> le logiciel antivirus Avast.
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.avast.com_antivirus&d=DwIDaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=cT4tgeEQ0Ll3SIlZDHE5AEXyKy6uKADMtf9_Eb7-vec&m=TBT2NNM2DXqDWHhSb_WdFPcfAjYk9hP2cvGksF001cQ&s=XURKAOQQ4h3_RhJlezSguD2kpSitAF-uBhQqVZLU4GU&e= 
> 
-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819


^ permalink raw reply

* Re: [PATCH v4 18/25] powernv/fadump: process architected register state data provided by firmware
From: Hari Bathini @ 2019-08-16  2:38 UTC (permalink / raw)
  To: mahesh
  Cc: Ananth N Mavinakayanahalli, Mahesh J Salgaonkar, Nicholas Piggin,
	linuxppc-dev, Oliver, Vasant Hegde, Stewart Smith, Daniel Axtens
In-Reply-To: <20190814171503.6mpm2r63ildemyyz@in.ibm.com>



On 14/08/19 10:45 PM, Mahesh J Salgaonkar wrote:
> On 2019-07-16 17:04:08 Tue, Hari Bathini wrote:
>> From: Hari Bathini <hbathini@linux.vnet.ibm.com>
>>
>> Firmware provides architected register state data at the time of crash.
>> Process this data and build CPU notes to append to ELF core.
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/kernel/fadump-common.h          |    4 +
>>  arch/powerpc/platforms/powernv/opal-fadump.c |  197 ++++++++++++++++++++++++--
>>  arch/powerpc/platforms/powernv/opal-fadump.h |   39 +++++
>>  3 files changed, 228 insertions(+), 12 deletions(-)
>>
> [...]
>> @@ -430,6 +577,32 @@ int __init opal_fadump_dt_scan(struct fw_dump *fadump_conf, ulong node)
>>  			return 1;
>>  		}
>>  
>> +		ret = opal_mpipl_query_tag(OPAL_MPIPL_TAG_CPU, &addr);
>> +		if ((ret != OPAL_SUCCESS) || !addr) {
>> +			pr_err("Failed to get CPU metadata (%lld)\n", ret);
>> +			return 1;
>> +		}
>> +
>> +		addr = be64_to_cpu(addr);
>> +		pr_debug("CPU metadata addr: %llx\n", addr);
>> +
>> +		opal_cpu_metadata = __va(addr);
>> +		r_opal_cpu_metadata = (void *)addr;
>> +		fadump_conf->cpu_state_data_version =
>> +			be32_to_cpu(r_opal_cpu_metadata->cpu_data_version);
>> +		if (fadump_conf->cpu_state_data_version !=
>> +		    HDAT_FADUMP_CPU_DATA_VERSION) {
>> +			pr_err("CPU data format version (%lu) mismatch!\n",
>> +			       fadump_conf->cpu_state_data_version);
>> +			return 1;
>> +		}

I think cpu data version mismatch check should still be done early on?

>> +		fadump_conf->cpu_state_entry_size =
>> +			be32_to_cpu(r_opal_cpu_metadata->cpu_data_size);
>> +		fadump_conf->cpu_state_destination_addr =
>> +			be64_to_cpu(r_opal_cpu_metadata->region[0].dest);
>> +		fadump_conf->cpu_state_data_size =
>> +			be64_to_cpu(r_opal_cpu_metadata->region[0].size);
>> +
> 
> opal_fadump_dt_scan isn't the right place to do this. Can you please move above
> cpu related data processing to opal_fadump_build_cpu_notes() ?

I will move the above cpu related data processing to opal_fadump_build_cpu_notes().

Thanks
Hari


^ permalink raw reply

* RE: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC
From: Xiaowei Bao @ 2019-08-16  2:55 UTC (permalink / raw)
  To: Andrew Murray
  Cc: mark.rutland@arm.com, Roy Zang, lorenzo.pieralisi@arm.com,
	arnd@arndb.de, gregkh@linuxfoundation.org, jingoohan1@gmail.com,
	Z.q. Hou, linuxppc-dev@lists.ozlabs.org,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	kishon@ti.com, M.h. Lian, devicetree@vger.kernel.org,
	gustavo.pimentel@synopsys.com, Leo Li, shawnguo@kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190815113129.GF43882@e119886-lin.cambridge.arm.com>



> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月15日 19:32
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
> bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org;
> M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> Roy Zang <roy.zang@nxp.com>; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for
> DWC
> 
> On Thu, Aug 15, 2019 at 04:37:07PM +0800, Xiaowei Bao wrote:
> > Add multiple PFs support for DWC, different PF have different config
> > space, we use pf-offset property which get from the DTS to access the
> > different pF config space.
> 
> Thanks for the patch. I haven't seen a cover letter for this series, is there one
> missing?
Maybe I miss, I will add you to review next time, thanks a lot for your comments.
> 
> 
> >
> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware-ep.c |  97
> +++++++++++++---------
> >  drivers/pci/controller/dwc/pcie-designware.c    | 105
> ++++++++++++++++++++++--
> >  drivers/pci/controller/dwc/pcie-designware.h    |  10 ++-
> >  include/linux/pci-epc.h                         |   1 +
> >  4 files changed, 164 insertions(+), 49 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 2bf5a35..75e2955 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -19,12 +19,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> >  	pci_epc_linkup(epc);
> >  }
> >
> > -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno
> bar,
> > -				   int flags)
> > +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> > +				   enum pci_barno bar, int flags)
> >  {
> >  	u32 reg;
> > +	struct pci_epc *epc = pci->ep.epc;
> > +	u32 pf_base = func_no * epc->pf_offset;
> >
> > -	reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > +	reg = pf_base + PCI_BASE_ADDRESS_0 + (4 * bar);
> 
> I think I'd rather see this arithmetic (and the one for determining pf_base)
> inside a macro or inline header function. This would make this code more
> readable and reduce the chances of an error by avoiding duplication of code.
> 
> For example look at cdns_pcie_ep_fn_writeb and
> ROCKCHIP_PCIE_EP_FUNC_BASE for examples of other EP drivers that do
> this.
Agree, this looks fine, thanks a lot for your comments, I will use this way to access
the registers in next version patch.
> 
> 
> >  	dw_pcie_dbi_ro_wr_en(pci);
> >  	dw_pcie_writel_dbi2(pci, reg, 0x0);
> >  	dw_pcie_writel_dbi(pci, reg, 0x0);
> > @@ -37,7 +39,12 @@ static void __dw_pcie_ep_reset_bar(struct dw_pcie
> > *pci, enum pci_barno bar,
> >
> >  void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)  {
> > -	__dw_pcie_ep_reset_bar(pci, bar, 0);
> > +	u8 func_no, funcs;
> > +
> > +	funcs = pci->ep.epc->max_functions;
> > +
> > +	for (func_no = 0; func_no < funcs; func_no++)
> > +		__dw_pcie_ep_reset_bar(pci, func_no, bar, 0);
> >  }
> >
> >  static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
> > @@ -78,28 +85,29 @@ static int dw_pcie_ep_write_header(struct pci_epc
> > *epc, u8 func_no,  {
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +	u32 pf_base = func_no * epc->pf_offset;
> >
> >  	dw_pcie_dbi_ro_wr_en(pci);
> > -	dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, hdr->vendorid);
> > -	dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, hdr->deviceid);
> > -	dw_pcie_writeb_dbi(pci, PCI_REVISION_ID, hdr->revid);
> > -	dw_pcie_writeb_dbi(pci, PCI_CLASS_PROG, hdr->progif_code);
> > -	dw_pcie_writew_dbi(pci, PCI_CLASS_DEVICE,
> > +	dw_pcie_writew_dbi(pci, pf_base + PCI_VENDOR_ID, hdr->vendorid);
> > +	dw_pcie_writew_dbi(pci, pf_base + PCI_DEVICE_ID, hdr->deviceid);
> > +	dw_pcie_writeb_dbi(pci, pf_base + PCI_REVISION_ID, hdr->revid);
> > +	dw_pcie_writeb_dbi(pci, pf_base + PCI_CLASS_PROG, hdr->progif_code);
> > +	dw_pcie_writew_dbi(pci, pf_base + PCI_CLASS_DEVICE,
> >  			   hdr->subclass_code | hdr->baseclass_code << 8);
> > -	dw_pcie_writeb_dbi(pci, PCI_CACHE_LINE_SIZE,
> > +	dw_pcie_writeb_dbi(pci, pf_base + PCI_CACHE_LINE_SIZE,
> >  			   hdr->cache_line_size);
> > -	dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_VENDOR_ID,
> > +	dw_pcie_writew_dbi(pci, pf_base + PCI_SUBSYSTEM_VENDOR_ID,
> >  			   hdr->subsys_vendor_id);
> > -	dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_ID, hdr->subsys_id);
> > -	dw_pcie_writeb_dbi(pci, PCI_INTERRUPT_PIN,
> > +	dw_pcie_writew_dbi(pci, pf_base + PCI_SUBSYSTEM_ID,
> hdr->subsys_id);
> > +	dw_pcie_writeb_dbi(pci, pf_base + PCI_INTERRUPT_PIN,
> >  			   hdr->interrupt_pin);
> >  	dw_pcie_dbi_ro_wr_dis(pci);
> >
> >  	return 0;
> >  }
> >
> > -static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum
> pci_barno bar,
> > -				  dma_addr_t cpu_addr,
> > +static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> > +				  enum pci_barno bar, dma_addr_t cpu_addr,
> >  				  enum dw_pcie_as_type as_type)
> >  {
> >  	int ret;
> > @@ -112,7 +120,7 @@ static int dw_pcie_ep_inbound_atu(struct
> dw_pcie_ep *ep, enum pci_barno bar,
> >  		return -EINVAL;
> >  	}
> >
> > -	ret = dw_pcie_prog_inbound_atu(pci, free_win, bar, cpu_addr,
> > +	ret = dw_pcie_prog_inbound_atu(pci, func_no, free_win, bar,
> > +cpu_addr,
> >  				       as_type);
> >  	if (ret < 0) {
> >  		dev_err(pci->dev, "Failed to program IB window\n"); @@ -125,7
> > +133,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep,
> enum pci_barno bar,
> >  	return 0;
> >  }
> >
> > -static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, phys_addr_t
> > phys_addr,
> > +static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> > +				   phys_addr_t phys_addr,
> >  				   u64 pci_addr, size_t size)
> >  {
> >  	u32 free_win;
> > @@ -137,8 +146,8 @@ static int dw_pcie_ep_outbound_atu(struct
> dw_pcie_ep *ep, phys_addr_t phys_addr,
> >  		return -EINVAL;
> >  	}
> >
> > -	dw_pcie_prog_outbound_atu(pci, free_win, PCIE_ATU_TYPE_MEM,
> > -				  phys_addr, pci_addr, size);
> > +	dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win,
> PCIE_ATU_TYPE_MEM,
> > +				     phys_addr, pci_addr, size);
> >
> >  	set_bit(free_win, ep->ob_window_map);
> >  	ep->outbound_addr[free_win] = phys_addr; @@ -154,7 +163,7 @@
> static
> > void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no,
> >  	enum pci_barno bar = epf_bar->barno;
> >  	u32 atu_index = ep->bar_to_atu[bar];
> >
> > -	__dw_pcie_ep_reset_bar(pci, bar, epf_bar->flags);
> > +	__dw_pcie_ep_reset_bar(pci, func_no, bar, epf_bar->flags);
> >
> >  	dw_pcie_disable_atu(pci, atu_index, DW_PCIE_REGION_INBOUND);
> >  	clear_bit(atu_index, ep->ib_window_map); @@ -170,14 +179,16 @@
> > static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
> >  	size_t size = epf_bar->size;
> >  	int flags = epf_bar->flags;
> >  	enum dw_pcie_as_type as_type;
> > -	u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > +	u32 pf_base = func_no * epc->pf_offset;
> > +	u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar) + pf_base;
> >
> >  	if (!(flags & PCI_BASE_ADDRESS_SPACE))
> >  		as_type = DW_PCIE_AS_MEM;
> >  	else
> >  		as_type = DW_PCIE_AS_IO;
> >
> > -	ret = dw_pcie_ep_inbound_atu(ep, bar, epf_bar->phys_addr, as_type);
> > +	ret = dw_pcie_ep_inbound_atu(ep, func_no, bar,
> > +				     epf_bar->phys_addr, as_type);
> >  	if (ret)
> >  		return ret;
> >
> > @@ -235,7 +246,7 @@ static int dw_pcie_ep_map_addr(struct pci_epc
> *epc, u8 func_no,
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >
> > -	ret = dw_pcie_ep_outbound_atu(ep, addr, pci_addr, size);
> > +	ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
> >  	if (ret) {
> >  		dev_err(pci->dev, "Failed to enable address\n");
> >  		return ret;
> > @@ -248,12 +259,13 @@ static int dw_pcie_ep_get_msi(struct pci_epc
> > *epc, u8 func_no)  {
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +	u32 pf_base = func_no * epc->pf_offset;
> >  	u32 val, reg;
> >
> >  	if (!ep->msi_cap)
> >  		return -EINVAL;
> >
> > -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> > +	reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
> >  	val = dw_pcie_readw_dbi(pci, reg);
> >  	if (!(val & PCI_MSI_FLAGS_ENABLE))
> >  		return -EINVAL;
> > @@ -267,12 +279,13 @@ static int dw_pcie_ep_set_msi(struct pci_epc
> > *epc, u8 func_no, u8 interrupts)  {
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +	u32 pf_base = func_no * epc->pf_offset;
> >  	u32 val, reg;
> >
> >  	if (!ep->msi_cap)
> >  		return -EINVAL;
> >
> > -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> > +	reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
> >  	val = dw_pcie_readw_dbi(pci, reg);
> >  	val &= ~PCI_MSI_FLAGS_QMASK;
> >  	val |= (interrupts << 1) & PCI_MSI_FLAGS_QMASK; @@ -287,12 +300,13
> > @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no)  {
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +	u32 pf_base = func_no * epc->pf_offset;
> >  	u32 val, reg;
> >
> >  	if (!ep->msix_cap)
> >  		return -EINVAL;
> >
> > -	reg = ep->msix_cap + PCI_MSIX_FLAGS;
> > +	reg = ep->msix_cap + pf_base + PCI_MSIX_FLAGS;
> >  	val = dw_pcie_readw_dbi(pci, reg);
> >  	if (!(val & PCI_MSIX_FLAGS_ENABLE))
> >  		return -EINVAL;
> > @@ -306,12 +320,13 @@ static int dw_pcie_ep_set_msix(struct pci_epc
> > *epc, u8 func_no, u16 interrupts)  {
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +	u32 pf_base = func_no * epc->pf_offset;
> >  	u32 val, reg;
> >
> >  	if (!ep->msix_cap)
> >  		return -EINVAL;
> >
> > -	reg = ep->msix_cap + PCI_MSIX_FLAGS;
> > +	reg = ep->msix_cap + pf_base + PCI_MSIX_FLAGS;
> >  	val = dw_pcie_readw_dbi(pci, reg);
> >  	val &= ~PCI_MSIX_FLAGS_QSIZE;
> >  	val |= interrupts;
> > @@ -400,6 +415,7 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep
> *ep, u8 func_no,
> >  	unsigned int aligned_offset;
> >  	u16 msg_ctrl, msg_data;
> >  	u32 msg_addr_lower, msg_addr_upper, reg;
> > +	u32 pf_base = func_no * epc->pf_offset;
> >  	u64 msg_addr;
> >  	bool has_upper;
> >  	int ret;
> > @@ -408,19 +424,19 @@ int dw_pcie_ep_raise_msi_irq(struct
> dw_pcie_ep *ep, u8 func_no,
> >  		return -EINVAL;
> >
> >  	/* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
> > -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> > +	reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
> >  	msg_ctrl = dw_pcie_readw_dbi(pci, reg);
> >  	has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
> > -	reg = ep->msi_cap + PCI_MSI_ADDRESS_LO;
> > +	reg = ep->msi_cap + pf_base + PCI_MSI_ADDRESS_LO;
> >  	msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
> >  	if (has_upper) {
> > -		reg = ep->msi_cap + PCI_MSI_ADDRESS_HI;
> > +		reg = ep->msi_cap + pf_base + PCI_MSI_ADDRESS_HI;
> >  		msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
> > -		reg = ep->msi_cap + PCI_MSI_DATA_64;
> > +		reg = ep->msi_cap + pf_base + PCI_MSI_DATA_64;
> >  		msg_data = dw_pcie_readw_dbi(pci, reg);
> >  	} else {
> >  		msg_addr_upper = 0;
> > -		reg = ep->msi_cap + PCI_MSI_DATA_32;
> > +		reg = ep->msi_cap + pf_base + PCI_MSI_DATA_32;
> >  		msg_data = dw_pcie_readw_dbi(pci, reg);
> >  	}
> >  	aligned_offset = msg_addr_lower & (epc->mem->page_size - 1); @@
> > -439,7 +455,7 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep,
> > u8 func_no,  }
> >
> >  int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> > -			     u16 interrupt_num)
> > +			      u16 interrupt_num)
> >  {
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >  	struct pci_epc *epc = ep->epc;
> > @@ -447,16 +463,17 @@ int dw_pcie_ep_raise_msix_irq(struct
> dw_pcie_ep *ep, u8 func_no,
> >  	u32 bar_addr_upper, bar_addr_lower;
> >  	u32 msg_addr_upper, msg_addr_lower;
> >  	u32 reg, msg_data, vec_ctrl;
> > +	u32 pf_base = func_no * epc->pf_offset;
> >  	u64 tbl_addr, msg_addr, reg_u64;
> >  	void __iomem *msix_tbl;
> >  	int ret;
> >
> > -	reg = ep->msix_cap + PCI_MSIX_TABLE;
> > +	reg = ep->msix_cap + pf_base + PCI_MSIX_TABLE;
> >  	tbl_offset = dw_pcie_readl_dbi(pci, reg);
> >  	bir = (tbl_offset & PCI_MSIX_TABLE_BIR);
> >  	tbl_offset &= PCI_MSIX_TABLE_OFFSET;
> >
> > -	reg = PCI_BASE_ADDRESS_0 + (4 * bir);
> > +	reg = PCI_BASE_ADDRESS_0 + pf_base + (4 * bir);
> >  	bar_addr_upper = 0;
> >  	bar_addr_lower = dw_pcie_readl_dbi(pci, reg);
> >  	reg_u64 = (bar_addr_lower & PCI_BASE_ADDRESS_MEM_TYPE_MASK);
> @@
> > -592,13 +609,17 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> >  	ep->epc = epc;
> >  	epc_set_drvdata(epc, ep);
> >
> > -	if (ep->ops->ep_init)
> > -		ep->ops->ep_init(ep);
> > -
> >  	ret = of_property_read_u8(np, "max-functions", &epc->max_functions);
> >  	if (ret < 0)
> >  		epc->max_functions = 1;
> >
> > +	ret = of_property_read_u32(np, "pf-offset", &epc->pf_offset);
> > +	if (ret < 0)
> > +		epc->pf_offset = 0;
> 
> Bad things will likely happen if max_functions > 1 and pf-offset isn't set.
> I think the driver should bail in this situation. It would be very easy for
> someone to misconfigure this.
Yes, you are right, but if the max-functions have defined in DTS, require the pf-offset
must define in DTS, I am not sure the correct value of pf-offsetfor other platforms, 
so I think the max-functions and pf-offset should not have the dependence.
even though I didn't define pf-offset when I defined max-functions, the pf-offset is 0, 
the DWC ep driver can continue run the progress of INIT but not return, of course, 
thus the PF1 will not work, I don't know which way is better.
> 
> 
> > +
> > +	if (ep->ops->ep_init)
> > +		ep->ops->ep_init(ep);
> > +
> >  	ret = __pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
> >  				 ep->page_size);
> >  	if (ret < 0) {
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > b/drivers/pci/controller/dwc/pcie-designware.c
> > index 7d25102..c99cee4 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -158,6 +158,43 @@ static void dw_pcie_writel_ob_unroll(struct
> dw_pcie *pci, u32 index, u32 reg,
> >  	dw_pcie_writel_atu(pci, offset + reg, val);  }
> >
> > +static void dw_pcie_prog_ep_outbound_atu_unroll(struct dw_pcie *pci, u8
> func_no,
> > +						int index, int type,
> > +						u64 cpu_addr, u64 pci_addr,
> > +						u32 size)
> > +{
> > +	u32 retries, val;
> > +
> > +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_BASE,
> > +				 lower_32_bits(cpu_addr));
> > +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_BASE,
> > +				 upper_32_bits(cpu_addr));
> > +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LIMIT,
> > +				 lower_32_bits(cpu_addr + size - 1));
> > +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_TARGET,
> > +				 lower_32_bits(pci_addr));
> > +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
> > +				 upper_32_bits(pci_addr));
> > +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> > +				 type | PCIE_ATU_FUNC_NUM(func_no));
> 
> With the exception of this line, the rest of this function is identical to
> dw_pcie_prog_outbound_atu_unroll.
Yes, I can integrate the same code, but I think we'd better use the different outbound 
window set function between RC and EP, because the RC don't need the func_num parameter.
> 
> > +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
> > +				 PCIE_ATU_ENABLE);
> > +
> > +	/*
> > +	 * Make sure ATU enable takes effect before any subsequent config
> > +	 * and I/O accesses.
> > +	 */
> > +	for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
> > +		val = dw_pcie_readl_ob_unroll(pci, index,
> > +					      PCIE_ATU_UNR_REGION_CTRL2);
> > +		if (val & PCIE_ATU_ENABLE)
> > +			return;
> > +
> > +		mdelay(LINK_WAIT_IATU);
> > +	}
> > +	dev_err(pci->dev, "Outbound iATU is not being enabled\n"); }
> > +
> >  static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int
> index,
> >  					     int type, u64 cpu_addr,
> >  					     u64 pci_addr, u32 size)
> > @@ -194,6 +231,51 @@ static void
> dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
> >  	dev_err(pci->dev, "Outbound iATU is not being enabled\n");  }
> >
> > +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int
> index,
> > +				  int type, u64 cpu_addr, u64 pci_addr,
> > +				  u32 size)
> > +{
> > +	u32 retries, val;
> > +
> > +	if (pci->ops->cpu_addr_fixup)
> > +		cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr);
> > +
> > +	if (pci->iatu_unroll_enabled) {
> > +		dw_pcie_prog_ep_outbound_atu_unroll(pci, func_no, index, type,
> > +						    cpu_addr, pci_addr, size);
> > +		return;
> > +	}
> > +
> > +	dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT,
> > +			   PCIE_ATU_REGION_OUTBOUND | index);
> > +	dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_BASE,
> > +			   lower_32_bits(cpu_addr));
> > +	dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_BASE,
> > +			   upper_32_bits(cpu_addr));
> > +	dw_pcie_writel_dbi(pci, PCIE_ATU_LIMIT,
> > +			   lower_32_bits(cpu_addr + size - 1));
> > +	dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_TARGET,
> > +			   lower_32_bits(pci_addr));
> > +	dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_TARGET,
> > +			   upper_32_bits(pci_addr));
> > +	dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
> > +			   PCIE_ATU_FUNC_NUM(func_no));
> 
> The same here, this is identical to dw_pcie_prog_outbound_atu with the
> exception of this line.
> 
> Is there a way you can avoid all of this duplicated code?
As above, I can integrate the same code, but I keep to think the different outbound 
Window set function should be used between RC and EP.
> 
> Thanks,
> 
> Andrew Murray
> 
> > +	dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE);
> > +
> > +	/*
> > +	 * Make sure ATU enable takes effect before any subsequent config
> > +	 * and I/O accesses.
> > +	 */
> > +	for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
> > +		val = dw_pcie_readl_dbi(pci, PCIE_ATU_CR2);
> > +		if (val & PCIE_ATU_ENABLE)
> > +			return;
> > +
> > +		mdelay(LINK_WAIT_IATU);
> > +	}
> > +	dev_err(pci->dev, "Outbound iATU is not being enabled\n"); }
> > +
> >  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> >  			       u64 cpu_addr, u64 pci_addr, u32 size)  { @@ -252,8
> +334,8
> > @@ static void dw_pcie_writel_ib_unroll(struct dw_pcie *pci, u32 index,
> u32 reg,
> >  	dw_pcie_writel_atu(pci, offset + reg, val);  }
> >
> > -static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, int index,
> > -					   int bar, u64 cpu_addr,
> > +static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, u8
> func_no,
> > +					   int index, int bar, u64 cpu_addr,
> >  					   enum dw_pcie_as_type as_type)  {
> >  	int type;
> > @@ -275,8 +357,10 @@ static int dw_pcie_prog_inbound_atu_unroll(struct
> dw_pcie *pci, int index,
> >  		return -EINVAL;
> >  	}
> >
> > -	dw_pcie_writel_ib_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> type);
> > +	dw_pcie_writel_ib_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> type |
> > +				 PCIE_ATU_FUNC_NUM(func_no));
> >  	dw_pcie_writel_ib_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
> > +				 PCIE_ATU_FUNC_NUM_MATCH_EN |
> >  				 PCIE_ATU_ENABLE |
> >  				 PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> >
> > @@ -297,14 +381,15 @@ static int
> dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, int index,
> >  	return -EBUSY;
> >  }
> >
> > -int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int bar,
> > -			     u64 cpu_addr, enum dw_pcie_as_type as_type)
> > +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > +			     int bar, u64 cpu_addr,
> > +			     enum dw_pcie_as_type as_type)
> >  {
> >  	int type;
> >  	u32 retries, val;
> >
> >  	if (pci->iatu_unroll_enabled)
> > -		return dw_pcie_prog_inbound_atu_unroll(pci, index, bar,
> > +		return dw_pcie_prog_inbound_atu_unroll(pci, func_no, index, bar,
> >  						       cpu_addr, as_type);
> >
> >  	dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT,
> PCIE_ATU_REGION_INBOUND |
> > @@ -323,9 +408,11 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie
> *pci, int index, int bar,
> >  		return -EINVAL;
> >  	}
> >
> > -	dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type);
> > -	dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE
> > -			   | PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> > +	dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
> > +			   PCIE_ATU_FUNC_NUM(func_no));
> > +	dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE |
> > +			   PCIE_ATU_FUNC_NUM_MATCH_EN |
> > +			   PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> >
> >  	/*
> >  	 * Make sure ATU enable takes effect before any subsequent config
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index ffed084..2b291e8 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -71,9 +71,11 @@
> >  #define PCIE_ATU_TYPE_IO		0x2
> >  #define PCIE_ATU_TYPE_CFG0		0x4
> >  #define PCIE_ATU_TYPE_CFG1		0x5
> > +#define PCIE_ATU_FUNC_NUM(pf)           (pf << 20)
> >  #define PCIE_ATU_CR2			0x908
> >  #define PCIE_ATU_ENABLE			BIT(31)
> >  #define PCIE_ATU_BAR_MODE_ENABLE	BIT(30)
> > +#define PCIE_ATU_FUNC_NUM_MATCH_EN      BIT(19)
> >  #define PCIE_ATU_LOWER_BASE		0x90C
> >  #define PCIE_ATU_UPPER_BASE		0x910
> >  #define PCIE_ATU_LIMIT			0x914
> > @@ -265,8 +267,12 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
> > void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> >  			       int type, u64 cpu_addr, u64 pci_addr,
> >  			       u32 size);
> > -int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int bar,
> > -			     u64 cpu_addr, enum dw_pcie_as_type as_type);
> > +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int
> index,
> > +				  int type, u64 cpu_addr, u64 pci_addr,
> > +				  u32 size);
> > +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > +			     int bar, u64 cpu_addr,
> > +			     enum dw_pcie_as_type as_type);
> >  void dw_pcie_disable_atu(struct dw_pcie *pci, int index,
> >  			 enum dw_pcie_region_type type);
> >  void dw_pcie_setup(struct dw_pcie *pci); diff --git
> > a/include/linux/pci-epc.h b/include/linux/pci-epc.h index
> > f641bad..fc2feee 100644
> > --- a/include/linux/pci-epc.h
> > +++ b/include/linux/pci-epc.h
> > @@ -96,6 +96,7 @@ struct pci_epc {
> >  	const struct pci_epc_ops	*ops;
> >  	struct pci_epc_mem		*mem;
> >  	u8				max_functions;
> > +	u32				pf_offset;
> >  	struct config_group		*group;
> >  	/* spinlock to protect against concurrent access of EP controller */
> >  	spinlock_t			lock;
> > --
> > 2.9.5
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&amp;data=02%
> 7C0
> >
> 1%7Cxiaowei.bao%40nxp.com%7C0e39168f6f144db6840308d721742040%7
> C686ea1d
> >
> 3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637014654998524452&amp;sd
> ata=bP7eh
> > cjlGXCMVFE2b4f12Q6fGV7lQ%2F5i9qIi9FoPlbI%3D&amp;reserved=0

^ permalink raw reply

* RE: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
From: Xiaowei Bao @ 2019-08-16  2:58 UTC (permalink / raw)
  To: Andrew Murray
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	lorenzo.pieralisi@arm.com, arnd@arndb.de,
	gregkh@linuxfoundation.org, jingoohan1@gmail.com, Z.q. Hou,
	linuxppc-dev@lists.ozlabs.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, Leo Li, M.h. Lian,
	robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org,
	gustavo.pimentel@synopsys.com, bhelgaas@google.com, kishon@ti.com,
	shawnguo@kernel.org, Mingkai Hu
In-Reply-To: <20190815115340.GG43882@e119886-lin.cambridge.arm.com>



> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月15日 19:54
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
> bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org;
> M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> Roy Zang <roy.zang@nxp.com>; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of
> MSI-X in EP mode
> 
> On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote:
> > Add the doorbell mode of MSI-X in EP mode.
> >
> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++
> >  drivers/pci/controller/dwc/pcie-designware.h    | 14 ++++++++++++++
> >  2 files changed, 28 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 75e2955..e3a7cdf 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep
> *ep, u8 func_no,
> >  	return 0;
> >  }
> >
> > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8
> func_no,
> > +				       u16 interrupt_num)
> > +{
> > +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +	u32 msg_data;
> > +
> > +	msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) |
> > +		   (interrupt_num - 1);
> > +
> > +	dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data);
> > +
> > +	return 0;
> > +}
> > +
> >  int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> >  			      u16 interrupt_num)
> 
> Have I understood correctly that the hardware provides an alternative
> mechanism that allows for raising MSI-X interrupts without the bother of
> reading the capabilities registers?
Yes, the hardware provide two way to MSI-X, please check the page 492 of 
DWC_pcie_dm_registers_4.30 Menu.
MSIX_DOORBELL_OFF on page 492 0x948 Description: MSI-X Doorbell Register....>
> 
> If so is there any good reason to keep dw_pcie_ep_raise_msix_irq? (And thus
> use it in dw_plat_pcie_ep_raise_irq also)?
I am not sure, but I think the dw_pcie_ep_raise_msix_irq function is not correct, 
because I think we can't get the MSIX table from the address ep->phys_base + tbl_addr, 
but I also don't know where I can get the correct MSIX table.
> 
> 
> >  {
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > b/drivers/pci/controller/dwc/pcie-designware.h
> > index 2b291e8..cd903e9 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -88,6 +88,11 @@
> >  #define PCIE_MISC_CONTROL_1_OFF		0x8BC
> >  #define PCIE_DBI_RO_WR_EN		BIT(0)
> >
> > +#define PCIE_MSIX_DOORBELL		0x948
> > +#define PCIE_MSIX_DOORBELL_PF_SHIFT	24
> > +#define PCIE_MSIX_DOORBELL_VF_SHIFT	16
> > +#define PCIE_MSIX_DOORBELL_VF_ACTIVE	BIT(15)
> 
> The _VF defines are not used, I'd suggest removing them.
In fact, I will add the SRIOV support in this file, the SRIOV feature have verified 
In my board, but I need wait the EP framework SRIOV patch merge, 
so I defined these two macros.
> 
> Thanks,
> 
> Andrew Murray
> 
> > +
> >  /*
> >   * iATU Unroll-specific register definitions
> >   * From 4.80 core version the address translation will be made by
> > unroll @@ -399,6 +404,8 @@ int dw_pcie_ep_raise_msi_irq(struct
> dw_pcie_ep *ep, u8 func_no,
> >  			     u8 interrupt_num);
> >  int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> >  			     u16 interrupt_num);
> > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8
> func_no,
> > +				       u16 interrupt_num);
> >  void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
> > #else  static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) @@
> > -431,6 +438,13 @@ static inline int dw_pcie_ep_raise_msix_irq(struct
> dw_pcie_ep *ep, u8 func_no,
> >  	return 0;
> >  }
> >
> > +static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep
> *ep,
> > +						     u8 func_no,
> > +						     u16 interrupt_num)
> > +{
> > +	return 0;
> > +}
> > +
> >  static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum
> > pci_barno bar)  {  }
> > --
> > 2.9.5
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&amp;data=02%
> 7C0
> >
> 1%7Cxiaowei.bao%40nxp.com%7C8489493003bb48a0139d08d721773972%
> 7C686ea1d
> >
> 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637014668369499298&amp;sd
> ata=dyrXB
> >
> avljJBFUSNXW7K%2FRoXvwfWTE%2FoU2KMd1bZkJow%3D&amp;reserved=0

^ permalink raw reply

* RE: [PATCH 05/10] PCI: layerscape: Modify the way of getting capability with different PEX
From: Xiaowei Bao @ 2019-08-16  3:00 UTC (permalink / raw)
  To: Andrew Murray
  Cc: mark.rutland@arm.com, Roy Zang, lorenzo.pieralisi@arm.com,
	arnd@arndb.de, gregkh@linuxfoundation.org, jingoohan1@gmail.com,
	Z.q. Hou, linuxppc-dev@lists.ozlabs.org,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Leo Li,
	M.h. Lian, devicetree@vger.kernel.org, robh+dt@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	gustavo.pimentel@synopsys.com, bhelgaas@google.com, kishon@ti.com,
	shawnguo@kernel.org, Mingkai Hu
In-Reply-To: <20190815125103.GH43882@e119886-lin.cambridge.arm.com>



> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月15日 20:51
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
> bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org;
> M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> Roy Zang <roy.zang@nxp.com>; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 05/10] PCI: layerscape: Modify the way of getting
> capability with different PEX
> 
> On Thu, Aug 15, 2019 at 04:37:11PM +0800, Xiaowei Bao wrote:
> > The different PCIe controller in one board may be have different
> > capability of MSI or MSIX, so change the way of getting the MSI
> > capability, make it more flexible.
> >
> > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 28
> > +++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > index be61d96..9404ca0 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > @@ -22,6 +22,7 @@
> >
> >  struct ls_pcie_ep {
> >  	struct dw_pcie		*pci;
> > +	struct pci_epc_features	*ls_epc;
> >  };
> >
> >  #define to_ls_pcie_ep(x)	dev_get_drvdata((x)->dev)
> > @@ -40,25 +41,26 @@ static const struct of_device_id
> ls_pcie_ep_of_match[] = {
> >  	{ },
> >  };
> >
> > -static const struct pci_epc_features ls_pcie_epc_features = {
> > -	.linkup_notifier = false,
> > -	.msi_capable = true,
> > -	.msix_capable = false,
> > -};
> > -
> >  static const struct pci_epc_features*  ls_pcie_ep_get_features(struct
> > dw_pcie_ep *ep)  {
> > -	return &ls_pcie_epc_features;
> > +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +	struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > +
> > +	return pcie->ls_epc;
> >  }
> >
> >  static void ls_pcie_ep_init(struct dw_pcie_ep *ep)  {
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +	struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> >  	enum pci_barno bar;
> >
> >  	for (bar = BAR_0; bar <= BAR_5; bar++)
> >  		dw_pcie_ep_reset_bar(pci, bar);
> > +
> > +	pcie->ls_epc->msi_capable = ep->msi_cap ? true : false;
> > +	pcie->ls_epc->msix_capable = ep->msix_cap ? true : false;
> >  }
> >
> >  static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no, @@
> > -118,6 +120,7 @@ static int __init ls_pcie_ep_probe(struct platform_device
> *pdev)
> >  	struct device *dev = &pdev->dev;
> >  	struct dw_pcie *pci;
> >  	struct ls_pcie_ep *pcie;
> > +	struct pci_epc_features *ls_epc;
> >  	struct resource *dbi_base;
> >  	int ret;
> >
> > @@ -129,6 +132,10 @@ static int __init ls_pcie_ep_probe(struct
> platform_device *pdev)
> >  	if (!pci)
> >  		return -ENOMEM;
> >
> > +	ls_epc = devm_kzalloc(dev, sizeof(*ls_epc), GFP_KERNEL);
> > +	if (!ls_epc)
> > +		return -ENOMEM;
> > +
> >  	dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "regs");
> >  	pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> >  	if (IS_ERR(pci->dbi_base))
> > @@ -139,6 +146,13 @@ static int __init ls_pcie_ep_probe(struct
> platform_device *pdev)
> >  	pci->ops = &ls_pcie_ep_ops;
> >  	pcie->pci = pci;
> >
> > +	ls_epc->linkup_notifier = false,
> > +	ls_epc->msi_capable = true,
> > +	ls_epc->msix_capable = true,
> 
> As [msi,msix]_capable is shortly set from ls_pcie_ep_init - is there any reason
> to set them here (to potentially incorrect values)?
This is a INIT value, maybe false is better for msi_capable and msix_capable, 
of course, we don't need to set it.
> 
> Thanks,
> 
> Andrew Murray
> 
> > +	ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
> > +
> > +	pcie->ls_epc = ls_epc;
> > +
> >  	platform_set_drvdata(pdev, pcie);
> >
> >  	ret = ls_add_pcie_ep(pcie, pdev);
> > --
> > 2.9.5
> >

^ permalink raw reply

* [PATCH 0/3] series to optionally disable tlbie for 64s/radix
From: Nicholas Piggin @ 2019-08-16  4:07 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Aneesh Kumar K.V, kvm-ppc, Nicholas Piggin

Since the RFC I accounted for feedback, and also made the patch more
complete so we can actually boot and run a 64s/radix kernel without
using tlbie at all.

KVM and bare metal hash are harder to support. Bare metal hash because
it does some TLB invation with interrupts disabled (can't use IPIs).
It might be possible those invalidates could be avoided or the paths
changed to enable interrupts, but it would be a much bigger change.

KVM with radix guests might be acutally quite simple to support, so I
can look at that next if we get this merged.

KVM with hash guests might not be feasible to do in a performant way,
because of the design of virtualised hash architecture (host flushes
guest effective addresses), at least it would need special real mode
IPI handlers for TLB flushes.

Thanks,
Nick

Nicholas Piggin (3):
  powerpc/64s: Remove mmu_partition_table_set
  powerpc/64s/radix: all CPUs should flush local translation structure
    before turning MMU on
  powerpc/64s: introduce options to disable use of the tlbie instruction

 .../admin-guide/kernel-parameters.txt         |   4 +
 arch/powerpc/include/asm/book3s/64/tlbflush.h |   9 +
 arch/powerpc/include/asm/mmu.h                |   2 -
 arch/powerpc/kvm/book3s_hv.c                  |  10 +-
 arch/powerpc/kvm/book3s_hv_nested.c           |  35 +++-
 arch/powerpc/mm/book3s64/hash_utils.c         |   4 +-
 arch/powerpc/mm/book3s64/mmu_context.c        |   4 +-
 arch/powerpc/mm/book3s64/pgtable.c            |  77 +++++---
 arch/powerpc/mm/book3s64/radix_pgtable.c      |  22 +--
 arch/powerpc/mm/book3s64/radix_tlb.c          | 182 ++++++++++++++++--
 arch/powerpc/mm/pgtable_64.c                  |   5 +-
 drivers/misc/cxl/main.c                       |   4 +
 drivers/misc/ocxl/main.c                      |   4 +
 13 files changed, 282 insertions(+), 80 deletions(-)

-- 
2.22.0


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox