Page MenuHomeFreeBSD

xen/efi: introduce a PV interface for EFI run time services for dom0
ClosedPublic

Authored by royger on Fri, Feb 12, 4:03 PM.

Details

Summary

FreeBSD when running as a dom0 under Xen is not supposed to access the
run time services directly, and instead should proxy the calls through
Xen using an hypercall interface that exposes access to selected run
time services.

Implement the efirt interface on top of the Xen provided hypercalls.

Sponsored by: Citrix Systems R&D

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kevans added inline comments.
sys/dev/xen/efi/pvefi.c
236

If pvefi isn't being used (disabled or not dom0), this would seem to break stock efirt upon load/unload as we set efi_ops to the null-filled prev.

Looks reasonable, modulo kevan's comments

Thanks for the comment!

sys/dev/xen/efi/pvefi.c
236

I originally had the check above returning ENXIO, but changed it as I didn't know whether that would be reported as an error. I guess returning ENXIO from load will prevent unload attempts?

sys/dev/xen/efi/pvefi.c
236

Annoyingly enough, if you return ENXIO it will immediately be issued a MOD_UNLOAD anyways -- it's kind of a no-win situation. :-(

kib added inline comments.
sys/dev/xen/efi/pvefi.c
236

Can you please explain more how it breaks bare-metal efirt?

That said, I am not sure the prev business is useful at all.

sys/dev/xen/efi/pvefi.c
236

Noting how it's implemented from the other review, we end up with an empty ops vector if prev is still .bss (early return in MOD_LOAD path)

sys/dev/xen/efi/pvefi.c
236

It's likely not of much usage restoring active_efi_ops to prev on module unload, since you either use one of the two interfaces, but not both. I'm just doing it to prevent ending up with active_efi_ops == NULL which will trigger a page-fault.

  • Fix module unload.
  • Make module depend on efirt.
kib added inline comments.
sys/dev/xen/efi/pvefi.c
236

But if prev is NULL, it is still the situation with dandling pointers.
I would just drop the prev stuff, but I do not insist.

This revision is now accepted and ready to land.Mon, Feb 15, 9:56 AM
sys/dev/xen/efi/pvefi.c
236

I just added a prev != NULL check in this last version before attempting to set active_efi_ops = prev in the unload case. I'm not sure what's the best way to deal with this, should we have a nop_efi_ops that has all pointers set to NULL in order to set it here on module unload if the module was in-use?

sys/dev/xen/efi/pvefi.c
236

nop_efi_ops is perhaps fine if the native vector is also in module with the same prev handling. But it is probably too much for useless scenarios.

sys/dev/xen/efi/pvefi.c
236

efirt module is also where active_efi_ops is defined, so if the module is unloaded so is the symbol itself, hence not sure it makes much sense to set it to nop_efi_ops anyway.