On Wed, Jan 14, 2026 at 4:14 PM Alexander Graf <[email protected]> wrote:
>
>
> On 14.01.26 12:51, Manos Pitsidianakis wrote:
> > On Wed, Jan 14, 2026 at 1:19 PM Alexander Graf <[email protected]> wrote:
> >> Hi Manos!
> >>
> >> On 14.01.26 09:35, Manos Pitsidianakis wrote:
> >>> SME2 support adds the following state for HVF guests:
> >>>
> >>> - Vector registers Z0, ... , Z31 (introduced by FEAT_SVE but HVF does
> >>>     not support it)
> >>> - Predicate registers P0, .., P15 (also FEAT_SVE)
> >>> - ZA register
> >>> - ZT0 register
> >>> - PSTATE.{SM,ZA} bits (SVCR pseudo-register)
> >>> - SMPRI_EL1 which handles the PE's priority in the SMCU
> >>> - TPIDR2_EL0 the thread local ID register for SME
> >>>
> >>> Signed-off-by: Manos Pitsidianakis <[email protected]>
> >>
> >> Thanks a lot for the patches. I don't have an M4/M5 handy (yet), so I
> >> can't test the code works correctly. But it sounds like you did that, so
> >> I have no concerns on functionality.
> > Yes, I tested it on Linaro's M4 machine. Adding a functional test
> > using Arm's benchmark is possible, but I couldn't be bothered to
> > create vm image assets just for this :)
> >
> > Note: testing migration is tricky because the CPU has to be in SME
> > mode for the Z/P registers to have meaningful values, so you need to
> > savevm while SME instructions are executed to get a useful migration
> > state. I modified Arm's benchmark to compare the SME matrix
> > calculations against the non-SME calculation to ensure they are the
> > same and I savevm/loadvm a bunch of times while it ran with added
> > printfs in the get/put methods to print whether SME was active or not.
> >
> >> However, I have concerns on a few maintainability aspects. You #ifdef
> >> out a lot of code conditionally on the target macOS version. Any of that
> >> code that is in ifdef may or may not end up getting compiled in CI or
> >> other test builds, which means you are missing out on a lot of
> >> compilation test coverage. As a general rule of thumb, please reduce
> >> #ifdef to the bare minimum.
> > I agree completely, the problem is two-fold here: the HVF exposes APIs
> > with API_AVAILABLE(...) (clang's availability attribute) and also with
> > plain ifdefs (__MAC_OS_X_VERSION_MAX_ALLOWED >= 150200), and
> > specifically the HV_SME_FOO_REG variants, which cannot be protected
> > with the availability attribute. )
> >
> > So every time an SME type such as hv_vcpu_sme_state_t is used, it has
> > to be guarded :/ __builtin_available(...) check is not sufficient
> > because it will not compile due to undefined types.
> >
> >
> >> One thing I like to do (when possible) is to use the ifdef to define a
> >> global const variable or an inline function. That way the compiler's
> >> dead code analysis will eliminate the non-active aspects of your code,
> >> but all previous compiler phases still run which means you get syntax
> >> checks.
> >>
> >> How much of the code down here really does require #ifdefs? And if it's
> >> a lot, maybe we just bump the minimum required macOS version instead.
> >>
> > The missing type definitions and missing function declarations will
> > still raise compiler errors unfortunately. We could add those missing
> > type definitions as stubs if they are not defined so that it compiles,
> > but I chose to ifdef instead. What would you prefer?
>
>
> How about you create a separate #include'd header that provides stubs
> and defines the same way the original headers would? You can #ifdef your
> way inside there all you want. And then you write the generic code with
> the base assumption that all definitions are available.
>
> Alex

I will do that, and add a comment to remind us to remove the
duplication once we raise the minimum macos version. Will send a V2.

Thanks!


-- 
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd

Reply via email to