diff mbox series

[v6,4/8] arm64: PCI: Support root bridge preparation for Hyper-V

Message ID 20210726180657.142727-5-boqun.feng@gmail.com (mailing list archive)
State New, archived
Headers show
Series PCI: hv: Support host bridge probing on ARM64 | expand

Commit Message

Boqun Feng July 26, 2021, 6:06 p.m. UTC
Currently at root bridge preparation, the corresponding ACPI device will
be set as the companion, however for a Hyper-V virtual PCI root bridge,
there is no corresponding ACPI device, because a Hyper-V virtual PCI
root bridge is discovered via VMBus rather than ACPI table. In order to
support this, we need to make pcibios_root_bridge_prepare() work with
cfg->parent being NULL.

Use a NULL pointer as the ACPI device if there is no corresponding ACPI
device, and this is fine because: 1) ACPI_COMPANION_SET() can work with
the second parameter being NULL, 2) semantically, if a NULL pointer is
set via ACPI_COMPANION_SET(), ACPI_COMPANION() (the read API for this
field) will return NULL, and since ACPI_COMPANION() may return NULL, so
users must have handled the cases where it returns NULL, and 3) since
there is no corresponding ACPI device, it would be wrong to use any
other value here.

Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 arch/arm64/kernel/pci.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Boqun Feng Aug. 19, 2021, 4:13 p.m. UTC | #1
Catalin and Will,

Appreciate it that you can have a look at this one and patch #4, note
that there exists an alternative solution at[1].

The difference is the way used to pass the corresponding ACPI device
pointers for PCI host bridges: currently pci_config_window->parent is
used, and this patch and patch #4 allow the field to be NULL, because
Hyper-V's PCI host bridges don't have ACPI devices, while [1] changes to
use pci_host_bridge->private. And I'm OK with either way, I don't have a
strong opinion here ;-)

Looking forwards to your suggestion, Thanks!

Regards,
Boqun

[1]: https://lore.kernel.org/lkml/20210811153619.88922-1-boqun.feng@gmail.com/

On Tue, Jul 27, 2021 at 02:06:53AM +0800, Boqun Feng wrote:
> Currently at root bridge preparation, the corresponding ACPI device will
> be set as the companion, however for a Hyper-V virtual PCI root bridge,
> there is no corresponding ACPI device, because a Hyper-V virtual PCI
> root bridge is discovered via VMBus rather than ACPI table. In order to
> support this, we need to make pcibios_root_bridge_prepare() work with
> cfg->parent being NULL.
> 
> Use a NULL pointer as the ACPI device if there is no corresponding ACPI
> device, and this is fine because: 1) ACPI_COMPANION_SET() can work with
> the second parameter being NULL, 2) semantically, if a NULL pointer is
> set via ACPI_COMPANION_SET(), ACPI_COMPANION() (the read API for this
> field) will return NULL, and since ACPI_COMPANION() may return NULL, so
> users must have handled the cases where it returns NULL, and 3) since
> there is no corresponding ACPI device, it would be wrong to use any
> other value here.
> 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  arch/arm64/kernel/pci.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 5148ae242780..2276689b5411 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -90,7 +90,17 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
>  		return 0;
>  
>  	cfg = bridge->bus->sysdata;
> -	adev = to_acpi_device(cfg->parent);
> +
> +	/*
> +	 * On Hyper-V there is no corresponding ACPI device for a root bridge,
> +	 * therefore ->parent is set as NULL by the driver. And set 'adev' as
> +	 * NULL in this case because there is no proper ACPI device.
> +	 */
> +	if (!cfg->parent)
> +		adev = NULL;
> +	else
> +		adev = to_acpi_device(cfg->parent);
> +
>  	bus_dev = &bridge->bus->dev;
>  
>  	ACPI_COMPANION_SET(&bridge->dev, adev);
> -- 
> 2.32.0
>
Catalin Marinas Aug. 20, 2021, 5:49 p.m. UTC | #2
Hi Boqun,

Sorry, I just got back from holiday and I'm still in the deleting emails
mode.

On Fri, Aug 20, 2021 at 12:13:22AM +0800, Boqun Feng wrote:
> Appreciate it that you can have a look at this one and patch #4, note
> that there exists an alternative solution at[1].
> 
> The difference is the way used to pass the corresponding ACPI device
> pointers for PCI host bridges: currently pci_config_window->parent is
> used, and this patch and patch #4 allow the field to be NULL, because
> Hyper-V's PCI host bridges don't have ACPI devices, while [1] changes to
> use pci_host_bridge->private. And I'm OK with either way, I don't have a
> strong opinion here ;-)
[...]
> [1]: https://lore.kernel.org/lkml/20210811153619.88922-1-boqun.feng@gmail.com/

I'm ok with the arm64 bits in this series and the one you linked above.
It's up to Lorenzo if he's happy with how pci-hyperv.c ends up looking,
I'm not a PCIe expert. My preference would be for a combined series
(this and [1] above).

Happy to ack the arm64 patches in a combined series (if you are going to
post one), the changes would look even simpler.
Lorenzo Pieralisi Aug. 23, 2021, 9:12 a.m. UTC | #3
On Fri, Aug 20, 2021 at 06:49:47PM +0100, Catalin Marinas wrote:
> Hi Boqun,
> 
> Sorry, I just got back from holiday and I'm still in the deleting emails
> mode.
> 
> On Fri, Aug 20, 2021 at 12:13:22AM +0800, Boqun Feng wrote:
> > Appreciate it that you can have a look at this one and patch #4, note
> > that there exists an alternative solution at[1].
> > 
> > The difference is the way used to pass the corresponding ACPI device
> > pointers for PCI host bridges: currently pci_config_window->parent is
> > used, and this patch and patch #4 allow the field to be NULL, because
> > Hyper-V's PCI host bridges don't have ACPI devices, while [1] changes to
> > use pci_host_bridge->private. And I'm OK with either way, I don't have a
> > strong opinion here ;-)
> [...]
> > [1]: https://lore.kernel.org/lkml/20210811153619.88922-1-boqun.feng@gmail.com/
> 
> I'm ok with the arm64 bits in this series and the one you linked above.
> It's up to Lorenzo if he's happy with how pci-hyperv.c ends up looking,
> I'm not a PCIe expert. My preference would be for a combined series
> (this and [1] above).
> 
> Happy to ack the arm64 patches in a combined series (if you are going to
> post one), the changes would look even simpler.

I believe [1] above is an experiment - therefore it is best to stick to
this series as it is for the time being, pending refactoring that
requires more time, I would not rush it.

If you can ACK the arm64 patches (3,4) please I will pull the series
into the PCI tree asap.

Thanks,
Lorenzo
Catalin Marinas Aug. 23, 2021, 9:43 a.m. UTC | #4
On Tue, Jul 27, 2021 at 02:06:53AM +0800, Boqun Feng wrote:
> Currently at root bridge preparation, the corresponding ACPI device will
> be set as the companion, however for a Hyper-V virtual PCI root bridge,
> there is no corresponding ACPI device, because a Hyper-V virtual PCI
> root bridge is discovered via VMBus rather than ACPI table. In order to
> support this, we need to make pcibios_root_bridge_prepare() work with
> cfg->parent being NULL.
> 
> Use a NULL pointer as the ACPI device if there is no corresponding ACPI
> device, and this is fine because: 1) ACPI_COMPANION_SET() can work with
> the second parameter being NULL, 2) semantically, if a NULL pointer is
> set via ACPI_COMPANION_SET(), ACPI_COMPANION() (the read API for this
> field) will return NULL, and since ACPI_COMPANION() may return NULL, so
> users must have handled the cases where it returns NULL, and 3) since
> there is no corresponding ACPI device, it would be wrong to use any
> other value here.
> 
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
diff mbox series

Patch

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 5148ae242780..2276689b5411 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -90,7 +90,17 @@  int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
 		return 0;
 
 	cfg = bridge->bus->sysdata;
-	adev = to_acpi_device(cfg->parent);
+
+	/*
+	 * On Hyper-V there is no corresponding ACPI device for a root bridge,
+	 * therefore ->parent is set as NULL by the driver. And set 'adev' as
+	 * NULL in this case because there is no proper ACPI device.
+	 */
+	if (!cfg->parent)
+		adev = NULL;
+	else
+		adev = to_acpi_device(cfg->parent);
+
 	bus_dev = &bridge->bus->dev;
 
 	ACPI_COMPANION_SET(&bridge->dev, adev);