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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.12.17 at 11:41, <paul.durrant@xxxxxxxxxx> 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?

> +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.

> +int arch_acquire_resource(struct domain *d, unsigned int type,
> +                          unsigned int id, unsigned long frame,
> +                          unsigned int nr_frames, xen_pfn_t mfn_list[])
> +{
> +    int rc;
> +
> +    switch ( type )
> +    {
> +    case XENMEM_resource_ioreq_server:
> +    {
> +        ioservid_t ioservid = id;
> +        unsigned int i;
> +
> +        rc = -EINVAL;
> +        if ( id != (unsigned int)ioservid )
> +            break;
> +
> +        rc = 0;
> +        for ( i = 0; i < nr_frames; i++ )
> +        {
> +            mfn_t mfn;
> +
> +            rc = hvm_get_ioreq_server_frame(d, id, frame + i, &mfn);

Neither up from here nor in the called function it is being checked
that d is actually a HVM domain.

Jan


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