Date:   Mon, 3 Jan 2022 12:43:46 -0600
From:   Venu Busireddy <venu.busireddy@oracle.com>
To:     Brijesh Singh <brijesh.singh@amd.com>
Cc:     x86@kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org,
        linux-coco@lists.linux.dev, linux-mm@kvack.org,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Joerg Roedel <jroedel@suse.de>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Ard Biesheuvel <ardb@kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Jim Mattson <jmattson@google.com>,
        Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Sergio Lopez <slp@redhat.com>, Peter Gonda <pgonda@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        David Rientjes <rientjes@google.com>,
        Dov Murik <dovmurik@linux.ibm.com>,
        Tobin Feldman-Fitzthum <tobin@ibm.com>,
        Borislav Petkov <bp@alien8.de>,
        Michael Roth <michael.roth@amd.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Andi Kleen <ak@linux.intel.com>,
        "Dr . David Alan Gilbert" <dgilbert@redhat.com>,
        tony.luck@intel.com, marcorr@google.com,
        sathyanarayanan.kuppuswamy@linux.intel.com
Subject: Re: [PATCH v8 09/40] x86/compressed: Add helper for validating pages
 in the decompression stage
Message-ID: <YdND4m/XjX6tRo0z@dt>
References: <20211210154332.11526-1-brijesh.singh@amd.com>
 <20211210154332.11526-10-brijesh.singh@amd.com>
 <Ybz3XFbThJTUySNY@dt>
 <ac5a0aac-5a48-9136-2d5d-595cb99d2a6f@amd.com>
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ac5a0aac-5a48-9136-2d5d-595cb99d2a6f@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?Hi4mbMGf4bH4QO41TBypknlH/r8/N5SZ4uUEXSdNmAU5deXJDjSk/3lniG?=
 =?iso-8859-1?Q?o+j/9ws+Rz9LDR0IO05v5M5Iaha8M8+12i8Hu5vbMX2evZljjulHw8PbhB?=
 =?iso-8859-1?Q?ixngrvP8TWIfSlNf172zW2DeL53kZYK7EMg7/wiPi6tLXpu0aVuvIoFQ5q?=
 =?iso-8859-1?Q?9od/Fqs9rG/QpaX7gLFZH2fvoetzxxEA/ws1NQF1eIBqzmPl/MG67nm4SI?=
 =?iso-8859-1?Q?ZYHxX4jwzcE6k8/o3XTA/fgSODkCeoGulWWzvI+5seBN/fsKYDG2X/+WyM?=
 =?iso-8859-1?Q?LimvnK93yhKMalzsUAG3btIhaFyHJrQeX1BVyufnOJDinFUyTvWn6ks/Fy?=
 =?iso-8859-1?Q?KSmGZzoy6bDexvrz+394wq3qNdJ1QbjrlIUAqWbWvkUtoEn3PDdsI34i/K?=
 =?iso-8859-1?Q?Z/fvSvH6UZlEXT/CYzZRNa/00t/upDmfSGSOame55ZvNI6sM41PysC0XaQ?=
 =?iso-8859-1?Q?YOioZpqSx+7ZTivnBcCDUV1Z+XVcl9r2+PpRfJiimPXPBf5pu5BujHFSTI?=
 =?iso-8859-1?Q?D0GKyXM3KjsESB5vATJzxjBVQc8xBNVCCNl502Rig4Bpny0UIKXLK5BLEi?=
 =?iso-8859-1?Q?jdrfjCMx6WwEtF+plLgfrDFK2cg/mTvlHnV69L/23mSVFMbDCOBwnhtnSX?=
 =?iso-8859-1?Q?mYp4SHb6cMj6O1PJ0rE6XNXSOauM8F1IkU9swewbfkc+5w33meWxvq+XDv?=
 =?iso-8859-1?Q?PdtPBMgrqYbqGrcRaZVPBEkQLAvkl5mKN9WPeihvZ3y7c3ARiuqMOmRjvc?=
 =?iso-8859-1?Q?w68Vc81S+0m9vES8bOXmKDQx1U1FbR+ulF35RgNl9S0SLhHZhNRlJSvCcE?=
 =?iso-8859-1?Q?ndfAU7pe2fNHEZrvTGFuqAL8o8LbIUU3gtD/CLQpY71atdeYww+Y4BQ+KU?=
 =?iso-8859-1?Q?+Zv59g5ZuvZZAKzBTh4r/wbawTkBkw53kDr8Dj5NamDJXaI9z5wLCD5cMk?=
 =?iso-8859-1?Q?+odBv+wzVtO2DJ4bVOq+V+NksmogX8afpbYc0aiJ3pjTzhri3rP2JyTPkI?=
 =?iso-8859-1?Q?wZMRRNmDnFsS/BgMXWsP80XbQ340Ovf/y/AnRrrIuOH4o5EKPpDxegDIBJ?=
 =?iso-8859-1?Q?gyRs+zs1xFnQ3S8mOYBZkStFxG1owzfZgw6VQppTVpuc433BfUvamTPdXt?=
 =?iso-8859-1?Q?5pkHfoxszCwb8WsiFm0m+Ck8CusA39PVUjwEamuivYGzjncO85WRFWYAX+?=
 =?iso-8859-1?Q?JHUqQ4z8Z4KbLrgoXVbFwSlWkLDOGl9VbLmTFBS/eo8OzHudfTbaWt8+T7?=
 =?iso-8859-1?Q?b6Ks0637HI5Bly1o8iqCfQr84bHwZtUOP22nKJXQ/egvdGJtBTr4Ex4Aus?=
 =?iso-8859-1?Q?/Ytq4ULo07NelThkYydk1RhiP9OyRNw36aElChfKBPGm0zGBm6sEJVoBDG?=
 =?iso-8859-1?Q?D6pqE7CIZbyhLfnQXe2gVolMWMBO8EAWykxB1E9SRfjY4+LwqKl53PQh4p?=
 =?iso-8859-1?Q?F6pmlIx3gZPZcWWtvyyCMfXo7EuUhfumnYq611Pks55S2uVVDvxCCVRUGD?=
 =?iso-8859-1?Q?ca9WwtmOfEbJXZQSqooIb3/Jklm1ImHOAUvQPQTIxJW3UHU/PHFhW3D3no?=
 =?iso-8859-1?Q?1xzXIoKkDtsnLvao9wwqHwhHIr/S9U6j936Auo3//6yvKmm6S9m6y2GqZu?=
 =?iso-8859-1?Q?/o0FeiAgms63b895BKDXHE4cilC1zCZg6WZtDuGKeSyw83Rj7N7pcjuVEH?=
 =?iso-8859-1?Q?nwhOnBB9y9yKprXLeY+b/vkrhpvPzIobRMC37lq+AXozs8p3+1YW3gCj9C?=
 =?iso-8859-1?Q?UVNQ=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b82881d6-f01b-4d6f-4799-08d9cee8fe4e
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB2576.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Jan 2022 18:43:54.6162
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: TiDltEYKMngxvVPV5Tbch8OxvzmK9218QxzT9Vj2skcHBVzV+pGxW8dNuGq5K7WT80dooiOuIJm2QOoVgZuAvVtSgg9d/7t1tpjwhMnP3jI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR10MB4795
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10216 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 mlxlogscore=999 mlxscore=0
 suspectscore=0 spamscore=0 phishscore=0 malwarescore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2201030127
X-Proofpoint-GUID: JkUwarGoaDsxvF2DHlHjUXTclA17_ZLz
X-Proofpoint-ORIG-GUID: JkUwarGoaDsxvF2DHlHjUXTclA17_ZLz
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2021-12-17 17:24:43 -0600, Brijesh Singh wrote:
> 
> On 12/17/21 2:47 PM, Venu Busireddy wrote:
> 
> >>  	 * the caches.
> >>  	 */
> >> -	if ((set | clr) & _PAGE_ENC)
> >> +	if ((set | clr) & _PAGE_ENC) {
> >>  		clflush_page(address);
> >>  
> >> +		/*
> >> +		 * If the encryption attribute is being cleared, then change
> >> +		 * the page state to shared in the RMP table.
> >> +		 */
> >> +		if (clr)
> > This function is also called by set_page_non_present() with clr set to
> > _PAGE_PRESENT. Do we want to change the page state to shared even when
> > the page is not present? If not, shouldn't the check be (clr & _PAGE_ENC)?
> 
> I am not able to follow your comment. Here we only pay attention to the
> encryption attribute, if encryption attribute is getting cleared then
> make PSC. In the case ov set_page_non_present(), the outer if() block
> will return false.? Am I missing something ?

You are right. I missed the outer check.

> > The page is not "added", right? Shouldn't we just say:
> 
> Technically, PSC modifies the RMP entry, so I should use that? instead
> of calling "added".
> 
> 
> >     Validate the page so that it is consistent with the RMP entry.
> 
> Yes, I am okay with it.

Thanks,

Venu