Re: [PATCH libacpica] Allow read/write to pci config

[Samuel Thibault]

Damien Zammit, le dim. 25 juin 2023 01:36:34 +0000, a ecrit:
> @@ -405,16 +408,16 @@ Add acgnu.h and acgnuex.h
>  +acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
>  +             u64 *value, u32 width)
>  +{
> -+  acpi_os_printf("ACPI: Tried to read pci config\n");
> -+  return 1;
> ++  pci_device_cfg_read(lookup_pci_dev(pci_id), (void *)value, reg, width, 
> NULL);

Please check whether lookup_pci_dev returned NULL.

There is no need to cast into (void *)

>  +static inline acpi_status
>  +acpi_os_write_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
>  +        u64 value, u32 width)
>  +{
> -+  acpi_os_printf("ACPI: Tried to write pci config\n");
> -+  return 1;
> ++  pci_device_cfg_write(lookup_pci_dev(pci_id), (const void *)&value, reg, 
> width, NULL);

This is assuming little-endian. We need to copy value into a variable of
the proper size and write from that.

> diff --git a/debian/patches/acpi-init-files.diff 
> b/debian/patches/acpi-init-files.diff
> index b83eafd..2fead2b 100644
> --- a/debian/patches/acpi-init-files.diff
> +++ b/debian/patches/acpi-init-files.diff
> ++#define NUMDEVS 32
> ++static struct pci_device *pci_devices[NUMDEVS];

Please use a dynamic allocation, we'll get bitten hard by that sooner or
later.

> @@ -40,6 +48,43 @@
>  +  struct slots *next;
>  +};
>  +
> ++static void
> ++pci_init(void)
> ++{
> ++  int i = 0;
> ++  struct pci_device_iterator *dev_iter;
> ++  struct pci_device *pci_dev;
> ++
> ++  pci_system_init ();
> ++
> ++  dev_iter = pci_slot_match_iterator_create (NULL);
> ++  while (((pci_dev = pci_device_next (dev_iter)) != NULL)
> ++      && (i < NUMDEVS))

You can use two loops to count first how many devices there are.

> ++    {
> ++      pci_device_probe(pci_dev);
> ++      pci_devices[i++] = pci_dev;
> ++    }
> ++  numdevs = i;
> ++}
> ++
> ++struct pci_device *
> ++lookup_pci_dev(struct acpi_pci_id *pci_id)
> ++{
> ++  int i;
> ++
> ++  for (i = 0; i < numdevs; i++)
> ++    {
> ++      if ((pci_devices[i]->bus == pci_id->bus)
> ++       && (pci_devices[i]->dev == pci_id->device)
> ++       && (pci_devices[i]->func == pci_id->function))

Please also compare pci_devices[i]->domain with pci_id->segment.

> -+  if (ioperm(0, 0x10000, 1))
> ++  /* Avoid giving ioperm to the PCI cfg registers
> ++   * since GNU/Hurd controls these

You mean pci-arbiter? Please say so, otherwise "GNU/Hurd" is really too
imprecise.

Samuel