On Thursday 23 October 2008 09:32:06 Matias Zabaljauregui wrote:
> This patch allows us to use KVM hypercalls.
Thanks!
A few minor comments....
> +static void rewrite_hypercall(struct lg_cpu *cpu)
> +{
> + unsigned long physaddr = guest_pa(cpu, cpu->regs->eip);
> +
> + /* This are the opcodes we use to patch the guest.
> + * The opcode for "int $ox1f" is 0xcd 0x1f
> + * but vmcall instruction is 3 bytes long, so we complete
> + * the sequence with a NOP (0x90). */
> + u8 insn[3] = {0xcd, 0x1f, 0x90};
> +
> + lgwrite(cpu, physaddr, u8, insn[0]);
> + lgwrite(cpu, physaddr + 1, u8, insn[1]);
> + lgwrite(cpu, physaddr + 2, u8, insn[2]);
I think this is a good opportunity to use __lgwrite().
Also, above this function there should be a longer explanation, showing
your benchmarks as to why it's worth having this second hcall method.
(I know, but it's good reading for new hackers!).
> + /* The eip contains the *virtual* address of the Guest's instruction:
> + * guest_pa just subtracts the Guest's page_offset. */
> + unsigned long physaddr = guest_pa(cpu, cpu->regs->eip);
> +
> + /* This must be the Guest kernel trying to do something.
> + * The bottom two bits of the CS segment register are the privilege
> + * level. */
> + if ((cpu->regs->cs & 3) != GUEST_PL)
> + return 0;
> +
> + /* Is it a vmcall? */
> + insn[0] = lgread(cpu, physaddr, u8);
> + insn[1] = lgread(cpu, physaddr + 1, u8);
> + insn[2] = lgread(cpu, physaddr + 2, u8);
I think this can be done in one line with __lgread, as well:
__lgread(cpu, insn, guest_pa(cpu, cpu->regs->eip), sizeof(insn));
> + if (insn[0] != 0x0f || insn[1] != 0x01 || insn[2] != 0xc1)
> + return 0;
> +
> + return 1;
I've become a fan of "bool", perhaps this function should return bool, and also:
return insn[0] == 0x0f && insn[1] == 0x01 && insn[2] == 0xc1;
Thanks!
Rusty.
_______________________________________________
Lguest mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/lguest
