Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0
@ 2026-09-13 22:10 wei.liu
  2026-09-13 22:19 ` sashiko-bot
  2026-09-13 22:52 ` Wei Liu
  0 siblings, 2 replies; 4+ messages in thread
From: wei.liu @ 2026-09-13 22:10 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>
---
 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..e6db82413add 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] 4+ messages in thread

* Re: [PATCH] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0
  2026-09-13 22:10 [PATCH] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0 wei.liu
@ 2026-09-13 22:19 ` sashiko-bot
  2026-09-13 22:51   ` Wei Liu
  2026-09-13 22:52 ` Wei Liu
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-09-13 22:19 UTC (permalink / raw)
  To: wei.liu; +Cc: linux-hyperv, linux-pci

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The patch fails to hard-wire `PCI_INTERRUPT_LINE` to 0 during 16/32-bit reads, introducing inconsistent emulation behavior.
- [Medium] Modifying the returned integer via `char *` pointer arithmetic is broken on big-endian architectures.
- [Low] Extraneous closing brace causes a syntax error and breaks compilation.
--

commit 34dba20e549ad1a394e94eeb91577f3b24614432
Author: Mukesh R <mrathor@linux.microsoft.com>

PCI: hv: hardwire PCI_INTERRUPT_PIN to 0

This commit hardwires PCI_INTERRUPT_PIN to 0 for Hyper-V Discrete Device
Assignment, as it does not support legacy (INTx) interrupts. It addresses
an issue where 16-bit or 32-bit reads miss the previous 8-bit emulation
and yield non-zero values.

> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 89816a2bd7cd3..e6db82413addb 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]
Does this logic miss PCI_INTERRUPT_LINE?

The previous emulation blocks in _hv_pcifront_read_config() explicitly
hard-wire the 8-bit read of PCI_INTERRUPT_LINE to 0.

It seems a 16-bit or 32-bit config read encompassing PCI_INTERRUPT_LINE
would leak the hardware's non-zero value, violating the design intent to
hide legacy interrupts.

> +			*((char *)val + PCI_INTERRUPT_PIN - where) = 0;

[Severity: Medium]
Will modifying the returned integer via char * pointer arithmetic work on
big-endian architectures?

For a 32-bit read on big-endian systems, casting a u32 * to char * and
indexing by byte offset modifies the wrong byte in memory.

This could fail to clear PCI_INTERRUPT_PIN and might accidentally zero
another byte like PCI_MIN_GNT.

> +		}

[Severity: Low]
Could this extraneous closing brace cause a compilation failure?

It doesn't appear to match any opening brace in the added block, which
would break the block structure and lead to a syntax error.

>  	} else {
>  		dev_err(dev, "Attempt to read beyond a function's config space.\n");
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913221039.2218950-1-wei.liu@kernel.org?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0
  2026-09-13 22:19 ` sashiko-bot
@ 2026-09-13 22:51   ` Wei Liu
  0 siblings, 0 replies; 4+ messages in thread
From: Wei Liu @ 2026-09-13 22:51 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: wei.liu, linux-hyperv, linux-pci

On Sun, Sep 13, 2026 at 10:19:39PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] The patch fails to hard-wire `PCI_INTERRUPT_LINE` to 0 during 16/32-bit reads, introducing inconsistent emulation behavior.
> - [Medium] Modifying the returned integer via `char *` pointer arithmetic is broken on big-endian architectures.
> - [Low] Extraneous closing brace causes a syntax error and breaks compilation.
> --
> 
> commit 34dba20e549ad1a394e94eeb91577f3b24614432
> Author: Mukesh R <mrathor@linux.microsoft.com>
> 
> PCI: hv: hardwire PCI_INTERRUPT_PIN to 0
> 
> This commit hardwires PCI_INTERRUPT_PIN to 0 for Hyper-V Discrete Device
> Assignment, as it does not support legacy (INTx) interrupts. It addresses
> an issue where 16-bit or 32-bit reads miss the previous 8-bit emulation
> and yield non-zero values.
> 
> > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> > index 89816a2bd7cd3..e6db82413addb 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]
> Does this logic miss PCI_INTERRUPT_LINE?
> 
> The previous emulation blocks in _hv_pcifront_read_config() explicitly
> hard-wire the 8-bit read of PCI_INTERRUPT_LINE to 0.
> 
> It seems a 16-bit or 32-bit config read encompassing PCI_INTERRUPT_LINE
> would leak the hardware's non-zero value, violating the design intent to
> hide legacy interrupts.
> 

We have not seen this in practice.

> > +			*((char *)val + PCI_INTERRUPT_PIN - where) = 0;
> 
> [Severity: Medium]
> Will modifying the returned integer via char * pointer arithmetic work on
> big-endian architectures?
> 

We don't support big-endian.

> For a 32-bit read on big-endian systems, casting a u32 * to char * and
> indexing by byte offset modifies the wrong byte in memory.
> 
> This could fail to clear PCI_INTERRUPT_PIN and might accidentally zero
> another byte like PCI_MIN_GNT.
> 
> > +		}
> 

Oops. I had somehow deleted the opening brace when cleaning up the
patch.

A new version will be sent out.

Wei

> [Severity: Low]
> Could this extraneous closing brace cause a compilation failure?
> 
> It doesn't appear to match any opening brace in the added block, which
> would break the block structure and lead to a syntax error.
> 
> >  	} else {
> >  		dev_err(dev, "Attempt to read beyond a function's config space.\n");
> >  	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260913221039.2218950-1-wei.liu@kernel.org?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0
  2026-09-13 22:10 [PATCH] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0 wei.liu
  2026-09-13 22:19 ` sashiko-bot
@ 2026-09-13 22:52 ` Wei Liu
  1 sibling, 0 replies; 4+ messages in thread
From: Wei Liu @ 2026-09-13 22:52 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

On Sun, Sep 13, 2026 at 03:10:39PM -0700, wei.liu@kernel.org wrote:
> 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>
> ---
>  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..e6db82413add 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;
> +		}

Please ignore this version. A new version will be sent out to add back the
opening brace.

Wei

>  	} else {
>  		dev_err(dev, "Attempt to read beyond a function's config space.\n");
>  	}
> -- 
> 2.53.0
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-13 22:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-13 22:10 [PATCH] PCI: hv: hardwire PCI_INTERRUPT_PIN to 0 wei.liu
2026-09-13 22:19 ` sashiko-bot
2026-09-13 22:51   ` Wei Liu
2026-09-13 22:52 ` Wei Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox