linux-kernel - Re: [PATCH v2 1/1] KVM: arm64: Allow cacheable stage 2 mapping using VMA flags

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241210141334.GD15607@willie-the-truck>
Date: Tue, 10 Dec 2024 14:13:35 +0000
From: Will Deacon <will@...nel.org>
To: ankita@...dia.com
Cc: jgg@...dia.com, maz@...nel.org, oliver.upton@...ux.dev,
	joey.gouly@....com, suzuki.poulose@....com, yuzenghui@...wei.com,
	catalin.marinas@....com, ryan.roberts@....com, shahuang@...hat.com,
	lpieralisi@...nel.org, aniketa@...dia.com, cjia@...dia.com,
	kwankhede@...dia.com, targupta@...dia.com, vsethi@...dia.com,
	acurrid@...dia.com, apopple@...dia.com, jhubbard@...dia.com,
	danw@...dia.com, zhiw@...dia.com, mochs@...dia.com,
	udhoke@...dia.com, dnigam@...dia.com, alex.williamson@...hat.com,
	sebastianene@...gle.com, coltonlewis@...gle.com,
	kevin.tian@...el.com, yi.l.liu@...el.com, ardb@...nel.org,
	akpm@...ux-foundation.org, gshan@...hat.com, linux-mm@...ck.org,
	kvmarm@...ts.linux.dev, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 1/1] KVM: arm64: Allow cacheable stage 2 mapping using
 VMA flags

On Mon, Nov 18, 2024 at 01:19:58PM +0000, ankita@...dia.com wrote:
> From: Ankit Agrawal <ankita@...dia.com>
> 
> Currently KVM determines if a VMA is pointing at IO memory by checking
> pfn_is_map_memory(). However, the MM already gives us a way to tell what
> kind of memory it is by inspecting the VMA.
> 
> This patch solves the problems where it is possible for the kernel to
> have VMAs pointing at cachable memory without causing
> pfn_is_map_memory() to be true, eg DAX memremap cases and CXL/pre-CXL
> devices. This memory is now properly marked as cachable in KVM.
> 
> The pfn_is_map_memory() is restrictive and allows only for the memory
> that is added to the kernel to be marked as cacheable. In most cases
> the code needs to know if there is a struct page, or if the memory is
> in the kernel map and pfn_valid() is an appropriate API for this.
> Extend the umbrella with pfn_valid() to include memory with no struct
> pages for consideration to be mapped cacheable in stage 2. A !pfn_valid()
> implies that the memory is unsafe to be mapped as cacheable.
> 
> Moreover take account of the mapping type in the VMA to make a decision
> on the mapping. The VMA's pgprot is tested to determine the memory type
> with the following mapping:
>  pgprot_noncached    MT_DEVICE_nGnRnE   device (or Normal_NC)
>  pgprot_writecombine MT_NORMAL_NC       device (or Normal_NC)
>  pgprot_device       MT_DEVICE_nGnRE    device (or Normal_NC)
>  pgprot_tagged       MT_NORMAL_TAGGED   RAM / Normal
>  -                   MT_NORMAL          RAM / Normal
> 
> Also take care of the following two cases that prevents the memory to
> be safely mapped as cacheable:
> 1. The VMA pgprot have VM_IO set alongwith MT_NORMAL or
>    MT_NORMAL_TAGGED. Although unexpected and wrong, presence of such
>    configuration cannot be ruled out.
> 2. Configurations where VM_MTE_ALLOWED is not set and KVM_CAP_ARM_MTE
>    is enabled. Otherwise a malicious guest can enable MTE at stage 1
>    without the hypervisor being able to tell. This could cause external
>    aborts.
> 
> Introduce a new variable noncacheable to represent whether the memory
> should not be mapped as cacheable. The noncacheable as false implies
> the memory is safe to be mapped cacheable. Use this to handle the
> aforementioned potentially unsafe cases for cacheable mapping.
> 
> Note when FWB is not enabled, the kernel expects to trivially do
> cache management by flushing the memory by linearly converting a
> kvm_pte to phys_addr to a KVA, see kvm_flush_dcache_to_poc(). This is
> only possibile for struct page backed memory. Do not allow non-struct
> page memory to be cachable without FWB.
> 
> The device memory such as on the Grace Hopper systems is interchangeable
> with DDR memory and retains its properties. Allow executable faults
> on the memory determined as Normal cacheable.

Sorry, but a change this subtle to the arch code is going to need a _much_
better explanation than the rambling text above.

I also suspect that you're trying to do too many things in one patch.
In particular, the changes to execute permission handling absolutely
need to be discussed as a single patch in the series.

Please can you split this up in to a set of changes with useful commit
messages so that we can review them?

Jason knows how to do this so you could ask him for help if you're stuck.
Otherwise, there are plenty of examples of well-written commit messages
on the mailing list and in the git history.

Thanks,

Will