linux-kernel - Re: [PATCH v3] kvm: svm: Use the hardware provided GPA instead of page walk

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: <d72fab48-bcad-8e83-36e7-24d669003f05@amd.com>
Date:   Tue, 29 Nov 2016 13:38:42 -0600
From:   Brijesh Singh <brijesh.singh@....com>
To:     Thomas Gleixner <tglx@...utronix.de>
CC:     <brijesh.singh@....com>, <kvm@...r.kernel.org>,
        <thomas.lendacky@....com>, <rkrcmar@...hat.com>, <joro@...tes.org>,
        <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
        <mingo@...hat.com>, <hpa@...or.com>, <pbonzini@...hat.com>,
        <bp@...e.de>
Subject: Re: [PATCH v3] kvm: svm: Use the hardware provided GPA instead of
 page walk



On 11/29/2016 12:20 PM, Thomas Gleixner wrote:
> On Tue, 29 Nov 2016, Brijesh Singh wrote:
>> --- a/arch/x86/kvm/emulate.c
>> +++ b/arch/x86/kvm/emulate.c
>> @@ -5483,3 +5483,11 @@ void emulator_writeback_register_cache(struct x86_emulate_ctxt *ctxt)
>>  {
>>  	writeback_registers(ctxt);
>>  }
>> +
>> +bool emulator_is_string_op(struct x86_emulate_ctxt *ctxt)
>> +{
>> +	if (ctxt->d & String)
>> +		return true;
>> +
>> +	return false;
>> +}
>
> Do we really need a full function call for this simple thing? Just because
> this horrible CamelCase constant is in emulate.c?
>
> What's wrong with moving that thing into a header and make it a trivial
> inline:
>
> #define INS_STRING		(1 << 13)
>
> static inline bool emulator_is_string_op(struct x86_emulate_ctxt *ctxt)
> {
> 	return ctxt->d & INS_STRING;
> }
>
> Hmm?

One of the recommendation from previous review feedback was to move the 
check inside emulator.c. I am fine with inlining it into kvm_emulate.h

Hi Paolo,

Do you want me to spin a new version ?

-Brijesh
>
> 	tglx
>