Page MenuHomeFreeBSD

xen: introduce xen_pv_disks_disabled()
ClosedPublic

Authored by ehem_freebsd_m5p.com on Mar 23 2021, 10:03 PM.

Details

Summary

ARM guest is considered as HVM in Freebsd but they only support PV disk
(no emulation available).

Submitted by: Elliott Mitchell <ehem+freebsd@m5p.com>
Original implementation: Julien Grall <julien@xen.org>, 2015-10-16 11:18:21

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

Forgot to ask on the previous patch, but I guess the tunable won't be available to Arm guests anyway, in which case it might be easier to just force xen_disable_pv_disks to false for Arm:

#define xen_disable_pv_disks false

in the Arm xen-os.h header?

That is pretty well what the series does. Introduces sys/arm64/include/xen/xen-os.h with xen_pv_disks_disabled() as a static inline which returns false. The real question is whether to adjust history by introducing all of these at the same time as the function is introduced generally. I suspect that is likely better, I think this may be better via e-mail.

That is pretty well what the series does. Introduces sys/arm64/include/xen/xen-os.h with xen_pv_disks_disabled() as a static inline which returns false. The real question is whether to adjust history by introducing all of these at the same time as the function is introduced generally. I suspect that is likely better, I think this may be better via e-mail.

My point is that maybe instead of an inline function you can introduce a define instead, ie:

#define xen_disable_pv_disks false

For Arm, and leave the x86 as-is, just moving the extern declaration into the arch specific xen-os.h.

My point is that maybe instead of an inline function you can introduce a define instead, ie:

#define xen_disable_pv_disks false

For Arm, and leave the x86 as-is, just moving the extern declaration into the arch specific xen-os.h.

Did you misread the commit or make a mistake when reading the Phabricator file list? Whether or not that approach is used, sys/xen/xen-os.h still needs to be modified to remove xen_disable_pv_disks from there. No definition or declaration is added for ARM in this commit. The commits for sys/arm64/include/xen/xen-os.h aren't yet present.

My point is that maybe instead of an inline function you can introduce a define instead, ie:

#define xen_disable_pv_disks false

For Arm, and leave the x86 as-is, just moving the extern declaration into the arch specific xen-os.h.

Did you misread the commit or make a mistake when reading the Phabricator file list? Whether or not that approach is used, sys/xen/xen-os.h still needs to be modified to remove xen_disable_pv_disks from there. No definition or declaration is added for ARM in this commit. The commits for sys/arm64/include/xen/xen-os.h aren't yet present.

Never mind, I see that part of the point is to also place the xen_hvm_domain() check as arch-specific.

This revision is now accepted and ready to land.Apr 21 2021, 7:39 AM
This revision was automatically updated to reflect the committed changes.