[RESEND 2/3] hv: TLFS: inject #GP to guest VM for writing of read-only MSRs #gp


Shuo A Liu
 

From: Jian Jun Chen <jian.jun.chen@intel.com>

TLFS spec defines that HV_X64_MSR_VP_INDEX and HV_X64_MSR_TIME_REF_COUNT
are read-only MSRs. Any attempt to write to them results in a #GP fault.

Fix the issue by returning error in handler hyperv_wrmsr() of MSRs
HV_X64_MSR_VP_INDEX/HV_X64_MSR_TIME_REF_COUNT emulation.

Signed-off-by: Jian Jun Chen <jian.jun.chen@intel.com>
Signed-off-by: Shuo A Liu <shuo.a.liu@intel.com>
---
hypervisor/arch/x86/guest/hyperv.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hypervisor/arch/x86/guest/hyperv.c b/hypervisor/arch/x86/guest/hyperv.c
index ee15faf0d703..d3bbfc43f987 100644
--- a/hypervisor/arch/x86/guest/hyperv.c
+++ b/hypervisor/arch/x86/guest/hyperv.c
@@ -180,15 +180,13 @@ hyperv_wrmsr(struct acrn_vcpu *vcpu, uint32_t msr, uint64_t wval)
vcpu->vm->arch_vm.hyperv.hypercall_page.val64 = wval;
hyperv_setup_hypercall_page(vcpu, wval);
break;
- case HV_X64_MSR_VP_INDEX:
- /* read only */
- break;
- case HV_X64_MSR_TIME_REF_COUNT:
- /* read only */
- break;
case HV_X64_MSR_REFERENCE_TSC:
hyperv_setup_tsc_page(vcpu, wval);
break;
+ case HV_X64_MSR_VP_INDEX:
+ case HV_X64_MSR_TIME_REF_COUNT:
+ /* read only */
+ /* fallthrough */
default:
pr_err("hv: %s: unexpected MSR[0x%x] write", __func__, msr);
ret = -1;
--
2.28.0


Eddie Dong
 

-----Original Message-----
From: acrn-dev@lists.projectacrn.org <acrn-dev@lists.projectacrn.org> On
Behalf Of Shuo A Liu
Sent: Monday, April 19, 2021 2:04 PM
To: acrn-dev@lists.projectacrn.org
Cc: Chen, Jian Jun <jian.jun.chen@intel.com>; Liu, Shuo A
<shuo.a.liu@intel.com>
Subject: [acrn-dev] [RESEND 2/3] hv: TLFS: inject #GP to guest VM for writing
of read-only MSRs

From: Jian Jun Chen <jian.jun.chen@intel.com>

TLFS spec defines that HV_X64_MSR_VP_INDEX and
HV_X64_MSR_TIME_REF_COUNT are read-only MSRs. Any attempt to write
to them results in a #GP fault.

Fix the issue by returning error in handler hyperv_wrmsr() of MSRs
HV_X64_MSR_VP_INDEX/HV_X64_MSR_TIME_REF_COUNT emulation.

Signed-off-by: Jian Jun Chen <jian.jun.chen@intel.com>
Signed-off-by: Shuo A Liu <shuo.a.liu@intel.com>
---
hypervisor/arch/x86/guest/hyperv.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hypervisor/arch/x86/guest/hyperv.c
b/hypervisor/arch/x86/guest/hyperv.c
index ee15faf0d703..d3bbfc43f987 100644
--- a/hypervisor/arch/x86/guest/hyperv.c
+++ b/hypervisor/arch/x86/guest/hyperv.c
@@ -180,15 +180,13 @@ hyperv_wrmsr(struct acrn_vcpu *vcpu, uint32_t
msr, uint64_t wval)
vcpu->vm->arch_vm.hyperv.hypercall_page.val64 = wval;
hyperv_setup_hypercall_page(vcpu, wval);
break;
- case HV_X64_MSR_VP_INDEX:
- /* read only */
- break;
- case HV_X64_MSR_TIME_REF_COUNT:
- /* read only */
- break;
case HV_X64_MSR_REFERENCE_TSC:
hyperv_setup_tsc_page(vcpu, wval);
break;
+ case HV_X64_MSR_VP_INDEX:
+ case HV_X64_MSR_TIME_REF_COUNT:
+ /* read only */
+ /* fallthrough */
default:
pr_err("hv: %s: unexpected MSR[0x%x] write", __func__, msr);
Then we may come here as expected. Pr_err is not precise.

ret = -1;
--
2.28.0





Shuo A Liu
 

On 4/21/2021 10:17, Dong, Eddie wrote:


-----Original Message-----
From: acrn-dev@lists.projectacrn.org <acrn-dev@lists.projectacrn.org> On
Behalf Of Shuo A Liu
Sent: Monday, April 19, 2021 2:04 PM
To: acrn-dev@lists.projectacrn.org
Cc: Chen, Jian Jun <jian.jun.chen@intel.com>; Liu, Shuo A
<shuo.a.liu@intel.com>
Subject: [acrn-dev] [RESEND 2/3] hv: TLFS: inject #GP to guest VM for writing
of read-only MSRs

From: Jian Jun Chen <jian.jun.chen@intel.com>

TLFS spec defines that HV_X64_MSR_VP_INDEX and
HV_X64_MSR_TIME_REF_COUNT are read-only MSRs. Any attempt to write
to them results in a #GP fault.

Fix the issue by returning error in handler hyperv_wrmsr() of MSRs
HV_X64_MSR_VP_INDEX/HV_X64_MSR_TIME_REF_COUNT emulation.

Signed-off-by: Jian Jun Chen <jian.jun.chen@intel.com>
Signed-off-by: Shuo A Liu <shuo.a.liu@intel.com>
---
hypervisor/arch/x86/guest/hyperv.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hypervisor/arch/x86/guest/hyperv.c
b/hypervisor/arch/x86/guest/hyperv.c
index ee15faf0d703..d3bbfc43f987 100644
--- a/hypervisor/arch/x86/guest/hyperv.c
+++ b/hypervisor/arch/x86/guest/hyperv.c
@@ -180,15 +180,13 @@ hyperv_wrmsr(struct acrn_vcpu *vcpu, uint32_t
msr, uint64_t wval)
vcpu->vm->arch_vm.hyperv.hypercall_page.val64 = wval;
hyperv_setup_hypercall_page(vcpu, wval);
break;
-case HV_X64_MSR_VP_INDEX:
-/* read only */
-break;
-case HV_X64_MSR_TIME_REF_COUNT:
-/* read only */
-break;
case HV_X64_MSR_REFERENCE_TSC:
hyperv_setup_tsc_page(vcpu, wval);
break;
+case HV_X64_MSR_VP_INDEX:
+case HV_X64_MSR_TIME_REF_COUNT:
+/* read only */
+/* fallthrough */
default:
pr_err("hv: %s: unexpected MSR[0x%x] write", __func__, msr);
Then we may come here as expected. Pr_err is not precise.
I can change it to pr_warn().


Eddie Dong
 

+case HV_X64_MSR_TIME_REF_COUNT:
+/* read only */
+/* fallthrough */
default:
pr_err("hv: %s: unexpected MSR[0x%x] write", __func__, msr);
Then we may come here as expected. Pr_err is not precise.
I can change it to pr_warn().
Why it is warning? This could be normal operation.


Shuo A Liu
 

On 4/21/2021 12:04, Eddie Dong wrote:
+case HV_X64_MSR_TIME_REF_COUNT:
+/* read only */
+/* fallthrough */
default:
pr_err("hv: %s: unexpected MSR[0x%x] write", __func__, msr);
Then we may come here as expected. Pr_err is not precise.
I can change it to pr_warn().
Why it is warning? This could be normal operation.
Those MSRs read operations are unexpected.

'-1' is returned with those MSRs read operations. So the pr_err() is
called before '-1' returning.