Re: [PATCH v9 00/11] acquire_resource size and external IPT monitoring

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/02/2021 12:20, Ian Jackson wrote:
> Since you mentioned patch 1 and asserted it didn't need a release-ack,
> I looked at it in a little more detail.  It seems to contain a
> moderate amount of (fairly localised) restructuring.  IDK whether
> XENMEM_acquire_resource is used by non-IPT configurations but I didn't
> see an assertion anywhere that it isn't.

Acquire resource is used by Qemu/demu/varstored/etc (for IO emulation)
and the the domain builder (seeding the grant table with
xenstore/console details).

None of these usecases used a size calculation, and made blind mapping
calls of 1 page in size.

IPT is the first usecase to want to map more than a single page in one go.

> I appreciate that whether something is "straightforward" on the one
> hand, vs involving "substantial refactoring" on the ohter, or this is
> a matter of judgement, which I have left up to the commiters during
> this part of the freeze.  But for the record my view is that this
> patch is not a "straightforward bugfix" and needs a release ack.

I have extensive testing, demonstrating the bug already present in
staging (unable to map the guests whole grant table in default
configurations), and demonstrating the correctness of the fix.

Some of this testing (specifically, the toos/tests/* binary) is
something I plan to fix up over the ARM IOERQ and other series, and
submit later this week.  It will demonstrate the current bug in staging,
and show it fixed with patch 1 committed.  (This is something I want to
become an autotest in due course.)

Other parts of this testing cannot be submitted.  To get the compat
layer correct, I needed an XTF test and modified Xen which had a known
pattern, to check the marshalling logic didn't lose anything when a
continuation hit an interesting.  I can talk you through these tests and
assert that I have run them, but its not testing logic we can commit
into Xen, and its not anything which gets tested by OSSTest because we
don't test 32bit PVH dom0's in anger.

~Andrew