diff mbox

[01/15] ARM: add mem_type prot_pte accessor

Message ID 20120915153436.21241.95691.stgit@ubuntu (mailing list archive)
State New, archived
Headers show

Commit Message

Christoffer Dall Sept. 15, 2012, 3:34 p.m. UTC
From: Marc Zyngier <marc.zyngier@arm.com>

The KVM hypervisor mmu code requires access to the mem_type prot_pte
field when setting up page tables pointing to a device. Unfortunately,
the mem_type structure is opaque.

Add an accessor (get_mem_type_prot_pte()) to retrieve the prot_pte
value.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
---
 arch/arm/include/asm/mach/map.h |    1 +
 arch/arm/mm/mmu.c               |    6 ++++++
 2 files changed, 7 insertions(+)


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Will Deacon Sept. 18, 2012, 12:23 p.m. UTC | #1
On Sat, Sep 15, 2012 at 04:34:36PM +0100, Christoffer Dall wrote:
> From: Marc Zyngier <marc.zyngier@arm.com>
> 
> The KVM hypervisor mmu code requires access to the mem_type prot_pte
> field when setting up page tables pointing to a device. Unfortunately,
> the mem_type structure is opaque.
> 
> Add an accessor (get_mem_type_prot_pte()) to retrieve the prot_pte
> value.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
> ---
>  arch/arm/include/asm/mach/map.h |    1 +
>  arch/arm/mm/mmu.c               |    6 ++++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/arch/arm/include/asm/mach/map.h b/arch/arm/include/asm/mach/map.h
> index a6efcdd..3787c9f 100644
> --- a/arch/arm/include/asm/mach/map.h
> +++ b/arch/arm/include/asm/mach/map.h
> @@ -37,6 +37,7 @@ extern void iotable_init(struct map_desc *, int);
>  
>  struct mem_type;
>  extern const struct mem_type *get_mem_type(unsigned int type);
> +extern pteval_t get_mem_type_prot_pte(unsigned int type);
>  /*
>   * external interface to remap single page with appropriate type
>   */
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 4c2d045..76bf4f5 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -301,6 +301,12 @@ const struct mem_type *get_mem_type(unsigned int type)
>  }
>  EXPORT_SYMBOL(get_mem_type);
>  
> +pteval_t get_mem_type_prot_pte(unsigned int type)
> +{
> +	return get_mem_type(type)->prot_pte;
> +}
> +EXPORT_SYMBOL(get_mem_type_prot_pte);
> +

get_mem_type can return NULL, so you should probably pass the error through
rather than dereferencing it.

Will
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux Sept. 18, 2012, 9:04 p.m. UTC | #2
On Sat, Sep 15, 2012 at 11:34:36AM -0400, Christoffer Dall wrote:
> From: Marc Zyngier <marc.zyngier@arm.com>
> 
> The KVM hypervisor mmu code requires access to the mem_type prot_pte
> field when setting up page tables pointing to a device. Unfortunately,
> the mem_type structure is opaque.
> 
> Add an accessor (get_mem_type_prot_pte()) to retrieve the prot_pte
> value.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>

Is there a reason why we need this to be exposed, along with all the
page table manipulation in patch 7?

Is there a reason why we can't have new MT_ types for PAGE_HYP and
the HYP MT_DEVICE type (which is the same as MT_DEVICE but with
PTE_USER set) and have the standard ARM/generic kernel code build
those mappings?

That would (it seems) also avoid the need to export the pXd_clear_bad()
acessors too...
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoffer Dall Sept. 18, 2012, 9:53 p.m. UTC | #3
On Tue, Sep 18, 2012 at 5:04 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sat, Sep 15, 2012 at 11:34:36AM -0400, Christoffer Dall wrote:
>> From: Marc Zyngier <marc.zyngier@arm.com>
>>
>> The KVM hypervisor mmu code requires access to the mem_type prot_pte
>> field when setting up page tables pointing to a device. Unfortunately,
>> the mem_type structure is opaque.
>>
>> Add an accessor (get_mem_type_prot_pte()) to retrieve the prot_pte
>> value.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
>
> Is there a reason why we need this to be exposed, along with all the
> page table manipulation in patch 7?
>
> Is there a reason why we can't have new MT_ types for PAGE_HYP and
> the HYP MT_DEVICE type (which is the same as MT_DEVICE but with
> PTE_USER set) and have the standard ARM/generic kernel code build
> those mappings?

For hyp mode we can do this, but we cannot do this for the cpu
interfaces that need to be mapped into each VM as they have each their
own pgd. We can move the Hyp mode mappings, and I was playing with the
though of having a PAGE_KVM_DEVICE set in pgtable.h and a
pgprot_guest_device setup in build_mem_type_table.

What do you think?

>
> That would (it seems) also avoid the need to export the pXd_clear_bad()
> acessors too...

for hyp mode mappings this would be true, then.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier Sept. 20, 2012, 10:01 a.m. UTC | #4
On 18/09/12 22:53, Christoffer Dall wrote:
> On Tue, Sep 18, 2012 at 5:04 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Sat, Sep 15, 2012 at 11:34:36AM -0400, Christoffer Dall wrote:
>>> From: Marc Zyngier <marc.zyngier@arm.com>
>>>
>>> The KVM hypervisor mmu code requires access to the mem_type prot_pte
>>> field when setting up page tables pointing to a device. Unfortunately,
>>> the mem_type structure is opaque.
>>>
>>> Add an accessor (get_mem_type_prot_pte()) to retrieve the prot_pte
>>> value.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
>>
>> Is there a reason why we need this to be exposed, along with all the
>> page table manipulation in patch 7?
>>
>> Is there a reason why we can't have new MT_ types for PAGE_HYP and
>> the HYP MT_DEVICE type (which is the same as MT_DEVICE but with
>> PTE_USER set) and have the standard ARM/generic kernel code build
>> those mappings?
> 
> For hyp mode we can do this, but we cannot do this for the cpu
> interfaces that need to be mapped into each VM as they have each their
> own pgd. 

Isn't that the same problem? The HYP mode has its own pgd too. I think
this is the main issue with the generic code. If we can come up with an
interface that allows the generic code to work on alternative pgds, we
could pretty much do what Russell suggests here.

	M.
Christoffer Dall Sept. 20, 2012, 1:21 p.m. UTC | #5
On Thu, Sep 20, 2012 at 6:01 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 18/09/12 22:53, Christoffer Dall wrote:
>> On Tue, Sep 18, 2012 at 5:04 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>> On Sat, Sep 15, 2012 at 11:34:36AM -0400, Christoffer Dall wrote:
>>>> From: Marc Zyngier <marc.zyngier@arm.com>
>>>>
>>>> The KVM hypervisor mmu code requires access to the mem_type prot_pte
>>>> field when setting up page tables pointing to a device. Unfortunately,
>>>> the mem_type structure is opaque.
>>>>
>>>> Add an accessor (get_mem_type_prot_pte()) to retrieve the prot_pte
>>>> value.
>>>>
>>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>>> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
>>>
>>> Is there a reason why we need this to be exposed, along with all the
>>> page table manipulation in patch 7?
>>>
>>> Is there a reason why we can't have new MT_ types for PAGE_HYP and
>>> the HYP MT_DEVICE type (which is the same as MT_DEVICE but with
>>> PTE_USER set) and have the standard ARM/generic kernel code build
>>> those mappings?
>>
>> For hyp mode we can do this, but we cannot do this for the cpu
>> interfaces that need to be mapped into each VM as they have each their
>> own pgd.
>
> Isn't that the same problem? The HYP mode has its own pgd too. I think
> this is the main issue with the generic code. If we can come up with an
> interface that allows the generic code to work on alternative pgds, we
> could pretty much do what Russell suggests here.
>
Hyp mode has its own pgd, but there will only be one of them which can
be allocated at boot and setup at boot in mmu.c

This will not be the case for guest pages.  On the other hand, this
locks us to only one user of such mappings and more users could
potentially (I know it's a stretch) clutter mmu.c later on, which is
why I suggest the PAGE_KVM_DEVICE approach for now, which should then
be renamed, PAGE_S2_DEVICE probably.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/include/asm/mach/map.h b/arch/arm/include/asm/mach/map.h
index a6efcdd..3787c9f 100644
--- a/arch/arm/include/asm/mach/map.h
+++ b/arch/arm/include/asm/mach/map.h
@@ -37,6 +37,7 @@  extern void iotable_init(struct map_desc *, int);
 
 struct mem_type;
 extern const struct mem_type *get_mem_type(unsigned int type);
+extern pteval_t get_mem_type_prot_pte(unsigned int type);
 /*
  * external interface to remap single page with appropriate type
  */
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 4c2d045..76bf4f5 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -301,6 +301,12 @@  const struct mem_type *get_mem_type(unsigned int type)
 }
 EXPORT_SYMBOL(get_mem_type);
 
+pteval_t get_mem_type_prot_pte(unsigned int type)
+{
+	return get_mem_type(type)->prot_pte;
+}
+EXPORT_SYMBOL(get_mem_type_prot_pte);
+
 /*
  * Adjust the PMD section entries according to the CPU in use.
  */