On 06.06.19 15:53, Ralf Ramsauer wrote:
On 6/5/19 12:06 AM, Jan Kiszka wrote:
On 04.06.19 23:02, Ralf Ramsauer wrote:
mov (%rax), %ax is a 16-bit data MOV_FROM_MEM that will emit
0x66 0x8b 0x00.
0x66 is the operand-size override prefix which we currently do not
support.
We should support it, as we can find this opcode, for example, for some
mmconfig space access from Linux (e.g., pci_generic_config_read).
This also adds appropriate mmio-access tests.
Tested in QEMU virtual target.
Signed-off-by: Ralf Ramsauer <[email protected]>
---
hypervisor/arch/x86/include/asm/processor.h | 1 +
hypervisor/arch/x86/mmio.c | 37 +++++++++++++++++----
inmates/tests/x86/mmio-access-32.c | 4 +++
inmates/tests/x86/mmio-access.c | 4 +++
4 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/hypervisor/arch/x86/include/asm/processor.h
b/hypervisor/arch/x86/include/asm/processor.h
index 70a6c3ff..d8111690 100644
--- a/hypervisor/arch/x86/include/asm/processor.h
+++ b/hypervisor/arch/x86/include/asm/processor.h
@@ -145,6 +145,7 @@
#define X86_REX_CODE 4
+#define X86_PREFIX_OP_SZ 0x66
#define X86_PREFIX_ADDR_SZ 0x67
#define X86_OP_MOVZX_OPC1 0x0f
diff --git a/hypervisor/arch/x86/mmio.c b/hypervisor/arch/x86/mmio.c
index b234bd79..6d9ad1c5 100644
--- a/hypervisor/arch/x86/mmio.c
+++ b/hypervisor/arch/x86/mmio.c
@@ -79,6 +79,26 @@ static unsigned int get_address_width(bool
has_addrsz_prefix)
(!!(cs_attr & VCPU_CS_DB) ^ has_addrsz_prefix) ? 4 : 2;
}
+static unsigned int get_op_width(bool has_rex_w, bool has_opsz_prefix)
We should move all the flags into parse_context so that we can pass them
around more easily.
Good point.
+{
+ u16 cs_attr;
+ bool long_mode;
+
+ /* Op size prefix is ignored if rex.w = 1 */
+ if (has_rex_w)
+ return 8;
+
+ cs_attr = vcpu_vendor_get_cs_attr();
+ long_mode = (vcpu_vendor_get_efer() & EFER_LMA) &&
+ (cs_attr & VCPU_CS_L);
This may mean accessing the same VMCS regs multiple times. I vaguely
recall from KVM that it pays off to avoid that and keep the results
cached (per vmexit).
See comment below…
(BTW: not that it solved this issue, but we could also consider to
inline VCPU_VENDOR_GET_ accessors)
+
+ if (long_mode)
+ /* CS.d is ignored in long mode */
+ return has_opsz_prefix ? 2 : 4;
+
+ return (!!(cs_attr & VCPU_CS_DB) ^ has_opsz_prefix) ? 4 : 2;
This does the same as get_address_width (minus different output values),
but its code format is different. Should be aligned.
Yeah. I chose this style, as it took me a while to understand what
long_mode ? (has_addrsz_prefix ? 4 : 8) : (!!(cs_attr & VCPU_CS_DB) ^
has_addrsz_prefix) ? 4 : 2;
actually means -- with respect to ?'s operator precedence (which is
clear in this case, but I got confused). May I propose to rather align
the line mentioned above? (but let's see -- maybe we can consolidate it
in any case)
I think you will still have two instances of the same pattern, but with
different results. But when folding them into the same function, please use the
same format for them. Readability trumps, no question.
In fact, I could imagine a combined helper:
void parse_widths(struct parse_context *ctx,
struct mmio_instruction *inst,
bool parse_addr_width)
That one would obtain cs_attr and long_mode only once, would do the
address width thing only if parse_addr_width is true, and would push its
results directly into *inst.
... as an inlined single-user function, right?
The compiler will do the inlining when it makes sense. In this case likely not
because there will remain multiple call sites.
Good idea, that would also save the potential double efer/cs_attr
access. I'm just curious: It's Intel's vmread that you worry about? At
least on AMD, guest's efer is directly read from vmcs region.
Yes. Possibly accessing the same VMCS register twice in the same vmexit cycle is
no longer as expensive as it used to be, but I doubt it will be faster than not
reading it twice (but rather taking it from a register or a cache).
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT 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].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/eb6da892-059f-16f7-2473-74fbff2cc2d8%40siemens.com.
For more options, visit https://groups.google.com/d/optout.
