On 3/26/21 2:22 PM, Borislav Petkov wrote: > On Fri, Mar 26, 2021 at 10:42:56AM -0500, Brijesh Singh wrote: >> There is no strong reason for a separate sev-snp.h. I will add a >> pre-patch to rename sev-es.h to sev.h and add SNP stuff to it. > Thx. > >> I was trying to adhere to existing functions which uses a direct >> instruction opcode. > That's not really always the case: > > arch/x86/include/asm/special_insns.h > > The "__" prefixed things should mean lower abstraction level helpers and > we drop the ball on those sometimes. > >> It's not duplicate error code. The EAX returns an actual error code. The >> rFlags contains additional information. We want both the codes available >> to the caller so that it can make a proper decision. >> >> e.g. >> >> 1. A callers validate an address 0x1000. The instruction validated it >> and return success. > Your function returns PVALIDATE_SUCCESS. > >> 2. Caller asked to validate the same address again. The instruction will >> return success but since the address was validated before hence >> rFlags.CF will be set to indicate that PVALIDATE instruction did not >> made any change in the RMP table. > Your function returns PVALIDATE_VALIDATED_ALREADY or so. > >> You are correct that currently I am using only carry flag. So far we >> don't need other flags. What do you think about something like this: >> >> * Add a new user defined error code >> >> #define PVALIDATE_FAIL_NOUPDATE 255 /* The error is returned if >> rFlags.CF set */ > Or that. > >> * Remove the rFlags parameters from the __pvalidate() > Yes, it seems unnecessary at the moment. And I/O function arguments are > always yuck. > >> * Update the __pvalidate to check the rFlags.CF and if set then return >> the new user-defined error code. > Yap, you can convert all that to pvalidate() return values, methinks, > and then make that function simpler for callers because they should > not have to deal with rFLAGS - only return values to denote what the > function did. Ack. I will made the required changes in next version. > > Thx. >