From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marc Zyngier <marc.zyngier@arm.com>
Subject: Re: [PATCH v3 02/32] arm64: KVM: define HYP and Stage-2 translation
 page flags
Date: Fri, 26 Apr 2013 18:11:07 +0100
Message-ID: <517AB52B.7000006@arm.com>
References: <1365437854-30214-1-git-send-email-marc.zyngier@arm.com> <1365437854-30214-3-git-send-email-marc.zyngier@arm.com> <20130426170152.GD11311@arm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=WINDOWS-1252
Content-Transfer-Encoding: 8BIT
Cc: "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Will Deacon <Will.Deacon@arm.com>,
	Christopher Covington <cov@codeaurora.org>
To: Catalin Marinas <catalin.marinas@arm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from service87.mimecast.com ([91.220.42.44]:54736 "EHLO
	service87.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753987Ab3DZRLN convert rfc822-to-8bit (ORCPT
	<rfc822;kvm@vger.kernel.org>); Fri, 26 Apr 2013 13:11:13 -0400
In-Reply-To: <20130426170152.GD11311@arm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 26/04/13 18:01, Catalin Marinas wrote:
> On Mon, Apr 08, 2013 at 05:17:04PM +0100, Marc Zyngier wrote:
>> diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
>> index 75fd13d..acb4ee5 100644
>> --- a/arch/arm64/include/asm/pgtable-hwdef.h
>> +++ b/arch/arm64/include/asm/pgtable-hwdef.h
>> @@ -68,6 +69,18 @@
>>  #define PTE_ATTRINDX_MASK	(_AT(pteval_t, 7) << 2)
>>  
>>  /*
>> + * 2nd stage PTE definitions
>> + */
>> +#define PTE_S2_RDONLY		 (_AT(pteval_t, 1) << 6)   /* HAP[1]   */
>> +#define PTE_S2_RDWR		 (_AT(pteval_t, 2) << 6)   /* HAP[2:1] */
> 
> RDWR should be 3 here (already the case in arch/arm). BTW, I would use

Yes, Will spotted this one already, and it is now fixed in my tree.

> HAP[2:1] comment in both cases since that's the attribute field.

Indeed.

>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index e333a24..7c84ab4 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -76,6 +76,12 @@ extern pgprot_t pgprot_default;
>>  #define PAGE_KERNEL		_MOD_PROT(pgprot_default, PTE_PXN | PTE_UXN | PTE_DIRTY)
>>  #define PAGE_KERNEL_EXEC	_MOD_PROT(pgprot_default, PTE_UXN | PTE_DIRTY)
>>  
>> +#define PAGE_HYP		_MOD_PROT(pgprot_default, PTE_HYP)
>> +#define PAGE_HYP_DEVICE		_MOD_PROT(__pgprot(PROT_DEVICE_nGnRE), PTE_HYP)
>> +
>> +#define PAGE_S2			_MOD_PROT(pgprot_default, PTE_USER | PTE_S2_RDONLY)
> 
> Why is this one read-only by default?

Because the guest pages start their life mapped RO. Only on the first
write access they become writeable.

>> +#define PAGE_S2_DEVICE		_MOD_PROT(__pgprot(PROT_DEVICE_nGnRE), PTE_USER | PTE_S2_RDWR)
> 
> You could write it directly as __pgprot(PROT_DEVICE_nGnRE | PTE_USER | PTE_S2_RDWR)

Good point! This code as changed a bit anyway, as it contains some other
odd things... ;-)

Thanks for reviewing,

	M.
-- 
Jazz is not dead. It just smells funny...