* [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN
@ 2024-06-21 21:00 Wei Liu
2024-06-23 22:05 ` Michael Kelley
2024-06-26 15:10 ` Bjorn Helgaas
0 siblings, 2 replies; 6+ messages in thread
From: Wei Liu @ 2024-06-21 21:00 UTC (permalink / raw)
To: Linux on Hyper-V List
Cc: Wei Liu, stable, K. Y. Srinivasan, Haiyang Zhang, Dexuan Cui,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN
2024-06-21 21:00 [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN Wei Liu
@ 2024-06-23 22:05 ` Michael Kelley
2024-06-26 15:10 ` Bjorn Helgaas
1 sibling, 0 replies; 6+ messages in thread
From: Michael Kelley @ 2024-06-23 22:05 UTC (permalink / raw)
To: Wei Liu, Linux on Hyper-V List
Cc: stable@kernel.org, K. Y. Srinivasan, Haiyang Zhang, Dexuan Cui,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN
2024-06-21 21:00 [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN Wei Liu
2024-06-23 22:05 ` Michael Kelley
@ 2024-06-26 15:10 ` Bjorn Helgaas
2024-07-01 6:16 ` Wei Liu
1 sibling, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2024-06-26 15:10 UTC (permalink / raw)
To: Wei Liu
Cc: Linux on Hyper-V List, stable, K. Y. Srinivasan, Haiyang Zhang,
Dexuan Cui, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN
2024-06-26 15:10 ` Bjorn Helgaas
@ 2024-07-01 6:16 ` Wei Liu
2024-07-01 17:20 ` Bjorn Helgaas
0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2024-07-01 6:16 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Wei Liu, Linux on Hyper-V List, stable, K. Y. Srinivasan,
Haiyang Zhang, Dexuan Cui, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
On Wed, Jun 26, 2024 at 10:10:39AM -0500, Bjorn Helgaas wrote:
> 1) Capitalize subject to match history
What do you mean here? I got the "PCI: hv: ..." format from recent
commits. "PCI" is capitalized. You want to to capitalize "fix"?
> 2) Say something more specific than "fix reading ..."
>
> Apparently this returns garbage in some case where you want to return
> zero?
Yes. *val is not changed in the old code, so garbage is returned.
Here is the updated commit message. I can resend once you confirm you're
happy with it.
PCI: hv: Fix reading of PCI_INTERRUPT_PIN
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. Garbage is returned in this case.
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>
Thanks,
Wei.
>
> 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
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN
2024-07-01 6:16 ` Wei Liu
@ 2024-07-01 17:20 ` Bjorn Helgaas
2024-07-01 20:01 ` Wei Liu
0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2024-07-01 17:20 UTC (permalink / raw)
To: Wei Liu
Cc: Linux on Hyper-V List, stable, K. Y. Srinivasan, Haiyang Zhang,
Dexuan Cui, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
On Mon, Jul 01, 2024 at 06:16:18AM +0000, Wei Liu wrote:
> On Wed, Jun 26, 2024 at 10:10:39AM -0500, Bjorn Helgaas wrote:
> > 1) Capitalize subject to match history
>
> What do you mean here? I got the "PCI: hv: ..." format from recent
> commits. "PCI" is capitalized. You want to to capitalize "fix"?
Yes. Look at the history:
$ git log --oneline --no-merges drivers/pci/controller/pci-hyperv.c
b5ff74c1ef50 PCI: hv: Fix ring buffer size calculation
07e8f88568f5 x86/apic: Drop apic::delivery_mode
f741bcadfe52 PCI: hv: Annotate struct hv_dr_state with __counted_by
04bbe863241a PCI: hv: Fix a crash in hv_pci_restore_msi_msg() during hibernation
067d6ec7ed5b PCI: hv: Add a per-bus mutex state_lock
a847234e24d0 Revert "PCI: hv: Fix a timing issue which causes kdump to fail occasionally"
add9195e69c9 PCI: hv: Remove the useless hv_pcichild_state from struct hv_pci_dev
2738d5ab7929 PCI: hv: Fix a race condition in hv_irq_unmask() that can cause panic
...
> > 2) Say something more specific than "fix reading ..."
> >
> > Apparently this returns garbage in some case where you want to return
> > zero?
>
> Yes. *val is not changed in the old code, so garbage is returned.
>
> Here is the updated commit message. I can resend once you confirm you're
> happy with it.
>
> PCI: hv: Fix reading of PCI_INTERRUPT_PIN
Maybe:
PCI: hv: Return zero, not garbage, when reading PCI_INTERRUPT_PIN
> 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. Garbage is returned in this case.
>
> 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>
Looks fine.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN
2024-07-01 17:20 ` Bjorn Helgaas
@ 2024-07-01 20:01 ` Wei Liu
0 siblings, 0 replies; 6+ messages in thread
From: Wei Liu @ 2024-07-01 20:01 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Wei Liu, Linux on Hyper-V List, stable, K. Y. Srinivasan,
Haiyang Zhang, Dexuan Cui, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
On Mon, Jul 01, 2024 at 12:20:53PM -0500, Bjorn Helgaas wrote:
> On Mon, Jul 01, 2024 at 06:16:18AM +0000, Wei Liu wrote:
> > On Wed, Jun 26, 2024 at 10:10:39AM -0500, Bjorn Helgaas wrote:
> > > 1) Capitalize subject to match history
> >
> > What do you mean here? I got the "PCI: hv: ..." format from recent
> > commits. "PCI" is capitalized. You want to to capitalize "fix"?
>
> Yes. Look at the history:
>
> $ git log --oneline --no-merges drivers/pci/controller/pci-hyperv.c
> b5ff74c1ef50 PCI: hv: Fix ring buffer size calculation
> 07e8f88568f5 x86/apic: Drop apic::delivery_mode
> f741bcadfe52 PCI: hv: Annotate struct hv_dr_state with __counted_by
> 04bbe863241a PCI: hv: Fix a crash in hv_pci_restore_msi_msg() during hibernation
> 067d6ec7ed5b PCI: hv: Add a per-bus mutex state_lock
> a847234e24d0 Revert "PCI: hv: Fix a timing issue which causes kdump to fail occasionally"
> add9195e69c9 PCI: hv: Remove the useless hv_pcichild_state from struct hv_pci_dev
> 2738d5ab7929 PCI: hv: Fix a race condition in hv_irq_unmask() that can cause panic
> ...
>
> > > 2) Say something more specific than "fix reading ..."
> > >
> > > Apparently this returns garbage in some case where you want to return
> > > zero?
> >
> > Yes. *val is not changed in the old code, so garbage is returned.
> >
> > Here is the updated commit message. I can resend once you confirm you're
> > happy with it.
> >
> > PCI: hv: Fix reading of PCI_INTERRUPT_PIN
>
> Maybe:
>
> PCI: hv: Return zero, not garbage, when reading PCI_INTERRUPT_PIN
>
> > 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. Garbage is returned in this case.
> >
> > 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>
>
> Looks fine.
Thanks. I will resend with the updated commit message.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-07-01 20:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-21 21:00 [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN Wei Liu
2024-06-23 22:05 ` Michael Kelley
2024-06-26 15:10 ` Bjorn Helgaas
2024-07-01 6:16 ` Wei Liu
2024-07-01 17:20 ` Bjorn Helgaas
2024-07-01 20:01 ` Wei Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).