[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 5/5] x86_64: add 64-bit syscall entry point
From: |
Sergey Bugaev |
Subject: |
Re: [PATCH 5/5] x86_64: add 64-bit syscall entry point |
Date: |
Tue, 28 Feb 2023 10:45:47 +0300 |
On Tue, Feb 28, 2023 at 10:18 AM Samuel Thibault
<samuel.thibault@gnu.org> wrote:
>
> Sergey Bugaev, le mar. 28 févr. 2023 09:39:40 +0300, a ecrit:
> > > + : \
> > > + : "c" (regaddr), "a" (low), "d" (high) \
> > > + : "memory" \
> > > + );
> > > +}
> >
> > Why "memory" here? Can wrmsr clobber unrelated memory?
>
> No, but if you don't put a memory clobber here, the compiler will
> optimize the asm away since it's not said to have side effect. Put
> another way, the MSR register is some kind of memory.
No? -- that's what the "volatile" is for [0]. "memory" simply tells
GCC that this piece of asm, *if executed*, may clobber arbitrary
memory; it does not prevent GCC from optimizing the whole thing away.
Here's [1] a simple demo on godbolt that shows both of these to be
true.
[0]: https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile
[1]: https://godbolt.org/z/M453v9Gco
Besides, "asm statements that have no output operands and asm goto
statements, are implicitly volatile", so even volatile is not
required, but better be explicit.
> > > +static inline uint64_t rdmsr(uint32_t regaddr)
> > > +{
> > > + uint32_t low, high;
> > > + asm volatile("rdmsr\n" \
> > > + : "=a" (low), "=d" (high) \
> > > + : "c" (regaddr) \
> > > + );
> > > + return ((uint64_t)high << 32) | low;
> >
> > Ditto about spacing -- and does this need volatile? As in, does
> > reading from a MSR have side effects that we're interested in,
>
> IIRC it does?
Okay.
>
> > > diff --git a/i386/include/mach/i386/syscall_sw.h
> > > b/i386/include/mach/i386/syscall_sw.h
> > > index 86f6ff2f..20ef7c13 100644
> > > --- a/i386/include/mach/i386/syscall_sw.h
> > > +++ b/i386/include/mach/i386/syscall_sw.h
> >
> > OK, so the x86_64 syscall definition stays in i386/syscall_sw.h, and
> > not in a separate x86_64/syscall_sw.h file? That's what I thought. In
> > this case, we do want that mach-machine patch in glibc. Samuel, does
> > this make sense to you?
>
> Better separate them indeed.
>
> > Predicating on USER32 is not really going to work here.
>
> Partly because of that :)
But not that USER32 makes sense here either, userspace is either
64-bit or not, there is no middle state. I.e. just drop that && !
defined(USER32), it's pointless since USER32 is never going to be
defined.
Either a separate file or not is fine by me, but let's settle on something.
Sergey
- Re: [PATCH 5/5] x86_64: add 64-bit syscall entry point, (continued)
- Re: [PATCH 5/5] x86_64: add 64-bit syscall entry point, Luca Dariz, 2023/02/28
- Re: [PATCH 5/5] x86_64: add 64-bit syscall entry point, Samuel Thibault, 2023/02/28
- Re: [PATCH 5/5] x86_64: add 64-bit syscall entry point, Sergey Bugaev, 2023/02/28
- Re: [PATCH 5/5] x86_64: add 64-bit syscall entry point, Samuel Thibault, 2023/02/28
- Re: [PATCH 5/5] x86_64: add 64-bit syscall entry point, Sergey Bugaev, 2023/02/28
- Re: [PATCH 5/5] x86_64: add 64-bit syscall entry point, Samuel Thibault, 2023/02/28
- Re: [PATCH 5/5] x86_64: add 64-bit syscall entry point, Sergey Bugaev, 2023/02/28
Re: [PATCH 5/5] x86_64: add 64-bit syscall entry point, Sergey Bugaev, 2023/02/28
Re: [PATCH 0/5] basic syscall support on x86_64, Samuel Thibault, 2023/02/27