Page MenuHomeFreeBSD

xen/pvh: fix initialization of environment
ClosedPublic

Authored by royger on Jul 23 2024, 1:24 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 7, 12:17 PM
Unknown Object (File)
Wed, Nov 6, 10:00 PM
Unknown Object (File)
Thu, Oct 17, 3:27 PM
Unknown Object (File)
Oct 16 2024, 7:53 AM
Unknown Object (File)
Oct 15 2024, 7:51 AM
Unknown Object (File)
Oct 14 2024, 5:38 AM
Unknown Object (File)
Oct 12 2024, 4:33 AM
Unknown Object (File)
Oct 10 2024, 9:11 PM

Details

Summary

Xen PVH entry point requires to modify the environment provided by the boot
loader, so that the ACPI RSDP is re-written to use the Xen generated RSDP
instead of the native one.

The current logic in the PVH entry point reserves a single page (4K) in order
to copy the contents of the environment passed from the boot loader, so that
the bootloader provided "acpi.rsdp" is dropped and a Xen specific one is added
afterwards.

This however doesn't scale well, as it's possible for the environment to be
bigger than 4K. Bumping the buffer, or attempting to peek at the size of the
metadata all seem to just add more complexity to a sensitive path. Instead
introduce a new ACPI hook that allows setting the RSDP address directly, and
use it from the PVH entry point to set the position of the Xen generated RSDP.

This allows to reduce the logic in the PVH metadata processing, as there's no
need to parse and filter the bootloader provided environment.

Note that modifying the environment blob in-place is likely to not work. The
RSDP address is provided as a string, it's possible the new RSDP location is
higher than the current one, and the string with the new location would overrun
the space used by the previous one.

Sponsored by: Cloud Software Group
PR: 277200
MFC: 3 days

Test Plan

Already tested by the original reporter of the issue, confirms issue is fixed.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I'm catching all Xen via a Herald rule. This is deep in the x86 side where I haven't done experimentation. Looks plausible, but I'm not your reviewer.

This seems reasonable to me aside from the comments.

sys/x86/acpica/OsdEnvironment.c
87

I think it'd be good to have AcpiOsGetRootPointer() use this helper function as well.

sys/x86/xen/pv.c
426

The whitespace change here looks wrong.

This revision is now accepted and ready to land.Aug 1 2024, 4:28 PM

Thanks, will adjust tomorrow and commit.

sys/x86/acpica/OsdEnvironment.c
87

Thanks, I have no strong opinion, but was under the impression that using it in AcpiOsGetRootPointer() would obfuscate the code unnecessarily. Will adjust and commit.

sys/x86/acpica/OsdEnvironment.c
87

It's not very important of course, it just seems preferable to have only one place where that variable is assigned, if possible. I don't object to the current version.

This revision was automatically updated to reflect the committed changes.