linux-kernel - Re: [PATCH v3 2/3] hvc_init(): Enforce one-time initialization.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAB8RdapuPRym2dH_9mgmGdzpCxnP1KpS+hS8JJ-geVkga5fMcw@mail.gmail.com>
Date:	Tue, 29 Nov 2011 09:04:28 -0800
From:	Miche Baker-Harvey <miche@...gle.com>
To:	Amit Shah <amit.shah@...hat.com>
Cc:	Rusty Russell <rusty@...tcorp.com.au>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	xen-devel@...ts.xensource.com,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	linux-kernel@...r.kernel.org,
	virtualization@...ts.linux-foundation.org,
	Anton Blanchard <anton@...ba.org>,
	Mike Waychison <mikew@...gle.com>,
	ppc-dev <linuxppc-dev@...ts.ozlabs.org>,
	Eric Northrup <digitaleric@...gle.com>
Subject: Re: [PATCH v3 2/3] hvc_init(): Enforce one-time initialization.

Amit,
We aren't using either QEMU or kvmtool, but we are using KVM.  All
theissues we are seeing happen when we try to establish multiple
virtioconsoles at boot time.  The command line isn't relevant, but I
cantell you the protocol that's passing between the host (kvm) and
theguest (see the end of this message).
We do go through the control_work_handler(), but it's not
providingsynchronization.  Here's a trace of the
control_work_handler() andhandle_control_message() calls; note that
there are two concurrentcalls to control_work_handler().
I decorated control_work_handler() with a "lifetime" marker, and
passedthis value to handle_control_message(), so we can see which
control
messages are being handled from which instance of
thecontrol_work_handler() thread.
Notice that we enter control_work_handler() a second time before
thehandling of the second PORT_ADD message is complete. The
firstCONSOLE_PORT message is handled by the second
control_work_handler()call, but the second is handled by the first
control_work_handler() call.

root@...buntu:~# dmesg | grep MBH[3371055.808738]
control_work_handler #1[3371055.809372] + #1 handle_control_message
PORT_ADD[3371055.810169] - handle_control_message
PORT_ADD[3371055.810170] + #1 handle_control_message PORT_ADD
[3371055.810244]  control_work_handler #2[3371055.810245] + #2
handle_control_message CONSOLE_PORT[3371055.810246]  got
hvc_ports_mutex[3371055.810578] - handle_control_message
PORT_ADD[3371055.810579] + #1 handle_control_message
CONSOLE_PORT[3371055.810580]  trylock of hvc_ports_mutex
failed[3371055.811352]  got hvc_ports_mutex[3371055.811370] -
handle_control_message CONSOLE_PORT[3371055.816609] -
handle_control_message CONSOLE_PORT  So, I'm guessing the bug is that
there shouldn't be two instances ofcontrol_work_handler() running
simultaneously?
Thanks,
Miche

Protocol:We set up the virtio console device registers during
initialization,specifying the multiport feature, and some number of
ports, n, where nis greater than 1.
In the guest, virtcons_probe() finds our device, and successfully
sendsthe VIRTIO_CONSOLE_DEVICE_READY=1 control message.
On the host, we receive the VIRTIO_CONSOLE_DEVICE_READY message,
andsend one VIRTIO_CONSOLE_PORT_ADD message via the Receive Control
queuefor each port in the number of ports.  These messages are
notserialized: they are all sent at once.
The VIRTIO_CONSOLE_PORT_ADD messages are received
inhandle_control_message() in virtio_console.c, and add_port() is
calledfor each.  After each port is added, the guest
sendsVIRTIO_CONSOLE_PORT_READY to the host, and in these messages, the
idof the port is included in the message.
On the host, in response to each VIRTIO_CONSOLE_PORT_READY=1
message,we may send a VIRTIO_CONSOLE_CONSOLE_PORT message.
On the guest, in response to each VIRTIO_CONSOLE_CONSOLE_PORT
message,init_port_console() is called on the individual port.  The
same id isused for these messages as was used for the PORT_READY
messages.
After each successful init_port_console(), the guest
sendsVIRTIO_CONSOLE_PORT_OPEN back to the host.

On Tue, Nov 29, 2011 at 6:21 AM, Amit Shah <amit.shah@...hat.com> wrote:
> Hi,
>
> On (Mon) 28 Nov 2011 [15:40:41], Miche Baker-Harvey wrote:
>> Amit,
>>
>> You said that the work would be serialized "due to port additions
>> being on work items on the same workqueue".  I'm not seeing that.
>
> You leave a lot of questions unanswered.  What's your environment?
> Are you hot-plugging ports?  Are you using qemu?  What's your command
> line?
>
>> I've double checked this by using a mutex_trylock in
>> hvc_console::hvc_alloc(), and here's the relevant output from dmesg:
>>
>> root@...buntu:~# dmesg | grep MBH
>> [3307216.210274] MBH: got hvc_ports_mutex
>> [3307216.210690] MBH: trylock of hvc_ports_mutex failed
>> [3307216.211143] MBH: got hvc_ports_mutex
>>
>> This is in a system with two virtio console ports, each of which is a
>> console.  I think if the VIRTIO_CONSOLE_CONSOLE_PORT message handling
>> were actually being serialized, the trylock should never fail.
>
> Agreed.
>
>> What's the source of the serialization for the workqueue items?  At
>> first reading it looks like the control_work_handler gets called for
>> each virtio interrupt?
>
> It all depends on how you add ports.  If you're using qemu, they
> happen via the control work handler.
>
>                Amit
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/