RE: [RFCv2 1/2] acpi:iort: Add new helper function to retrieve ITS base addr from dev IORT node

2017-06-06 Thread Shameerali Kolothum Thodi 


> -Original Message-
> From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com]
> Sent: Tuesday, June 06, 2017 3:10 PM
> To: Shameerali Kolothum Thodi
> Cc: marc.zyng...@arm.com; sudeep.ho...@arm.com; will.dea...@arm.com;
> robin.mur...@arm.com; hanjun@linaro.org; Gabriele Paoloni; John
> Garry; iommu@lists.linux-foundation.org; linux-arm-
> ker...@lists.infradead.org; linux-a...@vger.kernel.org; de...@acpica.org;
> Linuxarm; Wangzhou (B); Guohanjun (Hanjun Guo)
> Subject: Re: [RFCv2 1/2] acpi:iort: Add new helper function to retrieve ITS
> base addr from dev IORT node
> 
> On Wed, May 31, 2017 at 03:32:12PM +0100, shameer wrote:
> > This provides a helper function to find and retrieve the ITS base
> > address from the ID mappings array reference of a device IORT node(if
> > any).
> >
> > This is used in the subsequent patch to retrieve the ITS base address
> > associated with a pci dev IORT node.
> 
> "Add an IORT helper function to retrieve the ITS data through IORT device<-
> >ITS mappings".
> 
> > Signed-off-by: shameer <shameerali.kolothum.th...@huawei.com>
> > ---
> >  drivers/acpi/arm64/iort.c| 47
> +---
> >  drivers/irqchip/irq-gic-v3-its.c |  3 ++-
> >  include/linux/acpi_iort.h|  8 ++-
> >  3 files changed, 53 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index c5fecf9..12d7347 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -34,6 +34,7 @@
> >  struct iort_its_msi_chip {
> > struct list_headlist;
> > struct fwnode_handle*fw_node;
> > +   u64 base_addr;
> 
> phys_addr_t
> 
> > u32 translation_id;
> >  };
> >
> > @@ -132,13 +133,14 @@ typedef acpi_status (*iort_find_node_callback)
> >
> >  /**
> >   * iort_register_domain_token() - register domain token and related
> > ITS ID
> > - * to the list from where we can get it back later on.
> > + * and base address to the list from where we can get it back later on.
> >   * @trans_id: ITS ID.
> 
> Missing something here
> 
> >   * @fw_node: Domain token.
> >   *
> >   * Returns: 0 on success, -ENOMEM if no memory when allocating list
> element
> >   */
> > -int iort_register_domain_token(int trans_id, struct fwnode_handle
> > *fw_node)
> > +int iort_register_domain_token(int trans_id, u64 base,
> 
> phys_addr_t
> 
> > +  struct fwnode_handle *fw_node)
> >  {
> > struct iort_its_msi_chip *its_msi_chip;
> >
> > @@ -148,6 +150,7 @@ int iort_register_domain_token(int trans_id,
> > struct fwnode_handle *fw_node)
> >
> > its_msi_chip->fw_node = fw_node;
> > its_msi_chip->translation_id = trans_id;
> > +   its_msi_chip->base_addr = base;
> >
> > spin_lock(_msi_chip_lock);
> > list_add(_msi_chip->list, _msi_chip_list); @@ -370,7 +373,6
> > @@ static struct acpi_iort_node *iort_node_map_id(struct
> > acpi_iort_node *node,
> >
> > if (!node->mapping_offset || !node->mapping_count)
> > goto fail_map;
> > -
> 
> Unrelated change.
> 
> > map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
> >node->mapping_offset);
> >
> > @@ -491,6 +493,45 @@ int iort_pmsi_get_dev_id(struct device *dev, u32
> *dev_id)
> > return -ENODEV;
> >  }
> >
> > +int iort_dev_find_its_base(struct device *dev, u32 req_id,
> > +  unsigned int idx, u64 *its_base) {
> > +   struct acpi_iort_its_group *its;
> > +   struct acpi_iort_node *node;
> > +   struct iort_its_msi_chip *its_msi_chip;
> > +   u32  trans_id;
> > +
> > +   node = iort_find_dev_node(dev);
> > +   if (!node)
> > +   return -ENXIO;
> 
> -ENODEV, throughout

Ok. The reason for -ENXIO use is that this function is based on the existing
iort_dev_find_its_id() and it returns -ENXIO for those error cases.

I will incorporate all the other comments in the next revision.

Many thanks,
Shameer
 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFCv2 1/2] acpi:iort: Add new helper function to retrieve ITS base addr from dev IORT node

2017-06-06 Thread Lorenzo Pieralisi 
On Wed, May 31, 2017 at 03:32:12PM +0100, shameer wrote:
> This provides a helper function to find and retrieve the ITS
> base address from the ID mappings array reference of a device
> IORT node(if any).
> 
> This is used in the subsequent patch to retrieve the ITS base 
> address associated with a pci dev IORT node.

"Add an IORT helper function to retrieve the ITS data through IORT
device<->ITS mappings".

> Signed-off-by: shameer 
> ---
>  drivers/acpi/arm64/iort.c| 47 
> +---
>  drivers/irqchip/irq-gic-v3-its.c |  3 ++-
>  include/linux/acpi_iort.h|  8 ++-
>  3 files changed, 53 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index c5fecf9..12d7347 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -34,6 +34,7 @@
>  struct iort_its_msi_chip {
>   struct list_headlist;
>   struct fwnode_handle*fw_node;
> + u64 base_addr;

phys_addr_t

>   u32 translation_id;
>  };
>  
> @@ -132,13 +133,14 @@ typedef acpi_status (*iort_find_node_callback)
>  
>  /**
>   * iort_register_domain_token() - register domain token and related ITS ID
> - * to the list from where we can get it back later on.
> + * and base address to the list from where we can get it back later on.
>   * @trans_id: ITS ID.

Missing something here

>   * @fw_node: Domain token.
>   *
>   * Returns: 0 on success, -ENOMEM if no memory when allocating list element
>   */
> -int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node)
> +int iort_register_domain_token(int trans_id, u64 base,

phys_addr_t

> +struct fwnode_handle *fw_node)
>  {
>   struct iort_its_msi_chip *its_msi_chip;
>  
> @@ -148,6 +150,7 @@ int iort_register_domain_token(int trans_id, struct 
> fwnode_handle *fw_node)
>  
>   its_msi_chip->fw_node = fw_node;
>   its_msi_chip->translation_id = trans_id;
> + its_msi_chip->base_addr = base;
>  
>   spin_lock(_msi_chip_lock);
>   list_add(_msi_chip->list, _msi_chip_list);
> @@ -370,7 +373,6 @@ static struct acpi_iort_node *iort_node_map_id(struct 
> acpi_iort_node *node,
>  
>   if (!node->mapping_offset || !node->mapping_count)
>   goto fail_map;
> -

Unrelated change.

>   map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
>  node->mapping_offset);
>  
> @@ -491,6 +493,45 @@ int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
>   return -ENODEV;
>  }
>  
> +int iort_dev_find_its_base(struct device *dev, u32 req_id,
> +unsigned int idx, u64 *its_base)
> +{
> + struct acpi_iort_its_group *its;
> + struct acpi_iort_node *node;
> + struct iort_its_msi_chip *its_msi_chip;
> + u32  trans_id;
> +
> + node = iort_find_dev_node(dev);
> + if (!node)
> + return -ENXIO;

-ENODEV, throughout

> +
> + node = iort_node_map_id(node, req_id, NULL, IORT_MSI_TYPE);
> + if (!node)
> + return -ENXIO;
> +
> + /* Move to ITS specific data */
> + its = (struct acpi_iort_its_group *)node->node_data;
> + if (idx > its->its_count) {
> + dev_err(dev, "requested ITS ID index [%d] is greater than 
> available [%d]\n",
> + idx, its->its_count);
> + return -ENXIO;
> + }
> +
> + trans_id = its->identifiers[idx];
> +
> + spin_lock(_msi_chip_lock);
> + list_for_each_entry(its_msi_chip, _msi_chip_list, list) {
> + if (its_msi_chip->translation_id == trans_id) {
> + *its_base = its_msi_chip->base_addr;
> + spin_unlock(_msi_chip_lock);
> + return 0;
> + }
> + }
> + spin_unlock(_msi_chip_lock);
> +
> + return -ENXIO;

Cosmetics:

bool match = false;

[...]

spin_lock(_msi_chip_lock);
list_for_each_entry(its_msi_chip, _msi_chip_list, list) {
if (its_msi_chip->translation_id == trans_id) {
*its_base = its_msi_chip->base_addr;
match = true;
break;
}
}
spin_unlock(_msi_chip_lock);

return match ? 0 : -ENODEV;

}

>  /**
>   * iort_dev_find_its_id() - Find the ITS identifier for a device
>   * @dev: The device.
> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
> b/drivers/irqchip/irq-gic-v3-its.c
> index 45ea1933..c45a2ad 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1854,7 +1854,8 @@ static int __init gic_acpi_parse_madt_its(struct 
> acpi_subtable_header *header,
>   return -ENOMEM;
>   }
>  
> - err = iort_register_domain_token(its_entry->translation_id, dom_handle);
> + err =