On 03/10/2023 9:46 pm, Stefano Stabellini wrote: > On Tue, 3 Oct 2023, Nicola Vetrini wrote: >> As specified in rules.rst, these constants can be used >> in the code. >> Their deviation is now accomplished by using a SAF comment, >> rather than an ECLAIR configuration. >> >> Signed-off-by: Nicola Vetrini <nicola.vetr...@bugseng.com> > "SAF" discussion aside that can be resolved elsewhere: > > Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
Well no. "SAF" aside (and SAF does need fixing before reposting this patch, otherwise it's just unnecessary churn), ... >> diff --git a/xen/arch/x86/hvm/svm/svm.h b/xen/arch/x86/hvm/svm/svm.h >> index d2a781fc3fb5..d0623b72ccfa 100644 >> --- a/xen/arch/x86/hvm/svm/svm.h >> +++ b/xen/arch/x86/hvm/svm/svm.h >> @@ -57,14 +57,23 @@ static inline void svm_invlpga(unsigned long linear, >> uint32_t asid) >> #define INSTR_INT3 INSTR_ENC(X86EMUL_OPC( 0, 0xcc), 0) >> #define INSTR_ICEBP INSTR_ENC(X86EMUL_OPC( 0, 0xf1), 0) >> #define INSTR_HLT INSTR_ENC(X86EMUL_OPC( 0, 0xf4), 0) >> +/* SAF-2-safe */ >> #define INSTR_XSETBV INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0321) >> +/* SAF-2-safe */ >> #define INSTR_VMRUN INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0330) >> +/* SAF-2-safe */ >> #define INSTR_VMCALL INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0331) >> +/* SAF-2-safe */ >> #define INSTR_VMLOAD INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0332) >> +/* SAF-2-safe */ >> #define INSTR_VMSAVE INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0333) >> +/* SAF-2-safe */ >> #define INSTR_STGI INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0334) >> +/* SAF-2-safe */ >> #define INSTR_CLGI INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0335) >> +/* SAF-2-safe */ >> #define INSTR_INVLPGA INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0337) >> +/* SAF-2-safe */ >> #define INSTR_RDTSCP INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0371) >> #define INSTR_INVD INSTR_ENC(X86EMUL_OPC(0x0f, 0x08), 0) >> #define INSTR_WBINVD INSTR_ENC(X86EMUL_OPC(0x0f, 0x09), 0) ... this has broken a tabulated structure to have comments ahead of lines with octal numbers, while ... >> diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c >> index aa2c61c433b3..c5e3341c6316 100644 >> --- a/xen/arch/x86/hvm/svm/emulate.c >> +++ b/xen/arch/x86/hvm/svm/emulate.c >> @@ -90,9 +90,9 @@ unsigned int svm_get_insn_len(struct vcpu *v, unsigned int >> instr_enc) >> if ( !instr_modrm ) >> return emul_len; >> >> - if ( modrm_mod == MASK_EXTR(instr_modrm, 0300) && >> - (modrm_reg & 7) == MASK_EXTR(instr_modrm, 0070) && >> - (modrm_rm & 7) == MASK_EXTR(instr_modrm, 0007) ) >> + if ( modrm_mod == MASK_EXTR(instr_modrm, 0300) && /* >> SAF-2-safe */ >> + (modrm_reg & 7) == MASK_EXTR(instr_modrm, 0070) && /* >> SAF-2-safe */ >> + (modrm_rm & 7) == MASK_EXTR(instr_modrm, 0007) ) /* >> SAF-2-safe */ >> return emul_len; >> } ... this has comments at the end of lines with octal numbers. So which is it? ~Andrew