[PATCH v1 0/2] hvm/svm: Enable vm events for SVM

[PATCH v1 0/2] hvm/svm: Enable vm events for SVM

[Alexandru Isaila]

Hi all,

This series more capabilities to the common list. I've tested and added 
CPUID and EVENT INTERRUPT to the list.

Cheers,

Alexandru Isaila

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v1 1/2] asm-x86/monitor: Add MONITOR_EVENT_INTERRUPT to common capabilities

[Alexandru Isaila]

Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
---
 xen/include/asm-x86/monitor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 59a2610..7a9e1e8 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -80,13 +80,13 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
     capabilities = ((1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
                     (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
                     (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
+                    (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
                     (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG));
 
     if ( cpu_has_vmx )
     {
         capabilities |= ((1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
                          (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
-                         (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
                          (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED));
 
         /* Since we know this is on VMX, we can just call the hvm func */
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v1 2/2] hvm/svm: Implement CPUID events

[Alexandru Isaila]

At this moment the CPUID events for the AMD architecture are not
forwarded to the monitor layer.

This patch adds the CPUID event to the common capabilities and then
forwards the event to the monitor layer.

Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
---
 xen/arch/x86/hvm/svm/svm.c    | 8 +++++++-
 xen/include/asm-x86/monitor.h | 2 +-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index e36ad05..0f1c57d 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1804,6 +1804,7 @@ static void svm_vmexit_do_cpuid(struct cpu_user_regs *regs)
     struct vcpu *curr = current;
     unsigned int inst_len;
     struct cpuid_leaf res;
+    int rc = 0;
 
     if ( (inst_len = __get_instruction_length(curr, INSTR_CPUID)) == 0 )
         return;
@@ -1822,7 +1823,12 @@ static void svm_vmexit_do_cpuid(struct cpu_user_regs *regs)
     regs->rcx = res.c;
     regs->rdx = res.d;
 
-    __update_guest_eip(regs, inst_len);
+    rc = hvm_monitor_cpuid(inst_len, regs->eax, regs->ecx);
+
+    if ( !rc )
+    {
+        __update_guest_eip(regs, inst_len);
+    }
 }
 
 static void svm_vmexit_do_cr_access(
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 7a9e1e8..99ed4b87 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -81,12 +81,12 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
                     (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
                     (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
                     (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
+                    (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
                     (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG));
 
     if ( cpu_has_vmx )
     {
         capabilities |= ((1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
-                         (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
                          (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED));
 
         /* Since we know this is on VMX, we can just call the hvm func */
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v1 1/2] asm-x86/monitor: Add MONITOR_EVENT_INTERRUPT to common capabilities

[Razvan Cojocaru]

On 02/19/2018 03:07 PM, Alexandru Isaila wrote:
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
> ---
>  xen/include/asm-x86/monitor.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index 59a2610..7a9e1e8 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -80,13 +80,13 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>      capabilities = ((1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
>                      (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
>                      (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
> +                    (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
>                      (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG));
>  
>      if ( cpu_has_vmx )
>      {
>          capabilities |= ((1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
>                           (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
> -                         (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
>                           (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED));
>  
>          /* Since we know this is on VMX, we can just call the hvm func */
> 

Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v1 2/2] hvm/svm: Implement CPUID events

[Tamas K Lengyel]

On Mon, Feb 19, 2018 at 6:07 AM, Alexandru Isaila
<aisaila@bitdefender.com> wrote:
> At this moment the CPUID events for the AMD architecture are not
> forwarded to the monitor layer.
>
> This patch adds the CPUID event to the common capabilities and then
> forwards the event to the monitor layer.
>
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
> ---
>  xen/arch/x86/hvm/svm/svm.c    | 8 +++++++-
>  xen/include/asm-x86/monitor.h | 2 +-
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index e36ad05..0f1c57d 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1804,6 +1804,7 @@ static void svm_vmexit_do_cpuid(struct cpu_user_regs *regs)
>      struct vcpu *curr = current;
>      unsigned int inst_len;
>      struct cpuid_leaf res;
> +    int rc = 0;
>
>      if ( (inst_len = __get_instruction_length(curr, INSTR_CPUID)) == 0 )
>          return;
> @@ -1822,7 +1823,12 @@ static void svm_vmexit_do_cpuid(struct cpu_user_regs *regs)
>      regs->rcx = res.c;
>      regs->rdx = res.d;
>
> -    __update_guest_eip(regs, inst_len);
> +    rc = hvm_monitor_cpuid(inst_len, regs->eax, regs->ecx);
> +
> +    if ( !rc )

Missing the rc < 0 case handling.

> +    {
> +        __update_guest_eip(regs, inst_len);
> +    }
>  }
>
>  static void svm_vmexit_do_cr_access(
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index 7a9e1e8..99ed4b87 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -81,12 +81,12 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>                      (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
>                      (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
>                      (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
> +                    (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
>                      (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG));
>
>      if ( cpu_has_vmx )
>      {
>          capabilities |= ((1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
> -                         (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
>                           (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED));
>
>          /* Since we know this is on VMX, we can just call the hvm func */
> --
> 2.7.4

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v1 2/2] hvm/svm: Implement CPUID events

[Razvan Cojocaru]

On 02/19/2018 05:25 PM, Tamas K Lengyel wrote:
> On Mon, Feb 19, 2018 at 6:07 AM, Alexandru Isaila
> <aisaila@bitdefender.com> wrote:
>> At this moment the CPUID events for the AMD architecture are not
>> forwarded to the monitor layer.
>>
>> This patch adds the CPUID event to the common capabilities and then
>> forwards the event to the monitor layer.
>>
>> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>> ---
>>  xen/arch/x86/hvm/svm/svm.c    | 8 +++++++-
>>  xen/include/asm-x86/monitor.h | 2 +-
>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
>> index e36ad05..0f1c57d 100644
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -1804,6 +1804,7 @@ static void svm_vmexit_do_cpuid(struct cpu_user_regs *regs)
>>      struct vcpu *curr = current;
>>      unsigned int inst_len;
>>      struct cpuid_leaf res;
>> +    int rc = 0;
>>
>>      if ( (inst_len = __get_instruction_length(curr, INSTR_CPUID)) == 0 )
>>          return;
>> @@ -1822,7 +1823,12 @@ static void svm_vmexit_do_cpuid(struct cpu_user_regs *regs)
>>      regs->rcx = res.c;
>>      regs->rdx = res.d;
>>
>> -    __update_guest_eip(regs, inst_len);
>> +    rc = hvm_monitor_cpuid(inst_len, regs->eax, regs->ecx);
>> +
>> +    if ( !rc )
> 
> Missing the rc < 0 case handling.
> 
>> +    {
>> +        __update_guest_eip(regs, inst_len);
>> +    }

Please also lose the unnecessary scope here.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v1 2/2] hvm/svm: Implement CPUID events

[Jan Beulich]

>>> On 19.02.18 at 16:25, <tamas@tklengyel.com> wrote:
> On Mon, Feb 19, 2018 at 6:07 AM, Alexandru Isaila <aisaila@bitdefender.com> wrote:
>> @@ -1822,7 +1823,12 @@ static void svm_vmexit_do_cpuid(struct cpu_user_regs *regs)
>>      regs->rcx = res.c;
>>      regs->rdx = res.d;
>>
>> -    __update_guest_eip(regs, inst_len);
>> +    rc = hvm_monitor_cpuid(inst_len, regs->eax, regs->ecx);
>> +
>> +    if ( !rc )
> 
> Missing the rc < 0 case handling.
> 
>> +    {
>> +        __update_guest_eip(regs, inst_len);
>> +    }

Also please omit the braces in cases like this one.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v1 2/2] hvm/svm: Implement CPUID events

[Alexandru Stefan ISAILA]

On Lu, 2018-02-19 at 08:25 -0700, Tamas K Lengyel wrote:
> On Mon, Feb 19, 2018 at 6:07 AM, Alexandru Isaila
> <aisaila@bitdefender.com> wrote:
> >
> > At this moment the CPUID events for the AMD architecture are not
> > forwarded to the monitor layer.
> >
> > This patch adds the CPUID event to the common capabilities and then
> > forwards the event to the monitor layer.
> >
> > Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
> > ---
> >  xen/arch/x86/hvm/svm/svm.c    | 8 +++++++-
> >  xen/include/asm-x86/monitor.h | 2 +-
> >  2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/x86/hvm/svm/svm.c
> > b/xen/arch/x86/hvm/svm/svm.c
> > index e36ad05..0f1c57d 100644
> > --- a/xen/arch/x86/hvm/svm/svm.c
> > +++ b/xen/arch/x86/hvm/svm/svm.c
> > @@ -1804,6 +1804,7 @@ static void svm_vmexit_do_cpuid(struct
> > cpu_user_regs *regs)
> >      struct vcpu *curr = current;
> >      unsigned int inst_len;
> >      struct cpuid_leaf res;
> > +    int rc = 0;
> >
> >      if ( (inst_len = __get_instruction_length(curr, INSTR_CPUID))
> > == 0 )
> >          return;
> > @@ -1822,7 +1823,12 @@ static void svm_vmexit_do_cpuid(struct
> > cpu_user_regs *regs)
> >      regs->rcx = res.c;
> >      regs->rdx = res.d;
> >
> > -    __update_guest_eip(regs, inst_len);
> > +    rc = hvm_monitor_cpuid(inst_len, regs->eax, regs->ecx);
> > +
> > +    if ( !rc )
> Missing the rc < 0 case handling.
Hi Tamas,

I think we can resolve this in 2 ways:
1. I do it like on the vmx side and take the __update_guest_eip out of
the function, return the result from hvm_monitor_cpuid and handle the
result in the case statement.
2. I can handle the !rc in the function and the rc < 0 in the case
statement.

What do you prefer so we can skip version 3 of the patch?

~Alex

________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v1 2/2] hvm/svm: Implement CPUID events

[Tamas K Lengyel]

On Tue, Feb 20, 2018 at 3:37 AM, Alexandru Stefan ISAILA
<aisaila@bitdefender.com> wrote:
> On Lu, 2018-02-19 at 08:25 -0700, Tamas K Lengyel wrote:
>> On Mon, Feb 19, 2018 at 6:07 AM, Alexandru Isaila
>> <aisaila@bitdefender.com> wrote:
>> >
>> > At this moment the CPUID events for the AMD architecture are not
>> > forwarded to the monitor layer.
>> >
>> > This patch adds the CPUID event to the common capabilities and then
>> > forwards the event to the monitor layer.
>> >
>> > Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>> > ---
>> >  xen/arch/x86/hvm/svm/svm.c    | 8 +++++++-
>> >  xen/include/asm-x86/monitor.h | 2 +-
>> >  2 files changed, 8 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/xen/arch/x86/hvm/svm/svm.c
>> > b/xen/arch/x86/hvm/svm/svm.c
>> > index e36ad05..0f1c57d 100644
>> > --- a/xen/arch/x86/hvm/svm/svm.c
>> > +++ b/xen/arch/x86/hvm/svm/svm.c
>> > @@ -1804,6 +1804,7 @@ static void svm_vmexit_do_cpuid(struct
>> > cpu_user_regs *regs)
>> >      struct vcpu *curr = current;
>> >      unsigned int inst_len;
>> >      struct cpuid_leaf res;
>> > +    int rc = 0;
>> >
>> >      if ( (inst_len = __get_instruction_length(curr, INSTR_CPUID))
>> > == 0 )
>> >          return;
>> > @@ -1822,7 +1823,12 @@ static void svm_vmexit_do_cpuid(struct
>> > cpu_user_regs *regs)
>> >      regs->rcx = res.c;
>> >      regs->rdx = res.d;
>> >
>> > -    __update_guest_eip(regs, inst_len);
>> > +    rc = hvm_monitor_cpuid(inst_len, regs->eax, regs->ecx);
>> > +
>> > +    if ( !rc )
>> Missing the rc < 0 case handling.
> Hi Tamas,
>
> I think we can resolve this in 2 ways:
> 1. I do it like on the vmx side and take the __update_guest_eip out of
> the function, return the result from hvm_monitor_cpuid and handle the
> result in the case statement.

+1, I think it helps if the flow is the same across vmx and svm as
much as possible.

Thanks,
Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel