Re: [PATCH 3/3] pci-arbiter: Implement memory mapping over region files

[Samuel Thibault]

Neat!

:D

Joan Lledó, le dim. 12 déc. 2021 14:08:17 +0100, a ecrit:
> From: Joan Lledó <jlledom@member.fsf.org>
> 
> * pci-arbiter/Makefile:
>       * Add device_map.c to sources
> * pci-arbiter/device_map.c:
> * pci-arbiter/device_map.h:
>       * New module for device mapping
>       * Relies on libpciaccess mapping methods
> * pci-arbiter/func_files.c:
>       * io_region_file(): Use the new device mapping module
> * pci-arbiter/netfs_impl.c:
>       * Implements netfs_get_filemap():
>               * Uses the device mapping module to map the region to the
>                 arbiter space
>               * Calls the kernel RPC vm_region_create_proxy() to obtain the
>                 memory object proxy
>               * Only region files are mapped for now
> ---
>  pci-arbiter/Makefile     |  2 +-
>  pci-arbiter/device_map.c | 38 ++++++++++++++++++++++++++++++++++
>  pci-arbiter/device_map.h | 32 +++++++++++++++++++++++++++++
>  pci-arbiter/func_files.c | 16 ++++++---------
>  pci-arbiter/netfs_impl.c | 44 ++++++++++++++++++++++++++++++++++++----
>  5 files changed, 117 insertions(+), 15 deletions(-)
>  create mode 100644 pci-arbiter/device_map.c
>  create mode 100644 pci-arbiter/device_map.h
> 
> diff --git a/pci-arbiter/Makefile b/pci-arbiter/Makefile
> index d3d205ec..821c3ca9 100644
> --- a/pci-arbiter/Makefile
> +++ b/pci-arbiter/Makefile
> @@ -22,7 +22,7 @@ PORTDIR = $(srcdir)/port
>  
>  SRCS         = main.c pci-ops.c netfs_impl.c \
>                 pcifs.c ncache.c options.c func_files.c \
> -               pciServer.c startup_notifyServer.c
> +               device_map.c pciServer.c startup_notifyServer.c
>  OBJS         = $(SRCS:.c=.o) $(MIGSTUBS)
>  
>  HURDLIBS= fshelp ports shouldbeinlibc netfs iohelp ihash trivfs machdev
> diff --git a/pci-arbiter/device_map.c b/pci-arbiter/device_map.c
> new file mode 100644
> index 00000000..216adfb9
> --- /dev/null
> +++ b/pci-arbiter/device_map.c
> @@ -0,0 +1,38 @@
> +/*
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +
> +   This file is part of the GNU Hurd.
> +
> +   The GNU Hurd is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU General Public License as
> +   published by the Free Software Foundation; either version 2, or (at
> +   your option) any later version.
> +
> +   The GNU Hurd is distributed in the hope that it will be useful, but
> +   WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with the GNU Hurd.  If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +/* Implementation for device memory mapping functions */
> +
> +#include <pciaccess.h>
> +
> +#include "device_map.h"
> +
> +error_t
> +device_map_region (struct pci_device *device, struct pci_mem_region *region)
> +{
> +  error_t err = 0;
> +
> +  if (region->memory == 0)
> +    {
> +      err = pci_device_map_range (device, region->base_addr, region->size,
> +                               PCI_DEV_MAP_FLAG_WRITABLE, &region->memory);
> +    }
> +
> +  return err;
> +}
> diff --git a/pci-arbiter/device_map.h b/pci-arbiter/device_map.h
> new file mode 100644
> index 00000000..9062e901
> --- /dev/null
> +++ b/pci-arbiter/device_map.h
> @@ -0,0 +1,32 @@
> +/*
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +
> +   This file is part of the GNU Hurd.
> +
> +   The GNU Hurd is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU General Public License as
> +   published by the Free Software Foundation; either version 2, or (at
> +   your option) any later version.
> +
> +   The GNU Hurd is distributed in the hope that it will be useful, but
> +   WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with the GNU Hurd.  If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +/* Header for device memory mapping functions */
> +
> +#ifndef DEVICE_MAP_H
> +#define DEVICE_MAP_H
> +
> +#include <hurd.h>
> +
> +#include <pciaccess.h>
> +
> +error_t device_map_region (struct pci_device *device,
> +                        struct pci_mem_region *region);
> +
> +#endif /* DEVICE_MAP_H */
> diff --git a/pci-arbiter/func_files.c b/pci-arbiter/func_files.c
> index d7d8c5d5..81ebfded 100644
> --- a/pci-arbiter/func_files.c
> +++ b/pci-arbiter/func_files.c
> @@ -30,6 +30,8 @@
>  
>  #include <pciaccess.h>
>  
> +#include "device_map.h"
> +
>  /* Read or write a block of data from/to the configuration space */
>  static error_t
>  config_block_op (struct pci_device *dev, off_t offset, size_t * len,
> @@ -202,16 +204,10 @@ io_region_file (struct pcifs_dirent * e, off_t offset, 
> size_t * len,
>      region_block_ioport_op (region->base_addr, offset, len, data, read);
>    else
>      {
> -      /* First check whether the region is already mapped */
> -      if (region->memory == 0)
> -     {
> -       /* Not mapped, try to map it now */
> -       err =
> -         pci_device_map_range (e->device, region->base_addr, region->size,
> -                               PCI_DEV_MAP_FLAG_WRITABLE, &region->memory);
> -       if (err)
> -         return err;
> -     }
> +      /* Ensure the region is mapped */
> +      err = device_map_region (e->device, region);
> +      if (err)
> +     return err;
>        if (read)
>       memcpy (data, region->memory + offset, *len);
>        else
> diff --git a/pci-arbiter/netfs_impl.c b/pci-arbiter/netfs_impl.c
> index 9a4b4d30..8b4bd22b 100644
> --- a/pci-arbiter/netfs_impl.c
> +++ b/pci-arbiter/netfs_impl.c
> @@ -29,11 +29,16 @@
>  #include <unistd.h>
>  #include <sys/mman.h>
>  #include <hurd/netfs.h>
> +#include <hurd/paths.h>
> +#include <mach/mach4.h>
> +#include <device/device.h>
> +
> +#include <pciaccess.h>
>  
>  #include "pcifs.h"
>  #include "ncache.h"
> -#include <pciaccess.h>
>  #include "func_files.h"
> +#include "device_map.h"
>  
>  #define DIRENTS_CHUNK_SIZE      (8*1024)
>  /* Returned directory entries are aligned to blocks this many bytes long.
> @@ -564,12 +569,43 @@ netfs_node_norefs (struct node *node)
>    destroy_node (node);
>  }
>  
> -/* Return a memory object proxy port (send right) for the file contents of 
> NP.
> -   PROT is the maximum allowable access. On errors, return MACH_PORT_NULL and
> -   set errno. */
>  mach_port_t
>  netfs_get_filemap (struct node *node, vm_prot_t prot)
>  {
> +  error_t err;
> +  memory_object_t proxy;
> +  vm_prot_t max_prot;
> +  size_t reg_num;
> +  struct pci_mem_region *region;
> +
> +  /* Only regions files can be mapped */
> +  if (strncmp
> +      (node->nn->ln->name, FILE_REGION_NAME, strlen (FILE_REGION_NAME)))
> +    {
> +      goto error;
> +    }
> +
> +  /* Get region info */
> +  reg_num =
> +    strtol (&node->nn->ln->name[strlen (node->nn->ln->name) - 1], 0, 16);
> +  region = &node->nn->ln->device->regions[reg_num];
> +
> +  /* Ensure the region is mapped */
> +  err = device_map_region (node->nn->ln->device, region);
> +  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);
> +  if (err)
> +    goto error;
> +
> +  return proxy;
> +
> +error:
>    errno = EOPNOTSUPP;
>    return MACH_PORT_NULL;
>  }
> -- 
> 2.31.1