Re: [PATCH V4 15/23] acpi, mcfg: Implement two calls that might be used to inject/remove MCFG region.

[Tomasz Nowicki]

On 10.02.2016 15:06, Lorenzo Pieralisi wrote:

On Thu, Feb 04, 2016 at 06:28:53PM +0100, Tomasz Nowicki wrote:

>Lets abstract two calls which allow to inject and remove MCFG regions
>which may come from DSDT table. These calls will be used for x86 and ARM64
>PCI host bridge driver in the later patches.
>
>Signed-off-by: Tomasz Nowicki
>Tested-by: Duc Dang
>Tested-by: Dongdong Liu
>Tested-by: Hanjun Guo
>Tested-by: Graeme Gregory
>Tested-by: Sinan Kaya
>---
>  drivers/acpi/mcfg.c  | 38 ++
>  include/linux/pci-acpi.h |  9 +
>  2 files changed, 47 insertions(+)
>
>diff --git a/drivers/acpi/mcfg.c b/drivers/acpi/mcfg.c
>index 3e1e7be..dca4c4e 100644
>--- a/drivers/acpi/mcfg.c
>+++ b/drivers/acpi/mcfg.c
>@@ -10,6 +10,7 @@
>  #include 
>  #include 
>  #include 
>+#include 
>
>  #define   PREFIX  "MCFG: "
>
>@@ -77,6 +78,43 @@ int __init acpi_parse_mcfg(struct acpi_table_header *header)
>return 0;
>  }
>
>+int pci_mmcfg_setup_map(struct acpi_pci_root_info *ci)
>+{
>+   struct pci_mmcfg_region *cfg;
>+   struct acpi_pci_root *root;
>+   int seg, start, end, err;
>+
>+   root = ci->root;
>+   seg = root->segment;
>+   start = root->secondary.start;
>+   end = root->secondary.end;
>+
>+   cfg = pci_mmconfig_lookup(seg, start);
>+   if (cfg)
>+   return 0;
>+
>+   cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
>+   if (!cfg)
>+   return -ENOMEM;
>+
>+   err = pci_mmconfig_inject(cfg);
>+   return err;

When you integrate Jayachandran's patch this whole function will
become a pci_mmconfig_insert() and that's where hot_added should
be set.


>+}
>+
>+void pci_mmcfg_teardown_map(struct acpi_pci_root_info *ci)
>+{
>+   struct acpi_pci_root *root = ci->root;
>+   struct pci_mmcfg_region *cfg;
>+
>+   cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
>+   if (!cfg)
>+   return;
>+
>+   if (cfg->hot_added)

Move the hot_added check in pci_mmconfig_delete() (that does the look
up again), we do not want to carry out pci_mmconfig_lookup only to
check that flag here (and we miss rcu locking for the look-up BTW).


Makes sense to me, I will follow your suggestion.

Thanks,
Tomasz

Re: [PATCH V4 15/23] acpi, mcfg: Implement two calls that might be used to inject/remove MCFG region.

[Tomasz Nowicki]

On 10.02.2016 15:06, Lorenzo Pieralisi wrote:

On Thu, Feb 04, 2016 at 06:28:53PM +0100, Tomasz Nowicki wrote:

>Lets abstract two calls which allow to inject and remove MCFG regions
>which may come from DSDT table. These calls will be used for x86 and ARM64
>PCI host bridge driver in the later patches.
>
>Signed-off-by: Tomasz Nowicki
>Tested-by: Duc Dang
>Tested-by: Dongdong Liu
>Tested-by: Hanjun Guo
>Tested-by: Graeme Gregory
>Tested-by: Sinan Kaya
>---
>  drivers/acpi/mcfg.c  | 38 ++
>  include/linux/pci-acpi.h |  9 +
>  2 files changed, 47 insertions(+)
>
>diff --git a/drivers/acpi/mcfg.c b/drivers/acpi/mcfg.c
>index 3e1e7be..dca4c4e 100644
>--- a/drivers/acpi/mcfg.c
>+++ b/drivers/acpi/mcfg.c
>@@ -10,6 +10,7 @@
>  #include 
>  #include 
>  #include 
>+#include 
>
>  #define   PREFIX  "MCFG: "
>
>@@ -77,6 +78,43 @@ int __init acpi_parse_mcfg(struct acpi_table_header *header)
>return 0;
>  }
>
>+int pci_mmcfg_setup_map(struct acpi_pci_root_info *ci)
>+{
>+   struct pci_mmcfg_region *cfg;
>+   struct acpi_pci_root *root;
>+   int seg, start, end, err;
>+
>+   root = ci->root;
>+   seg = root->segment;
>+   start = root->secondary.start;
>+   end = root->secondary.end;
>+
>+   cfg = pci_mmconfig_lookup(seg, start);
>+   if (cfg)
>+   return 0;
>+
>+   cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
>+   if (!cfg)
>+   return -ENOMEM;
>+
>+   err = pci_mmconfig_inject(cfg);
>+   return err;

When you integrate Jayachandran's patch this whole function will
become a pci_mmconfig_insert() and that's where hot_added should
be set.


>+}
>+
>+void pci_mmcfg_teardown_map(struct acpi_pci_root_info *ci)
>+{
>+   struct acpi_pci_root *root = ci->root;
>+   struct pci_mmcfg_region *cfg;
>+
>+   cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
>+   if (!cfg)
>+   return;
>+
>+   if (cfg->hot_added)

Move the hot_added check in pci_mmconfig_delete() (that does the look
up again), we do not want to carry out pci_mmconfig_lookup only to
check that flag here (and we miss rcu locking for the look-up BTW).


Makes sense to me, I will follow your suggestion.

Thanks,
Tomasz

Re: [PATCH V4 15/23] acpi, mcfg: Implement two calls that might be used to inject/remove MCFG region.

[Lorenzo Pieralisi]

On Thu, Feb 04, 2016 at 06:28:53PM +0100, Tomasz Nowicki wrote:
> Lets abstract two calls which allow to inject and remove MCFG regions
> which may come from DSDT table. These calls will be used for x86 and ARM64
> PCI host bridge driver in the later patches.
> 
> Signed-off-by: Tomasz Nowicki 
> Tested-by: Duc Dang 
> Tested-by: Dongdong Liu 
> Tested-by: Hanjun Guo 
> Tested-by: Graeme Gregory 
> Tested-by: Sinan Kaya 
> ---
>  drivers/acpi/mcfg.c  | 38 ++
>  include/linux/pci-acpi.h |  9 +
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/acpi/mcfg.c b/drivers/acpi/mcfg.c
> index 3e1e7be..dca4c4e 100644
> --- a/drivers/acpi/mcfg.c
> +++ b/drivers/acpi/mcfg.c
> @@ -10,6 +10,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define  PREFIX  "MCFG: "
>  
> @@ -77,6 +78,43 @@ int __init acpi_parse_mcfg(struct acpi_table_header 
> *header)
>   return 0;
>  }
>  
> +int pci_mmcfg_setup_map(struct acpi_pci_root_info *ci)
> +{
> + struct pci_mmcfg_region *cfg;
> + struct acpi_pci_root *root;
> + int seg, start, end, err;
> +
> + root = ci->root;
> + seg = root->segment;
> + start = root->secondary.start;
> + end = root->secondary.end;
> +
> + cfg = pci_mmconfig_lookup(seg, start);
> + if (cfg)
> + return 0;
> +
> + cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
> + if (!cfg)
> + return -ENOMEM;
> +
> + err = pci_mmconfig_inject(cfg);
> + return err;

When you integrate Jayachandran's patch this whole function will
become a pci_mmconfig_insert() and that's where hot_added should
be set.

> +}
> +
> +void pci_mmcfg_teardown_map(struct acpi_pci_root_info *ci)
> +{
> + struct acpi_pci_root *root = ci->root;
> + struct pci_mmcfg_region *cfg;
> +
> + cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
> + if (!cfg)
> + return;
> +
> + if (cfg->hot_added)

Move the hot_added check in pci_mmconfig_delete() (that does the look
up again), we do not want to carry out pci_mmconfig_lookup only to
check that flag here (and we miss rcu locking for the look-up BTW).

Thanks,
Lorenzo

> + pci_mmconfig_delete(root->segment, root->secondary.start,
> + root->secondary.end);
> +}
> +
>  int __init __weak acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg,
>   struct acpi_mcfg_allocation *cfg)
>  {
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index a72e22d..65b91f3 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -80,6 +80,15 @@ extern struct pci_bus *acpi_pci_root_create(struct 
> acpi_pci_root *root,
>  void acpi_pci_add_bus(struct pci_bus *bus);
>  void acpi_pci_remove_bus(struct pci_bus *bus);
>  
> +#ifdef   CONFIG_PCI_MMCONFIG
> +int pci_mmcfg_setup_map(struct acpi_pci_root_info *ci);
> +void pci_mmcfg_teardown_map(struct acpi_pci_root_info *ci);
> +#else
> +static inline int pci_mmcfg_setup_map(struct acpi_pci_root_info *ci)
> +{ return 0; }
> +static inline void pci_mmcfg_teardown_map(struct acpi_pci_root_info *ci) { }
> +#endif
> +
>  #ifdef   CONFIG_ACPI_PCI_SLOT
>  void acpi_pci_slot_init(void);
>  void acpi_pci_slot_enumerate(struct pci_bus *bus);
> -- 
> 1.9.1
> 

Re: [PATCH V4 15/23] acpi, mcfg: Implement two calls that might be used to inject/remove MCFG region.

[Lorenzo Pieralisi]

On Thu, Feb 04, 2016 at 06:28:53PM +0100, Tomasz Nowicki wrote:
> Lets abstract two calls which allow to inject and remove MCFG regions
> which may come from DSDT table. These calls will be used for x86 and ARM64
> PCI host bridge driver in the later patches.
> 
> Signed-off-by: Tomasz Nowicki 
> Tested-by: Duc Dang 
> Tested-by: Dongdong Liu 
> Tested-by: Hanjun Guo 
> Tested-by: Graeme Gregory 
> Tested-by: Sinan Kaya 
> ---
>  drivers/acpi/mcfg.c  | 38 ++
>  include/linux/pci-acpi.h |  9 +
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/acpi/mcfg.c b/drivers/acpi/mcfg.c
> index 3e1e7be..dca4c4e 100644
> --- a/drivers/acpi/mcfg.c
> +++ b/drivers/acpi/mcfg.c
> @@ -10,6 +10,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define  PREFIX  "MCFG: "
>  
> @@ -77,6 +78,43 @@ int __init acpi_parse_mcfg(struct acpi_table_header 
> *header)
>   return 0;
>  }
>  
> +int pci_mmcfg_setup_map(struct acpi_pci_root_info *ci)
> +{
> + struct pci_mmcfg_region *cfg;
> + struct acpi_pci_root *root;
> + int seg, start, end, err;
> +
> + root = ci->root;
> + seg = root->segment;
> + start = root->secondary.start;
> + end = root->secondary.end;
> +
> + cfg = pci_mmconfig_lookup(seg, start);
> + if (cfg)
> + return 0;
> +
> + cfg = pci_mmconfig_alloc(seg, start, end, root->mcfg_addr);
> + if (!cfg)
> + return -ENOMEM;
> +
> + err = pci_mmconfig_inject(cfg);
> + return err;

When you integrate Jayachandran's patch this whole function will
become a pci_mmconfig_insert() and that's where hot_added should
be set.

> +}
> +
> +void pci_mmcfg_teardown_map(struct acpi_pci_root_info *ci)
> +{
> + struct acpi_pci_root *root = ci->root;
> + struct pci_mmcfg_region *cfg;
> +
> + cfg = pci_mmconfig_lookup(root->segment, root->secondary.start);
> + if (!cfg)
> + return;
> +
> + if (cfg->hot_added)

Move the hot_added check in pci_mmconfig_delete() (that does the look
up again), we do not want to carry out pci_mmconfig_lookup only to
check that flag here (and we miss rcu locking for the look-up BTW).

Thanks,
Lorenzo

> + pci_mmconfig_delete(root->segment, root->secondary.start,
> + root->secondary.end);
> +}
> +
>  int __init __weak acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg,
>   struct acpi_mcfg_allocation *cfg)
>  {
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index a72e22d..65b91f3 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -80,6 +80,15 @@ extern struct pci_bus *acpi_pci_root_create(struct 
> acpi_pci_root *root,
>  void acpi_pci_add_bus(struct pci_bus *bus);
>  void acpi_pci_remove_bus(struct pci_bus *bus);
>  
> +#ifdef   CONFIG_PCI_MMCONFIG
> +int pci_mmcfg_setup_map(struct acpi_pci_root_info *ci);
> +void pci_mmcfg_teardown_map(struct acpi_pci_root_info *ci);
> +#else
> +static inline int pci_mmcfg_setup_map(struct acpi_pci_root_info *ci)
> +{ return 0; }
> +static inline void pci_mmcfg_teardown_map(struct acpi_pci_root_info *ci) { }
> +#endif
> +
>  #ifdef   CONFIG_ACPI_PCI_SLOT
>  void acpi_pci_slot_init(void);
>  void acpi_pci_slot_enumerate(struct pci_bus *bus);
> -- 
> 1.9.1
>