Re: [Xen-devel] [PATCH v4 12/12] x86/hvm/ioreq: add a new mappable resource type...

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Sep 05, 2017 at 12:37:16PM +0100, Paul Durrant wrote:
> ... XENMEM_resource_ioreq_server
> 
> This patch adds support for a new resource type that can be mapped using
> the XENMEM_acquire_resource memory op.
> 
> If an emulator makes use of this resource type then, instead of mapping
> gfns, the IOREQ server will allocate pages from the heap. These pages
> will never be present in the P2M of the guest at any point and so are
> not vulnerable to any direct attack by the guest. They are only ever
> accessible by Xen and any domain that has mapping privilege over the
> guest (which may or may not be limited to the domain running the emulator).
> 
> NOTE: Use of the new resource type is not compatible with use of
>       XEN_DMOP_get_ioreq_server_info unless the XEN_DMOP_no_gfns flag is
>       set.
> 
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Acked-by: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> ---
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/ioreq.c        | 123 
> +++++++++++++++++++++++++++++++++++++++-
>  xen/arch/x86/mm.c               |  27 +++++++++
>  xen/include/asm-x86/hvm/ioreq.h |   2 +
>  xen/include/public/hvm/dm_op.h  |   4 ++
>  xen/include/public/memory.h     |   3 +
>  5 files changed, 158 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> index 158bfbba32..f4c06a7a2a 100644
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -250,6 +250,15 @@ static int hvm_map_ioreq_gfn(struct hvm_ioreq_server *s, 
> bool buf)
>      struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
>      int rc;
>  
> +    if ( iorp->page )
> +    {
> +        /* Make sure the page has not been allocated */
> +        if ( gfn_eq(iorp->gfn, INVALID_GFN) )
> +            return -EPERM;
> +
> +        return 0;
> +    }
> +
>      if ( d->is_dying )
>          return -EINVAL;
>  
> @@ -272,6 +281,57 @@ static int hvm_map_ioreq_gfn(struct hvm_ioreq_server *s, 
> bool buf)
>      return rc;
>  }
>  
> +static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
> +{
> +    struct domain *currd = current->domain;
> +    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
> +
> +    if ( iorp->page )
> +    {
> +        /* Make sure the page has not been mapped */

Maybe it's just me being slightly lost, but here you imply that
iorp->gfn being != INVALID_GFN means the page is mapped, while above
you mention allocated for the same check.

Is it because gfn has different usages depending on the context?

> +        if ( !gfn_eq(iorp->gfn, INVALID_GFN) )
> +            return -EPERM;
[...]
> +mfn_t hvm_get_ioreq_server_frame(struct domain *d, ioservid_t id,
> +                                 unsigned int idx)
> +{
> +    struct hvm_ioreq_server *s;
> +    mfn_t mfn = INVALID_MFN;
> +
> +    spin_lock_recursive(&d->arch.hvm_domain.ioreq_server.lock);
> +
> +    s = d->arch.hvm_domain.ioreq_server.server[id];

Can't you use GET_IOREQ_SERVER here?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel