[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Eclair false positive] Re: [PATCH] x86/msr: Rework wrmsr_safe() using asm goto()
- To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- From: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
- Date: Thu, 22 May 2025 14:45:49 +0200
- Arc-authentication-results: i=1; bugseng.com; arc=none smtp.remote-ip=162.55.131.47
- Arc-message-signature: i=1; d=bugseng.com; s=openarc; a=rsa-sha256; c=relaxed/relaxed; t=1747917950; h=DKIM-Signature:MIME-Version:Date:From:To:Cc:Subject:In-Reply-To: References:Message-ID:X-Sender:Organization:Content-Type: Content-Transfer-Encoding; bh=/byCIgRj4kR7pIiwC4BW3reIywL/OWkrGjOXyNmD55k=; b=r9RABhdWscKDn1w7fZ6LMKi3emqBlzzusz4sGcViVKOBcWsWmJgXWabo0TNtua0ILtpe WEJKvJB66LONKiMuI7SKZ1xkgWknNsnxfM1tugii+UgSzW1x/gxiyiAnOd/ppqOrWujKJ XC9kxinrGlh6xtWe9SzmkZ1Utx1CPVFzAaWZhXjQ3t90oXu/LjRqTpm5963tTlkV7hOW3 hmPECtHKBlA07dQuBgwa+JgDTIm/+Q41pSb2NZZ6YOkr13PdCBNMZin6A87GPsb8R3oGj E2ScTYZMMB9wmYU+VyJkukGmgjehbQkVTTjoIgzFwSbKbhEFfzr42YHLELo7BkECIL5V9 i5dxBXJwkYYnkxF4ErcOVNsbQ0xFBESs4xaKGTIEPrzGk/zdNvUJpPa2XoF7JiGhgVvT6 Jf+ZKG4+9hLvcSezSZzsEhDTrLedG/K6Nv0p0F+GfekE3hM8OfFoo8Qg7tChyPGTOhIjs VD4Rhzb5f09Ox16YtJjKlD/cRbxTrm+59x/2fmuiRePUSxtPEodXbgkaz3bstfM8bDz69 bw/B1GUCSW4+INqkd+HeQGCjRBfkNnrBrUVIYwJ447mpqQZUnmTcrigwnh5j0303Wdh2y d/lW/Ywv+D7kDcclLvQk+2603pSk9So3uAKeiOkNZQjyvspkI4WqP+5xE2GNzyI=
- Arc-seal: i=1; d=bugseng.com; s=openarc; a=rsa-sha256; cv=none; t=1747917950; b=ku5n8v/yax0xZ5Cve20i2DM/J5ihSpqynsFSJrpR3a4Whg+DL4X5b/tjOWBuUcPWmi7k GjLJlNqGtCC42chPPI0aTr98jXbuaSCns1/tsWKUxyRH2V12l63AnQNMmgrI1z5K2pMav WbZhyZ1sb4PgYFvsGvO6UZlAAyfgi0Ur7jygr3115XyF7XoKdWSkqFjcq8E0KSvaemrfQ wPYE4PZ3fJTkK/2missN0vylYny3SCRrb5AWQ2pHac6KQU1fnB7iGKzzG1Q6xyvFshvOf 1RkWHVObGxvHNY8pEvn0iLNcJMnyLyM0voWkgJnsgswnJ81yBowOZiHw3KS3nRvVDqP+7 k/Rd1+s3x8jS2FngnyIlnZUgNma18FC2CQsOMGxNY/FMEB7zaf+CiVVof3O6NHDYJo2aq 1QxavB32ZzfbMrPx/JzAebFgwOhkzTflO8Bq/yQBCM6XsFb4mq3NQ81F50JOvqW3uSysl 6HoKasgF9gNrOGlx9h5pWlb1P8hjP2CWjI1RpUAod4nWPSEyRhNdebzU1rMZqYPQAWqgT 0PPT4FBQJn3PCeuNhvgpmdf3E7c+V1yLgK2LSpvAjpn7TZmY05Vps1KkmGYYgFUDNsrqM h9JUyH1YAkCwrsKIr2VarvfzAgXamKQNE0J1r8zFsY2jSSHL6SLDzH4iyV89mZc=
- Authentication-results: bugseng.com; arc=none smtp.remote-ip=162.55.131.47
- Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, consulting@xxxxxxxxxxx, Stefano Stabellini <sstabellini@xxxxxxxxxx>
- Delivery-date: Thu, 22 May 2025 12:46:05 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 2025-05-21 20:00, Andrew Cooper wrote:
On 21/05/2025 3:36 pm, Andrew Cooper wrote:
diff --git a/xen/arch/x86/include/asm/msr.h
b/xen/arch/x86/include/asm/msr.h
index 0d3b1d637488..4c4f18b3a54d 100644
--- a/xen/arch/x86/include/asm/msr.h
+++ b/xen/arch/x86/include/asm/msr.h
@@ -69,20 +69,20 @@ static inline void wrmsr_ns(uint32_t msr, uint32_t
lo, uint32_t hi)
/* wrmsr with exception handling */
static inline int wrmsr_safe(unsigned int msr, uint64_t val)
{
- int rc;
- uint32_t lo, hi;
- lo = (uint32_t)val;
- hi = (uint32_t)(val >> 32);
-
- __asm__ __volatile__(
- "1: wrmsr\n2:\n"
- ".section .fixup,\"ax\"\n"
- "3: movl %5,%0\n; jmp 2b\n"
- ".previous\n"
- _ASM_EXTABLE(1b, 3b)
- : "=&r" (rc)
- : "c" (msr), "a" (lo), "d" (hi), "0" (0), "i" (-EFAULT));
- return rc;
+ uint32_t lo = val, hi = val >> 32;
+
+ asm_inline goto (
+ "1: wrmsr\n\t"
+ _ASM_EXTABLE(1b, %l[fault])
+ :
+ : "a" (lo), "c" (msr), "d" (hi)
+ :
+ : fault );
+
+ return 0;
+
+ fault:
+ return -EFAULT;
}
It turns out this is the first piece of Eclair-scanned code using asm
goto.
https://gitlab.com/xen-project/hardware/xen-staging/-/jobs/10108558677
(The run also contained an equivalent change to xsetbv())
We're getting R1.1 and R2.6 violations.
R1.1 complains about [STD.adrslabl] "label address" not being valid
C99.
R2.6 complains about unused labels.
I expect this means that Eclair doesn't know how to interpret asm
goto()
yet. The labels listed are reachable from inside the asm block.
That has been fixed upstream. I'll reach out to you when that fix
trickles down to the runners, so that you're able to test and push that
change.
Thanks,
Nicola
From a qualification point of view, this allows for some extensive
optimisations dropping emitted code.
~Andrew
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253
|
