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

[Wei Huang <wei.huang2@amd.com>]

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
>