On 25/06/18 18:37, David Hildenbrand wrote:
On 25.06.2018 17:32, Liran Alon wrote:
On 25/06/18 15:20, David Hildenbrand wrote:
On 23.06.2018 01:35, Liran Alon wrote:
No functionality change.
Signed-off-by: Liran Alon <liran.alon@xxxxxxxxxx>
Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
---
arch/x86/kvm/vmx.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 559a12b6184d..2fe86eb91aca 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -198,8 +198,13 @@ struct kvm_vmx {
#define NR_AUTOLOAD_MSRS 8
+struct vmcs_hdr {
+ u32 revision_id:31;
+ u32 shadow_vmcs:1;
+};
Just wondering if this struct is needed. (didn't check following patches)
It makes code more readable but of course I could have completed the
patch series without it.
It will just force me to have to signal the "shadow_vmcs" bit in
previous "uint32_t revision_id" field
manually (e.g. "|= (1u << 31)") which I think is less elegant.
I was rather wondering if the bitmap thingy can be also done directly in
the struct vmcs (avoiding the struct vmcs_hdr).
It's possible but I don't like it because I need to define the exact
same struct for both "struct vmcs" and "struct vmcs12".
I prefer it to be a separate struct vmcs_hdr like it is now.
+
struct vmcs {
- u32 revision_id;
+ struct vmcs_hdr hdr;
u32 abort;
char data[0];
};
@@ -253,7 +258,7 @@ struct __packed vmcs12 {
/* According to the Intel spec, a VMCS region must start with the
* following two fields. Then follow implementation-specific data.
*/
- u32 revision_id;
+ struct vmcs_hdr hdr;
u32 abort;
u32 launch_state; /* set to 0 by VMCLEAR, to 1 by VMLAUNCH */
@@ -421,7 +426,7 @@ struct __packed vmcs12 {
"Offset of " #field " in struct vmcs12 has changed.")
static inline void vmx_check_vmcs12_offsets(void) {
- CHECK_OFFSET(revision_id, 0);
+ CHECK_OFFSET(hdr, 0);
CHECK_OFFSET(abort, 4);
CHECK_OFFSET(launch_state, 8);
CHECK_OFFSET(io_bitmap_a, 40);
@@ -4385,7 +4390,7 @@ static struct vmcs *alloc_vmcs_cpu(int cpu)
return NULL;
vmcs = page_address(pages);
memset(vmcs, 0, vmcs_config.size);
- vmcs->revision_id = vmcs_config.revision_id; /* vmcs revision id */
+ vmcs->hdr.revision_id = vmcs_config.revision_id;
return vmcs;
}
@@ -7848,7 +7853,7 @@ static int enter_vmx_operation(struct kvm_vcpu *vcpu)
if (!shadow_vmcs)
goto out_shadow_vmcs;
/* mark vmcs as shadow */
- shadow_vmcs->revision_id |= (1u << 31);
+ shadow_vmcs->hdr.shadow_vmcs = 1;
/* init shadow vmcs */
vmcs_clear(shadow_vmcs);
vmx->vmcs01.shadow_vmcs = shadow_vmcs;
@@ -8418,7 +8423,7 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu)
return kvm_skip_emulated_instruction(vcpu);
}
new_vmcs12 = kmap(page);
- if (new_vmcs12->revision_id != VMCS12_REVISION) {
+ if (new_vmcs12->hdr.revision_id != VMCS12_REVISION) {
kunmap(page);
kvm_release_page_clean(page);
nested_vmx_failValid(vcpu,
