Re: [Xen-devel] [PATCH] Fix save/restore for HVM domains with viridian=1

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Tim Deegan [mailto:tim@xxxxxxx]
> Sent: 25 November 2011 15:01
> To: Paul Durrant
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH] Fix save/restore for HVM domains
> with viridian=1
> 
> At 14:50 +0000 on 25 Nov (1322232658), Paul Durrant wrote:
[snip]
> >
> > +    case XC_SAVE_ID_HVM_VIRIDIAN:
> > +        /* Skip padding 4 bytes then read the acpi ioport
> location.
> > + */
> 
> Ahem.
> ^^^^^^^^^^^^^^^^^^^^
> 

Yes, whoops.

[snip]
> > @@ -206,8 +206,11 @@ int wrmsr_viridian_regs(uint32_t idx, ui
> >      struct vcpu *v = current;
> >      struct domain *d = v->domain;
> >
> > -    if ( !is_viridian_domain(d) )
> > +    if ( !is_viridian_domain(d) ) {
> > +        gdprintk(XENLOG_WARNING, "%s: %d not a viridian
> domain\n", __func__,
> > +                 d->domain_id);
> 
> This should probably be rate-limited; a confused domain could cause
> a _lot_ of these.  Might we want to pause/kill the domain as well,
> or inject a fault?
> 

Yes, true.

> >          return 0;
> > +    }
> >
> >      switch ( idx )
> >      {
> > @@ -271,8 +274,11 @@ int rdmsr_viridian_regs(uint32_t idx, ui
> >      struct vcpu *v = current;
> >      struct domain *d = v->domain;
> >
> > -    if ( !is_viridian_domain(d) )
> > +    if ( !is_viridian_domain(d) ) {
> > +        gdprintk(XENLOG_WARNING, "%s: %d not a viridian
> domain\n", __func__,
> > +                 d->domain_id);
> 
> Likewise.
> 
> >          return 0;
> > +    }
> >
> >      switch ( idx )
> >      {
> > @@ -411,6 +417,8 @@ static int viridian_load_domain_ctxt(str
> >      if ( hvm_load_entry(VIRIDIAN_DOMAIN, h, &ctxt) != 0 )
> >          return -EINVAL;
> >
> > +    ASSERT(is_viridian_domain(d));
> > +
> 
> I don't think it's appropriate to crash Xen if the save file is
> bogus.
> 

There's a similar ASSERT in the hypercall function anyway; would you rather I 
turned that into a rate limited warning too?

  Paul

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