[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [RFC v3 4/4] xen/iommu: smmu-v3: Add Xen specific code to enable the ported driver
Hi Sameer,
On 12/05/2017 11:26 PM, Goel, Sameer wrote:
On 12/5/2017 7:17 AM, Julien Grall wrote:
On 05/12/17 03:59, Sameer Goel wrote:
+ * tables are shared
+ */
+
+ cfg->vmid = vmid;
+ cfg->vttbr = page_to_maddr(cfg->domain->arch.p2m.root);
+ cfg->vtcr = READ_SYSREG32(VTCR_EL2) & STRTAB_STE_2_VTCR_MASK;
I still think this is really fragile. You at least need a comment on the other
side (e.g where VTCR_EL2 is written) to explain you are relying the value in
other places.
I can add the comment.
Yes please in both side.
+ */
+ q->base = _xzalloc(qsz, sizeof(void *));
+
if (!q->base) {
dev_err(smmu->dev, "failed to allocate queue (0x%zx bytes)\n",
qsz);
return -ENOMEM;
}
+ q->base_dma = virt_to_maddr(q->base);
+
q->prod_reg = arm_smmu_page1_fixup(prod_off, smmu);
q->cons_reg = arm_smmu_page1_fixup(cons_off, smmu);
q->ent_dwords = dwords;
@@ -2056,6 +2205,7 @@ static int arm_smmu_init_strtab_2lvl(struct
arm_smmu_device *smmu)
u64 reg;
u32 size, l1size;
struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
+ u32 alignment;
/* Calculate the L1 size, capped to the SIDSIZE. */
size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
@@ -2069,14 +2219,17 @@ static int arm_smmu_init_strtab_2lvl(struct
arm_smmu_device *smmu)
size, smmu->sid_bits);
l1size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
- strtab = dmam_alloc_coherent(smmu->dev, l1size, &cfg->strtab_dma,
- GFP_KERNEL | __GFP_ZERO);
+ alignment = max_t(u32, cfg->num_l1_ents, 64);
Same as before. I know I didn't go through the rest of the code. But you could
have at least applied my comments on alignment here too. E.g where does the 64
come from?
But, it looks like to me you want to create a function dmam_alloc_coherent that
will do the allocation for you. This could be used in a few places within file
driver...
dmam_alloc_coherent uses the allocation size as the alignment. This is not as
per spec. But that being said I am fine replicating the code from Linux. That
will make my life easier :).
I am a bit confused. Does it mean Linux driver violate the spec? If so,
that should be fixed in both and not only Xen.
+ strtab = _xzalloc(l1size, l1size);
+
if (!strtab) {
dev_err(smmu->dev,
"failed to allocate l1 stream table (%u bytes)\n",
size);
return -ENOMEM;
}
+
+ cfg->strtab_dma = virt_to_maddr(strtab);
cfg->strtab = strtab;
/* Configure strtab_base_cfg for 2 levels */
@@ -2098,14 +2251,16 @@ static int arm_smmu_init_strtab_linear(struct
arm_smmu_device *smmu)
struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
size = (1 << smmu->sid_bits) * (STRTAB_STE_DWORDS << 3);
- strtab = dmam_alloc_coherent(smmu->dev, size, &cfg->strtab_dma,
- GFP_KERNEL | __GFP_ZERO);
... such as here.
+ strtab = _xzalloc(size, size);
Hmmm, _xzalloc contains the following assert:
ASSERT((align & (align - 1)) == 0);
How are you sure the size will always honor this check?
I can add another check or add a comment. Till now the size has passed this
check.
I was not able to convince myself that:
size = (1 << smmu->sid_bits) * (STRTAB_STE_DWORDS << 3)
will always honor the check. I would be ok with a comment explain why it
should always work.
Cheers
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
