Jan Beulich
2011-Dec-15 10:25 UTC
[PATCH] 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 } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2011-Dec-15 12:36 UTC
Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()
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
Jan Beulich
2011-Dec-15 13:06 UTC
Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()
>>> 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
Keir Fraser
2011-Dec-15 13:10 UTC
Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()
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 > > >
Keir Fraser
2011-Dec-16 09:09 UTC
Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()
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
Jan Beulich
2011-Dec-16 14:44 UTC
Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()
>>> 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
Keir Fraser
2011-Dec-16 14:48 UTC
Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()
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 > > >
Wei Huang
2011-Dec-16 17:59 UTC
Re: [PATCH] x86: convert BUG_ON()s to WARN_ON()s in read_descriptor()
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 >