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. > + *((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/[email protected]?part=1
