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
