Paths

Table of Contentst

riscv: Svpbmt extension support
ClosedPublic
Actions

Authored by mhorne on Jun 3 2024, 6:51 PM.

Details

Reviewers

markj
br
jrtc27
jhb

Group Reviewers

riscv

Commits

rGb7312c3df83d: riscv: Svpbmt extension support

Summary

The Svpbmt extension provides specification of "Page-Based Memory
Types", or memory attributes (e.g. cacheability constraints).

Extend pmap code to apply memory attributes when creating/updating PTEs.
This is done in a way which should be non-hostile to CPUs lacking memory
attribute (Svpbmt) support, and applicable to alternate encodings of
this feature (T-HEAD CPUs, see next review).

Test Plan

Basic smoketest in a VM with 'Svpbmt' extension enabled: this only has a couple mappings marked "I/O" in sysctl vm.pmap.kernel_maps.

pmap_change_attr() is not widely used, so to test this I swapped some driver malloc() calls to allocate memory/KVA but map it as VM_MEMATTR_NOCACHE. This seemed to work fine and the DMAP was updated accordingly, forcing a demotion of the L1 page created by pmap_bootstrap_dmap().

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 58278
Build 55166: arc lint + arc unit

Event Timeline

mhorne created this revision.Jun 3 2024, 6:51 PM

Herald added a subscriber: imp. · View Herald TranscriptJun 3 2024, 6:51 PM

mhorne requested review of this revision.Jun 3 2024, 6:51 PM

Harbormaster completed remote builds in B58036: Diff 139384.Jun 3 2024, 6:51 PM

mhorne added a child revision: D45472: riscv: T-HEAD PBMT support.Jun 3 2024, 6:51 PM

mhorne added a reviewer: jhb.Jun 3 2024, 6:52 PM

jrtc27 added inline comments.Jun 3 2024, 7:00 PM

sys/riscv/riscv/pmap.c
4884	Could you pull memattr_mask down into this commit? We'll need it in CheriBSD, as our existing CHERI-RISC-V spec collides with Svpbmt (there was no space to do custom extensions at the time we wrote it, so picked the high bits thinking the standard bits would be allocated from the bottom not the top...). That will eventually change as we move towards standardising within RISC-V, but we haven't got a draft spec for PTEs yet, and even then it's going to be a big lift to switch everything over to the draft standard encodings, mnemonics, PTEs, etc.

mhorne added inline comments.Jun 3 2024, 7:04 PM

sys/riscv/riscv/pmap.c
4884	Certainly, I can do that.

Generally looks good to me.

sys/riscv/riscv/pmap.c
544

One of my suggestions didn't get added and I have to add a dummy comment to add it.

sys/riscv/riscv/pmap.c
3421	You can remove the commented out line?

Finish implementation of pmap_change_attr_locked().

Bring in memattr_mask from the T-HEAD child review.

Harbormaster completed remote builds in B58221: Diff 139958.Jun 18 2024, 6:09 PM

mhorne marked 3 inline comments as done.Jun 18 2024, 6:10 PM

mhorne added a parent revision: D45628: riscv: implement pmap_demote_l1().

mhorne retitled this revision from [DRAFT/RFC] riscv: Svpbmt extension support to riscv: Svpbmt extension support.Jun 18 2024, 6:15 PM

mhorne edited the summary of this revision. (Show Details)

mhorne edited the test plan for this revision. (Show Details)

mhorne added inline comments.Jun 18 2024, 6:18 PM

sys/riscv/riscv/pmap.c
4947	Note: I do think it is possible to collapse this into the first loop, but I had a difficult time expressing this cleanly. This is not a performance critical path.
4965–4970	This logic could be improved to batch updates to the DMAP, akin to amd64's implementation. For now I did the simple thing.

mhorne mentioned this in D45628: riscv: implement pmap_demote_l1().Jun 20 2024, 4:51 PM

Apply memattr bits to mappings created at the end of pmap_bootstrap() (this was missed).

Harbormaster completed remote builds in B58278: Diff 140068.Jun 21 2024, 2:39 PM

Ping. I am ready to land this soon. Any further comments?

jhb accepted this revision as: jhb.Jul 30 2024, 4:41 PM

jhb added inline comments.

sys/riscv/riscv/pmap.c

4906

amd64 uses PDPMASK, etc. which are like PAGE_MASK here, so yes, that means you need the -1 here and in the other case below.

The code from amd64:

			/*
			 * If the current 1GB page already has the required
			 * properties, then we need not demote this page.  Just
			 * increment tmpva to the next 1GB page frame.
			 */
			if ((*pdpe & pde_mask) == pde_bits) {
				tmpva = trunc_1gpage(tmpva) + NBPDP;
				continue;
			}

			/*
			 * If the current offset aligns with a 1GB page frame
			 * and there is at least 1GB left within the range, then
			 * we need not break down this page into 2MB pages.
			 */
			if ((tmpva & PDPMASK) == 0 &&
			    tmpva + PDPMASK < base + size) {
				tmpva += NBPDP;
				continue;
			}
			if (!pmap_demote_pdpe(kernel_pmap, pdpe, tmpva))
				return (ENOMEM);

4947

amd64 does two loops as well, one to do demotions, a second to do all the PTE updates

This revision is now accepted and ready to land.Jul 30 2024, 4:41 PM

tested, works to me on Allwinner D1S

mhorne added inline comments.Jul 30 2024, 9:52 PM

sys/riscv/riscv/pmap.c
4906	Thanks, agreed. I've updated my local version committed version to express it as: if ((tmpva & L1_OFFSET) == 0 && tmpva + L1_SIZE <= base + size) {