Re: [PATCH] x86/cpu: correct values for GDT_ENTRY_INIT

From: Andrew Cooper
Date: Thu Nov 26 2020 - 18:53:27 EST


On 26/11/2020 19:15, Andy Lutomirski wrote:
> On Thu, Nov 26, 2020 at 11:07 AM Lukas Bulwahn <lukas.bulwahn@xxxxxxxxx> wrote:
>> On Thu, Nov 26, 2020 at 6:16 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 26/11/2020 11:54, Lukas Bulwahn wrote:
>>>> Commit 1e5de18278e6 ("x86: Introduce GDT_ENTRY_INIT()") unintentionally
>>>> transformed a few 0xffff values to 0xfffff (note: five times "f" instead of
>>>> four) as part of the refactoring.
>>> The transformation in that change is correct.
>>>
>>> Segment bases are 20 bits wide in x86,

I of course meant segment limits here, rather than bases.

>>> Does:
>>>
>>> diff --git a/arch/x86/include/asm/desc_defs.h
>>> b/arch/x86/include/asm/desc_defs.h
>>> index f7e7099af595..9561f3c66e9e 100644
>>> --- a/arch/x86/include/asm/desc_defs.h
>>> +++ b/arch/x86/include/asm/desc_defs.h
>>> @@ -22,7 +22,7 @@ struct desc_struct {
>>>
>>> #define GDT_ENTRY_INIT(flags, base, limit) \
>>> { \
>>> - .limit0 = (u16) (limit), \
>>> + .limit0 = (u16) (limit) & 0xFFFF, \
>>> .limit1 = ((limit) >> 16) & 0x0F, \
>>> .base0 = (u16) (base), \
>>> .base1 = ((base) >> 16) & 0xFF, \
>>>
>>> fix the warning?
>>>
>> Thanks, I will try that out, and try compiling a 32-bit kernel as well.
> You should also try comparing the objdump output before and after your
> patch. objdump -D will produce bizarre output but should work.

Expanding on this a little, if that does indeed fix the sparse warning,
then I'd make an argument for this being a bug in sparse.  Explicitly
casting to u16 is semantically and intentionally identical to & 0xffff.

~Andrew