diff mbox series

[v3,06/14] arm64: Override set_fixmap_io

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

Commit Message

Steven Price June 5, 2024, 9:29 a.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>
---
 arch/arm64/include/asm/fixmap.h |  4 +++-
 arch/arm64/mm/mmu.c             | 13 +++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

Comments

Catalin Marinas June 10, 2024, 5:49 p.m. UTC | #1 
On Wed, Jun 05, 2024 at 10:29:58AM +0100, Steven Price wrote:
> +void set_fixmap_io(enum fixed_addresses idx, phys_addr_t phys)
> +{
> +	pgprot_t prot = FIXMAP_PAGE_IO;
> +
> +	/*
> +	 * For now we consider all I/O as non-secure. For future
> +	 * filter the I/O base for setting appropriate permissions.
> +	 */
> +	prot = __pgprot(pgprot_val(prot) | PROT_NS_SHARED);
> +
> +	return __set_fixmap(idx, phys, prot);
> +}

In v2, Suzuki said that we want to keep this as a function rather than
just adding PROT_NS_SHARED to FIXMAP_PAGE_IO in case we want to change
this function in the future to allow protected MMIO.

https://lore.kernel.org/linux-arm-kernel/6ba1fd72-3bad-44ca-810d-572b70050772@arm.com/

What I don't understand is that all the other MMIO cases just statically
assume unprotected/shard MMIO. Should we drop this patch here as well,
adjust FIXMAP_PAGE_IO and think about protected MMIO later when we
actually have to do device assignment?
Steven Price June 27, 2024, 1:56 p.m. UTC | #2 
On 10/06/2024 18:49, Catalin Marinas wrote:
> On Wed, Jun 05, 2024 at 10:29:58AM +0100, Steven Price wrote:
>> +void set_fixmap_io(enum fixed_addresses idx, phys_addr_t phys)
>> +{
>> +	pgprot_t prot = FIXMAP_PAGE_IO;
>> +
>> +	/*
>> +	 * For now we consider all I/O as non-secure. For future
>> +	 * filter the I/O base for setting appropriate permissions.
>> +	 */
>> +	prot = __pgprot(pgprot_val(prot) | PROT_NS_SHARED);
>> +
>> +	return __set_fixmap(idx, phys, prot);
>> +}
> 
> In v2, Suzuki said that we want to keep this as a function rather than
> just adding PROT_NS_SHARED to FIXMAP_PAGE_IO in case we want to change
> this function in the future to allow protected MMIO.
> 
> https://lore.kernel.org/linux-arm-kernel/6ba1fd72-3bad-44ca-810d-572b70050772@arm.com/
> 
> What I don't understand is that all the other MMIO cases just statically
> assume unprotected/shard MMIO. Should we drop this patch here as well,
> adjust FIXMAP_PAGE_IO and think about protected MMIO later when we
> actually have to do device assignment?

I agree, there's not much point in this patch as it stands. I'll drop it
(and the previous one) for the next version of the series. We can add it
back if needed when protected MMIO is a thing.

Thanks,
Steve
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
index 87e307804b99..f765943b088c 100644
--- a/arch/arm64/include/asm/fixmap.h
+++ b/arch/arm64/include/asm/fixmap.h
@@ -107,7 +107,9 @@  void __init early_fixmap_init(void);
 #define __late_set_fixmap __set_fixmap
 #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);
+void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot);
 
 #include <asm-generic/fixmap.h>
 
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index c927e9312f10..9123df312842 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1192,6 +1192,19 @@  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;
+
+	/*
+	 * For now we consider all I/O as non-secure. For future
+	 * filter the I/O base for setting appropriate permissions.
+	 */
+	prot = __pgprot(pgprot_val(prot) | PROT_NS_SHARED);
+
+	return __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));