Re: [PATCH] xen: Modify domain_crash() to take a print string

[Julien Grall <julien@xxxxxxx>]

Hi Andrew,

On 03/02/2022 14:11, Andrew Cooper wrote:
> On 03/02/2022 13:48, Julien Grall wrote:
> > Hi,
> >
> > On 03/02/2022 13:38, Andrew Cooper wrote:
> > > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> > > index 37f78cc4c4c9..38b390d20371 100644
> > > --- a/xen/include/xen/sched.h
> > > +++ b/xen/include/xen/sched.h
> > > @@ -736,10 +736,15 @@ void vcpu_end_shutdown_deferral(struct vcpu *v);
> > >     * from any processor.
> > >     */
> > >    void __domain_crash(struct domain *d);
> > > -#define domain_crash(d) do
> > > {                                              \
> > > -    printk("domain_crash called from %s:%d\n", __FILE__,
> > > __LINE__);       \
> > > -
> > > __domain_crash(d);                                                    \
> > > -} while (0)
> > > +#define domain_crash(d, ...)                            \
> > > +    do {                                                \
> > > +        if ( count_args(__VA_ARGS__) == 0 )             \
> > > +            printk("domain_crash called from %s:%d\n",  \
> > > +                   __FILE__, __LINE__);                 \
> > I find a bit odd that here you are using a normal printk
> That's unmodified from before.  Only reformatted.
>
> > but...
> >
> >
> > > +        else                                            \
> > > +            printk(XENLOG_G_ERR __VA_ARGS__);           \
> > here it is XENLOG_G_ERR. In fact, isn't it ratelimited? If so,
> > wouldn't it be better to only use XENLOG_ERR so they can always be
> > seen? (A domain shouldn't be able to abuse it).
> Perhaps.  I suppose it is more important information than pretty much
> anything else about the guest.
>
> I've changed locally, but won't repost just for this.
Ok. With that:

Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall