[PATCH] Revert "tools/firmware/ovmf: Use OvmfXen platform file is exist"

[PATCH] Revert "tools/firmware/ovmf: Use OvmfXen platform file is exist"

[Andrew Cooper]

This reverts commit aad7b5c11d51d57659978e04702ac970906894e8.

The change from OvmfX64 to OvmfXen causes a change in behaviour, whereby
OvmfXen maps its shared info page at the top of address space.  When trying to
migrate such a domain, XENMEM_maximum_gpfn returns a very large value.  This
has uncovered multiple issues:

 1) The userspace hypercall wrappers truncate all return values to int on
    Linux and Solaris.  This needs fixing in Xen.
 2) 32bit toolstacks can't migrate any domain with RAM above the 2^40 mark,
    because of virtual address constraints.  This needs fixing in OVMF.

Fixes for both of these aren't completely trivial.  Revert the change to
unblock staging in the meantime.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@citrix.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <iwj@xenproject.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Wei Liu <wl@xen.org>
CC: Julien Grall <julien@xen.org>
---
 tools/firmware/ovmf-makefile | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
index 637ee509c3..55f9992145 100644
--- a/tools/firmware/ovmf-makefile
+++ b/tools/firmware/ovmf-makefile
@@ -17,14 +17,8 @@ all: build
 .PHONY: build
 build:
 	if test -e .git ; then $(GIT) submodule update --init --recursive ; fi
-	set -ex; \
-	if test -e OvmfPkg/OvmfXen.dsc; then \
-	  OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4 -p OvmfPkg/OvmfXen.dsc; \
-	  cp Build/OvmfXen/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin; \
-	else \
-	  OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4; \
-	  cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin; \
-	fi
+	OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
+	cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
 
 .PHONY: clean
 clean:
-- 
2.11.0

Re: [PATCH] Revert "tools/firmware/ovmf: Use OvmfXen platform file is exist"

[Anthony PERARD]

On Tue, Jun 22, 2021 at 04:39:30PM +0100, Andrew Cooper wrote:
> This reverts commit aad7b5c11d51d57659978e04702ac970906894e8.
> 
> The change from OvmfX64 to OvmfXen causes a change in behaviour, whereby
> OvmfXen maps its shared info page at the top of address space.  When trying to
> migrate such a domain, XENMEM_maximum_gpfn returns a very large value.  This
> has uncovered multiple issues:
> 
>  1) The userspace hypercall wrappers truncate all return values to int on
>     Linux and Solaris.  This needs fixing in Xen.
>  2) 32bit toolstacks can't migrate any domain with RAM above the 2^40 mark,
>     because of virtual address constraints.  This needs fixing in OVMF.
> 
> Fixes for both of these aren't completely trivial.  Revert the change to
> unblock staging in the meantime.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

Re: [PATCH] Revert "tools/firmware/ovmf: Use OvmfXen platform file is exist"

[Jan Beulich]

On 22.06.2021 17:39, Andrew Cooper wrote:
> This reverts commit aad7b5c11d51d57659978e04702ac970906894e8.
> 
> The change from OvmfX64 to OvmfXen causes a change in behaviour, whereby
> OvmfXen maps its shared info page at the top of address space.  When trying to
> migrate such a domain, XENMEM_maximum_gpfn returns a very large value.  This
> has uncovered multiple issues:
> 
>  1) The userspace hypercall wrappers truncate all return values to int on
>     Linux and Solaris.  This needs fixing in Xen.
>  2) 32bit toolstacks can't migrate any domain with RAM above the 2^40 mark,
>     because of virtual address constraints.  This needs fixing in OVMF.

And I suspect even that presently enforce boundary of 2^40 is actually
too high, and things still wouldn't work when getting close. At the
very least the tool stack then depends on a fairly big chunk of memory
(2^30 bytes) to be available in one single, virtually contiguous piece.
Iirc 32-bit Linux can be configured to not even leave this much space
for user mode.

Jan

Re: [PATCH] Revert "tools/firmware/ovmf: Use OvmfXen platform file is exist"

[Andrew Cooper]

On 22/06/2021 17:10, Jan Beulich wrote:
> On 22.06.2021 17:39, Andrew Cooper wrote:
>> This reverts commit aad7b5c11d51d57659978e04702ac970906894e8.
>>
>> The change from OvmfX64 to OvmfXen causes a change in behaviour, whereby
>> OvmfXen maps its shared info page at the top of address space.  When trying to
>> migrate such a domain, XENMEM_maximum_gpfn returns a very large value.  This
>> has uncovered multiple issues:
>>
>>  1) The userspace hypercall wrappers truncate all return values to int on
>>     Linux and Solaris.  This needs fixing in Xen.
>>  2) 32bit toolstacks can't migrate any domain with RAM above the 2^40 mark,
>>     because of virtual address constraints.  This needs fixing in OVMF.
> And I suspect even that presently enforce boundary of 2^40 is actually
> too high, and things still wouldn't work when getting close. At the
> very least the tool stack then depends on a fairly big chunk of memory
> (2^30 bytes) to be available in one single, virtually contiguous piece.
> Iirc 32-bit Linux can be configured to not even leave this much space
> for user mode.

I tested it once during Migration v2 development, and it worked for me,
but I do expect that that is as much testing as it has had since...

A 3G/1G split is the default under multiple 32bit kernels, and the
allocation is made right at the start, so there is a reasonable chance
of finding space.  After all, it only needs 4k alignment.

Whether ASLR has changed the chances in the meantime remains to be seen,
but honestly - 32bit toolstacks on x86 really don't exist in production
any more, and Arm still hasn't implemented logdirty support, so the
limit has little practical consequence.

~Andrew