date:20211014

flight 165510 linux-linus real [real]
flight 165517 linux-linus real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/165510/
http://logs.test-lab.xenproject.org/osstest/logs/165517/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 20 guest-start/debianhvm.repeat 
fail pass in 165517-retest

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds 20 guest-localmigrate/x10   fail REGR. vs. 165497

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 165497
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 165497
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 165497
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 165497
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 165497
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 165497
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 165497
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 165497
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass

version targeted for testing:
 linux26d6574109838b8fa40a8261421693015bab0579
baseline version:
 linux348949d9a4440abdab3b1dc99a9bb660e8c7da7c

Last test of basis   165497  2021-10-13 15:09:28 Z1 days
Testing same since   165510  2021-10-14 14:42:03 Z0 days1 attempts


People who touched

[PATCH] xen/pvcalls-back: Remove redundant 'flush_workqueue()' calls

2021-10-14 Thread Christophe JAILLET

'destroy_workqueue()' already drains the queue before destroying it, so
there is no need to flush it explicitly.

Remove the redundant 'flush_workqueue()' calls.

This was generated with coccinelle:

@@
expression E;
@@
-   flush_workqueue(E);
destroy_workqueue(E);

Signed-off-by: Christophe JAILLET 
---
 drivers/xen/pvcalls-back.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
index b47fd8435061..d6f945fd4147 100644
--- a/drivers/xen/pvcalls-back.c
+++ b/drivers/xen/pvcalls-back.c
@@ -465,7 +465,6 @@ static int pvcalls_back_release_passive(struct 
xenbus_device *dev,
write_unlock_bh(>sock->sk->sk_callback_lock);
}
sock_release(mappass->sock);
-   flush_workqueue(mappass->wq);
destroy_workqueue(mappass->wq);
kfree(mappass);
 
-- 
2.30.2

[PATCH v2 4/6] xen/arm: if direct-map domain use native addresses for GICv3

From: Stefano Stabellini 

Today we use native addresses to map the GICv3 for Dom0 and fixed
addresses for DomUs.

This patch changes the behavior so that native addresses are used for
all direct-map domains(including Dom0).

Considering that dom0 may not always be directly mapped in the future,
this patch introduces a new helper "is_domain_use_host_layout()" that
wraps both two check "is_domain_direct_mapped(d) || is_hardware_domain(d)"
for more flexible useage.

Signed-off-by: Stefano Stabellini 
Signed-off-by: Penny Zheng 
---
 xen/arch/arm/domain_build.c  | 46 +++-
 xen/arch/arm/vgic-v3.c   | 20 +---
 xen/include/asm-arm/domain.h |  3 +++
 3 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 6cd03e4d0f..7e0ee07e06 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2255,16 +2255,20 @@ static int __init make_gicv2_domU_node(struct 
kernel_info *kinfo)
 return res;
 }
 
+#ifdef CONFIG_ARM_64
 static int __init make_gicv3_domU_node(struct kernel_info *kinfo)
 {
 void *fdt = kinfo->fdt;
 int res = 0;
-__be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2];
+__be32 *reg;
 __be32 *cells;
+struct domain *d = kinfo->d;
+char buf[38];
+unsigned int i, len = 0;
 
-res = fdt_begin_node(fdt, 
"interrupt-controller@"__stringify(GUEST_GICV3_GICD_BASE));
-if ( res )
-return res;
+snprintf(buf, sizeof(buf), "interrupt-controller@%"PRIx64,
+ vgic_dist_base(>arch.vgic));
+res = fdt_begin_node(fdt, buf);
 
 res = fdt_property_cell(fdt, "#address-cells", 0);
 if ( res )
@@ -2282,35 +2286,55 @@ static int __init make_gicv3_domU_node(struct 
kernel_info *kinfo)
 if ( res )
 return res;
 
+/* reg specifies all re-distributors and Distributor. */
+len = (GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) *
+  (d->arch.vgic.nr_regions + 1) * sizeof(__be32);
+reg = xmalloc_bytes(len);
+if ( reg == NULL )
+return -ENOMEM;
+
 cells = [0];
 dt_child_set_range(, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
-   GUEST_GICV3_GICD_BASE, GUEST_GICV3_GICD_SIZE);
-dt_child_set_range(, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
-   GUEST_GICV3_GICR0_BASE, GUEST_GICV3_GICR0_SIZE);
+   vgic_dist_base(>arch.vgic), GUEST_GICV3_GICD_SIZE);
 
-res = fdt_property(fdt, "reg", reg, sizeof(reg));
+for ( i = 0;
+  i < d->arch.vgic.nr_regions;
+  i++, cells += (GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) )
+{
+dt_child_set_range(,
+   GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
+   d->arch.vgic.rdist_regions[i].base,
+   d->arch.vgic.rdist_regions[i].size);
+}
+
+res = fdt_property(fdt, "reg", reg, len);
 if (res)
-return res;
+goto out;
 
 res = fdt_property_cell(fdt, "linux,phandle", kinfo->phandle_gic);
 if (res)
-return res;
+goto out;
 
 res = fdt_property_cell(fdt, "phandle", kinfo->phandle_gic);
 if (res)
-return res;
+goto out;
 
 res = fdt_end_node(fdt);
 
+ out:
+xfree(reg);
 return res;
 }
+#endif
 
 static int __init make_gic_domU_node(struct kernel_info *kinfo)
 {
 switch ( kinfo->d->arch.vgic.version )
 {
+#ifdef CONFIG_ARM_64
 case GIC_V3:
 return make_gicv3_domU_node(kinfo);
+#endif
 case GIC_V2:
 return make_gicv2_domU_node(kinfo);
 default:
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index cb5a70c42e..70168ca1ac 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1641,14 +1641,15 @@ static inline unsigned int 
vgic_v3_max_rdist_count(struct domain *d)
  * Normally there is only one GICv3 redistributor region.
  * The GICv3 DT binding provisions for multiple regions, since there are
  * platforms out there which need those (multi-socket systems).
- * For Dom0 we have to live with the MMIO layout the hardware provides,
- * so we have to copy the multiple regions - as the first region may not
- * provide enough space to hold all redistributors we need.
+ * For direct-map domain(including dom0), we have to live with the MMIO
+ * layout the hardware provides, so we have to copy the multiple regions
+ * - as the first region may not provide enough space to hold all
+ * redistributors we need.
  * However DomU get a constructed memory map, so we can go with
  * the architected single redistributor region.
  */
-return is_hardware_domain(d) ? vgic_v3_hw.nr_rdist_regions :
-   GUEST_GICV3_RDIST_REGIONS;
+return is_domain_use_host_layout(d) ? vgic_v3_hw.nr_rdist_regions :
+

[PATCH v2 6/6] xen/docs: add a document to explain how to do passthrough without IOMMU

From: Stefano Stabellini 

Make sure to start with a WARNING about security.

Signed-off-by: Stefano Stabellini 
Signed-off-by: Penny Zheng 
---
 docs/misc/arm/passthrough-noiommu.txt | 54 +++
 1 file changed, 54 insertions(+)
 create mode 100644 docs/misc/arm/passthrough-noiommu.txt

diff --git a/docs/misc/arm/passthrough-noiommu.txt 
b/docs/misc/arm/passthrough-noiommu.txt
new file mode 100644
index 00..61aeb8a5cd
--- /dev/null
+++ b/docs/misc/arm/passthrough-noiommu.txt
@@ -0,0 +1,54 @@
+Request Device Assignment without IOMMU support
+===
+
+*WARNING:
+Users should be aware that it is not always secure to assign a device without
+IOMMU protection.
+When the device is not protected by the IOMMU, the administrator should make
+sure that:
+ 1. The device is assigned to a trusted guest.
+ 2. Users have additional security mechanism on the platform.
+
+
+This document assumes that the IOMMU is absent from the system or it is
+disabled (status = "disabled" in device tree).
+
+
+Add xen,force-assign-without-iommu; to the device tree snippet:
+
+ethernet: ethernet@ff0e {
+   compatible = "cdns,zynqmp-gem";
+   xen,path = "/amba/ethernet@ff0e";
+   xen,reg = <0x0 0xff0e 0x1000 0x0 0xff0e>;
+   xen,force-assign-without-iommu;
+};
+
+Request 1:1 memory mapping for the domain on static allocation
+==
+
+Add a direct-map property under the appropriate /chosen/domU node which
+is also statically allocated with physical memory ranges through
+xen,static-mem property as its guest RAM.
+
+Below is an example on how to specify the 1:1 memory mapping for the domain
+on static allocation in the device-tree:
+
+/ {
+   chosen {
+   domU1 {
+   compatible = "xen,domain";
+   #address-cells = <0x2>;
+   #size-cells = <0x2>;
+   cpus = <2>;
+   memory = <0x0 0x8>;
+   #xen,static-mem-address-cells = <0x1>;
+   #xen,static-mem-size-cells = <0x1>;
+   xen,static-mem = <0x3000 0x2000>;
+   direct-map;
+   ...
+   };
+   };
+};
+
+Besides reserving a 512MB region starting at the host physical address
+0x3000 to DomU1, it also requests 1:1 memory mapping.
-- 
2.25.1

[PATCH v2 5/6] xen/arm: if direct-map domain use native UART address and IRQ number for vPL011

From: Stefano Stabellini 

We always use a fix address to map the vPL011 to domains. The address
could be a problem for direct-map domains.

So, for domains that are directly mapped, reuse the address of the
physical UART on the platform to avoid potential clashes.

Do the same for the virtual IRQ number: instead of always using
GUEST_VPL011_SPI, try to reuse the physical SPI number if possible.

Signed-off-by: Stefano Stabellini 
Signed-off-by: Penny Zheng 
---
 xen/arch/arm/domain_build.c  | 41 ++-
 xen/arch/arm/vpl011.c| 54 +++-
 xen/include/asm-arm/vpl011.h |  2 ++
 3 files changed, 83 insertions(+), 14 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 7e0ee07e06..f3e87709f6 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -30,6 +30,7 @@
 
 #include 
 #include 
+#include 
 
 static unsigned int __initdata opt_dom0_max_vcpus;
 integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
@@ -2350,8 +2351,11 @@ static int __init make_vpl011_uart_node(struct 
kernel_info *kinfo)
 gic_interrupt_t intr;
 __be32 reg[GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS];
 __be32 *cells;
+struct domain *d = kinfo->d;
+char buf[27];
 
-res = fdt_begin_node(fdt, "sbsa-uart@"__stringify(GUEST_PL011_BASE));
+snprintf(buf, sizeof(buf), "sbsa-uart@%"PRIx64, d->arch.vpl011.base_addr);
+res = fdt_begin_node(fdt, buf);
 if ( res )
 return res;
 
@@ -2361,14 +2365,14 @@ static int __init make_vpl011_uart_node(struct 
kernel_info *kinfo)
 
 cells = [0];
 dt_child_set_range(, GUEST_ROOT_ADDRESS_CELLS,
-   GUEST_ROOT_SIZE_CELLS, GUEST_PL011_BASE,
+   GUEST_ROOT_SIZE_CELLS, d->arch.vpl011.base_addr,
GUEST_PL011_SIZE);
 
 res = fdt_property(fdt, "reg", reg, sizeof(reg));
 if ( res )
 return res;
 
-set_interrupt(intr, GUEST_VPL011_SPI, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
+set_interrupt(intr, d->arch.vpl011.virq, 0xf, DT_IRQ_TYPE_LEVEL_HIGH);
 
 res = fdt_property(fdt, "interrupts", intr, sizeof (intr));
 if ( res )
@@ -3083,6 +3087,14 @@ static int __init construct_domU(struct domain *d,
 allocate_static_memory(d, , node);
 }
 
+/*
+ * Base address and irq number are needed when creating vpl011 device
+ * tree node in prepare_dtb_domU, so initialization on related variables
+ * shall be dealt firstly.
+ */
+if ( kinfo.vpl011 )
+rc = domain_vpl011_init(d, NULL);
+
 rc = prepare_dtb_domU(d, );
 if ( rc < 0 )
 return rc;
@@ -3091,9 +3103,6 @@ static int __init construct_domU(struct domain *d,
 if ( rc < 0 )
 return rc;
 
-if ( kinfo.vpl011 )
-rc = domain_vpl011_init(d, NULL);
-
 return rc;
 }
 
@@ -3132,15 +3141,33 @@ void __init create_domUs(void)
 
 if ( !dt_property_read_u32(node, "nr_spis", _cfg.arch.nr_spis) )
 {
+unsigned int vpl011_virq = GUEST_VPL011_SPI;
+
 d_cfg.arch.nr_spis = gic_number_lines() - 32;
 
+/*
+ * The VPL011 virq is GUEST_VPL011_SPI, unless direct-map in
+ * set, in which case we'll try to match the hardware.
+ *
+ * Typically, d->arch.vpl011.virq has the vpl011 irq number
+ * but at this point of the boot sequence it is not
+ * initialized yet.
+ */
+if ( d_cfg.flags & XEN_DOMCTL_CDF_directmap )
+{
+vpl011_virq = serial_irq(SERHND_DTUART);
+if ( vpl011_virq < 0 )
+panic("Error getting IRQ number for this serial port %d\n",
+  SERHND_DTUART);
+}
+
 /*
  * vpl011 uses one emulated SPI. If vpl011 is requested, make
  * sure that we allocate enough SPIs for it.
  */
 if ( dt_property_read_bool(node, "vpl011") )
 d_cfg.arch.nr_spis = MAX(d_cfg.arch.nr_spis,
- GUEST_VPL011_SPI - 32 + 1);
+ vpl011_virq - 32 + 1);
 }
 
 /*
diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 895f436cc4..2de59e584d 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -71,11 +72,11 @@ static void vpl011_update_interrupt_status(struct domain *d)
  * status bit has been set since the last time.
  */
 if ( uartmis & ~vpl011->shadow_uartmis )
-vgic_inject_irq(d, NULL, GUEST_VPL011_SPI, true);
+vgic_inject_irq(d, NULL, vpl011->virq, true);
 
 vpl011->shadow_uartmis = uartmis;
 #else
-vgic_inject_irq(d, NULL, GUEST_VPL011_SPI, uartmis);
+vgic_inject_irq(d, NULL, vpl011->virq, uartmis);
 #endif
 }
 
@@ -347,7 +348,8 @@

[PATCH v2 3/6] xen/arm: if direct-map domain use native addresses for GICv2

From: Stefano Stabellini 

Today we use native addresses to map the GICv2 for Dom0 and fixed
addresses for DomUs.

This patch changes the behavior so that native addresses are used for
all domains that are direct-map memory map.

NEW VGIC has different naming schemes, like referring distributor base
address as vgic_dist_base, other than the dbase. So this patch also introduces
vgic_dist_base/vgic_cpu_base accessor to access correct distributor base
address/cpu interface base address on varied scenarios,

Signed-off-by: Stefano Stabellini 
Signed-off-by: Penny Zheng 
---
 xen/arch/arm/domain_build.c| 10 +++---
 xen/arch/arm/vgic-v2.c | 26 +-
 xen/arch/arm/vgic/vgic-v2.c| 27 ++-
 xen/include/asm-arm/new_vgic.h | 10 ++
 xen/include/asm-arm/vgic.h | 12 +++-
 5 files changed, 71 insertions(+), 14 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d9118e5bc1..6cd03e4d0f 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2207,8 +2207,12 @@ static int __init make_gicv2_domU_node(struct 
kernel_info *kinfo)
 int res = 0;
 __be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2];
 __be32 *cells;
+struct domain *d = kinfo->d;
+char buf[38];
 
-res = fdt_begin_node(fdt, 
"interrupt-controller@"__stringify(GUEST_GICD_BASE));
+snprintf(buf, sizeof(buf), "interrupt-controller@%"PRIx64,
+ vgic_dist_base(>arch.vgic));
+res = fdt_begin_node(fdt, buf);
 if ( res )
 return res;
 
@@ -2230,9 +2234,9 @@ static int __init make_gicv2_domU_node(struct kernel_info 
*kinfo)
 
 cells = [0];
 dt_child_set_range(, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
-   GUEST_GICD_BASE, GUEST_GICD_SIZE);
+   vgic_dist_base(>arch.vgic), GUEST_GICD_SIZE);
 dt_child_set_range(, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
-   GUEST_GICC_BASE, GUEST_GICC_SIZE);
+   vgic_cpu_base(>arch.vgic), GUEST_GICC_SIZE);
 
 res = fdt_property(fdt, "reg", reg, sizeof(reg));
 if (res)
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index b2da886adc..a8cf8173d0 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -652,7 +652,7 @@ static int vgic_v2_vcpu_init(struct vcpu *v)
 static int vgic_v2_domain_init(struct domain *d)
 {
 int ret;
-paddr_t cbase, csize;
+paddr_t csize;
 paddr_t vbase;
 
 /*
@@ -669,10 +669,26 @@ static int vgic_v2_domain_init(struct domain *d)
  * Note that we assume the size of the CPU interface is always
  * aligned to PAGE_SIZE.
  */
-cbase = vgic_v2_hw.cbase;
+d->arch.vgic.cbase = vgic_v2_hw.cbase;
 csize = vgic_v2_hw.csize;
 vbase = vgic_v2_hw.vbase;
 }
+else if ( is_domain_direct_mapped(d) )
+{
+/*
+ * For non-dom0 direct_mapped guests we only map a 8kB CPU
+ * interface but we make sure it is at a location occupied by
+ * the physical GIC in the host device tree.
+ *
+ * We need to add an offset to the virtual CPU interface base
+ * address when the GIC is aliased to get a 8kB contiguous
+ * region.
+ */
+d->arch.vgic.dbase = vgic_v2_hw.dbase;
+d->arch.vgic.cbase = vgic_v2_hw.cbase + vgic_v2_hw.aliased_offset;
+csize = GUEST_GICC_SIZE;
+vbase = vgic_v2_hw.vbase + vgic_v2_hw.aliased_offset;
+}
 else
 {
 d->arch.vgic.dbase = GUEST_GICD_BASE;
@@ -683,7 +699,7 @@ static int vgic_v2_domain_init(struct domain *d)
  * region.
  */
 BUILD_BUG_ON(GUEST_GICC_SIZE != SZ_8K);
-cbase = GUEST_GICC_BASE;
+d->arch.vgic.cbase = GUEST_GICC_BASE;
 csize = GUEST_GICC_SIZE;
 vbase = vgic_v2_hw.vbase + vgic_v2_hw.aliased_offset;
 }
@@ -692,8 +708,8 @@ static int vgic_v2_domain_init(struct domain *d)
  * Map the gic virtual cpu interface in the gic cpu interface
  * region of the guest.
  */
-ret = map_mmio_regions(d, gaddr_to_gfn(cbase), csize / PAGE_SIZE,
-   maddr_to_mfn(vbase));
+ret = map_mmio_regions(d, gaddr_to_gfn(d->arch.vgic.cbase),
+   csize / PAGE_SIZE, maddr_to_mfn(vbase));
 if ( ret )
 return ret;
 
diff --git a/xen/arch/arm/vgic/vgic-v2.c b/xen/arch/arm/vgic/vgic-v2.c
index b5ba4ace87..ce1f6e4373 100644
--- a/xen/arch/arm/vgic/vgic-v2.c
+++ b/xen/arch/arm/vgic/vgic-v2.c
@@ -258,7 +258,7 @@ void vgic_v2_enable(struct vcpu *vcpu)
 int vgic_v2_map_resources(struct domain *d)
 {
 struct vgic_dist *dist = >arch.vgic;
-paddr_t cbase, csize;
+paddr_t csize;
 paddr_t vbase;
 int ret;
 
@@ -276,10 +276,27 @@ int vgic_v2_map_resources(struct domain *d)
  * Note that we assume the size of the CPU interface is always
  *

[PATCH v2 2/6] xen/arm: introduce direct-map for domUs

From: Stefano Stabellini 

Cases where domU needs direct-map memory map:
  * IOMMU not present in the system.
  * IOMMU disabled if it doesn't cover a specific device and all the guests
are trusted. Thinking a mixed scenario, where a few devices with IOMMU and
a few without, then guest DMA security still could not be totally guaranteed.
So users may want to disable the IOMMU, to at least gain some performance
improvement from IOMMU disabled.
  * IOMMU disabled as a workaround when it doesn't have enough bandwidth.
To be specific, in a few extreme situation, when multiple devices do DMA
concurrently, these requests may exceed IOMMU's transmission capacity.
  * IOMMU disabled when it adds too much latency on DMA. For example,
TLB may be missing in some IOMMU hardware, which may bring latency in DMA
progress, so users may want to disable it in some realtime scenario.
  * Guest OS relies on the host memory layout-capablei

*WARNING:
Users should be aware that it is not always secure to assign a DMA-capable
device without IOMMU protection.
The administrator should make sure that:
 1. The device is assigned to a trusted guest.
 2. Users have additional security mechanism on the platform.

This commit also avoids setting XEN_DOMCTL_CDF_iommu when the IOMMU is
absent/disabled.

For now, direct-map is only supported when domain on Static Allocation, that is,
"xen.static-mem" must be also defined in the domain configuration.

This commit also introduces a new helper allocate_static_memory_11 to allocate
static memory as guest RAM for direct-map domain.

Signed-off-by: Stefano Stabellini 
Signed-off-by: Penny Zheng 
---
 docs/misc/arm/device-tree/booting.txt |  10 ++
 xen/arch/arm/domain_build.c   | 215 --
 2 files changed, 179 insertions(+), 46 deletions(-)

diff --git a/docs/misc/arm/device-tree/booting.txt 
b/docs/misc/arm/device-tree/booting.txt
index 71895663a4..297f8fa0c8 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -182,6 +182,16 @@ with the following properties:
 Both #address-cells and #size-cells need to be specified because
 both sub-nodes (described shortly) have reg properties.
 
+- direct-map
+
+Optional for Domain on Static Allocation.
+An empty property to request the memory of the domain to be
+direct-map (guest physical address == physical address).
+WARNING:
+Users must be aware of this risk, when doing DMA-capable device assignment,
+direct-map guest must be trusted or have additional security mechanism,
+otherwise it could use the DMA engine to access any other memory area.
+
 Under the "xen,domain" compatible node, one or more sub-nodes are present
 for the DomU kernel and ramdisk.
 
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 37e2d62d47..d9118e5bc1 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -492,8 +492,14 @@ static bool __init append_static_memory_to_bank(struct 
domain *d,
 {
 int res;
 unsigned int nr_pages = PFN_DOWN(size);
-/* Infer next GFN. */
-gfn_t sgfn = gaddr_to_gfn(bank->start + bank->size);
+gfn_t sgfn;
+
+if ( !is_domain_direct_mapped(d) )
+/* Infer next GFN when GFN != MFN. */
+sgfn = gaddr_to_gfn(bank->start + bank->size);
+else
+sgfn = gaddr_to_gfn(mfn_to_maddr(smfn));
+
 
 res = guest_physmap_add_pages(d, sgfn, smfn, nr_pages);
 if ( res )
@@ -507,12 +513,69 @@ static bool __init append_static_memory_to_bank(struct 
domain *d,
 return true;
 }
 
-/* Allocate memory from static memory as RAM for one specific domain d. */
+static int __init acquire_static_memory_bank(struct domain *d,
+ const __be32 **cell,
+ u32 addr_cells, u32 size_cells,
+ paddr_t *pbase, paddr_t *psize)
+{
+int res = 0;
+
+device_tree_get_reg(cell, addr_cells, size_cells, pbase, psize);
+ASSERT(IS_ALIGNED(*pbase, PAGE_SIZE) && IS_ALIGNED(*psize, PAGE_SIZE));
+if ( PFN_DOWN(*psize) > UINT_MAX )
+{
+printk(XENLOG_ERR "%pd: static memory size too large: %#"PRIpaddr,
+   d, *psize);
+return -EINVAL;
+}
+
+res = acquire_domstatic_pages(d, maddr_to_mfn(*pbase), PFN_DOWN(*psize), 
0);
+if ( res )
+printk(XENLOG_ERR
+   "%pd: failed to acquire static memory: %d.\n", d, res);
+
+return res;
+}
+
+static int __init parse_static_mem_prop(const struct dt_device_node *node,
+u32 *addr_cells, u32 *size_cells,
+int *length, const __be32 **cell)
+{
+const struct dt_property *prop;
+
+prop = dt_find_property(node, "xen,static-mem", NULL);
+if ( !dt_property_read_u32(node, "#xen,static-mem-address-cells",
+   addr_cells) )
+{
+

[PATCH v2 1/6] xen: introduce XEN_DOMCTL_CDF_directmap

From: Stefano Stabellini 

This commit introduces a new arm-specific flag XEN_DOMCTL_CDF_directmap to
specify that this domain should have its memory directly mapped
(guest physical address == physical address) at domain creation.

Refine is_domain_direct_mapped to check whether the flag
XEN_DOMCTL_CDF_directmap is set.

Signed-off-by: Stefano Stabellini 
Signed-off-by: Penny Zheng 
---
CC: andrew.coop...@citrix.com
CC: jbeul...@suse.com
CC: George Dunlap 
CC: Ian Jackson 
CC: Wei Liu 
CC: "Roger Pau Monné" 
---
 xen/arch/arm/domain.c| 3 ++-
 xen/arch/arm/domain_build.c  | 4 +++-
 xen/common/domain.c  | 3 ++-
 xen/include/asm-arm/domain.h | 4 ++--
 xen/include/public/domctl.h  | 4 +++-
 5 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index eef0661beb..8cee1c6349 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -628,7 +628,8 @@ int arch_sanitise_domain_config(struct 
xen_domctl_createdomain *config)
 {
 unsigned int max_vcpus;
 unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
-unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu);
+unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu |
+   XEN_DOMCTL_CDF_directmap);
 
 if ( (config->flags & ~flags_optional) != flags_required )
 {
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 0167731ab0..37e2d62d47 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -3069,8 +3069,10 @@ static int __init construct_dom0(struct domain *d)
 void __init create_dom0(void)
 {
 struct domain *dom0;
+/* DOM0 has always its memory directly mapped. */
 struct xen_domctl_createdomain dom0_cfg = {
-.flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
+.flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
+ XEN_DOMCTL_CDF_directmap,
 .max_evtchn_port = -1,
 .max_grant_frames = gnttab_dom0_frames(),
 .max_maptrack_frames = -1,
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 8b53c49d1e..7a6131db74 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -486,7 +486,8 @@ static int sanitise_domain_config(struct 
xen_domctl_createdomain *config)
  ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
-   XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu) )
+   XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
+   XEN_DOMCTL_CDF_directmap) )
 {
 dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
 return -EINVAL;
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 14e575288f..fc42c6a310 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -29,8 +29,8 @@ enum domain_type {
 #define is_64bit_domain(d) (0)
 #endif
 
-/* The hardware domain has always its memory direct mapped. */
-#define is_domain_direct_mapped(d) is_hardware_domain(d)
+/* Check if domain is direct-map memory map. */
+#define is_domain_direct_mapped(d) (d->options & XEN_DOMCTL_CDF_directmap)
 
 struct vtimer {
 struct vcpu *v;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 238384b5ae..b505a0db51 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -72,9 +72,11 @@ struct xen_domctl_createdomain {
 #define XEN_DOMCTL_CDF_nested_virt(1U << _XEN_DOMCTL_CDF_nested_virt)
 /* Should we expose the vPMU to the guest? */
 #define XEN_DOMCTL_CDF_vpmu   (1U << 7)
+/* If this domain has its memory directly mapped? (ARM only) */
+#define XEN_DOMCTL_CDF_directmap  (1U << 8)
 
 /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
-#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpmu
+#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_directmap
 
 uint32_t flags;
 
-- 
2.25.1

[PATCH v2 0/6] direct-map memory map

Cases where domU needs direct-map memory map:
  * IOMMU not present in the system.
  * IOMMU disabled if it doesn't cover a specific device and all the guests
are trusted. Thinking a mixed scenario, where a few devices with IOMMU and
a few without, then guest DMA security still could not be totally guaranteed.
So users may want to disable the IOMMU, to at least gain some performance
improvement from IOMMU disabled.
  * IOMMU disabled as a workaround when it doesn't have enough bandwidth.
To be specific, in a few extreme situation, when multiple devices do DMA
concurrently, these requests may exceed IOMMU's transmission capacity.
  * IOMMU disabled when it adds too much latency on DMA. For example,
TLB may be missing in some IOMMU hardware, which may bring latency in DMA
progress, so users may want to disable it in some realtime scenario.

*WARNING:
Users should be aware that it is not always secure to assign a DMA-capable
device without IOMMU protection.
When the device is not protected by the IOMMU, the administrator should make
sure that:
 1. The device is assigned to a trusted guest.
 2. Users have additional security mechanism on the platform.

Requesting direct-map memory mapping for the domain, when IOMMU is absent from
the system or it is disabled (status = "disabled" in device tree), In which
case, "direct-map" property is added under the appropriate domain node.

Right now, direct-map is only supported when domain on Static Allocation,
that is, "xen,static-mem" is also necessary in the domain configuration.

Looking into related [design link](
https://lists.xenproject.org/archives/html/xen-devel/2021-05/msg00882.html)
for more details.

The whole design is about Static Allocation and direct-map, and this
Patch Serie only covers parts of it, which are direct-map memory map.
Other features will be delievered through different patch series.

See https://lists.xenproject.org/archives/html/xen-devel/2021-09/msg00855.html
for Domain on Static Allocation.

This patch serie is based on
https://lists.xenproject.org/archives/html/xen-devel/2021-10/msg00822.html

---
v2 changes:
- remove the introduce of internal flag
- Refine is_domain_direct_mapped to check whether the flag
XEN_DOMCTL_CDF_directmap is set
- reword "1:1 direct-map" to just "direct-map"
- split the common codes into two helpers: parse_static_mem_prop and
acquire_static_memory_bank to deduce complexity.
- introduce a new helper allocate_static_memory_11 for allocating static
memory for direct-map guests
- remove panic action since it is fine to assign a non-DMA capable device when
IOMMU and direct-map both off
- remove redistributor accessor
- introduce new helper "is_domain_use_host_layout()"
- explain why vpl011 initialization before creating its device tree node
- error out if the domain is direct-mapped and the IRQ is not found
- harden the code and add a check/comment when the hardware UART region
is smaller than CUEST_VPL011_SIZE.

Stefano Stabellini (6):
  xen: introduce XEN_DOMCTL_CDF_directmap
  xen/arm: introduce direct-map for domUs
  xen/arm: if direct-map domain use native addresses for GICv2
  xen/arm: if direct-map domain use native addresses for GICv3
  xen/arm: if direct-map domain use native UART address and IRQ number
for vPL011
  xen/docs: add a document to explain how to do passthrough without
IOMMU

 docs/misc/arm/device-tree/booting.txt |  10 +
 docs/misc/arm/passthrough-noiommu.txt |  54 +
 xen/arch/arm/domain.c |   3 +-
 xen/arch/arm/domain_build.c   | 316 --
 xen/arch/arm/vgic-v2.c|  26 ++-
 xen/arch/arm/vgic-v3.c|  20 +-
 xen/arch/arm/vgic/vgic-v2.c   |  27 ++-
 xen/arch/arm/vpl011.c |  54 -
 xen/common/domain.c   |   3 +-
 xen/include/asm-arm/domain.h  |   7 +-
 xen/include/asm-arm/new_vgic.h|  10 +
 xen/include/asm-arm/vgic.h|  12 +-
 xen/include/asm-arm/vpl011.h  |   2 +
 xen/include/public/domctl.h   |   4 +-
 14 files changed, 449 insertions(+), 99 deletions(-)
 create mode 100644 docs/misc/arm/passthrough-noiommu.txt

-- 
2.25.1

[xen-unstable test] 165509: tolerable FAIL - PUSHED

flight 165509 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/165509/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-rtds 20 guest-localmigrate/x10   fail  like 165503
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 165503
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 165503
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 165503
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 165503
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 165503
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 165503
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 165503
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 165503
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 165503
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 165503
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 165503
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 165503
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen

Re: [PATCH v6 3/3] arm/libxl: Emulated PCI device tree node in libxl [and 1 more messages]

On Thu, 14 Oct 2021, Ian Jackson wrote:
> (Replying to both the earlier subthread on v5 and to the new v6
> patch.)
> 
> Bertrand Marquis writes ("Re: [PATCH v5 10/11] arm/libxl: Emulated PCI device 
> tree node in libxl"):
> > Now you suggest to add a new field arm_vpci in libxl__domain_create_state.
> 
> Hi.  I was handwaving, hence "probably" :-).  I hadn't actually looked
> at the existing code to see precisely how it would fit.
> 
> > Once we have done that I will need to access this structure to know if I 
> > need
> > to add the DT part and somehow to give it a value depending something which
> > for now would the number of pcidevs as there will be no user parameter 
> > anymore.
> 
> Right.
> 
> I looked at libxl_arm.c again:
> 
> It seems that the main entrypoint to all this is libxl__prepare_dtb,
> and it is that function you want to do new stuff.  That gets
> libxl_domain_build_info (which is specified by the IDL and comes from
> outside libxl, subject to libxl's default-filling machinery) and
> libxl__domain_build_state (which is the general state struct).
> 
> The information you need is is libxl_domain_create_info.
> libxl__domain_config_setdefault already arranges to set
> c_info->passthrough based on the number of PCI PT devices
> (search for `need_pt` in libxl_create.c).
> 
> That is, as I understand it on ARM vpci should be enabled if
> passthrough is enabled and not otherwise.  That is precisely what
> the variable c_info->passthrough is.
> 
> There is a slight issue because of the distinction between create_info
> and build_info and domain_config (and, the corresponding _state)
> structs.  Currently the DT code ony gets b_info, not the whole
> libxl_domain_config.  These distinctions largely historical nowadays.
> Certainly there is no reason not to pass a pointer to the whole
> libxl_domain_config, rather than just libxl_domain_build_info, into
> libxl__arch_domain_init_hw_description.
> 
> So I think the right approach for this looks something like this:
> 
> 1. Change libxl__arch_domain_init_hw_description to take
>libxl_domain_config* rather than libxl_domain_build_info*.
>libxl_domain_config contains libxl_domain_build_info so
>this is easy.
> 
>If you put in a convenience alias variable for the
>libxl_domain_build_info* you can avoid extra typing in the function
>body.  (If you call the convenience alias `info` you won't need to
>change the body at all, but maybe `info` isn't the best name so you
>could rename it to `b_info` maybe; up to you.)
> 
> 2. Make the same change to libxl__prepare_dtb.
> 
> 3. Now you can use d_config->c_info.passthrough to gate the addition
>of the additional stuff to the DT.  Ie, that is a respin of this
>patch 3/3.
> 
> 1 and 2 need to be separated out from 3, in a different patch (or two
> different patches) added to this series before what is now 3/3.  Ie
> 1+2, or 1 and 2 separately, should be pure plumbing changes with no
> functional change.
> 
> I hope this is helpful.

The explanation is clear and makes sense to me too.

FYI I suggested something similar and Rahul agreed on it; in fact, he
seemed to have already made the change and tested it:
https://marc.info/?l=xen-devel=163362066215155

I am just mentioning it in case Bertrand might be able to find Rahul's
updated patch somewhere and it might be useful as a base for his work.

The rest looks good, including the new changes.

Re: [PATCH v6 2/3] xen/arm: Enable the existing x86 virtual PCI support for ARM.

On Thu, 14 Oct 2021, Bertrand Marquis wrote:
> From: Rahul Singh 
> 
> The existing VPCI support available for X86 is adapted for Arm.
> When the device is added to XEN via the hyper call
> “PHYSDEVOP_pci_device_add”, VPCI handler for the config space
> access is added to the Xen to emulate the PCI devices config space.
> 
> A MMIO trap handler for the PCI ECAM space is registered in XEN
> so that when guest is trying to access the PCI config space,XEN
> will trap the access and emulate read/write using the VPCI and
> not the real PCI hardware.
> 
> For Dom0less systems scan_pci_devices() would be used to discover the
> PCI device in XEN and VPCI handler will be added during XEN boots.
> 
> This patch is also doing some small fixes to fix compilation errors on
> arm32 of vpci:
> - add a cast to unsigned long in print in header.c
> - add a cast to uint64_t in vpci_ecam_mmio_write

Thank you for these! Strictly speaking they are not required now but
they are welcome. I would also be OK if they were removed from this
patch but it is fine to have them in here.

There is an issues with this patch, see below at the bottom


> Signed-off-by: Rahul Singh 
> Signed-off-by: Bertrand Marquis 
>
> ---
> Changes in v6:
> - Use new vpci_ecam_ helpers for PCI access
> - Do not set XEN_DOMCTL_CDF_vpci for dom0 for now (will be done in a
> future patch once everything is ready)
> - rename REGISTER_OFFSET to ECAM_REG_OFFSET and move it to pci.h
> - remove not needed local variables in vpci_mmio_write, the one in read
> has been kept to ensure proper compilation on arm32
> - move call to vpci_add_handlers before iommu init to simplify exit path
> - move call to pci_cleanup_msi in the out section of pci_add_device if
> pdev is not NULL and on error
> - initialize pdev to NULL to handle properly exit path call of
> pci_cleanup_msi
> - keep has_vpci to return false for now as CFG_vpci has been removed.
> Added a comment on top of the definition.
> - fix compilation errors on arm32 (print in header.c, cast missing in
> mmio_write.
> - local variable was kept in vpci_mmio_read on arm to prevent a cast
> error in arm32.
> Change in v5:
> - Add pci_cleanup_msi(pdev) incleanup part.
> - Added Reviewed-by: Stefano Stabellini 
> Change in v4:
> - Move addition of XEN_DOMCTL_CDF_vpci flag to separate patch
> Change in v3:
> - Use is_pci_passthrough_enabled() in place of pci_passthrough_enabled 
> variable
> - Reject XEN_DOMCTL_CDF_vpci for x86 in arch_sanitise_domain_config()
> - Remove IS_ENABLED(CONFIG_HAS_VPCI) from has_vpci()
> Change in v2:
> - Add new XEN_DOMCTL_CDF_vpci flag
> - modify has_vpci() to include XEN_DOMCTL_CDF_vpci
> - enable vpci support when pci-passthough option is enabled.
> ---
> ---
>  xen/arch/arm/Makefile |  1 +
>  xen/arch/arm/domain.c |  4 ++
>  xen/arch/arm/vpci.c   | 74 +++
>  xen/arch/arm/vpci.h   | 36 +
>  xen/drivers/passthrough/pci.c | 18 -
>  xen/drivers/vpci/header.c |  3 +-
>  xen/drivers/vpci/vpci.c   |  2 +-
>  xen/include/asm-arm/domain.h  |  1 +
>  xen/include/asm-x86/pci.h |  2 -
>  xen/include/public/arch-arm.h |  7 
>  xen/include/xen/pci.h |  3 ++
>  11 files changed, 146 insertions(+), 5 deletions(-)
>  create mode 100644 xen/arch/arm/vpci.c
>  create mode 100644 xen/arch/arm/vpci.h
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 64518848b2..07f634508e 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -7,6 +7,7 @@ ifneq ($(CONFIG_NO_PLAT),y)
>  obj-y += platforms/
>  endif
>  obj-$(CONFIG_TEE) += tee/
> +obj-$(CONFIG_HAS_VPCI) += vpci.o
>  
>  obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
>  obj-y += bootfdt.init.o
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index eef0661beb..96e1b23550 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -39,6 +39,7 @@
>  #include 
>  #include 
>  
> +#include "vpci.h"
>  #include "vuart.h"
>  
>  DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
> @@ -773,6 +774,9 @@ int arch_domain_create(struct domain *d,
>  if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )
>  goto fail;
>  
> +if ( (rc = domain_vpci_init(d)) != 0 )
> +goto fail;
> +
>  return 0;
>  
>  fail:
> diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
> new file mode 100644
> index 00..7c3552b65d
> --- /dev/null
> +++ b/xen/arch/arm/vpci.c
> @@ -0,0 +1,74 @@
> +/*
> + * xen/arch/arm/vpci.c
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Re: [PATCH v6 1/3] xen/vpci: Move ecam access functions to common code

On Thu, 14 Oct 2021, Bertrand Marquis wrote:
> PCI standard is using ECAM and not MCFG which is coming from ACPI[1].
> Use ECAM/ecam instead of MCFG in common code and in new functions added
> in common vpci code by this patch.
> 
> Rename vpci_access_allowed to vpci_ecam_access_allowed and move it
> from arch/x86/hvm/io.c to drivers/vpci/vpci.c.
> 
> Create vpci_ecam_mmio_{read,write} in drivers/vpci/vpci.c that
> contains the common code to perform these operations, changed
> vpci_mmcfg_{read,write} accordingly to make use of these functions.
> 
> The vpci_ecam_mmio_{read,write} functions are returning 0 on error and 1
> on success. As the x86 code was previously always returning X86EMUL_OKAY
> the return code is ignored. A comment has been added in the code to show
> that this is intentional.
>
> Those functions will be used in a following patch inside by arm vpci
> implementation.
> 
> Rename MMCFG_SBDF to ECAM_SBDF.
> 
> Not functional change intended with this patch.
> 
> [1] https://wiki.osdev.org/PCI_Express
> 
> Suggested-by: Roger Pau Monné 
> Signed-off-by: Bertrand Marquis 

Everything checks out:

Reviewed-by: Stefano Stabellini 


FYI I like the new naming scheme and I think it is the right one for us
to use, but either way for me it wouldn't be a blocker for acceptance.


> ---
> Changes in v6: Patch added
> ---
>  xen/arch/x86/hvm/io.c | 50 +---
>  xen/drivers/vpci/vpci.c   | 60 +++
>  xen/include/asm-x86/pci.h |  2 +-
>  xen/include/xen/vpci.h| 10 +++
>  4 files changed, 78 insertions(+), 44 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index 046a8eb4ed..340b8c8b0e 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -260,20 +260,6 @@ unsigned int hvm_pci_decode_addr(unsigned int cf8, 
> unsigned int addr,
>  return CF8_ADDR_LO(cf8) | (addr & 3);
>  }
>  
> -/* Do some sanity checks. */
> -static bool vpci_access_allowed(unsigned int reg, unsigned int len)
> -{
> -/* Check access size. */
> -if ( len != 1 && len != 2 && len != 4 && len != 8 )
> -return false;
> -
> -/* Check that access is size aligned. */
> -if ( (reg & (len - 1)) )
> -return false;
> -
> -return true;
> -}
> -
>  /* vPCI config space IO ports handlers (0xcf8/0xcfc). */
>  static bool vpci_portio_accept(const struct hvm_io_handler *handler,
> const ioreq_t *p)
> @@ -305,7 +291,7 @@ static int vpci_portio_read(const struct hvm_io_handler 
> *handler,
>  
>  reg = hvm_pci_decode_addr(cf8, addr, );
>  
> -if ( !vpci_access_allowed(reg, size) )
> +if ( !vpci_ecam_access_allowed(reg, size) )
>  return X86EMUL_OKAY;
>  
>  *data = vpci_read(sbdf, reg, size);
> @@ -335,7 +321,7 @@ static int vpci_portio_write(const struct hvm_io_handler 
> *handler,
>  
>  reg = hvm_pci_decode_addr(cf8, addr, );
>  
> -if ( !vpci_access_allowed(reg, size) )
> +if ( !vpci_ecam_access_allowed(reg, size) )
>  return X86EMUL_OKAY;
>  
>  vpci_write(sbdf, reg, size, data);
> @@ -394,7 +380,7 @@ static unsigned int vpci_mmcfg_decode_addr(const struct 
> hvm_mmcfg *mmcfg,
> paddr_t addr, pci_sbdf_t *sbdf)
>  {
>  addr -= mmcfg->addr;
> -sbdf->bdf = MMCFG_BDF(addr);
> +sbdf->bdf = ECAM_BDF(addr);
>  sbdf->bus += mmcfg->start_bus;
>  sbdf->seg = mmcfg->segment;
>  
> @@ -434,25 +420,8 @@ static int vpci_mmcfg_read(struct vcpu *v, unsigned long 
> addr,
>  reg = vpci_mmcfg_decode_addr(mmcfg, addr, );
>  read_unlock(>arch.hvm.mmcfg_lock);
>  
> -if ( !vpci_access_allowed(reg, len) ||
> - (reg + len) > PCI_CFG_SPACE_EXP_SIZE )
> -return X86EMUL_OKAY;
> -
> -/*
> - * According to the PCIe 3.1A specification:
> - *  - Configuration Reads and Writes must usually be DWORD or smaller
> - *in size.
> - *  - Because Root Complex implementations are not required to support
> - *accesses to a RCRB that cross DW boundaries [...] software
> - *should take care not to cause the generation of such accesses
> - *when accessing a RCRB unless the Root Complex will support the
> - *access.
> - *  Xen however supports 8byte accesses by splitting them into two
> - *  4byte accesses.
> - */
> -*data = vpci_read(sbdf, reg, min(4u, len));
> -if ( len == 8 )
> -*data |= (uint64_t)vpci_read(sbdf, reg + 4, 4) << 32;
> +/* Ignore return code */
> +vpci_ecam_mmio_read(sbdf, reg, len, data);
>  
>  return X86EMUL_OKAY;
>  }
> @@ -476,13 +445,8 @@ static int vpci_mmcfg_write(struct vcpu *v, unsigned 
> long addr,
>  reg = vpci_mmcfg_decode_addr(mmcfg, addr, );
>  read_unlock(>arch.hvm.mmcfg_lock);
>  
> -if ( !vpci_access_allowed(reg, len) ||
> - (reg + len) > PCI_CFG_SPACE_EXP_SIZE )
> -return X86EMUL_OKAY;
> -
> -

[xen-unstable-smoke test] 165512: tolerable all pass - PUSHED

flight 165512 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/165512/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  b75838ad6c4f42c93efee83fc2508c78641e1b57
baseline version:
 xen  57f87857dc2de452a796d6bad4f476510efd2aba

Last test of basis   165511  2021-10-14 15:00:25 Z0 days
Testing same since   165512  2021-10-14 19:01:36 Z0 days1 attempts


People who touched revisions under test:
  Julien Grall 
  Oleksandr Tyshchenko 
  Volodymyr Babchuk 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   57f87857dc..b75838ad6c  b75838ad6c4f42c93efee83fc2508c78641e1b57 -> smoke

Re: Xen Booting Problem on ARM Machine

On Thu, 14 Oct 2021, Sai Kiran Kumar Reddy Y wrote:
> On Thu, Oct 14, 2021 at 5:45 AM Stefano Stabellini  
> wrote:
>   On Wed, 13 Oct 2021, Sai Kiran Kumar Reddy Y wrote:
>   > On Fri, Oct 1, 2021 at 8:17 AM Stefano Stabellini 
>  wrote:
>   >       Yes there are other ways but without serial is going to be 
> difficult
>   >       because you are not going to see anything until everything 
> works.
>   >
>   >       How do you boot Xen and Dom0 exactly from EDK2? Are you using 
> GRUB or
>   >       loading Xen directly from the EDK2 prompt? Please provide as 
> many
>   >       details as possible so that I might be able to spot any errors.
>   >
>   > I am using GRUB to load Xen. In the GRUB menu, I see two options. 
>   > Option 1: Debian 11 with latest Linux Kernel
>   > Option 2: Debian 11(with Xen hypervisor) with latest Kernel
>   >  
>   >       Can you provide the Device Tree you are using? If you are not 
> passing
>   >       any Device Tree  binary explicitely, then it might be passed
>   >       automatically from EDK2 to Linux/Xen. In that case, just boot 
> from Linux
>   >       then do the following to retrieve the Device Tree:
>   >
>   >       dtc -I fs -O dts /proc/device-tree > host.dts
>   >
>   >       Then please attach host.dts to this email thread.
>   >
>   > Yeah, you are right. It looks like LInux is booting from ACPI. In the 
> bootloader menu, "Automatic ACPI configuration" is
>   disabled. So, I
>   > thought that Linux may be booting from Device Tree. I have tried the 
> "dtc" command you mentioned. But it looks like there's
>   no device-tree
>   > under "/proc". I also tried to get DT info, from 
> "/sys/firmware/devicetree/base" . But, there's no info. under devicetree
>   folder. I am not
>   > quite sure how to get the DT info, if the Linux is booting from ACPI. 
> I am attaching .dsl files, that contain the acpi info.
> 
>   OK, so it is pretty clear that even if "Automatic ACPI configuration" is
>   disabled, it is still booting with ACPI.
> 
> 
>   >       Also for your information it looks like Linux actually booted 
> from ACPI,
>   >       not from Device Tree, as you can see from all the "ACPI" 
> messages in the
>   >       kernel logs.
>   >
>   >       If you need to boot from ACPI, then you need to enable ACPI 
> support in
>   >       Xen, which is disabled by default. You can do that using make
>   >       menuconfig.
>   >
>   > In the make menuconfig of Xen, I do not see any option to enable 
> ACPI.  
> 
>   You definitely need to enable ACPI support in Xen, if you are booting
>   from ACPI, otherwise nothing is going to work.
> 
>   On the latest staging (https://gitlab.com/xen-project/xen) you can
>   enable ACPI as follows:
> 
> 
>   # export CROSS_COMPILE=/path/to/cross-compiler
>   # export XEN_TARGET_ARCH=arm64
>   # cd xen.git/xen
>   # make menuconfig
>   #   --> Configure UNSUPPORTED features
>   #   --> Architecture Features --> ACPI
>   # make
> 
> 
> Hi
> 
> I got the code from Gitlab and installed it, enabling ACPI support in Xen. As 
> I reboot the system, I am able to see 2 options like before. 
> Option 1: Debian with latest kernel
> Option 2 : Debian with Xen
> 
> I have selected Option 2. I did not see any bootinfo membanks error. However, 
> there is the following error in GRUB(just for a fraction of a
> second). 
> 
> "Using modules provided by boot loader in FDT
>   Xen 4.16-unstable (c/s Wed Oct 13 13 13:28:43 2021 -0700 git:4cfab4425d) 
> EFI Loader
>   Couldn't obtain the File System Protocol Interface: ErrCode: 
> 0x8002"
> 
> I have enabled earlyprintk. I do not see any messages in the Serial. There 
> seems to be some problem with the gitlab version of Xen.

The error comes from xen/common/efi/boot.c:get_parent_handle

Xen is booted as EFI binary and it is trying to load other binaries
using the EFI File System Protocol Interface which is one of the EFI
Boot Services.

A wild guess is that somehow Grub is calling ExitBootServices, which
closes all Boot Services interfaces, before executing Xen. It should not
happen if Grub is executing Xen as EFI binary. I cannot explain how it
is possible. It looks like a bug in Grub.

Can you post the Grub config file that you are using?


Usually before Grub there is the proper EFI bootloader, tipically EDK2.
You should be able to boot Xen directly from the EFI bootloader too,
from its prompt, just by executing "xen". You need to place the xen
binary in the first FAT partition together with a xen.cfg config file as
follows:

---
options=console=com1 com1=115200 loglvl=all noreboot
kernel=vmlinuz-3.0.31-0.4-xen [domain 0 command line options]
ramdisk=initrd-3.0.31-0.4-xen
---

options is to specify the Xen command line.
kernel is to specify the Linux kernel to use and its

[qemu-mainline test] 165506: tolerable FAIL - PUSHED

flight 165506 qemu-mainline real [real]
flight 165513 qemu-mainline real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/165506/
http://logs.test-lab.xenproject.org/osstest/logs/165513/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-arndale  10 host-ping-check-xen fail pass in 165513-retest

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-arndale 15 migrate-support-check fail in 165513 never pass
 test-armhf-armhf-xl-arndale 16 saverestore-support-check fail in 165513 never 
pass
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 165498
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 165498
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 165498
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 165498
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 165498
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 165498
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 165498
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 165498
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass

version targeted for testing:
 qemuue5b2333f24ff207f08cf96e73d2e11438c985801
baseline version:
 qemuu946de558354c99e1989621abe053f2ab87dc8de9

Last test of basis   165498  2021-10-13

Re: [PATCH] xen/pvcalls-back: Remove redundant 'flush_workqueue()' calls

On Thu, 14 Oct 2021, Christophe JAILLET wrote:
> 'destroy_workqueue()' already drains the queue before destroying it, so
> there is no need to flush it explicitly.
> 
> Remove the redundant 'flush_workqueue()' calls.
> 
> This was generated with coccinelle:
> 
> @@
> expression E;
> @@
> - flush_workqueue(E);
>   destroy_workqueue(E);
> 
> Signed-off-by: Christophe JAILLET 

Acked-by: Stefano Stabellini 


> ---
>  drivers/xen/pvcalls-back.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
> index b47fd8435061..d6f945fd4147 100644
> --- a/drivers/xen/pvcalls-back.c
> +++ b/drivers/xen/pvcalls-back.c
> @@ -465,7 +465,6 @@ static int pvcalls_back_release_passive(struct 
> xenbus_device *dev,
>   write_unlock_bh(>sock->sk->sk_callback_lock);
>   }
>   sock_release(mappass->sock);
> - flush_workqueue(mappass->wq);
>   destroy_workqueue(mappass->wq);
>   kfree(mappass);
>  
> -- 
> 2.30.2
>

[xen-unstable-smoke test] 165511: tolerable all pass - PUSHED

flight 165511 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/165511/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  57f87857dc2de452a796d6bad4f476510efd2aba
baseline version:
 xen  2f5f0a1b77161993c16c4cc243467d75e5b7633b

Last test of basis   165507  2021-10-14 11:01:35 Z0 days
Testing same since   165511  2021-10-14 15:00:25 Z0 days1 attempts


People who touched revisions under test:
  Ian Jackson 
  Oleksandr Tyshchenko 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   2f5f0a1b77..57f87857dc  57f87857dc2de452a796d6bad4f476510efd2aba -> smoke

Re: [PATCH v6 3/3] arm/libxl: Emulated PCI device tree node in libxl [and 1 more messages]

(Replying to both the earlier subthread on v5 and to the new v6
patch.)

Bertrand Marquis writes ("Re: [PATCH v5 10/11] arm/libxl: Emulated PCI device 
tree node in libxl"):
> Now you suggest to add a new field arm_vpci in libxl__domain_create_state.

Hi.  I was handwaving, hence "probably" :-).  I hadn't actually looked
at the existing code to see precisely how it would fit.

> Once we have done that I will need to access this structure to know if I need
> to add the DT part and somehow to give it a value depending something which
> for now would the number of pcidevs as there will be no user parameter 
> anymore.

Right.

I looked at libxl_arm.c again:

It seems that the main entrypoint to all this is libxl__prepare_dtb,
and it is that function you want to do new stuff.  That gets
libxl_domain_build_info (which is specified by the IDL and comes from
outside libxl, subject to libxl's default-filling machinery) and
libxl__domain_build_state (which is the general state struct).

The information you need is is libxl_domain_create_info.
libxl__domain_config_setdefault already arranges to set
c_info->passthrough based on the number of PCI PT devices
(search for `need_pt` in libxl_create.c).

That is, as I understand it on ARM vpci should be enabled if
passthrough is enabled and not otherwise.  That is precisely what
the variable c_info->passthrough is.

There is a slight issue because of the distinction between create_info
and build_info and domain_config (and, the corresponding _state)
structs.  Currently the DT code ony gets b_info, not the whole
libxl_domain_config.  These distinctions largely historical nowadays.
Certainly there is no reason not to pass a pointer to the whole
libxl_domain_config, rather than just libxl_domain_build_info, into
libxl__arch_domain_init_hw_description.

So I think the right approach for this looks something like this:

1. Change libxl__arch_domain_init_hw_description to take
   libxl_domain_config* rather than libxl_domain_build_info*.
   libxl_domain_config contains libxl_domain_build_info so
   this is easy.

   If you put in a convenience alias variable for the
   libxl_domain_build_info* you can avoid extra typing in the function
   body.  (If you call the convenience alias `info` you won't need to
   change the body at all, but maybe `info` isn't the best name so you
   could rename it to `b_info` maybe; up to you.)

2. Make the same change to libxl__prepare_dtb.

3. Now you can use d_config->c_info.passthrough to gate the addition
   of the additional stuff to the DT.  Ie, that is a respin of this
   patch 3/3.

1 and 2 need to be separated out from 3, in a different patch (or two
different patches) added to this series before what is now 3/3.  Ie
1+2, or 1 and 2 separately, should be pure plumbing changes with no
functional change.

I hope this is helpful.

Thanks,
Ian.

Re: [PATCH V7 0/2] Add handling of extended regions (safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space")




Hello all


On 14.10.21 14:40, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

You can find an initial discussion at [1]-[7].

The extended region (safe range) is a region of guest physical address space
which is unused and could be safely used to create grant/foreign mappings 
instead
of wasting real RAM pages from the domain memory for establishing these 
mappings.

The extended regions are chosen at the domain creation time and advertised
to it via "reg" property under hypervisor node in the guest device-tree
(the indexes for extended regions are 1...N).
No device tree bindings update is needed, guest infers the presense of extended
regions from the number of regions in "reg" property.
New compatible/property will be needed (but only after this patch [8] or 
alternative
goes in) to indicate that "region 0 is safe to use". Until this patch is merged 
it is
not safe to use extended regions for the grant table space.

The extended regions are calculated differently for direct mapped Dom0 (with 
and without
IOMMU) and non-direct mapped DomUs.

Please note the following limitations:
- The extended region feature is only supported for 64-bit domain currently.
- The ACPI case is not covered.

Please note that support for Dom0 was already committed, so these patches are 
remaining DomU bits.

Xen patch series is also available at [9]. The corresponding Linux patch series 
is at [10]
for now (last 4 patches).


So, all Xen changes are already committed (thanks!) and I will start 
trying to push Linux changes. I believe that with your help in review, 
it will be possible to finish this enabling work.



[snip]

--
Regards,

Oleksandr Tyshchenko

Re: [PATCH v5 10/11] arm/libxl: Emulated PCI device tree node in libxl

Hi Ian,

> On 7 Oct 2021, at 17:11, Ian Jackson  wrote:
> 
> Rahul Singh writes ("Re: [PATCH v5 10/11] arm/libxl: Emulated PCI device tree 
> node in libxl"):
>> As Stefano suggested in another email that we can remove the vpci
>> option, if we reach to conclusion that we need vpci option I will
>> move it to internal structure.
> ...
>> Yes I agree with you VPCI is necessary for hot plugged PCI device
>> and once we implement the hotplug in future we will use the
>> passthrough= option to enable VPCI.
> 
> So, to summarise, I think the situation is:
> 
> * VCPI is necessry for passthrough on ARM, whether coldplug or
>   hotplug.  It's part of the way that PCI-PT works on ARM.
> 
> * Hotplug is not yet implemented.
> 
> * VPCI is not necessary on x86 (evidently, since we don't have it
>   there but we do have passthrough).
> 
> So when hotplug is added, vpci will need to be turned on when
> passthrough=yes is selected.  I don't fully understand the other
> possible values for passthrough= but maybe we can defer the question
> of whether they apply to ARM ?
> 
> I think that means that yes, this should be an internal variable.
> Probably in libxl__domain_create_state.  We don't currently arrange to
> elide arch-specific state in there, so perhaps it's fine just to
> invent a member called `arm_vpci`.
> 
> Maybe you could leave a comment somewhere so that if and when PCI PT
> hotplug is implemented for ARM, the implementor remembers to wire this
> up.

Sorry for missing this on the v6 serie.

Now you suggest to add a new field arm_vpci in libxl__domain_create_state.

Once we have done that I will need to access this structure to know if I need
to add the DT part and somehow to give it a value depending something which
for now would the number of pcidevs as there will be no user parameter anymore.

I had quite an understanding of the solution using libxl_domain_config and 
changing
The arguments of libxl__arch_domain_init_hw_description and libxl__prepare_dtb
Suggested by Stefano but I am a bit lost in this solution.

The following might be a stupid question but I did not dig a lot in libxl so:
If we add a parameter in the state structure how should we access it ?

Thank
Bertrand

> 
> Ian.

Re: [PATCH v6 1/3] xen/vpci: Move ecam access functions to common code

Hi Jan,

> On 14 Oct 2021, at 17:06, Jan Beulich  wrote:
> 
> On 14.10.2021 16:49, Bertrand Marquis wrote:
>> @@ -305,7 +291,7 @@ static int vpci_portio_read(const struct hvm_io_handler 
>> *handler,
>> 
>> reg = hvm_pci_decode_addr(cf8, addr, );
>> 
>> -if ( !vpci_access_allowed(reg, size) )
>> +if ( !vpci_ecam_access_allowed(reg, size) )
>> return X86EMUL_OKAY;
>> 
>> *data = vpci_read(sbdf, reg, size);
>> @@ -335,7 +321,7 @@ static int vpci_portio_write(const struct hvm_io_handler 
>> *handler,
>> 
>> reg = hvm_pci_decode_addr(cf8, addr, );
>> 
>> -if ( !vpci_access_allowed(reg, size) )
>> +if ( !vpci_ecam_access_allowed(reg, size) )
>> return X86EMUL_OKAY;
>> 
>> vpci_write(sbdf, reg, size, data);
> 
> Why would port I/O functions call an ECAM helper? And in how far is
> that helper actually ECAM-specific?

The function was global before.

> 
>> @@ -434,25 +420,8 @@ static int vpci_mmcfg_read(struct vcpu *v, unsigned 
>> long addr,
>> reg = vpci_mmcfg_decode_addr(mmcfg, addr, );
>> read_unlock(>arch.hvm.mmcfg_lock);
>> 
>> -if ( !vpci_access_allowed(reg, len) ||
>> - (reg + len) > PCI_CFG_SPACE_EXP_SIZE )
>> -return X86EMUL_OKAY;
> 
> While I assume this earlier behavior is the reason for ...

Yes :-)

> 
>> -/*
>> - * According to the PCIe 3.1A specification:
>> - *  - Configuration Reads and Writes must usually be DWORD or smaller
>> - *in size.
>> - *  - Because Root Complex implementations are not required to support
>> - *accesses to a RCRB that cross DW boundaries [...] software
>> - *should take care not to cause the generation of such accesses
>> - *when accessing a RCRB unless the Root Complex will support the
>> - *access.
>> - *  Xen however supports 8byte accesses by splitting them into two
>> - *  4byte accesses.
>> - */
>> -*data = vpci_read(sbdf, reg, min(4u, len));
>> -if ( len == 8 )
>> -*data |= (uint64_t)vpci_read(sbdf, reg + 4, 4) << 32;
>> +/* Ignore return code */
>> +vpci_ecam_mmio_read(sbdf, reg, len, data);
> 
> ... the commented-upon ignoring of the return value, I don't think
> that's a good way to deal with things anymore. Instead I think
> *data should be written to ~0 upon failure, unless it is intended
> for vpci_ecam_mmio_read() to take care of that case (in which case
> I'm not sure I would see why it needs to return an error indicator
> in the first place).

I am not sure in the first place why this is actually ignored and just
returning a -1 value.
If an access is not right, an exception should be generated to the
Guest instead.
When we do that on arm the function is returning an error to the upper
layer in that case, that’s why I did keep a generic function informing the
caller.

So I think it is right for the function to return an error if the access is not 
allowed but I agree the comment on x86 could get a better justification.
@Roger: could you help finding one here as I do not quite understand why it is 
ok to ignore this case ?

> 
>> @@ -476,13 +445,8 @@ static int vpci_mmcfg_write(struct vcpu *v, unsigned 
>> long addr,
>> reg = vpci_mmcfg_decode_addr(mmcfg, addr, );
>> read_unlock(>arch.hvm.mmcfg_lock);
>> 
>> -if ( !vpci_access_allowed(reg, len) ||
>> - (reg + len) > PCI_CFG_SPACE_EXP_SIZE )
>> -return X86EMUL_OKAY;
>> -
>> -vpci_write(sbdf, reg, min(4u, len), data);
>> -if ( len == 8 )
>> -vpci_write(sbdf, reg + 4, 4, data >> 32);
>> +/* Ignore return code */
>> +vpci_ecam_mmio_write(sbdf, reg, len, data);
> 
> Here ignoring is fine imo, but if you feel it is important to
> comment on this, then I think you need to prefer "why" over "what".

Agree I would just need some help on the why.
Now there was no comment before to explain why so I could also
remove the comment altogether.

> 
>> --- a/xen/drivers/vpci/vpci.c
>> +++ b/xen/drivers/vpci/vpci.c
>> @@ -478,6 +478,66 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg, 
>> unsigned int size,
>> spin_unlock(>vpci->lock);
>> }
>> 
>> +/* Helper function to check an access size and alignment on vpci space. */
>> +bool vpci_ecam_access_allowed(unsigned int reg, unsigned int len)
>> +{
>> +/*
>> + * Check access size.
>> + *
>> + * On arm32 or for 32bit guests on arm, 64bit accesses should be 
>> forbidden
>> + * but as for those platform ISV register, which gives the access size,
>> + * cannot have a value 3, checking this would just harden the code.
>> + */
>> +if ( len != 1 && len != 2 && len != 4 && len != 8 )
>> +return false;
> 
> I'm not convinced talking about Arm specifically here is
> warranted, unless there's something there that's clearly
> different from all other architectures. Otherwise the comment
> should imo be written in more general terms.

Other architectures might allow this case. So this is specific to Arm.

> 
>> +int

Re: [future abi] [RFC PATCH V3] xen/gnttab: Store frame GFN in struct page_info on Arm




Hello, all.

The potential issue on Arm (which might happen when remapping 
grant-table frame) is still present, it hasn't disappeared.
Some effort was put in trying to fix that by current patch. Although I 
have addressed (I hope) all review comments received for this patch, I 
realize this patch (in its current form) cannot go in without resolving 
locking issue I described in a post-commit message (we don't want to 
make things worse than the current state). I would appreciate any 
thoughts regarding that.



On 25.09.21 04:48, Julien Grall wrote:

Hi Roger,

On 24/09/2021 21:10, Roger Pau Monné wrote:

On Fri, Sep 24, 2021 at 07:52:24PM +0500, Julien Grall wrote:

Hi Roger,

On 24/09/2021 13:41, Roger Pau Monné wrote:

On Thu, Sep 23, 2021 at 09:59:26PM +0100, Andrew Cooper wrote:

On 23/09/2021 20:32, Oleksandr Tyshchenko wrote:

Suggested-by: Julien Grall 
Signed-off-by: Oleksandr Tyshchenko 
---
You can find the related discussions at:
https://lore.kernel.org/xen-devel/93d0df14-2c8a-c2e3-8c51-544121901...@xen.org/ 

https://lore.kernel.org/xen-devel/1628890077-12545-1-git-send-email-olekst...@gmail.com/ 

https://lore.kernel.org/xen-devel/1631652245-30746-1-git-send-email-olekst...@gmail.com/ 



! Please note, there is still unresolved locking question here 
for which

I failed to find a suitable solution. So, it is still an RFC !


Just FYI, I thought I'd share some of the plans for ABI v2.  
Obviously

these plans are future work and don't solve the current problem.

Guests mapping Xen pages is backwards.  There are reasons why it was
used for x86 PV guests, but the entire interface should have been 
design

differently for x86 HVM.

In particular, Xen should be mapping guest RAM, rather than the guest
manipulating the 2nd stage tables to map Xen RAM.  Amongst other 
things,

its far far lower overhead.


A much better design is one where the grant table looks like an MMIO
device.  The domain builder decides the ABI (v1 vs v2 - none of this
dynamic switch at runtime nonsense), and picks a block of guest 
physical
addresses, which are registered with Xen.  This forms the grant 
table,

status table (v2 only), and holes to map into.


I think this could be problematic for identity mapped Arm dom0, as
IIRC in that case grants are mapped so that gfn == mfn in order to
account for the lack of an IOMMU. You could use a bounce buffer, but
that would introduce a big performance penalty.


Or you could find a hole that is outside of the RAM regions. This is 
not

trivial but not impossible (see [1]).


I certainly not familiar with the Arm identity map.

If you map them at random areas (so no longer identity mapped), how do
you pass the addresses to the physical devices for DMA operations? I
assume there must be some kind of translation then that converts from
gfn to mfn in order to cope with the lack of an IOMMU, 


For grant mapping, the hypercall will return the machine address in 
dev_bus_addr. Dom0, will keep the conversion dom0 GFN <-> MFN for 
later use in the swiotlb.


For foreign mapping, AFAICT, we are expecting them to bounce 
everytime. But DMA into a foreign mapping should be rarer.



and because
dom0 doesn't know the mfn of the grant reference in order to map it at
the same gfn.


IIRC, we tried an approach where the grant mapping would be direct 
mapped in dom0. However, this was an issue on arm32 because Debian was 
(is?) using short descriptor page tables. This didn't allow dom0 to 
cover all the mappings and therefore some mappings would not be 
accessible.



--
Regards,

Oleksandr Tyshchenko

Re: [PATCH v6 1/3] xen/vpci: Move ecam access functions to common code

On 14.10.2021 16:49, Bertrand Marquis wrote:
> @@ -305,7 +291,7 @@ static int vpci_portio_read(const struct hvm_io_handler 
> *handler,
>  
>  reg = hvm_pci_decode_addr(cf8, addr, );
>  
> -if ( !vpci_access_allowed(reg, size) )
> +if ( !vpci_ecam_access_allowed(reg, size) )
>  return X86EMUL_OKAY;
>  
>  *data = vpci_read(sbdf, reg, size);
> @@ -335,7 +321,7 @@ static int vpci_portio_write(const struct hvm_io_handler 
> *handler,
>  
>  reg = hvm_pci_decode_addr(cf8, addr, );
>  
> -if ( !vpci_access_allowed(reg, size) )
> +if ( !vpci_ecam_access_allowed(reg, size) )
>  return X86EMUL_OKAY;
>  
>  vpci_write(sbdf, reg, size, data);

Why would port I/O functions call an ECAM helper? And in how far is
that helper actually ECAM-specific?

> @@ -434,25 +420,8 @@ static int vpci_mmcfg_read(struct vcpu *v, unsigned long 
> addr,
>  reg = vpci_mmcfg_decode_addr(mmcfg, addr, );
>  read_unlock(>arch.hvm.mmcfg_lock);
>  
> -if ( !vpci_access_allowed(reg, len) ||
> - (reg + len) > PCI_CFG_SPACE_EXP_SIZE )
> -return X86EMUL_OKAY;

While I assume this earlier behavior is the reason for ...

> -/*
> - * According to the PCIe 3.1A specification:
> - *  - Configuration Reads and Writes must usually be DWORD or smaller
> - *in size.
> - *  - Because Root Complex implementations are not required to support
> - *accesses to a RCRB that cross DW boundaries [...] software
> - *should take care not to cause the generation of such accesses
> - *when accessing a RCRB unless the Root Complex will support the
> - *access.
> - *  Xen however supports 8byte accesses by splitting them into two
> - *  4byte accesses.
> - */
> -*data = vpci_read(sbdf, reg, min(4u, len));
> -if ( len == 8 )
> -*data |= (uint64_t)vpci_read(sbdf, reg + 4, 4) << 32;
> +/* Ignore return code */
> +vpci_ecam_mmio_read(sbdf, reg, len, data);

... the commented-upon ignoring of the return value, I don't think
that's a good way to deal with things anymore. Instead I think
*data should be written to ~0 upon failure, unless it is intended
for vpci_ecam_mmio_read() to take care of that case (in which case
I'm not sure I would see why it needs to return an error indicator
in the first place).

> @@ -476,13 +445,8 @@ static int vpci_mmcfg_write(struct vcpu *v, unsigned 
> long addr,
>  reg = vpci_mmcfg_decode_addr(mmcfg, addr, );
>  read_unlock(>arch.hvm.mmcfg_lock);
>  
> -if ( !vpci_access_allowed(reg, len) ||
> - (reg + len) > PCI_CFG_SPACE_EXP_SIZE )
> -return X86EMUL_OKAY;
> -
> -vpci_write(sbdf, reg, min(4u, len), data);
> -if ( len == 8 )
> -vpci_write(sbdf, reg + 4, 4, data >> 32);
> +/* Ignore return code */
> +vpci_ecam_mmio_write(sbdf, reg, len, data);

Here ignoring is fine imo, but if you feel it is important to
comment on this, then I think you need to prefer "why" over "what".

> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -478,6 +478,66 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg, 
> unsigned int size,
>  spin_unlock(>vpci->lock);
>  }
>  
> +/* Helper function to check an access size and alignment on vpci space. */
> +bool vpci_ecam_access_allowed(unsigned int reg, unsigned int len)
> +{
> +/*
> + * Check access size.
> + *
> + * On arm32 or for 32bit guests on arm, 64bit accesses should be 
> forbidden
> + * but as for those platform ISV register, which gives the access size,
> + * cannot have a value 3, checking this would just harden the code.
> + */
> +if ( len != 1 && len != 2 && len != 4 && len != 8 )
> +return false;

I'm not convinced talking about Arm specifically here is
warranted, unless there's something there that's clearly
different from all other architectures. Otherwise the comment
should imo be written in more general terms.

> +int vpci_ecam_mmio_write(pci_sbdf_t sbdf, unsigned int reg, unsigned int len,
> + unsigned long data)
> +{
> +if ( !vpci_ecam_access_allowed(reg, len) ||
> + (reg + len) > PCI_CFG_SPACE_EXP_SIZE )
> +return 0;
> +
> +vpci_write(sbdf, reg, min(4u, len), data);
> +if ( len == 8 )
> +vpci_write(sbdf, reg + 4, 4, data >> 32);
> +
> +return 1;
> +}
> +
> +int vpci_ecam_mmio_read(pci_sbdf_t sbdf, unsigned int reg, unsigned int len,
> +unsigned long *data)
> +{
> +if ( !vpci_ecam_access_allowed(reg, len) ||
> + (reg + len) > PCI_CFG_SPACE_EXP_SIZE )
> +return 0;
> +
> +/*
> + * According to the PCIe 3.1A specification:
> + *  - Configuration Reads and Writes must usually be DWORD or smaller
> + *in size.
> + *  - Because Root Complex implementations are not required to support
> + *accesses to a RCRB that cross DW boundaries [...] software
> + *should take care not to cause the

[ovmf test] 165508: all pass - PUSHED

flight 165508 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/165508/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 785cfd33053f506d4a1c17100356a63f24e98f45
baseline version:
 ovmf e0c23cba5eaeb6c934a10ecdabcb235ef5d63799

Last test of basis   165505  2021-10-14 07:10:13 Z0 days
Testing same since   165508  2021-10-14 12:11:12 Z0 days1 attempts


People who touched revisions under test:
  Heinrich Schuchardt 
  Liu, Zhiguang 
  Zhiguang Liu 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   e0c23cba5e..785cfd3305  785cfd33053f506d4a1c17100356a63f24e98f45 -> 
xen-tested-master

Re: [PATCH V7 0/2] Add handling of extended regions (safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space")

On 14.10.21 17:28, Ian Jackson wrote:

Hi Ian

Oleksandr Tyshchenko writes ("[PATCH V7 0/2] Add handling of extended regions (safe ranges) on
Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space")"):

You can find an initial discussion at [1]-[7].

The extended region (safe range) is a region of guest physical address space
which is unused and could be safely used to create grant/foreign mappings
instead
of wasting real RAM pages from the domain memory for establishing these
mappings.

Thanks.

This patch has all the required acks, but I was aware of an
outstanding concern from Andrew, as set out in his most
recent mail on the subject:
Subject: Re: [RFC PATCH 1/3] xen: Introduce "gpaddr_bits" field to
XEN_SYSCTL_physinfo
Date: Tue, 7 Sep 2021 18:35:47 +0100
Message-ID: <973f5344-aa10-3ad6-ff02-ad5f358ad...@citrix.com>

I think it would be within the process to just commit the patch now,
but I thought it best to check as best I could that we weren't missing
anything. The process is supposed to support our continuing
development and also our quality, so I aim to do those things.

I reviewed that mail and had a conversation with Julien about it on
irc. My understanding is that Julien and Oleksandr's intent is that
Andrew's concerns have been addressed, although we don't have a
confirmation of that from Andrew.

In particular, I wanted to convince myself that if in fact there was
still a problem, we hadn't made a problem for ourselves with the API
here.

The new hypercalls are in unstable interfaces, so if we need to change
them in a future version (eg to make ARM migration work) that's OK.
Julien tells me that he doesn't believe there to be any impact on the
(x86) migration stream right now.

There is a new libxl stable interface. But I think it is
inoffensive. In particular, basically any mechanism to do this would
have that API. And that doesn't seem to touch on the implementation
issues described by Andrew.

Therefore, I think (i) we have tried to address the issues (ii) any
reminaing problems can be dealt with as followups, without trouble.

Completely agree with both statements.

So I have just pushed these two.

Thanks!

Thanks,
Ian.

--
Regards,

Oleksandr Tyshchenko

Re: [PING] Re: [PATCH] xen/arm: optee: Allocate anonymous domheap pages


Hi Stefano,

On 08/10/2021 22:49, Stefano Stabellini wrote:

On Fri, 8 Oct 2021, Julien Grall wrote:

On 07/10/2021 23:14, Stefano Stabellini wrote:

On Thu, 7 Oct 2021, Volodymyr Babchuk wrote:

Hi Stefano,

Stefano Stabellini  writes:


On Wed, 6 Oct 2021, Oleksandr wrote:

Hello all

Gentle reminder.

   Many thanks for the ping, this patch fell off my radar.


   

On 23.09.21 23:57, Volodymyr Babchuk wrote:

Hi Stefano,

Stefano Stabellini  writes:


On Mon, 6 Sep 2021, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

Allocate anonymous domheap pages as there is no strict need to
account them to a particular domain.

Since XSA-383 "xen/arm: Restrict the amount of memory that
dom0less
domU and dom0 can allocate" the dom0 cannot allocate memory
outside
of the pre-allocated region. This means if we try to allocate
non-anonymous page to be accounted to dom0 we will get an
over-allocation issue when assigning that page to the domain.
The anonymous page, in turn, is not assigned to any domain.

CC: Julien Grall 
Signed-off-by: Oleksandr Tyshchenko

Acked-by: Volodymyr Babchuk 

Only one question, which is more architectural: given that these
pages
are "unlimited", could the guest exploit the interface somehow to
force
Xen to allocate an very high number of anonymous pages?

E.g. could a domain call OPTEE_SMC_RPC_FUNC_ALLOC in a loop to
force Xen
to exaust all memory pages?

Generally, OP-TEE mediator tracks all resources allocated and
imposes
limits on them.

OPTEE_SMC_RPC_FUNC_ALLOC case is a bit different, because it is
issued
not by domain, but by OP-TEE itself. As OP-TEE is more trusted piece
of
system we allow it to request as many buffers as it wants. Also, we
know
that OP-TEE asks only for one such buffer per every standard call.
And
number of simultaneous calls is limited by number of OP-TEE threads,
which is quite low: typically only two.


So let me repeat it differently to see if I understood correctly:

- OPTEE_SMC_RPC_FUNC_ALLOC is only called by OP-TEE, not by the domain
- OPTEE is trusted and only call it twice anyway


Correct.


I am OK with this argument, but do we have a check to make sure a domU
cannot issue OPTEE_SMC_RPC_FUNC_ALLOC?


domU can't issue any RPC, because all RPCs are issued from OP-TEE
side. This is the nature of RPC - OP-TEE requests Normal World for some
service. But of course, Normal World can perform certain actions that
will make OP-TEE to issue a RPC. I discuss this in depth below.



Looking at the patch, there are other two places, in addition to
OPTEE_SMC_RPC_FUNC_ALLOC, where the anonymous memory pages can be
allocated:

1) copy_std_request
2) translate_noncontig

We need to prove that neither 1) or 2) can result in a domU exausting
Xen memory.

In the case of 1), it looks like the memory is freed before returning to
the DomU, right? If so, it should be no problem?


Yes, mediator makes shadow copy of every request buffer to hide
translated addresses from the guest. Number of requests is limited by
number of OP-TEE threads.


In the case of 2), it looks like the memory could outlive the call where
it is allocated. Is there any kind of protection against issuing
something like OPTEE_MSG_ATTR_TYPE_TMEM_INOUT in a loop? Is it OP-TEE
itself that would refuse the attempt? Thus, the idea is that
do_call_with_arg will return error and we'll just free the memory there?


Well, translate_noncontig() calls allocate_optee_shm_buf() which counts
all allocated buffers. So you can't call it more than
MAX_SHM_BUFFER_COUNT times, without de-allocating previous memory. But,
thanks to your question, I have found a bug there: memory is not freed
if allocate_optee_shm_buf() fails. I'll prepare patch later today.


I cannot see a check for errors returned by do_call_with_arg and memory
freeing done because of that. Sorry I am not super familiar with the
code, I am just trying to make sure we are not offering to DomUs an easy
way to crash the system.


I tried to eliminate all possibilities for a guest to crash the
system. Of course, this does not mean that there are none of them.

And yes, code is a bit hard to understand, because calls to OP-TEE are
stateful and you need to account for that state. From NW and SW this
looks quite fine, because state is handled naturally. But mediator sits
in a middle, so it's implementation is a bit messy.

I'll try to explain what is going on, so you it will be easier to
understand logic in the mediator.

There are two types of OP-TEE calls: fast calls and standard calls. Fast
call is simple: call SMC and get result. It does not allocate thread
context in OP-TEE and is non-preemptive. So yes, it should be fast. It
is used for simple things like "get OP-TEE version" or "exchange
capabilities". It is easy to handle them in mediator: just forward
the call, check result, return it back to a guest.

Standard calls are stateful. OP-TEE allocates thread for each call. This
call can be preempted either by IRQ or by RPC. For consistency IRQ
return

Re: [XEN PATCH v7 30/51] build: hook kconfig into xen build system

On Mon, Oct 11, 2021 at 05:38:47PM +0200, Jan Beulich wrote:
> On 24.08.2021 12:50, Anthony PERARD wrote:
> > Now that xen's build system is very close to Linux's ones, we can hook
> > "Makefile.host" into Xen's build system, and we can build Kconfig with
> > that.
> > 
> > Signed-off-by: Anthony PERARD 
> 
> Reviewed-by: Jan Beulich 

Thanks.

I'll be squashing the previous patch into this one i.e. adding
"$(XEN_ROOT)/.config: ;" into tools/kconfig/Makefile (the previous patch
was adding it into Rules.mk).

And will be adding the following into the commit message:

"tools/kconfig/Makefile" now needs a workaround to not rebuild
"$(XEN_ROOT)/.config", as `make` tries the rules "%.config" which
fails with:
tools/kconfig/Makefile:95: *** No configuration exists for this target 
on this architecture.  Stop.

-- 
Anthony PERARD

[PATCH v6 2/3] xen/arm: Enable the existing x86 virtual PCI support for ARM.

From: Rahul Singh 

The existing VPCI support available for X86 is adapted for Arm.
When the device is added to XEN via the hyper call
“PHYSDEVOP_pci_device_add”, VPCI handler for the config space
access is added to the Xen to emulate the PCI devices config space.

A MMIO trap handler for the PCI ECAM space is registered in XEN
so that when guest is trying to access the PCI config space,XEN
will trap the access and emulate read/write using the VPCI and
not the real PCI hardware.

For Dom0less systems scan_pci_devices() would be used to discover the
PCI device in XEN and VPCI handler will be added during XEN boots.

This patch is also doing some small fixes to fix compilation errors on
arm32 of vpci:
- add a cast to unsigned long in print in header.c
- add a cast to uint64_t in vpci_ecam_mmio_write

Signed-off-by: Rahul Singh 
Signed-off-by: Bertrand Marquis 
---
Changes in v6:
- Use new vpci_ecam_ helpers for PCI access
- Do not set XEN_DOMCTL_CDF_vpci for dom0 for now (will be done in a
future patch once everything is ready)
- rename REGISTER_OFFSET to ECAM_REG_OFFSET and move it to pci.h
- remove not needed local variables in vpci_mmio_write, the one in read
has been kept to ensure proper compilation on arm32
- move call to vpci_add_handlers before iommu init to simplify exit path
- move call to pci_cleanup_msi in the out section of pci_add_device if
pdev is not NULL and on error
- initialize pdev to NULL to handle properly exit path call of
pci_cleanup_msi
- keep has_vpci to return false for now as CFG_vpci has been removed.
Added a comment on top of the definition.
- fix compilation errors on arm32 (print in header.c, cast missing in
mmio_write.
- local variable was kept in vpci_mmio_read on arm to prevent a cast
error in arm32.
Change in v5:
- Add pci_cleanup_msi(pdev) incleanup part.
- Added Reviewed-by: Stefano Stabellini 
Change in v4:
- Move addition of XEN_DOMCTL_CDF_vpci flag to separate patch
Change in v3:
- Use is_pci_passthrough_enabled() in place of pci_passthrough_enabled variable
- Reject XEN_DOMCTL_CDF_vpci for x86 in arch_sanitise_domain_config()
- Remove IS_ENABLED(CONFIG_HAS_VPCI) from has_vpci()
Change in v2:
- Add new XEN_DOMCTL_CDF_vpci flag
- modify has_vpci() to include XEN_DOMCTL_CDF_vpci
- enable vpci support when pci-passthough option is enabled.
---
---
 xen/arch/arm/Makefile |  1 +
 xen/arch/arm/domain.c |  4 ++
 xen/arch/arm/vpci.c   | 74 +++
 xen/arch/arm/vpci.h   | 36 +
 xen/drivers/passthrough/pci.c | 18 -
 xen/drivers/vpci/header.c |  3 +-
 xen/drivers/vpci/vpci.c   |  2 +-
 xen/include/asm-arm/domain.h  |  1 +
 xen/include/asm-x86/pci.h |  2 -
 xen/include/public/arch-arm.h |  7 
 xen/include/xen/pci.h |  3 ++
 11 files changed, 146 insertions(+), 5 deletions(-)
 create mode 100644 xen/arch/arm/vpci.c
 create mode 100644 xen/arch/arm/vpci.h

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 64518848b2..07f634508e 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -7,6 +7,7 @@ ifneq ($(CONFIG_NO_PLAT),y)
 obj-y += platforms/
 endif
 obj-$(CONFIG_TEE) += tee/
+obj-$(CONFIG_HAS_VPCI) += vpci.o
 
 obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
 obj-y += bootfdt.init.o
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index eef0661beb..96e1b23550 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 
+#include "vpci.h"
 #include "vuart.h"
 
 DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
@@ -773,6 +774,9 @@ int arch_domain_create(struct domain *d,
 if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )
 goto fail;
 
+if ( (rc = domain_vpci_init(d)) != 0 )
+goto fail;
+
 return 0;
 
 fail:
diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
new file mode 100644
index 00..7c3552b65d
--- /dev/null
+++ b/xen/arch/arm/vpci.c
@@ -0,0 +1,74 @@
+/*
+ * xen/arch/arm/vpci.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include 
+#include 
+
+#include 
+
+static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
+  register_t *r, void *p)
+{
+pci_sbdf_t sbdf;
+/* data is needed to prevent a pointer cast on 32bit */
+unsigned long data = ~0ul;
+int ret;
+
+/* We ignore segment part and always handle segment 0 */
+sbdf.sbdf = ECAM_BDF(info->gpa);
+
+ret = vpci_ecam_mmio_read(sbdf, ECAM_REG_OFFSET(info->gpa),
+

[PATCH v6 3/3] arm/libxl: Emulated PCI device tree node in libxl

From: Rahul Singh 

libxl will create an emulated PCI device tree node in the device tree to
enable the guest OS to discover the virtual PCI during guest boot.
Emulated PCI device tree node will only be created when there is any
device assigned to guest.

A new area has been reserved in the arm guest physical map at
which the VPCI bus is declared in the device tree (reg and ranges
parameters of the node).

b_info.arch_arm.vpci is set by default to false
and it is not set to true anywhere. This way the
vpci node is not going to be created and we are only
introducing functions to create vpci DT node to be used
in the future.

Signed-off-by: Rahul Singh 
Signed-off-by: Michal Orzel 
---
Changes in v6:
According to https://marc.info/?l=xen-devel=163415838129479=2:
-do not set XEN_DOMCTL_CDF_vpci
-do not enable VPCI support (by setting b_info.arch_arm.vpci)
-do not define LIBXL_HAVE_BUILDINFO_ARM_VPCI
-keep b_info.arch_arm.vpci, make_vpci_node and its helpers
Change in v5:
- Move setting the arch_arm.vpci and XEN_DOMCTL_CDF_vpci to libxl_arm.c
Change in v4:
- Gate code for x86 for setting the XEN_DOMCTL_CDF_vpci for x86.
Change in v3:
- Make GUEST_VPCI_MEM_ADDR address 2MB aligned
Change in v2:
- enable doamin_vpci_init() when XEN_DOMCTL_CDF_vpci is set for domain.
---
 tools/libs/light/libxl_arm.c | 105 +++
 tools/libs/light/libxl_types.idl |   1 +
 xen/include/public/arch-arm.h|  10 +++
 3 files changed, 116 insertions(+)

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index e3140a6e00..52f1ddce48 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -269,6 +269,58 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
 return fdt_property(fdt, "reg", regs, sizeof(regs));
 }
 
+static int fdt_property_values(libxl__gc *gc, void *fdt,
+const char *name, unsigned num_cells, ...)
+{
+uint32_t prop[num_cells];
+be32 *cells = [0];
+int i;
+va_list ap;
+uint32_t arg;
+
+va_start(ap, num_cells);
+for (i = 0 ; i < num_cells; i++) {
+arg = va_arg(ap, uint32_t);
+set_cell(, 1, arg);
+}
+va_end(ap);
+
+return fdt_property(fdt, name, prop, sizeof(prop));
+}
+
+static int fdt_property_vpci_ranges(libxl__gc *gc, void *fdt,
+unsigned addr_cells,
+unsigned size_cells,
+unsigned num_regs, ...)
+{
+uint32_t regs[num_regs*((addr_cells*2)+size_cells+1)];
+be32 *cells = [0];
+int i;
+va_list ap;
+uint64_t arg;
+
+va_start(ap, num_regs);
+for (i = 0 ; i < num_regs; i++) {
+/* Set the memory bit field */
+arg = va_arg(ap, uint32_t);
+set_cell(, 1, arg);
+
+/* Set the vpci bus address */
+arg = addr_cells ? va_arg(ap, uint64_t) : 0;
+set_cell(, addr_cells , arg);
+
+/* Set the cpu bus address where vpci address is mapped */
+set_cell(, addr_cells, arg);
+
+/* Set the vpci size requested */
+arg = size_cells ? va_arg(ap, uint64_t) : 0;
+set_cell(, size_cells, arg);
+}
+va_end(ap);
+
+return fdt_property(fdt, "ranges", regs, sizeof(regs));
+}
+
 static int make_root_properties(libxl__gc *gc,
 const libxl_version_info *vers,
 void *fdt)
@@ -668,6 +720,53 @@ static int make_vpl011_uart_node(libxl__gc *gc, void *fdt,
 return 0;
 }
 
+static int make_vpci_node(libxl__gc *gc, void *fdt,
+const struct arch_info *ainfo,
+struct xc_dom_image *dom)
+{
+int res;
+const uint64_t vpci_ecam_base = GUEST_VPCI_ECAM_BASE;
+const uint64_t vpci_ecam_size = GUEST_VPCI_ECAM_SIZE;
+const char *name = GCSPRINTF("pcie@%"PRIx64, vpci_ecam_base);
+
+res = fdt_begin_node(fdt, name);
+if (res) return res;
+
+res = fdt_property_compat(gc, fdt, 1, "pci-host-ecam-generic");
+if (res) return res;
+
+res = fdt_property_string(fdt, "device_type", "pci");
+if (res) return res;
+
+res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
+GUEST_ROOT_SIZE_CELLS, 1, vpci_ecam_base, vpci_ecam_size);
+if (res) return res;
+
+res = fdt_property_values(gc, fdt, "bus-range", 2, 0, 255);
+if (res) return res;
+
+res = fdt_property_cell(fdt, "#address-cells", 3);
+if (res) return res;
+
+res = fdt_property_cell(fdt, "#size-cells", 2);
+if (res) return res;
+
+res = fdt_property_string(fdt, "status", "okay");
+if (res) return res;
+
+res = fdt_property_vpci_ranges(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
+GUEST_ROOT_SIZE_CELLS, 2,
+GUEST_VPCI_ADDR_TYPE_MEM, GUEST_VPCI_MEM_ADDR, GUEST_VPCI_MEM_SIZE,
+GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM, GUEST_VPCI_PREFETCH_MEM_ADDR,
+GUEST_VPCI_PREFETCH_MEM_SIZE);
+if (res) return res;
+
+res = fdt_end_node(fdt);
+if (res) return res;
+
+

[PATCH v6 1/3] xen/vpci: Move ecam access functions to common code

PCI standard is using ECAM and not MCFG which is coming from ACPI[1].
Use ECAM/ecam instead of MCFG in common code and in new functions added
in common vpci code by this patch.

Rename vpci_access_allowed to vpci_ecam_access_allowed and move it
from arch/x86/hvm/io.c to drivers/vpci/vpci.c.

Create vpci_ecam_mmio_{read,write} in drivers/vpci/vpci.c that
contains the common code to perform these operations, changed
vpci_mmcfg_{read,write} accordingly to make use of these functions.

The vpci_ecam_mmio_{read,write} functions are returning 0 on error and 1
on success. As the x86 code was previously always returning X86EMUL_OKAY
the return code is ignored. A comment has been added in the code to show
that this is intentional.

Those functions will be used in a following patch inside by arm vpci
implementation.

Rename MMCFG_SBDF to ECAM_SBDF.

Not functional change intended with this patch.

[1] https://wiki.osdev.org/PCI_Express

Suggested-by: Roger Pau Monné 
Signed-off-by: Bertrand Marquis 
---
Changes in v6: Patch added
---
 xen/arch/x86/hvm/io.c | 50 +---
 xen/drivers/vpci/vpci.c   | 60 +++
 xen/include/asm-x86/pci.h |  2 +-
 xen/include/xen/vpci.h| 10 +++
 4 files changed, 78 insertions(+), 44 deletions(-)

diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index 046a8eb4ed..340b8c8b0e 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -260,20 +260,6 @@ unsigned int hvm_pci_decode_addr(unsigned int cf8, 
unsigned int addr,
 return CF8_ADDR_LO(cf8) | (addr & 3);
 }
 
-/* Do some sanity checks. */
-static bool vpci_access_allowed(unsigned int reg, unsigned int len)
-{
-/* Check access size. */
-if ( len != 1 && len != 2 && len != 4 && len != 8 )
-return false;
-
-/* Check that access is size aligned. */
-if ( (reg & (len - 1)) )
-return false;
-
-return true;
-}
-
 /* vPCI config space IO ports handlers (0xcf8/0xcfc). */
 static bool vpci_portio_accept(const struct hvm_io_handler *handler,
const ioreq_t *p)
@@ -305,7 +291,7 @@ static int vpci_portio_read(const struct hvm_io_handler 
*handler,
 
 reg = hvm_pci_decode_addr(cf8, addr, );
 
-if ( !vpci_access_allowed(reg, size) )
+if ( !vpci_ecam_access_allowed(reg, size) )
 return X86EMUL_OKAY;
 
 *data = vpci_read(sbdf, reg, size);
@@ -335,7 +321,7 @@ static int vpci_portio_write(const struct hvm_io_handler 
*handler,
 
 reg = hvm_pci_decode_addr(cf8, addr, );
 
-if ( !vpci_access_allowed(reg, size) )
+if ( !vpci_ecam_access_allowed(reg, size) )
 return X86EMUL_OKAY;
 
 vpci_write(sbdf, reg, size, data);
@@ -394,7 +380,7 @@ static unsigned int vpci_mmcfg_decode_addr(const struct 
hvm_mmcfg *mmcfg,
paddr_t addr, pci_sbdf_t *sbdf)
 {
 addr -= mmcfg->addr;
-sbdf->bdf = MMCFG_BDF(addr);
+sbdf->bdf = ECAM_BDF(addr);
 sbdf->bus += mmcfg->start_bus;
 sbdf->seg = mmcfg->segment;
 
@@ -434,25 +420,8 @@ static int vpci_mmcfg_read(struct vcpu *v, unsigned long 
addr,
 reg = vpci_mmcfg_decode_addr(mmcfg, addr, );
 read_unlock(>arch.hvm.mmcfg_lock);
 
-if ( !vpci_access_allowed(reg, len) ||
- (reg + len) > PCI_CFG_SPACE_EXP_SIZE )
-return X86EMUL_OKAY;
-
-/*
- * According to the PCIe 3.1A specification:
- *  - Configuration Reads and Writes must usually be DWORD or smaller
- *in size.
- *  - Because Root Complex implementations are not required to support
- *accesses to a RCRB that cross DW boundaries [...] software
- *should take care not to cause the generation of such accesses
- *when accessing a RCRB unless the Root Complex will support the
- *access.
- *  Xen however supports 8byte accesses by splitting them into two
- *  4byte accesses.
- */
-*data = vpci_read(sbdf, reg, min(4u, len));
-if ( len == 8 )
-*data |= (uint64_t)vpci_read(sbdf, reg + 4, 4) << 32;
+/* Ignore return code */
+vpci_ecam_mmio_read(sbdf, reg, len, data);
 
 return X86EMUL_OKAY;
 }
@@ -476,13 +445,8 @@ static int vpci_mmcfg_write(struct vcpu *v, unsigned long 
addr,
 reg = vpci_mmcfg_decode_addr(mmcfg, addr, );
 read_unlock(>arch.hvm.mmcfg_lock);
 
-if ( !vpci_access_allowed(reg, len) ||
- (reg + len) > PCI_CFG_SPACE_EXP_SIZE )
-return X86EMUL_OKAY;
-
-vpci_write(sbdf, reg, min(4u, len), data);
-if ( len == 8 )
-vpci_write(sbdf, reg + 4, 4, data >> 32);
+/* Ignore return code */
+vpci_ecam_mmio_write(sbdf, reg, len, data);
 
 return X86EMUL_OKAY;
 }
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index cbd1bac7fc..c0853176d7 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -478,6 +478,66 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg, 
unsigned int size,
 spin_unlock(>vpci->lock);
 }
 
+/*

[PATCH v6 0/3] PCI devices passthrough on Arm

Hello All,

This serie is a follow up on Rahul serie where we included various fixes
required after review on the mailing list and a new patch to move some
of the x86 ecam related code to the common vpci code.

Most of the patches of the original serie have been merged and this
serie includes only 2 of the original patches reworked and 1 new patch:

Move some ECAM related functions from x86 to generic vpci
implementation:
- move vcpi mmcfg_{read/write} and vpci_access_allowed to common vpci.c.
- use ecam instead of mmcfg in common code.

Enable the existing x86 virtual PCI support for ARM:
- Add VPCI trap handler for each of the PCI device added for config
  space access.
- Register the trap handler in XEN for each of the host bridge PCI ECAM
  config space access.

Emulated PCI device tree node in libxl:
- Create a virtual PCI device tree node in libxl to enable the guest OS
  to discover the virtual PCI during guest boot.

The patch modifying xc_domain_ioport_permission has been removed from
the serie.

Bertrand Marquis (1):
  xen/vpci: Move ecam access functions to common code

Rahul Singh (2):
  xen/arm: Enable the existing x86 virtual PCI support for ARM.
  arm/libxl: Emulated PCI device tree node in libxl

 tools/libs/light/libxl_arm.c | 105 +++
 tools/libs/light/libxl_types.idl |   1 +
 xen/arch/arm/Makefile|   1 +
 xen/arch/arm/domain.c|   4 ++
 xen/arch/arm/vpci.c  |  74 ++
 xen/arch/arm/vpci.h  |  36 +++
 xen/arch/x86/hvm/io.c|  50 +++
 xen/drivers/passthrough/pci.c|  18 +-
 xen/drivers/vpci/header.c|   3 +-
 xen/drivers/vpci/vpci.c  |  60 ++
 xen/include/asm-arm/domain.h |   1 +
 xen/include/asm-x86/pci.h|   2 -
 xen/include/public/arch-arm.h|  17 +
 xen/include/xen/pci.h|   3 +
 xen/include/xen/vpci.h   |  10 +++
 15 files changed, 338 insertions(+), 47 deletions(-)
 create mode 100644 xen/arch/arm/vpci.c
 create mode 100644 xen/arch/arm/vpci.h

-- 
2.25.1

Re: [PATCH V7 0/2] Add handling of extended regions (safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space")

Oleksandr Tyshchenko writes ("[PATCH V7 0/2] Add handling of extended regions 
(safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide 
unallocated space")"):
> You can find an initial discussion at [1]-[7].
> 
> The extended region (safe range) is a region of guest physical address space
> which is unused and could be safely used to create grant/foreign mappings 
> instead
> of wasting real RAM pages from the domain memory for establishing these 
> mappings.

Thanks.

This patch has all the required acks, but I was aware of an
outstanding concern from Andrew, as set out in his most
recent mail on the subject:
  Subject: Re: [RFC PATCH 1/3] xen: Introduce "gpaddr_bits" field to 
XEN_SYSCTL_physinfo
  Date: Tue, 7 Sep 2021 18:35:47 +0100
  Message-ID: <973f5344-aa10-3ad6-ff02-ad5f358ad...@citrix.com>

I think it would be within the process to just commit the patch now,
but I thought it best to check as best I could that we weren't missing
anything.  The process is supposed to support our continuing
development and also our quality, so I aim to do those things.

I reviewed that mail and had a conversation with Julien about it on
irc.  My understanding is that Julien and Oleksandr's intent is that
Andrew's concerns have been addressed, although we don't have a
confirmation of that from Andrew.

In particular, I wanted to convince myself that if in fact there was
still a problem, we hadn't made a problem for ourselves with the API
here.

The new hypercalls are in unstable interfaces, so if we need to change
them in a future version (eg to make ARM migration work) that's OK.
Julien tells me that he doesn't believe there to be any impact on the
(x86) migration stream right now.

There is a new libxl stable interface.  But I think it is
inoffensive.  In particular, basically any mechanism to do this would
have that API.  And that doesn't seem to touch on the implementation
issues described by Andrew.

Therefore, I think (i) we have tried to address the issues (ii) any
reminaing problems can be dealt with as followups, without trouble.

So I have just pushed these two.

Thanks,
Ian.

Re: [XEN PATCH v7 38/51] build: use main rune to build host binary x86's mkelf32 and mkreloc

On Mon, Oct 11, 2021 at 05:56:06PM +0200, Jan Beulich wrote:
> On 24.08.2021 12:50, Anthony PERARD wrote:
> > Signed-off-by: Anthony PERARD 
> 
> Acked-by: Jan Beulich 
> 
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -81,6 +81,10 @@ endif
> >  extra-y += asm-macros.i
> >  extra-y += xen.lds
> >  
> > +hostprogs-y += boot/mkelf32
> > +HOSTCFLAGS_efi/mkreloc.o := -g
> 
> To be honest I don't think this extra option would have been necessary
> to retain.

Probably not, I'll remove it.

Thanks,

-- 
Anthony PERARD

Re: [XEN PATCH v7 32/51] build: Remove KBUILD_ specific from Makefile.host

On Mon, Oct 11, 2021 at 05:47:29PM +0200, Jan Beulich wrote:
> On 24.08.2021 12:50, Anthony PERARD wrote:
> > This will allow $(HOSTCFLAGS) to actually be used when building
> > programmes for the build-host.
> > 
> > The other variable don't exist in our build system.
> > 
> > Also remove $(KBUILD_EXTMOD) since it should always be empty.
> > 
> > Signed-off-by: Anthony PERARD 
> 
> Acked-by: Jan Beulich 
> 
> I wonder though whether their use of KBUILD_ prefixes doesn't match
> our XEN_ ones (e.g. KBUILD_CFLAGS vs XEN_CFLAGS), in which case
> replacing rather than stripping might be the way to go.

Well, at the moment, HOSTCFLAGS is defined in Config.mk, and xen/ makes
use of it. I don't think the other variable that I strip KBUILD_ from
exist, so maybe for those, changing prefix for XEN_ would be fine.
But for HOSTCFLAGS, I think we would want a new patch to set it in
xen/Makefile, and use it in xen/. But I don't think we need to spend
time on it at the moment.

Thanks,

-- 
Anthony PERARD

Re: [PATCH 2/2] memory: XENMEM_add_to_physmap (almost) wrapping checks

On 14.10.2021 13:29, Julien Grall wrote:
> On 13/09/2021 07:42, Jan Beulich wrote:
>> Determining that behavior is correct (i.e. results in failure) for a
>> passed in GFN equaling INVALID_GFN is non-trivial. Make this quite a
>> bit more obvious by checking input in generic code - both for singular
>> requests to not match the value and for range ones to not pass / wrap
>> through it.
>>
>> For Arm similarly make more obvious that no wrapping of MFNs passed
>> for XENMAPSPACE_dev_mmio and thus to map_dev_mmio_region() can occur:
>> Drop the "nr" parameter of the function to avoid future callers
>> appearing which might not themselves check for wrapping. Otherwise
>> the respective ASSERT() in rangeset_contains_range() could trigger.
>>
>> Signed-off-by: Jan Beulich 
>> ---
>> I find it odd that map_dev_mmio_region() returns success upon
>> iomem_access_permitted() indicating failure - is this really intended?
> 
> AFAIR yes. The hypercall is not used as "Map the region" but instead 
> "Make sure the region is mapped if the IOMEM region is accessible".
> 
> It is necessary to return 0 because dom0 OS cannot distinguished between 
> emulated and non-emulated. So we may report error when there is none.

Odd, but I clearly don't understand all the aspects here.

>> As per commit 102984bb1987 introducing it this also was added for ACPI
>> only - any reason XENMAPSPACE_dev_mmio isn't restricted to CONFIG_ACPI
>> builds?
> 
> There is nothing specific to ACPI in the implementation. So I don't 
> really see the reason to restrict to CONFIG_ACPI.
> 
> However, it is still possible to boot using DT when Xen is built with 
> CONFIG_ACPI. So if the restriction was desirable, then I think it should 
> be using !acpi_disabled.

My point was rather about this potentially being dead code in non-ACPI
builds (i.e. in particular uniformly on 32-bit).

>> @@ -841,6 +844,15 @@ int xenmem_add_to_physmap(struct domain
>>   if ( xatp->size < start )
>>   return -EILSEQ;
>>   
>> +if ( xatp->gpfn + xatp->size < xatp->gpfn ||
>> + xatp->idx + xatp->size < xatp->idx )
>> +{
>> +#define _gfn(x) (x)
> 
> AFAICT, _gfn() will already be defined. So some compiler may complain 
> because will be defined differently on debug build.

No - _gfn() is an inline function as per typesafe.h. (Or else it
wouldn't be just "some" compiler, but gcc at least would have
complained to me.)

> However...
> 
>> +BUILD_BUG_ON(INVALID_GFN + 1);
> 
> ... I might be missing something... but why can't use gfn_x(INVALID_GFN) 
> + 1 here?

Because gfn_x() also is an inline function, and that's not suitable
for a compile-time constant expression.

> In fact, I am not entirely sure what's the purpose of this 
> BUILD_BUG_ON(). Could you give more details?

The expression in the surrounding if() relies on INVALID_GFN being the
largest representable value, i.e. this ensures that INVALID_GFN doesn't
sit anywhere in [xatp->gpfn, xatp->gpfn + xatp->size).

Jan

[xen-unstable-smoke test] 165507: tolerable all pass - PUSHED

flight 165507 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/165507/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  2f5f0a1b77161993c16c4cc243467d75e5b7633b
baseline version:
 xen  4cfab4425d39f76660b4e76ba6ee4cbf0f92e7e5

Last test of basis   165501  2021-10-13 23:01:33 Z0 days
Testing same since   165507  2021-10-14 11:01:35 Z0 days1 attempts


People who touched revisions under test:
  Anthony PERARD 
  Christian Lindig 
mailto:christian.lin...@citrix.com>>
  Jan Beulich 
  Julien Grall 
  Michal Orzel 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   4cfab4425d..2f5f0a1b77  2f5f0a1b77161993c16c4cc243467d75e5b7633b -> smoke

Re: [XEN PATCH v7 27/51] build: introduce if_changed_deps

On Mon, Oct 11, 2021 at 04:20:36PM +0200, Jan Beulich wrote:
> On 24.08.2021 12:50, Anthony PERARD wrote:
> > @@ -400,7 +407,7 @@ $(TARGET).gz: $(TARGET)
> > gzip -n -f -9 < $< > $@.new
> > mv $@.new $@
> >  
> > -$(TARGET): FORCE
> > +$(TARGET): tools_fixdep FORCE
> > $(MAKE) -C tools
> 
> Shouldn't this include building fixdep, in which case the extra dependency
> here is unnecessary? I can see that it's needed ...

Indeed, we don't really needs "tools_fixdep" prerequisite because the
first command of the recipe is going to build fixdep.

Thanks,

-- 
Anthony PERARD

Re: [XEN PATCH v7 26/51] build: build everything from the root dir, use obj=$subdir

On 14.10.2021 15:33, Anthony PERARD wrote:
> On Wed, Oct 13, 2021 at 03:24:31PM +0100, Anthony PERARD wrote:
>> On Mon, Oct 11, 2021 at 04:02:22PM +0200, Jan Beulich wrote:
>>> On 24.08.2021 12:50, Anthony PERARD wrote:
  ifeq ($(CONFIG_ENFORCE_UNIQUE_SYMBOLS),y)
  cmd_cc_o_c = $(CC) $(c_flags) -c $< -o $(dot-target).tmp -MQ $@
  ifeq ($(CONFIG_CC_IS_CLANG),y)
 -cmd_objcopy_fix_sym = $(OBJCOPY) --redefine-sym $<=$(SRCPATH)/$< 
 $(dot-target).tmp $@
 +cmd_objcopy_fix_sym = $(OBJCOPY) --redefine-sym $(>>> $(dot-target).tmp $@
>>>
>>> Are you sure about the $< => $(>> was present only ...
>>
>> I have to check again. Maybe $< didn't work and it's more obvious with
>> this patch. Or maybe that depends on the version of clang.
> 
> With clang 12, the original line doesn't work for the few objects that
> are built from "subdir/source.c". I guess it is just by luck that they
> aren't any duplicated symbols.

Well, I recall that there was different behavior and hence different
treatment was needed. If that's no longer the case with your changes,
then that's certainly fine, but will want explaining in the description.

Jan

Re: [PATCH V7 2/2] libxl/arm: Add handling of extended regions for DomU

Julien Grall writes ("Re: [PATCH V7 2/2] libxl/arm: Add handling of extended 
regions for DomU"):
> On 14/10/2021 12:40, Oleksandr Tyshchenko wrote:
> > From: Oleksandr Tyshchenko 
...
> > Suggested-by: Julien Grall 
> > Signed-off-by: Oleksandr Tyshchenko 
> 
> Reviewed-by: Julien Grall 

Again, on that basis,

Acked-by: Ian Jackson 

Thanks all.

Ian.

Re: [XEN PATCH v7 26/51] build: build everything from the root dir, use obj=$subdir

On Wed, Oct 13, 2021 at 03:24:31PM +0100, Anthony PERARD wrote:
> On Mon, Oct 11, 2021 at 04:02:22PM +0200, Jan Beulich wrote:
> > On 24.08.2021 12:50, Anthony PERARD wrote:
> > >  ifeq ($(CONFIG_ENFORCE_UNIQUE_SYMBOLS),y)
> > >  cmd_cc_o_c = $(CC) $(c_flags) -c $< -o $(dot-target).tmp -MQ $@
> > >  ifeq ($(CONFIG_CC_IS_CLANG),y)
> > > -cmd_objcopy_fix_sym = $(OBJCOPY) --redefine-sym $<=$(SRCPATH)/$< 
> > > $(dot-target).tmp $@
> > > +cmd_objcopy_fix_sym = $(OBJCOPY) --redefine-sym $( > > $(dot-target).tmp $@
> > 
> > Are you sure about the $< => $( > was present only ...
> 
> I have to check again. Maybe $< didn't work and it's more obvious with
> this patch. Or maybe that depends on the version of clang.

With clang 12, the original line doesn't work for the few objects that
are built from "subdir/source.c". I guess it is just by luck that they
aren't any duplicated symbols.

> > >  else
> > > -cmd_objcopy_fix_sym = $(OBJCOPY) --redefine-sym 
> > > $( > > +cmd_objcopy_fix_sym = $(OBJCOPY) --redefine-sym $( > > $(dot-target).tmp $@
> > 
> > ... here.

-- 
Anthony PERARD

[libvirt test] 165504: regressions - FAIL

flight 165504 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/165504/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-libvirt   6 libvirt-buildfail REGR. vs. 151777
 build-amd64-libvirt   6 libvirt-buildfail REGR. vs. 151777
 build-arm64-libvirt   6 libvirt-buildfail REGR. vs. 151777
 build-i386-libvirt6 libvirt-buildfail REGR. vs. 151777

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-raw   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a

version targeted for testing:
 libvirt  61cb54e3cbefc389d64357d9c479750593855383
baseline version:
 libvirt  2c846fa6bcc11929c9fb857a22430fb9945654ad

Last test of basis   151777  2020-07-10 04:19:19 Z  461 days
Failing since151818  2020-07-11 04:18:52 Z  460 days  446 attempts
Testing same since   165504  2021-10-14 04:21:19 Z0 days1 attempts


People who touched revisions under test:
Adolfo Jayme Barrientos 
  Aleksandr Alekseev 
  Aleksei Zakharov 
  Andika Triwidada 
  Andrea Bolognani 
  Ani Sinha 
  Balázs Meskó 
  Barrett Schonefeld 
  Bastian Germann 
  Bastien Orivel 
  BiaoXiang Ye 
  Bihong Yu 
  Binfeng Wu 
  Bjoern Walk 
  Boris Fiuczynski 
  Brian Turek 
  Bruno Haible 
  Chris Mayo 
  Christian Borntraeger 
  Christian Ehrhardt 
  Christian Kirbach 
  Christian Schoenebeck 
  Cole Robinson 
  Collin Walling 
  Cornelia Huck 
  Cédric Bosdonnat 
  Côme Borsoi 
  Daniel Henrique Barboza 
  Daniel Letai 
  Daniel P. Berrange 
  Daniel P. Berrangé 
  Didik Supriadi 
  dinglimin 
  Dmytro Linkin 
  Eiichi Tsukata 
  Eric Farman 
  Erik Skultety 
  Fabian Affolter 
  Fabian Freyer 
  Fabiano Fidêncio 
  Fangge Jin 
  Farhan Ali 
  Fedora Weblate Translation 
  gongwei 
  Guoyi Tu
  Göran Uddeborg 
  Halil Pasic 
  Han Han 
  Hao Wang 
  Hela Basa 
  Helmut Grohne 
  Hiroki Narukawa 
  Ian Wienand 
  Jakob Meng 
  Jamie Strandboge 
  Jamie Strandboge 
  Jan Kuparinen 
  jason lee 
  Jean-Baptiste Holcroft 
  Jia Zhou 
  Jianan Gao 
  Jim Fehlig 
  Jin Yan 
  Jinsheng Zhang 
  Jiri Denemark 
  John Ferlan 
  Jonathan Watt 
  Jonathon Jongsma 
  Julio Faracco 
  Justin Gatzen 
  Ján Tomko 
  Kashyap Chamarthy 
  Kevin Locke 
  Kristina Hanicova 
  Laine Stump 
  Laszlo Ersek 
  Lee Yarwood 
  Lei Yang 
  Liao Pingfang 
  Lin Ma 
  Lin Ma 
  Lin Ma 
  Liu Yiding 
  Luke Yue 
  Luyao Zhong 
  Marc Hartmayer 
  Marc-André Lureau 
  Marek Marczykowski-Górecki 
  Markus Schade 
  Martin Kletzander 
  Masayoshi Mizuma 
  Matej Cepl 
  Matt Coleman 
  Matt Coleman 
  Mauro Matteo Cascella 
  Meina Li 
  Michal Privoznik 
  Michał Smyk 
  Milo Casagrande 
  Moshe Levi 
  Muha Aliss 
  Nathan 
  Neal Gompa 
  Nick Chevsky 
  Nick Shyrokovskiy 
  Nickys Music Group 
  Nico Pache 
  Nikolay Shirokovskiy 
  Olaf Hering 
  Olesya Gerasimenko 
  Orion Poplawski 
  Pany 
  Patrick Magauran 
  Paulo de Rezende Pinatti 
  Pavel Hrdina 
  Peng Liang 
  Peter Krempa 
  Pino Toscano 
  Pino Toscano 
  Piotr Drąg 
  Prathamesh Chavan 
  Praveen K Paladugu 
  Richard W.M. Jones 
  Ricky Tigg 
  Robin Lee 
  Roman Bogorodskiy 
  Roman Bolshakov 
  Ryan Gahagan 
  Ryan Schmidt 
  Sam Hartman 
  Scott Shambarger 
  Sebastian Mitterle 
  SeongHyun Jo 
  Shalini Chellathurai Saroja 
  Shaojun Yang 
  Shi Lei 
  simmon 
  Simon Chopin 
  Simon Gaiser 
  Simon Rowe 
  Stefan Bader 
  Stefan Berger 
  Stefan Berger 
  Stefan Hajnoczi 
  Stefan Hajnoczi 
  Szymon Scholz 
  Thomas Huth 
  Tim Wiederhake 
  Tomáš Golembiovský 
  Tomáš Janoušek 
  Tuguoyi 
  Victor Toso 
  Ville Skyttä 
  Vinayak Kale

[xen-unstable test] 165503: tolerable FAIL - PUSHED

flight 165503 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/165503/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-rtds 20 guest-localmigrate/x10   fail  like 165491
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 165491
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 165491
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 165491
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 165491
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 165491
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 165491
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 165491
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 165491
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 165491
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 165491
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 165491
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 165491
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen

Re: [PATCH V7 2/2] libxl/arm: Add handling of extended regions for DomU


Hi Oleksandr,

On 14/10/2021 12:40, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

The extended region (safe range) is a region of guest physical
address space which is unused and could be safely used to create
grant/foreign mappings instead of wasting real RAM pages from
the domain memory for establishing these mappings.

The extended regions are chosen at the domain creation time and
advertised to it via "reg" property under hypervisor node in
the guest device-tree. As region 0 is reserved for grant table
space (always present), the indexes for extended regions are 1...N.
If extended regions could not be allocated for some reason,
Xen doesn't fail and behaves as usual, so only inserts region 0.

Please note the following limitations:
- The extended region feature is only supported for 64-bit domain
   currently.
- The ACPI case is not covered.

***

The algorithm to choose extended regions for non-direct mapped
DomU is simpler in comparison with the algorithm for direct mapped
Dom0. We usually have a lot of unused space above 4GB, and might
have some unused space below 4GB (depends on guest memory size).
Try to allocate separate 2MB-aligned extended regions from the first
(below 4GB) and second (above 4GB) RAM banks taking into the account
the maximum supported guest physical address space size and the amount
of memory assigned to the guest. The minimum size of extended region
the same as for Dom0 (64MB).

Please note, we introduce fdt_property_reg_placeholder helper which
purpose is to create N ranges that are zeroed. The interesting fact
is that libfdt already has fdt_property_placeholder(). But this was
introduced only in 2017, so there is a risk that some distros may not
ship the last libfdt version. This is why we implement our own light
variant for now.

Suggested-by: Julien Grall 
Signed-off-by: Oleksandr Tyshchenko 


Reviewed-by: Julien Grall 

Cheers,

--
Julien Grall

[ovmf test] 165505: all pass - PUSHED

flight 165505 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/165505/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf e0c23cba5eaeb6c934a10ecdabcb235ef5d63799
baseline version:
 ovmf 978d428ec3ca2520c217819901c8465235c45c5e

Last test of basis   165502  2021-10-13 23:10:03 Z0 days
Testing same since   165505  2021-10-14 07:10:13 Z0 days1 attempts


People who touched revisions under test:
  Abner Chang 
  Gerd Hoffmann 
  Hua Ma 
  Jiewen Yao 
  Konstantin Aladyshev 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   978d428ec3..e0c23cba5e  e0c23cba5eaeb6c934a10ecdabcb235ef5d63799 -> 
xen-tested-master

Re: [PATCH V6 2/2] libxl/arm: Add handling of extended regions for DomU




Hi Stefano


On 13.10.21 23:24, Stefano Stabellini wrote:

On Wed, 13 Oct 2021, Oleksandr wrote:

On 13.10.21 21:26, Oleksandr wrote:

On 13.10.21 20:07, Julien Grall wrote:

Hi Julien


Hi,

On 13/10/2021 17:44, Oleksandr wrote:

On 13.10.21 19:24, Julien Grall wrote:

On 13/10/2021 16:49, Oleksandr wrote:

On 13.10.21 18:15, Julien Grall wrote:

On 13/10/2021 14:46, Oleksandr wrote:

Hi Julien

Hi Oleksandr,

Hi Julien


Thank you for the prompt response.



On 13.10.21 00:43, Oleksandr wrote:
diff --git a/tools/libs/light/libxl_arm.c
b/tools/libs/light/libxl_arm.c
index e3140a6..53ae0f3 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -615,9 +615,12 @@ static int make_hypervisor_node(libxl__gc
*gc, void *fdt,
     "xen,xen");
   if (res) return res;

-    /* reg 0 is grant table space */
+    /*
+ * reg 0 is a placeholder for grant table space, reg 1...N
are
+ * the placeholders for extended regions.
+ */
   res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
GUEST_ROOT_SIZE_CELLS,
-    1,GUEST_GNTTAB_BASE,
GUEST_GNTTAB_SIZE);
+    GUEST_RAM_BANKS + 1, 0, 0, 0, 0, 0,
0);

Here you are relying on GUEST_RAM_BANKS == 2. I think this is
pretty fragile as this may change in the future.

fdt_property_regs() is not really suitable for here. I would
suggest to create a new helper fdt_property_placeholder() which
takes the address cells, size cells and the number of ranges. The
function will then create N range that are zeroed.

You are right. Probably, we could add an assert here for now to be
triggered if "GUEST_RAM_BANKS != 2"?
But, if you still think we need to make it with the helper right
now, I will. However, I don't know how long this will take.

I would prefer if we introduce the helper now. Below a potential
version (not compiled):


You wouldn't believe)))
I decided to not wait for the answer and re-check. So, I ended up with
the similar to what you suggested below. Thank you.
Yes, it will work if add missing locals. However, I initially named it
exactly as was suggested (fdt_property_placeholder) and got a
compilation error, since fdt_property_placeholder is already present.
So, I was thinking to choose another name or to even open-code it, but I
see you already proposed new name fdt_property_reg_placeholder.

Ah I didn't realize that libfdt already implemented
fdt_property_placeholder(). The function sounds perfect for us (and the
code is nicer :)), but unfortunately this was introdcued only in 2017. So
it is possible that some distros may not ship the last libfdt.

So for now, I think we need to implement our own. In a follow-up we could
try to provide a compat layer as we did for other missing fdt_* helpers.

Cheers,


Well, I hope that I addressed all your comments. If so, I will prepare V7.


May I please ask, are you *preliminary* OK with new changes requested by
Julien? The main changes are to bring finalize_*() back (hopefully there is a
way how to downsize a placeholder), avoid relying on the fact that Bank 0 is
always below 4GB, adding a convenient helper to create a placeholder for N
ranges plus some code hardening, etc.

Yes, I am OK with it


Thank you for confirming. I have just pushed V7:

https://lore.kernel.org/xen-devel/1634211645-26912-1-git-send-email-olekst...@gmail.com/






diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index e3140a6..a780155 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -269,6 +269,21 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
  return fdt_property(fdt, "reg", regs, sizeof(regs));
  }

+static int fdt_property_reg_placeholder(libxl__gc *gc, void *fdt,
+    unsigned int addr_cells,
+    unsigned int size_cells,
+    unsigned int num_regs)
+{
+    uint32_t regs[num_regs * (addr_cells + size_cells)];
+    be32 *cells = [0];
+    unsigned int i;
+
+    for (i = 0; i < num_regs; i++)
+    set_range(, addr_cells, size_cells, 0, 0);
+
+    return fdt_property(fdt, "reg", regs, sizeof(regs));
+}
+
  static int make_root_properties(libxl__gc *gc,
  const libxl_version_info *vers,
  void *fdt)
@@ -615,9 +630,13 @@ static int make_hypervisor_node(libxl__gc *gc, void
*fdt,
    "xen,xen");
  if (res) return res;

-    /* reg 0 is grant table space */
-    res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
GUEST_ROOT_SIZE_CELLS,
-    1,GUEST_GNTTAB_BASE, GUEST_GNTTAB_SIZE);
+    /*
+ * reg 0 is a placeholder for grant table space, reg 1...N are
+ * the placeholders for extended regions.
+ */
+    res = fdt_property_reg_placeholder(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
+

[PATCH V7 2/2] libxl/arm: Add handling of extended regions for DomU

2021-10-14 Thread Oleksandr Tyshchenko

From: Oleksandr Tyshchenko 

The extended region (safe range) is a region of guest physical
address space which is unused and could be safely used to create
grant/foreign mappings instead of wasting real RAM pages from
the domain memory for establishing these mappings.

The extended regions are chosen at the domain creation time and
advertised to it via "reg" property under hypervisor node in
the guest device-tree. As region 0 is reserved for grant table
space (always present), the indexes for extended regions are 1...N.
If extended regions could not be allocated for some reason,
Xen doesn't fail and behaves as usual, so only inserts region 0.

Please note the following limitations:
- The extended region feature is only supported for 64-bit domain
  currently.
- The ACPI case is not covered.

***

The algorithm to choose extended regions for non-direct mapped
DomU is simpler in comparison with the algorithm for direct mapped
Dom0. We usually have a lot of unused space above 4GB, and might
have some unused space below 4GB (depends on guest memory size).
Try to allocate separate 2MB-aligned extended regions from the first
(below 4GB) and second (above 4GB) RAM banks taking into the account
the maximum supported guest physical address space size and the amount
of memory assigned to the guest. The minimum size of extended region
the same as for Dom0 (64MB).

Please note, we introduce fdt_property_reg_placeholder helper which
purpose is to create N ranges that are zeroed. The interesting fact
is that libfdt already has fdt_property_placeholder(). But this was
introduced only in 2017, so there is a risk that some distros may not
ship the last libfdt version. This is why we implement our own light
variant for now.

Suggested-by: Julien Grall 
Signed-off-by: Oleksandr Tyshchenko 
---
! Stefano, Ian I dropped your A-b/R-b again as the patch has changed
significantly !

Changes RFC -> V2:
   - update patch description
   - drop uneeded "extended-region" DT property
   - clear reg array in finalise_ext_region() and add a TODO

Changes V2 -> V3:
   - update patch description, comments in code
   - only pick up regions with size >= 64MB
   - move the region calculation to make_hypervisor_node() and drop
 finalise_ext_region()
   - extend the list of arguments for make_hypervisor_node()
   - do not show warning for 32-bit domain
   - change the region alignment from 1GB to 2MB
   - move EXT_REGION_SIZE to public/arch-arm.h

Changes V3 -> V4:
   - add R-b, A-b and T-b

Changes V4 -> V5:
   - update patch description and comments in code
   - reflect changes done in previous patch to pass gpaddr_bits
 via createdomain domctl (struct xen_arch_domainconfig)
   - drop R-b, A-b and T-b
   - drop limit for maximum extended region size (128GB)
   - try to also allocate region below 4GB, optimize code
 for calculating extended regions

Change V5 -> V6:
   - reflect changes done in previous patch to pass gpaddr_bits
 via getdomaininfo domctl (struct xen_domctl_getdomaininfo)
   - reduce the number of local variables, rework calculations

Change V6 -> V7:
   - return finalize_*() back and put all logic there with re-using
 fdt_setprop() to update placeholders
   - introduce fdt_property_reg_placeholder() helper
   - rework regions calculation to not rely on the fact that Bank 0
 is always below 4GB
   - drop check for 32-bit domain and assert for invalid gpaddr_bits
   - change a formula to calculate bankend value
   - move EXT_REGION_MIN_SIZE definition from the public header to
 libxl_arm.c
   - do not use asserts for the return values, propagate errors to
 the callers
   - add a comment in public header
---
 tools/libs/light/libxl_arm.c  | 106 --
 xen/include/public/arch-arm.h |   5 ++
 2 files changed, 106 insertions(+), 5 deletions(-)

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index e3140a6..a780155 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -269,6 +269,21 @@ static int fdt_property_regs(libxl__gc *gc, void *fdt,
 return fdt_property(fdt, "reg", regs, sizeof(regs));
 }
 
+static int fdt_property_reg_placeholder(libxl__gc *gc, void *fdt,
+unsigned int addr_cells,
+unsigned int size_cells,
+unsigned int num_regs)
+{
+uint32_t regs[num_regs * (addr_cells + size_cells)];
+be32 *cells = [0];
+unsigned int i;
+
+for (i = 0; i < num_regs; i++)
+set_range(, addr_cells, size_cells, 0, 0);
+
+return fdt_property(fdt, "reg", regs, sizeof(regs));
+}
+
 static int make_root_properties(libxl__gc *gc,
 const libxl_version_info *vers,
 void *fdt)
@@ -615,9 +630,13 @@ static int make_hypervisor_node(libxl__gc *gc, void *fdt,
   "xen,xen");
 if (res) return res;

[PATCH V7 1/2] xen/arm: Introduce gpaddr_bits field to struct xen_domctl_getdomaininfo

2021-10-14 Thread Oleksandr Tyshchenko

From: Oleksandr Tyshchenko 

We need to pass info about maximum supported guest physical
address space size to the toolstack on Arm in order to properly
calculate the base and size of the extended region (safe range)
for the guest. The extended region is unused address space which
could be safely used by domain for foreign/grant mappings on Arm.
The extended region itself will be handled by the subsequent
patch.

Currently the same guest physical address space size is used
for all guests (p2m_ipa_bits variable on Arm, the x86 equivalent
is hap_paddr_bits).

Add an explicit padding after "gpaddr_bits" field and also
(while at it) after "domain" field.

Also make sure that full structure is cleared in all cases by
moving the clearing into getdomaininfo(). Currently it is only
cleared by the sysctl caller (and only once).

Please note, we do not need to bump XEN_DOMCTL_INTERFACE_VERSION
as a bump has already occurred in this release cycle. But we do
need to bump XEN_SYSCTL_INTERFACE_VERSION as the structure is
re-used in a sysctl.

Suggested-by: Julien Grall 
Signed-off-by: Oleksandr Tyshchenko 
Reviewed-by: Ian Jackson 
[hypervisor parts]
Reviewed-by: Jan Beulich 
---
Changes RFC -> V2:
   - update patch subject/description
   - replace arch-specific sub-struct with common gpaddr_bits
 field and update code to reflect that

Changes V2 -> V3:
   - make the field uint8_t and add uint8_t pad[7] after
   - remove leading blanks in libxl.h

Changes V3 -> V4:
   - also print gpaddr_bits from output_physinfo()
   - add Michal's R-b

Changes V4 -> V5:
   - update patch subject and description
   - drop Michal's R-b
   - pass gpaddr_bits via createdomain domctl
 (struct xen_arch_domainconfig)

Changes V5 -> V6:
   - update patch subject and description
   - pass gpaddr_bits via getdomaininfo domctl
 (struct xen_domctl_getdomaininfo)

Changes V6 -> V7:
   - update patch description
   - do not bump XEN_DOMCTL_INTERFACE_VERSION
   - bump XEN_SYSCTL_INTERFACE_VERSION
   - add explicit paddings
   - clear the full structure in getdomaininfo()

Changes V7 -> V7.1:
   - add Jan's R-b
   - drop non-useful change (info->flags |= ...) in getdomaininfo()
---
 tools/include/libxl.h| 8 
 tools/include/xenctrl.h  | 1 +
 tools/libs/ctrl/xc_domain.c  | 1 +
 tools/libs/light/libxl_domain.c  | 1 +
 tools/libs/light/libxl_types.idl | 1 +
 xen/arch/arm/domctl.c| 2 ++
 xen/arch/x86/domctl.c| 1 +
 xen/common/domctl.c  | 4 ++--
 xen/common/sysctl.c  | 2 +-
 xen/include/public/domctl.h  | 3 +++
 xen/include/public/sysctl.h  | 2 +-
 11 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index ee73eb0..2e8679d 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -886,6 +886,14 @@ typedef struct libxl__ctx libxl_ctx;
 #define LIBXL_HAVE_DOMINFO_NEVER_STOP 1
 
 /*
+ * LIBXL_HAVE_DOMINFO_GPADDR_BITS
+ *
+ * If this is defined, libxl_dominfo will contain an uint8 field called
+ * gpaddr_bits, containing the guest physical address space size.
+ */
+#define LIBXL_HAVE_DOMINFO_GPADDR_BITS 1
+
+/*
  * LIBXL_HAVE_QXL
  *
  * If defined, then the libxl_vga_interface_type will contain another value:
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index a306399..07b96e6 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -462,6 +462,7 @@ typedef struct xc_dominfo {
 unsigned int  max_vcpu_id;
 xen_domain_handle_t handle;
 unsigned int  cpupool;
+uint8_t   gpaddr_bits;
 struct xen_arch_domainconfig arch_config;
 } xc_dominfo_t;
 
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index 23322b7..b155d6a 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -396,6 +396,7 @@ int xc_domain_getinfo(xc_interface *xch,
 info->nr_online_vcpus = domctl.u.getdomaininfo.nr_online_vcpus;
 info->max_vcpu_id = domctl.u.getdomaininfo.max_vcpu_id;
 info->cpupool = domctl.u.getdomaininfo.cpupool;
+info->gpaddr_bits = domctl.u.getdomaininfo.gpaddr_bits;
 info->arch_config = domctl.u.getdomaininfo.arch_config;
 
 memcpy(info->handle, domctl.u.getdomaininfo.handle,
diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
index 51a6127..544a9bf 100644
--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -306,6 +306,7 @@ void libxl__xcinfo2xlinfo(libxl_ctx *ctx,
 xlinfo->vcpu_max_id = xcinfo->max_vcpu_id;
 xlinfo->vcpu_online = xcinfo->nr_online_vcpus;
 xlinfo->cpupool = xcinfo->cpupool;
+xlinfo->gpaddr_bits = xcinfo->gpaddr_bits;
 xlinfo->domain_type = (xcinfo->flags & XEN_DOMINF_hvm_guest) ?
 LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PV;
 }
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index b96fb5c..608d55a 100644
---

[PATCH V7 0/2] Add handling of extended regions (safe ranges) on Arm (Was "xen/memory: Introduce a hypercall to provide unallocated space")

2021-10-14 Thread Oleksandr Tyshchenko

From: Oleksandr Tyshchenko 

You can find an initial discussion at [1]-[7].

The extended region (safe range) is a region of guest physical address space
which is unused and could be safely used to create grant/foreign mappings 
instead
of wasting real RAM pages from the domain memory for establishing these 
mappings.

The extended regions are chosen at the domain creation time and advertised
to it via "reg" property under hypervisor node in the guest device-tree
(the indexes for extended regions are 1...N).
No device tree bindings update is needed, guest infers the presense of extended
regions from the number of regions in "reg" property.
New compatible/property will be needed (but only after this patch [8] or 
alternative
goes in) to indicate that "region 0 is safe to use". Until this patch is merged 
it is
not safe to use extended regions for the grant table space.

The extended regions are calculated differently for direct mapped Dom0 (with 
and without
IOMMU) and non-direct mapped DomUs.

Please note the following limitations:
- The extended region feature is only supported for 64-bit domain currently.
- The ACPI case is not covered.

Please note that support for Dom0 was already committed, so these patches are 
remaining DomU bits.

Xen patch series is also available at [9]. The corresponding Linux patch series 
is at [10]
for now (last 4 patches).

Tested on Renesas Salvator-X board + H3 ES3.0 SoC (Arm64) with updated 
virtio-disk backend [11]
running in Dom0 (256MB RAM) and DomD (2GB RAM). In both cases the backend 
pre-maps DomU memory
which is 3GB. All foreign memory gets mapped into extended regions (so the 
amount of RAM in
the backend domain is not reduced). No issues were observed.

[1] 
https://lore.kernel.org/xen-devel/1627489110-25633-1-git-send-email-olekst...@gmail.com/
[2] 
https://lore.kernel.org/xen-devel/1631034578-12598-1-git-send-email-olekst...@gmail.com/
[3] 
https://lore.kernel.org/xen-devel/1631297924-8658-1-git-send-email-olekst...@gmail.com/
[4] 
https://lore.kernel.org/xen-devel/1632437334-12015-1-git-send-email-olekst...@gmail.com/
[5] 
https://lore.kernel.org/xen-devel/1632955927-27911-1-git-send-email-olekst...@gmail.com/
[6] 
https://lore.kernel.org/xen-devel/1633519346-3686-1-git-send-email-olekst...@gmail.com/
[7] 
https://lore.kernel.org/xen-devel/1633974539-7380-1-git-send-email-olekst...@gmail.com/
[8] 
https://lore.kernel.org/xen-devel/1632425551-18910-1-git-send-email-olekst...@gmail.com/
[9] https://github.com/otyshchenko1/xen/commits/map_opt_ml8
[10] https://github.com/otyshchenko1/linux/commits/map_opt_ml4
[11] https://github.com/otyshchenko1/virtio-disk/commits/map_opt_next

Oleksandr Tyshchenko (2):
  xen/arm: Introduce gpaddr_bits field to struct
xen_domctl_getdomaininfo
  libxl/arm: Add handling of extended regions for DomU

 tools/include/libxl.h|   8 +++
 tools/include/xenctrl.h  |   1 +
 tools/libs/ctrl/xc_domain.c  |   1 +
 tools/libs/light/libxl_arm.c | 106 +--
 tools/libs/light/libxl_domain.c  |   1 +
 tools/libs/light/libxl_types.idl |   1 +
 xen/arch/arm/domctl.c|   2 +
 xen/arch/x86/domctl.c|   1 +
 xen/common/domctl.c  |   4 +-
 xen/common/sysctl.c  |   2 +-
 xen/include/public/arch-arm.h|   5 ++
 xen/include/public/domctl.h  |   3 ++
 xen/include/public/sysctl.h  |   2 +-
 13 files changed, 128 insertions(+), 9 deletions(-)

-- 
2.7.4

Re: [PATCH] Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag"

Michal Orzel writes ("[PATCH] Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci 
flag""):
> This reverts commit 2075b410ee8087662c880213c3aca196fb7ade22.
> 
> During the discussion [1] that took place after
> the patch was merged it was agreed that it should
> be reverted to avoid introducing a bad interface.
> 
> Furthermore, the patch rejected usage of flag
> XEN_DOMCTL_CDF_vpci for x86 which is not true
> as it should be set for dom0 PVH.
> 
> Due to XEN_DOMCTL_CDF_vpmu being introduced after
> XEN_DOMCTL_CDF_vpci, modify its bit position
> from 8 to 7.
> 
> [1] https://marc.info/?t=163354215300039=1=2

FTAOD, I don't think this will be necessary, but preemptively,

Release-Acked-by: Ian Jackson

Re: [PATCH 2/2] memory: XENMEM_add_to_physmap (almost) wrapping checks


Hi Jan,

On 13/09/2021 07:42, Jan Beulich wrote:

Determining that behavior is correct (i.e. results in failure) for a
passed in GFN equaling INVALID_GFN is non-trivial. Make this quite a
bit more obvious by checking input in generic code - both for singular
requests to not match the value and for range ones to not pass / wrap
through it.

For Arm similarly make more obvious that no wrapping of MFNs passed
for XENMAPSPACE_dev_mmio and thus to map_dev_mmio_region() can occur:
Drop the "nr" parameter of the function to avoid future callers
appearing which might not themselves check for wrapping. Otherwise
the respective ASSERT() in rangeset_contains_range() could trigger.

Signed-off-by: Jan Beulich 
---
I find it odd that map_dev_mmio_region() returns success upon
iomem_access_permitted() indicating failure - is this really intended?


AFAIR yes. The hypercall is not used as "Map the region" but instead 
"Make sure the region is mapped if the IOMEM region is accessible".


It is necessary to return 0 because dom0 OS cannot distinguished between 
emulated and non-emulated. So we may report error when there is none.



As per commit 102984bb1987 introducing it this also was added for ACPI
only - any reason XENMAPSPACE_dev_mmio isn't restricted to CONFIG_ACPI
builds?


There is nothing specific to ACPI in the implementation. So I don't 
really see the reason to restrict to CONFIG_ACPI.


However, it is still possible to boot using DT when Xen is built with 
CONFIG_ACPI. So if the restriction was desirable, then I think it should 
be using !acpi_disabled.




I'd be happy to take suggestions towards avoiding the need to #define
_gfn() around the BUILD_BUG_ON() being added. I couldn't think of
anything prettier.

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1479,7 +1479,7 @@ int xenmem_add_to_physmap_one(
  break;
  }
  case XENMAPSPACE_dev_mmio:
-rc = map_dev_mmio_region(d, gfn, 1, _mfn(idx));
+rc = map_dev_mmio_region(d, gfn, _mfn(idx));
  return rc;
  
  default:

--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1360,19 +1360,18 @@ int unmap_mmio_regions(struct domain *d,
  
  int map_dev_mmio_region(struct domain *d,

  gfn_t gfn,
-unsigned long nr,
  mfn_t mfn)
  {
  int res;
  
-if ( !(nr && iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + nr - 1)) )

+if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn)) )
  return 0;
  
-res = p2m_insert_mapping(d, gfn, nr, mfn, p2m_mmio_direct_c);

+res = p2m_insert_mapping(d, gfn, 1, mfn, p2m_mmio_direct_c);
  if ( res < 0 )
  {
-printk(XENLOG_G_ERR "Unable to map MFNs [%#"PRI_mfn" - %#"PRI_mfn" in 
Dom%d\n",
-   mfn_x(mfn), mfn_x(mfn) + nr - 1, d->domain_id);
+printk(XENLOG_G_ERR "Unable to map MFN %#"PRI_mfn" in %pd\n",
+   mfn_x(mfn), d);
  return res;
  }
  
--- a/xen/common/grant_table.c

+++ b/xen/common/grant_table.c
@@ -4132,7 +4132,10 @@ int gnttab_map_frame(struct domain *d, u
  bool status = false;
  
  if ( gfn_eq(gfn, INVALID_GFN) )

+{
+ASSERT_UNREACHABLE();
  return -EINVAL;
+}
  
  grant_write_lock(gt);
  
--- a/xen/common/memory.c

+++ b/xen/common/memory.c
@@ -831,6 +831,9 @@ int xenmem_add_to_physmap(struct domain
  return -EACCES;
  }
  
+if ( gfn_eq(_gfn(xatp->gpfn), INVALID_GFN) )

+return -EINVAL;
+
  if ( xatp->space == XENMAPSPACE_gmfn_foreign )
  extra.foreign_domid = DOMID_INVALID;
  
@@ -841,6 +844,15 @@ int xenmem_add_to_physmap(struct domain

  if ( xatp->size < start )
  return -EILSEQ;
  
+if ( xatp->gpfn + xatp->size < xatp->gpfn ||

+ xatp->idx + xatp->size < xatp->idx )
+{
+#define _gfn(x) (x)


AFAICT, _gfn() will already be defined. So some compiler may complain 
because will be defined differently on debug build. However...



+BUILD_BUG_ON(INVALID_GFN + 1);


... I might be missing something... but why can't use gfn_x(INVALID_GFN) 
+ 1 here?


In fact, I am not entirely sure what's the purpose of this 
BUILD_BUG_ON(). Could you give more details?



+#undef _gfn
+return -EOVERFLOW;
+}
+
  xatp->idx += start;
  xatp->gpfn += start;
  xatp->size -= start;
@@ -961,6 +973,9 @@ static int xenmem_add_to_physmap_batch(s
 extent, 1)) )
  return -EFAULT;
  
+if ( gfn_eq(_gfn(gpfn), INVALID_GFN) )

+return -EINVAL;
+
  rc = xenmem_add_to_physmap_one(d, xatpb->space, extra,
 idx, _gfn(gpfn));
  
--- a/xen/include/asm-arm/p2m.h

+++ b/xen/include/asm-arm/p2m.h
@@ -297,7 +297,6 @@ int unmap_regions_p2mt(struct domain *d,
  
  int map_dev_mmio_region(struct domain *d,

  gfn_t gfn,
-unsigned long nr,

Re: [XEN PATCH v7 50/51] build: specify source tree in include/ for prerequisite

On 24.08.2021 12:50, Anthony PERARD wrote:
> When doing an out-of-tree build, and thus setting VPATH,
> GNU Make 0.81 on Ubuntu Trusty complains about Circular dependency of
> include/Makefile and include/xlat.lst and drop them. The build fails
> later due to headers malformed.

Doesn't this change need to come ahead of the one enabling out-of-tree
builds then? Also do you again mean 3.81?

Jan

Re: [PATCH] Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag"





On 14/10/2021 10:42, Christian Lindig wrote:



On 14 Oct 2021, at 10:33, Julien Grall > wrote:


Looking at the thread, we are only missing an ack for...


---
 tools/ocaml/libs/xc/xenctrl.ml  | 1 - > 
  tools/ocaml/libs/xc/xenctrl.mli | 1 -


Acked-by: Christian Lindig >


Committed. Thank you!

Cheers,


--
Julien Grall

Re: [XEN PATCH v7 49/51] build: adding out-of-tree support to the xen build

On 24.08.2021 12:50, Anthony PERARD wrote:
> This implement out-of-tree support, there's two ways to create an
> out-of-tree build tree (after that, `make` in that new directory
> works):
> make O=build
> mkdir build; cd build; make -f ../Makefile
> also works with an absolute path for both.
> 
> This implementation only works if the source tree is clean, as we use
> VPATH.
> 
> This patch copies most new code with handling out-of-tree build from
> Linux v5.12.
> 
> Signed-off-by: Anthony PERARD 
> ---
>  xen/Makefile| 173 +---
>  xen/Rules.mk|  11 ++-
>  xen/arch/arm/efi/Makefile   |   3 +
>  xen/arch/x86/arch.mk|   3 +
>  xen/arch/x86/boot/Makefile  |   5 +-
>  xen/arch/x86/efi/Makefile   |   3 +
>  xen/include/Makefile|   9 +-
>  xen/scripts/mkmakefile  |  17 
>  xen/test/livepatch/Makefile |   2 +
>  xen/xsm/flask/Makefile  |   3 +
>  xen/xsm/flask/ss/Makefile   |   3 +
>  11 files changed, 194 insertions(+), 38 deletions(-)
>  create mode 100755 xen/scripts/mkmakefile

Linux have done away with this script just recently; I don't think we
should introduce it.

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -1,3 +1,7 @@
> +# $(lastword,) for GNU Make older than 0.81

DYM 3.81?

> +lastword = $(word $(words $(1)),$(1))
> +this-makefile := $(call lastword,$(MAKEFILE_LIST))

Oh - is it documented somewhere that this would always be last?

> @@ -17,33 +21,18 @@ export XEN_BUILD_HOST ?= $(shell hostname)
>  PYTHON_INTERPRETER   := $(word 1,$(shell which python3 python python2 
> 2>/dev/null) python)
>  export PYTHON?= $(PYTHON_INTERPRETER)
>  
> -export XEN_ROOT := $(CURDIR)/..
> +$(if $(filter __%, $(MAKECMDGOALS)), \
> + $(error targets prefixed with '__' are only for internal use))
>  
> -srctree := .
> -objtree := .
> -export srctree objtree
> +# That's our default target when none is given on the command line
> +PHONY := __all
> +__all:
>  
>  # Do not use make's built-in rules and variables
>  MAKEFLAGS += -rR
>  
>  EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
>  
> -ARCH=$(XEN_TARGET_ARCH)
> -SRCARCH=$(shell echo $(ARCH) | \
> -  sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \
> -  -e s'/riscv.*/riscv/g')
> -export ARCH SRCARCH
> -
> -# Don't break if the build process wasn't called from the top level
> -# we need XEN_TARGET_ARCH to generate the proper config
> -include $(XEN_ROOT)/Config.mk
> -
> -# Set ARCH/SUBARCH appropriately.
> -export TARGET_SUBARCH  := $(XEN_TARGET_ARCH)
> -export TARGET_ARCH := $(shell echo $(XEN_TARGET_ARCH) | \
> -sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' 
> \
> --e s'/riscv.*/riscv/g')
> -
>  # Allow someone to change their config file
>  export KCONFIG_CONFIG ?= .config

Would it be possible to split off some of the pure movement of pieces,
to make it easier to see what (if anything) is actually changed at the
same time as getting moved?

> @@ -51,14 +40,9 @@ export CC CXX LD
>  
>  export TARGET := xen
>  
> -.PHONY: default
> -default: build
> -
>  .PHONY: dist
>  dist: install
>  
> -include scripts/Kbuild.include
> -
>  ifneq ($(root-make-done),y)
>  # section to run before calling Rules.mk, but only once.
>  
> @@ -130,14 +114,93 @@ endif
>  
>  ifneq ($(findstring s,$(filter-out --%,$(MAKEFLAGS))),)
>  quiet := silent_
> +KBUILD_VERBOSE = 0
>  endif

In how far is this related here? Doesn't this belong in an earlier patch?

>  export quiet Q KBUILD_VERBOSE
>  
> +# $(realpath,) for GNU Make older than 0.81

Again - 3.81?

> +ifeq ($(abs_srctree),$(abs_objtree))
> +# building in the source tree
> +srctree := .
> + building_out_of_srctree :=
> +else
> +ifeq ($(abs_srctree)/,$(dir $(abs_objtree)))
> +# building in a subdirectory of the source tree
> +srctree := ..
> +else
> +srctree := $(abs_srctree)
> +endif
> + building_out_of_srctree := 1
> +endif
> +
> +objtree  := .
> +VPATH:= $(srctree)

Would you mind using blank padding here, and perhaps fewer blanks than
the tabs are worth?

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -38,7 +38,7 @@ SPECIAL_DATA_SECTIONS := rodata $(foreach a,1 2 4 8 16, \
>   $(foreach r,rel rel.ro,data.$(r).local)
>  
>  # The filename build.mk has precedence over Makefile
> -mk-dir := $(src)
> +mk-dir := $(srctree)/$(src)
>  include $(if $(wildcard 
> $(mk-dir)/build.mk),$(mk-dir)/build.mk,$(mk-dir)/Makefile)

Perhaps already when it was changed to $(src) the name has become
slightly misleading, at least imo: I would rather expect a variable
with this name to refer to the build dir/tree. Maybe "srcdir" or
even shorted "sd" right from the start? (Reaching here I can finally
see why having a shorthand is helpful.)

> --- a/xen/arch/x86/efi/Makefile
> +++ b/xen/arch/x86/efi/Makefile

Re: [XEN PATCH v7 48/51] build: Rework "headers*.chk" prerequisite in include/

On 24.08.2021 12:50, Anthony PERARD wrote:
> Listing public headers when out-of-tree build are involved becomes
> more annoying where every path to every headers needs to start with
> "$(srctree)/$(src)", or $(wildcard ) will not work. This means more
> repetition.
> 
> This patch attempt to reduce the amount of duplication and make better
> use of make's meta programming capability. The filters are now listed
> in a variable and don't have to repeat the path to the headers files
> as this is added later as needed.
> 
> Signed-off-by: Anthony PERARD 

Sorry, just one nit here:

> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -78,10 +78,21 @@ ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
>  
>  all: $(obj)/headers.chk $(obj)/headers99.chk $(obj)/headers++.chk
>  
> -PUBLIC_HEADERS := $(filter-out $(src)/public/arch-% 
> $(src)/public/dom0_ops.h, $(wildcard $(src)/public/*.h $(src)/public/*/*.h))
> +hdrs-path := $(srctree)/$(src)/public
>  
> -PUBLIC_C99_HEADERS := $(src)/public/io/9pfs.h $(src)/public/io/pvcalls.h
> -PUBLIC_ANSI_HEADERS := $(filter-out $(src)/public/%ctl.h $(src)/public/xsm/% 
> $(src)/public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))

These all had / have "PUBLIC" in their names because the makefile doesn't
live in public/. I'd prefer if you could stick to this for all the new
variables/macros you add (lower case then of course).

Jan

> +list-headers = $(wildcard $1/*.h $1/*/*.h)
> +filter-headers = $(filter-out $(addprefix $(hdrs-path)/,$($1-filter)), $($1))
> +
> +c99-headers := io/9pfs.h io/pvcalls.h
> +public-headers := $(call list-headers,$(hdrs-path))
> +ansi-headers := $(public-headers)
> +
> +public-headers-filter := dom0_ops.h arch-%
> +ansi-headers-filter := %ctl.h xsm/% %hvm/save.h $(public-headers-filter) 
> $(c99-headers)
> +
> +PUBLIC_HEADERS := $(call filter-headers,public-headers)
> +PUBLIC_ANSI_HEADERS := $(call filter-headers,ansi-headers)
> +PUBLIC_C99_HEADERS := $(addprefix $(hdrs-path)/, $(c99-headers))
>  
>  $(src)/public/io/9pfs.h-prereq := string
>  $(src)/public/io/pvcalls.h-prereq := string
>

Re: [XEN PATCH v7 47/51] build: Rework "clean" to clean from the root dir

On 24.08.2021 12:50, Anthony PERARD wrote:
> This will allow "clean" to work from an out-of-tree build when
> it will be available.
> 
> Some of the file been removed in current "clean" target aren't added
> to $(clean-files) because they are already listed in $(extra-) or
> $(extra-y).
> 
> Also clean files in "arch/x86/boot" from that directory by allowing
> "clean" to descend into the subdir by adding "boot" into $(subdir-).

"descend into" (also used in a respective comment) looks contrary to
doing everything from the root now, at least to me.

> --- a/xen/scripts/Kbuild.include
> +++ b/xen/scripts/Kbuild.include
> @@ -98,7 +98,7 @@ build := -f $(srctree)/Rules.mk obj
>  # Shorthand for $(MAKE) clean
>  # Usage:
>  # $(MAKE) $(clean) dir
> -clean := -f $(abs_srctree)/scripts/Makefile.clean clean -C
> +clean := -f $(srctree)/scripts/Makefile.clean obj

Doesn't the comment want changing as well?

Jan

Re: [PATCH] Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag"

2021-10-14 Thread Christian Lindig



On 14 Oct 2021, at 10:33, Julien Grall mailto:jul...@xen.org>> 
wrote:

Looking at the thread, we are only missing an ack for...

---
 tools/ocaml/libs/xc/xenctrl.ml  | 1 - >   tools/ocaml/libs/xc/xenctrl.mli | 1 -

Acked-by: Christian Lindig 
mailto:christian.lin...@citrix.com>>

Re: [XEN PATCH v7 46/51] build: replace $(BASEDIR) by $(srctree)

On 24.08.2021 12:50, Anthony PERARD wrote:
> $(srctree) is a better description for the source directory than
> $(BASEDIR) that has been used for both source and build directory
> (which where the same).
> 
> "clean" is still changing directory, so we need to use absolute path
> for it.
> 
> Signed-off-by: Anthony PERARD 

Acked-by: Jan Beulich 

I'm curious though why this was split from patch 44. It doesn't look
to me as if patch 45 would have changed anything that's related here.

Jan

[qemu-mainline test] 165498: tolerable FAIL - PUSHED

flight 165498 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/165498/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 165488
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 165488
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 165488
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 165488
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 165488
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 165488
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 165488
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 165488
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass

version targeted for testing:
 qemuu946de558354c99e1989621abe053f2ab87dc8de9
baseline version:
 qemuuee26ce674a93c824713542cec3b6a9ca85459165

Last test of basis   165488  2021-10-13 02:58:54 Z1 days
Testing same since   165498  2021-10-13 17:39:31 Z0 days1 attempts


People who touched revisions under test:
  Alexander Graf 
  Cole Robinson 
  David

Re: [PATCH] Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag"


Hi Michal,

On 14/10/2021 09:47, Michal Orzel wrote:

This reverts commit 2075b410ee8087662c880213c3aca196fb7ade22.

During the discussion [1] that took place after
the patch was merged it was agreed that it should
be reverted to avoid introducing a bad interface.

Furthermore, the patch rejected usage of flag
XEN_DOMCTL_CDF_vpci for x86 which is not true
as it should be set for dom0 PVH.

Due to XEN_DOMCTL_CDF_vpmu being introduced after
XEN_DOMCTL_CDF_vpci, modify its bit position
from 8 to 7.

[1] https://marc.info/?t=163354215300039=1=2

Signed-off-by: Michal Orzel 


Acked-by: Julien Grall 

Looking at the thread, we are only missing an ack for...


---
  tools/ocaml/libs/xc/xenctrl.ml  | 1 - >   tools/ocaml/libs/xc/xenctrl.mli | 1 
-


the OCAML part. I can commit it once this is done.

Cheers,

--
Julien Grall

Re: [XEN PATCH v7 45/51] build: rework cloc recipe

On 24.08.2021 12:50, Anthony PERARD wrote:
> After folowing patches, the recipe doesn't work anymore.
> - build: build everything from the root dir, use obj=$subdir
> - build: introduce if_changed_deps

That was some 20 patches ago - shouldn't all make goals continue to
work at every step?

> First patch mean that $(names) already have $(path), and the second
> one, the .*.d files are replaced by .*.cmd files which are much
> simpler to parse here.
> 
> Also replace the makefile programming by a much simpler shell command.
> 
> This doesn't check anymore if the source file exist, but that can be
> fixed by running `make clean`, and probably doesn't impact the
> calculation. `cloc` just complain that some files don't exist.

Not sure whether that's acceptable - Stefano, iirc it was you who
introduced this goal.

Jan

Re: [PATCH v2] xen/arm: vgic to ignore GICD ICPENRn registers access


On 14/10/2021 07:55, Hongda Deng wrote:

Hi,

Hi,


-Original Message-
From: Julien Grall 
Sent: 2021年10月13日 5:58
To: Hongda Deng ; xen-devel@lists.xenproject.org;
sstabell...@kernel.org
Cc: Bertrand Marquis ; Wei Chen

Subject: Re: [PATCH v2] xen/arm: vgic to ignore GICD ICPENRn registers access

Hi,

On 12/10/2021 07:24, Hongda Deng wrote:

Currently, Xen will return IO unhandled when guests access GICD ICPENRn
registers. This will raise a data abort inside guest. For Linux Guest,
these virtual registers will not be accessed. But for Zephyr, in its
GIC initialization code, these virtual registers will be accessed. And
zephyr guest will get an IO data abort in initilization stage and enter
fatal error. Emulating ICPENDR is not easy with the existing vGIC, so
we currently ignore these virtual registers access and print a message
about whether they are already pending instead of returning unhandled.
More details can be found at [1].


The link you provide only states that I am happy with the warning. This
doesn't seem relevant for a future reader. Did you intend to point to
something different?



Yes, I would attach this link [1] then, which explains how zephyr accesses
ICPENDR at its initialization. ( Though I still don't understand why zephyr
would clear this register at initialization while linux wouldn't )


I am confused as well. From my understanding, clearing ICPENDR at 
initialization is pointless for level interrupts if you didn't quiesce 
the device beforehand.


The git history doesn't seem to give much details on the reason. But 
looking at the code, I am wondering if the intention was to clear the 
active bit rather than the pending bit.






[1] https://lists.xenproject.org/archives/html/xen-devel/2021-09/
msg00744.html

Signed-off-by: Hongda Deng 
---
   xen/arch/arm/vgic-v2.c | 26 +-
   xen/arch/arm/vgic-v3.c | 40 +++-
   2 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index b2da886adc..d7ffaeeb65 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -480,11 +480,35 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,

mmio_info_t *info,

   return 1;

   case VRANGE32(GICD_ICPENDR, GICD_ICPENDRN):
+{
+struct pending_irq *iter;
+unsigned int irq_start;
+unsigned int irq_end;
+uint32_t irq_pending = 0;
+
   if ( dabt.size != DABT_WORD ) goto bad_width;
   printk(XENLOG_G_ERR
  "%pv: vGICD: unhandled word write %#"PRIregister" to

ICPENDR%d\n",

  v, r, gicd_reg - GICD_ICPENDR);


As I wrote in v1, we should avoid to print a message when we know there
is no pending interrupts.



These are not the modifications made in this patch, and same codes also exist
for ICACTIVER. I didn't delete them for that I think they are used to give some
useful and coherent messages to user for reference. Should we delete it for both
or only for ICPENDR?


Looking at the implementation ICACTIVER, we simply ignore the write so 
it makes sense to print a message everytime.


This is quite different to the implementation of ICPENDR as we will 
partially emulate it. We technically emulated the register correctly 
when there is no pending interrupts, so I think it is wrong to print a 
message state this wasn't handled properly.


Therefore, I would like this message to only appear when we know the 
write wasn't handled properly.



-return 0;
+
+irq_start = (gicd_reg - GICD_ICPENDR) * 32;
+irq_end = irq_start + 31;
+/* go through inflight_irqs and print specified pending irqs */
+list_for_each_entry(iter, >arch.vgic.inflight_irqs, inflight)

You need to hold v->arch.vgic.lock (with interrupt disabled) to go
through the list of inflight irqs. Otherwise, the list may be modified
while you are walking it.



Ack.


However, I am a little bit concerned with this approached (I noticed
Stefano suggested). The list may in theory contains a few hundreds
interrupts (a malicious OS May decide to never read IAR). So we are
potentially doing more work than necessary (we need to think about the
worse case scenario).

Instead, I think it would be better to go through the 32 interrupts and
for each of them:
1) find the pending_irq() using irq_to_pending()
2) check if the IRQ in the inflight list with list_empty(>inflight)

In addition to that, you want to check that the rank exists so we don't
do any extra work if the guest is trying to clear an interrupts above
the number of interrupts we support.



Agreed, and that's quite helpful.


I forgot to mention that you may need to be careful with the locking. If 
I am not mistaken, "inflight" is protected with the arch.vgic.lock of 
vgic_get_target_vcpu();



+{
+if ( iter->irq < irq_start || irq_end < iter->irq )
+continue;
+
+if (

Re: [XEN PATCH v7 44/51] build: add $(srctree) in few key places

On 24.08.2021 12:50, Anthony PERARD wrote:
> This adds $(srctree) to a few path where make's VPATH=$(srctree) won't
> apply.
> 
> Signed-off-by: Anthony PERARD 

Acked-by: Jan Beulich

Re: [PATCH v5 08/11] xen/arm: Enable the existing x86 virtual PCI support for ARM.

On 14.10.2021 11:03, Bertrand Marquis wrote:
> Hi Jan,
> 
>> On 14 Oct 2021, at 07:33, Jan Beulich  wrote:
>>
>> On 13.10.2021 21:27, Stefano Stabellini wrote:
>>> On Wed, 13 Oct 2021, Jan Beulich wrote:
 On 13.10.2021 16:51, Oleksandr Andrushchenko wrote:
> On 13.10.21 16:00, Jan Beulich wrote:
>> On 13.10.2021 10:45, Roger Pau Monné wrote:
>>> On Wed, Oct 06, 2021 at 06:40:34PM +0100, Rahul Singh wrote:
 --- /dev/null
 +++ b/xen/arch/arm/vpci.c
 @@ -0,0 +1,102 @@
 +/*
 + * xen/arch/arm/vpci.c
 + *
 + * This program is free software; you can redistribute it and/or 
 modify
 + * it under the terms of the GNU General Public License as published 
 by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +#include 
 +
 +#include 
 +
 +#define REGISTER_OFFSET(addr)  ( (addr) & 0x0fff)
 +
 +/* Do some sanity checks. */
 +static bool vpci_mmio_access_allowed(unsigned int reg, unsigned int 
 len)
 +{
 +/* Check access size. */
 +if ( len > 8 )
 +return false;
 +
 +/* Check that access is size aligned. */
 +if ( (reg & (len - 1)) )
 +return false;
 +
 +return true;
 +}
 +
 +static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
 +  register_t *r, void *p)
 +{
 +unsigned int reg;
 +pci_sbdf_t sbdf;
 +unsigned long data = ~0UL;
 +unsigned int size = 1U << info->dabt.size;
 +
 +sbdf.sbdf = MMCFG_BDF(info->gpa);
 +reg = REGISTER_OFFSET(info->gpa);
 +
 +if ( !vpci_mmio_access_allowed(reg, size) )
 +return 0;
 +
 +data = vpci_read(sbdf, reg, min(4u, size));
 +if ( size == 8 )
 +data |= (uint64_t)vpci_read(sbdf, reg + 4, 4) << 32;
 +
 +*r = data;
 +
 +return 1;
 +}
 +
 +static int vpci_mmio_write(struct vcpu *v, mmio_info_t *info,
 +   register_t r, void *p)
 +{
 +unsigned int reg;
 +pci_sbdf_t sbdf;
 +unsigned long data = r;
 +unsigned int size = 1U << info->dabt.size;
 +
 +sbdf.sbdf = MMCFG_BDF(info->gpa);
 +reg = REGISTER_OFFSET(info->gpa);
 +
 +if ( !vpci_mmio_access_allowed(reg, size) )
 +return 0;
 +
 +vpci_write(sbdf, reg, min(4u, size), data);
 +if ( size == 8 )
 +vpci_write(sbdf, reg + 4, 4, data >> 32);
>>> I think those two helpers (and vpci_mmio_access_allowed) are very
>>> similar to the existing x86 ones (see vpci_mmcfg_{read,write}), up to
>>> the point where I would consider moving the shared code to vpci.c as
>>> vpci_ecam_{read,write} and call them from the arch specific trap
>>> handlers.
>> Except that please can we stick to mcfg or mmcfg instead of ecam
>> in names, as that's how the thing has been named in Xen from its
>> introduction? I've just grep-ed the code base (case insensitively)
>> and found no mention of ECAM. There are only a few "became".
> I do understand that this is historically that we do not have ECAM in Xen,
> but PCI is not about Xen. Thus, I think it is also acceptable to use
> a commonly known ECAM for the code that works with ECAM.

 ACPI, afaik, also doesn't call this ECAM. That's where MCFG / MMCFG
 actually come from, I believe.
>>>
>>> My understanding is that "MCFG" is the name of the ACPI table that
>>> describes the PCI config space [1]. The underlying PCI standard for the
>>> memory mapped layout of the PCI config space is called ECAM. Here, it
>>> makes sense to call it ECAM as it is firmware independent.
>>>
>>> [1] https://wiki.osdev.org/PCI_Express
>>
>> Okay, I can accept this, but then I'd expect all existing uses of
>> MCFG / MMCFG in the code which aren't directly ACPI-related to get
>> replaced. Otherwise it's needlessly harder to identify that one
>> piece of code relates to certain other pieces.
> 
> If that is ok with I will:
> - move function from x86/hw/io.c to vpci.c renaming them to ECAM
> - rename MMCFG_{BDF/REG_OFFSET) to ECAM_{BDF/REG_OFFSET}
> 
> For the rest of the occurrences in x86 I will not modify any of them as some
> are related

Re: [XEN PATCH v7 43/51] build: replace $(BASEDIR) by $(objtree)

On 24.08.2021 12:50, Anthony PERARD wrote:
> We need to differentiate between source files and generated/built
> files. We will be replacing $(BASEDIR) by $(objtree) for files that
> are generated, and $(abs_objtree) in cases where an absolute path is
> necessary.
> 
> The "clean" target is still changing to the subdir been cleaned, to
> remove file in the root we need to use $(abs_objtree).

Vaguely related more general question: How useful is "clean" for an
out-of-tree build? That ought to effectively remove the entire build
tree, which may not be overly sensible to do via "make clean-xen",
but instead simply "rm -rf ...".

> @@ -117,4 +117,4 @@ $(obj)/dtb.o: $(patsubst "%",%,$(CONFIG_DTB_FILE))
>  .PHONY: clean
>  clean::
>   rm -f $(obj)/xen.lds
> - rm -f $(BASEDIR)/.xen-syms.[0-9]*
> + rm -f $(abs_objtree)/.xen-syms.[0-9]*

This part is common - would it make sense to move to xen/Makefile, thus
- aiui - eliminating the need for using $(abs_objtree) here / there?

Jan

Re: [PATCH v5 08/11] xen/arm: Enable the existing x86 virtual PCI support for ARM.

Hi Jan,

> On 14 Oct 2021, at 07:33, Jan Beulich  wrote:
> 
> On 13.10.2021 21:27, Stefano Stabellini wrote:
>> On Wed, 13 Oct 2021, Jan Beulich wrote:
>>> On 13.10.2021 16:51, Oleksandr Andrushchenko wrote:
 On 13.10.21 16:00, Jan Beulich wrote:
> On 13.10.2021 10:45, Roger Pau Monné wrote:
>> On Wed, Oct 06, 2021 at 06:40:34PM +0100, Rahul Singh wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/arm/vpci.c
>>> @@ -0,0 +1,102 @@
>>> +/*
>>> + * xen/arch/arm/vpci.c
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +#include 
>>> +
>>> +#include 
>>> +
>>> +#define REGISTER_OFFSET(addr)  ( (addr) & 0x0fff)
>>> +
>>> +/* Do some sanity checks. */
>>> +static bool vpci_mmio_access_allowed(unsigned int reg, unsigned int 
>>> len)
>>> +{
>>> +/* Check access size. */
>>> +if ( len > 8 )
>>> +return false;
>>> +
>>> +/* Check that access is size aligned. */
>>> +if ( (reg & (len - 1)) )
>>> +return false;
>>> +
>>> +return true;
>>> +}
>>> +
>>> +static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
>>> +  register_t *r, void *p)
>>> +{
>>> +unsigned int reg;
>>> +pci_sbdf_t sbdf;
>>> +unsigned long data = ~0UL;
>>> +unsigned int size = 1U << info->dabt.size;
>>> +
>>> +sbdf.sbdf = MMCFG_BDF(info->gpa);
>>> +reg = REGISTER_OFFSET(info->gpa);
>>> +
>>> +if ( !vpci_mmio_access_allowed(reg, size) )
>>> +return 0;
>>> +
>>> +data = vpci_read(sbdf, reg, min(4u, size));
>>> +if ( size == 8 )
>>> +data |= (uint64_t)vpci_read(sbdf, reg + 4, 4) << 32;
>>> +
>>> +*r = data;
>>> +
>>> +return 1;
>>> +}
>>> +
>>> +static int vpci_mmio_write(struct vcpu *v, mmio_info_t *info,
>>> +   register_t r, void *p)
>>> +{
>>> +unsigned int reg;
>>> +pci_sbdf_t sbdf;
>>> +unsigned long data = r;
>>> +unsigned int size = 1U << info->dabt.size;
>>> +
>>> +sbdf.sbdf = MMCFG_BDF(info->gpa);
>>> +reg = REGISTER_OFFSET(info->gpa);
>>> +
>>> +if ( !vpci_mmio_access_allowed(reg, size) )
>>> +return 0;
>>> +
>>> +vpci_write(sbdf, reg, min(4u, size), data);
>>> +if ( size == 8 )
>>> +vpci_write(sbdf, reg + 4, 4, data >> 32);
>> I think those two helpers (and vpci_mmio_access_allowed) are very
>> similar to the existing x86 ones (see vpci_mmcfg_{read,write}), up to
>> the point where I would consider moving the shared code to vpci.c as
>> vpci_ecam_{read,write} and call them from the arch specific trap
>> handlers.
> Except that please can we stick to mcfg or mmcfg instead of ecam
> in names, as that's how the thing has been named in Xen from its
> introduction? I've just grep-ed the code base (case insensitively)
> and found no mention of ECAM. There are only a few "became".
 I do understand that this is historically that we do not have ECAM in Xen,
 but PCI is not about Xen. Thus, I think it is also acceptable to use
 a commonly known ECAM for the code that works with ECAM.
>>> 
>>> ACPI, afaik, also doesn't call this ECAM. That's where MCFG / MMCFG
>>> actually come from, I believe.
>> 
>> My understanding is that "MCFG" is the name of the ACPI table that
>> describes the PCI config space [1]. The underlying PCI standard for the
>> memory mapped layout of the PCI config space is called ECAM. Here, it
>> makes sense to call it ECAM as it is firmware independent.
>> 
>> [1] https://wiki.osdev.org/PCI_Express
> 
> Okay, I can accept this, but then I'd expect all existing uses of
> MCFG / MMCFG in the code which aren't directly ACPI-related to get
> replaced. Otherwise it's needlessly harder to identify that one
> piece of code relates to certain other pieces.

If that is ok with I will:
- move function from x86/hw/io.c to vpci.c renaming them to ECAM
- rename MMCFG_{BDF/REG_OFFSET) to ECAM_{BDF/REG_OFFSET}

For the rest of the occurrences in x86 I will not modify any of them as some
are related to ACPI so using (M)MCFG is right and for the others I am not 100%
sure.

Do you agree ?

Cheers
Bertrand

> 
> Jan
>

Re: [PATCH] Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag"

Hi Michal,

> On 14 Oct 2021, at 09:47, Michal Orzel  wrote:
> 
> This reverts commit 2075b410ee8087662c880213c3aca196fb7ade22.
> 
> During the discussion [1] that took place after
> the patch was merged it was agreed that it should
> be reverted to avoid introducing a bad interface.
> 
> Furthermore, the patch rejected usage of flag
> XEN_DOMCTL_CDF_vpci for x86 which is not true
> as it should be set for dom0 PVH.
> 
> Due to XEN_DOMCTL_CDF_vpmu being introduced after
> XEN_DOMCTL_CDF_vpci, modify its bit position
> from 8 to 7.
> 
> [1] https://marc.info/?t=163354215300039=1=2
> 
> Signed-off-by: Michal Orzel 
Reviewed-by: Bertrand Marquis 

Cheers
Bertrand

> ---
> tools/ocaml/libs/xc/xenctrl.ml  | 1 -
> tools/ocaml/libs/xc/xenctrl.mli | 1 -
> xen/arch/arm/domain.c   | 3 +--
> xen/arch/x86/domain.c   | 6 --
> xen/common/domain.c | 3 +--
> xen/include/public/domctl.h | 3 +--
> 6 files changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
> index 86758babb3..addcf4cc59 100644
> --- a/tools/ocaml/libs/xc/xenctrl.ml
> +++ b/tools/ocaml/libs/xc/xenctrl.ml
> @@ -69,7 +69,6 @@ type domain_create_flag =
>   | CDF_XS_DOMAIN
>   | CDF_IOMMU
>   | CDF_NESTED_VIRT
> - | CDF_VPCI
>   | CDF_VPMU
> 
> type domain_create_iommu_opts =
> diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
> index 0fdb0cc169..0a5ce529e9 100644
> --- a/tools/ocaml/libs/xc/xenctrl.mli
> +++ b/tools/ocaml/libs/xc/xenctrl.mli
> @@ -62,7 +62,6 @@ type domain_create_flag =
>   | CDF_XS_DOMAIN
>   | CDF_IOMMU
>   | CDF_NESTED_VIRT
> -  | CDF_VPCI
>   | CDF_VPMU
> 
> type domain_create_iommu_opts =
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index ad21c9b950..eef0661beb 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -628,8 +628,7 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
> {
> unsigned int max_vcpus;
> unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
> -unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | 
> XEN_DOMCTL_CDF_vpci |
> -   XEN_DOMCTL_CDF_vpmu);
> +unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | 
> XEN_DOMCTL_CDF_vpmu);
> 
> if ( (config->flags & ~flags_optional) != flags_required )
> {
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 79c2aa4636..ef1812dc14 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -662,12 +662,6 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
> return -EINVAL;
> }
> 
> -if ( config->flags & XEN_DOMCTL_CDF_vpci )
> -{
> -dprintk(XENLOG_INFO, "vPCI cannot be enabled yet\n");
> -return -EINVAL;
> -}
> -
> if ( config->vmtrace_size )
> {
> unsigned int size = config->vmtrace_size;
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 8543fb54fd..8b53c49d1e 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -486,8 +486,7 @@ static int sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>  ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
>XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
>XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
> -   XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpci |
> -   XEN_DOMCTL_CDF_vpmu) )
> +   XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu) )
> {
> dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
> return -EINVAL;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index a53cbd16f4..238384b5ae 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -70,9 +70,8 @@ struct xen_domctl_createdomain {
> #define XEN_DOMCTL_CDF_iommu  (1U<<_XEN_DOMCTL_CDF_iommu)
> #define _XEN_DOMCTL_CDF_nested_virt   6
> #define XEN_DOMCTL_CDF_nested_virt(1U << _XEN_DOMCTL_CDF_nested_virt)
> -#define XEN_DOMCTL_CDF_vpci   (1U << 7)
> /* Should we expose the vPMU to the guest? */
> -#define XEN_DOMCTL_CDF_vpmu   (1U << 8)
> +#define XEN_DOMCTL_CDF_vpmu   (1U << 7)
> 
> /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
> #define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpmu
> -- 
> 2.29.0
> 
>

Re: [PATCH] Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag"

On 14.10.2021 10:47, Michal Orzel wrote:
> This reverts commit 2075b410ee8087662c880213c3aca196fb7ade22.
> 
> During the discussion [1] that took place after
> the patch was merged it was agreed that it should
> be reverted to avoid introducing a bad interface.
> 
> Furthermore, the patch rejected usage of flag
> XEN_DOMCTL_CDF_vpci for x86 which is not true
> as it should be set for dom0 PVH.
> 
> Due to XEN_DOMCTL_CDF_vpmu being introduced after
> XEN_DOMCTL_CDF_vpci, modify its bit position
> from 8 to 7.
> 
> [1] https://marc.info/?t=163354215300039=1=2
> 
> Signed-off-by: Michal Orzel 
> ---
>  tools/ocaml/libs/xc/xenctrl.ml  | 1 -
>  tools/ocaml/libs/xc/xenctrl.mli | 1 -
>  xen/arch/arm/domain.c   | 3 +--
>  xen/arch/x86/domain.c   | 6 --
>  xen/common/domain.c | 3 +--
>  xen/include/public/domctl.h | 3 +--
>  6 files changed, 3 insertions(+), 14 deletions(-)

Applicable parts
Acked-by: Jan Beulich 

Jan

Re: [XEN PATCH v7 42/51] build: grab common EFI source files in arch specific dir

On 24.08.2021 12:50, Anthony PERARD wrote:
> Rather than preparing the efi source file, we will copy them as needed
> from the build location.
> 
> Avoid the links as they seems fragile in out-of-tree builds. Also by
> making a copy, we don't need to figure out the relative path or we
> don't need to use absolute path.

I agree that symlinks wouldn't be nice for the out-of-tree build case.
Otoh please see
https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=8c6740616cd244e5763e974cb737affbe71db385
albeit I'll admit the situation was a little different there because
it's pre-built files which get populated into the build tree.

> --- a/xen/arch/arm/efi/Makefile
> +++ b/xen/arch/arm/efi/Makefile
> @@ -1,4 +1,10 @@
>  CFLAGS-y += -fshort-wchar
> +CFLAGS-y += -I$(srctree)/common/efi

Perhaps another opportunity for -iquote?

>  obj-y += boot.init.o pe.init.o ebmalloc.o runtime.o
>  obj-$(CONFIG_ACPI) +=  efi-dom0.init.o
> +
> +$(obj)/%.c: common/efi/%.c
> + $(Q)cp -f $< $@

In case both trees are on the same file system, trying to hardlink first
would seem desirable. When copying, I think you should also pass -p.

> --- a/xen/arch/x86/efi/Makefile
> +++ b/xen/arch/x86/efi/Makefile
> @@ -1,4 +1,5 @@
>  CFLAGS-y += -fshort-wchar
> +CFLAGS-y += -I$(srctree)/common/efi
>  
>  quiet_cmd_objcopy_o_ihex = OBJCOPY $@
>  cmd_objcopy_o_ihex = $(OBJCOPY) -I ihex -O binary $< $@
> @@ -19,3 +20,8 @@ obj-$(XEN_BUILD_EFI) := $(filter-out %.init.o,$(EFIOBJ-y))
>  obj-bin-$(XEN_BUILD_EFI) := $(filter %.init.o,$(EFIOBJ-y))
>  extra-$(XEN_BUILD_EFI) += buildid.o relocs-dummy.o
>  nocov-$(XEN_BUILD_EFI) += stub.o
> +
> +$(obj)/%.c: common/efi/%.c
> + $(Q)cp -f $< $@
> +
> +.PRECIOUS: $(obj)/%.c

Seeing you repeat everything here, despite it not being all this much I
wonder if there wouldn't better be a makefile fragment in common/efi/
which all interested architectures' arch//efi/Makefile would then
include. This could then also subsume -fshort-wchar.

Jan

[PATCH] Revert "xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag"

2021-10-14 Thread Michal Orzel

This reverts commit 2075b410ee8087662c880213c3aca196fb7ade22.

During the discussion [1] that took place after
the patch was merged it was agreed that it should
be reverted to avoid introducing a bad interface.

Furthermore, the patch rejected usage of flag
XEN_DOMCTL_CDF_vpci for x86 which is not true
as it should be set for dom0 PVH.

Due to XEN_DOMCTL_CDF_vpmu being introduced after
XEN_DOMCTL_CDF_vpci, modify its bit position
from 8 to 7.

[1] https://marc.info/?t=163354215300039=1=2

Signed-off-by: Michal Orzel 
---
 tools/ocaml/libs/xc/xenctrl.ml  | 1 -
 tools/ocaml/libs/xc/xenctrl.mli | 1 -
 xen/arch/arm/domain.c   | 3 +--
 xen/arch/x86/domain.c   | 6 --
 xen/common/domain.c | 3 +--
 xen/include/public/domctl.h | 3 +--
 6 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 86758babb3..addcf4cc59 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -69,7 +69,6 @@ type domain_create_flag =
| CDF_XS_DOMAIN
| CDF_IOMMU
| CDF_NESTED_VIRT
-   | CDF_VPCI
| CDF_VPMU
 
 type domain_create_iommu_opts =
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 0fdb0cc169..0a5ce529e9 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -62,7 +62,6 @@ type domain_create_flag =
   | CDF_XS_DOMAIN
   | CDF_IOMMU
   | CDF_NESTED_VIRT
-  | CDF_VPCI
   | CDF_VPMU
 
 type domain_create_iommu_opts =
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index ad21c9b950..eef0661beb 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -628,8 +628,7 @@ int arch_sanitise_domain_config(struct 
xen_domctl_createdomain *config)
 {
 unsigned int max_vcpus;
 unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
-unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpci |
-   XEN_DOMCTL_CDF_vpmu);
+unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | XEN_DOMCTL_CDF_vpmu);
 
 if ( (config->flags & ~flags_optional) != flags_required )
 {
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 79c2aa4636..ef1812dc14 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -662,12 +662,6 @@ int arch_sanitise_domain_config(struct 
xen_domctl_createdomain *config)
 return -EINVAL;
 }
 
-if ( config->flags & XEN_DOMCTL_CDF_vpci )
-{
-dprintk(XENLOG_INFO, "vPCI cannot be enabled yet\n");
-return -EINVAL;
-}
-
 if ( config->vmtrace_size )
 {
 unsigned int size = config->vmtrace_size;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 8543fb54fd..8b53c49d1e 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -486,8 +486,7 @@ static int sanitise_domain_config(struct 
xen_domctl_createdomain *config)
  ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
-   XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpci |
-   XEN_DOMCTL_CDF_vpmu) )
+   XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu) )
 {
 dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
 return -EINVAL;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index a53cbd16f4..238384b5ae 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -70,9 +70,8 @@ struct xen_domctl_createdomain {
 #define XEN_DOMCTL_CDF_iommu  (1U<<_XEN_DOMCTL_CDF_iommu)
 #define _XEN_DOMCTL_CDF_nested_virt   6
 #define XEN_DOMCTL_CDF_nested_virt(1U << _XEN_DOMCTL_CDF_nested_virt)
-#define XEN_DOMCTL_CDF_vpci   (1U << 7)
 /* Should we expose the vPMU to the guest? */
-#define XEN_DOMCTL_CDF_vpmu   (1U << 8)
+#define XEN_DOMCTL_CDF_vpmu   (1U << 7)
 
 /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
 #define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpmu
-- 
2.29.0

Re: Xen Booting Problem on ARM Machine

2021-10-14 Thread Sai Kiran Kumar Reddy Y

On Thu, Oct 14, 2021 at 5:45 AM Stefano Stabellini 
wrote:

> On Wed, 13 Oct 2021, Sai Kiran Kumar Reddy Y wrote:
> > On Fri, Oct 1, 2021 at 8:17 AM Stefano Stabellini <
> sstabell...@kernel.org> wrote:
> >   Yes there are other ways but without serial is going to be
> difficult
> >   because you are not going to see anything until everything works.
> >
> >   How do you boot Xen and Dom0 exactly from EDK2? Are you using GRUB
> or
> >   loading Xen directly from the EDK2 prompt? Please provide as many
> >   details as possible so that I might be able to spot any errors.
> >
> > I am using GRUB to load Xen. In the GRUB menu, I see two options.
> > Option 1: Debian 11 with latest Linux Kernel
> > Option 2: Debian 11(with Xen hypervisor) with latest Kernel
> >
> >   Can you provide the Device Tree you are using? If you are not
> passing
> >   any Device Tree  binary explicitely, then it might be passed
> >   automatically from EDK2 to Linux/Xen. In that case, just boot from
> Linux
> >   then do the following to retrieve the Device Tree:
> >
> >   dtc -I fs -O dts /proc/device-tree > host.dts
> >
> >   Then please attach host.dts to this email thread.
> >
> > Yeah, you are right. It looks like LInux is booting from ACPI. In the
> bootloader menu, "Automatic ACPI configuration" is disabled. So, I
> > thought that Linux may be booting from Device Tree. I have tried the
> "dtc" command you mentioned. But it looks like there's no device-tree
> > under "/proc". I also tried to get DT info, from
> "/sys/firmware/devicetree/base" . But, there's no info. under devicetree
> folder. I am not
> > quite sure how to get the DT info, if the Linux is booting from ACPI. I
> am attaching .dsl files, that contain the acpi info.
>
> OK, so it is pretty clear that even if "Automatic ACPI configuration" is
> disabled, it is still booting with ACPI.
>
>
> >   Also for your information it looks like Linux actually booted from
> ACPI,
> >   not from Device Tree, as you can see from all the "ACPI" messages
> in the
> >   kernel logs.
> >
> >   If you need to boot from ACPI, then you need to enable ACPI
> support in
> >   Xen, which is disabled by default. You can do that using make
> >   menuconfig.
> >
> > In the make menuconfig of Xen, I do not see any option to enable ACPI.
>
> You definitely need to enable ACPI support in Xen, if you are booting
> from ACPI, otherwise nothing is going to work.
>
> On the latest staging (https://gitlab.com/xen-project/xen) you can
> enable ACPI as follows:
>
>
> # export CROSS_COMPILE=/path/to/cross-compiler
> # export XEN_TARGET_ARCH=arm64
> # cd xen.git/xen
> # make menuconfig
> #   --> Configure UNSUPPORTED features
> #   --> Architecture Features --> ACPI
> # make
>

Hi

I got the code from Gitlab and installed it, enabling ACPI support in Xen.
As I reboot the system, I am able to see 2 options like before.
Option 1: Debian with latest kernel
Option 2 : Debian with Xen

I have selected Option 2. I did not see any bootinfo membanks error.
However, there is the following error in GRUB(just for a fraction of a
second).

"Using modules provided by boot loader in FDT
  Xen 4.16-unstable (c/s Wed Oct 13 13 13:28:43 2021 -0700 git:4cfab4425d)
EFI Loader
  Couldn't obtain the File System Protocol Interface: ErrCode:
0x8002"

I have enabled earlyprintk. I do not see any messages in the Serial. There
seems to be some problem with the gitlab version of Xen.



> Cheers,
>
> Stefano
>
>
> >   On Thu, 30 Sep 2021, Sai Kiran Kumar Reddy Y wrote:
> >   > Hi,
> >   > Sorry about the delay. We have been trying to access the serial
> of the machine. Tried with couple of JTAG connectors. There's
> >   still no
> >   > debug messages on the serial. Is there any other way of figuring
> this out?
> >   >
> >   > On Wed, Sep 15, 2021, 7:02 AM Stefano Stabellini <
> sstabell...@kernel.org> wrote:
> >   >   Something is off. When you enabled earlyprintk in Xen, you
> should see
> >   >   something like this at boot time:
> >   >   https://marc.info/?l=xen-devel=158829968025334
> >   >
> >   >   All the Xen logs starting with "(XEN)" on the serial. Do
> you have access
> >   >   to the serial of the machine? Without it, it is going to
> be hard to
> >   >   debug.
> >   >
> >   >
> >   >   On Tue, 14 Sep 2021, Sai Kiran Kumar Reddy Y wrote:
> >   >   > In the folder "/var/log", there's a file called "xen",
> which is empty. As far as the boot logs are concerned, I don't
> >   see any
> >   >   debug
> >   >   > messages related to xen. I am attaching the log files,
> "kern.txt" and "boot.txt"
> >   >   >
> >   >   > On Tue, Sep 14, 2021 at 3:08 AM Stefano Stabellini <
> sstabell...@kernel.org> wrote:
> >   >   >   On Mon, 13 Sep 2021, Sai Kiran Kumar Reddy Y wrote:
> >   >   >

Re: [XEN PATCH v7 41/51] build,x86: remove the need for build32.mk

On 24.08.2021 12:50, Anthony PERARD wrote:
> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -1,23 +1,51 @@
>  obj-bin-y += head.o
> +head-objs := cmdline.S reloc.S
>  
> -DEFS_H_DEPS = $(BASEDIR)/$(src)/defs.h $(BASEDIR)/include/xen/stdbool.h
> +nocov-y += $(head-objs:.S=.o)
> +noubsan-y += $(head-objs:.S=.o)
> +targets += $(head-objs:.S=.o)

This working right depends on targets initially getting set with := ,
because of ...

> -CMDLINE_DEPS = $(DEFS_H_DEPS) $(BASEDIR)/$(src)/video.h \
> -$(BASEDIR)/include/xen/kconfig.h \
> -$(BASEDIR)/include/generated/autoconf.h
> +head-objs := $(addprefix $(obj)/, $(head-objs))

... this subsequent adjustment to the variable. Might it be more future
proof for start with

head-srcs := cmdline.S reloc.S

and then derive head-objs only here?

> -RELOC_DEPS = $(DEFS_H_DEPS) \
> -  $(BASEDIR)/include/generated/autoconf.h \
> -  $(BASEDIR)/include/xen/kconfig.h \
> -  $(BASEDIR)/include/xen/multiboot.h \
> -  $(BASEDIR)/include/xen/multiboot2.h \
> -  $(BASEDIR)/include/xen/const.h \
> -  $(BASEDIR)/include/public/arch-x86/hvm/start_info.h
> +$(obj)/head.o: $(head-objs)
>  
> -$(obj)/head.o: $(obj)/cmdline.S $(obj)/reloc.S
> +LDFLAGS_DIRECT_OpenBSD = _obsd
> +LDFLAGS_DIRECT_FreeBSD = _fbsd

This is somewhat ugly - it means needing to change things in two places
when config/x86_32.mk would change (e.g. to add another build OS). How
about ...

> +$(head-objs:.S=.lnk): LDFLAGS_DIRECT := -melf_i386$(LDFLAGS_DIRECT_$(XEN_OS))

... instead:

$(head-objs:.S=.lnk): LDFLAGS_DIRECT := $(subst x86_64,i386,$(LDFLAGS_DIRECT))

? Or if deemed still too broad

$(head-objs:.S=.lnk): LDFLAGS_DIRECT := $(subst 
elf_x86_64,elf_i386,$(LDFLAGS_DIRECT))

?

> -$(obj)/cmdline.S: $(src)/cmdline.c $(CMDLINE_DEPS) $(src)/build32.lds
> - $(MAKE) -f $(BASEDIR)/$(src)/build32.mk -C $(obj) $(@F) 
> CMDLINE_DEPS="$(CMDLINE_DEPS)"
> +CFLAGS_x86_32 := -m32 -march=i686
> +CFLAGS_x86_32 += -fno-strict-aliasing
> +CFLAGS_x86_32 += -std=gnu99
> +CFLAGS_x86_32 += -Wall -Wstrict-prototypes
> +$(call cc-option-add,CFLAGS_x86_32,CC,-Wdeclaration-after-statement)
> +$(call cc-option-add,CFLAGS_x86_32,CC,-Wno-unused-but-set-variable)
> +$(call cc-option-add,CFLAGS_x86_32,CC,-Wno-unused-local-typedefs)
> +$(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
> +CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float
> +CFLAGS_x86_32 += -I$(srctree)/include

I'm afraid I'm not convinced that having to keep this in sync with the
original is in fair balance with the removal of build32.mk.

Jan

Re: [XEN PATCH v7 40/51] build: fix dependencies in arch/x86/boot

On 24.08.2021 12:50, Anthony PERARD wrote:
> Temporary fix the list of headers that cmdline.c and reloc.c depends
> on, until the next time the list is out of sync again.
> 
> Also, add the linker script to the list.
> 
> Signed-off-by: Anthony PERARD 

Reviewed-by: Jan Beulich 

Afaict this is independent of all earlier patches, so I'll make an
attempt at committing this soon.

> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -2,19 +2,22 @@ obj-bin-y += head.o
>  
>  DEFS_H_DEPS = $(BASEDIR)/$(src)/defs.h $(BASEDIR)/include/xen/stdbool.h
>  
> -CMDLINE_DEPS = $(DEFS_H_DEPS) $(BASEDIR)/$(src)/video.h
> +CMDLINE_DEPS = $(DEFS_H_DEPS) $(BASEDIR)/$(src)/video.h \
> +$(BASEDIR)/include/xen/kconfig.h \
> +$(BASEDIR)/include/generated/autoconf.h

Especially with this now needed in two places, I think down the road
(unless we'll get dependency tracking automated here, which a part of
your description suggests is not going to happen soon) we want

kconfig.h := $(BASEDIR)/include/xen/kconfig.h \
 $(BASEDIR)/include/generated/autoconf.h

and then similarly ...

>  RELOC_DEPS = $(DEFS_H_DEPS) \
>$(BASEDIR)/include/generated/autoconf.h \
>$(BASEDIR)/include/xen/kconfig.h \
>$(BASEDIR)/include/xen/multiboot.h \
>$(BASEDIR)/include/xen/multiboot2.h \
> +  $(BASEDIR)/include/xen/const.h \

multiboot.h := $(BASEDIR)/include/xen/multiboot.h \
   $(BASEDIR)/include/xen/const.h \

(and by implication I think DEFS_H_DEPS would better be renamed to
deps.h as well).

Jan

Re: [PATCH v5 08/11] xen/arm: Enable the existing x86 virtual PCI support for ARM.

Hi,

> On 14 Oct 2021, at 07:33, Jan Beulich  wrote:
> 
> On 13.10.2021 21:27, Stefano Stabellini wrote:
>> On Wed, 13 Oct 2021, Jan Beulich wrote:
>>> On 13.10.2021 16:51, Oleksandr Andrushchenko wrote:
 On 13.10.21 16:00, Jan Beulich wrote:
> On 13.10.2021 10:45, Roger Pau Monné wrote:
>> On Wed, Oct 06, 2021 at 06:40:34PM +0100, Rahul Singh wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/arm/vpci.c
>>> @@ -0,0 +1,102 @@
>>> +/*
>>> + * xen/arch/arm/vpci.c
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +#include 
>>> +
>>> +#include 
>>> +
>>> +#define REGISTER_OFFSET(addr)  ( (addr) & 0x0fff)
>>> +
>>> +/* Do some sanity checks. */
>>> +static bool vpci_mmio_access_allowed(unsigned int reg, unsigned int 
>>> len)
>>> +{
>>> +/* Check access size. */
>>> +if ( len > 8 )
>>> +return false;
>>> +
>>> +/* Check that access is size aligned. */
>>> +if ( (reg & (len - 1)) )
>>> +return false;
>>> +
>>> +return true;
>>> +}
>>> +
>>> +static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
>>> +  register_t *r, void *p)
>>> +{
>>> +unsigned int reg;
>>> +pci_sbdf_t sbdf;
>>> +unsigned long data = ~0UL;
>>> +unsigned int size = 1U << info->dabt.size;
>>> +
>>> +sbdf.sbdf = MMCFG_BDF(info->gpa);
>>> +reg = REGISTER_OFFSET(info->gpa);
>>> +
>>> +if ( !vpci_mmio_access_allowed(reg, size) )
>>> +return 0;
>>> +
>>> +data = vpci_read(sbdf, reg, min(4u, size));
>>> +if ( size == 8 )
>>> +data |= (uint64_t)vpci_read(sbdf, reg + 4, 4) << 32;
>>> +
>>> +*r = data;
>>> +
>>> +return 1;
>>> +}
>>> +
>>> +static int vpci_mmio_write(struct vcpu *v, mmio_info_t *info,
>>> +   register_t r, void *p)
>>> +{
>>> +unsigned int reg;
>>> +pci_sbdf_t sbdf;
>>> +unsigned long data = r;
>>> +unsigned int size = 1U << info->dabt.size;
>>> +
>>> +sbdf.sbdf = MMCFG_BDF(info->gpa);
>>> +reg = REGISTER_OFFSET(info->gpa);
>>> +
>>> +if ( !vpci_mmio_access_allowed(reg, size) )
>>> +return 0;
>>> +
>>> +vpci_write(sbdf, reg, min(4u, size), data);
>>> +if ( size == 8 )
>>> +vpci_write(sbdf, reg + 4, 4, data >> 32);
>> I think those two helpers (and vpci_mmio_access_allowed) are very
>> similar to the existing x86 ones (see vpci_mmcfg_{read,write}), up to
>> the point where I would consider moving the shared code to vpci.c as
>> vpci_ecam_{read,write} and call them from the arch specific trap
>> handlers.
> Except that please can we stick to mcfg or mmcfg instead of ecam
> in names, as that's how the thing has been named in Xen from its
> introduction? I've just grep-ed the code base (case insensitively)
> and found no mention of ECAM. There are only a few "became".
 I do understand that this is historically that we do not have ECAM in Xen,
 but PCI is not about Xen. Thus, I think it is also acceptable to use
 a commonly known ECAM for the code that works with ECAM.
>>> 
>>> ACPI, afaik, also doesn't call this ECAM. That's where MCFG / MMCFG
>>> actually come from, I believe.
>> 
>> My understanding is that "MCFG" is the name of the ACPI table that
>> describes the PCI config space [1]. The underlying PCI standard for the
>> memory mapped layout of the PCI config space is called ECAM. Here, it
>> makes sense to call it ECAM as it is firmware independent.
>> 
>> [1] https://wiki.osdev.org/PCI_Express
> 
> Okay, I can accept this, but then I'd expect all existing uses of
> MCFG / MMCFG in the code which aren't directly ACPI-related to get
> replaced. Otherwise it's needlessly harder to identify that one
> piece of code relates to certain other pieces.

To sum up on this subject, here what I will do in version 6:
- move vpci_ecam_{read/write} to vpci.c and rename them
- same for access allowed and I will rename it to vpci_ecam_access_allowed

I will work on this and try to push v6 before the end of the day (also handling 
other remarks on this patch).

Cheers
Bertrand

> 
> Jan
>

Re: [PATCH v5 07/11] xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag

Hi,

> On 14 Oct 2021, at 07:23, Jan Beulich  wrote:
> 
> On 13.10.2021 22:41, Stefano Stabellini wrote:
>> On Wed, 13 Oct 2021, Jan Beulich wrote:
>>> On 13.10.2021 14:11, Bertrand Marquis wrote:
> On 13 Oct 2021, at 11:56, Roger Pau Monné  wrote:
> If vPCI for Arm on 4.16 is not going to be functional, why so much
> pressure in pushing those patches so fast? I understand the need to
> remove stuff from the queue, but I don't think it's worth the cost of
> introducing a broken interface deliberately on a release.
>>> 
>>> +1
>>> 
 We did not push that fast, those have been on the mailing list for a while 
 (this is the 5th version of the serie).
 PCI passthrough is a big change requiring lots of patches and we decided 
 to push it piece by piece to make
 the review easier.
>>> 
>>> 5 versions for a series like this one was to be expected. Imo it has
>>> been wrong in the past to rush in new features (even if experimental
>>> ones) very close to the freeze, and it has mislead people to think
>>> they can delay work until very (too?) late a point in time.
>> 
>> 
>> Hi Roger, Jan,
>> 
>> Let me take a few minutes to clarify and provide context for this work.
>> 
>> 
>> I don't think anyone "pushed hard" any of the ARM series close to the
>> release. I sent "one" email in public as a reminder of things that need
>> reviewing:
>> https://marc.info/?l=xen-devel=163373776611154
>> 
>> I did send a couple of private emails to Jan but they were to synchronize
>> ourselves rather than push; Jan, I hope you didn't take them the wrong
>> way.
> 
> Definitely not, no worries.
> 
>> At the same time it is certainly true that all the people involved here
>> worked very hard to get these series ready for 4.16. Oct 15 is the Xen
>> Project feature freeze. It is also the deadline for many of us working
>> with upstream Xen Project to upstream features and report back to our
>> management. I know it doesn't affect you guys directly, but you can
>> imagine that as employees of our respective companies we feel pressure
>> to complete our objectives in the given timeframe. Of course we need to
>> make sure to do that without compromising quality and without
>> overextending upstream review capacity.
>> 
>> 
>> The ARM PCI series are public since Dec 2020, pushed to a gitlab branch
>> for testing: 
>> https://gitlab.com/xen-project/fusa/xen-integration/-/tree/integration/pci-passthrough
>> I helped creating, managing and testing the branch.
>> 
>> So, I can see why Bertrand feels like they have been around for a while:
>> we have been dealing with these patches for almost a year, even if from
>> a xen-devel perspective we are "only" at version 5.
> 
> I'm afraid that anything not on xen-devel doesn't really count; the list
> is the only "official" channel. ISTR that for certain aspects there's a
> plan George has to make this more pronounced / formal in some of the docs
> we have.
> 
> Making internally set deadlines is a fully understandable aspect. But
> starting to post in September for a mid-October deadline is putting
> oneself at risk, I would say, for a (set of) series this involved. I see
> no problem with anyone doing so, as long as they accept that risk rather
> than expecting to get away by either extraordinary efforts by others
> (reviewers) or relaxation of what is permitted to go in.
> 
> Of course there's the opposite problem of feedback taking unusually (I'm
> inclined to say unduly) long to arrive, like with the gpaddr_bits
> addition still facing vague / unclear opposition by Andrew.
> 
>> My suggestion is to accept the TODO for patch #8 [1] but I agree with
>> Roger that we shouldn't introduce bad interfaces, especially as they
>> impact x86 which is not "tech preview". So I think we should revert
>> patch #7 (this patch.) I'll reply with more details in separate shorter
>> email.
>> 
>> [1] https://marc.info/?l=xen-devel=163412120531248
> 
> FWIW I agree with both proposals (acceptance of TODO and revert).

I want to say again that nothing I said was meant as a critic and I am very
grateful of all reviews and comments that we have (even if some of them
are implying work or pushing stuff in the future) as we want to do this right.

We will do the following before the end of the day today:
- send a reverting patch for patch 7
- send updated version of patch 8 handling all pending comments

We will try to (not sure as some changes might be more complex)
- send an updated version of patch 10
- try to find a solution acceptable for patch 1

Thanks everyone for the support here and the huge amount of time
spent on reviewing all patches that was pushed (and there was a lot of them).

Over the next month Arm is committed to put a lot more effort in reviewing and
testing patches pushed to xen-devel. We now have an internal CI that will help
us a lot and all people in Arm Xen team will start to have dedicated time to 
review
upstream patches and help the community.
Xen is a

[ovmf test] 165502: all pass - PUSHED

flight 165502 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/165502/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 978d428ec3ca2520c217819901c8465235c45c5e
baseline version:
 ovmf 6ed6abd6c116e8599876a2876b77e172e800b13e

Last test of basis   165494  2021-10-13 12:41:14 Z0 days
Testing same since   165502  2021-10-13 23:10:03 Z0 days1 attempts


People who touched revisions under test:
  Ma, Maurice 
  Maurice Ma 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   6ed6abd6c1..978d428ec3  978d428ec3ca2520c217819901c8465235c45c5e -> 
xen-tested-master

RE: [PATCH v2] xen/arm: vgic to ignore GICD ICPENRn registers access

2021-10-14 Thread Hongda Deng

Hi,

> -Original Message-
> From: Julien Grall 
> Sent: 2021年10月13日 5:58
> To: Hongda Deng ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org
> Cc: Bertrand Marquis ; Wei Chen
> 
> Subject: Re: [PATCH v2] xen/arm: vgic to ignore GICD ICPENRn registers access
> 
> Hi,
> 
> On 12/10/2021 07:24, Hongda Deng wrote:
> > Currently, Xen will return IO unhandled when guests access GICD ICPENRn
> > registers. This will raise a data abort inside guest. For Linux Guest,
> > these virtual registers will not be accessed. But for Zephyr, in its
> > GIC initialization code, these virtual registers will be accessed. And
> > zephyr guest will get an IO data abort in initilization stage and enter
> > fatal error. Emulating ICPENDR is not easy with the existing vGIC, so
> > we currently ignore these virtual registers access and print a message
> > about whether they are already pending instead of returning unhandled.
> > More details can be found at [1].
> 
> The link you provide only states that I am happy with the warning. This
> doesn't seem relevant for a future reader. Did you intend to point to
> something different?
> 

Yes, I would attach this link [1] then, which explains how zephyr accesses 
ICPENDR at its initialization. ( Though I still don't understand why zephyr 
would clear this register at initialization while linux wouldn't )

> >
> > [1] https://lists.xenproject.org/archives/html/xen-devel/2021-09/
> > msg00744.html
> >
> > Signed-off-by: Hongda Deng 
> > ---
> >   xen/arch/arm/vgic-v2.c | 26 +-
> >   xen/arch/arm/vgic-v3.c | 40 +++-
> >   2 files changed, 56 insertions(+), 10 deletions(-)
> >
> > diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
> > index b2da886adc..d7ffaeeb65 100644
> > --- a/xen/arch/arm/vgic-v2.c
> > +++ b/xen/arch/arm/vgic-v2.c
> > @@ -480,11 +480,35 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v,
> mmio_info_t *info,
> >   return 1;
> >
> >   case VRANGE32(GICD_ICPENDR, GICD_ICPENDRN):
> > +{
> > +struct pending_irq *iter;
> > +unsigned int irq_start;
> > +unsigned int irq_end;
> > +uint32_t irq_pending = 0;
> > +
> >   if ( dabt.size != DABT_WORD ) goto bad_width;
> >   printk(XENLOG_G_ERR
> >  "%pv: vGICD: unhandled word write %#"PRIregister" to
> ICPENDR%d\n",
> >  v, r, gicd_reg - GICD_ICPENDR);
> 
> As I wrote in v1, we should avoid to print a message when we know there
> is no pending interrupts.
> 

These are not the modifications made in this patch, and same codes also exist
for ICACTIVER. I didn't delete them for that I think they are used to give some
useful and coherent messages to user for reference. Should we delete it for both
or only for ICPENDR?

> > -return 0;
> > +
> > +irq_start = (gicd_reg - GICD_ICPENDR) * 32;
> > +irq_end = irq_start + 31;
> > +/* go through inflight_irqs and print specified pending irqs */
> > +list_for_each_entry(iter, >arch.vgic.inflight_irqs, inflight)
> You need to hold v->arch.vgic.lock (with interrupt disabled) to go
> through the list of inflight irqs. Otherwise, the list may be modified
> while you are walking it.
> 

Ack.

> However, I am a little bit concerned with this approached (I noticed
> Stefano suggested). The list may in theory contains a few hundreds
> interrupts (a malicious OS May decide to never read IAR). So we are
> potentially doing more work than necessary (we need to think about the
> worse case scenario).
> 
> Instead, I think it would be better to go through the 32 interrupts and
> for each of them:
>1) find the pending_irq() using irq_to_pending()
>2) check if the IRQ in the inflight list with list_empty(>inflight)
> 
> In addition to that, you want to check that the rank exists so we don't
> do any extra work if the guest is trying to clear an interrupts above
> the number of interrupts we support.
> 

Agreed, and that's quite helpful.

> > +{
> > +if ( iter->irq < irq_start || irq_end < iter->irq )
> > +continue;
> > +
> > +if ( test_bit(GIC_IRQ_GUEST_QUEUED, >status) )
> > +irq_pending = irq_pending | (1 << (iter->irq - irq_start));
> > +}
> > +
> > +if ( irq_pending != 0 )
> > +printk(XENLOG_G_ERR
> > +   "%pv: vGICD: ICPENDR%d=0x%08x\n",
> > +   v, gicd_reg - GICD_ICPENDR, irq_pending);
> 
> This message is a bit confusing. I think it would be worth to print a
> message for every interrupt not cleared. Maybe something like:
> 
> "%pv trying to clear pending interrupt %u."
> 

I was thinking that there may be too many interrupts there, to print each with
one message line would be too superfluous. 
But that worst case scenario should not be usual, and print a message for each 
interrupt would be much clearer.

Ack.

> > +goto write_ignore_32;
> > +

Re: [XEN PATCH v7 51/51] build: add %.E targets

On 13.10.2021 17:48, Jan Beulich wrote:
> On 24.08.2021 12:50, Anthony PERARD wrote:
>> I guess it's easier to remember that %.E does "$(CC) -E" or "$(CPP)".
> 
> I've never seen any *.E. I'm puzzled (and hence have reservations, but
> then again don't care enough to object).

Actually I've checked gcc, and it wouldn't know what to do with a *.E
file. Hence I'd like to ask that you add a reasonable reference to a
pre-existing use of this naming. Without such reference I'd feel the
addition would have more risk of confusion than possible value.

Jan

Re: [PATCH v5 08/11] xen/arm: Enable the existing x86 virtual PCI support for ARM.

On 13.10.2021 21:27, Stefano Stabellini wrote:
> On Wed, 13 Oct 2021, Jan Beulich wrote:
>> On 13.10.2021 16:51, Oleksandr Andrushchenko wrote:
>>> On 13.10.21 16:00, Jan Beulich wrote:
 On 13.10.2021 10:45, Roger Pau Monné wrote:
> On Wed, Oct 06, 2021 at 06:40:34PM +0100, Rahul Singh wrote:
>> --- /dev/null
>> +++ b/xen/arch/arm/vpci.c
>> @@ -0,0 +1,102 @@
>> +/*
>> + * xen/arch/arm/vpci.c
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +#include 
>> +
>> +#include 
>> +
>> +#define REGISTER_OFFSET(addr)  ( (addr) & 0x0fff)
>> +
>> +/* Do some sanity checks. */
>> +static bool vpci_mmio_access_allowed(unsigned int reg, unsigned int len)
>> +{
>> +/* Check access size. */
>> +if ( len > 8 )
>> +return false;
>> +
>> +/* Check that access is size aligned. */
>> +if ( (reg & (len - 1)) )
>> +return false;
>> +
>> +return true;
>> +}
>> +
>> +static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
>> +  register_t *r, void *p)
>> +{
>> +unsigned int reg;
>> +pci_sbdf_t sbdf;
>> +unsigned long data = ~0UL;
>> +unsigned int size = 1U << info->dabt.size;
>> +
>> +sbdf.sbdf = MMCFG_BDF(info->gpa);
>> +reg = REGISTER_OFFSET(info->gpa);
>> +
>> +if ( !vpci_mmio_access_allowed(reg, size) )
>> +return 0;
>> +
>> +data = vpci_read(sbdf, reg, min(4u, size));
>> +if ( size == 8 )
>> +data |= (uint64_t)vpci_read(sbdf, reg + 4, 4) << 32;
>> +
>> +*r = data;
>> +
>> +return 1;
>> +}
>> +
>> +static int vpci_mmio_write(struct vcpu *v, mmio_info_t *info,
>> +   register_t r, void *p)
>> +{
>> +unsigned int reg;
>> +pci_sbdf_t sbdf;
>> +unsigned long data = r;
>> +unsigned int size = 1U << info->dabt.size;
>> +
>> +sbdf.sbdf = MMCFG_BDF(info->gpa);
>> +reg = REGISTER_OFFSET(info->gpa);
>> +
>> +if ( !vpci_mmio_access_allowed(reg, size) )
>> +return 0;
>> +
>> +vpci_write(sbdf, reg, min(4u, size), data);
>> +if ( size == 8 )
>> +vpci_write(sbdf, reg + 4, 4, data >> 32);
> I think those two helpers (and vpci_mmio_access_allowed) are very
> similar to the existing x86 ones (see vpci_mmcfg_{read,write}), up to
> the point where I would consider moving the shared code to vpci.c as
> vpci_ecam_{read,write} and call them from the arch specific trap
> handlers.
 Except that please can we stick to mcfg or mmcfg instead of ecam
 in names, as that's how the thing has been named in Xen from its
 introduction? I've just grep-ed the code base (case insensitively)
 and found no mention of ECAM. There are only a few "became".
>>> I do understand that this is historically that we do not have ECAM in Xen,
>>> but PCI is not about Xen. Thus, I think it is also acceptable to use
>>> a commonly known ECAM for the code that works with ECAM.
>>
>> ACPI, afaik, also doesn't call this ECAM. That's where MCFG / MMCFG
>> actually come from, I believe.
> 
> My understanding is that "MCFG" is the name of the ACPI table that
> describes the PCI config space [1]. The underlying PCI standard for the
> memory mapped layout of the PCI config space is called ECAM. Here, it
> makes sense to call it ECAM as it is firmware independent.
> 
> [1] https://wiki.osdev.org/PCI_Express

Okay, I can accept this, but then I'd expect all existing uses of
MCFG / MMCFG in the code which aren't directly ACPI-related to get
replaced. Otherwise it's needlessly harder to identify that one
piece of code relates to certain other pieces.

Jan

Re: [PATCH v5 07/11] xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag