diff mbox series

[v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN

Message ID 20240621210018.350429-1-wei.liu@kernel.org (mailing list archive)
State New
Delegated to: Krzysztof WilczyƄski
Headers show
Series [v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN | expand

Commit Message

Wei Liu June 21, 2024, 9 p.m. UTC
The intent of the code snippet is to always return 0 for both
PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.

The check misses PCI_INTERRUPT_PIN. This patch fixes that.

This is discovered by this call in VFIO:

    pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);

The old code does not set *val to 0 because it misses the check for
PCI_INTERRUPT_PIN.

Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs")
Cc: stable@kernel.org
Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
v2:
* Change the commit subject line and message
* Change the code according to feedback
---
 drivers/pci/controller/pci-hyperv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Michael Kelley June 23, 2024, 10:05 p.m. UTC | #1
From: Wei Liu <wei.liu@kernel.org> Sent: Friday, June 21, 2024 2:00 PM
> 
> The intent of the code snippet is to always return 0 for both
> PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.
> 
> The check misses PCI_INTERRUPT_PIN. This patch fixes that.
> 
> This is discovered by this call in VFIO:
> 
>     pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
> 
> The old code does not set *val to 0 because it misses the check for
> PCI_INTERRUPT_PIN.
> 
> Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V
> VMs")
> Cc: stable@kernel.org
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
> ---
> v2:
> * Change the commit subject line and message
> * Change the code according to feedback
> ---
>  drivers/pci/controller/pci-hyperv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 5992280e8110..cdd5be16021d 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1130,8 +1130,8 @@ static void _hv_pcifront_read_config(struct hv_pci_dev
> *hpdev, int where,
>  		   PCI_CAPABILITY_LIST) {
>  		/* ROM BARs are unimplemented */
>  		*val = 0;
> -	} else if (where >= PCI_INTERRUPT_LINE && where + size <=
> -		   PCI_INTERRUPT_PIN) {
> +	} else if ((where >= PCI_INTERRUPT_LINE && where + size <=
> PCI_INTERRUPT_PIN) ||
> +		   (where >= PCI_INTERRUPT_PIN && where + size <= PCI_MIN_GNT)) {
>  		/*
>  		 * Interrupt Line and Interrupt PIN are hard-wired to zero
>  		 * because this front-end only supports message-signaled
> --
> 2.43.0
> 

Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Bjorn Helgaas June 26, 2024, 3:10 p.m. UTC | #2
1) Capitalize subject to match history
2) Say something more specific than "fix reading ..."

Apparently this returns garbage in some case where you want to return
zero?

On Fri, Jun 21, 2024 at 09:00:18PM +0000, Wei Liu wrote:
> The intent of the code snippet is to always return 0 for both
> PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.
> 
> The check misses PCI_INTERRUPT_PIN. This patch fixes that.
> 
> This is discovered by this call in VFIO:
> 
>     pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
> 
> The old code does not set *val to 0 because it misses the check for
> PCI_INTERRUPT_PIN.
> 
> Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs")
> Cc: stable@kernel.org
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
> ---
> v2:
> * Change the commit subject line and message
> * Change the code according to feedback
> ---
>  drivers/pci/controller/pci-hyperv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 5992280e8110..cdd5be16021d 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1130,8 +1130,8 @@ static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where,
>  		   PCI_CAPABILITY_LIST) {
>  		/* ROM BARs are unimplemented */
>  		*val = 0;
> -	} else if (where >= PCI_INTERRUPT_LINE && where + size <=
> -		   PCI_INTERRUPT_PIN) {
> +	} else if ((where >= PCI_INTERRUPT_LINE && where + size <= PCI_INTERRUPT_PIN) ||
> +		   (where >= PCI_INTERRUPT_PIN && where + size <= PCI_MIN_GNT)) {
>  		/*
>  		 * Interrupt Line and Interrupt PIN are hard-wired to zero
>  		 * because this front-end only supports message-signaled
> -- 
> 2.43.0
>
diff mbox series

Patch

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 5992280e8110..cdd5be16021d 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1130,8 +1130,8 @@  static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where,
 		   PCI_CAPABILITY_LIST) {
 		/* ROM BARs are unimplemented */
 		*val = 0;
-	} else if (where >= PCI_INTERRUPT_LINE && where + size <=
-		   PCI_INTERRUPT_PIN) {
+	} else if ((where >= PCI_INTERRUPT_LINE && where + size <= PCI_INTERRUPT_PIN) ||
+		   (where >= PCI_INTERRUPT_PIN && where + size <= PCI_MIN_GNT)) {
 		/*
 		 * Interrupt Line and Interrupt PIN are hard-wired to zero
 		 * because this front-end only supports message-signaled