linux-kernel - Re: [PART1 RFC v4 10/11] svm: Do not intercept CR8 when enable AVIC

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]

Date:	Wed, 13 Apr 2016 00:26:28 +0200
From:	Paolo Bonzini <pbonzini@...hat.com>
To:	Radim Krčmář <rkrcmar@...hat.com>,
	Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>
Cc:	joro@...tes.org, bp@...en8.de, gleb@...nel.org,
	alex.williamson@...hat.com, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org, wei@...hat.com,
	sherry.hurwitz@....com
Subject: Re: [PART1 RFC v4 10/11] svm: Do not intercept CR8 when enable AVIC



On 12/04/2016 16:18, Radim Krčmář wrote:
>> > @@ -4069,7 +4070,8 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
>> > -	if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK))
>> > +	if ((is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) ||
>> > +	     svm_vcpu_avic_enabled(svm))
>> > @@ -4255,14 +4257,15 @@ static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu)
>> >  static inline void sync_lapic_to_cr8(struct kvm_vcpu *vcpu)
>> >  {
>> >  	struct vcpu_svm *svm = to_svm(vcpu);
>> > -	u64 cr8;
>> > +	struct kvm_lapic *apic = vcpu->arch.apic;
>> >  
>> > -	if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK))
>> > +	if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK) &&
> Should be "||" at the end of line, like above.
> 
> (Naming this condition would reduce the chance of errors.)
> 

I think it's just "is_guest_mode(vcpu) && (vcpu->arch.hflags &
HF_VINTR_MASK)" that should become a static inline.  It is used also in
update_cr8_intercept.  Then something like

	if (svm_in_nested_interrupt_shadow(vcpu) &&
	    svm_vcpu_avic_enabled(svm))
		return;

makes little sense and stands out much better.

In fact, because nested SVM and AVIC have nothing to do with each other,
it's even better to write it like

	if (svm_in_nested_interrupt_shadow(vcpu))
		return;
	if (svm_vcpu_avic_enabled(svm))
		return;

Thanks,

Paolo