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	`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	I don't see any calls to this function.

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

sys/arm64/arm64/pmap.c
1028	`pvh` instead of `j`?
1097–1101	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	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.
1097–1101	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.