Define and use standard functions for alignment checking

[dougm]

Summary

Define simple functions for alignment and boundary checks and use them everywhere instead of having slightly different implementations scattered about. Define them in vm_page.h and use them where possible where vm_page.h is included.

[kib]

You may also convert sys/dev/iommu/iommu.h iommu_test_boundary()

sys/vm/vm_phys.h

90 ↗

(On Diff #100677)

We usually put a blank lines around functions definitions.

93 ↗

(On Diff #100677)

Isn't this line missed ';' ?

[dougm]

In D33685#761212, @kib wrote:

You may also convert sys/dev/iommu/iommu.h iommu_test_boundary()

What header file would work for that?

sys/vm/vm_phys.h

93 ↗

(On Diff #100677)

Yes.

[kib]

In D33685#761224, @dougm wrote:

In D33685#761212, @kib wrote:

You may also convert sys/dev/iommu/iommu.h iommu_test_boundary()

What header file would work for that?

I think that vm/vm_param.h would work for this, similarly how roundup() or powerof2() live in sys/param.h. Not sure about vm_phys_ prefix, might be vm_addr_ is a better choice, also from the functional PoV.

I suggest to commit vm_phys_set_pool() separately.

[dougm]

Drop set_pool. Move alignment checks to vm_page.h. Use them in sys/dev/iommu.

[dougm]

Find more places to use align_ok and bound_ok.

[dougm]

common.boundary -> boundary, on some architectures.

[dougm]

Drop a redundant zero-check.

[kib]

FYI D33684

sys/vm/vm_page.h

1029 ↗

(On Diff #100692)

I think some comment would be very useful there, at least about use of '-' instead of '~' (so that zero boundary is handled automatically, AFAIU).

[markj]

IMO vm_phys.h makes more sense than vm_page.h for these routines. The vm_page_ prefix is usually for functions that operate on a vm_page_t. Is there some reason these can't live in vm_param.h as kib suggested?

[dougm]

In D33685#761339, @markj wrote:

IMO vm_phys.h makes more sense than vm_page.h for these routines. The vm_page_ prefix is usually for functions that operate on a vm_page_t. Is there some reason these can't live in vm_param.h as kib suggested?

Using vm_phys would require that I include vm_phys.h in more files. vm file already included everywhere are:
#include <vm/vm.h>
#include <vm/vm_extern.h>
#include <vm/vm_kern.h>
#include <vm/vm_page.h>
#include <vm/vm_map.h>

If vm_param.h is indirectly included everywhere, I can use that.

[markj]

In D33685#761356, @dougm wrote:

In D33685#761339, @markj wrote:

IMO vm_phys.h makes more sense than vm_page.h for these routines. The vm_page_ prefix is usually for functions that operate on a vm_page_t. Is there some reason these can't live in vm_param.h as kib suggested?

Using vm_phys would require that I include vm_phys.h in more files. vm file already included everywhere are:
#include <vm/vm.h>
#include <vm/vm_extern.h>
#include <vm/vm_kern.h>
#include <vm/vm_page.h>
#include <vm/vm_map.h>

If vm_param.h is indirectly included everywhere, I can use that.

It's only included directly. All of the C files in sys/vm already have it, and I think it wouldn't be to onerous to add includes elsewhere where needed.

That said, vm_extern.h makes even more sense since these routines are used outside of sys/vm, and vm_extern.h already assumes that vm_paddr_t is defined. So I would suggest trying to add these routines there.

[dougm]

Move the functions from vm_page.h to vm_extern.h. Rename them, and add a couple of comments.

[kib]

sys/vm/vm_extern.h

138

I would add asserts that power of two args are indeed power of two. This probably needs to come under #ifdef _KERNEL. You may do this as a follow-up.