From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 4/9] xen: arm: correctly handle vtimer traps from userspace Date: Wed, 25 Feb 2015 14:37:18 +0000 Message-ID: <54EDDE1E.70204@linaro.org> References: <1423542956.5851.9.camel@citrix.com> <1423543523-8010-4-git-send-email-ian.campbell@citrix.com> <54D9A806.7030201@linaro.org> <1424347848.30924.53.camel@citrix.com> <54E5F658.4020904@linaro.org> <1424358801.30924.85.camel@citrix.com> <1424874753.20243.128.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1424874753.20243.128.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xen.org, tim@xen.org, Jan Beulich , stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 25/02/15 14:32, Ian Campbell wrote: > On Thu, 2015-02-19 at 15:13 +0000, Ian Campbell wrote: >> On Thu, 2015-02-19 at 14:42 +0000, Julien Grall wrote: >>>>> Although, I think the debug message in bad_trap is useful to keep. It >>>>> may be handy to have the HSR and the guest stack trace printed if Xen >>>>> hit the condition. >>>> >>>> Doesn't BUG_ON include all that? It should really. >>> >>> Not really BUG_ON will jump into the exception mode and therefore print >>> the HSR of the exception (breakpoint for ARM64 and undef for ARM32). >> >> Hrm, good point. >> >> Rather than reintroducing the goto idiom what about some form of >> noreturn panic helper for checking for sane h/w state (since these >> failures are really of the "buggy hardware" variety) e.g. >> ASSERT_GUEST_STATE(is_32bit_domain(...)) which would dump the guest >> state and then panic? > > Here is the sort of thing I was thinking about (only converted one > BUG_ON so far as an example, there are more candidates). > > Jan, would this be useful for x86 do you think, i.e. would you like me > to put it in lib.h with regular ASSERT? (Although making it more widely > available concerns me due to the pretty huge caveat in its use). > > Should it be on for debug=n too? (In which case it might want to become > GUEST_BUG_ON or similar). The argument for doing so is that it would > reduce the impact of potential security issues arising from h/w bugs (or > spec misunderstandings), in which case I would add to the comment: People may run a binary provided by a distribution on buggy hardware. So the GUEST_BUG_ON seems better. Although, I'm wondering what is the overhead of having a check at each traps? I guess none. Regards, -- Julien Grall