LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v5 5/9] powerpc/mm/book3s64: Update tlb flush routines to take a page walk cache flush argument
From: Guenter Roeck @ 2021-05-15 16:35 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: npiggin, linux-mm, kaleshsingh, joel, akpm, linuxppc-dev
In-Reply-To: <20210422054323.150993-6-aneesh.kumar@linux.ibm.com>

On Thu, Apr 22, 2021 at 11:13:19AM +0530, Aneesh Kumar K.V wrote:
> No functional change in this patch
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  .../include/asm/book3s/64/tlbflush-radix.h    | 19 +++++++-----
>  arch/powerpc/include/asm/book3s/64/tlbflush.h | 23 ++++++++++++---
>  arch/powerpc/mm/book3s64/radix_hugetlbpage.c  |  4 +--
>  arch/powerpc/mm/book3s64/radix_tlb.c          | 29 +++++++------------
>  4 files changed, 42 insertions(+), 33 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> index 8b33601cdb9d..171441a43b35 100644
> --- a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
> @@ -56,15 +56,18 @@ static inline void radix__flush_all_lpid_guest(unsigned int lpid)
>  }
>  #endif
>  
> -extern void radix__flush_hugetlb_tlb_range(struct vm_area_struct *vma,
> -					   unsigned long start, unsigned long end);
> -extern void radix__flush_tlb_range_psize(struct mm_struct *mm, unsigned long start,
> -					 unsigned long end, int psize);
> -extern void radix__flush_pmd_tlb_range(struct vm_area_struct *vma,
> -				       unsigned long start, unsigned long end);
> -extern void radix__flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
> +void radix__flush_hugetlb_tlb_range(struct vm_area_struct *vma,
> +				    unsigned long start, unsigned long end,
> +				    bool flush_pwc);
> +void radix__flush_pmd_tlb_range(struct vm_area_struct *vma,
> +				unsigned long start, unsigned long end,
> +				bool flush_pwc);
> +void radix__flush_tlb_pwc_range_psize(struct mm_struct *mm, unsigned long start,
> +				      unsigned long end, int psize, bool flush_pwc);
> +void radix__flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
>  			    unsigned long end);
> -extern void radix__flush_tlb_kernel_range(unsigned long start, unsigned long end);
> +void radix__flush_tlb_kernel_range(unsigned long start, unsigned long end);
> +
>  
>  extern void radix__local_flush_tlb_mm(struct mm_struct *mm);
>  extern void radix__local_flush_all_mm(struct mm_struct *mm);
> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush.h b/arch/powerpc/include/asm/book3s/64/tlbflush.h
> index 215973b4cb26..f9f8a3a264f7 100644
> --- a/arch/powerpc/include/asm/book3s/64/tlbflush.h
> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush.h
> @@ -45,13 +45,30 @@ static inline void tlbiel_all_lpid(bool radix)
>  		hash__tlbiel_all(TLB_INVAL_SCOPE_LPID);
>  }
>  
> +static inline void flush_pmd_tlb_pwc_range(struct vm_area_struct *vma,
                 ^^^^
> +					   unsigned long start,
> +					   unsigned long end,
> +					   bool flush_pwc)
> +{
> +	if (radix_enabled())
> +		return radix__flush_pmd_tlb_range(vma, start, end, flush_pwc);
> +	return hash__flush_tlb_range(vma, start, end);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> +}

>  
>  #define __HAVE_ARCH_FLUSH_PMD_TLB_RANGE
>  static inline void flush_pmd_tlb_range(struct vm_area_struct *vma,
                 ^^^^
>  				       unsigned long start, unsigned long end)
> +{
> +	return flush_pmd_tlb_pwc_range(vma, start, end, false);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Doesn't that cause build warnings/errors all over the place ?

Guenter

^ permalink raw reply

* [Bug 213079] IRQ problems and crashes on a PowerMac G5 with 5.12.3
From: bugzilla-daemon @ 2021-05-15 14:50 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-213079-206035@https.bugzilla.kernel.org/>

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

Erhard F. (erhard_f@mailbox.org) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Kernel Version|5.13-rc1                    |5.12.3
            Summary|IRQ problems and crashes on |IRQ problems and crashes on
                   |a PowerMac G5 with 5.13-rc1 |a PowerMac G5 with 5.12.3

--- Comment #3 from Erhard F. (erhard_f@mailbox.org) ---
Some time after the "irq 63: nobody cared" on 5.12.3:

[...]
--- interrupt: 500
[<000000000e5af612>] .nvme_irq
[<000000000e5af612>] .nvme_irq
Disabling IRQ #63
Call Trace:
Kernel panic - not syncing: corrupted stack end detected inside scheduler
CPU: 0 PID: 105549 Comm: kworker/u4:1 Tainted: G        W        
5.12.3-gentoo-PowerMacG5 #2
Workqueue:  0x0 (flush-259:0)
[c000000078dc79f0] [c00000000053950c] .dump_stack+0xe0/0x13c (unreliable)
[c000000078dc7a90] [c000000000066074] .panic+0x168/0x430
[c000000078dc7b40] [c0000000007f19f0] .__schedule+0x80/0x848
[c000000078dc7c20] [c0000000007f2270] .schedule+0xb8/0x110
[c000000078dc7ca0] [c000000000086d18] .worker_thread+0x278/0x290
[c000000078dc7d60] [c00000000008e75c] .kthread+0x134/0x13c
[c000000078dc7e10] [c00000000000b1f4] .ret_from_kernel_thread+0x58/0x64
Rebooting in 120 seconds..

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* Re: [FSL P50x0] KVM HV doesn't work anymore
From: Christian Zigotzky @ 2021-05-15 13:46 UTC (permalink / raw)
  To: Christophe Leroy, linuxppc-dev, kvm-ppc@vger.kernel.org
  Cc: Darren Stevens, R.T.Dickinson, mad skateman, Christian Zigotzky
In-Reply-To: <9af2c1c9-2caf-120b-2f97-c7722274eee3@csgroup.eu>

On 15 May 2021 at 12:08pm Christophe Leroy wrote:
>
>
> Le 15/05/2021 à 11:48, Christian Zigotzky a écrit :
>> Hi All,
>>
>> I bisected today [1] and the bisecting itself was OK but the 
>> reverting of the bad commit doesn't solve the issue. Do you have an 
>> idea which commit could be resposible for this issue? Maybe the 
>> bisecting wasn't successful. I will look in the kernel git log. Maybe 
>> there is a commit that affected KVM HV on FSL P50x0 machines.
>
> If the uImage doesn't load, it may be because of the size of uImage.
>
> See https://github.com/linuxppc/issues/issues/208
>
> Is there a significant size difference with and without KVM HV ?
>
> Maybe you can try to remove another option to reduce the size of the 
> uImage.
I tried it but it doesn't solve the issue. The uImage works without KVM 
HV in a virtual e5500 QEMU machine.

-Christian
>
> Or if you are using gzipped uImage you can try with an lzma uImage. 
> You can find a way to get an lzma uImage here: 
> https://github.com/linuxppc/issues/issues/208#issuecomment-477479951
>
> Christophe
>
>>
>> Thanks,
>> Christian
>>
>> [1] 
>> https://forum.hyperion-entertainment.com/viewtopic.php?p=53209#p53209
>>
>> On 14 May 2021 at 10:10 am, Christian Zigotzky wrote:
>>> Hi All,
>>>
>>> The RC1 of kernel 5.13 doesn't boot in a virtual e5500 QEMU machine 
>>> with KVM HV anymore. I see in the serial console that the uImage 
>>> doesn't load. I use the following QEMU command for booting:
>>>
>>> qemu-system-ppc64 -M ppce500 -cpu e5500 -enable-kvm -m 1024 -kernel 
>>> uImage-5.13 -drive 
>>> format=raw,file=MintPPC32-X5000.img,index=0,if=virtio -netdev 
>>> user,id=mynet0 -device e1000,netdev=mynet0 -append "rw 
>>> root=/dev/vda" -device virtio-vga -device virtio-mouse-pci -device 
>>> virtio-keyboard-pci -device pci-ohci,id=newusb -device 
>>> usb-audio,bus=newusb.0 -smp 4
>>>
>>> The kernel boots without KVM HV.
>>>
>>> Have you already tested KVM HV with the kernel 5.13?
>>>
>>> Thanks,
>>> Christian
>>>
>>>


^ permalink raw reply

* [Bug 213079] IRQ problems and crashes on a PowerMac G5 with 5.13-rc1
From: bugzilla-daemon @ 2021-05-15 12:30 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-213079-206035@https.bugzilla.kernel.org/>

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

--- Comment #2 from Erhard F. (erhard_f@mailbox.org) ---
Hmm... Just also happened on 5.12.3. But without the Kernel panic (yet).

[...]
irq 63: nobody cared (try booting with the "irqpoll" option)
Call Trace:
CPU: 1 PID: 43491 Comm: emerge Tainted: G        W        
5.12.3-gentoo-PowerMacG5 #2
[c00000000ffefae0] [c00000000053950c] .dump_stack+0xe0/0x13c (unreliable)
[c00000000ffefb80] [c0000000000ddb68] .__report_bad_irq+0x34/0xf0
[c00000000ffefc20] [c0000000000dda50] .note_interrupt+0x250/0x2f8
[c00000000ffefce0] [c0000000000d9cf8] .handle_irq_event_percpu+0x64/0x90
[c00000000ffefd70] [c0000000000d9d68] .handle_irq_event+0x44/0x70
[c00000000ffefe00] [c0000000000df164] .handle_fasteoi_irq+0xac/0x158
[c00000000ffefea0] [c0000000000d8db8] .generic_handle_irq+0x38/0x58
[c00000000ffeff10] [c000000000011314] .__do_irq+0x15c/0x238
[c00000000ffeff90] [c00000000001fe04] .call_do_irq+0x14/0x24
[c000000056e2fd70] [c00000000001154c] .do_IRQ+0x15c/0x164
[c000000056e2fe10] [c000000000007d38]
hardware_interrupt_common_virt+0x158/0x160
--- interrupt: 500 at 0x3fffb8a21520
handlers:
NIP:  00003fffb8a21520 LR: 00003fffb8a214a0 CTR: 00003fffb8ae6d20
REGS: c000000056e2fe80 TRAP: 0500   Tainted: G        W         
(5.12.3-gentoo-PowerMacG5)
MSR:  900000000200f032 <SF,HV,VEC,EE,PR,FP,ME,IR,DR,RI>  CR: 42482824  XER:
20000000
IRQMASK: 0 
GPR00: 00003fffb8a214a0 00003fffdb199650 00003fffb8df7200 000000014e8ddc60 
GPR04: 00003fffb210e000 95bfd31b66b69e10 00003fffdb199478 0000000000024d50 
GPR08: 000000014cb987c0 0000000000000002 0000000000000000 0000000000000000 
GPR12: 00003fffb8ae0e50 00003fffb8eb4850 00003fffdb199a58 000000014e8ddf60 
GPR16: 00003fffdb199a70 ffffffffffffffff 0000000000000001 000000014b5d8460 
GPR20: 0000000000000000 0000000000000002 000000014e8ddf38 00003fffb6b176e8 
GPR24: 000000014c126958 00003fffb2030390 000000014b94c380 000000014b5d8460 
GPR28: 000000014c1267f0 000000014c126a60 000000014c1267f0 0000000000000000 
NIP [00003fffb8a21520] 0x3fffb8a21520
LR [00003fffb8a214a0] 0x3fffb8a214a0
--- interrupt: 500
[<000000000e5af612>] .nvme_irq
[<000000000e5af612>] .nvme_irq
Disabling IRQ #63

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* [Bug 213079] IRQ problems and crashes on a PowerMac G5 with 5.13-rc1
From: bugzilla-daemon @ 2021-05-15 11:58 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-213079-206035@https.bugzilla.kernel.org/>

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

--- Comment #1 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 296761
  --> https://bugzilla.kernel.org/attachment.cgi?id=296761&action=edit
kernel .config (5.13-rc1, PowerMac G5 11,2)

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* [Bug 213079] New: IRQ problems and crashes on a PowerMac G5 with 5.13-rc1
From: bugzilla-daemon @ 2021-05-15 11:58 UTC (permalink / raw)
  To: linuxppc-dev

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

            Bug ID: 213079
           Summary: IRQ problems and crashes on a PowerMac G5 with
                    5.13-rc1
           Product: Platform Specific/Hardware
           Version: 2.5
    Kernel Version: 5.13-rc1
          Hardware: PPC-64
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: PPC-64
          Assignee: platform_ppc-64@kernel-bugs.osdl.org
          Reporter: erhard_f@mailbox.org
        Regression: No

Created attachment 296759
  --> https://bugzilla.kernel.org/attachment.cgi?id=296759&action=edit
dmesg (5.13-rc1, PowerMac G5 11,2)

With v5.13-rc1 I get IRQ problems and crashes on my G5 sooner or later. IRQ 63
is my NVMe SSD.

[...]
irq 63: nobody cared (try booting with the "irqpoll" option)
CPU: 1 PID: 11783 Comm: emerge Tainted: G        W        
5.13.0-rc1-PowerMacG5 #3
Call Trace:
[c00000000ffefae0] [c000000000549790] .dump_stack+0xe0/0x13c (unreliable)
[c00000000ffefb80] [c0000000000def44] .__report_bad_irq+0x34/0xf0
[c00000000ffefc20] [c0000000000dee2c] .note_interrupt+0x258/0x300
[c00000000ffefce0] [c0000000000db0a8] .handle_irq_event_percpu+0x64/0x90
[c00000000ffefd70] [c0000000000db118] .handle_irq_event+0x44/0x70
[c00000000ffefe00] [c0000000000e0530] .handle_fasteoi_irq+0xac/0x158
[c00000000ffefea0] [c0000000000da164] .generic_handle_irq+0x38/0x58
[c00000000ffeff10] [c000000000011674] .__do_irq+0x15c/0x238
[c00000000ffeff90] [c000000000012068] .do_IRQ+0x180/0x188
[c00000014d357d70] [c000000000011f88] .do_IRQ+0xa0/0x188
[c00000014d357e10] [c000000000007f94]
hardware_interrupt_common_virt+0x1a4/0x1b0
--- interrupt: 500 at 0x3fffb07a1a9c
NIP:  00003fffb07a1a9c LR: 00003fffb07a3d08 CTR: 00003fffb074cb30
REGS: c00000014d357e80 TRAP: 0500   Tainted: G        W         
(5.13.0-rc1-PowerMacG5)
MSR:  900000000000f032 <SF,HV,EE,PR,FP,ME,IR,DR,RI>  CR: 22482820  XER:
20000000
IRQMASK: 0 
GPR00: 00003fffb07a3d08 00003fffe84d07a0 00003fffb0ad1200 00003fffa8131100 
GPR04: 00003fffa9ea4bd0 a5a8b016e7fdc57d 00003fffe84d0810 00003fffb0aa7ac0 
GPR08: 00003fffb0ab3708 00003fffab4eb870 0000000000000000 0000000000000000 
GPR12: 00003fffb07b92a0 00003fffb0b8e850 00003fffe84d0a58 000000014df42388 
GPR16: 00003fffe84d0a70 ffffffffffffffff 00003fffafbf54c0 ffffffffffffffff 
GPR20: 0000000000000000 000000014df42338 000000014c677878 0000000000000000 
GPR24: 00003fffafc0b5b0 000000014c677830 00003fffafcc8a50 a5a8b016e7fdc57d 
GPR28: 00003fffa863bcc0 00003fffa8131100 00003fffa9ea4bd0 00003fffa8131100 
NIP [00003fffb07a1a9c] 0x3fffb07a1a9c
LR [00003fffb07a3d08] 0x3fffb07a3d08
--- interrupt: 500
handlers:
[<00000000370eb0ba>] .nvme_irq
[<00000000370eb0ba>] .nvme_irq
Disabling IRQ #63
Call Trace:
Kernel panic - not syncing: corrupted stack end detected inside scheduler
CPU: 0 PID: 814 Comm: kworker/u4:2 Tainted: G        W        
5.13.0-rc1-PowerMacG5 #3
Workqueue: writeback .wb_workfn (flush-254:1)
[c00000007db5ab40] [c000000000549790] .dump_stack+0xe0/0x13c (unreliable)
[c00000007db5abe0] [c0000000000680dc] .panic+0x168/0x430
[c00000007db5ac90] [c000000000811e40] .__schedule+0x80/0x840
[c00000007db5ad70] [c00000000081274c] .preempt_schedule_common+0x28/0x48
[c00000007db5adf0] [c00000000081279c] .__cond_resched+0x30/0x4c
[c00000007db5ae70] [c0000000001c6a98] .mempool_alloc+0x38/0x1a4
[c00000007db5af50] [c0000000004a1a70] .bio_alloc_bioset+0x94/0x174
[c00000007db5b000] [c000000000354840] .ext4_bio_write_page+0x314/0x480
[c00000007db5b0c0] [c0000000003334d4] .mpage_submit_page+0x70/0xa0
[c00000007db5b140] [c000000000333630] .mpage_process_page_bufs+0x12c/0x18c
[c00000007db5b1d0] [c0000000003338b8] .mpage_prepare_extent_to_map+0x1f8/0x228
[c00000007db5b320] [c000000000339088] .ext4_writepages+0x360/0xe5c
[c00000007db5b5d0] [c0000000001cee84] .do_writepages+0x54/0xa0
[c00000007db5b650] [c0000000002a49bc] .__writeback_single_inode+0x100/0x560
[c00000007db5b700] [c0000000002a53d8] .writeback_sb_inodes+0x2dc/0x4c8
[c00000007db5b880] [c0000000002a5654] .__writeback_inodes_wb+0x90/0xcc
[c00000007db5b930] [c0000000002a58c0] .wb_writeback+0x230/0x3dc
[c00000007db5ba50] [c0000000002a6790] .wb_workfn+0x380/0x460
[c00000007db5bbb0] [c0000000000890a0] .process_one_work+0x318/0x4dc
[c00000007db5bca0] [c000000000089730] .worker_thread+0x224/0x290
[c00000007db5bd60] [c000000000091200] .kthread+0x134/0x13c
[c00000007db5be10] [c00000000000bbf4] .ret_from_kernel_thread+0x58/0x64
Rebooting in 120 seconds..


 # lspci -vv -s 0001:08:00.0
0001:08:00.0 Non-Volatile memory controller: Intel Corporation SSD Pro
7600p/760p/E 6100p Series (rev 03) (prog-if 02 [NVM Express])
        Subsystem: Intel Corporation SSD Pro 7600p/760p/E 6100p Series [NVM
Express]
        Device tree node:
/sys/firmware/devicetree/base/ht@0,f2000000/pci@5/pci8086,390b@0
        Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-
        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx+
        Latency: 0, Cache Line Size: 64 bytes
        Interrupt: pin A routed to IRQ 63
        NUMA node: 0
        Region 0: Memory at a0000000 (64-bit, non-prefetchable) [size=16K]
        Capabilities: [40] Power Management version 3
                Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
        Capabilities: [50] MSI: Enable- Count=1/8 Maskable+ 64bit+
                Address: 0000000000000000  Data: 0000
                Masking: 00000000  Pending: 00000000
        Capabilities: [70] Express (v2) Endpoint, MSI 00
                DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s
unlimited, L1 unlimited
                        ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
SlotPowerLimit 0.000W
                DevCtl: CorrErr- NonFatalErr- FatalErr- UnsupReq-
                        RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop- FLReset-
                        MaxPayload 128 bytes, MaxReadReq 512 bytes
                DevSta: CorrErr- NonFatalErr- FatalErr- UnsupReq- AuxPwr+
TransPend-
                LnkCap: Port #0, Speed 8GT/s, Width x4, ASPM L1, Exit Latency
L1 <8us
                        ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp+
                LnkCtl: ASPM Disabled; RCB 64 bytes, Disabled- CommClk-
                        ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
                LnkSta: Speed 2.5GT/s (downgraded), Width x4 (ok)
                        TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
                DevCap2: Completion Timeout: Range ABCD, TimeoutDis+ NROPrPrP-
LTR+
                         10BitTagComp- 10BitTagReq- OBFF Not Supported, ExtFmt-
EETLPPrefix-
                         EmergencyPowerReduction Not Supported,
EmergencyPowerReductionInit-
                         FRS- TPHComp- ExtTPHComp-
                         AtomicOpsCap: 32bit- 64bit- 128bitCAS-
                DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis- LTR-
OBFF Disabled,
                         AtomicOpsCtl: ReqEn-
                LnkCap2: Supported Link Speeds: 2.5-8GT/s, Crosslink- Retimer-
2Retimers- DRS-
                LnkCtl2: Target Link Speed: 8GT/s, EnterCompliance- SpeedDis-
                         Transmit Margin: Normal Operating Range,
EnterModifiedCompliance- ComplianceSOS-
                         Compliance De-emphasis: -6dB
                LnkSta2: Current De-emphasis Level: -3.5dB,
EqualizationComplete- EqualizationPhase1-
                         EqualizationPhase2- EqualizationPhase3-
LinkEqualizationRequest-
                         Retimer- 2Retimers- CrosslinkRes: unsupported
        Capabilities: [b0] MSI-X: Enable- Count=16 Masked-
                Vector table: BAR=0 offset=00002000
                PBA: BAR=0 offset=00002100
        Kernel driver in use: nvme

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* Re: [PATCH 15/31] KVM: PPC: Book3S HV: XIVE: Fix mapping of passthrough interrupts
From: Marc Zyngier @ 2021-05-15 10:40 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linuxppc-dev, Cédric Le Goater
In-Reply-To: <87im3l2g48.ffs@nanos.tec.linutronix.de>

On Fri, 14 May 2021 21:51:51 +0100,
Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> On Fri, Apr 30 2021 at 10:03, Cédric Le Goater wrote:
> 
> CC: +Marc

Thanks Thomas.

> 
> > PCI MSI interrupt numbers are now mapped in a PCI-MSI domain but the
> > underlying calls handling the passthrough of the interrupt in the
> > guest need a number in the XIVE IRQ domain.
> >
> > Use the IRQ data mapped in the XIVE IRQ domain and not the one in the
> > PCI-MSI domain.
> >
> > Exporting irq_get_default_host() might not be the best solution.
> >
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Paul Mackerras <paulus@ozlabs.org>
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
> >  arch/powerpc/kvm/book3s_xive.c | 3 ++-
> >  kernel/irq/irqdomain.c         | 1 +
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> > index 3a7da42bed57..81b9f4fc3978 100644
> > --- a/arch/powerpc/kvm/book3s_xive.c
> > +++ b/arch/powerpc/kvm/book3s_xive.c
> > @@ -861,7 +861,8 @@ int kvmppc_xive_set_mapped(struct kvm *kvm, unsigned long guest_irq,
> >  	struct kvmppc_xive *xive = kvm->arch.xive;
> >  	struct kvmppc_xive_src_block *sb;
> >  	struct kvmppc_xive_irq_state *state;
> > -	struct irq_data *host_data = irq_get_irq_data(host_irq);
> > +	struct irq_data *host_data =
> > +		irq_domain_get_irq_data(irq_get_default_host(), host_irq);
> >  	unsigned int hw_irq = (unsigned int)irqd_to_hwirq(host_data);
> >  	u16 idx;
> >  	u8 prio;
> > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> > index d10ab1d689d5..8a073d1ce611 100644
> > --- a/kernel/irq/irqdomain.c
> > +++ b/kernel/irq/irqdomain.c
> > @@ -481,6 +481,7 @@ struct irq_domain *irq_get_default_host(void)
> >  {
> >  	return irq_default_domain;
> >  }
> > +EXPORT_SYMBOL_GPL(irq_get_default_host);
> >  
> >  static void irq_domain_clear_mapping(struct irq_domain *domain,
> >  				     irq_hw_number_t hwirq)
> 

Is there any reason why we should add more users of the "default host"
fallback? I would really hope that new code would actually track their
irqdomain in a more fine-grained way, specially when using the
hierarchical MSi setup, which seems to be the goal of this series.

Don't you have enough topology information that you can make use of to
correctly assign a domain identifier (of_node or otherwise)?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply

* [Bug 213069] kernel BUG at arch/powerpc/include/asm/book3s/64/hash-4k.h:147! Oops: Exception in kernel mode, sig: 5 [#1]
From: bugzilla-daemon @ 2021-05-15 10:20 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-213069-206035@https.bugzilla.kernel.org/>

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

--- Comment #1 from Erhard F. (erhard_f@mailbox.org) ---
The bug occurs with DEBUG_VM_PGTABLE=y. When DEBUG_VM_PGTABLE is not set the
kernel boots fine.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* Re: [FSL P50x0] KVM HV doesn't work anymore
From: Christophe Leroy @ 2021-05-15 10:08 UTC (permalink / raw)
  To: Christian Zigotzky, linuxppc-dev, kvm-ppc@vger.kernel.org
  Cc: Darren Stevens, R.T.Dickinson, mad skateman, Christian Zigotzky
In-Reply-To: <34617b1b-e213-668b-05f6-6fce7b549bf0@xenosoft.de>



Le 15/05/2021 à 11:48, Christian Zigotzky a écrit :
> Hi All,
> 
> I bisected today [1] and the bisecting itself was OK but the reverting of the bad commit doesn't 
> solve the issue. Do you have an idea which commit could be resposible for this issue? Maybe the 
> bisecting wasn't successful. I will look in the kernel git log. Maybe there is a commit that 
> affected KVM HV on FSL P50x0 machines.

If the uImage doesn't load, it may be because of the size of uImage.

See https://github.com/linuxppc/issues/issues/208

Is there a significant size difference with and without KVM HV ?

Maybe you can try to remove another option to reduce the size of the uImage.

Or if you are using gzipped uImage you can try with an lzma uImage. You can find a way to get an 
lzma uImage here: https://github.com/linuxppc/issues/issues/208#issuecomment-477479951

Christophe

> 
> Thanks,
> Christian
> 
> [1] https://forum.hyperion-entertainment.com/viewtopic.php?p=53209#p53209
> 
> On 14 May 2021 at 10:10 am, Christian Zigotzky wrote:
>> Hi All,
>>
>> The RC1 of kernel 5.13 doesn't boot in a virtual e5500 QEMU machine with KVM HV anymore. I see in 
>> the serial console that the uImage doesn't load. I use the following QEMU command for booting:
>>
>> qemu-system-ppc64 -M ppce500 -cpu e5500 -enable-kvm -m 1024 -kernel uImage-5.13 -drive 
>> format=raw,file=MintPPC32-X5000.img,index=0,if=virtio -netdev user,id=mynet0 -device 
>> e1000,netdev=mynet0 -append "rw root=/dev/vda" -device virtio-vga -device virtio-mouse-pci -device 
>> virtio-keyboard-pci -device pci-ohci,id=newusb -device usb-audio,bus=newusb.0 -smp 4
>>
>> The kernel boots without KVM HV.
>>
>> Have you already tested KVM HV with the kernel 5.13?
>>
>> Thanks,
>> Christian
>>
>>

^ permalink raw reply

* Re: [FSL P50x0] KVM HV doesn't work anymore
From: Christian Zigotzky @ 2021-05-15  9:48 UTC (permalink / raw)
  To: linuxppc-dev, kvm-ppc@vger.kernel.org
  Cc: Darren Stevens, mad skateman, R.T.Dickinson, Christian Zigotzky
In-Reply-To: <04526309-4653-3349-b6de-e7640c2258d6@xenosoft.de>

Hi All,

I bisected today [1] and the bisecting itself was OK but the reverting 
of the bad commit doesn't solve the issue. Do you have an idea which 
commit could be resposible for this issue? Maybe the bisecting wasn't 
successful. I will look in the kernel git log. Maybe there is a commit 
that affected KVM HV on FSL P50x0 machines.

Thanks,
Christian

[1] https://forum.hyperion-entertainment.com/viewtopic.php?p=53209#p53209

On 14 May 2021 at 10:10 am, Christian Zigotzky wrote:
> Hi All,
>
> The RC1 of kernel 5.13 doesn't boot in a virtual e5500 QEMU machine 
> with KVM HV anymore. I see in the serial console that the uImage 
> doesn't load. I use the following QEMU command for booting:
>
> qemu-system-ppc64 -M ppce500 -cpu e5500 -enable-kvm -m 1024 -kernel 
> uImage-5.13 -drive 
> format=raw,file=MintPPC32-X5000.img,index=0,if=virtio -netdev 
> user,id=mynet0 -device e1000,netdev=mynet0 -append "rw root=/dev/vda" 
> -device virtio-vga -device virtio-mouse-pci -device 
> virtio-keyboard-pci -device pci-ohci,id=newusb -device 
> usb-audio,bus=newusb.0 -smp 4
>
> The kernel boots without KVM HV.
>
> Have you already tested KVM HV with the kernel 5.13?
>
> Thanks,
> Christian
>
>


^ permalink raw reply

* Re: [PATCH] Raise the minimum GCC version to 5.2
From: Joe Perches @ 2021-05-15  7:27 UTC (permalink / raw)
  To: Pavel Machek, Masahiro Yamada
  Cc: Albert Ou, Arnd Bergmann, linux-kbuild, Greg Kroah-Hartman,
	Jonathan Corbet, linux-doc, linux-kernel, Linus Torvalds,
	Palmer Dabbelt, Paul Walmsley, Catalin Marinas, Miguel Ojeda,
	Paul Mackerras, linux-riscv, linuxppc-dev, Will Deacon,
	linux-arm-kernel
In-Reply-To: <20210515071410.GA27201@amd>

On Sat, 2021-05-15 at 09:14 +0200, Pavel Machek wrote:
> On Sun 2021-05-02 00:15:38, Masahiro Yamada wrote:
> > The current minimum GCC version is 4.9 except ARCH=arm64 requiring
> > GCC 5.1.
> 
> Please don't. I'm still on 4.9 on machine I can't easily update,

Why is that?  Later compiler versions are available.
http://cdn.kernel.org/pub/tools/crosstool/

Is there some other reason your machine can not have the compiler
version updated?



^ permalink raw reply

* Re: [PATCH] Raise the minimum GCC version to 5.2
From: Pavel Machek @ 2021-05-15  7:14 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Albert Ou, Arnd Bergmann, linux-kbuild, Greg Kroah-Hartman,
	Jonathan Corbet, linux-doc, linux-kernel, Linus Torvalds,
	Palmer Dabbelt, Paul Walmsley, Catalin Marinas, Miguel Ojeda,
	Paul Mackerras, linux-riscv, linuxppc-dev, Will Deacon,
	linux-arm-kernel
In-Reply-To: <20210501151538.145449-1-masahiroy@kernel.org>

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

On Sun 2021-05-02 00:15:38, Masahiro Yamada wrote:
> The current minimum GCC version is 4.9 except ARCH=arm64 requiring
> GCC 5.1.

Please don't. I'm still on 4.9 on machine I can't easily update,

>  Documentation/process/changes.rst | 2 +-
>  arch/arm64/Kconfig                | 2 +-
>  arch/powerpc/Kconfig              | 2 +-
>  arch/riscv/Kconfig                | 2 +-
>  include/linux/compiler-gcc.h      | 6 +-----
>  lib/Kconfig.debug                 | 2 +-
>  scripts/min-tool-version.sh       | 8 +-------
>  7 files changed, 7 insertions(+), 17 deletions(-)

and 10 lines of cleanups is really not worth that.

Best regards,
								Pavel

-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH] arm64: Define only {pud/pmd}_{set/clear}_huge when usefull
From: Christophe Leroy @ 2021-05-15  6:28 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-arch, naresh.kamboju, linux-kernel, Nicholas Piggin,
	linux-mm, linux-arm-kernel, sparclinux, linuxppc-dev,
	Mike Rapoport, Mike Kravetz
In-Reply-To: <20210514144200.b49ee77c9b2a7f9998ffbf22@linux-foundation.org>



Le 14/05/2021 à 23:42, Andrew Morton a écrit :
> On Fri, 14 May 2021 11:08:53 +0000 (UTC) Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> 
>> When PUD and/or PMD are folded, those functions are useless
>> and we now have a stub in linux/pgtable.h
> 
> OK, help me out here please.  What patch does this fix?
> 

Both this one and the x86 one from the day before fix 1cff41494b15cd82c1ec418bb5c ("mm/pgtable: add 
stubs for {pmd/pub}_{set/clear}_huge")

I think both the x86 fix and the arm64 fix should be squashed into that patch at the end.

I checked, the only other architecture involving pud_set_huge() and friends is powerpc, and powerpc 
doesn't have this problem as it only defined those for book3s/64 platforms which have 4 level page 
tables by definition.

Thanks
Christophe

^ permalink raw reply

* Re: [PATCH v2 04/14] PCI/MSI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-15  6:01 UTC (permalink / raw)
  To: Joe Perches
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Vidya Sagar,
	Oliver O'Halloran, Bjorn Helgaas, Paul Mackerras,
	Kurt Schwemmer, Logan Gunthorpe, Xiongfeng Wang
In-Reply-To: <d156a893bae41967e9fadddb3397cf47bcdde239.camel@perches.com>

Hi Joe,

[...]
> >  	if (entry)
> > -		return sprintf(buf, "%s\n",
> > -				entry->msi_attrib.is_msix ? "msix" : "msi");
> > +		return sysfs_emit(buf, "%s\n",
> > +				  entry->msi_attrib.is_msix ? "msix" : "msi");
> >  
> > 
> >  	return -ENODEV;
> >  }
> 
> trivia: reversing the test would be more common style
> 
> 	if (!entry)
> 		return -ENODEV;
> 
> 	return sysfs_emit(...);
> }

Excellent point.  I will send v3 later that includes this style change.

Thank you!

Krzysztof

^ permalink raw reply

* Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-15  5:59 UTC (permalink / raw)
  To: Joe Perches
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Vidya Sagar,
	Oliver O'Halloran, Bjorn Helgaas, Paul Mackerras,
	Kurt Schwemmer, Logan Gunthorpe, Xiongfeng Wang
In-Reply-To: <985813cafbbe58cd899737ee49b44798210a69f6.camel@perches.com>

Hi Joe,

[...]
> Ideally, the additional newline check below this would use sysfs_emit_at
> 
> drivers/pci/pci.c-      /*
> drivers/pci/pci.c:       * When set by the command line, resource_alignment_param will not
> drivers/pci/pci.c-       * have a trailing line feed, which is ugly. So conditionally add
> drivers/pci/pci.c-       * it here.
> drivers/pci/pci.c-       */
> drivers/pci/pci.c-      if (count >= 2 && buf[count - 2] != '\n' && count < PAGE_SIZE - 1) {
> drivers/pci/pci.c-              buf[count - 1] = '\n';
> drivers/pci/pci.c-              buf[count++] = 0;
> drivers/pci/pci.c-      }
> drivers/pci/pci.c-
> drivers/pci/pci.c-      return count;

I found some inconsistencies with adding newline this way, and decided
to change the code slightly, see:

  https://lore.kernel.org/linux-pci/20210515052434.1413236-12-kw@linux.com/

Krzysztof

^ permalink raw reply

* Re: [PATCH v2 04/14] PCI/MSI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Joe Perches @ 2021-05-15  5:46 UTC (permalink / raw)
  To: Krzysztof Wilczyński, Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Vidya Sagar,
	Oliver O'Halloran, Paul Mackerras, Kurt Schwemmer,
	Logan Gunthorpe, Xiongfeng Wang
In-Reply-To: <20210515052434.1413236-4-kw@linux.com>

On Sat, 2021-05-15 at 05:24 +0000, Krzysztof Wilczyński wrote:
> The sysfs_emit() and sysfs_emit_at() functions were introduced to make
> it less ambiguous which function is preferred when writing to the output
> buffer in a device attribute's "show" callback [1].
> 
> Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
> and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
> latter is aware of the PAGE_SIZE buffer and correctly returns the number
> of bytes written into the buffer.
> 
> No functional change intended.
[]
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
[]
> @@ -465,8 +465,8 @@ static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr,
>  
>  	entry = irq_get_msi_desc(irq);
>  	if (entry)
> -		return sprintf(buf, "%s\n",
> -				entry->msi_attrib.is_msix ? "msix" : "msi");
> +		return sysfs_emit(buf, "%s\n",
> +				  entry->msi_attrib.is_msix ? "msix" : "msi");
>  
> 
>  	return -ENODEV;
>  }

trivia: reversing the test would be more common style

	if (!entry)
		return -ENODEV;

	return sysfs_emit(...);
}



^ permalink raw reply

* Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Joe Perches @ 2021-05-15  5:43 UTC (permalink / raw)
  To: Krzysztof Wilczyński, Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Vidya Sagar,
	Oliver O'Halloran, Paul Mackerras, Kurt Schwemmer,
	Logan Gunthorpe, Xiongfeng Wang
In-Reply-To: <20210515052434.1413236-1-kw@linux.com>

On Sat, 2021-05-15 at 05:24 +0000, Krzysztof Wilczyński wrote:
> The sysfs_emit() and sysfs_emit_at() functions were introduced to make
> it less ambiguous which function is preferred when writing to the output
> buffer in a device attribute's "show" callback [1].
> 
> Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
> and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
> latter is aware of the PAGE_SIZE buffer and correctly returns the number
> of bytes written into the buffer.
[]
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
[]
> @@ -6439,7 +6439,7 @@ static ssize_t resource_alignment_show(struct bus_type *bus, char *buf)
>  
> 
>  	spin_lock(&resource_alignment_lock);
>  	if (resource_alignment_param)
> -		count = scnprintf(buf, PAGE_SIZE, "%s", resource_alignment_param);
> +		count = sysfs_emit(buf, "%s", resource_alignment_param);
>  	spin_unlock(&resource_alignment_lock);

Ideally, the additional newline check below this would use sysfs_emit_at

drivers/pci/pci.c-      /*
drivers/pci/pci.c:       * When set by the command line, resource_alignment_param will not
drivers/pci/pci.c-       * have a trailing line feed, which is ugly. So conditionally add
drivers/pci/pci.c-       * it here.
drivers/pci/pci.c-       */
drivers/pci/pci.c-      if (count >= 2 && buf[count - 2] != '\n' && count < PAGE_SIZE - 1) {
drivers/pci/pci.c-              buf[count - 1] = '\n';
drivers/pci/pci.c-              buf[count++] = 0;
drivers/pci/pci.c-      }
drivers/pci/pci.c-
drivers/pci/pci.c-      return count;
	


^ permalink raw reply

* Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-15  5:36 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Vidya Sagar,
	Oliver O'Halloran, Joe Perches, Paul Mackerras,
	Kurt Schwemmer, Logan Gunthorpe, Xiongfeng Wang
In-Reply-To: <20210515052434.1413236-1-kw@linux.com>

Hello,

[...]
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Please disregard this "Reviewed-by" from Logan for this version, as I've
forgotten to remove it before sending v2 after pulling patches using b4.

Apologies.

Krzysztof

^ permalink raw reply

* [PATCH v2 14/14] PCI/sysfs: Only show value when driver_override is not NULL
From: Krzysztof Wilczyński @ 2021-05-15  5:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Vidya Sagar,
	Oliver O'Halloran, Joe Perches, Paul Mackerras,
	Kurt Schwemmer, Logan Gunthorpe, Xiongfeng Wang
In-Reply-To: <20210515052434.1413236-1-kw@linux.com>

Only expose the value of the "driver_override" variable through the
corresponding sysfs object when a value is actually set.

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/pci-sysfs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 5d63df7c1820..4e9f582ca10f 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -580,10 +580,11 @@ static ssize_t driver_override_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
-	ssize_t len;
+	ssize_t len = 0;
 
 	device_lock(dev);
-	len = sysfs_emit(buf, "%s\n", pdev->driver_override);
+	if (pdev->driver_override)
+		len = sysfs_emit(buf, "%s\n", pdev->driver_override);
 	device_unlock(dev);
 	return len;
 }
-- 
2.31.1


^ permalink raw reply related

* [PATCH v2 13/14] PCI/sysfs: Add missing trailing newline to devspec_show()
From: Krzysztof Wilczyński @ 2021-05-15  5:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Vidya Sagar,
	Oliver O'Halloran, Joe Perches, Paul Mackerras,
	Kurt Schwemmer, Logan Gunthorpe, Xiongfeng Wang
In-Reply-To: <20210515052434.1413236-1-kw@linux.com>

At the moment, when the value of the "devspec" sysfs object is read from
the user space there will be no newline present, and the utilities such
as the "cat" command won't display the result of the read correctly in
a shell, as the trailing newline is currently missing.

To fix this, append a newline character in the show() function.

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/pci-sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index beb8d1f4fafe..5d63df7c1820 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -537,7 +537,7 @@ static ssize_t devspec_show(struct device *dev,
 
 	if (np == NULL)
 		return 0;
-	return sysfs_emit(buf, "%pOF", np);
+	return sysfs_emit(buf, "%pOF\n", np);
 }
 static DEVICE_ATTR_RO(devspec);
 #endif
-- 
2.31.1


^ permalink raw reply related

* [PATCH v2 12/14] PCI: Fix trailing newline handling of resource_alignment_param
From: Krzysztof Wilczyński @ 2021-05-15  5:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Vidya Sagar,
	Oliver O'Halloran, Joe Perches, Paul Mackerras,
	Kurt Schwemmer, Logan Gunthorpe, Xiongfeng Wang
In-Reply-To: <20210515052434.1413236-1-kw@linux.com>

The value of the "resource_alignment" can be specified using a kernel
command-line argument (using the "pci=resource_alignment=") or through
the corresponding sysfs object under the /sys/bus/pci path.

Currently, when the value is set via the kernel command-line argument,
and then subsequently accessed through sysfs object, the value read back
will not be correct, as per:

  # grep -oE 'pci=resource_alignment.+' /proc/cmdline
  pci=resource_alignment=20@00:1f.2
  # cat /sys/bus/pci/resource_alignment
  20@00:1f.

This is also true when the value is set through the sysfs object, but
the trailing newline has not been included, as per:

  # echo -n 20@00:1f.2 > /sys/bus/pci/resource_alignment
  # cat /sys/bus/pci/resource_alignment
  20@00:1f.

When the value set through the sysfs object includes the trailing
newline, then reading it back will work as intended, as per:

  # echo 20@00:1f.2 > /sys/bus/pci/resource_alignment
  # cat /sys/bus/pci/resource_alignment
  20@00:1f.2

To fix this inconsistency, append a trailing newline in the show()
function and strip the trailing line in the store() function if one is
present.

Also, allow for the value previously set using either a command-line
argument or through the sysfs object to be cleared at run-time.

Fixes: e499081da1a2 ("PCI: Force trailing new line to resource_alignment_param in sysfs")
Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/pci.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 5ed316ea5831..7cde86bdcc8e 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6439,34 +6439,37 @@ static ssize_t resource_alignment_show(struct bus_type *bus, char *buf)
 
 	spin_lock(&resource_alignment_lock);
 	if (resource_alignment_param)
-		count = sysfs_emit(buf, "%s", resource_alignment_param);
+		count = sysfs_emit(buf, "%s\n", resource_alignment_param);
 	spin_unlock(&resource_alignment_lock);
 
-	/*
-	 * When set by the command line, resource_alignment_param will not
-	 * have a trailing line feed, which is ugly. So conditionally add
-	 * it here.
-	 */
-	if (count >= 2 && buf[count - 2] != '\n' && count < PAGE_SIZE - 1) {
-		buf[count - 1] = '\n';
-		buf[count++] = 0;
-	}
-
 	return count;
 }
 
 static ssize_t resource_alignment_store(struct bus_type *bus,
 					const char *buf, size_t count)
 {
-	char *param = kstrndup(buf, count, GFP_KERNEL);
+	char *param, *old, *end;
 
+	param = kstrndup(buf, count, GFP_KERNEL);
 	if (!param)
 		return -ENOMEM;
 
+	end = strchr(param, '\n');
+	if (end)
+		*end = '\0';
+
 	spin_lock(&resource_alignment_lock);
-	kfree(resource_alignment_param);
-	resource_alignment_param = param;
+	old = resource_alignment_param;
+	if (strlen(param)) {
+		resource_alignment_param = param;
+	} else {
+		kfree(resource_alignment_param);
+		resource_alignment_param = NULL;
+	}
 	spin_unlock(&resource_alignment_lock);
+
+	kfree(old);
+
 	return count;
 }
 
-- 
2.31.1


^ permalink raw reply related

* [PATCH v2 11/14] PCI: shpchp: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-15  5:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Vidya Sagar,
	Oliver O'Halloran, Joe Perches, Paul Mackerras,
	Kurt Schwemmer, Logan Gunthorpe, Xiongfeng Wang
In-Reply-To: <20210515052434.1413236-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/hotplug/shpchp_sysfs.c | 38 +++++++++++++++++-------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/hotplug/shpchp_sysfs.c b/drivers/pci/hotplug/shpchp_sysfs.c
index 45658bb5c554..64beed7a26be 100644
--- a/drivers/pci/hotplug/shpchp_sysfs.c
+++ b/drivers/pci/hotplug/shpchp_sysfs.c
@@ -24,50 +24,54 @@
 static ssize_t show_ctrl(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct pci_dev *pdev;
-	char *out = buf;
 	int index, busnr;
 	struct resource *res;
 	struct pci_bus *bus;
+	size_t len = 0;
 
 	pdev = to_pci_dev(dev);
 	bus = pdev->subordinate;
 
-	out += sprintf(buf, "Free resources: memory\n");
+	len += sysfs_emit_at(buf, len, "Free resources: memory\n");
 	pci_bus_for_each_resource(bus, res, index) {
 		if (res && (res->flags & IORESOURCE_MEM) &&
 				!(res->flags & IORESOURCE_PREFETCH)) {
-			out += sprintf(out, "start = %8.8llx, length = %8.8llx\n",
-				       (unsigned long long)res->start,
-				       (unsigned long long)resource_size(res));
+			len += sysfs_emit_at(buf, len,
+					     "start = %8.8llx, length = %8.8llx\n",
+					     (unsigned long long)res->start,
+					     (unsigned long long)resource_size(res));
 		}
 	}
-	out += sprintf(out, "Free resources: prefetchable memory\n");
+	len += sysfs_emit_at(buf, len, "Free resources: prefetchable memory\n");
 	pci_bus_for_each_resource(bus, res, index) {
 		if (res && (res->flags & IORESOURCE_MEM) &&
 			       (res->flags & IORESOURCE_PREFETCH)) {
-			out += sprintf(out, "start = %8.8llx, length = %8.8llx\n",
-				       (unsigned long long)res->start,
-				       (unsigned long long)resource_size(res));
+			len += sysfs_emit_at(buf, len,
+					     "start = %8.8llx, length = %8.8llx\n",
+					     (unsigned long long)res->start,
+					     (unsigned long long)resource_size(res));
 		}
 	}
-	out += sprintf(out, "Free resources: IO\n");
+	len += sysfs_emit_at(buf, len, "Free resources: IO\n");
 	pci_bus_for_each_resource(bus, res, index) {
 		if (res && (res->flags & IORESOURCE_IO)) {
-			out += sprintf(out, "start = %8.8llx, length = %8.8llx\n",
-				       (unsigned long long)res->start,
-				       (unsigned long long)resource_size(res));
+			len += sysfs_emit_at(buf, len,
+					     "start = %8.8llx, length = %8.8llx\n",
+					     (unsigned long long)res->start,
+					     (unsigned long long)resource_size(res));
 		}
 	}
-	out += sprintf(out, "Free resources: bus numbers\n");
+	len += sysfs_emit_at(buf, len, "Free resources: bus numbers\n");
 	for (busnr = bus->busn_res.start; busnr <= bus->busn_res.end; busnr++) {
 		if (!pci_find_bus(pci_domain_nr(bus), busnr))
 			break;
 	}
 	if (busnr < bus->busn_res.end)
-		out += sprintf(out, "start = %8.8x, length = %8.8x\n",
-				busnr, (int)(bus->busn_res.end - busnr));
+		len += sysfs_emit_at(buf, len,
+				     "start = %8.8x, length = %8.8x\n",
+				     busnr, (int)(bus->busn_res.end - busnr));
 
-	return out - buf;
+	return len;
 }
 static DEVICE_ATTR(ctrl, S_IRUGO, show_ctrl, NULL);
 
-- 
2.31.1


^ permalink raw reply related

* [PATCH v2 10/14] PCI: hotplug: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-15  5:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Vidya Sagar,
	Oliver O'Halloran, Joe Perches, Paul Mackerras,
	Kurt Schwemmer, Logan Gunthorpe, Xiongfeng Wang
In-Reply-To: <20210515052434.1413236-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/hotplug/pci_hotplug_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 5ac31f683b85..058d5937d8a9 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -73,7 +73,7 @@ static ssize_t power_read_file(struct pci_slot *pci_slot, char *buf)
 	if (retval)
 		return retval;
 
-	return sprintf(buf, "%d\n", value);
+	return sysfs_emit(buf, "%d\n", value);
 }
 
 static ssize_t power_write_file(struct pci_slot *pci_slot, const char *buf,
@@ -130,7 +130,7 @@ static ssize_t attention_read_file(struct pci_slot *pci_slot, char *buf)
 	if (retval)
 		return retval;
 
-	return sprintf(buf, "%d\n", value);
+	return sysfs_emit(buf, "%d\n", value);
 }
 
 static ssize_t attention_write_file(struct pci_slot *pci_slot, const char *buf,
@@ -175,7 +175,7 @@ static ssize_t latch_read_file(struct pci_slot *pci_slot, char *buf)
 	if (retval)
 		return retval;
 
-	return sprintf(buf, "%d\n", value);
+	return sysfs_emit(buf, "%d\n", value);
 }
 
 static struct pci_slot_attribute hotplug_slot_attr_latch = {
@@ -192,7 +192,7 @@ static ssize_t presence_read_file(struct pci_slot *pci_slot, char *buf)
 	if (retval)
 		return retval;
 
-	return sprintf(buf, "%d\n", value);
+	return sysfs_emit(buf, "%d\n", value);
 }
 
 static struct pci_slot_attribute hotplug_slot_attr_presence = {
-- 
2.31.1


^ permalink raw reply related

* [PATCH v2 09/14] PCI: rpadlpar: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-15  5:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Vidya Sagar,
	Oliver O'Halloran, Joe Perches, Paul Mackerras,
	Kurt Schwemmer, Logan Gunthorpe, Xiongfeng Wang
In-Reply-To: <20210515052434.1413236-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/hotplug/rpadlpar_sysfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/hotplug/rpadlpar_sysfs.c b/drivers/pci/hotplug/rpadlpar_sysfs.c
index dbfa0b55d31a..068b7810a574 100644
--- a/drivers/pci/hotplug/rpadlpar_sysfs.c
+++ b/drivers/pci/hotplug/rpadlpar_sysfs.c
@@ -50,7 +50,7 @@ static ssize_t add_slot_store(struct kobject *kobj, struct kobj_attribute *attr,
 static ssize_t add_slot_show(struct kobject *kobj,
 			     struct kobj_attribute *attr, char *buf)
 {
-	return sprintf(buf, "0\n");
+	return sysfs_emit(buf, "0\n");
 }
 
 static ssize_t remove_slot_store(struct kobject *kobj,
@@ -80,7 +80,7 @@ static ssize_t remove_slot_store(struct kobject *kobj,
 static ssize_t remove_slot_show(struct kobject *kobj,
 				struct kobj_attribute *attr, char *buf)
 {
-	return sprintf(buf, "0\n");
+	return sysfs_emit(buf, "0\n");
 }
 
 static struct kobj_attribute add_slot_attr =
-- 
2.31.1


^ permalink raw reply related

* [PATCH v2 08/14] PCI: switchtec: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-15  5:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, linuxppc-dev, Vidya Sagar,
	Oliver O'Halloran, Joe Perches, Paul Mackerras,
	Kurt Schwemmer, Logan Gunthorpe, Xiongfeng Wang
In-Reply-To: <20210515052434.1413236-1-kw@linux.com>

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/switch/switchtec.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index ba52459928f7..0b301f8be9ed 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -280,7 +280,7 @@ static ssize_t device_version_show(struct device *dev,
 
 	ver = ioread32(&stdev->mmio_sys_info->device_version);
 
-	return sprintf(buf, "%x\n", ver);
+	return sysfs_emit(buf, "%x\n", ver);
 }
 static DEVICE_ATTR_RO(device_version);
 
@@ -292,7 +292,7 @@ static ssize_t fw_version_show(struct device *dev,
 
 	ver = ioread32(&stdev->mmio_sys_info->firmware_version);
 
-	return sprintf(buf, "%08x\n", ver);
+	return sysfs_emit(buf, "%08x\n", ver);
 }
 static DEVICE_ATTR_RO(fw_version);
 
@@ -344,7 +344,7 @@ static ssize_t component_vendor_show(struct device *dev,
 
 	/* component_vendor field not supported after gen3 */
 	if (stdev->gen != SWITCHTEC_GEN3)
-		return sprintf(buf, "none\n");
+		return sysfs_emit(buf, "none\n");
 
 	return io_string_show(buf, &si->gen3.component_vendor,
 			      sizeof(si->gen3.component_vendor));
@@ -359,9 +359,9 @@ static ssize_t component_id_show(struct device *dev,
 
 	/* component_id field not supported after gen3 */
 	if (stdev->gen != SWITCHTEC_GEN3)
-		return sprintf(buf, "none\n");
+		return sysfs_emit(buf, "none\n");
 
-	return sprintf(buf, "PM%04X\n", id);
+	return sysfs_emit(buf, "PM%04X\n", id);
 }
 static DEVICE_ATTR_RO(component_id);
 
@@ -373,9 +373,9 @@ static ssize_t component_revision_show(struct device *dev,
 
 	/* component_revision field not supported after gen3 */
 	if (stdev->gen != SWITCHTEC_GEN3)
-		return sprintf(buf, "255\n");
+		return sysfs_emit(buf, "255\n");
 
-	return sprintf(buf, "%d\n", rev);
+	return sysfs_emit(buf, "%d\n", rev);
 }
 static DEVICE_ATTR_RO(component_revision);
 
@@ -384,7 +384,7 @@ static ssize_t partition_show(struct device *dev,
 {
 	struct switchtec_dev *stdev = to_stdev(dev);
 
-	return sprintf(buf, "%d\n", stdev->partition);
+	return sysfs_emit(buf, "%d\n", stdev->partition);
 }
 static DEVICE_ATTR_RO(partition);
 
@@ -393,7 +393,7 @@ static ssize_t partition_count_show(struct device *dev,
 {
 	struct switchtec_dev *stdev = to_stdev(dev);
 
-	return sprintf(buf, "%d\n", stdev->partition_count);
+	return sysfs_emit(buf, "%d\n", stdev->partition_count);
 }
 static DEVICE_ATTR_RO(partition_count);
 
-- 
2.31.1


^ permalink raw reply related


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