linux-kernel - Re: [PATCH V3 6/8] xen: Hide get_phys_to_machine() to be able to tune common path

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: <546476AE.9080207@suse.com>
Date:	Thu, 13 Nov 2014 10:15:26 +0100
From:	Juergen Gross <jgross@...e.com>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:	linux-kernel@...r.kernel.org, xen-devel@...ts.xensource.com,
	david.vrabel@...rix.com, boris.ostrovsky@...cle.com,
	x86@...nel.org, tglx@...utronix.de, mingo@...hat.com, hpa@...or.com
Subject: Re: [PATCH V3 6/8] xen: Hide get_phys_to_machine() to be able to
 tune common path

On 11/12/2014 11:18 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, Nov 11, 2014 at 06:43:44AM +0100, Juergen Gross wrote:
>> Today get_phys_to_machine() is always called when the mfn for a pfn
>> is to be obtained. Add a wrapper __pfn_to_mfn() as inline function
>> to be able to avoid calling get_phys_to_machine() when possible as
>
> s/when/where/

No. It's not a matter of the caller, but of the p2m list entry.

>> soon as the switch to a linear mapped p2m list has been done.
>
> But your inline function still calls get_phys_to_machine?

Sure. The switch is done in the next patch. David asked me to split
the patch doing the preparation by adding __pfn_tom_mfn() in an own
patch.

>
>
>>
>> Signed-off-by: Juergen Gross <jgross@...e.com>
>> ---
>>   arch/x86/include/asm/xen/page.h | 27 +++++++++++++++++++++------
>>   arch/x86/xen/mmu.c              |  2 +-
>>   arch/x86/xen/p2m.c              |  6 +++---
>>   3 files changed, 25 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
>> index 28fa795..07d8a7b 100644
>> --- a/arch/x86/include/asm/xen/page.h
>> +++ b/arch/x86/include/asm/xen/page.h
>> @@ -59,6 +59,22 @@ extern int clear_foreign_p2m_mapping(struct gnttab_unmap_grant_ref *unmap_ops,
>>   				     struct page **pages, unsigned int count);
>>   extern unsigned long m2p_find_override_pfn(unsigned long mfn, unsigned long pfn);
>>
>> +/*
>> + * When to use pfn_to_mfn(), __pfn_to_mfn() or get_phys_to_machine():
>> + * - pfn_to_mfn() returns either INVALID_P2M_ENTRY or the mfn. In case of an
>> + *   identity entry the identity indicator will be cleared.
>
> Why don't you say : In case of identity PFN the same PFN is returned.
>
> But you did miss that also the FOREIGN_FRAME_BIT is cleared.

I'll reword the comment.

>
>> + * - __pfn_to_mfn() returns the found entry of the p2m table. A possibly set
>
> s/of the/in the/
>> + *   identity indicator will be still set. __pfn_to_mfn() is encapsulating
> .. be still set if the PFN is an identity one.
>> + *   get_phys_to_machine() and might skip that function if possible to speed
>> + *   up the common path.
>
> How is is skipping that function? The patch below does no such thing?

The next patch in this series does.

>
>> + * - get_phys_to_machine() is basically the same as __pfn_to_mfn(), but
>> + *   without any short cuts for the common fast path.
>
> Right. Perhpas we should call it 'slow_p2m' instead of the 'get_phys_to_machine'.

That's a matter of taste, I think. I can change it if nobody else
objects.


Juergen
--
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/