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 X-Spam-Level: X-Spam-Status: No, score=-1.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2173BC47254 for ; Tue, 5 May 2020 16:32:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EE42A20735 for ; Tue, 5 May 2020 16:32:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1588696343; bh=f2FnSNmx6/YrdynY8bpg6PqkifLdRaIEMjTJ5/Vhq2A=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=aPgPCrUZogZkzBm6VVkp8MvBLPuYO/+MqbTtvRNwFlXksl8ap/nKP8NVYjfzvknjS ViWl9ZCJqxfyYM/oykUwz61lCvZ/Mcgc+6SL5ndKe1XTrwrn2rT6omuJHO6SzyN+gV 3q4rqqVIyy/znv7d6mj4trr+Dqw1BuiMIJenhsgo= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729741AbgEEQcW (ORCPT ); Tue, 5 May 2020 12:32:22 -0400 Received: from mail.kernel.org ([198.145.29.99]:38272 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729310AbgEEQcV (ORCPT ); Tue, 5 May 2020 12:32:21 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DAA4F206FA; Tue, 5 May 2020 16:32:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1588696341; bh=f2FnSNmx6/YrdynY8bpg6PqkifLdRaIEMjTJ5/Vhq2A=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=aZc6+em+RB8cGHTqR6utzVkDZb6DwmmOJ2EeN3C1k//1ijmEfPb59pPJZniZs2f4b A9mmrdC5P+EHe6udAD6oi6DnjSiSaoCq3iyiOPKuMX8zo3F2HGzbgSdTGeZia6JU6B m5mbAIdcW15XG24xkqXJ1Ttk21rS/E+WdS77an7g= Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=big-swifty.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jW0UV-009YO4-5e; Tue, 05 May 2020 17:32:19 +0100 Date: Tue, 05 May 2020 17:32:18 +0100 Message-ID: <86pnbitka5.wl-maz@kernel.org> From: Marc Zyngier To: Andrew Scull Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, Will Deacon , Andre Przywara , Dave Martin , George Cherian , "Zengtao (B)" , Catalin Marinas Subject: Re: [PATCH 03/26] KVM: arm64: Factor out stage 2 page table data from struct kvm In-Reply-To: <20200505152648.GA237572@google.com> References: <20200422120050.3693593-1-maz@kernel.org> <20200422120050.3693593-4-maz@kernel.org> <20200505152648.GA237572@google.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 EasyPG/1.0.0 Emacs/26 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: ascull@google.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, will@kernel.org, andre.przywara@arm.com, Dave.Martin@arm.com, gcherian@marvell.com, prime.zeng@hisilicon.com, catalin.marinas@arm.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Andrew, On Tue, 05 May 2020 16:26:48 +0100, Andrew Scull wrote: > > Having a go at reviewing. Might turn out to be more useful as a learning > exercise for me rather than useful feedback but we've got to start > somewhere.. Thanks for making the effort. Asking questions is never a pointless exercise, as it usually means that something isn't as crystal clear as the author expects... ;-) > > > -struct kvm_arch { > > +struct kvm_s2_mmu { > > struct kvm_vmid vmid; > > > > - /* stage2 entry level table */ > > - pgd_t *pgd; > > - phys_addr_t pgd_phys; > > - > > - /* VTCR_EL2 value for this VM */ > > - u64 vtcr; > > + /* > > + * stage2 entry level table > > + * > > + * Two kvm_s2_mmu structures in the same VM can point to the same pgd > > + * here. This happens when running a non-VHE guest hypervisor which > > + * uses the canonical stage 2 page table for both vEL2 and for vEL1/0 > > + * with vHCR_EL2.VM == 0. > > + */ > > + pgd_t *pgd; > > + phys_addr_t pgd_phys; > > > > /* The last vcpu id that ran on each physical CPU */ > > int __percpu *last_vcpu_ran; > > > > + struct kvm *kvm; > > +}; > > + > > +struct kvm_arch { > > + struct kvm_s2_mmu mmu; > > + > > + /* VTCR_EL2 value for this VM */ > > + u64 vtcr; > > VTCR seems quite strongly tied to the MMU config. Is it not controlled > independently for the nested MMUs and so remains in this struct? This particular instance of VTCR_EL2 is the host's version. Which means it describes the virtual HW for the EL1 guest. It constraints, among other things, the number of IPA bits for the guest, for example, and is configured by the VMM. Once you start nesting, each vcpu has its own VTCR_EL2 which is still constrained by the main one (no nested guest can have a T0SZ bigger than the value imposed by userspace for this guest as a whole). Does it make sense? > > > -static void stage2_dissolve_pmd(struct kvm *kvm, phys_addr_t addr, pmd_t *pmd) > > +static void stage2_dissolve_pmd(struct kvm_s2_mmu *mmu, phys_addr_t addr, pmd_t *pmd) > > How strictly is the long line style rule enforced? checkpatch has 16 > such warnings on this patch. It isn't enforced at all for KVM/arm. I am perfectly happy with longish lines (I stupidly gave away my vt100 a very long time ago). In general, checkpatch warnings are to be looked into (it sometimes brings interesting stuff up), but this falls into the *cosmetic* department, and I cannot be bothered. > > > -static void stage2_dissolve_pud(struct kvm *kvm, phys_addr_t addr, pud_t *pudp) > > +static void stage2_dissolve_pud(struct kvm_s2_mmu *mmu, phys_addr_t addr, pud_t *pudp) > > { > > + struct kvm *kvm __maybe_unused = mmu->kvm; > > + > > if (!stage2_pud_huge(kvm, *pudp)) > > return; > > There're a couple places with `__maybe_unused` on variables that are > then used soon after. Can they be dropped in these cases so as not to > hide legitimate warning? Absolutely. I'll have a look. Thanks for the review! M. -- Jazz is not dead, it just smells funny.