[PATCH] hw/xen_pt: Don't grant opregion permissions

[PATCH] hw/xen_pt: Don't grant opregion permissions

[Grzegorz Uriasz]

With the upstreaming of linux based stubdomains to xen, qemu can't
assume it runs inside dom0 - permission assignment must be moved to
libxl running in dom0. This xen patch:
https://lists.xenproject.org/archives/html/xen-devel/2020-06/msg00973.html
implements granting the required permissions to the stubdomain running
qemu. This patch removes granting opregion permissions in qemu - this
should be fine as when qemu is running inside dom0 the memory mapping will
be successfully created without first explicitly granting the permission.

Signed-off-by: Grzegorz Uriasz <gorbak25@gmail.com>
---
 hw/xen/xen_pt_graphics.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 7d46e9c209..303674365b 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -283,19 +283,6 @@ void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val)
     igd_guest_opregion = (unsigned long)(val & ~XEN_PCI_INTEL_OPREGION_MASK)
                             | (igd_host_opregion & XEN_PCI_INTEL_OPREGION_MASK);
 
-    ret = xc_domain_iomem_permission(xen_xc, xen_domid,
-            (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
-            XEN_PCI_INTEL_OPREGION_PAGES,
-            XEN_PCI_INTEL_OPREGION_ENABLE_ACCESSED);
-
-    if (ret) {
-        XEN_PT_ERR(&s->dev, "[%d]:Can't enable to access IGD host opregion:"
-                    " 0x%lx.\n", ret,
-                    (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT)),
-        igd_guest_opregion = 0;
-        return;
-    }
-
     ret = xc_domain_memory_mapping(xen_xc, xen_domid,
             (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
             (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
-- 
2.27.0

Re: [PATCH] hw/xen_pt: Don't grant opregion permissions

[Philippe Mathieu-Daudé]

Hi Grzegorz,

On 6/15/20 1:21 AM, Grzegorz Uriasz wrote:
> With the upstreaming of linux based stubdomains to xen, qemu can't
> assume it runs inside dom0 - permission assignment must be moved to
> libxl running in dom0. This xen patch:
> https://lists.xenproject.org/archives/html/xen-devel/2020-06/msg00973.html
> implements granting the required permissions to the stubdomain running
> qemu. This patch removes granting opregion permissions in qemu - this
> should be fine as when qemu is running inside dom0 the memory mapping will
> be successfully created without first explicitly granting the permission.
> 
> Signed-off-by: Grzegorz Uriasz <gorbak25@gmail.com>
> ---
>  hw/xen/xen_pt_graphics.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
> index 7d46e9c209..303674365b 100644
> --- a/hw/xen/xen_pt_graphics.c
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -283,19 +283,6 @@ void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val)
>      igd_guest_opregion = (unsigned long)(val & ~XEN_PCI_INTEL_OPREGION_MASK)
>                              | (igd_host_opregion & XEN_PCI_INTEL_OPREGION_MASK);
>  
> -    ret = xc_domain_iomem_permission(xen_xc, xen_domid,
> -            (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
> -            XEN_PCI_INTEL_OPREGION_PAGES,
> -            XEN_PCI_INTEL_OPREGION_ENABLE_ACCESSED);
> -
> -    if (ret) {
> -        XEN_PT_ERR(&s->dev, "[%d]:Can't enable to access IGD host opregion:"
> -                    " 0x%lx.\n", ret,
> -                    (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT)),
> -        igd_guest_opregion = 0;
> -        return;
> -    }

Shouldn't this be somehow versioned? I.e. if the libxl does not have
the change then keep the current code?

> -
>      ret = xc_domain_memory_mapping(xen_xc, xen_domid,
>              (unsigned long)(igd_guest_opregion >> XC_PAGE_SHIFT),
>              (unsigned long)(igd_host_opregion >> XC_PAGE_SHIFT),
> 

Re: [PATCH] hw/xen_pt: Don't grant opregion permissions

[Roger Pau Monné]

On Sun, Jun 14, 2020 at 11:21:09PM +0000, Grzegorz Uriasz wrote:
> With the upstreaming of linux based stubdomains to xen, qemu can't
> assume it runs inside dom0 - permission assignment must be moved to
> libxl running in dom0. This xen patch:
> https://lists.xenproject.org/archives/html/xen-devel/2020-06/msg00973.html
> implements granting the required permissions to the stubdomain running
> qemu. This patch removes granting opregion permissions in qemu - this
> should be fine as when qemu is running inside dom0 the memory mapping will
> be successfully created without first explicitly granting the permission.

In order to avoid breaking certain libxl - QEMU combinations, could
you make the check below non-fatal?

So that the current code can be kept for dom0 while not throwing an
error when used inside of a stub domain?

Thanks, Roger.