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=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL 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 48856C12002 for ; Wed, 21 Jul 2021 07:36:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2D494606A5 for ; Wed, 21 Jul 2021 07:36:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235517AbhGUGzt (ORCPT ); Wed, 21 Jul 2021 02:55:49 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46674 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235899AbhGUGy1 (ORCPT ); Wed, 21 Jul 2021 02:54:27 -0400 Received: from mail-ot1-x334.google.com (mail-ot1-x334.google.com [IPv6:2607:f8b0:4864:20::334]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D5BF1C061574 for ; Wed, 21 Jul 2021 00:35:03 -0700 (PDT) Received: by mail-ot1-x334.google.com with SMTP id t4-20020a05683014c4b02904cd671b911bso1285758otq.1 for ; Wed, 21 Jul 2021 00:35:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=8+C3yWM1Aq25AnToXYe0OxrL1vCuAWOUW+a14IQfShM=; b=vtaMs7dhTXsjys2mWi5QB8shnliH3LrkB6s1LGUXdyJanimPJRQ7SUjC2I/vqoWGxY k3hviiV60qay40DJOSTvONJWAx1+PKwC0nu+4ZtW6HdXRuNwvO9S+1I39Qj8gQzE6zuV gLJk5Tic/HuzwEa1ubkMIUQbEjzfZ7tmpxdwFF4d8tzcMd1JVE7BQGvIdIeDsU37/ami IVklHeTuWAMtlrhMuic3GFQOWBFipA+CJt38rkdbMAR/Ls955ATKzwoU+dpMyed22fA1 y9ltCCmdh96NJRIpZlLXDrqVg0gP5ycxewWFVVblNPyYrPYJZ4F/rqky9ww5tsY3Agti YaPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8+C3yWM1Aq25AnToXYe0OxrL1vCuAWOUW+a14IQfShM=; b=O5WsAMMxli73H+udH6SVBo37hK0UgEQDENiieGbt8two7D/dZdq1jvu63VkKNWg7ho vRobv7P5mlthyMw9XM91YR/uFT6xxvFIHQ8oJAxHT1GefqeAvvNy40u7dUX8DPouUeZM J/kYcebcAQm5UuFGZhzh/47+haFctXMNvsXcGI1QhMW/h8aK3PyhsPDC1h90Ga74TMRU I9F0zfd0BMbbySkDQ9ggtXQmS3cjqtIs91HHyLSDH5ACT+/YxKvSJelee2pRJx8BEyYp TTtfN2lt3HbVlBnl5dZNbcrA1phJGHptUf87uQD/h1v0xmazqycWXWvHaUuFQVxBFS5G sN0Q== X-Gm-Message-State: AOAM530qGxEl2Bf8+aXzZo2Do4zLCz3paI2FcZpdi0+HrnyKa1T5M2b/ KpKgk7fOQyBxn+8Cp9D4NlcjiXCGDO7be9mP73tk2g== X-Google-Smtp-Source: ABdhPJxWS4P3/legZ/TlwXxppTSSemQXnWnvlkkGMpTD3obhtSOJNiFjn6I7+UP0SEsIAcukDCsLe6vdUfva3QrxYkQ= X-Received: by 2002:a05:6830:1455:: with SMTP id w21mr25226545otp.365.1626852902894; Wed, 21 Jul 2021 00:35:02 -0700 (PDT) MIME-Version: 1.0 References: <20210719104735.3681732-1-qperret@google.com> <20210719104735.3681732-9-qperret@google.com> In-Reply-To: From: Fuad Tabba Date: Wed, 21 Jul 2021 08:34:26 +0100 Message-ID: Subject: Re: [PATCH 08/14] KVM: arm64: Add support for tagging shared pages in page-table To: Quentin Perret Cc: maz@kernel.org, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, ardb@kernel.org, qwandor@google.com, dbrazdil@google.com, kernel-team@android.com Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Quentin, On Tue, Jul 20, 2021 at 3:06 PM Quentin Perret wrote: > > On Tuesday 20 Jul 2021 at 14:48:09 (+0100), Fuad Tabba wrote: > > This might tie in to Marc's comments for using enums, but > > consolidating the translation between prot and ignored/software bits > > in one place would be good: thinking about patch 10 as well, where you > > get the prot from the ignored bits. Even though you have documented > > it, I'm finding the part where a field can be borrowed and shared as > > opposed to being only shared not very intuitive, and I need to reread > > the comment here to remember the difference while going through the > > code. > > > > You also mention lending as potentially reserved for the future, but I > > think that lending is the other side of borrowing (depends on who's > > doing the giving/taking). I wonder if in this case it would be clearer > > to describe it in terms of whether it's exclusively owned vs owned but > > shared (for the owner), and just shared for the sharer... > > Argh so I actually found the encoding pretty neat :/ > The idea is the following: > > - an entity that has a page mapped as SHARED in its PT means it > doesn't have exclusive access to the page; > > - an entity that has a page mapped as BORROWED in its PT means it has > access to a page it doesn't own; > > From that we can build the states we need: > > - when an entity shares a page with another, the original owner gets a > SHARED mapping, and the recipient a SHARED+BORROWED mapping. > > - and in the future when/if we implement lending (which means an > entity gives exclusive access to a page to another entity, but > retains ownership) we can map the page in the recipient as > 'BORROWED' only, but not 'SHARED'. And the original owner will have > an invalid mapping with a new state 'LENT', which is encoded with > both SW bits set. > > How does that sound? Did you have something else in mind? The encoding is very neat by the way :D I see where you're going with the lent state now, and I understand the states as well as the possible transitions now that you've explained it. It's the terminology that confused me a bit (especially when you mention lending, which seemed to imply is something distinct from borrowing as opposed to just the other side of it). What for me would help is to document this, and the possible combinations/legal states. kvm_pgtable.h describes the prots a bit, but maybe you could expand that similar to what you've done in this email: @KVM_PGTABLE_STATE_BORROWED: Page borrowed from another entity: has access to the page but no ownership Not sure if defining aliases for all legal combinations would also help or add to the confusion, thinking out loud, something along the lines of cache state taxonomy (e.g., Sweazy and Smith fig 3 [1]). You have that in the borrowed (as opposed to owned), and shared (as opposed to exclusive). So aliases to build on these: #define KVM_PGTABLE_STATE_BORROWED_SHARED (KVM_PGTABLE_STATE_SHARED | KVM_PGTABLE_STATE_BORROWED) #define KVM_PGTABLE_STATE_BORROWED_EXCLUSIVE (KVM_PGTABLE_STATE_BORROWED) #define KVM_PGTABLE_STATE_OWNED_SHARED (KVM_PGTABLE_STATE_SHARED) #define KVM_PGTABLE_STATE_OWNED_EXCLUSIVE (0ULL) You have thought about this way more than I have. But I think that having clear documentation, ideally in the code itself via helpers/enums/aliases could help people like me who come to the code later not shoot themselves in the foot. Thanks! /fuad [1] https://www.cs.auckland.ac.nz/compsci703s1c/archive/2008/resources/Sweazey.pdf > Thanks, > Quentin 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=-3.6 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,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 2D87FC07E9B for ; Wed, 21 Jul 2021 07:35:09 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 8DED96113C for ; Wed, 21 Jul 2021 07:35:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8DED96113C Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id F39204B126; Wed, 21 Jul 2021 03:35:07 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@google.com Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0x2tpEqf8dyG; Wed, 21 Jul 2021 03:35:06 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id CDB964B116; Wed, 21 Jul 2021 03:35:06 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 022D34B101 for ; Wed, 21 Jul 2021 03:35:05 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 4mi0MwPrc3Tl for ; Wed, 21 Jul 2021 03:35:03 -0400 (EDT) Received: from mail-ot1-f48.google.com (mail-ot1-f48.google.com [209.85.210.48]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id C44C240CF8 for ; Wed, 21 Jul 2021 03:35:03 -0400 (EDT) Received: by mail-ot1-f48.google.com with SMTP id f12-20020a056830204cb029048bcf4c6bd9so1250612otp.8 for ; Wed, 21 Jul 2021 00:35:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=8+C3yWM1Aq25AnToXYe0OxrL1vCuAWOUW+a14IQfShM=; b=vtaMs7dhTXsjys2mWi5QB8shnliH3LrkB6s1LGUXdyJanimPJRQ7SUjC2I/vqoWGxY k3hviiV60qay40DJOSTvONJWAx1+PKwC0nu+4ZtW6HdXRuNwvO9S+1I39Qj8gQzE6zuV gLJk5Tic/HuzwEa1ubkMIUQbEjzfZ7tmpxdwFF4d8tzcMd1JVE7BQGvIdIeDsU37/ami IVklHeTuWAMtlrhMuic3GFQOWBFipA+CJt38rkdbMAR/Ls955ATKzwoU+dpMyed22fA1 y9ltCCmdh96NJRIpZlLXDrqVg0gP5ycxewWFVVblNPyYrPYJZ4F/rqky9ww5tsY3Agti YaPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8+C3yWM1Aq25AnToXYe0OxrL1vCuAWOUW+a14IQfShM=; b=gdWRGzQeLF3HHFxY9EM6RvvtisKoPp8iHjGl+B+lx1AAQqSkQ2KarLdNp5KGTii1gC fs8iJlUFAYG0df2HolOR83xRbhrjsKWPPj0i1heS1f5z5R74mSp1Y7JhoMZ2zr3pCl2s QkiSbNEgqItmn64qqsXCAbZwityPfU4mysKoQ99/3g0FFrHtBx0wVgV2cDJMnjqgPqc8 lj+LEmhzlzyBvhAV3yvCmih/s9dFPC3NE5NlLdyAm8RWzU4dvaqNZJOLkcqVl+Hy7QPc iAJVVph0WimwYYzmpPhrzAH8BAyRjC/xiyRD6dIV4rP98rX9SQBt6V2+AHwuRDzRIT0T kZVQ== X-Gm-Message-State: AOAM533GXHWSLoDGgRLGqaDq1zuWoflA8CQv56io2LQWAA3fqtkjeQMb vB2E6Kxdgj6scy4IS4Hf/Y/JmWYZm7SPi3U71VYCFA== X-Google-Smtp-Source: ABdhPJxWS4P3/legZ/TlwXxppTSSemQXnWnvlkkGMpTD3obhtSOJNiFjn6I7+UP0SEsIAcukDCsLe6vdUfva3QrxYkQ= X-Received: by 2002:a05:6830:1455:: with SMTP id w21mr25226545otp.365.1626852902894; Wed, 21 Jul 2021 00:35:02 -0700 (PDT) MIME-Version: 1.0 References: <20210719104735.3681732-1-qperret@google.com> <20210719104735.3681732-9-qperret@google.com> In-Reply-To: From: Fuad Tabba Date: Wed, 21 Jul 2021 08:34:26 +0100 Message-ID: Subject: Re: [PATCH 08/14] KVM: arm64: Add support for tagging shared pages in page-table To: Quentin Perret Cc: kernel-team@android.com, qwandor@google.com, maz@kernel.org, linux-kernel@vger.kernel.org, catalin.marinas@arm.com, will@kernel.org, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu Hi Quentin, On Tue, Jul 20, 2021 at 3:06 PM Quentin Perret wrote: > > On Tuesday 20 Jul 2021 at 14:48:09 (+0100), Fuad Tabba wrote: > > This might tie in to Marc's comments for using enums, but > > consolidating the translation between prot and ignored/software bits > > in one place would be good: thinking about patch 10 as well, where you > > get the prot from the ignored bits. Even though you have documented > > it, I'm finding the part where a field can be borrowed and shared as > > opposed to being only shared not very intuitive, and I need to reread > > the comment here to remember the difference while going through the > > code. > > > > You also mention lending as potentially reserved for the future, but I > > think that lending is the other side of borrowing (depends on who's > > doing the giving/taking). I wonder if in this case it would be clearer > > to describe it in terms of whether it's exclusively owned vs owned but > > shared (for the owner), and just shared for the sharer... > > Argh so I actually found the encoding pretty neat :/ > The idea is the following: > > - an entity that has a page mapped as SHARED in its PT means it > doesn't have exclusive access to the page; > > - an entity that has a page mapped as BORROWED in its PT means it has > access to a page it doesn't own; > > From that we can build the states we need: > > - when an entity shares a page with another, the original owner gets a > SHARED mapping, and the recipient a SHARED+BORROWED mapping. > > - and in the future when/if we implement lending (which means an > entity gives exclusive access to a page to another entity, but > retains ownership) we can map the page in the recipient as > 'BORROWED' only, but not 'SHARED'. And the original owner will have > an invalid mapping with a new state 'LENT', which is encoded with > both SW bits set. > > How does that sound? Did you have something else in mind? The encoding is very neat by the way :D I see where you're going with the lent state now, and I understand the states as well as the possible transitions now that you've explained it. It's the terminology that confused me a bit (especially when you mention lending, which seemed to imply is something distinct from borrowing as opposed to just the other side of it). What for me would help is to document this, and the possible combinations/legal states. kvm_pgtable.h describes the prots a bit, but maybe you could expand that similar to what you've done in this email: @KVM_PGTABLE_STATE_BORROWED: Page borrowed from another entity: has access to the page but no ownership Not sure if defining aliases for all legal combinations would also help or add to the confusion, thinking out loud, something along the lines of cache state taxonomy (e.g., Sweazy and Smith fig 3 [1]). You have that in the borrowed (as opposed to owned), and shared (as opposed to exclusive). So aliases to build on these: #define KVM_PGTABLE_STATE_BORROWED_SHARED (KVM_PGTABLE_STATE_SHARED | KVM_PGTABLE_STATE_BORROWED) #define KVM_PGTABLE_STATE_BORROWED_EXCLUSIVE (KVM_PGTABLE_STATE_BORROWED) #define KVM_PGTABLE_STATE_OWNED_SHARED (KVM_PGTABLE_STATE_SHARED) #define KVM_PGTABLE_STATE_OWNED_EXCLUSIVE (0ULL) You have thought about this way more than I have. But I think that having clear documentation, ideally in the code itself via helpers/enums/aliases could help people like me who come to the code later not shoot themselves in the foot. Thanks! /fuad [1] https://www.cs.auckland.ac.nz/compsci703s1c/archive/2008/resources/Sweazey.pdf > Thanks, > Quentin _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm 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=-5.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, 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 3DE50C12002 for ; Wed, 21 Jul 2021 07:36:43 +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 06C3760241 for ; Wed, 21 Jul 2021 07:36:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 06C3760241 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@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:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Pf6B7PqMf1UxjtQ3uIJW4/5gI39+SK/hZO9QpeOFKUU=; b=EdqKoRgAA0BpM9 BgPqy2gLq9YVcxdEk2K4++JDsy3xqThGFh396XtiaGMTqLfiCTfLBYXmoBM1JBG2uKtd2mAIO8e/d uIaR+p3Qu2DAJDKnrqX7slChzAmU3HW+oAkASCtopxGWQ+EdWsuOoaNZbXeMPxwJZKICYB0GIeRjY tuJU+fKRxUxSE/M2V3yXjHfCW0F2pwaRzl150Pdj7RDAdzUVIFcmmenPO9U7g8M9mgDlaJ9oWMExg z1yS0KNytgUGmwmhyDSGG0aLRp3xQqmaxngEIaRPk/KQSru4xdpnICi1d81eEsqL/Vt+aq0d4klwX Ba3/sFRVIOkjyvHWvGXw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m66lB-00EhNy-2P; Wed, 21 Jul 2021 07:35:17 +0000 Received: from mail-ot1-x32c.google.com ([2607:f8b0:4864:20::32c]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1m66l5-00EhLY-KO for linux-arm-kernel@lists.infradead.org; Wed, 21 Jul 2021 07:35:13 +0000 Received: by mail-ot1-x32c.google.com with SMTP id 59-20020a9d0ac10000b0290462f0ab0800so1235555otq.11 for ; Wed, 21 Jul 2021 00:35:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=8+C3yWM1Aq25AnToXYe0OxrL1vCuAWOUW+a14IQfShM=; b=vtaMs7dhTXsjys2mWi5QB8shnliH3LrkB6s1LGUXdyJanimPJRQ7SUjC2I/vqoWGxY k3hviiV60qay40DJOSTvONJWAx1+PKwC0nu+4ZtW6HdXRuNwvO9S+1I39Qj8gQzE6zuV gLJk5Tic/HuzwEa1ubkMIUQbEjzfZ7tmpxdwFF4d8tzcMd1JVE7BQGvIdIeDsU37/ami IVklHeTuWAMtlrhMuic3GFQOWBFipA+CJt38rkdbMAR/Ls955ATKzwoU+dpMyed22fA1 y9ltCCmdh96NJRIpZlLXDrqVg0gP5ycxewWFVVblNPyYrPYJZ4F/rqky9ww5tsY3Agti YaPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8+C3yWM1Aq25AnToXYe0OxrL1vCuAWOUW+a14IQfShM=; b=nmpQY4ozemsjpHGiTHJZWPWSH9+r4m7jWUDcOPUg31y71mIbIRuQJjk87yrgBXYHvb HfjP0oT0OoxOpkArJ/79I0Jw56iI9dvIf7SQxXHGQUybcip/09I916ve7IGyz/2eJMDr tczRdi7NFft91xaDX+apvGz5bpIvqWtJEyjv/oBu16cDCASuScEG2g/DqQUx1VTIwXy9 zdIQtpXddfRKOju5uYHPP30HDSqxAYIgKZiEyBcCX7X1m6ZP5tKWNpzL1SBP6eRfvN0J eMP7HfxxynXgBdsd2Y1QL0NGyZ1aQ9059sxQLl4M1VjxepyMSNCxS9KfdeW01LIN+MPW qy2g== X-Gm-Message-State: AOAM5313UITCEDdinahfmWEziUkJGb2AtIDJwPs5gLy/GJVGzDc2+B/g 2wNIhlD9RtnsBkpk/q06nR09R40hg+hHR0PrF8vz1g== X-Google-Smtp-Source: ABdhPJxWS4P3/legZ/TlwXxppTSSemQXnWnvlkkGMpTD3obhtSOJNiFjn6I7+UP0SEsIAcukDCsLe6vdUfva3QrxYkQ= X-Received: by 2002:a05:6830:1455:: with SMTP id w21mr25226545otp.365.1626852902894; Wed, 21 Jul 2021 00:35:02 -0700 (PDT) MIME-Version: 1.0 References: <20210719104735.3681732-1-qperret@google.com> <20210719104735.3681732-9-qperret@google.com> In-Reply-To: From: Fuad Tabba Date: Wed, 21 Jul 2021 08:34:26 +0100 Message-ID: Subject: Re: [PATCH 08/14] KVM: arm64: Add support for tagging shared pages in page-table To: Quentin Perret Cc: maz@kernel.org, james.morse@arm.com, alexandru.elisei@arm.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, ardb@kernel.org, qwandor@google.com, dbrazdil@google.com, kernel-team@android.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210721_003511_735279_AD4489EF X-CRM114-Status: GOOD ( 32.89 ) 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 Hi Quentin, On Tue, Jul 20, 2021 at 3:06 PM Quentin Perret wrote: > > On Tuesday 20 Jul 2021 at 14:48:09 (+0100), Fuad Tabba wrote: > > This might tie in to Marc's comments for using enums, but > > consolidating the translation between prot and ignored/software bits > > in one place would be good: thinking about patch 10 as well, where you > > get the prot from the ignored bits. Even though you have documented > > it, I'm finding the part where a field can be borrowed and shared as > > opposed to being only shared not very intuitive, and I need to reread > > the comment here to remember the difference while going through the > > code. > > > > You also mention lending as potentially reserved for the future, but I > > think that lending is the other side of borrowing (depends on who's > > doing the giving/taking). I wonder if in this case it would be clearer > > to describe it in terms of whether it's exclusively owned vs owned but > > shared (for the owner), and just shared for the sharer... > > Argh so I actually found the encoding pretty neat :/ > The idea is the following: > > - an entity that has a page mapped as SHARED in its PT means it > doesn't have exclusive access to the page; > > - an entity that has a page mapped as BORROWED in its PT means it has > access to a page it doesn't own; > > From that we can build the states we need: > > - when an entity shares a page with another, the original owner gets a > SHARED mapping, and the recipient a SHARED+BORROWED mapping. > > - and in the future when/if we implement lending (which means an > entity gives exclusive access to a page to another entity, but > retains ownership) we can map the page in the recipient as > 'BORROWED' only, but not 'SHARED'. And the original owner will have > an invalid mapping with a new state 'LENT', which is encoded with > both SW bits set. > > How does that sound? Did you have something else in mind? The encoding is very neat by the way :D I see where you're going with the lent state now, and I understand the states as well as the possible transitions now that you've explained it. It's the terminology that confused me a bit (especially when you mention lending, which seemed to imply is something distinct from borrowing as opposed to just the other side of it). What for me would help is to document this, and the possible combinations/legal states. kvm_pgtable.h describes the prots a bit, but maybe you could expand that similar to what you've done in this email: @KVM_PGTABLE_STATE_BORROWED: Page borrowed from another entity: has access to the page but no ownership Not sure if defining aliases for all legal combinations would also help or add to the confusion, thinking out loud, something along the lines of cache state taxonomy (e.g., Sweazy and Smith fig 3 [1]). You have that in the borrowed (as opposed to owned), and shared (as opposed to exclusive). So aliases to build on these: #define KVM_PGTABLE_STATE_BORROWED_SHARED (KVM_PGTABLE_STATE_SHARED | KVM_PGTABLE_STATE_BORROWED) #define KVM_PGTABLE_STATE_BORROWED_EXCLUSIVE (KVM_PGTABLE_STATE_BORROWED) #define KVM_PGTABLE_STATE_OWNED_SHARED (KVM_PGTABLE_STATE_SHARED) #define KVM_PGTABLE_STATE_OWNED_EXCLUSIVE (0ULL) You have thought about this way more than I have. But I think that having clear documentation, ideally in the code itself via helpers/enums/aliases could help people like me who come to the code later not shoot themselves in the foot. Thanks! /fuad [1] https://www.cs.auckland.ac.nz/compsci703s1c/archive/2008/resources/Sweazey.pdf > Thanks, > Quentin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel