On Sat, Jul 7, 2018 at 12:05 PM, Paul Durrant <[email protected]> wrote:
> @@ -787,7 +793,9 @@ int amd_iommu_reserve_domain_unity_map(struct domain
> *domain,
> gfn = phys_addr >> PAGE_SHIFT;
> for ( i = 0; i < npages; i++ )
> {
> - rt = amd_iommu_map_page(domain, gfn +i, gfn +i, flags);
> + unsigned long frame = gfn + i;
> +
> + rt = amd_iommu_map_page(domain, _bfn(frame), _mfn(frame), flags);
Here's an example where the intermediate variable may actually improve
things, if only by avoiding code duplication. :-)
> @@ -2766,14 +2766,14 @@ static int __must_check arm_smmu_map_page(struct
> domain *d, unsigned long bfn,
> * The function guest_physmap_add_entry replaces the current mapping
> * if there is already one...
> */
> - return guest_physmap_add_entry(d, _gfn(gfn), _mfn(mfn), 0, t);
> + return guest_physmap_add_entry(d, gfn, mfn, 0, t);
> }
>
> -static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned long
> bfn)
> +static int __must_check arm_smmu_unmap_page(struct domain *d, bfn_t bfn)
> {
> - unsigned long frame = bfn;
> - unsigned long gfn = frame;
> - unsigned long mfn = frame;
> + unsigned long frame = bfn_x(bfn);
> + gfn_t gfn = _gfn(frame);
> + mfn_t mfn = _mfn(frame);
Even if we decide to use an intermediate variable in many other cases,
this construct in particular seems excessive.
Everything else looks good, thanks.
-George
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel