From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id A1FA4941B97 for ; Wed, 7 Feb 2024 20:18:02 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=9bkO+nGztYpy355QE6Jq7mbU9k9xDsd9rqLjW/S7o0k=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1707337081; v=1; b=gqtPavL1YAD87mzwF2dWpTrVe/suYUpPbZ9svv6ujaamVigSh7vtlJqKkHQ0otoqeh2/03xJ XxxPOpWN6jH6bXfxGBRc2XgWhRrbGiL6IQesg9GGf3lJOVDrk5U4y1cER7fjkYyXWnkVRffqCX2 bIRH+H9fTTFYJUPy8f4Sny7k= X-Received: by 127.0.0.2 with SMTP id L2nWYY7687511xPLAukTvpU7; Wed, 07 Feb 2024 12:18:01 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.158.1707337080545275570 for ; Wed, 07 Feb 2024 12:18:00 -0800 X-Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-266-B9pLzNzjPuqW8jP4ntVTZw-1; Wed, 07 Feb 2024 15:17:58 -0500 X-MC-Unique: B9pLzNzjPuqW8jP4ntVTZw-1 X-Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id B66341C04B68; Wed, 7 Feb 2024 20:17:57 +0000 (UTC) X-Received: from [10.39.195.126] (unknown [10.39.195.126]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8F4717595; Wed, 7 Feb 2024 20:17:56 +0000 (UTC) Message-ID: <30ee67da-0b7c-5e98-5e81-c3aff5f98077@redhat.com> Date: Wed, 7 Feb 2024 21:17:55 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH 1/3] UefiCpuPkg: Reduce and optimize access to attribute To: "Zhou, Jianfeng" , Pedro Falcato , "devel@edk2.groups.io" Cc: "Tan, Dun" , "Ni, Ray" , "Kumar, Rahul R" , Gerd Hoffmann References: <20240205140345.1437-1-dun.tan@intel.com> <20240205140345.1437-2-dun.tan@intel.com> From: "Laszlo Ersek" In-Reply-To: X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: o2qkoj6x1t7o8aJqbDxnKIPMx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=gqtPavL1; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On 2/7/24 01:47, Zhou, Jianfeng wrote: > Hi Laszlo, Pedro, > > Clarify one thing, this change is not for racing introduced by MP reading/writing to the same page table at the same time, but for unexpected behavior introduced by compiler. I thought that patch#1 was related to MP paging because: - patch#3 *was* indeed related to MP paging, and these patches are, well, part of the same series; - more importantly, the commit message on patch#1 explicitly says, "If *some other core* is accessing the page, it may leads to expection" -- emphasis mine. (The first patch also uses the word "optimize", which I find very misleading.) Furthermore, the logic change in the first patch targets CpuPageTableLib. As far as I can tell, the logic change bubbles up to the public PageTableMap() API. I don't know if that API is designed for just UP usage (a processor can use it only for manipulating its own page tables), or for MP usage. Given the larger context of patch#1, I assumed there was an MP context to patch#1 as well. (And, again, the commit message explicitly references other cores.) > As my understanding, MP reading/writing to the same page table at the same time is not recommended, perhaps, it is not allowed. > > For bit operation code, such as Pnle->Bits.Present = Attribute->Bits.Present, we might think it is atomic assignment, while not. The assembly code looks like: > and dword [rcx], 0xfffffffe > and eax, 0x1 > or [rcx], eax > In case Pnle->Bits.Present = 1, Attribute->Bits.Present = 1, we might think it is harmless, as the value not changed. While actually, > and dword [rcx], 0xfffffffe // the present bit set to 0 ---- this is unexpected !!!!! we don’t want the present bit set to 0! > and eax, 0x1 > or [rcx], eax // the present bit set to right value 1 > > Let's consider such a MP scenario: > 1) one processor executing instruction "and dword [rcx], 0xfffffffe" > 2) other processors happened to access the memory mapped by Pnle, it may lead to exception. > > We hit this case recently. Several engineers pay days for test, root case and verification: the reproducibility rate is low and not reproduced on every system. OK, so it *does* happen in an MP context, too. > > We can fix it by other solution, while we decided to upstream this change for: > 1) the change is harmless > 2) It is a defect > 3) It hard to debug and root cause > 4) We don't want other engineers to spend a lot of time dealing with this kind of problem. This is all very nice; I can accept that the present code change may be a useful practical improvement. My issue is that the commit message does not represent either the problem faithfully, or the scope / intent of the code changes. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115249): https://edk2.groups.io/g/devel/message/115249 Mute This Topic: https://groups.io/mt/104176232/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-