| 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 Jul 2017 14:33:01 +0100
From: Mark Rutland <mark.rutland@....com>
To: Srinath Mannam <srinath.mannam@...adcom.com>
Cc: Rob Herring <robh+dt@...nel.org>, Joerg Roedel <joro@...tes.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Frank Rowand <frowand.list@...il.com>,
iommu@...ts.linux-foundation.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org,
bcm-kernel-feedback-list@...adcom.com,
Anup Patel <anup.patel@...adcom.com>,
Oza Pawandeep <oza.oza@...adcom.com>
Subject: Re: [RFC PATCH 2/2] pcie: sideband data by dropping RID bits
On Fri, Jul 07, 2017 at 12:39:59PM +0530, Srinath Mannam wrote:
> The MSI Device ID or Stream ID are passed as sideband
> data on the SOC bus to which PCI RC is connected.
>
> If sideband data on SOC bus is less than 16bits then
> PCI RC will have to derive sideband data from RID by
> dropping selected bits.
>
> This patch implements optional DT properties to generate
> smaller sideband data from RID which can be further
> mapped to MSI Device ID or Stream ID.
>
> Sideband data generation from RID is done by dropping
> bits corresponding zero bits in {iommu/msi}-map-drop-mask.
>
> Example: If drop-mask is 0xFF09 then sideband data is
> 8 bits bus number followed by 1 bit of device number and
> 1 bit function number. This means drop-mask=0xFF09 will
> convert RID=0x1a10 (16bits) to sideband data 0x6a (10bits).
As mentioned on the prior two patches, I don't think this is a good
idea, especially given this isn't a trivial masking operation.
It's already possible to express this mapping using a number of entries
in the the *-map properties, which you can generate programmatically
with a script.
Please do that instead.
Thanks,
Mark.
>
> Signed-off-by: Anup Patel <anup.patel@...adcom.com>
> Signed-off-by: Oza Pawandeep <oza.oza@...adcom.com>
> Signed-off-by: Srinath Mannam <srinath.mannam@...adcom.com>
> Reviewed-by: Ray Jui <ray.jui@...adcom.com>
> Reviewed-by: Scott Branden <scott.branden@...adcom.com>
> ---
> drivers/iommu/of_iommu.c | 4 ++--
> drivers/of/irq.c | 3 ++-
> drivers/of/of_pci.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
> include/linux/of_pci.h | 6 ++++--
> 4 files changed, 53 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 19779b8..f179724 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -169,8 +169,8 @@ static const struct iommu_ops
> */
> iommu_spec.np = NULL;
> err = of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map",
> - "iommu-map-mask", &iommu_spec.np,
> - iommu_spec.args);
> + "iommu-map-mask", "iommu-map-drop-mask",
> + &iommu_spec.np, iommu_spec.args);
> if (err)
> return err == -ENODEV ? NULL : ERR_PTR(err);
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index d11437c..454f47a 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -606,7 +606,8 @@ static u32 __of_msi_map_rid(struct device *dev, struct device_node **np,
> */
> for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent)
> if (!of_pci_map_rid(parent_dev->of_node, rid_in, "msi-map",
> - "msi-map-mask", np, &rid_out))
> + "msi-map-mask", "msi-map-drop-mask", np,
> + &rid_out))
> break;
> return rid_out;
> }
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index c9d4d3a..a914bcf 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -285,6 +285,35 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
> #endif /* CONFIG_OF_ADDRESS */
>
> +static inline u32 out_masked_rid(u32 rid, u32 drop_mask)
> +{
> + u32 id = 0;
> + u32 i = 0;
> +
> + /* RID's BUS, DEV, FUN values not inside the mask are invalid */
> + if (rid & ~drop_mask)
> + return -EINVAL;
> +
> + /*
> + * RID value is translated to sideband data using drop_mask
> + * by dropping bits corresponding zero bits in drop_mask.
> + *
> + * Example: If drop_mask is 0xFF09 then sideband data is
> + * 8 bits bus number followed by 1 bit of device number and
> + * 1 bit function number. This means drop_mask=0xFF09 will
> + * convert RID=0x1a10 (16bits) to sideband data 0x6a (10bits).
> + */
> + while (drop_mask) {
> + if (drop_mask & 0x1) {
> + id |= ((rid & 0x1) << i);
> + i++;
> + }
> + rid = rid >> 1;
> + drop_mask = drop_mask >> 1;
> + }
> +
> + return id;
> +}
> /**
> * of_pci_map_rid - Translate a requester ID through a downstream mapping.
> * @np: root complex device node.
> @@ -304,11 +333,11 @@ EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
> *
> * Return: 0 on success or a standard error code on failure.
> */
> -int of_pci_map_rid(struct device_node *np, u32 rid,
> - const char *map_name, const char *map_mask_name,
> +int of_pci_map_rid(struct device_node *np, u32 rid, const char *map_name,
> + const char *map_mask_name, const char *drop_mask_name,
> struct device_node **target, u32 *id_out)
> {
> - u32 map_mask, masked_rid;
> + u32 map_mask, masked_rid, drop_mask;
> int map_len;
> const __be32 *map = NULL;
>
> @@ -340,7 +369,20 @@ int of_pci_map_rid(struct device_node *np, u32 rid,
> if (map_mask_name)
> of_property_read_u32(np, map_mask_name, &map_mask);
>
> + /* The default is to select all bits. */
> + drop_mask = 0xffffffff;
> +
> + /*
> + * Can be overridden by "{iommu,msi}-map-drop-mask" property.
> + * If of_property_read_u32() fails, the default is used.
> + */
> + if (drop_mask_name)
> + of_property_read_u32(np, drop_mask_name, &drop_mask);
> +
> masked_rid = map_mask & rid;
> +
> + masked_rid = out_masked_rid(masked_rid, drop_mask);
> +
> for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
> struct device_node *phandle_node;
> u32 rid_base = be32_to_cpup(map + 0);
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index 518c8d2..98937ec 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -20,7 +20,8 @@ int of_pci_get_max_link_speed(struct device_node *node);
> void of_pci_check_probe_only(void);
> int of_pci_map_rid(struct device_node *np, u32 rid,
> const char *map_name, const char *map_mask_name,
> - struct device_node **target, u32 *id_out);
> + const char *drop_mask_name, struct device_node **target,
> + u32 *id_out);
> #else
> static inline int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq)
> {
> @@ -58,7 +59,8 @@ of_get_pci_domain_nr(struct device_node *node)
>
> static inline int of_pci_map_rid(struct device_node *np, u32 rid,
> const char *map_name, const char *map_mask_name,
> - struct device_node **target, u32 *id_out)
> + const char *drop_mask_name, struct device_node **target,
> + u32 *id_out)
> {
> return -EINVAL;
> }
> --
> 2.7.4
>
Powered by blists - more mailing lists