[PATCH] Add a comment about endian-ness of capabilities recorded in pci config space

[PATCH] Add a comment about endian-ness of capabilities recorded in pci config space

[Ani Sinha]

Values in PCI configuration space are written in little endian format. This
includes capability values recorded in pci config space for pci bridges etc.
Add comment to remind developers to make sure the values are written in little
endian for all platforms and that the code must be endian safe.
Please also see the following fix which resolves an issue related to code not
being endian safe for BE systems:

0e464f7d99311 ("pci: fix PCI resource reserve capability on BE")

Signed-off-by: Ani Sinha <ani@anisinha.ca>
---
 include/hw/pci/pci_bridge.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index a94d350034..5fc6841833 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -143,6 +143,8 @@ typedef struct PCIBridgeQemuCap {
 /*
  * PCI BUS/IO/MEM/PREFMEM additional resources recorded as a
  * capability in PCI configuration space to reserve on firmware init.
+ * The values of these parameters must be in little-endian and
+ * the code assigning the values must be endian safe for all platforms.
  */
 typedef struct PCIResReserve {
     uint32_t bus;
-- 
2.25.1

Re: [PATCH] Add a comment about endian-ness of capabilities recorded in pci config space

[Michael S. Tsirkin]

On Mon, Oct 25, 2021 at 03:47:36PM +0530, Ani Sinha wrote:
> Values in PCI configuration space are written in little endian format. This
> includes capability values recorded in pci config space for pci bridges etc.
> Add comment to remind developers to make sure the values are written in little
> endian for all platforms and that the code must be endian safe.
> Please also see the following fix which resolves an issue related to code not
> being endian safe for BE systems:

drop an empty line here maybe

> 
> 0e464f7d99311 ("pci: fix PCI resource reserve capability on BE")
> Signed-off-by: Ani Sinha <ani@anisinha.ca>

If we are doing this, I think the correct approach is
to start adding __bitwise__ annotations and fix up
make sparse so we can have endian-ness automatically checked.


> ---
>  include/hw/pci/pci_bridge.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
> index a94d350034..5fc6841833 100644
> --- a/include/hw/pci/pci_bridge.h
> +++ b/include/hw/pci/pci_bridge.h
> @@ -143,6 +143,8 @@ typedef struct PCIBridgeQemuCap {
>  /*
>   * PCI BUS/IO/MEM/PREFMEM additional resources recorded as a
>   * capability in PCI configuration space to reserve on firmware init.
> + * The values of these parameters must be in little-endian and
> + * the code assigning the values must be endian safe for all platforms.
>   */
>  typedef struct PCIResReserve {
>      uint32_t bus;
> -- 
> 2.25.1