linux-kernel - Re: [PATCH v1 06/11] KVM: x86: use hardware-compatible format for APIC ID register

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: <20160701145407.GC2301@potion>
Date:	Fri, 1 Jul 2016 16:54:07 +0200
From:	Radim Krčmář <rkrcmar@...hat.com>
To:	Paolo Bonzini <pbonzini@...hat.com>
Cc:	linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
	"Lan, Tianyu" <tianyu.lan@...el.com>,
	Igor Mammedov <imammedo@...hat.com>,
	Jan Kiszka <jan.kiszka@....de>, Peter Xu <peterx@...hat.com>
Subject: Re: [PATCH v1 06/11] KVM: x86: use hardware-compatible format for
 APIC ID register

2016-07-01 16:12+0200, Paolo Bonzini:
> On 01/07/2016 15:11, Radim Krčmář wrote:
>>>> >> +static void __kvm_apic_state_fixup(struct kvm_vcpu *vcpu,
>>>> >> +		struct kvm_lapic_state *s, bool set)
>>>> >> +{
>>>> >> +	if (apic_x2apic_mode(vcpu->arch.apic)) {
>>>> >> +		u32 *id = (u32 *)(s->regs + APIC_ID);
>>>> >> +		if (set)
>>>> >> +			*id >>= 24;
>>>> >> +		else
>>>> >> +			*id <<= 24;
>>>> >> +	}
>>> > 
>>> > When setting, this should read from the apic_base being set.  So I think
>>> > you cannot use apic_x2apic_mode.
>> 
>> apic_x2apic_mode uses apic_base MSR, so its value does not depend on
>> LAPIC_SET/GET.  I don't like the dependency much, but all combinations
>> of values/orders should work well.
> 
> You're right of course.  However it should be documented in the
> KVM_GET_LAPIC/KVM_SET_LAPIC ioctl docs that KVM_SET_MSR for apic_base
> should be performed first.

Ok, that will make our life easier.

> Should kvm_lapic_set_base change the value of the ID register when
> x2apic mode is left just like we do for entering x2apic mode?

Yes, the patch does it.  The only possible change from x2APIC mode is to
hardware disabled mode and re-enabled xAPIC starts with initial APIC ID
in upper 8 bits.

>                                                                (Hint: we
> want kvm-unit-tests for this).

:) Something like this?

  enable_xapic()
  id = apic_id()
  set_apic_id(id+1) // ?
  enable_x2apic()
  id == apic_id() & 0xff
  disable_apic()
  enable_xapic()
  id == apic_id()