Do you really need to use the dma subsystem for this? Isn't it enough to just malloc a region of memory and use it?

I'd decided against malloc() since it doesn't provide API-level alignment guarantees; while, implementation-wise, it does look like >= PAGE_SIZE-ed malloc(9) allocations will be page-aligned, my read of malloc(9) (particularly section IMPLEMENTATION NOTES) is that this should not be assumed by callers.

(FWIW, I had also noted (1) the precedent of hyperv's use of BUS_DMA(9) for its corresponding PV clock in-memory data and (2) internally, this draft's use of BUS_DMA(9) will ultimately do the same thing that the first draft was doing with its direct use of the physical memory pager).

Just recapping, the intention is that timeinfos be readily mmap-able---the allocation containing timeinfos should start and end on a page boundary (with any unused portion of the last page zeroed).

Does this choice make sense or am I missing something?

You could then also use M_WAITOK maybe?

This choice was considering use cases where the driver is loaded during boot (whether compiled in or via loader.conf(5)). My thinking was that a failure to satisfy a relatively small allocation like this during boot would be an indication of far greater problems and, as such, it wouldn't be worth adding any blocking+reclaims+retries to the boot process, especially since the system can still function without kvmclock.

But I suppose it's also ok to allow the reclaims+retries, so, I've switched bus_dmamem_alloc() from BUS_DMA_NOWAIT to BUS_DMA_WAITOK in this version as per this suggestion.

Do you really need to use the dma subsystem for this? Isn't it enough to just malloc a region of memory and use it?

I'd decided against malloc() since it doesn't provide API-level alignment guarantees; while, implementation-wise, it does look like >= PAGE_SIZE-ed malloc(9) allocations will be page-aligned, my read of malloc(9) (particularly section IMPLEMENTATION NOTES) is that this should not be assumed by callers.

Right, TBH I think malloc returning and address not aligned to a page boundary when allocating a region >= PAGE_SIZE would likely be a bug in the implementation.

(FWIW, I had also noted (1) the precedent of hyperv's use of BUS_DMA(9) for its corresponding PV clock in-memory data and (2) internally, this draft's use of BUS_DMA(9) will ultimately do the same thing that the first draft was doing with its direct use of the physical memory pager).

Just recapping, the intention is that timeinfos be readily mmap-able---the allocation containing timeinfos should start and end on a page boundary (with any unused portion of the last page zeroed).

Does this choice make sense or am I missing something?

Well, I think it's overly complicated (and cumbersome due to all the initialization involved) to use bus_dma just to allocate a memory region that's page aligned. I'm sure there are already instances in-kernel that assume that malloc(PAGE_SIZE) returns a page. I wouldn't be opposed to using:

sc->timeinfos = malloc(size, ...);
if ((sc->timeinfos & PAGE_MASK) != 0) {
     device_printf(..., "failed to allocate aligned memory region");
    KASSERT(0, ("Failed to allocate page aligned region"));
    return (ENOMEM);
}

But I understand others might have different opinions. Maybe @kib could provide some insight on whether there's another way to allocate aligned memory regions without having to resort to bus_dma, I don't have that much insight on the VM subsystem.

You could then also use M_WAITOK maybe?

This choice was considering use cases where the driver is loaded during boot (whether compiled in or via loader.conf(5)). My thinking was that a failure to satisfy a relatively small allocation like this during boot would be an indication of far greater problems and, as such, it wouldn't be worth adding any blocking+reclaims+retries to the boot process, especially since the system can still function without kvmclock.

Likely, if we fail to satisfy such small allocation it's likely the kernel won't be able to finish booting anyway. Adding a comment as to why BUS_DMA_NOWAIT is used (ie: not because the context prevents using BUS_DMA_WAITOK) would be OK also.

But I suppose it's also ok to allow the reclaims+retries, so, I've switched bus_dmamem_alloc() from BUS_DMA_NOWAIT to BUS_DMA_WAITOK in this version as per this suggestion.

While malloc(9) can't give these guarantees, uma(8) can. Creating the tag, allocating and loading it are basically creating a uma zone behind the scenes and using them directly would be better than using busdma just to get alignment.

We have malloc_domainset_aligned(), added exactly to export the alignment guarantees from the malloc subsystem.

Thanks! That's exactly what's needed here. I think this function has not been added to the malloc(9) man page?

Would it make sense to also add some syntactic sugar and introduce a:

#define malloc_aligned(s, a, t, f) malloc_domainset_aligned(s, a, t, DOMAINSET_RR(), f)

Or similar.

Of course, feel free to add. There were no users for it until now.

Right, all of this is as per the clock/RTC subsystem (sys/clock.h, kern/subr_rtc.c), which does use the clock kobj interface's entry points.