On Sun, Sep 13, 2026 at 10:19:39PM +0000, [email protected] 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 <[email protected]>
>
> 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/[email protected]?part=1
