From: Jean-Philippe Brucker <jean-philippe@linaro.org>
To: Auger Eric <eric.auger@redhat.com>
Cc: linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-acpi@vger.kernel.org, devicetree@vger.kernel.org,
iommu@lists.linux-foundation.org, joro@8bytes.org,
robh+dt@kernel.org, mark.rutland@arm.com,
lorenzo.pieralisi@arm.com, guohanjun@huawei.com,
sudeep.holla@arm.com, rjw@rjwysocki.net, lenb@kernel.org,
will@kernel.org, robin.murphy@arm.com, bhelgaas@google.com,
jonathan.cameron@huawei.com, zhangfei.gao@linaro.org
Subject: Re: [PATCH v3 07/13] iommu/arm-smmu-v3: Add support for Substream IDs
Date: Wed, 18 Dec 2019 17:07:59 +0100 [thread overview]
Message-ID: <20191218160759.GF2371701@myrica> (raw)
In-Reply-To: <aeed148a-3c55-6e97-8d97-efa4f8c6d9f4@redhat.com>
On Tue, Dec 17, 2019 at 05:43:59PM +0100, Auger Eric wrote:
> > -static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
> > - struct arm_smmu_s1_cfg *cfg)
> > +static int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
> > + int ssid, struct arm_smmu_ctx_desc *cd)
> > {
> > - u64 val;
> > - __le64 *cdptr = cfg->table.ptr;
> > -
> > /*
> > - * We don't need to issue any invalidation here, as we'll invalidate
> > - * the STE when installing the new entry anyway.
> > + * This function handles the following cases:
> > + *
> > + * (1) Install primary CD, for normal DMA traffic (SSID = 0).
> > + * (2) Install a secondary CD, for SID+SSID traffic.
> > + * (3) Update ASID of a CD. Atomically write the first 64 bits of the
> > + * CD, then invalidate the old entry and mappings.
> > + * (4) Remove a secondary CD.
> I see arm_smmu_write_ctx_desc getting called with non null cd in
> arm_smmu_attach_dev but I do not see it removed in the detach path?
No we don't have to remove the primary CD, since detach clears the STE and
frees the CD tables.
> > */
> > - val = arm_smmu_cpu_tcr_to_cd(cfg->cd.tcr) |
> > -#ifdef __BIG_ENDIAN
> > - CTXDESC_CD_0_ENDI |
> > -#endif
> > - CTXDESC_CD_0_R | CTXDESC_CD_0_A | CTXDESC_CD_0_ASET |
> > - CTXDESC_CD_0_AA64 | FIELD_PREP(CTXDESC_CD_0_ASID, cfg->cd.asid) |
> > - CTXDESC_CD_0_V;
> > + u64 val;
> > + bool cd_live;
> > + struct arm_smmu_device *smmu = smmu_domain->smmu;
> > + __le64 *cdptr = smmu_domain->s1_cfg.table.ptr + ssid *
> > + CTXDESC_CD_DWORDS;>
> > - /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
> > - if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
> > - val |= CTXDESC_CD_0_S;
> > + val = le64_to_cpu(cdptr[0]);
> > + cd_live = !!(val & CTXDESC_CD_0_V);
> >
> > - cdptr[0] = cpu_to_le64(val);
> > + if (!cd) { /* (4) */
> > + val = 0;
> > + } else if (cd_live) { /* (3) */
> > + val &= ~CTXDESC_CD_0_ASID;
> > + val |= FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid);
> > + /*
> > + * Until CD+TLB invalidation, both ASIDs may be used for tagging
> > + * this substream's traffic
> > + */
> > + } else { /* (1) and (2) */
> > + cdptr[1] = cpu_to_le64(cd->ttbr & CTXDESC_CD_1_TTB0_MASK);
> > + cdptr[2] = 0;
> > + cdptr[3] = cpu_to_le64(cd->mair);
> > +
> > + /*
> > + * STE is live, and the SMMU might fetch this CD at any
> > + * time. Ensure that it observes the rest of the CD before we
> > + * enable it.
> Mostly, on't you want the invalid state to be seen?
Sorry I didn't get your question. Without the sync, the SMMU could read an
invalid address in CD[1], read V=1 in CD[0] and proceed with dereferencing
the bogus pointer before seeing our update to CD[1].
To prevent this we follow the update procedure described in IHI0070Ca
3.21.3.1 (Configuration structure update procedure):
Because the SMMU can read any reachable structure at any time, and is not
required to read the double-words of the structure in order, Arm
recommends that the following procedure is used to initialize structures:
1. Structure starts invalid, having V == 0.
2. Fill in all fields, leaving V == 0, then perform a DSB operation to
ensure written data is observable from the SMMU.
3. Issue a CMD_CFGI_<STRUCT>, as appropriate.
4. Issue a CMD_SYNC, and wait for completion.
5. Set V to 1, then perform a DSB operation to ensure write is
observable by the SMMU.
6. Issue CMD_CFGI_<STRUCT>, as appropriate.
7. Optionally issue a CMD_SYNC, and wait for completion. This must be
done if a subsequent software operation, such as enabling device
DMA, depends on the SMMU using the new structure.
> > @@ -1664,6 +1739,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> > if (s1_cfg) {
> > BUG_ON(ste_live);
> > dst[1] = cpu_to_le64(
> > + FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) |
> > FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
> > FIELD_PREP(STRTAB_STE_1_S1COR, STRTAB_STE_1_S1C_CACHE_WBRA) |
> > FIELD_PREP(STRTAB_STE_1_S1CSH, ARM_SMMU_SH_ISH) |
> > @@ -1674,7 +1750,9 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> > dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
> >
> > val |= (s1_cfg->table.ptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
> > - FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS);
> > + FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS) |
> > + FIELD_PREP(STRTAB_STE_0_S1CDMAX, s1_cfg->s1cdmax) |
> > + FIELD_PREP(STRTAB_STE_0_S1FMT, s1_cfg->s1fmt);
> Do I understand correctly that each time we update the CD, we also
> update the STE. This is not wrong but theoretically this is not needed,
> right?
Only the primary CD. Once we add SVA (or auxiliary domains) we need to
update secondary CDs while the STE is live. This series is just
preparation for SVA support. I didn't include the full support because it
requires another 25+ patches.
> This also explains why you do not need the CMD_PREFETCH_CONFIG with ssid
> because you invalidate the config per sid?
Hm, I didn't add a CMD_PREFETCH_CONFIG to CDs because I don't know if it's
useful. Maybe as future optimization.
Thanks,
Jean
next prev parent reply other threads:[~2019-12-18 16:08 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-09 18:05 [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jean-Philippe Brucker
2019-12-09 18:05 ` [PATCH v3 01/13] iommu/arm-smmu-v3: Drop __GFP_ZERO flag from DMA allocation Jean-Philippe Brucker
2019-12-13 16:45 ` Jonathan Cameron
2019-12-17 10:59 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 02/13] dt-bindings: document PASID property for IOMMU masters Jean-Philippe Brucker
2019-12-09 18:05 ` [PATCH v3 03/13] iommu/arm-smmu-v3: Support platform SSID Jean-Philippe Brucker
2019-12-17 11:05 ` Auger Eric
2019-12-17 15:21 ` Jean-Philippe Brucker
2019-12-17 16:46 ` Auger Eric
2019-12-18 10:17 ` Auger Eric
2019-12-18 16:06 ` Jean-Philippe Brucker
2019-12-09 18:05 ` [PATCH v3 04/13] ACPI/IORT: Support PASID for platform devices Jean-Philippe Brucker
2019-12-17 13:27 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 05/13] iommu/arm-smmu-v3: Prepare arm_smmu_s1_cfg for SSID support Jean-Philippe Brucker
2019-12-17 13:27 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 06/13] iommu/arm-smmu-v3: Add context descriptor tables allocators Jean-Philippe Brucker
2019-12-17 13:36 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 07/13] iommu/arm-smmu-v3: Add support for Substream IDs Jean-Philippe Brucker
2019-12-17 16:43 ` Auger Eric
2019-12-18 16:07 ` Jean-Philippe Brucker [this message]
2019-12-18 16:59 ` Auger Eric
2019-12-18 17:03 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 08/13] iommu/arm-smmu-v3: Propate ssid_bits Jean-Philippe Brucker
2019-12-17 17:07 ` Auger Eric
2019-12-18 16:08 ` Jean-Philippe Brucker
2019-12-18 17:00 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 09/13] iommu/arm-smmu-v3: Handle failure of arm_smmu_write_ctx_desc() Jean-Philippe Brucker
2019-12-17 17:24 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 10/13] iommu/arm-smmu-v3: Add second level of context descriptor table Jean-Philippe Brucker
2019-12-13 17:07 ` Jonathan Cameron
2019-12-18 9:59 ` Auger Eric
2019-12-18 16:10 ` Jean-Philippe Brucker
2019-12-09 18:05 ` [PATCH v3 11/13] iommu/arm-smmu-v3: Improve add_device() error handling Jean-Philippe Brucker
2019-12-09 18:05 ` [PATCH v3 12/13] PCI/ATS: Add PASID stubs Jean-Philippe Brucker
2019-12-10 21:07 ` Bjorn Helgaas
2019-12-18 10:02 ` Auger Eric
2019-12-09 18:05 ` [PATCH v3 13/13] iommu/arm-smmu-v3: Add support for PCI PASID Jean-Philippe Brucker
2019-12-18 10:17 ` Auger Eric
2019-12-18 16:13 ` Jean-Philippe Brucker
2019-12-13 17:10 ` [PATCH v3 00/13] iommu: Add PASID support to Arm SMMUv3 Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191218160759.GF2371701@myrica \
--to=jean-philippe@linaro.org \
--cc=bhelgaas@google.com \
--cc=devicetree@vger.kernel.org \
--cc=eric.auger@redhat.com \
--cc=guohanjun@huawei.com \
--cc=iommu@lists.linux-foundation.org \
--cc=jonathan.cameron@huawei.com \
--cc=joro@8bytes.org \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=mark.rutland@arm.com \
--cc=rjw@rjwysocki.net \
--cc=robh+dt@kernel.org \
--cc=robin.murphy@arm.com \
--cc=sudeep.holla@arm.com \
--cc=will@kernel.org \
--cc=zhangfei.gao@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).