Re: [Xen-devel] PCI passthrough for HVM with stubdomain broken by "tools/libxl: handle the iomem parameter with the memory_mapping hcall"

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 06/23/2016 09:25 AM, Marek Marczykowski-Górecki wrote:
[...]
> Ok, after drawing a flowchart of the control in this function after your
> change, on a piece of paper, this case looks fine. But depending on how
> the domain was found (explicit loop or rcu_lock_domain_by_id), different
> locks are held, which makes it harder to follow what is going on.
>
> Crazy idea: how about making the code easy/easier to read instead of
> obfuscating it even more? XEN_DOMCTL_getdomaininfo semantic is
> convolved enough. How about this version (2 patches):
[...]
> xen: allow XEN_DOMCTL_getdomaininfo for device model
>
> Allow device model domain to get info about its target domain.
> It is used during PCI passthrough setup (xc_domain_memory_mapping
> checks for guest being auto-translated). While it happens in stubdomain,
> it failed, breaking PCI passthrough in such setup.
>
> While it is possible to workaround this at toolstack side, it seems
> logical to allow device model to get information about its target
> domain.
>
> The problem was exposed by c428c9f "tools/libxl: handle the iomem
> parameter with the memory_mapping hcall".
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> ---
>  xen/include/xsm/dummy.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index 406cd18..70a1633 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -128,7 +128,10 @@ static XSM_INLINE int xsm_domctl(XSM_DEFAULT_ARG struct 
> domain *d, int cmd)
>      case XEN_DOMCTL_unbind_pt_irq:
>          return xsm_default_action(XSM_DM_PRIV, current->domain, d);
>      case XEN_DOMCTL_getdomaininfo:
> -        return xsm_default_action(XSM_XS_PRIV, current->domain, d);
> +        if (current->domain->target)
> +            return xsm_default_action(XSM_DM_PRIV, current->domain, d);
> +        else
> +            return xsm_default_action(XSM_XS_PRIV, current->domain, d);
>      default:
>          return xsm_default_action(XSM_PRIV, current->domain, d);
>      }
I would prefer testing for the xenstore flag instead of testing for the
target field.  It ends up being the same thing in reality, since nobody
sane would make the xenstore also a device model (and not also dom0).

      case XEN_DOMCTL_getdomaininfo:
          if ( src->is_xenstore )
              return 0;
          return xsm_default_action(XSM_DM_PRIV, current->domain, d);

This makes it clear that xenstore is the special case, and removes the
need for the one-off XSM_XS_PRIV constant.  

--
Daniel De Graaf
National Security Agency

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