lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAErSpo5K-foOsdantKQhK0=KOmV6-GCRGfjYYrdyi7ijecv-3w@mail.gmail.com>
Date:	Wed, 2 May 2012 17:55:58 -0600
From:	Bjorn Helgaas <bhelgaas@...gle.com>
To:	Jiang Liu <liuj97@...il.com>
Cc:	Taku Izumi <izumi.taku@...fujitsu.com>,
	Kenji Kaneshige <kaneshige.kenji@...fujitsu.com>,
	Yinghai Lu <yinghai@...nel.org>,
	Jiang Liu <jiang.liu@...wei.com>,
	Keping Chen <chenkeping@...wei.com>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	"Pearson, Greg" <greg.pearson@...com>
Subject: Re: [PATCH 2/2] PCI, ACPI, x86: update MMCFG information when
 hot-plugging PCI host bridges

On Sun, Apr 8, 2012 at 11:12 AM, Jiang Liu <liuj97@...il.com> wrote:
> This patch enhances pci_root driver to update MMCFG information when
> hot-plugging PCI root bridges on x86 platforms.
>
> Signed-off-by: Jiang Liu <jiang.liu@...wei.com>
> ---
>  arch/x86/pci/acpi.c      |   58 ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/pci_root.c  |   20 ++++++++++++++++
>  include/acpi/acnames.h   |    1 +
>  include/linux/pci-acpi.h |    3 ++
>  4 files changed, 82 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index da0149d..9184970 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -488,6 +488,64 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
>        return bus;
>  }
>
> +int arch_acpi_pci_root_add(struct acpi_pci_root *root)
> +{
> +       int result = 0;
> +       acpi_status status;
> +       unsigned long long base_addr;
> +       struct pci_mmcfg_region *cfg;
> +
> +       /*
> +        * Try to insert MMCFG information for host bridges with _CBA method
> +        */
> +       status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
> +                                      NULL, &base_addr);

I would prefer to have _CBA evaluated in drivers/acpi/pci_root.c, not
in the arch code.  It's true that _CBA is probably only used by x86
today, but the spec says nothing about it being arch-dependent, and I
suspect it may be used on arm soon.

> +       if (ACPI_SUCCESS(status)) {
> +               result = pci_mmconfig_insert(root->segment,
> +                                            root->secondary.start,
> +                                            root->secondary.end,
> +                                            base_addr);
> +               /*
> +                * MMCFG information for hot-pluggable host bridges may have
> +                * already been added by __pci_mmcfg_init();
> +                */
> +               if (result == -EEXIST)
> +                       result = 0;
> +       } else if (status == AE_NOT_FOUND) {
> +               /*
> +                * Check whether MMCFG information has been added for
> +                * host bridges without _CBA method.
> +                */
> +               rcu_read_lock();
> +               cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
> +               if (!cfg || cfg->end_bus < root->secondary.end)
> +                       result = -ENODEV;
> +               rcu_read_unlock();
> +       } else
> +               result = -ENODEV;
> +
> +       return result;
> +}
> +
> +int arch_acpi_pci_root_remove(struct acpi_pci_root *root)
> +{
> +       acpi_status status;
> +       unsigned long long base_addr;
> +
> +       /* Remove MMCFG information for host bridges with _CBA method */
> +       status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
> +                                      NULL, &base_addr);

I think the arch-specific "map MMCONFIG space at this addr" should
return a pointer or something that we can save and use to unmap it on
remove.  That way we don't have to evaluate _CBA again.

> +       if (ACPI_SUCCESS(status))
> +               return pci_mmconfig_delete(root->segment,
> +                                          root->secondary.start,
> +                                          root->secondary.end,
> +                                          base_addr);
> +       else if (status != AE_NOT_FOUND)
> +               return -ENODEV;
> +
> +       return 0;
> +}
> +
>  int __init pci_acpi_init(void)
>  {
>        struct pci_dev *dev = NULL;
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 4a7d575..a62bfa8 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -448,6 +448,16 @@ out:
>  }
>  EXPORT_SYMBOL(acpi_pci_osc_control_set);
>
> +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root)
> +{
> +       return 0;
> +}
> +
> +int __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root)
> +{
> +       return 0;
> +}
> +
>  static int __devinit acpi_pci_root_add(struct acpi_device *device)
>  {
>        unsigned long long segment, bus;
> @@ -504,6 +514,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
>        strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
>        device->driver_data = root;
>
> +       if (arch_acpi_pci_root_add(root)) {
> +               printk(KERN_ERR PREFIX
> +                       "can't add MMCFG information for Bus %04x:%02x\n",
> +                       root->segment, (unsigned int)root->secondary.start);
> +               result = -ENODEV;
> +               goto out_free;
> +       }
> +
>        /*
>         * All supported architectures that use ACPI have support for
>         * PCI domains, so we indicate this in _OSC support capabilities.
> @@ -629,6 +647,7 @@ out_del_root:
>        list_del_rcu(&root->node);
>        mutex_unlock(&acpi_pci_root_lock);
>        synchronize_rcu();
> +       arch_acpi_pci_root_remove(root);
>  out_free:
>        kfree(root);
>        return result;
> @@ -679,6 +698,7 @@ out:
>        list_del_rcu(&root->node);
>        mutex_unlock(&acpi_pci_root_lock);
>        synchronize_rcu();
> +       arch_acpi_pci_root_remove(root);
>        kfree(root);
>
>        return 0;
> diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h
> index 38f5088..99bda75 100644
> --- a/include/acpi/acnames.h
> +++ b/include/acpi/acnames.h
> @@ -62,6 +62,7 @@
>  #define METHOD_NAME__AEI        "_AEI"
>  #define METHOD_NAME__PRW        "_PRW"
>  #define METHOD_NAME__SRS        "_SRS"
> +#define METHOD_NAME__CBA       "_CBA"
>
>  /* Method names - these methods must appear at the namespace root */
>
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index ac93634..816b971 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -38,6 +38,9 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
>
>  void acpi_pci_root_rescan(void);
>
> +extern int arch_acpi_pci_root_add(struct acpi_pci_root *root);
> +extern int arch_acpi_pci_root_remove(struct acpi_pci_root *root);
> +
>  #else
>
>  static inline void acpi_pci_root_rescan(void) { }

I don't know where to put this in the conversation, but I think we
should separate the question of whether we do blind probing from the
question of how we set up MMCONFIG space from MCFG.

We currently parse the entire MCFG in some sort of initcall, but I
think we *could* change the x86 blind probing routines to attempt to
set up MMCONFIG space for the buses they find, just like
acpi_pci_root_add() does it for the ACPI host bridges.

That way we can continue blind probing, but make MMCONFIG init more sensible.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ