Re: [PATCH v4 1/4] usb: dbc: early driver for xhci debug capability

[Lu Baolu <baolu.lu@linux.intel.com>]

Hi,

On 11/10/2016 04:56 PM, Thomas Gleixner wrote:
> On Thu, 10 Nov 2016, Lu Baolu wrote:
>> On 11/09/2016 05:37 PM, Thomas Gleixner wrote:
>>> On Tue, 1 Nov 2016, Lu Baolu wrote:
>>>> +static void early_xdbc_write(struct console *con, const char *str, u32 n)
>>>> +{
>>>> +	int chunk, ret;
>>>> +	static char buf[XDBC_MAX_PACKET];
>>>> +	int use_cr = 0;
>>>> +
>>>> +	if (!xdbc.xdbc_reg)
>>>> +		return;
>>>> +	memset(buf, 0, XDBC_MAX_PACKET);
>>> How is that dealing with reentrancy?
>>>
>>> early_printk() does not protect against it. Peter has a patch to prevent
>>> concurrent access from different cpus, but it cannot and will never prevent
>>> reentrancy on the same cpu (interrupt, nmi).
>> I can use a spinlock_irq to protect reentrancy of interrupt on the same
>> cpu. But I have no idea about the nmi one.
> spinlock wont work due to NMIs.

Yes, of course.

>
>> 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);
>    }
>
> 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.

Yes, you are right.

>
> 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.

Yes. But I want to add a bit more.

write()
{
	if (in_nmi() && raw_spin_is_locked(&lock)) {
		trace("... ...");
		return;
	}

	raw_spinlock_irqsave(&lock, flags);
	....


Best regards,
Lu Baolu