Re: [Xen-devel] [PATCH v17 06/11] x86/hvm/ioreq: add a new mappable resource type...

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 03 January 2018 15:48
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: JulienGrall <julien.grall@xxxxxxx>; Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; George
> Dunlap <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>;
> Stefano Stabellini <sstabellini@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Tim (Xen.org)
> <tim@xxxxxxx>
> Subject: Re: [PATCH v17 06/11] x86/hvm/ioreq: add a new mappable
> resource type...
> 
> >>> On 03.01.18 at 13:19, <paul.durrant@xxxxxxxxxx> wrote:
> > +static void hvm_free_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
> > +{
> > +    struct domain *d = s->domain;
> > +    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
> > +
> > +    if ( !iorp->page )
> > +        return;
> > +
> > +    page_list_add_tail(iorp->page, &d-
> >arch.hvm_domain.ioreq_server.pages);
> 
> Afaict s->domain is the guest, not the domain containing the
> emulator. Hence this new model of freeing the pages is safe only
> when the emulator domain is dead by the time the guest is being
> cleaned up.

From the investigations done w.r.t. the grant table pages I don't think this is 
the case. The emulating domain will have references on the pages and this keeps 
the target domain in existence, only completing domain destruction when the 
references are finally dropped. I've tested this by leaving an emulator running 
whilst I 'xl destroy' the domain; the domain remains as a zombie until emulator 
terminates.

> I'm not only unaware of this being enforced anywhere,
> but also unconvinced this is a desirable restriction: Why shouldn't
> emulator domains be allowed to be long lived, servicing more than
> a single guest if so desired by the admin?
> 
> What is additionally confusing me is the page ownership: Wasn't
> the (original) intention to make the pages owned by the emulator
> domain rather than the guest? I seem to recall you referring to
> restrictions in do_mmu_update(), but a domain should always be
> able to map pages it owns, shouldn't it?
> 

I'm sure we had this discussion before. I am trying to make resource mapping as 
uniform as possible so, like the grant table pages, the ioreq server pages are 
assigned to the target domain. Otherwise the domain trying to map resources has 
know which actual domain they are assigned to, rather than the domain they 
relate to... which is pretty ugly.

> Furthermore you continue to use Xen heap pages rather than
> domain heap ones.
> 

Yes, this seems reasonable since Xen will always need mappings of the pages and 
the aforementioned reference counting only works for Xen heap pages AIUI.

> And finally I'm still missing the is_hvm_domain() check in
> hvm_get_ioreq_server_frame(). Nor is there a NULL check for
> the return value of get_ioreq_server(), as I notice only now.
> 

Dammit. I forgot about the is_hvm_domain() check over Christmas. I'll add those 
two.

  Paul

> Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel