linux-kernel - Re: [PATCH v2 8/8] arm64: kvm: Check support for AArch32 for 32bit guests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56D6BEEF.3000208@arm.com>
Date:	Wed, 2 Mar 2016 10:22:39 +0000
From:	"Suzuki K. Poulose" <Suzuki.Poulose@....com>
To:	Marc Zyngier <marc.zyngier@....com>,
	linux-arm-kernel@...ts.infradead.org
Cc:	christoffer.dall@...aro.org, will.deacon@....com,
	ynorov@...iumnetworks.com, catalin.marinas@....com,
	mark.rutland@....com, linux-kernel@...r.kernel.org,
	kvmarm@...ts.cs.columbia.edu
Subject: Re: [PATCH v2 8/8] arm64: kvm: Check support for AArch32 for 32bit
 guests

On 02/03/16 09:08, Marc Zyngier wrote:
> On 25/02/16 09:52, Suzuki K Poulose wrote:

>> I really wanted to pass kvm_vcpu down to the helpers. But then, I can't
>> define the arch specific helper in asm/kvm_host.h due to lack of kvm_vcpu's
>> definition yet:
>>
>>   In file included from include/linux/kvm_host.h:35:0,
>>                    from arch/arm64/kernel/asm-offsets.c:24:
>>   ./arch/arm64/include/asm/kvm_host.h: In function ‘kvm_arch_vcpu_validate_features’:
>>   ./arch/arm64/include/asm/kvm_host.h:344:48: error: dereferencing pointer to incomplete type
>>     return  !test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features) ||
>
> Why don't you just have the prototype in kvm_host.h, and move the actual
> implementation to something like guest.c? But I think there is a better
> approach, see below.

I thought it would better be a static inline. But, the GCC can do that, silly me :)

>
> This is really convoluted (it took me 5 minutes staring at the
> expression and remembering that AArch32 EL1 implies AArch32 EL0 to get it).
>
> Now, we already have kvm_reset_vcpu() that validates AArch32 support. It
> would probably be better to move things there. Thoughts?

Definitely. I overlooked the function name to do something
specific to resetting the CPU than doing some checks :(.
I will respin it.

Cheers
Suzuki