[PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()

[PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 855 bytes --]

In the light of AMD erratum #700, and given that these checks happen
for debugging purposes only and also only in debug builds of the
hypervisor, make the failures non-fatal.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1544,11 +1544,11 @@ static int read_descriptor(unsigned int 
             asm volatile (
                 "larl %2,%0 ; setz %1"
                 : "=r" (a), "=qm" (valid) : "rm" (sel));
-            BUG_ON(valid && ((a & 0x00f0ff00) != *ar));
+            WARN_ON(valid && ((a & 0x00f0ff00) != *ar));
             asm volatile (
                 "lsll %2,%0 ; setz %1"
                 : "=r" (l), "=qm" (valid) : "rm" (sel));
-            BUG_ON(valid && (l != *limit));
+            WARN_ON(valid && (l != *limit));
         }
 #endif
     }




[-- Attachment #2: amd-erratum-700.patch --]
[-- Type: text/plain, Size: 910 bytes --]

x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()

In the light of AMD erratum #700, and given that these checks happen
for debugging purposes only and also only in debug builds of the
hypervisor, make the failures non-fatal.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1544,11 +1544,11 @@ static int read_descriptor(unsigned int 
             asm volatile (
                 "larl %2,%0 ; setz %1"
                 : "=r" (a), "=qm" (valid) : "rm" (sel));
-            BUG_ON(valid && ((a & 0x00f0ff00) != *ar));
+            WARN_ON(valid && ((a & 0x00f0ff00) != *ar));
             asm volatile (
                 "lsll %2,%0 ; setz %1"
                 : "=r" (l), "=qm" (valid) : "rm" (sel));
-            BUG_ON(valid && (l != *limit));
+            WARN_ON(valid && (l != *limit));
         }
 #endif
     }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()

[Keir Fraser]

On 15/12/2011 10:25, "Jan Beulich" <JBeulich@suse.com> wrote:

> In the light of AMD erratum #700, and given that these checks happen
> for debugging purposes only and also only in debug builds of the
> hypervisor, make the failures non-fatal.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Is this erratum published?

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1544,11 +1544,11 @@ static int read_descriptor(unsigned int
>              asm volatile (
>                  "larl %2,%0 ; setz %1"
>                  : "=r" (a), "=qm" (valid) : "rm" (sel));
> -            BUG_ON(valid && ((a & 0x00f0ff00) != *ar));
> +            WARN_ON(valid && ((a & 0x00f0ff00) != *ar));
>              asm volatile (
>                  "lsll %2,%0 ; setz %1"
>                  : "=r" (l), "=qm" (valid) : "rm" (sel));
> -            BUG_ON(valid && (l != *limit));
> +            WARN_ON(valid && (l != *limit));
>          }
>  #endif
>      }
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()

[Jan Beulich]

>>> On 15.12.11 at 13:36, Keir Fraser <keir@xen.org> wrote:
> On 15/12/2011 10:25, "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>> In the light of AMD erratum #700, and given that these checks happen
>> for debugging purposes only and also only in debug builds of the
>> hypervisor, make the failures non-fatal.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Is this erratum published?

Yes, it is in their most recent public revision guide.

Jan

>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -1544,11 +1544,11 @@ static int read_descriptor(unsigned int
>>              asm volatile (
>>                  "larl %2,%0 ; setz %1"
>>                  : "=r" (a), "=qm" (valid) : "rm" (sel));
>> -            BUG_ON(valid && ((a & 0x00f0ff00) != *ar));
>> +            WARN_ON(valid && ((a & 0x00f0ff00) != *ar));
>>              asm volatile (
>>                  "lsll %2,%0 ; setz %1"
>>                  : "=r" (l), "=qm" (valid) : "rm" (sel));
>> -            BUG_ON(valid && (l != *limit));
>> +            WARN_ON(valid && (l != *limit));
>>          }
>>  #endif
>>      }
>> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com 
>> http://lists.xensource.com/xen-devel 

Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()

[Keir Fraser]

On 15/12/2011 13:06, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 15.12.11 at 13:36, Keir Fraser <keir@xen.org> wrote:
>> On 15/12/2011 10:25, "Jan Beulich" <JBeulich@suse.com> wrote:
>> 
>>> In the light of AMD erratum #700, and given that these checks happen
>>> for debugging purposes only and also only in debug builds of the
>>> hypervisor, make the failures non-fatal.
>>> 
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> Is this erratum published?
> 
> Yes, it is in their most recent public revision guide.

Do you have a link? I can't find a published erratum > #686.

 -- Keir

> Jan
> 
>>> --- a/xen/arch/x86/traps.c
>>> +++ b/xen/arch/x86/traps.c
>>> @@ -1544,11 +1544,11 @@ static int read_descriptor(unsigned int
>>>              asm volatile (
>>>                  "larl %2,%0 ; setz %1"
>>>                  : "=r" (a), "=qm" (valid) : "rm" (sel));
>>> -            BUG_ON(valid && ((a & 0x00f0ff00) != *ar));
>>> +            WARN_ON(valid && ((a & 0x00f0ff00) != *ar));
>>>              asm volatile (
>>>                  "lsll %2,%0 ; setz %1"
>>>                  : "=r" (l), "=qm" (valid) : "rm" (sel));
>>> -            BUG_ON(valid && (l != *limit));
>>> +            WARN_ON(valid && (l != *limit));
>>>          }
>>>  #endif
>>>      }
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xensource.com
>>> http://lists.xensource.com/xen-devel
> 
> 
> 

Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()

[Keir Fraser]

On 15/12/2011 10:25, "Jan Beulich" <JBeulich@suse.com> wrote:

> In the light of AMD erratum #700, and given that these checks happen
> for debugging purposes only and also only in debug builds of the
> hypervisor, make the failures non-fatal.

I think the changeset comment should have a brief description of erratum
#700. I also some reference should be made in a comment above the first
WARN_ON, explaining why they are now WARN_Ons (again, with reference to #700
and its symptoms).

Apart from that:
Acked-by: Keir Fraser <keir@xen.org>

> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1544,11 +1544,11 @@ static int read_descriptor(unsigned int
>              asm volatile (
>                  "larl %2,%0 ; setz %1"
>                  : "=r" (a), "=qm" (valid) : "rm" (sel));
> -            BUG_ON(valid && ((a & 0x00f0ff00) != *ar));
> +            WARN_ON(valid && ((a & 0x00f0ff00) != *ar));
>              asm volatile (
>                  "lsll %2,%0 ; setz %1"
>                  : "=r" (l), "=qm" (valid) : "rm" (sel));
> -            BUG_ON(valid && (l != *limit));
> +            WARN_ON(valid && (l != *limit));
>          }
>  #endif
>      }
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()

[Jan Beulich]

>>> On 16.12.11 at 10:09, Keir Fraser <keir.xen@gmail.com> wrote:
> On 15/12/2011 10:25, "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>> In the light of AMD erratum #700, and given that these checks happen
>> for debugging purposes only and also only in debug builds of the
>> hypervisor, make the failures non-fatal.
> 
> I think the changeset comment should have a brief description of erratum
> #700.

I'd do this, but ...

> I also some reference should be made in a comment above the first
> WARN_ON, explaining why they are now WARN_Ons (again, with reference to #700
> and its symptoms).

... I now think that these should never have been BUG_ON()s in the
first place (despite probably having been the one who introduced
them).

Additionally, on a second look, check_descriptor() would not allow any
of the affected selector types to be installed into a descriptor table,
and we clearly don't put in any such descriptors ourselves, so from the
perspective of the erratum we're okay without the patch.

So if you prefer them to stay BUG_ON(), I think I'll just withdraw the
patch.

Jan

> Apart from that:
> Acked-by: Keir Fraser <keir@xen.org>
> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -1544,11 +1544,11 @@ static int read_descriptor(unsigned int
>>              asm volatile (
>>                  "larl %2,%0 ; setz %1"
>>                  : "=r" (a), "=qm" (valid) : "rm" (sel));
>> -            BUG_ON(valid && ((a & 0x00f0ff00) != *ar));
>> +            WARN_ON(valid && ((a & 0x00f0ff00) != *ar));
>>              asm volatile (
>>                  "lsll %2,%0 ; setz %1"
>>                  : "=r" (l), "=qm" (valid) : "rm" (sel));
>> -            BUG_ON(valid && (l != *limit));
>> +            WARN_ON(valid && (l != *limit));
>>          }
>>  #endif
>>      }
>> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com 
>> http://lists.xensource.com/xen-devel 

Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()

[Keir Fraser]

On 16/12/2011 14:44, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 16.12.11 at 10:09, Keir Fraser <keir.xen@gmail.com> wrote:
>> On 15/12/2011 10:25, "Jan Beulich" <JBeulich@suse.com> wrote:
>> 
>>> In the light of AMD erratum #700, and given that these checks happen
>>> for debugging purposes only and also only in debug builds of the
>>> hypervisor, make the failures non-fatal.
>> 
>> I think the changeset comment should have a brief description of erratum
>> #700.
> 
> I'd do this, but ...
> 
>> I also some reference should be made in a comment above the first
>> WARN_ON, explaining why they are now WARN_Ons (again, with reference to #700
>> and its symptoms).
> 
> ... I now think that these should never have been BUG_ON()s in the
> first place (despite probably having been the one who introduced
> them).
> 
> Additionally, on a second look, check_descriptor() would not allow any
> of the affected selector types to be installed into a descriptor table,
> and we clearly don't put in any such descriptors ourselves, so from the
> perspective of the erratum we're okay without the patch.
> 
> So if you prefer them to stay BUG_ON(), I think I'll just withdraw the
> patch.

If the erratum cannot affect us then they may as well stay as BUG_ON and we
sidestep the whole thing.

 -- Keir

> Jan
> 
>> Apart from that:
>> Acked-by: Keir Fraser <keir@xen.org>
>> 
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> 
>>> --- a/xen/arch/x86/traps.c
>>> +++ b/xen/arch/x86/traps.c
>>> @@ -1544,11 +1544,11 @@ static int read_descriptor(unsigned int
>>>              asm volatile (
>>>                  "larl %2,%0 ; setz %1"
>>>                  : "=r" (a), "=qm" (valid) : "rm" (sel));
>>> -            BUG_ON(valid && ((a & 0x00f0ff00) != *ar));
>>> +            WARN_ON(valid && ((a & 0x00f0ff00) != *ar));
>>>              asm volatile (
>>>                  "lsll %2,%0 ; setz %1"
>>>                  : "=r" (l), "=qm" (valid) : "rm" (sel));
>>> -            BUG_ON(valid && (l != *limit));
>>> +            WARN_ON(valid && (l != *limit));
>>>          }
>>>  #endif
>>>      }
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xensource.com
>>> http://lists.xensource.com/xen-devel
> 
> 
> 

Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()

[Wei Huang]

Hi Keir,

The public errata doc has been updated. You can find Erratum 700 info 
using the following steps:

1. Go to http://support.amd.com/us/psearch/Pages/psearch.aspx?type=2.7
2. Search with Product Type = "Processor", Product = "Any", Keyword = 
"Revision Guide".
3. Click on Technical Documents tab and 33610 is the one you are looking 
for.

Hope this helps...

-Wei

On 12/16/2011 08:48 AM, Keir Fraser wrote:
> On 16/12/2011 14:44, "Jan Beulich"<JBeulich@suse.com>  wrote:
>
>>>>> On 16.12.11 at 10:09, Keir Fraser<keir.xen@gmail.com>  wrote:
>>> On 15/12/2011 10:25, "Jan Beulich"<JBeulich@suse.com>  wrote:
>>>
>>>> In the light of AMD erratum #700, and given that these checks happen
>>>> for debugging purposes only and also only in debug builds of the
>>>> hypervisor, make the failures non-fatal.
>>> I think the changeset comment should have a brief description of erratum
>>> #700.
>> I'd do this, but ...
>>
>>> I also some reference should be made in a comment above the first
>>> WARN_ON, explaining why they are now WARN_Ons (again, with reference to #700
>>> and its symptoms).
>> ... I now think that these should never have been BUG_ON()s in the
>> first place (despite probably having been the one who introduced
>> them).
>>
>> Additionally, on a second look, check_descriptor() would not allow any
>> of the affected selector types to be installed into a descriptor table,
>> and we clearly don't put in any such descriptors ourselves, so from the
>> perspective of the erratum we're okay without the patch.
>>
>> So if you prefer them to stay BUG_ON(), I think I'll just withdraw the
>> patch.
> If the erratum cannot affect us then they may as well stay as BUG_ON and we
> sidestep the whole thing.
>
>   -- Keir
>
>> Jan
>>
>>> Apart from that:
>>> Acked-by: Keir Fraser<keir@xen.org>
>>>
>>>> Signed-off-by: Jan Beulich<jbeulich@suse.com>
>>>>
>>>> --- a/xen/arch/x86/traps.c
>>>> +++ b/xen/arch/x86/traps.c
>>>> @@ -1544,11 +1544,11 @@ static int read_descriptor(unsigned int
>>>>               asm volatile (
>>>>                   "larl %2,%0 ; setz %1"
>>>>                   : "=r" (a), "=qm" (valid) : "rm" (sel));
>>>> -            BUG_ON(valid&&  ((a&  0x00f0ff00) != *ar));
>>>> +            WARN_ON(valid&&  ((a&  0x00f0ff00) != *ar));
>>>>               asm volatile (
>>>>                   "lsll %2,%0 ; setz %1"
>>>>                   : "=r" (l), "=qm" (valid) : "rm" (sel));
>>>> -            BUG_ON(valid&&  (l != *limit));
>>>> +            WARN_ON(valid&&  (l != *limit));
>>>>           }
>>>>   #endif
>>>>       }
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xensource.com
>>>> http://lists.xensource.com/xen-devel
>>
>>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>