Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-03-09 Thread Wen Congyang
At 03/10/2011 12:31 PM, Ryan Harper Write:
> * Wen Congyang  [2011-03-09 01:21]:
>> At 03/09/2011 02:12 PM, Ryan Harper Write:
>>> * Wen Congyang  [2011-03-08 23:09]:
 At 03/09/2011 12:08 PM, Ryan Harper Write:
> * Wen Congyang  [2011-02-27 20:56]:
>> Hi Markus Armbruster
>>
>> At 02/23/2011 04:30 PM, Markus Armbruster Write:
>>> Isaku Yamahata  writes:
>>>
>>
>> 
>>
>>>
>>> I don't think this patch is correct.  Let me explain.
>>>
>>> Device hot unplug is *not* guaranteed to succeed.
>>>
>>> For some buses, such as USB, it always succeeds immediately, i.e. when
>>> the device_del monitor command finishes, the device is gone.  Live is
>>> good.
>>>
>>> But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
>>> doesn't wait for the dance to complete.  Why?  The dance can take an
>>> unpredictable amount of time, including forever.
>>>
>>> Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
>>> slot, and the unplug has not yet completed (race condition), or it
>>> failed.  Yes, Virginia, PCI hotplug *can* fail.
>>>
>>> When unplug succeeds, the qdev is automatically destroyed.
>>> pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
>>> does it for PCIE.
>>
>> I got a similar problem.  When I unplug a pci device by hand, it works
>> as expected, and I can hotplug it again. But when I use a srcipt to
>> do the same thing, sometimes it failed. I think I may find another bug.
>>
>> Steps to reproduce this bug:
>> 1. cat ./test-e1000.sh # RHEL6RC is domain name
>>#! /bin/bash
>>
>>while true; do
>>virsh attach-interface RHEL6RC network default --mac 
>> 52:54:00:1f:db:c7 --model e1000
>>if [[ $? -ne 0 ]]; then
>>break
>>fi
>>virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7
>>if [[ $? -ne 0 ]]; then
>>break
>>fi
>>sleep 5
>
> How do you know that the guest has responded at this point before you
> attempt to attach again at the top of the loop.  Any attach/detach
> requires the guest to respond to the request and it may not respond at
> all.

 When I attach/detach interface by hand, it works fine: I can see the new 
 interface
 when I attach it, and it disapears when I detached it.
>>>
>>> The point is that since the attach and detach require guest
>>> participation, this interface isn't reliable.  You have a sleep 5 in
>>> your loop, hoping to wait long enough for the guest to respond, but
>>> after a number of iterations in your loop it fails, you can bump the
>>> sleep to to 3600 seconds and the guest *still* might not respond...
>>
>> We use sci interrupt to tell the guest that a device has been 
>> attached/detached.
>> But the sci interrupt is *lost* in qemu, so the guest does not know a device 
>> has
>> been attached/detached, and does not respond it.
>>
>> If the sci interrupt is not lost, the guest can respond it.
> 
> *can* is the important word.  Even if the interrupt isn;t lost, you have
> no way to guarantee that the guest will respond at all.  That's not to
> say there isn't a bug around the lost interrupt; but rather a more
> general point about hotplug's current architecture.

I don't know whether a real hardware has the same behavior.
Should we make sure the sci interrupt not lost?

> 
>>
>>>
>>>
> 




Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-03-09 Thread Ryan Harper
* Wen Congyang  [2011-03-09 01:21]:
> At 03/09/2011 02:12 PM, Ryan Harper Write:
> > * Wen Congyang  [2011-03-08 23:09]:
> >> At 03/09/2011 12:08 PM, Ryan Harper Write:
> >>> * Wen Congyang  [2011-02-27 20:56]:
>  Hi Markus Armbruster
> 
>  At 02/23/2011 04:30 PM, Markus Armbruster Write:
> > Isaku Yamahata  writes:
> >
> 
>  
> 
> >
> > I don't think this patch is correct.  Let me explain.
> >
> > Device hot unplug is *not* guaranteed to succeed.
> >
> > For some buses, such as USB, it always succeeds immediately, i.e. when
> > the device_del monitor command finishes, the device is gone.  Live is
> > good.
> >
> > But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
> > doesn't wait for the dance to complete.  Why?  The dance can take an
> > unpredictable amount of time, including forever.
> >
> > Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
> > slot, and the unplug has not yet completed (race condition), or it
> > failed.  Yes, Virginia, PCI hotplug *can* fail.
> >
> > When unplug succeeds, the qdev is automatically destroyed.
> > pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
> > does it for PCIE.
> 
>  I got a similar problem.  When I unplug a pci device by hand, it works
>  as expected, and I can hotplug it again. But when I use a srcipt to
>  do the same thing, sometimes it failed. I think I may find another bug.
> 
>  Steps to reproduce this bug:
>  1. cat ./test-e1000.sh # RHEL6RC is domain name
> #! /bin/bash
> 
> while true; do
> virsh attach-interface RHEL6RC network default --mac 
>  52:54:00:1f:db:c7 --model e1000
> if [[ $? -ne 0 ]]; then
> break
> fi
> virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7
> if [[ $? -ne 0 ]]; then
> break
> fi
> sleep 5
> >>>
> >>> How do you know that the guest has responded at this point before you
> >>> attempt to attach again at the top of the loop.  Any attach/detach
> >>> requires the guest to respond to the request and it may not respond at
> >>> all.
> >>
> >> When I attach/detach interface by hand, it works fine: I can see the new 
> >> interface
> >> when I attach it, and it disapears when I detached it.
> > 
> > The point is that since the attach and detach require guest
> > participation, this interface isn't reliable.  You have a sleep 5 in
> > your loop, hoping to wait long enough for the guest to respond, but
> > after a number of iterations in your loop it fails, you can bump the
> > sleep to to 3600 seconds and the guest *still* might not respond...
> 
> We use sci interrupt to tell the guest that a device has been 
> attached/detached.
> But the sci interrupt is *lost* in qemu, so the guest does not know a device 
> has
> been attached/detached, and does not respond it.
> 
> If the sci interrupt is not lost, the guest can respond it.

*can* is the important word.  Even if the interrupt isn;t lost, you have
no way to guarantee that the guest will respond at all.  That's not to
say there isn't a bug around the lost interrupt; but rather a more
general point about hotplug's current architecture.

> 
> > 
> > 

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ry...@us.ibm.com



Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-03-08 Thread Wen Congyang
At 03/09/2011 02:12 PM, Ryan Harper Write:
> * Wen Congyang  [2011-03-08 23:09]:
>> At 03/09/2011 12:08 PM, Ryan Harper Write:
>>> * Wen Congyang  [2011-02-27 20:56]:
 Hi Markus Armbruster

 At 02/23/2011 04:30 PM, Markus Armbruster Write:
> Isaku Yamahata  writes:
>

 

>
> I don't think this patch is correct.  Let me explain.
>
> Device hot unplug is *not* guaranteed to succeed.
>
> For some buses, such as USB, it always succeeds immediately, i.e. when
> the device_del monitor command finishes, the device is gone.  Live is
> good.
>
> But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
> doesn't wait for the dance to complete.  Why?  The dance can take an
> unpredictable amount of time, including forever.
>
> Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
> slot, and the unplug has not yet completed (race condition), or it
> failed.  Yes, Virginia, PCI hotplug *can* fail.
>
> When unplug succeeds, the qdev is automatically destroyed.
> pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
> does it for PCIE.

 I got a similar problem.  When I unplug a pci device by hand, it works
 as expected, and I can hotplug it again. But when I use a srcipt to
 do the same thing, sometimes it failed. I think I may find another bug.

 Steps to reproduce this bug:
 1. cat ./test-e1000.sh # RHEL6RC is domain name
#! /bin/bash

while true; do
virsh attach-interface RHEL6RC network default --mac 
 52:54:00:1f:db:c7 --model e1000
if [[ $? -ne 0 ]]; then
break
fi
virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7
if [[ $? -ne 0 ]]; then
break
fi
sleep 5
>>>
>>> How do you know that the guest has responded at this point before you
>>> attempt to attach again at the top of the loop.  Any attach/detach
>>> requires the guest to respond to the request and it may not respond at
>>> all.
>>
>> When I attach/detach interface by hand, it works fine: I can see the new 
>> interface
>> when I attach it, and it disapears when I detached it.
> 
> The point is that since the attach and detach require guest
> participation, this interface isn't reliable.  You have a sleep 5 in
> your loop, hoping to wait long enough for the guest to respond, but
> after a number of iterations in your loop it fails, you can bump the
> sleep to to 3600 seconds and the guest *still* might not respond...

We use sci interrupt to tell the guest that a device has been attached/detached.
But the sci interrupt is *lost* in qemu, so the guest does not know a device has
been attached/detached, and does not respond it.

If the sci interrupt is not lost, the guest can respond it.

> 
> 




Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-03-08 Thread Ryan Harper
* Wen Congyang  [2011-03-08 23:09]:
> At 03/09/2011 12:08 PM, Ryan Harper Write:
> > * Wen Congyang  [2011-02-27 20:56]:
> >> Hi Markus Armbruster
> >>
> >> At 02/23/2011 04:30 PM, Markus Armbruster Write:
> >>> Isaku Yamahata  writes:
> >>>
> >>
> >> 
> >>
> >>>
> >>> I don't think this patch is correct.  Let me explain.
> >>>
> >>> Device hot unplug is *not* guaranteed to succeed.
> >>>
> >>> For some buses, such as USB, it always succeeds immediately, i.e. when
> >>> the device_del monitor command finishes, the device is gone.  Live is
> >>> good.
> >>>
> >>> But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
> >>> doesn't wait for the dance to complete.  Why?  The dance can take an
> >>> unpredictable amount of time, including forever.
> >>>
> >>> Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
> >>> slot, and the unplug has not yet completed (race condition), or it
> >>> failed.  Yes, Virginia, PCI hotplug *can* fail.
> >>>
> >>> When unplug succeeds, the qdev is automatically destroyed.
> >>> pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
> >>> does it for PCIE.
> >>
> >> I got a similar problem.  When I unplug a pci device by hand, it works
> >> as expected, and I can hotplug it again. But when I use a srcipt to
> >> do the same thing, sometimes it failed. I think I may find another bug.
> >>
> >> Steps to reproduce this bug:
> >> 1. cat ./test-e1000.sh # RHEL6RC is domain name
> >>#! /bin/bash
> >>
> >>while true; do
> >>virsh attach-interface RHEL6RC network default --mac 
> >> 52:54:00:1f:db:c7 --model e1000
> >>if [[ $? -ne 0 ]]; then
> >>break
> >>fi
> >>virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7
> >>if [[ $? -ne 0 ]]; then
> >>break
> >>fi
> >>sleep 5
> > 
> > How do you know that the guest has responded at this point before you
> > attempt to attach again at the top of the loop.  Any attach/detach
> > requires the guest to respond to the request and it may not respond at
> > all.
> 
> When I attach/detach interface by hand, it works fine: I can see the new 
> interface
> when I attach it, and it disapears when I detached it.

The point is that since the attach and detach require guest
participation, this interface isn't reliable.  You have a sleep 5 in
your loop, hoping to wait long enough for the guest to respond, but
after a number of iterations in your loop it fails, you can bump the
sleep to to 3600 seconds and the guest *still* might not respond...


-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ry...@us.ibm.com



Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-03-08 Thread Wen Congyang
At 03/09/2011 12:08 PM, Ryan Harper Write:
> * Wen Congyang  [2011-02-27 20:56]:
>> Hi Markus Armbruster
>>
>> At 02/23/2011 04:30 PM, Markus Armbruster Write:
>>> Isaku Yamahata  writes:
>>>
>>
>> 
>>
>>>
>>> I don't think this patch is correct.  Let me explain.
>>>
>>> Device hot unplug is *not* guaranteed to succeed.
>>>
>>> For some buses, such as USB, it always succeeds immediately, i.e. when
>>> the device_del monitor command finishes, the device is gone.  Live is
>>> good.
>>>
>>> But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
>>> doesn't wait for the dance to complete.  Why?  The dance can take an
>>> unpredictable amount of time, including forever.
>>>
>>> Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
>>> slot, and the unplug has not yet completed (race condition), or it
>>> failed.  Yes, Virginia, PCI hotplug *can* fail.
>>>
>>> When unplug succeeds, the qdev is automatically destroyed.
>>> pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
>>> does it for PCIE.
>>
>> I got a similar problem.  When I unplug a pci device by hand, it works
>> as expected, and I can hotplug it again. But when I use a srcipt to
>> do the same thing, sometimes it failed. I think I may find another bug.
>>
>> Steps to reproduce this bug:
>> 1. cat ./test-e1000.sh # RHEL6RC is domain name
>>#! /bin/bash
>>
>>while true; do
>>virsh attach-interface RHEL6RC network default --mac 
>> 52:54:00:1f:db:c7 --model e1000
>>if [[ $? -ne 0 ]]; then
>>break
>>fi
>>virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7
>>if [[ $? -ne 0 ]]; then
>>break
>>fi
>>sleep 5
> 
> How do you know that the guest has responded at this point before you
> attempt to attach again at the top of the loop.  Any attach/detach
> requires the guest to respond to the request and it may not respond at
> all.

When I attach/detach interface by hand, it works fine: I can see the new 
interface
when I attach it, and it disapears when I detached it.


> 
> 



Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-03-08 Thread Ryan Harper
* Wen Congyang  [2011-02-27 20:56]:
> Hi Markus Armbruster
> 
> At 02/23/2011 04:30 PM, Markus Armbruster Write:
> > Isaku Yamahata  writes:
> > 
> 
> 
> 
> > 
> > I don't think this patch is correct.  Let me explain.
> > 
> > Device hot unplug is *not* guaranteed to succeed.
> > 
> > For some buses, such as USB, it always succeeds immediately, i.e. when
> > the device_del monitor command finishes, the device is gone.  Live is
> > good.
> > 
> > But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
> > doesn't wait for the dance to complete.  Why?  The dance can take an
> > unpredictable amount of time, including forever.
> > 
> > Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
> > slot, and the unplug has not yet completed (race condition), or it
> > failed.  Yes, Virginia, PCI hotplug *can* fail.
> > 
> > When unplug succeeds, the qdev is automatically destroyed.
> > pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
> > does it for PCIE.
> 
> I got a similar problem.  When I unplug a pci device by hand, it works
> as expected, and I can hotplug it again. But when I use a srcipt to
> do the same thing, sometimes it failed. I think I may find another bug.
> 
> Steps to reproduce this bug:
> 1. cat ./test-e1000.sh # RHEL6RC is domain name
>#! /bin/bash
> 
>while true; do
>virsh attach-interface RHEL6RC network default --mac 
> 52:54:00:1f:db:c7 --model e1000
>if [[ $? -ne 0 ]]; then
>break
>fi
>virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7
>if [[ $? -ne 0 ]]; then
>break
>fi
>sleep 5

How do you know that the guest has responded at this point before you
attempt to attach again at the top of the loop.  Any attach/detach
requires the guest to respond to the request and it may not respond at
all.


>done
> 
> 2. ./test-e1000.sh
>Interface attached successfully
> 
>Interface detached successfully
> 
>Interface attached successfully
> 
>Interface detached successfully
> 
>error: Failed to attach interface
>error: operation failed: adding 
> e1000,netdev=hostnet1,id=net1,mac=52:54:00:1f:db:c7,bus=pci.0,addr=0x8 device 
> failed: Duplicate ID 'net1' for device
> 
> 
> I use ftrace to trace whether sci interrupt is passed to guest OS, here is 
> log:
> # cat trace | grep 'irq=9'
>   -0 [000] 118342.634772: irq_handler_entry: irq=9 name=acpi
>   -0 [000] 118342.634831: irq_handler_exit: irq=9 
> ret=handled
>   -0 [000] 118342.693216: irq_handler_entry: irq=9 name=acpi
>   -0 [000] 118342.693254: irq_handler_exit: irq=9 
> ret=handled
>   -0 [000] 118347.737689: irq_handler_entry: irq=9 name=acpi
>   -0 [000] 118347.737738: irq_handler_exit: irq=9 
> ret=handled
> According to the log, we only receive 3 sci interrupt, and one is lost.
> 
> I enable piix4's debug and 1 line printf into piix4_device_hotplug:
> printf("slot: %d, up: %d, down: %d, state: %d\n", slot, 
> s->pci0_status.up, s->pci0_status.down, state);
> here is the log:
> 
> PM: mapping to 0xb000
> PM readw port=0x0004 val=0x
> ...
> gpe write afe2 <== 0
> gpe write afe0 <== 255
> gpe write afe3 <== 0
> gpe write afe1 <== 255
> PM readw port=0x val=0x0001
> PM readw port=0x0002 val=0x
> gpe read afe0 == 0
> gpe read afe2 == 0
> gpe read afe1 == 0
> gpe read afe3 == 0
> PM writew port=0x val=0x0020
> PM readw port=0x0002 val=0x
> PM writew port=0x0002 val=0x0020
> PM readw port=0x0002 val=0x0020
> gpe write afe2 <== 255
> gpe write afe3 <== 255
> ...
> slot: 6, up: 0, down: 0, state: 1   < we attach interface the first time
> PM readw port=0x val=0x0001
> PM readw port=0x0002 val=0x0120
> gpe read afe0 == 2
> gpe read afe2 == ff
> gpe read afe2 == ff
> gpe write afe2 <== 253
> gpe read afe1 == 0
> gpe read afe3 == ff
> pcihotplug read ae00 == 40
> pcihotplug read ae04 == 0
> ...
> gpe write afe0 <== 2
> gpe write afe2 <== 255
> slot: 6, up: 64, down: 0, state: 0  <= we detach interface the first time
> PM readw port=0x val=0x0001
> PM readw port=0x0002 val=0x0120
> gpe read afe0 == 2
> gpe read afe2 == ff
> gpe read afe2 == ff
> gpe write afe2 <== 253
> gpe read afe1 == 0
> gpe read afe3 == ff
> pcihotplug read ae00 == 0
> pcihotplug read ae04 == 40
> ...
> pciej write ae08 <== 64   <=== we will call qdev_free()
> pciej write ae08 <== 64
> gpe write afe0 <== 2
> gpe write afe2 <== 255
> slot: 7, up: 0, down: 64, state: 1  <=== we attach interface the second time.
> PM readw port=0x val=0x0001
> PM readw port=0x0002 val=0x0120
> gpe read afe0 == 2
> gpe read afe2 == ff
> gpe read afe2 == ff
> gpe write afe2 <== 253
> gpe read afe1 == 0
> gpe read afe3 == ff
> pcihotplug read ae00 == 80
> pcihotplug read ae04 == 0
> pcihotplug read ae00 == 80
> pcihotplug read ae04 == 0
> pcihotplug read a

Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-03-01 Thread Isaku Yamahata
On Tue, Mar 01, 2011 at 03:32:08PM +0800, Wen Congyang wrote:
> > The issue is, Qemu injected sci interrupt into guest,
> > but before the guest completes to handled it,
> > users/qemu can start to inject the next sci event triggered by hot 
> > plug/unplug.
> > Thus qemu loses sci interrupt and up/down status.
> 
> Yes, you are right. But I still do not understand why introducing pending bits
> can not solve the issue?

Maybe your patch works.
The difference is to introduce new variables or new state machine.

Anyway I suppose that the eventual solution is to switch to 
express native hotplug which is much more reliable and doesn't rely
on ACPI. (note: I'm very biased here.)

thanks

> 
> Thanks
> Wen Congyang
> 
> > 
> > thanks
> >>
> >>>
> >>> - on power-on  (0, 0)
> >>> - on hot plug  (0, 0) -> (1, 0)
> >>>if other combination -> error
> >>> - on hot unpug (1, 0) or (0, 0) -> (0, 1)
> >>>(0, 0) is for cold plugged devices.
> >>>(1, 0) is for hot plugged devices.
> >>>if other combination -> error
> >>> - on pciej_write(write on PCI_EJ_BASE)
> >>>(0, 1) -> (0, 0) and qdev_free()
> >>>if other combination -> ignore
> >>>
> >>> When enabling sci, those bits are checked and raise sci if necessary.
> >>>
> >>> Any comments on this?
> >>> Anyway the current pci hotplug-related commands are somewhat broken,
> >>> and needs clean up with multifunction hotplug support which is
> >>> on my todo list.
> >>>
> >>> thanks
> >>>
> >>> On Mon, Feb 28, 2011 at 10:52:40AM +0800, Wen Congyang wrote:
>  Hi Markus Armbruster
> 
>  At 02/23/2011 04:30 PM, Markus Armbruster Write:
> > Isaku Yamahata  writes:
> >
> 
>  
> 
> >
> > I don't think this patch is correct.  Let me explain.
> >
> > Device hot unplug is *not* guaranteed to succeed.
> >
> > For some buses, such as USB, it always succeeds immediately, i.e. when
> > the device_del monitor command finishes, the device is gone.  Live is
> > good.
> >
> > But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
> > doesn't wait for the dance to complete.  Why?  The dance can take an
> > unpredictable amount of time, including forever.
> >
> > Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
> > slot, and the unplug has not yet completed (race condition), or it
> > failed.  Yes, Virginia, PCI hotplug *can* fail.
> >
> > When unplug succeeds, the qdev is automatically destroyed.
> > pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
> > does it for PCIE.
> 
>  I got a similar problem.  When I unplug a pci device by hand, it works
>  as expected, and I can hotplug it again. But when I use a srcipt to
>  do the same thing, sometimes it failed. I think I may find another bug.
> 
>  Steps to reproduce this bug:
>  1. cat ./test-e1000.sh # RHEL6RC is domain name
> #! /bin/bash
> 
> while true; do
> virsh attach-interface RHEL6RC network default --mac 
>  52:54:00:1f:db:c7 --model e1000
> if [[ $? -ne 0 ]]; then
> break
> fi
> virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7
> if [[ $? -ne 0 ]]; then
> break
> fi
> sleep 5
> done
> 
>  2. ./test-e1000.sh
> Interface attached successfully
> 
> Interface detached successfully
> 
> Interface attached successfully
> 
> Interface detached successfully
> 
> error: Failed to attach interface
> error: operation failed: adding 
>  e1000,netdev=hostnet1,id=net1,mac=52:54:00:1f:db:c7,bus=pci.0,addr=0x8 
>  device failed: Duplicate ID 'net1' for device
> 
> 
>  I use ftrace to trace whether sci interrupt is passed to guest OS, here 
>  is log:
>  # cat trace | grep 'irq=9'
>    -0 [000] 118342.634772: irq_handler_entry: irq=9 
>  name=acpi
>    -0 [000] 118342.634831: irq_handler_exit: irq=9 
>  ret=handled
>    -0 [000] 118342.693216: irq_handler_entry: irq=9 
>  name=acpi
>    -0 [000] 118342.693254: irq_handler_exit: irq=9 
>  ret=handled
>    -0 [000] 118347.737689: irq_handler_entry: irq=9 
>  name=acpi
>    -0 [000] 118347.737738: irq_handler_exit: irq=9 
>  ret=handled
>  According to the log, we only receive 3 sci interrupt, and one is lost.
> 
>  I enable piix4's debug and 1 line printf into piix4_device_hotplug:
>  printf("slot: %d, up: %d, down: %d, state: %d\n", slot, 
>  s->pci0_status.up, s->pci0_status.down, state);
>  here is the log:
>  
>  PM: mapping to 0xb000
>  PM read

Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-02-28 Thread Wen Congyang
At 03/01/2011 03:13 PM, Isaku Yamahata Write:
> On Tue, Mar 01, 2011 at 02:58:44PM +0800, Wen Congyang wrote:
>> At 03/01/2011 12:11 PM, Isaku Yamahata Write:
>>> Hi.
>>>
>>> I don't suppose that just introducing pending bits solve the issue.
>>> Your test only use single hot plug/unplug. How about mixing of multiple
>>> hot plug/unplug with different slots.
>>
>> The qemu uses the same thread to deal with monitor command and I/O request
>> from guest OS. So I think mixing of multiple hot plug/unplug with different
>> slots can also work fine.
>>
>>> Zeroing up/down on piix4_device_hotplug() is the culprit.
>>>
>>> State machine of (up, down) would be needed.
>>> (up, down) of each slots should be changed following
>>> something like
>>
>> Why we need this feature? We access s->pci0_status only in main thread and
>> do not accesss it when we deal with signal, so there is no competition to
>> access it.
> 
> We are talking about losing interrupt and events, aren't we?
> Not race caused by threads.

I am sorry, I do not understand what you are talk about.

> The issue is, Qemu injected sci interrupt into guest,
> but before the guest completes to handled it,
> users/qemu can start to inject the next sci event triggered by hot 
> plug/unplug.
> Thus qemu loses sci interrupt and up/down status.

Yes, you are right. But I still do not understand why introducing pending bits
can not solve the issue?

Thanks
Wen Congyang

> 
> thanks
>>
>>>
>>> - on power-on  (0, 0)
>>> - on hot plug  (0, 0) -> (1, 0)
>>>if other combination -> error
>>> - on hot unpug (1, 0) or (0, 0) -> (0, 1)
>>>(0, 0) is for cold plugged devices.
>>>(1, 0) is for hot plugged devices.
>>>if other combination -> error
>>> - on pciej_write(write on PCI_EJ_BASE)
>>>(0, 1) -> (0, 0) and qdev_free()
>>>if other combination -> ignore
>>>
>>> When enabling sci, those bits are checked and raise sci if necessary.
>>>
>>> Any comments on this?
>>> Anyway the current pci hotplug-related commands are somewhat broken,
>>> and needs clean up with multifunction hotplug support which is
>>> on my todo list.
>>>
>>> thanks
>>>
>>> On Mon, Feb 28, 2011 at 10:52:40AM +0800, Wen Congyang wrote:
 Hi Markus Armbruster

 At 02/23/2011 04:30 PM, Markus Armbruster Write:
> Isaku Yamahata  writes:
>

 

>
> I don't think this patch is correct.  Let me explain.
>
> Device hot unplug is *not* guaranteed to succeed.
>
> For some buses, such as USB, it always succeeds immediately, i.e. when
> the device_del monitor command finishes, the device is gone.  Live is
> good.
>
> But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
> doesn't wait for the dance to complete.  Why?  The dance can take an
> unpredictable amount of time, including forever.
>
> Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
> slot, and the unplug has not yet completed (race condition), or it
> failed.  Yes, Virginia, PCI hotplug *can* fail.
>
> When unplug succeeds, the qdev is automatically destroyed.
> pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
> does it for PCIE.

 I got a similar problem.  When I unplug a pci device by hand, it works
 as expected, and I can hotplug it again. But when I use a srcipt to
 do the same thing, sometimes it failed. I think I may find another bug.

 Steps to reproduce this bug:
 1. cat ./test-e1000.sh # RHEL6RC is domain name
#! /bin/bash

while true; do
virsh attach-interface RHEL6RC network default --mac 
 52:54:00:1f:db:c7 --model e1000
if [[ $? -ne 0 ]]; then
break
fi
virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7
if [[ $? -ne 0 ]]; then
break
fi
sleep 5
done

 2. ./test-e1000.sh
Interface attached successfully

Interface detached successfully

Interface attached successfully

Interface detached successfully

error: Failed to attach interface
error: operation failed: adding 
 e1000,netdev=hostnet1,id=net1,mac=52:54:00:1f:db:c7,bus=pci.0,addr=0x8 
 device failed: Duplicate ID 'net1' for device


 I use ftrace to trace whether sci interrupt is passed to guest OS, here is 
 log:
 # cat trace | grep 'irq=9'
   -0 [000] 118342.634772: irq_handler_entry: irq=9 
 name=acpi
   -0 [000] 118342.634831: irq_handler_exit: irq=9 
 ret=handled
   -0 [000] 118342.693216: irq_handler_entry: irq=9 
 name=acpi
   -0 [000] 118342.693254: irq_handler_exit: irq=9 
 ret=handled
   -0 [000] 118

Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-02-28 Thread Isaku Yamahata
On Tue, Mar 01, 2011 at 02:58:44PM +0800, Wen Congyang wrote:
> At 03/01/2011 12:11 PM, Isaku Yamahata Write:
> > Hi.
> > 
> > I don't suppose that just introducing pending bits solve the issue.
> > Your test only use single hot plug/unplug. How about mixing of multiple
> > hot plug/unplug with different slots.
> 
> The qemu uses the same thread to deal with monitor command and I/O request
> from guest OS. So I think mixing of multiple hot plug/unplug with different
> slots can also work fine.
> 
> > Zeroing up/down on piix4_device_hotplug() is the culprit.
> > 
> > State machine of (up, down) would be needed.
> > (up, down) of each slots should be changed following
> > something like
> 
> Why we need this feature? We access s->pci0_status only in main thread and
> do not accesss it when we deal with signal, so there is no competition to
> access it.

We are talking about losing interrupt and events, aren't we?
Not race caused by threads.
The issue is, Qemu injected sci interrupt into guest,
but before the guest completes to handled it,
users/qemu can start to inject the next sci event triggered by hot plug/unplug.
Thus qemu loses sci interrupt and up/down status.

thanks
> 
> > 
> > - on power-on  (0, 0)
> > - on hot plug  (0, 0) -> (1, 0)
> >if other combination -> error
> > - on hot unpug (1, 0) or (0, 0) -> (0, 1)
> >(0, 0) is for cold plugged devices.
> >(1, 0) is for hot plugged devices.
> >if other combination -> error
> > - on pciej_write(write on PCI_EJ_BASE)
> >(0, 1) -> (0, 0) and qdev_free()
> >if other combination -> ignore
> > 
> > When enabling sci, those bits are checked and raise sci if necessary.
> > 
> > Any comments on this?
> > Anyway the current pci hotplug-related commands are somewhat broken,
> > and needs clean up with multifunction hotplug support which is
> > on my todo list.
> > 
> > thanks
> > 
> > On Mon, Feb 28, 2011 at 10:52:40AM +0800, Wen Congyang wrote:
> >> Hi Markus Armbruster
> >>
> >> At 02/23/2011 04:30 PM, Markus Armbruster Write:
> >>> Isaku Yamahata  writes:
> >>>
> >>
> >> 
> >>
> >>>
> >>> I don't think this patch is correct.  Let me explain.
> >>>
> >>> Device hot unplug is *not* guaranteed to succeed.
> >>>
> >>> For some buses, such as USB, it always succeeds immediately, i.e. when
> >>> the device_del monitor command finishes, the device is gone.  Live is
> >>> good.
> >>>
> >>> But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
> >>> doesn't wait for the dance to complete.  Why?  The dance can take an
> >>> unpredictable amount of time, including forever.
> >>>
> >>> Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
> >>> slot, and the unplug has not yet completed (race condition), or it
> >>> failed.  Yes, Virginia, PCI hotplug *can* fail.
> >>>
> >>> When unplug succeeds, the qdev is automatically destroyed.
> >>> pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
> >>> does it for PCIE.
> >>
> >> I got a similar problem.  When I unplug a pci device by hand, it works
> >> as expected, and I can hotplug it again. But when I use a srcipt to
> >> do the same thing, sometimes it failed. I think I may find another bug.
> >>
> >> Steps to reproduce this bug:
> >> 1. cat ./test-e1000.sh # RHEL6RC is domain name
> >>#! /bin/bash
> >>
> >>while true; do
> >>virsh attach-interface RHEL6RC network default --mac 
> >> 52:54:00:1f:db:c7 --model e1000
> >>if [[ $? -ne 0 ]]; then
> >>break
> >>fi
> >>virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7
> >>if [[ $? -ne 0 ]]; then
> >>break
> >>fi
> >>sleep 5
> >>done
> >>
> >> 2. ./test-e1000.sh
> >>Interface attached successfully
> >>
> >>Interface detached successfully
> >>
> >>Interface attached successfully
> >>
> >>Interface detached successfully
> >>
> >>error: Failed to attach interface
> >>error: operation failed: adding 
> >> e1000,netdev=hostnet1,id=net1,mac=52:54:00:1f:db:c7,bus=pci.0,addr=0x8 
> >> device failed: Duplicate ID 'net1' for device
> >>
> >>
> >> I use ftrace to trace whether sci interrupt is passed to guest OS, here is 
> >> log:
> >> # cat trace | grep 'irq=9'
> >>   -0 [000] 118342.634772: irq_handler_entry: irq=9 
> >> name=acpi
> >>   -0 [000] 118342.634831: irq_handler_exit: irq=9 
> >> ret=handled
> >>   -0 [000] 118342.693216: irq_handler_entry: irq=9 
> >> name=acpi
> >>   -0 [000] 118342.693254: irq_handler_exit: irq=9 
> >> ret=handled
> >>   -0 [000] 118347.737689: irq_handler_entry: irq=9 
> >> name=acpi
> >>   -0 [000] 118347.737738: irq_handler_exit: irq=9 
> >> ret=handled
> >> According to the log, we only receive 3 sci interrupt, and one is lost.
> >>
> >> I enable piix4's debug and 1 l

Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-02-28 Thread Wen Congyang
At 03/01/2011 12:11 PM, Isaku Yamahata Write:
> Hi.
> 
> I don't suppose that just introducing pending bits solve the issue.
> Your test only use single hot plug/unplug. How about mixing of multiple
> hot plug/unplug with different slots.

The qemu uses the same thread to deal with monitor command and I/O request
from guest OS. So I think mixing of multiple hot plug/unplug with different
slots can also work fine.

> Zeroing up/down on piix4_device_hotplug() is the culprit.
> 
> State machine of (up, down) would be needed.
> (up, down) of each slots should be changed following
> something like

Why we need this feature? We access s->pci0_status only in main thread and
do not accesss it when we deal with signal, so there is no competition to
access it.

> 
> - on power-on  (0, 0)
> - on hot plug  (0, 0) -> (1, 0)
>if other combination -> error
> - on hot unpug (1, 0) or (0, 0) -> (0, 1)
>(0, 0) is for cold plugged devices.
>(1, 0) is for hot plugged devices.
>if other combination -> error
> - on pciej_write(write on PCI_EJ_BASE)
>(0, 1) -> (0, 0) and qdev_free()
>if other combination -> ignore
> 
> When enabling sci, those bits are checked and raise sci if necessary.
> 
> Any comments on this?
> Anyway the current pci hotplug-related commands are somewhat broken,
> and needs clean up with multifunction hotplug support which is
> on my todo list.
> 
> thanks
> 
> On Mon, Feb 28, 2011 at 10:52:40AM +0800, Wen Congyang wrote:
>> Hi Markus Armbruster
>>
>> At 02/23/2011 04:30 PM, Markus Armbruster Write:
>>> Isaku Yamahata  writes:
>>>
>>
>> 
>>
>>>
>>> I don't think this patch is correct.  Let me explain.
>>>
>>> Device hot unplug is *not* guaranteed to succeed.
>>>
>>> For some buses, such as USB, it always succeeds immediately, i.e. when
>>> the device_del monitor command finishes, the device is gone.  Live is
>>> good.
>>>
>>> But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
>>> doesn't wait for the dance to complete.  Why?  The dance can take an
>>> unpredictable amount of time, including forever.
>>>
>>> Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
>>> slot, and the unplug has not yet completed (race condition), or it
>>> failed.  Yes, Virginia, PCI hotplug *can* fail.
>>>
>>> When unplug succeeds, the qdev is automatically destroyed.
>>> pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
>>> does it for PCIE.
>>
>> I got a similar problem.  When I unplug a pci device by hand, it works
>> as expected, and I can hotplug it again. But when I use a srcipt to
>> do the same thing, sometimes it failed. I think I may find another bug.
>>
>> Steps to reproduce this bug:
>> 1. cat ./test-e1000.sh # RHEL6RC is domain name
>>#! /bin/bash
>>
>>while true; do
>>virsh attach-interface RHEL6RC network default --mac 
>> 52:54:00:1f:db:c7 --model e1000
>>if [[ $? -ne 0 ]]; then
>>break
>>fi
>>virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7
>>if [[ $? -ne 0 ]]; then
>>break
>>fi
>>sleep 5
>>done
>>
>> 2. ./test-e1000.sh
>>Interface attached successfully
>>
>>Interface detached successfully
>>
>>Interface attached successfully
>>
>>Interface detached successfully
>>
>>error: Failed to attach interface
>>error: operation failed: adding 
>> e1000,netdev=hostnet1,id=net1,mac=52:54:00:1f:db:c7,bus=pci.0,addr=0x8 
>> device failed: Duplicate ID 'net1' for device
>>
>>
>> I use ftrace to trace whether sci interrupt is passed to guest OS, here is 
>> log:
>> # cat trace | grep 'irq=9'
>>   -0 [000] 118342.634772: irq_handler_entry: irq=9 
>> name=acpi
>>   -0 [000] 118342.634831: irq_handler_exit: irq=9 
>> ret=handled
>>   -0 [000] 118342.693216: irq_handler_entry: irq=9 
>> name=acpi
>>   -0 [000] 118342.693254: irq_handler_exit: irq=9 
>> ret=handled
>>   -0 [000] 118347.737689: irq_handler_entry: irq=9 
>> name=acpi
>>   -0 [000] 118347.737738: irq_handler_exit: irq=9 
>> ret=handled
>> According to the log, we only receive 3 sci interrupt, and one is lost.
>>
>> I enable piix4's debug and 1 line printf into piix4_device_hotplug:
>> printf("slot: %d, up: %d, down: %d, state: %d\n", slot, 
>> s->pci0_status.up, s->pci0_status.down, state);
>> here is the log:
>> 
>> PM: mapping to 0xb000
>> PM readw port=0x0004 val=0x
>> ...
>> gpe write afe2 <== 0
>> gpe write afe0 <== 255
>> gpe write afe3 <== 0
>> gpe write afe1 <== 255
>> PM readw port=0x val=0x0001
>> PM readw port=0x0002 val=0x
>> gpe read afe0 == 0
>> gpe read afe2 == 0
>> gpe read afe1 == 0
>> gpe read afe3 == 0
>> PM writew port=0x val=0x0020
>> PM readw port=0x0002 val=0x
>> PM writew port=0x0002 val=0x0020
>> PM readw port=0x0002 v

Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-02-28 Thread Isaku Yamahata
Hi.

I don't suppose that just introducing pending bits solve the issue.
Your test only use single hot plug/unplug. How about mixing of multiple
hot plug/unplug with different slots.
Zeroing up/down on piix4_device_hotplug() is the culprit.

State machine of (up, down) would be needed.
(up, down) of each slots should be changed following
something like

- on power-on  (0, 0)
- on hot plug  (0, 0) -> (1, 0)
   if other combination -> error
- on hot unpug (1, 0) or (0, 0) -> (0, 1)
   (0, 0) is for cold plugged devices.
   (1, 0) is for hot plugged devices.
   if other combination -> error
- on pciej_write(write on PCI_EJ_BASE)
   (0, 1) -> (0, 0) and qdev_free()
   if other combination -> ignore

When enabling sci, those bits are checked and raise sci if necessary.

Any comments on this?
Anyway the current pci hotplug-related commands are somewhat broken,
and needs clean up with multifunction hotplug support which is
on my todo list.

thanks

On Mon, Feb 28, 2011 at 10:52:40AM +0800, Wen Congyang wrote:
> Hi Markus Armbruster
> 
> At 02/23/2011 04:30 PM, Markus Armbruster Write:
> > Isaku Yamahata  writes:
> > 
> 
> 
> 
> > 
> > I don't think this patch is correct.  Let me explain.
> > 
> > Device hot unplug is *not* guaranteed to succeed.
> > 
> > For some buses, such as USB, it always succeeds immediately, i.e. when
> > the device_del monitor command finishes, the device is gone.  Live is
> > good.
> > 
> > But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
> > doesn't wait for the dance to complete.  Why?  The dance can take an
> > unpredictable amount of time, including forever.
> > 
> > Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
> > slot, and the unplug has not yet completed (race condition), or it
> > failed.  Yes, Virginia, PCI hotplug *can* fail.
> > 
> > When unplug succeeds, the qdev is automatically destroyed.
> > pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
> > does it for PCIE.
> 
> I got a similar problem.  When I unplug a pci device by hand, it works
> as expected, and I can hotplug it again. But when I use a srcipt to
> do the same thing, sometimes it failed. I think I may find another bug.
> 
> Steps to reproduce this bug:
> 1. cat ./test-e1000.sh # RHEL6RC is domain name
>#! /bin/bash
> 
>while true; do
>virsh attach-interface RHEL6RC network default --mac 
> 52:54:00:1f:db:c7 --model e1000
>if [[ $? -ne 0 ]]; then
>break
>fi
>virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7
>if [[ $? -ne 0 ]]; then
>break
>fi
>sleep 5
>done
> 
> 2. ./test-e1000.sh
>Interface attached successfully
> 
>Interface detached successfully
> 
>Interface attached successfully
> 
>Interface detached successfully
> 
>error: Failed to attach interface
>error: operation failed: adding 
> e1000,netdev=hostnet1,id=net1,mac=52:54:00:1f:db:c7,bus=pci.0,addr=0x8 device 
> failed: Duplicate ID 'net1' for device
> 
> 
> I use ftrace to trace whether sci interrupt is passed to guest OS, here is 
> log:
> # cat trace | grep 'irq=9'
>   -0 [000] 118342.634772: irq_handler_entry: irq=9 name=acpi
>   -0 [000] 118342.634831: irq_handler_exit: irq=9 
> ret=handled
>   -0 [000] 118342.693216: irq_handler_entry: irq=9 name=acpi
>   -0 [000] 118342.693254: irq_handler_exit: irq=9 
> ret=handled
>   -0 [000] 118347.737689: irq_handler_entry: irq=9 name=acpi
>   -0 [000] 118347.737738: irq_handler_exit: irq=9 
> ret=handled
> According to the log, we only receive 3 sci interrupt, and one is lost.
> 
> I enable piix4's debug and 1 line printf into piix4_device_hotplug:
> printf("slot: %d, up: %d, down: %d, state: %d\n", slot, 
> s->pci0_status.up, s->pci0_status.down, state);
> here is the log:
> 
> PM: mapping to 0xb000
> PM readw port=0x0004 val=0x
> ...
> gpe write afe2 <== 0
> gpe write afe0 <== 255
> gpe write afe3 <== 0
> gpe write afe1 <== 255
> PM readw port=0x val=0x0001
> PM readw port=0x0002 val=0x
> gpe read afe0 == 0
> gpe read afe2 == 0
> gpe read afe1 == 0
> gpe read afe3 == 0
> PM writew port=0x val=0x0020
> PM readw port=0x0002 val=0x
> PM writew port=0x0002 val=0x0020
> PM readw port=0x0002 val=0x0020
> gpe write afe2 <== 255
> gpe write afe3 <== 255
> ...
> slot: 6, up: 0, down: 0, state: 1   < we attach interface the first time
> PM readw port=0x val=0x0001
> PM readw port=0x0002 val=0x0120
> gpe read afe0 == 2
> gpe read afe2 == ff
> gpe read afe2 == ff
> gpe write afe2 <== 253
> gpe read afe1 == 0
> gpe read afe3 == ff
> pcihotplug read ae00 == 40
> pcihotplug read ae04 == 0
> ...
> gpe write afe0 <== 2
> gpe write afe2 <== 255
> slot: 6, up: 64, down: 0, state: 0  <= we detach interface

Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-02-27 Thread Wen Congyang
Hi Markus Armbruster

At 02/23/2011 04:30 PM, Markus Armbruster Write:
> Isaku Yamahata  writes:
> 



> 
> I don't think this patch is correct.  Let me explain.
> 
> Device hot unplug is *not* guaranteed to succeed.
> 
> For some buses, such as USB, it always succeeds immediately, i.e. when
> the device_del monitor command finishes, the device is gone.  Live is
> good.
> 
> But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
> doesn't wait for the dance to complete.  Why?  The dance can take an
> unpredictable amount of time, including forever.
> 
> Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
> slot, and the unplug has not yet completed (race condition), or it
> failed.  Yes, Virginia, PCI hotplug *can* fail.
> 
> When unplug succeeds, the qdev is automatically destroyed.
> pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
> does it for PCIE.

I got a similar problem.  When I unplug a pci device by hand, it works
as expected, and I can hotplug it again. But when I use a srcipt to
do the same thing, sometimes it failed. I think I may find another bug.

Steps to reproduce this bug:
1. cat ./test-e1000.sh # RHEL6RC is domain name
   #! /bin/bash

   while true; do
   virsh attach-interface RHEL6RC network default --mac 
52:54:00:1f:db:c7 --model e1000
   if [[ $? -ne 0 ]]; then
   break
   fi
   virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7
   if [[ $? -ne 0 ]]; then
   break
   fi
   sleep 5
   done

2. ./test-e1000.sh
   Interface attached successfully

   Interface detached successfully

   Interface attached successfully

   Interface detached successfully

   error: Failed to attach interface
   error: operation failed: adding 
e1000,netdev=hostnet1,id=net1,mac=52:54:00:1f:db:c7,bus=pci.0,addr=0x8 device 
failed: Duplicate ID 'net1' for device


I use ftrace to trace whether sci interrupt is passed to guest OS, here is log:
# cat trace | grep 'irq=9'
  -0 [000] 118342.634772: irq_handler_entry: irq=9 name=acpi
  -0 [000] 118342.634831: irq_handler_exit: irq=9 ret=handled
  -0 [000] 118342.693216: irq_handler_entry: irq=9 name=acpi
  -0 [000] 118342.693254: irq_handler_exit: irq=9 ret=handled
  -0 [000] 118347.737689: irq_handler_entry: irq=9 name=acpi
  -0 [000] 118347.737738: irq_handler_exit: irq=9 ret=handled
According to the log, we only receive 3 sci interrupt, and one is lost.

I enable piix4's debug and 1 line printf into piix4_device_hotplug:
printf("slot: %d, up: %d, down: %d, state: %d\n", slot, s->pci0_status.up, 
s->pci0_status.down, state);
here is the log:

PM: mapping to 0xb000
PM readw port=0x0004 val=0x
...
gpe write afe2 <== 0
gpe write afe0 <== 255
gpe write afe3 <== 0
gpe write afe1 <== 255
PM readw port=0x val=0x0001
PM readw port=0x0002 val=0x
gpe read afe0 == 0
gpe read afe2 == 0
gpe read afe1 == 0
gpe read afe3 == 0
PM writew port=0x val=0x0020
PM readw port=0x0002 val=0x
PM writew port=0x0002 val=0x0020
PM readw port=0x0002 val=0x0020
gpe write afe2 <== 255
gpe write afe3 <== 255
...
slot: 6, up: 0, down: 0, state: 1   < we attach interface the first time
PM readw port=0x val=0x0001
PM readw port=0x0002 val=0x0120
gpe read afe0 == 2
gpe read afe2 == ff
gpe read afe2 == ff
gpe write afe2 <== 253
gpe read afe1 == 0
gpe read afe3 == ff
pcihotplug read ae00 == 40
pcihotplug read ae04 == 0
...
gpe write afe0 <== 2
gpe write afe2 <== 255
slot: 6, up: 64, down: 0, state: 0  <= we detach interface the first time
PM readw port=0x val=0x0001
PM readw port=0x0002 val=0x0120
gpe read afe0 == 2
gpe read afe2 == ff
gpe read afe2 == ff
gpe write afe2 <== 253
gpe read afe1 == 0
gpe read afe3 == ff
pcihotplug read ae00 == 0
pcihotplug read ae04 == 40
...
pciej write ae08 <== 64 <=== we will call qdev_free()
pciej write ae08 <== 64
gpe write afe0 <== 2
gpe write afe2 <== 255
slot: 7, up: 0, down: 64, state: 1  <=== we attach interface the second time.
PM readw port=0x val=0x0001
PM readw port=0x0002 val=0x0120
gpe read afe0 == 2
gpe read afe2 == ff
gpe read afe2 == ff
gpe write afe2 <== 253
gpe read afe1 == 0
gpe read afe3 == ff
pcihotplug read ae00 == 80
pcihotplug read ae04 == 0
pcihotplug read ae00 == 80
pcihotplug read ae04 == 0
pcihotplug read ae00 == 80
pcihotplug read ae04 == 0
pcihotplug read ae00 == 80
pcihotplug read ae04 == 0
pcihotplug read ae00 == 80
pcihotplug read ae04 == 0
pcihotplug read ae00 == 80
pcihotplug read ae04 == 0
pcihotplug read ae00 == 80
pcihotplug read ae04 == 0
pcihotplug read ae00 == 80
pcihotplug read ae04 == 0
pcihotplug read ae00 == 80
pcihotplug read ae04 == 0
pcihotplug read ae00 == 80
pcihotplug read ae04 == 0
pcihotplug read ae00 == 80
pcihotplug read ae04 == 0
pcihotplug read ae00 == 80
pcihotplug read ae04 == 0
pcihotplug read ae00 == 80
pcihotplug read ae

Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-02-23 Thread William Dauchy
Hi Markus,

On Wed, Feb 23, 2011 at 9:30 AM, Markus Armbruster  wrote:
> I don't think this patch is correct.  Let me explain.
>
> Device hot unplug is *not* guaranteed to succeed.
>
> For some buses, such as USB, it always succeeds immediately, i.e. when
> the device_del monitor command finishes, the device is gone.  Live is
> good.
>
> But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
> doesn't wait for the dance to complete.  Why?  The dance can take an
> unpredictable amount of time, including forever.
>
> Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
> slot, and the unplug has not yet completed (race condition), or it
> failed.  Yes, Virginia, PCI hotplug *can* fail.
>
> When unplug succeeds, the qdev is automatically destroyed.
> pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
> does it for PCIE.
>
> Your patch adds a *second* qdev_free().  No good.

Thanks for the complete explanation. I now understand my mistake and
find out why it wasn't working as expected.
On a Linux virtual machine I forgot to load the `acpiphp` module which
makes the pci device disappear when doing a detach. I thought that,
even without this module the detach should have freed the
corresponding device on qemu side, regardless if the virtual machine
was supporting or not acpi signals.
Please ignore my patch.

Regards,
-- 
William



Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-02-23 Thread Markus Armbruster
Isaku Yamahata  writes:

> On Tue, Feb 22, 2011 at 06:36:20PM +0100, William Dauchy wrote:
>>  `qdev_free` when unplug a pci device
>>  It resolves a bug when detaching/attaching a network device
>> 
>>  # virsh detach-interface dom0 network --mac 52:54:00:f6:84:ba
>>  Interface detached successfully
>> 
>>  # virsh attach-interface dom0 network default --mac 52:54:00:f6:84:ba
>>  error: Failed to attach interface
>>  error: internal error unable to execute QEMU command 'device_add': 
>> Duplicate ID 'net0' for device
>> 
>> A detached pci device was not freed of the list `qemu_device_opts`
>> ---
>>  hw/pci.c |5 -
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>> 
>> diff --git a/hw/pci.c b/hw/pci.c
>> index 8b76cea..1e9a8f0 100644
>> --- a/hw/pci.c
>> +++ b/hw/pci.c
>> @@ -1671,13 +1671,16 @@ static int pci_unplug_device(DeviceState *qdev)
>>  {
>>  PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, qdev);
>>  PCIDeviceInfo *info = container_of(qdev->info, PCIDeviceInfo, qdev);
>> +int unplug;
>>  
>>  if (info->no_hotplug) {
>>  qerror_report(QERR_DEVICE_NO_HOTPLUG, info->qdev.name);
>>  return -1;
>>  }
>> -return dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
>> +unplug = dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
>>   PCI_HOTPLUG_DISABLED);
>> +qdev_free(qdev);
>
> Good catch.
> qdev_free() should be called only when unplug had succeeded.
> Although piix4 unplug always successes, pcie unplug may fail.
>
>> +return unplug;
>>  }
>>  
>>  void pci_qdev_register(PCIDeviceInfo *info)

I don't think this patch is correct.  Let me explain.

Device hot unplug is *not* guaranteed to succeed.

For some buses, such as USB, it always succeeds immediately, i.e. when
the device_del monitor command finishes, the device is gone.  Live is
good.

But for PCI, device_del merely initiates the ACPI unplug rain dance.  It
doesn't wait for the dance to complete.  Why?  The dance can take an
unpredictable amount of time, including forever.

Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
slot, and the unplug has not yet completed (race condition), or it
failed.  Yes, Virginia, PCI hotplug *can* fail.

When unplug succeeds, the qdev is automatically destroyed.
pciej_write() does that for PIIX4.  Looks like pcie_cap_slot_event()
does it for PCIE.

Your patch adds a *second* qdev_free().  No good.



Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-02-22 Thread Isaku Yamahata
On Tue, Feb 22, 2011 at 06:36:20PM +0100, William Dauchy wrote:
>  `qdev_free` when unplug a pci device
>  It resolves a bug when detaching/attaching a network device
> 
>  # virsh detach-interface dom0 network --mac 52:54:00:f6:84:ba
>  Interface detached successfully
> 
>  # virsh attach-interface dom0 network default --mac 52:54:00:f6:84:ba
>  error: Failed to attach interface
>  error: internal error unable to execute QEMU command 'device_add': Duplicate 
> ID 'net0' for device
> 
> A detached pci device was not freed of the list `qemu_device_opts`
> ---
>  hw/pci.c |5 -
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 8b76cea..1e9a8f0 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1671,13 +1671,16 @@ static int pci_unplug_device(DeviceState *qdev)
>  {
>  PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, qdev);
>  PCIDeviceInfo *info = container_of(qdev->info, PCIDeviceInfo, qdev);
> +int unplug;
>  
>  if (info->no_hotplug) {
>  qerror_report(QERR_DEVICE_NO_HOTPLUG, info->qdev.name);
>  return -1;
>  }
> -return dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
> +unplug = dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
>   PCI_HOTPLUG_DISABLED);
> +qdev_free(qdev);

Good catch.
qdev_free() should be called only when unplug had succeeded.
Although piix4 unplug always successes, pcie unplug may fail.

> +return unplug;
>  }
>  
>  void pci_qdev_register(PCIDeviceInfo *info)
> -- 
> William
> 
> 

-- 
yamahata



[Qemu-devel] [PATCH] `qdev_free` when unplug a pci device

2011-02-22 Thread William Dauchy
 `qdev_free` when unplug a pci device
 It resolves a bug when detaching/attaching a network device

 # virsh detach-interface dom0 network --mac 52:54:00:f6:84:ba
 Interface detached successfully

 # virsh attach-interface dom0 network default --mac 52:54:00:f6:84:ba
 error: Failed to attach interface
 error: internal error unable to execute QEMU command 'device_add': Duplicate 
ID 'net0' for device

A detached pci device was not freed of the list `qemu_device_opts`
---
 hw/pci.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 8b76cea..1e9a8f0 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1671,13 +1671,16 @@ static int pci_unplug_device(DeviceState *qdev)
 {
 PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, qdev);
 PCIDeviceInfo *info = container_of(qdev->info, PCIDeviceInfo, qdev);
+int unplug;
 
 if (info->no_hotplug) {
 qerror_report(QERR_DEVICE_NO_HOTPLUG, info->qdev.name);
 return -1;
 }
-return dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
+unplug = dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
  PCI_HOTPLUG_DISABLED);
+qdev_free(qdev);
+return unplug;
 }
 
 void pci_qdev_register(PCIDeviceInfo *info)
-- 
William