> From: Jason Gunthorpe <[email protected]>
> Sent: Thursday, September 4, 2025 1:47 AM
> 
> +static __maybe_unused int __unmap_range(struct pt_range *range, void
> *arg,
> +                                     unsigned int level,
> +                                     struct pt_table_p *table)
> +{
> +     struct pt_state pts = pt_init(range, level, table);
> +     struct pt_unmap_args *unmap = arg;
> +     unsigned int num_oas = 0;
> +     unsigned int start_index;
> +     int ret = 0;
> +
> +     _pt_iter_first(&pts);
> +     start_index = pts.index;
> +     pts.type = pt_load_entry_raw(&pts);
> +     /*
> +      * A starting index is in the middle of a contiguous entry
> +      *
> +      * The IOMMU API does not require drivers to support unmapping
> parts of
> +      * large pages. Long ago VFIO would try to split maps but the current
> +      * version never does.
> +      *
> +      * Instead when unmap reaches a partial unmap of the start of a
> large
> +      * IOPTE it should remove the entire IOPTE and return that size to the
> +      * caller.
> +      */
> +     if (pts.type == PT_ENTRY_OA) {
> +             if (log2_mod(range->va, pt_entry_oa_lg2sz(&pts)))
> +                     return -EINVAL;
> +             goto start_oa;
> +     }

it's not typical goto a location inside a loop.

Actually even w/o that goto, the flow will continue to...

> +
> +     do {
> +             if (pts.type != PT_ENTRY_OA) {
> +                     bool fully_covered;
> +
> +                     if (pts.type != PT_ENTRY_TABLE) {
> +                             ret = -EINVAL;
> +                             break;
> +                     }
> +
> +                     if (pts.index != start_index)
> +                             pt_index_to_va(&pts);
> +                     pts.table_lower = pt_table_ptr(&pts);
> +
> +                     fully_covered = pt_item_fully_covered(
> +                             &pts, pt_table_item_lg2sz(&pts));
> +
> +                     ret = pt_descend(&pts, arg, __unmap_range);
> +                     if (ret)
> +                             break;
> +
> +                     /*
> +                      * If the unmapping range fully covers the table then
> we
> +                      * can free it as well. The clear is delayed until we
> +                      * succeed in clearing the lower table levels.
> +                      */
> +                     if (fully_covered) {
> +                             iommu_pages_list_add(&unmap->free_list,
> +                                                  pts.table_lower);
> +                             pt_clear_entry(&pts, ilog2(1));
> +                     }
> +                     pts.index++;
> +             } else {
> +                     unsigned int num_contig_lg2;

...here naturally.

> +start_oa:
> +                     /*
> +                      * If the caller requested an last that falls within a
> +                      * single entry then the entire entry is unmapped and
> +                      * the length returned will be larger than requested.
> +                      */
> +                     num_contig_lg2 = pt_entry_num_contig_lg2(&pts);
> +                     pt_clear_entry(&pts, num_contig_lg2);
> +                     num_oas += log2_to_int(num_contig_lg2);
> +                     pts.index += log2_to_int(num_contig_lg2);
> +             }
> +             if (pts.index >= pts.end_index)
> +                     break;
> +             pts.type = pt_load_entry_raw(&pts);
> +     } while (true);
> +
> +     unmap->unmapped += log2_mul(num_oas,
> pt_table_item_lg2sz(&pts));
> +     return ret;
> +}
> +

the rest looks good:

Reviewed-by: Kevin Tian <[email protected]>

Reply via email to