On 02/02/2021 12:20, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH v9 00/11] acquire_resource size and external IPT monitoring"):
> ...
>> Therefore, I'd like to request a release exception.
> Thanks for this writeup.
>
> There is discussion here of the upside of granting an exception, which
> certainly seems substantial enough to give this serious consideration.
>
>> It [is] fairly isolated in terms of interactions with the rest of
>> Xen, so the changes of a showstopper affecting other features is
>> very slim.
> This is encouraging (optimistic, even) but very general. I would like
> to see a frank and detailed assessment of the downside risks, ideally
> based on analysis of the individual patches.
>
> When I say a "frank and detailed assessment" I'm hoping to have a list
> of the specific design and code changes that pose a risk to non-IPT
> configurations, in decreasing order of risk.
>
> For each one there should be brief discussion of the measures that
> have exist to control that risk (eg, additional review, additional
> testing), and a characterisation of the resulting risk (both in terms
> of likelihood and severity of the resulting bug).
>
> All risks that would come to a diligent reviewer's mind should be
> mentioned and explicitly delath with, even if it is immediately clear
> that they are not a real risk.
>
> Do you think that would be feasible ? We would want to make a
> decision ASAP so it would have to be done quickly too - in the next
> few days and certainly by the end of the week.
Honestly, I think this is an unreasonably large paperwork expectation,
particularly for changes this-clearly isolated in terms of functionality.
I'm going to explicitly disregard build/compile issues because we're not
even at code freeze or -rc1 yet, with multiple weeks yet before any
potential release, and loads of tooling.
Patch 2 adds a new domain creation parameter, which is an internal
tools/xen interface. Default is off, and it needs explicit opting in to
(patch 3), so it will get all the testing it needs in an OSSTest smoke run.
This patch does introduce one new use of a preexisting refcounting
pattern currently under discussion. It many leak memory in theoretical
circumstances, not practical ones. Work to figure out how to unbreak
this pattern is in progress, and orthogonal, and needs applying uniformly
Patch 4 adds a new resource type, which is an API/ABI with userspace.
It is a new type/index so has no current users.
Patch 5 adds enumeration for the IPT feature in hardware, as well as
context switching logic. All context switching changes are behind an
opt-in flag, so a smoke run will be sufficient to prove no adverse
interaction in !vmtrace case.
Patch 6 adds a new domctl and subops for controlling vmtrace. All brand
new functionality with no users, and bounded by the opt-ins from patch 2
and 5.
Patch 7 adds libxc library functions wrapping the domctl of patch 6. No
users.
Patch 8 is example code demonstrating how to use all of the new
functionality. It is built, but not installed.
Patch 9 extends the existing VMFork feature to cope with VMs configured
with this new functionality. It is a no-op for regular VMs.
Patch 10 extends vm_event requests with additional optional metadata
about the tail pointer of data in the vmtrace buffer. Doesn't alter the
behaviour for regular VMs.
Patch 11 extends vm_event responses with an optional request to reset
the vmtrace buffer position. No users, and a no-op for regular VMs.
All of this new functionality is off-by-default and needs an explicit
opt in, for any behavioural changes to occur. While there is no
guarantee that the implementation of the new functionality is perfect,
the development of it has found and fixed a whole slew bugs elsewhere in
Xen, and the new functionality does have extensive testing itself.
~Andrew