[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000
From: |
liu ping fan |
Subject: |
Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000 |
Date: |
Mon, 29 Oct 2012 13:24:24 +0800 |
On Thu, Oct 25, 2012 at 9:34 PM, Jan Kiszka <address@hidden> wrote:
> On 2012-10-24 09:29, liu ping fan wrote:
>> On Tue, Oct 23, 2012 at 5:04 PM, Jan Kiszka <address@hidden> wrote:
>>> On 2012-10-22 11:23, Liu Ping Fan wrote:
>>>> Use local lock to protect e1000. When calling the system function,
>>>> dropping the fine lock before acquiring the big lock. This will
>>>> introduce broken device state, which need extra effort to fix.
>>>>
>>>> Signed-off-by: Liu Ping Fan <address@hidden>
>>>> ---
>>>> hw/e1000.c | 24 +++++++++++++++++++++++-
>>>> 1 files changed, 23 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/hw/e1000.c b/hw/e1000.c
>>>> index ae8a6c5..5eddab5 100644
>>>> --- a/hw/e1000.c
>>>> +++ b/hw/e1000.c
>>>> @@ -85,6 +85,7 @@ typedef struct E1000State_st {
>>>> NICConf conf;
>>>> MemoryRegion mmio;
>>>> MemoryRegion io;
>>>> + QemuMutex e1000_lock;
>>>>
>>>> uint32_t mac_reg[0x8000];
>>>> uint16_t phy_reg[0x20];
>>>> @@ -223,13 +224,27 @@ static const uint32_t mac_reg_init[] = {
>>>> static void
>>>> set_interrupt_cause(E1000State *s, int index, uint32_t val)
>>>> {
>>>> + QemuThread *t;
>>>> +
>>>> if (val && (E1000_DEVID >= E1000_DEV_ID_82547EI_MOBILE)) {
>>>> /* Only for 8257x */
>>>> val |= E1000_ICR_INT_ASSERTED;
>>>> }
>>>> s->mac_reg[ICR] = val;
>>>> s->mac_reg[ICS] = val;
>>>> - qemu_set_irq(s->dev.irq[0], (s->mac_reg[IMS] & s->mac_reg[ICR]) != 0);
>>>> +
>>>> + t = pthread_getspecific(qemu_thread_key);
>>>> + if (t->context_type == 1) {
>>>> + qemu_mutex_unlock(&s->e1000_lock);
>>>> + qemu_mutex_lock_iothread();
>>>> + }
>>>> + if (DEVICE(s)->state < DEV_STATE_STOPPING) {
>>>> + qemu_set_irq(s->dev.irq[0], (s->mac_reg[IMS] & s->mac_reg[ICR])
>>>> != 0);
>>>> + }
>>>> + if (t->context_type == 1) {
>>>> + qemu_mutex_unlock_iothread();
>>>> + qemu_mutex_lock(&s->e1000_lock);
>>>> + }
>>>
>>> This is ugly for many reasons. First of all, it is racy as the register
>>> content may change while dropping the device lock, no? Then you would
>>> raise or clear an IRQ spuriously.
>>>
>>> Second, it clearly shows that we need to address lock-less IRQ delivery.
>>> Almost nothing is won if we have to take the global lock again to push
>>> an IRQ event to the guest. I'm repeating myself, but the problem to be
>>> solved here is almost identical to fast IRQ delivery for assigned
>>> devices (which we only address pretty ad-hoc for PCI so far).
>>>
>> Interesting, could you show me more detail about it, so I can google...
>
> No need to look that far, just grep for pci_device_route_intx_to_irq,
> pci_device_set_intx_routing_notifier and related functions in the code.
>
I think, the major point here is to bypass the delivery process among
the irq chipset during runtime. Right?
Thanks and regards,
pingfan
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, (continued)
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Avi Kivity, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Avi Kivity, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, liu ping fan, 2012/10/29
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, liu ping fan, 2012/10/24
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Avi Kivity, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Avi Kivity, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000,
liu ping fan <=
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/31
[Qemu-devel] [patch v4 03/16] hotplug: introduce qdev_unplug_complete() to remove device from views, Liu Ping Fan, 2012/10/22
[Qemu-devel] [patch v4 01/16] atomic: introduce atomic operations, Liu Ping Fan, 2012/10/22
[Qemu-devel] [patch v4 13/16] e1000: add busy flag to anti broken device state, Liu Ping Fan, 2012/10/22
- Re: [Qemu-devel] [patch v4 13/16] e1000: add busy flag to anti broken device state, Avi Kivity, 2012/10/22
- Re: [Qemu-devel] [patch v4 13/16] e1000: add busy flag to anti broken device state, liu ping fan, 2012/10/23
- Re: [Qemu-devel] [patch v4 13/16] e1000: add busy flag to anti broken device state, Avi Kivity, 2012/10/23
- Re: [Qemu-devel] [patch v4 13/16] e1000: add busy flag to anti broken device state, Jan Kiszka, 2012/10/23
- Re: [Qemu-devel] [patch v4 13/16] e1000: add busy flag to anti broken device state, liu ping fan, 2012/10/23
- Re: [Qemu-devel] [patch v4 13/16] e1000: add busy flag to anti broken device state, Avi Kivity, 2012/10/23