From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yx1-f49.google.com (mail-yx1-f49.google.com [74.125.224.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 872AF35E1D8 for ; Fri, 10 Jul 2026 17:08:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.224.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783703337; cv=none; b=Oz+DVKoF97qPLeNTTDigK2RdW8GKPOojVBKw8cNi0Rl+8q+p1/Bc1+tfsAzDONxgosgRJMFlzHMFCwPI2TCNxcG0EN20RpqNykHlxe1s/lV58uLNIhVCBvmgdid6ZwodHAzBhYkoNs30mM5bQf1w7svY0JS+ZkJMCLIRN0Qzu8c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783703337; c=relaxed/simple; bh=g5tTg7DEkv7I+90dPyEuJLJcUEiNTQpXx5FFwVa7534=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Pfj9uIpH9MUoE0LWkMYUKAarQkvAGTKBif5JwTTxLx+eXSIHVPmh2hJ9FzaoCW8DOM+U9+IWp7Wd+/qV2X0L7/Gd2wi8gz96Hqkb2mUtUtkyCw7YorxFJg2TrTjSlYNJOnv2t5s73s6QMT7BekgUPMRqRMvgI/TIToh1FzqXKXE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=D5n8CnsJ; arc=none smtp.client-ip=74.125.224.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="D5n8CnsJ" Received: by mail-yx1-f49.google.com with SMTP id 956f58d0204a3-664c535f1a0so1976844d50.1 for ; Fri, 10 Jul 2026 10:08:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1783703334; x=1784308134; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :content-type:mime-version:references:message-id:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to:content-type; bh=/HsR2t+CM1gEbj7YDybNgOOuROSnjWp8XwwNRjSK1d0=; b=D5n8CnsJN81eECw0/bv0temsTfllrqTQuCKiyBiesI6L2m5Ov4YOqIQdg0ARMShSm/ K1LH2X2Tc2jxvXweT2tQTWPzvmkq+KkPfjzzct7DJ9o9hu33uoyS+RzO738+emWyGI0e U8hOwAfLd1P4reQb6dwZg5Ia9b+MryEAlLYasx3MT8+lxgDcGCEfMDqM1zeOIKXN17Jp R8+jSBCP0yvSHCWRZSQea1GnClMJ8bmaL+E8HJGTrCtHDDw0qAC0AhCUSIFVqiRFMi6m /4LwWodMzAi6vvoBRWgthpE3gSPrN3hAv1SVs43Fl41rKiBbCp3PrF4Zt7oeVlVy/kyU YkNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783703334; x=1784308134; h=in-reply-to:content-transfer-encoding:content-disposition :content-type:mime-version:references:message-id:subject:cc:to:from :date:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to:content-type; bh=/HsR2t+CM1gEbj7YDybNgOOuROSnjWp8XwwNRjSK1d0=; b=Pkw0GS7OLvUuBaxvNE0JU0F863nc+yrSeoV/nnUbUGrkm6u6bhpA737XUlJCQzbmbd Od28TUsZHr2DsH/DfCzuAM+/ZCNyjMOVXHKOUq16DpEuUX7FZbNCkxJuC1vqQSDn4a6S Zm963JjaHSA858dasUoGTnezf0FNXceP+fnN2z5rJh5rTU7va6gV2iWVhmhY87a3s//G 31PMpzixjj6O/IC5a7Jaa5K7B7ykVX1iz7pSFyeF+Du4ilSgGEUiMt384KV/s5FpD/Jw QONmi6qzDtDGIGFDgxpMWw/ob4+4ZSpqrFti2h2PBmXriVCoWuh7GWK1az40cPLUbYtz Q/9w== X-Forwarded-Encrypted: i=1; AHgh+Rrw02WqcOi8MkRAXpBvIp1ynCYnMM4MRkSi4638wQML7Z9kEEY5n11UndqW+XRWjNlHBfL1JPv+ytmI7zo=@vger.kernel.org X-Gm-Message-State: AOJu0YxbPa/PQCUlP0seWvQtJQugmKd1CNb2PaeyNosIjB5YVxX8PNj3 HPsYX+vgAHud4ube3Dd/dWThgdKF5Ds+VsK/En3JYAf7L+fRSN4Ec8c9iFVDQAfX1w== X-Gm-Gg: AfdE7cm05erS0FiQkmN0NrMb17g5uC2RJZXZpuC13UI8feY6yAv8cUY5kJRc22HGlf8 qL1P8Wg8uUJWne7oM93zLaYZIjZ4+W8kvdXXN4P0pw3iHctCxH4mhHcupMAIim5+qpHb6fJUUNy 8jvAA5JLutWc3dhlW3dyA632VDkKyPnXeTCAhHAwHT67crupuBg31A8o2CyebkhNFSaCNsBIVfE PmgX0Zmjpw5ZHNkGEU0N9U4qJx1xnUAPWHd5YweX6mud+1JfVAw4wh2GYC/dWDFaWfJ25pqjS/X zAFr2dxou7efZICmLQMewY3ICNYHdaVAd6LJ2UgBbZEIM3D6Vieoy6e7OD6Z208NymWEZEhVCh0 6hlFCFaIJKJrk4vCnoPqCJ6lFmfvQ1hSyFTP9577CdnBKAlGKnt4HL+7vjPkbhtAEVkcO8YbBAS sya5czOGjzhoraOQIi5DqIAzq2dBr1JPTCJh1FSCMwnrPbQGpV3KE= X-Received: by 2002:a05:690c:6e93:b0:81d:5489:a659 with SMTP id 00721157ae682-81dc10ed452mr89759497b3.63.1783703333968; Fri, 10 Jul 2026 10:08:53 -0700 (PDT) Received: from google.com (137.69.77.34.bc.googleusercontent.com. [34.77.69.137]) by smtp.gmail.com with ESMTPSA id 00721157ae682-81e6bea2114sm51042637b3.3.2026.07.10.10.08.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Jul 2026 10:08:53 -0700 (PDT) Date: Fri, 10 Jul 2026 18:08:47 +0100 From: Vincent Donnefort To: Mostafa Saleh Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kvmarm@lists.linux.dev, iommu@lists.linux.dev, catalin.marinas@arm.com, will@kernel.org, maz@kernel.org, oliver.upton@linux.dev, joey.gouly@arm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com, joro@8bytes.org, jean-philippe@linaro.org, jgg@ziepe.ca, mark.rutland@arm.com, qperret@google.com, tabba@google.com, sebastianene@google.com, keirf@google.com Subject: Re: [PATCH v6 02/25] KVM: arm64: Donate MMIO to the hypervisor Message-ID: References: <20260501111928.259252-1-smostafa@google.com> <20260501111928.259252-3-smostafa@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260501111928.259252-3-smostafa@google.com> On Fri, May 01, 2026 at 11:19:04AM +0000, Mostafa Saleh wrote: > Add a function to donate MMIO to the hypervisor so IOMMU hypervisor > drivers can protect and access the MMIO of IOMMUs. > > As donating MMIO is very rare, and we don’t need to encode the full > state, it’s reasonable to have a separate function to do this. > It will init the host s2 page table with an invalid leaf with the owner ID > to prevent the host from mapping the page on faults. > > Also, prevent kvm_pgtable_stage2_unmap() from removing owner ID from > stage-2 PTEs, as this can be triggered from recycle logic under memory > pressure. There is no code relying on this, as all ownership changes is > done via kvm_pgtable_stage2_set_owner() > > For the error path in IOMMU drivers, add a function to donate MMIO > back from hyp to host. However, that leaks the hypervisor virtual > address range which should be acceptable as this is quite rare and > it matches the behaviour of fix_map/block. > > Signed-off-by: Mostafa Saleh > --- > arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 2 + > arch/arm64/kvm/hyp/nvhe/mem_protect.c | 119 +++++++++++++++++- > arch/arm64/kvm/hyp/pgtable.c | 9 +- > 3 files changed, 121 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h > index 3cbfae0e3dda..ff440204d2c7 100644 > --- a/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h > +++ b/arch/arm64/kvm/hyp/include/nvhe/mem_protect.h > @@ -36,6 +36,8 @@ int __pkvm_guest_share_host(struct pkvm_hyp_vcpu *vcpu, u64 gfn); > int __pkvm_guest_unshare_host(struct pkvm_hyp_vcpu *vcpu, u64 gfn); > int __pkvm_host_unshare_hyp(u64 pfn); > int __pkvm_host_donate_hyp(u64 pfn, u64 nr_pages); > +int __pkvm_host_donate_hyp_mmio(phys_addr_t addr, size_t size, unsigned long *haddr); > +int __pkvm_hyp_donate_host_mmio(phys_addr_t addr, size_t size); Can we have u64 pfn, u64 nr_pages here to match all the other functions? Also, as this is used by a "module" perhaps a sort of kerneldoc would be nice, especially to explain this isn't mapped into the hyp idmap range. > int __pkvm_hyp_donate_host(u64 pfn, u64 nr_pages); > int __pkvm_host_share_ffa(u64 pfn, u64 nr_pages); > int __pkvm_host_unshare_ffa(u64 pfn, u64 nr_pages); > diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c > index 28a471d1927c..2fb20a63a417 100644 > --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c > +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c > @@ -353,6 +353,38 @@ int __pkvm_prot_finalize(void) > return 0; > } > > +/* Unmap MMIO region while skipping donated PTEs. */ > +static int host_stage2_unmap_mmio_region(u64 start, u64 size) > +{ > + struct kvm_pgtable *pgt = &host_mmu.pgt; > + u64 unmap_start = start; > + u64 addr = start; > + kvm_pte_t pte; > + int ret = 0; > + u8 level; > + > + while (addr < start + size) { > + ret = kvm_pgtable_get_leaf(pgt, addr, &pte, &level); > + if (ret) > + return ret; > + if (!kvm_pte_valid(pte) && pte != 0) { > + if (addr > unmap_start) { > + ret = kvm_pgtable_stage2_unmap(pgt, unmap_start, > + addr - unmap_start); > + if (ret) > + return ret; > + } > + addr += kvm_granule_size(level); > + unmap_start = addr; > + } else { > + addr += kvm_granule_size(level); > + } > + } > + if (addr > unmap_start) > + ret = kvm_pgtable_stage2_unmap(pgt, unmap_start, addr - unmap_start); Ah shame, that's a lot of table walking. I guess we could: * create our own custom walker here but that would duplicate a lot of logic from pgtable.c. So doesn't look great. * Retry kvm_pgtable_stage2_unmap() as long as it return -EPERM... but then we it needs to return where it has stopped? * Modify kvm_pgtable_stage2_unmap() to just skip the refcounted entries? There's already a flag something that looks like this: KVM_PGTABLE_WALK_IGNORE_EAGAIN I am not so convinced by the first two points but the last one is probably worth exploring. > + return ret; > +} > + > static int host_stage2_unmap_dev_all(void) > { > struct kvm_pgtable *pgt = &host_mmu.pgt; > @@ -363,11 +395,11 @@ static int host_stage2_unmap_dev_all(void) > /* Unmap all non-memory regions to recycle the pages */ > for (i = 0; i < hyp_memblock_nr; i++, addr = reg->base + reg->size) { > reg = &hyp_memory[i]; > - ret = kvm_pgtable_stage2_unmap(pgt, addr, reg->base - addr); > + ret = host_stage2_unmap_mmio_region(addr, reg->base - addr); > if (ret) > return ret; > } > - return kvm_pgtable_stage2_unmap(pgt, addr, BIT(pgt->ia_bits) - addr); > + return host_stage2_unmap_mmio_region(addr, BIT(pgt->ia_bits) - addr); > } > > /* > @@ -1087,6 +1119,89 @@ int __pkvm_host_donate_hyp(u64 pfn, u64 nr_pages) > return ret; > } > > +int __pkvm_host_donate_hyp_mmio(phys_addr_t addr, size_t size, unsigned long *haddr) > +{ > + kvm_pte_t pte; > + u64 offset; > + int ret; > + > + /* Only before de-privilege. */ > + if (static_branch_unlikely(&kvm_protected_mode_initialized)) > + return -EPERM; > + > + if (!PAGE_ALIGNED(addr | size)) > + return -EINVAL; Would be nice here to have pfn_range_is_valid() even though this is only used by modules? (ditto __pkvm_hyp_donate_host_mmio()) > + > + ret = __pkvm_create_private_mapping(addr, size, PAGE_HYP_DEVICE, haddr); > + if (ret) > + return ret; Perhaps better to do it once it's been annotated into the host s2, so that we don't burn the private range in case of failure. > + > + host_lock_component(); > + for (offset = 0; offset < size; offset += PAGE_SIZE) { > + if (addr_is_memory(addr + offset)) { > + ret = -EINVAL; > + goto unlock; > + } > + ret = kvm_pgtable_get_leaf(&host_mmu.pgt, addr + offset, &pte, NULL); > + if (ret) > + goto unlock; > + if (pte && !kvm_pte_valid(pte)) { > + ret = -EPERM; > + goto unlock; > + } > + } > + /* > + * We set HYP as the owner of the MMIO pages in the host stage-2, for: > + * - host aborts: host_stage2_adjust_range() would fail for invalid non zero PTEs. > + * - recycle under memory pressure: host_stage2_unmap_dev_all() would call > + * kvm_pgtable_stage2_unmap() which will not clear non zero invalid ptes (counted). > + * - other MMIO donation: Would fail as we check that the PTE is valid or empty. > + */ > + ret = host_stage2_try(kvm_pgtable_stage2_annotate, &host_mmu.pgt, > + addr, size, &host_s2_pool, > + KVM_HOST_INVALID_PTE_TYPE_DONATION, > + FIELD_PREP(KVM_HOST_DONATION_PTE_OWNER_MASK, PKVM_ID_HYP)); > +unlock: > + host_unlock_component(); > + return ret; > +} > + > +int __pkvm_hyp_donate_host_mmio(phys_addr_t addr, size_t size) > +{ > + kvm_pte_t pte; > + u64 offset; > + int ret = 0; > + > + if (static_branch_unlikely(&kvm_protected_mode_initialized)) > + return -EPERM; > + > + if (!PAGE_ALIGNED(addr | size)) > + return -EINVAL; > + > + host_lock_component(); > + for (offset = 0; offset < size; offset += PAGE_SIZE) { > + if (addr_is_memory(addr + offset)) { > + ret = -EINVAL; > + goto unlock; > + } > + ret = kvm_pgtable_get_leaf(&host_mmu.pgt, addr + offset, &pte, NULL); > + if (ret) > + goto unlock; > + if (!pte || kvm_pte_valid(pte)) { > + ret = -EINVAL; > + goto unlock; > + } It should probably verify it is also mapped as we expect into the hyp. > + if (FIELD_GET(KVM_HOST_DONATION_PTE_OWNER_MASK, pte) != PKVM_ID_HYP) { > + ret = -EPERM; > + goto unlock; > + } > + } > + WARN_ON(host_stage2_idmap_locked(addr, size, PKVM_HOST_MMIO_PROT)); Perhaps we are better of simply zapping the entry and not map it proactively? And even if we can't "reclaim" the VA-space from the private range, it would be good to unmap it. I have made a function for that as part of the allocator series: https://lore.kernel.org/all/20260706175415.2604046-3-vdonnefort@google.com/ > +unlock: > + host_unlock_component(); > + return ret; > +} > + > int __pkvm_hyp_donate_host(u64 pfn, u64 nr_pages) > { > u64 phys = hyp_pfn_to_phys(pfn); > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c > index 0c1defa5fb0f..b64a50f9bfa8 100644 > --- a/arch/arm64/kvm/hyp/pgtable.c > +++ b/arch/arm64/kvm/hyp/pgtable.c > @@ -1159,13 +1159,8 @@ static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx, > kvm_pte_t *childp = NULL; > bool need_flush = false; > > - if (!kvm_pte_valid(ctx->old)) { > - if (stage2_pte_is_counted(ctx->old)) { > - kvm_clear_pte(ctx->ptep); > - mm_ops->put_page(ctx->ptep); > - } > - return 0; > - } > + if (!kvm_pte_valid(ctx->old)) > + return stage2_pte_is_counted(ctx->old) ? -EPERM : 0; > > if (kvm_pte_table(ctx->old, ctx->level)) { > childp = kvm_pte_follow(ctx->old, mm_ops); > -- > 2.54.0.545.g6539524ca2-goog >