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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID:
 <SA1PR12MB71998E1E70F3A03D5E30DE40B0182@SA1PR12MB7199.namprd12.prod.outlook.com>
Date: Tue, 14 Jan 2025 23:13:48 +0000
From: Ankit Agrawal <ankita@...dia.com>
To: Jason Gunthorpe <jgg@...dia.com>, David Hildenbrand <david@...hat.com>
CC: "maz@...nel.org" <maz@...nel.org>, "oliver.upton@...ux.dev"
	<oliver.upton@...ux.dev>, "joey.gouly@....com" <joey.gouly@....com>,
	"suzuki.poulose@....com" <suzuki.poulose@....com>, "yuzenghui@...wei.com"
	<yuzenghui@...wei.com>, "catalin.marinas@....com" <catalin.marinas@....com>,
	"will@...nel.org" <will@...nel.org>, "ryan.roberts@....com"
	<ryan.roberts@....com>, "shahuang@...hat.com" <shahuang@...hat.com>,
	"lpieralisi@...nel.org" <lpieralisi@...nel.org>, Aniket Agashe
	<aniketa@...dia.com>, Neo Jia <cjia@...dia.com>, Kirti Wankhede
	<kwankhede@...dia.com>, "Tarun Gupta (SW-GPU)" <targupta@...dia.com>, Vikram
 Sethi <vsethi@...dia.com>, Andy Currid <acurrid@...dia.com>, Alistair Popple
	<apopple@...dia.com>, John Hubbard <jhubbard@...dia.com>, Dan Williams
	<danw@...dia.com>, Zhi Wang <zhiw@...dia.com>, Matt Ochs <mochs@...dia.com>,
	Uday Dhoke <udhoke@...dia.com>, Dheeraj Nigam <dnigam@...dia.com>,
	"alex.williamson@...hat.com" <alex.williamson@...hat.com>,
	"sebastianene@...gle.com" <sebastianene@...gle.com>, "coltonlewis@...gle.com"
	<coltonlewis@...gle.com>, "kevin.tian@...el.com" <kevin.tian@...el.com>,
	"yi.l.liu@...el.com" <yi.l.liu@...el.com>, "ardb@...nel.org"
	<ardb@...nel.org>, "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"gshan@...hat.com" <gshan@...hat.com>, "linux-mm@...ck.org"
	<linux-mm@...ck.org>, "kvmarm@...ts.linux.dev" <kvmarm@...ts.linux.dev>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 1/1] KVM: arm64: Allow cacheable stage 2 mapping using
 VMA flags

Thanks for the responses.

> Do we really want another weirdly defined VMA flag? I'd really like to
> avoid this.. 

I'd let Catalin chime in on this. My take of the reason for his suggestion is
that we want to reduce the affected configs to only the NVIDIA grace based
systems. The nvgrace-gpu module would be setting the flag and the
new codepath will only be applicable there. Or am I missing something here?


>>>>>> Likely you assume to never end up with COW VM_PFNMAP -- I think it's
>>>>>> possible when doing a MAP_PRIVATE /dev/mem mapping on systems that allow
>>>>>> for mapping /dev/mem. Maybe one could just reject such cases (if KVM PFN
>>>>>> lookup code not already rejects them, which might just be that case IIRC).
>>>>>
>>>>> At least VFIO enforces SHARED or it won't create the VMA.
>>>>>
>>>>> drivers/vfio/pci/vfio_pci_core.c:       if ((vma->vm_flags & VM_SHARED) == 0)
>>>>
>>>> That makes a lot of sense for VFIO.
>>>
>>> So, I suppose we don't need to check this? Specially if we only extend the
>>> changes to the following case:
>
> I would check if it is a VM_PFNMAP, and outright refuse any page if
> is_cow_mapping(vma->vm_flags) is true.

So IIUC, I'll add the check to return -EINVAL for
(vma->vm_flags & VM_PFNMAP) && is_cow_mapping(vma->vm_flags)


>>> - type is VM_PFNMAP &&
>>> - user mapping is cacheable (MT_NORMAL or MT_NORMAL_TAGGED) &&
>>> - The suggested VM_FORCE_CACHED is set.
>>
>> Do we really want another weirdly defined VMA flag? I'd really like to
>> avoid this..
>
> Agreed.
> 
> Can't we do a "this is a weird VM_PFNMAP thing, let's consult the VMA
> prot + whatever PFN information to find out if it is weird-device and
> how we could safely map it?"

My understanding was that the new suggested flag VM_FORCE_CACHED
was essentially to represent "whatever PFN information to find out if it is
weird-device". Is there an alternate reliable check to figure this out?


> Ideally, we'd separate this logic from the "this is a normal VMA that
> doesn't need any such special casing", and even stop playing PFN games
> on these normal VMAs completely.

Sorry David, it isn't clear to me what normal VMA mean here? I suppose you mean
the original KVM's non-nvgrace related code piece for MT_NORMAL memory?


>> I assume MTE does not apply at all to VM_PFNMAP, at least
>> arch_calc_vm_flag_bits() tells me that VM_MTE_ALLOWED should never get set
>> there.
>
> As far as I know, it is completely platform specific what addresses MTE
> will work on. For instance, I would expect a MTE capable platform with
> CXL to want to make MTE work on the CXL memory too.
>
> However, given we have no way of discovery, limiting MTE to boot time
> system memory seems like the right thing to do for now - which we can
> achieve by forbidding it from VM_PFNMAP.
>
> Having VFIO add VM_MTE_ALLOWED someday might make sense if someone
> solves the discoverability problem.

Currently in the patch we check the following. So Jason, is the suggestion that
we simply return error to forbid such condition if VM_PFNMAP is set?
+	else if (!mte_allowed && kvm_has_mte(kvm))

- Ankit Agrawal