Re: [PATCH] PCI: vmd: Flush initiator posted writes before demuxing interrupts on Meteor Lake

[Bjorn Helgaas <helgaas@kernel.org>]

On Mon, Aug 24, 2026 at 09:32:20PM -0700, Rickey Bartlett wrote:
> Meteor Lake VMD (8086:7d0b) is affected by erratum MTL016: the VMD can
> signal its MSI before the posted writes carrying a child device's DMA
> data have landed in memory. vmd_irq() then demuxes to the child handler
> while the child's completion queue is not yet coherent, so the handler
> observes no completion and returns without consuming it. The I/O is
> only recovered when the block layer timeout expires and polls the queue:
> 
>   nvme nvme0: I/O tag 253 (50fd) QID 1 timeout, completion polled
> 
> The practical effect is therefore not a lost I/O but a 30 second stall
> of the entire storage stack, repeated under any sustained read load.
> 
> This was originally diagnosed and fixed by Kai-Heng Feng in September
> 2024 [1]. That patch used udelay(4); Keith Busch objected that the delay
> is merely a side effect of the read, and that flushing the pending
> device-to-host writes is what the erratum actually requires. Kai-Heng
> agreed to respin with a dummy register read. The thread then stalled on
> an open question from Manivannan Sadhasivam [2]: whether the read must
> target the child device (the "MSI initiator" named by the erratum)
> rather than the VMD, and whether the workaround belongs in the NVMe
> driver instead. No revision followed, and the erratum has remained
> unmitigated in mainline since.
> 
> This implements the flush read Keith asked for, and answers the open
> question empirically: the read MUST complete at the initiating child
> device. A read of the VMD's own config BAR was tried first and does not
> help - it terminates at the VMD and never traverses the downstream
> link, so it does not order against the child's posted writes (measured:
> timeout rate unchanged). Reading the initiator's config space does
> order correctly: per PCIe ordering rules the read completion cannot
> pass the device's earlier posted writes, so returning from the read
> guarantees the completion queue entry is visible.
> 
> The initiator's config address is captured per-IRQ at MSI allocation
> time from the requesting device, so the hot path adds one config read
> only on affected parts, and only for vectors owned by a child device.
> The read is serialized with cfg_lock like all other VMD config access.
> No NVMe driver change is needed.
> 
> Measured on a Dell Pro Max 14 MC14250 (Meteor Lake, VMD 8086:7d0b,
> KIOXIA BG6 512GB, 7.0.0-30-generic). The drive is healthy (46C, 0 media
> errors, 2% used) and ASPM is disabled on the link with all L1 substates
> off, so neither ASPM nor APST is involved.
> 
> Dropping caches and reading 3000 shared libraries, measuring
> /proc/pressure/io "full" (every task on the system blocked on I/O),
> with nvme_core.io_timeout=5:
> 
>   unpatched:      round 1  wall 37.4s   full I/O stall 32.6s   timeouts 0
>                   round 2  wall 30.6s   full I/O stall 28.7s   timeouts 1
>                   round 3  wall 30.3s   full I/O stall 27.4s   timeouts 1
> 
>   VMD-BAR read:   round 1  wall 44.8s   full I/O stall 38.6s   timeouts 8
>   (insufficient)  round 2  wall 234.8s  full I/O stall 207.9s  timeouts 37
>                   round 3  wall 3.6s    full I/O stall 0.8s    timeouts 0
> 
>   this patch:     round 1  wall 3.6s    full I/O stall 1.3s    timeouts 0
>                   round 2  wall 3.6s    full I/O stall 1.3s    timeouts 0
>                   round 3  wall 3.6s    full I/O stall 1.4s    timeouts 0
> 
> Before any workaround, 208 seconds of total-system I/O stall
> accumulated in the first 13 minutes of uptime, roughly 27% of wall
> clock. Userspace experiences this as GUI applications taking 30-60+
> seconds to start while throughput between stalls looks entirely normal
> (1.9 GB/s QD1) - which is what makes the fault easy to misattribute to
> ASPM or to the drive.

Nice work debugging and fixing a super annoying problem!  Thank you!

> [1] https://lore.kernel.org/all/20240909082657.19660-1-kai.heng.feng@canonical.com/
> [2] https://lkml.iu.edu/hypermail/linux/kernel/2409.1/08047.html
> 
> Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Suggested-by: Keith Busch <kbusch@kernel.org>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Rickey Bartlett <subtexel@gmail.com>
> ---
> This revives the MTL016 workaround that Kai-Heng Feng posted in September
> 2024 [1]. To be clear about the provenance: the diagnosis and the fix are
> his. Intel confirmed the silicon bug in that thread. What is new here is
> the implementation Keith asked for, and the measurements needed to close
> the question the thread ended on.
> 
> That thread stalled on one unanswered point - whether the flush read has
> to target the NVMe child device, the "MSI initiator" named in the erratum,
> or whether the VMD's own register is enough, and whether the workaround
> belongs in the NVMe driver at all. Nobody had hardware in front of them to
> settle it, and the discussion stopped there. It has been two years, the
> erratum is still unmitigated in mainline, and every affected machine has
> been stalling that whole time.
> 
> I have the hardware, so here is the answer: the read must complete at
> the NVMe child device - the "MSI initiator" named in the erratum -
> exactly as Manivannan suspected. I first tried reading the VMD's own
> config BAR; it does not help (measured, numbers in the commit message:
> timeout rate was unchanged). A read that terminates at the VMD never
> traverses the downstream link, so PCIe ordering gives no guarantee
> about the child's posted writes. Reading the initiator's config space
> closes the race. The workaround still lives in the VMD driver rather
> than the NVMe driver: the VMD is the broken component, the initiator
> address is captured generically per-IRQ at MSI allocation time, and no
> NVMe-specific knowledge is needed.
> 
> Measurements are in the commit message.
> 
>  drivers/pci/controller/vmd.c | 52 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 50 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 241023e..e45ef8c 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -92,6 +92,22 @@ enum vmd_features {
>  	 * referred to as MEMBAR2 or MSI-X BAR.
>  	 */
>  	VMD_FEAT_USE_BIOS_INFO		= (1 << 6),
> +
> +	/*
> +	 * Meteor Lake VMD (device ID 0x7d0b) is affected by erratum MTL016:
> +	 * the VMD may signal its MSI before the posted writes that carry the
> +	 * child device's DMA data have landed in memory.  The demuxed handler
> +	 * then runs against a not-yet-coherent completion queue and misses the
> +	 * completion entirely, so the I/O is only recovered when the block
> +	 * layer timeout fires and polls the queue ("timeout, completion
> +	 * polled").  Intel's documented workaround is to issue a dummy read
> +	 * to the MSI initiator (the child device) before handling the
> +	 * interrupt: the read completion cannot pass the device's earlier
> +	 * posted writes, so it pulls them into memory per PCIe ordering
> +	 * rules.  A read that terminates at the VMD itself does not order
> +	 * against the child's writes and is not sufficient (measured).
> +	 */
> +	VMD_FEAT_INTERRUPT_QUIRK	= (1 << 7),
>  };
>  
>  #define VMD_BIOS_PM_QUIRK_LTR	0x1003	/* 3145728 ns */
> @@ -114,6 +130,9 @@ static DEFINE_RAW_SPINLOCK(list_lock);
>   * @irq:	back pointer to parent.
>   * @enabled:	true if driver enabled IRQ
>   * @virq:	the virtual IRQ value provided to the requesting driver.
> + * @flush_addr:	config space address of the initiating device, read before
> + *		demuxing to flush its posted writes (MTL016); NULL if the
> + *		VMD is not affected.
>   *
>   * Every MSI/MSI-X IRQ requested for a device in a VMD domain will be mapped to
>   * a VMD IRQ using this structure.
> @@ -123,8 +142,14 @@ struct vmd_irq {
>  	struct vmd_irq_list	*irq;
>  	bool			enabled;
>  	unsigned int		virq;
> +	void __iomem		*flush_addr;
>  };
>  
> +struct vmd_dev;
> +
> +static void __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
> +				  unsigned int devfn, int reg, int len);
> +
>  /**
>   * struct vmd_irq_list - list of driver requested IRQs mapping to a VMD vector
>   * @irq_list:	the list of irq's the VMD one demuxes to.
> @@ -132,12 +157,15 @@ struct vmd_irq {
>   * @count:	number of child IRQs assigned to this vector; used to track
>   *		sharing.
>   * @virq:	The underlying VMD Linux interrupt number
> + * @vmd:	back pointer to the owning VMD device; serializes the MTL016
> + *		flush read against other config space access.
>   */
>  struct vmd_irq_list {
>  	struct list_head	irq_list;
>  	struct srcu_struct	srcu;
>  	unsigned int		count;
>  	unsigned int		virq;
> +	struct vmd_dev		*vmd;
>  };
>  
>  struct vmd_dev {
> @@ -295,6 +323,13 @@ static int vmd_msi_alloc(struct irq_domain *domain, unsigned int virq,
>  		INIT_LIST_HEAD(&vmdirq->node);
>  		vmdirq->irq = vmd_next_irq(vmd, desc);
>  		vmdirq->virq = virq + i;
> +		if (vmd->features & VMD_FEAT_INTERRUPT_QUIRK) {
> +			struct pci_dev *pdev = msi_desc_to_pci_dev(desc);
> +
> +			vmdirq->flush_addr = vmd_cfg_addr(vmd, pdev->bus,
> +							  pdev->devfn,
> +							  PCI_VENDOR_ID, 2);
> +		}
>  
>  		irq_domain_set_info(domain, virq + i, vmdirq->irq->virq,
>  				    &vmd_msi_controller, vmdirq,
> @@ -756,8 +791,20 @@ static irqreturn_t vmd_irq(int irq, void *data)
>  	int idx;
>  
>  	idx = srcu_read_lock(&irqs->srcu);
> -	list_for_each_entry_rcu(vmdirq, &irqs->irq_list, node)
> +	list_for_each_entry_rcu(vmdirq, &irqs->irq_list, node) {
> +		/*
> +		 * MTL016: the MSI may have outrun the initiating device's
> +		 * posted writes (e.g. its NVMe completion entry).  A read
> +		 * that completes at the initiator flushes them, so the
> +		 * demuxed handler observes a coherent completion queue.
> +		 * The value is discarded; only the ordering matters.
> +		 */
> +		if (vmdirq->flush_addr) {
> +			guard(raw_spinlock)(&irqs->vmd->cfg_lock);
> +			readw(vmdirq->flush_addr);
> +		}
>  		generic_handle_irq(vmdirq->virq);
> +	}
>  	srcu_read_unlock(&irqs->srcu, idx);
>  
>  	return IRQ_HANDLED;
> @@ -788,6 +835,7 @@ static int vmd_alloc_irqs(struct vmd_dev *vmd)
>  			return err;
>  
>  		INIT_LIST_HEAD(&vmd->irqs[i].irq_list);
> +		vmd->irqs[i].vmd = vmd;
>  		vmd->irqs[i].virq = pci_irq_vector(dev, i);
>  		err = devm_request_irq(&dev->dev, vmd->irqs[i].virq,
>  				       vmd_irq, IRQF_NO_THREAD,
> @@ -1250,7 +1298,7 @@ static const struct pci_device_id vmd_ids[] = {
>  	{PCI_VDEVICE(INTEL, 0xa77f),
>  		.driver_data = VMD_FEATS_CLIENT,},
>  	{PCI_VDEVICE(INTEL, 0x7d0b),
> -		.driver_data = VMD_FEATS_CLIENT,},
> +		.driver_data = VMD_FEATS_CLIENT | VMD_FEAT_INTERRUPT_QUIRK,},
>  	{PCI_VDEVICE(INTEL, 0xad0b),
>  		.driver_data = VMD_FEATS_CLIENT,},
>  	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_VMD_9A0B),
> -- 
> 2.53.0
>