| Message ID | 20220209023722.2866009-1-boqun.feng@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] PCI: hv: Avoid the retarget interrupt hypercall in irq_unmask() on ARM64 | expand |
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 >
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 > >
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);
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(-)