| Message ID | 20210209195334.21206-1-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] xen: workaround missing device_type property in pci/pcie nodes | expand |
On Tue, 9 Feb 2021, Stefano Stabellini 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>; there is one more tab than needed here, sorry > }; > }; > }; > > 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@mattapan.m5p.com/ > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> > --- > 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, >
On 09/02/2021 19:53, Stefano Stabellini wrote: > 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) bool on the function, and spaces here, which I'm sure you can fix while committing :) ~Andrew
On Tue, 9 Feb 2021, Andrew Cooper wrote: > On 09/02/2021 19:53, Stefano Stabellini wrote: > > 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) > > bool on the function, and spaces here, which I'm sure you can fix while > committing :) Oops, thanks Andrew
On Tue, Feb 09, 2021 at 11:53:34AM -0800, Stefano Stabellini wrote: > This fixes Xen boot on RPi4. Some RPi4 kernels have the following node > on their device trees: IMO I like keeping the reference to Linux kernel d1ac0002dd29 in the commit message. The commit is distinctly informative and takes some searching to find in the thread archive. This though is merely /my/ opinion. Two builds later to ensure I've got a build which doesn't work due to the problematic device-tree and one with the patch to ensure it does fix the issue and: Tested-by: Elliott Mitchell <ehem+xen@m5p.com> Note to Jukka Kaartinen, people who merely build from source are useful for confirming proposed fixes work. The above line gets added to commit messages to note people have tried it and it works for them.
On 10.2.2021 3.59, Elliott Mitchell wrote: > On Tue, Feb 09, 2021 at 11:53:34AM -0800, Stefano Stabellini wrote: >> This fixes Xen boot on RPi4. Some RPi4 kernels have the following node >> on their device trees: > > IMO I like keeping the reference to Linux kernel d1ac0002dd29 in the > commit message. The commit is distinctly informative and takes some > searching to find in the thread archive. This though is merely /my/ > opinion. > > Two builds later to ensure I've got a build which doesn't work due to the > problematic device-tree and one with the patch to ensure it does fix the > issue and: > > Tested-by: Elliott Mitchell <ehem+xen@m5p.com> > > > Note to Jukka Kaartinen, people who merely build from source are useful > for confirming proposed fixes work. The above line gets added to commit > messages to note people have tried it and it works for them. > > Thanks for the info! I tested the fix and it works fine. Tested-by: Jukka Kaartinen <jukka.kaartinen@unikie.com> -Jukka
Hi Stefano, > On 9 Feb 2021, at 19:53, Stefano Stabellini <sstabellini@kernel.org> 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@mattapan.m5p.com/ > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> With the type, spaces and tab fixes already found: Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> 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, >
On 09/02/2021 19:53, Stefano Stabellini 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@mattapan.m5p.com/ > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> Acked-by: Julien Grall <jgrall@amazon.com> Cheers,
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,
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@mattapan.m5p.com/ Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> --- Changes in v2: - improve commit message