diff mbox

[2/2] vm_event: Add altp2m info to HVM events as well

Message ID 1450882432-10484-2-git-send-email-tamas@tklengyel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tamas K Lengyel Dec. 23, 2015, 2:53 p.m. UTC
Add altp2m information to HVM events as well when altp2m is active.

Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
---
 xen/arch/x86/hvm/event.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Razvan Cojocaru Dec. 23, 2015, 3:42 p.m. UTC | #1
On 12/23/2015 04:53 PM, Tamas K Lengyel wrote:
> Add altp2m information to HVM events as well when altp2m is active.
> 
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
>  xen/arch/x86/hvm/event.c | 7 +++++++
>  1 file changed, 7 insertions(+)

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


Thanks,
Razvan
Andrew Cooper Dec. 23, 2015, 5:18 p.m. UTC | #2
On 23/12/2015 15:42, Razvan Cojocaru wrote:
> On 12/23/2015 04:53 PM, Tamas K Lengyel wrote:
>> Add altp2m information to HVM events as well when altp2m is active.
>>
>> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
>> Cc: Keir Fraser <keir@xen.org>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
>> ---
>>  xen/arch/x86/hvm/event.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Jan Beulich Jan. 6, 2016, 11:32 a.m. UTC | #3
>>> On 23.12.15 at 15:53, <tamas@tklengyel.com> wrote:
> @@ -83,6 +84,12 @@ static int hvm_event_traps(uint8_t sync, vm_event_request_t *req)
>          vm_event_vcpu_pause(curr);
>      }
>  
> +    if ( altp2m_active(currd) )
> +    {
> +        req->flags |= VM_EVENT_FLAG_ALTERNATE_P2M;
> +        req->altp2m_idx = vcpu_altp2m(curr).p2midx;
> +    }

So far this info was provided just for MEM_ACCESS events. Now
you provide it for a few more ones, but wouldn't this then better
be supplied for all of them, namely also the other two MEM ones?

Jan
Tamas K Lengyel Jan. 6, 2016, 11:42 a.m. UTC | #4
On Wed, Jan 6, 2016 at 12:32 PM, Jan Beulich <JBeulich@suse.com> wrote:

> >>> On 23.12.15 at 15:53, <tamas@tklengyel.com> wrote:
> > @@ -83,6 +84,12 @@ static int hvm_event_traps(uint8_t sync,
> vm_event_request_t *req)
> >          vm_event_vcpu_pause(curr);
> >      }
> >
> > +    if ( altp2m_active(currd) )
> > +    {
> > +        req->flags |= VM_EVENT_FLAG_ALTERNATE_P2M;
> > +        req->altp2m_idx = vcpu_altp2m(curr).p2midx;
> > +    }
>
> So far this info was provided just for MEM_ACCESS events. Now
> you provide it for a few more ones, but wouldn't this then better
> be supplied for all of them, namely also the other two MEM ones?
>

AFAIK altp2m is currently incompatible with sharing. I'm not 100% sure but
I think it's also incompatible with paging.

Tamas
Andrew Cooper Jan. 6, 2016, 11:48 a.m. UTC | #5
On 06/01/16 11:42, Tamas K Lengyel wrote:
>
>
> On Wed, Jan 6, 2016 at 12:32 PM, Jan Beulich <JBeulich@suse.com
> <mailto:JBeulich@suse.com>> wrote:
>
>     >>> On 23.12.15 at 15:53, <tamas@tklengyel.com <mailto:tamas@tklengyel.com>> wrote:
>     > @@ -83,6 +84,12 @@ static int hvm_event_traps(uint8_t sync,
>     vm_event_request_t *req)
>     >          vm_event_vcpu_pause(curr);
>     >      }
>     >
>     > +    if ( altp2m_active(currd) )
>     > +    {
>     > +        req->flags |= VM_EVENT_FLAG_ALTERNATE_P2M;
>     > +        req->altp2m_idx = vcpu_altp2m(curr).p2midx;
>     > +    }
>
>     So far this info was provided just for MEM_ACCESS events. Now
>     you provide it for a few more ones, but wouldn't this then better
>     be supplied for all of them, namely also the other two MEM ones?
>
>
> AFAIK altp2m is currently incompatible with sharing. I'm not 100% sure
> but I think it's also incompatible with paging.

I don't think they are strictly incompatible; I don't see a technical
reason preventing some development work to make them function together.

Whether this happens or not is a very different matter.

~Andrew
Tamas K Lengyel Jan. 6, 2016, 11:50 a.m. UTC | #6
On Wed, Jan 6, 2016 at 12:48 PM, Andrew Cooper <andrew.cooper3@citrix.com>
wrote:

> On 06/01/16 11:42, Tamas K Lengyel wrote:
>
>
>
> On Wed, Jan 6, 2016 at 12:32 PM, Jan Beulich <JBeulich@suse.com> wrote:
>
>> >>> On 23.12.15 at 15:53, < <tamas@tklengyel.com>tamas@tklengyel.com>
>> wrote:
>> > @@ -83,6 +84,12 @@ static int hvm_event_traps(uint8_t sync,
>> vm_event_request_t *req)
>> >          vm_event_vcpu_pause(curr);
>> >      }
>> >
>> > +    if ( altp2m_active(currd) )
>> > +    {
>> > +        req->flags |= VM_EVENT_FLAG_ALTERNATE_P2M;
>> > +        req->altp2m_idx = vcpu_altp2m(curr).p2midx;
>> > +    }
>>
>> So far this info was provided just for MEM_ACCESS events. Now
>> you provide it for a few more ones, but wouldn't this then better
>> be supplied for all of them, namely also the other two MEM ones?
>>
>
> AFAIK altp2m is currently incompatible with sharing. I'm not 100% sure but
> I think it's also incompatible with paging.
>
>
> I don't think they are strictly incompatible; I don't see a technical
> reason preventing some development work to make them function together.
>
> Whether this happens or not is a very different matter.
>

Sure, the two systems can be made to work in tandem, this work just hasn't
been done yet. I would very much like to get that to work in the future.

Tamas
Jan Beulich Jan. 12, 2016, 10:21 a.m. UTC | #7
>>> On 06.01.16 at 12:50, <tamas@tklengyel.com> wrote:
> On Wed, Jan 6, 2016 at 12:48 PM, Andrew Cooper <andrew.cooper3@citrix.com>
> wrote:
> 
>> On 06/01/16 11:42, Tamas K Lengyel wrote:
>>
>>
>>
>> On Wed, Jan 6, 2016 at 12:32 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>
>>> >>> On 23.12.15 at 15:53, < <tamas@tklengyel.com>tamas@tklengyel.com>
>>> wrote:
>>> > @@ -83,6 +84,12 @@ static int hvm_event_traps(uint8_t sync,
>>> vm_event_request_t *req)
>>> >          vm_event_vcpu_pause(curr);
>>> >      }
>>> >
>>> > +    if ( altp2m_active(currd) )
>>> > +    {
>>> > +        req->flags |= VM_EVENT_FLAG_ALTERNATE_P2M;
>>> > +        req->altp2m_idx = vcpu_altp2m(curr).p2midx;
>>> > +    }
>>>
>>> So far this info was provided just for MEM_ACCESS events. Now
>>> you provide it for a few more ones, but wouldn't this then better
>>> be supplied for all of them, namely also the other two MEM ones?
>>>
>>
>> AFAIK altp2m is currently incompatible with sharing. I'm not 100% sure but
>> I think it's also incompatible with paging.
>>
>>
>> I don't think they are strictly incompatible; I don't see a technical
>> reason preventing some development work to make them function together.
>>
>> Whether this happens or not is a very different matter.
> 
> Sure, the two systems can be made to work in tandem, this work just hasn't
> been done yet. I would very much like to get that to work in the future.

Which re-raises the question: Shouldn't the information then be
made available uniformly for all events?

Jan
Tamas K Lengyel Jan. 12, 2016, 12:13 p.m. UTC | #8
On Jan 12, 2016 3:21 AM, "Jan Beulich" <JBeulich@suse.com> wrote:
>
> >>> On 06.01.16 at 12:50, <tamas@tklengyel.com> wrote:
> > On Wed, Jan 6, 2016 at 12:48 PM, Andrew Cooper <
andrew.cooper3@citrix.com>
> > wrote:
> >
> >> On 06/01/16 11:42, Tamas K Lengyel wrote:
> >>
> >>
> >>
> >> On Wed, Jan 6, 2016 at 12:32 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >>
> >>> >>> On 23.12.15 at 15:53, < <tamas@tklengyel.com>tamas@tklengyel.com>
> >>> wrote:
> >>> > @@ -83,6 +84,12 @@ static int hvm_event_traps(uint8_t sync,
> >>> vm_event_request_t *req)
> >>> >          vm_event_vcpu_pause(curr);
> >>> >      }
> >>> >
> >>> > +    if ( altp2m_active(currd) )
> >>> > +    {
> >>> > +        req->flags |= VM_EVENT_FLAG_ALTERNATE_P2M;
> >>> > +        req->altp2m_idx = vcpu_altp2m(curr).p2midx;
> >>> > +    }
> >>>
> >>> So far this info was provided just for MEM_ACCESS events. Now
> >>> you provide it for a few more ones, but wouldn't this then better
> >>> be supplied for all of them, namely also the other two MEM ones?
> >>>
> >>
> >> AFAIK altp2m is currently incompatible with sharing. I'm not 100% sure
but
> >> I think it's also incompatible with paging.
> >>
> >>
> >> I don't think they are strictly incompatible; I don't see a technical
> >> reason preventing some development work to make them function together.
> >>
> >> Whether this happens or not is a very different matter.
> >
> > Sure, the two systems can be made to work in tandem, this work just
hasn't
> > been done yet. I would very much like to get that to work in the future.
>
> Which re-raises the question: Shouldn't the information then be
> made available uniformly for all events?
>

IMHO there is no point doing so while the systems don't work together.

Tamas
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
index 73c8f14..a3d4892 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/event.c
@@ -22,6 +22,7 @@ 
 #include <xen/paging.h>
 #include <asm/hvm/event.h>
 #include <asm/monitor.h>
+#include <asm/altp2m.h>
 #include <public/vm_event.h>
 
 static void hvm_event_fill_regs(vm_event_request_t *req)
@@ -83,6 +84,12 @@  static int hvm_event_traps(uint8_t sync, vm_event_request_t *req)
         vm_event_vcpu_pause(curr);
     }
 
+    if ( altp2m_active(currd) )
+    {
+        req->flags |= VM_EVENT_FLAG_ALTERNATE_P2M;
+        req->altp2m_idx = vcpu_altp2m(curr).p2midx;
+    }
+
     hvm_event_fill_regs(req);
     vm_event_put_request(currd, &currd->vm_event->monitor, req);