From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7FC7FC433EF for ; Wed, 6 Oct 2021 11:39:59 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4B394604E9 for ; Wed, 6 Oct 2021 11:39:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4B394604E9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/QA58rrMjktX1kpibT8jYv3D89AMKzlbCkGH0uVEdkQ=; b=bbTKPRrxcUZt3Y f1P127x5enVzzDyWwNWq6vparqjEc5wjjxX6gWF3b935+WfCEILBD0h6FXhW5s1ui7DszqhMsgnIs nPIK8Da6JkTYXq7HlCQrF6xGRW6Gnv+X3L/JscwA4WO4p050zjEh8VosVQBXl5UYADqQtr4jD68ss DG1kbWGKMK0QxROU6lLWN3Er+n97eOpF4JVddP4+/SFUg0RY44obgTefTqrs6viQ4uSR6nni+1R+U WXkP7PK0W1IibwGo7AFg2JLqA9WovyIF9LmxrrSnmJA2fuitDYmurFnz17BEWJibquqlHeTIhQx6Y f4By1Wo7zNbU5y2Ima7w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mY5FK-00E5op-8f; Wed, 06 Oct 2021 11:38:02 +0000 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mY5FF-00E5no-AH for linux-arm-kernel@lists.infradead.org; Wed, 06 Oct 2021 11:37:59 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1633520276; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=tLSJ+0G6GaESS3A8wROTGiXCvt9MmKxGWQIQfuIzv50=; b=PZou4TXLbMlhmkMfilag7ylf2LfKZGSdyyLP/c8Vu4YWo2uDFF2ZyUo1ukbm2A0g92uspw 2ySyjkKTNGyaLY8YcyiCZSSl0AuYVKBVotlQ2S+neFPddyEfaO+JJq6yYZKv7YuFe4NIX/ eSzUYZiW2/iu1Ge9uDFeILc99YYX7N4= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-297-pfGRUtebOX2pUMYhMQlC0g-1; Wed, 06 Oct 2021 07:37:55 -0400 X-MC-Unique: pfGRUtebOX2pUMYhMQlC0g-1 Received: by mail-ed1-f70.google.com with SMTP id x5-20020a50f185000000b003db0f796903so2329116edl.18 for ; Wed, 06 Oct 2021 04:37:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=tLSJ+0G6GaESS3A8wROTGiXCvt9MmKxGWQIQfuIzv50=; b=dC2e4DVUmZ0dLwhHxAuj9LD34NWHUFh3du8+lPpP9we9WHV2jKh3GePXpjDc6NsgYD nPnVBvRO9/1vDZDRum2PFRutb7e4Pj3+Es/xYS9N6WyGKhffgFEekLUgfQnISWW7O0Me JiFoyMdYQzJzcdfRDtDa00OPryVm5Qy7+yOml1GaG7aynmaGYzwxNAXCWAwb4IGLd9FJ tz8EZQ/oUoTCF9OdpRa02QJ6sB7zwq3tPlIu86sRI2TCYNtLKWFXl8EyZit3nqizY0Iy wJCCO/wahvd1GQb529xGnkauQ2n1QAajSfAkrCueItWxMmcQqA+BJDmccRVoPW+UR599 dUtw== X-Gm-Message-State: AOAM5327ppRf/+eVlVXIuooWqpVJ/8H/XFPYrTklPepoOtp2e2C3mXPk a8WRfUQbL0VB74tusmRSxNSY+FgzIjVHHgCLkdcjIuhXRZdRBncXi3TSVUrI/UBQbiahYRn56Rs nFxhSDVAZZRFP8HzfpDjb0LEXIkT/AILpTbc= X-Received: by 2002:a17:907:628d:: with SMTP id nd13mr32691154ejc.7.1633520274162; Wed, 06 Oct 2021 04:37:54 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyEEcX7nQiCt05CZwz558uZNZfwfw1lYcM5ACjQLLwDhsTkZqZlZFhhEMl7qA8+8LcuiVSErA== X-Received: by 2002:a17:907:628d:: with SMTP id nd13mr32691131ejc.7.1633520273957; Wed, 06 Oct 2021 04:37:53 -0700 (PDT) Received: from gator.home (cst2-174-28.cust.vodafone.cz. [31.30.174.28]) by smtp.gmail.com with ESMTPSA id dh16sm9840424edb.63.2021.10.06.04.37.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Oct 2021 04:37:53 -0700 (PDT) Date: Wed, 6 Oct 2021 13:37:51 +0200 From: Andrew Jones To: Marc Zyngier Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, will@kernel.org, qperret@google.com, dbrazdil@google.com, Steven Price , Fuad Tabba , Srivatsa Vaddagiri , Shanker R Donthineni , James Morse , Suzuki K Poulose , Alexandru Elisei , kernel-team@android.com Subject: Re: [PATCH v2 04/16] KVM: arm64: Add MMIO checking infrastructure Message-ID: <20211006113751.damskwaz7akpk5fc@gator.home> References: <20211004174849.2831548-1-maz@kernel.org> <20211004174849.2831548-5-maz@kernel.org> MIME-Version: 1.0 In-Reply-To: <20211004174849.2831548-5-maz@kernel.org> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=drjones@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211006_043757_465911_0F0463ED X-CRM114-Status: GOOD ( 28.41 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Oct 04, 2021 at 06:48:37PM +0100, Marc Zyngier wrote: > Introduce the infrastructure required to identify an IPA region > that is expected to be used as an MMIO window. > > This include mapping, unmapping and checking the regions. Nothing > calls into it yet, so no expected functional change. > > Signed-off-by: Marc Zyngier > --- > arch/arm64/include/asm/kvm_host.h | 2 + > arch/arm64/include/asm/kvm_mmu.h | 5 ++ > arch/arm64/kvm/mmu.c | 109 ++++++++++++++++++++++++++++++ > 3 files changed, 116 insertions(+) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index f63ca8fb4e58..ba9781eb84d6 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -125,6 +125,8 @@ struct kvm_arch { > #define KVM_ARCH_FLAG_RETURN_NISV_IO_ABORT_TO_USER 0 > /* Memory Tagging Extension enabled for the guest */ > #define KVM_ARCH_FLAG_MTE_ENABLED 1 > + /* Gues has bought into the MMIO guard extension */ > +#define KVM_ARCH_FLAG_MMIO_GUARD 2 > unsigned long flags; > > /* > diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h > index 02d378887743..454a6265d45d 100644 > --- a/arch/arm64/include/asm/kvm_mmu.h > +++ b/arch/arm64/include/asm/kvm_mmu.h > @@ -170,6 +170,11 @@ phys_addr_t kvm_mmu_get_httbr(void); > phys_addr_t kvm_get_idmap_vector(void); > int kvm_mmu_init(u32 *hyp_va_bits); > > +/* MMIO guard */ > +bool kvm_install_ioguard_page(struct kvm_vcpu *vcpu, gpa_t ipa); > +bool kvm_remove_ioguard_page(struct kvm_vcpu *vcpu, gpa_t ipa); > +bool kvm_check_ioguard_page(struct kvm_vcpu *vcpu, gpa_t ipa); > + > static inline void *__kvm_vector_slot2addr(void *base, > enum arm64_hyp_spectre_vector slot) > { > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > index 1a94a7ca48f2..2470a55ca675 100644 > --- a/arch/arm64/kvm/mmu.c > +++ b/arch/arm64/kvm/mmu.c > @@ -1172,6 +1172,115 @@ static void handle_access_fault(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa) > kvm_set_pfn_accessed(pte_pfn(pte)); > } > > +/* Replace this with something more structured once day */ one day > +#define MMIO_NOTE (('M' << 24 | 'M' << 16 | 'I' << 8 | 'O') << 1) Would it be better to have kvm_pgtable_stage2_annotate() shift its inputs (<< 1) instead of requiring all annotations to remember that requirement? Although the owner id is shifted 2 bits, but I'm not sure why. > + > +bool kvm_install_ioguard_page(struct kvm_vcpu *vcpu, gpa_t ipa) > +{ > + struct kvm_mmu_memory_cache *memcache; > + struct kvm_memory_slot *memslot; > + struct kvm *kvm = vcpu->kvm; > + int ret, idx; > + > + if (!test_bit(KVM_ARCH_FLAG_MMIO_GUARD, &kvm->arch.flags)) > + return false; > + > + /* Must be page-aligned */ > + if (ipa & ~PAGE_MASK) > + return false; > + > + /* > + * The page cannot be in a memslot. At some point, this will > + * have to deal with device mappings though. > + */ > + idx = srcu_read_lock(&kvm->srcu); > + mutex_lock(&kvm->slots_arch_lock); > + memslot = gfn_to_memslot(kvm, ipa >> PAGE_SHIFT); > + if (memslot) { > + ret = -EINVAL; > + goto out; > + } > + > + /* Guest has direct access to the GICv2 virtual CPU interface */ > + if (irqchip_in_kernel(kvm) && > + kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2 && > + ipa == kvm->arch.vgic.vgic_cpu_base) { > + ret = 0; > + goto out; > + } > + > + memcache = &vcpu->arch.mmu_page_cache; > + if (kvm_mmu_topup_memory_cache(memcache, > + kvm_mmu_cache_min_pages(kvm))) { > + ret = -ENOMEM; > + goto out; > + } > + > + spin_lock(&kvm->mmu_lock); > + ret = kvm_pgtable_stage2_annotate(vcpu->arch.hw_mmu->pgt, > + ipa, PAGE_SIZE, memcache, > + MMIO_NOTE); > + spin_unlock(&kvm->mmu_lock); > + > +out: > + mutex_unlock(&kvm->slots_arch_lock); > + srcu_read_unlock(&kvm->srcu, idx); > + return ret == 0; I guess the callers need this to return a boolean? Just seems odd that pains were taken above to set ret to EINVAL/ENOMEM just to translate that to true/false here though. > +} > + > +static bool __check_ioguard_page(struct kvm_vcpu *vcpu, gpa_t ipa) > +{ > + kvm_pte_t pte = 0; > + u32 level = 0; > + int ret; > + > + lockdep_assert_held(&vcpu->kvm->mmu_lock); > + > + ret = kvm_pgtable_get_leaf(vcpu->arch.hw_mmu->pgt, ipa, &pte, &level); > + VM_BUG_ON(ret); > + VM_BUG_ON(level >= KVM_PGTABLE_MAX_LEVELS); > + > + /* Must be a PAGE_SIZE mapping with our annotation */ > + return (BIT(ARM64_HW_PGTABLE_LEVEL_SHIFT(level)) == PAGE_SIZE && > + pte == MMIO_NOTE); > +} > + > +bool kvm_remove_ioguard_page(struct kvm_vcpu *vcpu, gpa_t ipa) > +{ > + bool ret; > + > + if (!test_bit(KVM_ARCH_FLAG_MMIO_GUARD, &vcpu->kvm->arch.flags)) > + return false; > + > + /* Keep the PT locked across the two walks */ > + spin_lock(&vcpu->kvm->mmu_lock); > + > + ret = __check_ioguard_page(vcpu, ipa); > + if (ret) /* Drop the annotation */ > + kvm_pgtable_stage2_unmap(vcpu->arch.hw_mmu->pgt, > + ALIGN_DOWN(ipa, PAGE_SIZE), PAGE_SIZE); How about if (ret) { /* Drop the annotation */ kvm_pgtable_stage2_unmap(vcpu->arch.hw_mmu->pgt, ALIGN_DOWN(ipa, PAGE_SIZE), PAGE_SIZE); } to be a bit easier to read. > + > + spin_unlock(&vcpu->kvm->mmu_lock); > + return ret; > +} > + > +bool kvm_check_ioguard_page(struct kvm_vcpu *vcpu, gpa_t ipa) > +{ > + bool ret; > + > + if (!test_bit(KVM_ARCH_FLAG_MMIO_GUARD, &vcpu->kvm->arch.flags)) > + return true; > + > + spin_lock(&vcpu->kvm->mmu_lock); > + ret = __check_ioguard_page(vcpu, ipa & PAGE_MASK); > + spin_unlock(&vcpu->kvm->mmu_lock); > + > + if (!ret) > + kvm_inject_dabt(vcpu, kvm_vcpu_get_hfar(vcpu)); > + > + return ret; > +} > + > /** > * kvm_handle_guest_abort - handles all 2nd stage aborts > * @vcpu: the VCPU pointer > -- > 2.30.2 > Besides the nits Reviewed-by: Andrew Jones Thanks, drew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel