[+cc Aaron]

On Thu, Jan 30, 2014 at 12:20:38PM -0700, Bjorn Helgaas wrote:
> This reverts commit e8de1481fd71 ("resource: allow MMIO exclusivity for
> device drivers"), removing these exported interfaces:
> 
>   pci_request_region_exclusive()
>   pci_request_regions_exclusive()
>   pci_request_selected_regions_exclusive()
> 
> There's nothing wrong with the MMIO exclusivity code, but it is used only
> by the e1000e driver, and the only reason it's there is because it was
> added during a bug hunt.  The bug has been fixed, and apparently no other
> drivers have found it useful in the five years since then.
> 
> This is based on Stephen Hemminger's patch (see link below), but goes a bit
> further by removing the use in e1000e.
> 
> Link: 
> http://lkml.kernel.org/r/20131227132710.71906...@nehalam.linuxnetplumber.net
> Signed-off-by: Bjorn Helgaas <bhelg...@google.com>
> CC: Arjan van de Ven <ar...@linux.intel.com>
> CC: Stephen Hemminger <step...@networkplumber.org>

I'm dropping this one for now because it's used by:

  - e1000e
  - alpha pci_mmap_resource()
  - sp5100_tco_setupdevice()

That's still sort of a marginal number of users, but in any event, it's
clear that a little more work would be required to remove it.

> ---
>  Documentation/kernel-parameters.txt        |    4 -
>  arch/x86/mm/init.c                         |    2 -
>  drivers/net/ethernet/intel/e1000e/netdev.c |    3 -
>  drivers/pci/pci-sysfs.c                    |    3 -
>  drivers/pci/pci.c                          |  112 
> +++-------------------------
>  include/linux/ioport.h                     |    5 -
>  include/linux/pci.h                        |    3 -
>  kernel/resource.c                          |   54 --------------
>  8 files changed, 14 insertions(+), 172 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt 
> b/Documentation/kernel-parameters.txt
> index 8f441dab0396..4943bddeacc1 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1323,10 +1323,6 @@ bytes respectively. Such letter suffixes can also be 
> entirely omitted.
>                       no_x2apic_optout
>                               BIOS x2APIC opt-out request will be ignored
>  
> -     iomem=          Disable strict checking of access to MMIO memory
> -             strict  regions from userspace.
> -             relaxed
> -
>       iommu=          [x86]
>               off
>               force
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index f97130618113..575cbfd89238 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -583,8 +583,6 @@ int devmem_is_allowed(unsigned long pagenr)
>  {
>       if (pagenr < 256)
>               return 1;
> -     if (iomem_is_exclusive(pagenr << PAGE_SHIFT))
> -             return 0;
>       if (!page_is_ram(pagenr))
>               return 1;
>       return 0;
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
> b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 6d91933c4cdd..97a11b19e46f 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -6574,8 +6574,7 @@ static int e1000_probe(struct pci_dev *pdev, const 
> struct pci_device_id *ent)
>       }
>  
>       bars = pci_select_bars(pdev, IORESOURCE_MEM);
> -     err = pci_request_selected_regions_exclusive(pdev, bars,
> -                                                  e1000e_driver_name);
> +     err = pci_request_selected_regions(pdev, bars, e1000e_driver_name);
>       if (err)
>               goto err_pci_reg;
>  
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 276ef9c18802..4580fa859f38 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -993,9 +993,6 @@ pci_mmap_resource(struct kobject *kobj, struct 
> bin_attribute *attr,
>       vma->vm_pgoff += start >> PAGE_SHIFT;
>       mmap_type = res->flags & IORESOURCE_MEM ? pci_mmap_mem : pci_mmap_io;
>  
> -     if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(start))
> -             return -EINVAL;
> -
>       return pci_mmap_page_range(pdev, vma, mmap_type, write_combine);
>  }
>  
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 1febe90831b4..1011c0b281ca 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2432,26 +2432,20 @@ void pci_release_region(struct pci_dev *pdev, int bar)
>  }
>  
>  /**
> - *   __pci_request_region - Reserved PCI I/O and memory resource
> + *   pci_request_region - Reserved PCI I/O and memory resource
>   *   @pdev: PCI device whose resources are to be reserved
>   *   @bar: BAR to be reserved
>   *   @res_name: Name to be associated with resource.
> - *   @exclusive: whether the region access is exclusive or not
>   *
>   *   Mark the PCI region associated with PCI device @pdev BR @bar as
>   *   being reserved by owner @res_name.  Do not access any
>   *   address inside the PCI regions unless this call returns
>   *   successfully.
>   *
> - *   If @exclusive is set, then the region is marked so that userspace
> - *   is explicitly not allowed to map the resource via /dev/mem or
> - *   sysfs MMIO access.
> - *
>   *   Returns 0 on success, or %EBUSY on error.  A warning
>   *   message is also printed on failure.
>   */
> -static int __pci_request_region(struct pci_dev *pdev, int bar, const char 
> *res_name,
> -                                                                     int 
> exclusive)
> +int pci_request_region(struct pci_dev *pdev, int bar, const char *res_name)
>  {
>       struct pci_devres *dr;
>  
> @@ -2464,9 +2458,8 @@ static int __pci_request_region(struct pci_dev *pdev, 
> int bar, const char *res_n
>                       goto err_out;
>       }
>       else if (pci_resource_flags(pdev, bar) & IORESOURCE_MEM) {
> -             if (!__request_mem_region(pci_resource_start(pdev, bar),
> -                                     pci_resource_len(pdev, bar), res_name,
> -                                     exclusive))
> +             if (!request_mem_region(pci_resource_start(pdev, bar),
> +                                     pci_resource_len(pdev, bar), res_name))
>                       goto err_out;
>       }
>  
> @@ -2483,47 +2476,6 @@ err_out:
>  }
>  
>  /**
> - *   pci_request_region - Reserve PCI I/O and memory resource
> - *   @pdev: PCI device whose resources are to be reserved
> - *   @bar: BAR to be reserved
> - *   @res_name: Name to be associated with resource
> - *
> - *   Mark the PCI region associated with PCI device @pdev BAR @bar as
> - *   being reserved by owner @res_name.  Do not access any
> - *   address inside the PCI regions unless this call returns
> - *   successfully.
> - *
> - *   Returns 0 on success, or %EBUSY on error.  A warning
> - *   message is also printed on failure.
> - */
> -int pci_request_region(struct pci_dev *pdev, int bar, const char *res_name)
> -{
> -     return __pci_request_region(pdev, bar, res_name, 0);
> -}
> -
> -/**
> - *   pci_request_region_exclusive - Reserved PCI I/O and memory resource
> - *   @pdev: PCI device whose resources are to be reserved
> - *   @bar: BAR to be reserved
> - *   @res_name: Name to be associated with resource.
> - *
> - *   Mark the PCI region associated with PCI device @pdev BR @bar as
> - *   being reserved by owner @res_name.  Do not access any
> - *   address inside the PCI regions unless this call returns
> - *   successfully.
> - *
> - *   Returns 0 on success, or %EBUSY on error.  A warning
> - *   message is also printed on failure.
> - *
> - *   The key difference that _exclusive makes it that userspace is
> - *   explicitly not allowed to map the resource via /dev/mem or
> - *   sysfs.
> - */
> -int pci_request_region_exclusive(struct pci_dev *pdev, int bar, const char 
> *res_name)
> -{
> -     return __pci_request_region(pdev, bar, res_name, IORESOURCE_EXCLUSIVE);
> -}
> -/**
>   * pci_release_selected_regions - Release selected PCI I/O and memory 
> resources
>   * @pdev: PCI device whose resources were previously reserved
>   * @bars: Bitmask of BARs to be released
> @@ -2540,14 +2492,20 @@ void pci_release_selected_regions(struct pci_dev 
> *pdev, int bars)
>                       pci_release_region(pdev, i);
>  }
>  
> -static int __pci_request_selected_regions(struct pci_dev *pdev, int bars,
> -                              const char *res_name, int excl)
> +/**
> + * pci_request_selected_regions - Reserve selected PCI I/O and memory 
> resources
> + * @pdev: PCI device whose resources are to be reserved
> + * @bars: Bitmask of BARs to be requested
> + * @res_name: Name to be associated with resource
> + */
> +int pci_request_selected_regions(struct pci_dev *pdev, int bars,
> +                              const char *res_name)
>  {
>       int i;
>  
>       for (i = 0; i < 6; i++)
>               if (bars & (1 << i))
> -                     if (__pci_request_region(pdev, i, res_name, excl))
> +                     if (pci_request_region(pdev, i, res_name))
>                               goto err_out;
>       return 0;
>  
> @@ -2561,25 +2519,6 @@ err_out:
>  
>  
>  /**
> - * pci_request_selected_regions - Reserve selected PCI I/O and memory 
> resources
> - * @pdev: PCI device whose resources are to be reserved
> - * @bars: Bitmask of BARs to be requested
> - * @res_name: Name to be associated with resource
> - */
> -int pci_request_selected_regions(struct pci_dev *pdev, int bars,
> -                              const char *res_name)
> -{
> -     return __pci_request_selected_regions(pdev, bars, res_name, 0);
> -}
> -
> -int pci_request_selected_regions_exclusive(struct pci_dev *pdev,
> -                              int bars, const char *res_name)
> -{
> -     return __pci_request_selected_regions(pdev, bars, res_name,
> -                     IORESOURCE_EXCLUSIVE);
> -}
> -
> -/**
>   *   pci_release_regions - Release reserved PCI I/O and memory resources
>   *   @pdev: PCI device whose resources were previously reserved by 
> pci_request_regions
>   *
> @@ -2611,28 +2550,6 @@ int pci_request_regions(struct pci_dev *pdev, const 
> char *res_name)
>       return pci_request_selected_regions(pdev, ((1 << 6) - 1), res_name);
>  }
>  
> -/**
> - *   pci_request_regions_exclusive - Reserved PCI I/O and memory resources
> - *   @pdev: PCI device whose resources are to be reserved
> - *   @res_name: Name to be associated with resource.
> - *
> - *   Mark all PCI regions associated with PCI device @pdev as
> - *   being reserved by owner @res_name.  Do not access any
> - *   address inside the PCI regions unless this call returns
> - *   successfully.
> - *
> - *   pci_request_regions_exclusive() will mark the region so that
> - *   /dev/mem and the sysfs MMIO access will not be allowed.
> - *
> - *   Returns 0 on success, or %EBUSY on error.  A warning
> - *   message is also printed on failure.
> - */
> -int pci_request_regions_exclusive(struct pci_dev *pdev, const char *res_name)
> -{
> -     return pci_request_selected_regions_exclusive(pdev,
> -                                     ((1 << 6) - 1), res_name);
> -}
> -
>  static void __pci_set_master(struct pci_dev *dev, bool enable)
>  {
>       u16 old_cmd, cmd;
> @@ -4387,13 +4304,10 @@ EXPORT_SYMBOL(pci_find_capability);
>  EXPORT_SYMBOL(pci_bus_find_capability);
>  EXPORT_SYMBOL(pci_release_regions);
>  EXPORT_SYMBOL(pci_request_regions);
> -EXPORT_SYMBOL(pci_request_regions_exclusive);
>  EXPORT_SYMBOL(pci_release_region);
>  EXPORT_SYMBOL(pci_request_region);
> -EXPORT_SYMBOL(pci_request_region_exclusive);
>  EXPORT_SYMBOL(pci_release_selected_regions);
>  EXPORT_SYMBOL(pci_request_selected_regions);
> -EXPORT_SYMBOL(pci_request_selected_regions_exclusive);
>  EXPORT_SYMBOL(pci_set_master);
>  EXPORT_SYMBOL(pci_clear_master);
>  EXPORT_SYMBOL(pci_set_mwi);
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index 89b7c24a36e9..f2d45ea3ee53 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -49,7 +49,6 @@ struct resource {
>  #define IORESOURCE_WINDOW    0x00200000      /* forwarded by bridge */
>  #define IORESOURCE_MUXED     0x00400000      /* Resource is software muxed */
>  
> -#define IORESOURCE_EXCLUSIVE 0x08000000      /* Userland may not map this 
> resource */
>  #define IORESOURCE_DISABLED  0x10000000
>  #define IORESOURCE_UNSET     0x20000000
>  #define IORESOURCE_AUTO              0x40000000
> @@ -173,10 +172,7 @@ static inline unsigned long resource_type(const struct 
> resource *res)
>  /* Convenience shorthand with allocation */
>  #define request_region(start,n,name)         
> __request_region(&ioport_resource, (start), (n), (name), 0)
>  #define request_muxed_region(start,n,name)   
> __request_region(&ioport_resource, (start), (n), (name), IORESOURCE_MUXED)
> -#define __request_mem_region(start,n,name, excl) 
> __request_region(&iomem_resource, (start), (n), (name), excl)
>  #define request_mem_region(start,n,name) __request_region(&iomem_resource, 
> (start), (n), (name), 0)
> -#define request_mem_region_exclusive(start,n,name) \
> -     __request_region(&iomem_resource, (start), (n), (name), 
> IORESOURCE_EXCLUSIVE)
>  #define rename_region(region, newname) do { (region)->name = (newname); } 
> while (0)
>  
>  extern struct resource * __request_region(struct resource *,
> @@ -222,7 +218,6 @@ extern struct resource * __devm_request_region(struct 
> device *dev,
>  extern void __devm_release_region(struct device *dev, struct resource 
> *parent,
>                                 resource_size_t start, resource_size_t n);
>  extern int iomem_map_sanity_check(resource_size_t addr, unsigned long size);
> -extern int iomem_is_exclusive(u64 addr);
>  
>  extern int
>  walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages,
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index fb57c892b214..b3cd9d58f5a9 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1038,13 +1038,10 @@ void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),
>                   int (*)(const struct pci_dev *, u8, u8));
>  #define HAVE_PCI_REQ_REGIONS 2
>  int __must_check pci_request_regions(struct pci_dev *, const char *);
> -int __must_check pci_request_regions_exclusive(struct pci_dev *, const char 
> *);
>  void pci_release_regions(struct pci_dev *);
>  int __must_check pci_request_region(struct pci_dev *, int, const char *);
> -int __must_check pci_request_region_exclusive(struct pci_dev *, int, const 
> char *);
>  void pci_release_region(struct pci_dev *, int);
>  int pci_request_selected_regions(struct pci_dev *, int, const char *);
> -int pci_request_selected_regions_exclusive(struct pci_dev *, int, const char 
> *);
>  void pci_release_selected_regions(struct pci_dev *, int);
>  
>  /* drivers/pci/bus.c */
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 3f285dce9347..9a29d989aa5e 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -1306,57 +1306,3 @@ int iomem_map_sanity_check(resource_size_t addr, 
> unsigned long size)
>  
>       return err;
>  }
> -
> -#ifdef CONFIG_STRICT_DEVMEM
> -static int strict_iomem_checks = 1;
> -#else
> -static int strict_iomem_checks;
> -#endif
> -
> -/*
> - * check if an address is reserved in the iomem resource tree
> - * returns 1 if reserved, 0 if not reserved.
> - */
> -int iomem_is_exclusive(u64 addr)
> -{
> -     struct resource *p = &iomem_resource;
> -     int err = 0;
> -     loff_t l;
> -     int size = PAGE_SIZE;
> -
> -     if (!strict_iomem_checks)
> -             return 0;
> -
> -     addr = addr & PAGE_MASK;
> -
> -     read_lock(&resource_lock);
> -     for (p = p->child; p ; p = r_next(NULL, p, &l)) {
> -             /*
> -              * We can probably skip the resources without
> -              * IORESOURCE_IO attribute?
> -              */
> -             if (p->start >= addr + size)
> -                     break;
> -             if (p->end < addr)
> -                     continue;
> -             if (p->flags & IORESOURCE_BUSY &&
> -                  p->flags & IORESOURCE_EXCLUSIVE) {
> -                     err = 1;
> -                     break;
> -             }
> -     }
> -     read_unlock(&resource_lock);
> -
> -     return err;
> -}
> -
> -static int __init strict_iomem(char *str)
> -{
> -     if (strstr(str, "relaxed"))
> -             strict_iomem_checks = 0;
> -     if (strstr(str, "strict"))
> -             strict_iomem_checks = 1;
> -     return 1;
> -}
> -
> -__setup("iomem=", strict_iomem);
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to