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 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu