Re: [PATCH 4/8] x86/traps: Demand-populate PASID MSR via #GP

[Dave Hansen]

On 9/27/21 2:02 PM, Luck, Tony wrote:
> Or are you thinking of a helper that does both the check
> and the update ... so the code here could be:
> 
> 	update_one_xsave_feature(XFEATURE_PASID, &msr_val, sizeof(msr_val));
> 
> With the helper being something like:
> 
> void update_one_xsave_feature(enum xfeature xfeature, void *data, size_t size)
> {
> 	if (xsave_state_in_memory(args ...)) {
> 		addr = get_xsave_addr(xsave, xfeature);
> 		memcpy(addr, data, size);
> 		xsave->header.xfeatures |= (1 << xfeature);
> 		return;
> 	}
> 
> 	switch (xfeature) {
> 	case XFEATURE_PASID:
> 		wrmsrl(MSR_IA32_PASID, *(u64 *)data);
> 		break;
> 
> 	case each_of_the_other_XFEATURE_enums:
> 		code to update registers for that XFEATURE
> 	}
> }
> 
> either way needs the definitive correct coding for xsave_state_in_memory()

That's close to what we want.

The size should probably be implicit.  If it isn't implicit, it needs to
at least be double-checked against the state sizes.

Not to get too fancy, but I think we also want to have a "replace"
operation which is separate from the "update".  Think of a case where
you are trying to set a bit:

	struct pkru_state *pk = start_update_xstate(tsk, XSTATE_PKRU);
	pk->pkru |= 0x100;
	finish_update_xstate(tsk, XSTATE_PKRU, pk);

versus setting a whole new value:

	struct pkru_state *pk = start_replace_xstate(tsk, XSTATE_PKRU);
	memset(pkru, sizeof(*pk), 0);
	pk->pkru = 0x1234;
	finish_replace_xstate(tsk, XSTATE_PKRU, pk);

They look similar, but they actually might have very different costs for
big states like AMX.  We can also do some very different debugging for
them.  In normal operation, these _can_ just return pointers directly to
the fpu...->xstate in some case.  But, if we're debugging, we could
kmalloc() a buffer and do sanity checking before it goes into the task
buffer.

We don't have to do any of that fancy stuff now.  We don't even need to
do an "update" if all we need for now for XFEATURE_PASID is a "replace".

1. Hide whether we need to write to real registers
2. Hide whether we need to update the in-memory image
3. Hide other FPU infrastructure like the TIF flag.
4. Make the users deal with a *whole* state in the replace API