lkml.org 
[lkml]   [2021]   [Feb]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v13 05/15] iommu/smmuv3: Get prepared for nested stage support
    From
    Date
    Hi Eric,

    On 2020/11/18 19:21, Eric Auger wrote:
    > When nested stage translation is setup, both s1_cfg and
    > s2_cfg are set.
    >
    > We introduce a new smmu domain abort field that will be set
    > upon guest stage1 configuration passing.
    >
    > arm_smmu_write_strtab_ent() is modified to write both stage
    > fields in the STE and deal with the abort field.
    >
    > In nested mode, only stage 2 is "finalized" as the host does
    > not own/configure the stage 1 context descriptor; guest does.
    >
    > Signed-off-by: Eric Auger <eric.auger@redhat.com>
    >
    > ---
    > v10 -> v11:
    > - Fix an issue reported by Shameer when switching from with vSMMU
    > to without vSMMU. Despite the spec does not seem to mention it
    > seems to be needed to reset the 2 high 64b when switching from
    > S1+S2 cfg to S1 only. Especially dst[3] needs to be reset (S2TTB).
    > On some implementations, if the S2TTB is not reset, this causes
    > a C_BAD_STE error
    > ---
    > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 64 +++++++++++++++++----
    > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 +
    > 2 files changed, 56 insertions(+), 10 deletions(-)
    >
    > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
    > index 18ac5af1b284..412ea1bafa50 100644
    > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
    > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
    > @@ -1181,8 +1181,10 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
    > * three cases at the moment:
    > *
    > * 1. Invalid (all zero) -> bypass/fault (init)
    > - * 2. Bypass/fault -> translation/bypass (attach)
    > - * 3. Translation/bypass -> bypass/fault (detach)
    > + * 2. Bypass/fault -> single stage translation/bypass (attach)
    > + * 3. Single or nested stage Translation/bypass -> bypass/fault (detach)
    > + * 4. S2 -> S1 + S2 (attach_pasid_table)
    > + * 5. S1 + S2 -> S2 (detach_pasid_table)

    The following line "BUG_ON(ste_live && !nested);" forbids this transform.
    And I have a look at the 6th patch, the transform seems S1 + S2 -> abort.
    So after detach, the status is not the same as that before attach. Does it
    match our expectation?

    > *
    > * Given that we can't update the STE atomically and the SMMU
    > * doesn't read the thing in a defined order, that leaves us
    > @@ -1193,7 +1195,8 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
    > * 3. Update Config, sync
    > */
    > u64 val = le64_to_cpu(dst[0]);
    > - bool ste_live = false;
    > + bool s1_live = false, s2_live = false, ste_live;
    > + bool abort, nested = false, translate = false;
    > struct arm_smmu_device *smmu = NULL;
    > struct arm_smmu_s1_cfg *s1_cfg;
    > struct arm_smmu_s2_cfg *s2_cfg;
    > @@ -1233,6 +1236,8 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
    > default:
    > break;
    > }
    > + nested = s1_cfg->set && s2_cfg->set;
    > + translate = s1_cfg->set || s2_cfg->set;
    > }
    >
    > if (val & STRTAB_STE_0_V) {
    > @@ -1240,23 +1245,36 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
    > case STRTAB_STE_0_CFG_BYPASS:
    > break;
    > case STRTAB_STE_0_CFG_S1_TRANS:
    > + s1_live = true;
    > + break;
    > case STRTAB_STE_0_CFG_S2_TRANS:
    > - ste_live = true;
    > + s2_live = true;
    > + break;
    > + case STRTAB_STE_0_CFG_NESTED:
    > + s1_live = true;
    > + s2_live = true;
    > break;
    > case STRTAB_STE_0_CFG_ABORT:
    > - BUG_ON(!disable_bypass);
    > break;
    > default:
    > BUG(); /* STE corruption */
    > }
    > }
    >
    > + ste_live = s1_live || s2_live;
    > +
    > /* Nuke the existing STE_0 value, as we're going to rewrite it */
    > val = STRTAB_STE_0_V;
    >
    > /* Bypass/fault */
    > - if (!smmu_domain || !(s1_cfg->set || s2_cfg->set)) {
    > - if (!smmu_domain && disable_bypass)
    > +
    > + if (!smmu_domain)
    > + abort = disable_bypass;
    > + else
    > + abort = smmu_domain->abort;
    > +
    > + if (abort || !translate) {
    > + if (abort)
    > val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT);
    > else
    > val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS);
    > @@ -1274,8 +1292,16 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
    > return;
    > }
    >
    > + BUG_ON(ste_live && !nested);
    > +
    > + if (ste_live) {
    > + /* First invalidate the live STE */
    > + dst[0] = cpu_to_le64(STRTAB_STE_0_CFG_ABORT);
    > + arm_smmu_sync_ste_for_sid(smmu, sid);
    > + }
    > +
    [...]

    Thanks,
    Keqian

    \
     
     \ /
      Last update: 2021-02-02 08:17    [W:4.060 / U:0.448 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site