On 2017-09-14 01:37, Gustavo Lima Chaves wrote: > Those are: > > X86_OP_MOV_IMMEDIATE_TO_MEM 0xc7 > X86_OP_MOV_MEM_TO_RAX 0xa1 > X86_OP_MOV_RAX_TO_MEM 0xa3
Let's call this AX_TO_MEM / MEM_TO_AX - the register width depends on other parameters. Can we split them up? I suppose handling of the immediate access differs from the AX accesses. > > The reason to bring them in is that the code is simple enough and will > enable one more guest OS (Zephyr OS) to function in xAPIC mode as well. > > Signed-off-by: Gustavo Lima Chaves <[email protected]> > --- > hypervisor/arch/x86/apic.c | 6 ++++- > hypervisor/arch/x86/include/asm/mmio.h | 3 +++ > hypervisor/arch/x86/include/asm/processor.h | 3 +++ > hypervisor/arch/x86/mmio.c | 35 > ++++++++++++++++++++++++++++- > 4 files changed, 45 insertions(+), 2 deletions(-) > > diff --git a/hypervisor/arch/x86/apic.c b/hypervisor/arch/x86/apic.c > index f09d093..73e240b 100644 > --- a/hypervisor/arch/x86/apic.c > +++ b/hypervisor/arch/x86/apic.c > @@ -504,7 +504,11 @@ unsigned int apic_mmio_access(unsigned long rip, > return 0; > } > if (is_write) { > - val = this_cpu_data()->guest_regs.by_index[inst.reg_num]; > + if (inst.has_immediate) > + val = inst.immediate; > + else > + val = > this_cpu_data()->guest_regs.by_index[inst.reg_num]; > + > if (apic_accessing_reserved_bits(reg, val)) > return 0; > > diff --git a/hypervisor/arch/x86/include/asm/mmio.h > b/hypervisor/arch/x86/include/asm/mmio.h > index 72aecab..5c26c5c 100644 > --- a/hypervisor/arch/x86/include/asm/mmio.h > +++ b/hypervisor/arch/x86/include/asm/mmio.h > @@ -28,6 +28,9 @@ struct mmio_instruction { > /** Number of the register that holds the output value or should > * receive the input. */ > unsigned int reg_num; > + /** Immediate value, when due */ > + signed int immediate; Just "int". > + bool has_immediate; We could also extend this in different way: use reg_num only for input and define an output_value that is initialized by the parser to hold the register value or the immediate. Saves the bool here and its testing above. > }; > > /** > diff --git a/hypervisor/arch/x86/include/asm/processor.h > b/hypervisor/arch/x86/include/asm/processor.h > index a658039..e3c36ed 100644 > --- a/hypervisor/arch/x86/include/asm/processor.h > +++ b/hypervisor/arch/x86/include/asm/processor.h > @@ -148,6 +148,9 @@ > #define X86_OP_MOVB_TO_MEM 0x88 > #define X86_OP_MOV_TO_MEM 0x89 > #define X86_OP_MOV_FROM_MEM 0x8b > +#define X86_OP_MOV_IMMEDIATE_TO_MEM 0xc7 > +#define X86_OP_MOV_MEM_TO_RAX 0xa1 > +#define X86_OP_MOV_RAX_TO_MEM 0xa3 > > #define DB_VECTOR 1 > #define NMI_VECTOR 2 > diff --git a/hypervisor/arch/x86/mmio.c b/hypervisor/arch/x86/mmio.c > index 7ca2ba8..7d3a3ab 100644 > --- a/hypervisor/arch/x86/mmio.c > +++ b/hypervisor/arch/x86/mmio.c > @@ -78,7 +78,8 @@ struct mmio_instruction x86_mmio_parse(unsigned long pc, > const struct guest_paging_structures *pg_structs, bool is_write) > { > struct parse_context ctx = { .remaining = X86_MAX_INST_LEN }; > - struct mmio_instruction inst = { .inst_len = 0 }; > + struct mmio_instruction inst = { .inst_len = 0, .has_immediate = false > }; Overlong line, and strictly spoken unneeded (all fields will be zero-initialized). > + const u8 *inst_start = NULL; > union opcode op[4] = { }; > bool does_write = false; > bool has_rex_w = false; > @@ -88,6 +89,9 @@ restart: > if (!ctx_maybe_get_bytes(&ctx, &pc, pg_structs)) > goto error_noinst; > > + if (!inst_start) > + inst_start = ctx.inst; > + > op[0].raw = *(ctx.inst); > if (op[0].rex.code == X86_REX_CODE) { > if (op[0].rex.w) > @@ -126,6 +130,23 @@ restart: > inst.inst_len += 2; > inst.access_size = 4; > break; > + case X86_OP_MOV_IMMEDIATE_TO_MEM: > + inst.inst_len += 2; > + inst.access_size = 4; > + inst.has_immediate = true; > + does_write = true; > + break; > + case X86_OP_MOV_MEM_TO_RAX: > + inst.inst_len += 5; > + inst.access_size = 4; > + inst.reg_num = 15; > + goto final; > + case X86_OP_MOV_RAX_TO_MEM: > + inst.inst_len += 5; > + inst.access_size = 4; > + inst.reg_num = 15; > + does_write = true; > + goto final; > default: > goto error_unsupported; > } > @@ -138,6 +159,8 @@ restart: > case 0: > if (op[2].modrm.rm == 5) { /* 32-bit displacement */ > inst.inst_len += 4; > + if (inst.has_immediate) > + inst.inst_len += 4; inst.inst_len += inst.has_immediate ? 8 : 4; > break; > } else if (op[2].modrm.rm != 4) { /* no SIB */ > break; > @@ -168,6 +191,16 @@ restart: > else > inst.reg_num = 15 - op[2].modrm.reg; > > +final: > + /* FIXME: what if an instruction with immediate spans two > + * pages? */ That has to be addressed (another reason to split the patch, because we can then already handle the other two instructions). Just look at how we do this for the instruction so far and follow the same path. > + if (inst.has_immediate) { > + if (ctx.inst - inst_start + ctx.size < inst.inst_len) > + goto error_noinst; > + inst.immediate = *(typeof(inst.immediate)*) > + (inst_start + inst.inst_len - sizeof(inst.immediate)); > + } > + > if (does_write != is_write) > goto error_inconsitent; > > Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux -- 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.
