Hi, 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. > >> 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.
Doesn't sound like fun, but... > > 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). I have to correct myself, it is some memory that is accessed. We have some non page-aligned memory regions: 3f200000-d2bff017 : System RAM d2bff018-d2c1d657 : System RAM d2c1d658-d2c1e017 : System RAM d2c1e018-d2c2c057 : System RAM d2c2c058-dbf5efff : System RAM dbf5f000-dc086fff : Reserved The config generator will create one region per entry. This leads to subpaging, and Jailhouse will intercept access. (Like it did thousand times before on MMConfig, this is why I suspected MMconfig). Luckily, those pages can be consolidated, which is the best solution for me. 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.
