linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] PCI: hv: Avoid the retarget interrupt hypercall in irq_unmask() on ARM64
@ 2022-02-09  2:37 Boqun Feng
  2022-02-09 16:12 ` Bjorn Helgaas
  0 siblings, 1 reply; 3+ messages in thread
From: Boqun Feng @ 2022-02-09  2:37 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Wei Liu
  Cc: Sunil Muthuswamy, Boqun Feng, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Dexuan Cui, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas, linux-hyperv,
	linux-pci, linux-kernel

On ARM64 Hyper-V guests, SPIs are used for the interrupts of virtual PCI
devices, and SPIs can be managed directly via GICD registers. Therefore
the retarget interrupt hypercall is not needed on ARM64.

The retarget interrupt hypercall related code is now put in a helper
function and only called on x86.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 drivers/pci/controller/pci-hyperv.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 20ea2ee330b8..80aa33ef5bf0 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1457,7 +1457,7 @@ static void hv_irq_mask(struct irq_data *data)
 }
 
 /**
- * hv_irq_unmask() - "Unmask" the IRQ by setting its current
+ * __hv_irq_unmask() - "Unmask" the IRQ by setting its current
  * affinity.
  * @data:	Describes the IRQ
  *
@@ -1466,7 +1466,7 @@ static void hv_irq_mask(struct irq_data *data)
  * is built out of this PCI bus's instance GUID and the function
  * number of the device.
  */
-static void hv_irq_unmask(struct irq_data *data)
+static void __hv_irq_unmask(struct irq_data *data)
 {
 	struct msi_desc *msi_desc = irq_data_get_msi_desc(data);
 	struct hv_retarget_device_interrupt *params;
@@ -1569,6 +1569,13 @@ static void hv_irq_unmask(struct irq_data *data)
 	if (!hv_result_success(res) && hbus->state != hv_pcibus_removing)
 		dev_err(&hbus->hdev->device,
 			"%s() failed: %#llx", __func__, res);
+}
+
+static void hv_irq_unmask(struct irq_data *data)
+{
+	/* Only use a hypercall on x86 */
+	if (IS_ENABLED(CONFIG_X86))
+		__hv_irq_unmask(data);
 
 	if (data->parent_data->chip->irq_unmask)
 		irq_chip_unmask_parent(data);
-- 
2.35.1


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

* Re: [RFC PATCH] PCI: hv: Avoid the retarget interrupt hypercall in irq_unmask() on ARM64
  2022-02-09  2:37 [RFC PATCH] PCI: hv: Avoid the retarget interrupt hypercall in irq_unmask() on ARM64 Boqun Feng
@ 2022-02-09 16:12 ` Bjorn Helgaas
  2022-02-10  0:31   ` Boqun Feng
  0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Helgaas @ 2022-02-09 16:12 UTC (permalink / raw)
  To: Boqun Feng
  Cc: Lorenzo Pieralisi, Wei Liu, Sunil Muthuswamy, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Dexuan Cui, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas, linux-hyperv,
	linux-pci, linux-kernel

On Wed, Feb 09, 2022 at 10:37:20AM +0800, Boqun Feng wrote:
> On ARM64 Hyper-V guests, SPIs are used for the interrupts of virtual PCI
> devices, and SPIs can be managed directly via GICD registers. Therefore
> the retarget interrupt hypercall is not needed on ARM64.
> 
> The retarget interrupt hypercall related code is now put in a helper
> function and only called on x86.
> 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  drivers/pci/controller/pci-hyperv.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 20ea2ee330b8..80aa33ef5bf0 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1457,7 +1457,7 @@ static void hv_irq_mask(struct irq_data *data)
>  }
>  
>  /**
> - * hv_irq_unmask() - "Unmask" the IRQ by setting its current
> + * __hv_irq_unmask() - "Unmask" the IRQ by setting its current
>   * affinity.
>   * @data:	Describes the IRQ
>   *
> @@ -1466,7 +1466,7 @@ static void hv_irq_mask(struct irq_data *data)
>   * is built out of this PCI bus's instance GUID and the function
>   * number of the device.
>   */
> -static void hv_irq_unmask(struct irq_data *data)
> +static void __hv_irq_unmask(struct irq_data *data)
>  {
>  	struct msi_desc *msi_desc = irq_data_get_msi_desc(data);
>  	struct hv_retarget_device_interrupt *params;
> @@ -1569,6 +1569,13 @@ static void hv_irq_unmask(struct irq_data *data)
>  	if (!hv_result_success(res) && hbus->state != hv_pcibus_removing)
>  		dev_err(&hbus->hdev->device,
>  			"%s() failed: %#llx", __func__, res);
> +}
> +
> +static void hv_irq_unmask(struct irq_data *data)
> +{
> +	/* Only use a hypercall on x86 */

This comment isn't useful because it only repeats what we can already
see from the "IS_ENABLED(CONFIG_X86)" below and it doesn't say
anything about *why*.

Didn't we just go though an exercise of adding interfaces for
arch-specific things, i.e., 831c1ae725f7 ("PCI: hv: Make the code arch
neutral by adding arch specific interfaces")?  Maybe this should be
another such interface?

If you add Hyper-V support for a third arch, this #ifdef will likely
be silently incorrect.  If you add an interface, there's at least a
clue that this needs to be evaluated.

> +	if (IS_ENABLED(CONFIG_X86))
> +		__hv_irq_unmask(data);
>  
>  	if (data->parent_data->chip->irq_unmask)
>  		irq_chip_unmask_parent(data);
> -- 
> 2.35.1
> 

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

* Re: [RFC PATCH] PCI: hv: Avoid the retarget interrupt hypercall in irq_unmask() on ARM64
  2022-02-09 16:12 ` Bjorn Helgaas
@ 2022-02-10  0:31   ` Boqun Feng
  0 siblings, 0 replies; 3+ messages in thread
From: Boqun Feng @ 2022-02-10  0:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lorenzo Pieralisi, Wei Liu, Sunil Muthuswamy, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Dexuan Cui, Rob Herring,
	Krzysztof Wilczyński, Bjorn Helgaas, linux-hyperv,
	linux-pci, linux-kernel

On Wed, Feb 09, 2022 at 10:12:20AM -0600, Bjorn Helgaas wrote:
> On Wed, Feb 09, 2022 at 10:37:20AM +0800, Boqun Feng wrote:
> > On ARM64 Hyper-V guests, SPIs are used for the interrupts of virtual PCI
> > devices, and SPIs can be managed directly via GICD registers. Therefore
> > the retarget interrupt hypercall is not needed on ARM64.
> > 
> > The retarget interrupt hypercall related code is now put in a helper
> > function and only called on x86.
> > 
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> >  drivers/pci/controller/pci-hyperv.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> > index 20ea2ee330b8..80aa33ef5bf0 100644
> > --- a/drivers/pci/controller/pci-hyperv.c
> > +++ b/drivers/pci/controller/pci-hyperv.c
> > @@ -1457,7 +1457,7 @@ static void hv_irq_mask(struct irq_data *data)
> >  }
> >  
> >  /**
> > - * hv_irq_unmask() - "Unmask" the IRQ by setting its current
> > + * __hv_irq_unmask() - "Unmask" the IRQ by setting its current
> >   * affinity.
> >   * @data:	Describes the IRQ
> >   *
> > @@ -1466,7 +1466,7 @@ static void hv_irq_mask(struct irq_data *data)
> >   * is built out of this PCI bus's instance GUID and the function
> >   * number of the device.
> >   */
> > -static void hv_irq_unmask(struct irq_data *data)
> > +static void __hv_irq_unmask(struct irq_data *data)
> >  {
> >  	struct msi_desc *msi_desc = irq_data_get_msi_desc(data);
> >  	struct hv_retarget_device_interrupt *params;
> > @@ -1569,6 +1569,13 @@ static void hv_irq_unmask(struct irq_data *data)
> >  	if (!hv_result_success(res) && hbus->state != hv_pcibus_removing)
> >  		dev_err(&hbus->hdev->device,
> >  			"%s() failed: %#llx", __func__, res);
> > +}
> > +
> > +static void hv_irq_unmask(struct irq_data *data)
> > +{
> > +	/* Only use a hypercall on x86 */
> 
> This comment isn't useful because it only repeats what we can already
> see from the "IS_ENABLED(CONFIG_X86)" below and it doesn't say
> anything about *why*.
> 
> Didn't we just go though an exercise of adding interfaces for
> arch-specific things, i.e., 831c1ae725f7 ("PCI: hv: Make the code arch
> neutral by adding arch specific interfaces")?  Maybe this should be
> another such interface?
> 
> If you add Hyper-V support for a third arch, this #ifdef will likely
> be silently incorrect.  If you add an interface, there's at least a
> clue that this needs to be evaluated.
> 

You are right. I will make __hv_irq_unmask() as an arch-specific
interface in the next version (probably with a better name). Thank you!

Regards,
Boqun

> > +	if (IS_ENABLED(CONFIG_X86))
> > +		__hv_irq_unmask(data);
> >  
> >  	if (data->parent_data->chip->irq_unmask)
> >  		irq_chip_unmask_parent(data);
> > -- 
> > 2.35.1
> > 

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

end of thread, other threads:[~2022-02-10  1:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09  2:37 [RFC PATCH] PCI: hv: Avoid the retarget interrupt hypercall in irq_unmask() on ARM64 Boqun Feng
2022-02-09 16:12 ` Bjorn Helgaas
2022-02-10  0:31   ` Boqun Feng

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).