Page MenuHomeFreeBSD

xen: introduce xen_pv_disks_disabled()
AcceptedPublic

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

Details

Reviewers
royger
mhorne
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
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 38032
Build 34921: arc lint + arc unit

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.Wed, Apr 21, 7:39 AM