Re: [PATCH v3 02/13] xen: harmonize return types of hypercall handlers

[Juergen Gross]

On 17.12.21 08:45, Jan Beulich wrote:

On 17.12.2021 06:34, Juergen Gross wrote:

On 16.12.21 22:15, Stefano Stabellini wrote:

On Thu, 16 Dec 2021, Stefano Stabellini wrote:

On Thu, 16 Dec 2021, Juergen Gross wrote:

On 16.12.21 03:10, Stefano Stabellini wrote:

The case of XENMEM_maximum_ram_page is interesting but it is not a
problem in reality because the max physical address size is only 40-bit
for aarch32 guests, so 32-bit are always enough to return the highest
page in memory for 32-bit guests.


You are aware that this isn't the guest's max page, but the host's?


I can see now that you meant to say that, no matter what is the max
pseudo-physical address supported by the VM, XENMEM_maximum_ram_page is
supposed to return the max memory page, which could go above the
addressibility limit of the VM.

So XENMEM_maximum_ram_page should potentially be able to return (1<<44)
even when called by an aarch32 VM, with max IPA 40-bit.

I would imagine it could be useful if dom0 is 32-bit but domUs are
64-bit on a 64-bit hypervisor (which I think it would be a very rare
configuration on ARM.)

Then it looks like XENMEM_maximum_ram_page needs to be able to return a
value > 32-bit when called by a 32-bit guest.

The hypercall ABI follows the ARM C calling convention, so a 64-bit
value should be returned using r0 and r1. But looking at
xen/arch/arm/traps.c:do_trap_hypercall, it doesn't seem it ever sets r1
today. Only r0 is set, so effectively we only support 32-bit return
values on aarch32 and for aarch32 guests.

In other words, today all hypercalls on ARM return 64-bit to 64-bit
guests and 32-bit to 32-bit guests. Which in the case of memory_op is
"technically" the correct thing to do because it matches the C
declaration in xen/include/xen/hypercall.h:

extern long
do_memory_op(
  unsigned long cmd,
  XEN_GUEST_HANDLE_PARAM(void) arg);

So...  I guess the conclusion is that on ARM do_memory_op should return
"long" although it is not actually enough for a correct implementation
of XENMEM_maximum_ram_page for aarch32 guests ?



Hence my suggestion to check the return value of _all_ hypercalls to be
proper sign extended int values for 32-bit guests. This would fix all
potential issues without silently returning truncated values.


Are we absolutely certain we have no other paths left where a possibly
large unsigned values might be returned? In fact while
compat_memory_op() does the necessary saturation, I've never been fully
convinced of this being the best way of dealing with things. The range
of error indicators is much smaller than [-INT_MIN,-1], so almost
double the range of effectively unsigned values could be passed back
fine. (Obviously we can't change existing interfaces, so this mem-op
will need to remain as is.)


In fact libxenctrl tries do deal with this fact by wrapping a memory_op
for a 32-bit environment into a multicall. This will work fine for a
32-bit Arm guest, as xen_ulong_t is a uint64 there.

So do_memory_op should return long on Arm, yes. OTOH doing so will
continue to be a problem in case a 32-bit guest doesn't use the
multicall technique for handling possible 64-bit return values.

So I continue to argue that on Arm the return value of a hypercall
should be tested to fit into 32 bits. The only really clean alternative
would be to have separate hypercall function classes for Arm 32- and
64-bit guests (which still could share most of the functions by letting
those return "int"). This would allow to use the 64-bit variant even for
32-bit guests in multicall (fine as the return field is 64-bit wide),
and a probably saturating compat version for the 32-bit guest direct
hypercall.

The needed adaptions in my series would be rather limited (an additional
column in the hypercall table, a split which macro to use in
do_trap_hypercall() on Arm depending on the bitness of the guest, the
addition of the Arm compat variant of do_memory_op()).

Thoughts?


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH] xen/arm: vpci: Remove PCI I/O ranges property value

[Rahul Singh]

Hi Stefano,

> On 16 Dec 2021, at 9:48 pm, Stefano Stabellini  wrote:
> 
> On Thu, 16 Dec 2021, Rahul Singh wrote:
>> Hi Stefano,
>> 
>>> On 16 Dec 2021, at 2:33 am, Stefano Stabellini  
>>> wrote:
>>> 
>>> On Tue, 14 Dec 2021, Rahul Singh wrote:
 IO ports on ARM don't exist so all IO ports related hypercalls are going
 to fail on ARM when we passthrough a PCI device.
 Failure of xc_domain_ioport_permission(..) would turn into a critical
 failure at domain creation. We need to avoid this outcome, instead we
 want to continue with domain creation as normal even if
 xc_domain_ioport_permission(..) fails. XEN_DOMCTL_ioport_permission
 is not implemented on ARM so it would return -ENOSYS.
 
 To solve above issue remove PCI I/O ranges property value from dom0
 device tree node so that dom0 linux will not allocate I/O space for PCI
 devices if pci-passthrough is enabled.
 
 Another valid reason to remove I/O ranges is that PCI I/O space are not
 mapped to dom0 when PCI passthrough is enabled, also there is no vpci
 trap handler register for IO bar.
 
 Signed-off-by: Rahul Singh 
 ---
 xen/arch/arm/domain_build.c   | 14 +++
 xen/common/device_tree.c  | 72 +++
 xen/include/xen/device_tree.h | 10 +
 3 files changed, 96 insertions(+)
 
 diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
 index d02bacbcd1..60f6b2c73b 100644
 --- a/xen/arch/arm/domain_build.c
 +++ b/xen/arch/arm/domain_build.c
 @@ -749,6 +749,11 @@ static int __init write_properties(struct domain *d, 
 struct kernel_info *kinfo,
continue;
}
 
 +if ( is_pci_passthrough_enabled() &&
 +dt_device_type_is_equal(node, "pci") )
 +if ( dt_property_name_is_equal(prop, "ranges") )
 +continue;
>>> 
>>> It looks like we are skipping the "ranges" property entirely for the PCI
>>> node, is that right? Wouldn't that also remove the other (not ioports)
>>> address ranges?
>> 
>> We are skipping the “ranges” property here to avoid setting the “ranges” 
>> property when
>> pci_passthrough is enabled. We will remove only remove IO port and set the 
>> other ‘ranges” property 
>> value in dt_pci_remove_io_ranges() in next if() condition.
>> 
>> 
res = fdt_property(kinfo->fdt, prop->name, prop_data, prop_len);
 
if ( res )
 @@ -769,6 +774,15 @@ static int __init write_properties(struct domain *d, 
 struct kernel_info *kinfo,
if ( res )
return res;
}
 +
 +/*
 + * PCI IO bar are not mapped to dom0 when PCI passthrough is 
 enabled,
 + * also there is no trap handler registered for IO bar therefor 
 remove
 + * the IO range property from the device tree node for dom0.
 + */
 +res = dt_pci_remove_io_ranges(kinfo->fdt, node);
 +if ( res )
 +return res;
>>> 
>>> I tried to apply this patch to staging to make it easier to review but I
>>> think this chuck got applied wrongly: I can see
>>> dt_pci_remove_io_ranges() added to the function "handle_prop_pfdt" which
>>> is for guest partial DTBs and not for dom0.
>> 
>> Oleksandr’s patch series was merged yesterday because of that there is 
>> conflict in applying 
>> this patch. I will rebase the patch and will send it again for review.
>> 
>>> 
>>> Is dt_pci_remove_io_ranges() meant to be called from write_properties
>>> instead? It looks like it would be best to call it from
>>> write_properties, maybe it could be combined with the other new check
>>> just above in this patch?
>> 
>> Yes dt_pci_remove_io_ranges() is to be called from write_properties().
> 
> OK. In that case the only feedback that is I have is that it might be
> possible to avoid the first change of this patch to skip "ranges" by
> moving the call to dt_pci_remove_io_ranges() earlier in the
> write_properties function.

Ok. I will modify the code based on your comment.

Regards,
Rahul

Re: [PATCH] tools/xenstore: drop support for running under SunOS

[Andrew Cooper]

On 17/12/2021 07:50, Juergen Gross wrote:
> Since several years now xenstored is no longer capable to run under
> SunOS, as the needed libxengnttab interfaces are not available there.
>
> Several attempts to let the SunOS maintainers address this situation
> didn't change anything in this regard.
>
> For those reasons drop SunOS support in xenstored by removing the SunOS
> specific code.
>
> Signed-off-by: Juergen Gross 

FWIW, Acked-by: Andrew Cooper 

> -out:
> - /*
> -  * 6589130 dtrace -G fails for certain tail-calls on x86
> -  */
> - asm("nop");
> -}

Code like this makes me extra sad... It only works for the same reasons
as why asm ("") would be fine as well.

~Andrew

Re: [PATCH v3] xen/arm64: Zero the top 32 bits of gp registers on entry...

[Julien Grall]

Hi,

On 17/12/2021 07:21, Michal Orzel wrote:

to hypervisor when switching from AArch32 state.

According to section D1.20.2 of Arm Arm(DDI 0487A.j):
"If the general-purpose register was accessible from AArch32 state the
upper 32 bits either become zero, or hold the value that the same
architectural register held before any AArch32 execution.
The choice between these two options is IMPLEMENTATION DEFINED"

Currently Xen does not ensure that the top 32 bits are zeroed and this
needs to be fixed. The reason why is that there are places in Xen
where we assume that top 32bits are zero for AArch32 guests.
If they are not, this can lead to misinterpretation of Xen regarding
what the guest requested. For example hypercalls returning an error
encoded in a signed long like do_sched_op, do_hmv_op, do_memory_op
would return -ENOSYS if the command passed as the first argument was
clobbered.

Create a macro clobber_gp_top_halves to clobber top 32 bits of gp
registers when hyp == 0 (guest mode) and compat == 1 (AArch32 mode).
Add a compile time check to ensure that save_x0_x1 == 1 if
compat == 1.

Signed-off-by: Michal Orzel 
---
Changes since v2:
-add clobbering of w30
Changes since v1:
-put new code into macro
-add compile time check for save_x0_x1
---
  xen/arch/arm/arm64/entry.S | 29 +
  1 file changed, 29 insertions(+)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index fc3811ad0a..e351ef8639 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -102,6 +102,30 @@
  .endif
  
  .endm

+
+/*
+ * Clobber top 32 bits of gp registers when switching from AArch32
+ */
+.macro clobber_gp_top_halves, compat, save_x0_x1
+
+.if \compat == 1  /* AArch32 mode */
+
+/*
+ * save_x0_x1 is equal to 0 only for guest_sync (compat == 0).
+ * Add a compile time check to avoid violating this rule.
+ */


We may want in the future to allow save_x0_x1 == 1 with compat == 1 if 
we need to create fastpath like we did when entering AArch64.


So I would reword the comment to make clear this is an implementation 
decision. How about:


"At the moment, no-one is using save_x0_x1 == 0 with compat == 1. So the 
code is not handling it to simplify the implementation."


If you are happy with the new comment, I can update it on commit:

Acked-by: Julien Grall 

Cheers,

--
Julien Grall

Re: [PATCH v3 02/13] xen: harmonize return types of hypercall handlers

[Julien Grall]

Hi Juergen,

On 15/12/2021 07:03, Juergen Gross wrote:

On 14.12.21 18:36, Julien Grall wrote:

Hi,

On 08/12/2021 15:55, Juergen Gross wrote:

Today most hypercall handlers have a return type of long, while the
compat ones return an int. There are a few exceptions from that rule,
however.


So on Arm64, I don't think you can make use of the full 64-bit because 
a 32-bit domain would not be able to see the top 32-bit.


In fact, this could potentially cause us some trouble (see [1]) in Xen.
So it feels like the hypercalls should always return a 32-bit signed 
value on Arm.


This would break hypercalls like XENMEM_maximum_ram_page which are able
to return larger values, right?

The other advantage is it would be clear that the top 32-bit are not 
usuable. Stefano, what do you think?


Wouldn't it make more sense to check the return value to be a sign
extended 32-bit value for 32-bit guests in do_trap_hypercall() instead?

The question is what to return if this is not the case. -EDOM?


It looks like there was a lot of discussion afterwards. I will read 
everything and answer on the last part of the thread :).








Get rid of the exceptions by letting compat handlers always return int
and others always return long.

For the compat hvm case use eax instead of rax for the stored result as
it should have been from the beginning.

Additionally move some prototypes to include/asm-x86/hypercall.h
as they are x86 specific. Move the do_physdev_op() prototype from both
architecture dependant headers to the common one. Move the
compat_platform_op() prototype to the common header.

Switch some non style compliant types (u32, s32, s64) to style compliant
ones.


TBH, I think this should have been split because the modifications are 
done on lines that are untouched.


The extra patch would have made the review easier (even if this patch 
is still quite small).


I can split the patch if you want.


I have already reviewed this patch. So it is not going to help me :). 
However, I would appreciate if in the future the coding style changes 
are separated at least when they are not meant to be untouched.


Cheers,

--
Julien Grall

Re: [PATCH v3 02/13] xen: harmonize return types of hypercall handlers

[Julien Grall]

Hi Stefano,

On 16/12/2021 21:15, Stefano Stabellini wrote:

On Thu, 16 Dec 2021, Stefano Stabellini wrote:

On Thu, 16 Dec 2021, Juergen Gross wrote:

On 16.12.21 03:10, Stefano Stabellini wrote:

On Wed, 15 Dec 2021, Juergen Gross wrote:

On 14.12.21 18:36, Julien Grall wrote:

Hi,

On 08/12/2021 15:55, Juergen Gross wrote:

Today most hypercall handlers have a return type of long, while the
compat ones return an int. There are a few exceptions from that rule,
however.


So on Arm64, I don't think you can make use of the full 64-bit because a
32-bit domain would not be able to see the top 32-bit.

In fact, this could potentially cause us some trouble (see [1]) in Xen.
So it feels like the hypercalls should always return a 32-bit signed
value
on Arm.


This would break hypercalls like XENMEM_maximum_ram_page which are able
to return larger values, right?


The other advantage is it would be clear that the top 32-bit are not
usuable. Stefano, what do you think?


Wouldn't it make more sense to check the return value to be a sign
extended 32-bit value for 32-bit guests in do_trap_hypercall() instead?

The question is what to return if this is not the case. -EDOM?



I can see where Julien is coming from: we have been trying to keep the
arm32 and arm64 ABIs identical since the beginning of the project. So,
like Julien, my preference would be to always return 32-bit on ARM, both
aarch32 and aarch64. It would make things simple.

The case of XENMEM_maximum_ram_page is interesting but it is not a
problem in reality because the max physical address size is only 40-bit
for aarch32 guests, so 32-bit are always enough to return the highest
page in memory for 32-bit guests.


You are aware that this isn't the guest's max page, but the host's?


I can see now that you meant to say that, no matter what is the max
pseudo-physical address supported by the VM, XENMEM_maximum_ram_page is
supposed to return the max memory page, which could go above the
addressibility limit of the VM.

So XENMEM_maximum_ram_page should potentially be able to return (1<<44)
even when called by an aarch32 VM, with max IPA 40-bit.


I am a bit confused with what you wrote. Yes, 32-bit VM can only address 
40-bit, but this is only limiting its own (guest) physical address 
space. Such VM would still be able to map any host physical address 
(assuming GFN != MFN).




I would imagine it could be useful if dom0 is 32-bit but domUs are
64-bit on a 64-bit hypervisor (which I think it would be a very rare
configuration on ARM.)


Looking at the implementation, the hypercall is accessible by any 
domain. IOW a domU 32-bit could read a wrong value.


That said, it is not clear to me why an Arm or HVM x86 guest would want 
to read the value.


Cheers,

--
Julien Grall

Re: [PATCH v3 02/13] xen: harmonize return types of hypercall handlers

[Julien Grall]

Hi Juergen,

On 17/12/2021 08:50, Juergen Gross wrote:

On 17.12.21 08:45, Jan Beulich wrote:

On 17.12.2021 06:34, Juergen Gross wrote:

On 16.12.21 22:15, Stefano Stabellini wrote:

On Thu, 16 Dec 2021, Stefano Stabellini wrote:

On Thu, 16 Dec 2021, Juergen Gross wrote:

On 16.12.21 03:10, Stefano Stabellini wrote:

The case of XENMEM_maximum_ram_page is interesting but it is not a
problem in reality because the max physical address size is only 
40-bit
for aarch32 guests, so 32-bit are always enough to return the 
highest

page in memory for 32-bit guests.


You are aware that this isn't the guest's max page, but the host's?


I can see now that you meant to say that, no matter what is the max
pseudo-physical address supported by the VM, XENMEM_maximum_ram_page is
supposed to return the max memory page, which could go above the
addressibility limit of the VM.

So XENMEM_maximum_ram_page should potentially be able to return (1<<44)
even when called by an aarch32 VM, with max IPA 40-bit.

I would imagine it could be useful if dom0 is 32-bit but domUs are
64-bit on a 64-bit hypervisor (which I think it would be a very rare
configuration on ARM.)

Then it looks like XENMEM_maximum_ram_page needs to be able to return a
value > 32-bit when called by a 32-bit guest.

The hypercall ABI follows the ARM C calling convention, so a 64-bit
value should be returned using r0 and r1. But looking at
xen/arch/arm/traps.c:do_trap_hypercall, it doesn't seem it ever sets r1
today. Only r0 is set, so effectively we only support 32-bit return
values on aarch32 and for aarch32 guests.

In other words, today all hypercalls on ARM return 64-bit to 64-bit
guests and 32-bit to 32-bit guests. Which in the case of memory_op is
"technically" the correct thing to do because it matches the C
declaration in xen/include/xen/hypercall.h:

extern long
do_memory_op(
  unsigned long cmd,
  XEN_GUEST_HANDLE_PARAM(void) arg);

So...  I guess the conclusion is that on ARM do_memory_op should return
"long" although it is not actually enough for a correct implementation
of XENMEM_maximum_ram_page for aarch32 guests ?



Hence my suggestion to check the return value of _all_ hypercalls to be
proper sign extended int values for 32-bit guests. This would fix all
potential issues without silently returning truncated values.


Are we absolutely certain we have no other paths left where a possibly
large unsigned values might be returned? In fact while
compat_memory_op() does the necessary saturation, I've never been fully
convinced of this being the best way of dealing with things. The range
of error indicators is much smaller than [-INT_MIN,-1], so almost
double the range of effectively unsigned values could be passed back
fine. (Obviously we can't change existing interfaces, so this mem-op
will need to remain as is.)


In fact libxenctrl tries do deal with this fact by wrapping a memory_op
for a 32-bit environment into a multicall. This will work fine for a
32-bit Arm guest, as xen_ulong_t is a uint64 there.

So do_memory_op should return long on Arm, yes. OTOH doing so will
continue to be a problem in case a 32-bit guest doesn't use the
multicall technique for handling possible 64-bit return values.

So I continue to argue that on Arm the return value of a hypercall
should be tested to fit into 32 bits. 


It would make sense. But what would you return if the value doesn't fit?


The only really clean alternative
would be to have separate hypercall function classes for Arm 32- and
64-bit guests (which still could share most of the functions by letting
those return "int"). This would allow to use the 64-bit variant even for
32-bit guests in multicall (fine as the return field is 64-bit wide),
and a probably saturating compat version for the 32-bit guest direct
hypercall.


I am not entirely sure to understand this proposal. Can you clarify it?



The needed adaptions in my series would be rather limited (an additional
column in the hypercall table, a split which macro to use in
do_trap_hypercall() on Arm depending on the bitness of the guest, the
addition of the Arm compat variant of do_memory_op()).


Cheers,

--
Julien Grall

Re: [PATCH] tools/xenstore: drop support for running under SunOS

[Julien Grall]

Hi Juergen,

On 17/12/2021 07:50, Juergen Gross wrote:

Since several years now xenstored is no longer capable to run under
SunOS, as the needed libxengnttab interfaces are not available there.

Several attempts to let the SunOS maintainers address this situation
didn't change anything in this regard.

For those reasons drop SunOS support in xenstored by removing the SunOS
specific code.

Signed-off-by: Juergen Gross 


Reviewed-by: Julien Grall 

Cheers,


---
  tools/xenstore/Makefile|  15 +--
  tools/xenstore/xenstored_core.c|   7 --
  tools/xenstore/xenstored_core.h|   4 -
  tools/xenstore/xenstored_minios.c  |   4 -
  tools/xenstore/xenstored_posix.c   |   6 --
  tools/xenstore/xenstored_probes.d  |  28 -
  tools/xenstore/xenstored_solaris.c | 168 -
  7 files changed, 1 insertion(+), 231 deletions(-)
  delete mode 100644 tools/xenstore/xenstored_probes.d
  delete mode 100644 tools/xenstore/xenstored_solaris.c

diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
index 292b478fa1..c86278fdb1 100644
--- a/tools/xenstore/Makefile
+++ b/tools/xenstore/Makefile
@@ -27,7 +27,6 @@ XENSTORED_OBJS += xenstored_transaction.o xenstored_control.o
  XENSTORED_OBJS += xs_lib.o talloc.o utils.o tdb.o hashtable.o
  
  XENSTORED_OBJS_$(CONFIG_Linux) = xenstored_posix.o

-XENSTORED_OBJS_$(CONFIG_SunOS) = xenstored_solaris.o xenstored_posix.o 
xenstored_probes.o
  XENSTORED_OBJS_$(CONFIG_NetBSD) = xenstored_posix.o
  XENSTORED_OBJS_$(CONFIG_FreeBSD) = xenstored_posix.o
  XENSTORED_OBJS_$(CONFIG_MiniOS) = xenstored_minios.o
@@ -50,18 +49,6 @@ all: $(ALL_TARGETS)
  .PHONY: clients
  clients: xenstore $(CLIENTS) xenstore-control
  
-ifeq ($(CONFIG_SunOS),y)

-xenstored_probes.h: xenstored_probes.d
-   dtrace -C -h -s xenstored_probes.d
-
-xenstored_solaris.o: xenstored_probes.h
-
-xenstored_probes.o: xenstored_solaris.o
-   dtrace -C -G -s xenstored_probes.d xenstored_solaris.o
-
-CFLAGS += -DHAVE_DTRACE=1
-endif
-
  ifeq ($(CONFIG_SYSTEMD),y)
  $(XENSTORED_OBJS): CFLAGS += $(SYSTEMD_CFLAGS)
  xenstored: LDFLAGS += $(SYSTEMD_LIBS)
@@ -89,7 +76,7 @@ xs_tdb_dump: xs_tdb_dump.o utils.o tdb.o talloc.o
  
  .PHONY: clean

  clean:
-   rm -f *.a *.o xenstored_probes.h
+   rm -f *.a *.o
rm -f xenstored
rm -f xs_tdb_dump xenstore-control init-xenstore-domain
rm -f xenstore $(CLIENTS)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 91d093a12e..c386ae6129 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -146,10 +146,6 @@ static void trace_io(const struct connection *conn,
time_t now;
struct tm *tm;
  
-#ifdef HAVE_DTRACE

-   dtrace_io(conn, data, out);
-#endif
-
if (tracefd < 0)
return;
  
@@ -2337,9 +2333,6 @@ int main(int argc, char *argv[])

/* Get ready to listen to the tools. */
initialize_fds(&sock_pollfd_idx, &timeout);
  
-	/* Tell the kernel we're up and running. */

-   xenbus_notify_running();
-
  #if defined(XEN_SYSTEMD_ENABLED)
if (!live_update) {
sd_notify(1, "READY=1");
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 07d861d924..190d2447cd 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -225,7 +225,6 @@ int delay_request(struct connection *conn, struct 
buffered_data *in,
  void trace_create(const void *data, const char *type);
  void trace_destroy(const void *data, const char *type);
  void trace(const char *fmt, ...) __attribute__ ((format (printf, 1, 2)));
-void dtrace_io(const struct connection *conn, const struct buffered_data 
*data, int out);
  void reopen_log(void);
  void close_log(void);
  
@@ -250,9 +249,6 @@ static inline int xenbus_master_domid(void) { return dom0_domid; }

  /* Return the event channel used by xenbus. */
  evtchn_port_t xenbus_evtchn(void);
  
-/* Tell the kernel xenstored is running. */

-void xenbus_notify_running(void);
-
  /* Write out the pidfile */
  void write_pidfile(const char *pidfile);
  
diff --git a/tools/xenstore/xenstored_minios.c b/tools/xenstore/xenstored_minios.c

index c94493e52a..aa384e50c8 100644
--- a/tools/xenstore/xenstored_minios.c
+++ b/tools/xenstore/xenstored_minios.c
@@ -38,10 +38,6 @@ void init_pipe(int reopen_log_pipe[2])
reopen_log_pipe[1] = -1;
  }
  
-void xenbus_notify_running(void)

-{
-}
-
  evtchn_port_t xenbus_evtchn(void)
  {
return dom0_event;
diff --git a/tools/xenstore/xenstored_posix.c b/tools/xenstore/xenstored_posix.c
index 48c37ffe3e..b20504d1b6 100644
--- a/tools/xenstore/xenstored_posix.c
+++ b/tools/xenstore/xenstored_posix.c
@@ -111,7 +111,6 @@ void unmap_xenbus(void *interface)
munmap(interface, getpagesize());
  }
  
-#ifndef __sun__

  evtchn_port_t xenbus_evtchn(void)
  {
int fd;
@@ -158,8 +157,3 @@ void *xenbus_map(void)
  
  	return addr;

  }
-
-void xenbus_notify_

Re: [RFC v1 1/5] xen/arm: add support for Renesas R-Car Gen3 platform

[Oleksii Moisieiev]

Hi Julien,

On Wed, Dec 15, 2021 at 09:39:32AM +, Julien Grall wrote:
> Hi,
> 
> Looking at the rest of the series, this is going to be replaced in patch #2
> with:
> 
> return sci_handle_call();
> 
> SCMI is not specific to RCAR3. So I would expect the function to be called
> from common code.
> 
> If it still needs some platform specific code, then I think it would be best
> to introduce rcar3.c at the end of the series. So we don't introduce a dummy
> platform and not hook the code in the middle of patch#2 which is meant to be
> generic.
> 
> I will have a proper review of the rest of the series in the new year.
> 
> Cheers,
> 
> -- 
> Julien Grall

That's sound reasonable. My first thought was to move SCM func_id to the
different section, such as Arm Architecture Service (see Section 6
of DEN0028D). But I think that SiP service func_id fits best in this
case because from guest standpoint all clocks\resets\power-domain ids
are SoC implementation specific.

I'm going to leave SMC func_id in SiP range, but refactor SIP smc
handler, so no RCAR3 specific code will be needed. So there will be no
need to introduce rcar3.c in this patch series.

What do you think about that? 

Best regards,
Oleksii.

Re: [RFC v1 1/5] xen/arm: add support for Renesas R-Car Gen3 platform

[Oleksii Moisieiev]

Hi Oleksandr,

On Wed, Dec 15, 2021 at 11:57:29AM +0200, Oleksandr Tyshchenko wrote:
> On Tue, Dec 14, 2021 at 11:35 AM Oleksii Moisieiev <
> oleksii_moisie...@epam.com> wrote:
> 
> Hi Oleksii
> 
> [sorry for the possible format issues]
> 
> Implementation includes platform-specific smc handler for rcar3 platform.
> >
> > Signed-off-by: Oleksii Moisieiev 
> > ---
> >  xen/arch/arm/platforms/Makefile |  1 +
> >  xen/arch/arm/platforms/rcar3.c  | 46 +
> >  2 files changed, 47 insertions(+)
> >  create mode 100644 xen/arch/arm/platforms/rcar3.c
> >
> > diff --git a/xen/arch/arm/platforms/Makefile
> > b/xen/arch/arm/platforms/Makefile
> > index 8632f4115f..b64c25de6c 100644
> > --- a/xen/arch/arm/platforms/Makefile
> > +++ b/xen/arch/arm/platforms/Makefile
> > @@ -4,6 +4,7 @@ obj-$(CONFIG_ALL32_PLAT) += exynos5.o
> >  obj-$(CONFIG_ALL32_PLAT) += midway.o
> >  obj-$(CONFIG_ALL32_PLAT) += omap5.o
> >  obj-$(CONFIG_ALL32_PLAT) += rcar2.o
> > +obj-$(CONFIG_RCAR3) += rcar3.o
> >  obj-$(CONFIG_ALL64_PLAT) += seattle.o
> >  obj-$(CONFIG_ALL_PLAT)   += sunxi.o
> >  obj-$(CONFIG_ALL64_PLAT) += thunderx.o
> > diff --git a/xen/arch/arm/platforms/rcar3.c
> > b/xen/arch/arm/platforms/rcar3.c
> > new file mode 100644
> > index 00..d740145c71
> > --- /dev/null
> > +++ b/xen/arch/arm/platforms/rcar3.c
> > @@ -0,0 +1,46 @@
> > +/*
> > + * xen/arch/arm/platforms/rcar3.c
> > + *
> > + * Renesas R-Car Gen3 specific settings
> > + *
> > + * Oleksii Moisieiev 
> > + * Copyright (C) 2021 EPAM Systems
> > + *
> > + * 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 
> > +
> > +static bool rcar3_smc(struct cpu_user_regs *regs)
> > +{
> > +return false;
> > +}
> > +
> > +static const char *const rcar3_dt_compat[] __initconst =
> > +{
> > +"renesas,r8a7795",
> > +"renesas,r8a7796",
> >
> 
> 
> Please note that since Linux commit:
> "9c9f7891093b02eb64ca4e1c7ab776a4296c058f soc: renesas: Identify R-Car
> M3-W+"
> the compatible string for R-Car M3-W+ (ES3.0) SoC is "renesas,r8a77961". So
> in case we want to have vSCMI feature on this new SoC revision as well we
> will need
> to extend the compatible list.
> 

Thank you for the point, this will be fixed in v2.

Best regards,
Oleksii

> 
> +NULL
> > +};
> > +
> > +PLATFORM_START(rcar3, "Renesas R-Car Gen3")
> > +.compatible = rcar3_dt_compat,
> > +.smc = rcar3_smc
> > +PLATFORM_END
> > +
> > +/*
> > + * Local variables:
> > + * mode: C
> > + * c-file-style: "BSD"
> > + * c-basic-offset: 4
> > + * indent-tabs-mode: nil
> > + * End:
> > + */
> > --
> > 2.27.0
> >
> >
> 
> -- 
> Regards,
> 
> Oleksandr Tyshchenko

Re: [RFC v1 4/5] tools/arm: add "scmi_smc" option to xl.cfg

[Oleksii Moisieiev]

Hi Oleksandr,

On Wed, Dec 15, 2021 at 11:51:01PM +0200, Oleksandr wrote:
> 
> On 14.12.21 11:34, Oleksii Moisieiev wrote:
> 
> 
> Hi Oleksii
> 
> > This enumeration sets SCI type for the domain. Currently there is
> > two possible options: either 'none' or 'scmi_smc'.
> > 
> > 'none' is the default value and it disables SCI support at all.
> > 
> > 'scmi_smc' enables access to the Firmware from the domains using SCMI
> > protocol and SMC as transport.
> > 
> > Signed-off-by: Oleksii Moisieiev 
> > ---
> >   docs/man/xl.cfg.5.pod.in | 22 ++
> >   tools/include/libxl.h|  5 +
> >   tools/libs/light/libxl_types.idl |  6 ++
> >   tools/xl/xl_parse.c  |  9 +
> >   4 files changed, 42 insertions(+)
> > 
> > diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> > index b98d161398..92d0593875 100644
> > --- a/docs/man/xl.cfg.5.pod.in
> > +++ b/docs/man/xl.cfg.5.pod.in
> > @@ -1614,6 +1614,28 @@ This feature is a B.
> >   =back
> > +=item B
> > +
> > +B Set SCI type for the guest. SCI is System Control Protocol -
> > +allows domain to manage various functions that are provided by HW platform.
> > +
> > +=over 4
> > +
> > +=item B
> > +
> > +Don't allow guest to use SCI if present on the platform. This is the 
> > default
> > +value.
> > +
> > +=item B
> > +
> > +Enables SCMI-SMC support for the guest. SCMI is System Control Management
> > +Inferface - allows domain to manage various functions that are provided by 
> > HW
> > +platform, such as clocks, resets and power-domains. Xen will mediate 
> > access to
> > +clocks, power-domains and resets between Domains and ATF. Disabled by 
> > default.
> > +SMC is used as transport.
> > +
> > +=back
> > +
> >   =back
> >   =head2 Paravirtualised (PV) Guest Specific Options
> > diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> > index 2bbbd21f0b..30e5aee119 100644
> > --- a/tools/include/libxl.h
> > +++ b/tools/include/libxl.h
> > @@ -278,6 +278,11 @@
> >*/
> >   #define LIBXL_HAVE_BUILDINFO_ARCH_ARM_TEE 1
> > +/*
> > + * libxl_domain_build_info has the arch_arm.sci field.
> > + */
> > +#define LIBXL_HAVE_BUILDINFO_ARCH_ARM_SCI 1
> > +
> >   /*
> >* LIBXL_HAVE_SOFT_RESET indicates that libxl supports performing
> >* 'soft reset' for domains and there is 'soft_reset' shutdown reason
> > diff --git a/tools/libs/light/libxl_types.idl 
> > b/tools/libs/light/libxl_types.idl
> > index 2a42da2f7d..9067b509f4 100644
> > --- a/tools/libs/light/libxl_types.idl
> > +++ b/tools/libs/light/libxl_types.idl
> 
> I assume that at least goland bindings want updating.
> 

Thanks for the notice. I will fix that in v2.

> 
> > @@ -480,6 +480,11 @@ libxl_tee_type = Enumeration("tee_type", [
> >   (1, "optee")
> >   ], init_val = "LIBXL_TEE_TYPE_NONE")
> > +libxl_sci_type = Enumeration("sci_type", [
> > +(0, "none"),
> > +(1, "scmi_smc")
> > +], init_val = "LIBXL_SCI_TYPE_NONE")
> > +
> >   libxl_rdm_reserve = Struct("rdm_reserve", [
> >   ("strategy",libxl_rdm_reserve_strategy),
> >   ("policy",  libxl_rdm_reserve_policy),
> > @@ -564,6 +569,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
> >   ("apic", libxl_defbool),
> >   ("dm_restrict",  libxl_defbool),
> >   ("tee",  libxl_tee_type),
> > +("sci",  libxl_sci_type),
> >   ("u", KeyedUnion(None, libxl_domain_type, "type",
> >   [("hvm", Struct(None, [("firmware", string),
> >  ("bios", 
> > libxl_bios_type),
> > diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> > index 117fcdcb2b..c37bf6298b 100644
> > --- a/tools/xl/xl_parse.c
> > +++ b/tools/xl/xl_parse.c
> > @@ -2747,6 +2747,15 @@ skip_usbdev:
> >   }
> >   }
> > +if (!xlu_cfg_get_string (config, "sci", &buf, 1)) {
> > +e = libxl_sci_type_from_string(buf, &b_info->sci);
> > +if (e) {
> > +fprintf(stderr,
> > +"Unknown sci \"%s\" specified\n", buf);
> > +exit(-ERROR_FAIL);
> > +}
> > +}
> > +
> >   parse_vkb_list(config, d_config);
> >   xlu_cfg_get_defbool(config, "xend_suspend_evtchn_compat",
> 
> -- 
> Regards,
> 
> Oleksandr Tyshchenko
> 

Re: [PATCH v3 02/13] xen: harmonize return types of hypercall handlers

[Juergen Gross]

On 17.12.21 11:41, Julien Grall wrote:

Hi Juergen,

On 17/12/2021 08:50, Juergen Gross wrote:

On 17.12.21 08:45, Jan Beulich wrote:

On 17.12.2021 06:34, Juergen Gross wrote:

On 16.12.21 22:15, Stefano Stabellini wrote:

On Thu, 16 Dec 2021, Stefano Stabellini wrote:

On Thu, 16 Dec 2021, Juergen Gross wrote:

On 16.12.21 03:10, Stefano Stabellini wrote:

The case of XENMEM_maximum_ram_page is interesting but it is not a
problem in reality because the max physical address size is only 
40-bit
for aarch32 guests, so 32-bit are always enough to return the 
highest

page in memory for 32-bit guests.


You are aware that this isn't the guest's max page, but the host's?


I can see now that you meant to say that, no matter what is the max
pseudo-physical address supported by the VM, 
XENMEM_maximum_ram_page is

supposed to return the max memory page, which could go above the
addressibility limit of the VM.

So XENMEM_maximum_ram_page should potentially be able to return 
(1<<44)

even when called by an aarch32 VM, with max IPA 40-bit.

I would imagine it could be useful if dom0 is 32-bit but domUs are
64-bit on a 64-bit hypervisor (which I think it would be a very rare
configuration on ARM.)

Then it looks like XENMEM_maximum_ram_page needs to be able to 
return a

value > 32-bit when called by a 32-bit guest.

The hypercall ABI follows the ARM C calling convention, so a 64-bit
value should be returned using r0 and r1. But looking at
xen/arch/arm/traps.c:do_trap_hypercall, it doesn't seem it ever 
sets r1

today. Only r0 is set, so effectively we only support 32-bit return
values on aarch32 and for aarch32 guests.

In other words, today all hypercalls on ARM return 64-bit to 64-bit
guests and 32-bit to 32-bit guests. Which in the case of memory_op is
"technically" the correct thing to do because it matches the C
declaration in xen/include/xen/hypercall.h:

extern long
do_memory_op(
  unsigned long cmd,
  XEN_GUEST_HANDLE_PARAM(void) arg);

So...  I guess the conclusion is that on ARM do_memory_op should 
return

"long" although it is not actually enough for a correct implementation
of XENMEM_maximum_ram_page for aarch32 guests ?



Hence my suggestion to check the return value of _all_ hypercalls to be
proper sign extended int values for 32-bit guests. This would fix all
potential issues without silently returning truncated values.


Are we absolutely certain we have no other paths left where a possibly
large unsigned values might be returned? In fact while
compat_memory_op() does the necessary saturation, I've never been fully
convinced of this being the best way of dealing with things. The range
of error indicators is much smaller than [-INT_MIN,-1], so almost
double the range of effectively unsigned values could be passed back
fine. (Obviously we can't change existing interfaces, so this mem-op
will need to remain as is.)


In fact libxenctrl tries do deal with this fact by wrapping a memory_op
for a 32-bit environment into a multicall. This will work fine for a
32-bit Arm guest, as xen_ulong_t is a uint64 there.

So do_memory_op should return long on Arm, yes. OTOH doing so will
continue to be a problem in case a 32-bit guest doesn't use the
multicall technique for handling possible 64-bit return values.

So I continue to argue that on Arm the return value of a hypercall
should be tested to fit into 32 bits. 


It would make sense. But what would you return if the value doesn't fit?


I guess some errno value would be appropriate, like -EDOM, -ERANGE or
-E2BIG.




The only really clean alternative
would be to have separate hypercall function classes for Arm 32- and
64-bit guests (which still could share most of the functions by letting
those return "int"). This would allow to use the 64-bit variant even for
32-bit guests in multicall (fine as the return field is 64-bit wide),
and a probably saturating compat version for the 32-bit guest direct
hypercall.


I am not entirely sure to understand this proposal. Can you clarify it?


1. In patch 5 modify the hypercall table by adding another column, so
   instead of:
   +table:   pv32 pv64 hvm32hvm64arm
   use:
   +table:   pv32 pv64 hvm32hvm64arm32arm64

2. Let most of the hypercalls just return int instead of long:
   +rettype: do int

3. Have an explicit 64-bit variant of memory_op (the 32-bit one is the
   compat variant existing already):
   +rettype: do64 long
   +prefix: do64 PREFIX_hvm
   +memory_op(unsigned long cmd, void *arg)

4. Use the appropriate calls in each column:
   +memory_op compat   do64 hvm  hvm  compat  do64

5. In the Arm hypercall trap handler do:
   if ( is_32bit_domain(current->domain) )
   call_handlers_arm32(...);
   else
   call_handlers_arm64(...);

6. In the multicall handler always do:
   call_handlers_arm64(...);


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital s

Re: [PATCH] xen/arm: vpci: Remove PCI I/O ranges property value

[Julien Grall]

Hi Rahul,

I have a few comments on top of what Stefano already wrote.

On 14/12/2021 10:45, Rahul Singh wrote:

IO ports on ARM don't exist so all IO ports related hypercalls are going
to fail on ARM when we passthrough a PCI device.


Well. Arm doesn't have specific instructions to access I/O port. But 
they still exists because they are mapped in the memory address space.


It is quite likely we would need the xc_domain_ioport_permission() & co 
to work on Arm once we decide to expose the I/O region to the guest.



Failure of xc_domain_ioport_permission(..) would turn into a critical
failure at domain creation. We need to avoid this outcome, instead we
want to continue with domain creation as normal even if
xc_domain_ioport_permission(..) fails. XEN_DOMCTL_ioport_permission
is not implemented on ARM so it would return -ENOSYS.

To solve above issue remove PCI I/O ranges property value from dom0
device tree node so that dom0 linux will not allocate I/O space for PCI
devices if pci-passthrough is enabled.

Another valid reason to remove I/O ranges is that PCI I/O space are not
mapped to dom0 when PCI passthrough is enabled, also there is no vpci
trap handler register for IO bar.


TBH, this should be the main reason of this change. We should not expose 
the PCI I/O space because the vPCI is not supporting it.


The rest is just an implementation details to avoid major refactoring 
that may need some revert in the future.




Signed-off-by: Rahul Singh 
---
  xen/arch/arm/domain_build.c   | 14 +++
  xen/common/device_tree.c  | 72 +++
  xen/include/xen/device_tree.h | 10 +
  3 files changed, 96 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d02bacbcd1..60f6b2c73b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -749,6 +749,11 @@ static int __init write_properties(struct domain *d, 
struct kernel_info *kinfo,
  continue;
  }
  
+if ( is_pci_passthrough_enabled() &&

+dt_device_type_is_equal(node, "pci") )


This check is not going to change for a given node. So I think this 
wants to be moved outside of the loop to avoid expensive check.


In addition to that, this may also cover PCI devices. I think we want to 
use the same heuristic as in handle_linux_pci_domain(). So I would move 
the logic in a separate helper.



+if ( dt_property_name_is_equal(prop, "ranges") )
+continue;
+
  res = fdt_property(kinfo->fdt, prop->name, prop_data, prop_len);
  
  if ( res )

@@ -769,6 +774,15 @@ static int __init write_properties(struct domain *d, 
struct kernel_info *kinfo,
  if ( res )
  return res;
  }
+
+/*
+ * PCI IO bar are not mapped to dom0 when PCI passthrough is enabled,
+ * also there is no trap handler registered for IO bar therefor remove


Typo: s/therefor/therefore/


+ * the IO range property from the device tree node for dom0.
+ */
+res = dt_pci_remove_io_ranges(kinfo->fdt, node);


This is called unconditionally. Couldn't this potentially misinterpret 
some node?



+if ( res )
+return res;
  }
  
  /*

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 4aae281e89..9fa25f6723 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -2195,6 +2195,78 @@ int dt_get_pci_domain_nr(struct dt_device_node *node)
  return (u16)domain;
  }
  
+int dt_pci_remove_io_ranges(void *fdt, const struct dt_device_node *dev)

+{
+const struct dt_device_node *parent = NULL;
+const struct dt_bus *bus, *pbus;
+unsigned int rlen;
+int na, ns, pna, pns, rone, ret;
+const __be32 *ranges;
+__be32 regs[((GUEST_ROOT_ADDRESS_CELLS * 2) + GUEST_ROOT_SIZE_CELLS + 1)
+   * 2];
+__be32 *addr = ®s[0];
+
+bus = dt_match_bus(dev);
+if ( !bus )


NIT: I don't particularly care whether we use !bus or bus == 0 but it 
would be nice to stay consistent at least within a function (below you 
use rlen == 0).



+return 0; /* device is not a bus */
+
+parent = dt_get_parent(dev);
+if ( parent == NULL )
+return -EINVAL;
+
+ranges = dt_get_property(dev, "ranges", &rlen);
+if ( ranges == NULL )
+{
+printk(XENLOG_ERR "DT: no ranges; cannot enumerate %s\n",
+   dev->full_name);
+return -EINVAL;
+}
+if ( rlen == 0 ) /* Nothing to do */
+return 0;
+
+bus->count_cells(dev, &na, &ns);
+if ( !DT_CHECK_COUNTS(na, ns) )
+{
+printk(XENLOG_ERR "dt_parse: Bad cell count for device %s\n",
+  dev->full_name);
+return -EINVAL;
+}
+pbus = dt_match_bus(parent);
+if ( pbus == NULL )
+{
+printk("DT: %s is not a valid bus\n", parent->full_name);
+return -EINVAL;
+}
+
+pbus->count_cells(dev, &pna, &pns);
+ 

Re: [RFC v1 3/5] xen/arm: introduce SCMI-SMC mediator driver

[Oleksandr]

On 14.12.21 11:34, Oleksii Moisieiev wrote:


Hi Oleksii


This is the implementation of SCI interface, called SCMI-SMC driver,
which works as the mediator between XEN Domains and Firmware (SCP, ATF etc).
This allows devices from the Domains to work with clocks, resets and
power-domains without access to CPG.

The following features are implemented:
- request SCMI channels from ATF and pass channels to Domains;
- set device permissions for Domains based on the Domain partial
device-tree. Devices with permissions are able to work with clocks,
resets and power-domains via SCMI;
- redirect scmi messages from Domains to ATF.

Signed-off-by: Oleksii Moisieiev 
---
  xen/arch/arm/Kconfig  |   2 +
  xen/arch/arm/sci/Kconfig  |  10 +
  xen/arch/arm/sci/Makefile |   1 +
  xen/arch/arm/sci/scmi_smc.c   | 795 ++
  xen/include/public/arch-arm.h |   1 +
  5 files changed, 809 insertions(+)
  create mode 100644 xen/arch/arm/sci/Kconfig
  create mode 100644 xen/arch/arm/sci/scmi_smc.c

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 186e1db389..02d96c6cfc 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -114,6 +114,8 @@ config SCI
  support. It allows guests to control system resourcess via one of
  SCI mediators implemented in XEN.
  
+source "arch/arm/sci/Kconfig"

+
  endmenu
  
  menu "ARM errata workaround via the alternative framework"

diff --git a/xen/arch/arm/sci/Kconfig b/xen/arch/arm/sci/Kconfig
new file mode 100644
index 00..9563067ddc
--- /dev/null
+++ b/xen/arch/arm/sci/Kconfig
@@ -0,0 +1,10 @@
+config SCMI_SMC
+   bool "Enable SCMI-SMC mediator driver"
+   default n
+   depends on SCI
+   ---help---
+
+   Enables mediator in XEN to pass SCMI requests from Domains to ATF.
+   This feature allows drivers from Domains to work with System
+   Controllers (such as power,resets,clock etc.). SCP is used as transport
+   for communication.
diff --git a/xen/arch/arm/sci/Makefile b/xen/arch/arm/sci/Makefile
index 837dc7492b..67f2611872 100644
--- a/xen/arch/arm/sci/Makefile
+++ b/xen/arch/arm/sci/Makefile
@@ -1 +1,2 @@
  obj-y += sci.o
+obj-$(CONFIG_SCMI_SMC) += scmi_smc.o
diff --git a/xen/arch/arm/sci/scmi_smc.c b/xen/arch/arm/sci/scmi_smc.c
new file mode 100644
index 00..2eb01ea82d
--- /dev/null
+++ b/xen/arch/arm/sci/scmi_smc.c
@@ -0,0 +1,795 @@
+/*
+ * xen/arch/arm/sci/scmi_smc.c
+ *
+ * SCMI mediator driver, using SCP as transport.
+ *
+ * Oleksii Moisieiev 
+ * Copyright (C) 2021, EPAM Systems.
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SCMI_BASE_PROTOCOL  0x10
+#define SCMI_BASE_PROTOCOL_ATTIBUTES0x1
+#define SCMI_BASE_SET_DEVICE_PERMISSIONS0x9
+#define SCMI_BASE_RESET_AGENT_CONFIGURATION 0xB
+#define SCMI_BASE_DISCOVER_AGENT0x7
+
+/* SCMI return codes. See section 4.1.4 of SCMI spec (DEN0056C) */
+#define SCMI_SUCCESS  0
+#define SCMI_NOT_SUPPORTED  (-1)
+#define SCMI_INVALID_PARAMETERS (-2)
+#define SCMI_DENIED (-3)
+#define SCMI_NOT_FOUND  (-4)
+#define SCMI_OUT_OF_RANGE   (-5)
+#define SCMI_BUSY   (-6)
+#define SCMI_COMMS_ERROR(-7)
+#define SCMI_GENERIC_ERROR  (-8)
+#define SCMI_HARDWARE_ERROR (-9)
+#define SCMI_PROTOCOL_ERROR (-10)
+
+#define DT_MATCH_SCMI_SMC DT_MATCH_COMPATIBLE("arm,scmi-smc")
+
+#define SCMI_SMC_ID"arm,smc-id"
+#define SCMI_SHARED_MEMORY "linux,scmi_mem"
+#define SCMI_SHMEM "shmem"
+
+#define HYP_CHANNEL  0x0
+
+#define HDR_ID GENMASK(7,0)
+#define HDR_TYPE   GENMASK(9, 8)
+#define HDR_PROTO  GENMASK(17, 10)
+
+/* SCMI protocol, refer to section 4.2.2.2 (DEN0056C) */
+#define MSG_N_AGENTS_MASK  GENMASK(15, 8)
+
+#define FIELD_GET(_mask, _reg)\
+((typeof(_mask))(((_reg) & (_mask)) >> (ffs64(_mask) - 1)))
+#define FIELD_PREP(_mask, _val)\
+(((typeof(_mask))(_val) << (ffs64(_mask) - 1)) & (_mask))
+
+typedef struct scmi_msg_header {
+uint8_t id;
+uint8_t type;
+uint8_t protocol;
+} scmi_msg_header_t;
+
+typedef struct scmi_perms_tx {
+uint32_t agent_id;
+uint32_t device_id;
+

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

[osstest service owner]

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

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  1c4589280ae4e9ba34266e674459fffd6f0282dc
baseline version:
 xen  f3999bc2e099c571e4583bff8f494b834b2f5f76

Last test of basis   167456  2021-12-17 03:02:55 Z0 days
Testing same since   167458  2021-12-17 08:01:39 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 

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
   f3999bc2e0..1c4589280a  1c4589280ae4e9ba34266e674459fffd6f0282dc -> smoke

Re: [PATCH v3] xen/arm64: Zero the top 32 bits of gp registers on entry...

[Michal Orzel]

Hi Julien,

On 17.12.2021 11:01, Julien Grall wrote:
> Hi,
> 
> On 17/12/2021 07:21, Michal Orzel wrote:
>> to hypervisor when switching from AArch32 state.
>>
>> According to section D1.20.2 of Arm Arm(DDI 0487A.j):
>> "If the general-purpose register was accessible from AArch32 state the
>> upper 32 bits either become zero, or hold the value that the same
>> architectural register held before any AArch32 execution.
>> The choice between these two options is IMPLEMENTATION DEFINED"
>>
>> Currently Xen does not ensure that the top 32 bits are zeroed and this
>> needs to be fixed. The reason why is that there are places in Xen
>> where we assume that top 32bits are zero for AArch32 guests.
>> If they are not, this can lead to misinterpretation of Xen regarding
>> what the guest requested. For example hypercalls returning an error
>> encoded in a signed long like do_sched_op, do_hmv_op, do_memory_op
>> would return -ENOSYS if the command passed as the first argument was
>> clobbered.
>>
>> Create a macro clobber_gp_top_halves to clobber top 32 bits of gp
>> registers when hyp == 0 (guest mode) and compat == 1 (AArch32 mode).
>> Add a compile time check to ensure that save_x0_x1 == 1 if
>> compat == 1.
>>
>> Signed-off-by: Michal Orzel 
>> ---
>> Changes since v2:
>> -add clobbering of w30
>> Changes since v1:
>> -put new code into macro
>> -add compile time check for save_x0_x1
>> ---
>>   xen/arch/arm/arm64/entry.S | 29 +
>>   1 file changed, 29 insertions(+)
>>
>> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
>> index fc3811ad0a..e351ef8639 100644
>> --- a/xen/arch/arm/arm64/entry.S
>> +++ b/xen/arch/arm/arm64/entry.S
>> @@ -102,6 +102,30 @@
>>   .endif
>>     .endm
>> +
>> +/*
>> + * Clobber top 32 bits of gp registers when switching from AArch32
>> + */
>> +    .macro clobber_gp_top_halves, compat, save_x0_x1
>> +
>> +    .if \compat == 1  /* AArch32 mode */
>> +
>> +    /*
>> + * save_x0_x1 is equal to 0 only for guest_sync (compat == 0).
>> + * Add a compile time check to avoid violating this rule.
>> + */
> 
> We may want in the future to allow save_x0_x1 == 1 with compat == 1 if we 
> need to create fastpath like we did when entering AArch64.
> 
> So I would reword the comment to make clear this is an implementation 
> decision. How about:
> 
> "At the moment, no-one is using save_x0_x1 == 0 with compat == 1. So the code 
> is not handling it to simplify the implementation."
> 
> If you are happy with the new comment, I can update it on commit:
> 
Please do. The comment looks ok.

> Acked-by: Julien Grall 
> 
> Cheers,
> 

Cheers,
Michal

[libvirt test] 167457: regressions - FAIL

[osstest service owner]

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

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  e4e873e9b653f4b8aa8c2ed64ce2d607e619e93d
baseline version:
 libvirt  2c846fa6bcc11929c9fb857a22430fb9945654ad

Last test of basis   151777  2020-07-10 04:19:19 Z  525 days
Failing since151818  2020-07-11 04:18:52 Z  524 days  506 attempts
Testing same since   167457  2021-12-17 04:18:55 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 
  Dmitrii Shcherbakov 
  Dmytro Linkin 
  Eiichi Tsukata 
  Eric Farman 
  Erik Skultety 
  Fabian Affolter 
  Fabian Freyer 
  Fabiano Fidêncio 
  Fangge Jin 
  Farhan Ali 
  Fedora Weblate Translation 
  Franck Ridel 
  Gavi Teitz 
  gongwei 
  Guoyi Tu
  Göran Uddeborg 
  Halil Pasic 
  Han Han 
  Hao Wang 
  Hela Basa 
  Helmut Grohne 
  Hiroki Narukawa 
  Hyman Huang(黄勇) 
  Ian Wienand 
  Ioanna Alifieraki 
  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 
  Joachim Falk 
  John Ferlan 
  Jonathan Watt 
  Jonathon Jongsma 
  Julio Faracco 
  Justin Gatzen 
  Ján Tomko 
  Kashyap Chamarthy 
  Kevin Locke 
  Koichi Murase 
  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 
  Or Ozeri 
  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 

Re: [RFC v1 5/5] xen/arm: add SCI mediator support for DomUs

[Oleksii Moisieiev]

Hi Oleksandr,

On Thu, Dec 16, 2021 at 02:04:35AM +0200, Oleksandr wrote:
> 
> On 14.12.21 11:34, Oleksii Moisieiev wrote:
> 
> Hi Oleksii
> 
> > Integration of the SCMI mediator with xen libs:
> > - add hypercalls to aquire SCI channel and set device permissions
> > for DomUs;
> > - add SCMI_SMC nodes to DomUs device-tree based on partial device-tree;
> > - SCI requests redirection from DomUs to Firmware.
> > 
> > Signed-off-by: Oleksii Moisieiev 
> > ---
> >   tools/include/xenctrl.h   |   3 +
> >   tools/include/xenguest.h  |   2 +
> >   tools/libs/ctrl/xc_domain.c   |  23 ++
> >   tools/libs/guest/xg_dom_arm.c |   5 +-
> >   tools/libs/light/libxl_arm.c  | 122 +++---
> >   tools/libs/light/libxl_create.c   |  54 -
> >   tools/libs/light/libxl_dom.c  |   1 +
> >   tools/libs/light/libxl_internal.h |   4 +
> >   xen/arch/arm/domctl.c |  15 
> >   xen/include/public/domctl.h   |   9 +++
> >   10 files changed, 223 insertions(+), 15 deletions(-)
> > 
> > diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
> > index 07b96e6671..cdd14f465f 100644
> > --- a/tools/include/xenctrl.h
> > +++ b/tools/include/xenctrl.h
> > @@ -1238,6 +1238,9 @@ int xc_domain_getvnuma(xc_interface *xch,
> >   int xc_domain_soft_reset(xc_interface *xch,
> >uint32_t domid);
> > +int xc_domain_add_sci_device(xc_interface *xch,
> > +  uint32_t domid, char *path);
> > +
> >   #if defined(__i386__) || defined(__x86_64__)
> >   /*
> >* PC BIOS standard E820 types and structure.
> > diff --git a/tools/include/xenguest.h b/tools/include/xenguest.h
> > index 61d0a82f48..35c611ac73 100644
> > --- a/tools/include/xenguest.h
> > +++ b/tools/include/xenguest.h
> > @@ -242,6 +242,8 @@ struct xc_dom_image {
> >   /* Number of vCPUs */
> >   unsigned int max_vcpus;
> > +
> > +xen_pfn_t sci_shmem_gfn;
> >   };
> >   /* --- arch specific hooks - */
> > diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
> > index b155d6afd2..07bb390e17 100644
> > --- a/tools/libs/ctrl/xc_domain.c
> > +++ b/tools/libs/ctrl/xc_domain.c
> > @@ -2206,6 +2206,29 @@ int xc_domain_soft_reset(xc_interface *xch,
> >   domctl.domain = domid;
> >   return do_domctl(xch, &domctl);
> >   }
> > +
> > +int xc_domain_add_sci_device(xc_interface *xch,
> > +  uint32_t domid, char *path)
> > +{
> > +size_t size = strlen(path);
> > +int rc;
> > +DECLARE_DOMCTL;
> > +DECLARE_HYPERCALL_BOUNCE(path, size, XC_HYPERCALL_BUFFER_BOUNCE_IN);
> > +
> > +if ( xc_hypercall_bounce_pre(xch, path) )
> > +return -1;
> > +
> > +domctl.cmd = XEN_DOMCTL_add_sci_device;
> > +domctl.domain = domid;
> > +domctl.u.sci_device_op.size = size;
> > +set_xen_guest_handle(domctl.u.sci_device_op.path, path);
> > +rc = do_domctl(xch, &domctl);
> > +
> > +xc_hypercall_bounce_post(xch, path);
> > +
> > +return rc;
> > +}
> > +
> >   /*
> >* Local variables:
> >* mode: C
> > diff --git a/tools/libs/guest/xg_dom_arm.c b/tools/libs/guest/xg_dom_arm.c
> > index 5e3b76355e..368a670c46 100644
> > --- a/tools/libs/guest/xg_dom_arm.c
> > +++ b/tools/libs/guest/xg_dom_arm.c
> > @@ -25,11 +25,12 @@
> >   #include "xg_private.h"
> > -#define NR_MAGIC_PAGES 4
> > +#define NR_MAGIC_PAGES 5
> >   #define CONSOLE_PFN_OFFSET 0
> >   #define XENSTORE_PFN_OFFSET 1
> >   #define MEMACCESS_PFN_OFFSET 2
> >   #define VUART_PFN_OFFSET 3
> > +#define SCI_SHMEM_OFFSET 4
> >   #define LPAE_SHIFT 9
> > @@ -69,11 +70,13 @@ static int alloc_magic_pages(struct xc_dom_image *dom)
> >   dom->console_pfn = base + CONSOLE_PFN_OFFSET;
> >   dom->xenstore_pfn = base + XENSTORE_PFN_OFFSET;
> >   dom->vuart_gfn = base + VUART_PFN_OFFSET;
> > +dom->sci_shmem_gfn = base + SCI_SHMEM_OFFSET;
> >   xc_clear_domain_page(dom->xch, dom->guest_domid, dom->console_pfn);
> >   xc_clear_domain_page(dom->xch, dom->guest_domid, dom->xenstore_pfn);
> >   xc_clear_domain_page(dom->xch, dom->guest_domid, base + 
> > MEMACCESS_PFN_OFFSET);
> >   xc_clear_domain_page(dom->xch, dom->guest_domid, dom->vuart_gfn);
> > +xc_clear_domain_page(dom->xch, dom->guest_domid, dom->sci_shmem_gfn);
> >   xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_CONSOLE_PFN,
> >   dom->console_pfn);
> > diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> > index eef1de0939..73ef591822 100644
> > --- a/tools/libs/light/libxl_arm.c
> > +++ b/tools/libs/light/libxl_arm.c
> > @@ -8,6 +8,8 @@
> >   #include 
> >   #include 
> > +#define SCMI_NODE_PATH  "/firmware/scmi"
> > +
> >   static const char *gicv_to_string(libxl_gic_version gic_version)
> >   {
> >   switch (gic_version) {
> > @@ -101,6 +103,19 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> >   return ERROR_FA

Re: [RFC v1 0/5] Introduce SCI-mediator feature

[Oleksii Moisieiev]

Hi Oleksandr,

On Thu, Dec 16, 2021 at 02:33:28AM +0200, Oleksandr wrote:
> 
> On 14.12.21 11:34, Oleksii Moisieiev wrote:
> 
> 
> Hi Oleksii
> 
> > Introducing the feature, called SCI mediator.
> > It's purpose is to redirect SCMI requests from the domains to firmware
> > (SCP, ATF etc), which controls the power/clock/resets etc.
> > The idea is to make SCP firmware (or similar, such as AT-F) responsible for
> > control power/clock/resets and provide SCMI interface so controls can be 
> > shared
> > between the Domains.
> > Originally, we've met a problem, that the devices, shared between different
> > Domains, can't have an access to HW registers to work with 
> > clocks/resets/power
> > etc. You have to pass cpg to the Domain, so the devices can access HW 
> > directly.
> > The solution for this is to move HW controls over power/clock/resets to
> > SCP firmware and use Linux-kernel SCMI drivers to pass requests to SCP.
> > Xen is responsible for permissions setting, so Domain can access only to
> > power/clock/resets which are related to this Domain. Also XEN is the 
> > mediator
> > which redirects SCMI requests, adding agentID so firmware should know the
> > sender.
> > SMC is currently used as transport, but this should be configurable.
> > 
> > Here is the high level design:
> > 
> > SCI (System Control Interface) feature can be enabled in xen_config:
> > > CONFIG_SCI=y
> > Mediator can be configured:
> > > CONFIG_SCMI_SMC=y
> > Currently, only SCMI_SMC mediator is implemented, which using shared memory
> > region to communicate with firmware and SMC as transport.
> > 
> > Xen scmi should be configured in the device-tree.
> > Format is the following:
> > cpu_scp_shm: scp-shmem@0x53FF {
> > compatible = "arm,scmi-shmem";
> > reg = <0x0 0x53FF 0x0 0x1000>;
> > };
> > 
> > firmware {
> > scmi {
> > compatible = "arm,scmi-smc";
> > arm,smc-id = <0x8202>;
> > shmem = <&cpu_scp_shm>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > 
> > scmi_power: protocol@11 {
> > reg = <0x11>;
> > #power-domain-cells = <1>;
> > };
> > 
> > scmi_clock: protocol@14 {
> > reg = <0x14>;
> > #clock-cells = <1>;
> > };
> > 
> > scmi_reset: protocol@16 {
> > reg = <0x16>;
> > #reset-cells = <1>;
> > };
> > };
> > };
> > 
> > Where:
> > &cpu_scp_shm is the shared memory for scmi buffers;
> > 0x53FF, size 0x1000 is the platform specific free address, which provide
> > space for the communication.
> > &scmi node, which should be copied to Dom0 device-tree.
> > 
> > Device configured to use scmi:
> > &avb {
> > scmi_devid = <0>;
> > clocks = <&scmi_clock 0>;
> > power-domains = <&scmi_power 0>;
> > resets = <&scmi_reset 0>;
> > };
> > 
> > Where:
> > scmi_devid - id from the firmware, which is assigned for AVB.
> > 
> > During initialization, XEN scans probes the first SCI-mediator driver which 
> > has
> > matching node in the device-tree. If no device-tree was provided, then the
> > first registered mediator driver should be probed.
> > 
> > DomX should be configured:
> > Device-tree should include the same nodes, described above.
> > &cpu_scp_shm should be altered during domain creation. Xen allocates free 
> > page
> > from the memory region, provided in &cpu_scp_shm in XEN device-tree, so each
> > domain should have unique page. Nodes &cpu_scp_shm and /firmware/scmi 
> > should be
> > copied from partial device-tree to domain device-tree, so kernel can 
> > initialize
> > scmi driver.
> > 
> > SCI mediator can be enabled in dom.cfg the following way:
> > > sci = "scmi_smc"
> > which sets scmi_smc to be used for the domain.
> 
> 
> Great work! I can imagine this is going to be nice feature once upstreamed.
> 
> I am wondering, would the Xen (with the required updates of course) also be
> able to send it's own requests to the SCP? For example, to control overall
> system performance (CPU frequency)
> 
> or other let's say important power management task.
> 

I think yes. In current implementation Xen register itself as
privilleged agent and use it's own channel to request 
which is used to get agent configuration and process device permissions.
I think this channel can also be used to do some configuration tasks
via SCMI. But this will require updates.

--
Oleksii.
> 
> > 
> > Oleksii Moisieiev (5):
> >xen/arm: add support for Renesas R-Car Gen3 platform
> >xen/arm: add generic SCI mediator framework
> >xen/arm: introduce SCMI-SMC mediator driver
> >tools/arm: add "scmi_smc" option to xl.cfg
> >xen/arm: add SCI mediator support 

Re: [RFC v1 2/5] xen/arm: add generic SCI mediator framework

[Oleksii Moisieiev]

Hi Stefano,

On Thu, Dec 16, 2021 at 06:45:11PM -0800, Stefano Stabellini wrote:
> On Tue, 14 Dec 2021, Oleksii Moisieiev wrote:
> > This patch adds the basic framework for SCI mediator.
> > SCI is System Control Interface, which is designed to redirect
> > requests for the Hardware (such as power-domain/clock/resets etc)
> > from the Domains to the firmware. Originally, cpg should be passed
> > to the domain so it can work with power-domains/clocks/resets etc.
> > Considering that cpg can't be split between the Domains, we get the
> > limitation that the devices, which are using power-domains/clocks/resets
> > etc, couldn't be split between the domains.
> > The solution is to move the power-domain/clock/resets etc to the
> > Firmware (such as SCP firmware or ATF) and provide interface for the 
> > Domains.
> > XEN shoud have an entity, caled SCI-Mediator, which is responsible for 
> > messages
> > redirection between Domains and Firmware and for permission handling.
> > 
> > This is how it works: user can build XEN with multiple SCI mediators.
> > See the next patches, where SCMI-SMC mediator is introduced.
> > SCI mediator register itself with REGISTER_SCI_MEDIATOR() macro.
> > 
> > At run-time, during initialization, framework calls probe for the first
> > matching device in the device-tree. When no device-tree is present - the
> > first registered mediator should be probed.
> > 
> > Signed-off-by: Oleksii Moisieiev 
> > ---
> >  MAINTAINERS|   6 ++
> >  xen/arch/arm/Kconfig   |   8 ++
> >  xen/arch/arm/Makefile  |   1 +
> >  xen/arch/arm/domain.c  |  24 +
> >  xen/arch/arm/domain_build.c|  11 +++
> >  xen/arch/arm/platforms/rcar3.c |   3 +-
> >  xen/arch/arm/sci/Makefile  |   1 +
> >  xen/arch/arm/sci/sci.c | 128 ++
> >  xen/arch/arm/setup.c   |   1 +
> >  xen/arch/arm/xen.lds.S |   7 ++
> >  xen/include/asm-arm/domain.h   |   4 +
> >  xen/include/asm-arm/sci/sci.h  | 162 +
> >  xen/include/public/arch-arm.h  |  10 ++
> >  13 files changed, 365 insertions(+), 1 deletion(-)
> >  create mode 100644 xen/arch/arm/sci/Makefile
> >  create mode 100644 xen/arch/arm/sci/sci.c
> >  create mode 100644 xen/include/asm-arm/sci/sci.h
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index e43dc0edce..5f96ea35ba 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -512,6 +512,12 @@ S: Supported
> >  F: xen/arch/arm/tee/
> >  F: xen/include/asm-arm/tee
> >  
> > +SCI MEDIATORS
> > +M: Oleksii Moisieiev 
> > +S: Supported
> > +F: xen/arch/arm/sci
> > +F: xen/include/asm-arm/sci
> > +
> >  TOOLSTACK
> >  M: Wei Liu 
> >  S: Supported
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index ecfa6822e4..186e1db389 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -106,6 +106,14 @@ config TEE
> >  
> >  source "arch/arm/tee/Kconfig"
> >  
> > +config SCI
> > +   bool "Enable SCI mediators support"
> > +   default n
> > +   help
> > + This option enables generic SCI (System Control Interface) mediators
> > + support. It allows guests to control system resourcess via one of
> > + SCI mediators implemented in XEN.
> > +
> >  endmenu
> >  
> >  menu "ARM errata workaround via the alternative framework"
> > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> > index 07f634508e..6366ff55e5 100644
> > --- a/xen/arch/arm/Makefile
> > +++ b/xen/arch/arm/Makefile
> > @@ -8,6 +8,7 @@ obj-y += platforms/
> >  endif
> >  obj-$(CONFIG_TEE) += tee/
> >  obj-$(CONFIG_HAS_VPCI) += vpci.o
> > +obj-$(CONFIG_SCI) += sci/
> >  
> >  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 96e1b23550..80d0a23767 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -34,6 +34,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -688,6 +689,13 @@ int arch_sanitise_domain_config(struct 
> > xen_domctl_createdomain *config)
> >  return -EINVAL;
> >  }
> >  
> > +if ( config->arch.sci_type != XEN_DOMCTL_CONFIG_SCI_NONE &&
> > + config->arch.sci_type != sci_get_type() )
> > +{
> > +dprintk(XENLOG_INFO, "Unsupported SCI type\n");
> > +return -EINVAL;
> > +}
> > +
> >  return 0;
> >  }
> >  
> > @@ -764,6 +772,15 @@ int arch_domain_create(struct domain *d,
> >  /* At this stage vgic_reserve_virq should never fail */
> >  if ( !vgic_reserve_virq(d, GUEST_EVTCHN_PPI) )
> >  BUG();
> > +
> > +if ( config->arch.sci_type != XEN_DOMCTL_CONFIG_SCI_NONE )
> > +{
> > +if ( (rc = sci_domain_init(d, config->arch.sci_type)) != 0)
> > +goto fail;
> > +
> > +if ( (rc = sci_get_channel_info(d, &config->arch)) != 0)
> > +goto fail;
> > +

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

[osstest service owner]

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds18 guest-start/debian.repeat fail REGR. vs. 167443

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 167443
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 167443
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 167443
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 167443
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 167443
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 167443
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 167443
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 167443
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-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-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-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-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-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-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-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-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 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  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-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass

version targeted for testing:
 qemuu29eb5c2c86f935b0e9700fad2ecfe8a32b011d57
baseline version:
 qemuue630bc7ec9dda656a452ed28cac4d1e9ed605d71

Last test of basis   167443  2021-12-16 03:09:44 Z1 days
Testing same since   167452  2021-12-16 20:09:29 Z0 days1 attempts

---

[linux-linus test] 167451: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 167438
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 167438
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 167438
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 167438
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 167438
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 167438
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 167438
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 167438
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-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-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  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-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-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-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  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-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 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-raw 14 migrate-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
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 linuxfa36bbe6d43f3bbce1f10a187e153587c7584d83
baseline version:
 linux2b14864acbaaf03d9c01982e243a84632524c3ac

Last test of basis   167438  2021-12-15 19:40:05 Z1 days
Testing same since   167451  2021-12-16 19:11:06 Z0 days1 attempts


People who touched revisions under test:
  Catalin Marinas 
  David Howells 
  Joe Thornber 
  kafs-testing+fedora34_64checkkafs-build-...@auristor.com
  Lakshmi Ramasubramanian 
  Linus Torvalds 
  Mike Snitzer 
  Will Deacon 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm  

Re: [RFC v1 3/5] xen/arm: introduce SCMI-SMC mediator driver

[Oleksii Moisieiev]

Hi Oleksandr,

On Fri, Dec 17, 2021 at 01:35:35PM +0200, Oleksandr wrote:
> 
> On 14.12.21 11:34, Oleksii Moisieiev wrote:
> 
> 
> Hi Oleksii
> 
> > This is the implementation of SCI interface, called SCMI-SMC driver,
> > which works as the mediator between XEN Domains and Firmware (SCP, ATF etc).
> > This allows devices from the Domains to work with clocks, resets and
> > power-domains without access to CPG.
> > 
> > The following features are implemented:
> > - request SCMI channels from ATF and pass channels to Domains;
> > - set device permissions for Domains based on the Domain partial
> > device-tree. Devices with permissions are able to work with clocks,
> > resets and power-domains via SCMI;
> > - redirect scmi messages from Domains to ATF.
> > 
> > Signed-off-by: Oleksii Moisieiev 
> > ---
> >   xen/arch/arm/Kconfig  |   2 +
> >   xen/arch/arm/sci/Kconfig  |  10 +
> >   xen/arch/arm/sci/Makefile |   1 +
> >   xen/arch/arm/sci/scmi_smc.c   | 795 ++
> >   xen/include/public/arch-arm.h |   1 +
> >   5 files changed, 809 insertions(+)
> >   create mode 100644 xen/arch/arm/sci/Kconfig
> >   create mode 100644 xen/arch/arm/sci/scmi_smc.c
> > 
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index 186e1db389..02d96c6cfc 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -114,6 +114,8 @@ config SCI
> >   support. It allows guests to control system resourcess via one of
> >   SCI mediators implemented in XEN.
> > +source "arch/arm/sci/Kconfig"
> > +
> >   endmenu
> >   menu "ARM errata workaround via the alternative framework"
> > diff --git a/xen/arch/arm/sci/Kconfig b/xen/arch/arm/sci/Kconfig
> > new file mode 100644
> > index 00..9563067ddc
> > --- /dev/null
> > +++ b/xen/arch/arm/sci/Kconfig
> > @@ -0,0 +1,10 @@
> > +config SCMI_SMC
> > +   bool "Enable SCMI-SMC mediator driver"
> > +   default n
> > +   depends on SCI
> > +   ---help---
> > +
> > +   Enables mediator in XEN to pass SCMI requests from Domains to ATF.
> > +   This feature allows drivers from Domains to work with System
> > +   Controllers (such as power,resets,clock etc.). SCP is used as transport
> > +   for communication.
> > diff --git a/xen/arch/arm/sci/Makefile b/xen/arch/arm/sci/Makefile
> > index 837dc7492b..67f2611872 100644
> > --- a/xen/arch/arm/sci/Makefile
> > +++ b/xen/arch/arm/sci/Makefile
> > @@ -1 +1,2 @@
> >   obj-y += sci.o
> > +obj-$(CONFIG_SCMI_SMC) += scmi_smc.o
> > diff --git a/xen/arch/arm/sci/scmi_smc.c b/xen/arch/arm/sci/scmi_smc.c
> > new file mode 100644
> > index 00..2eb01ea82d
> > --- /dev/null
> > +++ b/xen/arch/arm/sci/scmi_smc.c
> > @@ -0,0 +1,795 @@
> > +/*
> > + * xen/arch/arm/sci/scmi_smc.c
> > + *
> > + * SCMI mediator driver, using SCP as transport.
> > + *
> > + * Oleksii Moisieiev 
> > + * Copyright (C) 2021, EPAM Systems.
> > + *
> > + * 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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define SCMI_BASE_PROTOCOL  0x10
> > +#define SCMI_BASE_PROTOCOL_ATTIBUTES0x1
> > +#define SCMI_BASE_SET_DEVICE_PERMISSIONS0x9
> > +#define SCMI_BASE_RESET_AGENT_CONFIGURATION 0xB
> > +#define SCMI_BASE_DISCOVER_AGENT0x7
> > +
> > +/* SCMI return codes. See section 4.1.4 of SCMI spec (DEN0056C) */
> > +#define SCMI_SUCCESS  0
> > +#define SCMI_NOT_SUPPORTED  (-1)
> > +#define SCMI_INVALID_PARAMETERS (-2)
> > +#define SCMI_DENIED (-3)
> > +#define SCMI_NOT_FOUND  (-4)
> > +#define SCMI_OUT_OF_RANGE   (-5)
> > +#define SCMI_BUSY   (-6)
> > +#define SCMI_COMMS_ERROR(-7)
> > +#define SCMI_GENERIC_ERROR  (-8)
> > +#define SCMI_HARDWARE_ERROR (-9)
> > +#define SCMI_PROTOCOL_ERROR (-10)
> > +
> > +#define DT_MATCH_SCMI_SMC DT_MATCH_COMPATIBLE("arm,scmi-smc")
> > +
> > +#define SCMI_SMC_ID"arm,smc-id"
> > +#define SCMI_SHARED_MEMORY "linux,scmi_mem"
> > +#define SCMI_SHMEM "shmem"
> > +
> > +#define HYP_CHANNEL  0x0
> > +
> > +#define HDR_ID GENMASK(7,0)
> > +#define HDR_TYPE   GENMASK(9, 8)
> > +#define HDR_PROTO

Re: [PATCH v3] xen/arm64: Zero the top 32 bits of gp registers on entry...

[Julien Grall]

Hi Michal,

On 17/12/2021 11:52, Michal Orzel wrote:

On 17.12.2021 11:01, Julien Grall wrote:

On 17/12/2021 07:21, Michal Orzel wrote:

to hypervisor when switching from AArch32 state.

According to section D1.20.2 of Arm Arm(DDI 0487A.j):
"If the general-purpose register was accessible from AArch32 state the
upper 32 bits either become zero, or hold the value that the same
architectural register held before any AArch32 execution.
The choice between these two options is IMPLEMENTATION DEFINED"

Currently Xen does not ensure that the top 32 bits are zeroed and this
needs to be fixed. The reason why is that there are places in Xen
where we assume that top 32bits are zero for AArch32 guests.
If they are not, this can lead to misinterpretation of Xen regarding
what the guest requested. For example hypercalls returning an error
encoded in a signed long like do_sched_op, do_hmv_op, do_memory_op
would return -ENOSYS if the command passed as the first argument was
clobbered.

Create a macro clobber_gp_top_halves to clobber top 32 bits of gp
registers when hyp == 0 (guest mode) and compat == 1 (AArch32 mode).
Add a compile time check to ensure that save_x0_x1 == 1 if
compat == 1.

Signed-off-by: Michal Orzel 
---
Changes since v2:
-add clobbering of w30
Changes since v1:
-put new code into macro
-add compile time check for save_x0_x1
---
   xen/arch/arm/arm64/entry.S | 29 +
   1 file changed, 29 insertions(+)

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index fc3811ad0a..e351ef8639 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -102,6 +102,30 @@
   .endif
     .endm
+
+/*
+ * Clobber top 32 bits of gp registers when switching from AArch32
+ */
+    .macro clobber_gp_top_halves, compat, save_x0_x1
+
+    .if \compat == 1  /* AArch32 mode */
+
+    /*
+ * save_x0_x1 is equal to 0 only for guest_sync (compat == 0).
+ * Add a compile time check to avoid violating this rule.
+ */


We may want in the future to allow save_x0_x1 == 1 with compat == 1 if we need 
to create fastpath like we did when entering AArch64.

So I would reword the comment to make clear this is an implementation decision. 
How about:

"At the moment, no-one is using save_x0_x1 == 0 with compat == 1. So the code is not 
handling it to simplify the implementation."

If you are happy with the new comment, I can update it on commit:


Please do. The comment looks ok.


Done. It is now committed.

Thanks for the fix!

Cheers,

--
Julien Grall

Re: [RFC v1 3/5] xen/arm: introduce SCMI-SMC mediator driver

[Julien Grall]

Hi,

On 17/12/2021 13:23, Oleksii Moisieiev wrote:

+static int map_memory_to_domain(struct domain *d, uint64_t addr, uint64_t len)
+{
+return iomem_permit_access(d, paddr_to_pfn(addr),
+paddr_to_pfn(PAGE_ALIGN(addr + len -1)));
+}
+
+static int unmap_memory_from_domain(struct domain *d, uint64_t addr,
+ uint64_t len)
+{
+return iomem_deny_access(d, paddr_to_pfn(addr),
+paddr_to_pfn(PAGE_ALIGN(addr + len -1)));
+}


I wonder, why we need an extra level of indirection here. And if this is
really needed, I wonder why map(unmap)_memory* name was chosen, as there is
no memory mapping/unmapping really happens here.



I've added extra indirection to hide math like
paddr_to_pfn(PAGE_ALIGN(addr + len -1)
so you don't have to math in each call. unmap_memory_from_domain called
from 2 places, so I moved both calls to separate function.
Although, I agree that map/unmap is not perfect name. I consider
renaming it to mem_permit_acces and mam_deny_access.


I haven't looked at the rest of the series. But this discussion caught 
my eye. This code implies that the address is page-aligned but the 
length not. Is that intended?


That said, if you give permission to the domain on a full page then it 
means it may be able to access address it should not. Can you explain 
why this is fine?


Cheers,

--
Julien Grall

Re: [RFC v1 5/5] xen/arm: add SCI mediator support for DomUs

[Oleksii Moisieiev]

Hi Jan,

On Fri, Dec 17, 2021 at 08:16:05AM +0100, Jan Beulich wrote:
> On 17.12.2021 08:12, Jan Beulich wrote:
> > On 16.12.2021 18:36, Oleksii Moisieiev wrote:
> >> On Tue, Dec 14, 2021 at 10:41:30AM +0100, Jan Beulich wrote:
> >>> On 14.12.2021 10:34, Oleksii Moisieiev wrote:
>  --- a/xen/include/public/domctl.h
>  +++ b/xen/include/public/domctl.h
>  @@ -1177,6 +1177,13 @@ struct xen_domctl_vmtrace_op {
>   #define XEN_DOMCTL_vmtrace_get_option 5
>   #define XEN_DOMCTL_vmtrace_set_option 6
>   };
>  +
>  +/* XEN_DOMCTL_add_sci_device: set sci device permissions */
>  +struct xen_domctl_sci_device_op {
>  +uint32_t size; /* Length of the path */
>  +XEN_GUEST_HANDLE_64(char) path; /* path to the device tree node */
>  +};
> >>>
> >>> This being - aiui - Arm-only, please enclose it by respective #if,
> >>> just like we do for certain x86-only ops.
> >>>
> >>
> >> I agree. I will add #ifdefs in v2.
> >>
> >>> I'm further afraid the term "SCI" is ambiguous with ACPI's System
> >>> Control Interrupt, so there's some further tag needed in the names
> >>> used here.
> >>>
> >>
> >> Thank you for remark. I'm thinking about SC as System Control.
> >> What do you think?
> > 
> > I guess "SC" could even more so stand for various things. Even the
> > spelled out "System Control" looks overly generic. If this isn't
> > Arm-specific (in which case adding "arm" into the name might at least
> > help the situation a little), then I guess some further disambiguation
> > is going to be wanted. Since I don't know any of the context of this,
> > I'm afraid you're in a far better position than me to come up with a
> > non-ambiguous name.
> 
> Actually, looking at the title again - any reason not to add "mediator"
> into the name? While I have no idea whether there could be other
> mediators with an ambiguous acronym, this would at least address the
> ACPI related concern (I don't expect anything mediator-like to appear
> there, but then again I might be wrong).
> 
> Jan
> 

I wanted the name to be abbriveation. Also tee option in xen doesn't
have "mediator" in it's name either. As for the SC - the only 2 uses I
found are - Spreadsheet Calculator and some script name for
traffic-shaper.
But I think name still need to be discussed.

--
Oleksii.

Re: [RFC v1 3/5] xen/arm: introduce SCMI-SMC mediator driver

[Oleksii Moisieiev]

Hi Julien,

On Fri, Dec 17, 2021 at 01:37:35PM +, Julien Grall wrote:
> Hi,
> 
> On 17/12/2021 13:23, Oleksii Moisieiev wrote:
> > > > +static int map_memory_to_domain(struct domain *d, uint64_t addr, 
> > > > uint64_t len)
> > > > +{
> > > > +return iomem_permit_access(d, paddr_to_pfn(addr),
> > > > +paddr_to_pfn(PAGE_ALIGN(addr + len -1)));
> > > > +}
> > > > +
> > > > +static int unmap_memory_from_domain(struct domain *d, uint64_t addr,
> > > > + uint64_t len)
> > > > +{
> > > > +return iomem_deny_access(d, paddr_to_pfn(addr),
> > > > +paddr_to_pfn(PAGE_ALIGN(addr + len -1)));
> > > > +}
> > > 
> > > I wonder, why we need an extra level of indirection here. And if this is
> > > really needed, I wonder why map(unmap)_memory* name was chosen, as there 
> > > is
> > > no memory mapping/unmapping really happens here.
> > > 
> > 
> > I've added extra indirection to hide math like
> > paddr_to_pfn(PAGE_ALIGN(addr + len -1)
> > so you don't have to math in each call. unmap_memory_from_domain called
> > from 2 places, so I moved both calls to separate function.
> > Although, I agree that map/unmap is not perfect name. I consider
> > renaming it to mem_permit_acces and mam_deny_access.
> 
> I haven't looked at the rest of the series. But this discussion caught my
> eye. This code implies that the address is page-aligned but the length not.
> Is that intended?
> 
> That said, if you give permission to the domain on a full page then it means
> it may be able to access address it should not. Can you explain why this is
> fine?
> 

The idea was that xen receives some memory from the dt_node linux,scmi_mem,
then we split memory between the agents, so each agent get 1 page (we
allocate 0x10 pages right now).
But this is a good point, I think we should check length to be aligned
before making any permission changes.
I will add it in v2. 
Thank you for remark.

Oleksii.

Re: [PATCH] MAINTAINERS: remove Josh from ARINC 653 maintainers

[Andrew Cooper]

On 22/11/2021 14:54, Stewart Hildebrand wrote:
> On Monday, November 22, 2021 9:39 AM, From: Jan Beulich wrote:
>> On 22.11.2021 15:17, Stewart Hildebrand wrote:
>>> Josh works at another company now
>> You don't happen to know his email there, do you? Else if would have been
>> good to Cc him so he could confirm.
>>
>> Jan
> I added Josh in CC. Josh - will you confirm?

Given the request was made by an existing maintainer and there's been no
reply for almost a month, I'm going to commit this patch now.

If further adjustments are wanted/needed in the future, we can accommodate.

~Andrew

RE: [PATCH] MAINTAINERS: remove Josh from ARINC 653 maintainers

[Stewart Hildebrand]

On Friday, December 17, 2021 9:20 AM, Andrew Cooper wrote:
> On 22/11/2021 14:54, Stewart Hildebrand wrote:
> > On Monday, November 22, 2021 9:39 AM, From: Jan Beulich wrote:
> >> On 22.11.2021 15:17, Stewart Hildebrand wrote:
> >>> Josh works at another company now
> >> You don't happen to know his email there, do you? Else if would have
> >> been good to Cc him so he could confirm.
> >>
> >> Jan
> > I added Josh in CC. Josh - will you confirm?
> 
> Given the request was made by an existing maintainer and there's been no
> reply for almost a month, I'm going to commit this patch now.

Thank you, though it appears the patch was already committed.

Stew

Re: [PATCH] MAINTAINERS: remove Josh from ARINC 653 maintainers

[Andrew Cooper]

On 17/12/2021 14:22, Stewart Hildebrand wrote:
> On Friday, December 17, 2021 9:20 AM, Andrew Cooper wrote:
>> On 22/11/2021 14:54, Stewart Hildebrand wrote:
>>> On Monday, November 22, 2021 9:39 AM, From: Jan Beulich wrote:
 On 22.11.2021 15:17, Stewart Hildebrand wrote:
> Josh works at another company now
 You don't happen to know his email there, do you? Else if would have
 been good to Cc him so he could confirm.

 Jan
>>> I added Josh in CC. Josh - will you confirm?
>> Given the request was made by an existing maintainer and there's been no
>> reply for almost a month, I'm going to commit this patch now.
> Thank you, though it appears the patch was already committed.

Yeah - just spotted that while pulling it off the list.

Sorry for the noise.

~Andrew

Re: [PATCH] xen/vpci: msix: move x86 specific code to x86 file

[Julien Grall]

Hi Jan,

On 16/12/2021 13:37, Jan Beulich wrote:

On 16.12.2021 12:01, Roger Pau Monné wrote:

On Thu, Dec 16, 2021 at 10:18:32AM +, Rahul Singh wrote:

Hi Roger,

Thanks for reviewing the code.


On 14 Dec 2021, at 12:37 pm, Roger Pau Monné  wrote:

On Tue, Dec 14, 2021 at 10:45:17AM +, Rahul Singh wrote:

+  unsigned long *data)
{
-const struct domain *d = v->domain;
-struct vpci_msix *msix = msix_find(d, addr);
 const struct vpci_msix_entry *entry;
 unsigned int offset;

 *data = ~0ul;

 if ( !msix )
-return X86EMUL_RETRY;
+return VPCI_EMUL_RETRY;

 if ( !access_allowed(msix->pdev, addr, len) )
-return X86EMUL_OKAY;
+return VPCI_EMUL_OKAY;

 if ( VMSIX_ADDR_IN_RANGE(addr, msix->pdev->vpci, VPCI_MSIX_PBA) )
 {
@@ -210,11 +194,11 @@ static int msix_read(struct vcpu *v, unsigned long addr, 
unsigned int len,
 switch ( len )
 {
 case 4:
-*data = readl(addr);
+*data = vpci_arch_readl(addr);


Why do you need a vpci wrapper around the read/write handlers? AFAICT
arm64 also has {read,write}{l,q}. And you likely want to protect the
64bit read with CONFIG_64BIT if this code is to be made available to
arm32.


I need the wrapper because {read,write}{l,q} function argument is different for 
ARM and x86.
ARM {read,wrie}(l,q}  function argument is pointer to the address whereas X86  
{read,wrie}(l,q}
function argument is address itself.


Oh, that's a shame. I don't think there's a need to tag those helpers
with the vpci_ prefix though. Could we maybe introduce
bus_{read,write}{b,w,l,q} helpers that take the same parameters on all
arches?

It would be even better to fix the current ones so they take the same
parameters on x86 and Arm, but that would mean changing all the call
places in one of the arches.


Yet still: +1 for removing the extra level of indirection. Imo these
trivial helpers should never have diverged between arches; I have
always been under the impression that on Linux they can be used by
arch-independent code (or else drivers would be quite hard to write).


So technically both helpers are able to cope with pointer. The x86 one 
is also allowing to pass an address.


From a brief look at the x86, it looks like most of the users are using 
a pointer. However, the vPCI msix code is one example where addresses 
are passed.


AFAICT, the read*/write* helpers on Linux only works with pointers. So I 
think the actions should be:

   1) Modify the vPCI MSIx code to use pointer
   2) Modify the x86 read*/write* helpers to forbid any access other 
than pointer.


Cheers,

--
Julien Grall

Re: [PATCH 2/5] x86/perfc: conditionalize HVM and shadow counters

[Andrew Cooper]

On 03/12/2021 12:04, Jan Beulich wrote:
> There's no point including them when the respective functionality isn't
> enabled in the build. Note that this covers only larger groups; more
> fine grained exclusion may want to be done later on.
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

Re: [PATCH v2 14/18] IOMMU: fold flush-all hook into "flush one"

[Julien Grall]

On 24/09/2021 10:53, Jan Beulich wrote:

Having a separate flush-all hook has always been puzzling me some. We
will want to be able to force a full flush via accumulated flush flags
from the map/unmap functions. Introduce a respective new flag and fold
all flush handling to use the single remaining hook.

Note that because of the respective comments in SMMU and IPMMU-VMSA
code, I've folded the two prior hook functions into one. For SMMU-v3,
which lacks a comment towards incapable hardware, I've left both
functions in place on the assumption that selective and full flushes
will eventually want separating.

Signed-off-by: Jan Beulich 


For the Arm part:

Acked-by: Julien Grall 

Cheers,

--
Julien Grall

Re: [PATCH v2 04/18] IOMMU: add order parameter to ->{,un}map_page() hooks

[Julien Grall]

Hi Jan,

On 24/09/2021 10:44, Jan Beulich wrote:

Or really, in the case of ->map_page(), accommodate it in the existing
"flags" parameter. All call sites will pass 0 for now.

Signed-off-by: Jan Beulich 
Reviewed-by: Kevin Tian 


For the Arm bits:

Acked-by: Julien Grall 

Cheers,

--
Julien Grall

Re: [PATCH v2 03/18] IOMMU: have vendor code announce supported page sizes

[Julien Grall]

Hi Jan,

On 24/09/2021 10:43, Jan Beulich wrote:

Generic code will use this information to determine what order values
can legitimately be passed to the ->{,un}map_page() hooks. For now all
ops structures simply get to announce 4k mappings (as base page size),
and there is (and always has been) an assumption that this matches the
CPU's MMU base page size (eventually we will want to permit IOMMUs with
a base page size smaller than the CPU MMU's).

Signed-off-by: Jan Beulich 
Reviewed-by: Kevin Tian 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall

Re: [PATCH v2 1/2] mm: introduce INVALID_{G,M}FN_RAW

[Julien Grall]

Hi Jan,

On 10/12/2021 09:39, Jan Beulich wrote:

This allows properly tying together INVALID_{G,M}FN and
INVALID_{G,M}FN_INITIALIZER as well as using the actual values in
compile time constant expressions (or even preprocessor dirctives).

Since INVALID_PFN is unused, and with x86'es paging_mark_pfn_dirty()
being the only user of pfn_t it also doesn't seem likely that new uses
would appear, remove that one at this same occasion.

Signed-off-by: Jan Beulich 


Reviewed-by: Julien Grall 

Cheers,

--
Julien Grall

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

[osstest service owner]

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

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  c81fc9f313c031f1201181cfb40e3c3ee599e04f
baseline version:
 xen  1c4589280ae4e9ba34266e674459fffd6f0282dc

Last test of basis   167458  2021-12-17 08:01:39 Z0 days
Testing same since   167460  2021-12-17 12:01:45 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Juergen Gross 

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
   1c4589280a..c81fc9f313  c81fc9f313c031f1201181cfb40e3c3ee599e04f -> smoke

Re: [PATCH] xen/vpci: msix: move x86 specific code to x86 file

[Rahul Singh]

Hi Julien

> On 17 Dec 2021, at 2:32 pm, Julien Grall  wrote:
> 
> Hi Jan,
> 
> On 16/12/2021 13:37, Jan Beulich wrote:
>> On 16.12.2021 12:01, Roger Pau Monné wrote:
>>> On Thu, Dec 16, 2021 at 10:18:32AM +, Rahul Singh wrote:
 Hi Roger,
 
 Thanks for reviewing the code.
 
> On 14 Dec 2021, at 12:37 pm, Roger Pau Monné  wrote:
> 
> On Tue, Dec 14, 2021 at 10:45:17AM +, Rahul Singh wrote:
>> +  unsigned long *data)
>> {
>> -const struct domain *d = v->domain;
>> -struct vpci_msix *msix = msix_find(d, addr);
>> const struct vpci_msix_entry *entry;
>> unsigned int offset;
>> 
>> *data = ~0ul;
>> 
>> if ( !msix )
>> -return X86EMUL_RETRY;
>> +return VPCI_EMUL_RETRY;
>> 
>> if ( !access_allowed(msix->pdev, addr, len) )
>> -return X86EMUL_OKAY;
>> +return VPCI_EMUL_OKAY;
>> 
>> if ( VMSIX_ADDR_IN_RANGE(addr, msix->pdev->vpci, VPCI_MSIX_PBA) )
>> {
>> @@ -210,11 +194,11 @@ static int msix_read(struct vcpu *v, unsigned long 
>> addr, unsigned int len,
>> switch ( len )
>> {
>> case 4:
>> -*data = readl(addr);
>> +*data = vpci_arch_readl(addr);
> 
> Why do you need a vpci wrapper around the read/write handlers? AFAICT
> arm64 also has {read,write}{l,q}. And you likely want to protect the
> 64bit read with CONFIG_64BIT if this code is to be made available to
> arm32.
 
 I need the wrapper because {read,write}{l,q} function argument is 
 different for ARM and x86.
 ARM {read,wrie}(l,q}  function argument is pointer to the address whereas 
 X86  {read,wrie}(l,q}
 function argument is address itself.
>>> 
>>> Oh, that's a shame. I don't think there's a need to tag those helpers
>>> with the vpci_ prefix though. Could we maybe introduce
>>> bus_{read,write}{b,w,l,q} helpers that take the same parameters on all
>>> arches?
>>> 
>>> It would be even better to fix the current ones so they take the same
>>> parameters on x86 and Arm, but that would mean changing all the call
>>> places in one of the arches.
>> Yet still: +1 for removing the extra level of indirection. Imo these
>> trivial helpers should never have diverged between arches; I have
>> always been under the impression that on Linux they can be used by
>> arch-independent code (or else drivers would be quite hard to write).
> 
> So technically both helpers are able to cope with pointer. The x86 one is 
> also allowing to pass an address.
> 
> From a brief look at the x86, it looks like most of the users are using a 
> pointer. However, the vPCI msix code is one example where addresses are 
> passed.

Yes you are right.
> 
> AFAICT, the read*/write* helpers on Linux only works with pointers. So I 
> think the actions should be:
>   1) Modify the vPCI MSIx code to use pointer

I am also thinking to change the misx_read/write to use a pointer to address to 
avoid change in {read,write}{b,w,l,q}
If everyone is ok I will send the next version to modify the same.

Regards,
Rahul
>   2) Modify the x86 read*/write* helpers to forbid any access other than 
> pointer.
> 
> Cheers,
> 
> -- 
> Julien Grall

Re: [PATCH 3/5] VMX: sync VM-exit perf counters with known VM-exit reasons

[Andrew Cooper]

On 03/12/2021 12:05, Jan Beulich wrote:
> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
> @@ -220,6 +220,8 @@ static inline void pi_clear_sn(struct pi
>  #define EXIT_REASON_XSAVES  63
>  #define EXIT_REASON_XRSTORS 64
>  
> +#define EXIT_REASON_LASTEXIT_REASON_XRSTORS
> +

Given the problems with sentinals like this in the domctl/sysctl
headers, I think this scheme would be less fragile if EXIT_REASON was an
enum.  (Also, we seriously need to reduce the scope of these headers. 
It's only just dawned on me why Intel uses EXIT_* and AMD uses VMEXIT_*)

Alternatively, what about simply this:

 #define EXIT_REASON_XSAVES  63
 #define EXIT_REASON_XRSTORS 64
+/* Remember to update VMX_PERF_EXIT_REASON_SIZE too. */

?

This avoids having yet another sentinel in the mix, and will be visible
in *every* patch review.  It also gets rid of the ifdefary in the vmexit
handler.

Another good move might be to have perfc_incra() have a printk_once()
for out-of-range indices.  That way, people using perfcounters will have
an easier time noticing if something is wrong.

~Andrew

Re: [PATCH 4/5] SVM: sync VM-exit perf counters with known VM-exit reasons

[Andrew Cooper]

On 03/12/2021 12:06, Jan Beulich wrote:
> This has gone out of sync over time, resulting in NPF and XSETBV exits
> incrementing the same counter. Introduce a simplistic mechanism to
> hopefully keep things in better sync going forward.
>
> Signed-off-by: Jan Beulich 
> ---
> Given their large (and growing) number, I wonder whether we shouldn't
> fold "SVMexits" and "vmexits". They can't both be active at the same
> time.

Oh yeah - that's just silly having them split like this, especially as
there's no associated element name.

> --- a/xen/include/asm-x86/perfc_defn.h
> +++ b/xen/include/asm-x86/perfc_defn.h
> @@ -11,8 +11,8 @@ PERFCOUNTER_ARRAY(exceptions,
>  PERFCOUNTER_ARRAY(vmexits,  "vmexits", VMX_PERF_EXIT_REASON_SIZE)
>  PERFCOUNTER_ARRAY(cause_vector, "cause vector", VMX_PERF_VECTOR_SIZE)
>  
> -#define VMEXIT_NPF_PERFC 141
> -#define SVM_PERF_EXIT_REASON_SIZE (1+141)
> +#define VMEXIT_NPF_PERFC 143
> +#define SVM_PERF_EXIT_REASON_SIZE (VMEXIT_NPF_PERFC + 1)

How does this work in the first place?  perfc_incra() is still passed 1024.

Furthermore, it's already worse than this.

401/402 are AVIC exits, and 403 is an SEV exit.  We've also gained -2 as
"busy" for transient SEV events too.

~Andrew

Re: NULL scheduler DoS

[Ahmed, Daniele]

Hi!

Is there any update on this issue? How should we move forward?

Thank you,


On 11/08/2021, 12:09, "Dario Faggioli"  wrote:

On Mon, 2021-08-09 at 21:38 +0100, Julien Grall wrote:
> On 09/08/2021 18:35, Julien Grall wrote:
> > 
> > This implies that a pCPU may temporarily be assigned to two vCPUs
> > and we 
> > expect to be fixed up afterwards. However, a domain may be
> > destroyed 
> > before this is happening.
> > 
> > So it looks like that unit_deassign() is not able to cope with this
> > case. From a brief look, I think we may want to check if the pCPU
> > is in 
> > the wait list. If it is, then we should bail out.
> 
> Actually, I was wrong. It looks like null_unit_remove() is already 
> checking this condition. Also, the vCPU should be offline (and the
> unit 
> as well) because they haven't come online yet:
> 
That's what is currently puzzling me.

I mean, the crash happens in unit_deassign(), called by
null_unit_remove(), called by sched_destroy_vcpu().

And I agree that the unit should be offline, but null_unit_remove()
calls unit_deassign() only if the unit *is* online, so... What's going
on? :-/

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879

Re: [PATCH 5/5] xenperf: name "newer" hypercalls

[Andrew Cooper]

On 03/12/2021 12:07, Jan Beulich wrote:
> This table must not have got updated in quite a while; tmem_op for
> example has managed to not only appear since then, but also disappear
> again (adding a name for it nevertheless, to make more obvious that
> something strange is going on if the slot would ever have a non-zero
> value).
>
> Signed-off-by: Jan Beulich 
> ---
> I wasn't really sure about x86's real names for arch_0 and arch_1. The
> tool runs on the same host as the hypervisor, so __i386__ / __x86_64__
> conditionals wouldn't be inappropriate to use ...

This is a developer tool.  Noone is going to have a perfcounter enabled
hypervisor in production.

Therefore, I think the ifdef's will be fine.

Preferably with, but either way, Acked-by: Andrew Cooper

[xen-unstable test] 167455: tolerable FAIL

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 167448
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 167448
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 167448
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 167448
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 167448
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail like 
167448
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 167448
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 167448
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 167448
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 167448
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 167448
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 167448
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 167448
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-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-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-libvirt  15 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-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-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-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-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-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-amd64-amd64-libvirt-vhd 14 migrate-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-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  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  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-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 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-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 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-raw 14 migrate-support-checkfail   never pass

version targeted for testing:
 xen  dc27c174b2f

Re: [PATCH V6 1/2] libxl: Add support for Virtio disk configuration

[Juergen Gross]

On 15.12.21 22:36, Oleksandr wrote:


On 15.12.21 17:58, Juergen Gross wrote:


Hi Juergen



On 15.12.21 16:02, Oleksandr wrote:


On 15.12.21 08:08, Juergen Gross wrote:

Hi Juergen


On 14.12.21 18:44, Oleksandr wrote:


On 14.12.21 18:03, Anthony PERARD wrote:

Hi Anthony



On Wed, Dec 08, 2021 at 06:59:43PM +0200, Oleksandr Tyshchenko wrote:

From: Oleksandr Tyshchenko 

This patch adds basic support for configuring and assisting 
virtio-disk
backend (emualator) which is intended to run out of Qemu and 
could be

run in any domain.
Although the Virtio block device is quite different from traditional
Xen PV block device (vbd) from the toolstack point of view:
  - as the frontend is virtio-blk which is not a Xenbus driver, 
nothing

    written to Xenstore are fetched by the frontend (the vdev is not
    passed to the frontend)
  - the ring-ref/event-channel are not used for the 
backend<->frontend

    communication, the proposed IPC for Virtio is IOREQ/DM
it is still a "block device" and ought to be integrated in existing
"disk" handling. So, re-use (and adapt) "disk" parsing/configuration
logic to deal with Virtio devices as well.
How backend are intended to be created? Is there something 
listening on

xenstore?

You mention QEMU as been the backend, do you intend to have QEMU
listening on xenstore to create a virtio backend? Or maybe it is 
on the

command line? There is QMP as well, but it's probably a lot more
complicated as I think libxl needs refactoring for that.



No, QEMU is not involved there. The backend is a standalone 
application,
it is launched from the command line. The backend reads the 
Xenstore to get
the configuration and to detect when guest with the frontend is 
created/destroyed.


I think this should be reflected somehow in the configuration, as I
expect qemu might gain this functionality in the future.


I understand this and agree in general (however I am wondering 
whether this can be postponed until it is actually needed), but ...


This might lead to the need to support some "legacy" options in future.
I think we should at least think whether these scheme will cover (or
prohibit) extensions which are already on the horizon.


ok






I'm wondering whether we shouldn't split the backend from the protocol
(or specification?). Something like "protocol=virtio" (default would be
e.g. "xen") and then you could add "backend=external" for your use 
case?


... I am afraid, I didn't get the idea. Are we speaking about the 
(new?) disk configuration options
here or these are not disk specific things at all and to be 
applicable for all possible backends?


I was talking of a general approach using the disk as an example. For
disks it is just rather obvious.

If the former, then could the new backendtype simply do the job? For 
example, "backendtype=virtio_external" for our current use-case and 
"backendtype=virtio_qemu"

for the possible future use-cases? Could you please clarify the idea.


I want to avoid overloading the backendtype with information which is
in general not really related by the backend. You can have a qemu based
qdisk backend serving a Xen PV-disk (like today) or a virtio disk.

A similar approach has been chosen for the disk format: it is not part
of the backend, but a parameter of its own. This way e.g. the qdisk
backend can use the original qdisk format, or the qcow format.

In practice we are having something like the "protocol" already today:
the disk device name is encoding that ("xvd*" is a Xen PV disk, while
"sd*" is an emulated SCSI disk, which happens to be presented to the
guest as "xvd*", too). And this is an additional information not
related to the backendtype.

So we have basically the following configuration items, which are
orthogonal to each other (some combinations might not make sense,
but in theory most would be possible):

1. protocol: emulated (not PV), Xen (like today), virtio

2. backendtype: phy (blkback), qdisk (qemu), other (e.g. a daemon)

3. format: raw, qcow, qcow2, vhd, qed

The combination virtio+phy would be equivalent to vhost, BTW. And
virtio+other might even use vhost-user, depending on the daemon.

yes, BTW the combination virtio+other is close to our use-case.


Thank you for the detailed explanation, now I see your point why using 
backendtype=virtio is not flexible option in the long term
and why we would want/need to an extra configuration option such as 
protocol, etc. I think, it makes sense and would be correct.


If we take a disk as an example, then from the configuration PoV we will 
need to:

- add an optional "protocol" option


I'm not sure regarding the name of the option. "protocol" was just a
suggestion by me.


- add new backendtype: external/other/daemon/etc.
This seems to cover all possible combinations describe above (although I 
agree that some of them might not make sense). Is my understanding correct?


Yes.

Unfortunately, disk configuration/management code is spread over 
multiple sources (includi

Re: [PATCH 3/5] xen/sort: Switch to an extern inline implementation

[Andrew Cooper]

On 16/11/2021 00:41, Andrew Cooper wrote:
> On 16/11/2021 00:36, Stefano Stabellini wrote:
>> On Thu, 11 Nov 2021, Julien Grall wrote:
>>> On 11/11/2021 17:57, Andrew Cooper wrote:
 There are exactly 3 callers of sort() in the hypervisor.

 Both arm callers pass in NULL for the swap function.  While this might seem
 like an attractive option at first, it causes generic_swap() to be used
 which
 forced a byte-wise copy.  Provide real swap functions which the compiler 
 can
 optimise sensibly.
>>> I understand the theory, but none of the two calls are in hot paths or deal
>>> with large set on Arm. So I am rather hesitant to introduce specialised swap
>>> in each case as it doesn't seem we will gain much from this change.
>> While I like Andrew's optimization, like Julien, I would also rather not
>> have to introduce specialized swap functions any time a sort() is
>> called. Why not keeping around generic_swap as extern gnu_inline in
>> sort.h as well so that the ARM callers can simply:
>>
>> sort(bootinfo.mem.bank, bootinfo.mem.nr_banks, sizeof(struct membank),
>>  cmp_memory_node, generic_swap);
>>
>> ?
>>
>> That looks like a good option, although it would still result in a
>> small increase in bloat.
> That is basically what Jan suggested.
>
> I'm still unconvinced that you actually want to be doing byte-wise
> swapping, even if it isn't a hotpath.  A custom swap function will
> compile to less code than using generic_swap() like this, and run faster.

ARM Ping.

I really think this is a bad course of action.  If you're going to
insist on it, I want something in writing.

~Andrew

Re: [PATCH 3/5] xen/sort: Switch to an extern inline implementation

[Julien Grall]

Hi Andrew,

On 17/12/2021 15:56, Andrew Cooper wrote:

On 16/11/2021 00:41, Andrew Cooper wrote:

On 16/11/2021 00:36, Stefano Stabellini wrote:

On Thu, 11 Nov 2021, Julien Grall wrote:

On 11/11/2021 17:57, Andrew Cooper wrote:

There are exactly 3 callers of sort() in the hypervisor.

Both arm callers pass in NULL for the swap function.  While this might seem
like an attractive option at first, it causes generic_swap() to be used
which
forced a byte-wise copy.  Provide real swap functions which the compiler can
optimise sensibly.

I understand the theory, but none of the two calls are in hot paths or deal
with large set on Arm. So I am rather hesitant to introduce specialised swap
in each case as it doesn't seem we will gain much from this change.

While I like Andrew's optimization, like Julien, I would also rather not
have to introduce specialized swap functions any time a sort() is
called. Why not keeping around generic_swap as extern gnu_inline in
sort.h as well so that the ARM callers can simply:

 sort(bootinfo.mem.bank, bootinfo.mem.nr_banks, sizeof(struct membank),
  cmp_memory_node, generic_swap);

?

That looks like a good option, although it would still result in a
small increase in bloat.

That is basically what Jan suggested.

I'm still unconvinced that you actually want to be doing byte-wise
swapping, even if it isn't a hotpath.  A custom swap function will
compile to less code than using generic_swap() like this, and run faster.


ARM Ping.

I really think this is a bad course of action.  If you're going to
insist on it, I want something in writing.


I agree with all the points you wrote. However, you also have to put 
into perspective how this is used. On Arm, the two callers happen either 
during boot a domain creation. Neither of them have a significant impact 
on the time spent. In fact, I would be surprised if you notice the change.


So unless there are other (good) reasons to suppress generic_swap(), I 
still want to be able to use generic_swap() when the performance doesn't 
matter (like the two Arm callers).


Cheers,

--
Julien Grall

Re: [RFC v1 3/5] xen/arm: introduce SCMI-SMC mediator driver

[Julien Grall]

On 17/12/2021 13:58, Oleksii Moisieiev wrote:

Hi Julien,


Hi,


On Fri, Dec 17, 2021 at 01:37:35PM +, Julien Grall wrote:

Hi,

On 17/12/2021 13:23, Oleksii Moisieiev wrote:

+static int map_memory_to_domain(struct domain *d, uint64_t addr, uint64_t len)
+{
+return iomem_permit_access(d, paddr_to_pfn(addr),
+paddr_to_pfn(PAGE_ALIGN(addr + len -1)));
+}
+
+static int unmap_memory_from_domain(struct domain *d, uint64_t addr,
+ uint64_t len)
+{
+return iomem_deny_access(d, paddr_to_pfn(addr),
+paddr_to_pfn(PAGE_ALIGN(addr + len -1)));
+}


I wonder, why we need an extra level of indirection here. And if this is
really needed, I wonder why map(unmap)_memory* name was chosen, as there is
no memory mapping/unmapping really happens here.



I've added extra indirection to hide math like
paddr_to_pfn(PAGE_ALIGN(addr + len -1)
so you don't have to math in each call. unmap_memory_from_domain called
from 2 places, so I moved both calls to separate function.
Although, I agree that map/unmap is not perfect name. I consider
renaming it to mem_permit_acces and mam_deny_access.


I haven't looked at the rest of the series. But this discussion caught my
eye. This code implies that the address is page-aligned but the length not.
Is that intended?

That said, if you give permission to the domain on a full page then it means
it may be able to access address it should not. Can you explain why this is
fine?



The idea was that xen receives some memory from the dt_node linux,scmi_mem,
then we split memory between the agents, so each agent get 1 page (we
allocate 0x10 pages right now).


Thanks for the clarification. Does this imply the guest will be able to 
write message directly to the firmware?


If so, this brings a few more questions:
  1) What will the guest write in it? Can it contains addresses?
  2) What are the expected memory attribute for the regions?
  3) What's the threat model for the firmware? Will it verify every 
request?


Cheers,

--
Julien Grall

Re: [PATCH V6 1/2] libxl: Add support for Virtio disk configuration

[Oleksandr]

On 17.12.21 17:26, Juergen Gross wrote:


Hi Juergen, all.



On 15.12.21 22:36, Oleksandr wrote:


On 15.12.21 17:58, Juergen Gross wrote:


Hi Juergen



On 15.12.21 16:02, Oleksandr wrote:


On 15.12.21 08:08, Juergen Gross wrote:

Hi Juergen


On 14.12.21 18:44, Oleksandr wrote:


On 14.12.21 18:03, Anthony PERARD wrote:

Hi Anthony


On Wed, Dec 08, 2021 at 06:59:43PM +0200, Oleksandr Tyshchenko 
wrote:

From: Oleksandr Tyshchenko 

This patch adds basic support for configuring and assisting 
virtio-disk
backend (emualator) which is intended to run out of Qemu and 
could be

run in any domain.
Although the Virtio block device is quite different from 
traditional

Xen PV block device (vbd) from the toolstack point of view:
  - as the frontend is virtio-blk which is not a Xenbus driver, 
nothing
    written to Xenstore are fetched by the frontend (the vdev 
is not

    passed to the frontend)
  - the ring-ref/event-channel are not used for the 
backend<->frontend

    communication, the proposed IPC for Virtio is IOREQ/DM
it is still a "block device" and ought to be integrated in 
existing
"disk" handling. So, re-use (and adapt) "disk" 
parsing/configuration

logic to deal with Virtio devices as well.
How backend are intended to be created? Is there something 
listening on

xenstore?

You mention QEMU as been the backend, do you intend to have QEMU
listening on xenstore to create a virtio backend? Or maybe it is 
on the

command line? There is QMP as well, but it's probably a lot more
complicated as I think libxl needs refactoring for that.



No, QEMU is not involved there. The backend is a standalone 
application,
it is launched from the command line. The backend reads the 
Xenstore to get
the configuration and to detect when guest with the frontend is 
created/destroyed.


I think this should be reflected somehow in the configuration, as I
expect qemu might gain this functionality in the future.


I understand this and agree in general (however I am wondering 
whether this can be postponed until it is actually needed), but ...


This might lead to the need to support some "legacy" options in future.
I think we should at least think whether these scheme will cover (or
prohibit) extensions which are already on the horizon.


ok





I'm wondering whether we shouldn't split the backend from the 
protocol
(or specification?). Something like "protocol=virtio" (default 
would be
e.g. "xen") and then you could add "backend=external" for your use 
case?


... I am afraid, I didn't get the idea. Are we speaking about the 
(new?) disk configuration options
here or these are not disk specific things at all and to be 
applicable for all possible backends?


I was talking of a general approach using the disk as an example. For
disks it is just rather obvious.

If the former, then could the new backendtype simply do the job? 
For example, "backendtype=virtio_external" for our current use-case 
and "backendtype=virtio_qemu"

for the possible future use-cases? Could you please clarify the idea.


I want to avoid overloading the backendtype with information which is
in general not really related by the backend. You can have a qemu based
qdisk backend serving a Xen PV-disk (like today) or a virtio disk.

A similar approach has been chosen for the disk format: it is not part
of the backend, but a parameter of its own. This way e.g. the qdisk
backend can use the original qdisk format, or the qcow format.

In practice we are having something like the "protocol" already today:
the disk device name is encoding that ("xvd*" is a Xen PV disk, while
"sd*" is an emulated SCSI disk, which happens to be presented to the
guest as "xvd*", too). And this is an additional information not
related to the backendtype.

So we have basically the following configuration items, which are
orthogonal to each other (some combinations might not make sense,
but in theory most would be possible):

1. protocol: emulated (not PV), Xen (like today), virtio

2. backendtype: phy (blkback), qdisk (qemu), other (e.g. a daemon)

3. format: raw, qcow, qcow2, vhd, qed

The combination virtio+phy would be equivalent to vhost, BTW. And
virtio+other might even use vhost-user, depending on the daemon.

yes, BTW the combination virtio+other is close to our use-case.


Thank you for the detailed explanation, now I see your point why 
using backendtype=virtio is not flexible option in the long term
and why we would want/need to an extra configuration option such as 
protocol, etc. I think, it makes sense and would be correct.


If we take a disk as an example, then from the configuration PoV we 
will need to:

- add an optional "protocol" option


I'm not sure regarding the name of the option. "protocol" was just a
suggestion by me.


Yes, personally I would be fine with either "protocol" or 
"specification", with a little preference for the former. What other 
people think of the name?






- add new backendtype: external/other/daemon/etc.
This seems to cov

[ovmf test] 167463: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf ab5ab2f60348138a4b7b1c95ad6f5d0954fb96f1
baseline version:
 ovmf b451c6908878c448c2a2aa6e9ca2a2dfe078fbb8

Last test of basis   167450  2021-12-16 18:10:32 Z0 days
Testing same since   167463  2021-12-17 15:10:34 Z0 days1 attempts


People who touched revisions under test:
  Rodrigo Gonzalez del Cueto 

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
   b451c69088..ab5ab2f603  ab5ab2f60348138a4b7b1c95ad6f5d0954fb96f1 -> 
xen-tested-master

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

[osstest service owner]

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

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  32365f3476ac4655f2f26111cd7879912808cd77
baseline version:
 xen  c81fc9f313c031f1201181cfb40e3c3ee599e04f

Last test of basis   167460  2021-12-17 12:01:45 Z0 days
Testing same since   167462  2021-12-17 15:01:42 Z0 days1 attempts


People who touched revisions under test:
  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
   c81fc9f313..32365f3476  32365f3476ac4655f2f26111cd7879912808cd77 -> smoke

Re: [PATCH V4 0/6] xen: Add support of extended regions (safe ranges) on Arm

[Boris Ostrovsky]

On 12/16/21 5:02 PM, Oleksandr wrote:


On 09.12.21 22:05, Oleksandr Tyshchenko wrote:


Hello Juergen, Boris


May I please ask, are you happy (or otherwise) with current patch series (I 
assume, especially with commits #3-4)?



I think I mentioned last time, in patch 4:


+   if (target_resource != &iomem_resource) {
+   tmp_res = kzalloc(sizeof(*tmp_res), GFP_KERNEL);
+   if (!res) {


Other than that --- LGTM.


-boris

Re: [PATCH V4 0/6] xen: Add support of extended regions (safe ranges) on Arm

[Boris Ostrovsky]

On 12/16/21 5:02 PM, Oleksandr wrote:


On 09.12.21 22:05, Oleksandr Tyshchenko wrote:


Hello Juergen, Boris


May I please ask, are you happy (or otherwise) with current patch series (I 
assume, especially with commits #3-4)?



LGTM, I have no more comments.


-boris

Re: [PATCH V4 0/6] xen: Add support of extended regions (safe ranges) on Arm

[Oleksandr]

On 17.12.21 21:13, Boris Ostrovsky wrote:

Hi Boris



On 12/16/21 5:02 PM, Oleksandr wrote:


On 09.12.21 22:05, Oleksandr Tyshchenko wrote:


Hello Juergen, Boris


May I please ask, are you happy (or otherwise) with current patch 
series (I assume, especially with commits #3-4)?



I think I mentioned last time, in patch 4:


+    if (target_resource != &iomem_resource) {
+    tmp_res = kzalloc(sizeof(*tmp_res), GFP_KERNEL);
+    if (!res) {


Yes, this is already fixed in V4 of patch [1]






Other than that --- LGTM.

Thank you!


[1] 
https://lore.kernel.org/xen-devel/1639080336-26573-5-git-send-email-olekst...@gmail.com/






-boris


--
Regards,

Oleksandr Tyshchenko

Re: [PATCH V4 0/6] xen: Add support of extended regions (safe ranges) on Arm

[Boris Ostrovsky]

On 12/17/21 2:19 PM, Oleksandr wrote:


On 17.12.21 21:13, Boris Ostrovsky wrote:

Hi Boris



On 12/16/21 5:02 PM, Oleksandr wrote:


On 09.12.21 22:05, Oleksandr Tyshchenko wrote:


Hello Juergen, Boris


May I please ask, are you happy (or otherwise) with current patch series (I 
assume, especially with commits #3-4)?



I think I mentioned last time, in patch 4:


+    if (target_resource != &iomem_resource) {
+    tmp_res = kzalloc(sizeof(*tmp_res), GFP_KERNEL);
+    if (!res) {


Yes, this is already fixed in V4 of patch [1]




So this email did escape. Yes, I realized I was looking at V3 but apparently 
didn't hit ^C quickly enough ;-)



-boris

[RFC 00/10] Hyperlaunch x86 Dom0 launch

[Daniel P. Smith]

This RFC is to provide the base approach of how hyperlaunch would integrate
into the x86 start sequence with the specific intention to vet the
implementation approach early. What this RFC provides is a working
implementation of hyperlaunch for x86 that is capable of strictly starting a
Dom0 configuration. Even so the majority of the remaining hyperlaunch work will
be focused on domain construction and enabling the transient boot domain (DomB)
ability.

It is important to note that for this RFC patch set the work was split into
smaller commits to ease review. When the first hyperlaunch patch set is
submitted for acceptance, it is expected to see 5-7 rolled up and 9 & 10 also
rolled up.

There are a couple of areas that explicitly need review and discussion and the
commit message for a patch with one of these in them has the details, which
will be repeated here for them to also all be centralized for comment.

- For hyperlaunch it has become necessary to track decompression headroom at an
individual boot module level. The most sensible place for this information on
x86 is in the module_t structure. The challenge encountered with this is that
Xen's multiboot support is implemented by reusing MB module_t structure as a
common structure for MB and MB2. Early in Xen's x86 startup the multiboot chain
is copied(MB)/converted(MB2) into the trampoline data heap. To minimize change
for this RFC the MB module_t reserved field was split into a bitfield to allow
it to serve the existing usage of the field (relocation flag) and for headroom
tracking. While this provided a clean solution, it is viewed as a short term
solution. A suggestion might be to unify the existing architectures'
representation of a boot module and refactor x86's reloc.c to convert MB and
MB2 module descriptions into this common representation. This would have the
added benefit of reducing the static allocations currently being used for
hyperlaunch.

- When enabling device tree for use with x86, it exposed some arch specific
code in the device tree code in common and shared includes. Specifically both
Arm and x86 define a `struct device` but are totally different structures. The
device tree code uses references to Arm's version of `struct device` which
creates a conflict when the device tree code is used under x86. It was also
found that generic device tree code was currently in the Arm arch tree. Patch 4
contains a work around for the first issue and patch 7 copies the reusable
device tree parsing code from Arm arch for local/internal usage. Each patch
provides a few additional details and ultimately a generalized solution for
both problems is the preferable approach versus a minimal solution for enabling
hyperlaunch 

- To ease getting a Dom0 construction working under hyperlaunch the existing
`create_dom0()` function was exposed and reused. This was for the RFC only
and will be dropped in lieu of hyperlaunch gaining its own more general domain
creation function.
 
Christopher Clark (2):
  introduce hyperlaunch kconfig and core
  is_system_domain: replace open-coded instances

Daniel P. Smith (8):
  multiboot: moving headroom to per module_t
  device-tree: split agnostic device-tree from arm
  hyperlaunch: update device tree documentation
  hyperlaunch: add structures to hold parsed dtb
  hyperlaunch: add parsing of dtb
  hyperlaunch: make create_dom0 externally callable
  hyperlaunch: add domain creation logic
  hyperlaunch: integrate dtb parse and domain creation

 .../designs/launch/hyperlaunch-devicetree.rst | 448 ++---
 xen/arch/x86/boot/reloc.c |   1 +
 xen/arch/x86/cpu/vpmu.c   |   2 +-
 xen/arch/x86/include/asm/setup.h  |   5 +
 xen/arch/x86/setup.c  |  75 +--
 xen/common/Kconfig|  15 +
 xen/common/Makefile   |   5 +-
 xen/common/device_tree.c  |   2 +
 xen/common/domctl.c   |   2 +-
 xen/common/sched/core.c   |   4 +-
 xen/common/setup.c| 450 ++
 xen/include/xen/device_tree.h |   4 +
 xen/include/xen/multiboot.h   |   3 +-
 xen/include/xen/sched.h   |   5 +
 xen/include/xen/setup.h   | 131 +
 15 files changed, 948 insertions(+), 204 deletions(-)
 create mode 100644 xen/common/setup.c
 create mode 100644 xen/include/xen/setup.h

-- 
2.20.1

[RFC 01/10] introduce hyperlaunch kconfig and core

[Daniel P. Smith]

From: Christopher Clark 

Add a CONFIG_HYPERLAUNCH to allow enabling/disabling the hyperlaunch code
paths. Introduce an initial use of the hyperlaunch_enabled variable to suppress
the warning about multiple initrd candidates when multiboot modules are
detected when the hyperlaunch boolean is true.

Added to common code since this option will apply to all architectures.

Signed-off-by: Christopher Clark 
Signed-off-by: Daniel P. Smith 
---
 xen/arch/x86/setup.c|  4 +++-
 xen/common/Kconfig  | 10 ++
 xen/common/Makefile |  1 +
 xen/common/setup.c  |  8 
 xen/include/xen/setup.h | 14 ++
 5 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 xen/common/setup.c
 create mode 100644 xen/include/xen/setup.h

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index f40a9fe5d3..190d7aefb5 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #ifdef CONFIG_COMPAT
 #include 
@@ -1891,7 +1892,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
cpu_has_nx ? "" : "not ");
 
 initrdidx = find_first_bit(module_map, mbi->mods_count);
-if ( bitmap_weight(module_map, mbi->mods_count) > 1 )
+if ( !hyperlaunch_enabled &&
+ bitmap_weight(module_map, mbi->mods_count) > 1 )
 printk(XENLOG_WARNING
"Multiple initrd candidates, picking module #%u\n",
initrdidx);
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index db687b1785..5e6aad644e 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -331,6 +331,16 @@ config ARGO
 
  If unsure, say N.
 
+config HYPERLAUNCH
+   bool "Hyperlaunch support (UNSUPPORTED)" if UNSUPPORTED
+   ---help---
+ Enables launch of multiple VMs at host boot as an alternative
+ method of starting a Xen system.
+
+ This feature is currently experimental.
+
+ If unsure, say N.
+
 source "common/sched/Kconfig"
 
 config CRYPTO
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 141d7d40d3..a6337e065a 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -35,6 +35,7 @@ obj-y += rangeset.o
 obj-y += radix-tree.o
 obj-y += rcupdate.o
 obj-y += rwlock.o
+obj-y += setup.o
 obj-y += shutdown.o
 obj-y += softirq.o
 obj-y += smp.o
diff --git a/xen/common/setup.c b/xen/common/setup.c
new file mode 100644
index 00..e18ea14fe0
--- /dev/null
+++ b/xen/common/setup.c
@@ -0,0 +1,8 @@
+#include 
+#include 
+
+#ifdef CONFIG_HYPERLAUNCH
+
+bool __initdata hyperlaunch_enabled;
+
+#endif
diff --git a/xen/include/xen/setup.h b/xen/include/xen/setup.h
new file mode 100644
index 00..6fbe87860e
--- /dev/null
+++ b/xen/include/xen/setup.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef XEN_SETUP_H
+#define XEN_SETUP_H
+
+#include 
+
+#ifdef CONFIG_HYPERLAUNCH
+extern bool hyperlaunch_enabled;
+#else
+#define hyperlaunch_enabled false
+#endif
+
+#endif /* XEN_SETUP_H */
-- 
2.20.1

[RFC 02/10] is_system_domain: replace open-coded instances

[Daniel P. Smith]

From: Christopher Clark 

There were several instances of open-coded domid range checking. This commit
replaces those with the is_system_domain inline function.

Signed-off-by: Christopher Clark 
Signed-off-by: Daniel P. Smith 
---
 xen/arch/x86/cpu/vpmu.c | 2 +-
 xen/common/domctl.c | 2 +-
 xen/common/sched/core.c | 4 ++--
 xen/include/xen/sched.h | 5 +
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 8ec4547bed..179f3dcc5a 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -188,7 +188,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs)
  * in XENPMU_MODE_ALL, for everyone.
  */
 if ( (vpmu_mode & XENPMU_MODE_ALL) ||
- (sampled->domain->domain_id >= DOMID_FIRST_RESERVED) )
+ (is_system_domain(sampled->domain)) )
 {
 sampling = choose_hwdom_vcpu();
 if ( !sampling )
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 879a2adcbe..67021cc54b 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -536,7 +536,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
 if ( !d )
 {
 ret = -EINVAL;
-if ( op->domain >= DOMID_FIRST_RESERVED )
+if ( is_system_domain_id(op->domain) )
 break;
 
 rcu_read_lock(&domlist_read_lock);
diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index 8f4b1ca10d..6ea8bcf62f 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -821,7 +821,7 @@ int sched_init_domain(struct domain *d, unsigned int poolid)
 int ret;
 
 ASSERT(d->cpupool == NULL);
-ASSERT(d->domain_id < DOMID_FIRST_RESERVED);
+ASSERT(!is_system_domain(d));
 
 if ( (ret = cpupool_add_domain(d, poolid)) )
 return ret;
@@ -845,7 +845,7 @@ int sched_init_domain(struct domain *d, unsigned int poolid)
 
 void sched_destroy_domain(struct domain *d)
 {
-ASSERT(d->domain_id < DOMID_FIRST_RESERVED);
+ASSERT(!is_system_domain(d));
 
 if ( d->cpupool )
 {
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 28146ee404..1df09bcb77 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -613,6 +613,11 @@ extern struct vcpu *idle_vcpu[NR_CPUS];
 #define is_idle_domain(d) ((d)->domain_id == DOMID_IDLE)
 #define is_idle_vcpu(v)   (is_idle_domain((v)->domain))
 
+static inline bool is_system_domain_id(domid_t id)
+{
+return (id >= DOMID_FIRST_RESERVED);
+}
+
 static inline bool is_system_domain(const struct domain *d)
 {
 return d->domain_id >= DOMID_FIRST_RESERVED;
-- 
2.20.1

[RFC 03/10] multiboot: moving headroom to per module_t

[Daniel P. Smith]

Hyperlaunch will require multiple modules to be allocated headroom. Therefore a
headroom variable has been added to module_t for tracking headroom on a per
module_t basis. To provide backwards compatibility with multiboot, the
reserved field was bitfield split to allow it to continue to be used to flag
module was relocated and to add to track the headroom needed when being
relocated.

RFC Note: The approach taken here was to produce something that was acceptable
but likely not the ultimate solution. A more complete solution would likely be
one that looked to unify a cross-architecture approach to represent the
material provided by bootloaders.

Signed-off-by: Daniel P. Smith 
Reviewed-by: Christopher Clark 
---
 xen/arch/x86/boot/reloc.c   |  1 +
 xen/arch/x86/setup.c| 17 -
 xen/include/xen/multiboot.h |  3 ++-
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
index 4f4039bb7c..9c68777db3 100644
--- a/xen/arch/x86/boot/reloc.c
+++ b/xen/arch/x86/boot/reloc.c
@@ -250,6 +250,7 @@ static multiboot_info_t *mbi2_reloc(u32 mbi_in)
 mbi_out_mods[mod_idx].mod_end = get_mb2_data(tag, module, mod_end);
 ptr = get_mb2_string(tag, module, cmdline);
 mbi_out_mods[mod_idx].string = copy_string(ptr);
+mbi_out_mods[mod_idx].headroom = 0;
 mbi_out_mods[mod_idx].reserved = 0;
 ++mod_idx;
 break;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 190d7aefb5..4bf84c17e4 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -855,7 +855,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 unsigned int initrdidx, num_parked = 0;
 multiboot_info_t *mbi;
 module_t *mod;
-unsigned long nr_pages, raw_max_page, modules_headroom, module_map[1];
+unsigned long nr_pages, raw_max_page, module_map[1];
 int i, j, e820_warn = 0, bytes = 0;
 unsigned long eb_start, eb_end;
 bool acpi_boot_table_init_done = false, relocated = false;
@@ -1158,7 +1158,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 mod[mbi->mods_count].mod_end = __2M_rwdata_end - _stext;
 }
 
-modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end);
+mod->headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end);
 bootstrap_map(NULL);
 
 #ifndef highmem_start
@@ -1321,8 +1321,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 /* Is the region suitable for relocating the multiboot modules? */
 for ( j = mbi->mods_count - 1; j >= 0; j-- )
 {
-unsigned long headroom = j ? 0 : modules_headroom;
-unsigned long size = PAGE_ALIGN(headroom + mod[j].mod_end);
+unsigned long size = PAGE_ALIGN(mod[j].headroom + mod[j].mod_end);
 
 if ( mod[j].reserved )
 continue;
@@ -1335,14 +1334,14 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 continue;
 
 if ( s < end &&
- (headroom ||
+ (mod[j].headroom ||
   ((end - size) >> PAGE_SHIFT) > mod[j].mod_start) )
 {
-move_memory(end - size + headroom,
+move_memory(end - size + mod[j].headroom,
 (uint64_t)mod[j].mod_start << PAGE_SHIFT,
 mod[j].mod_end, 0);
 mod[j].mod_start = (end - size) >> PAGE_SHIFT;
-mod[j].mod_end += headroom;
+mod[j].mod_end += mod[j].headroom;
 mod[j].reserved = 1;
 }
 }
@@ -1369,7 +1368,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 #endif
 }
 
-if ( modules_headroom && !mod->reserved )
+if ( mod->headroom && !mod->reserved )
 panic("Not enough memory to relocate the dom0 kernel image\n");
 for ( i = 0; i < mbi->mods_count; ++i )
 {
@@ -1902,7 +1901,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  * We're going to setup domain0 using the module(s) that we stashed safely
  * above our heap. The second module, if present, is an initrd ramdisk.
  */
-dom0 = create_dom0(mod, modules_headroom,
+dom0 = create_dom0(mod, mod->headroom,
initrdidx < mbi->mods_count ? mod + initrdidx : NULL,
kextra, loader);
 if ( !dom0 )
diff --git a/xen/include/xen/multiboot.h b/xen/include/xen/multiboot.h
index d1b43e1183..0ae37dfa2b 100644
--- a/xen/include/xen/multiboot.h
+++ b/xen/include/xen/multiboot.h
@@ -108,7 +108,8 @@ typedef struct {
 u32 mod_start;
 u32 mod_end;
 u32 string;
-u32 reserved;
+u32 reserved:1;
+u32 headroom:31;
 } module_t;
 
 /* The memory map. Be careful that the offset 0 is base_addr_low
-- 
2.20.1

[RFC 04/10] device-tree: split agnostic device-tree from arm

[Daniel P. Smith]

This commit is purely for RFC to highlight some concerns and changes necessary
in order for hyperlaunch to utilize device tree as the boot configuration
container.

Despite being in common, the core device tree support enabled through
CONFIG_HAS_DEVICE_TREE will not build for an x86 configuration. This is due to
the fact that despite `struct device` appearing to be intended as a common data
structure is in fact unique on x86 and Arm. There is code in
xen/common/device_tree.c and include/xen/iommu.h that uses struct members found
in Arm's `struct device` that are not present in x86's version. This obviously
ends in compilation errors when enabling CONFIG_HAS_DEVICE_TREE on x86.

As a result this commit seeks to separate between requiring the ability to
parse DTB files and obtaining hardware definition from those DTB files. The
Kconfig parameter CORE_DEVICE_TREE was introduced for when only the former is
necessary and not the latter. It specifically allows for the inclusion of the
device tree parsing code without enabling the areas that make use of Arm
specific `struct device`.

For the RFC when the Arm specific `struct device` is referenced within device
tree parsing code, check for Arm has been added as an interim solution until a
long term solution, that is beyond the scope of the hyperlaunch patch set, can
be proposed and implemented.

Signed-off-by: Daniel P. Smith 
Reviewed-by: Christopher Clark 
---
 xen/common/Kconfig| 5 +
 xen/common/Makefile   | 4 ++--
 xen/common/device_tree.c  | 2 ++
 xen/include/xen/device_tree.h | 4 
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 5e6aad644e..aece21c9e5 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -31,8 +31,12 @@ config HAS_ALTERNATIVE
 config HAS_COMPAT
bool
 
+config CORE_DEVICE_TREE
+   bool
+
 config HAS_DEVICE_TREE
bool
+   select CORE_DEVICE_TREE
 
 config HAS_EX_TABLE
bool
@@ -333,6 +337,7 @@ config ARGO
 
 config HYPERLAUNCH
bool "Hyperlaunch support (UNSUPPORTED)" if UNSUPPORTED
+   select CORE_DEVICE_TREE
---help---
  Enables launch of multiple VMs at host boot as an alternative
  method of starting a Xen system.
diff --git a/xen/common/Makefile b/xen/common/Makefile
index a6337e065a..f22aec72a0 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -4,7 +4,7 @@ obj-$(CONFIG_HYPFS_CONFIG) += config_data.o
 obj-$(CONFIG_CORE_PARKING) += core_parking.o
 obj-y += cpu.o
 obj-$(CONFIG_DEBUG_TRACE) += debugtrace.o
-obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
+obj-$(CONFIG_CORE_DEVICE_TREE) += device_tree.o
 obj-$(CONFIG_IOREQ_SERVER) += dm.o
 obj-y += domain.o
 obj-y += event_2l.o
@@ -73,7 +73,7 @@ obj-y += sched/
 obj-$(CONFIG_UBSAN) += ubsan/
 
 obj-$(CONFIG_NEEDS_LIBELF) += libelf/
-obj-$(CONFIG_HAS_DEVICE_TREE) += libfdt/
+obj-$(CONFIG_CORE_DEVICE_TREE) += libfdt/
 
 CONF_FILE := $(if $(patsubst 
/%,,$(KCONFIG_CONFIG)),$(BASEDIR)/)$(KCONFIG_CONFIG)
 config.gz: $(CONF_FILE)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 4aae281e89..d92fad2998 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -2012,9 +2012,11 @@ static unsigned long __init unflatten_dt_node(const void 
*fdt,
 ((char *)pp->value)[sz - 1] = 0;
 dt_dprintk("fixed up name for %s -> %s\n", pathp,
(char *)pp->value);
+#ifdef CONFIG_ARM
 /* Generic device initialization */
 np->dev.type = DEV_DT;
 np->dev.of_node = np;
+#endif
 }
 }
 if ( allnextpp )
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index fd6cd00b43..ca9f7672e9 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -101,9 +101,12 @@ struct dt_device_node {
  */
 struct list_head domain_list;
 
+#ifdef CONFIG_ARM
 struct device dev;
+#endif
 };
 
+#ifdef CONFIG_ARM
 #define dt_to_dev(dt_node)  (&(dt_node)->dev)
 
 static inline struct dt_device_node *dev_to_dt(struct device *dev)
@@ -112,6 +115,7 @@ static inline struct dt_device_node *dev_to_dt(struct 
device *dev)
 
 return container_of(dev, struct dt_device_node, dev);
 }
+#endif
 
 #define MAX_PHANDLE_ARGS 16
 struct dt_phandle_args {
-- 
2.20.1

[RFC 05/10] hyperlaunch: update device tree documentation

[Daniel P. Smith]

This commit is to update the hyperlaunch device tree documentation to align
with the DTB parsing implementation.

Signed-off-by: Daniel P. Smith 
Reviewed-by: Christopher Clark 
---
 .../designs/launch/hyperlaunch-devicetree.rst | 448 +++---
 1 file changed, 278 insertions(+), 170 deletions(-)

diff --git a/docs/designs/launch/hyperlaunch-devicetree.rst 
b/docs/designs/launch/hyperlaunch-devicetree.rst
index b49c98cfbd..7be23b9e71 100644
--- a/docs/designs/launch/hyperlaunch-devicetree.rst
+++ b/docs/designs/launch/hyperlaunch-devicetree.rst
@@ -13,12 +13,263 @@ difference is the introduction of the ``hypervisor`` node 
that is under the
 2. Allows for the domain construction information to easily be sanitized by
simple removing the ``/chosen/hypervisor`` node.
 
+
+The Hypervisor node
+---
+
+The ``hypervisor`` node is a top level container for the domains that will be 
built
+by hypervisor on start up. The node will be named ``hypervisor``  with a 
``compatible``
+property to identify which hypervisors the configuration is intended. The 
hypervisor
+node will consist of one or more config nodes and one or more domain nodes.
+
+Properties
+""
+
+compatible
+  Identifies which hypervisors the configuration is compatible. Required.
+
+  Format: "hypervisor,", e.g "hypervisor,xen"
+
+Child Nodes
+"""
+
+* config
+* domain
+
+Config Node
+---
+
+A ``config`` node is for passing configuration data and identifying any boot
+modules that is of interest to the hypervisor.  For example this would be where
+Xen would be informed of microcode or XSM policy locations. Each ``config``
+node will require a unique device-tree compliant name as there may be one or
+more ``config`` nodes present in a single dtb file. To identify which
+hypervisor the configuration is intended, the required ``compatible`` property
+must be present.
+
+While the config node is not meant to replace the hypervisor commandline, there
+may be cases where it is better suited for passing configuration details at
+boot time.  This additional information may be carried in properties assigned
+to a ``config`` node. If there are any boot modules that are intended for the
+hypervisor, then a ``module`` child node should be provided to identify the
+boot module.
+
+Properties
+""
+
+compatible
+  Identifies the hypervisor the confiugration is intended. Required.
+
+  Format: ",config", e.g "xen,config"
+
+Child Nodes
+"""
+
+* module
+
+Domain Node
+---
+
+A ``domain`` node is for describing the construction of a domain. Since there
+may be one or more domain nodes, each one requires a unique, DTB compliant name
+and a ``compatible`` property to identify as a domain node.
+
+A ``domain`` node  may provide a ``domid`` property which will be used as the
+requested domain id for the domain with a value of “0” signifying to use the
+next available domain id, which is the default behavior if omitted. It should
+be noted that a domain configuration is not able to request a domid of “0”.
+Beyond that a domain node may have any of the following optional properties.
+
+Properties
+""
+
+compatible
+  Identifies the node as a domain node and for which hypervisor. Required.
+
+  Format: ",domain", e.g "xen,domain"
+
+domid
+  Identifies the domid requested to assign to the domain.
+
+  Format: Integer, e.g <0>
+
+permissions
+  This sets what Discretionary Access Control permissions
+  a domain is assigned. Optional, default is none.
+
+  Format: Bitfield, e.g <3> or <0x0003>
+
+  PERMISSION_NONE  (0)
+  PERMISSION_CONTROL   (1 << 0)
+  PERMISSION_HARDWARE  (1 << 1)
+
+functions
+  This identifies what system functions a domain will fulfill.
+  Optional, the default is none.
+
+  Format: Bitfield, e.g <3221225487> or <0xC007>
+
+  FUNCTION_NONE(0)
+  FUNCTION_BOOT(1 << 0)
+  FUNCTION_CRASH   (1 << 1)
+  FUNCTION_CONSOLE (1 << 2)
+  FUNCTION_XENSTORE(1 << 30)
+  FUNCTION_LEGACY_DOM0 (1 << 31)
+
+.. note::  The `functions` bits that have been selected to indicate
+   ``FUNCTION_XENSTORE`` and ``FUNCTION_LEGACY_DOM0`` are the last two bits
+   (30, 31) such that should these features ever be fully replaced or retired,
+   the flags may be dropped without leaving a gap in the flag set.
+
+mode
+  The mode the domain will be executed under. Required.
+
+  Format: Bitfield, e.g <5> or <0x0005>
+
+  MODE_PARAVIRTUALIZED (1 << 0) PV | PVH/HVM
+  MODE_ENABLE_DEVICE_MODEL (1 << 1) HVM | PVH
+  MODE_LONG(1 << 2) 64 BIT | 32 BIT
+
+domain-uuid
+  A globally unique identifier for the domain. Optional,
+  the default is NULL.
+
+  Format: Byte Array, e.g [B3 FB 98 FB 8F 9F 67 A3]
+
+cpus
+  The number of vCPUs to be assigned to the domain. Optional,
+  the default is “1”.
+
+  Format: Integer, e.g 

[RFC 06/10] hyperlaunch: add structures to hold parsed dtb

[Daniel P. Smith]

Hyperlaunch builds from dom0less' `struct bootmodule` as the representation of
a boot module provided by a bootloader. From there it expands to cover the
representations proposed in the hyperlaunch design documentation.

Signed-off-by: Daniel P. Smith 
Reviewed-by: Christopher Clark 
---
 xen/include/xen/setup.h | 81 +
 1 file changed, 81 insertions(+)

diff --git a/xen/include/xen/setup.h b/xen/include/xen/setup.h
index 6fbe87860e..69ea042716 100644
--- a/xen/include/xen/setup.h
+++ b/xen/include/xen/setup.h
@@ -5,6 +5,87 @@
 
 #include 
 
+/* Reusing Dom0less definitions */
+typedef enum {
+BOOTMOD_XEN,
+BOOTMOD_FDT,
+BOOTMOD_KERNEL,
+BOOTMOD_RAMDISK,
+BOOTMOD_MICROCODE,
+BOOTMOD_XSM,
+BOOTMOD_GUEST_DTB,
+BOOTMOD_GUEST_CONF,
+BOOTMOD_UNKNOWN
+}  bootmodule_kind;
+
+struct bootmodule {
+bootmodule_kind kind;
+bool domU;
+paddr_t start;
+paddr_t size;
+};
+
+/* End reuse */
+
+struct memrange {
+paddr_t start;
+paddr_t size;
+};
+
+/* Currently only two config modules supported, microcode and xsm policy */
+#define HL_MAX_CONFIG_MODULES 2
+struct bootconfig {
+uint16_t nr_mods;
+struct bootmodule mods[HL_MAX_CONFIG_MODULES];
+};
+
+struct bootdomain {
+#define HL_PERMISSION_NONE  (0)
+#define HL_PERMISSION_CONTROL   (1 << 0)
+#define HL_PERMISSION_HARDWARE  (1 << 1)
+uint32_t permissions;
+
+#define HL_FUNCTION_NONE(0)
+#define HL_FUNCTION_BOOT(1 << 0)
+#define HL_FUNCTION_CRASH   (1 << 1)
+#define HL_FUNCTION_CONSOLE (1 << 2)
+#define HL_FUNCTION_XENSTORE(1 << 30)
+#define HL_FUNCTION_LEGACY_DOM0 (1 << 31)
+uint32_t functions;
+
+#define HL_MODE_PARAVIRTUALIZED (1 << 0) /* PV | PVH/HVM */
+#define HL_MODE_ENABLE_DEVICE_MODEL (1 << 1) /* HVM | PVH */
+#define HL_MODE_LONG(1 << 2) /* 64 BIT | 32 BIT */
+uint32_t mode;
+
+domid_t domid;
+uint8_t uuid[16];
+
+uint32_t ncpus;
+bool maxmem_set;
+struct memrange memrange[2]; /* 0: min; 1: max */
+
+#define HL_MAX_SECID_LEN 64
+unsigned char secid[HL_MAX_SECID_LEN];
+
+#define HL_MAX_DOMAIN_MODULES 3
+uint16_t nr_mods;
+struct bootmodule modules[HL_MAX_DOMAIN_MODULES];
+#define HL_MAX_CMDLINE_LEN 1024
+unsigned char cmdline[HL_MAX_CMDLINE_LEN];
+};
+
+struct hyperlaunch_config {
+const void *fdt;
+#ifdef CONFIG_MULTIBOOT
+module_t *mods;
+#endif
+struct bootconfig config;
+#define HL_MAX_BOOT_DOMAINS 64
+uint16_t nr_doms;
+struct bootdomain domains[HL_MAX_BOOT_DOMAINS];
+};
+
 #ifdef CONFIG_HYPERLAUNCH
 extern bool hyperlaunch_enabled;
 #else
-- 
2.20.1

[RFC 07/10] hyperlaunch: add parsing of dtb

[Daniel P. Smith]

This commit implements the parsing of a hyperlaunch device tree. It is
implemented to support a device tree containing any number of Config nodes and
Domain nodes.

RFC Note: Xen supports working with flattened device trees (FDT), DTB files,
and unflattened device trees, in-memory structures. The library libfdt provides
low-level interaction with FDTs with higher order functions spread between
arch/arm/bootfdt.c and a few in device_tree.c. While there are some FDT
handlers in device_tree.c, it primarily provides the functionality for handling
unflattened device trees.

For this RFC the general higher order FDT functions in bootfdt.c usable by
hyperlaunch were duplicated into common/setup.c for hyperlaunch usage. Longer
term it would be desired to see all reusable FDT functions centralized in
common separate from the unflattened device tree handling code in
device_tree.c.

Signed-off-by: Daniel P. Smith 
Reviewed-by: Christopher Clark 
---
 xen/common/setup.c  | 367 +++-
 xen/include/xen/setup.h |  22 ++-
 2 files changed, 387 insertions(+), 2 deletions(-)

diff --git a/xen/common/setup.c b/xen/common/setup.c
index e18ea14fe0..af2b1a422d 100644
--- a/xen/common/setup.c
+++ b/xen/common/setup.c
@@ -1,8 +1,373 @@
-#include 
+#include  /* needed by device_tree.h */
+#include 
 #include 
+#include 
+#include 
+#include 
+#include 
 
 #ifdef CONFIG_HYPERLAUNCH
 
 bool __initdata hyperlaunch_enabled;
+static struct hyperlaunch_config __initdata hl_config;
+
+/* Reusing from arch/arm/bootfdt.c */
+
+static bool __init device_tree_node_compatible(const void *fdt, int node,
+   const char *match)
+{
+int len, l;
+int mlen;
+const void *prop;
+
+mlen = strlen(match);
+
+prop = fdt_getprop(fdt, node, "compatible", &len);
+if ( prop == NULL )
+return false;
+
+while ( len > 0 ) {
+if ( !dt_compat_cmp(prop, match) )
+return true;
+l = strlen(prop) + 1;
+prop += l;
+len -= l;
+}
+
+return false;
+}
+
+static void __init device_tree_get_reg(
+const __be32 **cell, uint32_t address_cells, uint32_t size_cells,
+uint64_t *start, uint64_t *size)
+{
+*start = dt_next_cell(address_cells, cell);
+*size = dt_next_cell(size_cells, cell);
+}
+
+static uint32_t __init device_tree_get_u32(
+const void *fdt, int node, const char *prop_name, u32 dflt)
+{
+const struct fdt_property *prop;
+
+prop = fdt_get_property(fdt, node, prop_name, NULL);
+if ( !prop || prop->len < sizeof(u32) )
+return dflt;
+
+return fdt32_to_cpu(*(uint32_t*)prop->data);
+}
+
+/**
+ * device_tree_for_each_node - iterate over all device tree sub-nodes
+ * @fdt: flat device tree.
+ * @node: parent node to start the search from
+ * @func: function to call for each sub-node.
+ * @data: data to pass to @func.
+ *
+ * Any nodes nested at DEVICE_TREE_MAX_DEPTH or deeper are ignored.
+ *
+ * Returns 0 if all nodes were iterated over successfully.  If @func
+ * returns a value different from 0, that value is returned immediately.
+ */
+int __init device_tree_for_each_node(
+const void *fdt, int node, device_tree_node_func func, void *data)
+{
+/*
+ * We only care about relative depth increments, assume depth of
+ * node is 0 for simplicity.
+ */
+int depth = 0;
+const int first_node = node;
+u32 address_cells[DEVICE_TREE_MAX_DEPTH];
+u32 size_cells[DEVICE_TREE_MAX_DEPTH];
+int ret;
+
+do {
+const char *name = fdt_get_name(fdt, node, NULL);
+u32 as, ss;
+
+if ( depth >= DEVICE_TREE_MAX_DEPTH )
+{
+printk("Warning: device tree node `%s' is nested too deep\n",
+   name);
+continue;
+}
+
+as = depth > 0 ? address_cells[depth-1] : 
DT_ROOT_NODE_ADDR_CELLS_DEFAULT;
+ss = depth > 0 ? size_cells[depth-1] : DT_ROOT_NODE_SIZE_CELLS_DEFAULT;
+
+address_cells[depth] = device_tree_get_u32(fdt, node,
+   "#address-cells", as);
+size_cells[depth] = device_tree_get_u32(fdt, node,
+"#size-cells", ss);
+
+/* skip the first node */
+if ( node != first_node )
+{
+ret = func(fdt, node, name, depth, as, ss, data);
+if ( ret != 0 )
+return ret;
+}
+
+node = fdt_next_node(fdt, node, &depth);
+} while ( node >= 0 && depth > 0 );
+
+return 0;
+}
+
+/* End reuse */
+
+static bool read_module(
+const void *fdt, int node, uint32_t address_cells, uint32_t size_cells,
+struct hyperlaunch_config *config, struct bootmodule *bm)
+{
+const struct fdt_property *prop;
+const __be32 *cell;
+bootmodule_kind kind = BOOTMOD_UNKNOWN;
+int len;
+
+if ( device_tree_node_compatible(fdt, node, "module,kernel") )
+kind = BOOTMOD_KERNEL;
+

[RFC 09/10] hyperlaunch: add domain creation logic

[Daniel P. Smith]

This commit introduces the skeleton of hyperlaunch domain construction
mechanics and adds the preliminary ability to construct dom0.

Signed-off-by: Daniel P. Smith 
Reviewed-by: Christopher Clark 
---
 xen/common/setup.c  | 77 +
 xen/include/xen/setup.h | 16 +
 2 files changed, 93 insertions(+)

diff --git a/xen/common/setup.c b/xen/common/setup.c
index af2b1a422d..cd24f60297 100644
--- a/xen/common/setup.c
+++ b/xen/common/setup.c
@@ -1,3 +1,4 @@
+#include  /* for bzimage_headroom */
 #include  /* needed by device_tree.h */
 #include 
 #include 
@@ -368,6 +369,82 @@ bool __init hyperlaunch_mb_init(module_t *mods)
 
 return ret;
 }
+
+void __init hyperlaunch_mb_headroom(void)
+{
+int i,j;
+
+for( i = 0; i < hl_config.nr_doms; i++ )
+{
+for ( j = 0; j < hl_config.domains[i].nr_mods; j++ )
+{
+if ( hl_config.domains[i].modules[j].kind == BOOTMOD_KERNEL )
+{
+module_t *kern =
+(module_t *)_p(hl_config.domains[i].modules[j].start);
+
+kern->headroom = bzimage_headroom(bootstrap_map(kern),
+  kern->mod_end);
+bootstrap_map(NULL);
+}
+}
+}
+}
 #endif
 
+uint32_t __init hyperlaunch_create_domains(
+struct domain **hwdom, const char *kextra, const char *loader)
+{
+uint32_t dom_count = 0, functions_used = 0;
+int i;
+
+*hwdom = NULL;
+
+for ( i = 0; i < hl_config.nr_doms; i++ )
+{
+struct bootdomain *d = &(hl_config.domains[i]);
+
+/* build a legacy dom0 and set it as the hwdom */
+if ( (d->functions & HL_FUNCTION_LEGACY_DOM0) &&
+ !(functions_used & HL_FUNCTION_LEGACY_DOM0) )
+{
+module_t *image = NULL, *initrd = NULL;
+int j;
+
+for ( j = 0; j < d->nr_mods; j++ )
+{
+if ( d->modules[j].kind == BOOTMOD_KERNEL )
+image = (module_t *)_p(d->modules[j].start);
+
+if ( d->modules[j].kind == BOOTMOD_RAMDISK )
+initrd = (module_t *)_p(d->modules[j].start);
+
+if ( image && initrd )
+break;
+}
+
+if ( image == NULL )
+return 0;
+
+#ifdef CONFIG_MULTIBOOT
+*hwdom = create_dom0(image, image->headroom, initrd, kextra,
+ loader);
+#endif
+if ( *hwdom )
+{
+functions_used |= HL_FUNCTION_LEGACY_DOM0;
+dom_count++;
+}
+else
+panic("HYPERLAUNCH: "
+  "Dom0 config present but dom0 construction failed\n");
+}
+else
+printk(XENLOG_WARNING "hyperlaunch: "
+   "currently only supports classic dom0 construction");
+}
+
+return dom_count;
+}
+
 #endif
diff --git a/xen/include/xen/setup.h b/xen/include/xen/setup.h
index fd4c23c08f..3833867470 100644
--- a/xen/include/xen/setup.h
+++ b/xen/include/xen/setup.h
@@ -93,8 +93,12 @@ int __init hyperlaunch_init(const void *fdt);
 
 #ifdef CONFIG_MULTIBOOT
 bool __init hyperlaunch_mb_init(module_t *mods);
+void __init hyperlaunch_mb_headroom(void);
 #endif
 
+uint32_t __init hyperlaunch_create_domains(
+struct domain **hwdom, const char *kextra, const char *loader);
+
 #else /* CONFIG_HYPERLAUNCH */
 
 #define hyperlaunch_enabled false
@@ -109,7 +113,19 @@ static inline bool __init hyperlaunch_mb_init(module_t 
*mods)
 {
 return false;
 }
+
+void __init hyperlaunch_mb_headroom(void)
+{
+return;
+}
 #endif
 
+static inline uint32_t __init hyperlaunch_create_domains(
+struct domain **hwdom, const char *kextra, const char *loader)
+{
+return 0;
+}
+
 #endif /* CONFIG_HYPERLAUNCH */
+
 #endif /* XEN_SETUP_H */
-- 
2.20.1

[RFC 08/10] hyperlaunch: make create_dom0 externally callable

[Daniel P. Smith]

This commit is for the RFC only to enable a quick demonstration of starting Xen
via hyperlaunch. Final version of hyperlaunch will have its own domain
construction function that will replace the RFC usage of create_dom0.

Signed-off-by: Daniel P. Smith 
Reviewed-by: Christopher Clark 
---
 xen/arch/x86/include/asm/setup.h | 5 +
 xen/arch/x86/setup.c | 8 
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index 7dc03b6b8d..71f5aaa9b1 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -34,6 +34,11 @@ static inline void vesa_init(void) {};
 static inline void vesa_mtrr_init(void) {};
 #endif
 
+struct domain *__init create_dom0(const module_t *image,
+  unsigned long headroom,
+  module_t *initrd, const char *kextra,
+  const char *loader);
+
 int construct_dom0(
 struct domain *d,
 const module_t *kernel, unsigned long kernel_headroom,
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 4bf84c17e4..bee221d5ee 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -755,10 +755,10 @@ static unsigned int __init copy_bios_e820(struct 
e820entry *map, unsigned int li
 return n;
 }
 
-static struct domain *__init create_dom0(const module_t *image,
- unsigned long headroom,
- module_t *initrd, const char *kextra,
- const char *loader)
+struct domain *__init create_dom0(const module_t *image,
+  unsigned long headroom,
+  module_t *initrd, const char *kextra,
+  const char *loader)
 {
 struct xen_domctl_createdomain dom0_cfg = {
 .flags = IS_ENABLED(CONFIG_TBOOT) ? XEN_DOMCTL_CDF_s3_integrity : 0,
-- 
2.20.1

[RFC 10/10] hyperlaunch: integrate dtb parse and domain creation

[Daniel P. Smith]

This commit introduces into x86 start_xen the detection and parsing of a
hyperlaunch DTB file and then using that information to construct the domains
contained in the hyperlaunch configuration.

Signed-off-by: Daniel P. Smith 
Reviewed-by: Christopher Clark 
---
 xen/arch/x86/setup.c | 54 ++--
 1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index bee221d5ee..c007c421b0 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1020,6 +1020,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 bitmap_fill(module_map, mbi->mods_count);
 __clear_bit(0, module_map); /* Dom0 kernel is always first */
 
+if ( hyperlaunch_mb_init(mod) )
+printk(XENLOG_INFO "Hyperlaunch enabled\n");
+
 if ( pvh_boot )
 {
 /* pvh_init() already filled in e820_raw */
@@ -1142,6 +1145,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 panic("Bootloader didn't honor module alignment request\n");
 mod[i].mod_end -= mod[i].mod_start;
 mod[i].mod_start >>= PAGE_SHIFT;
+mod[i].headroom = 0;
 mod[i].reserved = 0;
 }
 
@@ -1158,8 +1162,12 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 mod[mbi->mods_count].mod_end = __2M_rwdata_end - _stext;
 }
 
-mod->headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end);
-bootstrap_map(NULL);
+if ( hyperlaunch_enabled ) {
+hyperlaunch_mb_headroom();
+} else {
+mod->headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end);
+bootstrap_map(NULL);
+}
 
 #ifndef highmem_start
 /* Don't allow split below 4Gb. */
@@ -1890,22 +1898,34 @@ void __init noreturn __start_xen(unsigned long mbi_p)
cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ",
cpu_has_nx ? "" : "not ");
 
-initrdidx = find_first_bit(module_map, mbi->mods_count);
-if ( !hyperlaunch_enabled &&
- bitmap_weight(module_map, mbi->mods_count) > 1 )
-printk(XENLOG_WARNING
-   "Multiple initrd candidates, picking module #%u\n",
-   initrdidx);
+if ( hyperlaunch_enabled )
+{
+uint32_t ndoms;
 
-/*
- * We're going to setup domain0 using the module(s) that we stashed safely
- * above our heap. The second module, if present, is an initrd ramdisk.
- */
-dom0 = create_dom0(mod, mod->headroom,
-   initrdidx < mbi->mods_count ? mod + initrdidx : NULL,
-   kextra, loader);
-if ( !dom0 )
-panic("Could not set up DOM0 guest OS\n");
+printk(XENLOG_INFO "Hyperlaunch starting domain construction...\n");
+ndoms = hyperlaunch_create_domains(&dom0, kextra, loader);
+if ( ndoms == 0 )
+panic("Hyperlaunch could not set up the domains\n");
+
+printk(XENLOG_INFO "Hyperlaunch created %u domains\n", ndoms);
+} else {
+initrdidx = find_first_bit(module_map, mbi->mods_count);
+if ( bitmap_weight(module_map, mbi->mods_count) > 1 )
+printk(XENLOG_WARNING
+   "Multiple initrd candidates, picking module #%u\n",
+   initrdidx);
+
+/*
+ * We're going to setup domain0 using the module(s) that we stashed
+ * safely above our heap. The second module, if present, is an initrd
+ * ramdisk.
+ */
+dom0 = create_dom0(mod, mod->headroom,
+   initrdidx < mbi->mods_count ? mod + initrdidx : 
NULL,
+   kextra, loader);
+if ( !dom0 )
+panic("Could not set up DOM0 guest OS\n");
+}
 
 heap_init_late();
 
-- 
2.20.1

Re: [RFC 02/10] is_system_domain: replace open-coded instances

[Andrew Cooper]

On 17/12/2021 23:34, Daniel P. Smith wrote:
> From: Christopher Clark 
>
> There were several instances of open-coded domid range checking. This commit
> replaces those with the is_system_domain inline function.
>
> Signed-off-by: Christopher Clark 
> Signed-off-by: Daniel P. Smith 

Ah - probably my fault.  When I added is_system_domain(), I didn't think
to scan for other opencodes - I was guts deep in the domain creation logic.

In addition to the ones you've got here...

xen/arch/x86/cpu/mcheck/mce.c:1521
xen/common/domain.c:586
common/domctl.c:55, 411 and 421

according to `git grep DOMID_FIRST_RESERVED`

> diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
> index 8ec4547bed..179f3dcc5a 100644
> --- a/xen/arch/x86/cpu/vpmu.c
> +++ b/xen/arch/x86/cpu/vpmu.c
> @@ -188,7 +188,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs)
>   * in XENPMU_MODE_ALL, for everyone.
>   */
>  if ( (vpmu_mode & XENPMU_MODE_ALL) ||
> - (sampled->domain->domain_id >= DOMID_FIRST_RESERVED) )
> + (is_system_domain(sampled->domain)) )

Can drop one set of brackets now.

> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 28146ee404..1df09bcb77 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -613,6 +613,11 @@ extern struct vcpu *idle_vcpu[NR_CPUS];
>  #define is_idle_domain(d) ((d)->domain_id == DOMID_IDLE)
>  #define is_idle_vcpu(v)   (is_idle_domain((v)->domain))
>  
> +static inline bool is_system_domain_id(domid_t id)
> +{
> +return (id >= DOMID_FIRST_RESERVED);
> +}
> +
>  static inline bool is_system_domain(const struct domain *d)
>  {
>  return d->domain_id >= DOMID_FIRST_RESERVED;

is_system_domain() wants implementing in terms of is_system_domain_id().

That said, could I talk you into is_system_domid() as a better name?

This is all sufficiently trivial that I'm tempted to fix on commit if
you'd like.  This patch is cleanup that stands on its own merit, and
isn't tied to hyperlaunch specifically.

~Andrew

[ovmf test] 167465: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf ee1f8262b83dd88b30091e6e81221ff299796099
baseline version:
 ovmf ab5ab2f60348138a4b7b1c95ad6f5d0954fb96f1

Last test of basis   167463  2021-12-17 15:10:34 Z0 days
Testing same since   167465  2021-12-17 17:41:43 Z0 days1 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  Jiewen Yao 
  Leif Lindholm 
  Nhi Pham 
  Shivanshu Goyal 
  Stefan Berger 

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
   ab5ab2f603..ee1f8262b8  ee1f8262b83dd88b30091e6e81221ff299796099 -> 
xen-tested-master

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

[osstest service owner]

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

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  8e3edefb880caeeaaf80123d5599139e8c2c9ecf
baseline version:
 xen  32365f3476ac4655f2f26111cd7879912808cd77

Last test of basis   167462  2021-12-17 15:01:42 Z0 days
Testing same since   167467  2021-12-17 19:01:56 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 

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
   32365f3476..8e3edefb88  8e3edefb880caeeaaf80123d5599139e8c2c9ecf -> smoke

Re: [patch V3 28/35] PCI/MSI: Simplify pci_irq_get_affinity()

[Nathan Chancellor]

Hi Thomas,

On Fri, Dec 10, 2021 at 11:19:26PM +0100, Thomas Gleixner wrote:
> From: Thomas Gleixner 
> 
> Replace open coded MSI descriptor chasing and use the proper accessor
> functions instead.
> 
> Signed-off-by: Thomas Gleixner 
> Reviewed-by: Greg Kroah-Hartman 
> Reviewed-by: Jason Gunthorpe 

Apologies if this has already been reported somewhere else or already
fixed, I did a search of all of lore and did not see anything similar to
it and I did not see any new commits in -tip around this.

I just bisected a boot failure on my AMD test desktop to this patch as
commit f48235900182 ("PCI/MSI: Simplify pci_irq_get_affinity()") in
-next. It looks like there is a problem with the NVMe drive after this
change according to the logs. Given that the hard drive is not getting
mounted for journald to write logs to, I am not really sure how to get
them from the machine so I have at least taken a picture of what I see
on my screen; open to ideas on that front!

https://github.com/nathanchance/bug-files/blob/0d25d78b5bc1d5e9c15192b3bc80676364de8287/f48235900182/crash.jpg

Please let me know what information I can provide to make debugging this
easier and I am more than happy to apply and test patches as needed.

Cheers,
Nathan

Re: [PATCH v3 02/13] xen: harmonize return types of hypercall handlers

[Stefano Stabellini]

On Fri, 17 Dec 2021, Juergen Gross wrote:
> On 17.12.21 11:41, Julien Grall wrote:
> > Hi Juergen,
> > 
> > On 17/12/2021 08:50, Juergen Gross wrote:
> > > On 17.12.21 08:45, Jan Beulich wrote:
> > > > On 17.12.2021 06:34, Juergen Gross wrote:
> > > > > On 16.12.21 22:15, Stefano Stabellini wrote:
> > > > > > On Thu, 16 Dec 2021, Stefano Stabellini wrote:
> > > > > > > On Thu, 16 Dec 2021, Juergen Gross wrote:
> > > > > > > > On 16.12.21 03:10, Stefano Stabellini wrote:
> > > > > > > > > The case of XENMEM_maximum_ram_page is interesting but it is
> > > > > > > > > not a
> > > > > > > > > problem in reality because the max physical address size is
> > > > > > > > > only 40-bit
> > > > > > > > > for aarch32 guests, so 32-bit are always enough to return the
> > > > > > > > > highest
> > > > > > > > > page in memory for 32-bit guests.
> > > > > > > > 
> > > > > > > > You are aware that this isn't the guest's max page, but the
> > > > > > > > host's?
> > > > > > 
> > > > > > I can see now that you meant to say that, no matter what is the max
> > > > > > pseudo-physical address supported by the VM, XENMEM_maximum_ram_page
> > > > > > is
> > > > > > supposed to return the max memory page, which could go above the
> > > > > > addressibility limit of the VM.
> > > > > > 
> > > > > > So XENMEM_maximum_ram_page should potentially be able to return
> > > > > > (1<<44)
> > > > > > even when called by an aarch32 VM, with max IPA 40-bit.
> > > > > > 
> > > > > > I would imagine it could be useful if dom0 is 32-bit but domUs are
> > > > > > 64-bit on a 64-bit hypervisor (which I think it would be a very rare
> > > > > > configuration on ARM.)
> > > > > > 
> > > > > > Then it looks like XENMEM_maximum_ram_page needs to be able to
> > > > > > return a
> > > > > > value > 32-bit when called by a 32-bit guest.
> > > > > > 
> > > > > > The hypercall ABI follows the ARM C calling convention, so a 64-bit
> > > > > > value should be returned using r0 and r1. But looking at
> > > > > > xen/arch/arm/traps.c:do_trap_hypercall, it doesn't seem it ever sets
> > > > > > r1
> > > > > > today. Only r0 is set, so effectively we only support 32-bit return
> > > > > > values on aarch32 and for aarch32 guests.
> > > > > > 
> > > > > > In other words, today all hypercalls on ARM return 64-bit to 64-bit
> > > > > > guests and 32-bit to 32-bit guests. Which in the case of memory_op
> > > > > > is
> > > > > > "technically" the correct thing to do because it matches the C
> > > > > > declaration in xen/include/xen/hypercall.h:
> > > > > > 
> > > > > > extern long
> > > > > > do_memory_op(
> > > > > >   unsigned long cmd,
> > > > > >   XEN_GUEST_HANDLE_PARAM(void) arg);
> > > > > > 
> > > > > > So...  I guess the conclusion is that on ARM do_memory_op should
> > > > > > return
> > > > > > "long" although it is not actually enough for a correct
> > > > > > implementation
> > > > > > of XENMEM_maximum_ram_page for aarch32 guests ?
> > > > > > 
> > > > > 
> > > > > Hence my suggestion to check the return value of _all_ hypercalls to
> > > > > be
> > > > > proper sign extended int values for 32-bit guests. This would fix all
> > > > > potential issues without silently returning truncated values.
> > > > 
> > > > Are we absolutely certain we have no other paths left where a possibly
> > > > large unsigned values might be returned? In fact while
> > > > compat_memory_op() does the necessary saturation, I've never been fully
> > > > convinced of this being the best way of dealing with things. The range
> > > > of error indicators is much smaller than [-INT_MIN,-1], so almost
> > > > double the range of effectively unsigned values could be passed back
> > > > fine. (Obviously we can't change existing interfaces, so this mem-op
> > > > will need to remain as is.)
> > > 
> > > In fact libxenctrl tries do deal with this fact by wrapping a memory_op
> > > for a 32-bit environment into a multicall. This will work fine for a
> > > 32-bit Arm guest, as xen_ulong_t is a uint64 there.
> > > 
> > > So do_memory_op should return long on Arm, yes. OTOH doing so will
> > > continue to be a problem in case a 32-bit guest doesn't use the
> > > multicall technique for handling possible 64-bit return values.
> > > 
> > > So I continue to argue that on Arm the return value of a hypercall
> > > should be tested to fit into 32 bits. 
> > 
> > It would make sense. But what would you return if the value doesn't fit?
> 
> I guess some errno value would be appropriate, like -EDOM, -ERANGE or
> -E2BIG.

This seems to be better than the alternative below as it is a lot
simpler.


> > > The only really clean alternative
> > > would be to have separate hypercall function classes for Arm 32- and
> > > 64-bit guests (which still could share most of the functions by letting
> > > those return "int"). This would allow to use the 64-bit variant even for
> > > 32-bit guests in multicall (fine as the return field is 64-bit wide),
> > > and a probably saturating compat version for 

Re: [PATCH v3 02/13] xen: harmonize return types of hypercall handlers

[Stefano Stabellini]

On Fri, 17 Dec 2021, Julien Grall wrote:
> Hi Stefano,
> 
> On 16/12/2021 21:15, Stefano Stabellini wrote:
> > On Thu, 16 Dec 2021, Stefano Stabellini wrote:
> > > On Thu, 16 Dec 2021, Juergen Gross wrote:
> > > > On 16.12.21 03:10, Stefano Stabellini wrote:
> > > > > On Wed, 15 Dec 2021, Juergen Gross wrote:
> > > > > > On 14.12.21 18:36, Julien Grall wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > On 08/12/2021 15:55, Juergen Gross wrote:
> > > > > > > > Today most hypercall handlers have a return type of long, while
> > > > > > > > the
> > > > > > > > compat ones return an int. There are a few exceptions from that
> > > > > > > > rule,
> > > > > > > > however.
> > > > > > > 
> > > > > > > So on Arm64, I don't think you can make use of the full 64-bit
> > > > > > > because a
> > > > > > > 32-bit domain would not be able to see the top 32-bit.
> > > > > > > 
> > > > > > > In fact, this could potentially cause us some trouble (see [1]) in
> > > > > > > Xen.
> > > > > > > So it feels like the hypercalls should always return a 32-bit
> > > > > > > signed
> > > > > > > value
> > > > > > > on Arm.
> > > > > > 
> > > > > > This would break hypercalls like XENMEM_maximum_ram_page which are
> > > > > > able
> > > > > > to return larger values, right?
> > > > > > 
> > > > > > > The other advantage is it would be clear that the top 32-bit are
> > > > > > > not
> > > > > > > usuable. Stefano, what do you think?
> > > > > > 
> > > > > > Wouldn't it make more sense to check the return value to be a sign
> > > > > > extended 32-bit value for 32-bit guests in do_trap_hypercall()
> > > > > > instead?
> > > > > > 
> > > > > > The question is what to return if this is not the case. -EDOM?
> > > > > 
> > > > > 
> > > > > I can see where Julien is coming from: we have been trying to keep the
> > > > > arm32 and arm64 ABIs identical since the beginning of the project. So,
> > > > > like Julien, my preference would be to always return 32-bit on ARM,
> > > > > both
> > > > > aarch32 and aarch64. It would make things simple.
> > > > > 
> > > > > The case of XENMEM_maximum_ram_page is interesting but it is not a
> > > > > problem in reality because the max physical address size is only
> > > > > 40-bit
> > > > > for aarch32 guests, so 32-bit are always enough to return the highest
> > > > > page in memory for 32-bit guests.
> > > > 
> > > > You are aware that this isn't the guest's max page, but the host's?
> > 
> > I can see now that you meant to say that, no matter what is the max
> > pseudo-physical address supported by the VM, XENMEM_maximum_ram_page is
> > supposed to return the max memory page, which could go above the
> > addressibility limit of the VM.
> > 
> > So XENMEM_maximum_ram_page should potentially be able to return (1<<44)
> > even when called by an aarch32 VM, with max IPA 40-bit.
> 
> I am a bit confused with what you wrote. Yes, 32-bit VM can only address
> 40-bit, but this is only limiting its own (guest) physical address space. Such
> VM would still be able to map any host physical address (assuming GFN != MFN).
 
I meant to say the same thing that you wrote, sorry it wasn't clear

 
> > I would imagine it could be useful if dom0 is 32-bit but domUs are
> > 64-bit on a 64-bit hypervisor (which I think it would be a very rare
> > configuration on ARM.)
> 
> Looking at the implementation, the hypercall is accessible by any domain. IOW
> a domU 32-bit could read a wrong value.
> 
> That said, it is not clear to me why an Arm or HVM x86 guest would want to
> read the value.

Right, indeed. AFAICT it is currently unused on ARM.

Going through the code, the only caller that could potentially use it on
ARM is libxl_domain_core_dump and xc_maximum_ram_page is called on the
if ( !auto_translated_physmap ) code path.

[linux-5.4 test] 167459: tolerable FAIL - PUSHED

[osstest service owner]

flight 167459 linux-5.4 real [real]
flight 167468 linux-5.4 real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/167459/
http://logs.test-lab.xenproject.org/osstest/logs/167468/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail pass 
in 167468-retest

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 167449
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 167449
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 167449
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 167449
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 167449
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 167449
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 167449
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 167449
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 167449
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 167449
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 167449
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 167449
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-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-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  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  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-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-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-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-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-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-check   

[linux-linus test] 167461: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-vhd   8 xen-bootfail pass in 167469-retest
 test-amd64-amd64-xl22 guest-start/debian.repeat fail pass in 167469-retest

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-vhd 14 migrate-support-check fail in 167469 never pass
 test-armhf-armhf-xl-vhd 15 saverestore-support-check fail in 167469 never pass
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 167451
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 167451
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 167451
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 167451
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 167451
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 167451
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 167451
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 167451
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-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-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  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-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-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-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  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-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-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-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 linux6441998e2e37131b0a4c310af9156d79d3351c16
baseline version:
 linuxfa36bbe6d43f3bbce1f10a187e153587c7584d83

Last test of basis   167451  2021-12-16 19:11:06 Z1 days
Testing same since   167461  2021-12-17 13:00:28 Z0 days1 attempts


People who touched revisions under test:
  Aaro Koskinen 
  Adam Ford 
  Ahmed Zaki 
  Alex Bee 
  Alexei Starov

Re: [RFC v1 3/5] xen/arm: introduce SCMI-SMC mediator driver

[Stefano Stabellini]

On Tue, 14 Dec 2021, Oleksii Moisieiev wrote:
> This is the implementation of SCI interface, called SCMI-SMC driver,
> which works as the mediator between XEN Domains and Firmware (SCP, ATF etc).
> This allows devices from the Domains to work with clocks, resets and
> power-domains without access to CPG.
> 
> The following features are implemented:
> - request SCMI channels from ATF and pass channels to Domains;
> - set device permissions for Domains based on the Domain partial
> device-tree. Devices with permissions are able to work with clocks,
> resets and power-domains via SCMI;
> - redirect scmi messages from Domains to ATF.
> 
> Signed-off-by: Oleksii Moisieiev 
> ---
>  xen/arch/arm/Kconfig  |   2 +
>  xen/arch/arm/sci/Kconfig  |  10 +
>  xen/arch/arm/sci/Makefile |   1 +
>  xen/arch/arm/sci/scmi_smc.c   | 795 ++
>  xen/include/public/arch-arm.h |   1 +
>  5 files changed, 809 insertions(+)
>  create mode 100644 xen/arch/arm/sci/Kconfig
>  create mode 100644 xen/arch/arm/sci/scmi_smc.c
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 186e1db389..02d96c6cfc 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -114,6 +114,8 @@ config SCI
> support. It allows guests to control system resourcess via one of
> SCI mediators implemented in XEN.
>  
> +source "arch/arm/sci/Kconfig"
> +
>  endmenu
>  
>  menu "ARM errata workaround via the alternative framework"
> diff --git a/xen/arch/arm/sci/Kconfig b/xen/arch/arm/sci/Kconfig
> new file mode 100644
> index 00..9563067ddc
> --- /dev/null
> +++ b/xen/arch/arm/sci/Kconfig
> @@ -0,0 +1,10 @@
> +config SCMI_SMC
> + bool "Enable SCMI-SMC mediator driver"
> + default n
> + depends on SCI
> + ---help---
> +
> + Enables mediator in XEN to pass SCMI requests from Domains to ATF.
> + This feature allows drivers from Domains to work with System
> + Controllers (such as power,resets,clock etc.). SCP is used as transport
> + for communication.
> diff --git a/xen/arch/arm/sci/Makefile b/xen/arch/arm/sci/Makefile
> index 837dc7492b..67f2611872 100644
> --- a/xen/arch/arm/sci/Makefile
> +++ b/xen/arch/arm/sci/Makefile
> @@ -1 +1,2 @@
>  obj-y += sci.o
> +obj-$(CONFIG_SCMI_SMC) += scmi_smc.o
> diff --git a/xen/arch/arm/sci/scmi_smc.c b/xen/arch/arm/sci/scmi_smc.c
> new file mode 100644
> index 00..2eb01ea82d
> --- /dev/null
> +++ b/xen/arch/arm/sci/scmi_smc.c
> @@ -0,0 +1,795 @@
> +/*
> + * xen/arch/arm/sci/scmi_smc.c
> + *
> + * SCMI mediator driver, using SCP as transport.
> + *
> + * Oleksii Moisieiev 
> + * Copyright (C) 2021, EPAM Systems.
> + *
> + * 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define SCMI_BASE_PROTOCOL  0x10
> +#define SCMI_BASE_PROTOCOL_ATTIBUTES0x1
> +#define SCMI_BASE_SET_DEVICE_PERMISSIONS0x9
> +#define SCMI_BASE_RESET_AGENT_CONFIGURATION 0xB
> +#define SCMI_BASE_DISCOVER_AGENT0x7
> +
> +/* SCMI return codes. See section 4.1.4 of SCMI spec (DEN0056C) */
> +#define SCMI_SUCCESS  0
> +#define SCMI_NOT_SUPPORTED  (-1)
> +#define SCMI_INVALID_PARAMETERS (-2)
> +#define SCMI_DENIED (-3)
> +#define SCMI_NOT_FOUND  (-4)
> +#define SCMI_OUT_OF_RANGE   (-5)
> +#define SCMI_BUSY   (-6)
> +#define SCMI_COMMS_ERROR(-7)
> +#define SCMI_GENERIC_ERROR  (-8)
> +#define SCMI_HARDWARE_ERROR (-9)
> +#define SCMI_PROTOCOL_ERROR (-10)
> +
> +#define DT_MATCH_SCMI_SMC DT_MATCH_COMPATIBLE("arm,scmi-smc")
> +
> +#define SCMI_SMC_ID"arm,smc-id"
> +#define SCMI_SHARED_MEMORY "linux,scmi_mem"

I could find the following SCMI binding in Linux, which describes
the arm,scmi-smc compatible and the arm,smc-id property:

Documentation/devicetree/bindings/firmware/arm,scmi.yaml

However, linux,scmi_mem is not described. Aren't you supposed to read
the "shmem" property instead? And the compatible string used for this
seems to be "arm,scmi-shmem".


> +#define SCMI_SHMEM "shmem"
> +
> +#define HYP_CHANNEL  0x0
> +
> +#define HDR_ID GENMASK(7,0)
> +#define HDR_TYPE   GENMASK(9, 8)
> +#define HDR_PR

[xen-unstable test] 167464: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i3866 xen-buildfail REGR. vs. 167455
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail REGR. 
vs. 167455

Tests which did not succeed, but are not blocking:
 test-amd64-i386-livepatch 1 build-check(1)   blocked  n/a
 test-amd64-i386-migrupgrade   1 build-check(1)   blocked  n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-qemut-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-qemut-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-pvshim 1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-ws16-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-shadow 1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-vhd1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-ws16-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemut-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-coresched-i386-xl  1 build-check(1)   blocked  n/a
 test-amd64-i386-examine   1 build-check(1)   blocked  n/a
 test-amd64-i386-examine-bios  1 build-check(1)   blocked  n/a
 test-amd64-i386-examine-uefi  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  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-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 167455
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 167455
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 167455
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 167455
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 167455
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 167455
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 167455
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 167455
 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-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-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-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-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-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 tes

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 167452
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 167452
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 167452
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 167452
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 167452
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 167452
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 167452
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 167452
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-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-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-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-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-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-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  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-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-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-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass

version targeted for testing:
 qemuu48c03a0e13f49e8dff9a8ac37fff43e00a0a9bb3
baseline version:
 qemuu29eb5c2c86f935b0e9700fad2ecfe8a32b011d57

Last test of basis   167452  2021-12-16 20:09:29 Z1 days
Testing same since   167466  2021-12-17 18:07:11 Z0 days1 attempts


People who touched revisions under test:
  Christian Borntraeger 
  Collin Walling 
  Matthew R