Hi Hiroshi,
> > @@ -241,6 +238,11 @@ int load_iotlb_entry(struct iommu *obj, struct
> iotlb_entry *e)
> > break;
> > }
> >
> > + if (l.base == obj->nr_tlb_entries) {
> > + dev_dbg(obj->dev, "%s: preserve entries full\n", __func__);
> > + err = -EBUSY;
>
> I am afraid that this doesn't work. There are two cases where TLB
> entries are set/updated:
>
> 1) H/W TWL always updates TLB entries automatically, IOW, increments
> "vict" implicitly.
> 2) A client module adds preservation TLB entry with explictly calling
> "load_iotlb_entry()". IOW, increments both "vict" and "base"
> intentionally.
>
> Because of 1), "vict" gets full soon and this is normal, but even if
> this "vict" is full, a preservation entry "base" should be able to be
> set by a client module unless "base" itself rearches full. IOW, Even
> if all TLB entries are valid, a new preservation TLB entry should be
> set by replacing one of the existing normal entry.
>
> In the case of "e" with perservation bit , "load_iotlb_entry()"
> doesn't usually fail because it replaces the exisiting normal entry,
> but it would fail only if all TLB entries are preserved.
>
-- I agree with your comments. My implementation was under the assumption that the User locks the TLB entries only at startup time, but as you mentioned this would not work during run time as the entries would already be having valid entries. DSPBridge and the OMAP4 IPCs both locks the entries at init time.
The following changes in load_tlb_entry function should handle the run time case too. This change would lock an entry irrespective of existing entry is valid or not.
Please let me know if you agree.
@@ -230,22 +227,32 @@ int load_iotlb_entry(struct iommu *obj, struct iotlb_entry *e)
clk_enable(obj->clk);
- for (i = 0; i < obj->nr_tlb_entries; i++) {
- struct cr_regs tmp;
-
- iotlb_lock_get(obj, &l);
- l.vict = i;
- iotlb_lock_set(obj, &l);
- iotlb_read_cr(obj, &tmp);
- if (!iotlb_cr_valid(&tmp))
- break;
- }
-
- if (i == obj->nr_tlb_entries) {
- dev_dbg(obj->dev, "%s: full: no entry\n", __func__);
+ iotlb_lock_get(obj, &l);
+ if (l.base == obj->nr_tlb_entries) {
+ dev_dbg(obj->dev, "%s: preserve entries full\n", __func__);
err = -EBUSY;
goto out;
}
+ if (!e->prsvd) {
+ for (i = l.base; i < obj->nr_tlb_entries; i++) {
+ struct cr_regs tmp;
+
+ iotlb_lock_get(obj, &l);
+ l.vict = i;
+ iotlb_lock_set(obj, &l);
+ iotlb_read_cr(obj, &tmp);
+ if (!iotlb_cr_valid(&tmp))
+ break;
+ }
+ if (i == obj->nr_tlb_entries) {
+ dev_dbg(obj->dev, "%s: full: no entry\n", __func__);
+ err = -EBUSY;
+ goto out;
+ }
+ } else {
+ l.vict = l.base;
+ iotlb_lock_set(obj, &l);
+ }
cr = iotlb_alloc_cr(obj, e);
if (IS_ERR(cr)) {
@@ -256,9 +263,11 @@ int load_iotlb_entry(struct iommu *obj, struct iotlb_entry *e)
iotlb_load_cr(obj, cr);
kfree(cr);
+ if (e->prsvd)
+ l.base++;
/* increment victim for next tlb load */
if (++l.vict == obj->nr_tlb_entries)
- l.vict = 0;
+ l.vict = l.base;
iotlb_lock_set(obj, &l);
out:
clk_disable(obj->clk);
--
Thank you,
Best regards,
Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
