Re: [PATCH] pci-arbiter: Stop using deprecated region memory pointer

[Samuel Thibault]

Joan Lledó, le sam. 08 janv. 2022 13:15:37 +0100, a ecrit:
> From: Joan Lledó <jlledom@member.fsf.org>
> 
> Use a internal array of pointers instead

Applied, thanks!

Samuel

> * pci-arbiter/device_map.h:
>   * Update device_map_region() prototype
>     * Now it receives an output address as parameter
> * pci-arbiter/device_map.c:
>   * Update device_map_region() definition to match the new prototype
>   * Support for legacy mappings
>     * When the base address is lower than 1 mb
> * pci-arbiter/func_files.c:
> * pci-arbiter/netfs_impl.c:
>   * Update calls to device_map_region to match the new prototype
>   * Use the internal array of pointers instead of region->memory
> * pci-arbiter/pcifs.h:
>   * struct pcifs_dirent: Declare the internal array of pointers
> ---
>  pci-arbiter/device_map.c | 19 +++++++++++++++----
>  pci-arbiter/device_map.h |  2 +-
>  pci-arbiter/func_files.c |  6 +++---
>  pci-arbiter/netfs_impl.c |  8 +++++---
>  pci-arbiter/pcifs.h      |  7 +++++++
>  5 files changed, 31 insertions(+), 11 deletions(-)
> 
> diff --git a/pci-arbiter/device_map.c b/pci-arbiter/device_map.c
> index 216adfb9..1627746d 100644
> --- a/pci-arbiter/device_map.c
> +++ b/pci-arbiter/device_map.c
> @@ -24,14 +24,25 @@
>  #include "device_map.h"
>  
>  error_t
> -device_map_region (struct pci_device *device, struct pci_mem_region *region)
> +device_map_region (struct pci_device *device, struct pci_mem_region *region,
> +                void **addr)
>  {
>    error_t err = 0;
>  
> -  if (region->memory == 0)
> +  if (*addr == 0)
>      {
> -      err = pci_device_map_range (device, region->base_addr, region->size,
> -                               PCI_DEV_MAP_FLAG_WRITABLE, &region->memory);
> +      /*
> +       * We could use the non-legacy call for all ranges, but libpciaccess
> +       * offers a call for ranges under 1Mb. We call it for those cases, even
> +       * when there's no difference for us.
> +       */
> +      if (region->base_addr > 0x100000
> +       || region->base_addr + region->size > 0x100000)
> +        err = pci_device_map_range (device, region->base_addr, region->size,
> +                                 PCI_DEV_MAP_FLAG_WRITABLE, addr);
> +      else
> +        err = pci_device_map_legacy (device, region->base_addr, region->size,
> +                                 PCI_DEV_MAP_FLAG_WRITABLE, addr);
>      }
>  
>    return err;
> diff --git a/pci-arbiter/device_map.h b/pci-arbiter/device_map.h
> index 9062e901..0d92650c 100644
> --- a/pci-arbiter/device_map.h
> +++ b/pci-arbiter/device_map.h
> @@ -27,6 +27,6 @@
>  #include <pciaccess.h>
>  
>  error_t device_map_region (struct pci_device *device,
> -                        struct pci_mem_region *region);
> +                        struct pci_mem_region *region, void **addr);
>  
>  #endif /* DEVICE_MAP_H */
> diff --git a/pci-arbiter/func_files.c b/pci-arbiter/func_files.c
> index 81ebfded..40706135 100644
> --- a/pci-arbiter/func_files.c
> +++ b/pci-arbiter/func_files.c
> @@ -205,13 +205,13 @@ io_region_file (struct pcifs_dirent * e, off_t offset, 
> size_t * len,
>    else
>      {
>        /* Ensure the region is mapped */
> -      err = device_map_region (e->device, region);
> +      err = device_map_region (e->device, region, &e->region_maps[reg_num]);
>        if (err)
>       return err;
>        if (read)
> -     memcpy (data, region->memory + offset, *len);
> +     memcpy (data, e->region_maps[reg_num] + offset, *len);
>        else
> -     memcpy (region->memory + offset, data, *len);
> +     memcpy (e->region_maps[reg_num] + offset, data, *len);
>      }
>  
>    return err;
> diff --git a/pci-arbiter/netfs_impl.c b/pci-arbiter/netfs_impl.c
> index 8b4bd22b..63f8354e 100644
> --- a/pci-arbiter/netfs_impl.c
> +++ b/pci-arbiter/netfs_impl.c
> @@ -591,15 +591,17 @@ netfs_get_filemap (struct node *node, vm_prot_t prot)
>    region = &node->nn->ln->device->regions[reg_num];
>  
>    /* Ensure the region is mapped */
> -  err = device_map_region (node->nn->ln->device, region);
> +  err = device_map_region (node->nn->ln->device, region,
> +                        &node->nn->ln->region_maps[reg_num]);
>    if (err)
>      return err;
>  
>    /* Create a new memory object proxy with the required protection */
>    max_prot = (VM_PROT_READ | VM_PROT_WRITE) & prot;
>    err =
> -    vm_region_create_proxy(mach_task_self (), (vm_address_t)region->memory,
> -                         max_prot, region->size, &proxy);
> +    vm_region_create_proxy(mach_task_self (),
> +                        (vm_address_t)node->nn->ln->region_maps[reg_num],
> +                        max_prot, region->size, &proxy);
>    if (err)
>      goto error;
>  
> diff --git a/pci-arbiter/pcifs.h b/pci-arbiter/pcifs.h
> index 18f2141c..050c9e32 100644
> --- a/pci-arbiter/pcifs.h
> +++ b/pci-arbiter/pcifs.h
> @@ -91,6 +91,13 @@ struct pcifs_dirent
>     * Only for entries having a full B/D/F address.
>     */
>    struct pci_device *device;
> +
> +  /*
> +   * Array of addresses where regions are mapped
> +   *
> +   * Only when a device is present
> +   */
> +  void *region_maps[6];
>  };
>  
>  /*
> -- 
> 2.31.1