| 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
| ||
|
Message-ID: <20250619141631.GD1643312@nvidia.com>
Date: Thu, 19 Jun 2025 11:16:31 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Ankit Agrawal <ankita@...dia.com>
Cc: Catalin Marinas <catalin.marinas@....com>,
"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>,
"will@...nel.org" <will@...nel.org>,
"ryan.roberts@....com" <ryan.roberts@....com>,
"shahuang@...hat.com" <shahuang@...hat.com>,
"lpieralisi@...nel.org" <lpieralisi@...nel.org>,
"david@...hat.com" <david@...hat.com>,
"ddutile@...hat.com" <ddutile@...hat.com>,
"seanjc@...gle.com" <seanjc@...gle.com>,
Aniket Agashe <aniketa@...dia.com>, Neo Jia <cjia@...dia.com>,
Kirti Wankhede <kwankhede@...dia.com>,
Krishnakant Jaju <kjaju@...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>,
"tabba@...gle.com" <tabba@...gle.com>,
"qperret@...gle.com" <qperret@...gle.com>,
"kvmarm@...ts.linux.dev" <kvmarm@...ts.linux.dev>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
"maobibo@...ngson.cn" <maobibo@...ngson.cn>
Subject: Re: [PATCH v7 4/5] KVM: arm64: Allow cacheable stage 2 mapping using
VMA flags
On Thu, Jun 19, 2025 at 12:14:38PM +0000, Ankit Agrawal wrote:
> >> > - disable_cmo = true;
> >> > + if (!is_vma_cacheable)
> >> > + disable_cmo = true;
> >>
> >> I'm tempted to stick to the 'device' variable name. Or something like
> >> s2_noncacheable. As I commented, it's not just about disabling CMOs.
> >
> > I think it would be clearer to have two concepts/variable then because
> > the cases where it is really about preventing cachable access to
> > prevent aborts are not linked to the logic that checks pfn valid. We
> > have to detect those cases separately (through the VMA flags was it?).
> >
> > Having these two things together is IMHO confusing..
> >
> > Jason
>
> Thanks Catalin and Jason for the comments.
>
> Considering the feedback, I think we may do the following here:
> 1. Rename the device variable to S2_noncacheable to represent if the S2
> is going to be marked non cacheable. Otherwise S2 will be mapped
> NORMAL.
How about "s2_force_noncachable" for extra clarity what is going on.
> 2. Detect what PFN has to be marked S2_noncacheable. If a PFN is not in the
> kernel map, mark as S2 except for PFNMAP + VMA cacheable.
> 3. Prohibit cacheable PFNMAP if hardware doesn't support FWB and CACHE DIC.
> 4. Prohibit S2 non cached mapping for cacheable VMA for all cases, whether
> pre-FWB hardware or not.
Logic sounds right
> This would be how the patch would look.
>
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 339194441a25..979668d475bd 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1516,8 +1516,8 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> {
> int ret = 0;
> bool write_fault, writable, force_pte = false;
> - bool exec_fault, mte_allowed, is_vma_cacheable;
> - bool device = false, vfio_allow_any_uc = false;
> + bool exec_fault, mte_allowed, is_vma_cacheable, cacheable_pfnmap = false;
> + bool s2_noncacheable = false, vfio_allow_any_uc = false;
> unsigned long mmu_seq;
> phys_addr_t ipa = fault_ipa;
> struct kvm *kvm = vcpu->kvm;
> @@ -1660,6 +1660,15 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>
> is_vma_cacheable = kvm_vma_is_cacheable(vma);
>
> + if (vma->vm_flags & VM_PFNMAP) {
> + /* Reject COW VM_PFNMAP */
> + if (is_cow_mapping(vma->vm_flags))
> + return -EINVAL;
The comment should explain why we have to reject COW PFNMAP, it is
obvious that is what the code does.
> +
> + if (is_vma_cacheable)
> + cacheable_pfnmap = true;
> + }
> +
> /* Don't use the VMA after the unlock -- it may have vanished */
> vma = NULL;
>
> @@ -1684,8 +1693,16 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> return -EFAULT;
>
> if (kvm_is_device_pfn(pfn)) {
We are changing this to !pfn_is_map_memory() ?
We should really only call pfn_is_map_memory() if VM_PFNMAP or
VM_MIXEDMAP, otherwise the VMA has only struct pages in it.
Can it look more like this?
if (vm_flags & (VM_PFNMAP | VM_MIXEDMAP) && !pfn_is_map_memory()) {
/* the memory is non-struct page memory, it cannot be cache flushed
and may be unsafe to be accessed as cachable */
if (cachable_pfnmap) {
/* the VMA owner has said the physical address is safe for cachable
access. When FWB ..... */
if (!kvm_arch_supports_cacheable_pfnmap())
return -EFAULT;
/* Cannot degrade cachable to non cachable */
if (s2_force_noncachable)
return -EINVAL;
} else {
/* Assume the address is unsafe for cachable access */
s2_force_noncachable = true;
}
}
/* nothing beyond here writes to s2_forcE_noncachable? */
Jason
Powered by blists - more mailing lists