linux-kernel - Re: [PATCH v2] x86: svm: use kvm_fast_pio

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] [day] [month] [year] [list]

Date:	Mon, 2 Mar 2015 14:57:50 -0600
From:	Joel Schopp <joel.schopp@....com>
To:	Radim Krčmář <rkrcmar@...hat.com>
CC:	Gleb Natapov <gleb@...nel.org>,
	Paolo Bonzini <pbonzini@...hat.com>, <kvm@...r.kernel.org>,
	David Kaplan <David.Kaplan@....com>,
	Joerg Roedel <joro@...tes.org>, <linux-kernel@...r.kernel.org>,
	Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH v2] x86: svm: use kvm_fast_pio_in()


>
>>   		return emulate_instruction(vcpu, 0) == EMULATE_DONE;
>>   
>>   	port = io_info >> 16;
>> @@ -1907,6 +1907,8 @@ static int io_interception(struct vcpu_svm *svm)
>>   	svm->next_rip = svm->vmcb->control.exit_info_2;
>>   	skip_emulated_instruction(&svm->vcpu);
>>   
>> +	if (in)
>> +		return kvm_fast_pio_in(vcpu, size, port);
>>   	return kvm_fast_pio_out(vcpu, size, port);
> (kvm_fast_pio() comes to mind.)
If you combined them you'd have to have an extra argument to say if it 
was in or out. You'd then have to have code to parse that.  I prefer 
this way.

>
>>   }
>>   
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index bd7a70b..089247c 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -5463,6 +5463,39 @@ int kvm_fast_pio_out(struct kvm_vcpu *vcpu, int size, unsigned short port)
>>   }
>>   EXPORT_SYMBOL_GPL(kvm_fast_pio_out);
>>   
>> +static int complete_fast_pio(struct kvm_vcpu *vcpu)
>> +{
>> +	u32 new_rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
> u64.
Good call.  I'll use unsigned long like kvm_fast_pio_out() uses.

> arch/x86/kvm/x86.c
>
>> +
>> +	if (!vcpu->arch.pio.count)
>> +		return 0;
>> +	if (vcpu->arch.pio.count * vcpu->arch.pio.size > 8)
>> +		return 0;
> sizeof(new_rax).  (safer and easier to understand)
>
> Both should never happen in KVM code, BUG_ON().
Agreed on both counts.

>
>> +
>> +	memcpy(&new_rax, vcpu->arch.pio_data,
>> +	       vcpu->arch.pio.count * vcpu->arch.pio.size);
> Use emulator_pio_in_emulated() here, for code sharing.
> (We want to trace the read here too;  it could be better to split
>   the path from emulator_pio_in_emulated() first.)
I looked at pulling this out, it was a painful.  I'll add the trace hook.

>
>> +	kvm_register_write(vcpu, VCPU_REGS_RAX, new_rax);
>> +
>> +	vcpu->arch.pio.count = 0;
>> +	return 1;
>> +}
>> +
>> +int kvm_fast_pio_in(struct kvm_vcpu *vcpu, int size, unsigned short port)
>> +{
>> +	unsigned long val;
>> +	int ret = emulator_pio_in_emulated(&vcpu->arch.emulate_ctxt, size,
>> +					   port, &val, 1);
>> +
>> +	if (ret) {
>> +		kvm_register_write(vcpu, VCPU_REGS_RAX, val);
>> +		vcpu->arch.pio.count = 0;
> (emulator_pio_in_emulated() sets count to zero if it returns true.)
will remove = 0 line
--
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/