> -----Original Message----- > From: mika.westerberg@xxxxxxxxxxxxxxx > [mailto:mika.westerberg@xxxxxxxxxxxxxxx] > Sent: 13 November 2018 12:25 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx> > Cc: linux-pci@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Wangzhou (B) > <wangzhou1@xxxxxxxxxxxxx>; Linuxarm <linuxarm@xxxxxxxxxx>; Lukas > Wunner <lukas@xxxxxxxxx> > Subject: Re: Qemu Guest kernel 4.20-rc1 PCIe hotplug issue > > +Lukas > > On Tue, Nov 13, 2018 at 11:45:42AM +0000, Shameerali Kolothum Thodi wrote: > > Hi Mika, > > Hi, > > > Since the commit commit 720d6a671a6e("PCI: pciehp: Do not handle events > > if interrupts are masked"), the hotplug support on Qemu Guest(4.120-rc1) > > with a vfio passthrough device seems to be broken. This is on an ARM64 > platform. > > > > I am booting a Guest with below command line options with the intention of > > hot add a ixgbevf dev later, > > > > ./qemu-system-aarch64 -machine virt,kernel_irqchip=on,gic-version=3 -cpu > host \ > > -kernel Image_4.20-rc1 \ > > -initrd rootfs-iperf.cpio \ > > -device ioh3420,id=rp1 \ > > -net none \ > > -m 4096 \ > > -nographic -D -d -enable-kvm \ > > -append "console=ttyAMA0 root=/dev/vda -m 4096 rw > pciehp.pciehp_debug=1 > > pcie_ports=native searlycon=pl011,0x9000000" > > > > But receives this on boot, > > > > [ 1.327852] pciehp 0000:00:01.0:pcie004: Timeout > > on hotplug command 0x03f1 (issued 1016 msec ago) > > [ 1.335842] pciehp 0000:00:01.0:pcie004: Timeout on hotplug command > > 0x03f1 (issued 1024 msec ago) > > [ 3.847843] pciehp 0000:00:01.0:pcie004: Failed to check link status > > [ 3.855843] pciehp 0000:00:01.0:pcie004: Timeout on hotplug command > > 0x02f1 (issued 2520 msec ago) > > [ 4.879846] pciehp 0000:00:01.0:pcie004: Timeout on hotplug command > > 0x06f1 (issued 1024 msec ago) > > [ 5.911840] pciehp 0000:00:01.0:pcie004: Timeout on hotplug command > > 0x06f1 (issued 2056 msec ago) > > [ 6.927844] pciehp 0000:00:01.0:pcie004: Timeout on hotplug command > > 0x07f1 (issued 1016 msec ago) > > [ 7.951843] pciehp 0000:00:01.0:pcie004: Timeout on hotplug command > > 0x0771 (issued 1024 msec ago) > > > > Trying to hot add using "device_addvfio- > pci,host=0000:01:10.1,id=net0,bus=rp1" > > doesn't work either. And if I boot the guest with an assigned device > > (-device vfio-pci,host=0000:01:10.1,id=net0,bus=rp1), I can see the dev listed > in > > the Guest but then hot remove doesn't work. > > > > This all works on 4.19 and bisect points to the above mentioned commit, > where an > > additional check is added in pciehp_isr(), > > > > - * Interrupts only occur in D3hot or shallower (PCIe r4.0, sec 6.7.3.4). > > + * Interrupts only occur in D3hot or shallower and only if enabled > > + * in the Slot Control register (PCIe r4.0, sec 6.7.3.4). > > */ > > - if (pdev->current_state == PCI_D3cold) > > + if (pdev->current_state == PCI_D3cold || > > + (!(ctrl->slot_ctrl & PCI_EXP_SLTCTL_HPIE) && !pciehp_poll_mode)) > > return IRQ_NONE; > > > > I think this doesn't work for the first time, where the cmd with > PCI_EXP_SLTCTL_HPIE bit set > > is written, > > pciehp_probe() > > pcie_init_notification() > > pcie_enable_notification() > > pcie_do_write_cmd() > > > > to begin with, ctrl->slot_ctrl = 0 in pciehp_isr() as this is only set once the > write > > is returned. > > > > Or else I am missing something here. Please take a look and let me know. > > Thanks for the detailed report and analysis. I think you are right and > the ctrl->slot_ctrl is only set after the actual value has been > programmed to the hardware, so if there was interrupt "pending" it will > trigger immediately (just to find ctrl->slot_ctrl == 0). > > I wonder if the following change helps here? > > diff --git a/drivers/pci/hotplug/pciehp_hpc.c > b/drivers/pci/hotplug/pciehp_hpc.c > index 7dd443aea5a5..cd9eae650aa5 100644 > --- a/drivers/pci/hotplug/pciehp_hpc.c > +++ b/drivers/pci/hotplug/pciehp_hpc.c > @@ -156,9 +156,9 @@ static void pcie_do_write_cmd(struct controller *ctrl, > u16 cmd, > slot_ctrl |= (cmd & mask); > ctrl->cmd_busy = 1; > smp_mb(); > + ctrl->slot_ctrl = slot_ctrl; Actually I tried this one, but it doesn't help in this case as the initial pcie_capability_read_word() returns the slot_ctrl without PCI_EXP_SLTCTL_HPIE bit set. It looks to me pcie_enable_notification() function enables this, if (!pciehp_poll_mode) cmd |= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE; I don't know this is as per the spec or not as the initial cap read doesn't seems to have the PCI_EXP_SLTCTL_HPIE bit set. Thanks, Shameer > pcie_capability_write_word(pdev, PCI_EXP_SLTCTL, slot_ctrl); > ctrl->cmd_started = jiffies; > - ctrl->slot_ctrl = slot_ctrl; > > /* > * Controllers with the Intel CF118 and similar errata advertise
