Re: [PATCH] KVM: arm64: Fix error checking for FFA_VERSION

[Marc Zyngier <maz@kernel.org>]

On Mon, 24 Nov 2025 11:49:08 +0000,
Kornel Dulęba <korneld@google.com> wrote:
> 
> On Sat, Nov 22, 2025 at 12:36 PM Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Fri, 14 Nov 2025 11:11:53 +0000,
> > "=?utf-8?q?Kornel_Dul=C4=99ba?=" <korneld@google.com> wrote:
> > >
> > > According to section 13.2 of the DEN0077 FF-A specification, when
> > > firmware does not support the requested version, it should reply with
> > > FFA_RET_NOT_SUPPORTED(-1). Table 13.6 specifies the type of the error
> > > code as int32.
> > > Currently, the error checking logic compares the unsigned long return
> > > value it got from the SMC layer, against a "-1" literal. This fails due
> > > to a type mismatch: the literal is extended to 64 bits, whereas the
> > > register contains only 32 bits of ones(0x00000000ffffffff).
> > > Consequently, hyp_ffa_init misinterprets the "-1" return value as an
> > > invalid FF-A version. This prevents pKVM initialization on devices where
> > > FF-A is not supported in firmware.
> >
> > Is this statement accurate? I regularly boot KVM in protected mode in
> > environments that really cannot be suspected of implementing FF-A
> > (there is no EL3 to start with). And yet I don't see any failure of
> > the sort.
> >
> > Please clarify the circumstances this is triggered.
> 
> I do have EL3 enabled, but the FF-A itself is not implemented.
> 
> Without this change kvm initialization fails with the following:
> 
> [    0.946776][    T1] kvm [1]: nv: 554 coarse grained trap handlers
> [    0.952880][    T1] kvm [1]: nv: 669 fine grained trap handlers
> [    0.958813][    T1] kvm [1]: IPA Size Limit: 44 bits
> (...)
> [    1.034089][    T1] kvm [1]: Failed to init hyp memory protection
> [    1.041213][    T1] kvm [1]: error initializing Hyp mode: -95

[    0.045492] kvm [1]: nv: 568 coarse grained trap handlers
[    0.045860] kvm [1]: nv: 664 fine grained trap handlers
[    0.046194] kvm [1]: IPA Size Limit: 42 bits
[    0.220184] kvm [1]: GICv3: no GICV resource entry
[    0.220525] kvm [1]: disabling GICv2 emulation
[    0.220852] kvm [1]: GIC system register CPU interface enabled
[    0.221264] kvm [1]: vgic interrupt IRQ9
[    0.221565] kvm [1]: Protected hVHE mode initialized successfully

Ergo, it works here without FFA (this is in a nested guest that is not
exposed anything but PSCI in the FW emulation).

> I managed to narrow this down to the FFA version check by examining
> all of the places in kvm initialization logic where -EOPNOTSUPP is
> returned. Since printing anything in this part of the code is somewhat
> problematic I replaced “return -EOPNOTSUPP” with “return res.s0” to
> examine the problematic register value:
> 
> [1.041229][    T1] kvm [1]: error initializing Hyp mode: -1
> 
> Note that the return code itself is cast to int before being printed.
> Then a colleague of mine recommended looking into the arm_ffa driver.
> (“drivers/firmware/arm_ffa/driver.c”) There I found that in the
> ffa_version_check function, the return value from the SMC call is cast
> to s32 before being checked for errors.
> I did the same in the kvm initialization logic, which is how this
> patch was created. Furthermore I also examined the FF-A
> specification(DEN0077), where the error code value type is specified
> as int32.
> With this change applied I can now see that kvm is up and running:
> 
> [    0.946839][    T1] kvm [1]: nv: 554 coarse grained trap handlers
> [    0.952940][    T1] kvm [1]: nv: 669 fine grained trap handlers
> [    0.958867][    T1] kvm [1]: IPA Size Limit: 44 bits
> (...)
> [    1.061717][    T1] kvm [1]: Protected hVHE mode initialized successfully
> 
> The /dev/kvm file is also there.

I don't dispute the bug. I dispute the assertion that the absence of
FFA triggers it. So either your testing environment influences the
behaviour, or you have extra patches that also change the behaviour,
or something else.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.