From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934709AbcKKEde (ORCPT ); Thu, 10 Nov 2016 23:33:34 -0500 Received: from mga06.intel.com ([134.134.136.31]:9966 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750846AbcKKEdd (ORCPT ); Thu, 10 Nov 2016 23:33:33 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,620,1473145200"; d="scan'208";a="29978044" Subject: Re: [PATCH v4 1/4] usb: dbc: early driver for xhci debug capability To: Peter Zijlstra , Thomas Gleixner References: <1477976354-13291-1-git-send-email-baolu.lu@linux.intel.com> <1477976354-13291-2-git-send-email-baolu.lu@linux.intel.com> <5823CB63.7090603@linux.intel.com> <20161110114445.GW3142@twins.programming.kicks-ass.net> Cc: Greg Kroah-Hartman , Mathias Nyman , Ingo Molnar , linux-usb@vger.kernel.org, x86@kernel.org, LKML From: Lu Baolu Message-ID: <58254A19.8030003@linux.intel.com> Date: Fri, 11 Nov 2016 12:33:29 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20161110114445.GW3142@twins.programming.kicks-ass.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Peter, On 11/10/2016 07:44 PM, Peter Zijlstra wrote: > On Thu, Nov 10, 2016 at 09:56:41AM +0100, Thomas Gleixner wrote: >> On Thu, 10 Nov 2016, Lu Baolu wrote: >>> This seems to be a common issue for all early printk drivers. >> No. The other early printk drivers like serial do not have that problem as >> they simply do: >> >> while (*buf) { >> while (inb(UART) & TX_BUSY) >> cpu_relax(); >> outb(*buf++, UART); >> } > Right, which is why actual UARTs rule. If only laptops still had pinouts > for them life would be sooooo much better. > > Ideally the USB debug port would be a virtual UART and its interface as > simple and robust. > >> The wait for the UART to become ready is independent of the context as it >> solely depends on the hardware. >> >> As a result you can see the output from irq/nmi intermingled with the one >> from thread context, but that's the only problem they have. >> >> The only thing you can do to make this work is to prevent printing in NMI >> context: >> >> write() >> { >> if (in_nmi()) >> return; >> >> raw_spinlock_irqsave(&lock, flags); >> .... >> >> That fully serializes the writes and just ignores NMI context printks. Not >> optimal, but I fear that's all you can do. > I would also suggest telling the hardware people they have designed > something near the brink of useless. If you cannot do random exception > context debugging (#DB, #NMI, #MCE etc..) then there's a whole host of > problems that simply cannot be debugged. > > Also note that kdb runs from NMI context, so you'll not be able to > support that either. > Things become complicated when it comes to USB debug port. But it's still addressable. At this time, we can do it like this. write() { if (in_nmi() && raw_spin_is_locked(&lock)) return; raw_spinlock_irqsave(&lock, flags); .... This will filter some messages from NMI handler in case that another thread is holding the spinlock. I have no idea about how much chance could a debug user faces this. But it might further be fixed with below enhancement. write() { if (in_nmi() && raw_spin_is_locked(&lock)) { produce_a_pending_item_in_ring(); return; } raw_spinlock_irqsave(&lock, flags); while (!pending_item_ring_is_empty) consume_a_pending_item_in_ring(); .... We can design the pending item ring in a producer-consumer model. It's easy to avoid race between the producer and consumer. Best regards, Lu Baolu