I augmented vm_phys_seg with a void *md_first field that is used for the pointer to the starting entry in pv_table. Is void * alright, or should I be making some new md_something type for the field?

markj added a reviewer: kib.Aug 27 2020, 4:56 PM

In D26132#581494, @scottph wrote:

I augmented vm_phys_seg with a void *md_first field that is used for the pointer to the starting entry in pv_table. Is void * alright, or should I be making some new md_something type for the field?

I think void * is ok for now.

I do wonder if it'd be a bit nicer to have external arrays of size VM_PHYSSEG_MAX for the pv_table and vm_reserv arrays, instead of embedding external data in the vm_phys segment array. I don't have a strong preference either way.

This revision is now accepted and ready to land.Aug 27 2020, 5:10 PM

kib accepted this revision.Aug 27 2020, 10:03 PM

alc added inline comments.Aug 27 2020, 11:00 PM

sys/arm64/arm64/pmap.c
203 ↗	(On Diff #76186)	`VM_PAGE_TO_PHYS(m)` instead of `m->phys_addr`

alc added inline comments.Aug 27 2020, 11:30 PM

sys/arm64/arm64/pmap.c
182–183 ↗	(On Diff #76186)	I don't see any calls to this function.

alc added inline comments.Aug 28 2020, 12:12 AM

sys/arm64/arm64/pmap.c
1052 ↗	(On Diff #76186)	`pvh` instead of `j`?
1124–1128 ↗	(On Diff #76186)	The same situation can arise in vm_reserv.c as here. I'm curious why you sought to handle it differently here.

dch added a subscriber: dch.Aug 28 2020, 10:10 PM

scottph updated this revision to Diff 76443.Aug 31 2020, 10:28 PM

This revision now requires review to proceed.Aug 31 2020, 10:28 PM

scottph marked 3 inline comments as done.Aug 31 2020, 10:47 PM

scottph added inline comments.

sys/arm64/arm64/pmap.c
182–183 ↗	(On Diff #76186)	Some callers of pa_to_pvh() do remain, but the call sites aren't modified in this change so you'll need to go into the context lines to see them.
1124–1128 ↗	(On Diff #76186)	That was choice was based on this comment in vm_reserv.c line 216: This array is analoguous in function to vm_page_array. It differs in the respect that it may contain a greater number of useful reservation structures than there are (physical) superpages. These "invalid" reservation structures exist to trade-off space for time in the implementation of vm_reserv_from_page(). Invalid reservation structures are distinguishable from "valid" reservation structures by inspecting the reservation's "pages" field. Invalid reservation structures have a NULL "pages" field. So I didn't add in extra code to ensure the same "invalid" sentinel entry was used. I could see that possibly being surprising to someone in the future though, so I can make the same adjustment there if you'd like. I wasn't sure what the result would be if partial pages were represented by different entries in this code, but it seemed that they are more than just sentinel values, so I added in this logic.