diff mbox

[OPW,kernel] This patch eliminates the warnings caused by -Wpointer-sign in the file arch/x86/kernel/xsave.c. The variables holding the register values have been changed from signed to unsinged in order to match the type of variables that they are assigne

Message ID 20131110014007.GA10049@debian
State New, archived
Headers show

Commit Message

Matina Maria Trompouki Nov. 10, 2013, 1:40 a.m. UTC
Signed-off-by: Matina Maria Trompouki <mtrompou@gmail.com>
---
 arch/x86/kernel/xsave.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Waskiewicz Jr, Peter P Nov. 10, 2013, 1:55 a.m. UTC | #1
On Sun, 2013-11-10 at 01:40 +0000, Matina Maria Trompouki wrote:

Hi Matina,

A few things regarding this patch.

First, this is something outside of the staging tree.  While this is ok
to work on, the OPW application cycle would like any changes submitted
to this list to only focus on staging.  That way Greg can take the
patches into that tree, and we won't have to deal with the larger
mailing lists during the application process.  So while the content of
this patch appears ok, this patch really belongs on the Linux Kernel
Mailing List (LKML) if you want it to be merged.

Second, I think you may have missed putting a blank line in between your
patch title, and your commit message.  If you notice, your whole commit
message is in the email subject.  Your git commit should look something
like this: 

subsystem: Short description of change

Detailed commit message covering change, why it's a good change
to make, etc., etc.

Signed-off-by: me <me@me.org>

If you miss the blank line in between the subsystem line and the commit
message, well, you get what happened here, and the whole message is the
subject line.

Also, when building your subject line, you want to put the subsystem on
the line.  That helps reviewers and maintainers to spot changes easier
in the sea of email that hits the mailing lists.  So a good subject for
this would be:

[PATCH] arch/x86: Fix warnings from -Wpointer-sign

or just:

[PATCH] x86: Fix warnings from -Wpointer-sign

Lastly, this type of a change is to core CPU architecture code.  This
code will be more scrutinized, since if it has a bug, well, most systems
on the planet won't work anymore if they pull this kernel.  I don't know
if you just build-tested this, or if you made sure to find a good test
case to exercise this code path.  This one you will want to make sure
you exercised the code path change, because I can guarantee you someone
will ask.

Regards,
-PJ

> Signed-off-by: Matina Maria Trompouki <mtrompou@gmail.com>
> ---
>  arch/x86/kernel/xsave.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/xsave.c b/arch/x86/kernel/xsave.c
> index 422fd82..136ed1e 100644
> --- a/arch/x86/kernel/xsave.c
> +++ b/arch/x86/kernel/xsave.c
> @@ -81,8 +81,8 @@ void __sanitize_i387_state(struct task_struct *tsk)
>  	 */
>  	while (xstate_bv) {
>  		if (xstate_bv & 0x1) {
> -			int offset = xstate_offsets[feature_bit];
> -			int size = xstate_sizes[feature_bit];
> +			unsigned int offset = xstate_offsets[feature_bit];
> +			unsigned int size = xstate_sizes[feature_bit];
>  
>  			memcpy(((void *) fx) + offset,
>  			       ((void *) init_xstate_buf) + offset,
> @@ -459,11 +459,11 @@ static inline void xstate_enable(void)
>   */
>  static void __init setup_xstate_features(void)
>  {
> -	int eax, ebx, ecx, edx, leaf = 0x2;
> +	unsigned int eax, ebx, ecx, edx, leaf = 0x2;
>  
>  	xstate_features = fls64(pcntxt_mask);
> -	xstate_offsets = alloc_bootmem(xstate_features * sizeof(int));
> -	xstate_sizes = alloc_bootmem(xstate_features * sizeof(int));
> +	xstate_offsets = alloc_bootmem(xstate_features * sizeof(unsigned int));
> +	xstate_sizes = alloc_bootmem(xstate_features * sizeof(unsigned int));
>  
>  	do {
>  		cpuid_count(XSTATE_CPUID, leaf, &eax, &ebx, &ecx, &edx);
> -- 
> 1.8.4.rc3
>
Matina Maria Trompouki Nov. 10, 2013, 2:19 p.m. UTC | #2
Hi Peter,

>First, this is something outside of the staging tree. While this is ok
>to work on, the OPW application cycle would like any changes submitted
>to this list to only focus on staging. That way Greg can take the
>patches into that tree, and we won't have to deal with the larger
>mailing lists during the application process. So while the content of
>this patch appears ok, this patch really belongs on the Linux Kernel
>Mailing List (LKML) if you want it to be merged.

You're right, I was working on the latest linux kernel tree instead of
Greg's
staging tree. I have now applied my patch on Greg's tree and I will send it.

>Second, I think you may have missed putting a blank line in between your
>patch title, and your commit message.

I'm sorry about this, it was my first patch. Thank you for the detailed
explanation, I will follow your advice in the patch I will send.

>Lastly, this type of a change is to core CPU architecture code. This
>code will be more scrutinized, since if it has a bug, well, most systems
>on the planet won't work anymore if they pull this kernel. I don't know
>if you just build-tested this, or if you made sure to find a good test
>case to exercise this code path. This one you will want to make sure
>you exercised the code path change, because I can guarantee you someone
>will ask.

I understand your concerns about the correctness of the code, since it is
in a
really critical part of the kernel. However, my changes are harmless for
the
following reasons:
a) my change from int to unsigned in line 462, is in line with the
pre-existing
line 528 in function xstate_enable_boot_cpu, which performs similar work
b) variables offset (line 84) and size (line 85) must always be non-negative
because offset is used in pointer arithmetic and size is used to specify the
number of bytes to be copied by memcpy, otherwise they might cause undefined
results or corruption.
c) It is quite difficult to produce a test case for this piece of code, so
for
this reason I have created a short (userlevel) test to verify that my
changes
do not introduce any change in the expected behaviour. Actually this test
shows
that the original code behaves correctly, too, but there is a conceptual
error
of assigning signed variables to unsigned and vice versa. In the past I have
experienced problems related to these issues, when working with image
processing, therefore I think it will be safer if this patch will be
applied.

You can find the code of this test below. Do you think I have to include the
above information (and the test case) also in my patch?

Finally, I would like to ask if this contribution is enough for my
application
in opw to be considered.

Regards,
Matina Maria Trompouki

#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
#include <string.h>

static unsigned int *xstate_offsets, *xstate_sizes, xstate_features;

int main(void)
{
    /*original register values to be saved*/
    unsigned int eax, ebx, ecx, edx, leaf = 0x2;
    /*restored register values*/
    unsigned int retrieved_eax, retrieved_ebx;
    /*emulate*/
    unsigned char buf[8]={0xF0, 0xFF, 0xFF, 0xFF,
                          0xF0,  0x0,  0x0,  0x0};

    /*emulate cpuid_count(XSTATE_CPUID, leaf, &eax, &ebx, &ecx, &edx);*/
    xstate_features = 5;
    memcpy(&eax, buf, 4);
    memcpy(&ebx, buf+4, 4);
    printf("eax:0x%x\n",eax);
    printf("eax:0x%x\n",ebx);
    printf("eax:%d\n",eax);
    printf("eax:%d\n",ebx);
    printf("eax:%u\n",eax);
    printf("eax:%u\n",ebx);

    xstate_offsets = malloc(xstate_features * sizeof(unsigned int));
    xstate_sizes = malloc(xstate_features * sizeof(unsigned int));

    /*save the register values*/
    xstate_offsets[leaf] = ebx;
    xstate_sizes[leaf] = eax;

    /*retrieve the saved the register values*/
    retrieved_ebx = xstate_offsets[leaf];
    retrieved_eax = xstate_sizes[leaf];

    /*ensure that the original register value and the retrieved are the
same*/
    assert(retrieved_eax == eax);
    assert(retrieved_ebx == ebx);

    return 0;
}



On Sun, Nov 10, 2013 at 2:55 AM, Waskiewicz Jr, Peter P <
peter.p.waskiewicz.jr@intel.com> wrote:

> On Sun, 2013-11-10 at 01:40 +0000, Matina Maria Trompouki wrote:
>
> Hi Matina,
>
> A few things regarding this patch.
>
> First, this is something outside of the staging tree.  While this is ok
> to work on, the OPW application cycle would like any changes submitted
> to this list to only focus on staging.  That way Greg can take the
> patches into that tree, and we won't have to deal with the larger
> mailing lists during the application process.  So while the content of
> this patch appears ok, this patch really belongs on the Linux Kernel
> Mailing List (LKML) if you want it to be merged.
>
> Second, I think you may have missed putting a blank line in between your
> patch title, and your commit message.  If you notice, your whole commit
> message is in the email subject.  Your git commit should look something
> like this:
>
> subsystem: Short description of change
>
> Detailed commit message covering change, why it's a good change
> to make, etc., etc.
>
> Signed-off-by: me <me@me.org>
>
> If you miss the blank line in between the subsystem line and the commit
> message, well, you get what happened here, and the whole message is the
> subject line.
>
> Also, when building your subject line, you want to put the subsystem on
> the line.  That helps reviewers and maintainers to spot changes easier
> in the sea of email that hits the mailing lists.  So a good subject for
> this would be:
>
> [PATCH] arch/x86: Fix warnings from -Wpointer-sign
>
> or just:
>
> [PATCH] x86: Fix warnings from -Wpointer-sign
>
> Lastly, this type of a change is to core CPU architecture code.  This
> code will be more scrutinized, since if it has a bug, well, most systems
> on the planet won't work anymore if they pull this kernel.  I don't know
> if you just build-tested this, or if you made sure to find a good test
> case to exercise this code path.  This one you will want to make sure
> you exercised the code path change, because I can guarantee you someone
> will ask.
>
> Regards,
> -PJ
>
> > Signed-off-by: Matina Maria Trompouki <mtrompou@gmail.com>
> > ---
> >  arch/x86/kernel/xsave.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/kernel/xsave.c b/arch/x86/kernel/xsave.c
> > index 422fd82..136ed1e 100644
> > --- a/arch/x86/kernel/xsave.c
> > +++ b/arch/x86/kernel/xsave.c
> > @@ -81,8 +81,8 @@ void __sanitize_i387_state(struct task_struct *tsk)
> >        */
> >       while (xstate_bv) {
> >               if (xstate_bv & 0x1) {
> > -                     int offset = xstate_offsets[feature_bit];
> > -                     int size = xstate_sizes[feature_bit];
> > +                     unsigned int offset = xstate_offsets[feature_bit];
> > +                     unsigned int size = xstate_sizes[feature_bit];
> >
> >                       memcpy(((void *) fx) + offset,
> >                              ((void *) init_xstate_buf) + offset,
> > @@ -459,11 +459,11 @@ static inline void xstate_enable(void)
> >   */
> >  static void __init setup_xstate_features(void)
> >  {
> > -     int eax, ebx, ecx, edx, leaf = 0x2;
> > +     unsigned int eax, ebx, ecx, edx, leaf = 0x2;
> >
> >       xstate_features = fls64(pcntxt_mask);
> > -     xstate_offsets = alloc_bootmem(xstate_features * sizeof(int));
> > -     xstate_sizes = alloc_bootmem(xstate_features * sizeof(int));
> > +     xstate_offsets = alloc_bootmem(xstate_features * sizeof(unsigned
> int));
> > +     xstate_sizes = alloc_bootmem(xstate_features * sizeof(unsigned
> int));
> >
> >       do {
> >               cpuid_count(XSTATE_CPUID, leaf, &eax, &ebx, &ecx, &edx);
> > --
> > 1.8.4.rc3
> >
>
>
Waskiewicz Jr, Peter P Nov. 10, 2013, 8:18 p.m. UTC | #3
And I should read my email further down before responding to a new
version of a patch...see below.

On Sun, 2013-11-10 at 15:19 +0100, Martina Trompouki wrote:
> Hi Peter, 
> 
> >First, this is something outside of the staging tree. While this is
> ok
> >to work on, the OPW application cycle would like any changes
> submitted
> >to this list to only focus on staging. That way Greg can take the
> >patches into that tree, and we won't have to deal with the larger
> >mailing lists during the application process. So while the content of
> >this patch appears ok, this patch really belongs on the Linux Kernel
> >Mailing List (LKML) if you want it to be merged.
> 
> You're right, I was working on the latest linux kernel tree instead of
> Greg's
> staging tree. I have now applied my patch on Greg's tree and I will
> send it.

Right.  But staging is under drivers/staging.  Just because you're using
Greg's tree that is the staging git tree doesn't imply that it's all
staging.  This is also how all the other kernel trees work on
git.kernel.org; they are used to maintain a certain portion of the
kernel, and then they eventually get all rolled back up to a final
release by Linus.

>  
> >Second, I think you may have missed putting a blank line in between
> your
> >patch title, and your commit message. 
> 
> I'm sorry about this, it was my first patch. Thank you for the
> detailed 
> explanation, I will follow your advice in the patch I will send.
> 
> >Lastly, this type of a change is to core CPU architecture code. This
> >code will be more scrutinized, since if it has a bug, well, most
> systems
> >on the planet won't work anymore if they pull this kernel. I don't
> know
> >if you just build-tested this, or if you made sure to find a good
> test
> >case to exercise this code path. This one you will want to make sure
> >you exercised the code path change, because I can guarantee you
> someone
> >will ask.
> 
> I understand your concerns about the correctness of the code, since it
> is in a
> really critical part of the kernel. However, my changes are harmless
> for the 
> following reasons:

I never assume any change to code is harmless.  I've been burned by that
too many times in the past.  I don't disagree this change appears
harmless (and it probably is harmless), but early boot code and early
CPU initialization code is very heavily guarded.

> a) my change from int to unsigned in line 462, is in line with the
> pre-existing 
> line 528 in function xstate_enable_boot_cpu, which performs similar
> work
> b) variables offset (line 84) and size (line 85) must always be
> non-negative
> because offset is used in pointer arithmetic and size is used to
> specify the
> number of bytes to be copied by memcpy, otherwise they might cause
> undefined
> results or corruption.
> c) It is quite difficult to produce a test case for this piece of
> code, so for
> this reason I have created a short (userlevel) test to verify that my
> changes 
> do not introduce any change in the expected behaviour. Actually this
> test shows
> that the original code behaves correctly, too, but there is a
> conceptual error
> of assigning signed variables to unsigned and vice versa. In the past
> I have
> experienced problems related to these issues, when working with image
> processing, therefore I think it will be safer if this patch will be
> applied.
> You can find the code of this test below. Do you think I have to
> include the
> above information (and the test case) also in my patch?
> 
> Finally, I would like to ask if this contribution is enough for my
> application
> in opw to be considered.

This is a good find, I don't disagree with that.  However, for the OPW
application process, it requests patches be submitted for things under
drivers/staging.  Our smaller list for opw-kernel can't apply a patch to
core x86 architecture code, that has to go through LKML.  So I'd
recommend you look at http://kernelnewbies.org/OPWfirstpatch and look at
Section 9, Find a driver to clean up.  That will help get you pointed at
the right area of the tree for the application process.

Hope this helps, sorry for the confusion.

Regards,
-PJ

> Regards, 
> Matina Maria Trompouki
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <assert.h>
> #include <string.h>
> 
> static unsigned int *xstate_offsets, *xstate_sizes, xstate_features;
> 
> int main(void)
> {
>     /*original register values to be saved*/
>     unsigned int eax, ebx, ecx, edx, leaf = 0x2;
>     /*restored register values*/
>     unsigned int retrieved_eax, retrieved_ebx;
>     /*emulate*/
>     unsigned char buf[8]={0xF0, 0xFF, 0xFF, 0xFF,
>                           0xF0,  0x0,  0x0,  0x0};
> 
>     /*emulate cpuid_count(XSTATE_CPUID, leaf, &eax, &ebx, &ecx,
> &edx);*/
>     xstate_features = 5;
>     memcpy(&eax, buf, 4);
>     memcpy(&ebx, buf+4, 4);
>     printf("eax:0x%x\n",eax);
>     printf("eax:0x%x\n",ebx);
>     printf("eax:%d\n",eax);
>     printf("eax:%d\n",ebx);
>     printf("eax:%u\n",eax);
>     printf("eax:%u\n",ebx);
> 
>     xstate_offsets = malloc(xstate_features * sizeof(unsigned int));
>     xstate_sizes = malloc(xstate_features * sizeof(unsigned int));
> 
>     /*save the register values*/
>     xstate_offsets[leaf] = ebx;
>     xstate_sizes[leaf] = eax;
> 
>     /*retrieve the saved the register values*/
>     retrieved_ebx = xstate_offsets[leaf];
>     retrieved_eax = xstate_sizes[leaf];
> 
>     /*ensure that the original register value and the retrieved are
> the same*/
>     assert(retrieved_eax == eax);
>     assert(retrieved_ebx == ebx);
> 
>     return 0;
> }
> 
> 
> 
> 
> On Sun, Nov 10, 2013 at 2:55 AM, Waskiewicz Jr, Peter P
> <peter.p.waskiewicz.jr@intel.com> wrote:
>         On Sun, 2013-11-10 at 01:40 +0000, Matina Maria Trompouki
>         wrote:
>         
>         Hi Matina,
>         
>         A few things regarding this patch.
>         
>         First, this is something outside of the staging tree.  While
>         this is ok
>         to work on, the OPW application cycle would like any changes
>         submitted
>         to this list to only focus on staging.  That way Greg can take
>         the
>         patches into that tree, and we won't have to deal with the
>         larger
>         mailing lists during the application process.  So while the
>         content of
>         this patch appears ok, this patch really belongs on the Linux
>         Kernel
>         Mailing List (LKML) if you want it to be merged.
>         
>         Second, I think you may have missed putting a blank line in
>         between your
>         patch title, and your commit message.  If you notice, your
>         whole commit
>         message is in the email subject.  Your git commit should look
>         something
>         like this:
>         
>         subsystem: Short description of change
>         
>         Detailed commit message covering change, why it's a good
>         change
>         to make, etc., etc.
>         
>         Signed-off-by: me <me@me.org>
>         
>         If you miss the blank line in between the subsystem line and
>         the commit
>         message, well, you get what happened here, and the whole
>         message is the
>         subject line.
>         
>         Also, when building your subject line, you want to put the
>         subsystem on
>         the line.  That helps reviewers and maintainers to spot
>         changes easier
>         in the sea of email that hits the mailing lists.  So a good
>         subject for
>         this would be:
>         
>         [PATCH] arch/x86: Fix warnings from -Wpointer-sign
>         
>         or just:
>         
>         [PATCH] x86: Fix warnings from -Wpointer-sign
>         
>         Lastly, this type of a change is to core CPU architecture
>         code.  This
>         code will be more scrutinized, since if it has a bug, well,
>         most systems
>         on the planet won't work anymore if they pull this kernel.  I
>         don't know
>         if you just build-tested this, or if you made sure to find a
>         good test
>         case to exercise this code path.  This one you will want to
>         make sure
>         you exercised the code path change, because I can guarantee
>         you someone
>         will ask.
>         
>         Regards,
>         -PJ
>         
>         > Signed-off-by: Matina Maria Trompouki <mtrompou@gmail.com>
>         > ---
>         >  arch/x86/kernel/xsave.c | 10 +++++-----
>         >  1 file changed, 5 insertions(+), 5 deletions(-)
>         >
>         > diff --git a/arch/x86/kernel/xsave.c
>         b/arch/x86/kernel/xsave.c
>         > index 422fd82..136ed1e 100644
>         > --- a/arch/x86/kernel/xsave.c
>         > +++ b/arch/x86/kernel/xsave.c
>         > @@ -81,8 +81,8 @@ void __sanitize_i387_state(struct
>         task_struct *tsk)
>         >        */
>         >       while (xstate_bv) {
>         >               if (xstate_bv & 0x1) {
>         > -                     int offset =
>         xstate_offsets[feature_bit];
>         > -                     int size = xstate_sizes[feature_bit];
>         > +                     unsigned int offset =
>         xstate_offsets[feature_bit];
>         > +                     unsigned int size =
>         xstate_sizes[feature_bit];
>         >
>         >                       memcpy(((void *) fx) + offset,
>         >                              ((void *) init_xstate_buf) +
>         offset,
>         > @@ -459,11 +459,11 @@ static inline void xstate_enable(void)
>         >   */
>         >  static void __init setup_xstate_features(void)
>         >  {
>         > -     int eax, ebx, ecx, edx, leaf = 0x2;
>         > +     unsigned int eax, ebx, ecx, edx, leaf = 0x2;
>         >
>         >       xstate_features = fls64(pcntxt_mask);
>         > -     xstate_offsets = alloc_bootmem(xstate_features *
>         sizeof(int));
>         > -     xstate_sizes = alloc_bootmem(xstate_features *
>         sizeof(int));
>         > +     xstate_offsets = alloc_bootmem(xstate_features *
>         sizeof(unsigned int));
>         > +     xstate_sizes = alloc_bootmem(xstate_features *
>         sizeof(unsigned int));
>         >
>         >       do {
>         >               cpuid_count(XSTATE_CPUID, leaf, &eax, &ebx,
>         &ecx, &edx);
>         > --
>         > 1.8.4.rc3
>         >
>         
> 
>
Matina Maria Trompouki Nov. 17, 2013, 8:25 p.m. UTC | #4
Hi Peter,

I apologize for the late reply, I was trying to finish with some
contributions
in order not to miss the deadline. Thank you very much for the explanation,
I
understand why my patch was out of the scope of the opw application process.
Fortunately you pointed it out early, so I had enough time to do my
contribution
and have some patches accepted.

At the moment I had to decide about my first contribution, I selected this
because I noticed a lot of warnings of this type while I was compiling the
kernel, and I decided to get rid of some of them. For this reason I set
-Werror=pointer-sign in the Makefile and I started correcting all the
errors of
this type I was getting. The majority of them were easily removed by
casting,
but I didn't include these changes in my patch because I wasn't sure if its
desirable to silence these warnings, by doing explicitly what the compiler
does
by default, and increase the chances that my patch would be accepted.

So, I sent only the patch about a piece of code that looked suspicious and
I was
sure that can cause problems that I faced in the past. Moreover, I could
give
good arguments about my proposed changes.

Do you think I should try to send this patch in the Linux Kernel Mailing
List?

Best regards,
Matina Maria Trompouki
Waskiewicz Jr, Peter P Nov. 19, 2013, 5:42 p.m. UTC | #5
On Sun, 2013-11-17 at 21:25 +0100, Martina Trompouki wrote:
> 
> Hi Peter, 
> 
> I apologize for the late reply, I was trying to finish with some
> contributions
> in order not to miss the deadline. Thank you very much for the
> explanation, I
> understand why my patch was out of the scope of the opw application
> process.
> Fortunately you pointed it out early, so I had enough time to do my
> contribution
> and have some patches accepted.

Good!

> At the moment I had to decide about my first contribution, I selected
> this
> because I noticed a lot of warnings of this type while I was compiling
> the
> kernel, and I decided to get rid of some of them. For this reason I
> set
> -Werror=pointer-sign in the Makefile and I started correcting all the
> errors of
> this type I was getting. The majority of them were easily removed by
> casting,
> but I didn't include these changes in my patch because I wasn't sure
> if its
> desirable to silence these warnings, by doing explicitly what the
> compiler does
> by default, and increase the chances that my patch would be accepted.

This is a tricky one to give any recommendations.  While it's good
practice to not ignore warnings, they're not always a bad thing.  The
kernel is not a typical program in many ways, so some warnings may be
there based on levels of indirection or abstraction that may be
happening.

The other reason this can be a tricky subject is not all Linux
distributions ship their compilers with the same default compiler
options.  So someone running Red Hat 6.4 might see different warnings
from a kernel build than someone running Ubuntu 13.10.  So fixing a
compiler warning by using a compiler switch that isn't necessarily in a
current distribution will be a harder sell to the community.

> So, I sent only the patch about a piece of code that looked suspicious
> and I was
> sure that can cause problems that I faced in the past. Moreover, I
> could give
> good arguments about my proposed changes.
> 
> Do you think I should try to send this patch in the Linux Kernel
> Mailing List?

It never hurts to try by all means.  If you do send it to the Linux
Kernel list, please be sure to format the patch correctly, have a good
commit message, and a proper subject line.

If you want to have someone look at the patch before you send it, you
can send it directly to me and I'll review it before you send it off to
the rest of the world.

Cheers,
-PJ
diff mbox

Patch

diff --git a/arch/x86/kernel/xsave.c b/arch/x86/kernel/xsave.c
index 422fd82..136ed1e 100644
--- a/arch/x86/kernel/xsave.c
+++ b/arch/x86/kernel/xsave.c
@@ -81,8 +81,8 @@  void __sanitize_i387_state(struct task_struct *tsk)
 	 */
 	while (xstate_bv) {
 		if (xstate_bv & 0x1) {
-			int offset = xstate_offsets[feature_bit];
-			int size = xstate_sizes[feature_bit];
+			unsigned int offset = xstate_offsets[feature_bit];
+			unsigned int size = xstate_sizes[feature_bit];
 
 			memcpy(((void *) fx) + offset,
 			       ((void *) init_xstate_buf) + offset,
@@ -459,11 +459,11 @@  static inline void xstate_enable(void)
  */
 static void __init setup_xstate_features(void)
 {
-	int eax, ebx, ecx, edx, leaf = 0x2;
+	unsigned int eax, ebx, ecx, edx, leaf = 0x2;
 
 	xstate_features = fls64(pcntxt_mask);
-	xstate_offsets = alloc_bootmem(xstate_features * sizeof(int));
-	xstate_sizes = alloc_bootmem(xstate_features * sizeof(int));
+	xstate_offsets = alloc_bootmem(xstate_features * sizeof(unsigned int));
+	xstate_sizes = alloc_bootmem(xstate_features * sizeof(unsigned int));
 
 	do {
 		cpuid_count(XSTATE_CPUID, leaf, &eax, &ebx, &ecx, &edx);