Re: [PATCH v3 2/2] Add new MSR modules (rdmsr/wrmsr)

[Jesús Diéguez Fernández]

El 22/3/19 a las 13:16, Daniel Kiper escribió:
> On Thu, Mar 21, 2019 at 05:23:58PM -0600, Eric Snowberg wrote:
>>> On Mar 7, 2019, at 5:26 PM, Jesús Diéguez Fernández <address@hidden> wrote:
> 
> [...]
> 
>>> diff --git a/include/grub/i386/rdmsr.h b/include/grub/i386/rdmsr.h
>>> new file mode 100644
>>> index 000000000..f6d7b72ca
>>> --- /dev/null
>>> +++ b/include/grub/i386/rdmsr.h
>>> @@ -0,0 +1,34 @@
>>> +/*
>>> + *  GRUB  --  GRand Unified Bootloader
>>> + *  Copyright (C) 2019  Free Software Foundation, Inc.
>>> + *
>>> + *  GRUB is free software: you can redistribute it and/or modify
>>> + *  it under the terms of the GNU General Public License as published by
>>> + *  the Free Software Foundation, either version 3 of the License, or
>>> + *  (at your option) any later version.
>>> + *
>>> + *  GRUB is distributed in the hope that it will be useful,
>>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + *  GNU General Public License for more details.
>>> + *
>>> + *  You should have received a copy of the GNU General Public License
>>> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#ifndef GRUB_RDMSR_H
>>> +#define GRUB_RDMSR_H 1
>>> +
>>> +/* TODO: Add a general protection exception handler.
>>> +         Accessing a reserved or unimplemented MSR address results in a 
>>> GP#. */
>>> +
>>> +extern __inline grub_uint64_t grub_msr_read (grub_uint32_t msr_id)
>>
>> I’m seeing a compile error with this patch:
>>
>> In file included from commands/i386/rdmsr.c:29:0:
>> ../include/grub/i386/rdmsr.h:27:29: error: no previous prototype for 
>> _grub_msr_read_ [-Werror=missing-prototypes]
>>  extern inline grub_uint64_t grub_msr_read (grub_uint32_t msr_id)
>>                              ^
>> cc1: all warnings being treated as errors
> 
> I am not able to reproduce this. Eric, could you tell us more about your
> build environment and how you compile the GRUB?
> 
> Anyway, I missed that during review but
> 
> extern inline grub_uint64_t grub_msr_read (grub_uint32_t msr_id)
> {
> ...
> 
> should be changed to
> 
> grub_uint64_t inline
> grub_msr_read (grub_uint32_t msr_id)
> {
> ...
> 
> Jesús, please post the fix for that.
> 
> Daniel
> 

Done. I inverted the order (inline first) to prevent this warning:

In file included from commands/i386/rdmsr.c:29:0:
../include/grub/i386/rdmsr.h:27:1: warning: ‘inline’ is not at beginning
of declaration [-Wold-style-declaration]
 grub_uint64_t inline
 ^~~~~~~~~~~~~
In file included from commands/i386/wrmsr.c:29:0:
../include/grub/i386/wrmsr.h:27:1: warning: ‘inline’ is not at beginning
of declaration [-Wold-style-declaration]
 void inline
 ^~~~

I also added the missing prototypes.

Jesus.