On 5/5/19 10:25 AM, Jan Kiszka wrote: > On 03.05.19 16:29, Ralf Ramsauer wrote: >> Hi, >> >> On 4/23/18 6:18 PM, Jan Kiszka wrote: >>> From: Jan Kiszka <[email protected]> >>> >>> The parser my bail out on opcode byte 0-2. >>> >>> Signed-off-by: Jan Kiszka <[email protected]> >>> --- >>> hypervisor/arch/x86/mmio.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/hypervisor/arch/x86/mmio.c b/hypervisor/arch/x86/mmio.c >>> index c1b9f10e8..775ec4b7b 100644 >>> --- a/hypervisor/arch/x86/mmio.c >>> +++ b/hypervisor/arch/x86/mmio.c >>> @@ -219,7 +219,7 @@ error_noinst: >>> >>> error_unsupported: >>> panic_printk("FATAL: unsupported instruction " >>> - "(0x%02x [0x%02x] 0x%02x 0x%02x)\n", >>> + "(0x%02x 0x%02x 0x%02x 0x%02x)\n", >>> op[0].raw, op[1].raw, op[2].raw, op[3].raw); >> >> sorry for digging out this old patch, but I think we have a logical bug >> here: >> >> There are cases, where we jump to error_unsupported before all opcodes >> are set. In this case, the Hypervisor reports the wrong instruction, >> which really confused us for a while (Andrej found this). >> > > Yes, this is a known limitation, and the patch here just tried to reduce > the > level of confusion a bit. Before that patch, the output suggested that > we are > always at opcode byte 2. But we can fail at any byte. > > This is an expert debug tool. You are expected to parse the opcode bytes > yourself from left to right. And as soon as you hit an opcode that is no > longer > supported by us, you know that the succeeding zeros are invalid. If you > want to > full instruction, use RIP and disassemble the guest. Adding that logic > to the > hypervisor is not planned.
Okay -- maybe we could strike out (0xXX) unknown bytes and simply count successfully parsed ops. This would at least give a hint that sth. went wrong. > >> We have such a case, if decoding of op[0] fails. Jailhouse will then >> report sth. like: >> >> FATAL: unsupported instruction (0x83 0x00 0x00 0x00) >> >> Which is wrong, the code behind is: >> ffffffff819705d6: 83 78 08 03 cmpl $0x3,0x8(%rax) >> >> Which brought us to the next issue: CMPL might be intercepted, when >> accessing, e.g., PCI MMConfig space. In Linux' pcibios_add_device, we >> have the upper code fragment that accesses MMConfig with CMPL. >> > > Hmpf. Would be complicated to emulate as we would then also have to > handle the > actual comparison with its impact on flags. > > But looking at the function is 5.1-rc7, I don't see MMConfig accesses > yet. Are > you sure about the access address? I would rather guess we are touching > reserved > and, thus, not permitted BIOS or bootloader RAM here (struct setup_data). Let me doublecheck the address... Thanks Ralf > > Jan > -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
