Messages in this thread | | | Date | Tue, 17 May 2022 23:17:10 +0300 | From | "Kirill A. Shutemov" <> | Subject | Re: [PATCH] x86/tdx: Handle load_unaligned_zeropad() page-cross to a shared page |
| |
On Tue, May 17, 2022 at 11:14:13AM -0700, Dave Hansen wrote: > On 5/17/22 10:40, Kirill A. Shutemov wrote: > >> > >> ve_info is a software structure. Why not just add a: > >> > >> bool ip_adjusted; > >> > >> which defaults to false, then we have: > >> > >> /* > >> * Adjust RIP if the exception was handled > >> * but RIP was not adjusted. > >> */ > >> if (!ret && !ve_info->ip_adjusted) > >> regs->ip += ve_info->instr_len; > >> > >> One other oddity I just stumbled upon: > >> > >> static bool handle_mmio(struct pt_regs *regs, struct ve_info *ve) > >> { > >> ... > >> ve->instr_len = insn.length; > >> > >> Why does that need to override 've->instr_len'? What was wrong with the > >> gunk in r10 that came out of TDX_GET_VEINFO? > > TDX module doesn't decode MMIO instruction and does not provide valid size > > of it. We had to do it manually, based on decoding. > > That's worth a comment, don't you think? I'd add one both in where the > ve_info is filled and where ve->instr_len is adjusted.
Okay. Will do.
> > Given that we had to adjust IP in handle_mmio() anyway, do you still think > > "ve->instr_len = 0;" is wrong? I dislike ip_adjusted more. > > Something is wrong about it. > > You could call it 've->instr_bytes_to_handle' or something. Then it > makes actual logical sense when you handle it to zero it out. I just > want it to be more explicit when the upper levels need to do something. > > Does ve->instr_len==0 both when the TDX module isn't providing > instruction sizes *and* when no handling is necessary? That seems like > an unfortunate logical multiplexing of 0.
For EPT violation, ve->instr_len has *something* (not zero) that doesn't match the actual instruction size. I dig out that it is filled with data from VMREAD(0x440C), but I don't know where is the ultimate origin of the data.
I don't understand virtualization side of the thing well enough.
Maybe someone who knows virtualtion could comment here. Sean?
-- Kirill A. Shutemov
| |