diff mbox series

[v5,10/19] arm64: Override set_fixmap_io

Message ID 20240819131924.372366-11-steven.price@arm.com (mailing list archive)
State New
Headers show
Series arm64: Support for running as a guest in Arm CCA | expand

Commit Message

Steven Price Aug. 19, 2024, 1:19 p.m. UTC
From: Suzuki K Poulose <suzuki.poulose@arm.com>

Override the set_fixmap_io to set shared permission for the host
in case of a CC guest. For now we mark it shared unconditionally.

If/when support for device assignment and device emulation in the realm
is added in the future then this will need to filter the physical
address and make the decision accordingly.

Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Steven Price <steven.price@arm.com>
---
New patch for v5
---
 arch/arm64/include/asm/fixmap.h |  2 ++
 arch/arm64/mm/mmu.c             | 17 +++++++++++++++++
 2 files changed, 19 insertions(+)

Comments

Suzuki K Poulose Aug. 19, 2024, 2:13 p.m. UTC | #1
On 19/08/2024 14:19, Steven Price wrote:
> From: Suzuki K Poulose <suzuki.poulose@arm.com>
> 
> Override the set_fixmap_io to set shared permission for the host
> in case of a CC guest. For now we mark it shared unconditionally.
> 
> If/when support for device assignment and device emulation in the realm
> is added in the future then this will need to filter the physical
> address and make the decision accordingly.
> 
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> New patch for v5
> ---
>   arch/arm64/include/asm/fixmap.h |  2 ++
>   arch/arm64/mm/mmu.c             | 17 +++++++++++++++++
>   2 files changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
> index 87e307804b99..2c20da3a468c 100644
> --- a/arch/arm64/include/asm/fixmap.h
> +++ b/arch/arm64/include/asm/fixmap.h
> @@ -108,6 +108,8 @@ void __init early_fixmap_init(void);
>   #define __late_clear_fixmap(idx) __set_fixmap((idx), 0, FIXMAP_PAGE_CLEAR)
>   
>   extern void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
> +#define set_fixmap_io set_fixmap_io
> +void set_fixmap_io(enum fixed_addresses idx, phys_addr_t phys);
>   
>   #include <asm-generic/fixmap.h>
>   
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 353ea5dc32b8..06b66c23c124 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1193,6 +1193,23 @@ void vmemmap_free(unsigned long start, unsigned long end,
>   }
>   #endif /* CONFIG_MEMORY_HOTPLUG */
>   
> +void set_fixmap_io(enum fixed_addresses idx, phys_addr_t phys)
> +{
> +	pgprot_t prot = FIXMAP_PAGE_IO;
> +
> +	/*
> +	 * The set_fixmap_io maps a single Page covering phys.
> +	 * To make better decision, we stick to the smallest page
> +	 * size supported (4K).
> +	 */
> +	if (!arm64_is_iomem_private(phys, SZ_4K))
> +		prot = pgprot_decrypted(prot);
> +	else
> +		prot = pgprot_encrypted(prot);

With the ioremap_prot_hook introduction, this one looks like should use
that, instead of open coding the same thing. Thoughts ?

Suzuki
Steven Price Aug. 30, 2024, 3:54 p.m. UTC | #2
On 19/08/2024 15:13, Suzuki K Poulose wrote:
> On 19/08/2024 14:19, Steven Price wrote:
>> From: Suzuki K Poulose <suzuki.poulose@arm.com>
>>
>> Override the set_fixmap_io to set shared permission for the host
>> in case of a CC guest. For now we mark it shared unconditionally.
>>
>> If/when support for device assignment and device emulation in the realm
>> is added in the future then this will need to filter the physical
>> address and make the decision accordingly.
>>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> New patch for v5
>> ---
>>   arch/arm64/include/asm/fixmap.h |  2 ++
>>   arch/arm64/mm/mmu.c             | 17 +++++++++++++++++
>>   2 files changed, 19 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/fixmap.h
>> b/arch/arm64/include/asm/fixmap.h
>> index 87e307804b99..2c20da3a468c 100644
>> --- a/arch/arm64/include/asm/fixmap.h
>> +++ b/arch/arm64/include/asm/fixmap.h
>> @@ -108,6 +108,8 @@ void __init early_fixmap_init(void);
>>   #define __late_clear_fixmap(idx) __set_fixmap((idx), 0,
>> FIXMAP_PAGE_CLEAR)
>>     extern void __set_fixmap(enum fixed_addresses idx, phys_addr_t
>> phys, pgprot_t prot);
>> +#define set_fixmap_io set_fixmap_io
>> +void set_fixmap_io(enum fixed_addresses idx, phys_addr_t phys);
>>     #include <asm-generic/fixmap.h>
>>   diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 353ea5dc32b8..06b66c23c124 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -1193,6 +1193,23 @@ void vmemmap_free(unsigned long start, unsigned
>> long end,
>>   }
>>   #endif /* CONFIG_MEMORY_HOTPLUG */
>>   +void set_fixmap_io(enum fixed_addresses idx, phys_addr_t phys)
>> +{
>> +    pgprot_t prot = FIXMAP_PAGE_IO;
>> +
>> +    /*
>> +     * The set_fixmap_io maps a single Page covering phys.
>> +     * To make better decision, we stick to the smallest page
>> +     * size supported (4K).
>> +     */
>> +    if (!arm64_is_iomem_private(phys, SZ_4K))
>> +        prot = pgprot_decrypted(prot);
>> +    else
>> +        prot = pgprot_encrypted(prot);
> 
> With the ioremap_prot_hook introduction, this one looks like should use
> that, instead of open coding the same thing. Thoughts ?

I can see the logic, but it's not quite that simple to implement. Either
the ioremap_prot_hook needs moving out (and presumably) renaming to be a
more generic "override device pgprot" mechanism, we it would be a horrid
hack to put ioremap code in arch/arm64/mm/ioremap.c to handle this.

Given that we're not even sure that the set_fixmap hack is a good idea,
I'll leave this alone for now - the open coding is clear even if it's a
duplicate.

Thanks,
Steve
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
index 87e307804b99..2c20da3a468c 100644
--- a/arch/arm64/include/asm/fixmap.h
+++ b/arch/arm64/include/asm/fixmap.h
@@ -108,6 +108,8 @@  void __init early_fixmap_init(void);
 #define __late_clear_fixmap(idx) __set_fixmap((idx), 0, FIXMAP_PAGE_CLEAR)
 
 extern void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
+#define set_fixmap_io set_fixmap_io
+void set_fixmap_io(enum fixed_addresses idx, phys_addr_t phys);
 
 #include <asm-generic/fixmap.h>
 
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 353ea5dc32b8..06b66c23c124 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1193,6 +1193,23 @@  void vmemmap_free(unsigned long start, unsigned long end,
 }
 #endif /* CONFIG_MEMORY_HOTPLUG */
 
+void set_fixmap_io(enum fixed_addresses idx, phys_addr_t phys)
+{
+	pgprot_t prot = FIXMAP_PAGE_IO;
+
+	/*
+	 * The set_fixmap_io maps a single Page covering phys.
+	 * To make better decision, we stick to the smallest page
+	 * size supported (4K).
+	 */
+	if (!arm64_is_iomem_private(phys, SZ_4K))
+		prot = pgprot_decrypted(prot);
+	else
+		prot = pgprot_encrypted(prot);
+
+	__set_fixmap(idx, phys, prot);
+}
+
 int pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot)
 {
 	pud_t new_pud = pfn_pud(__phys_to_pfn(phys), mk_pud_sect_prot(prot));