Re: [PATCH v2] xen: workaround missing device_type property in pci/pcie nodes

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Stefano,

> On 9 Feb 2021, at 19:53, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> 
> PCI buses differ from default buses in a few important ways, so it is
> important to detect them properly. Normally, PCI buses are expected to
> have the following property:
> 
>    device_type = "pci"
> 
> In reality, it is not always the case. To handle PCI bus nodes that
> don't have the device_type property, also consider the node name: if the
> node name is "pcie" or "pci" then consider the bus as a PCI bus.
> 
> This commit is based on the Linux kernel commit
> d1ac0002dd29 "of: address: Work around missing device_type property in
> pcie nodes".
> 
> This fixes Xen boot on RPi4. Some RPi4 kernels have the following node
> on their device trees:
> 
> &pcie0 {
>       pci@1,0 {
>               #address-cells = <3>;
>               #size-cells = <2>;
>               ranges;
> 
>               reg = <0 0 0 0 0>;
> 
>               usb@1,0 {
>                               reg = <0x10000 0 0 0 0>;
>                               resets = <&reset 
> RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
>               };
>       };
> };
> 
> The pci@1,0 node is a PCI bus. If we parse the node and its children as
> a default bus, the reg property under usb@1,0 would have to be
> interpreted as an address range mappable by the CPU, which is not the
> case and would break.
> 
> Link: https://lore.kernel.org/xen-devel/YBmQQ3Tzu++AadKx@xxxxxxxxxxxxxxxx/
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
With the type, spaces and tab fixes already found:

Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Thanks the commit message is more clear :-)

Cheers
Bertrand

> ---
> Changes in v2:
> - improve commit message
> 
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 18825e333e..f1a96a3b90 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -563,14 +563,28 @@ static unsigned int dt_bus_default_get_flags(const 
> __be32 *addr)
>  * PCI bus specific translator
>  */
> 
> +static bool_t dt_node_is_pci(const struct dt_device_node *np)
> +{
> +    bool is_pci = !strcmp(np->name, "pcie") || !strcmp(np->name, "pci");
> +
> +    if (is_pci)
> +        printk(XENLOG_WARNING "%s: Missing device_type\n", np->full_name);
> +
> +    return is_pci;
> +}
> +
> static bool_t dt_bus_pci_match(const struct dt_device_node *np)
> {
>     /*
>      * "pciex" is PCI Express "vci" is for the /chaos bridge on 1st-gen PCI
>      * powermacs "ht" is hypertransport
> +     *
> +     * If none of the device_type match, and that the node name is
> +     * "pcie" or "pci", accept the device as PCI (with a warning).
>      */
>     return !strcmp(np->type, "pci") || !strcmp(np->type, "pciex") ||
> -        !strcmp(np->type, "vci") || !strcmp(np->type, "ht");
> +        !strcmp(np->type, "vci") || !strcmp(np->type, "ht") ||
> +        dt_node_is_pci(np);
> }
> 
> static void dt_bus_pci_count_cells(const struct dt_device_node *np,
>