Requesting a freeze exception for vm_event memory introspection helpers

Requesting a freeze exception for vm_event memory introspection helpers

[Razvan Cojocaru]

Hello,

I'd like to ask for a freeze exception for the "vm_event memory
introspection helpers" series.

[PATCH 1/3] xen/mem_access: Support for memory-content hiding
[PATCH 2/3] xen/vm_event: Support for guest-requested events
[PATCH 3/3] xen/vm_event: Deny register writes if refused by
vm_event reply

All patches have been acked by at least one person (though patch 1 is
still under some discussion).

1. Benefits of the series making it in this release:

* Probably the most important benefit is that 4.6 development has been
very open to refactoring vm_events, and patch 3/3 makes vm_events behave
in a consistent manner (all register-write vm_events are pre-write events).

* There are 3rd parties interested in these features (Tamas, for
example, has already expressed interest in uses of patch 1/1).

2. Risks of including the series:

* Since two of the three patches have already received acks from 3+
people, I would assume that the risks for those are minimal. As for the
first patch, unless a vm_event consumer uses the new
VM_EVENT_FLAG_SET_EMUL_READ_DATA vm_event response flag in conjunction
with VM_EVENT_FLAG_EMULATE, the impact should be close to 0 (only a few
"if ( unlikely(set_context_enabled) )" extra statements).

A new series will follow as soon as possible, addressing Jan's comments
on the first patch and cleaning up patch 3 a little more.


Thank you in advance for considering this,
Razvan

Re: Requesting a freeze exception for vm_event memory introspection helpers

[Wei Liu]

On Mon, Jul 13, 2015 at 10:35:44AM +0300, Razvan Cojocaru wrote:
> Hello,
> 
> I'd like to ask for a freeze exception for the "vm_event memory
> introspection helpers" series.
> 
> [PATCH 1/3] xen/mem_access: Support for memory-content hiding
> [PATCH 2/3] xen/vm_event: Support for guest-requested events
> [PATCH 3/3] xen/vm_event: Deny register writes if refused by
> vm_event reply
> 
> All patches have been acked by at least one person (though patch 1 is
> still under some discussion).
> 
> 1. Benefits of the series making it in this release:
> 
> * Probably the most important benefit is that 4.6 development has been
> very open to refactoring vm_events, and patch 3/3 makes vm_events behave
> in a consistent manner (all register-write vm_events are pre-write events).
> 
> * There are 3rd parties interested in these features (Tamas, for
> example, has already expressed interest in uses of patch 1/1).
> 

These aren't really good arguments for getting this in for 4.6. It would
be easy for you and Tamas to carry three patches for a while.

What are the impact to end users (end users -- meaning either system
administrator that sets up Xen and other developers who want to use the
new interface you introduce)?  Does this series consist of the last
missing pieces of a feature?

Wei.

> 2. Risks of including the series:
> 
> * Since two of the three patches have already received acks from 3+
> people, I would assume that the risks for those are minimal. As for the
> first patch, unless a vm_event consumer uses the new
> VM_EVENT_FLAG_SET_EMUL_READ_DATA vm_event response flag in conjunction
> with VM_EVENT_FLAG_EMULATE, the impact should be close to 0 (only a few
> "if ( unlikely(set_context_enabled) )" extra statements).
> 
> A new series will follow as soon as possible, addressing Jan's comments
> on the first patch and cleaning up patch 3 a little more.
> 
> 
> Thank you in advance for considering this,
> Razvan

Re: Requesting a freeze exception for vm_event memory introspection helpers

[Razvan Cojocaru]

On 07/13/2015 04:51 PM, Wei Liu wrote:
> On Mon, Jul 13, 2015 at 10:35:44AM +0300, Razvan Cojocaru wrote:
>> Hello,
>>
>> I'd like to ask for a freeze exception for the "vm_event memory
>> introspection helpers" series.
>>
>> [PATCH 1/3] xen/mem_access: Support for memory-content hiding
>> [PATCH 2/3] xen/vm_event: Support for guest-requested events
>> [PATCH 3/3] xen/vm_event: Deny register writes if refused by
>> vm_event reply
>>
>> All patches have been acked by at least one person (though patch 1 is
>> still under some discussion).
>>
>> 1. Benefits of the series making it in this release:
>>
>> * Probably the most important benefit is that 4.6 development has been
>> very open to refactoring vm_events, and patch 3/3 makes vm_events behave
>> in a consistent manner (all register-write vm_events are pre-write events).
>>
>> * There are 3rd parties interested in these features (Tamas, for
>> example, has already expressed interest in uses of patch 1/1).
>>
> 
> These aren't really good arguments for getting this in for 4.6. It would
> be easy for you and Tamas to carry three patches for a while.
> 
> What are the impact to end users (end users -- meaning either system
> administrator that sets up Xen and other developers who want to use the
> new interface you introduce)?  Does this series consist of the last
> missing pieces of a feature?

Not just missing pieces of a feature, the guest-requested (formerly
VMCALL) vm_event patch is crucial for memory introspection, and without
the first patch it's very difficult to resume monitoring Windows guests
that have been put in hybernate-mode (instead of powered off).

I don't follow the impact question, sorry. If an administrator doesn't
use these features, the impact (in terms of speed and resources) should
be next to nothing.

If a freeze exception for the whole series is not an option at this
point, would it be appropriate to just drop patches requiring more
review and get the more acked ones in?


Thanks,
Razvan

Re: Requesting a freeze exception for vm_event memory introspection helpers

[Wei Liu]

On Mon, Jul 13, 2015 at 05:10:21PM +0300, Razvan Cojocaru wrote:
> On 07/13/2015 04:51 PM, Wei Liu wrote:
> > On Mon, Jul 13, 2015 at 10:35:44AM +0300, Razvan Cojocaru wrote:
> >> Hello,
> >>
> >> I'd like to ask for a freeze exception for the "vm_event memory
> >> introspection helpers" series.
> >>
> >> [PATCH 1/3] xen/mem_access: Support for memory-content hiding
> >> [PATCH 2/3] xen/vm_event: Support for guest-requested events
> >> [PATCH 3/3] xen/vm_event: Deny register writes if refused by
> >> vm_event reply
> >>
> >> All patches have been acked by at least one person (though patch 1 is
> >> still under some discussion).
> >>
> >> 1. Benefits of the series making it in this release:
> >>
> >> * Probably the most important benefit is that 4.6 development has been
> >> very open to refactoring vm_events, and patch 3/3 makes vm_events behave
> >> in a consistent manner (all register-write vm_events are pre-write events).
> >>
> >> * There are 3rd parties interested in these features (Tamas, for
> >> example, has already expressed interest in uses of patch 1/1).
> >>
> > 
> > These aren't really good arguments for getting this in for 4.6. It would
> > be easy for you and Tamas to carry three patches for a while.
> > 
> > What are the impact to end users (end users -- meaning either system
> > administrator that sets up Xen and other developers who want to use the
> > new interface you introduce)?  Does this series consist of the last
> > missing pieces of a feature?
> 
> Not just missing pieces of a feature, the guest-requested (formerly
> VMCALL) vm_event patch is crucial for memory introspection, and without
> the first patch it's very difficult to resume monitoring Windows guests
> that have been put in hybernate-mode (instead of powered off).
> 

Right, so with these three patches, memory introspection is a complete
feature (as in, to cover most use cases)? Or is it still a work in
progress even after these patches are merged?

> I don't follow the impact question, sorry. If an administrator doesn't
> use these features, the impact (in terms of speed and resources) should
> be next to nothing.
> 

OK, let me be specific. If I'm to build a product by either using Xen as
out-of-box virtualisation platform or building my own software on top of
Xen, would it make any *big* difference if I have your three patches?

The "big difference" is the impact (benefit) I'm looking at, from users'
point of view.

> If a freeze exception for the whole series is not an option at this
> point, would it be appropriate to just drop patches requiring more
> review and get the more acked ones in?
> 

I'm not saying it's not possible. I'm trying to make sense of what is
going on at the moment.

Maybe other maintainers can help enlighten me and express their
preference? And I guess with your maintainer's hat on it would be a
"yes, let's merge them" from you. :-)

Wei.

> 
> Thanks,
> Razvan

Re: Requesting a freeze exception for vm_event memory introspection helpers

[Razvan Cojocaru]

On 07/13/2015 05:25 PM, Wei Liu wrote:
> On Mon, Jul 13, 2015 at 05:10:21PM +0300, Razvan Cojocaru wrote:
>> On 07/13/2015 04:51 PM, Wei Liu wrote:
>>> On Mon, Jul 13, 2015 at 10:35:44AM +0300, Razvan Cojocaru wrote:
>>>> Hello,
>>>>
>>>> I'd like to ask for a freeze exception for the "vm_event memory
>>>> introspection helpers" series.
>>>>
>>>> [PATCH 1/3] xen/mem_access: Support for memory-content hiding
>>>> [PATCH 2/3] xen/vm_event: Support for guest-requested events
>>>> [PATCH 3/3] xen/vm_event: Deny register writes if refused by
>>>> vm_event reply
>>>>
>>>> All patches have been acked by at least one person (though patch 1 is
>>>> still under some discussion).
>>>>
>>>> 1. Benefits of the series making it in this release:
>>>>
>>>> * Probably the most important benefit is that 4.6 development has been
>>>> very open to refactoring vm_events, and patch 3/3 makes vm_events behave
>>>> in a consistent manner (all register-write vm_events are pre-write events).
>>>>
>>>> * There are 3rd parties interested in these features (Tamas, for
>>>> example, has already expressed interest in uses of patch 1/1).
>>>>
>>>
>>> These aren't really good arguments for getting this in for 4.6. It would
>>> be easy for you and Tamas to carry three patches for a while.
>>>
>>> What are the impact to end users (end users -- meaning either system
>>> administrator that sets up Xen and other developers who want to use the
>>> new interface you introduce)?  Does this series consist of the last
>>> missing pieces of a feature?
>>
>> Not just missing pieces of a feature, the guest-requested (formerly
>> VMCALL) vm_event patch is crucial for memory introspection, and without
>> the first patch it's very difficult to resume monitoring Windows guests
>> that have been put in hybernate-mode (instead of powered off).
>>
> 
> Right, so with these three patches, memory introspection is a complete
> feature (as in, to cover most use cases)? Or is it still a work in
> progress even after these patches are merged?

Indeed, with these three patches x86 memory introspection is a complete
feature. There are still tweaks, of course, such as the ones mentioned
before (ARM support would be nice, some optimizations for REP emulation,
extending the emulator to be able to handle everything, etc.), but the
important part would be complete with these.

>> I don't follow the impact question, sorry. If an administrator doesn't
>> use these features, the impact (in terms of speed and resources) should
>> be next to nothing.
>>
> 
> OK, let me be specific. If I'm to build a product by either using Xen as
> out-of-box virtualisation platform or building my own software on top of
> Xen, would it make any *big* difference if I have your three patches?

I would say yes, and this from experience. I've been maintaining a
series of patches internally for a while now (I think since Xen 4.0),
and I've come across two main categories of non-trivial issues:

1. Things tend to change a lot with Xen, especially lately, so at least
for individual patches it has come more often than I would have thought
to almost complete rewrites when going up to the next Xen version.

2. Even with a lot of work and testing, I still think a patch that has
been reviewed for a few months on xen-devel is better that an in-house
one. Xen is one of the best written and most elegant C projects I've
seen, but it's still evolving quickly and largely undocumented, so
there's always the chance a lone developer will miss something somewhere.

So yes, again, I'd say that being able to use Xen out-of-the box for
introspection is the (much) better choice, at least in my experience.

>> If a freeze exception for the whole series is not an option at this
>> point, would it be appropriate to just drop patches requiring more
>> review and get the more acked ones in?
>>
> 
> I'm not saying it's not possible. I'm trying to make sense of what is
> going on at the moment.
> 
> Maybe other maintainers can help enlighten me and express their
> preference? And I guess with your maintainer's hat on it would be a
> "yes, let's merge them" from you. :-)

I understand. Thanks for having this conversation!


Thanks,
Razvan

Re: Requesting a freeze exception for vm_event memory introspection helpers

[Wei Liu]

On Mon, Jul 13, 2015 at 05:42:48PM +0300, Razvan Cojocaru wrote:
> On 07/13/2015 05:25 PM, Wei Liu wrote:
> > On Mon, Jul 13, 2015 at 05:10:21PM +0300, Razvan Cojocaru wrote:
> >> On 07/13/2015 04:51 PM, Wei Liu wrote:
> >>> On Mon, Jul 13, 2015 at 10:35:44AM +0300, Razvan Cojocaru wrote:
> >>>> Hello,
> >>>>
> >>>> I'd like to ask for a freeze exception for the "vm_event memory
> >>>> introspection helpers" series.
> >>>>
> >>>> [PATCH 1/3] xen/mem_access: Support for memory-content hiding
> >>>> [PATCH 2/3] xen/vm_event: Support for guest-requested events
> >>>> [PATCH 3/3] xen/vm_event: Deny register writes if refused by
> >>>> vm_event reply
> >>>>
> >>>> All patches have been acked by at least one person (though patch 1 is
> >>>> still under some discussion).
> >>>>
> >>>> 1. Benefits of the series making it in this release:
> >>>>
> >>>> * Probably the most important benefit is that 4.6 development has been
> >>>> very open to refactoring vm_events, and patch 3/3 makes vm_events behave
> >>>> in a consistent manner (all register-write vm_events are pre-write events).
> >>>>
> >>>> * There are 3rd parties interested in these features (Tamas, for
> >>>> example, has already expressed interest in uses of patch 1/1).
> >>>>
> >>>
> >>> These aren't really good arguments for getting this in for 4.6. It would
> >>> be easy for you and Tamas to carry three patches for a while.
> >>>
> >>> What are the impact to end users (end users -- meaning either system
> >>> administrator that sets up Xen and other developers who want to use the
> >>> new interface you introduce)?  Does this series consist of the last
> >>> missing pieces of a feature?
> >>
> >> Not just missing pieces of a feature, the guest-requested (formerly
> >> VMCALL) vm_event patch is crucial for memory introspection, and without
> >> the first patch it's very difficult to resume monitoring Windows guests
> >> that have been put in hybernate-mode (instead of powered off).
> >>
> > 
> > Right, so with these three patches, memory introspection is a complete
> > feature (as in, to cover most use cases)? Or is it still a work in
> > progress even after these patches are merged?
> 
> Indeed, with these three patches x86 memory introspection is a complete
> feature. There are still tweaks, of course, such as the ones mentioned
> before (ARM support would be nice, some optimizations for REP emulation,
> extending the emulator to be able to handle everything, etc.), but the
> important part would be complete with these.
> 
> >> I don't follow the impact question, sorry. If an administrator doesn't
> >> use these features, the impact (in terms of speed and resources) should
> >> be next to nothing.
> >>
> > 
> > OK, let me be specific. If I'm to build a product by either using Xen as
> > out-of-box virtualisation platform or building my own software on top of
> > Xen, would it make any *big* difference if I have your three patches?
> 
> I would say yes, and this from experience. I've been maintaining a
> series of patches internally for a while now (I think since Xen 4.0),
> and I've come across two main categories of non-trivial issues:
> 
> 1. Things tend to change a lot with Xen, especially lately, so at least
> for individual patches it has come more often than I would have thought
> to almost complete rewrites when going up to the next Xen version.
> 
> 2. Even with a lot of work and testing, I still think a patch that has
> been reviewed for a few months on xen-devel is better that an in-house
> one. Xen is one of the best written and most elegant C projects I've
> seen, but it's still evolving quickly and largely undocumented, so
> there's always the chance a lone developer will miss something somewhere.
> 
> So yes, again, I'd say that being able to use Xen out-of-the box for
> introspection is the (much) better choice, at least in my experience.
> 
> >> If a freeze exception for the whole series is not an option at this
> >> point, would it be appropriate to just drop patches requiring more
> >> review and get the more acked ones in?
> >>
> > 
> > I'm not saying it's not possible. I'm trying to make sense of what is
> > going on at the moment.
> > 
> > Maybe other maintainers can help enlighten me and express their
> > preference? And I guess with your maintainer's hat on it would be a
> > "yes, let's merge them" from you. :-)
> 
> I understand. Thanks for having this conversation!
> 

Thanks for your clarification. If you can get your next version all
acked /reviewed I would be fine with it going in.  Do note that the cut
off day for applying patches with freeze exception is next Friday.

Wei.

> 
> Thanks,
> Razvan

Re: Requesting a freeze exception for vm_event memory introspection helpers

[Razvan Cojocaru]

On 07/13/2015 05:56 PM, Wei Liu wrote:
> On Mon, Jul 13, 2015 at 05:42:48PM +0300, Razvan Cojocaru wrote:
>> On 07/13/2015 05:25 PM, Wei Liu wrote:
>>> On Mon, Jul 13, 2015 at 05:10:21PM +0300, Razvan Cojocaru wrote:
>>>> On 07/13/2015 04:51 PM, Wei Liu wrote:
>>>>> On Mon, Jul 13, 2015 at 10:35:44AM +0300, Razvan Cojocaru wrote:
>>>>>> Hello,
>>>>>>
>>>>>> I'd like to ask for a freeze exception for the "vm_event memory
>>>>>> introspection helpers" series.
>>>>>>
>>>>>> [PATCH 1/3] xen/mem_access: Support for memory-content hiding
>>>>>> [PATCH 2/3] xen/vm_event: Support for guest-requested events
>>>>>> [PATCH 3/3] xen/vm_event: Deny register writes if refused by
>>>>>> vm_event reply
>>>>>>
>>>>>> All patches have been acked by at least one person (though patch 1 is
>>>>>> still under some discussion).
>>>>>>
>>>>>> 1. Benefits of the series making it in this release:
>>>>>>
>>>>>> * Probably the most important benefit is that 4.6 development has been
>>>>>> very open to refactoring vm_events, and patch 3/3 makes vm_events behave
>>>>>> in a consistent manner (all register-write vm_events are pre-write events).
>>>>>>
>>>>>> * There are 3rd parties interested in these features (Tamas, for
>>>>>> example, has already expressed interest in uses of patch 1/1).
>>>>>>
>>>>>
>>>>> These aren't really good arguments for getting this in for 4.6. It would
>>>>> be easy for you and Tamas to carry three patches for a while.
>>>>>
>>>>> What are the impact to end users (end users -- meaning either system
>>>>> administrator that sets up Xen and other developers who want to use the
>>>>> new interface you introduce)?  Does this series consist of the last
>>>>> missing pieces of a feature?
>>>>
>>>> Not just missing pieces of a feature, the guest-requested (formerly
>>>> VMCALL) vm_event patch is crucial for memory introspection, and without
>>>> the first patch it's very difficult to resume monitoring Windows guests
>>>> that have been put in hybernate-mode (instead of powered off).
>>>>
>>>
>>> Right, so with these three patches, memory introspection is a complete
>>> feature (as in, to cover most use cases)? Or is it still a work in
>>> progress even after these patches are merged?
>>
>> Indeed, with these three patches x86 memory introspection is a complete
>> feature. There are still tweaks, of course, such as the ones mentioned
>> before (ARM support would be nice, some optimizations for REP emulation,
>> extending the emulator to be able to handle everything, etc.), but the
>> important part would be complete with these.
>>
>>>> I don't follow the impact question, sorry. If an administrator doesn't
>>>> use these features, the impact (in terms of speed and resources) should
>>>> be next to nothing.
>>>>
>>>
>>> OK, let me be specific. If I'm to build a product by either using Xen as
>>> out-of-box virtualisation platform or building my own software on top of
>>> Xen, would it make any *big* difference if I have your three patches?
>>
>> I would say yes, and this from experience. I've been maintaining a
>> series of patches internally for a while now (I think since Xen 4.0),
>> and I've come across two main categories of non-trivial issues:
>>
>> 1. Things tend to change a lot with Xen, especially lately, so at least
>> for individual patches it has come more often than I would have thought
>> to almost complete rewrites when going up to the next Xen version.
>>
>> 2. Even with a lot of work and testing, I still think a patch that has
>> been reviewed for a few months on xen-devel is better that an in-house
>> one. Xen is one of the best written and most elegant C projects I've
>> seen, but it's still evolving quickly and largely undocumented, so
>> there's always the chance a lone developer will miss something somewhere.
>>
>> So yes, again, I'd say that being able to use Xen out-of-the box for
>> introspection is the (much) better choice, at least in my experience.
>>
>>>> If a freeze exception for the whole series is not an option at this
>>>> point, would it be appropriate to just drop patches requiring more
>>>> review and get the more acked ones in?
>>>>
>>>
>>> I'm not saying it's not possible. I'm trying to make sense of what is
>>> going on at the moment.
>>>
>>> Maybe other maintainers can help enlighten me and express their
>>> preference? And I guess with your maintainer's hat on it would be a
>>> "yes, let's merge them" from you. :-)
>>
>> I understand. Thanks for having this conversation!
>>
> 
> Thanks for your clarification. If you can get your next version all
> acked /reviewed I would be fine with it going in.  Do note that the cut
> off day for applying patches with freeze exception is next Friday.

Thank you, I appreciate it! Duly noted.


Thanks,
Razvan