| 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
| ||
|
Date: Fri, 7 Sep 2018 16:36:13 +0100
From: Robin Murphy <robin.murphy@....com>
To: Shameer Kolothum <shameerali.kolothum.thodi@...wei.com>,
lorenzo.pieralisi@....com
Cc: will.deacon@....com, mark.rutland@....com, guohanjun@...wei.com,
john.garry@...wei.com, pabba@...eaurora.org,
vkilari@...eaurora.org, rruigrok@...eaurora.org,
linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linuxarm@...wei.com,
neil.m.leeder@...il.com
Subject: Re: [PATCH v2 1/4] acpi: arm64: add iort support for PMCG
On 24/07/18 12:45, Shameer Kolothum wrote:
> From: Neil Leeder <nleeder@...eaurora.org>
>
> Add support for the SMMU Performance Monitor Counter Group
> information from ACPI. This is in preparation for its use
> in the SMMU v3 PMU driver.
>
> Signed-off-by: Neil Leeder <nleeder@...eaurora.org>
> Signed-off-by: Hanjun Guo <guohanjun@...wei.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@...wei.com>
> ---
> drivers/acpi/arm64/iort.c | 95 +++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 83 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 7a3a541..ac4d0d6 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -356,7 +356,8 @@ static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
> if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
> if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT ||
> node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX ||
> - node->type == ACPI_IORT_NODE_SMMU_V3) {
> + node->type == ACPI_IORT_NODE_SMMU_V3 ||
> + node->type == ACPI_IORT_NODE_PMCG) {
> *id_out = map->output_base;
> return parent;
> }
> @@ -394,6 +395,8 @@ static int iort_get_id_mapping_index(struct acpi_iort_node *node)
> }
>
> return smmu->id_mapping_index;
> + case ACPI_IORT_NODE_PMCG:
> + return 0;
Why do we need a PMCG case here? AIUI this whole get_id_mapping_index
business is only relevant to SMMUv3 nodes where we have some need to
disambiguate the difference between the SMMU's own IDs and
StreamID-to-DeviceID mappings within the same table. PMCGs simply have
zero or one single ID mappings so should be equivalent to most named
components (other than their mappings pointing straight to the ITS).
> default:
> return -EINVAL;
> }
> @@ -1287,6 +1290,63 @@ static bool __init arm_smmu_is_coherent(struct acpi_iort_node *node)
> return smmu->flags & ACPI_IORT_SMMU_COHERENT_WALK;
> }
>
> +static void __init arm_smmu_common_dma_configure(struct device *dev,
> + enum dev_dma_attr attr)
> +{
> + /* We expect the dma masks to be equivalent for all SMMUs set-ups */
> + dev->dma_mask = &dev->coherent_dma_mask;
> +
> + /* Configure DMA for the page table walker */
> + acpi_dma_configure(dev, attr);
Hmm, I don't think we actually need this call any more, since it should
now happen later anyway via platform_dma_configure() as the relevant
SMMU/PMCG driver binds.
> +}
> +
> +static int __init arm_smmu_v3_pmu_count_resources(struct acpi_iort_node *node)
Can we be consistent with "pmcg" rather than "pmu" within IORT please?
> +{
> + struct acpi_iort_pmcg *pmcg;
> +
> + /* Retrieve PMCG specific data */
> + pmcg = (struct acpi_iort_pmcg *)node->node_data;
> +
> + /*
> + * There are always 2 memory resources.
> + * If the overflow_gsiv is present then add that for a total of 3.
> + */
> + return pmcg->overflow_gsiv > 0 ? 3 : 2;
> +}
> +
> +static void __init arm_smmu_v3_pmu_init_resources(struct resource *res,
> + struct acpi_iort_node *node)
> +{
> + struct acpi_iort_pmcg *pmcg;
> +
> + /* Retrieve PMCG specific data */
> + pmcg = (struct acpi_iort_pmcg *)node->node_data;
> +
> + res[0].start = pmcg->page0_base_address;
> + res[0].end = pmcg->page0_base_address + SZ_4K - 1;
> + res[0].flags = IORESOURCE_MEM;
> + res[1].start = pmcg->page1_base_address;
> + res[1].end = pmcg->page1_base_address + SZ_4K - 1;
> + res[1].flags = IORESOURCE_MEM;
> +
> + if (pmcg->overflow_gsiv)
> + acpi_iort_register_irq(pmcg->overflow_gsiv, "overflow",
> + ACPI_EDGE_SENSITIVE, &res[2]);
> +}
> +
> +static struct acpi_iort_node *iort_find_pmcg_ref(struct acpi_iort_node *node)
> +{
> + struct acpi_iort_pmcg *pmcg;
> + struct acpi_iort_node *ref_node = NULL;
> +
> + /* Retrieve PMCG specific data */
> + pmcg = (struct acpi_iort_pmcg *)node->node_data;
> + if (pmcg->node_reference)
> + ref_node = ACPI_ADD_PTR(struct acpi_iort_node,
> + iort_table, pmcg->node_reference);
> + return ref_node;
> +}
> +
> struct iort_dev_config {
> const char *name;
> int (*dev_init)(struct acpi_iort_node *node);
> @@ -1296,6 +1356,8 @@ struct iort_dev_config {
> struct acpi_iort_node *node);
> void (*dev_set_proximity)(struct device *dev,
> struct acpi_iort_node *node);
> + void (*dev_dma_configure)(struct device *dev,
> + enum dev_dma_attr attr);
> };
>
> static const struct iort_dev_config iort_arm_smmu_v3_cfg __initconst = {
> @@ -1304,23 +1366,38 @@ static const struct iort_dev_config iort_arm_smmu_v3_cfg __initconst = {
> .dev_count_resources = arm_smmu_v3_count_resources,
> .dev_init_resources = arm_smmu_v3_init_resources,
> .dev_set_proximity = arm_smmu_v3_set_proximity,
> + .dev_dma_configure = arm_smmu_common_dma_configure
> };
>
> static const struct iort_dev_config iort_arm_smmu_cfg __initconst = {
> .name = "arm-smmu",
> .dev_is_coherent = arm_smmu_is_coherent,
> .dev_count_resources = arm_smmu_count_resources,
> - .dev_init_resources = arm_smmu_init_resources
> + .dev_init_resources = arm_smmu_init_resources,
> + .dev_dma_configure = arm_smmu_common_dma_configure
> +};
> +
> +static const struct iort_dev_config iort_arm_smmu_v3_pmcg_cfg __initconst = {
> + .name = "arm-smmu-v3-pmu",
> + .dev_count_resources = arm_smmu_v3_pmu_count_resources,
> + .dev_init_resources = arm_smmu_v3_pmu_init_resources
> };
>
> static __init const struct iort_dev_config *iort_get_dev_cfg(
> struct acpi_iort_node *node)
> {
> + struct acpi_iort_node *ref_node;
> +
> switch (node->type) {
> case ACPI_IORT_NODE_SMMU_V3:
> return &iort_arm_smmu_v3_cfg;
> case ACPI_IORT_NODE_SMMU:
> return &iort_arm_smmu_cfg;
> + case ACPI_IORT_NODE_PMCG:
> + /* Check this is associated with SMMUv3 */
> + ref_node = iort_find_pmcg_ref(node);
This seems overly restrictive - admittedly the only non-SMMU DTI
components I know of don't actually implement a PMCG for their internal
TBU, but I'm sure something will eventually, and there doesn't seem to
be any good reason for Linux to forcibly ignore it when it does.
> + if (ref_node && ref_node->type == ACPI_IORT_NODE_SMMU_V3)
> + return &iort_arm_smmu_v3_pmcg_cfg;
> default:
> return NULL;
> }
> @@ -1376,12 +1453,6 @@ static int __init iort_add_platform_device(struct acpi_iort_node *node,
> if (ret)
> goto dev_put;
>
> - /*
> - * We expect the dma masks to be equivalent for
> - * all SMMUs set-ups
> - */
> - pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> -
> fwnode = iort_get_fwnode(node);
>
> if (!fwnode) {
> @@ -1391,11 +1462,11 @@ static int __init iort_add_platform_device(struct acpi_iort_node *node,
>
> pdev->dev.fwnode = fwnode;
>
> - attr = ops->dev_is_coherent && ops->dev_is_coherent(node) ?
> + if (ops->dev_dma_configure) {
> + attr = ops->dev_is_coherent && ops->dev_is_coherent(node) ?
> DEV_DMA_COHERENT : DEV_DMA_NON_COHERENT;
Oh, right, these guys don't have an ACPI companion or a standard DMA
attr because they're from a static table, so platform_dma_configure()
won't pick them up. Oh well. We probably don't want to start dragging
IORT internals into the platform bus code, so I guess it's not worth
trying to change that.
Robin.
> -
> - /* Configure DMA for the page table walker */
> - acpi_dma_configure(&pdev->dev, attr);
> + ops->dev_dma_configure(&pdev->dev, attr);
> + }
>
> iort_set_device_domain(&pdev->dev, node);
>
>
Powered by blists - more mailing lists