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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).