>>> On 20.12.17 at 18:02, <[email protected]> wrote:
>> From: Xen-devel [mailto:[email protected]] On Behalf
>> Of Jan Beulich
>> Sent: 20 December 2017 16:35
>> >>> On 15.12.17 at 11:41, <[email protected]> wrote:
>> > +static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
>> > +{
>> > + struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
>> > +
>> > + if ( iorp->page )
>> > + {
>> > + /*
>> > + * If a guest frame has already been mapped (which may happen
>> > + * on demand if hvm_get_ioreq_server_info() is called), then
>> > + * allocating a page is not permitted.
>> > + */
>> > + if ( !gfn_eq(iorp->gfn, INVALID_GFN) )
>> > + return -EPERM;
>> > +
>> > + return 0;
>> > + }
>> > +
>> > + iorp->va = alloc_xenheap_page();
>> > + if ( !iorp->va )
>> > + return -ENOMEM;
>> > +
>> > + clear_page(iorp->va);
>> > +
>> > + iorp->page = virt_to_page(iorp->va);
>> > + share_xen_page_with_guest(iorp->page, s->domain,
>> XENSHARE_writable);
>> > + return 0;
>> > +}
>>
>> Why the much more limited (on huge systems) Xen heap all of the
>> sudden?
>
> Largely I'm trying to follow the same procedure used for the grant tables.
> Also, Xen is always going to need a mapping for these pages so using xenheap
> is convenient. If you think that's too limited then I can go back to domheap
> (but for the target domain rather than the tools domain) and map the page
> into Xen explicitly.
With the accounting problem below in mind, I think it'll be better
to use ordinary domain pages here anyway.
>> > +static void hvm_free_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
>> > +{
>> > + struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
>> > +
>> > + if ( !iorp->page )
>> > + return;
>> > +
>> > + iorp->page = NULL;
>> > +
>> > + free_xenheap_page(iorp->va);
>> > + iorp->va = NULL;
>> > +}
>>
>> I've looked over the code paths coming here, and I can't convince
>> myself that any mapping that the server has established would be
>> gone by the time the page is being freed. I'm likely (hopefully)
>> overlooking some aspect here.
>
> Hmm. Maybe you're right. The lack of ref counting might be a problem. It was
> so much simpler to allocate from the tools domain's heap, but the
> restrictions in do_mmu_update() rule that out. I'm really not sure how to fix
> this.
I'm afraid I don't see that particular restriction: It is the tools
domain which wants to map the page. Owners of a page are
permitted to map such pages (hence the removal of ownership
in the XSA-248 fix). So I don't understand why the tools domain
wouldn't be able to map that page if ownership is set that way,
perhaps even without the new sub-op. In the end, the domain
being serviced has no need to know of the page at all, it's a
shared entity between hypervisor and ioreq server. But likely
I'm missing some part of the whole picture here.
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
