* [PATCH v2] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0
@ 2026-09-13 23:22 wei.liu
2026-09-13 23:34 ` sashiko-bot
2026-09-25 2:18 ` Michael Kelley
0 siblings, 2 replies; 3+ messages in thread
From: wei.liu @ 2026-09-13 23:22 UTC (permalink / raw)
To: Linux on Hyper-V List
Cc: mukeshrathor, anirudh, schakrabarti, Mukesh R, Asher Kariv,
Wei Liu, K. Y. Srinivasan, Haiyang Zhang, Dexuan Cui, Long Li,
Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
From: Mukesh R <mrathor@linux.microsoft.com>
Hyper-V Discrete Device Assignment (DDA) does not support legacy (INTX)
interrupts. When PCI_INTERRUPT_PIN is read, the pci-hyperv controller,
which captures configuration space accesses, returns 0 to the caller.
However, the current code only captures 8bit read access, thus 16bit/32bit
are missed, forwarded to the host/HW, and may yield a non zero value.
This fix addresses this issue, and makes sure that PCI_INTERRUPT_PIN is
hardwired to 0, while the other bytes, are those returned by the host/HW.
Signed-off-by: Asher Kariv <askariv@microsoft.com>
Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
I found this in our internal tree, massaged the code and commit message,
and posted it.
v2: fix a compilation error
---
drivers/pci/controller/pci-hyperv.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 89816a2bd7cd..bb4e6ca4ac1c 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1240,6 +1240,16 @@ static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where,
mb();
}
spin_unlock_irqrestore(&hbus->config_lock, flags);
+
+ /*
+ * Make sure PCI_INTERRUPT_PIN is hard-wired to 0, since it
+ * may be read using a 32bit read, which is skipped by the
+ * above emulation.
+ */
+ if (where <= PCI_INTERRUPT_PIN &&
+ PCI_INTERRUPT_PIN < (where + size))
+ *((char *)val + PCI_INTERRUPT_PIN - where) = 0;
+
} else {
dev_err(dev, "Attempt to read beyond a function's config space.\n");
}
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0
2026-09-13 23:22 [PATCH v2] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0 wei.liu
@ 2026-09-13 23:34 ` sashiko-bot
2026-09-25 2:18 ` Michael Kelley
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-13 23:34 UTC (permalink / raw)
To: wei.liu; +Cc: linux-pci, linux-hyperv
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Directly modifying a u32 buffer via a char pointer cast assumes Little Endian memory layout, clearing the wrong byte and corrupting adjacent registers on Big Endian architectures.
- [Medium] The patch hardwires PCI_INTERRUPT_PIN to 0 for 16/32-bit reads but completely ignores PCI_INTERRUPT_LINE, creating an inconsistent configuration space state.
--
commit badaaf9419fe573b12895ca135de8cb0ded77d3e
Author: Mukesh R <mrathor@linux.microsoft.com>
PCI: hv: hardwire PCI_INTERRUPT_PIN to 0
Hyper-V DDA does not support legacy interrupts, so the driver intercepts
8-bit configuration reads to hardwire PCI_INTERRUPT_PIN to 0. This commit
attempts to also hardwire the pin for 16-bit and 32-bit reads, which
previously bypassed the emulation and leaked host hardware values.
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 89816a2bd7cd3..bb4e6ca4ac1ce 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1240,6 +1240,16 @@ static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where,
> mb();
> }
> spin_unlock_irqrestore(&hbus->config_lock, flags);
> +
> + /*
> + * Make sure PCI_INTERRUPT_PIN is hard-wired to 0, since it
> + * may be read using a 32bit read, which is skipped by the
> + * above emulation.
> + */
> + if (where <= PCI_INTERRUPT_PIN &&
> + PCI_INTERRUPT_PIN < (where + size))
[Severity: Medium]
Will this leave PCI_INTERRUPT_LINE leaking host hardware state on 16-bit
and 32-bit reads?
The original code correctly hardwires both PCI_INTERRUPT_LINE and
PCI_INTERRUPT_PIN to 0 for 8-bit reads. By only checking and clearing
PCI_INTERRUPT_PIN here, a 16-bit or 32-bit read starting at
PCI_INTERRUPT_LINE will bypass the emulation and return the raw host
hardware legacy configuration to the guest.
> + *((char *)val + PCI_INTERRUPT_PIN - where) = 0;
[Severity: High]
Could this corrupt adjacent configuration registers on big-endian
architectures?
This pointer cast directly modifies the memory backing the u32 val buffer.
Because a u32 is stored in CPU native endianness, treating it as a char
array assumes a little-endian layout.
On a big-endian guest, the computed byte offset will point to the wrong byte
in the u32, such as clearing PCI_MIN_GNT instead of PCI_INTERRUPT_PIN. This
would leave the intended pin unmodified while corrupting another register.
> +
> } else {
> dev_err(dev, "Attempt to read beyond a function's config space.\n");
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260913232255.2801797-1-wei.liu@kernel.org?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH v2] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0
2026-09-13 23:22 [PATCH v2] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0 wei.liu
2026-09-13 23:34 ` sashiko-bot
@ 2026-09-25 2:18 ` Michael Kelley
1 sibling, 0 replies; 3+ messages in thread
From: Michael Kelley @ 2026-09-25 2:18 UTC (permalink / raw)
To: wei.liu@kernel.org, Linux on Hyper-V List
Cc: mukeshrathor@linux.microsoft.com, anirudh@anirudhrb.com,
schakrabarti@linux.microsoft.com, Mukesh R, Asher Kariv,
K. Y. Srinivasan, Haiyang Zhang, Dexuan Cui, Long Li,
Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
From: wei.liu@kernel.org <wei.liu@kernel.org> Sent: Sunday, September 13, 2026 4:23 PM
>
In the patch Subject, capitalize "Hardwire" for historical consistency. See [1].
> Hyper-V Discrete Device Assignment (DDA) does not support legacy (INTX)
> interrupts. When PCI_INTERRUPT_PIN is read, the pci-hyperv controller,
> which captures configuration space accesses, returns 0 to the caller.
>
> However, the current code only captures 8bit read access, thus 16bit/32bit
> are missed, forwarded to the host/HW, and may yield a non zero value.
>
> This fix addresses this issue, and makes sure that PCI_INTERRUPT_PIN is
> hardwired to 0, while the other bytes, are those returned by the host/HW.
It took me a bit to figure out what the above two paragraphs mean. For
more precision, how about:
The current code recognizes PCI_INTERRUPT_PIN reads and forces a zero
value, but only for 8 bit reads. If a 16 or 32 bit read overlaps the field, it
may be returned as a non-zero value. Fix this by recognizing such overlap,
and forcing the PCI_INTERRUPT_PIN byte to zero while retaining the
adjacent bytes returned by the host/HW.
>
> Signed-off-by: Asher Kariv <askariv@microsoft.com>
> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
> ---
> I found this in our internal tree, massaged the code and commit message,
> and posted it.
>
> v2: fix a compilation error
> ---
> drivers/pci/controller/pci-hyperv.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 89816a2bd7cd..bb4e6ca4ac1c 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1240,6 +1240,16 @@ static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where,
> mb();
> }
> spin_unlock_irqrestore(&hbus->config_lock, flags);
> +
> + /*
> + * Make sure PCI_INTERRUPT_PIN is hard-wired to 0, since it
> + * may be read using a 32bit read, which is skipped by the
"may be included in a 16 bit or 32 bit read, which is skipped by the"
> + * above emulation.
> + */
> + if (where <= PCI_INTERRUPT_PIN &&
> + PCI_INTERRUPT_PIN < (where + size))
> + *((char *)val + PCI_INTERRUPT_PIN - where) = 0;
> +
Does this same handling need to be applied to PCI_INTERRUPT_LINE?
> } else {
> dev_err(dev, "Attempt to read beyond a function's config space.\n");
> }
Michael
[1] https://lore.kernel.org/linux-hyperv/20240701172053.GA10100@bhelgaas/
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-25 2:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-13 23:22 [PATCH v2] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0 wei.liu
2026-09-13 23:34 ` sashiko-bot
2026-09-25 2:18 ` Michael Kelley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox