[PATCH v2 02/11] HV: change INVD vmexit handler to trigger #ud


Qian1,Wang
 

From: Qian Wang <qian1.wang@intel.com>

hv: vmexit: change INVD vmexit handler to trigger #UD

Changed the vmexit handler of INVD instruction from
unhandled_vmexit_handler to undefined_vmexit_handler to meet our design.
Now INVD will trigger an #UD.

Signed-off-by: Qian Wang <qian1.wang@intel.com>
---
hypervisor/arch/x86/guest/vmexit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hypervisor/arch/x86/guest/vmexit.c b/hypervisor/arch/x86/guest/vmexit.c
index dfab7ff4..7524d4f0 100644
--- a/hypervisor/arch/x86/guest/vmexit.c
+++ b/hypervisor/arch/x86/guest/vmexit.c
@@ -62,7 +62,7 @@ static const struct vm_exit_dispatch dispatch_table[NR_VMX_EXIT_REASONS] = {
[VMX_EXIT_REASON_HLT] = {
.handler = hlt_vmexit_handler},
[VMX_EXIT_REASON_INVD] = {
- .handler = unhandled_vmexit_handler},
+ .handler = undefined_vmexit_handler,},
[VMX_EXIT_REASON_INVLPG] = {
.handler = unhandled_vmexit_handler,},
[VMX_EXIT_REASON_RDPMC] = {
--
2.17.1


Eddie Dong
 

-----Original Message-----
From: Wang, Qian1 <qian1.wang@intel.com>
Sent: Tuesday, October 27, 2020 3:17 PM
To: acrn-dev@lists.projectacrn.org
Cc: Dong, Eddie <eddie.dong@intel.com>
Subject: [PATCH v2 02/11] HV: change INVD vmexit handler to trigger #UD

From: Qian Wang <qian1.wang@intel.com>

hv: vmexit: change INVD vmexit handler to trigger #UD

Changed the vmexit handler of INVD instruction from
unhandled_vmexit_handler to undefined_vmexit_handler to meet our design.
Now INVD will trigger an #UD.

Signed-off-by: Qian Wang <qian1.wang@intel.com>
---
hypervisor/arch/x86/guest/vmexit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hypervisor/arch/x86/guest/vmexit.c
b/hypervisor/arch/x86/guest/vmexit.c
index dfab7ff4..7524d4f0 100644
--- a/hypervisor/arch/x86/guest/vmexit.c
+++ b/hypervisor/arch/x86/guest/vmexit.c
@@ -62,7 +62,7 @@ static const struct vm_exit_dispatch
dispatch_table[NR_VMX_EXIT_REASONS] = {
[VMX_EXIT_REASON_HLT] = {
.handler = hlt_vmexit_handler},
[VMX_EXIT_REASON_INVD] = {
- .handler = unhandled_vmexit_handler},
+ .handler = undefined_vmexit_handler,},

Normally the guest won't issue this instruction. But if it issues:
The original code prints and do nothing (for debug only).
Now, you inject #UD. I am not sure why we need this. Can you explain? We don't have INVD capability to remove from the guest.


[VMX_EXIT_REASON_INVLPG] = {
.handler = unhandled_vmexit_handler,},
[VMX_EXIT_REASON_RDPMC] = {
--
2.17.1


Li, Fei1
 

On Tue, Oct 27, 2020 at 08:50:38AM +0000, Eddie Dong wrote:


-----Original Message-----
From: Wang, Qian1 <qian1.wang@intel.com>
Sent: Tuesday, October 27, 2020 3:17 PM
To: acrn-dev@lists.projectacrn.org
Cc: Dong, Eddie <eddie.dong@intel.com>
Subject: [PATCH v2 02/11] HV: change INVD vmexit handler to trigger #UD

From: Qian Wang <qian1.wang@intel.com>

hv: vmexit: change INVD vmexit handler to trigger #UD

Changed the vmexit handler of INVD instruction from
unhandled_vmexit_handler to undefined_vmexit_handler to meet our design.
Now INVD will trigger an #UD.

Signed-off-by: Qian Wang <qian1.wang@intel.com>
---
hypervisor/arch/x86/guest/vmexit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hypervisor/arch/x86/guest/vmexit.c
b/hypervisor/arch/x86/guest/vmexit.c
index dfab7ff4..7524d4f0 100644
--- a/hypervisor/arch/x86/guest/vmexit.c
+++ b/hypervisor/arch/x86/guest/vmexit.c
@@ -62,7 +62,7 @@ static const struct vm_exit_dispatch
dispatch_table[NR_VMX_EXIT_REASONS] = {
[VMX_EXIT_REASON_HLT] = {
.handler = hlt_vmexit_handler},
[VMX_EXIT_REASON_INVD] = {
- .handler = unhandled_vmexit_handler},
+ .handler = undefined_vmexit_handler,},

Normally the guest won't issue this instruction. But if it issues:
The original code prints and do nothing (for debug only).
Now, you inject #UD. I am not sure why we need this. Can you explain? We don't have INVD capability to remove from the guest.
Hi Eddie

If the guest issue the invd, the HV can't handle this case. IMHO,
unandled means we didn't support this instruction handle, we may
add this support later; undefined means we didn't define this,
so we can't support it.

For PTCM, we don't alloc HV or SOS to invd this PSRAM from cache.
So it requires: if there triggers INVD instruction, an #UD should
be raised.

Thx



[VMX_EXIT_REASON_INVLPG] = {
.handler = unhandled_vmexit_handler,},
[VMX_EXIT_REASON_RDPMC] = {
--
2.17.1





Eddie Dong
 

diff --git a/hypervisor/arch/x86/guest/vmexit.c
b/hypervisor/arch/x86/guest/vmexit.c
index dfab7ff4..7524d4f0 100644
--- a/hypervisor/arch/x86/guest/vmexit.c
+++ b/hypervisor/arch/x86/guest/vmexit.c
@@ -62,7 +62,7 @@ static const struct vm_exit_dispatch
dispatch_table[NR_VMX_EXIT_REASONS] = {
[VMX_EXIT_REASON_HLT] = {
.handler = hlt_vmexit_handler},
[VMX_EXIT_REASON_INVD] = {
- .handler = unhandled_vmexit_handler},
+ .handler = undefined_vmexit_handler,},

Normally the guest won't issue this instruction. But if it issues:
The original code prints and do nothing (for debug only).
Now, you inject #UD. I am not sure why we need this. Can you explain?
We don't have INVD capability to remove from the guest.
Hi Eddie

If the guest issue the invd, the HV can't handle this case. IMHO, unandled
IN any case, we cannot handle this gracefully.

Ideally, if a guest issue it unexpectedly, we can kill the guest -- it is safe but rude.
Print it and do nothing is a kind way to ask developers to debug it. This is acceptable to me.

means we didn't support this instruction handle, we may add this support
later; undefined means we didn't define this, so we can't support it.
The guest, theotically, can use the instruction. We don't have a way to remove the capability.


For PTCM, we don't alloc HV or SOS to invd this PSRAM from cache.
So it requires: if there triggers INVD instruction, an #UD should be raised.
I am not convinced why #UD is better. How will guest do then?


Li, Fei1
 

On Tue, Oct 27, 2020 at 09:56:36PM +0800, Dong, Eddie wrote:

diff --git a/hypervisor/arch/x86/guest/vmexit.c
b/hypervisor/arch/x86/guest/vmexit.c
index dfab7ff4..7524d4f0 100644
--- a/hypervisor/arch/x86/guest/vmexit.c
+++ b/hypervisor/arch/x86/guest/vmexit.c
@@ -62,7 +62,7 @@ static const struct vm_exit_dispatch
dispatch_table[NR_VMX_EXIT_REASONS] = {
[VMX_EXIT_REASON_HLT] = {
.handler = hlt_vmexit_handler},
[VMX_EXIT_REASON_INVD] = {
-.handler = unhandled_vmexit_handler},
+.handler = undefined_vmexit_handler,},

Normally the guest won't issue this instruction. But if it issues:
The original code prints and do nothing (for debug only).
Now, you inject #UD. I am not sure why we need this. Can you explain?
We don't have INVD capability to remove from the guest.
Hi Eddie

If the guest issue the invd, the HV can't handle this case. IMHO, unandled
IN any case, we cannot handle this gracefully.

Ideally, if a guest issue it unexpectedly, we can kill the guest -- it is safe but rude.
Print it and do nothing is a kind way to ask developers to debug it. This is acceptable to me.
Kill the guest could also ask the developers to debug it, just like AC lock. ^_^.

means we didn't support this instruction handle, we may add this support
later; undefined means we didn't define this, so we can't support it.
The guest, theotically, can use the instruction. We don't have a way to remove the capability.
If the guest know it's a guest, it should not use the instruction.


For PTCM, we don't alloc HV or SOS to invd this PSRAM from cache.
So it requires: if there triggers INVD instruction, an #UD should be raised.
I am not convinced why #UD is better. How will guest do then?
Could Qian explain this ? I only know it's a request comes from Qian/QA ?


Li, Fei1
 

On Tue, Oct 27, 2020 at 10:24:10PM +0800, Li, Fei1 wrote:
On Tue, Oct 27, 2020 at 09:56:36PM +0800, Dong, Eddie wrote:

diff --git a/hypervisor/arch/x86/guest/vmexit.c
b/hypervisor/arch/x86/guest/vmexit.c
index dfab7ff4..7524d4f0 100644
--- a/hypervisor/arch/x86/guest/vmexit.c
+++ b/hypervisor/arch/x86/guest/vmexit.c
@@ -62,7 +62,7 @@ static const struct vm_exit_dispatch
dispatch_table[NR_VMX_EXIT_REASONS] = {
[VMX_EXIT_REASON_HLT] = {
.handler = hlt_vmexit_handler},
[VMX_EXIT_REASON_INVD] = {
-.handler = unhandled_vmexit_handler},
+.handler = undefined_vmexit_handler,},

Normally the guest won't issue this instruction. But if it issues:
The original code prints and do nothing (for debug only).
Now, you inject #UD. I am not sure why we need this. Can you explain?
We don't have INVD capability to remove from the guest.
Hi Eddie

If the guest issue the invd, the HV can't handle this case. IMHO, unandled
IN any case, we cannot handle this gracefully.

Ideally, if a guest issue it unexpectedly, we can kill the guest -- it is safe but rude.
Print it and do nothing is a kind way to ask developers to debug it. This is acceptable to me.
Kill the guest could also ask the developers to debug it, just like AC lock. ^_^.

means we didn't support this instruction handle, we may add this support
later; undefined means we didn't define this, so we can't support it.
The guest, theotically, can use the instruction. We don't have a way to remove the capability.
If the guest know it's a guest, it should not use the instruction.


For PTCM, we don't alloc HV or SOS to invd this PSRAM from cache.
So it requires: if there triggers INVD instruction, an #UD should be raised.
I am not convinced why #UD is better. How will guest do then?
Could Qian explain this ? I only know it's a request comes from Qian/QA ?
Seems it comes from the PTCM HLD:
pSRAM Protection: INVD: Hide in cpuid, trap and trigger panic.

Will drop this patch since INVD can't hided by CPUID. And the current code could prevent pSRAM be flushed by INVD.