From: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Zheng Xiang <zhengxiang9@huawei.com>,
qemu-devel@nongnu.org, wanghaibin.wang@huawei.com,
Zheng Xiang <xiang.zheng@linaro.org>
Subject: Re: [Qemu-devel] [PATCH] pcie: set link state inactive/active after hot unplug/plug
Date: Tue, 18 Dec 2018 15:59:31 +0200 [thread overview]
Message-ID: <30b5f72c-12f9-3820-6363-f012079fae73@gmail.com> (raw)
In-Reply-To: <20181217211833-mutt-send-email-mst@kernel.org>
On 12/18/18 4:18 AM, Michael S. Tsirkin wrote:
> On Wed, Dec 05, 2018 at 08:23:35AM +0200, Marcel Apfelbaum wrote:
>>
>> On 12/3/18 9:05 AM, Zheng Xiang wrote:
>>> When VM boots from the latest version of linux kernel, after
>>> hot-unpluging virtio-blk disks which are hotplugged into
>>> pcie-root-port, the VM's dmesg log shows:
>>>
>>> [ 151.046242] pciehp 0000:00:05.0:pcie004: pending interrupts 0x0001 from Slot Status
>>> [ 151.046365] pciehp 0000:00:05.0:pcie004: Slot(0-3): Attention button pressed
>>> [ 151.046369] pciehp 0000:00:05.0:pcie004: Slot(0-3): Powering off due to button press
>>> [ 151.046420] pciehp 0000:00:05.0:pcie004: pending interrupts 0x0010 from Slot Status
>>> [ 151.046425] pciehp 0000:00:05.0:pcie004: pciehp_green_led_blink: SLOTCTRL a8 write cmd 200
>>> [ 151.046464] pciehp 0000:00:05.0:pcie004: pending interrupts 0x0010 from Slot Status
>>> [ 151.046468] pciehp 0000:00:05.0:pcie004: pciehp_set_attention_status: SLOTCTRL a8 write cmd c0
>>> [ 156.163421] pciehp 0000:00:05.0:pcie004: pciehp_get_power_status: SLOTCTRL a8 value read 2f1
>>> [ 156.163427] pciehp 0000:00:05.0:pcie004: pciehp_unconfigure_device: domain:bus:dev = 0000:06:00
>>> [ 156.198736] pciehp 0000:00:05.0:pcie004: pending interrupts 0x0010 from Slot Status
>>> [ 156.198772] pciehp 0000:00:05.0:pcie004: pciehp_power_off_slot: SLOTCTRL a8 write cmd 400
>>> [ 157.224124] pciehp 0000:00:05.0:pcie004: pending interrupts 0x0018 from Slot Status
>>> [ 157.224194] pciehp 0000:00:05.0:pcie004: pciehp_green_led_off: SLOTCTRL a8 write cmd 300
>>> [ 157.224220] pciehp 0000:00:05.0:pcie004: pciehp_check_link_active: lnk_status = 2011
>>> [ 157.224223] pciehp 0000:00:05.0:pcie004: Slot(0-3): Link Up
>>> [ 157.224233] pciehp 0000:00:05.0:pcie004: pciehp_get_power_status: SLOTCTRL a8 value read 7f1
>>> [ 157.224281] pciehp 0000:00:05.0:pcie004: pending interrupts 0x0010 from Slot Status
>>> [ 157.224285] pciehp 0000:00:05.0:pcie004: pciehp_power_on_slot: SLOTCTRL a8 write cmd 0
>>> [ 157.224300] pciehp 0000:00:05.0:pcie004: __pciehp_link_set: lnk_ctrl = 0
>>> [ 157.224336] pciehp 0000:00:05.0:pcie004: pending interrupts 0x0010 from Slot Status
>>> [ 157.224339] pciehp 0000:00:05.0:pcie004: pciehp_green_led_blink: SLOTCTRL a8 write cmd 200
>>> [ 159.739294] pci 0000:06:00.0 id reading try 50 times with interval 20 ms to get ffffffff
>>> [ 159.739315] pciehp 0000:00:05.0:pcie004: pciehp_check_link_status: lnk_status = 2011
>>> [ 159.739318] pciehp 0000:00:05.0:pcie004: Failed to check link status
>>> [ 159.739371] pciehp 0000:00:05.0:pcie004: pending interrupts 0x0010 from Slot Status
>>> [ 159.739394] pciehp 0000:00:05.0:pcie004: pciehp_power_off_slot: SLOTCTRL a8 write cmd 400
>>> [ 160.771426] pciehp 0000:00:05.0:pcie004: pending interrupts 0x0010 from Slot Status
>>> [ 160.771452] pciehp 0000:00:05.0:pcie004: pciehp_green_led_off: SLOTCTRL a8 write cmd 300
>>> [ 160.771495] pciehp 0000:00:05.0:pcie004: pending interrupts 0x0010 from Slot Status
>>> [ 160.771499] pciehp 0000:00:05.0:pcie004: pciehp_set_attention_status: SLOTCTRL a8 write cmd 40
>>> [ 160.771535] pciehp 0000:00:05.0:pcie004: pending interrupts 0x0010 from Slot Status
>>> [ 160.771539] pciehp 0000:00:05.0:pcie004: pciehp_green_led_off: SLOTCTRL a8 write cmd 300
>>>
>>> After analyzing the log information, it seems that qemu doesn't
>>> change the Link Status from active to inactive after hot-unplug.
>>> This results in the abnormal log after the linux kernel commit
>>> d331710ea78fea merged.
>>>
>>> Furthermore, If I hotplug the same virtio-blk disk after hot-unplug,
>>> the virtio-blk would turn on and then back off.
>>>
>>> So this patch set the Link Status inactive after hot-unplug and
>>> active after hot-plug.
>>>
>>> Signed-off-by: Zheng Xiang <zhengxiang9@huawei.com>
>>> Signed-off-by: Zheng Xiang <xiang.zheng@linaro.org>
>>> Cc: Wang Haibin <wanghaibin.wang@huawei.com>
>>> ---
>>> hw/pci/pcie.c | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
>>> index 6c91bd4..66b73b8 100644
>>> --- a/hw/pci/pcie.c
>>> +++ b/hw/pci/pcie.c
>>> @@ -345,6 +345,10 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
>>> if (!dev->hotplugged) {
>>> pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
>>> PCI_EXP_SLTSTA_PDS);
>>> + if (pci_dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) {
>>> + pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
>>> + PCI_EXP_LNKSTA_DLLLA);
>>> + }
>>> return;
>>> }
>>> @@ -355,6 +359,10 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
>>> if (pci_get_function_0(pci_dev)) {
>>> pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
>>> PCI_EXP_SLTSTA_PDS);
>>> + if (pci_dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) {
>>> + pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
>>> + PCI_EXP_LNKSTA_DLLLA);
>>> + }
>>> pcie_cap_slot_event(PCI_DEVICE(hotplug_dev),
>>> PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP);
>>> }
>>> @@ -531,6 +539,10 @@ void pcie_cap_slot_write_config(PCIDevice *dev,
>>> pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
>>> PCI_EXP_SLTSTA_PDS);
>>> + if (dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) {
>>> + pci_word_test_and_clear_mask(exp_cap + PCI_EXP_LNKSTA,
>>> + PCI_EXP_LNKSTA_DLLLA);
>>> + }
>>> pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
>>> PCI_EXP_SLTSTA_PDC);
>>> }
>> Looks good to me,
>>
>> Reviewed-by: Marcel Apfelbaum<marcel.apfelbaum@gmail.com>
>> Thanks,
>> Marcel
>>
>>
> Marcel you need to fix your signature. I can't use it without
> a space before <.
Ooops, sorry,
I'll take care of it.
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Thanks,
Marcel
> Thanks!
>
next prev parent reply other threads:[~2018-12-18 14:05 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-03 7:05 [Qemu-devel] [PATCH] pcie: set link state inactive/active after hot unplug/plug Zheng Xiang
2018-12-05 6:23 ` Marcel Apfelbaum
2018-12-18 2:18 ` Michael S. Tsirkin
2018-12-18 13:59 ` Marcel Apfelbaum [this message]
-- strict thread matches above, loose matches on Subject: below --
2018-12-03 3:38 Zheng Xiang
2018-12-03 3:46 ` Zheng Xiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=30b5f72c-12f9-3820-6363-f012079fae73@gmail.com \
--to=marcel.apfelbaum@gmail.com \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=wanghaibin.wang@huawei.com \
--cc=xiang.zheng@linaro.org \
--cc=zhengxiang9@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.