On 31/03/2021 10:32, David Hildenbrand wrote:

On 31.03.21 11:21, Catalin Marinas wrote:

quoted

On Wed, Mar 31, 2021 at 09:34:44AM +0200, David Hildenbrand wrote:

quoted

On 30.03.21 12:30, Catalin Marinas wrote:

quoted

On Mon, Mar 29, 2021 at 05:06:51PM +0100, Steven Price wrote:

quoted

On 28/03/2021 13:21, Catalin Marinas wrote:

quoted

On Sat, Mar 27, 2021 at 03:23:24PM +0000, Catalin Marinas wrote:

quoted

On Fri, Mar 12, 2021 at 03:18:58PM +0000, Steven Price wrote:

quoted

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 77cb2d28f2a4..b31b7a821f90 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c

@@ -879,6 +879,22 @@ static int user_mem_abort(struct kvm_vcpu 

*vcpu, phys_addr_t fault_ipa,
        if (vma_pagesize == PAGE_SIZE && !force_pte)
            vma_pagesize = transparent_hugepage_adjust(memslot, 
hva,
                                   &pfn, &fault_ipa);
+
+    if (fault_status != FSC_PERM && kvm_has_mte(kvm) && 
pfn_valid(pfn)) {
+        /*
+         * VM will be able to see the page's tags, so we must 
ensure
+         * they have been initialised. if PG_mte_tagged is set, 
tags
+         * have already been initialised.
+         */
+        struct page *page = pfn_to_page(pfn);
+        unsigned long i, nr_pages = vma_pagesize >> PAGE_SHIFT;
+
+        for (i = 0; i < nr_pages; i++, page++) {
+            if (!test_and_set_bit(PG_mte_tagged, &page->flags))
+                mte_clear_page_tags(page_address(page));
+        }
+    }

This pfn_valid() check may be problematic. Following commit 
eeb0753ba27b
("arm64/mm: Fix pfn_valid() for ZONE_DEVICE based memory"), it 
returns
true for ZONE_DEVICE memory but such memory is allowed not to 
support
MTE.

Some more thinking, this should be safe as any ZONE_DEVICE would be
mapped as untagged memory in the kernel linear map. It could be 
slightly
inefficient if it unnecessarily tries to clear tags in ZONE_DEVICE,
untagged memory. Another overhead is pfn_valid() which will likely 
end
up calling memblock_is_map_memory().

However, the bigger issue is that Stage 2 cannot disable tagging for
Stage 1 unless the memory is Non-cacheable or Device at S2. Is 
there a
way to detect what gets mapped in the guest as Normal Cacheable 
memory
and make sure it's only early memory or hotplug but no ZONE_DEVICE 
(or
something else like on-chip memory)?  If we can't guarantee that all
Cacheable memory given to a guest supports tags, we should disable 
the
feature altogether.

In stage 2 I believe we only have two types of mapping - 'normal' or
DEVICE_nGnRE (see stage2_map_set_prot_attr()). Filtering out the 
latter is a
case of checking the 'device' variable, and makes sense to avoid the
overhead you describe.

This should also guarantee that all stage-2 cacheable memory 
supports tags,
as kvm_is_device_pfn() is simply !pfn_valid(), and pfn_valid() 
should only
be true for memory that Linux considers "normal".

If you think "normal" == "normal System RAM", that's wrong; see below.

By "normal" I think both Steven and I meant the Normal Cacheable memory
attribute (another being the Device memory attribute).

Sadly there's no good standardised terminology here. Aarch64 provides 
the "normal (cacheable)" definition. Memory which is mapped as "Normal 
Cacheable" is implicitly MTE capable when shared with a guest (because 
the stage 2 mappings don't allow restricting MTE other than mapping it 
as Device memory).

So MTE also forces us to have a definition of memory which is "bog 
standard memory"[1] separate from the mapping attributes. This is the 
main memory which fully supports MTE.

Separate from the "bog standard" we have the "special"[1] memory, e.g. 
ZONE_DEVICE memory may be mapped as "Normal Cacheable" at stage 1 but 
that memory may not support MTE tags. This memory can only be safely 
shared with a guest in the following situations:

  1. MTE is completely disabled for the guest

  2. The stage 2 mappings are 'device' (e.g. DEVICE_nGnRE)

  3. We have some guarantee that guest MTE access are in some way safe.

(1) is the situation today (without this patch series). But it prevents 
the guest from using MTE in any form.

(2) is pretty terrible for general memory, but is the get-out clause for 
mapping devices into the guest.

(3) isn't something we have any architectural way of discovering. We'd 
need to know what the device did with the MTE accesses (and any caches 
between the CPU and the device) to ensure there aren't any side-channels 
or h/w lockup issues. We'd also need some way of describing this memory 
to the guest.

So at least for the time being the approach is to avoid letting a guest 
with MTE enabled have access to this sort of memory.

[1] Neither "bog standard" nor "special" are real terms - like I said 
there's a lack of standardised terminology.

quoted

quoted

quoted

That's the problem. With Anshuman's commit I mentioned above,
pfn_valid() returns true for ZONE_DEVICE mappings (e.g. persistent
memory, not talking about some I/O mapping that requires Device_nGnRE).
So kvm_is_device_pfn() is false for such memory and it may be mapped as
Normal but it is not guaranteed to support tagging.

pfn_valid() means "there is a struct page"; if you do pfn_to_page() and
touch the page, you won't fault. So Anshuman's commit is correct.

I agree.

quoted

pfn_to_online_page() means, "there is a struct page and it's system RAM
that's in use; the memmap has a sane content"

Does pfn_to_online_page() returns a valid struct page pointer for
ZONE_DEVICE pages? IIUC, these are not guaranteed to be system RAM, for
some definition of system RAM (I assume NVDIMM != system RAM). For
example, pmem_attach_disk() calls devm_memremap_pages() and this would
use the Normal Cacheable memory attribute without necessarily being
system RAM.

No, not for ZONE_DEVICE.

However, if you expose PMEM via dax/kmem as System RAM to the system (-> 
add_memory_driver_managed()), then PMEM (managed via ZONE_NOMRAL or 
ZONE_MOVABLE) would work with pfn_to_online_page() -- as the system 
thinks it's "ordinary system RAM" and the memory is managed by the buddy.

So if I'm understanding this correctly for KVM we need to use 
pfn_to_online_pages() and reject if NULL is returned? In the case of 
dax/kmem there already needs to be validation that the memory supports 
MTE (otherwise we break user space) before it's allowed into the 
"ordinary system RAM" bucket.

Steve

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel