Re: [Xen-devel] [PATCH V5] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Razvan Cojocaru]

On 12/13/2016 03:18 PM, Jan Beulich wrote:
 On 13.12.16 at 09:50,  wrote:
>> On 11/17/2016 07:11 AM, Tian, Kevin wrote:
 From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
 Sent: Wednesday, November 16, 2016 3:49 PM

 On 11/16/2016 09:22 AM, Tian, Kevin wrote:
> Looks not working with APICv virtual interrupt delivery...

 It's only meant to work with "regular" injections (we'd like to be able
 to tell if xc_hvm_inject_trap() can succeed). APICv support could be a
 later patch, if desirable (AFAICT, the two types shouldn't collide, i.e.
 impede trap delivery as done by the xc_hvm_inject_trap() hypercall).


>>>
>>> OK then. Acked-by: Kevin Tian 
>>
>> AFAIK this has been acked by everyone a while back, is further action
>> required on my part, or is just low in the patch queue?
> 
> Before asking, did you check the patch still applies and builds?
> I've taken the time to make it apply, but it really needs more
> than that. Please re-base.

While re-basing the patch on staging, I've got this (unrelated) error
while doing 'make dist':

make[9]: Entering directory `~/work/xen.git/tools/firmware/rombios/32bit'
gcc   -m32 -march=i686 -DBUILD_ID -g -fno-strict-aliasing -std=gnu99
-Wall -Wstrict-prototypes -Wdeclaration-after-statement
-Wno-unused-but-set-variable -Wno-unused-local-typedefs   -O0 -g3
-fno-omit-frame-pointer
-D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MF
.util.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE
-mno-tls-direct-seg-refs  -Werror -fno-stack-protector -fno-exceptions
-fno-builtin -msoft-float
-I/home/red/work/xen.git/tools/firmware/rombios/32bit/../../../../tools/include
-I.. -I../../../libacpi  -c -o util.o util.c
util.c: In function ‘get_s3_waking_vector’:
util.c:453:23: error: dereferencing pointer to incomplete type
 if (!fadt || (fadt->header.signature != ACPI_2_0_FADT_SIGNATURE))
   ^
util.c:453:45: error: ‘ACPI_2_0_FADT_SIGNATURE’ undeclared (first use in
this function)
 if (!fadt || (fadt->header.signature != ACPI_2_0_FADT_SIGNATURE))
 ^
util.c:453:45: note: each undeclared identifier is reported only once
for each function it appears in
util.c:456:45: error: dereferencing pointer to incomplete type
 facs = (struct acpi_20_facs *)(long)fadt->x_firmware_ctrl;
 ^
make[9]: *** [util.o] Error 1

I'm not where this is best reported, but thought someone might like to know.


Thanks,
Razvan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V5] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Razvan Cojocaru]

On 12/13/2016 03:18 PM, Jan Beulich wrote:
 On 13.12.16 at 09:50,  wrote:
>> On 11/17/2016 07:11 AM, Tian, Kevin wrote:
 From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
 Sent: Wednesday, November 16, 2016 3:49 PM

 On 11/16/2016 09:22 AM, Tian, Kevin wrote:
> Looks not working with APICv virtual interrupt delivery...

 It's only meant to work with "regular" injections (we'd like to be able
 to tell if xc_hvm_inject_trap() can succeed). APICv support could be a
 later patch, if desirable (AFAICT, the two types shouldn't collide, i.e.
 impede trap delivery as done by the xc_hvm_inject_trap() hypercall).


>>>
>>> OK then. Acked-by: Kevin Tian 
>>
>> AFAIK this has been acked by everyone a while back, is further action
>> required on my part, or is just low in the patch queue?
> 
> Before asking, did you check the patch still applies and builds?
> I've taken the time to make it apply, but it really needs more
> than that. Please re-base.

I did not. Will check ASAP.


Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V5] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Jan Beulich]

>>> On 13.12.16 at 09:50,  wrote:
> On 11/17/2016 07:11 AM, Tian, Kevin wrote:
>>> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
>>> Sent: Wednesday, November 16, 2016 3:49 PM
>>>
>>> On 11/16/2016 09:22 AM, Tian, Kevin wrote:
 Looks not working with APICv virtual interrupt delivery...
>>>
>>> It's only meant to work with "regular" injections (we'd like to be able
>>> to tell if xc_hvm_inject_trap() can succeed). APICv support could be a
>>> later patch, if desirable (AFAICT, the two types shouldn't collide, i.e.
>>> impede trap delivery as done by the xc_hvm_inject_trap() hypercall).
>>>
>>>
>> 
>> OK then. Acked-by: Kevin Tian 
> 
> AFAIK this has been acked by everyone a while back, is further action
> required on my part, or is just low in the patch queue?

Before asking, did you check the patch still applies and builds?
I've taken the time to make it apply, but it really needs more
than that. Please re-base.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V5] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Julien Grall]

Hi Razvan,

On 13/12/16 09:14, Razvan Cojocaru wrote:

On 12/13/2016 11:09 AM, Jan Beulich wrote:

On 13.12.16 at 09:50,  wrote:

On 11/17/2016 07:11 AM, Tian, Kevin wrote:

From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
Sent: Wednesday, November 16, 2016 3:49 PM

On 11/16/2016 09:22 AM, Tian, Kevin wrote:

Looks not working with APICv virtual interrupt delivery...


It's only meant to work with "regular" injections (we'd like to be able
to tell if xc_hvm_inject_trap() can succeed). APICv support could be a
later patch, if desirable (AFAICT, the two types shouldn't collide, i.e.
impede trap delivery as done by the xc_hvm_inject_trap() hypercall).




OK then. Acked-by: Kevin Tian 


AFAIK this has been acked by everyone a while back,


Has it? I've meant to apply it the other day, but couldn't find
any ARM or AMD SVM maintainer acks.


Oh I see. At one point Julien came into the conversation but it was just
a friendly discussion about spelling somewhere - I've misremembered that
as an ack, I now see.


Thank you for reminding me:

Acked-by: Julien Grall 

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V5] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Suravee Suthikulpanit]

On 12/13/2016 04:09 PM, Jan Beulich wrote:

On 13.12.16 at 09:50,  wrote:

On 11/17/2016 07:11 AM, Tian, Kevin wrote:

From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
Sent: Wednesday, November 16, 2016 3:49 PM

On 11/16/2016 09:22 AM, Tian, Kevin wrote:

Looks not working with APICv virtual interrupt delivery...


It's only meant to work with "regular" injections (we'd like to be able
to tell if xc_hvm_inject_trap() can succeed). APICv support could be a
later patch, if desirable (AFAICT, the two types shouldn't collide, i.e.
impede trap delivery as done by the xc_hvm_inject_trap() hypercall).




OK then. Acked-by: Kevin Tian 


AFAIK this has been acked by everyone a while back,


Has it? I've meant to apply it the other day, but couldn't find
any ARM or AMD SVM maintainer acks.

Jan



Acked-by: Suravee Suthikulpanit 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V5] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Razvan Cojocaru]

On 12/13/2016 11:09 AM, Jan Beulich wrote:
 On 13.12.16 at 09:50,  wrote:
>> On 11/17/2016 07:11 AM, Tian, Kevin wrote:
 From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
 Sent: Wednesday, November 16, 2016 3:49 PM

 On 11/16/2016 09:22 AM, Tian, Kevin wrote:
> Looks not working with APICv virtual interrupt delivery...

 It's only meant to work with "regular" injections (we'd like to be able
 to tell if xc_hvm_inject_trap() can succeed). APICv support could be a
 later patch, if desirable (AFAICT, the two types shouldn't collide, i.e.
 impede trap delivery as done by the xc_hvm_inject_trap() hypercall).


>>>
>>> OK then. Acked-by: Kevin Tian 
>>
>> AFAIK this has been acked by everyone a while back,
> 
> Has it? I've meant to apply it the other day, but couldn't find
> any ARM or AMD SVM maintainer acks.

Oh I see. At one point Julien came into the conversation but it was just
a friendly discussion about spelling somewhere - I've misremembered that
as an ack, I now see.


Thanks,
Razvan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V5] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Jan Beulich]

>>> On 13.12.16 at 09:50,  wrote:
> On 11/17/2016 07:11 AM, Tian, Kevin wrote:
>>> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
>>> Sent: Wednesday, November 16, 2016 3:49 PM
>>>
>>> On 11/16/2016 09:22 AM, Tian, Kevin wrote:
 Looks not working with APICv virtual interrupt delivery...
>>>
>>> It's only meant to work with "regular" injections (we'd like to be able
>>> to tell if xc_hvm_inject_trap() can succeed). APICv support could be a
>>> later patch, if desirable (AFAICT, the two types shouldn't collide, i.e.
>>> impede trap delivery as done by the xc_hvm_inject_trap() hypercall).
>>>
>>>
>> 
>> OK then. Acked-by: Kevin Tian 
> 
> AFAIK this has been acked by everyone a while back,

Has it? I've meant to apply it the other day, but couldn't find
any ARM or AMD SVM maintainer acks.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V5] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Razvan Cojocaru]

On 11/17/2016 07:11 AM, Tian, Kevin wrote:
>> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
>> Sent: Wednesday, November 16, 2016 3:49 PM
>>
>> On 11/16/2016 09:22 AM, Tian, Kevin wrote:
>>> Looks not working with APICv virtual interrupt delivery...
>>
>> It's only meant to work with "regular" injections (we'd like to be able
>> to tell if xc_hvm_inject_trap() can succeed). APICv support could be a
>> later patch, if desirable (AFAICT, the two types shouldn't collide, i.e.
>> impede trap delivery as done by the xc_hvm_inject_trap() hypercall).
>>
>>
> 
> OK then. Acked-by: Kevin Tian 

AFAIK this has been acked by everyone a while back, is further action
required on my part, or is just low in the patch queue?


Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V5] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Tian, Kevin]

> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
> Sent: Wednesday, November 16, 2016 3:49 PM
> 
> On 11/16/2016 09:22 AM, Tian, Kevin wrote:
> > Looks not working with APICv virtual interrupt delivery...
> 
> It's only meant to work with "regular" injections (we'd like to be able
> to tell if xc_hvm_inject_trap() can succeed). APICv support could be a
> later patch, if desirable (AFAICT, the two types shouldn't collide, i.e.
> impede trap delivery as done by the xc_hvm_inject_trap() hypercall).
> 
> 

OK then. Acked-by: Kevin Tian 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V5] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Razvan Cojocaru]

On 11/16/2016 09:22 AM, Tian, Kevin wrote:
> Looks not working with APICv virtual interrupt delivery...

It's only meant to work with "regular" injections (we'd like to be able
to tell if xc_hvm_inject_trap() can succeed). APICv support could be a
later patch, if desirable (AFAICT, the two types shouldn't collide, i.e.
impede trap delivery as done by the xc_hvm_inject_trap() hypercall).


Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V5] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Tian, Kevin]

> From: Razvan Cojocaru [mailto:rcojoc...@bitdefender.com]
> Sent: Saturday, November 12, 2016 12:09 AM
> 
> Added support for a new event type, VM_EVENT_REASON_INTERRUPT,
> which is now fired in a one-shot manner when enabled via the new
> VM_EVENT_FLAG_GET_NEXT_INTERRUPT vm_event response flag.
> The patch also fixes the behaviour of the xc_hvm_inject_trap()
> hypercall, which would lead to non-architectural interrupts
> overwriting pending (specifically reinjected) architectural ones.
> 
> Signed-off-by: Razvan Cojocaru 
> Acked-by: Tamas K Lengyel 
> 
> ---
> Changes since V4:
>  - Fixed typo in vm_event_monitor_next_interrupt()
>( v->arch.monitor.next_interrupt_enabled should have been set
>to true, not false ).
> 
> Changes in V4 since V3:
>  - Const-ified struct vmcb_struct *vmcb in svm_get_pending_event().
>  - Made next_interrupt_enabled into a bool.
> ---
>  xen/arch/x86/hvm/hvm.c| 22 +-
>  xen/arch/x86/hvm/monitor.c| 14 ++
>  xen/arch/x86/hvm/svm/svm.c| 15 +++
>  xen/arch/x86/hvm/vmx/vmx.c| 20 
>  xen/arch/x86/vm_event.c   |  5 +
>  xen/common/vm_event.c |  3 +++
>  xen/include/asm-arm/vm_event.h|  6 ++
>  xen/include/asm-x86/domain.h  |  4 
>  xen/include/asm-x86/hvm/hvm.h |  1 +
>  xen/include/asm-x86/hvm/monitor.h |  2 ++
>  xen/include/asm-x86/monitor.h |  3 ++-
>  xen/include/public/domctl.h   |  1 +
>  xen/include/public/vm_event.h | 18 ++
>  xen/include/xen/vm_event.h|  2 ++
>  14 files changed, 114 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 704fd64..93af5b8 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -469,6 +469,12 @@ void hvm_migrate_pirqs(struct vcpu *v)
>  spin_unlock(>event_lock);
>  }
> 
> +static bool hvm_get_pending_event(struct vcpu *v, struct hvm_trap *info)
> +{
> +info->cr2 = v->arch.hvm_vcpu.guest_cr[2];
> +return hvm_funcs.get_pending_event(v, info);
> +}
> +
>  void hvm_do_resume(struct vcpu *v)
>  {
>  check_wakeup_from_wait();
> @@ -535,9 +541,23 @@ void hvm_do_resume(struct vcpu *v)
>  /* Inject pending hw/sw trap */
>  if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
>  {
> -hvm_inject_trap(>arch.hvm_vcpu.inject_trap);
> +if ( !hvm_event_pending(v) )
> +hvm_inject_trap(>arch.hvm_vcpu.inject_trap);
> +
>  v->arch.hvm_vcpu.inject_trap.vector = -1;
>  }
> +
> +if ( unlikely(v->arch.vm_event) && 
> v->arch.monitor.next_interrupt_enabled )
> +{
> +struct hvm_trap info;
> +
> +if ( hvm_get_pending_event(v, ) )
> +{
> +hvm_monitor_interrupt(info.vector, info.type, info.error_code,
> +  info.cr2);
> +v->arch.monitor.next_interrupt_enabled = false;
> +}
> +}
>  }
> 
>  static int hvm_print_line(
> diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
> index 401a8c6..69a88ad 100644
> --- a/xen/arch/x86/hvm/monitor.c
> +++ b/xen/arch/x86/hvm/monitor.c
> @@ -150,6 +150,20 @@ int hvm_monitor_cpuid(unsigned long insn_length, 
> unsigned int
> leaf,
>  return monitor_traps(curr, 1, );
>  }
> 
> +void hvm_monitor_interrupt(unsigned int vector, unsigned int type,
> +   unsigned int err, uint64_t cr2)
> +{
> +vm_event_request_t req = {
> +.reason = VM_EVENT_REASON_INTERRUPT,
> +.u.interrupt.x86.vector = vector,
> +.u.interrupt.x86.type = type,
> +.u.interrupt.x86.error_code = err,
> +.u.interrupt.x86.cr2 = cr2,
> +};
> +
> +monitor_traps(current, 1, );
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index c9dbbea..7fe27e4 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -,6 +,20 @@ static void svm_invlpg(struct vcpu *v, unsigned long 
> vaddr)
>  svm_asid_g_invlpg(v, vaddr);
>  }
> 
> +static bool svm_get_pending_event(struct vcpu *v, struct hvm_trap *info)
> +{
> +const struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
> +
> +if ( vmcb->eventinj.fields.v )
> +return false;
> +
> +info->vector = vmcb->eventinj.fields.vector;
> +info->type = vmcb->eventinj.fields.type;
> +info->error_code = vmcb->eventinj.fields.errorcode;
> +
> +return true;
> +}
> +
>  static struct hvm_function_table __initdata svm_function_table = {
>  .name = "SVM",
>  .cpu_up_prepare   = svm_cpu_up_prepare,
> @@ -2252,6 +2266,7 @@ static struct hvm_function_table __initdata 
> svm_function_table
> = {
>  .inject_trap  = svm_inject_trap,
>  .init_hypercall_page  = svm_init_hypercall_page,
>  .event_pending= svm_event_pending,
> +   

Re: [Xen-devel] [PATCH V5] x86/vm_event: Added support for VM_EVENT_REASON_INTERRUPT

[Jan Beulich]

>>> On 11.11.16 at 17:09,  wrote:
> Added support for a new event type, VM_EVENT_REASON_INTERRUPT,
> which is now fired in a one-shot manner when enabled via the new
> VM_EVENT_FLAG_GET_NEXT_INTERRUPT vm_event response flag.
> The patch also fixes the behaviour of the xc_hvm_inject_trap()
> hypercall, which would lead to non-architectural interrupts
> overwriting pending (specifically reinjected) architectural ones.
> 
> Signed-off-by: Razvan Cojocaru 
> Acked-by: Tamas K Lengyel 

Acked-by: Jan Beulich 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel