linux-kernel - Re: [PATCH] arm64: KVM: export current vcpu->pause state via pseudo regs

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: <87fvhgin6s.fsf@linaro.org>
Date:	Fri, 01 Aug 2014 10:11:52 +0100
From:	Alex Bennée <alex.bennee@...aro.org>
To:	Christoffer Dall <christoffer.dall@...aro.org>
Cc:	kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org,
	kvm@...r.kernel.org, Marc Zyngier <marc.zyngier@....com>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	Gleb Natapov <gleb@...nel.org>,
	Paolo Bonzini <pbonzini@...hat.com>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] arm64: KVM: export current vcpu->pause state via pseudo regs


Christoffer Dall writes:

> On Thu, Jul 31, 2014 at 04:14:51PM +0100, Alex Bennée wrote:
>> 
>> Christoffer Dall writes:
>> 
>> > On Wed, Jul 09, 2014 at 02:55:12PM +0100, Alex Bennée wrote:
>> >> To cleanly restore an SMP VM we need to ensure that the current pause
>> >> state of each vcpu is correctly recorded. Things could get confused if
>> >> the CPU starts running after migration restore completes when it was
>> >> paused before it state was captured.
>> >> 
>> <snip>
>> >> +/* Power state (PSCI), not real registers */
>> >> +#define KVM_REG_ARM_PSCI		(0x0014 << KVM_REG_ARM_COPROC_SHIFT)
>> >> +#define KVM_REG_ARM_PSCI_REG(n) \
>> >> +	(KVM_REG_ARM64 | KVM_REG_SIZE_U64 | KVM_REG_ARM_PSCI | \
>> >> +         (n & ~KVM_REG_ARM_COPROC_MASK))
>> >
>> > I don't understand this mask, why isn't this
>> >             (n & 0xffff))
>> 
>> I was trying to use the existing masks, but of course if anyone changes
>> that it would be an ABI change so probably not worth it.
>> 
>
> the KVM_REG_ARM_COPROC_MASK is part of the uapi IIRC, so that's not the
> issue, but that mask doesn't cover all the upper bits, so it feels weird
> to use that to me.

Yeah I missed that. I could do a:

#define KVM_REG_ARM_COPROC_INDEX_MASK   ((1<<KVM_REG_ARM_COPROC_SHIFT)-1)

and use that. I'm generally try to avoid hardcoded numbers but I could
be being a little OCD here ;-)

>> > Can you add the 32-bit counterpart as part of this patch?
>> 
>> Same patch? Sure.
>
> really up to you if you want to split it up into two patches, but I
> think it's small enough that you can just create one patch.

Given the similarity of this code between arm and arm64 I'm wondering if
it's worth doing a arch/arm/kvm/guest_common.c or something to reduce
the amount of copy paste stuff?

-- 
Alex Bennée
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/